Merge "VMware: Detach volume should not delete vmdk"

nova/tests/virt/vmwareapi/test_vmwareapi.py

@@ -953,7 +953,7 @@ class VMwareAPIVMTestCase(test.NoDBTestCase):
         self.mox.StubOutWithMock(volumeops.VMwareVolumeOps,
                                  'detach_disk_from_vm')
         volumeops.VMwareVolumeOps.detach_disk_from_vm(mox.IgnoreArg(),
-                self.instance, device)
+                self.instance, device, destroy_disk=True)
         self.mox.ReplayAll()
         self.conn.detach_volume(connection_info, self.instance, mount_point,
                                 encryption=None)

nova/tests/virt/vmwareapi/test_vmwareapi_vm_util.py

@@ -377,3 +377,23 @@ class VMwareVMUtilTestCase(test.NoDBTestCase):
         self.assertTrue(hasattr(propdict['some.thing'], 'value'))
         self.assertEqual("else", propdict['some.thing'].value)
         self.assertEqual("value", propdict['another.thing'])
+
+    def _test_detach_virtual_disk_spec(self, destroy_disk=False):
+        virtual_device_config = vm_util.detach_virtual_disk_spec(
+                                                     fake.FakeFactory(),
+                                                     'fake_device',
+                                                     destroy_disk)
+        self.assertEqual('remove', virtual_device_config.operation)
+        self.assertEqual('fake_device', virtual_device_config.device)
+        self.assertEqual('ns0:VirtualDeviceConfigSpec',
+                         virtual_device_config.obj_name)
+        if destroy_disk:
+            self.assertEqual('destroy', virtual_device_config.fileOperation)
+        else:
+            self.assertFalse(hasattr(virtual_device_config, 'fileOperation'))
+
+    def test_detach_virtual_disk_spec(self):
+        self._test_detach_virtual_disk_spec(destroy_disk=False)
+
+    def test_detach_virtual_disk_destroy_spec(self):
+        self._test_detach_virtual_disk_spec(destroy_disk=True)

nova/tests/virt/vmwareapi/test_vmwareapi_volumeops.py

@@ -0,0 +1,70 @@
+# vim: tabstop=4 shiftwidth=4 softtabstop=43
+
+#    Copyright 2013 IBM Corp.
+#
+#    Licensed under the Apache License, Version 2.0 (the "License"); you may
+#    not use this file except in compliance with the License. You may obtain
+#    a copy of the License at
+#
+#         http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#    License for the specific language governing permissions and limitations
+#    under the License.
+
+import contextlib
+import mock
+
+from nova import test
+from nova.tests.virt.vmwareapi import stubs
+from nova.virt.vmwareapi import driver
+from nova.virt.vmwareapi import fake as vmwareapi_fake
+from nova.virt.vmwareapi import volumeops
+
+
+class VMwareVolumeOpsTestCase(test.NoDBTestCase):
+
+    def setUp(self):
+        super(VMwareVolumeOpsTestCase, self).setUp()
+        vmwareapi_fake.reset()
+        stubs.set_stubs(self.stubs)
+
+        self._session = driver.VMwareAPISession()
+        self._volumeops = volumeops.VMwareVolumeOps(self._session)
+        self.instance = {'name': 'fake_name', 'uuid': 'fake_uuid'}
+
+    def _test_detach_disk_from_vm(self, destroy_disk=False):
+        def fake_call_method(module, method, *args, **kwargs):
+            vmdk_detach_config_spec = kwargs.get('spec')
+            virtual_device_config = vmdk_detach_config_spec.deviceChange[0]
+            self.assertEqual('remove', virtual_device_config.operation)
+            self.assertEqual('ns0:VirtualDeviceConfigSpec',
+                              virtual_device_config.obj_name)
+            if destroy_disk:
+                self.assertEqual('destroy',
+                                 virtual_device_config.fileOperation)
+            else:
+                self.assertFalse(hasattr(virtual_device_config,
+                                 'fileOperation'))
+            return 'fake_configure_task'
+        with contextlib.nested(
+            mock.patch.object(self._session, '_wait_for_task'),
+            mock.patch.object(self._session, '_call_method',
+                              fake_call_method)
+        ) as (_wait_for_task, _call_method):
+            fake_device = vmwareapi_fake.DataObject()
+            fake_device.backing = vmwareapi_fake.DataObject()
+            fake_device.backing.fileName = 'fake_path'
+            fake_device.key = 'fake_key'
+            self._volumeops.detach_disk_from_vm('fake_vm_ref', self.instance,
+                                                fake_device, destroy_disk)
+            _wait_for_task.assert_has_calls([
+                   mock.call(self.instance['uuid'], 'fake_configure_task')])
+
+    def test_detach_with_destroy_disk_from_vm(self):
+        self._test_detach_disk_from_vm(destroy_disk=True)
+
+    def test_detach_without_destroy_disk_from_vm(self):
+        self._test_detach_disk_from_vm(destroy_disk=False)

