diff --git a/devstack/lib/ironic b/devstack/lib/ironic index 7c3072a0e1..be02c56bc2 100644 --- a/devstack/lib/ironic +++ b/devstack/lib/ironic @@ -1210,9 +1210,6 @@ function configure_ironic_conductor { iniset $IRONIC_CONF_FILE glance swift_account AUTH_${tenant_id} iniset $IRONIC_CONF_FILE glance swift_container glance iniset $IRONIC_CONF_FILE glance swift_temp_url_duration 3600 - iniset $IRONIC_CONF_FILE glance glance_protocol $GLANCE_SERVICE_PROTOCOL - iniset $IRONIC_CONF_FILE glance glance_host $SERVICE_HOST - iniset $IRONIC_CONF_FILE glance glance_port $GLANCE_SERVICE_PORT fi if is_deployed_by_agent; then diff --git a/etc/ironic/ironic.conf.sample b/etc/ironic/ironic.conf.sample index 6109895ca4..f692758514 100644 --- a/etc/ironic/ironic.conf.sample +++ b/etc/ironic/ironic.conf.sample @@ -1540,7 +1540,9 @@ # A list of the glance api servers available to ironic. Prefix # with https:// for SSL-based glance API servers. Format is -# [hostname|IP]:port. (list value) +# [hostname|IP]:port. If neither this option nor glance_host +# is set, the service catalog is used. It is recommended to +# rely on the service catalog, if possible. (list value) #glance_api_servers = # DEPRECATED: Glance API version (1 or 2) to use. (integer @@ -1558,21 +1560,31 @@ # when glance_api_insecure is set to False. (string value) #glance_cafile = -# Default glance hostname or IP address. (string value) -#glance_host = $my_ip +# DEPRECATED: Default glance hostname or IP address. The +# service catalog is used when not defined. Deprecated, use +# glance_api_servers instead. (string value) +# This option is deprecated for removal. +# Its value may be silently ignored in the future. +#glance_host = # Number of retries when downloading an image from glance. # (integer value) #glance_num_retries = 0 -# Default glance port. (port value) +# DEPRECATED: Default glance port. Deprecated, use +# glance_api_servers instead. (port value) # Minimum value: 0 # Maximum value: 65535 +# This option is deprecated for removal. +# Its value may be silently ignored in the future. #glance_port = 9292 -# Default protocol to use when connecting to glance. Set to -# https for SSL. (string value) +# DEPRECATED: Default protocol to use when connecting to +# glance. Set to https for SSL. Deprecated, use +# glance_api_services instead. (string value) # Allowed values: http, https +# This option is deprecated for removal. +# Its value may be silently ignored in the future. #glance_protocol = http # Verify HTTPS connections. (boolean value) diff --git a/ironic/common/exception.py b/ironic/common/exception.py index a64e490004..283a7207be 100644 --- a/ironic/common/exception.py +++ b/ironic/common/exception.py @@ -495,7 +495,7 @@ class UnsupportedDriverExtension(Invalid): class GlanceConnectionFailed(IronicException): - _msg_fmt = _("Connection to glance host %(host)s:%(port)s failed: " + _msg_fmt = _("Connection to glance host %(endpoint)s failed: " "%(reason)s") diff --git a/ironic/common/glance_service/base_image_service.py b/ironic/common/glance_service/base_image_service.py index 72cbc47bef..7cd47eafb7 100644 --- a/ironic/common/glance_service/base_image_service.py +++ b/ironic/common/glance_service/base_image_service.py @@ -59,13 +59,8 @@ def check_image_service(func): return func(self, *args, **kwargs) image_href = kwargs.get('image_href') - (image_id, self.glance_host, - self.glance_port, use_ssl) = service_utils.parse_image_ref(image_href) + _id, self.endpoint, use_ssl = service_utils.parse_image_ref(image_href) - if use_ssl: - scheme = 'https' - else: - scheme = 'http' params = {} params['insecure'] = CONF.glance.glance_api_insecure if (not params['insecure'] and CONF.glance.glance_cafile @@ -73,9 +68,8 @@ def check_image_service(func): params['cacert'] = CONF.glance.glance_cafile if CONF.glance.auth_strategy == 'keystone': params['token'] = self.context.auth_token - endpoint = '%s://%s:%s' % (scheme, self.glance_host, self.glance_port) self.client = client.Client(self.version, - endpoint, **params) + self.endpoint, **params) return func(self, *args, **kwargs) return wrapper @@ -86,6 +80,7 @@ class BaseImageService(object): self.client = client self.version = version self.context = context + self.endpoint = None def call(self, method, *args, **kwargs): """Call a glance client method. @@ -115,20 +110,16 @@ class BaseImageService(object): try: return getattr(self.client.images, method)(*args, **kwargs) except retry_excs as e: - host = self.glance_host - port = self.glance_port error_msg = ("Error contacting glance server " - "'%(host)s:%(port)s' for '%(method)s', attempt" + "'%(endpoint)s' for '%(method)s', attempt" " %(attempt)s of %(num_attempts)s failed.") - LOG.exception(error_msg, {'host': host, - 'port': port, + LOG.exception(error_msg, {'endpoint': self.endpoint, 'num_attempts': num_attempts, 'attempt': attempt, 'method': method}) if attempt == num_attempts: - raise exception.GlanceConnectionFailed(host=host, - port=port, - reason=str(e)) + raise exception.GlanceConnectionFailed( + endpoint=self.endpoint, reason=str(e)) time.sleep(1) except image_excs as e: exc_type, exc_value, exc_trace = sys.exc_info() @@ -147,8 +138,7 @@ class BaseImageService(object): """ LOG.debug("Getting image metadata from glance. Image: %s", image_href) - (image_id, self.glance_host, - self.glance_port, use_ssl) = service_utils.parse_image_ref(image_href) + image_id = service_utils.parse_image_ref(image_href)[0] image = self.call(method, image_id) @@ -165,8 +155,7 @@ class BaseImageService(object): :param image_id: The opaque image identifier. :param data: (Optional) File object to write data to. """ - (image_id, self.glance_host, - self.glance_port, use_ssl) = service_utils.parse_image_ref(image_href) + image_id = service_utils.parse_image_ref(image_href)[0] if (self.version == 2 and 'file' in CONF.glance.allowed_direct_url_schemes): diff --git a/ironic/common/glance_service/service_utils.py b/ironic/common/glance_service/service_utils.py index 56bfdcf95f..1320177f3b 100644 --- a/ironic/common/glance_service/service_utils.py +++ b/ironic/common/glance_service/service_utils.py @@ -27,6 +27,7 @@ import six.moves.urllib.parse as urlparse from ironic.common import exception from ironic.common import image_service +from ironic.common import keystone CONF = cfg.CONF @@ -34,18 +35,6 @@ _GLANCE_API_SERVER = None """ iterator that cycles (indefinitely) over glance API servers. """ -def generate_glance_url(): - """Generate the URL to glance.""" - return "%s://%s:%d" % (CONF.glance.glance_protocol, - CONF.glance.glance_host, - CONF.glance.glance_port) - - -def generate_image_url(image_ref): - """Generate an image URL from an image_ref.""" - return "%s/images/%s" % (generate_glance_url(), image_ref) - - def _extract_attributes(image): IMAGE_ATTRIBUTES = ['size', 'disk_format', 'owner', 'container_format', 'checksum', 'id', @@ -110,24 +99,28 @@ def _get_api_server_iterator(): If CONF.glance.glance_api_servers isn't set, we fall back to using this as the server: CONF.glance.glance_host:CONF.glance.glance_port. + If CONF.glance.glance_host is also not set, fetch the endpoint from the + service catalog. :returns: iterator that cycles (indefinitely) over shuffled glance API - servers. The iterator returns tuples of (host, port, use_ssl). + servers. """ api_servers = [] - configured_servers = (CONF.glance.glance_api_servers or - ['%s:%s' % (CONF.glance.glance_host, - CONF.glance.glance_port)]) - for api_server in configured_servers: - if '//' not in api_server: - api_server = '%s://%s' % (CONF.glance.glance_protocol, api_server) - url = urlparse.urlparse(api_server) - port = url.port or 80 - host = url.netloc.split(':', 1)[0] - use_ssl = (url.scheme == 'https') - api_servers.append((host, port, use_ssl)) - random.shuffle(api_servers) + if not CONF.glance.glance_api_servers and not CONF.glance.glance_host: + session = keystone.get_session('service_catalog') + api_servers = [keystone.get_service_url(session, service_type='image', + endpoint_type='public')] + else: + configured_servers = (CONF.glance.glance_api_servers or + ['%s:%s' % (CONF.glance.glance_host, + CONF.glance.glance_port)]) + for api_server in configured_servers: + if '//' not in api_server: + api_server = '%s://%s' % (CONF.glance.glance_protocol, + api_server) + api_servers.append(api_server) + random.shuffle(api_servers) return itertools.cycle(api_servers) @@ -145,29 +138,31 @@ def _get_api_server(): def parse_image_ref(image_href): - """Parse an image href into composite parts. + """Parse an image href. :param image_href: href of an image - :returns: a tuple of the form (image_id, host, port, use_ssl) + :returns: a tuple (image ID, glance URL, whether to use SSL) :raises ValueError: when input image href is invalid """ if '/' not in six.text_type(image_href): - image_id = image_href - (glance_host, glance_port, use_ssl) = _get_api_server() - return (image_id, glance_host, glance_port, use_ssl) + endpoint = _get_api_server() + return (image_href, endpoint, endpoint.startswith('https')) else: try: url = urlparse.urlparse(image_href) if url.scheme == 'glance': - (glance_host, glance_port, use_ssl) = _get_api_server() + endpoint = _get_api_server() image_id = image_href.split('/')[-1] + return (image_id, endpoint, endpoint.startswith('https')) else: glance_port = url.port or 80 glance_host = url.netloc.split(':', 1)[0] image_id = url.path.split('/')[-1] use_ssl = (url.scheme == 'https') - return (image_id, glance_host, glance_port, use_ssl) + endpoint = '%s://%s:%s' % (url.scheme, glance_host, + glance_port) + return (image_id, endpoint, use_ssl) except ValueError: raise exception.InvalidImageRef(image_href=image_href) diff --git a/ironic/conf/glance.py b/ironic/conf/glance.py index b29500e321..0bbbfdbacc 100644 --- a/ironic/conf/glance.py +++ b/ironic/conf/glance.py @@ -104,20 +104,29 @@ opts = [ 'multiple containers to store images, and this value ' 'will determine how many containers are created.')), cfg.StrOpt('glance_host', - default='$my_ip', - help=_('Default glance hostname or IP address.')), + help=_('Default glance hostname or IP address. The service ' + 'catalog is used when not defined. Deprecated, ' + 'use glance_api_servers instead.'), + deprecated_for_removal=True), cfg.PortOpt('glance_port', default=9292, - help=_('Default glance port.')), + help=_('Default glance port. Deprecated, use ' + 'glance_api_servers instead.'), + deprecated_for_removal=True), cfg.StrOpt('glance_protocol', default='http', choices=['http', 'https'], help=_('Default protocol to use when connecting to glance. ' - 'Set to https for SSL.')), + 'Set to https for SSL. Deprecated, use ' + 'glance_api_services instead.'), + deprecated_for_removal=True), cfg.ListOpt('glance_api_servers', 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.')), + 'servers. Format is [hostname|IP]:port. If neither ' + 'this option nor glance_host is set, the service ' + 'catalog is used. It is recommended to rely on the ' + 'service catalog, if possible.')), cfg.BoolOpt('glance_api_insecure', default=False, help=_('Allow to perform insecure SSL (https) requests to ' diff --git a/ironic/tests/unit/common/test_glance_service.py b/ironic/tests/unit/common/test_glance_service.py index d65b2df798..cd6793fc7e 100644 --- a/ironic/tests/unit/common/test_glance_service.py +++ b/ironic/tests/unit/common/test_glance_service.py @@ -807,24 +807,6 @@ class TestSwiftTempUrlCache(base.TestCase): self.assertNotIn(fake_image['id'], self.glance_service._cache) -class TestGlanceUrl(base.TestCase): - - def test_generate_glance_http_url(self): - self.config(glance_host="127.0.0.1", group='glance') - generated_url = service_utils.generate_glance_url() - http_url = "http://%s:%d" % (CONF.glance.glance_host, - CONF.glance.glance_port) - self.assertEqual(http_url, generated_url) - - def test_generate_glance_https_url(self): - self.config(glance_protocol="https", group='glance') - self.config(glance_host="127.0.0.1", group='glance') - generated_url = service_utils.generate_glance_url() - https_url = "https://%s:%d" % (CONF.glance.glance_host, - CONF.glance.glance_port) - self.assertEqual(https_url, generated_url) - - class TestServiceUtils(base.TestCase): def test_parse_image_ref_no_ssl(self): @@ -832,24 +814,14 @@ class TestServiceUtils(base.TestCase): u'image_\u00F9\u00FA\u00EE\u0111' parsed_href = service_utils.parse_image_ref(image_href) self.assertEqual((u'image_\u00F9\u00FA\u00EE\u0111', - '127.0.0.1', 9292, False), parsed_href) + 'http://127.0.0.1:9292', False), parsed_href) def test_parse_image_ref_ssl(self): image_href = 'https://127.0.0.1:9292/image_path/'\ u'image_\u00F9\u00FA\u00EE\u0111' parsed_href = service_utils.parse_image_ref(image_href) self.assertEqual((u'image_\u00F9\u00FA\u00EE\u0111', - '127.0.0.1', 9292, True), parsed_href) - - def test_generate_image_url(self): - image_href = u'image_\u00F9\u00FA\u00EE\u0111' - self.config(glance_host='123.123.123.123', group='glance') - self.config(glance_port=1234, group='glance') - self.config(glance_protocol='https', group='glance') - generated_url = service_utils.generate_image_url(image_href) - self.assertEqual('https://123.123.123.123:1234/images/' - u'image_\u00F9\u00FA\u00EE\u0111', - generated_url) + 'https://127.0.0.1:9292', True), parsed_href) def test_is_glance_image(self): image_href = u'uui\u0111' @@ -884,18 +856,34 @@ class TestGlanceAPIServers(base.TestCase): super(TestGlanceAPIServers, self).setUp() service_utils._GLANCE_API_SERVER = None - def test__get_api_servers_default(self): - host, port, use_ssl = service_utils._get_api_server() - self.assertEqual(CONF.glance.glance_host, host) - self.assertEqual(CONF.glance.glance_port, port) - self.assertEqual(CONF.glance.glance_protocol == 'https', use_ssl) + @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_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_service_url.assert_called_once_with(mock_session.return_value, + service_type='image', + endpoint_type='public') + + def test__get_api_servers_with_host_port(self): + CONF.set_override('glance_host', 'example.com', 'glance') + CONF.set_override('glance_port', 42, 'glance') + CONF.set_override('glance_protocol', 'https', 'glance') + endpoint = service_utils._get_api_server() + self.assertEqual('https://example.com:42', endpoint) def test__get_api_servers_one(self): CONF.set_override('glance_api_servers', ['https://10.0.0.1:9293'], 'glance') s1 = service_utils._get_api_server() s2 = service_utils._get_api_server() - self.assertEqual(('10.0.0.1', 9293, True), s1) + self.assertEqual('https://10.0.0.1:9293', s1) # Only one server, should always get the same one self.assertEqual(s1, s2) diff --git a/ironic/tests/unit/drivers/modules/test_image_cache.py b/ironic/tests/unit/drivers/modules/test_image_cache.py index b50d3fe40d..eb12cfbd32 100644 --- a/ironic/tests/unit/drivers/modules/test_image_cache.py +++ b/ironic/tests/unit/drivers/modules/test_image_cache.py @@ -38,6 +38,8 @@ def touch(filename): open(filename, 'w').close() +@mock.patch('ironic.common.glance_service.service_utils.parse_image_ref', + lambda image: (image, 'example.com', True)) class TestImageCacheFetch(base.TestCase): def setUp(self): diff --git a/releasenotes/notes/glance-keystone-dd30b884f07f83fb.yaml b/releasenotes/notes/glance-keystone-dd30b884f07f83fb.yaml new file mode 100644 index 0000000000..f9db5fa69f --- /dev/null +++ b/releasenotes/notes/glance-keystone-dd30b884f07f83fb.yaml @@ -0,0 +1,11 @@ +--- +upgrade: + - | + The configuration option ``[glance]glance_host`` is now empty by default. + If neither it nor ``[glance]glance_api_servers`` are provided, ironic will + now try fetching the Image service endpoint from the service catalog. +deprecations: + - | + The configuration options ``[glance]glance_host``, ``[glance]glance_port`` + and ``[glance]glance_protocol`` are deprecated in favor of either using + ``[glance]glance_api_servers`` or using the service catalog.