diff --git a/ironic/common/glance_service/base_image_service.py b/ironic/common/glance_service/base_image_service.py index 6d62704607..7738edbbc6 100644 --- a/ironic/common/glance_service/base_image_service.py +++ b/ironic/common/glance_service/base_image_service.py @@ -38,13 +38,6 @@ LOG = log.getLogger(__name__) _GLANCE_SESSION = None -def _get_glance_session(**session_kwargs): - global _GLANCE_SESSION - if not _GLANCE_SESSION: - _GLANCE_SESSION = keystone.get_session('glance', **session_kwargs) - return _GLANCE_SESSION - - def _translate_image_exception(image_id, exc_value): if isinstance(exc_value, (glance_exc.Forbidden, glance_exc.Unauthorized)): @@ -56,10 +49,6 @@ def _translate_image_exception(image_id, exc_value): return exc_value -# NOTE(pas-ha) while looking very ugly currently, this will be simplified -# in Rocky after all deprecated [glance] options are removed and -# keystone catalog is always used with 'keystone' auth strategy -# together with session always loaded from config options def check_image_service(func): """Creates a glance client if doesn't exists and calls the function.""" @six.wraps(func) @@ -72,35 +61,16 @@ def check_image_service(func): if self.client: return func(self, *args, **kwargs) - # TODO(pas-ha) remove in Rocky - session_params = {} - if CONF.glance.glance_api_insecure and not CONF.glance.insecure: - session_params['insecure'] = CONF.glance.glance_api_insecure - if CONF.glance.glance_cafile and not CONF.glance.cafile: - session_params['cacert'] = CONF.glance.glance_cafile + global _GLANCE_SESSION + if not _GLANCE_SESSION: + _GLANCE_SESSION = keystone.get_session('glance') + # NOTE(pas-ha) glanceclient uses Adapter-based SessionClient, # so we can pass session and auth separately, makes things easier - session = _get_glance_session(**session_params) - - # TODO(pas-ha) remove in Rocky - # NOTE(pas-ha) new option must win if configured - if (CONF.glance.glance_api_servers - and not CONF.glance.endpoint_override): - # NOTE(pas-ha) all the 2 methods have image_href as the first - # positional arg, but check in kwargs too - image_href = args[0] if args else kwargs.get('image_href') - url = service_utils.get_glance_api_server(image_href) - CONF.set_override('endpoint_override', url, group='glance') - - # TODO(pas-ha) remove in Rocky - if CONF.glance.auth_strategy == 'noauth': - CONF.set_override('auth_type', 'none', group='glance') - service_auth = keystone.get_auth('glance') - adapter_params = {} - adapter = keystone.get_adapter('glance', session=session, - auth=service_auth, **adapter_params) + adapter = keystone.get_adapter('glance', session=_GLANCE_SESSION, + auth=service_auth) self.endpoint = adapter.get_endpoint() user_auth = None @@ -110,7 +80,7 @@ def check_image_service(func): if self.context.auth_token: user_auth = keystone.get_service_auth(self.context, self.endpoint, service_auth) - self.client = client.Client(2, session=session, + self.client = client.Client(2, session=_GLANCE_SESSION, auth=user_auth or service_auth, endpoint_override=self.endpoint, global_request_id=self.context.global_id) @@ -130,7 +100,7 @@ class BaseImageService(object): """Call a glance client method. If we get a connection error, - retry the request according to CONF.glance_num_retries. + retry the request according to CONF.num_retries. :param context: The request context, for access checks. :param method: The method requested to be called. @@ -146,7 +116,7 @@ class BaseImageService(object): glance_exc.Unauthorized, glance_exc.NotFound, glance_exc.BadRequest) - num_attempts = 1 + CONF.glance.glance_num_retries + num_attempts = 1 + CONF.glance.num_retries # TODO(pas-ha) use retrying lib here for attempt in range(1, num_attempts + 1): diff --git a/ironic/common/glance_service/service_utils.py b/ironic/common/glance_service/service_utils.py index f58cdd8731..601895e015 100644 --- a/ironic/common/glance_service/service_utils.py +++ b/ironic/common/glance_service/service_utils.py @@ -15,23 +15,13 @@ # under the License. import copy -import itertools -import random -from oslo_log import log from oslo_serialization import jsonutils from oslo_utils import timeutils from oslo_utils import uuidutils import six from ironic.common import exception -from ironic.conf import CONF - - -LOG = log.getLogger(__name__) - -_GLANCE_API_SERVER = None -""" iterator that cycles (indefinitely) over glance API servers. """ _IMAGE_ATTRIBUTES = ['size', 'disk_format', 'owner', @@ -105,29 +95,6 @@ def parse_image_id(image_href): return image_id -# TODO(pas-ha) remove in Rocky -def get_glance_api_server(image_href): - """Construct a glance API url from config options - - Returns a random server from the CONF.glance.glance_api_servers list - of servers. - - :param image_href: href of an image - :returns: glance API URL - - :raises InvalidImageRef: when input image href is invalid - """ - image_href = six.text_type(image_href) - if not is_glance_image(image_href): - raise exception.InvalidImageRef(image_href=image_href) - global _GLANCE_API_SERVER - if not _GLANCE_API_SERVER: - _GLANCE_API_SERVER = itertools.cycle( - random.sample(CONF.glance.glance_api_servers, - len(CONF.glance.glance_api_servers))) - return six.next(_GLANCE_API_SERVER) - - def translate_from_glance(image): image_meta = _extract_attributes(image) image_meta = _convert_timestamps_to_datetimes(image_meta) diff --git a/ironic/conf/glance.py b/ironic/conf/glance.py index af6c833b50..a96f38a63c 100644 --- a/ironic/conf/glance.py +++ b/ironic/conf/glance.py @@ -107,42 +107,12 @@ opts = [ 'value between 1 and 32, a single-tenant store will use ' 'multiple containers to store images, and this value ' 'will determine how many containers are created.')), - cfg.ListOpt('glance_api_servers', - deprecated_for_removal=True, - deprecated_reason=_("Use [glance]/endpoint_override option " - "to set the full load-balanced glance API " - "URL instead."), - help=_('A list of the glance api servers available to ironic. ' - 'Prefix with https:// for SSL-based glance API ' - 'servers. Format is [hostname|IP]:port.')), - cfg.BoolOpt('glance_api_insecure', - default=False, - deprecated_for_removal=True, - deprecated_reason=_("Use [glance]/insecure option instead."), - help=_('Allow to perform insecure SSL (https) requests to ' - 'glance.')), - cfg.IntOpt('glance_num_retries', + cfg.IntOpt('num_retries', + # TODO(dtantsur): remove in U + deprecated_name='glance_num_retries', default=0, help=_('Number of retries when downloading an image from ' 'glance.')), - cfg.StrOpt('auth_strategy', - default='keystone', - choices=[('keystone', _('use the Identity service for ' - 'authentication')), - ('noauth', _('no authentication'))], - deprecated_for_removal=True, - deprecated_reason=_("To configure glance in noauth mode, " - "set [glance]/auth_type=none and " - "[glance]/endpoint_override=" - " instead."), - help=_('Authentication strategy to use when connecting to ' - 'glance.')), - cfg.StrOpt('glance_cafile', - deprecated_for_removal=True, - deprecated_reason=_("Use [glance]/cafile option instead."), - help=_('Optional path to a CA certificate bundle to be used to ' - 'validate the SSL certificate served by glance. It is ' - 'used when glance_api_insecure is set to False.')), ] diff --git a/ironic/tests/unit/common/test_glance_service.py b/ironic/tests/unit/common/test_glance_service.py index 283f763c73..86ffd32c9f 100644 --- a/ironic/tests/unit/common/test_glance_service.py +++ b/ironic/tests/unit/common/test_glance_service.py @@ -95,9 +95,6 @@ class TestGlanceImageService(base.TestCase): self.context.project_id = 'fake' self.service = service.GlanceImageService(self.client, self.context) - self.config(glance_api_servers=['http://localhost'], group='glance') - self.config(auth_strategy='keystone', group='glance') - @staticmethod def _make_fixture(**kwargs): fixture = {'name': None, @@ -201,13 +198,13 @@ class TestGlanceImageService(base.TestCase): writer = NullWriter() # When retries are disabled, we should get an exception - self.config(glance_num_retries=0, group='glance') + self.config(num_retries=0, group='glance') self.assertRaises(exception.GlanceConnectionFailed, stub_service.download, image_id, writer) # Now lets enable retries. No exception should happen now. tries = [0] - self.config(glance_num_retries=1, group='glance') + self.config(num_retries=1, group='glance') stub_service.download(image_id, writer) self.assertTrue(mock_sleep.called) @@ -358,7 +355,6 @@ class CheckImageServiceTestCase(base.TestCase): service_type='image', region_name='SomeRegion', interface='internal', - auth_strategy='keystone', group='glance') base_image_service._GLANCE_SESSION = None @@ -432,42 +428,13 @@ class CheckImageServiceTestCase(base.TestCase): mock.sentinel.auth) mock_auth.assert_called_once_with('glance') - def test_check_image_service__deprecated_opts(self, mock_gclient, - mock_sess, mock_adapter, - mock_sauth, mock_auth): - def func(service, *args, **kwargs): - return args, kwargs - - mock_adapter.return_value = adapter = mock.Mock() - adapter.get_endpoint.return_value = 'glance_url' - uuid = uuidutils.generate_uuid() - params = {'image_href': uuid} - self.config(glance_api_servers='https://localhost:1234', - glance_api_insecure=True, - glance_cafile='cafile', - region_name=None, - group='glance') - - wrapped_func = base_image_service.check_image_service(func) - self.assertEqual(((), params), wrapped_func(self.service, **params)) - self.assertEqual('https://localhost:1234', - base_image_service.CONF.glance.endpoint_override) - self._assert_client_call(mock_gclient, 'glance_url') - mock_sess.assert_called_once_with('glance', insecure=True, - cacert='cafile') - mock_adapter.assert_called_once_with( - 'glance', session=mock.sentinel.session, - auth=mock.sentinel.auth) - self.assertEqual(0, mock_sauth.call_count) - mock_auth.assert_called_once_with('glance') - def test_check_image_service__no_auth(self, mock_gclient, mock_sess, mock_adapter, mock_sauth, mock_auth): def func(service, *args, **kwargs): return args, kwargs self.config(endpoint_override='foo', - auth_strategy='noauth', + auth_type='none', group='glance') mock_adapter.return_value = adapter = mock.Mock() adapter.get_endpoint.return_value = 'foo' @@ -978,20 +945,6 @@ class TestServiceUtils(base.TestCase): service_utils.parse_image_id, u'http://spam.ham/eggs') - def test_get_glance_api_server_fail(self): - self.assertRaises(exception.InvalidImageRef, - service_utils.get_glance_api_server, - u'http://spam.ham/eggs') - - # TODO(pas-ha) remove in Rocky - def test_get_glance_api_server(self): - self.config(glance_api_servers='http://spam:1234, https://ham', - group='glance') - api_servers = {service_utils.get_glance_api_server( - uuidutils.generate_uuid()) for i in range(2)} - self.assertEqual({'http://spam:1234', 'https://ham'}, - api_servers) - def test_is_glance_image(self): image_href = u'uui\u0111' self.assertFalse(service_utils.is_glance_image(image_href)) diff --git a/ironic/tests/unit/common/test_image_service.py b/ironic/tests/unit/common/test_image_service.py index 2f15da595b..97c374c042 100644 --- a/ironic/tests/unit/common/test_image_service.py +++ b/ironic/tests/unit/common/test_image_service.py @@ -15,7 +15,6 @@ import os import shutil import mock -from oslo_config import cfg from oslo_utils import uuidutils import requests import sendfile @@ -341,7 +340,3 @@ class ServiceGetterTestCase(base.TestCase): for image_ref in invalid_refs: self.assertRaises(exception.ImageRefValidationFailed, image_service.get_image_service, image_ref) - - def test_out_range_auth_strategy(self): - self.assertRaises(ValueError, cfg.CONF.set_override, - 'auth_strategy', 'fake', 'glance') diff --git a/releasenotes/notes/glance-deprecations-21e7014b72a1bcef.yaml b/releasenotes/notes/glance-deprecations-21e7014b72a1bcef.yaml new file mode 100644 index 0000000000..c6b3a7e9e3 --- /dev/null +++ b/releasenotes/notes/glance-deprecations-21e7014b72a1bcef.yaml @@ -0,0 +1,11 @@ +--- +upgrade: + - | + The deprecated options ``glance_api_servers``, ``glance_api_insecure``, + ``glance_cafile`` and ``auth_strategy`` from the ``[glance]`` section have + been remove. Please use the corresponding keystoneauth options instead. +deprecations: + - | + The configuration option ``[glance]glance_num_retries`` has been renamed + to ``[glance]num_retries``. The old name will be removed in a future + release.