Ironic: Lightweight fetching of nodes

The Ironic API version >= 1.8 supports fetching only a small number of
a resource instead of it's full representation. This patch is pinning
the API version for Ironic at 1.8 so the nova driver can benefit from
this feature by fetching only the required fields for the deployment,
making it more lightweight and improving the readability of the logs.

The configuration option "api_version" was marked as deprecated and
should be removed in the future. The only possible value for that
configuration was "1" (because Ironic only have 1 API version) and the
ironic team came to an agreement that setting the API version via
configuration option should not be supported anymore.

Closes-Bug: #1458934
Change-Id: Iebfa06da6811889e11fd4edf15d47ca4e0feea6f
This commit is contained in:
Lucas Alvares Gomes 2016-01-13 16:15:12 +00:00
parent df0fca62cf
commit 26c69a6a62
6 changed files with 62 additions and 30 deletions

View File

@ -22,7 +22,9 @@ ironic_group = cfg.OptGroup(
api_version = cfg.IntOpt(
'api_version',
default=1,
help='Version of Ironic API service endpoint.')
deprecated_for_removal=True,
help='Version of Ironic API service endpoint. '
'DEPRECATED: Setting the API version is not possible anymore.')
api_endpoint = cfg.StrOpt(
'api_endpoint',

View File

@ -73,7 +73,8 @@ class IronicClientWrapperTestCase(test.NoDBTestCase):
'os_endpoint_type': 'public',
'ironic_url': CONF.ironic.api_endpoint,
'max_retries': CONF.ironic.api_max_retries,
'retry_interval': CONF.ironic.api_retry_interval}
'retry_interval': CONF.ironic.api_retry_interval,
'os_ironic_api_version': '1.8'}
mock_ir_cli.assert_called_once_with(CONF.ironic.api_version,
**expected)
@ -86,7 +87,8 @@ class IronicClientWrapperTestCase(test.NoDBTestCase):
expected = {'os_auth_token': 'fake-token',
'ironic_url': CONF.ironic.api_endpoint,
'max_retries': CONF.ironic.api_max_retries,
'retry_interval': CONF.ironic.api_retry_interval}
'retry_interval': CONF.ironic.api_retry_interval,
'os_ironic_api_version': '1.8'}
mock_ir_cli.assert_called_once_with(CONF.ironic.api_version,
**expected)

View File

