From b9d7d1e640aaa4cfec96e99f5271c5600beddcca Mon Sep 17 00:00:00 2001
From: Monty Taylor <mordred@inaugust.com>
Date: Fri, 5 Jan 2018 19:56:46 -0600
Subject: [PATCH] Update openstacksdk construction

The queens release of python-openstacksdk breaks the sdk and senlin code
that's in heat right now. This shifts us to the new world order.

python-senlinclient is actually just a thin wrapper around
python-openstacksdk. Instead of reworking the constructor in the Senlin
plugin, just remove use of python-senlinclient and use the openstacksdk
client plugin instead.

Change-Id: Idf0acebf7b3774db26e335b3f3229227bfe68502
---
 README.rst                                   |  1 -
 heat/engine/clients/os/openstacksdk.py       | 50 +++++++++++++-------
 heat/engine/clients/os/senlin.py             | 48 ++++++++-----------
 heat/tests/clients/test_senlin_client.py     | 18 +++----
 heat/tests/openstack/senlin/test_cluster.py  |  4 +-
 heat/tests/openstack/senlin/test_node.py     |  4 +-
 heat/tests/openstack/senlin/test_policy.py   |  8 ++--
 heat/tests/openstack/senlin/test_receiver.py |  4 +-
 requirements.txt                             |  1 -
 9 files changed, 72 insertions(+), 66 deletions(-)

diff --git a/README.rst b/README.rst
index 0983068bfd..bfec829632 100644
--- a/README.rst
+++ b/README.rst
@@ -60,4 +60,3 @@ We have integration with
 * https://git.openstack.org/cgit/openstack/python-mistralclient (workflow service)
 * https://git.openstack.org/cgit/openstack/python-zaqarclient (messaging service)
 * https://git.openstack.org/cgit/openstack/python-monascaclient (monitoring service)
-* https://git.openstack.org/cgit/openstack/python-senlinclient (clustering service)
diff --git a/heat/engine/clients/os/openstacksdk.py b/heat/engine/clients/os/openstacksdk.py
index af6f32b989..8be8612dcc 100644
--- a/heat/engine/clients/os/openstacksdk.py
+++ b/heat/engine/clients/os/openstacksdk.py
@@ -11,12 +11,15 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+from openstack.config import cloud_region
 from openstack import connection
 from openstack import exceptions
-from openstack import profile
+import os_service_types
 
+from heat.common import config
 from heat.engine.clients import client_plugin
 from heat.engine import constraints
+import heat.version
 
 CLIENT_NAME = 'openstack'
 
@@ -25,24 +28,39 @@ class OpenStackSDKPlugin(client_plugin.ClientPlugin):
 
     exceptions_module = exceptions
 
-    service_types = [NETWORK] = ['network']
-    service_client_map = {NETWORK: 'neutron'}
-    api_version_map = {NETWORK: '2.0'}
+    service_types = [NETWORK, CLUSTERING] = ['network', 'clustering']
 
     def _create(self, version=None):
-        prof = profile.Profile()
-        for svc_type in self.service_types:
-            interface = self._get_client_option(
-                self.service_client_map[svc_type], 'endpoint_type')
-            prof.set_interface(svc_type, interface)
-            prof.set_region(svc_type, self._get_region_name())
-            prof.set_version(svc_type, self.api_version_map[svc_type])
+        config = cloud_region.from_session(
+            # TODO(mordred) The way from_session calculates a cloud name
+            # doesn't interact well with the mocks in the test cases. The
+            # name is used in logging to distinguish requests made to different
+            # clouds. For now, set it to local - but maybe find a way to set
+            # it to something more meaningful later.
+            name='local',
+            session=self.context.keystone_session,
+            config=self._get_service_interfaces(),
+            region_name=self._get_region_name(),
+            app_name='heat',
+            app_version=heat.version.version_info.version_string())
+        return connection.Connection(config=config)
 
-        key_session = self.context.keystone_session
-        return connection.Connection(authenticator=key_session.auth,
-                                     verify=key_session.verify,
-                                     cert=key_session.cert,
-                                     profile=prof)
+    def _get_service_interfaces(self):
+        interfaces = {}
+        if not os_service_types:
+            return interfaces
+        types = os_service_types.ServiceTypes()
+        for name, _ in config.list_opts():
+            if not name or not name.startswith('clients_'):
+                continue
+            project_name = name.split("_", 1)[0]
+            service_data = types.get_service_data_for_project(project_name)
+            if not service_data:
+                continue
+            service_type = service_data['service_type']
+            interfaces[service_type + '_interface'] = self._get_client_option(
+                service_type, 'endpoint_type')
+        return interfaces
 
     def is_not_found(self, ex):
         return isinstance(ex, exceptions.ResourceNotFound)
