[GitHub] cloudstack pull request: CLOUDSTACK-9349: Enable root disk detach ...

2016-04-28 Thread asfgit
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] cloudstack pull request: CLOUDSTACK-9349: Enable root disk detach ...

2016-04-28 Thread kiwiflyer
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] cloudstack pull request: CLOUDSTACK-9349: Enable root disk detach ...

2016-04-28 Thread ustcweizhou
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] cloudstack pull request: CLOUDSTACK-9349: Enable root disk detach ...

2016-04-28 Thread kiwiflyer
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] cloudstack pull request: CLOUDSTACK-9349: Enable root disk detach ...

2016-04-28 Thread koushik-das
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] cloudstack pull request: CLOUDSTACK-9349: Enable root disk detach ...

2016-04-27 Thread nitt10prashant
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] cloudstack pull request: CLOUDSTACK-9349: Enable root disk detach ...

2016-04-27 Thread dmabry
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] cloudstack pull request: CLOUDSTACK-9349: Enable root disk detach ...

2016-04-27 Thread swill
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] cloudstack pull request: CLOUDSTACK-9349: Enable root disk detach ...

2016-04-27 Thread dmabry
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] cloudstack pull request: CLOUDSTACK-9349: Enable root disk detach ...

2016-04-27 Thread swill
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] cloudstack pull request: CLOUDSTACK-9349: Enable root disk detach ...

2016-04-27 Thread kiwiflyer
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] cloudstack pull request: CLOUDSTACK-9349: Enable root disk detach ...

2016-04-27 Thread swill
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] cloudstack pull request: CLOUDSTACK-9349: Enable root disk detach ...

2016-04-27 Thread dmabry
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] cloudstack pull request: CLOUDSTACK-9349: Enable root disk detach ...

2016-04-27 Thread dmabry
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] cloudstack pull request: CLOUDSTACK-9349: Enable root disk detach ...

2016-04-23 Thread dmabry
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] cloudstack pull request: CLOUDSTACK-9349: Enable root disk detach ...

2016-04-21 Thread dmabry
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)
+# 

[GitHub] cloudstack pull request: CLOUDSTACK-9349: Enable root disk detach ...

2016-04-21 Thread dmabry
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] cloudstack pull request: CLOUDSTACK-9349: Enable root disk detach ...

2016-04-21 Thread dmabry
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] cloudstack pull request: CLOUDSTACK-9349: Enable root disk detach ...

2016-04-21 Thread dmabry
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)
+# 

[GitHub] cloudstack pull request: CLOUDSTACK-9349: Enable root disk detach ...

2016-04-21 Thread dmabry
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)
+# 

[GitHub] cloudstack pull request: CLOUDSTACK-9349: Enable root disk detach ...

2016-04-21 Thread dmabry
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)
+# 

[GitHub] cloudstack pull request: CLOUDSTACK-9349: Enable root disk detach ...

2016-04-21 Thread jburwell
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)
+

[GitHub] cloudstack pull request: CLOUDSTACK-9349: Enable root disk detach ...

2016-04-21 Thread dmabry
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] cloudstack pull request: CLOUDSTACK-9349: Enable root disk detach ...

2016-04-21 Thread dmabry
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] cloudstack pull request: CLOUDSTACK-9349: Enable root disk detach ...

2016-04-21 Thread nitt10prashant
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)
+   

[GitHub] cloudstack pull request: CLOUDSTACK-9349: Enable root disk detach ...

2016-04-21 Thread nitt10prashant
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] cloudstack pull request: CLOUDSTACK-9349: Enable root disk detach ...

2016-04-21 Thread nitt10prashant
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)
+   

[GitHub] cloudstack pull request: CLOUDSTACK-9349: Enable root disk detach ...

2016-04-21 Thread nitt10prashant
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] cloudstack pull request: CLOUDSTACK-9349: Enable root disk detach ...

2016-04-21 Thread nitt10prashant
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] cloudstack pull request: CLOUDSTACK-9349: Enable root disk detach ...

2016-04-20 Thread swill
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] cloudstack pull request: CLOUDSTACK-9349: Enable root disk detach ...

2016-04-20 Thread dmabry
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] cloudstack pull request: CLOUDSTACK-9349: Enable root disk detach ...

2016-04-20 Thread kiwiflyer
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] cloudstack pull request: CLOUDSTACK-9349: Enable root disk detach ...

2016-04-19 Thread koushik-das
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] cloudstack pull request: CLOUDSTACK-9349: Enable root disk detach ...

2016-04-19 Thread koushik-das
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] cloudstack pull request: CLOUDSTACK-9349: Enable root disk detach ...

2016-04-19 Thread dmabry
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] cloudstack pull request: CLOUDSTACK-9349: Enable root disk detach ...

2016-04-19 Thread dmabry
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 --


[GitHub] cloudstack pull request: CLOUDSTACK-9349: Enable root disk detach ...

2016-04-19 Thread dmabry
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 --


[GitHub] cloudstack pull request: CLOUDSTACK-9349: Enable root disk detach ...

2016-04-19 Thread dmabry
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] cloudstack pull request: CLOUDSTACK-9349: Enable root disk detach ...

2016-04-19 Thread shwetaag
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 --

   

[GitHub] cloudstack pull request: CLOUDSTACK-9349: Enable root disk detach ...

2016-04-19 Thread shwetaag
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] cloudstack pull request: CLOUDSTACK-9349: Enable root disk detach ...

2016-04-18 Thread swill
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---