openstack
/
nova
Code Issues Proposed changes

Merge "Remove [ironic]api_endpoint option"

This commit is contained in:
Zuul 2019-05-03 21:08:38 +00:00 committed by Gerrit Code Review
parent 2b337acf59 d54f35d54e
commit d1243d3473
5 changed files with 37 additions and 74 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
nova/api/openstack/compute/baremetal_nodes.py
View File

@ -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

22
nova/conf/ironic.py
View File

@ -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:
'If partition_key is unset, this option 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))
}

29
nova/tests/unit/virt/ironic/test_client_wrapper.py
View File

@ -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,

48
nova/virt/ironic/client_wrapper.py
View File

@ -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],

7
releasenotes/notes/remove-ironic-api_endpoint-db922182356b8ac2.yaml Normal file
View File

@ -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.)