openstack
/
ironic
Code Issues Proposed changes

Fetch Glance endpoint from Keystone if it's not provided in the configuration 

This is needed to fix the CI broken by glance switching to running
under wsgi, and thus breaking our assumption that glance is accessible
by host:port only.

The options glance_host, glance_port and glance_protocol were
deprecated.  Standalone deployments should use glance_api_servers
instead.

Also removes two unused utility functions.

Change-Id: I54dc04ab084aeb7208c9dd9940c6434c029bf41c
Partial-Bug: #1699542
This commit is contained in:
Dmitry Tantsur 2017-06-22 14:33:32 +02:00
parent f0e6a07ade
commit cb7cdd3534
9 changed files with 106 additions and 103 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
devstack/lib/ironic
View File

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

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

@ -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 = <None>
# 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 = <None>
# 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 = <None>
# 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)

2
ironic/common/exception.py
View File

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

29
ironic/common/glance_service/base_image_service.py
View File

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

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

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

19
ironic/conf/glance.py
View File

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

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

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

2
ironic/tests/unit/drivers/modules/test_image_cache.py
View File

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

11
releasenotes/notes/glance-keystone-dd30b884f07f83fb.yaml Normal file
View File

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