Merge pull request #153 from stackhpc/upstream/2023.1-2025-10-06

Synchronise 2023.1 with upstream

Author: priteau

nova/cmd/manage.py

@@ -3306,9 +3306,10 @@ def _validate_image_properties(self, image_properties):
         # Return the dict so we can update the instance system_metadata
         return image_properties
 
-    def _update_image_properties(self, instance, image_properties):
+    def _update_image_properties(self, ctxt, instance, image_properties):
         """Update instance image properties
 
+        :param ctxt: nova.context.RequestContext
         :param instance: The instance to update
         :param image_properties: List of image properties and values to update
         """
@@ -3332,8 +3333,13 @@ def _update_image_properties(self, instance, image_properties):
         for image_property, value in image_properties.items():
             instance.system_metadata[f'image_{image_property}'] = value
 
+        request_spec = objects.RequestSpec.get_by_instance_uuid(
+            ctxt, instance.uuid)
+        request_spec.image = instance.image_meta
+
         # Save and return 0
         instance.save()
+        request_spec.save()
         return 0
 
     @action_description(_(
@@ -3368,7 +3374,7 @@ def set(self, instance_uuid=None, image_properties=None):
                 instance = objects.Instance.get_by_uuid(
                     cctxt, instance_uuid, expected_attrs=['system_metadata'])
                 return self._update_image_properties(
-                    instance, image_properties)
+                    ctxt, instance, image_properties)
         except ValueError as e:
             print(str(e))
             return 6

nova/compute/manager.py

@@ -4990,6 +4990,50 @@ def _delete_volume_attachments(self, ctxt, bdms):
                               'Error: %s', bdm.attachment_id, str(e),
                               instance_uuid=bdm.instance_uuid)
 
+    def _update_bdm_for_swap_to_finish_resize(
+            self, context, instance, confirm=True):
+        """This updates bdm.swap with new swap info"""
+
+        bdms = instance.get_bdms()
+        if not (instance.old_flavor and instance.new_flavor):
+            return bdms
+
+        if instance.old_flavor.swap == instance.new_flavor.swap:
+            return bdms
+
+        old_swap = instance.old_flavor.swap
+        new_swap = instance.new_flavor.swap
+        if not confirm:
+            # revert flavor on _finish_revert_resize
+            old_swap = instance.new_flavor.swap
+            new_swap = instance.old_flavor.swap
+
+        # add swap
+        if old_swap == 0 and new_swap:
+            # (auniyal)old_swap = 0 means we did not have swap bdm
+            # for this instance.
+            # and as there is a new_swap, its a swap addition
+            new_swap_bdm = block_device.create_blank_bdm(new_swap, 'swap')
+            bdm_obj = objects.BlockDeviceMapping(
+                context, instance_uuid=instance.uuid, **new_swap_bdm)
+            bdm_obj.update_or_create()
+            return instance.get_bdms()
+
+        # update swap
+        for bdm in bdms:
+            if bdm.guest_format == 'swap' and bdm.device_type == 'disk':
+                if new_swap > 0:
+                    LOG.info('Adding swap BDM.', instance=instance)
+                    bdm.volume_size = new_swap
+                    bdm.save()
+                    break
+                elif new_swap == 0:
+                    LOG.info('Deleting swap BDM.', instance=instance)
+                    bdm.destroy()
+                    bdms.objects.remove(bdm)
+                    break
+        return bdms
+
     @wrap_exception()
     @reverts_task_state
     @wrap_instance_event(prefix='compute')
@@ -5327,8 +5371,9 @@ def _finish_revert_resize(
     ):
         """Inner version of finish_revert_resize."""
         with self._error_out_instance_on_exception(context, instance):
