From ffcb17678c7e5409a1f12a09945b18e8879a677d Mon Sep 17 00:00:00 2001
From: Gary Kotton <gkotton@vmware.com>
Date: Thu, 13 Mar 2014 06:53:58 -0700
Subject: [PATCH] VMware: ensure rescue instance is deleted when instance is
 deleted

If the user creates a rescue instance and then proceeded to delete
the original instance then the rescue instance would still be up
and running on the backend.

This patch ensures that the rescue instance is cleaned up if
necessary.

The vmops unrescue method has a new parameter indicating if
the original VM should be powered on.

Closes-bug: 1269418
(cherry picked from commit efb66531bc37ee416778a70d46c657608ca767af)

Conflicts:

	nova/virt/vmwareapi/vmops.py

Change-Id: I3c1d0b1d003392b306094b80ea1ac99377441fbf
---
 nova/tests/virt/vmwareapi/test_driver_api.py | 26 +++++++++++++
 nova/virt/vmwareapi/vmops.py                 | 55 ++++++++++++++++++++--------
 2 files changed, 65 insertions(+), 16 deletions(-)

diff --git a/nova/tests/virt/vmwareapi/test_driver_api.py b/nova/tests/virt/vmwareapi/test_driver_api.py
index c1481aa..63f0c59 100644
--- a/nova/tests/virt/vmwareapi/test_driver_api.py
+++ b/nova/tests/virt/vmwareapi/test_driver_api.py
@@ -34,6 +34,7 @@
 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.openstack.common import jsonutils
@@ -1191,6 +1192,31 @@ def test_get_info(self):
                                    'node': self.instance_node})
         self._check_vm_info(info, power_state.RUNNING)
 
