diff --git a/shade/_tasks.py b/shade/_tasks.py index c4c58053c..f775e95db 100644 --- a/shade/_tasks.py +++ b/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) diff --git a/shade/operatorcloud.py b/shade/operatorcloud.py index 66da61494..c8a090431 100644 --- a/shade/operatorcloud.py +++ b/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 diff --git a/shade/task_manager.py b/shade/task_manager.py index 4cd03a6bd..e903f6738 100644 --- a/shade/task_manager.py +++ b/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: diff --git a/shade/tests/unit/test_flavors.py b/shade/tests/unit/test_flavors.py index 820a7a055..f16e0e1c3 100644 --- a/shade/tests/unit/test_flavors.py +++ b/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):