openstack
/
nova
Code Issues Proposed changes

[Ironic]Not count available resources of deployed ironic node 

Changing node's properties in Ironic after node is deployed will
count as available resources in Nova, as nova's resource_tracker
will override the *_used keys we set in the driver.

This patch changes the caculation of available resources based on
the instance flavor instead of node to avoid the issue.

Change-Id: I1a6e4501122c4781e103fd83a146e0914c7671c9
Closes-Bug: #1301279
This commit is contained in:
Zhenguo Niu 2015-07-21 21:16:22 +08:00
parent 3f9a4a765e
commit b99fb0a51c
5 changed files with 111 additions and 28 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -75,6 +75,12 @@ def _get_properties():
'capabilities': None}
def _get_instance_info():
return {'vcpus': 1,
'memory_mb': 1024,
'local_gb': 10}
def _get_stats():
return {'cpu_arch': 'x86_64'}
@ -242,8 +248,10 @@ class IronicDriverTestCase(test.NoDBTestCase):
node_uuid = uuidutils.generate_uuid()
props = _get_properties()
stats = _get_stats()
instance_info = _get_instance_info()
node = ironic_utils.get_test_node(uuid=node_uuid,
instance_uuid=self.instance_uuid,
instance_info=instance_info,
properties=props)
result = self.driver._node_resource(node)
@ -260,12 +268,12 @@ class IronicDriverTestCase(test.NoDBTestCase):
gotkeys = result.keys()
gotkeys.sort()
self.assertEqual(wantkeys, gotkeys)
self.assertEqual(props['cpus'], result['vcpus'])
self.assertEqual(props['cpus'], result['vcpus_used'])
self.assertEqual(props['memory_mb'], result['memory_mb'])
self.assertEqual(props['memory_mb'], result['memory_mb_used'])
self.assertEqual(props['local_gb'], result['local_gb'])
self.assertEqual(props['local_gb'], result['local_gb_used'])
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'])
self.assertEqual(node_uuid, result['hypervisor_hostname'])
self.assertEqual(stats, jsonutils.loads(result['stats']))
@ -363,19 +371,21 @@ class IronicDriverTestCase(test.NoDBTestCase):
node_uuid = uuidutils.generate_uuid()
props = _get_properties()
stats = _get_stats()
instance_info = _get_instance_info()
node = ironic_utils.get_test_node(
uuid=node_uuid,
instance_uuid=uuidutils.generate_uuid(),
provision_state=ironic_states.ACTIVE,
properties=props)
properties=props,
instance_info=instance_info)
result = self.driver._node_resource(node)
self.assertEqual(props['cpus'], result['vcpus'])
self.assertEqual(props['cpus'], result['vcpus_used'])
self.assertEqual(props['memory_mb'], result['memory_mb'])
self.assertEqual(props['memory_mb'], result['memory_mb_used'])
self.assertEqual(props['local_gb'], result['local_gb'])
self.assertEqual(props['local_gb'], result['local_gb_used'])
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'])
self.assertEqual(node_uuid, result['hypervisor_hostname'])
self.assertEqual(stats, jsonutils.loads(result['stats']))
@ -418,6 +428,32 @@ class IronicDriverTestCase(test.NoDBTestCase):
self.assertEqual(expected_props, parsed)
self.assertEqual(4, mock_warning.call_count)
@mock.patch.object(ironic_driver.LOG, 'warning')
def test__parse_node_instance_info(self, mock_warning):
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)
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):
instance_info = _get_instance_info()
instance_info['vcpus'] = 'bad-value'
instance_info['memory_mb'] = 'bad-value'
instance_info['local_gb'] = 'bad-value'
node = ironic_utils.get_test_node(
uuid=uuidutils.generate_uuid(),
instance_info=instance_info)
parsed = self.driver._parse_node_instance_info(node)
expected = {'vcpus': 0, 'memory_mb': 0, 'local_gb': 0}
self.assertEqual(expected, parsed)
self.assertEqual(3, mock_warning.call_count)
@mock.patch.object(ironic_driver.LOG, 'warning')
def test__parse_node_properties_canonicalize_cpu_arch(self, mock_warning):
props = _get_properties()
@ -843,6 +879,12 @@ class IronicDriverTestCase(test.NoDBTestCase):
'value': str(flavor['swap'])},
{'path': '/instance_info/display_name',
'value': instance.display_name, 'op': 'add'},
{'path': '/instance_info/vcpus', 'op': 'add',
'value': str(instance.vcpus)},
{'path': '/instance_info/memory_mb', 'op': 'add',
'value': str(instance.memory_mb)},
{'path': '/instance_info/local_gb', 'op': 'add',
'value': str(node.properties.get('local_gb', 0))},
{'path': '/instance_uuid', 'op': 'add',
'value': instance.uuid}]
mock_update.assert_called_once_with(node.uuid, expected_patch)

