From 86e0125f29d863ec739db6c5eca702e03a250137 Mon Sep 17 00:00:00 2001 From: Dirk Mueller Date: Sat, 1 Jun 2013 14:25:37 +0200 Subject: [PATCH] Handle IPv6 specifid glance servers gracefully IPv6 netlocs can for valid reasons contain ':', so splitting by ':' is dangerous. Instead of splitting by host and port in order to only reassemble it into a netloc again later, simply always pass netloc, which avoids the bug. Add extra textcases to verify the new behavior. Fix Flake8 warnings. Fixes LP Bug #1182830 Change-Id: I130528ae946e8888d35c25e468b4ea6ac29db0cf --- cinder/image/glance.py | 60 +++++++++++++++---------------- cinder/tests/image/test_glance.py | 22 +++++++----- 2 files changed, 41 insertions(+), 41 deletions(-) diff --git a/cinder/image/glance.py b/cinder/image/glance.py index bfec5288e..ca1dd0964 100644 --- a/cinder/image/glance.py +++ b/cinder/image/glance.py @@ -44,21 +44,20 @@ def _parse_image_ref(image_href): """Parse an image href into composite parts. :param image_href: href of an image - :returns: a tuple of the form (image_id, host, port) + :returns: a tuple of the form (image_id, netloc, use_ssl) :raises ValueError """ url = urlparse.urlparse(image_href) - port = url.port or 80 - host = url.netloc.split(':', 1)[0] + netloc = url.netloc image_id = url.path.split('/')[-1] use_ssl = (url.scheme == 'https') - return (image_id, host, port, use_ssl) + return (image_id, netloc, use_ssl) -def _create_glance_client(context, host, port, use_ssl, +def _create_glance_client(context, netloc, use_ssl, version=FLAGS.glance_api_version): - """Instantiate a new glanceclient.Client object""" + """Instantiate a new glanceclient.Client object.""" if version is None: version = FLAGS.glance_api_version if use_ssl: @@ -69,12 +68,13 @@ def _create_glance_client(context, host, port, use_ssl, params['insecure'] = FLAGS.glance_api_insecure if FLAGS.auth_strategy == 'keystone': params['token'] = context.auth_token - endpoint = '%s://%s:%s' % (scheme, host, port) + endpoint = '%s://%s' % (scheme, netloc) return glanceclient.Client(str(version), endpoint, **params) def get_api_servers(): - """ + """Return Iterable over shuffled api servers. + Shuffle a list of FLAGS.glance_api_servers and return an iterator that will cycle through the list, looping around to the beginning if necessary. @@ -84,10 +84,9 @@ def get_api_servers(): if '//' not in api_server: api_server = 'http://' + api_server url = urlparse.urlparse(api_server) - port = url.port or 80 - host = url.netloc.split(':', 1)[0] + netloc = url.netloc use_ssl = (url.scheme == 'https') - api_servers.append((host, port, use_ssl)) + api_servers.append((netloc, use_ssl)) random.shuffle(api_servers) return itertools.cycle(api_servers) @@ -95,39 +94,39 @@ def get_api_servers(): class GlanceClientWrapper(object): """Glance client wrapper class that implements retries.""" - def __init__(self, context=None, host=None, port=None, use_ssl=False, + def __init__(self, context=None, netloc=None, use_ssl=False, version=None): - if host is not None: + if netloc is not None: self.client = self._create_static_client(context, - host, port, + netloc, use_ssl, version) else: self.client = None self.api_servers = None self.version = version - def _create_static_client(self, context, host, port, use_ssl, version): + def _create_static_client(self, context, netloc, use_ssl, version): """Create a client that we'll use for every call.""" - self.host = host - self.port = port + self.netloc = netloc self.use_ssl = use_ssl self.version = version return _create_glance_client(context, - self.host, self.port, + self.netloc, self.use_ssl, self.version) def _create_onetime_client(self, context, version): """Create a client that will be used for one call.""" if self.api_servers is None: self.api_servers = get_api_servers() - self.host, self.port, self.use_ssl = self.api_servers.next() + self.netloc, self.use_ssl = self.api_servers.next() return _create_glance_client(context, - self.host, self.port, + self.netloc, self.use_ssl, version) def call(self, context, method, *args, **kwargs): - """ - Call a glance client method. If we get a connection error, + """Call a glance client method. + + If we get a connection error, retry the request according to FLAGS.glance_num_retries. """ version = self.version @@ -145,17 +144,15 @@ class GlanceClientWrapper(object): try: return getattr(client.images, method)(*args, **kwargs) except retry_excs as e: - host = self.host - port = self.port + netloc = self.netloc extra = "retrying" error_msg = _("Error contacting glance server " - "'%(host)s:%(port)s' for '%(method)s', " + "'%(netloc)s' for '%(method)s', " "%(extra)s.") if attempt == num_attempts: extra = 'done trying' LOG.exception(error_msg, locals()) - raise exception.GlanceConnectionFailed(host=host, - port=port, + raise exception.GlanceConnectionFailed(netloc=netloc, reason=str(e)) LOG.exception(error_msg, locals()) time.sleep(1) @@ -212,7 +209,8 @@ class GlanceImageService(object): def get_location(self, context, image_id): """Returns the direct url representing the backend storage location, - or None if this attribute is not shown by Glance.""" + or None if this attribute is not shown by Glance. + """ try: client = GlanceClientWrapper() image_meta = client.call(context, 'get', image_id) @@ -443,11 +441,9 @@ def get_remote_image_service(context, image_href): return image_service, image_href try: - (image_id, glance_host, glance_port, use_ssl) = \ - _parse_image_ref(image_href) + (image_id, glance_netloc, use_ssl) = _parse_image_ref(image_href) glance_client = GlanceClientWrapper(context=context, - host=glance_host, - port=glance_port, + netloc=glance_netloc, use_ssl=use_ssl) except ValueError: raise exception.InvalidImageRef(image_href=image_href) diff --git a/cinder/tests/image/test_glance.py b/cinder/tests/image/test_glance.py index 46bbafa9c..2a80e2dfe 100644 --- a/cinder/tests/image/test_glance.py +++ b/cinder/tests/image/test_glance.py @@ -17,8 +17,6 @@ import datetime -import random -import time import glanceclient.exc @@ -78,8 +76,7 @@ class TestGlanceSerializer(test.TestCase): class TestGlanceImageService(test.TestCase): - """ - Tests the Glance image service. + """Tests the Glance image service. At a high level, the translations involved are: @@ -110,7 +107,7 @@ class TestGlanceImageService(test.TestCase): self.stubs.Set(glance.time, 'sleep', lambda s: None) def _create_image_service(self, client): - def _fake_create_glance_client(context, host, port, use_ssl, version): + def _fake_create_glance_client(context, netloc, use_ssl, version): return client self.stubs.Set(glance, @@ -166,7 +163,8 @@ class TestGlanceImageService(test.TestCase): self.assertDictMatch(image_metas[0], expected) def test_create_without_instance_id(self): - """ + """Test Creating images without instance_id. + Ensure we can create an image without having to specify an instance_id. Public images are an example of an image not tied to an instance. @@ -538,12 +536,18 @@ class TestGlanceImageService(test.TestCase): (service, same_id) = glance.get_remote_image_service(self.context, image_url) self.assertEquals(same_id, image_id) - self.assertEquals(service._client.host, + self.assertEquals(service._client.netloc, 'something-less-likely') + for ipv6_url in ('[::1]', '::1', '[::1]:444'): + image_url = 'http://%s/%s' % (ipv6_url, image_id) + (service, same_id) = glance.get_remote_image_service(self.context, + image_url) + self.assertEquals(same_id, image_id) + self.assertEquals(service._client.netloc, ipv6_url) class TestGlanceClientVersion(test.TestCase): - """Tests the version of the glance client generated""" + """Tests the version of the glance client generated.""" def setUp(self): super(TestGlanceClientVersion, self).setUp() @@ -554,7 +558,7 @@ class TestGlanceClientVersion(test.TestCase): fake_get_image_model) def test_glance_version_by_flag(self): - """Test glance version set by flag is honoured""" + """Test glance version set by flag is honoured.""" client_wrapper_v1 = glance.GlanceClientWrapper('fake', 'fake_host', 9292) self.assertEquals(client_wrapper_v1.client.__module__, -- 2.45.2