diff --git a/compute_hyperv/nova/migrationops.py b/compute_hyperv/nova/migrationops.py index 700deb2c..05b0fdb4 100644 --- a/compute_hyperv/nova/migrationops.py +++ b/compute_hyperv/nova/migrationops.py @@ -127,7 +127,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 diff --git a/compute_hyperv/nova/vmops.py b/compute_hyperv/nova/vmops.py index 385d7484..446192f0 100644 --- a/compute_hyperv/nova/vmops.py +++ b/compute_hyperv/nova/vmops.py @@ -292,7 +292,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) @@ -800,23 +800,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) @@ -844,7 +853,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) diff --git a/compute_hyperv/tests/unit/test_migrationops.py b/compute_hyperv/tests/unit/test_migrationops.py index 44171e63..f68e3a36 100644 --- a/compute_hyperv/tests/unit/test_migrationops.py +++ b/compute_hyperv/tests/unit/test_migrationops.py @@ -170,7 +170,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( diff --git a/compute_hyperv/tests/unit/test_vmops.py b/compute_hyperv/tests/unit/test_vmops.py index 71d9a949..eaab4729 100644 --- a/compute_hyperv/tests/unit/test_vmops.py +++ b/compute_hyperv/tests/unit/test_vmops.py @@ -489,7 +489,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( @@ -1227,11 +1227,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) - def test_delete_disk_files(self, passed_instance_path): - mock_instance = fake_instance.fake_instance_obj(self.context) - self._vmops._delete_disk_files(mock_instance.name, - passed_instance_path) + @ddt.data({'passed_instance_path': True}, + {'cleanup_migr_files': True}) + @ddt.unpack + def test_delete_disk_files(self, passed_instance_path=None, + 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) @@ -1246,8 +1252,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_inst_path) + exp_check_remove_dir_calls = [mock.call(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}, @@ -1266,9 +1281,11 @@ class VMOpsTestCase(test_base.HyperVBaseTestCase): self._vmops._migrutils.planned_vm_exists.return_value = ( planned_vm_exists) - self._vmops.destroy(instance=mock_instance, - block_device_info=mock.sentinel.FAKE_BD_INFO, - network_info=mock.sentinel.fake_network_info) + self._vmops.destroy( + instance=mock_instance, + 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) @@ -1296,8 +1313,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, - self._pathutils.get_instance_dir.return_value) + mock_instance, + 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):