Merge "Decorate volume.base functions - fix cleanup"

tempest/api/volume/admin/test_group_snapshots.py

@@ -91,9 +91,15 @@ class GroupSnapshotsTest(BaseGroupSnapshotsTest):
         grp = self.create_group(group_type=group_type['id'],
                                 volume_types=[volume_type['id']])
 
-        # Create volume
-        vol = self.create_volume(volume_type=volume_type['id'],
-                                 group_id=grp['id'])
+        # Create volume is instance level, can not be deleted before group.
+        # Volume delete handled by delete_group method, cleanup method.
+        params = {'name': data_utils.rand_name("volume"),
+                  'volume_type': volume_type['id'],
+                  'group_id': grp['id'],
+                  'size': CONF.volume.volume_size}
+        vol = self.volumes_client.create_volume(**params)['volume']
+        waiters.wait_for_volume_resource_status(
+            self.volumes_client, vol['id'], 'available')
 
         # Create group snapshot
         group_snapshot_name = data_utils.rand_name('group_snapshot')
@@ -153,9 +159,15 @@ class GroupSnapshotsTest(BaseGroupSnapshotsTest):
         grp = self.create_group(group_type=group_type['id'],
                                 volume_types=[volume_type['id']])
 
-        # Create volume
-        vol = self.create_volume(volume_type=volume_type['id'],
-                                 group_id=grp['id'])
+        # Create volume is instance level, can not be deleted before group.
+        # Volume delete handled by delete_group method, cleanup method.
+        params = {'name': data_utils.rand_name("volume"),
+                  'volume_type': volume_type['id'],
+                  'group_id': grp['id'],
+                  'size': CONF.volume.volume_size}
+        vol = self.volumes_client.create_volume(**params)['volume']
+        waiters.wait_for_volume_resource_status(
+            self.volumes_client, vol['id'], 'available')
 
         # Create group_snapshot
         group_snapshot_name = data_utils.rand_name('group_snapshot')
@@ -215,8 +227,15 @@ class GroupSnapshotsTest(BaseGroupSnapshotsTest):
         # volume-type and group id.
         volume_list = []
         for _ in range(2):
-            volume = self.create_volume(volume_type=volume_type['id'],
-                                        group_id=grp['id'])
+            # Create volume is instance level, can't be deleted before group.
+            # Volume delete handled by delete_group method, cleanup method.
+            params = {'name': data_utils.rand_name("volume"),
+                      'volume_type': volume_type['id'],
+                      'group_id': grp['id'],
+                      'size': CONF.volume.volume_size}
+            volume = self.volumes_client.create_volume(**params)['volume']
+            waiters.wait_for_volume_resource_status(
+                self.volumes_client, volume['id'], 'available')
             volume_list.append(volume['id'])
 
         for vol in volume_list:
@@ -268,9 +287,15 @@ class GroupSnapshotsV319Test(BaseGroupSnapshotsTest):
         group = self.create_group(group_type=group_type['id'],
                                   volume_types=[volume_type['id']])
 
-        # Create volume
-        volume = self.create_volume(volume_type=volume_type['id'],
-                                    group_id=group['id'])
+        # Create volume is instance level, can not be deleted before group.
+        # Volume delete handled by delete_group method, cleanup method.
+        params = {'name': data_utils.rand_name("volume"),
+                  'volume_type': volume_type['id'],
+                  'group_id': group['id'],
+                  'size': CONF.volume.volume_size}
+        volume = self.volumes_client.create_volume(**params)['volume']
+        waiters.wait_for_volume_resource_status(
+            self.volumes_client, volume['id'], 'available')
 
         # Create group snapshot
         group_snapshot = self._create_group_snapshot(group_id=group['id'])

tempest/api/volume/admin/test_groups.py

@@ -108,11 +108,17 @@ class GroupsTest(base.BaseVolumeAdminTest):
         grp = self.create_group(group_type=group_type['id'],
                                 volume_types=[volume_type['id']])
 
