Claims: Make sure move claims create a migration context records

This way we will be able to keep track of the resources that were
actually claimed for those resources (such as NUMA topology) which are
not just a simple sum but actually have state.

This puts in the infra for tracking the claimed resources, however 2
additional steps are needed to fully use this and make resource tracking
work for all instances when migrating:

1) We don't use the resources claimed and stashed in the Migration
context to do resource tracking. This will be a follow-on fix in the
resource tracker.

2) We still don't properly assign the newly claimed topology to the
instance once the migration is confirmed - this will be done in a follow
up patch that builds on this work.

Change-Id: Ib0532f0cbebc5568d9b94b74b7a1555fcf871df5
Partial-bug: #1417667
Related-blueprint: migration-fix-resource-tracking

nova/compute/claims.py

@@ -293,3 +293,17 @@ class MoveClaim(Claim):
             self.context,
             self.instance, instance_type=self.instance_type,
             image_meta=self.image_meta)
+
+    def create_migration_context(self):
+        if not self.migration:
+            # FIXME(ndipanov): Move this to a LOG.warn once Mitaka opens up
+            LOG.debug("Can't create a migration_context record without a "
+                      "migration object specified.")
+            return
+
+        mig_context = objects.MigrationContext(
+            context=self.context, instance_uuid=self.instance.uuid,
+            migration_id=self.migration.id,
+            old_numa_topology=self.instance.numa_topology,
+            new_numa_topology=self.claimed_numa_topology)
+        return mig_context

nova/compute/resource_tracker.py

@@ -238,6 +238,8 @@ class ResourceTracker(object):
                                  image_meta, self, self.compute_node,
                                  overhead=overhead, limits=limits)
         claim.migration = migration
+        instance.migration_context = claim.create_migration_context()
+        instance.save()
 
         # Mark the resources in-use for the resize landing on this
         # compute host:
@@ -339,6 +341,8 @@ class ResourceTracker(object):
                 ctxt = context.elevated()
                 self._update(ctxt)
 
