Skip to content

[CLOUDSTACK-10226] CloudStack is not importing Local storage properly - #2401

Merged
rafaelweingartner merged 2 commits into
apache:masterfrom
rafaelweingartner:CLOUDSTACK-10226
Apr 10, 2018
Merged

[CLOUDSTACK-10226] CloudStack is not importing Local storage properly#2401
rafaelweingartner merged 2 commits into
apache:masterfrom
rafaelweingartner:CLOUDSTACK-10226

Conversation

@rafaelweingartner

Copy link
Copy Markdown
Member

CloudStack is importing as Local storage any XenServer SR that is of type LVM or EXT. This causes a problem when one wants to use both Direct attach storage and local storage. Moreover, CloudStack was not importing all of the local storage that a host has available when local storage is enabled. It was only importing the First SR it sees.

To fix the first problem we started ignoring SRs that have the flag shared=true when discovering local storages. SRs configured to be shared are used as direct attached storage, and therefore should not be imported again as local ones.

To fix the second problem, we started loading all Local storage and importing them accordingly to ACS.

Tip for reviewers: the code changes is mainly in CitrixResourceBase, method initializeLocalSR

rafaelweingartner added a commit to rafaelweingartner/cloudstack that referenced this pull request Jan 10, 2018
…e ZONE XenServer and shared storage on ScaleIO
This code incorporates PR: apache#2401
rafaelweingartner added a commit to rafaelweingartner/cloudstack that referenced this pull request Jan 10, 2018
…e ZONE XenServer and shared storage on ScaleIO
This code incorporates PR: apache#2401
rafaelweingartner added a commit to rafaelweingartner/cloudstack that referenced this pull request Jan 10, 2018
…e ZONE XenServer and shared storage on ScaleIO
This code incorporates PR: apache#2401
rafaelweingartner added a commit to rafaelweingartner/cloudstack that referenced this pull request Jan 10, 2018
…e ZONE XenServer and shared storage on ScaleIO
This code incorporates PR: apache#2401
rafaelweingartner added a commit to rafaelweingartner/cloudstack that referenced this pull request Jan 10, 2018
…e ZONE XenServer and shared storage on ScaleIO
This code incorporates PR: apache#2401
rafaelweingartner added a commit to rafaelweingartner/cloudstack that referenced this pull request Jan 10, 2018
…e ZONE XenServer and shared storage on ScaleIO
This code incorporates PR: apache#2401
@rafaelweingartner
rafaelweingartnerforce-pushed the CLOUDSTACK-10226 branch 11 times, most recently from 4eac1a1 to 6f1a4bfCompareJanuary 15, 2018 20:24
@rafaelweingartnerrafaelweingartner self-assigned this Jan 29, 2018
@rafaelweingartnerrafaelweingartner added this to the 4.12 milestone Jan 30, 2018
@rafaelweingartner

Copy link
Copy Markdown
MemberAuthor

@blueorangutan package

@blueorangutan

Copy link
Copy Markdown

@rafaelweingartner a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

@blueorangutan

Copy link
Copy Markdown

Packaging result: ✔centos6 ✔centos7 ✔debian. JID-1792

@DaanHoogland

Copy link
Copy Markdown
Contributor

@blueorangutan help

@blueorangutan

Copy link
Copy Markdown

@DaanHoogland I understand these words: "help", "hello", "thanks", "package", "test"
Test command usage: test [mgmt os] [hypervisor] [additional tests]
Mgmt OS options: ['centos6', 'centos7', 'ubuntu']
Hypervisor options: ['kvm-centos6', 'kvm-centos7', 'kvm-ubuntu', 'xenserver-71', 'xenserver-65sp1', 'xenserver-62sp1', 'vmware-65', 'vmware-60u2', 'vmware-55u3', 'vmware-51u1', 'vmware-50u1']
Additional tests: list of space separated tests with paths relative to the test/integration directory, for example: component/test_acl_listvm.py component/test_volumes.py
Note: when additional tests are passed, you need to specify mgmt server os and hypervisor or use the matrix command.

Blessed contributors for kicking Trillian test jobs: ['rhtyd', 'nvazquez', 'PaulAngus', 'borisstoyanov', 'DaanHoogland']

@DaanHoogland

