Use direct requests for flavor extra_specs set/unset

Same with the get - we need to resort to direct REST to do set/unset.

Change-Id: Iec5b66b05bbd63071c5558782de7a67253ae209c

shade/_tasks.py

@@ -95,6 +95,24 @@ class FlavorGetExtraSpecs(task_manager.RequestTask):
             "/flavors/{id}/os-extra_specs".format(**self.args))
 
 
+class FlavorSetExtraSpecs(task_manager.RequestTask):
+    result_key = 'extra_specs'
+
+    def main(self, client):
+        return client._compute_client.post(
+            "/flavors/{id}/os-extra_specs".format(**self.args),
+            json=self.args['json']
+            )
+
+
+class FlavorUnsetExtraSpecs(task_manager.RequestTask):
+
+    def main(self, client):
+        return client._compute_client.delete(
+            "/flavors/{id}/os-extra_specs/{key}".format(**self.args),
+            )
+
+
 class FlavorCreate(task_manager.Task):
     def main(self, client):
         return client.nova_client.flavors.create(**self.args)

shade/operatorcloud.py

@@ -14,7 +14,6 @@ import jsonpatch
 
 from ironicclient import client as ironic_client
 from ironicclient import exceptions as ironic_exceptions
-from novaclient import exceptions as nova_exceptions
 
 from shade.exc import *  # noqa
 from shade import openstackcloud
@@ -1452,47 +1451,6 @@ class OperatorCloud(openstackcloud.OpenStackCloud):
 
         return True
 
-    def _mod_flavor_specs(self, action, flavor_id, specs):
-        """Common method for modifying flavor extra specs.
-
-        Nova (very sadly) doesn't expose this with a public API, so we
-        must get the actual flavor object and make a method call on it.
-
-        Two separate try-except blocks are used because Nova can raise
-        a NotFound exception if FlavorGet() is given an invalid flavor ID,
-        or if the unset_keys() method of the flavor object is given an
-        invalid spec key. We need to be able to differentiate between these
-        actions, thus the separate blocks.
-        """
-        try:
-            flavor = self.manager.submitTask(
-                _tasks.FlavorGet(flavor=flavor_id), raw=True
-            )
-        except nova_exceptions.NotFound:
-            self.log.debug(
-                "Flavor ID {0} not found. "
-                "Cannot {1} extra specs.".format(flavor_id, action)
-            )
-            raise OpenStackCloudResourceNotFound(
-                "Flavor ID {0} not found".format(flavor_id)
-            )
-        except OpenStackCloudException:
-            raise
-        except Exception as e:
-            raise OpenStackCloudException(
-                "Error getting flavor ID {0}: {1}".format(flavor_id, e)
-            )
-
-        try:
-            if action == 'set':
-                flavor.set_keys(specs)
-            elif action == 'unset':
-                flavor.unset_keys(specs)
-        except Exception as e:
-            raise OpenStackCloudException(
-                "Unable to {0} flavor specs: {1}".format(action, e)
-            )
-
     def set_flavor_specs(self, flavor_id, extra_specs):
         """Add extra specs to a flavor
 
@@ -1502,7 +1460,14 @@ class OperatorCloud(openstackcloud.OpenStackCloud):
         :raises: OpenStackCloudException on operation error.
         :raises: OpenStackCloudResourceNotFound if flavor ID is not found.
         """
-        self._mod_flavor_specs('set', flavor_id, extra_specs)
+        try:
+            self.manager.submitTask(
+                _tasks.FlavorSetExtraSpecs(
+                    id=flavor_id, json=dict(extra_specs=extra_specs)))
+        except Exception as e:
+            raise OpenStackCloudException(
+                "Unable to set flavor specs: {0}".format(str(e))
+            )
 
     def unset_flavor_specs(self, flavor_id, keys):
         """Delete extra specs from a flavor
@@ -1513,7 +1478,14 @@ class OperatorCloud(openstackcloud.OpenStackCloud):
         :raises: OpenStackCloudException on operation error.
         :raises: OpenStackCloudResourceNotFound if flavor ID is not found.
         """