-            bdms = objects.BlockDeviceMappingList.get_by_instance_uuid(
-                context, instance.uuid)
+            bdms = self._update_bdm_for_swap_to_finish_resize(
+                context, instance, confirm=False)
+
             self._notify_about_instance_usage(
                     context, instance, "resize.revert.start")
             compute_utils.notify_about_instance_action(context, instance,
@@ -6265,8 +6310,7 @@ def _finish_resize_helper(self, context, disk_info, image, instance,
         The caller must revert the instance's allocations if the migration
         process failed.
         """
-        bdms = objects.BlockDeviceMappingList.get_by_instance_uuid(
-            context, instance.uuid)
+        bdms = self._update_bdm_for_swap_to_finish_resize(context, instance)
 
         with self._error_out_instance_on_exception(context, instance):
             image_meta = objects.ImageMeta.from_dict(image)

nova/objects/instance_numa.py

@@ -14,6 +14,7 @@
 
 import itertools
 
+from oslo_log import log as logging
 from oslo_serialization import jsonutils
 from oslo_utils import versionutils
 
@@ -24,6 +25,8 @@
 from nova.objects import fields as obj_fields
 from nova.virt import hardware
 
+LOG = logging.getLogger(__name__)
+
 
 # TODO(berrange): Remove NovaObjectDictCompat
 @base.NovaObjectRegistry.register
@@ -61,6 +64,11 @@ def obj_make_compatible(self, primitive, target_version):
                     obj_fields.CPUAllocationPolicy.DEDICATED):
                 primitive['cpuset'] = primitive['pcpuset']
             primitive.pop('pcpuset', None)
+            LOG.warning(
+                f'Downgrading InstanceNUMACell to version {target_version} '
+                f'may cause the loss of pinned CPUs if mixing different '
+                f'verisons of nova on different hosts. This should not '
+                f'happen on any supported version after Victoria.')
 
         if target_version < (1, 4):
             primitive.pop('cpuset_reserved', None)
@@ -188,15 +196,79 @@ def _migrate_legacy_dedicated_instance_cpuset(cls, obj):
         # come from instance_extra or request_spec too.
         update_db = False
         for cell in obj.cells:
-            if len(cell.cpuset) == 0:
+            version = versionutils.convert_version_to_tuple(cell.VERSION)
+
+            if version < (1, 4):
+                LOG.warning(
+                    "InstanceNUMACell %s with version %s for instance %s has "
+                    "too old version in the DB, don't know how to update, "
+                    "ignoring.", cell, cell.VERSION, obj.instance_uuid)
                 continue
 
-            if cell.cpu_policy != obj_fields.CPUAllocationPolicy.DEDICATED:
+            if (version >= (1, 5) and
+                cell.cpu_policy == obj_fields.CPUAllocationPolicy.DEDICATED and
+                (cell.cpuset or not cell.pcpuset)
+            ):
+                LOG.warning(
+                    "InstanceNUMACell %s with version %s is inconsistent as "
+                    "the version is 1.5 or greater, cpu_policy is dedicated, "
+                    "but cpuset is not empty or pcpuset is empty.",
+                    cell, cell.VERSION)
                 continue
 
-            cell.pcpuset = cell.cpuset
-            cell.cpuset = set()
-            update_db = True
+            # NOTE(gibi): The data migration between 1.4. and 1.5 populates the
+            # pcpuset field that is new in version 1.5. However below we update
+            # the object version to 1.6 directly. This is intentional. The
+            # version 1.6 introduced a new possible value 'mixed' for the
+            # cpu_policy field. As that is a forward compatible change we don't
+            # have a specific data migration for it. But we also don't have an
+            # automated way to update old object versions from 1.5 to 1.6. So
+            # we do it here just to avoid inconsistency between data and
+            # version in the DB.
+            if version < (1, 6):
+                if cell.cpu_policy == obj_fields.CPUAllocationPolicy.DEDICATED:
+                    if "pcpuset" not in cell or not cell.pcpuset:
+                        # this cell was never migrated to 1.6, migrate it.
+                        cell.pcpuset = cell.cpuset
+                        cell.cpuset = set()
+                        cell.VERSION = '1.6'
+                        update_db = True
+                    else:
+                        # This data was already migrated to 1.6 format but the
+                        # version string wasn't updated to 1.6. This happened
+                        # before the fix
+                        # https://bugs.launchpad.net/nova/+bug/2097360
+                        # Only update the version string.
+                        cell.VERSION = '1.6'
+                        update_db = True
+                elif cell.cpu_policy in (
+                        None, obj_fields.CPUAllocationPolicy.SHARED):
+                    # no data migration needed just add the new field and
+                    # stamp the new version in the DB
+                    cell.pcpuset = set()
+                    cell.VERSION = '1.6'
+                    update_db = True
+                else:  # obj_fields.CPUAllocationPolicy.MIXED
+                    # This means the cell data already got updated to the 1.6
+                    # content as MIXED only supported with 1.6 but the version
+                    # was not updated to 1.6.
+                    # We should not do the data migration as that would trample
+                    # the pcpuset field. Just stamp the 1.6 version in the DB
+                    # and hope for the best.
+                    LOG.warning(
+                        "InstanceNUMACell %s with version %s for instance %s "
+                        "has older than 1.6 version in the DB but using the "
+                        "1.6 feature CPUAllocationPolicy.MIXED. So nova "
+                        "assumes that the data is in 1.6 format and only the "
+                        "version string is old. Correcting the version string "
+                        "in the DB.", cell, cell.VERSION, obj.instance_uuid)
+                    cell.VERSION = '1.6'
+                    update_db = True
+
+            # When the next ovo version 1.7 is added it needs to be handed
+            # here to do any migration if needed and to ensure the version in
+            # the DB is stamped to 1.7
+
         return update_db
 
     # TODO(huaqiang): Remove after Yoga once we are sure these objects have

nova/test.py

@@ -482,7 +482,7 @@ def _start_compute(self, host, cell_name=None):
         self.computes[host] = compute
         return compute
 
-    def _run_periodics(self):
+    def _run_periodics(self, raise_on_error=False):
         """Run the update_available_resource task on every compute manager
 
         This runs periodics on the computes in an undefined order; some child
@@ -497,6 +497,15 @@ class redefine this function to force a specific order.
             with context.target_cell(
                     ctx, self.host_mappings[host].cell_mapping) as cctxt:
                 compute.manager.update_available_resource(cctxt)
+
+        if raise_on_error:
+            if 'Traceback (most recent call last' in self.stdlog.logger.output:
+                # Get the last line of the traceback, for example:
+                #   TypeError: virNodeDeviceLookupByName() argument 2 must be
+                #   str or None, not Proxy
+                last_tb_line = self.stdlog.logger.output.splitlines()[-1]
+                raise TestingException(last_tb_line)
+
         LOG.info('Finished with periodics')
 
     def restart_compute_service(self, compute, keep_hypervisor_state=True):

nova/tests/fixtures/libvirt.py

@@ -2067,6 +2067,17 @@ def device_lookup_by_name(self, dev_name):
         return self.pci_info.get_device_by_name(dev_name)
 
     def nodeDeviceLookupByName(self, name):
+        # See bug https://bugs.launchpad.net/nova/+bug/2098892
+        # We don't test this by importing the libvirt module because the
+        # libvirt module is forbidden to be imported into our test
+        # environment. It is excluded from test-requirements.txt and we
+        # also use the ImportModulePoisonFixture in nova/test.py to prevent
+        # use of modules such as libvirt.
+        if not isinstance(name, str) and name is not None:
+            raise TypeError(
+                'virNodeDeviceLookupByName() argument 2 must be str or '
+                f'None, not {type(name)}')
+
         if name.startswith('mdev'):
             return self.mdev_info.get_device_by_name(name)
 

nova/tests/functional/regressions/test_bug_2098892.py

@@ -0,0 +1,52 @@
+# Licensed under the Apache License, Version 2.0 (the "License"); you may
+# not use this file except in compliance with the License. You may obtain
+# a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+# License for the specific language governing permissions and limitations
+# under the License.
+
+from nova.tests.fixtures import libvirt as fakelibvirt
+from nova.tests.functional.libvirt import test_vgpu
+
+
+class VGPUTestsListDevices(test_vgpu.VGPUTestBase):
+    """Regression test for bug 2098892.
+
+    Test that nodeDeviceLookupByName() is called with valid types to prevent:
+
+    File "/usr/lib64/python3.9/site-packages/libvirt.py", line 5201, in
+      nodeDeviceLookupByName ret =
+      libvirtmod.virNodeDeviceLookupByName(self._o, name)
+      TypeError: virNodeDeviceLookupByName() argument 2 must be str or None,
+      not Proxy
+
+    in the future. This test relies on the LibvirtFixture checking for the
+    correct types in its nodeDeviceLookupByName() method and raising TypeError
+    if they are invalid.
+
+    We don't test this by importing the libvirt module because the libvirt
+    module is forbidden to be imported into our test environment. It is
+    excluded from test-requirements.txt and we also use the
+    ImportModulePoisonFixture in nova/test.py to prevent use of modules such as
+    libvirt.
+    """
+
+    def setUp(self):
+        super().setUp()
+
+        # Start compute supporting only nvidia-11
+        self.flags(
+            enabled_mdev_types=fakelibvirt.NVIDIA_11_VGPU_TYPE,
+            group='devices')
+
+        self.start_compute_with_vgpu('host1')
+
+    def test_update_available_resource(self):
+        # We only want to verify no errors were logged by
+        # update_available_resource (logging under the 'except Exception:').
+        self._run_periodics(raise_on_error=True)

nova/tests/functional/test_servers.py

@@ -3207,6 +3207,73 @@ def test_finish_resize_fails_allocation_cleanup(self):
         self._test_resize_to_same_host_instance_fails(
             '_finish_resize', 'compute_finish_resize')
 
+    def _verify_swap_resize_in_bdm(self, server_id, swap_size):
+        """Verify swap dev in BDM"""
+        ctxt = context.get_admin_context()
+        bdms = objects.BlockDeviceMappingList.get_by_instance_uuid(
+            ctxt, server_id)
+        if swap_size != 0:
+            self.assertIn('swap', [bdm.guest_format for bdm in bdms])
+            swaps = [
+                bdm.volume_size for bdm in bdms if bdm.guest_format == 'swap']
+            self.assertEqual(len(swaps), 1)
+            self.assertIn(swap_size, swaps)
+        else:
+            self.assertNotIn('swap', [bdm.guest_format for bdm in bdms])
+
+    def _test_swap_resize(self, swap1, swap2, confirm=True):
+        fl_1 = self._create_flavor(swap=swap1)
+        fl_2 = self._create_flavor(swap=swap2)
+        server = self._create_server(flavor_id=fl_1, networks=[])
+        # before resize
+        self.assertEqual(server['flavor']['swap'], swap1)
+        server = self._resize_server(server, fl_2)
+        self.assertEqual(server['flavor']['swap'], swap2)
+        self._verify_swap_resize_in_bdm(server['id'], swap2)
+
+        if confirm:
+            server = self._confirm_resize(server)
+            # after resize
+            self.assertEqual(server['flavor']['swap'], swap2)
+            # verify block device mapping
+            self._verify_swap_resize_in_bdm(server['id'], swap2)
+        else:
+            server = self._revert_resize(server)
+            # after revert
+            self.assertEqual(server['flavor']['swap'], swap1)
+            # verify block device mapping
+            self._verify_swap_resize_in_bdm(server['id'], swap1)
+
+    def test_swap_expand_0_to_0_confirm(self):
+        self._test_swap_resize(0, 0)
+
+    def test_swap_expand_0_to_1024_confirm(self):
+        self._test_swap_resize(0, 1024)
+
+    def test_swap_expand_0_to_1024_revert(self):
+        self._test_swap_resize(0, 1024, confirm=False)
+
+    def test_swap_expand_1024_to_2048_confirm(self):
+        self._test_swap_resize(1024, 2048)
+
+    def test_swap_expand_1024_to_2048_revert(self):
+        self._test_swap_resize(1024, 2048, confirm=False)
+
+    def test_swap_expand_2048_to_2048_confirm(self):
+        self._test_swap_resize(2048, 2048)
+
+    def test_swap_shrink_1024_to_0_confirm(self):
+        self._test_swap_resize(1024, 0)
+
+    def test_swap_shrink_1024_to_0_revert(self):
+        self._test_swap_resize(1024, 0, confirm=False)
+
+    def test_swap_shrink_2048_to_1024_confirm(self):
+        self._test_swap_resize(2048, 1024)
+
+    def test_swap_shrink_2048_to_1024_revert(self):
+        self._test_swap_resize(2048, 1024, confirm=False)
+
     def _server_created_with_host(self):
         hostname = self.compute1.host
         server_req = self._build_server(