Bug fix: Make set/unset of flavor specs work again

The 1.2.0 release broke the API methods to set and unset
flavor extra specs because we need the raw object, as returned
from the nova client, to make method calls to change those
values. As a result, a new 'raw' parameter was added to the
submitTask() method of TaskManager to allow us to get these
raw objects back.

Additionally, we were never displaying the 'extra_specs' for
a flavor. This, too, requires a raw object method call.

Flavors are now normalized to remove client cruft and make sure
that 'extra_specs' is always an attribute.

As if that weren't enough, we now do functional tests for these
things! What more could one ask for???

Change-Id: Ie5c132317392cf26df2c8f43e9f07d040119eca0

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.

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.

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):

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(

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)

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):

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(
+        self.new_item_name = self.getUniqueString('flavor')
-            random.choice(string.ascii_lowercase) for _ in range(5))
 
         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'])

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 = fakes.FakeFlavor('555', 'vanilla', 100)
-        fake_flavor_dict = meta.obj_to_dict(fake_flavor)
+        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())

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,

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):
-
+        vanilla = fakes.FakeFlavor('1', 'vanilla ice cream', 100)
-        class Flavor1(object):
+        chocolate = fakes.FakeFlavor('1', 'chocolate ice cream', 200)
-            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())
         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):
+        vanilla = fakes.FakeFlavor('1', 'vanilla ice cream', 100)
-            id = '1'
+        chocolate = fakes.FakeFlavor('2', 'chocoliate ice cream', 200)
-            name = 'vanilla ice cream'
+        strawberry = fakes.FakeFlavor('3', 'strawberry ice cream', 250)
-            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())
         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):
+        vanilla = fakes.FakeFlavor('1', 'vanilla ice cream', 100)
-            id = '1'
-            name = 'vanilla ice cream'
-            ram = 100
-
-        vanilla = meta.obj_to_dict(Flavor1())
         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(