Allow strict_proxies for sdk Connection

In version 0.35.0, openstacksdk added a strict_proxies kwarg to the
Connection constructor [1].

Without it, openstacksdk tries really hard to give us an Adapter, which
in the case of the service being down can mean we default to the catalog
endpoint without doing any discovery. This should usually work; but may
break in cases where the discovery document (at the catalog endpoint)
points to different URLs for versioned endpoints.

This commit adds a check_service bool kwarg to get_sdk_adapter which, if
True, uses strict_proxies to create the Connection, and causing
get_sdk_adapter to raise a ServiceUnavailable exception if the service
is down.

This can be used for services like Ironic, where we're set up to
tolerate connect failures on startup. But it should not be used for
services like Placement, where we expect getting the adapter to succeed,
and are instead tolerant of failures making the actual API calls.

[1] https://review.opendev.org/#/c/676837/

This dependency bumps the openstacksdk u-c in the requirements project.
Depends-On: https://review.opendev.org/678207
Change-Id: I86e038af8a96e113a754b2fdb3698acd3783c1c8

lower-constraints.txt

@@ -63,7 +63,7 @@ netaddr==0.7.18
 netifaces==0.10.4
 networkx==1.11
 numpy==1.14.2
-openstacksdk==0.34.0
+openstacksdk==0.35.0
 os-brick==2.6.1
 os-client-config==1.29.0
 os-resource-classes==0.1.0

nova/tests/unit/test_utils.py

@@ -19,12 +19,14 @@ import os.path
 import tempfile
 
 import eventlet
+import fixtures
 from keystoneauth1 import adapter as ks_adapter
 from keystoneauth1 import exceptions as ks_exc
 from keystoneauth1.identity import base as ks_identity
 from keystoneauth1 import session as ks_session
 import mock
 import netaddr
+from openstack import exceptions as sdk_exc
 from oslo_config import cfg
 from oslo_context import context as common_context
 from oslo_context import fixture as context_fixture
@@ -1305,41 +1307,69 @@ class TestGetAuthAndSession(test.NoDBTestCase):
 
 class TestGetSDKAdapter(test.NoDBTestCase):
     """Tests for nova.utils.get_sdk_adapter"""
-    @mock.patch('nova.utils._get_conf_group')
-    @mock.patch('nova.utils._get_auth_and_session')
-    @mock.patch('nova.utils.connection.Connection')
-    @mock.patch('nova.utils.CONF')
-    def test_get_sdk_adapter(self, mock_conf, mock_connection,
-                             mock_get_auth_sess, mock_get_confgrp):
-        service_type = 'test_service'
-        mock_conn = mock.Mock()
-        mock_proxy = mock.Mock()
-        setattr(mock_conn, service_type, mock_proxy)
-        mock_connection.return_value = mock_conn
-        mock_session = mock.Mock()
-        mock_get_auth_sess.return_value = (None, mock_session)
-        mock_get_confgrp.return_value = mock_confgrp = mock.Mock()
 
-        actual = utils.get_sdk_adapter(service_type)
+    def setUp(self):
+        super(TestGetSDKAdapter, self).setUp()
 
-        self.assertEqual(actual, mock_proxy)
-        mock_get_confgrp.assert_called_once_with(service_type)
-        mock_get_auth_sess.assert_called_once_with(mock_confgrp)
-        mock_connection.assert_called_once_with(
-            session=mock_session, oslo_conf=mock_conf,
-            service_types={'test_service'})
+        self.mock_get_confgrp = self.useFixture(fixtures.MockPatch(
+            'nova.utils._get_conf_group')).mock
 
