Fix cleanup for object_storage tests

This reverts change I98a75cbf119ba8126253a681c046f4cf44b1607e.

Volume backup tests create a container when Swift is used as a backup
driver. This causes a failure of an object storage test [1] as it
expects no containers being present before the testing.

This patch fixes the cleanup by deleting properly the containers
created during the volume backup tests. Before the deletion we check
using a new tempest.conf option whether swift is used as a backup
driver.

This patche also un-skips the
test_volume_backup_create_get_detailed_list_restore_delete test as the
patch ensures that we do not use the container parameter in the API
call when Swift is not used as a backup driver.

[1] https://opendev.org/openstack/tempest/src/tempest/api/object_storage/test_account_services.py#L67

Closes-Bug: #2034913
Change-Id: I33ba1838bf0bfcf94424e7288249dae3feeeb2a2

releasenotes/notes/backup-driver-config-option-5f12e71c75b7f01b.yaml

@@ -0,0 +1,7 @@
+---
+features:
+  - |
+    A new tempest.conf option called backup_driver was added to tempest to
+    indicate which backup driver is used by Cinder. This option allows us
+    to create a resource properly and avoid cleanup issues when Swift is used as the
+    backup driver.

tempest/api/object_storage/base.py

@@ -18,6 +18,7 @@ import time
 from oslo_log import log
 
 from tempest.common import custom_matchers
+from tempest.common import object_storage
 from tempest.common import waiters
 from tempest import config
 from tempest.lib.common.utils import data_utils
@@ -28,51 +29,6 @@ CONF = config.CONF
 LOG = log.getLogger(__name__)
 
 
-def delete_containers(containers, container_client, object_client):
-    """Remove containers and all objects in them.
-
-    The containers should be visible from the container_client given.
-    Will not throw any error if the containers don't exist.
-
-    :param containers: List of containers(or string of a container)
-                       to be deleted
-    :param container_client: Client to be used to delete containers
-    :param object_client: Client to be used to delete objects
-    """
-    if isinstance(containers, str):
-        containers = [containers]
-
-    for cont in containers:
-        try:
-            delete_objects(cont, container_client, object_client)
-            container_client.delete_container(cont)
-            container_client.wait_for_resource_deletion(cont)
-        except lib_exc.NotFound:
-            LOG.warning(f"Container {cont} wasn't deleted as it wasn't found.")
-
-
-def delete_objects(container, container_client, object_client):
-    """Remove all objects from container.
-
-    Will not throw any error if the objects do not exist
-
-    :param container: Name of the container that contains the objects to be
-                      deleted
-    :param container_client: Client to be used to list objects in
-                             the container
-    :param object_client: Client to be used to delete objects
-    """
-    params = {'limit': 9999, 'format': 'json'}
-    _, objlist = container_client.list_container_objects(container, params)
-
-    for obj in objlist:
-        try:
-            object_client.delete_object(container, obj['name'])
-            object_client.wait_for_resource_deletion(obj['name'], container)
-        except lib_exc.NotFound:
-            LOG.warning(f"Object {obj} wasn't deleted as it wasn't found.")
-
-
 class BaseObjectTest(tempest.test.BaseTestCase):
 
     credentials = [['operator', CONF.object_storage.operator_role]]
@@ -160,7 +116,8 @@ class BaseObjectTest(tempest.test.BaseTestCase):
             container_client = cls.container_client
         if object_client is None:
             object_client = cls.object_client
-        delete_containers(cls.containers, container_client, object_client)
+        object_storage.delete_containers(cls.containers, container_client,
+                                         object_client)
 
     def assertHeaders(self, resp, target, method):
         """Check the existence and the format of response headers"""

tempest/api/object_storage/test_account_bulk.py

@@ -16,6 +16,7 @@ import tarfile
 import tempfile
 
 from tempest.api.object_storage import base
+from tempest.common import object_storage
 from tempest.common import utils
 from tempest.lib import decorators
 
@@ -30,7 +31,8 @@ class BulkTest(base.BaseObjectTest):
     def tearDown(self):
         # NOTE(andreaf) BulkTests needs to cleanup containers after each
         # test is executed.
