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:
Lucian Petrut 2017-12-05 18:26:35 +02:00
parent aaf41258c5
commit 24cc1f3fff
4 changed files with 51 additions and 21 deletions

View File

@ -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

View File

@ -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)

View File

@ -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(

View File

@ -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):