Synchronise 2025.1 with upstream

nova/compute/manager.py

@@ -72,6 +72,7 @@
 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 _
@@ -940,6 +941,9 @@ def _complete_deletion(self, context, instance):
         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

nova/exception.py

@@ -2245,7 +2245,7 @@ class PlacementReshapeConflict(PlacementAPIConflict):
     """
     msg_fmt = _(
         "A conflict was encountered attempting to reshape a provider tree: "
-        "$(error)s"
+        "%(error)s"
     )
 
 

nova/tests/fixtures/libvirt.py

@@ -100,6 +100,7 @@ def _reset():
 
 VIR_DOMAIN_UNDEFINE_MANAGED_SAVE = 1
 VIR_DOMAIN_UNDEFINE_NVRAM = 4
+VIR_DOMAIN_UNDEFINE_KEEP_TPM = 64
 
 VIR_DOMAIN_AFFECT_CURRENT = 0
 VIR_DOMAIN_AFFECT_LIVE = 1

nova/tests/functional/libvirt/test_vtpm.py

@@ -17,6 +17,7 @@
 
 from castellan.common.objects import passphrase
 from castellan.key_manager import key_manager
+import fixtures
 from oslo_log import log as logging
 from oslo_utils import uuidutils
 
@@ -239,6 +240,51 @@ def test_hard_reboot_server(self):
         # is still correct
         self.assertInstanceHasSecret(server)
 
+    def _test_resize_revert_server__vtpm_to_vtpm(self, extra_specs=None):
+        """Test behavior of revert when a vTPM is retained across a resize.
+
+        Other tests cover going from no vTPM => vTPM and vice versa.
+        """
+        for host in ('test_compute0', 'test_compute1'):
+            self.start_compute(host)
+
+        server = self._create_server_with_vtpm()
+
+        # Create a different flavor with a vTPM.
+        extra_specs = extra_specs or {
+            'hw:tpm_model': 'tpm-tis', 'hw:tpm_version': '1.2'}
+        flavor_id = self._create_flavor(extra_spec=extra_specs)
+
+        with mock.patch(
+            'nova.virt.libvirt.driver.LibvirtDriver'
+            '.migrate_disk_and_power_off', return_value='{}',
+        ):
+            server = self._resize_server(server, flavor_id=flavor_id)
+
+        # ensure our instance's system_metadata field and key manager inventory
+        # is updated to reflect the new vTPM requirement
+        self.assertInstanceHasSecret(server)
+
+        # revert the instance rather than confirming it, and ensure the secret
+        # is correctly cleaned up
+
+        with mock.patch(
+            'nova.virt.libvirt.driver.LibvirtDriver'
+            '.migrate_disk_and_power_off', return_value='{}',
+        ):
+            server = self._revert_resize(server)
+
+        # Should still have a secret because we had a vTPM before too.
+        self.assertInstanceHasSecret(server)
+
+    def test_resize_revert_server__vtpm_to_vtpm_same_config(self):
+        self._test_resize_revert_server__vtpm_to_vtpm()
+
+    def test_resize_revert_server__vtpm_to_vtpm_different_config(self):
+        extra_specs = {'hw:tpm_model': 'tpm-tis', 'hw:tpm_version': '2.0'}
+        self._test_resize_revert_server__vtpm_to_vtpm(
+            extra_specs=extra_specs)
+
     def test_resize_server__no_vtpm_to_vtpm(self):
         for host in ('test_compute0', 'test_compute1'):
             self.start_compute(host)
@@ -379,3 +425,12 @@ def test_shelve_server(self):
         self.assertRaises(
             client.OpenStackApiException,
             self._shelve_server, server)
+
+
+class VTPMServersTestNonShared(VTPMServersTest):
+
+    def setUp(self):
+        super().setUp()
+        self.useFixture(fixtures.MockPatch(
+            'nova.compute.manager.ComputeManager._is_instance_storage_shared',
+            return_value=False))

nova/tests/functional/test_report_client.py

@@ -1569,13 +1569,17 @@ def test_update_from_provider_tree_reshape_conflict_retry(self):
         # So we expect that it is signalled with an exception so that the
         # upper layer can re-drive the reshape process with a fresh tree that
         # now has the inventories
-        self.assertRaises(
+        ex = self.assertRaises(
             exception.PlacementReshapeConflict,
             self.client.update_from_provider_tree,
             self.context,
             ptree,
             allocations=allocs,
         )
+        ex_msg = str(ex)
+        self.assertNotIn('$', ex_msg)
+        self.assertIn("A conflict was encountered attempting to reshape "
+                      "a provider tree", ex_msg)
         # also we except that the internal caches is cleared so that the
         # re-drive will have a chance to load fresh data from placement
         self.assertEqual(0, len(self.client._provider_tree.roots))

nova/tests/unit/compute/test_compute_mgr.py

@@ -1766,6 +1766,40 @@ def test__get_power_state_NotFound(self):
                               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')
@@ -1806,6 +1840,7 @@ def test_init_instance_failed_resume_sets_error(self, mock_set_inst,
         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')
@@ -1814,7 +1849,7 @@ def test_init_instance_failed_resume_sets_error(self, mock_set_inst,
     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
         """
