Add strict_proxies option for Connection

Add a strict_proxies kwarg to Connection, defaulting to False. When
True, ServiceDescription will perform an extra check when creating a
proxy. If it doesn't look like a valid connection is available, raise a
new SDKException called ServiceDiscoveryException.

Change-Id: I0b404d5744a4465d365780a4273aa8dc1cebeb14
Co-Authored-By: Monty Taylor <mordred@inaugust.com>

openstack/connection.py

@@ -273,6 +273,7 @@ class Connection(six.with_metaclass(_meta.ConnectionMeta,
                  oslo_conf=None,
                  service_types=None,
                  global_request_id=None,
+                 strict_proxies=False,
                  **kwargs):
         """Create a connection to a cloud.
 
@@ -330,12 +331,23 @@ class Connection(six.with_metaclass(_meta.ConnectionMeta,
             **Currently only supported in conjunction with the ``oslo_conf``
             kwarg.**
         :param global_request_id: A Request-id to send with all interactions.
+        :param strict_proxies:
+            If True, check proxies on creation and raise
+            ServiceDiscoveryException if the service is unavailable.
+        :type strict_proxies: bool
+            Throw an ``openstack.exceptions.ServiceDiscoveryException`` if the
+            endpoint for a given service doesn't work. This is useful for
+            OpenStack services using sdk to talk to other OpenStack services
+            where it can be expected that the deployer config is correct and
+            errors should be reported immediately.
+            Default false.
         :param kwargs: If a config is not provided, the rest of the parameters
             provided are assumed to be arguments to be passed to the
             CloudRegion constructor.
         """
         self.config = config
         self._extra_services = {}
+        self._strict_proxies = strict_proxies
         if extra_services:
             for service in extra_services:
                 self._extra_services[service.service_type] = service

openstack/exceptions.py

@@ -257,3 +257,7 @@ class TaskManagerStopped(SDKException):
 
 class ServiceDisabledException(ConfigException):
     """This service is disabled for reasons."""
+
+
+class ServiceDiscoveryException(SDKException):
+    """The service cannot be discovered."""

openstack/service_description.py

@@ -17,6 +17,7 @@ import os_service_types
 
 from openstack import _log
 from openstack import exceptions
+from openstack import proxy as proxy_mod
 
 __all__ = [
     'ServiceDescription',
@@ -83,10 +84,34 @@ class ServiceDescription(object):
         if instance is None:
             return self
         if self.service_type not in instance._proxies:
-            instance._proxies[self.service_type] = self._make_proxy(instance)
+            proxy = self._make_proxy(instance)
-            instance._proxies[self.service_type]._connection = instance
+            if not isinstance(proxy, _ServiceDisabledProxyShim):
+                # The keystone proxy has a method called get_endpoint
+                # that is about managing keystone endpoints. This is
+                # unfortunate.
+                endpoint = proxy_mod.Proxy.get_endpoint(proxy)
+                if instance._strict_proxies:
+                    self._validate_proxy(proxy, endpoint)
+                proxy._connection = instance
+            instance._proxies[self.service_type] = proxy
         return instance._proxies[self.service_type]
 
+    def _validate_proxy(self, proxy, endpoint):
+        exc = None
+        service_url = getattr(proxy, 'skip_discovery', None)
+        try:
+            # Don't go too wild for e.g. swift
+            if service_url is None:
+                service_url = proxy.get_endpoint_data().service_url
+        except Exception as e:
+            exc = e
+        if exc or not endpoint or not service_url:
+            raise exceptions.ServiceDiscoveryException(
+                "Failed to create a working proxy for service {service_type}: "
+                "{message}".format(
+                    service_type=self.service_type,
+                    message=exc or "No valid endpoint was discoverable."))
+
     def _make_proxy(self, instance):
         """Create a Proxy for the service in question.
 
@@ -108,8 +133,6 @@ class ServiceDescription(object):
                 self.service_type,
                 allow_version_hack=True,
             )
-            # trigger EndpointNotFound exception if this is bogus
-            temp_client.get_endpoint()
             return temp_client
 
         # Check to see if we've got config that matches what we
@@ -173,6 +196,12 @@ class ServiceDescription(object):
                 return proxy_obj
 
             data = proxy_obj.get_endpoint_data()
+            if not data and instance._strict_proxies:
+                raise exceptions.ServiceDiscoveryException(
+                    "Failed to create a working proxy for service "
+                    "{service_type}: No endpoint data found.".format(
+                        service_type=self.service_type))
+
             # If we've gotten here with a proxy object it means we have
             # an endpoint_override in place. If the catalog_url and
             # service_url don't match, which can happen if there is a

openstack/tests/unit/base.py

@@ -673,7 +673,8 @@ class TestCase(base.TestCase):
             if 'content-type' not in headers:
                 headers[u'content-type'] = 'application/json'
 
-            to_mock['headers'] = headers
+            if 'exc' not in to_mock:
+                to_mock['headers'] = headers
 
             self.calls += [
                 dict(

openstack/tests/unit/config/test_from_conf.py

@@ -10,6 +10,7 @@
 # License for the specific language governing permissions and limitations
 # under the License.
 
+import requests.exceptions
 import uuid
 
 from keystoneauth1 import exceptions as ks_exc
@@ -31,7 +32,7 @@ class TestFromConf(base.TestCase):
             **from_conf_kwargs)
         self.assertEqual('from_conf.example.com', config.name)
 
-        return connection.Connection(config=config)
+        return connection.Connection(config=config, strict_proxies=True)
 
     def test_adapter_opts_set(self):
         """Adapter opts specified in the conf."""
@@ -75,16 +76,10 @@ class TestFromConf(base.TestCase):
         """Adapter opts are registered, but all defaulting in conf."""
         conn = self._get_conn()
 
-        # Nova has empty adapter config, so these default
-        adap = conn.compute
-        self.assertIsNone(adap.region_name)
-        self.assertEqual('compute', adap.service_type)
-        self.assertEqual('public', adap.interface)
-        self.assertIsNone(adap.endpoint_override)
-
         server_id = str(uuid.uuid4())
         server_name = self.getUniqueString('name')
         fake_server = fakes.make_fake_server(server_id, server_name)
+
         self.register_uris([
             self.get_nova_discovery_mock_dict(),
             dict(method='GET',
@@ -92,6 +87,49 @@ class TestFromConf(base.TestCase):
                      'compute', 'public', append=['servers', 'detail']),
                  json={'servers': [fake_server]}),
         ])
+
+        # Nova has empty adapter config, so these default
+        adap = conn.compute
+        self.assertIsNone(adap.region_name)
+        self.assertEqual('compute', adap.service_type)
+        self.assertEqual('public', adap.interface)
+        self.assertIsNone(adap.endpoint_override)
+
+        s = next(adap.servers())
+        self.assertEqual(s.id, server_id)
+        self.assertEqual(s.name, server_name)
+        self.assert_calls()
+
+    def test_service_not_ready_catalog(self):
+        """Adapter opts are registered, but all defaulting in conf."""
+        conn = self._get_conn()
+
+        server_id = str(uuid.uuid4())
+        server_name = self.getUniqueString('name')
+        fake_server = fakes.make_fake_server(server_id, server_name)
+
+        self.register_uris([
+            dict(method='GET',
+                 uri='https://compute.example.com/v2.1/',
+                 exc=requests.exceptions.ConnectionError),
+            self.get_nova_discovery_mock_dict(),
+            dict(method='GET',
+                 uri=self.get_mock_url(
+                     'compute', 'public', append=['servers', 'detail']),
+                 json={'servers': [fake_server]}),
+        ])
+
+        self.assertRaises(
+            exceptions.ServiceDiscoveryException,
+            getattr, conn, 'compute')
+
+        # Nova has empty adapter config, so these default
+        adap = conn.compute
+        self.assertIsNone(adap.region_name)
+        self.assertEqual('compute', adap.service_type)
+        self.assertEqual('public', adap.interface)
+        self.assertIsNone(adap.endpoint_override)
+
         s = next(adap.servers())
         self.assertEqual(s.id, server_id)
         self.assertEqual(s.name, server_name)
@@ -119,6 +157,11 @@ class TestFromConf(base.TestCase):
             dict(method='GET',
                  uri='https://example.org:5050',
                  json=discovery),
+            # strict-proxies means we're going to fetch the discovery
+            # doc from the versioned endpoint to verify it works.
+            dict(method='GET',
+                 uri='https://example.org:5050/v1',
+                 json=discovery),
             dict(method='GET',
                  uri='https://example.org:5050/v1/introspection/abcd',
                  json=status),
@@ -131,6 +174,52 @@ class TestFromConf(base.TestCase):
 
         self.assertTrue(adap.get_introspection('abcd').is_finished)
 
+    def test_service_not_ready_endpoint_override(self):
+        conn = self._get_conn()
+
+        discovery = {
+            "versions": {
+                "values": [
+                    {"status": "stable",
+                     "id": "v1",
+                     "links": [{
+                         "href": "https://example.org:5050/v1",
+                         "rel": "self"}]
+                     }]
+            }
+        }
+        status = {
+            'finished': True,
+            'error': None
+        }
+        self.register_uris([
+            dict(method='GET',
+                 uri='https://example.org:5050',
+                 exc=requests.exceptions.ConnectTimeout),
+            dict(method='GET',
+                 uri='https://example.org:5050',
+                 json=discovery),
+            # strict-proxies means we're going to fetch the discovery
+            # doc from the versioned endpoint to verify it works.
+            dict(method='GET',
+                 uri='https://example.org:5050/v1',
+                 json=discovery),
+            dict(method='GET',
+                 uri='https://example.org:5050/v1/introspection/abcd',
+                 json=status),
+        ])
+
+        self.assertRaises(
+            exceptions.ServiceDiscoveryException,
+            getattr, conn, 'baremetal_introspection')
+
+        adap = conn.baremetal_introspection
+        self.assertEqual('baremetal-introspection', adap.service_type)
+        self.assertEqual('public', adap.interface)
+        self.assertEqual('https://example.org:5050/v1', adap.endpoint_override)
+
+        self.assertTrue(adap.get_introspection('abcd').is_finished)
+
     def assert_service_disabled(self, service_type, expected_reason,
                                 **from_conf_kwargs):
         conn = self._get_conn(**from_conf_kwargs)

releasenotes/notes/strict-proxies-4a315f68f387ee89.yaml

@@ -0,0 +1,8 @@
+---
+features:
+  - |
+    Added new option for Connection, ``strict_proxies``. When set to ``True``,
+    Connection will throw a ``ServiceDiscoveryException`` if the endpoint for
+    a given service doesn't work. This is useful for OpenStack services using
+    sdk to talk to other OpenStack services where it can be expected that the
+    deployer config is correct and errors should be reported immediately.