diff --git a/heat/engine/clients/os/senlin.py b/heat/engine/clients/os/senlin.py
index c88b795d1e..66cb7dccb1 100644
--- a/heat/engine/clients/os/senlin.py
+++ b/heat/engine/clients/os/senlin.py
@@ -11,30 +11,23 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+from openstack import exceptions
+
 from heat.common import exception
 from heat.common.i18n import _
-from heat.engine.clients import client_plugin
+from heat.engine.clients.os import openstacksdk as sdk_plugin
 from heat.engine import constraints
 
-from openstack import profile
-from senlinclient.common import exc
-from senlinclient.v1 import client
-
 CLIENT_NAME = 'senlin'
 
 
-class SenlinClientPlugin(client_plugin.ClientPlugin):
+class SenlinClientPlugin(sdk_plugin.OpenStackSDKPlugin):
 
-    service_types = [CLUSTERING] = ['clustering']
+    exceptions_module = exceptions
 
-    def _create(self):
-        interface = self._get_client_option(CLIENT_NAME, 'endpoint_type')
-        prof = profile.Profile()
-        prof.set_interface(self.CLUSTERING, interface)
-        prof.set_region(self.CLUSTERING, self._get_region_name())
-        key_session = self.context.keystone_session
-        return client.Client(prof=prof,
-                             authenticator=key_session.auth)
+    def _create(self, version=None):
+        client = super(SenlinClientPlugin, self)._create(version=version)
+        return client.clustering
 
     def generate_spec(self, spec_type, spec_props):
         spec = {'properties': spec_props}
@@ -64,11 +57,8 @@ class SenlinClientPlugin(client_plugin.ClientPlugin):
         policy = self.client().get_policy(policy_name)
         return policy.id
 
-    def is_not_found(self, ex):
-        return isinstance(ex, exc.sdkexc.ResourceNotFound)
-
     def is_bad_request(self, ex):
-        return (isinstance(ex, exc.sdkexc.HttpException) and
+        return (isinstance(ex, exceptions.HttpException) and
                 ex.status_code == 400)
 
     def execute_actions(self, actions):
@@ -93,24 +83,24 @@ class SenlinClientPlugin(client_plugin.ClientPlugin):
 
 
 class ProfileConstraint(constraints.BaseCustomConstraint):
-    # If name is not unique, will raise exc.sdkexc.HttpException
-    expected_exceptions = (exc.sdkexc.HttpException,)
+    # If name is not unique, will raise exceptions.HttpException
+    expected_exceptions = (exceptions.HttpException,)
 
     def validate_with_client(self, client, profile):
         client.client(CLIENT_NAME).get_profile(profile)
 
 
 class ClusterConstraint(constraints.BaseCustomConstraint):
-    #  If name is not unique, will raise exc.sdkexc.HttpException
-    expected_exceptions = (exc.sdkexc.HttpException,)
+    #  If name is not unique, will raise exceptions.HttpException
+    expected_exceptions = (exceptions.HttpException,)
 
     def validate_with_client(self, client, value):
         client.client(CLIENT_NAME).get_cluster(value)
 
 
 class PolicyConstraint(constraints.BaseCustomConstraint):
-    #  If name is not unique, will raise exc.sdkexc.HttpException
-    expected_exceptions = (exc.sdkexc.HttpException,)
+    #  If name is not unique, will raise exceptions.HttpException
+    expected_exceptions = (exceptions.HttpException,)
 
     def validate_with_client(self, client, value):
         client.client(CLIENT_NAME).get_policy(value)
@@ -121,8 +111,8 @@ class ProfileTypeConstraint(constraints.BaseCustomConstraint):
     expected_exceptions = (exception.StackValidationFailed,)
 
     def validate_with_client(self, client, value):
-        senlin_client = client.client(CLIENT_NAME)
-        type_list = senlin_client.profile_types()
+        conn = client.client(CLIENT_NAME)
+        type_list = conn.profile_types()
         names = [pt.name for pt in type_list]
         if value not in names:
             not_found_message = (
@@ -138,8 +128,8 @@ class PolicyTypeConstraint(constraints.BaseCustomConstraint):
     expected_exceptions = (exception.StackValidationFailed,)
 
     def validate_with_client(self, client, value):
-        senlin_client = client.client(CLIENT_NAME)
-        type_list = senlin_client.policy_types()
+        conn = client.client(CLIENT_NAME)
+        type_list = conn.policy_types()
         names = [pt.name for pt in type_list]
         if value not in names:
             not_found_message = (
diff --git a/heat/tests/clients/test_senlin_client.py b/heat/tests/clients/test_senlin_client.py
index 7bad95aba2..9510050d16 100644
--- a/heat/tests/clients/test_senlin_client.py
+++ b/heat/tests/clients/test_senlin_client.py
@@ -12,7 +12,7 @@
 #    under the License.
 
 import mock
-from senlinclient.common import exc
+from openstack import exceptions
 
 from heat.engine.clients.os import senlin as senlin_plugin
 from heat.tests import common
@@ -32,10 +32,10 @@ class SenlinClientPluginTest(common.HeatTestCase):
 
     def test_is_bad_request(self):
         self.assertTrue(self.plugin.is_bad_request(
-            exc.sdkexc.HttpException(http_status=400)))
+            exceptions.HttpException(http_status=400)))
         self.assertFalse(self.plugin.is_bad_request(Exception))
         self.assertFalse(self.plugin.is_bad_request(
-            exc.sdkexc.HttpException(http_status=404)))
+            exceptions.HttpException(http_status=404)))
 
     def test_check_action_success(self):
         mock_action = mock.MagicMock()
