From 07db53c2139586acbb6a14cf2a221be7c5bb50c3 Mon Sep 17 00:00:00 2001 From: Stephen Finucane Date: Wed, 14 Dec 2022 18:23:37 +0000 Subject: [PATCH] Remove Ironic client wrapper We no longer need this since everything is using SDK. The removal of this highlights a couple of areas where there were outstanding unused mocks. These are cleaned up in the process. Change-Id: I6247bfbb157b37eff66ac3974bf91780a3d01c1d Signed-off-by: Stephen Finucane --- .../unit/virt/ironic/test_client_wrapper.py | 184 ---------------- nova/tests/unit/virt/ironic/test_driver.py | 114 +++++----- nova/tests/unit/virt/ironic/utils.py | 86 -------- nova/virt/ironic/client_wrapper.py | 198 ------------------ nova/virt/ironic/driver.py | 9 +- nova/virt/ironic/patcher.py | 2 +- test-requirements.txt | 1 - 7 files changed, 52 insertions(+), 542 deletions(-) delete mode 100644 nova/tests/unit/virt/ironic/test_client_wrapper.py delete mode 100644 nova/virt/ironic/client_wrapper.py diff --git a/nova/tests/unit/virt/ironic/test_client_wrapper.py b/nova/tests/unit/virt/ironic/test_client_wrapper.py deleted file mode 100644 index 512f1438d694..000000000000 --- a/nova/tests/unit/virt/ironic/test_client_wrapper.py +++ /dev/null @@ -1,184 +0,0 @@ -# Copyright 2014 Red Hat, Inc. -# All Rights Reserved. -# -# Licensed under the Apache License, Version 2.0 (the "License"); you may -# not use this file except in compliance with the License. You may obtain -# a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT -# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the -# License for the specific language governing permissions and limitations -# under the License. - -from unittest import mock - -from ironicclient import client as ironic_client -from ironicclient import exc as ironic_exception -from keystoneauth1 import discover as ksa_disc -import keystoneauth1.session -from oslo_config import cfg - -import nova.conf -from nova import exception -from nova import test -from nova.tests.unit.virt.ironic import utils as ironic_utils -from nova.virt.ironic import client_wrapper - -CONF = nova.conf.CONF - -FAKE_CLIENT = ironic_utils.FakeClient() - - -def get_new_fake_client(*args, **kwargs): - return ironic_utils.FakeClient() - - -class IronicClientWrapperTestCase(test.NoDBTestCase): - - def setUp(self): - super(IronicClientWrapperTestCase, self).setUp() - self.ironicclient = client_wrapper.IronicClientWrapper() - # Do not waste time sleeping - cfg.CONF.set_override('api_retry_interval', 0, 'ironic') - cfg.CONF.set_override('region_name', 'RegionOne', 'ironic') - get_ksa_adapter_p = mock.patch('nova.utils.get_ksa_adapter') - self.addCleanup(get_ksa_adapter_p.stop) - self.get_ksa_adapter = get_ksa_adapter_p.start() - get_auth_plugin_p = mock.patch('nova.virt.ironic.client_wrapper.' - 'IronicClientWrapper._get_auth_plugin') - self.addCleanup(get_auth_plugin_p.stop) - self.get_auth_plugin = get_auth_plugin_p.start() - - @mock.patch.object(client_wrapper.IronicClientWrapper, '_multi_getattr') - @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.ironicclient.call("node.list") - mock_get_client.assert_called_once_with(retry_on_conflict=True) - mock_multi_getattr.assert_called_once_with(FAKE_CLIENT, "node.list") - mock_multi_getattr.return_value.assert_called_once_with() - - @mock.patch.object(client_wrapper.IronicClientWrapper, '_multi_getattr') - @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.ironicclient.call("node.list", 'test', associated=True) - mock_get_client.assert_called_once_with(retry_on_conflict=True) - mock_multi_getattr.assert_called_once_with(FAKE_CLIENT, "node.list") - mock_multi_getattr.return_value.assert_called_once_with( - 'test', associated=True) - - @mock.patch.object(keystoneauth1.session, 'Session') - @mock.patch.object(ironic_client, 'get_client') - def test__get_client_session(self, mock_ir_cli, mock_session): - """An Ironicclient is called with a keystoneauth1 Session""" - mock_session.return_value = 'session' - ironicclient = client_wrapper.IronicClientWrapper() - # dummy call to have _get_client() called - ironicclient.call("node.list") - self.get_ksa_adapter.assert_called_once_with( - 'baremetal', ksa_auth=self.get_auth_plugin.return_value, - ksa_session='session', min_version=(1, 0), - max_version=(1, ksa_disc.LATEST)) - expected = {'session': 'session', - 'max_retries': CONF.ironic.api_max_retries, - 'retry_interval': CONF.ironic.api_retry_interval, - 'os_ironic_api_version': ['1.46', '1.38'], - 'endpoint': - self.get_ksa_adapter.return_value.get_endpoint.return_value, - 'interface': ['internal', 'public']} - mock_ir_cli.assert_called_once_with(1, **expected) - - @mock.patch.object(keystoneauth1.session, 'Session') - @mock.patch.object(ironic_client, 'get_client') - def test__get_client_session_service_not_found(self, mock_ir_cli, - mock_session): - """Validate behavior when get_endpoint_data raises.""" - mock_session.return_value = 'session' - self.get_ksa_adapter.side_effect = ( - exception.ConfGroupForServiceTypeNotFound(stype='baremetal')) - ironicclient = client_wrapper.IronicClientWrapper() - # dummy call to have _get_client() called - ironicclient.call("node.list") - self.get_ksa_adapter.assert_called_once_with( - 'baremetal', ksa_auth=self.get_auth_plugin.return_value, - ksa_session='session', min_version=(1, 0), - max_version=(1, ksa_disc.LATEST)) - # When get_endpoint_data raises any ServiceNotFound, None is returned. - expected = {'session': 'session', - 'max_retries': CONF.ironic.api_max_retries, - 'retry_interval': CONF.ironic.api_retry_interval, - 'os_ironic_api_version': ['1.46', '1.38'], - 'endpoint': None, - 'region_name': CONF.ironic.region_name, - 'interface': ['internal', 'public']} - mock_ir_cli.assert_called_once_with(1, **expected) - - @mock.patch.object(keystoneauth1.session, 'Session') - @mock.patch.object(ironic_client, 'get_client') - def test__get_client_and_valid_interfaces(self, mock_ir_cli, mock_session): - """Confirm explicit setting of valid_interfaces.""" - mock_session.return_value = 'session' - endpoint = 'https://baremetal.example.com/endpoint' - self.get_ksa_adapter.return_value.get_endpoint.return_value = endpoint - self.flags(endpoint_override=endpoint, group='ironic') - self.flags(valid_interfaces='admin', group='ironic') - ironicclient = client_wrapper.IronicClientWrapper() - # dummy call to have _get_client() called - ironicclient.call("node.list") - expected = {'session': 'session', - 'max_retries': CONF.ironic.api_max_retries, - 'retry_interval': CONF.ironic.api_retry_interval, - 'os_ironic_api_version': ['1.46', '1.38'], - 'endpoint': endpoint, - 'interface': ['admin']} - mock_ir_cli.assert_called_once_with(1, **expected) - - @mock.patch.object(client_wrapper.IronicClientWrapper, '_multi_getattr') - @mock.patch.object(client_wrapper.IronicClientWrapper, '_get_client') - def test_call_fail_exception(self, mock_get_client, mock_multi_getattr): - test_obj = mock.Mock() - 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.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.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.ironicclient._get_client) - - def test__multi_getattr_good(self): - 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.ironicclient._multi_getattr, - FAKE_CLIENT, "nonexistent") - - @mock.patch.object(ironic_client, 'get_client') - def test__client_is_cached(self, mock_get_client): - mock_get_client.side_effect = get_new_fake_client - ironicclient = client_wrapper.IronicClientWrapper() - first_client = ironicclient._get_client() - second_client = ironicclient._get_client() - self.assertEqual(id(first_client), id(second_client)) - - @mock.patch.object(ironic_client, 'get_client') - def test_call_uses_cached_client(self, mock_get_client): - mock_get_client.side_effect = get_new_fake_client - ironicclient = client_wrapper.IronicClientWrapper() - for n in range(0, 4): - ironicclient.call("node.list") - self.assertEqual(1, mock_get_client.call_count) diff --git a/nova/tests/unit/virt/ironic/test_driver.py b/nova/tests/unit/virt/ironic/test_driver.py index 9b32fd288b5d..6fda8e0a275a 100644 --- a/nova/tests/unit/virt/ironic/test_driver.py +++ b/nova/tests/unit/virt/ironic/test_driver.py @@ -53,23 +53,15 @@ from nova.virt import configdrive from nova.virt import driver from nova.virt import fake from nova.virt import hardware -from nova.virt.ironic import client_wrapper as cw from nova.virt.ironic import driver as ironic_driver from nova.virt.ironic import ironic_states CONF = cfg.CONF -FAKE_CLIENT = ironic_utils.FakeClient() - SENTINEL = object() -class FakeClientWrapper(cw.IronicClientWrapper): - def _get_client(self, retry_on_conflict=True): - return FAKE_CLIENT - - class FakeLoopingCall(object): def __init__(self): self.wait = mock.MagicMock() @@ -104,14 +96,8 @@ def _make_compute_service(hostname): return objects.Service(host=hostname) -FAKE_CLIENT_WRAPPER = FakeClientWrapper() - - -@mock.patch.object(cw, 'IronicClientWrapper', lambda *_: FAKE_CLIENT_WRAPPER) class IronicDriverTestCase(test.NoDBTestCase): - @mock.patch.object(cw, 'IronicClientWrapper', - lambda *_: FAKE_CLIENT_WRAPPER) @mock.patch.object(ironic_driver.IronicDriver, '_refresh_hash_ring') @mock.patch.object(servicegroup, 'API', autospec=True) def setUp(self, mock_sg, mock_hash): @@ -674,27 +660,31 @@ class IronicDriverTestCase(test.NoDBTestCase): self.mock_conn.get_node.return_value = node self.mock_conn.nodes.return_value = iter([node]) - self.assertTrue(self.driver.node_is_available(node.uuid)) + result = self.driver.node_is_available(node.uuid) + + self.assertTrue(result) self.mock_conn.nodes.assert_called_with( fields=ironic_driver._NODE_FIELDS) self.assertEqual(0, self.mock_conn.get_node.call_count) - @mock.patch.object(FAKE_CLIENT.node, 'list') - @mock.patch.object(FAKE_CLIENT.node, 'get') @mock.patch.object(objects.InstanceList, 'get_uuids_by_host') @mock.patch.object(objects.ServiceList, 'get_all_computes_by_hv_type') - def test_node_is_available_with_cache(self, mock_services, mock_instances, - mock_get, mock_list): + def test_node_is_available_with_cache(self, mock_services, mock_instances): node = _get_cached_node() - mock_get.return_value = node - mock_list.return_value = [node] - # populate the cache + self.mock_conn.nodes.return_value = iter([node]) + + # populate the cache, ensuring we called the mock once but then don't + # call it again self.driver.get_available_nodes(refresh=True) - # prove that zero calls are made after populating cache - mock_list.reset_mock() - self.assertTrue(self.driver.node_is_available(node.uuid)) - self.assertEqual(0, mock_list.call_count) - self.assertEqual(0, mock_get.call_count) + self.mock_conn.nodes.assert_called_once_with( + fields=ironic_driver._NODE_FIELDS, + ) + self.mock_conn.nodes.reset_mock() + + result = self.driver.node_is_available(node.uuid) + + self.assertTrue(result) + self.mock_conn.nodes.assert_not_called() def test__node_resources_unavailable(self): node_dicts = [ @@ -1103,18 +1093,18 @@ class IronicDriverTestCase(test.NoDBTestCase): result = self.ptree.data(mock.sentinel.nodename).traits self.assertEqual(set(traits), result) - @mock.patch.object(FAKE_CLIENT.node, 'list') @mock.patch.object(objects.InstanceList, 'get_uuids_by_host') @mock.patch.object(objects.ServiceList, 'get_all_computes_by_hv_type') @mock.patch.object(ironic_driver.IronicDriver, '_node_resource') - def test_get_available_resource(self, mock_nr, mock_services, - mock_instances, mock_list): + def test_get_available_resource( + self, mock_nr, mock_services, mock_instances, + ): node = _get_cached_node() node_2 = _get_cached_node(id=uuidutils.generate_uuid()) fake_resource = 'fake-resource' self.mock_conn.get_node.return_value = node # ensure cache gets populated without the node we want - mock_list.return_value = [node_2] + self.mock_conn.nodes.return_value = [node_2] mock_nr.return_value = fake_resource result = self.driver.get_available_resource(node.id) @@ -1250,6 +1240,21 @@ class IronicDriverTestCase(test.NoDBTestCase): instance_id=instance.uuid, fields=ironic_driver._NODE_FIELDS) + @mock.patch.object(ironic_driver.LOG, 'error') + def test__get_node_list_bad_response(self, mock_error): + fake_nodes = [_get_cached_node(), + ironic_utils.get_test_node(driver='fake', + id=uuidutils.generate_uuid())] + self.mock_conn.nodes.side_effect = [iter(fake_nodes), Exception()] + + result = self.driver._get_node_list() + mock_error.assert_not_called() + self.assertEqual(fake_nodes, result) + + self.assertRaises(exception.VirtDriverNotReady, + self.driver._get_node_list) + mock_error.assert_called_once() + @mock.patch.object(objects.InstanceList, 'get_uuids_by_host') @mock.patch.object(objects.ServiceList, 'get_all_computes_by_hv_type') def test_get_info_http_not_found(self, mock_svc_by_hv, @@ -2232,16 +2237,15 @@ class IronicDriverTestCase(test.NoDBTestCase): @mock.patch.object(configdrive, 'required_by') @mock.patch.object(ironic_driver.IronicDriver, '_add_instance_info_to_node') - @mock.patch.object(FAKE_CLIENT.node, 'get') @mock.patch.object(objects.Instance, 'save') - def test_rebuild_with_configdrive_failure(self, mock_save, mock_get, + def test_rebuild_with_configdrive_failure(self, mock_save, mock_add_instance_info, mock_required_by, mock_configdrive): node_uuid = uuidutils.generate_uuid() node = _get_cached_node( uuid=node_uuid, instance_uuid=self.instance_uuid) - mock_get.return_value = node + self.mock_conn.get_node.return_value = node mock_required_by.return_value = True mock_configdrive.side_effect = exception.NovaException() @@ -2262,15 +2266,14 @@ class IronicDriverTestCase(test.NoDBTestCase): @mock.patch.object(configdrive, 'required_by') @mock.patch.object(ironic_driver.IronicDriver, '_add_instance_info_to_node') - @mock.patch.object(FAKE_CLIENT.node, 'get') @mock.patch.object(objects.Instance, 'save') - def test_rebuild_failures(self, mock_save, mock_get, + def test_rebuild_failures(self, mock_save, mock_add_instance_info, mock_required_by, mock_configdrive): node_uuid = uuidutils.generate_uuid() node = _get_cached_node( uuid=node_uuid, instance_uuid=self.instance_uuid) - mock_get.return_value = node + self.mock_conn.get_node.return_value = node mock_required_by.return_value = False image_meta = ironic_utils.get_test_image_meta() @@ -2289,8 +2292,7 @@ class IronicDriverTestCase(test.NoDBTestCase): bdms=None, detach_block_devices=None, attach_block_devices=None) - @mock.patch.object(FAKE_CLIENT.node, 'get') - def test_network_binding_host_id(self, mock_get): + def test_network_binding_host_id(self): node_uuid = uuidutils.generate_uuid() hostname = 'ironic-compute' instance = fake_instance.fake_instance_obj(self.ctx, @@ -2299,7 +2301,7 @@ class IronicDriverTestCase(test.NoDBTestCase): node = ironic_utils.get_test_node(uuid=node_uuid, instance_uuid=self.instance_uuid, network_interface='flat') - mock_get.return_value = node + self.mock_conn.get_node.return_value = node host_id = self.driver.network_binding_host_id(self.ctx, instance) self.assertIsNone(host_id) @@ -2469,23 +2471,7 @@ class IronicDriverTestCase(test.NoDBTestCase): self.driver.clean_networks_preparation(instance, network_info) mock_upvifs.assert_called_once_with(instance, network_info) - @mock.patch.object(ironic_driver.LOG, 'error') - def test_ironicclient_bad_response(self, mock_error): - fake_nodes = [_get_cached_node(), - ironic_utils.get_test_node(driver='fake', - id=uuidutils.generate_uuid())] - self.mock_conn.nodes.side_effect = [iter(fake_nodes), Exception()] - - result = self.driver._get_node_list() - mock_error.assert_not_called() - self.assertEqual(fake_nodes, result) - - self.assertRaises(exception.VirtDriverNotReady, - self.driver._get_node_list) - mock_error.assert_called_once() - - @mock.patch.object(cw.IronicClientWrapper, 'call') - def test_prepare_for_spawn(self, mock_call): + def test_prepare_for_spawn(self): node = ironic_utils.get_test_node( driver='fake', instance_uuid=None, provision_state=ironic_states.AVAILABLE, @@ -2577,9 +2563,7 @@ class IronicDriverTestCase(test.NoDBTestCase): @mock.patch.object(ironic_driver.IronicDriver, '_unplug_vifs') @mock.patch.object(ironic_driver.IronicDriver, '_cleanup_volume_target_info') - @mock.patch.object(cw.IronicClientWrapper, 'call') - def test__cleanup_deploy_no_remove_ii(self, mock_call, mock_vol, - mock_unvif): + def test__cleanup_deploy_no_remove_ii(self, mock_vol, mock_unvif): # TODO(TheJulia): This REALLY should be updated to cover all of the # calls that take place. node = ironic_utils.get_test_node(driver='fake') @@ -2588,7 +2572,6 @@ class IronicDriverTestCase(test.NoDBTestCase): self.driver._cleanup_deploy(node, instance, remove_instance_info=False) mock_vol.assert_called_once_with(instance) mock_unvif.assert_called_once_with(node, instance, None) - self.assertFalse(mock_call.called) class IronicDriverSyncTestCase(IronicDriverTestCase): @@ -2725,7 +2708,7 @@ class IronicDriverSyncTestCase(IronicDriverTestCase): def test__can_send_version(self, mock_supports_microversion): mock_supports_microversion.return_value = True - version = '%d.%d' % cw.IRONIC_API_VERSION + version = '%d.%d' % ironic_driver.IRONIC_API_VERSION self.assertIsNone( self.driver._can_send_version(version) ) @@ -2735,7 +2718,8 @@ class IronicDriverSyncTestCase(IronicDriverTestCase): mock_supports_microversion.return_value = False version = '%d.%d' % ( - cw.IRONIC_API_VERSION[0], cw.IRONIC_API_VERSION[1] + 1, + ironic_driver.IRONIC_API_VERSION[0], + ironic_driver.IRONIC_API_VERSION[1] + 1, ) self.assertRaises( exception.IronicAPIVersionNotAvailable, @@ -2756,8 +2740,6 @@ class IronicDriverSyncTestCase(IronicDriverTestCase): class IronicDriverGenerateConfigDriveTestCase(test.NoDBTestCase): @mock.patch.object(objects.ServiceList, 'get_all_computes_by_hv_type') - @mock.patch.object(cw, 'IronicClientWrapper', - lambda *_: FAKE_CLIENT_WRAPPER) def setUp(self, mock_services): super(IronicDriverGenerateConfigDriveTestCase, self).setUp() self.driver = ironic_driver.IronicDriver(None) @@ -3223,8 +3205,6 @@ class NodeCacheTestCase(test.NoDBTestCase): class IronicDriverConsoleTestCase(test.NoDBTestCase): - @mock.patch.object(cw, 'IronicClientWrapper', - lambda *_: FAKE_CLIENT_WRAPPER) @mock.patch.object(objects.ServiceList, 'get_all_computes_by_hv_type') def setUp(self, mock_services): super().setUp() diff --git a/nova/tests/unit/virt/ironic/utils.py b/nova/tests/unit/virt/ironic/utils.py index 3560a80da459..6cd652d177a6 100644 --- a/nova/tests/unit/virt/ironic/utils.py +++ b/nova/tests/unit/virt/ironic/utils.py @@ -16,7 +16,6 @@ from openstack.baremetal.v1 import node as _node from nova import objects -from nova.virt.ironic import client_wrapper from nova.virt.ironic import ironic_states @@ -190,88 +189,3 @@ def get_test_flavor(**kw): def get_test_image_meta(**kw): return objects.ImageMeta.from_dict( {'id': kw.get('id', 'cccccccc-cccc-cccc-cccc-cccccccccccc')}) - - -class FakeVolumeTargetClient(object): - - def create(self, node_uuid, volume_type, properties, boot_index, - volume_id): - pass - - def delete(self, volume_target_id): - pass - - -class FakePortClient(object): - - def list(self, address=None, limit=None, marker=None, sort_key=None, - sort_dir=None, detail=False, fields=None, node=None, - portgroup=None): - pass - - def get(self, port_uuid): - pass - - def update(self, port_uuid, patch): - pass - - -class FakePortgroupClient(object): - - def list(self, node=None, address=None, limit=None, marker=None, - sort_key=None, sort_dir=None, detail=False, fields=None): - pass - - -class FakeNodeClient(object): - - def list(self, associated=None, maintenance=None, marker=None, limit=None, - detail=False, sort_key=None, sort_dir=None, fields=None, - provision_state=None, driver=None, resource_class=None, - chassis=None): - return [] - - def get(self, node_uuid, fields=None): - pass - - def get_by_instance_uuid(self, instance_uuid, fields=None): - pass - - def list_ports(self, node_uuid, detail=False): - pass - - def set_power_state(self, node_uuid, target, soft=False, timeout=None): - pass - - def set_provision_state(self, node_uuid, state, configdrive=None, - cleansteps=None, rescue_password=None, - os_ironic_api_version=None): - pass - - def update(self, node_uuid, patch): - pass - - def validate(self, node_uuid): - pass - - def vif_attach(self, node_uuid, port_id): - pass - - def vif_detach(self, node_uuid, port_id): - pass - - def inject_nmi(self, node_uuid): - pass - - def list_volume_targets(self, node_uuid, detail=False): - pass - - -class FakeClient(object): - - node = FakeNodeClient() - port = FakePortClient() - portgroup = FakePortgroupClient() - volume_target = FakeVolumeTargetClient() - current_api_version = '%d.%d' % client_wrapper.IRONIC_API_VERSION - is_api_version_negotiated = True diff --git a/nova/virt/ironic/client_wrapper.py b/nova/virt/ironic/client_wrapper.py deleted file mode 100644 index 4ad3364aa403..000000000000 --- a/nova/virt/ironic/client_wrapper.py +++ /dev/null @@ -1,198 +0,0 @@ -# Copyright 2014 Hewlett-Packard Development Company, L.P. -# All Rights Reserved. -# -# Licensed under the Apache License, Version 2.0 (the "License"); you may -# not use this file except in compliance with the License. You may obtain -# a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT -# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the -# License for the specific language governing permissions and limitations -# under the License. - -from keystoneauth1 import discover as ks_disc -from keystoneauth1 import loading as ks_loading -from oslo_log import log as logging -from oslo_utils import importutils - -import nova.conf -from nova import exception -from nova.i18n import _ -from nova import utils - - -LOG = logging.getLogger(__name__) -CONF = nova.conf.CONF - -ironic = None - -IRONIC_GROUP = nova.conf.ironic.ironic_group - -# The API version required by the Ironic driver -IRONIC_API_VERSION = (1, 46) -# NOTE(TheJulia): This version should ALWAYS be the _last_ release -# supported version of the API version used by nova. If a feature -# needs a higher version to be negotiated to operate properly, then the version -# above should be updated, and this version should only be changed -# once a cycle to the API version desired for features merging in -# that cycle. -PRIOR_IRONIC_API_VERSION = (1, 38) - - -class IronicClientWrapper(object): - """Ironic client wrapper class that encapsulates authentication logic.""" - - def __init__(self): - """Initialise the IronicClientWrapper for use. - - Initialise IronicClientWrapper by loading ironicclient - dynamically so that ironicclient is not a dependency for - Nova. - """ - global ironic - if ironic is None: - ironic = importutils.import_module('ironicclient') - self._cached_client = None - - def _get_auth_plugin(self): - """Load an auth plugin from CONF options.""" - # If an auth plugin name is defined in `auth_type` option of [ironic] - # group, register its options and load it. - auth_plugin = ks_loading.load_auth_from_conf_options(CONF, - IRONIC_GROUP.name) - - return auth_plugin - - def _get_client(self, retry_on_conflict=True): - max_retries = CONF.ironic.api_max_retries if retry_on_conflict else 1 - retry_interval = (CONF.ironic.api_retry_interval - if retry_on_conflict else 0) - - # If we've already constructed a valid, authed client, just return - # that. - if retry_on_conflict and self._cached_client is not None: - return self._cached_client - - auth_plugin = self._get_auth_plugin() - - sess = ks_loading.load_session_from_conf_options(CONF, - IRONIC_GROUP.name, - auth=auth_plugin) - - # Retries for Conflict exception - kwargs = {} - kwargs['max_retries'] = max_retries - kwargs['retry_interval'] = retry_interval - # NOTE(TheJulia): The ability for a list of available versions to be - # accepted was added in python-ironicclient 2.2.0. The highest - # available version will be utilized by the client for the lifetime - # of the client. - kwargs['os_ironic_api_version'] = [ - '%d.%d' % IRONIC_API_VERSION, '%d.%d' % PRIOR_IRONIC_API_VERSION] - - ironic_conf = CONF[IRONIC_GROUP.name] - # valid_interfaces is a list. ironicclient passes this kwarg through to - # ksa, which is set up to handle 'interface' as either a list or a - # single value. - kwargs['interface'] = ironic_conf.valid_interfaces - - # NOTE(clenimar/efried): by default, the endpoint is taken from the - # service catalog. Use `endpoint_override` if you want to override it. - try: - ksa_adap = utils.get_ksa_adapter( - nova.conf.ironic.DEFAULT_SERVICE_TYPE, - ksa_auth=auth_plugin, ksa_session=sess, - min_version=(IRONIC_API_VERSION[0], 0), - max_version=(IRONIC_API_VERSION[0], ks_disc.LATEST)) - ironic_url = ksa_adap.get_endpoint() - ironic_url_none_reason = 'returned None' - except exception.ServiceNotFound: - # NOTE(efried): No reason to believe service catalog lookup - # won't also fail in ironic client init, but this way will - # yield the expected exception/behavior. - ironic_url = None - ironic_url_none_reason = 'raised ServiceNotFound' - - if ironic_url is None: - LOG.warning("Could not discover ironic_url via keystoneauth1: " - "Adapter.get_endpoint %s", ironic_url_none_reason) - # NOTE(eandersson): We pass in region here to make sure - # that the Ironic client can make an educated decision when - # we don't have a valid endpoint to pass on. - kwargs['region_name'] = ironic_conf.region_name - - try: - cli = ironic.client.get_client(IRONIC_API_VERSION[0], - endpoint=ironic_url, - session=sess, **kwargs) - # Cache the client so we don't have to reconstruct and - # reauthenticate it every time we need it. - if retry_on_conflict: - self._cached_client = cli - - except ironic.exc.Unauthorized: - msg = _("Unable to authenticate Ironic client.") - LOG.error(msg) - raise exception.NovaException(msg) - - return cli - - def _multi_getattr(self, obj, attr): - """Support nested attribute path for getattr(). - - :param obj: Root object. - :param attr: Path of final attribute to get. E.g., "a.b.c.d" - - :returns: The value of the final named attribute. - :raises: AttributeError will be raised if the path is invalid. - """ - for attribute in attr.split("."): - obj = getattr(obj, attribute) - return obj - - def call(self, method, *args, **kwargs): - """Call an Ironic client method and retry on stale token. - - :param method: Name of the client method to call as a string. - :param args: Client method arguments. - :param kwargs: Client method keyword arguments. - :param retry_on_conflict: Boolean value. Whether the request should be - retried in case of a conflict error - (HTTP 409) or not. If retry_on_conflict is - False the cached instance of the client - won't be used. Defaults to True. - """ - retry_on_conflict = kwargs.pop('retry_on_conflict', True) - - # authentication retry for token expiration is handled in keystone - # session, other retries are handled by ironicclient starting with - # 0.8.0 - client = self._get_client(retry_on_conflict=retry_on_conflict) - return self._multi_getattr(client, method)(*args, **kwargs) - - @property - def current_api_version(self): - """Value representing the negotiated API client version. - - This value represents the current negotiated API version that - is being utilized by the client to permit the caller to make - decisions based upon that version. - - :returns: The highest available negotiatable version or None - if a version has not yet been negotiated by the underlying - client library. - """ - return self._get_client().current_api_version - - @property - def is_api_version_negotiated(self): - """Boolean to indicate if the client version has been negotiated. - - :returns: True if the underlying client library has completed API - version negotiation. Otherwise the value returned is - False. - """ - return self._get_client().is_api_version_negotiated diff --git a/nova/virt/ironic/driver.py b/nova/virt/ironic/driver.py index ab400c7b9474..78eb4fd77df5 100644 --- a/nova/virt/ironic/driver.py +++ b/nova/virt/ironic/driver.py @@ -52,17 +52,16 @@ from nova import utils from nova.virt import configdrive from nova.virt import driver as virt_driver from nova.virt import hardware -from nova.virt.ironic import client_wrapper from nova.virt.ironic import ironic_states from nova.virt.ironic import patcher from nova.virt import netutils - LOG = logging.getLogger(__name__) - - CONF = nova.conf.CONF +# The API version required by the Ironic driver +IRONIC_API_VERSION = (1, 46) + _POWER_STATE_MAP = { ironic_states.POWER_ON: power_state.RUNNING, ironic_states.NOSTATE: power_state.NOSTATE, @@ -575,7 +574,7 @@ class IronicDriver(virt_driver.ComputeDriver): def _get_hypervisor_version(self): """Returns the version of the Ironic API service endpoint.""" - return client_wrapper.IRONIC_API_VERSION[0] + return IRONIC_API_VERSION[0] def instance_exists(self, instance): """Checks the existence of an instance. diff --git a/nova/virt/ironic/patcher.py b/nova/virt/ironic/patcher.py index a8a66cae0fb7..e1f9a6256c78 100644 --- a/nova/virt/ironic/patcher.py +++ b/nova/virt/ironic/patcher.py @@ -29,7 +29,7 @@ CONF = nova.conf.CONF def create(node): """Create an instance of the appropriate DriverFields class. - :param node: a node object returned from ironicclient + :param node: a node object returned from openstacksdk :returns: A GenericDriverFields instance. """ return GenericDriverFields(node) diff --git a/test-requirements.txt b/test-requirements.txt index a5238bb96d96..74c4fca2177e 100644 --- a/test-requirements.txt +++ b/test-requirements.txt @@ -11,7 +11,6 @@ fixtures>=3.0.0 # Apache-2.0/BSD psycopg2-binary>=2.8 # LGPL/ZPL PyMySQL>=0.8.0 # MIT License python-barbicanclient>=4.5.2 # Apache-2.0 -python-ironicclient>=3.0.0 # Apache-2.0 requests-mock>=1.2.0 # Apache-2.0 oslotest>=3.8.0 # Apache-2.0 stestr>=2.0.0 # Apache-2.0