From 77e7a9042dec9fb6f60f01ad7a6c4e30daf59856 Mon Sep 17 00:00:00 2001 From: Ruby Loo Date: Tue, 21 Oct 2014 17:27:08 +0000 Subject: [PATCH] Iterate over glance API servers The configuration option CONF.glance.glance_api_servers is meant to be a list of Glance API servers, and should behave similar to its use in Nova and Cinder: that each time a Glance API server is wanted, one of the servers in the list is returned. This changes CONF.glance.glance_api_servers to be a list instead of a string. The list is randomly shuffled, and an iterator is used to cycle through the list of servers. This is backwards compatible and handles the case where the user may have set [glance]/glance_api_servers = 'host:port' (or 'host1:port1, host2:port2'). The configuration library will convert that to the corresponding list. Closes-Bug: #1381135 Change-Id: Ic7cab502f38407aa0c282203804023dfa45b6623 --- etc/ironic/ironic.conf.sample | 2 +- ironic/common/glance_service/service_utils.py | 57 +++++++++++++++---- ironic/common/image_service.py | 8 +-- ironic/tests/test_glance_service.py | 36 ++++++++++++ 4 files changed, 87 insertions(+), 16 deletions(-) diff --git a/etc/ironic/ironic.conf.sample b/etc/ironic/ironic.conf.sample index 9aaae0e67e..a176dd8a2f 100644 --- a/etc/ironic/ironic.conf.sample +++ b/etc/ironic/ironic.conf.sample @@ -828,7 +828,7 @@ # A list of the glance api servers available to ironic. Prefix # with https:// for SSL-based glance API servers. Format is -# [hostname|IP]:port. (string value) +# [hostname|IP]:port. (list value) #glance_api_servers= # Allow to perform insecure SSL (https) requests to glance. diff --git a/ironic/common/glance_service/service_utils.py b/ironic/common/glance_service/service_utils.py index da2201bf57..baa8e88407 100644 --- a/ironic/common/glance_service/service_utils.py +++ b/ironic/common/glance_service/service_utils.py @@ -15,7 +15,9 @@ # under the License. import copy +import itertools import logging +import random from oslo.config import cfg from oslo.serialization import jsonutils @@ -29,6 +31,9 @@ from ironic.common import exception CONF = cfg.CONF LOG = logging.getLogger(__name__) +_GLANCE_API_SERVER = None +""" iterator that cycles (indefinitely) over glance API servers. """ + def generate_glance_url(): """Generate the URL to glance.""" @@ -105,24 +110,54 @@ def _remove_read_only(image_meta): return output +def _get_api_server_iterator(): + """Return iterator over shuffled API servers. + + Shuffle a list of CONF.glance.glance_api_servers and return an iterator + that will cycle through the list, looping around to the beginning if + necessary. + + 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. + + :returns: iterator that cycles (indefinitely) over shuffled glance API + servers. The iterator returns tuples of (host, port, use_ssl). + """ + 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) + return itertools.cycle(api_servers) + + def _get_api_server(): - """Get the Glance API server information.""" - api_server = (CONF.glance.glance_api_servers or - CONF.glance.glance_host + ':' + str(CONF.glance.glance_port)) - if '//' not in api_server: - api_server = 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') - return host, port, use_ssl + """Return a Glance API server. + + :returns: for an API server, the tuple (host-or-IP, port, use_ssl), where + use_ssl is True to use the 'https' scheme, and False to use 'http'. + """ + global _GLANCE_API_SERVER + + if not _GLANCE_API_SERVER: + _GLANCE_API_SERVER = _get_api_server_iterator() + return _GLANCE_API_SERVER.next() 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, host, port, use_ssl) :raises ValueError """ diff --git a/ironic/common/image_service.py b/ironic/common/image_service.py index d20b50dda3..8d4f6de750 100644 --- a/ironic/common/image_service.py +++ b/ironic/common/image_service.py @@ -30,10 +30,10 @@ glance_opts = [ default='http', help='Default protocol to use when connecting to glance. ' 'Set to https for SSL.'), - cfg.StrOpt('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.'), + 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.'), cfg.BoolOpt('glance_api_insecure', default=False, help='Allow to perform insecure SSL (https) requests to ' diff --git a/ironic/tests/test_glance_service.py b/ironic/tests/test_glance_service.py index 1d82f5798c..c9bd466dc1 100644 --- a/ironic/tests/test_glance_service.py +++ b/ironic/tests/test_glance_service.py @@ -752,3 +752,39 @@ class TestServiceUtils(base.TestCase): generated_url = service_utils.generate_image_url(image_href) self.assertEqual('https://123.123.123.123:1234/images/image_uuid', generated_url) + + +class TestGlanceAPIServers(base.TestCase): + + def setUp(self): + 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) + + 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) + + # Only one server, should always get the same one + self.assertEqual(s1, s2) + + def test__get_api_servers_two(self): + CONF.set_override('glance_api_servers', + ['http://10.0.0.1:9293', 'http://10.0.0.2:9294'], + 'glance') + s1 = service_utils._get_api_server() + s2 = service_utils._get_api_server() + s3 = service_utils._get_api_server() + + self.assertNotEqual(s1, s2) + + # 2 servers, so cycles to the first again + self.assertEqual(s1, s3)