Ironic: Fix bad capacity reporting if instance_info is unset

If node.instance_info is unset for a node that has an instance on it
(for example, when upgrading Nova from before the patch that set these
values on spawn), the Ironic driver reports 0 for both $resource and
$resource_used. The resource tracker will correct $resource_used, and
resources available will be reported as a negative number.

Correct this by reporting the original value if instance_info is unset.

Closes-Bug: #1502177
Change-Id: I13c5e5430fd305cd0ee2f24cd95304660ccf11eb

nova/tests/unit/virt/ironic/test_driver.py

@@ -244,11 +244,14 @@ class IronicDriverTestCase(test.NoDBTestCase):
                 FAKE_CLIENT, instance, 'fake message')
         self.assertTrue(fake_validate.called)
 
-    def test__node_resource(self):
+    def _test__node_resource(self, has_inst_info):
         node_uuid = uuidutils.generate_uuid()
         props = _get_properties()
         stats = _get_stats()
-        instance_info = _get_instance_info()
+        if has_inst_info:
+            instance_info = _get_instance_info()
+        else:
+            instance_info = {}
         node = ironic_utils.get_test_node(uuid=node_uuid,
                                           instance_uuid=self.instance_uuid,
                                           instance_info=instance_info,
@@ -269,16 +272,30 @@ class IronicDriverTestCase(test.NoDBTestCase):
         gotkeys = result.keys()
         gotkeys.sort()
         self.assertEqual(wantkeys, gotkeys)
-        self.assertEqual(instance_info['vcpus'], result['vcpus'])
+
-        self.assertEqual(instance_info['vcpus'], result['vcpus_used'])
+        if has_inst_info:
-        self.assertEqual(instance_info['memory_mb'], result['memory_mb'])
+            props_dict = instance_info
-        self.assertEqual(instance_info['memory_mb'], result['memory_mb_used'])
+            expected_cpus = instance_info['vcpus']
-        self.assertEqual(instance_info['local_gb'], result['local_gb'])
+        else:
-        self.assertEqual(instance_info['local_gb'], result['local_gb_used'])
+            props_dict = props
+            expected_cpus = props['cpus']
+        self.assertEqual(expected_cpus, result['vcpus'])
+        self.assertEqual(expected_cpus, result['vcpus_used'])
+        self.assertEqual(props_dict['memory_mb'], result['memory_mb'])
+        self.assertEqual(props_dict['memory_mb'], result['memory_mb_used'])
+        self.assertEqual(props_dict['local_gb'], result['local_gb'])
+        self.assertEqual(props_dict['local_gb'], result['local_gb_used'])
+
         self.assertEqual(node_uuid, result['hypervisor_hostname'])
         self.assertEqual(stats, jsonutils.loads(result['stats']))
         self.assertIsNone(result['numa_topology'])
 
+    def test__node_resource(self):
+        self._test__node_resource(True)
+
+    def test__node_resource_no_instance_info(self):
+        self._test__node_resource(False)
+
     def test__node_resource_canonicalizes_arch(self):
         node_uuid = uuidutils.generate_uuid()
         props = _get_properties()
@@ -433,17 +450,19 @@ class IronicDriverTestCase(test.NoDBTestCase):
 
     @mock.patch.object(ironic_driver.LOG, 'warning')
     def test__parse_node_instance_info(self, mock_warning):
+        props = _get_properties()
         instance_info = _get_instance_info()
         node = ironic_utils.get_test_node(
             uuid=uuidutils.generate_uuid(),
             instance_info=instance_info)
-        parsed = self.driver._parse_node_instance_info(node)
+        parsed = self.driver._parse_node_instance_info(node, props)
 
         self.assertEqual(instance_info, parsed)
         self.assertFalse(mock_warning.called)
 
     @mock.patch.object(ironic_driver.LOG, 'warning')
     def test__parse_node_instance_info_bad_values(self, mock_warning):
+        props = _get_properties()
         instance_info = _get_instance_info()
         instance_info['vcpus'] = 'bad-value'
         instance_info['memory_mb'] = 'bad-value'
@@ -451,9 +470,13 @@ class IronicDriverTestCase(test.NoDBTestCase):
         node = ironic_utils.get_test_node(
             uuid=uuidutils.generate_uuid(),
             instance_info=instance_info)
-        parsed = self.driver._parse_node_instance_info(node)
+        parsed = self.driver._parse_node_instance_info(node, props)
 
-        expected = {'vcpus': 0, 'memory_mb': 0, 'local_gb': 0}
+        expected = {
+            'vcpus': props['cpus'],
+            'memory_mb': props['memory_mb'],
+            'local_gb': props['local_gb']
+        }
         self.assertEqual(expected, parsed)
         self.assertEqual(3, mock_warning.call_count)
 

nova/virt/ironic/driver.py

@@ -261,20 +261,30 @@ class IronicDriver(virt_driver.ComputeDriver):
         properties['capabilities'] = node.properties.get('capabilities')
         return properties
 
-    def _parse_node_instance_info(self, node):
+    def _parse_node_instance_info(self, node, props):
-        """Helper method to parse the node's instance info."""
+        """Helper method to parse the node's instance info.
+
+        If a property cannot be looked up via instance_info, use the original
+        value from the properties dict. This is most likely to be correct;
+        it should only be incorrect if the properties were changed directly
+        in Ironic while an instance was deployed.
+        """
         instance_info = {}
 
+        # add this key because it's different in instance_info for some reason
+        props['vcpus'] = props['cpus']
         for prop in ('vcpus', 'memory_mb', 'local_gb'):
+            original = props[prop]
             try:
-                instance_info[prop] = int(node.instance_info.get(prop, 0))
+                instance_info[prop] = int(node.instance_info.get(prop,
+                                                                 original))
             except (TypeError, ValueError):
                 LOG.warning(_LW('Node %(uuid)s has a malformed "%(prop)s". '
                                 'It should be an integer but its value '
                                 'is "%(value)s".'),
                             {'uuid': node.uuid, 'prop': prop,
                              'value': node.instance_info.get(prop)})
-                instance_info[prop] = 0
+                instance_info[prop] = original
 
         return instance_info
 
@@ -325,7 +335,7 @@ class IronicDriver(virt_driver.ComputeDriver):
             # Node is in the process of deploying, is deployed, or is in
             # the process of cleaning up from a deploy. Report all of its
             # resources as in use.
-            instance_info = self._parse_node_instance_info(node)
+            instance_info = self._parse_node_instance_info(node, properties)
 
             # Use instance_info instead of properties here is because the
             # properties of a deployed node can be changed which will count