Merge "Update InstanceNUMACell version in more cases" into unmaintained/2023.1

Author: Zuul

nova/objects/instance_numa.py

@@ -196,26 +196,78 @@ 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 (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
-            # NOTE(gibi): This data migration populates the pcpuset field that
-            # is new in version 1.5. However below we bump 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 bump
-            # 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 cell.cpu_policy == obj_fields.CPUAllocationPolicy.DEDICATED:
-                cell.pcpuset = cell.cpuset
-                cell.cpuset = set()
-                cell.VERSION = '1.6'
-                update_db = True
-            else:
-                if 'pcpuset' not in cell:
+
+            # 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
 

nova/tests/unit/objects/test_instance_numa.py

@@ -363,6 +363,7 @@ def test_obj_from_db_obj(self):
         fake_topo_obj = copy.deepcopy(fake_topo_obj_w_cell_v1_4)
         for cell in fake_topo_obj.cells:
             cell.cpu_policy = objects.fields.CPUAllocationPolicy.DEDICATED
+            cell.VERSION = '1.4'
 
         numa_topology = objects.InstanceNUMATopology.obj_from_db_obj(
             self.context, fake_instance_uuid, fake_topo_obj._to_json())
@@ -402,6 +403,7 @@ def test_obj_from_db_obj_no_pinning(self):
         fake_topo_obj = copy.deepcopy(fake_topo_obj_w_cell_v1_4)
         for cell in fake_topo_obj.cells:
             cell.cpu_policy = objects.fields.CPUAllocationPolicy.SHARED
+            cell.VERSION = '1.4'
 
         numa_topology = objects.InstanceNUMATopology.obj_from_db_obj(
             self.context, fake_instance_uuid, fake_topo_obj._to_json())
@@ -412,7 +414,7 @@ def test_obj_from_db_obj_no_pinning(self):
             self.assertEqual(topo_cell.cpuset, obj_cell.cpuset)
             self.assertEqual(set(), obj_cell.pcpuset)
 
-    def test__migrate_legacy_dedicated_instance_cpuset(self):
+    def test__migrate_legacy_dedicated_instance_cpuset_dedicate_1_4(self):
         # Create a topology with a cell on latest version. Would be nice
         # to create one the old 1.4 cell version directly but that is only
         # possible indirectly as done below.
@@ -446,7 +448,118 @@ def test__migrate_legacy_dedicated_instance_cpuset(self):
         # pcpuset
         self.assertEqual(set(), topo_loaded.cells[0].cpuset)
         self.assertEqual({0, 1}, topo_loaded.cells[0].pcpuset)
-        # and the version is bumped to 1.6
+        # and the version is updated to 1.6
+        self.assertEqual('1.6', topo_loaded.cells[0].VERSION)
+
+    def test__migrate_legacy_dedicated_instance_cpuset_shared_1_4(self):
+        # Create a topology with a cell on latest version. Would be nice
+        # to create one the old 1.4 cell version directly but that is only
+        # possible indirectly as done below.
+        topo = objects.InstanceNUMATopology(
+            instance_uuid=fake_instance_uuid,
+            cells=[
+                objects.InstanceNUMACell(id=0, cpuset={0, 1}, pcpuset=set()),
+            ])
+        topo.cells[0].cpu_policy = objects.fields.CPUAllocationPolicy.SHARED
+
+        # Use the builtin backlevelling logic to pull it back to old cell
+        # version
+        topo_with_cell_1_4 = topo.obj_to_primitive(
+            target_version='1.3', version_manifest={'InstanceNUMACell': '1.4'})
+
+        # Just check that the backlevelling works, and we have a cell with
+        # version and data on 1.4 level
+        cell_1_4_primitive = topo_with_cell_1_4['nova_object.data']['cells'][0]
+        self.assertEqual('1.4', cell_1_4_primitive['nova_object.version'])
+        self.assertEqual(
+            (0, 1), cell_1_4_primitive['nova_object.data']['cpuset'])
+        self.assertNotIn('pcpuset', cell_1_4_primitive['nova_object.data'])
+
+        # Now simulate that such old data is loaded from the DB and migrated
+        # from 1.4 to 1.6 by the data migration
+        topo_loaded = objects.InstanceNUMATopology.obj_from_db_obj(
+            self.context, fake_instance_uuid,
+            jsonutils.dumps(topo_with_cell_1_4))
+
+        # In place data migration did not move the data as the cpu_policy is
+        # shared
+        self.assertEqual({0, 1}, topo_loaded.cells[0].cpuset)
+        self.assertEqual(set(), topo_loaded.cells[0].pcpuset)
+        # but the version is updated to 1.6
+        self.assertEqual('1.6', topo_loaded.cells[0].VERSION)
+
+    def test__migrate_legacy_dedicated_instance_cpuset_dedicated_half_migrated(
+        self
+    ):
+        # Before the fix for https://bugs.launchpad.net/nova/+bug/2097360
+        # landed Nova only half migrated the InstanceNUMACell from 1.4 to 1.6
+        # by doing the data move but not updating the version string in the DB.
+        # This test case ensures that if such migration happened before the fix
+        # was deployed then Nova fixed the DB content on the next load as well.
+
+        # Create a topology with a cell on latest version. Would be nice
+        # to create one the old 1.4 cell version directly but that is only
+        # possible indirectly as done below.
+        topo = objects.InstanceNUMATopology(
+            instance_uuid=fake_instance_uuid,
+            cells=[
+                objects.InstanceNUMACell(id=0, cpuset=set(), pcpuset={0, 1}),
+            ])
+        topo.cells[0].cpu_policy = objects.fields.CPUAllocationPolicy.DEDICATED
+
+        # simulate the half done migration by pulling back the version string
+        # in the primitive form to 1.4 while keeping the data on 1.6 format.
+        topo_primitive = topo.obj_to_primitive()
+        cell_primitive = topo_primitive['nova_object.data']['cells'][0]
+        self.assertEqual('1.6', cell_primitive['nova_object.version'])
+        cell_primitive['nova_object.version'] = '1.4'
+
+        topo_loaded = objects.InstanceNUMATopology.obj_from_db_obj(
+            self.context, fake_instance_uuid, jsonutils.dumps(topo_primitive))
+
+        # the data did not change
+        self.assertEqual(set(), topo_loaded.cells[0].cpuset)
+        self.assertEqual({0, 1}, topo_loaded.cells[0].pcpuset)
+        # but the version is updated to 1.6
+        self.assertEqual('1.6', topo_loaded.cells[0].VERSION)
+
+    def test__migrate_legacy_dedicated_instance_cpuset_mixed_1_4(self):
+        # Before the fix for https://bugs.launchpad.net/nova/+bug/2097360
+        # landed Nova only half migrated the InstanceNUMACell from 1.4 to 1.6
+        # by doing the data move but not updating the version string in the DB.
+        # After this the instance is resized to flavor with mixed cpu_policy
+        # then there is a good chance that the DB has version string 1.4 but
+        # the data is on 1.6 format with the cpu_policy set to mixed.
+        # This test case ensures that if such situation happened before the fix
+        # was deployed then Nova fixed the DB content on the next load as well.
+
+        # Create a topology with a cell on latest version. Would be nice
+        # to create one the old 1.4 cell version directly but that is only
+        # possible indirectly as done below.
+        topo = objects.InstanceNUMATopology(
+            instance_uuid=fake_instance_uuid,
+            cells=[
+                objects.InstanceNUMACell(id=0, cpuset={0, 1}, pcpuset={2, 3}),
+            ])
+        topo.cells[0].cpu_policy = objects.fields.CPUAllocationPolicy.MIXED
+
+        # simulate the half done migration by pulling back the version string
+        # in the primitive form to 1.4 while keeping the data on 1.6 format.
+        topo_primitive = topo.obj_to_primitive()
+        cell_primitive = topo_primitive['nova_object.data']['cells'][0]
+        self.assertEqual('1.6', cell_primitive['nova_object.version'])
+        cell_primitive['nova_object.version'] = '1.4'
+
+        topo_loaded = objects.InstanceNUMATopology.obj_from_db_obj(
+            self.context, fake_instance_uuid, jsonutils.dumps(topo_primitive))
+
+        # the data did not change
+        self.assertEqual({0, 1}, topo_loaded.cells[0].cpuset)
+        self.assertEqual({2, 3}, topo_loaded.cells[0].pcpuset)
+        self.assertEqual(
+            objects.fields.CPUAllocationPolicy.MIXED,
+            topo_loaded.cells[0].cpu_policy)
+        # but the version is updated to 1.6
         self.assertEqual('1.6', topo_loaded.cells[0].VERSION)
 
 

nova/tests/unit/objects/test_request_spec.py

@@ -722,6 +722,9 @@ def test_get_by_instance_uuid_numa_topology_migration(
                     id=1, cpuset={3, 4}, memory=512, cpu_policy="dedicated"),
             ]
         )
+        for cell in numa_topology.cells:
+            cell.VERSION = '1.4'
+
         spec_obj = fake_request_spec.fake_spec_obj()
         spec_obj.numa_topology = numa_topology
         fake_spec = fake_request_spec.fake_db_spec(spec_obj)