You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by dmabry <gi...@git.apache.org> on 2016/04/18 15:20:11 UTC
[GitHub] cloudstack pull request: CLOUDSTACK-9349
GitHub user dmabry opened a pull request:
https://github.com/apache/cloudstack/pull/1500
CLOUDSTACK-9349
This PR addresses the KVM detach/attach ROOT disks from VMs (CLOUDSTACK-9349). In short, this allows the KVM Hypervisor, and I added the Simulator as a valid hypervisor for ease of development and testing of marvin, to detach a root volume and the reattach a root volume using the deviceid=0 flag to the attachVolume API. I have also written a marvin integration test that verifies this feature works for both KVM and the Simulator.
Below is the marvin results files of the full marvin test_volumes.py. All tests pass, including the new root detach/attach, on our KVM lab running with the patches in this PR.
[test_volumes_KIR4G3.zip](https://github.com/apache/cloudstack/files/223799/test_volumes_KIR4G3.zip)
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/myENA/cloudstack KVM_root_detach
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/cloudstack/pull/1500.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #1500
----
commit 48ce76344040de2ab8014f76292abe0421d42f85
Author: Simon Weller <si...@gmail.com>
Date: 2016-03-24T19:55:34Z
Merge pull request #4 from apache/4.7
4.7 PR
commit d0a02640dfd4878da81a2e59588c4b5ff2a06401
Author: Simon Weller <sw...@ena.com>
Date: 2016-04-14T13:28:37Z
Let hypervisor type KVM detach root volumes
commit 7807955433cea390bb7358e3bb90dbc9cc06bbea
Author: David Mabry <dm...@ena.com>
Date: 2016-04-15T12:30:07Z
updated test_volumes.py to include a test for detaching and reattaching a root volume from a vm. I also had to update base.py to all attach_volume to have the parameter deviceid to be passed as needed.
commit d7d55630daff4a5e17c9a374dc2e9bc478dff808
Author: David Mabry <dm...@ena.com>
Date: 2016-04-18T02:41:29Z
Added Simulator as valid hypervisor for root detach
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-9349: Enable root disk detach ...
Posted by nitt10prashant <gi...@git.apache.org>.
Github user nitt10prashant commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1500#discussion_r60533118
--- Diff: test/integration/component/test_volumes.py ---
@@ -603,7 +603,144 @@ def test_01_volume_attach_detach(self):
"Check the state of VM"
)
except Exception as e:
- self.fail("Exception occuered: %s" % e)
+ self.fail("Exception occurred: %s" % e)
+ return
+
+ @attr(tags=["advanced", "advancedns"], required_hardware="false")
+ def test_02_root_volume_attach_detach(self):
+ """Test Root Volume attach/detach to VM
+ """
+
+ # Validate the following
+ # 1. Deploy a VM
+ # 2. Verify that we are testing a supported hypervisor
+ # 3. Check for root volume
+ # 4. Stop VM
+ # 5. Detach root volume
+ # 6. Verify root volume detached
+ # 7. Attach root volume
+ # 8. Start VM
+
+ # Verify we are using a supported hypervisor
+ if (self.hypervisor.lower() == 'vmware'
+ or self.hypervisor.lower() == 'kvm'
+ or self.hypervisor.lower() == 'simulator'
+ or self.hypervisor.lower() == 'xenserver'):
+
+ try:
+ # Check for root volume
+ root_volume_response = Volume.list(
+ self.apiclient,
+ virtualmachineid=self.virtual_machine.id,
+ type='ROOT',
+ listall=True
+ )
+ self.assertNotEqual(
+ root_volume_response,
+ None,
+ "Check if root volume exists in ListVolumes"
+ )
+ self.assertEqual(
+ isinstance(root_volume_response, list),
+ True,
+ "Check list volumes response for valid list"
+ )
+ # Grab the root volume for later use
+ root_volume = root_volume_response[0]
+
+ # Stop VM
+ self.debug("Stopping the VM: %s" % self.virtual_machine.id)
+ self.virtual_machine.stop(self.apiclient)
+
+ # Ensure VM is stopped before detaching the root volume
+ time.sleep(self.services["sleep"])
--- End diff --
No need use sleep time .Marvin automatically wait until it gets stop vm reply
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-9349: Enable root disk detach ...
Posted by kiwiflyer <gi...@git.apache.org>.
Github user kiwiflyer commented on the pull request:
https://github.com/apache/cloudstack/pull/1500#issuecomment-215410772
We'll take a look at this in a separate PR. We don't use the UI.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-9349: Enable root disk detach ...
Posted by koushik-das <gi...@git.apache.org>.
Github user koushik-das commented on the pull request:
https://github.com/apache/cloudstack/pull/1500#issuecomment-212267285
@dmabry Thanks for the fix. I ran it on simulator and it worked as expected. So +1 on the changes. Although it existed for XenServer and Vmware, I am trying to understand the use case for this. Is it used for root disk recovery by attaching it to a new VM?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-9349: Enable root disk detach ...
Posted by dmabry <gi...@git.apache.org>.
Github user dmabry commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1500#discussion_r60581671
--- Diff: test/integration/component/test_volumes.py ---
@@ -603,7 +603,144 @@ def test_01_volume_attach_detach(self):
"Check the state of VM"
)
except Exception as e:
- self.fail("Exception occuered: %s" % e)
+ self.fail("Exception occurred: %s" % e)
+ return
+
+ @attr(tags=["advanced", "advancedns"], required_hardware="false")
+ def test_02_root_volume_attach_detach(self):
+ """Test Root Volume attach/detach to VM
+ """
+
+ # Validate the following
+ # 1. Deploy a VM
+ # 2. Verify that we are testing a supported hypervisor
+ # 3. Check for root volume
+ # 4. Stop VM
+ # 5. Detach root volume
+ # 6. Verify root volume detached
+ # 7. Attach root volume
+ # 8. Start VM
+
+ # Verify we are using a supported hypervisor
+ if (self.hypervisor.lower() == 'vmware'
+ or self.hypervisor.lower() == 'kvm'
+ or self.hypervisor.lower() == 'simulator'
+ or self.hypervisor.lower() == 'xenserver'):
+
+ try:
+ # Check for root volume
+ root_volume_response = Volume.list(
+ self.apiclient,
+ virtualmachineid=self.virtual_machine.id,
+ type='ROOT',
+ listall=True
+ )
+ self.assertNotEqual(
--- End diff --
@nitt10prashant - Thanks for the feedback. I was merely following an established pattern that I observed earlier in the file. Please look at lines 499 - 510 as an example. I have put the code below for your convenience. Should that code be refactored as well? I just want to understand if my situation that I'm testing is different than the code I'm referencing below. In the end, I'm happy to make the change. Thanks again for the feedback.
`
# Check List Volume response for newly created volume
list_volume_response = Volume.list(
self.apiclient,
id=volume.id
)
self.assertNotEqual(
list_volume_response,
None,
"Check if volume exists in ListVolumes")
self.assertEqual(
isinstance(list_volume_response, list),
True,
"Check list volumes response for valid list")`
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-9349: Enable root disk detach ...
Posted by nitt10prashant <gi...@git.apache.org>.
Github user nitt10prashant commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1500#discussion_r60531171
--- Diff: test/integration/component/test_volumes.py ---
@@ -603,7 +603,144 @@ def test_01_volume_attach_detach(self):
"Check the state of VM"
)
except Exception as e:
- self.fail("Exception occuered: %s" % e)
+ self.fail("Exception occurred: %s" % e)
+ return
+
+ @attr(tags=["advanced", "advancedns"], required_hardware="false")
+ def test_02_root_volume_attach_detach(self):
+ """Test Root Volume attach/detach to VM
+ """
+
+ # Validate the following
+ # 1. Deploy a VM
+ # 2. Verify that we are testing a supported hypervisor
+ # 3. Check for root volume
+ # 4. Stop VM
+ # 5. Detach root volume
+ # 6. Verify root volume detached
+ # 7. Attach root volume
+ # 8. Start VM
+
+ # Verify we are using a supported hypervisor
+ if (self.hypervisor.lower() == 'vmware'
+ or self.hypervisor.lower() == 'kvm'
+ or self.hypervisor.lower() == 'simulator'
+ or self.hypervisor.lower() == 'xenserver'):
+
+ try:
+ # Check for root volume
+ root_volume_response = Volume.list(
+ self.apiclient,
+ virtualmachineid=self.virtual_machine.id,
+ type='ROOT',
+ listall=True
+ )
+ self.assertNotEqual(
--- End diff --
utils.py have a method validateList to check the response and instance type , can please you use it
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-9349: Enable root disk detach ...
Posted by dmabry <gi...@git.apache.org>.
Github user dmabry commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1500#discussion_r60602084
--- Diff: test/integration/component/test_volumes.py ---
@@ -603,7 +603,144 @@ def test_01_volume_attach_detach(self):
"Check the state of VM"
)
except Exception as e:
- self.fail("Exception occuered: %s" % e)
+ self.fail("Exception occurred: %s" % e)
+ return
+
+ @attr(tags=["advanced", "advancedns"], required_hardware="false")
+ def test_02_root_volume_attach_detach(self):
+ """Test Root Volume attach/detach to VM
+ """
+
+ # Validate the following
+ # 1. Deploy a VM
+ # 2. Verify that we are testing a supported hypervisor
+ # 3. Check for root volume
+ # 4. Stop VM
+ # 5. Detach root volume
+ # 6. Verify root volume detached
+ # 7. Attach root volume
+ # 8. Start VM
+
+ # Verify we are using a supported hypervisor
+ if (self.hypervisor.lower() == 'vmware'
+ or self.hypervisor.lower() == 'kvm'
+ or self.hypervisor.lower() == 'simulator'
+ or self.hypervisor.lower() == 'xenserver'):
+
+ try:
+ # Check for root volume
+ root_volume_response = Volume.list(
+ self.apiclient,
+ virtualmachineid=self.virtual_machine.id,
+ type='ROOT',
+ listall=True
+ )
+ self.assertNotEqual(
--- End diff --
I have implemented this change and I'm validating it now in my lab.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-9349: Enable root disk detach ...
Posted by kiwiflyer <gi...@git.apache.org>.
Github user kiwiflyer commented on the pull request:
https://github.com/apache/cloudstack/pull/1500#issuecomment-215401063
@swill CI Test good, 2+ reviews. Ready to Merge.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-9349: Enable root disk detach ...
Posted by nitt10prashant <gi...@git.apache.org>.
Github user nitt10prashant commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1500#discussion_r60533148
--- Diff: test/integration/component/test_volumes.py ---
@@ -603,7 +603,144 @@ def test_01_volume_attach_detach(self):
"Check the state of VM"
)
except Exception as e:
- self.fail("Exception occuered: %s" % e)
+ self.fail("Exception occurred: %s" % e)
+ return
+
+ @attr(tags=["advanced", "advancedns"], required_hardware="false")
+ def test_02_root_volume_attach_detach(self):
+ """Test Root Volume attach/detach to VM
+ """
+
+ # Validate the following
+ # 1. Deploy a VM
+ # 2. Verify that we are testing a supported hypervisor
+ # 3. Check for root volume
+ # 4. Stop VM
+ # 5. Detach root volume
+ # 6. Verify root volume detached
+ # 7. Attach root volume
+ # 8. Start VM
+
+ # Verify we are using a supported hypervisor
+ if (self.hypervisor.lower() == 'vmware'
+ or self.hypervisor.lower() == 'kvm'
+ or self.hypervisor.lower() == 'simulator'
+ or self.hypervisor.lower() == 'xenserver'):
+
+ try:
+ # Check for root volume
+ root_volume_response = Volume.list(
+ self.apiclient,
+ virtualmachineid=self.virtual_machine.id,
+ type='ROOT',
+ listall=True
+ )
+ self.assertNotEqual(
+ root_volume_response,
+ None,
+ "Check if root volume exists in ListVolumes"
+ )
+ self.assertEqual(
+ isinstance(root_volume_response, list),
+ True,
+ "Check list volumes response for valid list"
+ )
+ # Grab the root volume for later use
+ root_volume = root_volume_response[0]
+
+ # Stop VM
+ self.debug("Stopping the VM: %s" % self.virtual_machine.id)
+ self.virtual_machine.stop(self.apiclient)
+
+ # Ensure VM is stopped before detaching the root volume
+ time.sleep(self.services["sleep"])
+
+ vm_response = VirtualMachine.list(
+ self.apiclient,
+ id=self.virtual_machine.id,
+ )
+ vm = vm_response[0]
+ self.assertEqual(
+ vm.state,
+ 'Stopped',
+ "Check the state of VM"
+ )
+
+ # Detach root volume from VM
+ self.virtual_machine.detach_volume(
+ self.apiclient,
+ root_volume
+ )
+
+ # Verify that root disk is gone
+ no_root_volume_response = Volume.list(
+ self.apiclient,
+ virtualmachineid=self.virtual_machine.id,
+ type='ROOT',
+ listall=True
+ )
+ self.assertEqual(
+ no_root_volume_response,
+ None,
+ "Check if root volume exists in ListVolumes"
+ )
+
+ # Attach root volume to VM
+ self.virtual_machine.attach_volume(
+ self.apiclient,
+ root_volume,
+ 0
+ )
+
+ # Check for root volume
+ new_root_volume_response = Volume.list(
+ self.apiclient,
+ virtualmachineid=self.virtual_machine.id,
+ type='ROOT',
+ listall=True
+ )
+ self.assertNotEqual(
+ new_root_volume_response,
+ None,
+ "Check if root volume exists in ListVolumes"
+ )
+ self.assertEqual(
+ isinstance(new_root_volume_response, list),
+ True,
+ "Check list volumes response for valid list"
+ )
+
+ # Start VM
+ self.virtual_machine.start(self.apiclient)
+ # Sleep to ensure that VM is in ready state
+ time.sleep(self.services["sleep"])
--- End diff --
No need use sleep time .Marvin automatically wait until it gets start vm reply
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-9349: Enable root disk detach ...
Posted by shwetaag <gi...@git.apache.org>.
Github user shwetaag commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1500#discussion_r60193089
--- Diff: test/integration/component/test_volumes.py ---
@@ -603,7 +603,134 @@ def test_01_volume_attach_detach(self):
"Check the state of VM"
)
except Exception as e:
- self.fail("Exception occuered: %s" % e)
+ self.fail("Exception occurred: %s" % e)
+ return
+
+ @attr(tags=["advanced", "advancedns"])
+ def test_02_root_volume_attach_detach(self):
+ """Test Root Volume attach/detach to VM
+ """
+
+ # Validate the following
+ # 1. Deploy a VM
+ # 2. Check for root volume
+ # 3. Stop VM
+ # 4. Detach root volume
+ # 5. Verify root volume detached
+ # 6. Attach root volume
+ # 7. Start VM
+
+ try:
+ # Check for root volume
--- End diff --
First Verify the hypervisor type is KVM in order for this test to execute else skip this test . As root volume detach is supported for KVM .
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-9349: Enable root disk detach ...
Posted by koushik-das <gi...@git.apache.org>.
Github user koushik-das commented on the pull request:
https://github.com/apache/cloudstack/pull/1500#issuecomment-215372191
LGTM. Had already given one earlier based on simulator testing.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-9349: Enable root disk detach ...
Posted by nitt10prashant <gi...@git.apache.org>.
Github user nitt10prashant commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1500#discussion_r60532285
--- Diff: test/integration/component/test_volumes.py ---
@@ -603,7 +603,144 @@ def test_01_volume_attach_detach(self):
"Check the state of VM"
)
except Exception as e:
- self.fail("Exception occuered: %s" % e)
+ self.fail("Exception occurred: %s" % e)
+ return
+
+ @attr(tags=["advanced", "advancedns"], required_hardware="false")
+ def test_02_root_volume_attach_detach(self):
+ """Test Root Volume attach/detach to VM
+ """
+
+ # Validate the following
+ # 1. Deploy a VM
+ # 2. Verify that we are testing a supported hypervisor
+ # 3. Check for root volume
+ # 4. Stop VM
+ # 5. Detach root volume
+ # 6. Verify root volume detached
+ # 7. Attach root volume
+ # 8. Start VM
+
+ # Verify we are using a supported hypervisor
+ if (self.hypervisor.lower() == 'vmware'
+ or self.hypervisor.lower() == 'kvm'
+ or self.hypervisor.lower() == 'simulator'
+ or self.hypervisor.lower() == 'xenserver'):
+
+ try:
+ # Check for root volume
+ root_volume_response = Volume.list(
+ self.apiclient,
+ virtualmachineid=self.virtual_machine.id,
+ type='ROOT',
+ listall=True
+ )
+ self.assertNotEqual(
+ root_volume_response,
+ None,
+ "Check if root volume exists in ListVolumes"
+ )
+ self.assertEqual(
+ isinstance(root_volume_response, list),
+ True,
+ "Check list volumes response for valid list"
+ )
+ # Grab the root volume for later use
+ root_volume = root_volume_response[0]
+
+ # Stop VM
+ self.debug("Stopping the VM: %s" % self.virtual_machine.id)
+ self.virtual_machine.stop(self.apiclient)
+
+ # Ensure VM is stopped before detaching the root volume
+ time.sleep(self.services["sleep"])
+
+ vm_response = VirtualMachine.list(
+ self.apiclient,
+ id=self.virtual_machine.id,
+ )
+ vm = vm_response[0]
+ self.assertEqual(
+ vm.state,
+ 'Stopped',
+ "Check the state of VM"
+ )
+
+ # Detach root volume from VM
+ self.virtual_machine.detach_volume(
+ self.apiclient,
+ root_volume
+ )
+
+ # Verify that root disk is gone
+ no_root_volume_response = Volume.list(
+ self.apiclient,
+ virtualmachineid=self.virtual_machine.id,
+ type='ROOT',
+ listall=True
+ )
+ self.assertEqual(
+ no_root_volume_response,
+ None,
+ "Check if root volume exists in ListVolumes"
+ )
+
+ # Attach root volume to VM
+ self.virtual_machine.attach_volume(
+ self.apiclient,
+ root_volume,
+ 0
+ )
+
+ # Check for root volume
+ new_root_volume_response = Volume.list(
+ self.apiclient,
+ virtualmachineid=self.virtual_machine.id,
+ type='ROOT',
+ listall=True
+ )
+ self.assertNotEqual(
+ new_root_volume_response,
+ None,
+ "Check if root volume exists in ListVolumes"
+ )
+ self.assertEqual(
+ isinstance(new_root_volume_response, list),
+ True,
+ "Check list volumes response for valid list"
+ )
+
+ # Start VM
+ self.virtual_machine.start(self.apiclient)
+ # Sleep to ensure that VM is in ready state
+ time.sleep(self.services["sleep"])
+
+ vm_response = VirtualMachine.list(
+ self.apiclient,
+ id=self.virtual_machine.id,
+ )
+ # Verify VM response to check whether VM deployment was successful
+ self.assertEqual(
+ isinstance(vm_response, list),
+ True,
+ "Check list VM response for valid list"
+ )
+ self.assertNotEqual(
+ len(vm_response),
+ 0,
+ "Check VMs available in List VMs response"
+ )
+ vm = vm_response[0]
+ self.assertEqual(
+ vm.state,
+ 'Running',
+ "Check the state of VM"
+ )
+ except Exception as e:
+ self.fail("Exception occurred: %s" % e)
+
+ else:
+ self.skipTest("Root Volume attach/detach is not supported on %s " % self.hypervisor)
return
--- End diff --
where ever you are listing you can use validatelist , instead of checking len and instance type separately .Code will look cleaner and smaller
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-9349: Enable root disk detach ...
Posted by jburwell <gi...@git.apache.org>.
Github user jburwell commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1500#discussion_r60582658
--- Diff: test/integration/component/test_volumes.py ---
@@ -603,7 +603,144 @@ def test_01_volume_attach_detach(self):
"Check the state of VM"
)
except Exception as e:
- self.fail("Exception occuered: %s" % e)
+ self.fail("Exception occurred: %s" % e)
+ return
+
+ @attr(tags=["advanced", "advancedns"], required_hardware="false")
+ def test_02_root_volume_attach_detach(self):
+ """Test Root Volume attach/detach to VM
+ """
+
+ # Validate the following
+ # 1. Deploy a VM
+ # 2. Verify that we are testing a supported hypervisor
+ # 3. Check for root volume
+ # 4. Stop VM
+ # 5. Detach root volume
+ # 6. Verify root volume detached
+ # 7. Attach root volume
+ # 8. Start VM
+
+ # Verify we are using a supported hypervisor
+ if (self.hypervisor.lower() == 'vmware'
+ or self.hypervisor.lower() == 'kvm'
+ or self.hypervisor.lower() == 'simulator'
+ or self.hypervisor.lower() == 'xenserver'):
+
+ try:
+ # Check for root volume
+ root_volume_response = Volume.list(
+ self.apiclient,
+ virtualmachineid=self.virtual_machine.id,
+ type='ROOT',
+ listall=True
+ )
+ self.assertNotEqual(
+ root_volume_response,
+ None,
+ "Check if root volume exists in ListVolumes"
+ )
+ self.assertEqual(
+ isinstance(root_volume_response, list),
+ True,
+ "Check list volumes response for valid list"
+ )
+ # Grab the root volume for later use
+ root_volume = root_volume_response[0]
+
+ # Stop VM
+ self.debug("Stopping the VM: %s" % self.virtual_machine.id)
+ self.virtual_machine.stop(self.apiclient)
+
+ # Ensure VM is stopped before detaching the root volume
+ time.sleep(self.services["sleep"])
+
+ vm_response = VirtualMachine.list(
+ self.apiclient,
+ id=self.virtual_machine.id,
+ )
+ vm = vm_response[0]
+ self.assertEqual(
+ vm.state,
+ 'Stopped',
+ "Check the state of VM"
+ )
+
+ # Detach root volume from VM
+ self.virtual_machine.detach_volume(
+ self.apiclient,
+ root_volume
+ )
+
+ # Verify that root disk is gone
+ no_root_volume_response = Volume.list(
+ self.apiclient,
+ virtualmachineid=self.virtual_machine.id,
+ type='ROOT',
+ listall=True
+ )
+ self.assertEqual(
+ no_root_volume_response,
+ None,
+ "Check if root volume exists in ListVolumes"
+ )
+
+ # Attach root volume to VM
+ self.virtual_machine.attach_volume(
+ self.apiclient,
+ root_volume,
+ 0
+ )
+
+ # Check for root volume
+ new_root_volume_response = Volume.list(
+ self.apiclient,
+ virtualmachineid=self.virtual_machine.id,
+ type='ROOT',
+ listall=True
+ )
+ self.assertNotEqual(
+ new_root_volume_response,
+ None,
+ "Check if root volume exists in ListVolumes"
+ )
+ self.assertEqual(
+ isinstance(new_root_volume_response, list),
+ True,
+ "Check list volumes response for valid list"
+ )
+
+ # Start VM
+ self.virtual_machine.start(self.apiclient)
+ # Sleep to ensure that VM is in ready state
+ time.sleep(self.services["sleep"])
--- End diff --
At a minimum, there should be an assert that the VM is ready. If some reason it has not become ready, failing fast will make diagnosing the failure much easier.
Also, if you do need to wait, please consider the new ``wait_until`` function that was recently added to Marvin utils which provides a more robust mechanism for waiting for a condition before proceeding with a test.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-9349: Enable root disk detach ...
Posted by dmabry <gi...@git.apache.org>.
Github user dmabry commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1500#discussion_r60584441
--- Diff: test/integration/component/test_volumes.py ---
@@ -603,7 +603,144 @@ def test_01_volume_attach_detach(self):
"Check the state of VM"
)
except Exception as e:
- self.fail("Exception occuered: %s" % e)
+ self.fail("Exception occurred: %s" % e)
+ return
+
+ @attr(tags=["advanced", "advancedns"], required_hardware="false")
+ def test_02_root_volume_attach_detach(self):
+ """Test Root Volume attach/detach to VM
+ """
+
+ # Validate the following
+ # 1. Deploy a VM
+ # 2. Verify that we are testing a supported hypervisor
+ # 3. Check for root volume
+ # 4. Stop VM
+ # 5. Detach root volume
+ # 6. Verify root volume detached
+ # 7. Attach root volume
+ # 8. Start VM
+
+ # Verify we are using a supported hypervisor
+ if (self.hypervisor.lower() == 'vmware'
+ or self.hypervisor.lower() == 'kvm'
+ or self.hypervisor.lower() == 'simulator'
+ or self.hypervisor.lower() == 'xenserver'):
+
+ try:
+ # Check for root volume
+ root_volume_response = Volume.list(
+ self.apiclient,
+ virtualmachineid=self.virtual_machine.id,
+ type='ROOT',
+ listall=True
+ )
+ self.assertNotEqual(
+ root_volume_response,
+ None,
+ "Check if root volume exists in ListVolumes"
+ )
+ self.assertEqual(
+ isinstance(root_volume_response, list),
+ True,
+ "Check list volumes response for valid list"
+ )
+ # Grab the root volume for later use
+ root_volume = root_volume_response[0]
+
+ # Stop VM
+ self.debug("Stopping the VM: %s" % self.virtual_machine.id)
+ self.virtual_machine.stop(self.apiclient)
+
+ # Ensure VM is stopped before detaching the root volume
+ time.sleep(self.services["sleep"])
+
+ vm_response = VirtualMachine.list(
+ self.apiclient,
+ id=self.virtual_machine.id,
+ )
+ vm = vm_response[0]
+ self.assertEqual(
+ vm.state,
+ 'Stopped',
+ "Check the state of VM"
+ )
+
+ # Detach root volume from VM
+ self.virtual_machine.detach_volume(
+ self.apiclient,
+ root_volume
+ )
+
+ # Verify that root disk is gone
+ no_root_volume_response = Volume.list(
+ self.apiclient,
+ virtualmachineid=self.virtual_machine.id,
+ type='ROOT',
+ listall=True
+ )
+ self.assertEqual(
+ no_root_volume_response,
+ None,
+ "Check if root volume exists in ListVolumes"
+ )
+
+ # Attach root volume to VM
+ self.virtual_machine.attach_volume(
+ self.apiclient,
+ root_volume,
+ 0
+ )
+
+ # Check for root volume
+ new_root_volume_response = Volume.list(
+ self.apiclient,
+ virtualmachineid=self.virtual_machine.id,
+ type='ROOT',
+ listall=True
+ )
+ self.assertNotEqual(
+ new_root_volume_response,
+ None,
+ "Check if root volume exists in ListVolumes"
+ )
+ self.assertEqual(
+ isinstance(new_root_volume_response, list),
+ True,
+ "Check list volumes response for valid list"
+ )
+
+ # Start VM
+ self.virtual_machine.start(self.apiclient)
+ # Sleep to ensure that VM is in ready state
+ time.sleep(self.services["sleep"])
--- End diff --
Understood. Please see my earlier comment. I was merely following an established pattern that I observed earlier in the code. Please look at the following code taken start at line 547 in test_volumes.py. Does it need to be addressed as well?
` # Reboot VM
self.debug("Rebooting the VM: %s" % self.virtual_machine.id)
self.virtual_machine.reboot(self.apiclient)
# Sleep to ensure that VM is in ready state
time.sleep(self.services["sleep"])
vm_response = VirtualMachine.list(
self.apiclient,
id=self.virtual_machine.id,
)
# Verify VM response to check whether VM deployment was successful
self.assertEqual(
isinstance(vm_response, list),
True,
"Check list VM response for valid list"
)
self.assertNotEqual(
len(vm_response),
0,
"Check VMs available in List VMs response"
)
vm = vm_response[0]
self.assertEqual(
vm.state,
'Running',
"Check the state of VM"
)
# Stop VM
self.virtual_machine.stop(self.apiclient)
# Start VM
self.virtual_machine.start(self.apiclient)
# Sleep to ensure that VM is in ready state
time.sleep(self.services["sleep"])
`
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-9349: Enable root disk detach ...
Posted by dmabry <gi...@git.apache.org>.
Github user dmabry commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1500#discussion_r60585058
--- Diff: test/integration/component/test_volumes.py ---
@@ -603,7 +603,144 @@ def test_01_volume_attach_detach(self):
"Check the state of VM"
)
except Exception as e:
- self.fail("Exception occuered: %s" % e)
+ self.fail("Exception occurred: %s" % e)
+ return
+
+ @attr(tags=["advanced", "advancedns"], required_hardware="false")
+ def test_02_root_volume_attach_detach(self):
+ """Test Root Volume attach/detach to VM
+ """
+
+ # Validate the following
+ # 1. Deploy a VM
+ # 2. Verify that we are testing a supported hypervisor
+ # 3. Check for root volume
+ # 4. Stop VM
+ # 5. Detach root volume
+ # 6. Verify root volume detached
+ # 7. Attach root volume
+ # 8. Start VM
+
+ # Verify we are using a supported hypervisor
+ if (self.hypervisor.lower() == 'vmware'
+ or self.hypervisor.lower() == 'kvm'
+ or self.hypervisor.lower() == 'simulator'
+ or self.hypervisor.lower() == 'xenserver'):
+
+ try:
+ # Check for root volume
+ root_volume_response = Volume.list(
+ self.apiclient,
+ virtualmachineid=self.virtual_machine.id,
+ type='ROOT',
+ listall=True
+ )
+ self.assertNotEqual(
+ root_volume_response,
+ None,
+ "Check if root volume exists in ListVolumes"
+ )
+ self.assertEqual(
+ isinstance(root_volume_response, list),
+ True,
+ "Check list volumes response for valid list"
+ )
+ # Grab the root volume for later use
+ root_volume = root_volume_response[0]
+
+ # Stop VM
+ self.debug("Stopping the VM: %s" % self.virtual_machine.id)
+ self.virtual_machine.stop(self.apiclient)
+
+ # Ensure VM is stopped before detaching the root volume
+ time.sleep(self.services["sleep"])
+
+ vm_response = VirtualMachine.list(
+ self.apiclient,
+ id=self.virtual_machine.id,
+ )
+ vm = vm_response[0]
+ self.assertEqual(
+ vm.state,
+ 'Stopped',
+ "Check the state of VM"
+ )
+
+ # Detach root volume from VM
+ self.virtual_machine.detach_volume(
+ self.apiclient,
+ root_volume
+ )
+
+ # Verify that root disk is gone
+ no_root_volume_response = Volume.list(
+ self.apiclient,
+ virtualmachineid=self.virtual_machine.id,
+ type='ROOT',
+ listall=True
+ )
+ self.assertEqual(
+ no_root_volume_response,
+ None,
+ "Check if root volume exists in ListVolumes"
+ )
+
+ # Attach root volume to VM
+ self.virtual_machine.attach_volume(
+ self.apiclient,
+ root_volume,
+ 0
+ )
+
+ # Check for root volume
+ new_root_volume_response = Volume.list(
+ self.apiclient,
+ virtualmachineid=self.virtual_machine.id,
+ type='ROOT',
+ listall=True
+ )
+ self.assertNotEqual(
+ new_root_volume_response,
+ None,
+ "Check if root volume exists in ListVolumes"
+ )
+ self.assertEqual(
+ isinstance(new_root_volume_response, list),
+ True,
+ "Check list volumes response for valid list"
+ )
+
+ # Start VM
+ self.virtual_machine.start(self.apiclient)
+ # Sleep to ensure that VM is in ready state
+ time.sleep(self.services["sleep"])
--- End diff --
@jburwell - Thanks for the feedback. I will make the suggested changes as it make complete sense to wait_until instead of waiting for an arbitrary amount of time that may be too long or not long enough.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-9349: Enable root disk detach ...
Posted by dmabry <gi...@git.apache.org>.
Github user dmabry commented on the pull request:
https://github.com/apache/cloudstack/pull/1500#issuecomment-212426369
@koushik-das - Thanks for the feedback. I have added the require_hardware="false" as you suggested and pushed a new commit to the branch.
If all looks good, I'll squash the commits in prep for the final merge.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-9349: Enable root disk detach ...
Posted by koushik-das <gi...@git.apache.org>.
Github user koushik-das commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1500#discussion_r60352254
--- Diff: test/integration/component/test_volumes.py ---
@@ -603,7 +603,144 @@ def test_01_volume_attach_detach(self):
"Check the state of VM"
)
except Exception as e:
- self.fail("Exception occuered: %s" % e)
+ self.fail("Exception occurred: %s" % e)
+ return
+
+ @attr(tags=["advanced", "advancedns"])
--- End diff --
@dmabry Since the test can run on simulator, please add required_hardware="false" to the tags.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-9349: Enable root disk detach ...
Posted by dmabry <gi...@git.apache.org>.
Github user dmabry commented on the pull request:
https://github.com/apache/cloudstack/pull/1500#issuecomment-215103808
Yea, I saw that jenkins failed pulling 4.7 branch, so I kicked it off again. I'm going to keep an eye on travis and jenkins. There is no reason those tests should fail. Hopefully, I'll get a green this time around. ;)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-9349: Enable root disk detach ...
Posted by dmabry <gi...@git.apache.org>.
Github user dmabry commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1500#discussion_r60582064
--- Diff: test/integration/component/test_volumes.py ---
@@ -603,7 +603,144 @@ def test_01_volume_attach_detach(self):
"Check the state of VM"
)
except Exception as e:
- self.fail("Exception occuered: %s" % e)
+ self.fail("Exception occurred: %s" % e)
+ return
+
+ @attr(tags=["advanced", "advancedns"], required_hardware="false")
+ def test_02_root_volume_attach_detach(self):
+ """Test Root Volume attach/detach to VM
+ """
+
+ # Validate the following
+ # 1. Deploy a VM
+ # 2. Verify that we are testing a supported hypervisor
+ # 3. Check for root volume
+ # 4. Stop VM
+ # 5. Detach root volume
+ # 6. Verify root volume detached
+ # 7. Attach root volume
+ # 8. Start VM
+
+ # Verify we are using a supported hypervisor
+ if (self.hypervisor.lower() == 'vmware'
+ or self.hypervisor.lower() == 'kvm'
+ or self.hypervisor.lower() == 'simulator'
+ or self.hypervisor.lower() == 'xenserver'):
+
+ try:
+ # Check for root volume
+ root_volume_response = Volume.list(
+ self.apiclient,
+ virtualmachineid=self.virtual_machine.id,
+ type='ROOT',
+ listall=True
+ )
+ self.assertNotEqual(
+ root_volume_response,
+ None,
+ "Check if root volume exists in ListVolumes"
+ )
+ self.assertEqual(
+ isinstance(root_volume_response, list),
+ True,
+ "Check list volumes response for valid list"
+ )
+ # Grab the root volume for later use
+ root_volume = root_volume_response[0]
+
+ # Stop VM
+ self.debug("Stopping the VM: %s" % self.virtual_machine.id)
+ self.virtual_machine.stop(self.apiclient)
+
+ # Ensure VM is stopped before detaching the root volume
+ time.sleep(self.services["sleep"])
+
+ vm_response = VirtualMachine.list(
+ self.apiclient,
+ id=self.virtual_machine.id,
+ )
--- End diff --
Understood. I'll make the change.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-9349: Enable root disk detach ...
Posted by dmabry <gi...@git.apache.org>.
Github user dmabry commented on the pull request:
https://github.com/apache/cloudstack/pull/1500#issuecomment-215120369
OK, all checks have passed! ;) Looking for another LGTM.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-9349: Enable root disk detach ...
Posted by swill <gi...@git.apache.org>.
Github user swill commented on the pull request:
https://github.com/apache/cloudstack/pull/1500#issuecomment-211551366
would you mind squashing your commits for this PR?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-9349: Enable root disk detach ...
Posted by dmabry <gi...@git.apache.org>.
Github user dmabry commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1500#discussion_r60225742
--- Diff: test/integration/component/test_volumes.py ---
@@ -603,7 +603,134 @@ def test_01_volume_attach_detach(self):
"Check the state of VM"
)
except Exception as e:
- self.fail("Exception occuered: %s" % e)
+ self.fail("Exception occurred: %s" % e)
+ return
+
+ @attr(tags=["advanced", "advancedns"])
+ def test_02_root_volume_attach_detach(self):
+ """Test Root Volume attach/detach to VM
+ """
+
+ # Validate the following
+ # 1. Deploy a VM
+ # 2. Check for root volume
+ # 3. Stop VM
+ # 4. Detach root volume
+ # 5. Verify root volume detached
+ # 6. Attach root volume
+ # 7. Start VM
+
+ try:
+ # Check for root volume
+ root_volume_response = Volume.list(
+ self.apiclient,
+ virtualmachineid=self.virtual_machine.id,
+ type='ROOT',
+ listall=True
+ )
+ self.assertNotEqual(
+ root_volume_response,
+ None,
+ "Check if root volume exists in ListVolumes"
+ )
+ self.assertEqual(
+ isinstance(root_volume_response, list),
+ True,
+ "Check list volumes response for valid list"
+ )
+ # Grab the root volume for later use
+ root_volume = root_volume_response[0]
+
+ # Stop VM
+ self.debug("Stopping the VM: %s" % self.virtual_machine.id)
+ self.virtual_machine.stop(self.apiclient)
+
+ # Ensure VM is stopped before detaching the root volume
+ time.sleep(self.services["sleep"])
+
+ vm_response = VirtualMachine.list(
+ self.apiclient,
+ id=self.virtual_machine.id,
+ )
+ vm = vm_response[0]
+ self.assertEqual(
+ vm.state,
+ 'Stopped',
+ "Check the state of VM"
+ )
+
+ # Detach root volume from VM
+ self.virtual_machine.detach_volume(
+ self.apiclient,
+ root_volume
+ )
+
+ # Verify that root disk is gone
+ no_root_volume_response = Volume.list(
+ self.apiclient,
+ virtualmachineid=self.virtual_machine.id,
+ type='ROOT',
+ listall=True
+ )
+ self.assertEqual(
+ no_root_volume_response,
+ None,
+ "Check if root volume exists in ListVolumes"
+ )
+
+ # Attach root volume to VM
+ self.virtual_machine.attach_volume(
+ self.apiclient,
+ root_volume,
+ 0
+ )
+
+ # Check for root volume
+ new_root_volume_response = Volume.list(
+ self.apiclient,
+ virtualmachineid=self.virtual_machine.id,
+ type='ROOT',
+ listall=True
+ )
+ self.assertNotEqual(
+ new_root_volume_response,
+ None,
+ "Check if root volume exists in ListVolumes"
+ )
+ self.assertEqual(
+ isinstance(new_root_volume_response, list),
+ True,
+ "Check list volumes response for valid list"
+ )
+
+ # Start VM
+ self.virtual_machine.start(self.apiclient)
+ # Sleep to ensure that VM is in ready state
+ time.sleep(self.services["sleep"])
+
+ vm_response = VirtualMachine.list(
+ self.apiclient,
+ id=self.virtual_machine.id,
+ )
+ # Verify VM response to check whether VM deployment was successful
+ self.assertEqual(
+ isinstance(vm_response, list),
+ True,
+ "Check list VM response for valid list"
+ )
+ self.assertNotEqual(
+ len(vm_response),
+ 0,
+ "Check VMs available in List VMs response"
+ )
--- End diff --
The more I look at this, I think I understand why you need 2 different tests. The first test ensures that returned object is a list, however that only verifies the type and not the content of the list. The second test ensures that the returned list isn't empty. I guess it is conceivable that the VirtualMachine.list could return an empty (0 len) list, which would indeed by a problem. I would need to look deeper into the cloudstackAPI code to verify VirtualMachine.list behavior, but that is the only logical reason I can think of for both tests. I think it is best to leave this second test in here for now, as I think protects us from an empty list being returned and causing the assert vm.state= "Running" to fail.
Thoughts?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-9349: Enable root disk detach ...
Posted by swill <gi...@git.apache.org>.
Github user swill commented on the pull request:
https://github.com/apache/cloudstack/pull/1500#issuecomment-215103065
@koushik-das can I get your code review on this? Thanks. :)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-9349: Enable root disk detach ...
Posted by dmabry <gi...@git.apache.org>.
Github user dmabry commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1500#discussion_r60602140
--- Diff: test/integration/component/test_volumes.py ---
@@ -603,7 +603,144 @@ def test_01_volume_attach_detach(self):
"Check the state of VM"
)
except Exception as e:
- self.fail("Exception occuered: %s" % e)
+ self.fail("Exception occurred: %s" % e)
+ return
+
+ @attr(tags=["advanced", "advancedns"], required_hardware="false")
+ def test_02_root_volume_attach_detach(self):
+ """Test Root Volume attach/detach to VM
+ """
+
+ # Validate the following
+ # 1. Deploy a VM
+ # 2. Verify that we are testing a supported hypervisor
+ # 3. Check for root volume
+ # 4. Stop VM
+ # 5. Detach root volume
+ # 6. Verify root volume detached
+ # 7. Attach root volume
+ # 8. Start VM
+
+ # Verify we are using a supported hypervisor
+ if (self.hypervisor.lower() == 'vmware'
+ or self.hypervisor.lower() == 'kvm'
+ or self.hypervisor.lower() == 'simulator'
+ or self.hypervisor.lower() == 'xenserver'):
+
+ try:
+ # Check for root volume
+ root_volume_response = Volume.list(
+ self.apiclient,
+ virtualmachineid=self.virtual_machine.id,
+ type='ROOT',
+ listall=True
+ )
+ self.assertNotEqual(
+ root_volume_response,
+ None,
+ "Check if root volume exists in ListVolumes"
+ )
+ self.assertEqual(
+ isinstance(root_volume_response, list),
+ True,
+ "Check list volumes response for valid list"
+ )
+ # Grab the root volume for later use
+ root_volume = root_volume_response[0]
+
+ # Stop VM
+ self.debug("Stopping the VM: %s" % self.virtual_machine.id)
+ self.virtual_machine.stop(self.apiclient)
+
+ # Ensure VM is stopped before detaching the root volume
+ time.sleep(self.services["sleep"])
+
+ vm_response = VirtualMachine.list(
+ self.apiclient,
+ id=self.virtual_machine.id,
+ )
--- End diff --
I have made this change and I'm validating this now in my lab.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-9349: Enable root disk detach ...
Posted by nitt10prashant <gi...@git.apache.org>.
Github user nitt10prashant commented on the pull request:
https://github.com/apache/cloudstack/pull/1500#issuecomment-215308516
LGTM for test code based on code review
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-9349: Enable root disk detach ...
Posted by swill <gi...@git.apache.org>.
Github user swill commented on the pull request:
https://github.com/apache/cloudstack/pull/1500#issuecomment-215102785
Thanks @kiwiflyer. :) I was about to ask you to re-push to kick off the runs again and you did it as I was typing. 👍
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-9349: Enable root disk detach ...
Posted by kiwiflyer <gi...@git.apache.org>.
Github user kiwiflyer commented on the pull request:
https://github.com/apache/cloudstack/pull/1500#issuecomment-212400883
@koushik-das - Our use case is to emulate a snapshot revert with Ceph by using createVolume sourced from a snapshot, then detaching and reattach the root volume of a VM with device id of 0.
This preserves the previous volume history and allows the user to switch back and forth between different snapshots.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-9349: Enable root disk detach ...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/cloudstack/pull/1500
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-9349: Enable root disk detach ...
Posted by swill <gi...@git.apache.org>.
Github user swill commented on the pull request:
https://github.com/apache/cloudstack/pull/1500#issuecomment-215087358
Thank you for the results. I think that is all we need from a CI perspective. Jenkins and Travis have been kicked off again as well, so that is good. Hopefully we will be all green on that front too.
We need some LGTM code reviews on this one. Thanks...
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-9349: Enable root disk detach ...
Posted by dmabry <gi...@git.apache.org>.
Github user dmabry commented on the pull request:
https://github.com/apache/cloudstack/pull/1500#issuecomment-215084188
[test_volumes_NV6XFO.zip](https://github.com/apache/cloudstack/files/238694/test_volumes_NV6XFO.zip)
Here is the zip file containing the test results.
Thanks in advance for the reviews!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-9349: Enable root disk detach ...
Posted by shwetaag <gi...@git.apache.org>.
Github user shwetaag commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1500#discussion_r60193647
--- Diff: test/integration/component/test_volumes.py ---
@@ -603,7 +603,134 @@ def test_01_volume_attach_detach(self):
"Check the state of VM"
)
except Exception as e:
- self.fail("Exception occuered: %s" % e)
+ self.fail("Exception occurred: %s" % e)
+ return
+
+ @attr(tags=["advanced", "advancedns"])
+ def test_02_root_volume_attach_detach(self):
+ """Test Root Volume attach/detach to VM
+ """
+
+ # Validate the following
+ # 1. Deploy a VM
+ # 2. Check for root volume
+ # 3. Stop VM
+ # 4. Detach root volume
+ # 5. Verify root volume detached
+ # 6. Attach root volume
+ # 7. Start VM
+
+ try:
+ # Check for root volume
+ root_volume_response = Volume.list(
+ self.apiclient,
+ virtualmachineid=self.virtual_machine.id,
+ type='ROOT',
+ listall=True
+ )
+ self.assertNotEqual(
+ root_volume_response,
+ None,
+ "Check if root volume exists in ListVolumes"
+ )
+ self.assertEqual(
+ isinstance(root_volume_response, list),
+ True,
+ "Check list volumes response for valid list"
+ )
+ # Grab the root volume for later use
+ root_volume = root_volume_response[0]
+
+ # Stop VM
+ self.debug("Stopping the VM: %s" % self.virtual_machine.id)
+ self.virtual_machine.stop(self.apiclient)
+
+ # Ensure VM is stopped before detaching the root volume
+ time.sleep(self.services["sleep"])
+
+ vm_response = VirtualMachine.list(
+ self.apiclient,
+ id=self.virtual_machine.id,
+ )
+ vm = vm_response[0]
+ self.assertEqual(
+ vm.state,
+ 'Stopped',
+ "Check the state of VM"
+ )
+
+ # Detach root volume from VM
+ self.virtual_machine.detach_volume(
+ self.apiclient,
+ root_volume
+ )
+
+ # Verify that root disk is gone
+ no_root_volume_response = Volume.list(
+ self.apiclient,
+ virtualmachineid=self.virtual_machine.id,
+ type='ROOT',
+ listall=True
+ )
+ self.assertEqual(
+ no_root_volume_response,
+ None,
+ "Check if root volume exists in ListVolumes"
+ )
+
+ # Attach root volume to VM
+ self.virtual_machine.attach_volume(
+ self.apiclient,
+ root_volume,
+ 0
+ )
+
+ # Check for root volume
+ new_root_volume_response = Volume.list(
+ self.apiclient,
+ virtualmachineid=self.virtual_machine.id,
+ type='ROOT',
+ listall=True
+ )
+ self.assertNotEqual(
+ new_root_volume_response,
+ None,
+ "Check if root volume exists in ListVolumes"
+ )
+ self.assertEqual(
+ isinstance(new_root_volume_response, list),
+ True,
+ "Check list volumes response for valid list"
+ )
+
+ # Start VM
+ self.virtual_machine.start(self.apiclient)
+ # Sleep to ensure that VM is in ready state
+ time.sleep(self.services["sleep"])
+
+ vm_response = VirtualMachine.list(
+ self.apiclient,
+ id=self.virtual_machine.id,
+ )
+ # Verify VM response to check whether VM deployment was successful
+ self.assertEqual(
+ isinstance(vm_response, list),
+ True,
+ "Check list VM response for valid list"
+ )
+ self.assertNotEqual(
+ len(vm_response),
+ 0,
+ "Check VMs available in List VMs response"
+ )
--- End diff --
No need of this assert because if assertequal (isinstance) is true then this will surely be true .As the query of list virtual machine is based on VM id
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-9349: Enable root disk detach ...
Posted by dmabry <gi...@git.apache.org>.
Github user dmabry commented on the pull request:
https://github.com/apache/cloudstack/pull/1500#issuecomment-212085924
Ok, I have made the requested changes. Please review and let me know if there is anything else I need to change.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-9349: Enable root disk detach ...
Posted by kiwiflyer <gi...@git.apache.org>.
Github user kiwiflyer commented on the pull request:
https://github.com/apache/cloudstack/pull/1500#issuecomment-215099770
Tested this patch manually on a KVM 4.8 lab. We also ran the new Marvin unit tests successfully against the simulator, bubble instance and a hardware lab. Works as designed.
LGTM
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-9349: Enable root disk detach ...
Posted by dmabry <gi...@git.apache.org>.
Github user dmabry commented on the pull request:
https://github.com/apache/cloudstack/pull/1500#issuecomment-213853837
I made all requested changes. I'm going to run a full bubble CI test against this PR and I will post the results.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-9349: Enable root disk detach ...
Posted by nitt10prashant <gi...@git.apache.org>.
Github user nitt10prashant commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1500#discussion_r60531417
--- Diff: test/integration/component/test_volumes.py ---
@@ -603,7 +603,144 @@ def test_01_volume_attach_detach(self):
"Check the state of VM"
)
except Exception as e:
- self.fail("Exception occuered: %s" % e)
+ self.fail("Exception occurred: %s" % e)
+ return
+
+ @attr(tags=["advanced", "advancedns"], required_hardware="false")
+ def test_02_root_volume_attach_detach(self):
+ """Test Root Volume attach/detach to VM
+ """
+
+ # Validate the following
+ # 1. Deploy a VM
+ # 2. Verify that we are testing a supported hypervisor
+ # 3. Check for root volume
+ # 4. Stop VM
+ # 5. Detach root volume
+ # 6. Verify root volume detached
+ # 7. Attach root volume
+ # 8. Start VM
+
+ # Verify we are using a supported hypervisor
+ if (self.hypervisor.lower() == 'vmware'
+ or self.hypervisor.lower() == 'kvm'
+ or self.hypervisor.lower() == 'simulator'
+ or self.hypervisor.lower() == 'xenserver'):
+
+ try:
+ # Check for root volume
+ root_volume_response = Volume.list(
+ self.apiclient,
+ virtualmachineid=self.virtual_machine.id,
+ type='ROOT',
+ listall=True
+ )
+ self.assertNotEqual(
+ root_volume_response,
+ None,
+ "Check if root volume exists in ListVolumes"
+ )
+ self.assertEqual(
+ isinstance(root_volume_response, list),
+ True,
+ "Check list volumes response for valid list"
+ )
+ # Grab the root volume for later use
+ root_volume = root_volume_response[0]
+
+ # Stop VM
+ self.debug("Stopping the VM: %s" % self.virtual_machine.id)
+ self.virtual_machine.stop(self.apiclient)
+
+ # Ensure VM is stopped before detaching the root volume
+ time.sleep(self.services["sleep"])
+
+ vm_response = VirtualMachine.list(
+ self.apiclient,
+ id=self.virtual_machine.id,
+ )
--- End diff --
if vm_respone is empty addressing zeroth element will error out,Can you please use validate list and provide proper debug message
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-9349: Enable root disk detach ...
Posted by dmabry <gi...@git.apache.org>.
Github user dmabry commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1500#discussion_r60602700
--- Diff: test/integration/component/test_volumes.py ---
@@ -603,7 +603,144 @@ def test_01_volume_attach_detach(self):
"Check the state of VM"
)
except Exception as e:
- self.fail("Exception occuered: %s" % e)
+ self.fail("Exception occurred: %s" % e)
+ return
+
+ @attr(tags=["advanced", "advancedns"], required_hardware="false")
+ def test_02_root_volume_attach_detach(self):
+ """Test Root Volume attach/detach to VM
+ """
+
+ # Validate the following
+ # 1. Deploy a VM
+ # 2. Verify that we are testing a supported hypervisor
+ # 3. Check for root volume
+ # 4. Stop VM
+ # 5. Detach root volume
+ # 6. Verify root volume detached
+ # 7. Attach root volume
+ # 8. Start VM
+
+ # Verify we are using a supported hypervisor
+ if (self.hypervisor.lower() == 'vmware'
+ or self.hypervisor.lower() == 'kvm'
+ or self.hypervisor.lower() == 'simulator'
+ or self.hypervisor.lower() == 'xenserver'):
+
+ try:
+ # Check for root volume
+ root_volume_response = Volume.list(
+ self.apiclient,
+ virtualmachineid=self.virtual_machine.id,
+ type='ROOT',
+ listall=True
+ )
+ self.assertNotEqual(
+ root_volume_response,
+ None,
+ "Check if root volume exists in ListVolumes"
+ )
+ self.assertEqual(
+ isinstance(root_volume_response, list),
+ True,
+ "Check list volumes response for valid list"
+ )
+ # Grab the root volume for later use
+ root_volume = root_volume_response[0]
+
+ # Stop VM
+ self.debug("Stopping the VM: %s" % self.virtual_machine.id)
+ self.virtual_machine.stop(self.apiclient)
+
+ # Ensure VM is stopped before detaching the root volume
+ time.sleep(self.services["sleep"])
+
+ vm_response = VirtualMachine.list(
+ self.apiclient,
+ id=self.virtual_machine.id,
+ )
+ vm = vm_response[0]
+ self.assertEqual(
+ vm.state,
+ 'Stopped',
+ "Check the state of VM"
+ )
+
+ # Detach root volume from VM
+ self.virtual_machine.detach_volume(
+ self.apiclient,
+ root_volume
+ )
+
+ # Verify that root disk is gone
+ no_root_volume_response = Volume.list(
+ self.apiclient,
+ virtualmachineid=self.virtual_machine.id,
+ type='ROOT',
+ listall=True
+ )
+ self.assertEqual(
+ no_root_volume_response,
+ None,
+ "Check if root volume exists in ListVolumes"
+ )
+
+ # Attach root volume to VM
+ self.virtual_machine.attach_volume(
+ self.apiclient,
+ root_volume,
+ 0
+ )
+
+ # Check for root volume
+ new_root_volume_response = Volume.list(
+ self.apiclient,
+ virtualmachineid=self.virtual_machine.id,
+ type='ROOT',
+ listall=True
+ )
+ self.assertNotEqual(
+ new_root_volume_response,
+ None,
+ "Check if root volume exists in ListVolumes"
+ )
+ self.assertEqual(
+ isinstance(new_root_volume_response, list),
+ True,
+ "Check list volumes response for valid list"
+ )
+
+ # Start VM
+ self.virtual_machine.start(self.apiclient)
+ # Sleep to ensure that VM is in ready state
+ time.sleep(self.services["sleep"])
+
+ vm_response = VirtualMachine.list(
+ self.apiclient,
+ id=self.virtual_machine.id,
+ )
+ # Verify VM response to check whether VM deployment was successful
+ self.assertEqual(
+ isinstance(vm_response, list),
+ True,
+ "Check list VM response for valid list"
+ )
+ self.assertNotEqual(
+ len(vm_response),
+ 0,
+ "Check VMs available in List VMs response"
+ )
+ vm = vm_response[0]
+ self.assertEqual(
+ vm.state,
+ 'Running',
+ "Check the state of VM"
+ )
+ except Exception as e:
+ self.fail("Exception occurred: %s" % e)
+
+ else:
+ self.skipTest("Root Volume attach/detach is not supported on %s " % self.hypervisor)
return
--- End diff --
I've implemented this change and I'm currently testing this in my lab
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-9349
Posted by dmabry <gi...@git.apache.org>.
Github user dmabry commented on the pull request:
https://github.com/apache/cloudstack/pull/1500#issuecomment-211377778
Here is a manual test using cloudmonkey against our KVM lab.
`(local) 🐵 > list volumes virtualmachineid=f2870d90-d294-474b-b7da-95bad01e6c09 listall=true
count = 1
volume:
id = 6bd1cf36-2225-4f9c-a378-fe2959142912
name = ROOT-42
account = admin-2177
created = 2016-04-14T07:54:23-0500
destroyed = False
deviceid = 0
displayvolume = True
domain = 2177
domainid = cf23158e-d33c-40c2-b176-0db4a163a93f
hypervisor = KVM
isextractable = False
path = 6bd1cf36-2225-4f9c-a378-fe2959142912
provisioningtype = thin
quiescevm = False
serviceofferingdisplaytext = Small Instance
serviceofferingid = b85e77fd-b897-4471-8fce-1190e71e5156
serviceofferingname = Small Instance
size = 5368709120
state = Ready
storage = rbd1
storageid = f0dbafaa-52a3-3077-bc53-d7ad3a5ac132
storagetype = shared
tags:
templatedisplaytext = Base CentOS 7
templateid = cf02d86b-145b-4777-999c-7f37b16c945b
templatename = Linux_CentOS_7-Minimal
type = ROOT
virtualmachineid = f2870d90-d294-474b-b7da-95bad01e6c09
vmdisplayname = VM-381
vmname = VM-f2870d90-d294-474b-b7da-95bad01e6c09
vmstate = Stopped
zoneid = f58958e8-d24f-4a52-9a9f-8ae11cf4a3b0
zonename = Zone1
(local) 🐵 > detach volume id=6bd1cf36-2225-4f9c-a378-fe2959142912
accountid = 6b00c3ed-fc3f-11e5-9789-000c29b79f06
cmd = org.apache.cloudstack.api.command.admin.volume.DetachVolumeCmdByAdmin
created = 2016-04-16T13:45:56-0500
jobid = 8d499364-6c4a-49be-bc43-3a80ea800ffa
jobinstanceid = 6bd1cf36-2225-4f9c-a378-fe2959142912
jobinstancetype = Volume
jobprocstatus = 0
jobresult:
volume:
id = 6bd1cf36-2225-4f9c-a378-fe2959142912
name = ROOT-42
account = admin-2177
created = 2016-04-14T07:54:23-0500
destroyed = False
displayvolume = True
domain = 2177
domainid = cf23158e-d33c-40c2-b176-0db4a163a93f
hypervisor = KVM
isextractable = True
jobid = 8d499364-6c4a-49be-bc43-3a80ea800ffa
jobstatus = 0
path = 6bd1cf36-2225-4f9c-a378-fe2959142912
provisioningtype = thin
quiescevm = False
serviceofferingdisplaytext = Small Instance
serviceofferingid = b85e77fd-b897-4471-8fce-1190e71e5156
serviceofferingname = Small Instance
size = 5368709120
state = Ready
storage = rbd1
storageid = f0dbafaa-52a3-3077-bc53-d7ad3a5ac132
storagetype = shared
tags:
templatedisplaytext = Base CentOS 7
templateid = cf02d86b-145b-4777-999c-7f37b16c945b
templatename = Linux_CentOS_7-Minimal
type = DATADISK
zoneid = f58958e8-d24f-4a52-9a9f-8ae11cf4a3b0
zonename = Zone1
jobresultcode = 0
jobresulttype = object
jobstatus = 1
userid = 6b00cc09-fc3f-11e5-9789-000c29b79f06
(local) 🐵 >
(local) 🐵 >
(local) 🐵 > list volumes virtualmachineid=f2870d90-d294-474b-b7da-95bad01e6c09 listall=true
(local) 🐵 >
(local) 🐵 >
(local) 🐵 > attach volume id=6bd1cf36-2225-4f9c-a378-fe2959142912 virtualmachineid=f2870d90-d294-474b-b7da-95bad01e6c09
deviceid= filter= id= virtualmachineid=
(local) 🐵 > attach volume id=6bd1cf36-2225-4f9c-a378-fe2959142912 virtualmachineid=f2870d90-d294-474b-b7da-95bad01e6c09 deviceid=0
accountid = 6b00c3ed-fc3f-11e5-9789-000c29b79f06
cmd = org.apache.cloudstack.api.command.admin.volume.AttachVolumeCmdByAdmin
created = 2016-04-16T13:46:43-0500
jobid = 8b36e694-376a-457e-b2be-dc71872177ca
jobinstanceid = 6bd1cf36-2225-4f9c-a378-fe2959142912
jobinstancetype = Volume
jobprocstatus = 0
jobresult:
volume:
id = 6bd1cf36-2225-4f9c-a378-fe2959142912
name = ROOT-42
account = admin-2177
attached = 2016-04-16T13:46:44-0500
created = 2016-04-14T07:54:23-0500
destroyed = False
deviceid = 0
displayvolume = True
domain = 2177
domainid = cf23158e-d33c-40c2-b176-0db4a163a93f
hypervisor = KVM
isextractable = False
jobid = 8b36e694-376a-457e-b2be-dc71872177ca
jobstatus = 0
path = 6bd1cf36-2225-4f9c-a378-fe2959142912
provisioningtype = thin
quiescevm = False
serviceofferingdisplaytext = Small Instance
serviceofferingid = b85e77fd-b897-4471-8fce-1190e71e5156
serviceofferingname = Small Instance
size = 5368709120
state = Ready
storage = rbd1
storageid = f0dbafaa-52a3-3077-bc53-d7ad3a5ac132
storagetype = shared
tags:
templatedisplaytext = Base CentOS 7
templateid = cf02d86b-145b-4777-999c-7f37b16c945b
templatename = Linux_CentOS_7-Minimal
type = ROOT
virtualmachineid = f2870d90-d294-474b-b7da-95bad01e6c09
vmdisplayname = VM-381
vmname = VM-f2870d90-d294-474b-b7da-95bad01e6c09
vmstate = Stopped
zoneid = f58958e8-d24f-4a52-9a9f-8ae11cf4a3b0
zonename = Zone1
jobresultcode = 0
jobresulttype = object
jobstatus = 1
userid = 6b00cc09-fc3f-11e5-9789-000c29b79f06
(local) 🐵 >
(local) 🐵 >
(local) 🐵 > list volumes virtualmachineid=f2870d90-d294-474b-b7da-95bad01e6c09 listall=true
count = 1
volume:
id = 6bd1cf36-2225-4f9c-a378-fe2959142912
name = ROOT-42
account = admin-2177
attached = 2016-04-16T13:46:44-0500
created = 2016-04-14T07:54:23-0500
destroyed = False
deviceid = 0
displayvolume = True
domain = 2177
domainid = cf23158e-d33c-40c2-b176-0db4a163a93f
hypervisor = KVM
isextractable = False
path = 6bd1cf36-2225-4f9c-a378-fe2959142912
provisioningtype = thin
quiescevm = False
serviceofferingdisplaytext = Small Instance
serviceofferingid = b85e77fd-b897-4471-8fce-1190e71e5156
serviceofferingname = Small Instance
size = 5368709120
state = Ready
storage = rbd1
storageid = f0dbafaa-52a3-3077-bc53-d7ad3a5ac132
storagetype = shared
tags:
templatedisplaytext = Base CentOS 7
templateid = cf02d86b-145b-4777-999c-7f37b16c945b
templatename = Linux_CentOS_7-Minimal
type = ROOT
virtualmachineid = f2870d90-d294-474b-b7da-95bad01e6c09
vmdisplayname = VM-381
vmname = VM-f2870d90-d294-474b-b7da-95bad01e6c09
vmstate = Stopped
zoneid = f58958e8-d24f-4a52-9a9f-8ae11cf4a3b0
zonename = Zone1
(local) 🐵 > start virtualmachine id=f2870d90-d294-474b-b7da-95bad01e6c09
accountid = 6b00c3ed-fc3f-11e5-9789-000c29b79f06
cmd = org.apache.cloudstack.api.command.admin.vm.StartVMCmdByAdmin
created = 2016-04-16T13:47:04-0500
jobid = e3c76fd4-1543-4480-9aac-0d8a0ada8763
jobinstanceid = f2870d90-d294-474b-b7da-95bad01e6c09
jobinstancetype = VirtualMachine
jobprocstatus = 0
jobresult:
virtualmachine:
id = f2870d90-d294-474b-b7da-95bad01e6c09
name = VM-f2870d90-d294-474b-b7da-95bad01e6c09
account = admin-2177
affinitygroup:
cpunumber = 1
cpuspeed = 500
created = 2016-04-14T07:54:23-0500
diskofferingid = 5cd05daa-9e18-4b52-90a7-534098f09b1a
diskofferingname = Small
displayname = VM-381
displayvm = True
domain = 2177
domainid = cf23158e-d33c-40c2-b176-0db4a163a93f
guestosid = 586974da-fc3f-11e5-9789-000c29b79f06
haenable = True
hostid = bf77a9df-6216-48fe-8eac-54540622965f
hostname = njcloudhost.dev.ena.net
hypervisor = KVM
instancename = i-7-42-VM
isdynamicallyscalable = False
jobid = e3c76fd4-1543-4480-9aac-0d8a0ada8763
jobstatus = 0
memory = 256
nic:
id = d37450f4-be55-4842-8ad5-e2bdc08a12d4
broadcasturi = vxlan://2378
gateway = 10.0.0.1
ipaddress = 10.0.0.168
isdefault = True
isolationuri = vxlan://2378
macaddress = 02:00:3e:41:00:0a
netmask = 255.255.255.0
networkid = 0eb4004f-38ce-4b90-a508-4ef73e837949
networkname = network-2177
secondaryip:
traffictype = Guest
type = Isolated
ostypeid = 246
passwordenabled = False
rootdeviceid = 0
rootdevicetype = ROOT
securitygroup:
serviceofferingid = b85e77fd-b897-4471-8fce-1190e71e5156
serviceofferingname = Small Instance
state = Running
tags:
templatedisplaytext = Base CentOS 7
templateid = cf02d86b-145b-4777-999c-7f37b16c945b
templatename = Linux_CentOS_7-Minimal
userid = ec083c86-e849-4b9d-bdc4-db80181d565e
username = computeenaadmin@ena.com
zoneid = f58958e8-d24f-4a52-9a9f-8ae11cf4a3b0
zonename = Zone1
jobresultcode = 0
jobresulttype = object
jobstatus = 1
userid = 6b00cc09-fc3f-11e5-9789-000c29b79f06`
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-9349: Enable root disk detach ...
Posted by ustcweizhou <gi...@git.apache.org>.
Github user ustcweizhou commented on the pull request:
https://github.com/apache/cloudstack/pull/1500#issuecomment-215408560
is there any UI change possible?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-9349: Enable root disk detach ...
Posted by swill <gi...@git.apache.org>.
Github user swill commented on the pull request:
https://github.com/apache/cloudstack/pull/1500#issuecomment-212533078
We need some LGTM code reviews of this one. I would also like to run CI against it because it changes code that is common to other functionality to make sure nothing else gets broken from this change.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-9349: Enable root disk detach ...
Posted by dmabry <gi...@git.apache.org>.
Github user dmabry commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1500#discussion_r60222679
--- Diff: test/integration/component/test_volumes.py ---
@@ -603,7 +603,134 @@ def test_01_volume_attach_detach(self):
"Check the state of VM"
)
except Exception as e:
- self.fail("Exception occuered: %s" % e)
+ self.fail("Exception occurred: %s" % e)
+ return
+
+ @attr(tags=["advanced", "advancedns"])
+ def test_02_root_volume_attach_detach(self):
+ """Test Root Volume attach/detach to VM
+ """
+
+ # Validate the following
+ # 1. Deploy a VM
+ # 2. Check for root volume
+ # 3. Stop VM
+ # 4. Detach root volume
+ # 5. Verify root volume detached
+ # 6. Attach root volume
+ # 7. Start VM
+
+ try:
+ # Check for root volume
+ root_volume_response = Volume.list(
+ self.apiclient,
+ virtualmachineid=self.virtual_machine.id,
+ type='ROOT',
+ listall=True
+ )
+ self.assertNotEqual(
+ root_volume_response,
+ None,
+ "Check if root volume exists in ListVolumes"
+ )
+ self.assertEqual(
+ isinstance(root_volume_response, list),
+ True,
+ "Check list volumes response for valid list"
+ )
+ # Grab the root volume for later use
+ root_volume = root_volume_response[0]
+
+ # Stop VM
+ self.debug("Stopping the VM: %s" % self.virtual_machine.id)
+ self.virtual_machine.stop(self.apiclient)
+
+ # Ensure VM is stopped before detaching the root volume
+ time.sleep(self.services["sleep"])
+
+ vm_response = VirtualMachine.list(
+ self.apiclient,
+ id=self.virtual_machine.id,
+ )
+ vm = vm_response[0]
+ self.assertEqual(
+ vm.state,
+ 'Stopped',
+ "Check the state of VM"
+ )
+
+ # Detach root volume from VM
+ self.virtual_machine.detach_volume(
+ self.apiclient,
+ root_volume
+ )
+
+ # Verify that root disk is gone
+ no_root_volume_response = Volume.list(
+ self.apiclient,
+ virtualmachineid=self.virtual_machine.id,
+ type='ROOT',
+ listall=True
+ )
+ self.assertEqual(
+ no_root_volume_response,
+ None,
+ "Check if root volume exists in ListVolumes"
+ )
+
+ # Attach root volume to VM
+ self.virtual_machine.attach_volume(
+ self.apiclient,
+ root_volume,
+ 0
+ )
+
+ # Check for root volume
+ new_root_volume_response = Volume.list(
+ self.apiclient,
+ virtualmachineid=self.virtual_machine.id,
+ type='ROOT',
+ listall=True
+ )
+ self.assertNotEqual(
+ new_root_volume_response,
+ None,
+ "Check if root volume exists in ListVolumes"
+ )
+ self.assertEqual(
+ isinstance(new_root_volume_response, list),
+ True,
+ "Check list volumes response for valid list"
+ )
+
+ # Start VM
+ self.virtual_machine.start(self.apiclient)
+ # Sleep to ensure that VM is in ready state
+ time.sleep(self.services["sleep"])
+
+ vm_response = VirtualMachine.list(
+ self.apiclient,
+ id=self.virtual_machine.id,
+ )
+ # Verify VM response to check whether VM deployment was successful
+ self.assertEqual(
+ isinstance(vm_response, list),
+ True,
+ "Check list VM response for valid list"
+ )
+ self.assertNotEqual(
+ len(vm_response),
+ 0,
+ "Check VMs available in List VMs response"
+ )
--- End diff --
@shwetaag Agreed. I thought this was redundant as well, but I was merely following the pattern that I saw previously established earlier in test_volumes.py. I assumed, perhaps incorrectly, that there was a reason for testing this twice. It didn't make sense to me that a VM would be an instance, the len of the array returned would be zero. I will remove the array length assertion.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-9349: Enable root disk detach ...
Posted by dmabry <gi...@git.apache.org>.
Github user dmabry commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1500#discussion_r60222132
--- Diff: test/integration/component/test_volumes.py ---
@@ -603,7 +603,134 @@ def test_01_volume_attach_detach(self):
"Check the state of VM"
)
except Exception as e:
- self.fail("Exception occuered: %s" % e)
+ self.fail("Exception occurred: %s" % e)
+ return
+
+ @attr(tags=["advanced", "advancedns"])
+ def test_02_root_volume_attach_detach(self):
+ """Test Root Volume attach/detach to VM
+ """
+
+ # Validate the following
+ # 1. Deploy a VM
+ # 2. Check for root volume
+ # 3. Stop VM
+ # 4. Detach root volume
+ # 5. Verify root volume detached
+ # 6. Attach root volume
+ # 7. Start VM
+
+ try:
+ # Check for root volume
--- End diff --
@shwetaag Thanks for the feedback. You are absolutely right, I should test to ensure the hypervisor supports Root Volume detach. Root Volume detach is currently supported by a number of hypervisors, so I will put in an if block that will make sure that it only tests the supported hypervisors.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-9349: Enable root disk detach ...
Posted by dmabry <gi...@git.apache.org>.
Github user dmabry commented on the pull request:
https://github.com/apache/cloudstack/pull/1500#issuecomment-215083560
Ok, I have run this through my lab again and specifically ran the test_volumes.py file. All tests passed. See below. I have also attached the results dir as a zip file for review.
Test Volume attach/detach to VM (5 data volumes) ... === TestName: test_01_volume_attach_detach | Status : SUCCESS ===
ok
Test Root Volume attach/detach to VM ... === TestName: test_02_root_volume_attach_detach | Status : SUCCESS ===
ok
Test Attach volumes (max capacity) ... === TestName: test_01_volume_attach | Status : SUCCESS ===
ok
Test attach volumes (more than max) to an instance ... === TestName: test_02_volume_attach_max | Status : SUCCESS ===
ok
Test custom disk sizes beyond range ... === TestName: test_deployVmWithCustomDisk | Status : SUCCESS ===
ok
Attach a created Volume to a Running VM ... === TestName: test_01_attach_volume | Status : SUCCESS ===
ok
Detach a Volume attached to a VM ... === TestName: test_02_detach_volume | Status : SUCCESS ===
ok
Delete a Volume unattached to an VM ... === TestName: test_03_delete_detached_volume | Status : SUCCESS ===
ok
Create a volume under a non-root domain as non-root-domain user ... === TestName: test_create_volume_under_domain | Status : SUCCESS ===
ok
----------------------------------------------------------------------
Ran 9 tests in 539.126s
OK
@swill - Looking at the standard bubble test suite that is run, it doesn't even run this test, so I thought it prudent in this case to run it specifically and post the results. Once we get the bubble stuff standardized, I'm happy to run the full test and post those results. Until then, I would like to submit this PR for code review again.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-9349: Enable root disk detach ...
Posted by dmabry <gi...@git.apache.org>.
Github user dmabry commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1500#discussion_r60582709
--- Diff: test/integration/component/test_volumes.py ---
@@ -603,7 +603,144 @@ def test_01_volume_attach_detach(self):
"Check the state of VM"
)
except Exception as e:
- self.fail("Exception occuered: %s" % e)
+ self.fail("Exception occurred: %s" % e)
+ return
+
+ @attr(tags=["advanced", "advancedns"], required_hardware="false")
+ def test_02_root_volume_attach_detach(self):
+ """Test Root Volume attach/detach to VM
+ """
+
+ # Validate the following
+ # 1. Deploy a VM
+ # 2. Verify that we are testing a supported hypervisor
+ # 3. Check for root volume
+ # 4. Stop VM
+ # 5. Detach root volume
+ # 6. Verify root volume detached
+ # 7. Attach root volume
+ # 8. Start VM
+
+ # Verify we are using a supported hypervisor
+ if (self.hypervisor.lower() == 'vmware'
+ or self.hypervisor.lower() == 'kvm'
+ or self.hypervisor.lower() == 'simulator'
+ or self.hypervisor.lower() == 'xenserver'):
+
+ try:
+ # Check for root volume
+ root_volume_response = Volume.list(
+ self.apiclient,
+ virtualmachineid=self.virtual_machine.id,
+ type='ROOT',
+ listall=True
+ )
+ self.assertNotEqual(
+ root_volume_response,
+ None,
+ "Check if root volume exists in ListVolumes"
+ )
+ self.assertEqual(
+ isinstance(root_volume_response, list),
+ True,
+ "Check list volumes response for valid list"
+ )
+ # Grab the root volume for later use
+ root_volume = root_volume_response[0]
+
+ # Stop VM
+ self.debug("Stopping the VM: %s" % self.virtual_machine.id)
+ self.virtual_machine.stop(self.apiclient)
+
+ # Ensure VM is stopped before detaching the root volume
+ time.sleep(self.services["sleep"])
+
+ vm_response = VirtualMachine.list(
+ self.apiclient,
+ id=self.virtual_machine.id,
+ )
+ vm = vm_response[0]
+ self.assertEqual(
+ vm.state,
+ 'Stopped',
+ "Check the state of VM"
+ )
+
+ # Detach root volume from VM
+ self.virtual_machine.detach_volume(
+ self.apiclient,
+ root_volume
+ )
+
+ # Verify that root disk is gone
+ no_root_volume_response = Volume.list(
+ self.apiclient,
+ virtualmachineid=self.virtual_machine.id,
+ type='ROOT',
+ listall=True
+ )
+ self.assertEqual(
+ no_root_volume_response,
+ None,
+ "Check if root volume exists in ListVolumes"
+ )
+
+ # Attach root volume to VM
+ self.virtual_machine.attach_volume(
+ self.apiclient,
+ root_volume,
+ 0
+ )
+
+ # Check for root volume
+ new_root_volume_response = Volume.list(
+ self.apiclient,
+ virtualmachineid=self.virtual_machine.id,
+ type='ROOT',
+ listall=True
+ )
+ self.assertNotEqual(
+ new_root_volume_response,
+ None,
+ "Check if root volume exists in ListVolumes"
+ )
+ self.assertEqual(
+ isinstance(new_root_volume_response, list),
+ True,
+ "Check list volumes response for valid list"
+ )
+
+ # Start VM
+ self.virtual_machine.start(self.apiclient)
+ # Sleep to ensure that VM is in ready state
+ time.sleep(self.services["sleep"])
+
+ vm_response = VirtualMachine.list(
+ self.apiclient,
+ id=self.virtual_machine.id,
+ )
+ # Verify VM response to check whether VM deployment was successful
+ self.assertEqual(
+ isinstance(vm_response, list),
+ True,
+ "Check list VM response for valid list"
+ )
+ self.assertNotEqual(
+ len(vm_response),
+ 0,
+ "Check VMs available in List VMs response"
+ )
+ vm = vm_response[0]
+ self.assertEqual(
+ vm.state,
+ 'Running',
+ "Check the state of VM"
+ )
+ except Exception as e:
+ self.fail("Exception occurred: %s" % e)
+
+ else:
+ self.skipTest("Root Volume attach/detach is not supported on %s " % self.hypervisor)
return
--- End diff --
Please see my earlier comment. I'm happy to make the change, but I also want to follow earlier established pattern for consistency.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---