Get rid of magnumclient dependency

One more client library down.

Note there is a change to one of the tests. That's mainly because
testtools is not matching the exception the way an end user would. We'll
follow up on it, but it's not a real issue.

Change-Id: Ic6d7a37799e72bfb1bef7aeaf8f4894aed27dcea

releasenotes/notes/remove-magnumclient-875b3e513f98f57c.yaml

@@ -0,0 +1,4 @@
+---
+upgrade:
+  - magnumclient is no longer a direct dependency as
+    magnum API calls are now made directly via REST.

requirements.txt

@@ -18,6 +18,5 @@ python-neutronclient>=2.3.10
 python-ironicclient>=0.10.0
 python-heatclient>=1.0.0
 python-designateclient>=2.1.0
-python-magnumclient>=2.1.0
 
 dogpile.cache>=0.5.3

shade/_tasks.py

@@ -824,32 +824,6 @@ class NeutronQuotasDelete(task_manager.Task):
         return client.neutron_client.delete_quota(**self.args)
 
 
-class ClusterTemplateList(task_manager.Task):
-    def main(self, client):
-        return client.magnum_client.baymodels.list(**self.args)
-
-
-class ClusterTemplateCreate(task_manager.Task):
-    def main(self, client):
-        return client.magnum_client.baymodels.create(**self.args)
-
-
-class ClusterTemplateDelete(task_manager.Task):
-    def main(self, client):
-        return client.magnum_client.baymodels.delete(self.args['id'])
-
-
-class ClusterTemplateUpdate(task_manager.Task):
-    def main(self, client):
-        return client.magnum_client.baymodels.update(
-            self.args['id'], self.args['patch'])
-
-
-class MagnumServicesList(task_manager.Task):
-    def main(self, client):
-        return client.magnum_client.mservices.list(detail=False)
-
-
 class NovaLimitsGet(task_manager.Task):
     def main(self, client):
         return client.nova_client.limits.get(**self.args).to_dict()

shade/_utils.py

@@ -664,7 +664,7 @@ def generate_patches_from_kwargs(operation, **kwargs):
                  'value': v,
                  'path': '/%s' % k}
         patches.append(patch)
-    return patches
+    return sorted(patches)
 
 
 class FileSegment(object):

shade/openstackcloud.py

@@ -31,7 +31,6 @@ import requestsexceptions
 from six.moves import urllib
 
 import cinderclient.exceptions as cinder_exceptions
-import magnumclient.exceptions as magnum_exceptions
 from heatclient import exc as heat_exceptions
 import keystoneauth1.exceptions
 import novaclient.exceptions as nova_exceptions
@@ -1192,6 +1191,10 @@ class OpenStackCloud(_normalize.Normalizer):
 
     @property
     def magnum_client(self):
+        warnings.warn(
+            'Using shade to get a magnum object is deprecated. If you'
+            ' need a raw magnumclient.client.Client object, please use'
+            ' make_legacy_client in os-client-config instead')
         if self._magnum_client is None:
             self._magnum_client = self._get_client('container-infra')
         return self._magnum_client
@@ -7126,8 +7129,8 @@ class OpenStackCloud(_normalize.Normalizer):
             the OpenStack API call.
         """
         with _utils.shade_exceptions("Error fetching cluster template list"):
-            cluster_templates = self.manager.submit_task(
+            cluster_templates = self._container_infra_client.get(
-                _tasks.ClusterTemplateList(detail=True))
+                '/baymodels/detail')
         return self._normalize_cluster_templates(cluster_templates)
     list_baymodels = list_cluster_templates
 
@@ -7192,14 +7195,18 @@ class OpenStackCloud(_normalize.Normalizer):
         :raises: ``OpenStackCloudException`` if something goes wrong during
             the OpenStack API call
         """
