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:
parent
df0fca62cf
commit
26c69a6a62
|
@ -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',
|
||||
|
|
|
@ -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)
|
||||
|
||||
|
|
|
@ -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)
|
||||
|
|
|
@ -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):
|
||||
|
|
|
@ -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:
|
||||
|
|
|
@ -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(
|
||||
|
|
Loading…
Reference in New Issue