@@ -87,10 +87,10 @@ class ProfileConstraintTest(common.HeatTestCase):
         self.assertTrue(self.constraint.validate("PROFILE_ID", self.ctx))
 
     def test_validate_false(self):
-        self.mock_get_profile.side_effect = exc.sdkexc.ResourceNotFound(
+        self.mock_get_profile.side_effect = exceptions.ResourceNotFound(
             'PROFILE_ID')
         self.assertFalse(self.constraint.validate("PROFILE_ID", self.ctx))
-        self.mock_get_profile.side_effect = exc.sdkexc.HttpException(
+        self.mock_get_profile.side_effect = exceptions.HttpException(
             'PROFILE_ID')
         self.assertFalse(self.constraint.validate("PROFILE_ID", self.ctx))
 
@@ -112,10 +112,10 @@ class ClusterConstraintTest(common.HeatTestCase):
         self.assertTrue(self.constraint.validate("CLUSTER_ID", self.ctx))
 
     def test_validate_false(self):
-        self.mock_get_cluster.side_effect = exc.sdkexc.ResourceNotFound(
+        self.mock_get_cluster.side_effect = exceptions.ResourceNotFound(
             'CLUSTER_ID')
         self.assertFalse(self.constraint.validate("CLUSTER_ID", self.ctx))
-        self.mock_get_cluster.side_effect = exc.sdkexc.HttpException(
+        self.mock_get_cluster.side_effect = exceptions.HttpException(
             'CLUSTER_ID')
         self.assertFalse(self.constraint.validate("CLUSTER_ID", self.ctx))
 
@@ -137,10 +137,10 @@ class PolicyConstraintTest(common.HeatTestCase):
         self.assertTrue(self.constraint.validate("POLICY_ID", self.ctx))
 
     def test_validate_false(self):
-        self.mock_get_policy.side_effect = exc.sdkexc.ResourceNotFound(
+        self.mock_get_policy.side_effect = exceptions.ResourceNotFound(
             'POLICY_ID')
         self.assertFalse(self.constraint.validate("POLICY_ID", self.ctx))
-        self.mock_get_policy.side_effect = exc.sdkexc.HttpException(
+        self.mock_get_policy.side_effect = exceptions.HttpException(
             'POLICY_ID')
         self.assertFalse(self.constraint.validate("POLICY_ID", self.ctx))
 
diff --git a/heat/tests/openstack/senlin/test_cluster.py b/heat/tests/openstack/senlin/test_cluster.py
index 9f3467cb58..88656df447 100644
--- a/heat/tests/openstack/senlin/test_cluster.py
+++ b/heat/tests/openstack/senlin/test_cluster.py
@@ -26,7 +26,7 @@ from heat.engine import scheduler
 from heat.engine import template
 from heat.tests import common
 from heat.tests import utils
-from senlinclient.common import exc
+from openstack import exceptions
 
 
 cluster_stack_template = """
@@ -165,7 +165,7 @@ class SenlinClusterTest(common.HeatTestCase):
     def test_cluster_delete_success(self):
         cluster = self._create_cluster(self.t)
         self.senlin_mock.get_cluster.side_effect = [
-            exc.sdkexc.ResourceNotFound('SenlinCluster'),
+            exceptions.ResourceNotFound('SenlinCluster'),
         ]
         scheduler.TaskRunner(cluster.delete)()
         self.senlin_mock.delete_cluster.assert_called_once_with(
diff --git a/heat/tests/openstack/senlin/test_node.py b/heat/tests/openstack/senlin/test_node.py
index 7638a04929..3966fb2651 100644
--- a/heat/tests/openstack/senlin/test_node.py
+++ b/heat/tests/openstack/senlin/test_node.py
@@ -25,7 +25,7 @@ from heat.engine import scheduler
 from heat.engine import template
 from heat.tests import common
 from heat.tests import utils
-from senlinclient.common import exc
+from openstack import exceptions
 
 
 node_stack_template = """
@@ -127,7 +127,7 @@ class SenlinNodeTest(common.HeatTestCase):
     def test_node_delete_success(self):
         node = self._create_node()
         self.senlin_mock.get_node.side_effect = [
-            exc.sdkexc.ResourceNotFound('SenlinNode'),
+            exceptions.ResourceNotFound('SenlinNode'),
         ]
         scheduler.TaskRunner(node.delete)()
         self.senlin_mock.delete_node.assert_called_once_with(
diff --git a/heat/tests/openstack/senlin/test_policy.py b/heat/tests/openstack/senlin/test_policy.py
index f2f8ad7688..b8795550bd 100644
--- a/heat/tests/openstack/senlin/test_policy.py
+++ b/heat/tests/openstack/senlin/test_policy.py
@@ -15,8 +15,8 @@
 import copy
 import mock
 
+from openstack import exceptions
 from oslo_config import cfg
-from senlinclient.common import exc
 
 from heat.common import exception
 from heat.common import template_format
@@ -134,7 +134,7 @@ class SenlinPolicyTest(common.HeatTestCase):
             'action': 'fake_action'}
         policy = self._create_policy(self.t)
         self.senlin_mock.get_policy.side_effect = [
-            exc.sdkexc.ResourceNotFound('SenlinPolicy'),
+            exceptions.ResourceNotFound('SenlinPolicy'),
         ]
         scheduler.TaskRunner(policy.delete)()
         self.senlin_mock.cluster_detach_policy.assert_called_once_with(
@@ -145,10 +145,10 @@ class SenlinPolicyTest(common.HeatTestCase):
     def test_policy_delete_not_attached(self):
         policy = self._create_policy(self.t)
         self.senlin_mock.get_policy.side_effect = [
-            exc.sdkexc.ResourceNotFound('SenlinPolicy'),
+            exceptions.ResourceNotFound('SenlinPolicy'),
         ]
         self.senlin_mock.cluster_detach_policy.side_effect = [
-            exc.sdkexc.HttpException(http_status=400),
+            exceptions.HttpException(http_status=400),
         ]
         scheduler.TaskRunner(policy.delete)()
         self.senlin_mock.cluster_detach_policy.assert_called_once_with(
diff --git a/heat/tests/openstack/senlin/test_receiver.py b/heat/tests/openstack/senlin/test_receiver.py
index 8195e82b3c..138de4c6b2 100644
--- a/heat/tests/openstack/senlin/test_receiver.py
+++ b/heat/tests/openstack/senlin/test_receiver.py
@@ -14,7 +14,7 @@
 
 import mock
 
-from senlinclient.common import exc
+from openstack import exceptions
 
 from heat.common import template_format
 from heat.engine.clients.os import senlin
@@ -106,7 +106,7 @@ class SenlinReceiverTest(common.HeatTestCase):
 
     def test_recv_delete_not_found(self):
         self.senlin_mock.delete_receiver.side_effect = [
-            exc.sdkexc.ResourceNotFound(http_status=404)
+            exceptions.ResourceNotFound(http_status=404)
         ]
         recv = self._create_recv(self.t)
         scheduler.TaskRunner(recv.delete)()
diff --git a/requirements.txt b/requirements.txt
index 596deb4571..8bfb18b095 100644
--- a/requirements.txt
+++ b/requirements.txt
@@ -46,7 +46,6 @@ python-novaclient>=9.1.0 # Apache-2.0
 python-octaviaclient>=1.3.0 # Apache-2.0
 python-openstackclient>=3.12.0 # Apache-2.0
 python-saharaclient>=1.4.0 # Apache-2.0
-python-senlinclient>=1.1.0 # Apache-2.0
 python-swiftclient>=3.2.0 # Apache-2.0
 python-troveclient>=2.2.0 # Apache-2.0
 python-zaqarclient>=1.0.0 # Apache-2.0