diff --git a/compute_hyperv/nova/vmops.py b/compute_hyperv/nova/vmops.py index a4a06f2d..3d9a6581 100644 --- a/compute_hyperv/nova/vmops.py +++ b/compute_hyperv/nova/vmops.py @@ -92,6 +92,7 @@ class VMOps(object): self._metricsutils = utilsfactory.get_metricsutils() self._vhdutils = utilsfactory.get_vhdutils() self._hostutils = utilsfactory.get_hostutils() + self._migrutils = utilsfactory.get_migrationutils() self._pathutils = pathutils.PathUtils() self._volumeops = volumeops.VolumeOps() self._imagecache = imagecache.ImageCache() @@ -820,13 +821,18 @@ class VMOps(object): # Stop the VM first. self._vmutils.stop_vm_jobs(instance_name) self.power_off(instance) - self.unplug_vifs(instance, network_info) - self._vmutils.destroy_vm(instance_name) - self._volumeops.disconnect_volumes(block_device_info) + elif self._migrutils.planned_vm_exists(instance_name): + self._migrutils.destroy_planned_vm(instance_name) else: LOG.debug("Instance not found", instance=instance) + # NOTE(claudiub): The vifs should be unplugged and the volumes + # should be disconnected even if the VM doesn't exist anymore, + # so they are not leaked. + self.unplug_vifs(instance, network_info) + self._volumeops.disconnect_volumes(block_device_info) + if destroy_disks: self._delete_disk_files(instance_name, instance_path) except Exception: diff --git a/compute_hyperv/nova/volumeops.py b/compute_hyperv/nova/volumeops.py index 8f9ff577..83203aa3 100644 --- a/compute_hyperv/nova/volumeops.py +++ b/compute_hyperv/nova/volumeops.py @@ -401,6 +401,10 @@ class BaseVolumeDriver(object): slot) def detach_volume(self, connection_info, instance_name): + if self._vmutils.planned_vm_exists(instance_name): + LOG.warning("Instance %s is a Planned VM, cannot detach " + "volumes from it.", instance_name) + return # Retrieving the disk path can be a time consuming operation in # case of passthrough disks. As such disks attachments will be # tagged using the volume id, we'll just use that instead. diff --git a/compute_hyperv/tests/unit/test_vmops.py b/compute_hyperv/tests/unit/test_vmops.py index a9f9af63..b57b4419 100644 --- a/compute_hyperv/tests/unit/test_vmops.py +++ b/compute_hyperv/tests/unit/test_vmops.py @@ -73,6 +73,7 @@ class VMOpsTestCase(test_base.HyperVBaseTestCase): self._vmops._vhdutils = mock.MagicMock() self._vmops._pathutils = mock.MagicMock() self._vmops._hostutils = mock.MagicMock() + self._vmops._migrutils = mock.MagicMock() self._vmops._pdk = mock.MagicMock() self._vmops._serial_console_ops = mock.MagicMock() self._vmops._block_dev_man = mock.MagicMock() @@ -1219,29 +1220,37 @@ class VMOpsTestCase(test_base.HyperVBaseTestCase): self._pathutils.check_remove_dir.assert_called_once_with( exp_inst_path) + @ddt.data(True, False) @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, mock_unplug_vifs, mock_power_off, + def test_destroy(self, vm_exists, mock_unplug_vifs, mock_power_off, mock_delete_disk_files, mock_disconnect_volumes): mock_instance = fake_instance.fake_instance_obj(self.context) - self._vmops._vmutils.vm_exists.return_value = True + self._vmops._vmutils.vm_exists.return_value = vm_exists self._vmops.destroy(instance=mock_instance, block_device_info=mock.sentinel.FAKE_BD_INFO, network_info=mock.sentinel.fake_network_info) - self._pathutils.get_instance_dir.assert_called_once_with( - mock_instance.name, - create_dir=False) self._vmops._vmutils.vm_exists.assert_called_with( mock_instance.name) - mock_power_off.assert_called_once_with(mock_instance) + + if vm_exists: + self._vmops._vmutils.stop_vm_jobs.assert_called_once_with( + mock_instance.name) + mock_power_off.assert_called_once_with(mock_instance) + self._vmops._vmutils.destroy_vm.assert_called_once_with( + mock_instance.name) + else: + 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) + mock_unplug_vifs.assert_called_once_with( mock_instance, mock.sentinel.fake_network_info) - self._vmops._vmutils.destroy_vm.assert_called_once_with( - mock_instance.name) mock_disconnect_volumes.assert_called_once_with( mock.sentinel.FAKE_BD_INFO) mock_delete_disk_files.assert_called_once_with( @@ -1251,6 +1260,7 @@ class VMOpsTestCase(test_base.HyperVBaseTestCase): 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) diff --git a/compute_hyperv/tests/unit/test_volumeops.py b/compute_hyperv/tests/unit/test_volumeops.py index 4cb56a7f..433b2431 100644 --- a/compute_hyperv/tests/unit/test_volumeops.py +++ b/compute_hyperv/tests/unit/test_volumeops.py @@ -613,10 +613,16 @@ class BaseVolumeDriverTestCase(test_base.HyperVBaseTestCase): def test_attach_volume_block_dev(self): self._test_attach_volume(is_block_dev=True) + def test_detach_volume_planned_vm(self): + self._base_vol_driver.detach_volume(mock.sentinel.connection_info, + mock.sentinel.inst_name) + self._vmutils.detach_vm_disk.assert_not_called() + @ddt.data(True, False) @mock.patch.object(volumeops.BaseVolumeDriver, 'get_disk_resource_path') def test_detach_volume(self, is_block_dev, mock_get_disk_resource_path): + self._vmutils.planned_vm_exists.return_value = False connection_info = get_fake_connection_info() self._base_vol_driver._is_block_dev = is_block_dev