From 553d91b51e12be77e5d6d38d666cd0b23b12e626 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luk=C3=A1=C5=A1=20Piwowarski?= Date: Thu, 21 Sep 2023 11:14:24 +0200 Subject: [PATCH] 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 --- ...driver-config-option-5f12e71c75b7f01b.yaml | 7 ++ tempest/api/object_storage/base.py | 49 +------------- .../api/object_storage/test_account_bulk.py | 6 +- .../object_storage/test_account_services.py | 3 +- .../api/object_storage/test_container_sync.py | 3 +- .../api/object_storage/test_object_version.py | 5 +- tempest/api/volume/base.py | 21 +++++- tempest/api/volume/test_volumes_backup.py | 25 ++++---- tempest/common/object_storage.py | 64 +++++++++++++++++++ tempest/config.py | 7 ++ 10 files changed, 124 insertions(+), 66 deletions(-) create mode 100644 releasenotes/notes/backup-driver-config-option-5f12e71c75b7f01b.yaml create mode 100644 tempest/common/object_storage.py diff --git a/releasenotes/notes/backup-driver-config-option-5f12e71c75b7f01b.yaml b/releasenotes/notes/backup-driver-config-option-5f12e71c75b7f01b.yaml new file mode 100644 index 0000000000..0126a06997 --- /dev/null +++ b/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. diff --git a/tempest/api/object_storage/base.py b/tempest/api/object_storage/base.py index 58ad9d4eda..8adbe7d791 100644 --- a/tempest/api/object_storage/base.py +++ b/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""" diff --git a/tempest/api/object_storage/test_account_bulk.py b/tempest/api/object_storage/test_account_bulk.py index 687fe5750a..0ecae85789 100644 --- a/tempest/api/object_storage/test_account_bulk.py +++ b/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,8 +31,9 @@ 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, - self.object_client) + object_storage.delete_containers(self.containers, + self.container_client, + self.object_client) super(BulkTest, self).tearDown() def _create_archive(self): diff --git a/tempest/api/object_storage/test_account_services.py b/tempest/api/object_storage/test_account_services.py index 4966ec4017..b7a413e178 100644 --- a/tempest/api/object_storage/test_account_services.py +++ b/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) diff --git a/tempest/api/object_storage/test_container_sync.py b/tempest/api/object_storage/test_container_sync.py index b31ff7654b..9b1d3c7600 100644 --- a/tempest/api/object_storage/test_container_sync.py +++ b/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]) diff --git a/tempest/api/object_storage/test_object_version.py b/tempest/api/object_storage/test_object_version.py index b64b1729dd..2a1f63e5bf 100644 --- a/tempest/api/object_storage/test_object_version.py +++ b/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) diff --git a/tempest/api/volume/base.py b/tempest/api/volume/base.py index ad8f573a33..b9e1e58c37 100644 --- a/tempest/api/volume/base.py +++ b/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, diff --git a/tempest/api/volume/test_volumes_backup.py b/tempest/api/volume/test_volumes_backup.py index 89ff497761..c775292ad7 100644 --- a/tempest/api/volume/test_volumes_backup.py +++ b/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") - backup = self.create_backup(volume_id=volume['id'], - name=backup_name, - description=description, - container='container') - self.assertEqual(backup_name, backup['name']) + kwargs["description"] = data_utils.rand_name("backup-description") + if CONF.volume.backup_driver == "swift": + kwargs["container"] = data_utils.rand_name( + self.__class__.__name__ + '-Backup-container') + backup = self.create_backup(volume_id=volume['id'], **kwargs) + 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(description, backup['description']) - self.assertEqual('container', backup['container']) + self.assertEqual(kwargs["name"], backup['name']) + self.assertEqual(kwargs["description"], backup['description']) + 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'] diff --git a/tempest/common/object_storage.py b/tempest/common/object_storage.py new file mode 100644 index 0000000000..7ffdc428bb --- /dev/null +++ b/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.") diff --git a/tempest/config.py b/tempest/config.py index ee083d8365..7978755aea 100644 --- a/tempest/config.py +++ b/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. '