From f6891826c9ccaaca0614239d1f162a3f66a43162 Mon Sep 17 00:00:00 2001 From: Dan Prince Date: Thu, 15 Mar 2018 17:31:51 -0400 Subject: [PATCH] Use swift bulk-delete when deleting a plan Deleting swift containers (plans) with large numbers of files can take longer than we'd like. This patch uses the SwiftService API which contains an implementation of bulk delete used by the swift CLI which speeds up delete operations. Change-Id: I8362a7986e2e4ff0aecaf867faf47c24b658d15b Closes-bug: #1615825 --- tripleo_common/actions/config.py | 3 ++- tripleo_common/actions/plan.py | 8 +++--- tripleo_common/tests/actions/test_config.py | 5 +++- tripleo_common/tests/actions/test_plan.py | 20 +++++++------- tripleo_common/tests/utils/test_swift.py | 26 +++++++----------- tripleo_common/utils/swift.py | 30 ++++----------------- 6 files changed, 35 insertions(+), 57 deletions(-) diff --git a/tripleo_common/actions/config.py b/tripleo_common/actions/config.py index 9481c3ec1..e1ef4d594 100644 --- a/tripleo_common/actions/config.py +++ b/tripleo_common/actions/config.py @@ -52,6 +52,7 @@ class GetOvercloudConfig(templates.ProcessTemplatesAction): def run(self, context): heat = self.get_orchestration_client(context) swift = self.get_object_client(context) + swiftservice = self.get_object_service(context) # Since the config-download directory is now a git repo, first download # the existing config container if it exists so we can reuse the @@ -61,7 +62,7 @@ class GetOvercloudConfig(templates.ProcessTemplatesAction): self.config_dir) # Delete the existing container before we re-upload, otherwise # files may not be fully overwritten. - swiftutils.delete_container(swift, self.container_config) + swiftutils.delete_container(swiftservice, self.container_config) except swiftexceptions.ClientException as err: if err.http_status != 404: raise diff --git a/tripleo_common/actions/plan.py b/tripleo_common/actions/plan.py index c9a41d04d..53de78806 100644 --- a/tripleo_common/actions/plan.py +++ b/tripleo_common/actions/plan.py @@ -119,11 +119,11 @@ class DeletePlanAction(base.TripleOAction): raise exception.StackInUseError(name=self.container) try: - swift = self.get_object_client(context) - swiftutils.delete_container(swift, self.container) - swiftutils.delete_container(swift, + swift_service = self.get_object_service(context) + swiftutils.delete_container(swift_service, self.container) + swiftutils.delete_container(swift_service, "%s-swift-rings" % self.container) - swiftutils.delete_container(swift, + swiftutils.delete_container(swift_service, "%s-messages" % self.container) except swiftexceptions.ClientException as ce: LOG.exception("Swift error deleting plan.") diff --git a/tripleo_common/tests/actions/test_config.py b/tripleo_common/tests/actions/test_config.py index 80cb998e7..ee3eab8bc 100644 --- a/tripleo_common/tests/actions/test_config.py +++ b/tripleo_common/tests/actions/test_config.py @@ -57,13 +57,16 @@ class GetOvercloudConfigActionTest(base.TestCase): self.ctx = mock.MagicMock() + @mock.patch('tripleo_common.actions.base.TripleOAction.' + 'get_object_service') @mock.patch('tripleo_common.actions.base.TripleOAction.' 'get_orchestration_client') @mock.patch('tripleo_common.utils.config.Config.download_config') @mock.patch('tripleo_common.utils.tarball.create_tarball') def test_run(self, mock_create_tarball, mock_config, - mock_orchestration_client): + mock_orchestration_client, + mock_object_service): heat = mock.MagicMock() heat.stacks.get.return_value = mock.MagicMock( stack_name='stack', id='stack_id') diff --git a/tripleo_common/tests/actions/test_plan.py b/tripleo_common/tests/actions/test_plan.py index fe79df438..5ef04ba99 100644 --- a/tripleo_common/tests/actions/test_plan.py +++ b/tripleo_common/tests/actions/test_plan.py @@ -264,10 +264,12 @@ class DeletePlanActionTest(base.TestCase): self.assertRaises(exception.StackInUseError, action.run, self.ctx) heat.stacks.get.assert_called_with(self.container_name) + @mock.patch('tripleo_common.actions.base.TripleOAction.get_object_service') @mock.patch('tripleo_common.actions.base.TripleOAction.get_object_client') @mock.patch( 'tripleo_common.actions.base.TripleOAction.get_orchestration_client') - def test_run(self, get_orchestration_client, get_obj_client_mock): + def test_run(self, get_orchestration_client, get_obj_client_mock, + get_obj_service_mock): # setup swift swift = mock.MagicMock() @@ -283,9 +285,14 @@ class DeletePlanActionTest(base.TestCase): {'name': 'finally-another-name.yaml'} ] ) - get_obj_client_mock.return_value = swift + swift_service = mock.MagicMock() + swift_service.delete.return_value = ([ + {'success': True}, + ]) + get_obj_service_mock.return_value = swift_service + # setup heat heat = mock.MagicMock() heat.stacks.get = mock.Mock( @@ -296,16 +303,11 @@ class DeletePlanActionTest(base.TestCase): action.run(self.ctx) mock_calls = [ - mock.call('overcloud', 'some-name.yaml'), - mock.call('overcloud', 'some-other-name.yaml'), - mock.call('overcloud', 'yet-some-other-name.yaml'), - mock.call('overcloud', 'finally-another-name.yaml') + mock.call(container='overcloud'), ] - swift.delete_object.assert_has_calls( + swift_service.delete.assert_has_calls( mock_calls, any_order=True) - swift.delete_container.assert_called_with(self.container_name) - class ListRolesActionTest(base.TestCase): diff --git a/tripleo_common/tests/utils/test_swift.py b/tripleo_common/tests/utils/test_swift.py index cd268cf85..19a7326df 100644 --- a/tripleo_common/tests/utils/test_swift.py +++ b/tripleo_common/tests/utils/test_swift.py @@ -37,29 +37,21 @@ class SwiftTest(base.TestCase): ] ) + self.swiftservice = mock.MagicMock() + self.swiftservice.delete.return_value = ([ + {'success': True}, + ]) + def test_delete_container_success(self): - swift_utils.empty_container(self.swiftclient, self.container_name) + swift_utils.delete_container(self.swiftservice, + self.container_name) mock_calls = [ - mock.call('overcloud', 'some-name.yaml'), - mock.call('overcloud', 'some-other-name.yaml'), - mock.call('overcloud', 'yet-some-other-name.yaml'), - mock.call('overcloud', 'finally-another-name.yaml') + mock.call(container=self.container_name), ] - self.swiftclient.delete_object.assert_has_calls( + self.swiftservice.delete.assert_has_calls( mock_calls, any_order=True) - self.swiftclient.get_account.assert_called() - self.swiftclient.get_container.assert_called_with(self.container_name) - - def test_delete_container_not_found(self): - self.assertRaises(ValueError, - swift_utils.empty_container, - self.swiftclient, 'idontexist') - self.swiftclient.get_account.assert_called() - self.swiftclient.get_container.assert_not_called() - self.swiftclient.delete_object.assert_not_called() - def test_create_container(self): swift_utils.create_container(self.swiftclient, 'abc') self.swiftclient.put_container.assert_called() diff --git a/tripleo_common/utils/swift.py b/tripleo_common/utils/swift.py index 9dc640dcd..edbfe6897 100644 --- a/tripleo_common/utils/swift.py +++ b/tripleo_common/utils/swift.py @@ -18,7 +18,6 @@ import logging import os import tempfile -import six from swiftclient.service import SwiftError from swiftclient.service import SwiftUploadObject @@ -27,30 +26,11 @@ from tripleo_common.utils import tarball LOG = logging.getLogger(__name__) -def empty_container(swiftclient, name): - container_names = [container["name"] for container - in swiftclient.get_account()[1]] - - if name in container_names: - headers, objects = swiftclient.get_container(name) - # FIXME(rbrady): remove delete_object loop when - # LP#1615830 is fixed. See LP#1615825 for more info. - # delete files from plan - for o in objects: - swiftclient.delete_object(name, o['name']) - else: - error_text = "The {name} container does not exist.".format(name=name) - raise ValueError(error_text) - - -def delete_container(swiftclient, name): - try: - empty_container(swiftclient, name) - swiftclient.delete_container(name) - except ValueError as e: - # ValueError is raised when we can't find the container, which means - # that it's already deleted. - LOG.info(six.text_type(e)) +def delete_container(swiftservice, name): + delres = swiftservice.delete(container=name) + if delres is None: + # A None return means the container didn't exist + LOG.info('Container %s not found', name) def download_container(swiftclient, container, dest):