Rename cli variable in ironic driver

Code in ironic driver used "icli" as variable name to reference to
ironic client instance. It has been renamed to "ironicclient" to make
the code clearer.

Change-Id: Ic9e314e882f5e5e305a5ea84f7c2f102f91140dd
Closes-Bug: #1365228
This commit is contained in:
Kamil Rykowski
2014-09-10 15:25:58 +02:00
parent ef85102dd4
commit 0260844e20
4 changed files with 95 additions and 93 deletions

View File

@@ -88,8 +88,8 @@ def _get_ironic_client():
'os_endpoint_type': 'public',
'insecure': 'true',
'ironic_url': CONF.ironic.api_endpoint}
icli = ironic_client.get_client(CONF.ironic.api_version, **kwargs)
return icli
ironicclient = ironic_client.get_client(CONF.ironic.api_version, **kwargs)
return ironicclient
def _no_ironic_proxy(cmd):
@@ -151,8 +151,8 @@ class BareMetalNodeController(wsgi.Controller):
authorize(context)
nodes = []
# proxy command to Ironic
icli = _get_ironic_client()
ironic_nodes = icli.node.list(detail=True)
ironicclient = _get_ironic_client()
ironic_nodes = ironicclient.node.list(detail=True)
for inode in ironic_nodes:
node = {'id': inode.uuid,
'interfaces': [],

View File

@@ -32,7 +32,7 @@ class IronicClientWrapperTestCase(test.NoDBTestCase):
def setUp(self):
super(IronicClientWrapperTestCase, self).setUp()
self.icli = client_wrapper.IronicClientWrapper()
self.ironicclient = client_wrapper.IronicClientWrapper()
# Do not waste time sleeping
cfg.CONF.set_override('api_retry_interval', 0, 'ironic')
@@ -40,7 +40,7 @@ class IronicClientWrapperTestCase(test.NoDBTestCase):
@mock.patch.object(client_wrapper.IronicClientWrapper, '_get_client')
def test_call_good_no_args(self, mock_get_client, mock_multi_getattr):
mock_get_client.return_value = FAKE_CLIENT
self.icli.call("node.list")
self.ironicclient.call("node.list")
mock_get_client.assert_called_once_with()
mock_multi_getattr.assert_called_once_with(FAKE_CLIENT, "node.list")
mock_multi_getattr.return_value.assert_called_once_with()
@@ -49,7 +49,7 @@ class IronicClientWrapperTestCase(test.NoDBTestCase):
@mock.patch.object(client_wrapper.IronicClientWrapper, '_get_client')
def test_call_good_with_args(self, mock_get_client, mock_multi_getattr):
mock_get_client.return_value = FAKE_CLIENT
self.icli.call("node.list", 'test', associated=True)
self.ironicclient.call("node.list", 'test', associated=True)
mock_get_client.assert_called_once_with()
mock_multi_getattr.assert_called_once_with(FAKE_CLIENT, "node.list")
mock_multi_getattr.return_value.assert_called_once_with(
@@ -58,9 +58,9 @@ class IronicClientWrapperTestCase(test.NoDBTestCase):
@mock.patch.object(ironic_client, 'get_client')
def test__get_client_no_auth_token(self, mock_ir_cli):
self.flags(admin_auth_token=None, group='ironic')
icli = client_wrapper.IronicClientWrapper()
ironicclient = client_wrapper.IronicClientWrapper()
# dummy call to have _get_client() called
icli.call("node.list")
ironicclient.call("node.list")
expected = {'os_username': CONF.ironic.admin_username,
'os_password': CONF.ironic.admin_password,
'os_auth_url': CONF.ironic.admin_url,
@@ -74,9 +74,9 @@ class IronicClientWrapperTestCase(test.NoDBTestCase):
@mock.patch.object(ironic_client, 'get_client')
def test__get_client_with_auth_token(self, mock_ir_cli):
self.flags(admin_auth_token='fake-token', group='ironic')
icli = client_wrapper.IronicClientWrapper()
ironicclient = client_wrapper.IronicClientWrapper()
# dummy call to have _get_client() called
icli.call("node.list")
ironicclient.call("node.list")
expected = {'os_auth_token': 'fake-token',
'ironic_url': CONF.ironic.api_endpoint}
mock_ir_cli.assert_called_once_with(CONF.ironic.api_version,
@@ -90,7 +90,8 @@ class IronicClientWrapperTestCase(test.NoDBTestCase):
test_obj.side_effect = ironic_exception.HTTPServiceUnavailable
mock_multi_getattr.return_value = test_obj
mock_get_client.return_value = FAKE_CLIENT
self.assertRaises(exception.NovaException, self.icli.call, "node.list")
self.assertRaises(exception.NovaException, self.ironicclient.call,
"node.list")
self.assertEqual(2, test_obj.call_count)
@mock.patch.object(client_wrapper.IronicClientWrapper, '_multi_getattr')
@@ -101,24 +102,25 @@ class IronicClientWrapperTestCase(test.NoDBTestCase):
test_obj.side_effect = ironic_exception.HTTPNotFound
mock_multi_getattr.return_value = test_obj
mock_get_client.return_value = FAKE_CLIENT
self.assertRaises(ironic_exception.HTTPNotFound, self.icli.call,
"node.list")
self.assertRaises(ironic_exception.HTTPNotFound,
self.ironicclient.call, "node.list")
@mock.patch.object(ironic_client, 'get_client')
def test__get_client_unauthorized(self, mock_get_client):
mock_get_client.side_effect = ironic_exception.Unauthorized
self.assertRaises(exception.NovaException, self.icli._get_client)
self.assertRaises(exception.NovaException,
self.ironicclient._get_client)
@mock.patch.object(ironic_client, 'get_client')
def test__get_client_unexpected_exception(self, mock_get_client):
mock_get_client.side_effect = ironic_exception.ConnectionRefused
self.assertRaises(ironic_exception.ConnectionRefused,
self.icli._get_client)
self.ironicclient._get_client)
def test__multi_getattr_good(self):
response = self.icli._multi_getattr(FAKE_CLIENT, "node.list")
response = self.ironicclient._multi_getattr(FAKE_CLIENT, "node.list")
self.assertEqual(FAKE_CLIENT.node.list, response)
def test__multi_getattr_fail(self):
self.assertRaises(AttributeError, self.icli._multi_getattr,
self.assertRaises(AttributeError, self.ironicclient._multi_getattr,
FAKE_CLIENT, "nonexistent")

View File

@@ -109,22 +109,23 @@ class IronicDriverTestCase(test.NoDBTestCase):
instance_uuid=instance_uuid)
instance = fake_instance.fake_instance_obj(self.ctx,
uuid=instance_uuid)
icli = cw.IronicClientWrapper()
ironicclient = cw.IronicClientWrapper()
mock_gbiui.return_value = node
result = ironic_driver._validate_instance_and_node(icli, instance)
result = ironic_driver._validate_instance_and_node(ironicclient,
instance)
self.assertEqual(result.uuid, node_uuid)
@mock.patch.object(FAKE_CLIENT.node, 'get_by_instance_uuid')
def test__validate_instance_and_node_failed(self, mock_gbiui):
icli = cw.IronicClientWrapper()
ironicclient = cw.IronicClientWrapper()
mock_gbiui.side_effect = ironic_exception.NotFound()
instance_uuid = uuidutils.generate_uuid(),
instance = fake_instance.fake_instance_obj(self.ctx,
uuid=instance_uuid)
self.assertRaises(exception.InstanceNotFound,
ironic_driver._validate_instance_and_node,
icli, instance)
ironicclient, instance)
@mock.patch.object(ironic_driver, '_validate_instance_and_node')
def test__wait_for_active_pass(self, fake_validate):

View File

@@ -104,16 +104,14 @@ def map_power_state(state):
return power_state.NOSTATE
def _validate_instance_and_node(icli, instance):
def _validate_instance_and_node(ironicclient, instance):
"""Get the node associated with the instance.
Check with the Ironic service that this instance is associated with a
node, and return the node.
"""
try:
# TODO(mrda): Bug ID 1365228 icli should be renamed ironicclient
# throughout
return icli.call("node.get_by_instance_uuid", instance['uuid'])
return ironicclient.call("node.get_by_instance_uuid", instance['uuid'])
except ironic.exc.NotFound:
raise exception.InstanceNotFound(instance_id=instance['uuid'])
@@ -175,9 +173,9 @@ class IronicDriver(virt_driver.ComputeDriver):
# TODO(mrda): Bug ID 1365230 Logging configurability needs
# to be addressed
icli_log_level = CONF.ironic.client_log_level
if icli_log_level:
level = py_logging.getLevelName(icli_log_level)
ironicclient_log_level = CONF.ironic.client_log_level
if ironicclient_log_level:
level = py_logging.getLevelName(ironicclient_log_level)
logger = py_logging.getLogger('ironicclient')
logger.setLevel(level)
@@ -285,7 +283,7 @@ class IronicDriver(virt_driver.ComputeDriver):
def _add_driver_fields(self, node, instance, image_meta, flavor,
preserve_ephemeral=None):
icli = client_wrapper.IronicClientWrapper()
ironicclient = client_wrapper.IronicClientWrapper()
patch = patcher.create(node).get_deploy_patch(instance,
image_meta,
flavor,
@@ -295,7 +293,7 @@ class IronicDriver(virt_driver.ComputeDriver):
patch.append({'path': '/instance_uuid', 'op': 'add',
'value': instance['uuid']})
try:
icli.call('node.update', node.uuid, patch)
ironicclient.call('node.update', node.uuid, patch)
except ironic.exc.BadRequest:
msg = (_("Failed to add deploy parameters on node %(node)s "
"when provisioning the instance %(instance)s")
@@ -305,7 +303,7 @@ class IronicDriver(virt_driver.ComputeDriver):
def _cleanup_deploy(self, context, node, instance, network_info,
flavor=None):
icli = client_wrapper.IronicClientWrapper()
ironicclient = client_wrapper.IronicClientWrapper()
if flavor is None:
# TODO(mrda): It would be better to use instance.get_flavor() here
# but right now that doesn't include extra_specs which are required
@@ -317,7 +315,7 @@ class IronicDriver(virt_driver.ComputeDriver):
# Unassociate the node
patch.append({'op': 'remove', 'path': '/instance_uuid'})
try:
icli.call('node.update', node.uuid, patch)
ironicclient.call('node.update', node.uuid, patch)
except ironic.exc.BadRequest:
LOG.error(_LE("Failed to clean up the parameters on node %(node)s "
"when unprovisioning the instance %(instance)s"),
@@ -328,9 +326,9 @@ class IronicDriver(virt_driver.ComputeDriver):
self._unplug_vifs(node, instance, network_info)
self._stop_firewall(instance, network_info)
def _wait_for_active(self, icli, instance):
def _wait_for_active(self, ironicclient, instance):
"""Wait for the node to be marked as ACTIVE in Ironic."""
node = _validate_instance_and_node(icli, instance)
node = _validate_instance_and_node(ironicclient, instance)
if node.provision_state == ironic_states.ACTIVE:
# job is done
LOG.debug("Ironic node %(node)s is now ACTIVE",
@@ -353,9 +351,9 @@ class IronicDriver(virt_driver.ComputeDriver):
_log_ironic_polling('become ACTIVE', node, instance)
def _wait_for_power_state(self, icli, instance, message):
def _wait_for_power_state(self, ironicclient, instance, message):
"""Wait for the node to complete a power state change."""
node = _validate_instance_and_node(icli, instance)
node = _validate_instance_and_node(ironicclient, instance)
if node.target_power_state == ironic_states.NOSTATE:
raise loopingcall.LoopingCallDone()
@@ -388,9 +386,9 @@ class IronicDriver(virt_driver.ComputeDriver):
:returns: True if the instance exists. False if not.
"""
icli = client_wrapper.IronicClientWrapper()
ironicclient = client_wrapper.IronicClientWrapper()
try:
_validate_instance_and_node(icli, instance)
_validate_instance_and_node(ironicclient, instance)
return True
except exception.InstanceNotFound:
return False
@@ -401,8 +399,8 @@ class IronicDriver(virt_driver.ComputeDriver):
:returns: a list of instance names.
"""
icli = client_wrapper.IronicClientWrapper()
node_list = icli.call("node.list", associated=True)
ironicclient = client_wrapper.IronicClientWrapper()
node_list = ironicclient.call("node.list", associated=True)
context = nova_context.get_admin_context()
return [objects.Instance.get_by_uuid(context,
i.instance_uuid).name
@@ -414,8 +412,8 @@ class IronicDriver(virt_driver.ComputeDriver):
:returns: a list of instance UUIDs.
"""
icli = client_wrapper.IronicClientWrapper()
node_list = icli.call("node.list", associated=True)
ironicclient = client_wrapper.IronicClientWrapper()
node_list = ironicclient.call("node.list", associated=True)
return list(n.instance_uuid for n in node_list)
def node_is_available(self, nodename):
@@ -439,16 +437,16 @@ class IronicDriver(virt_driver.ComputeDriver):
# NOTE(comstud): Fallback and check Ironic. This case should be
# rare.
icli = client_wrapper.IronicClientWrapper()
ironicclient = client_wrapper.IronicClientWrapper()
try:
icli.call("node.get", nodename)
ironicclient.call("node.get", nodename)
return True
except ironic.exc.NotFound:
return False
def _refresh_cache(self):
icli = client_wrapper.IronicClientWrapper()
node_list = icli.call('node.list', detail=True)
ironicclient = client_wrapper.IronicClientWrapper()
node_list = ironicclient.call('node.list', detail=True)
node_cache = {}
for node in node_list:
node_cache[node.uuid] = node
@@ -501,8 +499,8 @@ class IronicDriver(virt_driver.ComputeDriver):
else:
LOG.debug("Node %(node)s not found in cache, age: %(age)s",
{'node': nodename, 'age': cache_age})
icli = client_wrapper.IronicClientWrapper()
node = icli.call("node.get", nodename)
ironicclient = client_wrapper.IronicClientWrapper()
node = ironicclient.call("node.get", nodename)
return self._node_resource(node)
def get_info(self, instance):
@@ -522,9 +520,9 @@ class IronicDriver(virt_driver.ComputeDriver):
this driver.
"""
icli = client_wrapper.IronicClientWrapper()
ironicclient = client_wrapper.IronicClientWrapper()
try:
node = _validate_instance_and_node(icli, instance)
node = _validate_instance_and_node(ironicclient, instance)
except exception.InstanceNotFound:
return {'state': map_power_state(ironic_states.NOSTATE),
'max_mem': 0,
@@ -573,12 +571,12 @@ class IronicDriver(virt_driver.ComputeDriver):
None means 'no constraints', a set means 'these and only these
MAC addresses'.
"""
icli = client_wrapper.IronicClientWrapper()
ironicclient = client_wrapper.IronicClientWrapper()
try:
node = icli.call("node.get", instance['node'])
node = ironicclient.call("node.get", instance['node'])
except ironic.exc.NotFound:
return None
ports = icli.call("node.list_ports", node.uuid)
ports = ironicclient.call("node.list_ports", node.uuid)
return set([p.address for p in ports])
def spawn(self, context, instance, image_meta, injected_files,
@@ -606,8 +604,8 @@ class IronicDriver(virt_driver.ComputeDriver):
_("Ironic node uuid not supplied to "
"driver for instance %s.") % instance['uuid'])
icli = client_wrapper.IronicClientWrapper()
node = icli.call("node.get", node_uuid)
ironicclient = client_wrapper.IronicClientWrapper()
node = ironicclient.call("node.get", node_uuid)
flavor = objects.Flavor.get_by_id(context,
instance['instance_type_id'])
@@ -621,7 +619,7 @@ class IronicDriver(virt_driver.ComputeDriver):
instance.save()
# validate we are ready to do the deploy
validate_chk = icli.call("node.validate", node_uuid)
validate_chk = ironicclient.call("node.validate", node_uuid)
if not validate_chk.deploy or not validate_chk.power:
# something is wrong. undo what we have done
self._cleanup_deploy(context, node, instance, network_info,
@@ -648,8 +646,8 @@ class IronicDriver(virt_driver.ComputeDriver):
# trigger the node deploy
try:
icli.call("node.set_provision_state", node_uuid,
ironic_states.ACTIVE)
ironicclient.call("node.set_provision_state", node_uuid,
ironic_states.ACTIVE)
except Exception as e:
with excutils.save_and_reraise_exception():
msg = (_LE("Failed to request Ironic to provision instance "
@@ -661,7 +659,7 @@ class IronicDriver(virt_driver.ComputeDriver):
flavor=flavor)
timer = loopingcall.FixedIntervalLoopingCall(self._wait_for_active,
icli, instance)
ironicclient, instance)
try:
timer.start(interval=CONF.ironic.api_retry_interval).wait()
except Exception:
@@ -672,12 +670,12 @@ class IronicDriver(virt_driver.ComputeDriver):
'node': node_uuid})
self.destroy(context, instance, network_info)
def _unprovision(self, icli, instance, node):
def _unprovision(self, ironicclient, instance, node):
"""This method is called from destroy() to unprovision
already provisioned node after required checks.
"""
try:
icli.call("node.set_provision_state", node.uuid, "deleted")
ironicclient.call("node.set_provision_state", node.uuid, "deleted")
except Exception as e:
# if the node is already in a deprovisioned state, continue
# This should be fixed in Ironic.
@@ -691,7 +689,7 @@ class IronicDriver(virt_driver.ComputeDriver):
data = {'tries': 0}
def _wait_for_provision_state():
node = _validate_instance_and_node(icli, instance)
node = _validate_instance_and_node(ironicclient, instance)
if not node.provision_state:
LOG.debug("Ironic node %(node)s is now unprovisioned",
dict(node=node.uuid), instance=instance)
@@ -727,9 +725,9 @@ class IronicDriver(virt_driver.ComputeDriver):
:param migrate_data: implementation specific params.
Ignored by this driver.
"""
icli = client_wrapper.IronicClientWrapper()
ironicclient = client_wrapper.IronicClientWrapper()
try:
node = _validate_instance_and_node(icli, instance)
node = _validate_instance_and_node(ironicclient, instance)
except exception.InstanceNotFound:
LOG.warning(_LW("Destroy called on non-existing instance %s."),
instance['uuid'])
@@ -743,7 +741,7 @@ class IronicDriver(virt_driver.ComputeDriver):
ironic_states.DEPLOYFAIL,
ironic_states.ERROR,
ironic_states.DEPLOYWAIT):
self._unprovision(icli, instance, node)
self._unprovision(ironicclient, instance, node)
self._cleanup_deploy(context, node, instance, network_info)
@@ -768,13 +766,13 @@ class IronicDriver(virt_driver.ComputeDriver):
encountered. Ignored by this driver.
"""
icli = client_wrapper.IronicClientWrapper()
node = _validate_instance_and_node(icli, instance)
icli.call("node.set_power_state", node.uuid, 'reboot')
ironicclient = client_wrapper.IronicClientWrapper()
node = _validate_instance_and_node(ironicclient, instance)
ironicclient.call("node.set_power_state", node.uuid, 'reboot')
timer = loopingcall.FixedIntervalLoopingCall(
self._wait_for_power_state,
icli, instance, 'reboot')
ironicclient, instance, 'reboot')
timer.start(interval=CONF.ironic.api_retry_interval).wait()
def power_off(self, instance, timeout=0, retry_interval=0):
@@ -791,13 +789,13 @@ class IronicDriver(virt_driver.ComputeDriver):
:param retry_interval: How often to signal node while waiting
for it to shutdown. Ignored by this driver.
"""
icli = client_wrapper.IronicClientWrapper()
node = _validate_instance_and_node(icli, instance)
icli.call("node.set_power_state", node.uuid, 'off')
ironicclient = client_wrapper.IronicClientWrapper()
node = _validate_instance_and_node(ironicclient, instance)
ironicclient.call("node.set_power_state", node.uuid, 'off')
timer = loopingcall.FixedIntervalLoopingCall(
self._wait_for_power_state,
icli, instance, 'power off')
ironicclient, instance, 'power off')
timer.start(interval=CONF.ironic.api_retry_interval).wait()
def power_on(self, context, instance, network_info,
@@ -815,13 +813,13 @@ class IronicDriver(virt_driver.ComputeDriver):
information. Ignored by this driver.
"""
icli = client_wrapper.IronicClientWrapper()
node = _validate_instance_and_node(icli, instance)
icli.call("node.set_power_state", node.uuid, 'on')
ironicclient = client_wrapper.IronicClientWrapper()
node = _validate_instance_and_node(ironicclient, instance)
ironicclient.call("node.set_power_state", node.uuid, 'on')
timer = loopingcall.FixedIntervalLoopingCall(
self._wait_for_power_state,
icli, instance, 'power on')
ironicclient, instance, 'power on')
timer.start(interval=CONF.ironic.api_retry_interval).wait()
def refresh_security_group_rules(self, security_group_id):
@@ -890,8 +888,8 @@ class IronicDriver(virt_driver.ComputeDriver):
# start by ensuring the ports are clear
self._unplug_vifs(node, instance, network_info)
icli = client_wrapper.IronicClientWrapper()
ports = icli.call("node.list_ports", node.uuid)
ironicclient = client_wrapper.IronicClientWrapper()
ports = ironicclient.call("node.list_ports", node.uuid)
if len(network_info) > len(ports):
raise exception.NovaException(_(
@@ -910,7 +908,7 @@ class IronicDriver(virt_driver.ComputeDriver):
patch = [{'op': 'add',
'path': '/extra/vif_port_id',
'value': port_id}]
icli.call("port.update", pif.uuid, patch)
ironicclient.call("port.update", pif.uuid, patch)
def _unplug_vifs(self, node, instance, network_info):
# NOTE(PhilDay): Accessing network_info will block if the thread
@@ -921,8 +919,9 @@ class IronicDriver(virt_driver.ComputeDriver):
{'uuid': instance['uuid'],
'network_info': network_info_str})
if network_info and len(network_info) > 0:
icli = client_wrapper.IronicClientWrapper()
ports = icli.call("node.list_ports", node.uuid, detail=True)
ironicclient = client_wrapper.IronicClientWrapper()
ports = ironicclient.call("node.list_ports", node.uuid,
detail=True)
# not needed if no vif are defined
for vif, pif in zip(network_info, ports):
@@ -930,7 +929,7 @@ class IronicDriver(virt_driver.ComputeDriver):
# we can not attach a dict directly
patch = [{'op': 'remove', 'path': '/extra/vif_port_id'}]
try:
icli.call("port.update", pif.uuid, patch)
ironicclient.call("port.update", pif.uuid, patch)
except ironic.exc.BadRequest:
pass
@@ -941,8 +940,8 @@ class IronicDriver(virt_driver.ComputeDriver):
:param network_info: Instance network information.
"""
icli = client_wrapper.IronicClientWrapper()
node = icli.call("node.get", instance['node'])
ironicclient = client_wrapper.IronicClientWrapper()
node = ironicclient.call("node.get", instance['node'])
self._plug_vifs(node, instance, network_info)
def unplug_vifs(self, instance, network_info):
@@ -952,8 +951,8 @@ class IronicDriver(virt_driver.ComputeDriver):
:param network_info: Instance network information.
"""
icli = client_wrapper.IronicClientWrapper()
node = icli.call("node.get", instance['node'])
ironicclient = client_wrapper.IronicClientWrapper()
node = ironicclient.call("node.get", instance['node'])
self._unplug_vifs(node, instance, network_info)
def rebuild(self, context, instance, image_meta, injected_files,
@@ -1003,8 +1002,8 @@ class IronicDriver(virt_driver.ComputeDriver):
instance.save(expected_task_state=[task_states.REBUILDING])
node_uuid = instance.node
icli = client_wrapper.IronicClientWrapper()
node = icli.call("node.get", node_uuid)
ironicclient = client_wrapper.IronicClientWrapper()
node = ironicclient.call("node.get", node_uuid)
flavor = objects.Flavor.get_by_id(context,
instance['instance_type_id'])
@@ -1013,8 +1012,8 @@ class IronicDriver(virt_driver.ComputeDriver):
# Trigger the node rebuild/redeploy.
try:
icli.call("node.set_provision_state",
node_uuid, ironic_states.REBUILD)
ironicclient.call("node.set_provision_state",
node_uuid, ironic_states.REBUILD)
except (exception.NovaException, # Retry failed
ironic.exc.InternalServerError, # Validations
ironic.exc.BadRequest) as e: # Maintenance
@@ -1026,5 +1025,5 @@ class IronicDriver(virt_driver.ComputeDriver):
# Although the target provision state is REBUILD, it will actually go
# to ACTIVE once the redeploy is finished.
timer = loopingcall.FixedIntervalLoopingCall(self._wait_for_active,
icli, instance)
ironicclient, instance)
timer.start(interval=CONF.ironic.api_retry_interval).wait()