@@ -1847,16 +1882,19 @@ def fake_inst_destroy():
                                                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)
 
         # Make sure that instance.destroy method was called and
         # 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):

nova/tests/unit/virt/libvirt/test_driver.py

@@ -1661,6 +1661,39 @@ def test__check_vtpm_support_supported(
 
         mock_which.assert_not_called()
 
+    @mock.patch.object(libvirt_driver.LibvirtDriver,
+                       '_register_all_undefined_instance_details',
+                       new=mock.Mock())
+    @mock.patch.object(host.Host, 'has_min_version', return_value=True)
+    def test_keep_tpm_supported(self, mock_version):
+        drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), True)
+        drvr.init_host('dummyhost')
+        self.assertTrue(
+            drvr._may_keep_vtpm,
+            "LibvirtDriver did not correctly detect libvirt version "
+            "supporting KEEP_TPM"
+        )
+
+    @mock.patch.object(libvirt_driver.LibvirtDriver,
+                       '_register_all_undefined_instance_details',
+                       new=mock.Mock())
+    @mock.patch.object(host.Host, 'has_min_version')
+    def test_keep_tpm_unsupported(self, mock_version):
+        def version_check(lv_ver=None, **kwargs):
+            if lv_ver == libvirt_driver.MIN_VERSION_INT_FOR_KEEP_TPM:
+                return False
+            return True
+
+        mock_version.side_effect = version_check
+
+        drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), True)
+        drvr.init_host('dummyhost')
+        self.assertFalse(
+            drvr._may_keep_vtpm,
+            "LibvirtDriver did not correctly detect libvirt version which "
+            "does not support KEEP_TPM"
+        )
+
     def test__check_multipath_misconfiguration(self):
         self.flags(volume_use_multipath=False, volume_enforce_multipath=True,
                    group='libvirt')
@@ -15242,8 +15275,11 @@ def test_create_images_and_backing_ephemeral_gets_created(
                              'ephemeral_foo')
             ]
 
+            # This also asserts that the filesystem label name is generated
+            # correctly as 'ephemeral0' to help prevent regression of the
+            # related bug fix from https://launchpad.net/bugs/2061701
             create_ephemeral_mock.assert_called_once_with(
-                ephemeral_size=1, fs_label='ephemeral_foo',
+                ephemeral_size=1, fs_label='ephemeral0',
                 os_type='linux', target=ephemeral_backing)
 
             fetch_image_mock.assert_called_once_with(
@@ -18846,6 +18882,51 @@ def test_undefine_domain_handles_libvirt_errors(self, mock_get):
         # ensure no raise for no such domain
         drvr._undefine_domain(instance)
 
+    @mock.patch.object(host.Host, "get_guest")
+    def test_undefine_domain_disarms_keep_vtpm_if_not_supported(
+            self, mock_get):
+        drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)
+        drvr._may_keep_vtpm = False  # normally set by init_host
+        instance = objects.Instance(**self.test_instance)
+        fake_guest = mock.Mock()
+        mock_get.return_value = fake_guest
+
+        drvr._undefine_domain(instance, keep_vtpm=True)
+
+        fake_guest.delete_configuration.assert_called_once_with(
+            keep_vtpm=False,
+        )
+
+        # Check that it truly forces it to False and doesn't do a `not` or
+        # something weird :-).
+        fake_guest.reset_mock()
+        drvr._undefine_domain(instance, keep_vtpm=False)
+
+        fake_guest.delete_configuration.assert_called_once_with(
+            keep_vtpm=False,
+        )
+
+    @mock.patch.object(host.Host, "get_guest")
+    def test_undefine_domain_passes_keep_vtpm_if_supported(self, mock_get):
+        drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)
+        drvr._may_keep_vtpm = True  # normally set by init_host
+        instance = objects.Instance(**self.test_instance)
+        fake_guest = mock.Mock()
+        mock_get.return_value = fake_guest
+
+        drvr._undefine_domain(instance, keep_vtpm=True)
+
+        fake_guest.delete_configuration.assert_called_once_with(keep_vtpm=True)
+
+        # Check that it does not force keep_vtpm to true, just because it is
+        # supported.
+        fake_guest.reset_mock()
+        drvr._undefine_domain(instance, keep_vtpm=False)
+
+        fake_guest.delete_configuration.assert_called_once_with(
+            keep_vtpm=False,
+        )
+
     @mock.patch.object(host.Host, "list_instance_domains")
     @mock.patch.object(objects.BlockDeviceMappingList, "bdms_by_instance_uuid")
     @mock.patch.object(objects.InstanceList, "get_by_filters")
