Merge "rebuild: Apply migration context before calling the driver"

nova/compute/manager.py

@@ -2597,9 +2597,10 @@ class ComputeManager(manager.Manager):
         instance.save(
             expected_task_state=[task_states.REBUILD_BLOCK_DEVICE_MAPPING])
 
-        self.driver.spawn(context, instance, image_meta, injected_files,
-                          admin_password, network_info=network_info,
-                          block_device_info=new_block_device_info)
+        with instance.mutated_migration_context():
+            self.driver.spawn(context, instance, image_meta, injected_files,
+                              admin_password, network_info=network_info,
+                              block_device_info=new_block_device_info)
 
     @messaging.expected_exceptions(exception.PreserveEphemeralNotSupported)
     @wrap_exception()
@@ -2821,7 +2822,8 @@ class ComputeManager(manager.Manager):
             preserve_ephemeral=preserve_ephemeral,
             recreate=recreate)
         try:
-            self.driver.rebuild(**kwargs)
+            with instance.mutated_migration_context():
+                self.driver.rebuild(**kwargs)
         except NotImplementedError:
             # NOTE(rpodolyaka): driver doesn't provide specialized version
             # of rebuild, fall back to the default implementation

nova/objects/instance.py

@@ -823,6 +823,21 @@ class _BaseInstance(base.NovaPersistentObject, base.NovaObject,
                          "seem to be set for this instance"),
                          instance=self)
 
+    @contextlib.contextmanager
+    def mutated_migration_context(self):
+        """Context manager to temporarily apply the migration context.
+
+        Calling .save() from within the context manager means that the mutated
+        context will be saved which can cause incorrect resource tracking, and
+        should be avoided.
+        """
+        current_numa_topo = self.numa_topology
+        self.apply_migration_context()
+        try:
+            yield
+        finally:
+            self.numa_topology = current_numa_topo
+
     @base.remotable
     def drop_migration_context(self):
         if self.migration_context:

nova/tests/unit/compute/test_compute.py

@@ -11497,8 +11497,16 @@ class EvacuateHostTestCase(BaseTestCase):
         numa_topology = (
             test_instance_numa_topology.get_fake_obj_numa_topology(
                 self.context))
+
+        # NOTE(ndipanov): Make sure that we pass the topology from the context
+        def fake_spawn(context, instance, image_meta, injected_files,
+                       admin_password, network_info=None,
+                       block_device_info=None):
+            self.assertIsNone(instance.numa_topology)
+
         self.inst.numa_topology = numa_topology
-        patch_spawn = mock.patch.object(self.compute.driver, 'spawn')
+        patch_spawn = mock.patch.object(self.compute.driver, 'spawn',
+                                        side_effect=fake_spawn)
         patch_on_disk = mock.patch.object(
             self.compute.driver, 'instance_on_disk', return_value=True)
         with patch_spawn, patch_on_disk:

nova/tests/unit/compute/test_compute_mgr.py

@@ -2681,6 +2681,8 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase):
              mock_power_off
         ):
             instance = fake_instance.fake_instance_obj(self.context)
+            instance.migration_context = None
+            instance.numa_topology = None
             instance.task_state = task_states.REBUILDING
             instance.save(expected_task_state=[task_states.REBUILDING])
             self.compute._rebuild_default_impl(self.context,

nova/tests/unit/objects/test_instance.py

@@ -1216,6 +1216,21 @@ class _TestInstanceObject(object):
             update_extra.assert_called_once_with(self.context, inst.uuid,
                                                  {"migration_context": None})
 
+    def test_mutated_migration_context(self):
+        numa_topology = (test_instance_numa_topology.
+                            fake_obj_numa_topology.obj_clone())
+        numa_topology.cells[0].memory = 1024
+        numa_topology.cells[1].memory = 1024
+
+        inst = instance.Instance(context=self.context,
+                                 uuid='fake-uuid', numa_topology=numa_topology)
+        inst.migration_context = test_mig_ctxt.get_fake_migration_context_obj(
+            self.context)
+        with inst.mutated_migration_context():
+            self.assertIs(inst.numa_topology,
+                          inst.migration_context.new_numa_topology)
+        self.assertIs(numa_topology, inst.numa_topology)
+
     @mock.patch.object(objects.Instance, 'get_by_uuid')
     def test_load_generic(self, mock_get):
         inst2 = instance.Instance(metadata={'foo': 'bar'})