Merge "Do not untrack resources of a server being unshelved" into stable/zed

Author: Zuul

nova/compute/manager.py

@@ -6809,9 +6809,9 @@ def _shelve_offload_instance(self, context, instance, clean_shutdown,
 
         instance.power_state = current_power_state
         # NOTE(mriedem): The vm_state has to be set before updating the
-        # resource tracker, see vm_states.ALLOW_RESOURCE_REMOVAL. The host/node
-        # values cannot be nulled out until after updating the resource tracker
-        # though.
+        # resource tracker, see vm_states.allow_resource_removal(). The
+        # host/node values cannot be nulled out until after updating the
+        # resource tracker though.
         instance.vm_state = vm_states.SHELVED_OFFLOADED
         instance.task_state = None
         instance.save(expected_task_state=[task_states.SHELVING,

nova/compute/resource_tracker.py

@@ -1546,7 +1546,8 @@ def _update_usage_from_instance(self, context, instance, nodename,
         # NOTE(sfinucan): Both brand new instances as well as instances that
         # are being unshelved will have is_new_instance == True
         is_removed_instance = not is_new_instance and (is_removed or
-            instance['vm_state'] in vm_states.ALLOW_RESOURCE_REMOVAL)
+            vm_states.allow_resource_removal(
+                vm_state=instance['vm_state'], task_state=instance.task_state))
 
         if is_new_instance:
             self.tracked_instances.add(uuid)
@@ -1605,7 +1606,9 @@ def _update_usage_from_instances(self, context, instances, nodename):
 
         instance_by_uuid = {}
         for instance in instances:
-            if instance.vm_state not in vm_states.ALLOW_RESOURCE_REMOVAL:
+            if not vm_states.allow_resource_removal(
+                    vm_state=instance['vm_state'],
+                    task_state=instance.task_state):
                 self._update_usage_from_instance(context, instance, nodename)
             instance_by_uuid[instance.uuid] = instance
         return instance_by_uuid

nova/compute/stats.py

@@ -105,7 +105,8 @@ def update_stats_for_instance(self, instance, is_removed=False):
         (vm_state, task_state, os_type, project_id) = \
                 self._extract_state_from_instance(instance)
 
-        if is_removed or vm_state in vm_states.ALLOW_RESOURCE_REMOVAL:
+        if is_removed or vm_states.allow_resource_removal(
+                vm_state=vm_state, task_state=task_state):
             self._decrement("num_instances")
             self.states.pop(uuid)
         else:

nova/compute/vm_states.py

@@ -27,6 +27,7 @@
 See http://wiki.openstack.org/VMState
 """
 
+from nova.compute import task_states
 from nova.objects import fields
 
 
@@ -74,5 +75,11 @@
 # states we allow to trigger crash dump
 ALLOW_TRIGGER_CRASH_DUMP = [ACTIVE, PAUSED, RESCUED, RESIZED, ERROR]
 
-# states we allow resources to be freed in
-ALLOW_RESOURCE_REMOVAL = [DELETED, SHELVED_OFFLOADED]
+
+def allow_resource_removal(vm_state, task_state=None):
+    """(vm_state, task_state) combinations we allow resources to be freed in"""
+
+    return (
+        vm_state == DELETED or
+        vm_state == SHELVED_OFFLOADED and task_state != task_states.SPAWNING
+    )

nova/tests/functional/regressions/test_bug_2025480.py

@@ -82,6 +82,5 @@ def fake_spawn(*args, **kwargs):
 
         node = compute_node.ComputeNode.get_by_nodename(
             context.get_admin_context(), 'compute1')
-        # This is the bug, the instance should have resources claimed
-        # self.assertEqual(1, node.vcpus_used)
-        self.assertEqual(0, node.vcpus_used)
+        # After the fix, the instance should have resources claimed
+        self.assertEqual(1, node.vcpus_used)

nova/tests/unit/compute/test_stats.py

@@ -208,6 +208,22 @@ def test_update_stats_for_instance_offloaded(self):
         self.assertEqual(0, self.stats.num_os_type("Linux"))
         self.assertEqual(0, self.stats["num_vm_" + vm_states.BUILDING])
 
+    def test_update_stats_for_instance_being_unshelved(self):
+        instance = self._create_instance()
+        self.stats.update_stats_for_instance(instance)
+        self.assertEqual(1, self.stats.num_instances_for_project("1234"))
+
+        instance["vm_state"] = vm_states.SHELVED_OFFLOADED
+        instance["task_state"] = task_states.SPAWNING
+        self.stats.update_stats_for_instance(instance)
+
+        self.assertEqual(1, self.stats.num_instances)
+        self.assertEqual(1, self.stats.num_instances_for_project(1234))
+        self.assertEqual(1, self.stats["num_os_type_Linux"])
+        self.assertEqual(1, self.stats["num_vm_%s" %
+                                       vm_states.SHELVED_OFFLOADED])
+        self.assertEqual(1, self.stats["num_task_%s" % task_states.SPAWNING])
+
     def test_io_workload(self):
         vms = [vm_states.ACTIVE, vm_states.BUILDING, vm_states.PAUSED]
         tasks = [task_states.RESIZE_MIGRATING, task_states.REBUILDING,