@@ -21415,8 +21496,35 @@ def test_cleanup_pass(
         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)
-        mock_undefine.assert_called_once_with(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, keep_vtpm=False)
+
+    @mock.patch('nova.virt.libvirt.driver.LibvirtDriver._undefine_domain')
+    @mock.patch('nova.crypto.delete_vtpm_secret')
+    @mock.patch('nova.virt.libvirt.driver.LibvirtDriver.delete_instance_files')
+    @mock.patch('nova.virt.driver.block_device_info_get_mapping')
+    @mock.patch('nova.virt.libvirt.driver.LibvirtDriver._unplug_vifs')
+    @mock.patch('nova.virt.libvirt.driver.LibvirtDriver._get_vpmems',
+                new=mock.Mock(return_value=None))
+    def test_cleanup_preserves_tpm_if_not_destroying_disks(
+        self, mock_unplug, mock_get_mapping, mock_delete_files,
+        mock_delete_vtpm, mock_undefine,
+    ):
+        """Test with default parameters."""
+        drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI())
+        fake_inst = objects.Instance(**self.test_instance)
+        mock_get_mapping.return_value = []
+        mock_delete_files.return_value = True
+
+        with mock.patch.object(fake_inst, 'save'):
+            drvr.cleanup('ctxt', fake_inst, 'netinfo', destroy_disks=False)
+
+        mock_unplug.assert_called_once_with(fake_inst, 'netinfo', True)
+        mock_get_mapping.assert_called_once_with(None)
+        mock_delete_files.assert_not_called()
+        mock_delete_vtpm.assert_not_called()
+        mock_undefine.assert_called_once_with(fake_inst, keep_vtpm=True)
 
     @mock.patch('nova.virt.libvirt.driver.LibvirtDriver._undefine_domain')
     @mock.patch('nova.crypto.delete_vtpm_secret')
@@ -21439,8 +21547,9 @@ def test_cleanup_instance_marked_deleted(
             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)
-        mock_undefine.assert_called_once_with(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, keep_vtpm=False)
 
     @mock.patch.object(libvirt_driver.LibvirtDriver, 'delete_instance_files',
                        return_value=True)

nova/tests/unit/virt/libvirt/test_guest.py

@@ -138,6 +138,13 @@ def test_delete_configuration(self):
             fakelibvirt.VIR_DOMAIN_UNDEFINE_MANAGED_SAVE |
             fakelibvirt.VIR_DOMAIN_UNDEFINE_NVRAM)
 
+    def test_delete_configuration_with_keep_vtpm_true(self):
+        self.guest.delete_configuration(keep_vtpm=True)
+        self.domain.undefineFlags.assert_called_once_with(
+            fakelibvirt.VIR_DOMAIN_UNDEFINE_MANAGED_SAVE |
+            fakelibvirt.VIR_DOMAIN_UNDEFINE_NVRAM |
+            fakelibvirt.VIR_DOMAIN_UNDEFINE_KEEP_TPM)
+
     def test_delete_configuration_exception(self):
         self.domain.undefineFlags.side_effect = fakelibvirt.libvirtError(
             'oops')