@ -526,7 +526,8 @@ class IronicDriverTestCase(test.NoDBTestCase):
uuid=self.instance_uuid)
self.assertTrue(self.driver.instance_exists(instance))
mock_call.assert_called_once_with('node.get_by_instance_uuid',
self.instance_uuid)
self.instance_uuid,
fields=ironic_driver._NODE_FIELDS)
@mock.patch.object(cw.IronicClientWrapper, 'call')
def test_instance_exists_fail(self, mock_call):
@ -535,7 +536,8 @@ class IronicDriverTestCase(test.NoDBTestCase):
uuid=self.instance_uuid)
self.assertFalse(self.driver.instance_exists(instance))
mock_call.assert_called_once_with('node.get_by_instance_uuid',
self.instance_uuid)
self.instance_uuid,
fields=ironic_driver._NODE_FIELDS)
@mock.patch.object(cw.IronicClientWrapper, 'call')
@mock.patch.object(objects.Instance, 'get_by_uuid')
@ -591,7 +593,8 @@ class IronicDriverTestCase(test.NoDBTestCase):
mock_get.return_value = node
mock_list.return_value = []
self.assertTrue(self.driver.node_is_available(node.uuid))
mock_get.assert_called_with(node.uuid)
mock_get.assert_called_with(node.uuid,
fields=ironic_driver._NODE_FIELDS)
mock_list.assert_called_with(detail=True, limit=0)
mock_get.side_effect = ironic_exception.NotFound
@ -722,7 +725,8 @@ class IronicDriverTestCase(test.NoDBTestCase):
result = self.driver.get_available_resource(node.uuid)
self.assertEqual(fake_resource, result)
mock_nr.assert_called_once_with(node)
mock_get.assert_called_once_with(node.uuid)
mock_get.assert_called_once_with(node.uuid,
fields=ironic_driver._NODE_FIELDS)
@mock.patch.object(FAKE_CLIENT.node, 'get')
@mock.patch.object(FAKE_CLIENT.node, 'list')
@ -823,7 +827,8 @@ class IronicDriverTestCase(test.NoDBTestCase):
self.driver.spawn(self.ctx, instance, image_meta, [], None)
mock_node.get.assert_called_once_with(node_uuid)
mock_node.get.assert_called_once_with(
node_uuid, fields=ironic_driver._NODE_FIELDS)
mock_node.validate.assert_called_once_with(node_uuid)
mock_adf.assert_called_once_with(node, instance,
test.MatchType(objects.ImageMeta),
@ -1020,7 +1025,8 @@ class IronicDriverTestCase(test.NoDBTestCase):
self.assertRaises(exception.ValidationError, self.driver.spawn,
self.ctx, instance, image_meta, [], None)
mock_node.get.assert_called_once_with(node_uuid)
mock_node.get.assert_called_once_with(
node_uuid, fields=ironic_driver._NODE_FIELDS)
mock_node.validate.assert_called_once_with(node_uuid)
@mock.patch.object(configdrive, 'required_by')
@ -1048,7 +1054,8 @@ class IronicDriverTestCase(test.NoDBTestCase):
self.assertRaises(TestException, self.driver.spawn,
self.ctx, instance, image_meta, [], None)
mock_node.get.assert_called_once_with(node_uuid)
mock_node.get.assert_called_once_with(
node_uuid, fields=ironic_driver._NODE_FIELDS)
mock_node.validate.assert_called_once_with(node_uuid)
mock_cleanup_deploy.assert_called_with(self.ctx, node, instance, None,
flavor=flavor)
@ -1076,7 +1083,8 @@ class IronicDriverTestCase(test.NoDBTestCase):
self.assertRaises(exception.NovaException, self.driver.spawn,
self.ctx, instance, image_meta, [], None)
mock_node.get.assert_called_once_with(node_uuid)
mock_node.get.assert_called_once_with(
node_uuid, fields=ironic_driver._NODE_FIELDS)
mock_node.validate.assert_called_once_with(node_uuid)
mock_cleanup_deploy.assert_called_once_with(self.ctx, node,
instance, None,
@ -1105,7 +1113,8 @@ class IronicDriverTestCase(test.NoDBTestCase):
self.driver.spawn,
self.ctx, instance, image_meta, [], None)
mock_node.get.assert_called_once_with(node_uuid)
mock_node.get.assert_called_once_with(
node_uuid, fields=ironic_driver._NODE_FIELDS)
mock_node.validate.assert_called_once_with(node_uuid)
mock_cleanup_deploy.assert_called_once_with(self.ctx, node,
instance, None,
@ -1184,7 +1193,8 @@ class IronicDriverTestCase(test.NoDBTestCase):
mock_node.set_provision_state.side_effect = fake_set_provision_state
self.driver.destroy(self.ctx, instance, network_info, None)
mock_node.get_by_instance_uuid.assert_called_with(instance.uuid)
mock_node.get_by_instance_uuid.assert_called_with(
instance.uuid, fields=ironic_driver._NODE_FIELDS)
mock_cleanup_deploy.assert_called_with(self.ctx, node,
instance, network_info)
@ -1279,7 +1289,8 @@ class IronicDriverTestCase(test.NoDBTestCase):
self.ctx, instance, None, None)
mock_node.set_provision_state.assert_called_once_with(node_uuid,
'deleted')
mock_node.get_by_instance_uuid.assert_called_with(instance.uuid)
mock_node.get_by_instance_uuid.assert_called_with(
instance.uuid, fields=ironic_driver._NODE_FIELDS)
@mock.patch.object(loopingcall, 'FixedIntervalLoopingCall')
@mock.patch.object(ironic_driver, '_validate_instance_and_node')
@ -1364,7 +1375,8 @@ class IronicDriverTestCase(test.NoDBTestCase):
network_info = utils.get_test_network_info()
self.driver.plug_vifs(instance, network_info)
mock_get.assert_called_once_with(node_uuid)
mock_get.assert_called_once_with(node_uuid,
fields=ironic_driver._NODE_FIELDS)
mock__plug_vifs.assert_called_once_with(node, instance, network_info)
@mock.patch.object(FAKE_CLIENT.port, 'update')
@ -1433,7 +1445,8 @@ class IronicDriverTestCase(test.NoDBTestCase):
utils.get_test_network_info())
# asserts
mock_node.get.assert_called_once_with(node_uuid)
mock_node.get.assert_called_once_with(
node_uuid, fields=ironic_driver._NODE_FIELDS)
mock_node.list_ports.assert_called_once_with(node_uuid, detail=True)
mock_update.assert_called_once_with(port.uuid, expected_patch)
@ -1449,7 +1462,8 @@ class IronicDriverTestCase(test.NoDBTestCase):
instance = fake_instance.fake_instance_obj(self.ctx, node=node_uuid)
self.driver.unplug_vifs(instance, utils.get_test_network_info())
mock_node.get.assert_called_once_with(node_uuid)
mock_node.get.assert_called_once_with(
node_uuid, fields=ironic_driver._NODE_FIELDS)
mock_node.list_ports.assert_called_once_with(node_uuid, detail=True)
# assert port.update() was not called
self.assertFalse(mock_update.called)

View File

@ -92,10 +92,10 @@ class FakeNodeClient(object):
def list(self, detail=False):
return []
def get(self, node_uuid):
def get(self, node_uuid, fields=None):
pass
def get_by_instance_uuid(self, instance_uuid):
def get_by_instance_uuid(self, instance_uuid, fields=None):
pass
def list_ports(self, node_uuid):

View File