36
nova/tests/unit/virt/ironic/test_patcher.py
View File

@ -32,19 +32,31 @@ class IronicDriverFieldsTestCase(test.NoDBTestCase):
self.flavor = ironic_utils.get_test_flavor()
self.ctx = nova_context.get_admin_context()
self.instance = fake_instance.fake_instance_obj(self.ctx)
self.node = ironic_utils.get_test_node(driver='fake')
# Generic expected patches
self._expected_deploy_patch = [{'path': '/instance_info/image_source',
'value': self.image_meta['id'],
'op': 'add'},
{'path': '/instance_info/root_gb',
'value': str(self.instance['root_gb']),
'op': 'add'},
{'path': '/instance_info/swap_mb',
'value': str(self.flavor['swap']),
'op': 'add'},
{'path': '/instance_info/display_name',
'value': self.instance['display_name'],
'op': 'add'}]
self._expected_deploy_patch = [
{'path': '/instance_info/image_source',
'value': self.image_meta['id'],
'op': 'add'},
{'path': '/instance_info/root_gb',
'value': str(self.instance['root_gb']),
'op': 'add'},
{'path': '/instance_info/swap_mb',
'value': str(self.flavor['swap']),
'op': 'add'},
{'path': '/instance_info/display_name',
'value': self.instance['display_name'],
'op': 'add'},
{'path': '/instance_info/vcpus',
'value': str(self.instance['vcpus']),
'op': 'add'},
{'path': '/instance_info/memory_mb',
'value': str(self.instance['memory_mb']),
'op': 'add'},
{'path': '/instance_info/local_gb',
'value': str(self.node.properties.get('local_gb', 0)),
'op': 'add'}
]
self._expected_cleanup_patch = []
def test_create_generic(self):

1
nova/tests/unit/virt/ironic/utils.py
View File

@ -39,6 +39,7 @@ def get_test_node(**kw):
ironic_states.NOSTATE),
'last_error': kw.get('last_error'),
'instance_uuid': kw.get('instance_uuid'),
'instance_info': kw.get('instance_info'),
'driver': kw.get('driver', 'fake'),
'driver_info': kw.get('driver_info', {}),
'properties': kw.get('properties', {}),

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

@ -256,6 +256,23 @@ 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."""
instance_info = {}
for prop in ('vcpus', 'memory_mb', 'local_gb'):
try:
instance_info[prop] = int(node.instance_info.get(prop, 0))
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
return instance_info
def _node_resource(self, node):
"""Helper method to create resource dict from node stats."""
properties = self._parse_node_properties(node)
@ -303,9 +320,14 @@ 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.
vcpus_used = vcpus
memory_mb_used = memory_mb
local_gb_used = local_gb
instance_info = self._parse_node_instance_info(node)
# Use instance_info instead of properties here is because the
# properties of a deployed node can be changed which will count
# as available resources.
vcpus_used = vcpus = instance_info['vcpus']
memory_mb_used = memory_mb = instance_info['memory_mb']
local_gb_used = local_gb = instance_info['local_gb']
elif self._node_resources_unavailable(node):
# The node's current state is such that it should not present any
# of its resources to Nova

6
nova/virt/ironic/patcher.py
View File

@ -64,6 +64,12 @@ class GenericDriverFields(object):
'value': str(flavor['swap'])})
patch.append({'path': '/instance_info/display_name',
'op': 'add', 'value': instance.display_name})
patch.append({'path': '/instance_info/vcpus', 'op': 'add',
'value': str(instance.vcpus)})
patch.append({'path': '/instance_info/memory_mb', 'op': 'add',
'value': str(instance.memory_mb)})
patch.append({'path': '/instance_info/local_gb', 'op': 'add',
'value': str(self.node.properties.get('local_gb', 0))})
if instance.ephemeral_gb:
patch.append({'path': '/instance_info/ephemeral_gb',