-        # Create volumes
+        # Create volume is instance level, can not be deleted before group.
+        # Volume delete handled by delete_group method, cleanup method.
         grp_vols = []
         for _ in range(2):
-            vol = self.create_volume(volume_type=volume_type['id'],
-                                     group_id=grp['id'])
+            params = {'name': data_utils.rand_name("volume"),
+                      'volume_type': volume_type['id'],
+                      'group_id': grp['id'],
+                      'size': CONF.volume.volume_size}
+            vol = self.volumes_client.create_volume(**params)['volume']
+            waiters.wait_for_volume_resource_status(
+                self.volumes_client, vol['id'], 'available')
             grp_vols.append(vol)
         vol2 = grp_vols[1]
 
@@ -171,8 +177,15 @@ class GroupsV314Test(base.BaseVolumeAdminTest):
         grp = self.create_group(group_type=group_type['id'],
                                 volume_types=[volume_type['id']])
 
-        # Create volume
-        self.create_volume(volume_type=volume_type['id'], group_id=grp['id'])
+        # Create volume is instance level, can not be deleted before group.
+        # Volume delete handled by delete_group method, cleanup method.
+        params = {'name': data_utils.rand_name("volume"),
+                  'volume_type': volume_type['id'],
+                  'group_id': grp['id'],
+                  'size': CONF.volume.volume_size}
+        vol = self.volumes_client.create_volume(**params)['volume']
+        waiters.wait_for_volume_resource_status(
+            self.volumes_client, vol['id'], 'available')
 
         # Create Group from Group
         grp_name2 = data_utils.rand_name('Group_from_grp')

tempest/api/volume/base.py

@@ -19,6 +19,7 @@ from tempest import config
 from tempest.lib.common import api_version_utils
 from tempest.lib.common.utils import data_utils
 from tempest.lib.common.utils import test_utils
+from tempest.lib.decorators import cleanup_order
 import tempest.test
 
 CONF = config.CONF
