VMware: detach cinder volume when instance destroyed

Ensure that the cinder volume is detached when the instance is destroyed.
When an instance has an attached cinder volume we need to ensure that
the volume is detached prior to deleting the VM on the backend.

Closes-bug: #1439168

Change-Id: I95dfbec3036270c07c7edbda35574fee5f029ddb

3 files changed, 78 insertions, 4 deletions

diff --git a/nova/tests/unit/virt/vmwareapi/test_driver_api.py b/nova/tests/unit/virt/vmwareapi/test_driver_api.py
index 3c4e4dcf45..6a16d05e9a 100644
--- a/nova/tests/unit/virt/vmwareapi/test_driver_api.py
+++ b/nova/tests/unit/virt/vmwareapi/test_driver_api.py
@@ -39,6 +39,7 @@ from nova import block_device
 from nova.compute import api as compute_api
 from nova.compute import power_state
 from nova.compute import task_states
+from nova.compute import vm_states
 from nova import context
 from nova import exception
 from nova.image import glance
@@ -662,6 +663,49 @@ class VMwareAPIVMTestCase(test.NoDBTestCase):
         self._create_vm()
         self.assertTrue(self.cd_attach_called)
 
+    @mock.patch.object(vmops.VMwareVMOps, 'power_off')
+    @mock.patch.object(driver.VMwareVCDriver, 'detach_volume')
+    @mock.patch.object(vmops.VMwareVMOps, 'destroy')
+    def test_destroy_with_attached_volumes(self,
+                                           mock_destroy,
+                                           mock_detach_volume,
+                                           mock_power_off):
+        self._create_vm()
+        connection_info = {'data': 'fake-data', 'serial': 'volume-fake-id'}
+        bdm = [{'connection_info': connection_info,
+                'disk_bus': 'fake-bus',
+                'device_name': 'fake-name',
+                'mount_device': '/dev/sdb'}]
+        bdi = {'block_device_mapping': bdm, 'root_device_name': '/dev/sda'}
+        self.assertNotEqual(vm_states.STOPPED, self.instance.vm_state)
+        self.conn.destroy(self.context, self.instance, self.network_info,
+                          block_device_info=bdi)
+        mock_power_off.assert_called_once_with(self.instance)
+        self.assertEqual(vm_states.STOPPED, self.instance.vm_state)
+        mock_detach_volume.assert_called_once_with(
+            connection_info, self.instance, 'fake-name')
+        mock_destroy.assert_called_once_with(self.instance, True)
+
+    @mock.patch.object(driver.VMwareVCDriver, 'detach_volume',
+                       side_effect=exception.StorageError(reason='oh man'))
+    @mock.patch.object(vmops.VMwareVMOps, 'destroy')
+    def test_destroy_with_attached_volumes_with_exception(self,
+                                                          mock_destroy,
+                                                          mock_detach_volume):
+        self._create_vm()
+        connection_info = {'data': 'fake-data', 'serial': 'volume-fake-id'}
+        bdm = [{'connection_info': connection_info,
+                'disk_bus': 'fake-bus',
+                'device_name': 'fake-name',
+                'mount_device': '/dev/sdb'}]
+        bdi = {'block_device_mapping': bdm, 'root_device_name': '/dev/sda'}
+        self.assertRaises(exception.StorageError,
+                          self.conn.destroy, self.context, self.instance,
+                          self.network_info, block_device_info=bdi)
+        mock_detach_volume.assert_called_once_with(
+            connection_info, self.instance, 'fake-name')
+        self.assertFalse(mock_destroy.called)
+
     def test_spawn(self):
         self._create_vm()
         info = self._get_info()
diff --git a/nova/virt/vmwareapi/driver.py b/nova/virt/vmwareapi/driver.py
index ed387d6b9a..09f71abfaa 100644
--- a/nova/virt/vmwareapi/driver.py
+++ b/nova/virt/vmwareapi/driver.py
@@ -25,6 +25,7 @@ from oslo_config import cfg
 from oslo_log import log as logging
 from oslo_log import versionutils
 from oslo_serialization import jsonutils
+from oslo_utils import excutils
 from oslo_vmware import api
 from oslo_vmware import exceptions as vexc
 from oslo_vmware import pbm
@@ -32,9 +33,11 @@ from oslo_vmware import vim
 from oslo_vmware import vim_util
 import six
 
+from nova.compute import task_states
+from nova.compute import vm_states
 from nova import exception
 from nova import utils
-from nova.i18n import _, _LI, _LW
+from nova.i18n import _, _LI, _LE, _LW
 from nova.virt import driver
 from nova.virt.vmwareapi import constants
 from nova.virt.vmwareapi import error_util
@@ -578,6 +581,36 @@ class VMwareVCDriver(driver.ComputeDriver):
         if not instance.node:
             return
 
+        # A resize uses the same instance on the VC. We do not delete that
+        # VM in the event of a revert
+        if instance.task_state == task_states.RESIZE_REVERTING:
+            return
+
+        # We need to detach attached volumes
+        if block_device_info is not None:
+            block_device_mapping = driver.block_device_info_get_mapping(
+                block_device_info)
+            if block_device_mapping:
+                # Certain disk types, for example 'IDE' do not support hot
+                # plugging. Hence we need to power off the instance and update
+                # the instance state.
+                self._vmops.power_off(instance)
+                # TODO(garyk): update the volumeops to read the state form the
+                # VM instead of relying on a instance flag
+                instance.vm_state = vm_states.STOPPED
+                for disk in block_device_mapping:
+                    connection_info = disk['connection_info']
+                    try:
+                        self.detach_volume(connection_info, instance,
+                                           disk.get('device_name'))
+                    except Exception as e:
+                        with excutils.save_and_reraise_exception():
+                            LOG.error(_LE("Failed to detach %(device_name)s. "
+                                          "Exception: %(exc)s"),
+                                      {'device_name': disk.get('device_name'),
+                                       'exc': e},
+                                      instance=instance)
+
         self._vmops.destroy(instance, destroy_disks)
 
     def pause(self, instance):
diff --git a/nova/virt/vmwareapi/vmops.py b/nova/virt/vmwareapi/vmops.py
index 7fb1f61d49..18b6847d01 100644
--- a/nova/virt/vmwareapi/vmops.py
+++ b/nova/virt/vmwareapi/vmops.py
@@ -979,9 +979,6 @@ class VMwareVMOps(object):
         2. Un-register.
         3. Delete the contents of the folder holding the VM related data.
         """
-        if instance.task_state == task_states.RESIZE_REVERTING:
-            return
-
         LOG.debug("Destroying instance", instance=instance)
         self._destroy_instance(instance, destroy_disks=destroy_disks)
         LOG.debug("Instance destroyed", instance=instance)