Cleanup migration files
At the moment, if an instance migration/resize fails and is deleted while being in error state, the instance revert files are not deleted. This change fixes this issue, ensuring that instance migration files are cleaned up when an instance is destroyed. Closes-Bug: #1724240 Change-Id: I3752e7d0bd96a7d418563afd997739e88a87914d
This commit is contained in:
parent
aaf41258c5
commit
24cc1f3fff
|
@ -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
|
||||
|
|
|
@ -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)
|
||||
|
|
|
@ -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(
|
||||
|
|
|
@ -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):
|
||||
|
|
Loading…
Reference in New Issue