-    @mock.patch('nova.utils._get_conf_group')
-    @mock.patch('nova.utils._get_auth_and_session')
-    @mock.patch('nova.utils.connection.Connection')
-    def test_get_sdk_adapter_fail(self, mock_connection, mock_get_auth_sess,
-                                  mock_get_confgrp):
-        service_type = 'test_service'
-        mock_get_confgrp.side_effect = \
-            exception.ConfGroupForServiceTypeNotFound(stype=service_type)
+        self.mock_get_auth_sess = self.useFixture(fixtures.MockPatch(
+            'nova.utils._get_auth_and_session')).mock
+        self.mock_get_auth_sess.return_value = (None, mock.sentinel.session)
+
+        self.service_type = 'test_service'
+        self.mock_connection = self.useFixture(fixtures.MockPatch(
+            'nova.utils.connection.Connection')).mock
+        self.mock_connection.return_value = mock.Mock(
+            test_service=mock.sentinel.proxy)
+
+        # We need to stub the CONF global in nova.utils to assert that the
+        # Connection constructor picks it up.
+        self.mock_conf = self.useFixture(fixtures.MockPatch(
+            'nova.utils.CONF')).mock
+
+    def test_get_sdk_adapter(self):
+        actual = utils.get_sdk_adapter(self.service_type)
+
+        self.assertEqual(actual, mock.sentinel.proxy)
+        self.mock_get_confgrp.assert_called_once_with(self.service_type)
+        self.mock_get_auth_sess.assert_called_once_with(
+            self.mock_get_confgrp.return_value)
+        self.mock_connection.assert_called_once_with(
+            session=mock.sentinel.session, oslo_conf=self.mock_conf,
+            service_types={'test_service'}, strict_proxies=False)
+
+    def test_get_sdk_adapter_strict(self):
+        actual = utils.get_sdk_adapter(self.service_type, check_service=True)
+
+        self.assertEqual(actual, mock.sentinel.proxy)
+        self.mock_get_confgrp.assert_called_once_with(self.service_type)
+        self.mock_get_auth_sess.assert_called_once_with(
+            self.mock_get_confgrp.return_value)
+        self.mock_connection.assert_called_once_with(
+            session=mock.sentinel.session, oslo_conf=self.mock_conf,
+            service_types={'test_service'}, strict_proxies=True)
+
+    def test_get_sdk_adapter_conf_group_fail(self):
+        self.mock_get_confgrp.side_effect = (
+            exception.ConfGroupForServiceTypeNotFound(stype=self.service_type))
 
         self.assertRaises(exception.ConfGroupForServiceTypeNotFound,
-                          utils.get_sdk_adapter, service_type)
-        mock_get_confgrp.assert_called_once_with(service_type)
-        mock_connection.assert_not_called()
-        mock_get_auth_sess.assert_not_called()
+                          utils.get_sdk_adapter, self.service_type)
+        self.mock_get_confgrp.assert_called_once_with(self.service_type)
+        self.mock_connection.assert_not_called()
+        self.mock_get_auth_sess.assert_not_called()
+
+    def test_get_sdk_adapter_strict_fail(self):
+        self.mock_connection.side_effect = sdk_exc.ServiceDiscoveryException()
+
+        self.assertRaises(
+            exception.ServiceUnavailable,
+            utils.get_sdk_adapter, self.service_type, check_service=True)
+        self.mock_get_confgrp.assert_called_once_with(self.service_type)
+        self.mock_get_auth_sess.assert_called_once_with(
+            self.mock_get_confgrp.return_value)
+        self.mock_connection.assert_called_once_with(
+            session=mock.sentinel.session, oslo_conf=self.mock_conf,
+            service_types={'test_service'}, strict_proxies=True)

nova/utils.py

@@ -34,6 +34,7 @@ from keystoneauth1 import exceptions as ks_exc
 from keystoneauth1 import loading as ks_loading
 import netaddr
 from openstack import connection
+from openstack import exceptions as sdk_exc
 import os_resource_classes as orc
 from os_service_types import service_types
 from oslo_concurrency import lockutils
@@ -52,7 +53,7 @@ from six.moves import range
 
 import nova.conf
 from nova import exception
-from nova.i18n import _LE, _LW
+from nova.i18n import _, _LE, _LW
 import nova.network
 from nova import safe_utils
 
@@ -1009,7 +1010,7 @@ def get_ksa_adapter(service_type, ksa_auth=None, ksa_session=None,
         min_version=min_version, max_version=max_version, raise_exc=False)
 
 
-def get_sdk_adapter(service_type):
+def get_sdk_adapter(service_type, check_service=False):
     """Construct an openstacksdk-brokered Adapter for a given service type.
 
     We expect to find a conf group whose name corresponds to the service_type's
@@ -1018,14 +1019,23 @@ def get_sdk_adapter(service_type):
 
     :param service_type: String name of the service type for which the Adapter
                          is to be constructed.
+    :param check_service: If True, we will query the endpoint to make sure the
+            service is alive, raising ServiceUnavailable if it is not.
     :return: An openstack.proxy.Proxy object for the specified service_type.
     :raise: ConfGroupForServiceTypeNotFound If no conf group name could be
             found for the specified service_type.
+    :raise: ServiceUnavailable if check_service is True and the service is down
     """
     confgrp = _get_conf_group(service_type)
-    _, sess = _get_auth_and_session(confgrp)
-    conn = connection.Connection(
-        session=sess, oslo_conf=CONF, service_types={service_type})
+    sess = _get_auth_and_session(confgrp)[1]
+    try:
+        conn = connection.Connection(
+            session=sess, oslo_conf=CONF, service_types={service_type},
+            strict_proxies=check_service)
+    except sdk_exc.ServiceDiscoveryException as e:
+        raise exception.ServiceUnavailable(
+            _("The %(service_type)s service is unavailable: %(error)s") %
+            {'service_type': service_type, 'error': six.text_type(e)})
     return getattr(conn, service_type)
 
 

requirements.txt

@@ -71,4 +71,4 @@ taskflow>=2.16.0 # Apache-2.0
 python-dateutil>=2.5.3 # BSD
 zVMCloudConnector>=1.3.0;sys_platform!='win32'  # Apache 2.0 License
 futurist>=1.8.0 # Apache-2.0
-openstacksdk>=0.34.0 # Apache-2.0
+openstacksdk>=0.35.0 # Apache-2.0