-        self._mod_flavor_specs('unset', flavor_id, keys)
+        for key in keys:
+            try:
+                self.manager.submitTask(
+                    _tasks.FlavorUnsetExtraSpecs(id=flavor_id, key=key))
+            except Exception as e:
+                raise OpenStackCloudException(
+                    "Unable to delete flavor spec {0}: {0}".format(
+                        key, str(e)))
 
     def _mod_flavor_access(self, action, flavor_id, project_id):
         """Common method for adding and removing flavor access

shade/task_manager.py

@@ -21,6 +21,7 @@ import time
 import types
 
 import keystoneauth1.exceptions
+import simplejson
 import six
 
 from shade import _log
@@ -100,6 +101,7 @@ class Task(object):
             return self._result
 
     def run(self, client):
+        self._client = client
         try:
             # Retry one time if we get a retriable connection failure
             try:
@@ -117,10 +119,21 @@ class Task(object):
 
 class RequestTask(Task):
 
+    # It's totally legit for calls to not return things
+    result_key = None
+
     # keystoneauth1 throws keystoneauth1.exceptions.http.HttpError on !200
     def done(self, result):
         self._response = result
-        result_json = self._response.json()
+
+        try:
+            result_json = self._response.json()
+        except (simplejson.scanner.JSONDecodeError, ValueError) as e:
+            result_json = self._response.text
+            self._client.log.debug(
+                'Could not decode json in response: {e}'.format(e=str(e)))
+            self._client.log.debug(result_json)
+
         if self.result_key:
             self._result = result_json[self.result_key]
         else:

shade/tests/unit/test_flavors.py

@@ -69,23 +69,25 @@ class TestFlavors(base.TestCase):
         self.op_cloud.list_flavors()
         mock_nova.flavors.list.assert_called_once_with(is_public=None)
 
-    @mock.patch.object(shade.OpenStackCloud, 'nova_client')
-    def test_set_flavor_specs(self, mock_nova):
-        flavor = mock.Mock(id=1, name='orange')
-        mock_nova.flavors.get.return_value = flavor
+    @mock.patch.object(shade.OpenStackCloud, '_compute_client')
+    def test_set_flavor_specs(self, mock_compute):
         extra_specs = dict(key1='value1')
         self.op_cloud.set_flavor_specs(1, extra_specs)
-        mock_nova.flavors.get.assert_called_once_with(flavor=1)
-        flavor.set_keys.assert_called_once_with(extra_specs)
+        mock_compute.post.assert_called_once_with(
+            '/flavors/{id}/os-extra_specs'.format(id=1),
+            json=dict(extra_specs=extra_specs))
 
-    @mock.patch.object(shade.OpenStackCloud, 'nova_client')
-    def test_unset_flavor_specs(self, mock_nova):
-        flavor = mock.Mock(id=1, name='orange')
-        mock_nova.flavors.get.return_value = flavor
+    @mock.patch.object(shade.OpenStackCloud, '_compute_client')
+    def test_unset_flavor_specs(self, mock_compute):
         keys = ['key1', 'key2']
         self.op_cloud.unset_flavor_specs(1, keys)
-        mock_nova.flavors.get.assert_called_once_with(flavor=1)
-        flavor.unset_keys.assert_called_once_with(keys)
+        api_spec = '/flavors/{id}/os-extra_specs/{key}'
+        self.assertEqual(
+            mock_compute.delete.call_args_list[0],
+            mock.call(api_spec.format(id=1, key='key1')))
+        self.assertEqual(
+            mock_compute.delete.call_args_list[1],
+            mock.call(api_spec.format(id=1, key='key2')))
 
     @mock.patch.object(shade.OpenStackCloud, 'nova_client')
     def test_add_flavor_access(self, mock_nova):