diff --git a/nova/compute/manager.py b/nova/compute/manager.py index 022128612e..ff4ada222f 100644 --- a/nova/compute/manager.py +++ b/nova/compute/manager.py @@ -69,6 +69,7 @@ from nova.compute import vm_states from nova import conductor import nova.conf import nova.context +from nova import crypto from nova import exception from nova import exception_wrapper from nova.i18n import _ @@ -963,6 +964,9 @@ class ComputeManager(manager.Manager): self._clean_instance_console_tokens(context, instance) self._delete_scheduler_instance_info(context, instance.uuid) + # Delete the vTPM secret in the key manager service if needed. + crypto.delete_vtpm_secret(context, instance) + def _validate_pinning_configuration(self, instances): if not self.driver.capabilities.get('supports_pcpus', False): return diff --git a/nova/tests/functional/libvirt/test_vtpm.py b/nova/tests/functional/libvirt/test_vtpm.py index e39c87288a..6fb36dc404 100644 --- a/nova/tests/functional/libvirt/test_vtpm.py +++ b/nova/tests/functional/libvirt/test_vtpm.py @@ -434,21 +434,3 @@ class VTPMServersTestNonShared(VTPMServersTest): self.useFixture(fixtures.MockPatch( 'nova.compute.manager.ComputeManager._is_instance_storage_shared', return_value=False)) - - # FIXME: Remove this entire method when - # https://bugs.launchpad.net/nova/+bug/2125030 is fixed. - def test_resize_revert_server__vtpm_to_vtpm_same_config(self): - ex = self.assertRaises( - client.OpenStackApiException, - self._test_resize_revert_server__vtpm_to_vtpm) - self.assertEqual(500, ex.response.status_code) - - # FIXME: Remove this entire method when - # https://bugs.launchpad.net/nova/+bug/2125030 is fixed. - def test_resize_revert_server__vtpm_to_vtpm_different_config(self): - extra_specs = {'hw:tpm_model': 'tpm-tis', 'hw:tpm_version': '2.0'} - ex = self.assertRaises( - client.OpenStackApiException, - self._test_resize_revert_server__vtpm_to_vtpm, - extra_specs=extra_specs) - self.assertEqual(500, ex.response.status_code) diff --git a/nova/tests/unit/compute/test_compute_mgr.py b/nova/tests/unit/compute/test_compute_mgr.py index fe7d236d5e..8f5d014912 100644 --- a/nova/tests/unit/compute/test_compute_mgr.py +++ b/nova/tests/unit/compute/test_compute_mgr.py @@ -1752,6 +1752,40 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase, self.compute._get_power_state, instance) + @mock.patch('nova.scheduler.client.report.SchedulerReportClient.' + 'delete_allocation_for_instance') + @mock.patch('nova.crypto.delete_vtpm_secret') + @ddt.data(0, 3600) + def test__complete_deletion( + self, reclaim_instance_interval, mock_delete_vtpm, + mock_delete_alloc): + self.flags(reclaim_instance_interval=reclaim_instance_interval) + instance = objects.Instance(uuid=uuids.instance) + + with mock.patch.multiple( + self.compute, + _update_resource_tracker=mock.DEFAULT, + _clean_instance_console_tokens=mock.DEFAULT, + _delete_scheduler_instance_info=mock.DEFAULT) as mocks: + self.compute._complete_deletion(self.context, instance) + + mocks['_update_resource_tracker'].assert_called_once_with( + self.context, instance) + mocks['_clean_instance_console_tokens'].assert_called_once_with( + self.context, instance) + mocks['_delete_scheduler_instance_info'].assert_called_once_with( + self.context, instance.uuid) + mock_delete_vtpm.assert_called_once_with(self.context, instance) + # _complete_deletion() is only called at actual delete time (either + # regular delete or when reaping after soft delete). The force argument + # differs based on actual or reap delete for other reasons. + if reclaim_instance_interval > 0: + mock_delete_alloc.assert_called_once_with( + self.context, instance.uuid, force=False) + else: + mock_delete_alloc.assert_called_once_with( + self.context, instance.uuid, force=True) + @mock.patch.object(manager.ComputeManager, '_mount_all_shares') @mock.patch.object(manager.ComputeManager, '_get_share_info') @mock.patch.object(manager.ComputeManager, '_get_power_state') @@ -1792,6 +1826,7 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase, mock_get_share_info.assert_called_once_with(mock.ANY, instance) mock_mount.assert_called_once_with(mock.ANY, instance, share_info) + @mock.patch('nova.crypto.delete_vtpm_secret') @mock.patch.object(objects.BlockDeviceMapping, 'destroy') @mock.patch.object(objects.BlockDeviceMappingList, 'get_by_instance_uuid') @mock.patch.object(objects.Instance, 'destroy') @@ -1800,7 +1835,7 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase, def test_init_instance_complete_partial_deletion( self, mock_ids_from_instance, mock_inst_destroy, mock_obj_load_attr, mock_get_by_instance_uuid, - mock_bdm_destroy): + mock_bdm_destroy, mock_delete_vtpm): """Test to complete deletion for instances in DELETED status but not marked as deleted in the DB """ @@ -1833,9 +1868,10 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase, instance.user_id) mock_inst_destroy.side_effect = fake_inst_destroy() - with mock.patch( - "nova.compute.manager.ComputeManager._get_share_info", - return_value=objects.ShareMappingList(), + with mock.patch.multiple( + self.compute, + _get_share_info=mock.Mock(return_value=objects.ShareMappingList()), + _clean_instance_console_tokens=mock.DEFAULT, ): self.compute._init_instance(self.context, instance) @@ -1843,6 +1879,8 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase, # instance was deleted from db. self.assertNotEqual(0, instance.deleted) + mock_delete_vtpm.assert_called_once_with(self.context, instance) + @mock.patch('nova.compute.manager.LOG') def test_init_instance_complete_partial_deletion_raises_exception( self, mock_log): diff --git a/nova/tests/unit/virt/libvirt/test_driver.py b/nova/tests/unit/virt/libvirt/test_driver.py index c25ea49e14..ccdf992153 100644 --- a/nova/tests/unit/virt/libvirt/test_driver.py +++ b/nova/tests/unit/virt/libvirt/test_driver.py @@ -21756,7 +21756,8 @@ class LibvirtConnTestCase(test.NoDBTestCase, mock_unplug.assert_called_once_with(fake_inst, 'netinfo', True) mock_get_mapping.assert_called_once_with(None) mock_delete_files.assert_called_once_with(fake_inst) - mock_delete_vtpm.assert_called_once_with('ctxt', fake_inst) + # vTPM secret should not be deleted until instance is deleted. + mock_delete_vtpm.assert_not_called() mock_undefine.assert_called_once_with(fake_inst) @mock.patch('nova.virt.libvirt.driver.LibvirtDriver._undefine_domain') @@ -21780,7 +21781,8 @@ class LibvirtConnTestCase(test.NoDBTestCase, instance_save.side_effect = exception.InstanceNotFound( instance_id=uuids.instance) drvr.cleanup('ctxt', fake_inst, 'netinfo') - mock_delete_vtpm.assert_called_once_with('ctxt', fake_inst) + # vTPM secret should not be deleted until instance is deleted. + mock_delete_vtpm.assert_not_called() mock_undefine.assert_called_once_with(fake_inst) @mock.patch.object(libvirt_driver.LibvirtDriver, 'delete_instance_files', diff --git a/nova/virt/libvirt/driver.py b/nova/virt/libvirt/driver.py index 4375dc754b..4a8d7718bf 100644 --- a/nova/virt/libvirt/driver.py +++ b/nova/virt/libvirt/driver.py @@ -1649,7 +1649,8 @@ class LibvirtDriver(driver.ComputeDriver): :param destroy_disks: if local ephemeral disks should be destroyed :param migrate_data: optional migrate_data object :param destroy_vifs: if plugged vifs should be unplugged - :param destroy_secrets: Indicates if secrets should be destroyed + :param destroy_secrets: Indicates if libvirt secrets for Cinder volume + encryption should be destroyed """ cleanup_instance_dir = False cleanup_instance_disks = False @@ -1703,8 +1704,8 @@ class LibvirtDriver(driver.ComputeDriver): :param cleanup_instance_dir: If the instance dir should be removed :param cleanup_instance_disks: If the instance disks should be removed. Also removes ephemeral encryption secrets, if present. - :param destroy_secrets: If the cinder volume encryption secrets should - be deleted. + :param destroy_secrets: If the cinder volume encryption libvirt secrets + should be deleted. """ # zero the data on backend pmem device vpmems = self._get_vpmems(instance) @@ -1769,7 +1770,6 @@ class LibvirtDriver(driver.ComputeDriver): pass if cleanup_instance_disks: - crypto.delete_vtpm_secret(context, instance) # Make sure that the instance directory files were successfully # deleted before destroying the encryption secrets in the case of # image backends that are not 'lvm' or 'rbd'. We don't want to