diff --git a/nova/api/openstack/compute/contrib/baremetal_nodes.py b/nova/api/openstack/compute/contrib/baremetal_nodes.py index 3f9f710c4a..8509711d17 100644 --- a/nova/api/openstack/compute/contrib/baremetal_nodes.py +++ b/nova/api/openstack/compute/contrib/baremetal_nodes.py @@ -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': [], diff --git a/nova/tests/virt/ironic/test_client_wrapper.py b/nova/tests/virt/ironic/test_client_wrapper.py index 851b8a150b..9b1b923580 100644 --- a/nova/tests/virt/ironic/test_client_wrapper.py +++ b/nova/tests/virt/ironic/test_client_wrapper.py @@ -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") diff --git a/nova/tests/virt/ironic/test_driver.py b/nova/tests/virt/ironic/test_driver.py index b3e9be11ed..4c92132ffe 100644 --- a/nova/tests/virt/ironic/test_driver.py +++ b/nova/tests/virt/ironic/test_driver.py @@ -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): diff --git a/nova/virt/ironic/driver.py b/nova/virt/ironic/driver.py index 4016d7bb95..7919b20091 100644 --- a/nova/virt/ironic/driver.py +++ b/nova/virt/ironic/driver.py @@ -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()