From f0ca0f73b131838f1441ff4affae10fe78ff7150 Mon Sep 17 00:00:00 2001
From: Jim Rollenhagen <jim@jimrollenhagen.com>
Date: Thu, 4 Jun 2015 19:22:15 -0700
Subject: [PATCH] Ironic: Don't report resources for nodes without instances

Ironic falsely reports resources as consumed for certain states. Some
of these states are not associated with an instance, and the resource
tracker will notice that and "correct" the driver. This allows nodes
in CLEANING, etc. to be scheduled to.

Fix this by only reporting resources consumed when an instance UUID
is associated with the node. This association happens before the deploy
starts, and is removed after Nova sees tear down as complete, so it
should be safe to only look at the instance UUID for this. For any
"unavailable" state, report that there are zero resources available,
never allowing the node to be scheduled to.

Closes-Bug: #1462374
Change-Id: I48913f724a1bbe3711ea543a50b9ba096d5e95d7
---
 nova/tests/unit/virt/ironic/test_driver.py | 26 +++++++++++++---------
 nova/virt/ironic/driver.py                 | 13 +++++------
 2 files changed, 22 insertions(+), 17 deletions(-)

diff --git a/nova/tests/unit/virt/ironic/test_driver.py b/nova/tests/unit/virt/ironic/test_driver.py
index dfa6b5285c26..47d08e0fcae9 100644
--- a/nova/tests/unit/virt/ironic/test_driver.py
+++ b/nova/tests/unit/virt/ironic/test_driver.py
@@ -509,9 +509,21 @@ class IronicDriverTestCase(test.NoDBTestCase):
              'power_state': ironic_states.NOSTATE,
              'provision_state': ironic_states.AVAILABLE},
             # a node not in maintenance or bad power state, bad provision state
-            {'uuid': uuidutils.generate_uuid,
+            {'uuid': uuidutils.generate_uuid(),
              'power_state': ironic_states.POWER_ON,
-             'provision_state': ironic_states.MANAGEABLE}
+             'provision_state': ironic_states.MANAGEABLE},
+            # a node in cleaning
+            {'uuid': uuidutils.generate_uuid(),
+             'power_state': ironic_states.POWER_ON,
+             'provision_state': ironic_states.CLEANING},
+            # a node in deleting
+            {'uuid': uuidutils.generate_uuid(),
+             'power_state': ironic_states.POWER_ON,
+             'provision_state': ironic_states.DELETING},
+            # a node in deleted
+            {'uuid': uuidutils.generate_uuid(),
+             'power_state': ironic_states.POWER_ON,
+             'provision_state': ironic_states.DELETED}
         ]
         for n in node_dicts:
             node = ironic_utils.get_test_node(**n)
@@ -531,20 +543,14 @@ class IronicDriverTestCase(test.NoDBTestCase):
             {'uuid': uuidutils.generate_uuid(),
              'instance_uuid': uuidutils.generate_uuid(),
              'provision_state': ironic_states.ACTIVE},
-            # a node in deploying but no instance yet
-            {'uuid': uuidutils.generate_uuid(),
-             'provision_state': ironic_states.DEPLOYWAIT},
-            # a node that made it to cleaning before losing its instance uuid
-            {'uuid': uuidutils.generate_uuid,
-            'instance_uuid': uuidutils.generate_uuid(),
-            'provision_state': ironic_states.CLEANING},
         ]
         for n in node_dicts:
             node = ironic_utils.get_test_node(**n)
             self.assertTrue(self.driver._node_resources_used(node))
 
         unused_node = ironic_utils.get_test_node(
-            power_state=ironic_states.AVAILABLE)
+            instance_uuid=None,
+            provision_state=ironic_states.AVAILABLE)
         self.assertFalse(self.driver._node_resources_used(unused_node))
 
     @mock.patch.object(FAKE_CLIENT.node, 'list')
diff --git a/nova/virt/ironic/driver.py b/nova/virt/ironic/driver.py
index a342e4ea982b..1a284691bfed 100644
--- a/nova/virt/ironic/driver.py
+++ b/nova/virt/ironic/driver.py
@@ -222,14 +222,13 @@ class IronicDriver(virt_driver.ComputeDriver):
         a new instance on the node, has an instance on the node, or is in
         the process of cleaning up from a deleted instance. Returns True if
         used.
+
+        If we report resources as consumed for a node that does not have an
+        instance on it, the resource tracker will notice there's no instances
+        consuming resources and try to correct us. So only nodes with an
+        instance attached should report as consumed here.
         """
-        used_provision_states = [
-            ironic_states.CLEANING, ironic_states.DEPLOYING,
-            ironic_states.DEPLOYWAIT, ironic_states.DEPLOYDONE,
-            ironic_states.ACTIVE, ironic_states.DELETING,
-            ironic_states.DELETED]
-        return (node_obj.instance_uuid is not None or
-                node_obj.provision_state in used_provision_states)
+        return node_obj.instance_uuid is not None
 
     def _node_resource(self, node):
         """Helper method to create resource dict from node stats."""