-        with _utils.shade_exceptions(
+        error_message = ("Error creating cluster template of name"
-                "Error creating cluster template of name"
+                         " {cluster_template_name}".format(
-                " {cluster_template_name}".format(
+                             cluster_template_name=name))
-                    cluster_template_name=name)):
+        with _utils.shade_exceptions(error_message):
-            cluster_template = self.manager.submit_task(
+            body = kwargs.copy()
-                _tasks.ClusterTemplateCreate(
+            body['name'] = name
-                    name=name, image_id=image_id,
+            body['image_id'] = image_id
-                    keypair_id=keypair_id, coe=coe, **kwargs))
+            body['keypair_id'] = keypair_id
+            body['coe'] = coe
+
+            cluster_template = self._container_infra_client.post(
+                '/baymodels', json=body)
 
         self.list_cluster_templates.invalidate(self)
         return cluster_template
@@ -7215,7 +7222,6 @@ class OpenStackCloud(_normalize.Normalizer):
         :raises: OpenStackCloudException on operation error.
         """
 
-        self.list_cluster_templates.invalidate(self)
         cluster_template = self.get_cluster_template(name_or_id)
 
         if not cluster_template:
@@ -7226,16 +7232,10 @@ class OpenStackCloud(_normalize.Normalizer):
             return False
 
         with _utils.shade_exceptions("Error in deleting cluster template"):
-            try:
+            self._container_infra_client.delete(
-                self.manager.submit_task(
+                '/baymodels/{id}'.format(id=cluster_template['id']))
-                    _tasks.ClusterTemplateDelete(id=cluster_template['id']))
+            self.list_cluster_templates.invalidate(self)
-            except magnum_exceptions.NotFound:
-                self.log.debug(
-                    "Cluster template %(id)s not found when deleting."
-                    " Ignoring.", {'id': cluster_template['id']})
-                return False
 
-        self.list_cluster_templates.invalidate(self)
         return True
     delete_baymodel = delete_cluster_template
 
@@ -7268,12 +7268,15 @@ class OpenStackCloud(_normalize.Normalizer):
                 "%s operation not in 'add', 'replace', 'remove'" % operation)
 
         patches = _utils.generate_patches_from_kwargs(operation, **kwargs)
+        # No need to fire an API call if there is an empty patch
+        if not patches:
+            return cluster_template
 
         with _utils.shade_exceptions(
                 "Error updating cluster template {0}".format(name_or_id)):
-            self.manager.submit_task(
+            self._container_infra_client.patch(
-                _tasks.ClusterTemplateUpdate(
+                '/baymodels/{id}'.format(id=cluster_template['id']),
-                    id=cluster_template['id'], patch=patches))
+                json=patches)
 
         new_cluster_template = self.get_cluster_template(name_or_id)
         return new_cluster_template

shade/operatorcloud.py

@@ -2252,4 +2252,4 @@ class OperatorCloud(openstackcloud.OpenStackCloud):
         """
         with _utils.shade_exceptions("Error fetching Magnum services list"):
             return self._normalize_magnum_services(
-                self.manager.submit_task(_tasks.MagnumServicesList()))
+                self._container_infra_client.get('/mservices'))

shade/tests/unit/test_cluster_templates.py

@@ -144,10 +144,14 @@ class TestClusterTemplates(base.RequestsMockTestCase):
             method='POST',
             uri='https://container-infra.example.com/v1/baymodels',
             status_code=403)])
-        with testtools.ExpectedException(
+        # TODO(mordred) requests here doens't give us a great story
-            shade.OpenStackCloudException,
+        # for matching the old error message text. Investigate plumbing
-            "Error creating cluster template of name fake-cluster-template"
+        # an error message in to the adapter call so that we can give a
-        ):
+        # more informative error. Also, the test was originally catching
+        # OpenStackCloudException - but for some reason testtools will not
+        # match the more specific HTTPError, even though it's a subclass
+        # of OpenStackCloudException.
+        with testtools.ExpectedException(shade.OpenStackCloudHTTPError):
             self.cloud.create_cluster_template('fake-cluster-template')
         self.assert_calls()