@ -30,6 +30,9 @@ CONF = cfg.CONF
ironic = None
# The API version required by the Ironic driver
IRONIC_API_VERSION = (1, 8)
class IronicClientWrapper(object):
"""Ironic client wrapper class that encapsulates retry logic."""
@ -82,8 +85,9 @@ class IronicClientWrapper(object):
# Retries for Conflict exception
kwargs['max_retries'] = max_retries
kwargs['retry_interval'] = retry_interval
kwargs['os_ironic_api_version'] = '%d.%d' % IRONIC_API_VERSION
try:
cli = ironic.client.get_client(CONF.ironic.api_version, **kwargs)
cli = ironic.client.get_client(IRONIC_API_VERSION[0], **kwargs)
# Cache the client so we don't have to reconstruct and
# reauthenticate it every time we need it.
if retry_on_conflict:

View File

@ -74,6 +74,10 @@ _UNPROVISION_STATES = (ironic_states.ACTIVE, ironic_states.DEPLOYFAIL,
ironic_states.ERROR, ironic_states.DEPLOYWAIT,
ironic_states.DEPLOYING)
_NODE_FIELDS = ('uuid', 'power_state', 'target_power_state', 'provision_state',
'target_provision_state', 'last_error', 'maintenance',
'properties', 'instance_uuid')
def map_power_state(state):
try:
@ -90,7 +94,8 @@ def _validate_instance_and_node(ironicclient, instance):
node, and return the node.
"""
try:
return ironicclient.call("node.get_by_instance_uuid", instance.uuid)
return ironicclient.call('node.get_by_instance_uuid', instance.uuid,
fields=_NODE_FIELDS)
except ironic.exc.NotFound:
raise exception.InstanceNotFound(instance_id=instance.uuid)
@ -159,6 +164,11 @@ class IronicDriver(virt_driver.ComputeDriver):
self.ironicclient = client_wrapper.IronicClientWrapper()
def _get_node(self, node_uuid):
"""Get a node by its UUID."""
return self.ironicclient.call('node.get', node_uuid,
fields=_NODE_FIELDS)
def _node_resources_unavailable(self, node_obj):
"""Determine whether the node's resources are in an acceptable state.
@ -446,7 +456,7 @@ class IronicDriver(virt_driver.ComputeDriver):
def _get_hypervisor_version(self):
"""Returns the version of the Ironic API service endpoint."""
return CONF.ironic.api_version
return client_wrapper.IRONIC_API_VERSION[0]
def instance_exists(self, instance):
"""Checks the existence of an instance.
@ -525,7 +535,7 @@ class IronicDriver(virt_driver.ComputeDriver):
# NOTE(comstud): Fallback and check Ironic. This case should be
# rare.
try:
self.ironicclient.call("node.get", nodename)
self._get_node(nodename)
return True
except ironic.exc.NotFound:
return False
@ -585,7 +595,7 @@ class IronicDriver(virt_driver.ComputeDriver):
else:
LOG.debug("Node %(node)s not found in cache, age: %(age)s",
{'node': nodename, 'age': cache_age})
node = self.ironicclient.call("node.get", nodename)
node = self._get_node(nodename)
return self._node_resource(node)
def get_info(self, instance):
@ -643,7 +653,7 @@ class IronicDriver(virt_driver.ComputeDriver):
MAC addresses'.
"""
try:
node = self.ironicclient.call("node.get", instance.node)
node = self._get_node(instance.node)
except ironic.exc.NotFound:
return None
ports = self.ironicclient.call("node.list_ports", node.uuid)
@ -712,7 +722,7 @@ class IronicDriver(virt_driver.ComputeDriver):
_("Ironic node uuid not supplied to "
"driver for instance %s.") % instance.uuid)
node = self.ironicclient.call("node.get", node_uuid)
node = self._get_node(node_uuid)
flavor = instance.flavor
self._add_driver_fields(node, instance, image_meta, flavor)
@ -1071,7 +1081,7 @@ class IronicDriver(virt_driver.ComputeDriver):
:param network_info: Instance network information.
"""
node = self.ironicclient.call("node.get", instance.node)
node = self._get_node(instance.node)
self._plug_vifs(node, instance, network_info)
def unplug_vifs(self, instance, network_info):
@ -1081,7 +1091,7 @@ class IronicDriver(virt_driver.ComputeDriver):
:param network_info: Instance network information.
"""
node = self.ironicclient.call("node.get", instance.node)
node = self._get_node(instance.node)
self._unplug_vifs(node, instance, network_info)
def rebuild(self, context, instance, image_meta, injected_files,
@ -1133,7 +1143,7 @@ class IronicDriver(virt_driver.ComputeDriver):
instance.save(expected_task_state=[task_states.REBUILDING])
node_uuid = instance.node
node = self.ironicclient.call("node.get", node_uuid)
node = self._get_node(node_uuid)
self._add_driver_fields(node, instance, image_meta, instance.flavor,
preserve_ephemeral)
@ -1174,7 +1184,7 @@ class IronicDriver(virt_driver.ComputeDriver):
:returns: a string representing the host ID
"""
node = self.ironicclient.call("node.get", instance.node)
node = self._get_node(instance.node)
if getattr(node, 'network_provider', 'none') == 'none':
# flat network, go ahead and allow the port to be bound
return super(IronicDriver, self).network_binding_host_id(