Decorate volume.base functions - fix cleanup
There are functions set as classmethod while used in testcase
The cleanup done on class level and not part of the testcase
The problem is that we may see testcase pass while the
resource_cleanup fails on classlevel instead of the test fail.
Maintaine and support proper cleanup ordering when functions
are classbase while other instances.
Current fix contains class descriptor that learn caller
in case its a class it will use the class cleanup
in case its an instance it will use instance cleanup.
Expecting to see failures due to wrong cleanup in tests
result from the wrong impelementaions.
Added unittests for cleanup_order decorator
Updated test_groups cleanup , volume can not be deleted
before group, class level cleanup hides it because group
clean was instance level.
See scenario methods as reference (manager)
Change-Id: I27328bf6c176840c7762bd97f596481ffa6f5736
diff --git a/tempest/api/volume/admin/test_group_snapshots.py b/tempest/api/volume/admin/test_group_snapshots.py
index 73903cf..8af8435 100644
--- a/tempest/api/volume/admin/test_group_snapshots.py
+++ b/tempest/api/volume/admin/test_group_snapshots.py
@@ -91,9 +91,15 @@
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 @@
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 @@
# 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 @@
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'])
diff --git a/tempest/api/volume/admin/test_groups.py b/tempest/api/volume/admin/test_groups.py
index f16e4d2..094f142 100644
--- a/tempest/api/volume/admin/test_groups.py
+++ b/tempest/api/volume/admin/test_groups.py
@@ -108,11 +108,17 @@
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 @@
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')
diff --git a/tempest/api/volume/base.py b/tempest/api/volume/base.py
index 172b6ed..49f9e22 100644
--- a/tempest/api/volume/base.py
+++ b/tempest/api/volume/base.py
@@ -19,6 +19,7 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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)
diff --git a/tempest/lib/decorators.py b/tempest/lib/decorators.py
index a4633ca..bb588be 100644
--- a/tempest/lib/decorators.py
+++ b/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 @@
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)
diff --git a/tempest/tests/lib/test_decorators.py b/tempest/tests/lib/test_decorators.py
index fc93f76..f9a12b6 100644
--- a/tempest/tests/lib/test_decorators.py
+++ b/tempest/tests/lib/test_decorators.py
@@ -21,6 +21,7 @@
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 @@
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])