Insure resource tracker updated for deleted instances

Instances left in a deleted state at compute manager startup were
not being explicitly cleaned up in the resoruce tracker. To address
this _update_resource_tracker is now called from solely the
_complete_deletion method. That method is called both by the
_init_instance and _delete_instance code paths, both of which should
be doing cleanup in the resource tracker when the vm is DELETED.

Note that this change does mean that if _delete_instance has an
exception, _update_resource_tracker will not be called. This is
considered okay because we cannot guarantee at which stage the
exception happened and the state of the resources involved. For safety
it is better to err on the side of "in use".

Change-Id: I586c0305ea0e20d98697bc9173ca0f89dc2b228c
Related-Bug: #1227925

Author: cdent

nova/compute/manager.py

@@ -918,6 +918,7 @@ def _complete_deletion(self, context, instance, bdms,
         for bdm in bdms:
             bdm.destroy()
 
+        self._update_resource_tracker(context, instance)
         self._notify_about_instance_usage(context, instance, "delete.end",
                 system_metadata=system_meta)
 
@@ -2433,7 +2434,6 @@ def _delete_instance(self, context, instance, bdms, quotas):
             instance.power_state = power_state.NOSTATE
             instance.terminated_at = timeutils.utcnow()
             instance.save()
-            self._update_resource_tracker(context, instance)
             system_meta = instance.system_metadata
             instance.destroy()
         except Exception:

nova/tests/unit/compute/test_compute.py

@@ -6817,6 +6817,46 @@ def fake_destroy():
 
         self.assertNotEqual(0, instance.deleted)
 
+    def test_terminate_instance_updates_tracker(self):
+        rt = self.compute._get_resource_tracker(NODENAME)
+        admin_context = context.get_admin_context()
+
+        self.assertEqual(0, rt.compute_node.vcpus_used)
+        instance = self._create_fake_instance_obj()
+        instance.vcpus = 1
+
+        rt.instance_claim(admin_context, instance)
+        self.assertEqual(1, rt.compute_node.vcpus_used)
+
+        self.compute.terminate_instance(admin_context, instance, [], [])
+        self.assertEqual(0, rt.compute_node.vcpus_used)
+
+    @mock.patch('nova.compute.manager.ComputeManager'
+                '._notify_about_instance_usage')
+    @mock.patch('nova.objects.Quotas.reserve')
+    # NOTE(cdent): At least in this test destroy() on the instance sets it
+    # state back to active, meaning the resource tracker won't
+    # update properly.
+    @mock.patch('nova.objects.Instance.destroy')
+    def test_init_deleted_instance_updates_tracker(self, noop1, noop2, noop3):
+        rt = self.compute._get_resource_tracker(NODENAME)
+        admin_context = context.get_admin_context()
+
+        self.assertEqual(0, rt.compute_node.vcpus_used)
+        instance = self._create_fake_instance_obj()
+        instance.vcpus = 1
+
+        self.assertEqual(0, rt.compute_node.vcpus_used)
+
+        rt.instance_claim(admin_context, instance)
+        self.compute._init_instance(admin_context, instance)
+        self.assertEqual(1, rt.compute_node.vcpus_used)
+
+        instance.vm_state = vm_states.DELETED
+        self.compute._init_instance(admin_context, instance)
+
+        self.assertEqual(0, rt.compute_node.vcpus_used)
+
     def test_init_instance_for_partial_deletion(self):
         admin_context = context.get_admin_context()
         instance = objects.Instance(admin_context)