From c56fc55170d16e31f2f304e526df975ced857ba6 Mon Sep 17 00:00:00 2001
From: Eric Fried <efried@us.ibm.com>
Date: Tue, 19 Sep 2017 11:03:27 -0500
Subject: [PATCH] Don't fix protocol-less glance api_servers anymore

Operators omitting the protocol (http(s)) from their [glance]api_servers
value(s) have had long enough to fix it.  Use it as is; and if the
protocol is omitted, let it fail hard in the request.

Change-Id: Ifebc9192e1e754180c97a3e40806c1c496a8b715
---
 nova/image/glance.py                            | 15 +++------------
 nova/tests/unit/image/test_glance.py            | 17 ++++++-----------
 .../tests/unit/virt/xenapi/image/test_glance.py |  2 +-
 ...i-servers-must-be-urls-558298647cbfc81c.yaml |  6 ++++++
 4 files changed, 16 insertions(+), 24 deletions(-)
 create mode 100644 releasenotes/notes/glance-api-servers-must-be-urls-558298647cbfc81c.yaml

diff --git a/nova/image/glance.py b/nova/image/glance.py
index eb4e96772da4..388083b00214 100644
--- a/nova/image/glance.py
+++ b/nova/image/glance.py
@@ -113,17 +113,8 @@ def get_api_servers():
     # NOTE(efried): utils.get_ksa_adapter().get_endpoint() is the preferred
     # mechanism for endpoint discovery. Only use `api_servers` if you really
     # need to shuffle multiple endpoints.
-    api_servers = []
     if CONF.glance.api_servers:
-        for api_server in CONF.glance.api_servers:
-            if '//' not in api_server:
-                api_server = 'http://' + api_server
-                # NOTE(sdague): remove in O.
-                LOG.warning("No protocol specified in for api_server '%s', "
-                            "please update [glance] api_servers with fully "
-                            "qualified url including scheme (http / https)",
-                            api_server)
-            api_servers.append(api_server)
+        api_servers = CONF.glance.api_servers
         random.shuffle(api_servers)
     else:
         # TODO(efried): Plumb in a reasonable auth from callers' contexts
@@ -131,8 +122,8 @@ def get_api_servers():
             nova.conf.glance.DEFAULT_SERVICE_TYPE,
             min_version='2.0', max_version='2.latest')
         # TODO(efried): Use ksa_adap.get_endpoint() when bug #1707995 is fixed.
-        api_servers.append(ksa_adap.endpoint_override or
-                           ksa_adap.get_endpoint_data().catalog_url)
+        api_servers = [ksa_adap.endpoint_override or
+                       ksa_adap.get_endpoint_data().catalog_url]
 
     return itertools.cycle(api_servers)
 
diff --git a/nova/tests/unit/image/test_glance.py b/nova/tests/unit/image/test_glance.py
index 347314a7028d..cf86935835d2 100644
--- a/nova/tests/unit/image/test_glance.py
+++ b/nova/tests/unit/image/test_glance.py
@@ -395,7 +395,7 @@ class TestGlanceClientWrapperRetries(test.NoDBTestCase):
         super(TestGlanceClientWrapperRetries, self).setUp()
         self.ctx = context.RequestContext('fake', 'fake')
         api_servers = [
-            'host1:9292',
+            'http://host1:9292',
             'https://host2:9293',
             'http://host3:9294'
         ]
@@ -1575,20 +1575,15 @@ class TestGlanceApiServers(test.NoDBTestCase):
 
     def test_get_api_servers_multiple(self):
         """Test get_api_servers via `api_servers` conf option."""
-        glance_servers = ['10.0.1.1:9292',
-                          'https://10.0.0.1:9293',
-                          'http://10.0.2.2:9294']
-        expected_servers = ['http://10.0.1.1:9292',
+        glance_servers = ['http://10.0.1.1:9292',
                           'https://10.0.0.1:9293',
                           'http://10.0.2.2:9294']
+        expected_servers = set(glance_servers)
         self.flags(api_servers=glance_servers, group='glance')
         api_servers = glance.get_api_servers()
-        i = 0
-        for server in api_servers:
-            i += 1
-            self.assertIn(server, expected_servers)
-            if i > 2:
-                break
+        # In len(expected_servers) cycles, we should get all the endpoints
+        self.assertEqual(expected_servers,
+                         {next(api_servers) for _ in expected_servers})
 
     @mock.patch('keystoneauth1.adapter.Adapter.get_endpoint_data')
     def test_get_api_servers_get_ksa_adapter(self, mock_epd):
diff --git a/nova/tests/unit/virt/xenapi/image/test_glance.py b/nova/tests/unit/virt/xenapi/image/test_glance.py
index 816ff2b23570..49592689c0b6 100644
--- a/nova/tests/unit/virt/xenapi/image/test_glance.py
+++ b/nova/tests/unit/virt/xenapi/image/test_glance.py
@@ -106,7 +106,7 @@ class TestGlanceStore(stubs.XenAPITestBaseNoDB):
                            endpoint='http://10.0.0.1:9293',
                            **params)]
 
-        glance_api_servers = ['10.0.1.1:9292',
+        glance_api_servers = ['http://10.0.1.1:9292',
                               'http://10.0.0.1:9293']
         self.flags(api_servers=glance_api_servers, group='glance')
 
diff --git a/releasenotes/notes/glance-api-servers-must-be-urls-558298647cbfc81c.yaml b/releasenotes/notes/glance-api-servers-must-be-urls-558298647cbfc81c.yaml
new file mode 100644
index 000000000000..90da5f9950c2
--- /dev/null
+++ b/releasenotes/notes/glance-api-servers-must-be-urls-558298647cbfc81c.yaml
@@ -0,0 +1,6 @@
+---
+upgrade:
+  - |
+    If using the ``api_servers`` option in the ``[glance]`` configuration
+    section, the values therein must be URLs.  Raw IP addresses will result
+    in hard failure of API requests.