nova/virt/vmwareapi/vm_util.py

@@ -264,13 +264,15 @@ def get_cdrom_attach_config_spec(client_factory,
     return config_spec
 
 
-def get_vmdk_detach_config_spec(client_factory, device):
+def get_vmdk_detach_config_spec(client_factory, device,
+                                destroy_disk=False):
     """Builds the vmdk detach config spec."""
     config_spec = client_factory.create('ns0:VirtualMachineConfigSpec')
 
     device_config_spec = []
-    virtual_device_config_spec = delete_virtual_disk_spec(client_factory,
-                                                          device)
+    virtual_device_config_spec = detach_virtual_disk_spec(client_factory,
+                                                          device,
+                                                          destroy_disk)
 
     device_config_spec.append(virtual_device_config_spec)
 
@@ -470,14 +472,15 @@ def create_virtual_disk_spec(client_factory, controller_key,
     return virtual_device_config
 
 
-def delete_virtual_disk_spec(client_factory, device):
+def detach_virtual_disk_spec(client_factory, device, destroy_disk=False):
     """
-    Builds spec for the deletion of an already existing Virtual Disk from VM.
+    Builds spec for the detach of an already existing Virtual Disk from VM.
     """
     virtual_device_config = client_factory.create(
                             'ns0:VirtualDeviceConfigSpec')
     virtual_device_config.operation = "remove"
-    virtual_device_config.fileOperation = "destroy"
+    if destroy_disk:
+        virtual_device_config.fileOperation = "destroy"
     virtual_device_config.device = device
 
     return virtual_device_config

nova/virt/vmwareapi/volumeops.py

@@ -111,7 +111,8 @@ class VMwareVolumeOps(object):
                 if option.key == volume_option:
                     return option.value
 
-    def detach_disk_from_vm(self, vm_ref, instance, device):
+    def detach_disk_from_vm(self, vm_ref, instance, device,
+                            destroy_disk=False):
         """
         Detach disk from VM by reconfiguration.
         """
@@ -119,7 +120,7 @@ class VMwareVolumeOps(object):
         instance_uuid = instance['uuid']
         client_factory = self._session._get_vim().client.factory
         vmdk_detach_config_spec = vm_util.get_vmdk_detach_config_spec(
-                                    client_factory, device)
+                                    client_factory, device, destroy_disk)
         disk_key = device.key
         LOG.debug(_("Reconfiguring VM instance %(instance_name)s to detach "
                     "disk %(disk_key)s"),
@@ -359,7 +360,8 @@ class VMwareVolumeOps(object):
         self._relocate_vmdk_volume(volume_ref, res_pool, datastore)
 
         # Delete the original disk from the volume_ref
-        self.detach_disk_from_vm(volume_ref, instance, original_device)
+        self.detach_disk_from_vm(volume_ref, instance, original_device,
+                                 destroy_disk=True)
         # Attach the current disk to the volume_ref
         # Get details required for adding disk device such as
         # adapter_type, unit_number, controller_key
@@ -433,7 +435,7 @@ class VMwareVolumeOps(object):
         device = vm_util.get_rdm_disk(hardware_devices, uuid)
         if device is None:
             raise volume_util.StorageError(_("Unable to find volume"))
-        self.detach_disk_from_vm(vm_ref, instance, device)
+        self.detach_disk_from_vm(vm_ref, instance, device, destroy_disk=True)
         LOG.info(_("Mountpoint %(mountpoint)s detached from "
                    "instance %(instance_name)s"),
                  {'mountpoint': mountpoint, 'instance_name': instance_name})