openstack
/
ironic
Code Issues Proposed changes

Refactor common keystone methods 

- Split loading session and auth from config to separate functions,
  allow to override options loaded from config.
  This will lay ground to more efficiently load clients that allow
  passing in both session and auth plugin objects separately.

- When resoving a service endpoint, either fetch the requested interface,
  or first try 'internal' and then 'public'.
  This is done due to our config lacking any options to set the default
  interface for service endpoints to use, and we've used 'internal' by
  default, although DevStack has no such endpoints for most services
  any more [0].
  This will be changed again when gradually introducing usage of
  keystoneauth Adapters to ironic.

- Remove get_admin_token method, it was used only in glance-related code
  once, and was simply moved there.

[0] https://review.openstack.org/#/c/433272

Change-Id: I73b21098f15af4d0445f89fdd6ad4e4a42177df6
Partial-Bug: #1699547
This commit is contained in:
Pavlo Shchelokovskyy 2017-06-23 12:03:16 +00:00 committed by Pavlo Shchelokovskyy
parent 352120378f
commit 7337fefd97
12 changed files with 117 additions and 88 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
ironic/common/cinder.py
View File

@ -35,7 +35,8 @@ _CINDER_SESSION = None
def _get_cinder_session():
global _CINDER_SESSION
if not _CINDER_SESSION:
_CINDER_SESSION = keystone.get_session('cinder')
auth = keystone.get_auth('cinder')
_CINDER_SESSION = keystone.get_session('cinder', auth=auth)
return _CINDER_SESSION

3
ironic/common/glance_service/service_utils.py
View File

@ -108,7 +108,8 @@ def _get_api_server_iterator():
api_servers = []
if not CONF.glance.glance_api_servers and not CONF.glance.glance_host:
session = keystone.get_session('service_catalog')
session = keystone.get_session('glance',
auth=keystone.get_auth('glance'))
api_servers = [keystone.get_service_url(session, service_type='image',
endpoint_type='public')]
else:

6
ironic/common/image_service.py
View File

@ -41,7 +41,8 @@ _GLANCE_SESSION = None
def _get_glance_session():
global _GLANCE_SESSION
if not _GLANCE_SESSION:
_GLANCE_SESSION = keystone.get_session('glance')
auth = keystone.get_auth('glance')
_GLANCE_SESSION = keystone.get_session('glance', auth=auth)
return _GLANCE_SESSION
@ -55,8 +56,7 @@ def GlanceImageService(client=None, version=None, context=None):
service_class = getattr(module, 'GlanceImageService')
if (context is not None and CONF.glance.auth_strategy == 'keystone'
and not context.auth_token):
session = _get_glance_session()
context.auth_token = keystone.get_admin_auth_token(session)
context.auth_token = _get_glance_session().get_token()
return service_class(client, version, context)

76
ironic/common/keystone.py
View File

