openstack/ironic
Code Issues Proposed changes

Introduce keystoneauth adapters for clients

Browse Source 
Currently ironic explicitly or implicitly sets the API urls
for most services in the config.
This is quite fragile and we should move to discovery from
the keystone catalog eventually.

To support this, this patch registers `keystoneauth1.adapter.Adapter`
options to all config sections for service clients auth.
Among others it exports `interfaces` option that we set to
['internal', 'public'] by default.
Other exported options are `service_type`, `service_name`, `region_name`
and `endpoint_override`.
The latter will eventually be used by all clients to specify a specific
endpoint to use (for example in noauth mode).

Effectively this patch starts to move all clients code to load client
configuration from config for all of auth, session and adapter.

The first to move is [service_catalog] section, with [conductor]api_url
option being deprecated in favor of [service_catalog]endpoint_override.
A sane default of 'service_type' = 'baremetal' is set for this config
section as well.

More patches moving other clients to consume these new options and
deprecate some other options will follow.

Change-Id: I1283ef3b4d736ac089df0cc74a5850a93b24b6ab
Partial-Bug: #1699547
Related-Bug: #1699542
This commit is contained in:
Pavlo Shchelokovskyy 2017-06-23 14:17:26 +00:00
parent cd81528a4e
commit 308e414a57
11 changed files with 217 additions and 69 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

43
devstack/lib/ironic
View File

