diff --git a/nova/objects/instance_numa.py b/nova/objects/instance_numa.py index b2cb8a8a6899..28e877574d09 100644 --- a/nova/objects/instance_numa.py +++ b/nova/objects/instance_numa.py @@ -166,8 +166,10 @@ class InstanceNUMATopology(base.NovaObject, if 'nova_object.name' in primitive: obj = cls.obj_from_primitive(primitive) - cls._migrate_legacy_dedicated_instance_cpuset( - context, instance_uuid, obj) + updated = cls._migrate_legacy_dedicated_instance_cpuset(obj) + if updated: + cls._save_migrated_cpuset_to_instance_extra( + context, obj, instance_uuid) else: obj = cls._migrate_legacy_object(context, instance_uuid, primitive) @@ -176,13 +178,14 @@ class InstanceNUMATopology(base.NovaObject, # TODO(huaqiang): Remove after Wallaby once we are sure these objects have # been loaded at least once. @classmethod - def _migrate_legacy_dedicated_instance_cpuset(cls, context, instance_uuid, - obj): + def _migrate_legacy_dedicated_instance_cpuset(cls, obj): # NOTE(huaqiang): We may meet some topology object with the old version # 'InstanceNUMACell' cells, in that case, the 'dedicated' CPU is kept # in 'InstanceNUMACell.cpuset' field, but it should be kept in # 'InstanceNUMACell.pcpuset' field since Victoria. Making an upgrade - # and persisting to database. + # here but letting the caller persist the result if needed as we + # don't know which table the InstanceNUMACell is coming from. It can + # come from instance_extra or request_spec too. update_db = False for cell in obj.cells: if len(cell.cpuset) == 0: @@ -194,14 +197,20 @@ class InstanceNUMATopology(base.NovaObject, cell.pcpuset = cell.cpuset cell.cpuset = set() update_db = True + return update_db - if update_db: - db_obj = jsonutils.dumps(obj.obj_to_primitive()) - values = { - 'numa_topology': db_obj, - } - db.instance_extra_update_by_uuid(context, instance_uuid, - values) + # TODO(huaqiang): Remove after Yoga once we are sure these objects have + # been loaded at least once. + @classmethod + def _save_migrated_cpuset_to_instance_extra( + cls, context, obj, instance_uuid + ): + db_obj = jsonutils.dumps(obj.obj_to_primitive()) + values = { + 'numa_topology': db_obj, + } + db.instance_extra_update_by_uuid( + context, instance_uuid, values) # TODO(stephenfin): Remove in X or later, once this has bedded in @classmethod diff --git a/nova/objects/request_spec.py b/nova/objects/request_spec.py index 854db3045c69..9ce77a40435d 100644 --- a/nova/objects/request_spec.py +++ b/nova/objects/request_spec.py @@ -596,6 +596,8 @@ class RequestSpec(base.NovaObject): @staticmethod def _from_db_object(context, spec, db_spec): spec_obj = spec.obj_from_primitive(jsonutils.loads(db_spec['spec'])) + data_migrated = False + for key in spec.fields: # Load these from the db model not the serialized object within, # though they should match. @@ -616,6 +618,13 @@ class RequestSpec(base.NovaObject): # fields. If they are not set, set None. if not spec.obj_attr_is_set(key): setattr(spec, key, None) + elif key == "numa_topology": + if key in spec_obj: + spec.numa_topology = spec_obj.numa_topology + if spec.numa_topology: + data_migrated = objects.InstanceNUMATopology.\ + _migrate_legacy_dedicated_instance_cpuset( + spec.numa_topology) elif key in spec_obj: setattr(spec, key, getattr(spec_obj, key)) spec._context = context @@ -637,6 +646,9 @@ class RequestSpec(base.NovaObject): # NOTE(danms): Instance group may have been deleted spec.instance_group = None + if data_migrated: + spec.save() + spec.obj_reset_changes() return spec diff --git a/nova/tests/unit/objects/test_request_spec.py b/nova/tests/unit/objects/test_request_spec.py index 7d656c555ecf..31797f8133b6 100644 --- a/nova/tests/unit/objects/test_request_spec.py +++ b/nova/tests/unit/objects/test_request_spec.py @@ -615,29 +615,12 @@ class _TestRequestSpecObject(object): self.assertIsInstance(req_obj.instance_group, objects.InstanceGroup) self.assertEqual('fresh', req_obj.instance_group.name) - # FIXME(gibi): This is bug 1952941. When the cpuset -> pcpuset data - # migration was added to InstanceNUMATopology it was missed that such - # object is not only hydrated via - # InstanceNUMATopology.get_by_instance_uuid() but also hydrated by - # RequestSpec.get_by_instance_uuid() indirectly. However the - # latter code patch does not call InstanceNUMATopology.obj_from_db_obj() - # that triggers the data migration via - # InstanceNUMATopology._migrate_legacy_dedicated_instance_cpuset. - # This causes that when the new nova code loads an old RequestSpec object - # from the DB (e.g. during migration of an instance) the - # InstanceNUMATopology in the RequestSpec will not be migrated to the new - # object version and it will lead to errors when the pcpuset field is read - # during scheduling. - @mock.patch( - 'nova.objects.instance_numa.InstanceNUMATopology.' - '_migrate_legacy_dedicated_instance_cpuset', - new=mock.NonCallableMock() - ) + @mock.patch('nova.objects.request_spec.RequestSpec.save') @mock.patch.object( request_spec.RequestSpec, '_get_by_instance_uuid_from_db') @mock.patch('nova.objects.InstanceGroup.get_by_uuid') def test_get_by_instance_uuid_numa_topology_migration( - self, mock_get_ig, get_by_uuid + self, mock_get_ig, get_by_uuid, mock_save ): # Simulate a pre-Victoria RequestSpec where the pcpuset field is not # defined for the embedded InstanceNUMACell objects but the cpu_policy @@ -665,18 +648,10 @@ class _TestRequestSpecObject(object): self.context, fake_spec['instance_uuid']) self.assertEqual(2, len(req_obj.numa_topology.cells)) + self.assertEqual({1, 2}, req_obj.numa_topology.cells[0].pcpuset) + self.assertEqual({3, 4}, req_obj.numa_topology.cells[1].pcpuset) - # This is bug 1952941 as the pcpuset is not defined in object as the - # object is not migrated - ex = self.assertRaises( - NotImplementedError, - lambda: req_obj.numa_topology.cells[0].pcpuset - ) - self.assertIn("Cannot load 'pcpuset' in the base class", str(ex)) - - # This is the expected behavior - # self.assertEqual({1, 2}, req_obj.numa_topology.cells[0].pcpuset) - # self.assertEqual({3, 4}, req_obj.numa_topology.cells[1].pcpuset) + mock_save.assert_called_once() def _check_update_primitive(self, req_obj, changes): self.assertEqual(req_obj.instance_uuid, changes['instance_uuid']) diff --git a/releasenotes/notes/bug-1952941-request-spec-numa-topology-migration-c97dbd51b3c6c116.yaml b/releasenotes/notes/bug-1952941-request-spec-numa-topology-migration-c97dbd51b3c6c116.yaml new file mode 100644 index 000000000000..f582bef0c165 --- /dev/null +++ b/releasenotes/notes/bug-1952941-request-spec-numa-topology-migration-c97dbd51b3c6c116.yaml @@ -0,0 +1,9 @@ +--- +fixes: + - | + The `bug 1952941`_ is fixed where a pre-Victoria server with pinned + CPUs cannot be migrated or evacuated after the cloud is upgraded to + Victoria or newer as the scheduling fails with + ``NotImplementedError: Cannot load 'pcpuset'`` error. + + .. _bug 1952941: https://bugs.launchpad.net/nova/+bug/1952941