From d54f35d54e0d8274fc729d62fdaff426bb112486 Mon Sep 17 00:00:00 2001 From: Eric Fried Date: Thu, 14 Mar 2019 18:01:54 -0500 Subject: [PATCH] Remove [ironic]api_endpoint option [ironic]api_endpoint was deprecated in Queens [1] and is hereby annihilated. [1] If625411f40be0ba642baeb02950f568f43673655 Change-Id: I527f512b371705b490ba55dfab101340d417edb6 --- nova/api/openstack/compute/baremetal_nodes.py | 5 +- nova/conf/ironic.py | 22 +-------- .../unit/virt/ironic/test_client_wrapper.py | 29 ++--------- nova/virt/ironic/client_wrapper.py | 48 ++++++++----------- ...-ironic-api_endpoint-db922182356b8ac2.yaml | 7 +++ 5 files changed, 37 insertions(+), 74 deletions(-) create mode 100644 releasenotes/notes/remove-ironic-api_endpoint-db922182356b8ac2.yaml diff --git a/nova/api/openstack/compute/baremetal_nodes.py b/nova/api/openstack/compute/baremetal_nodes.py index e47f1122792b..2fec08669291 100644 --- a/nova/api/openstack/compute/baremetal_nodes.py +++ b/nova/api/openstack/compute/baremetal_nodes.py @@ -49,6 +49,9 @@ def _check_ironic_client_enabled(): def _get_ironic_client(): """return an Ironic client.""" # TODO(NobodyCam): Fix insecure setting + # NOTE(efried): This should all be replaced by ksa adapter options; but the + # nova-to-baremetal API is deprecated, so not changing it. + # https://developer.openstack.org/api-ref/compute/#bare-metal-nodes-os-baremetal-nodes-deprecated # noqa kwargs = {'os_username': CONF.ironic.admin_username, 'os_password': CONF.ironic.admin_password, 'os_auth_url': CONF.ironic.admin_url, @@ -56,7 +59,7 @@ def _get_ironic_client(): 'os_service_type': 'baremetal', 'os_endpoint_type': 'public', 'insecure': 'true', - 'endpoint': CONF.ironic.api_endpoint} + 'endpoint': CONF.ironic.endpoint_override} # NOTE(mriedem): The 1 api_version arg here is the only valid value for # the client, but it's not even used so it doesn't really matter. The # ironic client wrapper in the virt driver actually uses a hard-coded diff --git a/nova/conf/ironic.py b/nova/conf/ironic.py index b6f6c088b250..b1d09a5fb658 100644 --- a/nova/conf/ironic.py +++ b/nova/conf/ironic.py @@ -37,18 +37,6 @@ If using the Ironic driver following options must be set: """) ironic_options = [ - cfg.URIOpt( - 'api_endpoint', - schemes=['http', 'https'], - deprecated_for_removal=True, - deprecated_reason='Endpoint lookup uses the service catalog via ' - 'common keystoneauth1 Adapter configuration ' - 'options. In the current release, api_endpoint will ' - 'override this behavior, but will be ignored and/or ' - 'removed in a future release. To achieve the same ' - 'result, use the endpoint_override option instead.', - sample_default='http://ironic.example.org:6385/', - help='URL override for the Ironic API endpoint.'), cfg.IntOpt( 'api_max_retries', # TODO(raj_singh): Change this default to some sensible number @@ -103,16 +91,11 @@ Related options: 'is ignored.'), ] -deprecated_opts = { - 'endpoint_override': [cfg.DeprecatedOpt('api_endpoint', - group=ironic_group.name)]} - def register_opts(conf): conf.register_group(ironic_group) conf.register_opts(ironic_options, group=ironic_group) - confutils.register_ksa_opts(conf, ironic_group, DEFAULT_SERVICE_TYPE, - deprecated_opts=deprecated_opts) + confutils.register_ksa_opts(conf, ironic_group, DEFAULT_SERVICE_TYPE) def list_opts(): @@ -121,6 +104,5 @@ def list_opts(): ks_loading.get_session_conf_options() + ks_loading.get_auth_common_conf_options() + ks_loading.get_auth_plugin_conf_options('v3password') + - confutils.get_ksa_adapter_opts(DEFAULT_SERVICE_TYPE, - deprecated_opts=deprecated_opts)) + confutils.get_ksa_adapter_opts(DEFAULT_SERVICE_TYPE)) } diff --git a/nova/tests/unit/virt/ironic/test_client_wrapper.py b/nova/tests/unit/virt/ironic/test_client_wrapper.py index 0ffb6125b095..9c2ffe3dca0c 100644 --- a/nova/tests/unit/virt/ironic/test_client_wrapper.py +++ b/nova/tests/unit/virt/ironic/test_client_wrapper.py @@ -78,8 +78,6 @@ class IronicClientWrapperTestCase(test.NoDBTestCase): ironicclient = client_wrapper.IronicClientWrapper() # dummy call to have _get_client() called ironicclient.call("node.list") - # With no api_endpoint in the conf, endpoint is retrieved from - # nova.utils.get_ksa_adapter().get_endpoint() self.get_ksa_adapter.assert_called_once_with( 'baremetal', ksa_auth=self.get_auth_plugin.return_value, ksa_session='session', min_version=(1, 0), @@ -104,8 +102,6 @@ class IronicClientWrapperTestCase(test.NoDBTestCase): ironicclient = client_wrapper.IronicClientWrapper() # dummy call to have _get_client() called ironicclient.call("node.list") - # With no api_endpoint in the conf, endpoint is retrieved from - # nova.utils.get_endpoint_data self.get_ksa_adapter.assert_called_once_with( 'baremetal', ksa_auth=self.get_auth_plugin.return_value, ksa_session='session', min_version=(1, 0), @@ -120,37 +116,18 @@ class IronicClientWrapperTestCase(test.NoDBTestCase): '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_legacy(self, mock_ir_cli, mock_session): - """Endpoint discovery via legacy api_endpoint conf option.""" - mock_session.return_value = 'session' - endpoint = 'https://baremetal.example.com/endpoint' - self.flags(api_endpoint=endpoint, group='ironic') - ironicclient = client_wrapper.IronicClientWrapper() - # dummy call to have _get_client() called - ironicclient.call("node.list") - self.get_ksa_adapter.assert_not_called() - 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': ['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): - """Endpoint discovery via legacy api_endpoint conf option.""" + """Confirm explicit setting of valid_interfaces.""" mock_session.return_value = 'session' endpoint = 'https://baremetal.example.com/endpoint' - self.flags(api_endpoint=endpoint, group='ironic') + 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") - self.get_ksa_adapter.assert_not_called() expected = {'session': 'session', 'max_retries': CONF.ironic.api_max_retries, 'retry_interval': CONF.ironic.api_retry_interval, diff --git a/nova/virt/ironic/client_wrapper.py b/nova/virt/ironic/client_wrapper.py index 5b46ee9d5e1a..64539c583e2b 100644 --- a/nova/virt/ironic/client_wrapper.py +++ b/nova/virt/ironic/client_wrapper.py @@ -107,34 +107,28 @@ class IronicClientWrapper(object): # NOTE(clenimar/efried): by default, the endpoint is taken from the # service catalog. Use `endpoint_override` if you want to override it. - if CONF.ironic.api_endpoint: - # NOTE(efried): `api_endpoint` still overrides service catalog and - # `endpoint_override` conf options. This will be removed in a - # future release. - ironic_url = CONF.ironic.api_endpoint - else: - 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' + 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 + 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], diff --git a/releasenotes/notes/remove-ironic-api_endpoint-db922182356b8ac2.yaml b/releasenotes/notes/remove-ironic-api_endpoint-db922182356b8ac2.yaml new file mode 100644 index 000000000000..613ce7ccc205 --- /dev/null +++ b/releasenotes/notes/remove-ironic-api_endpoint-db922182356b8ac2.yaml @@ -0,0 +1,7 @@ +--- +upgrade: + - | + Config option ``[ironic]api_endpoint`` was deprecated in the 17.0.0 Queens + release and is now removed. To achieve the same effect, set the + ``[ironic]endpoint_override`` option. (However, it is preferred to omit + this setting and let the endpoint be discovered via the service catalog.)