Copy link
Copy Markdown
Contributor

@blueorangutan test centos7 xenserver-71

@blueorangutan

Copy link
Copy Markdown

@DaanHoogland a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests

@rafaelweingartner

Copy link
Copy Markdown
MemberAuthor

I checked the logs. The problems did not seem to be related to the PR.
However, it is always good to have the feedback of an expert ;)

@blueorangutan

Copy link
Copy Markdown

Trillian test result (tid-2436)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 22786 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr2401-t2436-kvm-centos7.zip
Intermitten failure detected: /marvin/tests/smoke/test_certauthority_root.py
Intermitten failure detected: /marvin/tests/smoke/test_vpc_redundant.py
Intermitten failure detected: /marvin/tests/smoke/test_vpc_vpn.py
Intermitten failure detected: /marvin/tests/smoke/test_host_maintenance.py
Intermitten failure detected: /marvin/tests/smoke/test_hostha_kvm.py
Smoke tests completed. 66 look OK, 1 have error(s)
Only failed tests results shown below:

TestResultTime (s)Test File
test_04_rvpc_network_garbage_collector_nicsFailure452.86test_vpc_redundant.py

@rafaelweingartner

Copy link
Copy Markdown
MemberAuthor

@DaanHoogland I checked this last log result. Can I consider that the tests is successful then?

@DaanHoogland

Copy link
Copy Markdown
Contributor

@rafaelweingartner , i'm cautious here. This one seems consistent. Did you explain the cause of the failure?
@mike-tutkowski can you review this (and add yourself to the apache org)?

@rafaelweingartner

Copy link
Copy Markdown
MemberAuthor

@DaanHoogland I checked the error, and I still do not understand how it could be related to the code of this PR. Here I am only changing how local storages are detected and loaded to the system. Whereas, the error is related to redundant VRs in VPCs.

Anyways, I am trying to run the tests locally with simulator, but I am getting this error:

ERROR: do_vpc_test (integration.smoke.test_vpc_redundant.TestVPCRedundancy)
----------------------------------------------------------------------
TypeError: do_vpc_test() takes exactly 2 arguments (1 given)
-------------------- >> begin captured stdout << ---------------------
=== TestName: do_vpc_test | Status : EXCEPTION ===

I am not understanding the cause of that error. This is the command I am using to execute the test:
nosetests --with-marvin --marvin-config=/root/marvinCloudStackConfig.cfg --hypervisor=simulator /root/cloudstack/test/integration/smoke/test_vpc_redundant.py

Am I missing something else that is required to run tests?

CloudStack is importing as Local storage any XenServer SR that is of type LVM or EXT. This causes a problem when one wants to use both Direct attach storage and local storage. Moreover, CloudStack was not importing all of the local storage that a host has available when local storage is enabled. It was only importing the First SR it sees.
To fix the first problem we started ignoring SRs that have the flag shared=true when discovering local storages. SRs configured to be shared are used as direct attached storage, and therefore should not be imported again as local ones.
To fix the second problem, we started loading all Local storage and importing them accordingly to ACS.
@rafaelweingartner

Copy link
Copy Markdown
MemberAuthor

@mike-tutkowski could you share your wisdom with us here?

@mike-tutkowski

Copy link
Copy Markdown
Member

Sorry @DaanHoogland - I didn't seem to get notified when you tagged me. I just noticed this PR when @rafaelweingartner tagged me today. Let me review this PR and think about it.

@mike-tutkowski

Copy link
Copy Markdown
Member

I do not see how the VPC issue would be related to the code of this PR either. The PR code LGTM.

@rafaelweingartner

Copy link
Copy Markdown
MemberAuthor

@borisstoyanov and @DaanHoogland shall we execute the tests again?

@DaanHoogland

Copy link
Copy Markdown
Contributor

I don't think it makes sense but no hurt
@blueorangutan centos7 xenserver-71

@rafaelweingartner

Copy link
Copy Markdown
MemberAuthor

@DaanHoogland I do not think @blueorangutan heard your request for testing here...

@borisstoyanov

Copy link
Copy Markdown
Contributor

@blueorangutan test centos7 xenserver-71

@blueorangutan

Copy link
Copy Markdown

@borisstoyanov a Trillian-Jenkins test job (centos7 mgmt + xenserver-71) has been kicked to run smoke tests