@@ -94,8 +95,8 @@ class BaseVolumeTest(api_version_utils.BaseMicroversionTest,
         cls.build_interval = CONF.volume.build_interval
         cls.build_timeout = CONF.volume.build_timeout
 
-    @classmethod
-    def create_volume(cls, wait_until='available', **kwargs):
+    @cleanup_order
+    def create_volume(self, wait_until='available', **kwargs):
         """Wrapper utility that returns a test volume.
 
            :param wait_until: wait till volume status, None means no wait.
@@ -104,12 +105,12 @@ class BaseVolumeTest(api_version_utils.BaseMicroversionTest,
             kwargs['size'] = CONF.volume.volume_size
 
         if 'imageRef' in kwargs:
-            image = cls.images_client.show_image(kwargs['imageRef'])
+            image = self.images_client.show_image(kwargs['imageRef'])
             min_disk = image['min_disk']
             kwargs['size'] = max(kwargs['size'], min_disk)
 
         if 'name' not in kwargs:
-            name = data_utils.rand_name(cls.__name__ + '-Volume')
+            name = data_utils.rand_name(self.__name__ + '-Volume')
             kwargs['name'] = name
 
         if CONF.volume.volume_type and 'volume_type' not in kwargs:
@@ -123,27 +124,26 @@ class BaseVolumeTest(api_version_utils.BaseMicroversionTest,
             kwargs.setdefault('availability_zone',
                               CONF.compute.compute_volume_common_az)
 
-        volume = cls.volumes_client.create_volume(**kwargs)['volume']
-        cls.addClassResourceCleanup(test_utils.call_and_ignore_notfound_exc,
-                                    cls.delete_volume, cls.volumes_client,
-                                    volume['id'])
+        volume = self.volumes_client.create_volume(**kwargs)['volume']
+        self.cleanup(test_utils.call_and_ignore_notfound_exc,
+                     self.delete_volume, self.volumes_client, volume['id'])
         if wait_until:
-            waiters.wait_for_volume_resource_status(cls.volumes_client,
+            waiters.wait_for_volume_resource_status(self.volumes_client,
                                                     volume['id'], wait_until)
         return volume
 
-    @classmethod
-    def create_snapshot(cls, volume_id=1, **kwargs):
+    @cleanup_order
+    def create_snapshot(self, volume_id=1, **kwargs):
         """Wrapper utility that returns a test snapshot."""
         if 'name' not in kwargs:
-            name = data_utils.rand_name(cls.__name__ + '-Snapshot')
+            name = data_utils.rand_name(self.__name__ + '-Snapshot')
             kwargs['name'] = name
 
-        snapshot = cls.snapshots_client.create_snapshot(
+        snapshot = self.snapshots_client.create_snapshot(
             volume_id=volume_id, **kwargs)['snapshot']
-        cls.addClassResourceCleanup(test_utils.call_and_ignore_notfound_exc,
-                                    cls.delete_snapshot, snapshot['id'])
-        waiters.wait_for_volume_resource_status(cls.snapshots_client,
+        self.cleanup(test_utils.call_and_ignore_notfound_exc,
+                     self.delete_snapshot, snapshot['id'])
+        waiters.wait_for_volume_resource_status(self.snapshots_client,
                                                 snapshot['id'], 'available')
         return snapshot
 
@@ -175,11 +175,11 @@ class BaseVolumeTest(api_version_utils.BaseMicroversionTest,
         client.delete_volume(volume_id)
         client.wait_for_resource_deletion(volume_id)
 
-    @classmethod
-    def delete_snapshot(cls, snapshot_id, snapshots_client=None):
+    @cleanup_order
+    def delete_snapshot(self, snapshot_id, snapshots_client=None):
         """Delete snapshot by the given client"""
         if snapshots_client is None:
-            snapshots_client = cls.snapshots_client
+            snapshots_client = self.snapshots_client
         snapshots_client.delete_snapshot(snapshot_id)
         snapshots_client.wait_for_resource_deletion(snapshot_id)
 
@@ -278,23 +278,23 @@ class BaseVolumeAdminTest(BaseVolumeTest):
         cls.admin_scheduler_stats_client = \
             cls.os_admin.volume_scheduler_stats_client_latest
 
-    @classmethod
-    def create_test_qos_specs(cls, name=None, consumer=None, **kwargs):
+    @cleanup_order
+    def create_test_qos_specs(self, name=None, consumer=None, **kwargs):
         """create a test Qos-Specs."""
-        name = name or data_utils.rand_name(cls.__name__ + '-QoS')
+        name = name or data_utils.rand_name(self.__name__ + '-QoS')
         consumer = consumer or 'front-end'
-        qos_specs = cls.admin_volume_qos_client.create_qos(
+        qos_specs = self.admin_volume_qos_client.create_qos(
             name=name, consumer=consumer, **kwargs)['qos_specs']
-        cls.addClassResourceCleanup(cls.clear_qos_spec, qos_specs['id'])
+        self.cleanup(self.clear_qos_spec, qos_specs['id'])
         return qos_specs
 
-    @classmethod
-    def create_volume_type(cls, name=None, **kwargs):
+    @cleanup_order
+    def create_volume_type(self, name=None, **kwargs):
         """Create a test volume-type"""
-        name = name or data_utils.rand_name(cls.__name__ + '-volume-type')
-        volume_type = cls.admin_volume_types_client.create_volume_type(
+        name = name or data_utils.rand_name(self.__name__ + '-volume-type')
+        volume_type = self.admin_volume_types_client.create_volume_type(
             name=name, **kwargs)['volume_type']
-        cls.addClassResourceCleanup(cls.clear_volume_type, volume_type['id'])
+        self.cleanup(self.clear_volume_type, volume_type['id'])
         return volume_type
 
     def create_encryption_type(self, type_id=None, provider=None,
@@ -328,19 +328,19 @@ class BaseVolumeAdminTest(BaseVolumeTest):
                         group_type['id'])
         return group_type
 
-    @classmethod
-    def clear_qos_spec(cls, qos_id):
+    @cleanup_order
+    def clear_qos_spec(self, qos_id):
         test_utils.call_and_ignore_notfound_exc(
-            cls.admin_volume_qos_client.delete_qos, qos_id)
+            self.admin_volume_qos_client.delete_qos, qos_id)
 
         test_utils.call_and_ignore_notfound_exc(
-            cls.admin_volume_qos_client.wait_for_resource_deletion, qos_id)
+            self.admin_volume_qos_client.wait_for_resource_deletion, qos_id)
 
-    @classmethod
-    def clear_volume_type(cls, vol_type_id):
+    @cleanup_order
+    def clear_volume_type(self, vol_type_id):
         test_utils.call_and_ignore_notfound_exc(
-            cls.admin_volume_types_client.delete_volume_type, vol_type_id)
+            self.admin_volume_types_client.delete_volume_type, vol_type_id)
 
         test_utils.call_and_ignore_notfound_exc(
-            cls.admin_volume_types_client.wait_for_resource_deletion,
+            self.admin_volume_types_client.wait_for_resource_deletion,
             vol_type_id)

tempest/lib/decorators.py

@@ -13,6 +13,7 @@
 # under the License.
 
 import functools
+from types import MethodType
 import uuid
 
 from oslo_log import log as logging
@@ -189,3 +190,34 @@ def unstable_test(*args, **kwargs):
                     raise e
         return inner
     return decor
+
+
+class cleanup_order:
+    """Descriptor for base create function to cleanup based on caller.
+
+    There are functions created as classmethod and the cleanup
+    was managed by the class with addClassResourceCleanup,
+    In case the function called from a class level (resource_setup) its ok
+    But when it is called from testcase level there is no reson to delete the
+    resource when class tears down.
+
+    The testcase results will not reflect the resources cleanup because test
+    may pass but the class cleanup fails. if the resources were created by
+    testcase its better to let the testcase delete them and report failure
+    part of the testcase
+    """
+
+    def __init__(self, func):
+        self.func = func
+        functools.update_wrapper(self, func)
+
+    def __get__(self, instance, owner):
+        if instance:
+            # instance is the caller
+            instance.cleanup = instance.addCleanup
+            instance.__name__ = owner.__name__
+            return MethodType(self.func, instance)
+        elif owner:
+            # class is the caller
+            owner.cleanup = owner.addClassResourceCleanup
+            return MethodType(self.func, owner)

tempest/tests/lib/test_decorators.py

@@ -21,6 +21,7 @@ import testtools
 from tempest.lib import base as test
 from tempest.lib.common.utils import data_utils
 from tempest.lib import decorators
+from tempest.lib import exceptions
 from tempest.lib import exceptions as lib_exc
 from tempest.tests import base
 
@@ -289,3 +290,109 @@ class TestRelatedBugDecorator(base.TestCase):
 
         with mock.patch.object(decorators.LOG, 'error'):
             self.assertRaises(lib_exc.InvalidParam, test_foo, object())
+
+
+class TestCleanupOrderDecorator(base.TestCase):
+
+    @decorators.cleanup_order
+    def _create_volume(self, raise_exception=False):
+        """Test doc"""
+        vol_id = "487ef6b6-546a-40c7-bc3f-b405d6239fc8"
+        self.cleanup(self._delete_dummy, vol_id)
+        if raise_exception:
+            raise exceptions.NotFound("Not found")
+        return "volume"
+
+    def _delete_dummy(self, vol_id):
+        pass
+
+    class DummyClassResourceCleanup(list):
+        """dummy list class simulate ClassResourceCleanup"""
+
+        def __call__(self, func, vol_id):
+            self.append((func, vol_id))
+
+    @classmethod
+    def resource_setup(cls):
+        cls.addClassResourceCleanup = cls.DummyClassResourceCleanup()
+        cls.volume = cls._create_volume()
+
+    @classmethod
+    def resource_setup_exception(cls):
+        cls.addClassResourceCleanup = cls.DummyClassResourceCleanup()
+        cls.volume = cls._create_volume(raise_exception=True)
+
+    def setUp(self):
+        super().setUp()
+        self.volume_instance = self._create_volume()
+
+    def test_cleanup_order_when_called_from_instance_testcase(self):
+        # create a volume
+        my_vol = self._create_volume()
+        # Verify method runs and return value
+        self.assertEqual(my_vol, "volume")
+        # Verify __doc__ exists from original function
+        self.assertEqual(self._create_volume.__doc__, "Test doc")
+        # New cleanup created and refers to addCleanup
+        self.assertTrue(hasattr(self, "cleanup"))
+        self.assertEqual(self.cleanup, self.addCleanup)
+        # New __name__ created from type(self)
+        self.assertEqual(self.__name__, type(self).__name__)
+        # Verify function added to instance _cleanups
+        self.assertIn(self._delete_dummy, [e[0] for e in self._cleanups])
+
+    def test_cleanup_order_when_called_from_setup_instance(self):
+        # create a volume
+        my_vol = self.volume_instance
+        # Verify method runs and return value
+        self.assertEqual(my_vol, "volume")
+        # Verify __doc__ exists from original function
+        self.assertEqual(self._create_volume.__doc__, "Test doc")
+        # New cleanup created and refers to addCleanup
+        self.assertTrue(hasattr(self, "cleanup"))
+        self.assertEqual(self.cleanup, self.addCleanup)
+        # New __name__ created from type(self)
+        self.assertEqual(self.__name__, type(self).__name__)
+        # Verify function added to instance _cleanups
+        self.assertIn(self._delete_dummy, [e[0] for e in self._cleanups])
+
+    def test_cleanup_order_when_called_from_instance_raise(self):
+        # create a volume when raised exceptions
+        self.assertRaises(exceptions.NotFound, self._create_volume,
+                          raise_exception=True)
+        # before raise exceptions
+        self.assertTrue(hasattr(self, "cleanup"))
+        self.assertEqual(self.cleanup, self.addCleanup)
+        # New __name__ created from type(self)
+        self.assertEqual(self.__name__, type(self).__name__)
+        # Verify function added to instance _cleanups before exception
+        self.assertIn(self._delete_dummy, [e[0] for e in self._cleanups])
+
+    def test_cleanup_order_when_called_from_class_method(self):
+        # call class method
+        type(self).resource_setup()
+        # create a volume
+        my_vol = self.volume
+        # Verify method runs and return value
+        self.assertEqual(my_vol, "volume")
+        # Verify __doc__ exists from original function
+        self.assertEqual(self._create_volume.__doc__, "Test doc")
+        # New cleanup created and refers to addClassResourceCleanup
+        self.assertTrue(hasattr(self, "cleanup"))
+        self.assertEqual(type(self).cleanup, self.addClassResourceCleanup)
+        # Verify function added to instance addClassResourceCleanup
+        self.assertIn(type(self)._delete_dummy,
+                      [e[0] for e in self.addClassResourceCleanup])
+
+    def test_cleanup_order_when_called_from_class_method_raise(self):
+        # call class method
+        self.assertRaises(exceptions.NotFound,
+                          type(self).resource_setup_exception)
+        # Verify __doc__ exists from original function
+        self.assertEqual(self._create_volume.__doc__, "Test doc")
+        # New cleanup created and refers to addClassResourceCleanup
+        self.assertTrue(hasattr(self, "cleanup"))
+        self.assertEqual(type(self).cleanup, self.addClassResourceCleanup)
+        # Verify function added to instance addClassResourceCleanup
+        self.assertIn(type(self)._delete_dummy,
+                      [e[0] for e in self.addClassResourceCleanup])