Perform proper cleanup after failed instance spawns
This change ensures that vif ports as well as volume connections
are properly removed after an instance fails to spawn.
In order to avoid having similar issues in the future, the
'block_device_info' and 'network_info' arguments become mandatory
for the VMOps.destroy method.
Side note: for convenience reasons, one redundant unit test has
been squashed.
Closes-Bug: #1714285
Change-Id: Ifa701459b15b5a2046528fa45eca7ab382f1f7e8
(cherry picked from commit 6e715ed580
)
This commit is contained in:
parent
6887bbf70b
commit
01f12ef8bf
|
@ -328,7 +328,7 @@ class VMOps(object):
|
|||
should_plug_vifs=False)
|
||||
except Exception:
|
||||
with excutils.save_and_reraise_exception():
|
||||
self.destroy(instance)
|
||||
self.destroy(instance, network_info, block_device_info)
|
||||
|
||||
@contextlib.contextmanager
|
||||
def wait_vif_plug_events(self, instance, network_info):
|
||||
|
@ -805,7 +805,7 @@ class VMOps(object):
|
|||
|
||||
self._pathutils.check_remove_dir(instance_path)
|
||||
|
||||
def destroy(self, instance, network_info=None, block_device_info=None,
|
||||
def destroy(self, instance, network_info, block_device_info,
|
||||
destroy_disks=True):
|
||||
instance_name = instance.name
|
||||
LOG.info("Got request to destroy instance", instance=instance)
|
||||
|
|
|
@ -462,17 +462,19 @@ class VMOpsTestCase(test_base.HyperVBaseTestCase):
|
|||
self.assertRaises(exception.InstanceExists, self._vmops.spawn,
|
||||
self.context, mock_instance, mock_image_meta,
|
||||
[mock.sentinel.FILE], mock.sentinel.PASSWORD,
|
||||
mock.sentinel.INFO, block_device_info)
|
||||
mock.sentinel.network_info, block_device_info)
|
||||
elif fail is os_win_exc.HyperVException:
|
||||
self.assertRaises(os_win_exc.HyperVException, self._vmops.spawn,
|
||||
self.context, mock_instance, mock_image_meta,
|
||||
[mock.sentinel.FILE], mock.sentinel.PASSWORD,
|
||||
mock.sentinel.INFO, block_device_info)
|
||||
mock_destroy.assert_called_once_with(mock_instance)
|
||||
mock.sentinel.network_info, block_device_info)
|
||||
mock_destroy.assert_called_once_with(mock_instance,
|
||||
mock.sentinel.network_info,
|
||||
block_device_info)
|
||||
else:
|
||||
self._vmops.spawn(self.context, mock_instance, mock_image_meta,
|
||||
[mock.sentinel.FILE], mock.sentinel.PASSWORD,
|
||||
mock.sentinel.INFO, block_device_info)
|
||||
mock.sentinel.network_info, block_device_info)
|
||||
self._vmops._vmutils.vm_exists.assert_called_once_with(
|
||||
mock_instance.name)
|
||||
mock_delete_disk_files.assert_called_once_with(
|
||||
|
@ -487,14 +489,15 @@ class VMOpsTestCase(test_base.HyperVBaseTestCase):
|
|||
fake_vm_gen)
|
||||
mock_create_ephemerals.assert_called_once_with(
|
||||
mock_instance, block_device_info['ephemerals'])
|
||||
mock_get_neutron_events.assert_called_once_with(mock.sentinel.INFO)
|
||||
mock_get_neutron_events.assert_called_once_with(
|
||||
mock.sentinel.network_info)
|
||||
mock_get_image_vm_gen.assert_called_once_with(mock_instance.uuid,
|
||||
mock_image_meta)
|
||||
mock_create_instance.assert_called_once_with(
|
||||
self.context, mock_instance, mock.sentinel.INFO,
|
||||
self.context, mock_instance, mock.sentinel.network_info,
|
||||
block_device_info, fake_vm_gen, mock_image_meta)
|
||||
mock_plug_vifs.assert_called_once_with(mock_instance,
|
||||
mock.sentinel.INFO)
|
||||
mock.sentinel.network_info)
|
||||
mock_save_device_metadata.assert_called_once_with(
|
||||
self.context, mock_instance, block_device_info)
|
||||
mock_configdrive_required.assert_called_once_with(mock_instance)
|
||||
|
@ -502,14 +505,14 @@ class VMOpsTestCase(test_base.HyperVBaseTestCase):
|
|||
mock_create_config_drive.assert_called_once_with(
|
||||
self.context, mock_instance, [mock.sentinel.FILE],
|
||||
mock.sentinel.PASSWORD,
|
||||
mock.sentinel.INFO)
|
||||
mock.sentinel.network_info)
|
||||
mock_attach_config_drive.assert_called_once_with(
|
||||
mock_instance, fake_config_drive_path, fake_vm_gen)
|
||||
mock_set_boot_order.assert_called_once_with(
|
||||
mock_instance.name, fake_vm_gen, block_device_info)
|
||||
mock_power_on.assert_called_once_with(
|
||||
mock_instance,
|
||||
network_info=mock.sentinel.INFO,
|
||||
network_info=mock.sentinel.network_info,
|
||||
should_plug_vifs=False)
|
||||
|
||||
def test_spawn(self):
|
||||
|
@ -1220,15 +1223,22 @@ class VMOpsTestCase(test_base.HyperVBaseTestCase):
|
|||
self._pathutils.check_remove_dir.assert_called_once_with(
|
||||
exp_inst_path)
|
||||
|
||||
@ddt.data(True, False)
|
||||
@ddt.data({},
|
||||
{'vm_exists': False, 'planned_vm_exists': False},
|
||||
{'vm_exists': False, 'planned_vm_exists': True})
|
||||
@ddt.unpack
|
||||
@mock.patch('compute_hyperv.nova.volumeops.VolumeOps.disconnect_volumes')
|
||||
@mock.patch('compute_hyperv.nova.vmops.VMOps._delete_disk_files')
|
||||
@mock.patch('compute_hyperv.nova.vmops.VMOps.power_off')
|
||||
@mock.patch('compute_hyperv.nova.vmops.VMOps.unplug_vifs')
|
||||
def test_destroy(self, vm_exists, mock_unplug_vifs, mock_power_off,
|
||||
mock_delete_disk_files, mock_disconnect_volumes):
|
||||
def test_destroy(self, mock_unplug_vifs, mock_power_off,
|
||||
mock_delete_disk_files, mock_disconnect_volumes,
|
||||
vm_exists=True,
|
||||
planned_vm_exists=False):
|
||||
mock_instance = fake_instance.fake_instance_obj(self.context)
|
||||
self._vmops._vmutils.vm_exists.return_value = vm_exists
|
||||
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,
|
||||
|
@ -1243,11 +1253,14 @@ class VMOpsTestCase(test_base.HyperVBaseTestCase):
|
|||
mock_power_off.assert_called_once_with(mock_instance)
|
||||
self._vmops._vmutils.destroy_vm.assert_called_once_with(
|
||||
mock_instance.name)
|
||||
else:
|
||||
elif planned_vm_exists:
|
||||
self._vmops._migrutils.planned_vm_exists.assert_called_once_with(
|
||||
mock_instance.name)
|
||||
self._vmops._migrutils.destroy_planned_vm.assert_called_once_with(
|
||||
mock_instance.name)
|
||||
self.assertFalse(self._vmops._vmutils.destroy_vm.called)
|
||||
else:
|
||||
self.assertFalse(self._vmops._migrutils.destroy_planned_vm.called)
|
||||
|
||||
mock_unplug_vifs.assert_called_once_with(
|
||||
mock_instance, mock.sentinel.fake_network_info)
|
||||
|
@ -1257,14 +1270,6 @@ class VMOpsTestCase(test_base.HyperVBaseTestCase):
|
|||
mock_instance.name,
|
||||
self._pathutils.get_instance_dir.return_value)
|
||||
|
||||
def test_destroy_inexistent_instance(self):
|
||||
mock_instance = fake_instance.fake_instance_obj(self.context)
|
||||
self._vmops._vmutils.vm_exists.return_value = False
|
||||
self._vmops._vmutils.planned_vm_exists.return_value = False
|
||||
|
||||
self._vmops.destroy(instance=mock_instance)
|
||||
self.assertFalse(self._vmops._vmutils.destroy_vm.called)
|
||||
|
||||
@mock.patch('compute_hyperv.nova.vmops.VMOps.power_off')
|
||||
def test_destroy_exception(self, mock_power_off):
|
||||
mock_instance = fake_instance.fake_instance_obj(self.context)
|
||||
|
@ -1273,7 +1278,9 @@ class VMOpsTestCase(test_base.HyperVBaseTestCase):
|
|||
self._vmops._vmutils.vm_exists.return_value = True
|
||||
|
||||
self.assertRaises(os_win_exc.HyperVException,
|
||||
self._vmops.destroy, mock_instance)
|
||||
self._vmops.destroy, mock_instance,
|
||||
mock.sentinel.network_info,
|
||||
mock.sentinel.block_device_info)
|
||||
|
||||
def test_reboot_hard(self):
|
||||
self._test_reboot(vmops.REBOOT_TYPE_HARD,
|
||||
|
|
Loading…
Reference in New Issue