@ -1058,9 +1058,13 @@ function configure_ironic_api {
cp -p $IRONIC_DIR/etc/ironic/policy.json $IRONIC_POLICY_JSON
}
function configure_auth_for {
# configure_client_for() - is used by configure_ironic_conductor.
# Sets options to instantiate clients for other services
# single argument - config section to fill
function configure_client_for {
local service_config_section
service_config_section=$1
# keystoneauth auth plugin options
iniset $IRONIC_CONF_FILE $service_config_section auth_type password
iniset $IRONIC_CONF_FILE $service_config_section auth_url $KEYSTONE_SERVICE_URI
iniset $IRONIC_CONF_FILE $service_config_section username ironic
@ -1068,24 +1072,39 @@ function configure_auth_for {
iniset $IRONIC_CONF_FILE $service_config_section project_name $SERVICE_PROJECT_NAME
iniset $IRONIC_CONF_FILE $service_config_section user_domain_id default
iniset $IRONIC_CONF_FILE $service_config_section project_domain_id default
# keystoneauth session options
iniset $IRONIC_CONF_FILE $service_config_section cafile $SSL_BUNDLE_FILE
}
# TODO(pas-ha) this function is for transition period only,
# after all clients are moved to use keystoneauth adapters, it will be merged
# into configure_client_for function
function configure_adapter_for {
local service_config_section
service_config_section=$1
# keystoneauth adapter options
# NOTE(pas-ha) relying on defaults for valid_interfaces being "internal,public" in ironic
iniset $IRONIC_CONF_FILE $service_config_section region_name $REGION_NAME
}
# configure_ironic_conductor() - Is used by configure_ironic().
# Sets conductor specific settings.
function configure_ironic_conductor {
# set keystone region for all services
iniset $IRONIC_CONF_FILE keystone region_name $REGION_NAME
# NOTE(pas-ha) service_catalog section is used to discover
# ironic API endpoint from keystone catalog
local client_sections="neutron swift glance inspector cinder service_catalog"
for conf_section in $client_sections; do
configure_client_for $conf_section
done
# set keystone auth plugin options for services
configure_auth_for neutron
configure_auth_for swift
configure_auth_for glance
configure_auth_for inspector
configure_auth_for cinder
# this one is needed for lookup of Ironic API endpoint via Keystone
configure_auth_for service_catalog
# TODO(pas-ha) this block is for transition period only,
# after all clients are moved to use keystoneauth adapters,
# it will be deleted
local sections_with_adapter="service_catalog"
for conf_section in $sections_with_adapter; do
configure_adapter_for $conf_section
done
cp $IRONIC_DIR/etc/ironic/rootwrap.conf $IRONIC_ROOTWRAP_CONF
cp -r $IRONIC_DIR/etc/ironic/rootwrap.d $IRONIC_CONF_DIR
@ -1239,8 +1258,6 @@ function create_ironic_accounts {
get_or_create_service "ironic" "baremetal" "Ironic baremetal provisioning service"
get_or_create_endpoint "baremetal" \
"$REGION_NAME" \
"$IRONIC_SERVICE_PROTOCOL://$IRONIC_HOSTPORT" \
"$IRONIC_SERVICE_PROTOCOL://$IRONIC_HOSTPORT" \
"$IRONIC_SERVICE_PROTOCOL://$IRONIC_HOSTPORT"
# Create ironic service user

50
etc/ironic/ironic.conf.sample
View File

@ -1026,10 +1026,15 @@
# Seconds between conductor heart beats. (integer value)
#heartbeat_interval = 10
# URL of Ironic API service. If not set ironic can get the
# current value from the keystone service catalog. If set, the
# value must start with either http:// or https://. (uri
# value)
# DEPRECATED: URL of Ironic API service. If not set ironic can
# get the current value from the keystone service catalog. If
# set, the value must start with either http:// or https://.
# (uri value)
# This option is deprecated for removal.
# Its value may be silently ignored in the future.
# Reason: Use [service_catalog]endpoint_override option
# instead if required to use a specific ironic api address,
# for example in noauth mode.
#api_url = <None>
# Maximum time (in seconds) since the last check-in of a
@ -3545,12 +3550,28 @@
# Domain name to scope to (string value)
#domain_name = <None>
# Always use this endpoint URL for requests for this client.
# (string value)
#endpoint_override = <None>
# Verify HTTPS connections. (boolean value)
#insecure = false
# PEM encoded client certificate key file (string value)
#keyfile = <None>
# The maximum major version of a given API, intended to be
# used as the upper bound of a range with min_version.
# Mutually exclusive with version. (string value)
#max_version = <None>
# The minimum major version of a given API, intended to be
# used as the lower bound of a range with max_version.
# Mutually exclusive with version. If min_version is given
# with no max_version it is as if max version is "latest".
# (string value)
#min_version = <None>
# User's password (string value)
#password = <None>
@ -3568,6 +3589,18 @@
# Deprecated group/name - [service_catalog]/tenant_name
#project_name = <None>
# The default region_name for endpoint URL discovery. (string
# value)
#region_name = <None>
# The default service_name for endpoint URL discovery. (string
# value)
#service_name = <None>
# The default service_type for endpoint URL discovery. (string
# value)
#service_type = baremetal
# Tenant ID (string value)
#tenant_id = <None>
@ -3593,6 +3626,15 @@
# Deprecated group/name - [service_catalog]/user_name
#username = <None>
# List of interfaces, in order of preference, for endpoint
# URL. (list value)
#valid_interfaces = internal,public
# Minimum Major API version within a given Major API version
# for endpoint URL discovery. Mutually exclusive with
# min_version and max_version (string value)
#version = <None>
[snmp]

24
ironic/common/keystone.py
View File

@ -20,6 +20,7 @@ from oslo_log import log as logging
import six
from ironic.common import exception
from ironic.conf import auth as auth_conf
from ironic.conf import CONF
@ -86,7 +87,22 @@ def get_auth(group, **auth_kwargs):
return auth
# NOTE(pas-ha) Used by neutronclient and resolving ironic API only
@ks_exceptions
def get_adapter(group, **adapter_kwargs):
"""Loads adapter from options in a configuration file section.
The adapter_kwargs will be passed directly to keystoneauth1 Adapter
and will override the values loaded from config.
Consult keystoneauth1 docs for available adapter options.
:param group: name of the config section to load adapter options from
"""
return kaloading.load_adapter_from_conf_options(CONF, group,
**adapter_kwargs)
# NOTE(pas-ha) Used by neutronclient and glanceclient only
# FIXME(pas-ha) remove this while moving to kesytoneauth adapters
@ks_exceptions
def get_service_url(session, **kwargs):
@ -103,7 +119,5 @@ def get_service_url(session, **kwargs):
if 'interface' in kwargs:
return session.get_endpoint(**kwargs)
try:
return session.get_endpoint(interface='internal', **kwargs)
except kaexception.EndpointNotFound:
return session.get_endpoint(interface='public', **kwargs)
return session.get_endpoint(interface=auth_conf.DEFAULT_VALID_INTERFACES,
**kwargs)

19
ironic/conf/auth.py
View File

@ -15,13 +15,16 @@
import copy
from keystoneauth1 import loading as kaloading
from oslo_config import cfg
from oslo_log import log
LOG = log.getLogger(__name__)
DEFAULT_VALID_INTERFACES = ['internal', 'public']
def register_auth_opts(conf, group):
def register_auth_opts(conf, group, service_type=None):
"""Register session- and auth-related options
Registers only basic auth options shared by all auth plugins.
@ -29,9 +32,14 @@ def register_auth_opts(conf, group):
"""
kaloading.register_session_conf_options(conf, group)
kaloading.register_auth_conf_options(conf, group)
if service_type:
kaloading.register_adapter_conf_options(conf, group)
conf.set_default('valid_interfaces', DEFAULT_VALID_INTERFACES,
group=group)
conf.set_default('service_type', service_type, group=group)
def add_auth_opts(options):
def add_auth_opts(options, service_type=None):
"""Add auth options to sample config
As these are dynamically registered at runtime,
@ -55,5 +63,12 @@ def add_auth_opts(options):
plugin = kaloading.get_plugin_loader(name)
add_options(opts, kaloading.get_auth_plugin_conf_options(plugin))
add_options(opts, kaloading.get_session_conf_options())
if service_type:
adapter_opts = kaloading.get_adapter_conf_options(
include_deprecated=False)
# adding defaults for valid interfaces
cfg.set_defaults(adapter_opts, service_type=service_type,
valid_interfaces=DEFAULT_VALID_INTERFACES)
add_options(opts, adapter_opts)
opts.sort(key=lambda x: x.name)
return opts

5
ironic/conf/conductor.py
View File

@ -30,6 +30,11 @@ opts = [
help=_('Seconds between conductor heart beats.')),
cfg.URIOpt('api_url',
schemes=('http', 'https'),
deprecated_for_removal=True,
deprecated_reason=_("Use [service_catalog]endpoint_override "
"option instead if required to use "
"a specific ironic api address, "
"for example in noauth mode."),
help=_('URL of Ironic API service. If not set ironic can '
'get the current value from the keystone service '
'catalog. If set, the value must start with either '

5
ironic/conf/service_catalog.py
View File

@ -26,8 +26,9 @@ SERVICE_CATALOG_GROUP = cfg.OptGroup(
def register_opts(conf):
auth.register_auth_opts(conf, SERVICE_CATALOG_GROUP.name)
auth.register_auth_opts(conf, SERVICE_CATALOG_GROUP.name,
service_type='baremetal')
def list_opts():
return auth.add_auth_opts([])
return auth.add_auth_opts([], service_type='baremetal')

38
ironic/drivers/modules/deploy_utils.py
View File

@ -77,9 +77,7 @@ _IRONIC_SESSION = None
def _get_ironic_session():
global _IRONIC_SESSION
if not _IRONIC_SESSION:
auth = keystone.get_auth('service_catalog')
_IRONIC_SESSION = keystone.get_session('service_catalog',
auth=auth)
_IRONIC_SESSION = keystone.get_session('service_catalog')
return _IRONIC_SESSION
@ -94,18 +92,28 @@ def get_ironic_api_url():
either from config of from Keystone catalog.
"""
ironic_api = CONF.conductor.api_url
if not ironic_api:
try:
ironic_session = _get_ironic_session()
ironic_api = keystone.get_service_url(ironic_session)
except (exception.KeystoneFailure,
exception.CatalogNotFound,
exception.KeystoneUnauthorized) as e:
raise exception.InvalidParameterValue(_(
"Couldn't get the URL of the Ironic API service from the "
"configuration file or keystone catalog. Keystone error: "
"%s") % six.text_type(e))
adapter_opts = {'session': _get_ironic_session()}
# NOTE(pas-ha) force 'none' auth plugin for noauth mode
if CONF.auth_strategy != 'keystone':
CONF.set_override('auth_type', 'none', group='service_catalog')
adapter_opts['auth'] = keystone.get_auth('service_catalog')
# TODO(pas-ha) remove in Rocky
# NOTE(pas-ha) if both set, the new options win
if CONF.conductor.api_url and not CONF.service_catalog.endpoint_override:
adapter_opts['endpoint_override'] = CONF.conductor.api_url
if CONF.keystone.region_name and not CONF.service_catalog.region_name:
adapter_opts['region_name'] = CONF.keystone.region_name
adapter = keystone.get_adapter('service_catalog', **adapter_opts)
try:
ironic_api = adapter.get_endpoint()
except (exception.KeystoneFailure,
exception.CatalogNotFound,
exception.KeystoneUnauthorized) as e:
raise exception.InvalidParameterValue(_(
"Couldn't get the URL of the Ironic API service from the "
"configuration file or keystone catalog. Keystone error: "
"%s") % six.text_type(e))
# NOTE: we should strip '/' from the end because it might be used in
# hardcoded ramdisk script
ironic_api = ironic_api.rstrip('/')

27
ironic/tests/unit/common/test_keystone.py
View File

@ -12,7 +12,6 @@
# License for the specific language governing permissions and limitations
# under the License.
from keystoneauth1 import exceptions as ksexception
from keystoneauth1 import loading as kaloading
import mock
from oslo_config import cfg
@ -32,7 +31,8 @@ class KeystoneTestCase(base.TestCase):
group='keystone')
self.test_group = 'test_group'
self.cfg_fixture.conf.register_group(cfg.OptGroup(self.test_group))
ironic_auth.register_auth_opts(self.cfg_fixture.conf, self.test_group)
ironic_auth.register_auth_opts(self.cfg_fixture.conf, self.test_group,
service_type='vikings')
self.config(auth_type='password',
group=self.test_group)
# NOTE(pas-ha) this is due to auth_plugin options
@ -76,20 +76,19 @@ class KeystoneTestCase(base.TestCase):
self.assertEqual('spam', keystone.get_service_url(session, **params))
session.get_endpoint.assert_called_once_with(**params)
def test_get_service_url_internal(self):
def test_get_service_url(self):
session = mock.Mock()
session.get_endpoint.return_value = 'spam'
params = {'ham': 'eggs'}
self.assertEqual('spam', keystone.get_service_url(session, **params))
session.get_endpoint.assert_called_once_with(interface='internal',
**params)
session.get_endpoint.assert_called_once_with(
interface=['internal', 'public'], **params)
def test_get_service_url_internal_fail(self):
session = mock.Mock()
session.get_endpoint.side_effect = [ksexception.EndpointNotFound(),
'spam']
params = {'ham': 'eggs'}
self.assertEqual('spam', keystone.get_service_url(session, **params))
session.get_endpoint.assert_has_calls([
mock.call(interface='internal', **params),
mock.call(interface='public', **params)])
def test_get_adapter_from_config(self):
self.config(valid_interfaces=['internal', 'public'],
group=self.test_group)
session = keystone.get_session(self.test_group)
adapter = keystone.get_adapter(self.test_group, session=session,
interface='admin')
self.assertEqual('admin', adapter.interface)
self.assertEqual(session, adapter.session)

30
ironic/tests/unit/drivers/modules/test_deploy_utils.py
View File

@ -1214,38 +1214,42 @@ class OtherFunctionTestCase(db_base.DbTestCase):
mock_clean_up_caches.assert_called_once_with(None, 'master_dir',
[('uuid', 'path')])
@mock.patch('ironic.common.keystone.get_auth')
@mock.patch.object(utils, '_get_ironic_session')
@mock.patch('ironic.common.keystone.get_service_url')
def test_get_ironic_api_url_from_config(self, mock_get_url, mock_ks):
def test_get_ironic_api_url_from_config(self, mock_ks, mock_auth):
mock_sess = mock.Mock()
mock_ks.return_value = mock_sess
fake_api_url = 'http://foo/'
mock_get_url.side_effect = exception.KeystoneFailure
self.config(api_url=fake_api_url, group='conductor')
url = utils.get_ironic_api_url()
# also checking for stripped trailing slash
self.assertEqual(fake_api_url[:-1], url)
self.assertFalse(mock_get_url.called)
self.assertEqual(fake_api_url[:-1], utils.get_ironic_api_url())
@mock.patch('ironic.common.keystone.get_auth')
@mock.patch.object(utils, '_get_ironic_session')
@mock.patch('ironic.common.keystone.get_service_url')
def test_get_ironic_api_url_from_keystone(self, mock_get_url, mock_ks):
@mock.patch('ironic.common.keystone.get_adapter')
def test_get_ironic_api_url_from_keystone(self, mock_ka, mock_ks,
mock_auth):
mock_sess = mock.Mock()
mock_ks.return_value = mock_sess
fake_api_url = 'http://foo/'
mock_get_url.return_value = fake_api_url
mock_ka.return_value.get_endpoint.return_value = fake_api_url
# NOTE(pas-ha) endpoint_override is None by default
self.config(api_url=None, group='conductor')
url = utils.get_ironic_api_url()
# also checking for stripped trailing slash
self.assertEqual(fake_api_url[:-1], url)
mock_get_url.assert_called_with(mock_sess)
mock_ka.assert_called_with('service_catalog', session=mock_sess,
auth=mock_auth.return_value)
mock_ka.return_value.get_endpoint.assert_called_once_with()
@mock.patch('ironic.common.keystone.get_auth')
@mock.patch.object(utils, '_get_ironic_session')
@mock.patch('ironic.common.keystone.get_service_url')
def test_get_ironic_api_url_fail(self, mock_get_url, mock_ks):
@mock.patch('ironic.common.keystone.get_adapter')
def test_get_ironic_api_url_fail(self, mock_ka, mock_ks, mock_auth):
mock_sess = mock.Mock()
mock_ks.return_value = mock_sess
mock_get_url.side_effect = exception.KeystoneFailure()
mock_ka.return_value.get_endpoint.side_effect = (
exception.KeystoneFailure())
self.config(api_url=None, group='conductor')
self.assertRaises(exception.InvalidParameterValue,
utils.get_ironic_api_url)

4
ironic/tests/unit/drivers/modules/test_iscsi_deploy.py
View File

@ -521,7 +521,9 @@ class IscsiDeployMethodsTestCase(db_base.DbTestCase):
iscsi_deploy.validate, task)
mock_get_url.assert_called_once_with()
def test_validate_invalid_root_device_hints(self):
@mock.patch('ironic.drivers.modules.deploy_utils.get_ironic_api_url')
def test_validate_invalid_root_device_hints(self, mock_get_url):
mock_get_url.return_value = 'http://spam.ham/baremetal'
with task_manager.acquire(self.context, self.node.uuid,
shared=True) as task:
task.node.properties['root_device'] = {'size': 'not-int'}

41
releasenotes/notes/keystoneauth-adapter-opts-ca4f68f568e6cf6f.yaml Normal file
View File

@ -0,0 +1,41 @@
---
upgrade:
- |
To facilitate automatic discovery of services from the keystone catalog,
the configuration file sections for service clients may include these
configuration options: ``service_type``, ``service_name``,
``valid_interfaces``, ``region_name`` and other keystoneauth Adaper-related
options.
These options together must uniquely specify an endpoint for a service
registered in the keystone catalog.
Consult the ``keystoneauth`` library documentation for full list of
available options, their meaning and possible values.
Default values for ``service_type`` are set by ironic to sane defaults
based on required services and their entries in ``service-types-authority``.
The ``valid_interfaces`` option defaults to ``['internal', 'public']``.
The ``region_name`` option defaults to ``None`` and must be explicitly set
for multi-regional setup for endpoint discovery to succeed.
The configuration file sections where these new options are available are:
- service_catalog
features:
- |
Options for service endpoint discovery can now be set per-service in
appropriate service configuration file sections:
- ``[service_catalog]`` for baremetal API discovery
deprecations:
- |
Configuration option ``[conductor]api_url`` is deprecated
and will be ignored in the Rocky release.
Instead, use ``[service_catalog]endpoint_override`` configuration option
to set specific ironic API address if automatic discovery of ironic API
endpoint from keystone catalog is not desired.
This new option defaults to ``None`` and must be set explicitly if needed.