Merge "Cleanup migration files" into stable/pike

compute_hyperv/nova/migrationops.py

@@ -105,7 +105,8 @@ class MigrationOps(object):
         instance.save()
 
         self._vmops.destroy(instance, network_info,
-                            block_device_info, destroy_disks=True)
+                            block_device_info, destroy_disks=True,
+                            cleanup_migration_files=False)
 
         # return the instance's path location.
         return instance_path

compute_hyperv/nova/vmops.py

@@ -291,7 +291,7 @@ class VMOps(object):
             raise exception.InstanceExists(name=instance_name)
 
         # Make sure we're starting with a clean slate.
-        self._delete_disk_files(instance_name)
+        self._delete_disk_files(instance)
 
         vm_gen = self.get_image_vm_generation(instance.uuid, image_meta)
 
@@ -804,23 +804,32 @@ class VMOps(object):
                 self._pathutils.remove(configdrive_path)
 
     @serialconsoleops.instance_synchronized
-    def _delete_disk_files(self, instance_name, instance_path=None):
+    def _delete_disk_files(self, instance, instance_path=None,
+                           cleanup_migration_files=True):
         # We want to avoid the situation in which serial console workers
         # are started while we perform this operation, preventing us from
         # deleting the instance log files (bug #1556189). This can happen
         # due to delayed instance lifecycle events.
         #
         # The unsynchronized method is being used to avoid a deadlock.
-        self._serial_console_ops.stop_console_handler_unsync(instance_name)
+        self._serial_console_ops.stop_console_handler_unsync(instance.name)
 
         # This may be a 'non-default' location.
         if not instance_path:
-            instance_path = self._pathutils.get_instance_dir(instance_name)
+            instance_path = self._pathutils.get_instance_dir(instance.name)
 
         self._pathutils.check_remove_dir(instance_path)
 
+        if cleanup_migration_files:
+            self._pathutils.get_instance_migr_revert_dir(
+                instance_path, remove_dir=True)
+
+            backup_location = instance.system_metadata.get('backup_location')
+            if backup_location:
+                self._pathutils.check_remove_dir(backup_location)
+
     def destroy(self, instance, network_info, block_device_info,
-                destroy_disks=True):
+                destroy_disks=True, cleanup_migration_files=True):
         instance_name = instance.name
         LOG.info("Got request to destroy instance", instance=instance)
 
@@ -848,7 +857,8 @@ class VMOps(object):
             self._volumeops.disconnect_volumes(block_device_info)
 
             if destroy_disks:
-                self._delete_disk_files(instance_name, instance_path)
+                self._delete_disk_files(instance, instance_path,
+                                        cleanup_migration_files)
         except Exception:
             with excutils.save_and_reraise_exception():
                 LOG.exception('Failed to destroy instance: %s', instance_name)

compute_hyperv/tests/unit/test_migrationops.py

@@ -137,7 +137,8 @@ class MigrationOpsTestCase(test_base.HyperVBaseTestCase):
                          instance.system_metadata['backup_location'])
         instance.save.assert_called_once_with()
         self._migrationops._vmops.destroy.assert_called_once_with(
-            instance, network_info, mock.sentinel.bdi, destroy_disks=True)
+            instance, network_info, mock.sentinel.bdi, destroy_disks=True,
+            cleanup_migration_files=False)
 
     def test_confirm_migration(self):
         mock_instance = fake_instance.fake_instance_obj(

compute_hyperv/tests/unit/test_vmops.py

@@ -478,7 +478,7 @@ class VMOpsTestCase(test_base.HyperVBaseTestCase):
             self._vmops._vmutils.vm_exists.assert_called_once_with(
                 mock_instance.name)
             mock_delete_disk_files.assert_called_once_with(
-                mock_instance.name)
+                mock_instance)
             mock_validate_and_update_bdi = (
                 self._vmops._block_dev_man.validate_and_update_bdi)
             mock_validate_and_update_bdi.assert_called_once_with(
@@ -1213,11 +1213,17 @@ class VMOpsTestCase(test_base.HyperVBaseTestCase):
         self._vmops._pathutils.remove.assert_called_once_with(
             mock.sentinel.configdrive_path)
 
-    @ddt.data(None, mock.sentinel.instance_path)
+    @ddt.data({'passed_instance_path': True},
-    def test_delete_disk_files(self, passed_instance_path):
+              {'cleanup_migr_files': True})
-        mock_instance = fake_instance.fake_instance_obj(self.context)
+    @ddt.unpack
-        self._vmops._delete_disk_files(mock_instance.name,
+    def test_delete_disk_files(self, passed_instance_path=None,
-                                       passed_instance_path)
+                               cleanup_migr_files=False):
+        mock_instance = mock.Mock(
+            system_metadata=dict(
+                backup_location=mock.sentinel.backup_location))
+        self._vmops._delete_disk_files(mock_instance,
+                                       passed_instance_path,
+                                       cleanup_migr_files)
 
         stop_console_handler = (
             self._vmops._serial_console_ops.stop_console_handler_unsync)
@@ -1232,8 +1238,17 @@ class VMOpsTestCase(test_base.HyperVBaseTestCase):
         exp_inst_path = (passed_instance_path or
                          self._pathutils.get_instance_dir.return_value)
 
-        self._pathutils.check_remove_dir.assert_called_once_with(
+        exp_check_remove_dir_calls = [mock.call(exp_inst_path)]
-            exp_inst_path)
+
+        mock_get_migr_dir = self._pathutils.get_instance_migr_revert_dir
+        if cleanup_migr_files:
+            mock_get_migr_dir.assert_called_once_with(
+                exp_inst_path, remove_dir=True)
+            exp_check_remove_dir_calls.append(
+                mock.call(mock.sentinel.backup_location))
+
+        self._pathutils.check_remove_dir.assert_has_calls(
+            exp_check_remove_dir_calls)
 
     @ddt.data({},
               {'vm_exists': False, 'planned_vm_exists': False},
@@ -1252,9 +1267,11 @@ class VMOpsTestCase(test_base.HyperVBaseTestCase):
         self._vmops._migrutils.planned_vm_exists.return_value = (
             planned_vm_exists)
 
-        self._vmops.destroy(instance=mock_instance,
+        self._vmops.destroy(
-                            block_device_info=mock.sentinel.FAKE_BD_INFO,
+            instance=mock_instance,
-                            network_info=mock.sentinel.fake_network_info)
+            block_device_info=mock.sentinel.FAKE_BD_INFO,
+            network_info=mock.sentinel.fake_network_info,
+            cleanup_migration_files=mock.sentinel.cleanup_migr_files)
 
         self._vmops._vmutils.vm_exists.assert_called_with(
             mock_instance.name)
@@ -1282,8 +1299,9 @@ class VMOpsTestCase(test_base.HyperVBaseTestCase):
         mock_disconnect_volumes.assert_called_once_with(
             mock.sentinel.FAKE_BD_INFO)
         mock_delete_disk_files.assert_called_once_with(
-            mock_instance.name,
+            mock_instance,
-            self._pathutils.get_instance_dir.return_value)
+            self._pathutils.get_instance_dir.return_value,
+            mock.sentinel.cleanup_migr_files)
 
     @mock.patch('compute_hyperv.nova.vmops.VMOps.power_off')
     def test_destroy_exception(self, mock_power_off):