Uh oh!
There was an error while loading. Please reload this page.
CLOUDSTACK-9604: Root disk resize support for VMware and XenServer. - #1813
Merged
Conversation
priyankpariharforce-pushed
the
CLOUDSTACK-9604
branch
from
December 3, 2016 03:07
53382f5 to
d146d25Comparepdube
reviewed
Jan 27, 2017
| if(command.getCurrentSize() <= newSize) { | ||
| s_logger.info("No need to resize volume: " + volId +", current size " + command.getCurrentSize() + " is same as new size " + newSize); | ||
| return new ResizeVolumeAnswer(command, true, "success", newSize); | ||
| } |
Contributor
There was a problem hiding this comment.
So you can't increase the size of a volume? This seems flawed
| verify(citrixResourceBase, times(1)).getConnection(); | ||
| assertFalse(answer.getResult()); | ||
| //assertFalse(answer.getResult()); |
There was a problem hiding this comment.
@pdube That test is flawed as it always expects that command to fail. Till now it was working fine as it was always resizing which results in xapi command failure. But now since there is check for resize only in case of increase of volume size then the test is failing.
Contributor
There was a problem hiding this comment.
@priyankparihar@anshul1886 Is this commented statement required for future reference? If not, better to remove this line.
| rootDiskSize = Long.parseLong(customParameters.get("rootdisksize")); | ||
| // only KVM supports rootdisksize override | ||
| if (hypervisorType != HypervisorType.KVM) { |
There was a problem hiding this comment.
@pdube There was no resource layer code available for XS for this and root volume resize was only implemented for KVM.
Contributor
Quickly tested with 4.10 pre jdk8 and it work creating vm and resize the root volume on XenServer 6.5. tested using lvmiscsi and SolidFire storage plugins. LGTM, but please answer pdube questions. Thanks for this PR! |
borisstoyanov
commented
Feb 15, 2017
Contributor
@blueorangutan package |
blueorangutan
commented
Feb 15, 2017
@borisstoyanov a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. |
blueorangutan
commented
Feb 15, 2017
Packaging result: ✔centos6 ✔centos7 ✔debian. JID-489 |
priyankpariharforce-pushed
the
CLOUDSTACK-9604
branch
from
February 15, 2017 10:32
d146d25 to
2102b44Comparepriyankparihar
commented
Feb 15, 2017
ContributorAuthor
Hi @pdion891, @borisstoyanov, @anshul1886 and @pdube, |
borisstoyanov
commented
Feb 15, 2017
Contributor
@blueorangutan test centos7 vmware-65 |
blueorangutan
commented
Feb 15, 2017
@borisstoyanov unsupported parameters provided. Supported mgmt server os are: |
borisstoyanov
commented
Feb 15, 2017
Contributor
@blueorangutan test centos7 vmware-60u2 |
blueorangutan
commented
Feb 15, 2017
@borisstoyanov a Trillian-Jenkins test job (centos7 mgmt + vmware-60u2) has been kicked to run smoke tests |
borisstoyanov
commented
Feb 15, 2017
Contributor
@blueorangutan test centos7 xenserver-62sp1 |
blueorangutan
commented
Feb 15, 2017
@borisstoyanov a Trillian-Jenkins test job (centos7 mgmt + xenserver-62sp1) has been kicked to run smoke tests |
blueorangutan
commented
Feb 15, 2017
Trillian test result (tid-827)
|
blueorangutan
commented
Feb 15, 2017
Trillian test result (tid-828)
|
serg38
commented
Feb 16, 2017
@borisstoyanov Looks like some environmental issues with backend NFS. Can you kick it again for vmware ? u'2017-02-15T20:21:38+0000', jobresult : {errorcode : 530, errortext : u'Failed to create templatecreate template from volume exception: Exception: java.lang.Exception\nMessage: unable to prepare template directory: template/tmpl/2/221, storage: NFS://10.2.0.16/acs/secondary/pr1813-t827-vmware-60u2/pr1813-t827-vmware-60u2-sec1, error msg: mkdir: cannot create directory \u2018/var/cloudstack/mnt/VM/7110653970250.2ffe4b26/template/tmpl/2/221\u2019: Permission denied\n'}, cmd : |
serg38
commented
Feb 16, 2017
@priyankparihar You need to adjust smoke test test_deploy_vm_root_resize.py to support vmware root resize. Otherwise it will keep failing 2017-02-15 12:48:59,150 - CRITICAL - FAILED: test_00_deploy_vm_root_resize: ['Traceback (most recent call last):\n', ' File "/usr/lib64/python2.7/unittest/case.py", line 369, in run\n testMethod()\n', ' File "/marvin/tests/smoke/test_deploy_vm_root_resize.py", line 205, in test_00_deploy_vm_root_resize\n self.assertEqual(success, True, "Check if unsupported hypervisor %s fails appropriately" % self.hypervisor)\n', ' File "/usr/lib64/python2.7/unittest/case.py", line 553, in assertEqual\n assertion_func(first, second, msg=msg)\n', ' File "/usr/lib64/python2.7/unittest/case.py", line 546, in _baseAssertEqual\n raise self.failureException(msg)\n', 'AssertionError: Check if unsupported hypervisor vmware fails appropriately\n'] |
borisstoyanov
commented
Feb 16, 2017
Contributor
Thanks @priyankparihar@serg38 |
priyankpariharforce-pushed
the
CLOUDSTACK-9604
branch
from
February 16, 2017 09:01
2102b44 to
ad7d6f5Comparepriyankparihar
commented
Feb 16, 2017
ContributorAuthor
Hi @borisstoyanov and @serg38 , -- |
borisstoyanov
commented
Feb 16, 2017
Contributor
@blueorangutan package |
blueorangutan
commented
Feb 16, 2017
@borisstoyanov a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. |
blueorangutan
commented
Feb 16, 2017
Packaging result: ✔centos6 ✔centos7 ✔debian. JID-493 |
borisstoyanov
commented
Feb 16, 2017
Contributor
@blueorangutan test centos7 vmware-60u2 |
borisstoyanov
commented
Feb 16, 2017
Contributor
@blueorangutan test centos7 xenserver-65sp1 |
blueorangutan
commented
Feb 16, 2017
@borisstoyanov a Trillian-Jenkins test job (centos7 mgmt + xenserver-65sp1) has been kicked to run smoke tests |
borisstoyanov
commented
Feb 16, 2017
Contributor
@blueorangutan test centos7 vmware-60u2 |
blueorangutan
commented
Feb 16, 2017
@borisstoyanov a Trillian-Jenkins test job (centos7 mgmt + vmware-60u2) has been kicked to run smoke tests |
blueorangutan
commented
Feb 16, 2017
Trillian test result (tid-836)
|
blueorangutan
commented
Apr 26, 2017
@borisstoyanov a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests |
ContributorAuthor
Hi @DaanHoogland Sir,
Yes, I am also on the same page. I asked to merge it soon. Because some people have dependency on this code they want to make something good on top of this code. I was excited because i want to see these good features. My intention of 'Merge it soon' is just pay more attention on this PR. I am not saying to merge a code which does not pass all relevant test cases. Please do not take it in other way. Please do all kind of testing. |
DaanHoogland
commented
Apr 26, 2017
Contributor
Thanks @priyankparihar , looking forward to the dependend PRs ;) |
priyankparihar
commented
Apr 26, 2017
ContributorAuthor
Hi @borisstoyanov Bhai,
Yes, currently user can use this feature from Cloudstack console also. |
priyankparihar
commented
Apr 26, 2017
ContributorAuthor
I think @karuturi is one of the most reliable person of cloudstack. She would not merge without test case pass. Even if code is submitted by CS VIP like @chiradeep or @alex. :) |
borisstoyanov
commented
Apr 26, 2017
Contributor
Hi @priyankparihar Bhai, |
ContributorAuthor
Hi @borisstoyanov Bhai,
For that answer is already provided. Please consider it. |
cloudmonger
commented
Apr 26, 2017
ACS CI BVT RunSumarry: Link to logs Folder (search by build_no):https://www.dropbox.com/sh/yj3wnzbceo9uef2/AAB6u-Iap-xztdm6jHX9SjPja?dl=0 Failed tests:
Skipped tests: Passed test suits: |
blueorangutan
commented
Apr 26, 2017
Trillian test result (tid-1037)
|
blueorangutan
commented
Apr 26, 2017
Trillian test result (tid-1036)
|
priyankparihar
commented
Apr 27, 2017
ContributorAuthor
Hi @koushik-das , Only in case of VMware Root disk controller setting is checked (and checks are not reachable if hypervisor is not VMware). Same could be verified from the code. And this test is KVM related. PS: @sadhugit ji: Do you want to add something ? |
DaanHoogland
commented
Apr 27, 2017
Contributor
@priyankparihar these virtio errors are addressed in #2066 |
priyankparihar
commented
Apr 27, 2017
ContributorAuthor
Hi @borisstoyanov,
What is your opinion ? |
borisstoyanov
commented
Apr 27, 2017
Contributor
@priyankparihar, test_deploy_vm_root_resize.py: test_vm_snapshots.py: test_volumes.py: Checkout the vmware test run |
borisstoyanov
commented
Apr 27, 2017
Contributor
@blueorangutan test centos7 xenserver-65sp1 |
blueorangutan
commented
Apr 27, 2017
@borisstoyanov a Trillian-Jenkins test job (centos7 mgmt + xenserver-65sp1) has been kicked to run smoke tests |
blueorangutan
commented
Apr 28, 2017
Trillian test result (tid-1046)
|
cloudsadhu
commented
Apr 28, 2017
via email
Contributor
test_deploy_vm_root_resize.py:
CRITICAL: EXCEPTION: None: [\'Traceback (most recent call last):\\n\'<smb://n/'>, \' File "/usr/lib/python2.7/site-packages/nose/suite.py", line 209, in run\\n self.setUp()\\n\'<smb://n/'>, \' File "/usr/lib/python2.7/site-packages/nose/suite.py", line 292, in setUp\\n self.setupContext(ancestor)\\n\'<smb://n/'>, \' File "/usr/lib/python2.7/site-packages/nose/suite.py", line 315, in setupContext\\n try_run(context, names)\\n\'<smb://n/'>, \' File "/usr/lib/python2.7/site-packages/nose/util.py", line 471, in try_run\\n return func()\\n\'<smb://n/'>, \' File "/marvin/tests/smoke/test_deploy_vm_root_resize.py", line 110, in setUpClass\\n domainid=cls.domain.id\\n\', \' File "/usr/lib/python2.7/site-packages/marvin/lib/base.py", line 1238, in register\\n "Unable to find Ostype is required for registering template")\\n\'<smb://n/'>, \'Exception: Unable to find Ostype is required for registering template\\n\']
i think Its test_data.py config change- we need to change to right url .if you add templateregister with os type and working template this test case will pass for vmware and for other hypervisors its working fine.[No code change required]
regards
sadhu
On Apr 27, 2017, at 5:26 PM, Boris Stoyanov <notifications@github.com<mailto:notifications@github.com>> wrote:
@priyankparihar<https://github.com/priyankparihar>,
Here are the exceptions related to the tests and I think they are related to the latest changes in this PR:
test_deploy_vm_root_resize.py:
CRITICAL: EXCEPTION: None: [\'Traceback (most recent call last):\\n\'<smb://n/'>, \' File "/usr/lib/python2.7/site-packages/nose/suite.py", line 209, in run\\n self.setUp()\\n\'<smb://n/'>, \' File "/usr/lib/python2.7/site-packages/nose/suite.py", line 292, in setUp\\n self.setupContext(ancestor)\\n\'<smb://n/'>, \' File "/usr/lib/python2.7/site-packages/nose/suite.py", line 315, in setupContext\\n try_run(context, names)\\n\'<smb://n/'>, \' File "/usr/lib/python2.7/site-packages/nose/util.py", line 471, in try_run\\n return func()\\n\'<smb://n/'>, \' File "/marvin/tests/smoke/test_deploy_vm_root_resize.py", line 110, in setUpClass\\n domainid=cls.domain.id\\n\', \' File "/usr/lib/python2.7/site-packages/marvin/lib/base.py", line 1238, in register\\n "Unable to find Ostype is required for registering template")\\n\'<smb://n/'>, \'Exception: Unable to find Ostype is required for registering template\\n\']
test_vm_snapshots.py:
2017-04-26 16:39:34,301 - CRITICAL - FAILED: test_01_test_vm_volume_snapshot: ['Traceback (most recent call last):\n', ' File "/usr/lib64/python2.7/unittest/case.py", line 369, in run\n testMethod()\n', ' File "/marvin/tests/smoke/test_vm_snapshots.py", line 408, in test_01_test_vm_volume_snapshot\n volume_id=volume.id)\n', ' File "/usr/lib64/python2.7/unittest/case.py", line 154, in __exit__\n "{0} not raised".format(exc_name))\n', 'AssertionError: Exception not raised\n']
test_volumes.py:
2017-04-26 17:18:13,947 - CRITICAL - FAILED: test_01_create_volume: ['Traceback (most recent call last):\n', ' File "/usr/lib64/python2.7/unittest/case.py", line 369, in run\n testMethod()\n', ' File "/marvin/tests/smoke/test_volumes.py", line 251, in test_01_create_volume\n self.assertEqual(ret[0],SUCCESS,"Check if promised disk size actually available")\n', ' File "/usr/lib64/python2.7/unittest/case.py", line 553, in assertEqual\n assertion_func(first, second, msg=msg)\n', ' File "/usr/lib64/python2.7/unittest/case.py", line 546, in _baseAssertEqual\n raise self.failureException(msg)\n', 'AssertionError: Check if promised disk size actually available\n']
Checkout the vmware test run
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub<#1813 (comment)>, or mute the thread<https://github.com/notifications/unsubscribe-auth/AM1P-QtdpmlQpDKkRXU922PdfjnEcGtTks5r0IJTgaJpZM4LC277>.
DISCLAIMER
==========
This e-mail may contain privileged and confidential information which is the property of Accelerite, a Persistent Systems business. It is intended only for the use of the individual or entity to which it is addressed. If you are not the intended recipient, you are not authorized to read, retain, copy, print, distribute or use this message. If you have received this communication in error, please notify the sender and delete all copies of this message. Accelerite, a Persistent Systems business does not accept any liability for virus infected mails. |
karuturi
commented
May 2, 2017
Member
Whats the current status of this PR? :) |
pdion891
commented
May 2, 2017
Contributor
I have not rebase recently but we have strongly tested this PR using XenServer 6.5sp1. also briefly tested with xenserver 7.1. |
karuturi
commented
May 8, 2017
Member
Thanks @pdion891 for review. merging this now. |
Member
I faced an issue today when vmware.create.full.clone is set to false, the code should read the config and allow resizing of root disk only when vmware.create.full.clone is true. Alternatively, the resize operation should not throw exception when disk length is > 1 but instead skip the resize operation. |
sateesh-chodapuneedi
commented
Dec 16, 2017
Member
@priyankparihar Can you please check and open a PR to address the issue reported by @rhtyd? |
yadvr
commented
Dec 17, 2017
Member
@sateesh-chodapuneedi@priyankparihar see my fixes in #2211 I've made the code to skip when VM starts if the root disk chain length is > 1 logging a warning. I think we should not throw any exception during VM boots. I've also fixed the related marvin test. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
18 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
For complete description please refer -> CLOUDSTACK-9604