From a298455398eb71f81a65782266450dcec444e56b Mon Sep 17 00:00:00 2001 From: Nikola Dipanov Date: Fri, 28 Aug 2015 23:23:02 +0100 Subject: [PATCH] compute: migrate/resize paths properly handle stashed numa_topology This patch makes the migrate and resize code paths in the compute service take advantage of the now stashed data in the context_migration field of the instance to apply the claimed NUMA topology to the instance and provision it (and to potentially revert back to the old one on the source host) Change-Id: Ib91f211e87c1770c1997b3b8ff01d55092c896bf Partial-bug: #1417667 Related-blueprint: migration-fix-resource-tracking --- nova/compute/manager.py | 11 ++++++ nova/tests/unit/compute/test_compute.py | 38 +++++++++++++++++++-- nova/tests/unit/compute/test_compute_mgr.py | 4 ++- 3 files changed, 50 insertions(+), 3 deletions(-) diff --git a/nova/compute/manager.py b/nova/compute/manager.py index 7f518a8d42ea..02135c3c3ab1 100644 --- a/nova/compute/manager.py +++ b/nova/compute/manager.py @@ -3448,6 +3448,16 @@ class ComputeManager(manager.Manager): with migration.obj_as_admin(): migration.save() + # NOTE(ndipanov): We need to do this here because dropping the + # claim means we lose the migration_context data. We really should + # fix this by moving the drop_move_claim call to the + # finish_revert_resize method as this is racy (revert is dropped, + # but instance resources will be tracked with the new flavor until + # it gets rolled back in finish_revert_resize, which is + # potentially wrong for a period of time). + instance.revert_migration_context() + instance.save() + rt = self._get_resource_tracker(instance.node) rt.drop_move_claim(context, instance) @@ -3788,6 +3798,7 @@ class ComputeManager(manager.Manager): if old_instance_type[key] != instance_type[key]: resize_instance = True break + instance.apply_migration_context() # NOTE(tr3buchet): setup networks on destination host self.network_api.setup_networks_on_host(context, instance, diff --git a/nova/tests/unit/compute/test_compute.py b/nova/tests/unit/compute/test_compute.py index 7e56d5fcbf7d..87ab90d2a924 100644 --- a/nova/tests/unit/compute/test_compute.py +++ b/nova/tests/unit/compute/test_compute.py @@ -84,6 +84,7 @@ from nova.tests.unit import fake_server_actions from nova.tests.unit.image import fake as fake_image from nova.tests.unit import matchers from nova.tests.unit.objects import test_flavor +from nova.tests.unit.objects import test_instance_numa_topology from nova.tests.unit.objects import test_migration from nova.tests.unit import utils as test_utils from nova import utils @@ -5148,7 +5149,7 @@ class ComputeTestCase(BaseTestCase): def test_resize_instance_forced_shutdown(self): self._test_resize_instance(clean_shutdown=False) - def _test_confirm_resize(self, power_on): + def _test_confirm_resize(self, power_on, numa_topology=None): # Common test case method for confirm_resize def fake(*args, **kwargs): pass @@ -5188,6 +5189,7 @@ class ComputeTestCase(BaseTestCase): instance.vm_state = old_vm_state instance.power_state = p_state + instance.numa_topology = numa_topology instance.save() new_instance_type_ref = flavors.get_flavor_by_flavor_id(3) @@ -5200,6 +5202,11 @@ class ComputeTestCase(BaseTestCase): migration = objects.Migration.get_by_instance_and_status( self.context.elevated(), instance.uuid, 'pre-migrating') + migration_context = objects.MigrationContext.get_by_instance_uuid( + self.context.elevated(), instance.uuid) + self.assertIsInstance(migration_context.old_numa_topology, + numa_topology.__class__) + self.assertIsNone(migration_context.new_numa_topology) # NOTE(mriedem): ensure prep_resize set old_vm_state in system_metadata sys_meta = instance.system_metadata @@ -5220,6 +5227,9 @@ class ComputeTestCase(BaseTestCase): instance_type_ref = db.flavor_get(self.context, instance.instance_type_id) self.assertEqual(instance_type_ref['flavorid'], '3') + # Prove that the NUMA topology has also been updated to that of the new + # flavor - meaning None + self.assertIsNone(instance.numa_topology) # Finally, confirm the resize and verify the new flavor is applied instance.task_state = None @@ -5236,6 +5246,7 @@ class ComputeTestCase(BaseTestCase): self.assertEqual('fake-mini', migration.source_compute) self.assertEqual(old_vm_state, instance.vm_state) self.assertIsNone(instance.task_state) + self.assertIsNone(instance.migration_context) self.assertEqual(p_state, instance.power_state) self.compute.terminate_instance(self.context, instance, [], []) @@ -5245,8 +5256,15 @@ class ComputeTestCase(BaseTestCase): def test_confirm_resize_from_stopped(self): self._test_confirm_resize(power_on=False) + def test_confirm_resize_with_migration_context(self): + numa_topology = ( + test_instance_numa_topology.get_fake_obj_numa_topology( + self.context)) + self._test_confirm_resize(power_on=True, numa_topology=numa_topology) + def _test_finish_revert_resize(self, power_on, - remove_old_vm_state=False): + remove_old_vm_state=False, + numa_topology=None): """Convenience method that does most of the work for the test_finish_revert_resize tests. :param power_on -- True if testing resize from ACTIVE state, False if @@ -5291,6 +5309,7 @@ class ComputeTestCase(BaseTestCase): instance.host = 'foo' instance.vm_state = old_vm_state + instance.numa_topology = numa_topology instance.save() new_instance_type_ref = flavors.get_flavor_by_flavor_id(3) @@ -5304,6 +5323,10 @@ class ComputeTestCase(BaseTestCase): migration = objects.Migration.get_by_instance_and_status( self.context.elevated(), instance.uuid, 'pre-migrating') + migration_context = objects.MigrationContext.get_by_instance_uuid( + self.context.elevated(), instance.uuid) + self.assertIsInstance(migration_context.old_numa_topology, + numa_topology.__class__) # NOTE(mriedem): ensure prep_resize set old_vm_state in system_metadata sys_meta = instance.system_metadata @@ -5323,6 +5346,9 @@ class ComputeTestCase(BaseTestCase): # Prove that the instance size is now the new size instance_type_ref = flavors.get_flavor_by_flavor_id(3) self.assertEqual(instance_type_ref['flavorid'], '3') + # Prove that the NUMA topology has also been updated to that of the new + # flavor - meaning None + self.assertIsNone(instance.numa_topology) instance.task_state = task_states.RESIZE_REVERTING instance.save() @@ -5352,6 +5378,7 @@ class ComputeTestCase(BaseTestCase): self.assertEqual(instance_type_ref['flavorid'], '1') self.assertEqual(instance.host, migration.source_compute) self.assertEqual(migration.dest_compute, migration.source_compute) + self.assertIsInstance(instance.numa_topology, numa_topology.__class__) if remove_old_vm_state: self.assertEqual(vm_states.ACTIVE, instance.vm_state) @@ -5371,6 +5398,13 @@ class ComputeTestCase(BaseTestCase): self._test_finish_revert_resize(power_on=False, remove_old_vm_state=True) + def test_finish_revert_resize_migration_context(self): + numa_topology = ( + test_instance_numa_topology.get_fake_obj_numa_topology( + self.context)) + self._test_finish_revert_resize(power_on=True, + numa_topology=numa_topology) + def _test_cleanup_stored_instance_types(self, old, new, revert=False): instance = self._create_fake_instance_obj() instance.system_metadata = dict(instance_type_id=old) diff --git a/nova/tests/unit/compute/test_compute_mgr.py b/nova/tests/unit/compute/test_compute_mgr.py index accf805c4faf..8dc65d695825 100644 --- a/nova/tests/unit/compute/test_compute_mgr.py +++ b/nova/tests/unit/compute/test_compute_mgr.py @@ -3972,6 +3972,7 @@ class ComputeManagerMigrationTestCase(test.NoDBTestCase): # revert_resize() and the return value is passed to driver.destroy(). # Otherwise we could regress this. + @mock.patch.object(self.instance, 'revert_migration_context') @mock.patch.object(self.compute.network_api, 'get_instance_nw_info') @mock.patch.object(self.compute, '_is_instance_storage_shared') @mock.patch.object(self.compute, 'finish_revert_resize') @@ -3994,7 +3995,8 @@ class ComputeManagerMigrationTestCase(test.NoDBTestCase): _instance_update, finish_revert_resize, _is_instance_storage_shared, - get_instance_nw_info): + get_instance_nw_info, + revert_migration_context): self.migration.source_compute = self.instance['host']