@ -50,44 +50,60 @@ def ks_exceptions(f):
@ks_exceptions
def get_session(group):
def get_session(group, **session_kwargs):
"""Loads session object from options in a configuration file section.
The session_kwargs will be passed directly to keystoneauth1 Session
and will override the values loaded from config.
Consult keystoneauth1 docs for available options.
:param group: name of the config section to load session options from
"""
return kaloading.load_session_from_conf_options(
CONF, group, **session_kwargs)
@ks_exceptions
def get_auth(group, **auth_kwargs):
"""Loads auth plugin from options in a configuration file section.
The auth_kwargs will be passed directly to keystoneauth1 auth plugin
and will override the values loaded from config.
Note that the accepted kwargs will depend on auth plugin type as defined
by [group]auth_type option.
Consult keystoneauth1 docs for available auth plugins and their options.
:param group: name of the config section to load auth plugin options from
"""
try:
auth = kaloading.load_auth_from_conf_options(CONF, group)
auth = kaloading.load_auth_from_conf_options(CONF, group,
**auth_kwargs)
except kaexception.MissingRequiredOptions:
LOG.error('Failed to load auth plugin from group %s', group)
raise
session = kaloading.load_session_from_conf_options(
CONF, group, auth=auth)
return session
return auth
# TODO(pas-ha) we actually should barely need this at all:
# if we instantiate a identity.Token auth plugin from incoming
# request context we could build a session with it, and each client
# would know its service_type already, looking up the endpoint by itself
# NOTE(pas-ha) Used by neutronclient and resolving ironic API only
# FIXME(pas-ha) remove this while moving to kesytoneauth adapters
@ks_exceptions
def get_service_url(session, service_type='baremetal',
endpoint_type='internal'):
"""Wrapper for get service url from keystone service catalog.
def get_service_url(session, **kwargs):
"""Find endpoint for given service in keystone catalog.
Given a service_type and an endpoint_type, this method queries
keystone service catalog and provides the url for the desired
endpoint.
If 'interrace' is provided, fetches service url of this interface.
Otherwise, first tries to fetch 'internal' endpoint,
and then the 'public' one.
:param session: keystoneauth Session object
:param kwargs: any other arguments accepted by Session.get_endpoint method
:param service_type: the keystone service for which url is required.
:param endpoint_type: the type of endpoint for the service.
:returns: an http/https url for the desired endpoint.
"""
return session.get_endpoint(service_type=service_type,
interface=endpoint_type,
region_name=CONF.keystone.region_name)
# TODO(pas-ha) move all clients to sessions, then we do not need this
@ks_exceptions
def get_admin_auth_token(session):
"""Get admin token.
Currently used for inspector, glance and swift clients.
"""
return session.get_token()
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)

7
ironic/common/neutron.py
View File

@ -30,7 +30,8 @@ _NEUTRON_SESSION = None
def _get_neutron_session():
global _NEUTRON_SESSION
if not _NEUTRON_SESSION:
_NEUTRON_SESSION = keystone.get_session('neutron')
auth = keystone.get_auth('neutron')
_NEUTRON_SESSION = keystone.get_session('neutron', auth=auth)
return _NEUTRON_SESSION
@ -60,7 +61,9 @@ def get_client(token=None):
else:
params['token'] = token
params['endpoint_url'] = url or keystone.get_service_url(
session, service_type='network')
session,
service_type='network',
region_name=CONF.keystone.region_name)
params.update({
'timeout': CONF.neutron.url_timeout or CONF.neutron.timeout,
'insecure': CONF.neutron.insecure,

3
ironic/common/swift.py
View File

@ -31,7 +31,8 @@ _SWIFT_SESSION = None
def _get_swift_session():
global _SWIFT_SESSION
if not _SWIFT_SESSION:
_SWIFT_SESSION = keystone.get_session('swift')
auth = keystone.get_auth('swift')
_SWIFT_SESSION = keystone.get_session('swift', auth=auth)
return _SWIFT_SESSION

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

@ -77,7 +77,9 @@ _IRONIC_SESSION = None
def _get_ironic_session():
global _IRONIC_SESSION
if not _IRONIC_SESSION:
_IRONIC_SESSION = keystone.get_session('service_catalog')
auth = keystone.get_auth('service_catalog')
_IRONIC_SESSION = keystone.get_session('service_catalog',
auth=auth)
return _IRONIC_SESSION

3
ironic/drivers/modules/inspector.py
View File

@ -45,7 +45,8 @@ def _get_inspector_session():
global _INSPECTOR_SESSION
if not _INSPECTOR_SESSION:
_INSPECTOR_SESSION = keystone.get_session('inspector')
_INSPECTOR_SESSION = keystone.get_session(
'inspector', auth=keystone.get_auth('inspector'))
return _INSPECTOR_SESSION

9
ironic/tests/unit/common/test_cinder.py
View File

@ -30,6 +30,7 @@ from ironic.tests.unit.db import base as db_base
from ironic.tests.unit.objects import utils as object_utils
@mock.patch.object(keystone, 'get_auth', autospec=True)
@mock.patch.object(keystone, 'get_session', autospec=True)
class TestCinderSession(base.TestCase):
@ -39,17 +40,21 @@ class TestCinderSession(base.TestCase):
retries=2,
group='cinder')
def test__get_cinder_session(self, mock_keystone_session):
def test__get_cinder_session(self, mock_keystone_session, mock_auth):
"""Check establishing new session when no session exists."""
mock_keystone_session.return_value = 'session1'
self.assertEqual('session1', cinder._get_cinder_session())
mock_keystone_session.assert_called_once_with('cinder')
mock_keystone_session.assert_called_once_with(
'cinder', auth=mock_auth.return_value)
mock_auth.assert_called_once_with('cinder')
"""Check if existing session is used."""
mock_keystone_session.reset_mock()
mock_auth.reset_mock()
mock_keystone_session.return_value = 'session2'
self.assertEqual('session1', cinder._get_cinder_session())
self.assertFalse(mock_keystone_session.called)
self.assertFalse(mock_auth.called)
@mock.patch.object(cinder, '_get_cinder_session', autospec=True)

7
ironic/tests/unit/common/test_glance_service.py
View File

@ -859,14 +859,17 @@ class TestGlanceAPIServers(base.TestCase):
@mock.patch.object(service_utils.keystone, 'get_service_url',
autospec=True)
@mock.patch.object(service_utils.keystone, 'get_session', autospec=True)
def test__get_api_servers_with_keystone(self, mock_session,
@mock.patch.object(service_utils.keystone, 'get_auth', autospec=True)
def test__get_api_servers_with_keystone(self, mock_auth, mock_session,
mock_service_url):
mock_service_url.return_value = 'https://example.com'
endpoint = service_utils._get_api_server()
self.assertEqual('https://example.com', endpoint)
mock_session.assert_called_once_with('service_catalog')
mock_auth.assert_called_once_with('glance')
mock_session.assert_called_once_with('glance',
auth=mock_auth.return_value)
mock_service_url.assert_called_once_with(mock_session.return_value,
service_type='image',
endpoint_type='public')

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

@ -13,7 +13,6 @@
# under the License.
from keystoneauth1 import exceptions as ksexception
from keystoneauth1 import identity as kaidentity
from keystoneauth1 import loading as kaloading
import mock
from oslo_config import cfg
@ -52,52 +51,45 @@ class KeystoneTestCase(base.TestCase):
self.cfg_fixture = self.useFixture(fixture.Config())
self.addCleanup(cfg.CONF.reset)
def test_get_url(self):
fake_url = 'http://127.0.0.1:6385'
mock_sess = mock.Mock()
mock_sess.get_endpoint.return_value = fake_url
res = keystone.get_service_url(mock_sess)
mock_sess.get_endpoint.assert_called_with(
interface='internal', region_name='fake_region',
service_type='baremetal')
self.assertEqual(fake_url, res)
def test_get_url_failure(self):
exc_map = (
(ksexception.Unauthorized, exception.KeystoneUnauthorized),
(ksexception.EndpointNotFound, exception.CatalogNotFound),
(ksexception.EmptyCatalog, exception.CatalogNotFound),
(ksexception.Unauthorized, exception.KeystoneUnauthorized),
)
for kexc, irexc in exc_map:
mock_sess = mock.Mock()
mock_sess.get_endpoint.side_effect = kexc
self.assertRaises(irexc, keystone.get_service_url, mock_sess)
def test_get_admin_auth_token(self):
mock_sess = mock.Mock()
mock_sess.get_token.return_value = 'fake_token'
self.assertEqual('fake_token',
keystone.get_admin_auth_token(mock_sess))
def test_get_admin_auth_token_failure(self):
mock_sess = mock.Mock()
mock_sess.get_token.side_effect = ksexception.Unauthorized
self.assertRaises(exception.KeystoneUnauthorized,
keystone.get_admin_auth_token, mock_sess)
def test_get_session(self):
session = keystone.get_session(self.test_group)
# NOTE(pas-ha) 'password' auth_plugin is used
self.assertIsInstance(session.auth,
kaidentity.generic.password.Password)
self.assertEqual('http://127.0.0.1:9898', session.auth.auth_url)
self.config(timeout=10, group=self.test_group)
session = keystone.get_session(self.test_group, timeout=20)
self.assertEqual(20, session.timeout)
def test_get_session_fail(self):
def test_get_auth(self):
auth = keystone.get_auth(self.test_group)
self.assertEqual('http://127.0.0.1:9898', auth.auth_url)
def test_get_auth_fail(self):
# NOTE(pas-ha) 'password' auth_plugin is used,
# so when we set the required auth_url to None,
# MissingOption is raised
self.config(auth_url=None, group=self.test_group)
self.assertRaises(exception.ConfigInvalid,
keystone.get_session,
keystone.get_auth,
self.test_group)
def test_get_service_url_with_interface(self):
session = mock.Mock()
session.get_endpoint.return_value = 'spam'
params = {'interface': 'admin', 'ham': 'eggs'}
self.assertEqual('spam', keystone.get_service_url(session, **params))
session.get_endpoint.assert_called_once_with(**params)
def test_get_service_url_internal(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)
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)])

8
ironic/tests/unit/drivers/modules/test_inspector.py
View File

@ -89,8 +89,10 @@ class CommonFunctionsTestCase(BaseTestCase):
self.assertTrue(warn_mock.called)
@mock.patch('ironic.common.keystone.get_auth',
lambda _section, **kwargs: mock.sentinel.auth)
@mock.patch('ironic.common.keystone.get_session',
lambda _n: mock.sentinel.session)
lambda _section, **kwargs: mock.sentinel.session)
@mock.patch.object(eventlet, 'spawn_n', lambda f, *a, **kw: f(*a, **kw))
@mock.patch.object(client.ClientV1, 'introspect')
@mock.patch.object(client.ClientV1, '__init__', return_value=None)
@ -134,8 +136,10 @@ class InspectHardwareTestCase(BaseTestCase):
mock_introspect.assert_called_once_with(self.node.uuid)
@mock.patch('ironic.common.keystone.get_auth',
lambda _section, **kwargs: mock.sentinel.auth)
@mock.patch('ironic.common.keystone.get_session',
lambda _n: mock.sentinel.session)
lambda _section, **kwargs: mock.sentinel.session)
@mock.patch.object(client.ClientV1, 'get_status')
@mock.patch.object(client.ClientV1, '__init__', return_value=None)
class CheckStatusTestCase(BaseTestCase):