-        base.delete_containers(self.containers, self.container_client,
+        object_storage.delete_containers(self.containers,
+                                         self.container_client,
                                          self.object_client)
         super(BulkTest, self).tearDown()
 

tempest/api/object_storage/test_account_services.py

@@ -18,6 +18,7 @@ import testtools
 
 from tempest.api.object_storage import base
 from tempest.common import custom_matchers
+from tempest.common import object_storage
 from tempest import config
 from tempest.lib.common.utils import data_utils
 from tempest.lib import decorators
@@ -43,7 +44,7 @@ class AccountTest(base.BaseObjectTest):
         for i in range(ord('a'), ord('f') + 1):
             name = data_utils.rand_name(name='%s-' % bytes((i,)))
             cls.container_client.update_container(name)
-            cls.addClassResourceCleanup(base.delete_containers,
+            cls.addClassResourceCleanup(object_storage.delete_containers,
                                         [name],
                                         cls.container_client,
                                         cls.object_client)

tempest/api/object_storage/test_container_sync.py

@@ -19,6 +19,7 @@ from urllib import parse as urlparse
 import testtools
 
 from tempest.api.object_storage import base
+from tempest.common import object_storage
 from tempest import config
 from tempest.lib.common.utils import data_utils
 from tempest.lib import decorators
@@ -74,7 +75,7 @@ class ContainerSyncTest(base.BaseObjectTest):
             (cls.container_client_alt, cls.object_client_alt)
         for cont_name, client in cls.clients.items():
             client[0].create_container(cont_name)
-            cls.addClassResourceCleanup(base.delete_containers,
+            cls.addClassResourceCleanup(object_storage.delete_containers,
                                         cont_name,
                                         client[0],
                                         client[1])

tempest/api/object_storage/test_object_version.py

@@ -16,6 +16,7 @@
 import testtools
 
 from tempest.api.object_storage import base
+from tempest.common import object_storage
 from tempest import config
 from tempest.lib.common.utils import data_utils
 from tempest.lib import decorators
@@ -53,7 +54,7 @@ class ContainerTest(base.BaseObjectTest):
         # create container
         vers_container_name = data_utils.rand_name(name='TestVersionContainer')
         resp, _ = self.container_client.update_container(vers_container_name)
-        self.addCleanup(base.delete_containers,
+        self.addCleanup(object_storage.delete_containers,
                         [vers_container_name],
                         self.container_client,
                         self.object_client)
@@ -65,7 +66,7 @@ class ContainerTest(base.BaseObjectTest):
         resp, _ = self.container_client.update_container(
             base_container_name,
             **headers)
-        self.addCleanup(base.delete_containers,
+        self.addCleanup(object_storage.delete_containers,
                         [base_container_name],
                         self.container_client,
                         self.object_client)

tempest/api/volume/base.py

@@ -14,6 +14,7 @@
 #    under the License.
 
 from tempest.common import compute
+from tempest.common import object_storage
 from tempest.common import waiters
 from tempest import config
 from tempest.lib.common import api_version_utils
@@ -68,6 +69,8 @@ class BaseVolumeTest(api_version_utils.BaseMicroversionTest,
         if CONF.service_available.glance:
             cls.images_client = cls.os_primary.image_client_v2
 
+        cls.container_client = cls.os_primary.container_client
+        cls.object_client = cls.os_primary.object_client
         cls.backups_client = cls.os_primary.backups_client_latest
         cls.volumes_client = cls.os_primary.volumes_client_latest
         cls.messages_client = cls.os_primary.volume_messages_client_latest
@@ -174,16 +177,32 @@ class BaseVolumeTest(api_version_utils.BaseMicroversionTest,
                                                 snapshot['id'], 'available')
         return snapshot
 
-    def create_backup(self, volume_id, backup_client=None, **kwargs):
+    def create_backup(self, volume_id, backup_client=None, object_client=None,
+                      container_client=None, **kwargs):
         """Wrapper utility that returns a test backup."""
         if backup_client is None:
             backup_client = self.backups_client
+        if container_client is None:
+            container_client = self.container_client
+        if object_client is None:
+            object_client = self.object_client
         if 'name' not in kwargs:
             name = data_utils.rand_name(self.__class__.__name__ + '-Backup')
             kwargs['name'] = name
 
+        if CONF.volume.backup_driver == "swift":
+            if 'container' not in kwargs:
+                cont_name = self.__class__.__name__ + '-backup-container'
+                cont = data_utils.rand_name(cont_name)
+                kwargs['container'] = cont
+
+            self.addCleanup(object_storage.delete_containers,
+                            kwargs['container'], container_client,
+                            object_client)
+
         backup = backup_client.create_backup(
             volume_id=volume_id, **kwargs)['backup']
+
         # addCleanup uses list pop to cleanup. Wait should be added before
         # the backup is deleted
         self.addCleanup(backup_client.wait_for_resource_deletion,

tempest/api/volume/test_volumes_backup.py

@@ -13,7 +13,6 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
-import testtools
 from testtools import matchers
 
 from tempest.api.volume import base
@@ -53,8 +52,6 @@ class VolumesBackupsTest(base.BaseVolumeTest):
                                                 'available')
         return restored_volume
 
-    @testtools.skipIf(CONF.volume.storage_protocol == 'ceph',
-                      'ceph does not support arbitrary container names')
     @decorators.idempotent_id('a66eb488-8ee1-47d4-8e9f-575a095728c6')
     def test_volume_backup_create_get_detailed_list_restore_delete(self):
         """Test create/get/list/restore/delete volume backup
@@ -75,22 +72,24 @@ class VolumesBackupsTest(base.BaseVolumeTest):
         self.addCleanup(self.delete_volume, self.volumes_client, volume['id'])
 
         # Create a backup
-        backup_name = data_utils.rand_name(
+        kwargs = {}
+        kwargs["name"] = data_utils.rand_name(
             self.__class__.__name__ + '-Backup')
-        description = data_utils.rand_name("volume-backup-description")
+        kwargs["description"] = data_utils.rand_name("backup-description")
-        backup = self.create_backup(volume_id=volume['id'],
+        if CONF.volume.backup_driver == "swift":
-                                    name=backup_name,
+            kwargs["container"] = data_utils.rand_name(
-                                    description=description,
+                self.__class__.__name__ + '-Backup-container')
-                                    container='container')
+        backup = self.create_backup(volume_id=volume['id'], **kwargs)
-        self.assertEqual(backup_name, backup['name'])
+        self.assertEqual(kwargs["name"], backup['name'])
         waiters.wait_for_volume_resource_status(self.volumes_client,
                                                 volume['id'], 'available')
 
         # Get a given backup
         backup = self.backups_client.show_backup(backup['id'])['backup']
-        self.assertEqual(backup_name, backup['name'])
+        self.assertEqual(kwargs["name"], backup['name'])
-        self.assertEqual(description, backup['description'])
+        self.assertEqual(kwargs["description"], backup['description'])
-        self.assertEqual('container', backup['container'])
+        if CONF.volume.backup_driver == "swift":
+            self.assertEqual(kwargs["container"], backup['container'])
 
         # Get all backups with detail
         backups = self.backups_client.list_backups(detail=True)['backups']

tempest/common/object_storage.py

@@ -0,0 +1,64 @@
+#    Licensed under the Apache License, Version 2.0 (the "License"); you may
+#    not use this file except in compliance with the License. You may obtain
+#    a copy of the License at
+#
+#         http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#    License for the specific language governing permissions and limitations
+#    under the License.
+
+from oslo_log import log
+
+from tempest import config
+from tempest.lib import exceptions as lib_exc
+
+CONF = config.CONF
+LOG = log.getLogger(__name__)
+
+
+def delete_containers(containers, container_client, object_client):
+    """Remove containers and all objects in them.
+
+    The containers should be visible from the container_client given.
+    Will not throw any error if the containers don't exist.
+
+    :param containers: List of containers(or string of a container)
+                       to be deleted
+    :param container_client: Client to be used to delete containers
+    :param object_client: Client to be used to delete objects
+    """
+    if isinstance(containers, str):
+        containers = [containers]
+
+    for cont in containers:
+        try:
+            delete_objects(cont, container_client, object_client)
+            container_client.delete_container(cont)
+            container_client.wait_for_resource_deletion(cont)
+        except lib_exc.NotFound:
+            LOG.warning(f"Container {cont} wasn't deleted as it wasn't found.")
+
+
+def delete_objects(container, container_client, object_client):
+    """Remove all objects from container.
+
+    Will not throw any error if the objects do not exist
+
+    :param container: Name of the container that contains the objects to be
+                      deleted
+    :param container_client: Client to be used to list objects in
+                             the container
+    :param object_client: Client to be used to delete objects
+    """
+    params = {'limit': 9999, 'format': 'json'}
+    _, objlist = container_client.list_container_objects(container, params)
+
+    for obj in objlist:
+        try:
+            object_client.delete_object(container, obj['name'])
+            object_client.wait_for_resource_deletion(obj['name'], container)
+        except lib_exc.NotFound:
+            LOG.warning(f"Object {obj} wasn't deleted as it wasn't found.")

tempest/config.py

@@ -1005,6 +1005,13 @@ VolumeGroup = [
                choices=['public', 'admin', 'internal',
                         'publicURL', 'adminURL', 'internalURL'],
                help="The endpoint type to use for the volume service."),
+    cfg.StrOpt('backup_driver',
+               default='ceph',
+               choices=['ceph', 'swift', 'nfs', 'glusterfs', 'posix', 'google',
+                        's3'],
+               help="What kind of backup_driver does cinder use?"
+                    "https://docs.openstack.org/cinder/latest/configuration/"
+                    "block-storage/backup-drivers.html"),
     cfg.ListOpt('backend_names',
                 default=['BACKEND_1', 'BACKEND_2'],
                 help='A list of backend names separated by comma. '