@blueorangutan

Copy link
Copy Markdown

Trillian test result (tid-2478)
Environment: xenserver-71 (x2), Advanced Networking with Mgmt server 7
Total time taken: 22757 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr2401-t2478-xenserver-71.zip
Intermitten failure detected: /marvin/tests/smoke/test_certauthority_root.py
Intermitten failure detected: /marvin/tests/smoke/test_scale_vm.py
Intermitten failure detected: /marvin/tests/smoke/test_service_offerings.py
Intermitten failure detected: /marvin/tests/smoke/test_ssvm.py
Intermitten failure detected: /marvin/tests/smoke/test_vpc_redundant.py
Smoke tests completed. 64 look OK, 3 have error(s)
Only failed tests results shown below:

TestResultTime (s)Test File
test_01_scale_vmError9.23test_scale_vm.py
ContextSuite context=TestCpuCapServiceOfferings>:teardownError0.00test_service_offerings.py
test_04_rvpc_network_garbage_collector_nicsFailure518.16test_vpc_redundant.py

@rafaelweingartner

Copy link
Copy Markdown
MemberAuthor

@borisstoyanov now I believe that the test result is ok. These failures seem to be intermittent, not just here, but in other PRs as well.

@borisstoyanovborisstoyanov left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@DaanHooglandDaanHoogland left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks great as usual @rafaelweingartner

@rafaelweingartner
rafaelweingartner merged commit bbc9204 into apache:masterApr 10, 2018
rafaelweingartner added a commit to rafaelweingartner/cloudstack that referenced this pull request Apr 10, 2018
…apache#2401)
* [CLOUDSTACK-10226] CloudStack is not importing Local storage properly
CloudStack is importing as Local storage any XenServer SR that is of type LVM or EXT. This causes a problem when one wants to use both Direct attach storage and local storage. Moreover, CloudStack was not importing all of the local storage that a host has available when local storage is enabled. It was only importing the First SR it sees.
To fix the first problem we started ignoring SRs that have the flag shared=true when discovering local storages. SRs configured to be shared are used as direct attached storage, and therefore should not be imported again as local ones.
To fix the second problem, we started loading all Local storage and importing them accordingly to ACS.
* Cleanups and formatting
rafaelweingartner added a commit to rafaelweingartner/cloudstack that referenced this pull request Apr 10, 2018
…apache#2401)
* [CLOUDSTACK-10226] CloudStack is not importing Local storage properly
CloudStack is importing as Local storage any XenServer SR that is of type LVM or EXT. This causes a problem when one wants to use both Direct attach storage and local storage. Moreover, CloudStack was not importing all of the local storage that a host has available when local storage is enabled. It was only importing the First SR it sees.
To fix the first problem we started ignoring SRs that have the flag shared=true when discovering local storages. SRs configured to be shared are used as direct attached storage, and therefore should not be imported again as local ones.
To fix the second problem, we started loading all Local storage and importing them accordingly to ACS.
* Cleanups and formatting
@yadvryadvr mentioned this pull request May 3, 2018
12 tasks
@yadvr

yadvr commented May 4, 2018

Copy link
Copy Markdown
Member

@rafaelweingartner I scanned PRs merged on master but not 4.11 and came across this PR, do you think this would be useful for 4.11? If so, can you help create a backport PR for 4.11?

@rafaelweingartner

Copy link
Copy Markdown
MemberAuthor

I do not think so. It seems that no other people had this necessity so far. This PR enables people to use local storage and pre-setup storage at the same time. Moreover, I organized things, and now it is possible to have more than a single local storage per host.

@yadvr

yadvr commented May 4, 2018

Copy link
Copy Markdown
Member

@rafaelweingartner fair enough, but this seems a useful fix for LTS nonetheless. I'll leave the decision of backporting to you on this (and other PRs).

bernardodemarco pushed a commit to scclouds/cloudstack that referenced this pull request Jul 16, 2025
…ouds'
Flexibilização de escolha de IP ao implementar rede _guest_
Closesapache#2401
See merge request scclouds/scclouds!1186
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants

@rafaelweingartner@blueorangutan@DaanHoogland@mike-tutkowski@borisstoyanov@yadvr