diff --git a/nova/objects/instance_numa.py b/nova/objects/instance_numa.py index acf387534813..2ece54a38938 100644 --- a/nova/objects/instance_numa.py +++ b/nova/objects/instance_numa.py @@ -198,26 +198,78 @@ class InstanceNUMATopology(base.NovaObject, # 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 - # 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: + + 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): 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 diff --git a/nova/tests/unit/objects/test_instance_numa.py b/nova/tests/unit/objects/test_instance_numa.py index c99431e56783..bd742e058ac1 100644 --- a/nova/tests/unit/objects/test_instance_numa.py +++ b/nova/tests/unit/objects/test_instance_numa.py @@ -363,6 +363,7 @@ class _TestInstanceNUMATopology(object): 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 @@ class _TestInstanceNUMATopology(object): 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 @@ class _TestInstanceNUMATopology(object): 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 @@ class _TestInstanceNUMATopology(object): # 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) diff --git a/nova/tests/unit/objects/test_request_spec.py b/nova/tests/unit/objects/test_request_spec.py index 58b985923400..fd2f4c38d14f 100644 --- a/nova/tests/unit/objects/test_request_spec.py +++ b/nova/tests/unit/objects/test_request_spec.py @@ -722,6 +722,9 @@ class _TestRequestSpecObject(object): 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)