diff --git a/releasenotes/notes/flavor_fix-a53c6b326dc34a2c.yaml b/releasenotes/notes/flavor_fix-a53c6b326dc34a2c.yaml new file mode 100644 index 000000000..9a7ba7de1 --- /dev/null +++ b/releasenotes/notes/flavor_fix-a53c6b326dc34a2c.yaml @@ -0,0 +1,7 @@ +--- +features: + - Flavors will always contain an 'extra_specs' attribute. Client cruft, + such as 'links', 'HUMAN_ID', etc. has been removed. +fixes: + - Setting and unsetting flavor extra specs now works. This had + been broken since the 1.2.0 release. diff --git a/shade/_utils.py b/shade/_utils.py index f0833a726..7c6eaaf02 100644 --- a/shade/_utils.py +++ b/shade/_utils.py @@ -468,6 +468,18 @@ def normalize_stacks(stacks): return stacks +def normalize_flavors(flavors): + """ Normalize a list of flavor objects """ + for flavor in flavors: + flavor.pop('links', None) + flavor.pop('NAME_ATTR', None) + flavor.pop('HUMAN_ID', None) + flavor.pop('human_id', None) + if 'extra_specs' not in flavor: + flavor['extra_specs'] = {} + return flavors + + def valid_kwargs(*valid_args): # This decorator checks if argument passed as **kwargs to a function are # present in valid_args. diff --git a/shade/openstackcloud.py b/shade/openstackcloud.py index 647c698cd..b89b8a816 100644 --- a/shade/openstackcloud.py +++ b/shade/openstackcloud.py @@ -1137,7 +1137,14 @@ class OpenStackCloud(object): """ with _utils.shade_exceptions("Error fetching flavor list"): - return self.manager.submitTask(_tasks.FlavorList(is_public=None)) + raw_flavors = self.manager.submitTask( + _tasks.FlavorList(is_public=None), raw=True) + for flavor in raw_flavors: + flavor.extra_specs = flavor.get_keys() + + return _utils.normalize_flavors( + meta.obj_list_to_dict(raw_flavors) + ) @_utils.cache_on_arguments(should_cache_fn=_no_pending_stacks) def list_stacks(self): diff --git a/shade/operatorcloud.py b/shade/operatorcloud.py index ea5342abc..66da61494 100644 --- a/shade/operatorcloud.py +++ b/shade/operatorcloud.py @@ -1429,7 +1429,7 @@ class OperatorCloud(openstackcloud.OpenStackCloud): is_public=is_public) ) - return flavor + return _utils.normalize_flavors([flavor])[0] def delete_flavor(self, name_or_id): """Delete a flavor @@ -1466,7 +1466,7 @@ class OperatorCloud(openstackcloud.OpenStackCloud): """ try: flavor = self.manager.submitTask( - _tasks.FlavorGet(flavor=flavor_id) + _tasks.FlavorGet(flavor=flavor_id), raw=True ) except nova_exceptions.NotFound: self.log.debug( diff --git a/shade/task_manager.py b/shade/task_manager.py index 620347b15..3b9d0c9a1 100644 --- a/shade/task_manager.py +++ b/shade/task_manager.py @@ -70,7 +70,7 @@ class Task(object): self._traceback = tb self._finished.set() - def wait(self): + def wait(self, raw): self._finished.wait() # TODO(mordred): We store the raw requests response if there is # one now. So we should probably do an error handler to throw @@ -79,6 +79,10 @@ class Task(object): six.reraise(type(self._exception), self._exception, self._traceback) + if raw: + # Do NOT convert the result. + return self._result + # NOTE(Shrews): Since the client API might decide to subclass one # of these result types, we use isinstance() here instead of type(). if (isinstance(self._result, list) or @@ -126,7 +130,13 @@ class TaskManager(object): """ This is a direct action passthrough TaskManager """ pass - def submitTask(self, task): + def submitTask(self, task, raw=False): + """Submit and execute the given task. + + :param task: The task to execute. + :param bool raw: If True, return the raw result as received from the + underlying client call. + """ self.log.debug( "Manager %s running task %s" % (self.name, type(task).__name__)) start = time.time() @@ -135,4 +145,4 @@ class TaskManager(object): self.log.debug( "Manager %s ran task %s in %ss" % ( self.name, type(task).__name__, (end - start))) - return task.wait() + return task.wait(raw) diff --git a/shade/tests/fakes.py b/shade/tests/fakes.py index f14314f75..1f3444aaf 100644 --- a/shade/tests/fakes.py +++ b/shade/tests/fakes.py @@ -41,9 +41,13 @@ class FakeEndpointv3(object): class FakeFlavor(object): - def __init__(self, id, name): + def __init__(self, id, name, ram): self.id = id self.name = name + self.ram = ram + + def get_keys(self): + return {} class FakeFloatingIP(object): diff --git a/shade/tests/functional/test_flavor.py b/shade/tests/functional/test_flavor.py index 123b3c850..aefc00452 100644 --- a/shade/tests/functional/test_flavor.py +++ b/shade/tests/functional/test_flavor.py @@ -21,9 +21,6 @@ test_flavor Functional tests for `shade` flavor resource. """ -import string -import random - import shade from shade.exc import OpenStackCloudException from shade.tests import base @@ -37,8 +34,7 @@ class TestFlavor(base.TestCase): self.operator_cloud = shade.operator_cloud(cloud='devstack-admin') # Generate a random name for flavors in this test - self.new_item_name = 'flavor_' + ''.join( - random.choice(string.ascii_lowercase) for _ in range(5)) + self.new_item_name = self.getUniqueString('flavor') self.addCleanup(self._cleanup_flavors) @@ -67,6 +63,12 @@ class TestFlavor(base.TestCase): flavor = self.operator_cloud.create_flavor(**flavor_kwargs) self.assertIsNotNone(flavor['id']) + + # When properly normalized, we should always get an extra_specs + # and expect empty dict on create. + self.assertIn('extra_specs', flavor) + self.assertEqual({}, flavor['extra_specs']) + for key in flavor_kwargs.keys(): self.assertIn(key, flavor) for key, value in flavor_kwargs.items(): @@ -93,6 +95,8 @@ class TestFlavor(base.TestCase): # to make sure both of the flavors we just created are present. found = [] for f in flavors: + # extra_specs should be added within list_flavors() + self.assertIn('extra_specs', f) if f['name'] in (pub_flavor_name, priv_flavor_name): found.append(f) self.assertEqual(2, len(found)) @@ -125,3 +129,32 @@ class TestFlavor(base.TestCase): project['id']) flavors = self.demo_cloud.search_flavors(priv_flavor_name) self.assertEqual(0, len(flavors)) + + def test_set_unset_flavor_specs(self): + """ + Test setting and unsetting flavor extra specs + """ + flavor_name = self.new_item_name + '_spec_test' + kwargs = dict( + name=flavor_name, ram=1024, vcpus=2, disk=10 + ) + new_flavor = self.operator_cloud.create_flavor(**kwargs) + + # Expect no extra_specs + self.assertEqual({}, new_flavor['extra_specs']) + + # Now set them + extra_specs = {'foo': 'aaa', 'bar': 'bbb'} + self.operator_cloud.set_flavor_specs(new_flavor['id'], extra_specs) + mod_flavor = self.operator_cloud.get_flavor(new_flavor['id']) + + # Verify extra_specs were set + self.assertIn('extra_specs', mod_flavor) + self.assertEqual(extra_specs, mod_flavor['extra_specs']) + + # Unset the 'foo' value + self.operator_cloud.unset_flavor_specs(mod_flavor['id'], ['foo']) + mod_flavor = self.operator_cloud.get_flavor(new_flavor['id']) + + # Verify 'foo' is unset and 'bar' is still set + self.assertEqual({'bar': 'bbb'}, mod_flavor['extra_specs']) diff --git a/shade/tests/unit/test_caching.py b/shade/tests/unit/test_caching.py index 76ae9cf22..3b30cdfd3 100644 --- a/shade/tests/unit/test_caching.py +++ b/shade/tests/unit/test_caching.py @@ -303,8 +303,10 @@ class TestMemoryCache(base.TestCase): nova_mock.flavors.list.return_value = [] self.assertEqual([], self.cloud.list_flavors()) - fake_flavor = fakes.FakeFlavor('555', 'vanilla') - fake_flavor_dict = meta.obj_to_dict(fake_flavor) + fake_flavor = fakes.FakeFlavor('555', 'vanilla', 100) + fake_flavor_dict = _utils.normalize_flavors( + [meta.obj_to_dict(fake_flavor)] + )[0] nova_mock.flavors.list.return_value = [fake_flavor] self.cloud.list_flavors.invalidate(self.cloud) self.assertEqual([fake_flavor_dict], self.cloud.list_flavors()) diff --git a/shade/tests/unit/test_flavors.py b/shade/tests/unit/test_flavors.py index c44b29717..9342ab55f 100644 --- a/shade/tests/unit/test_flavors.py +++ b/shade/tests/unit/test_flavors.py @@ -40,7 +40,7 @@ class TestFlavors(base.TestCase): @mock.patch.object(shade.OpenStackCloud, 'nova_client') def test_delete_flavor(self, mock_nova): mock_nova.flavors.list.return_value = [ - fakes.FakeFlavor('123', 'lemon') + fakes.FakeFlavor('123', 'lemon', 100) ] self.assertTrue(self.op_cloud.delete_flavor('lemon')) mock_nova.flavors.delete.assert_called_once_with(flavor='123') @@ -54,7 +54,7 @@ class TestFlavors(base.TestCase): @mock.patch.object(shade.OpenStackCloud, 'nova_client') def test_delete_flavor_exception(self, mock_nova): mock_nova.flavors.list.return_value = [ - fakes.FakeFlavor('123', 'lemon') + fakes.FakeFlavor('123', 'lemon', 100) ] mock_nova.flavors.delete.side_effect = Exception() self.assertRaises(shade.OpenStackCloudException, diff --git a/shade/tests/unit/test_shade.py b/shade/tests/unit/test_shade.py index 0ed6ba9f1..9cffa1fde 100644 --- a/shade/tests/unit/test_shade.py +++ b/shade/tests/unit/test_shade.py @@ -24,7 +24,6 @@ from os_client_config import cloud_config import shade from shade import _utils from shade import exc -from shade import meta from shade.tests import fakes from shade.tests.unit import base @@ -493,47 +492,21 @@ class TestShade(base.TestCase): @mock.patch.object(shade.OpenStackCloud, 'nova_client') def test_get_flavor_by_ram(self, mock_nova_client): - - class Flavor1(object): - id = '1' - name = 'vanilla ice cream' - ram = 100 - - class Flavor2(object): - id = '2' - name = 'chocolate ice cream' - ram = 200 - - vanilla = meta.obj_to_dict(Flavor1()) - chocolate = meta.obj_to_dict(Flavor2()) + vanilla = fakes.FakeFlavor('1', 'vanilla ice cream', 100) + chocolate = fakes.FakeFlavor('1', 'chocolate ice cream', 200) mock_nova_client.flavors.list.return_value = [vanilla, chocolate] flavor = self.cloud.get_flavor_by_ram(ram=150) - self.assertEquals(chocolate, flavor) + self.assertEquals(chocolate.id, flavor['id']) @mock.patch.object(shade.OpenStackCloud, 'nova_client') def test_get_flavor_by_ram_and_include(self, mock_nova_client): - class Flavor1(object): - id = '1' - name = 'vanilla ice cream' - ram = 100 - - class Flavor2(object): - id = '2' - name = 'chocolate ice cream' - ram = 200 - - class Flavor3(object): - id = '3' - name = 'strawberry ice cream' - ram = 250 - - vanilla = meta.obj_to_dict(Flavor1()) - chocolate = meta.obj_to_dict(Flavor2()) - strawberry = meta.obj_to_dict(Flavor3()) + vanilla = fakes.FakeFlavor('1', 'vanilla ice cream', 100) + chocolate = fakes.FakeFlavor('2', 'chocoliate ice cream', 200) + strawberry = fakes.FakeFlavor('3', 'strawberry ice cream', 250) mock_nova_client.flavors.list.return_value = [ vanilla, chocolate, strawberry] flavor = self.cloud.get_flavor_by_ram(ram=150, include='strawberry') - self.assertEquals(strawberry, flavor) + self.assertEquals(strawberry.id, flavor['id']) @mock.patch.object(shade.OpenStackCloud, 'nova_client') def test_get_flavor_by_ram_not_found(self, mock_nova_client): @@ -544,17 +517,12 @@ class TestShade(base.TestCase): @mock.patch.object(shade.OpenStackCloud, 'nova_client') def test_get_flavor_string_and_int(self, mock_nova_client): - class Flavor1(object): - id = '1' - name = 'vanilla ice cream' - ram = 100 - - vanilla = meta.obj_to_dict(Flavor1()) + vanilla = fakes.FakeFlavor('1', 'vanilla ice cream', 100) mock_nova_client.flavors.list.return_value = [vanilla] flavor1 = self.cloud.get_flavor('1') - self.assertEquals(vanilla, flavor1) + self.assertEquals(vanilla.id, flavor1['id']) flavor2 = self.cloud.get_flavor(1) - self.assertEquals(vanilla, flavor2) + self.assertEquals(vanilla.id, flavor2['id']) def test__neutron_exceptions_resource_not_found(self): with mock.patch.object(