+    def destroy_rescued(self, fake_method):
+        self._rescue()
+        with (
+            mock.patch.object(self.conn._volumeops, "detach_disk_from_vm",
+                              fake_method)
+        ):
+            self.instance['vm_state'] = vm_states.RESCUED
+            self.conn.destroy(self.context, self.instance, self.network_info)
+            inst_path = '[%s] %s/%s.vmdk' % (self.ds, self.uuid, self.uuid)
+            self.assertFalse(vmwareapi_fake.get_file(inst_path))
+            rescue_file_path = '[%s] %s-rescue/%s-rescue.vmdk' % (self.ds,
+                                                                  self.uuid,
+                                                                  self.uuid)
+            self.assertFalse(vmwareapi_fake.get_file(rescue_file_path))
+
+    def test_destroy_rescued(self):
+        def fake_detach_disk_from_vm(*args, **kwargs):
+            pass
+        self.destroy_rescued(fake_detach_disk_from_vm)
+
+    def test_destroy_rescued_with_exception(self):
+        def fake_detach_disk_from_vm(*args, **kwargs):
+            raise exception.NovaException('Here is my fake exception')
+        self.destroy_rescued(fake_detach_disk_from_vm)
+
     def test_destroy(self):
         self._create_vm()
         info = self.conn.get_info({'uuid': self.uuid,
diff --git a/nova/virt/vmwareapi/vmops.py b/nova/virt/vmwareapi/vmops.py
index 30f8373..831da48 100644
--- a/nova/virt/vmwareapi/vmops.py
+++ b/nova/virt/vmwareapi/vmops.py
@@ -29,6 +29,7 @@
 from nova import compute
 from nova.compute import power_state
 from nova.compute import task_states
+from nova.compute import vm_states
 from nova import context as nova_context
 from nova import exception
 from nova.openstack.common import excutils
@@ -985,13 +986,9 @@ def _delete(self, instance, network_info):
         except Exception as exc:
             LOG.exception(exc, instance=instance)
 
-    def destroy(self, instance, network_info, destroy_disks=True,
-                instance_name=None):
-        """Destroy a VM instance. Steps followed are:
-        1. Power off the VM, if it is in poweredOn state.
-        2. Un-register a VM.
-        3. Delete the contents of the folder holding the VM related data.
-        """
+    def _destroy_instance(self, instance, network_info, destroy_disks=True,
+                          instance_name=None):
+        # Destroy a VM instance
         # Get the instance name. In some cases this may differ from the 'uuid',
         # for example when the spawn of a rescue instance takes place.
         if not instance_name:
@@ -1029,8 +1026,9 @@ def destroy(self, instance, network_info, destroy_disks=True,
                                            "UnregisterVM", vm_ref)
                 LOG.debug(_("Unregistered the VM"), instance=instance)
             except Exception as excep:
-                LOG.warn(_("In vmwareapi:vmops:destroy, got this exception"
-                           " while un-registering the VM: %s") % str(excep))
+                LOG.warn(_("In vmwareapi:vmops:_destroy_instance, got this "
+                           "exception while un-registering the VM: %s"),
+                         excep)
             # Delete the folder holding the VM related content on
             # the datastore.
             if destroy_disks and datastore_name:
@@ -1053,15 +1051,39 @@ def destroy(self, instance, network_info, destroy_disks=True,
                                {'datastore_name': datastore_name},
                               instance=instance)
                 except Exception as excep:
-                    LOG.warn(_("In vmwareapi:vmops:destroy, "
-                                 "got this exception while deleting"
-                                 " the VM contents from the disk: %s")
-                                 % str(excep))
+                    LOG.warn(_("In vmwareapi:vmops:_destroy_instance, "
+                                "got this exception while deleting "
+                                "the VM contents from the disk: %s"),
+                             excep)
         except Exception as exc:
             LOG.exception(exc, instance=instance)
         finally:
             vm_util.vm_ref_cache_delete(instance_name)
 
+    def destroy(self, instance, network_info, destroy_disks=True):
+        """Destroy a VM instance.
+
+        Steps followed for each VM are:
+        1. Power off, if it is in poweredOn state.
+        2. Un-register.
+        3. Delete the contents of the folder holding the VM related data.
+        """
+        # If there is a rescue VM then we need to destroy that one too.
+        LOG.debug(_("Destroying instance"), instance=instance)
+        if instance['vm_state'] == vm_states.RESCUED:
+            LOG.debug(_("Rescue VM configured"), instance=instance)
+            try:
+                self.unrescue(instance, power_on=False)
+                LOG.debug(_("Rescue VM destroyed"), instance=instance)
+            except Exception:
+                rescue_name = instance['uuid'] + self._rescue_suffix
+                self._destroy_instance(instance, network_info,
+                                       destroy_disks=destroy_disks,
+                                       instance_name=rescue_name)
+        self._destroy_instance(instance, network_info,
+                               destroy_disks=destroy_disks)
+        LOG.debug(_("Instance destroyed"), instance=instance)
+
     def pause(self, instance):
         msg = _("pause not supported for vmwareapi")
         raise NotImplementedError(msg)
@@ -1139,7 +1161,7 @@ def rescue(self, context, instance, network_info, image_meta):
                                 adapter_type, disk_type, vmdk_path)
         self._power_on(instance, vm_ref=rescue_vm_ref)
 
-    def unrescue(self, instance):
+    def unrescue(self, instance, power_on=True):
         """Unrescue the specified instance."""
         # Get the original vmdk_path
         vm_ref = vm_util.get_vm_ref(self._session, instance)
@@ -1161,8 +1183,9 @@ def unrescue(self, instance):
         device = vm_util.get_vmdk_volume_disk(hardware_devices, path=vmdk_path)
         self._power_off_vm_ref(vm_rescue_ref)
         self._volumeops.detach_disk_from_vm(vm_rescue_ref, r_instance, device)
-        self.destroy(r_instance, None, instance_name=instance_name)
-        self._power_on(instance)
+        self._destroy_instance(r_instance, None, instance_name=instance_name)
+        if power_on:
+            self._power_on(instance)
 
     def _power_off_vm_ref(self, vm_ref):
         """Power off the specifed vm.
-- 
1.9.3

