openstack
/
nova
Code Issues Proposed changes

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
This commit is contained in:
Jim Rollenhagen 2015-10-01 21:12:53 -07:00
parent 20e1bce530
commit 047da6498d
2 changed files with 49 additions and 16 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

45
nova/tests/unit/virt/ironic/test_driver.py
View File

@ -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'])
self.assertEqual(instance_info['memory_mb'], result['memory_mb'])
self.assertEqual(instance_info['memory_mb'], result['memory_mb_used'])
self.assertEqual(instance_info['local_gb'], result['local_gb'])
self.assertEqual(instance_info['local_gb'], result['local_gb_used'])
if has_inst_info:
props_dict = instance_info
expected_cpus = instance_info['vcpus']
else:
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)

20
nova/virt/ironic/driver.py
View File

@ -261,20 +261,30 @@ class IronicDriver(virt_driver.ComputeDriver):
properties['capabilities'] = node.properties.get('capabilities')
return properties
def _parse_node_instance_info(self, node):
"""Helper method to parse the node's instance info."""
def _parse_node_instance_info(self, node, props):
"""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