Merge "Don't lookup service url when bypass_url is given"

novaclient/client.py

@@ -214,8 +214,6 @@ class HTTPClient(object):
                 # otherwise we will get all the requests logging messages
                 rql.setLevel(logging.WARNING)
 
-        # NOTE(melwitt): Service catalog is only set if bypass_url isn't
-        #                used. Otherwise, we can cache using services_url.
         self.service_catalog = None
         self.services_url = {}
 
@@ -410,11 +408,9 @@ class HTTPClient(object):
             path = re.sub(r'v[1-9]/[a-z0-9]+$', '', path)
             url = parse.urlunsplit((scheme, netloc, path, None, None))
         else:
-            if self.service_catalog:
+            if self.service_catalog and not self.bypass_url:
                 url = self.get_service_url(self.service_type) + url
             else:
-                # NOTE(melwitt): The service catalog is not available
-                #                when bypass_url is used.
                 url = self.management_url + url
 
         # Perform the request once. If we get a 401 back then it

novaclient/tests/functional/test_readonly_nova.py

@@ -169,3 +169,9 @@ class SimpleReadOnlyNovaClientTest(base.ClientTestBase):
 
     def test_admin_timing(self):
         self.nova('list', flags='--timing')
+
+    def test_admin_invalid_bypass_url(self):
+        self.assertRaises(exceptions.CommandFailed,
+                          self.nova,
+                          'list',
+                          flags='--bypass-url badurl')

novaclient/tests/unit/test_client.py

@@ -262,6 +262,45 @@ class ClientTest(utils.TestCase):
         self.assertEqual("compute/v100", cs.bypass_url)
         self.assertEqual("compute/v100", cs.management_url)
 
+    def test_service_url_lookup(self):
+        service_type = 'compute'
+        cs = novaclient.client.HTTPClient(None, None, None,
+                                          auth_url='foo/v2',
+                                          service_type=service_type)
+
+        @mock.patch.object(cs, 'get_service_url', return_value='compute/v5')
+        @mock.patch.object(cs, 'request', return_value=(200, '{}'))
+        @mock.patch.object(cs, 'authenticate')
+        def do_test(mock_auth, mock_request, mock_get):
+
+            def set_service_catalog():
+                cs.service_catalog = 'catalog'
+
+            mock_auth.side_effect = set_service_catalog
+            cs.get('/servers')
+            mock_get.assert_called_once_with(service_type)
+            mock_request.assert_called_once_with('compute/v5/servers',
+                                                 'GET', headers=mock.ANY)
+            mock_auth.assert_called_once_with()
+
+        do_test()
+
+    def test_bypass_url_no_service_url_lookup(self):
+        bypass_url = 'compute/v100'
+        cs = novaclient.client.HTTPClient(None, None, None,
+                                          auth_url='foo/v2',
+                                          bypass_url=bypass_url)
+
+        @mock.patch.object(cs, 'get_service_url')
+        @mock.patch.object(cs, 'request', return_value=(200, '{}'))
+        def do_test(mock_request, mock_get):
+            cs.get('/servers')
+            self.assertFalse(mock_get.called)
+            mock_request.assert_called_once_with(bypass_url + '/servers',
+                                                 'GET', headers=mock.ANY)
+
+        do_test()
+
     @mock.patch("novaclient.client.requests.Session")
     def test_session(self, mock_session):
         fake_session = mock.Mock()