+            instance.drop_migration_context()
+
     @utils.synchronized(COMPUTE_RESOURCE_SEMAPHORE)
     def update_usage(self, context, instance):
         """Update the resource usage and stats after a change in an

nova/tests/unit/compute/test_claims.py

@@ -415,3 +415,25 @@ class MoveClaimTestCase(ClaimTestCase):
     def test_abort(self, mock_get):
         claim = self._abort()
         self.assertTrue(claim.tracker.rcalled)
+
+    @mock.patch('nova.objects.InstancePCIRequests.get_by_instance_uuid',
+                return_value=objects.InstancePCIRequests(requests=[]))
+    def test_create_migration_context(self, mock_get):
+        numa_topology = objects.InstanceNUMATopology(
+                cells=[objects.InstanceNUMACell(
+                    id=1, cpuset=set([1, 2]), memory=512)])
+        self.instance.numa_topology = None
+        claim = self._claim(numa_topology=numa_topology)
+        migration = objects.Migration(context=self.context, id=42)
+        claim.migration = migration
+        fake_mig_context = mock.Mock(spec=objects.MigrationContext)
+        with mock.patch('nova.objects.MigrationContext',
+                        return_value=fake_mig_context) as ctxt_mock:
+            claim.create_migration_context()
+            ctxt_mock.assert_called_once_with(
+                context=self.context, instance_uuid=self.instance.uuid,
+                migration_id=42, old_numa_topology=None,
+                new_numa_topology=mock.ANY)
+            self.assertIsInstance(ctxt_mock.call_args[1]['new_numa_topology'],
+                                  objects.InstanceNUMATopology)
+            self.assertEqual(migration, claim.migration)

nova/tests/unit/compute/test_resource_tracker.py

@@ -1090,9 +1090,10 @@ class _MoveClaimTestCase(BaseTrackerTestCase):
         self.instance_type = self._fake_flavor_create()
         self.claim_method = self.tracker._move_claim
 
+    @mock.patch('nova.objects.Instance.save')
     @mock.patch('nova.objects.InstancePCIRequests.get_by_instance_uuid',
                 return_value=objects.InstancePCIRequests(requests=[]))
-    def test_abort(self, mock_get):
+    def test_abort(self, mock_get, mock_save):
         try:
             with self.claim_method(self.context, self.instance,
                     self.instance_type, limits=self.limits):
@@ -1104,10 +1105,12 @@ class _MoveClaimTestCase(BaseTrackerTestCase):
         self._assert(0, 'local_gb_used')
         self._assert(0, 'vcpus_used')
         self.assertEqual(0, len(self.tracker.tracked_migrations))
+        mock_save.assert_called_once_with()
 
+    @mock.patch('nova.objects.Instance.save')
     @mock.patch('nova.objects.InstancePCIRequests.get_by_instance_uuid',
                 return_value=objects.InstancePCIRequests(requests=[]))
-    def test_additive_claims(self, mock_get):
+    def test_additive_claims(self, mock_get, mock_save):
 
         limits = self._limits(
               2 * FAKE_VIRT_MEMORY_WITH_OVERHEAD,
@@ -1115,20 +1118,25 @@ class _MoveClaimTestCase(BaseTrackerTestCase):
               2 * FAKE_VIRT_VCPUS)
         self.claim_method(
             self.context, self.instance, self.instance_type, limits=limits)
+        mock_save.assert_called_once_with()
+        mock_save.reset_mock()
         instance2 = self._fake_instance_obj()
         self.claim_method(
             self.context, instance2, self.instance_type, limits=limits)
+        mock_save.assert_called_once_with()
 
         self._assert(2 * FAKE_VIRT_MEMORY_WITH_OVERHEAD, 'memory_mb_used')
         self._assert(2 * FAKE_VIRT_LOCAL_GB, 'local_gb_used')
         self._assert(2 * FAKE_VIRT_VCPUS, 'vcpus_used')
 
+    @mock.patch('nova.objects.Instance.save')
     @mock.patch('nova.objects.InstancePCIRequests.get_by_instance_uuid',
                 return_value=objects.InstancePCIRequests(requests=[]))
-    def test_revert(self, mock_get):
+    def test_revert(self, mock_get, mock_save):
         self.claim_method(
             self.context, self.instance, self.instance_type,
             image_meta={}, limits=self.limits)
+        mock_save.assert_called_once_with()
         self.tracker.drop_move_claim(self.context, self.instance)
 
         self.assertEqual(0, len(self.tracker.tracked_instances))
@@ -1137,20 +1145,23 @@ class _MoveClaimTestCase(BaseTrackerTestCase):
         self._assert(0, 'local_gb_used')
         self._assert(0, 'vcpus_used')
 
+    @mock.patch('nova.objects.Instance.save')
     @mock.patch('nova.objects.InstancePCIRequests.get_by_instance_uuid',
                 return_value=objects.InstancePCIRequests(requests=[]))
-    def test_move_type_not_tracked(self, mock_get):
+    def test_move_type_not_tracked(self, mock_get, mock_save):
         self.claim_method(self.context, self.instance,
                 self.instance_type, limits=self.limits, move_type="evacuation")
+        mock_save.assert_called_once_with()
 
         self._assert(0, 'memory_mb_used')
         self._assert(0, 'local_gb_used')
         self._assert(0, 'vcpus_used')
         self.assertEqual(0, len(self.tracker.tracked_migrations))
 
+    @mock.patch('nova.objects.Instance.save')
     @mock.patch.object(objects.Migration, 'save')
-    def test_existing_migration(self, save_mock):
+    def test_existing_migration(self, save_mock, save_inst_mock):
-        migration = objects.Migration(self.context,
+        migration = objects.Migration(self.context, id=42,
                                       instance_uuid=self.instance.uuid,
                                       status='accepted',
                                       migration_type='evacuation')
@@ -1161,6 +1172,7 @@ class _MoveClaimTestCase(BaseTrackerTestCase):
         self.assertEqual("pre-migrating", migration.status)
         self.assertEqual(0, len(self.tracker.tracked_migrations))
         save_mock.assert_called_once_with()
+        save_inst_mock.assert_called_once_with()
 
 
 class ResizeClaimTestCase(_MoveClaimTestCase):

nova/tests/unit/compute/test_tracker.py

@@ -304,6 +304,19 @@ _MIGRATION_INSTANCE_FIXTURES = {
     ),
 }
 
+_MIGRATION_CONTEXT_FIXTURES = {
+    'f4f0bfea-fe7e-4264-b598-01cb13ef1997': objects.MigrationContext(
+        instance_uuid='f4f0bfea-fe7e-4264-b598-01cb13ef1997',
+        migration_id=3,
+        new_numa_topology=None,
+        old_numa_topology=None),
+    'c17741a5-6f3d-44a8-ade8-773dc8c29124': objects.MigrationContext(
+        instance_uuid='c17741a5-6f3d-44a8-ade8-773dc8c29124',
+        migration_id=3,
+        new_numa_topology=None,
+        old_numa_topology=None),
+}
+
 
 def overhead_zero(instance):
     # Emulate that the driver does not adjust the memory
@@ -1163,6 +1176,7 @@ class TestInstanceClaim(BaseTestCase):
             self.assertEqualNUMAHostTopology(expected_numa, new_numa)
 
 
+@mock.patch('nova.objects.Instance.save')
 @mock.patch('nova.objects.MigrationList.get_in_progress_by_host_and_node')
 @mock.patch('nova.objects.Instance.get_by_uuid')
 @mock.patch('nova.objects.InstanceList.get_by_host_and_node')
@@ -1195,11 +1209,12 @@ class TestMoveClaim(BaseTestCase):
             self.rt.update_available_resource(self.ctx)
 
     def register_mocks(self, pci_mock, inst_list_mock, inst_by_uuid,
-            migr_mock):
+            migr_mock, inst_save_mock):
         pci_mock.return_value = objects.InstancePCIRequests(requests=[])
         self.inst_list_mock = inst_list_mock
         self.inst_by_uuid = inst_by_uuid
         self.migr_mock = migr_mock
+        self.inst_save_mock = inst_save_mock
 
     def audit(self, rt, instances, migrations, migr_inst):
         self.inst_list_mock.return_value = \
@@ -1232,62 +1247,83 @@ class TestMoveClaim(BaseTestCase):
 
     @mock.patch('nova.objects.Flavor.get_by_id')
     def test_claim(self, flavor_mock, pci_mock, inst_list_mock, inst_by_uuid,
-            migr_mock):
+            migr_mock, inst_save_mock):
         """Resize self.instance and check that the expected quantities of each
         resource have been consumed.
         """
 
-        self.register_mocks(pci_mock, inst_list_mock, inst_by_uuid, migr_mock)
+        self.register_mocks(pci_mock, inst_list_mock, inst_by_uuid, migr_mock,
+                            inst_save_mock)
         self.driver_mock.get_host_ip_addr.return_value = "fake-ip"
         flavor_mock.return_value = objects.Flavor(**self.flavor)
+        mig_context_obj = _MIGRATION_CONTEXT_FIXTURES[self.instance.uuid]
 
         expected = copy.deepcopy(self.rt.compute_node)
         self.adjust_expected(expected, self.flavor)
 
-        with mock.patch.object(self.rt, '_create_migration') as migr_mock:
+        create_mig_mock = mock.patch.object(self.rt, '_create_migration')
+        mig_ctxt_mock = mock.patch('nova.objects.MigrationContext',
+                                   return_value=mig_context_obj)
+        with create_mig_mock as migr_mock, mig_ctxt_mock as ctxt_mock:
             migr_mock.return_value = _MIGRATION_FIXTURES['source-only']
             claim = self.rt.resize_claim(
                 self.ctx, self.instance, self.flavor, None)
+            self.assertEqual(1, ctxt_mock.call_count)
 
         self.assertIsInstance(claim, claims.MoveClaim)
+        inst_save_mock.assert_called_once_with()
         self.assertTrue(obj_base.obj_equal_prims(expected,
                                                  self.rt.compute_node))
 
     def test_same_host(self, pci_mock, inst_list_mock, inst_by_uuid,
-            migr_mock):
+            migr_mock, inst_save_mock):
         """Resize self.instance to the same host but with a different flavor.
         Then abort the claim. Check that the same amount of resources are
         available afterwards as we started with.
         """
 
-        self.register_mocks(pci_mock, inst_list_mock, inst_by_uuid, migr_mock)
+        self.register_mocks(pci_mock, inst_list_mock, inst_by_uuid, migr_mock,
+                            inst_save_mock)
         migr_obj = _MIGRATION_FIXTURES['source-and-dest']
         self.instance = _MIGRATION_INSTANCE_FIXTURES[migr_obj['instance_uuid']]
+        self.instance._context = self.ctx
+        mig_context_obj = _MIGRATION_CONTEXT_FIXTURES[self.instance.uuid]
 
         with mock.patch.object(self.instance, 'save'):
             self.rt.instance_claim(self.ctx, self.instance, None)
         expected = copy.deepcopy(self.rt.compute_node)
 
-        with mock.patch.object(self.rt, '_create_migration') as migr_mock:
+        create_mig_mock = mock.patch.object(self.rt, '_create_migration')
+        mig_ctxt_mock = mock.patch('nova.objects.MigrationContext',
+                                   return_value=mig_context_obj)
+
+        with create_mig_mock as migr_mock, mig_ctxt_mock as ctxt_mock:
             migr_mock.return_value = migr_obj
             claim = self.rt.resize_claim(self.ctx, self.instance,
                     _INSTANCE_TYPE_OBJ_FIXTURES[1], None)
+            self.assertEqual(1, ctxt_mock.call_count)
 
         self.audit(self.rt, [self.instance], [migr_obj], self.instance)
+        inst_save_mock.assert_called_once_with()
         self.assertNotEqual(expected, self.rt.compute_node)
 
-        claim.abort()
+        claim.instance.migration_context = mig_context_obj
-        self.assertTrue(obj_base.obj_equal_prims(expected,
+        with mock.patch('nova.objects.MigrationContext._destroy') as destroy_m:
-                                                 self.rt.compute_node))
+            claim.abort()
+            self.assertTrue(obj_base.obj_equal_prims(expected,
+                                                     self.rt.compute_node))
+            destroy_m.assert_called_once_with(self.ctx, claim.instance.uuid)
 
     def test_revert_reserve_source(
-            self, pci_mock, inst_list_mock, inst_by_uuid, migr_mock):
+            self, pci_mock, inst_list_mock, inst_by_uuid, migr_mock,
+            inst_save_mock):
         """Check that the source node of an instance migration reserves
         resources until the migration has completed, even if the migration is
         reverted.
         """
 
-        self.register_mocks(pci_mock, inst_list_mock, inst_by_uuid, migr_mock)
+        self.register_mocks(pci_mock, inst_list_mock, inst_by_uuid, migr_mock,
+                            inst_save_mock)
 
         # Get our migrations, instances and itypes in a row
         src_migr = _MIGRATION_FIXTURES['source-only']
@@ -1342,8 +1378,9 @@ class TestMoveClaim(BaseTestCase):
                                                  src_rt.compute_node))
 
     def test_dupe_filter(self, pci_mock, inst_list_mock, inst_by_uuid,
-            migr_mock):
+            migr_mock, inst_save_mock):
-        self.register_mocks(pci_mock, inst_list_mock, inst_by_uuid, migr_mock)
+        self.register_mocks(pci_mock, inst_list_mock, inst_by_uuid, migr_mock,
+                            inst_save_mock)
 
         migr_obj = _MIGRATION_FIXTURES['source-and-dest']
         # This is good enough to prevent a lazy-load; value is unimportant