Skip to content
This repository has been archived by the owner on Feb 29, 2024. It is now read-only.

Commit

Permalink
Use swift bulk-delete when deleting a plan
Browse files Browse the repository at this point in the history
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
  • Loading branch information
dprince authored and bcrochet committed Aug 29, 2018
1 parent d82e9ed commit f689182
Show file tree
Hide file tree
Showing 6 changed files with 35 additions and 57 deletions.
3 changes: 2 additions & 1 deletion tripleo_common/actions/config.py
Expand Up @@ -52,6 +52,7 @@ def __init__(self, container=constants.DEFAULT_CONTAINER_NAME,
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
Expand All @@ -61,7 +62,7 @@ def run(self, context):
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
Expand Down
8 changes: 4 additions & 4 deletions tripleo_common/actions/plan.py
Expand Up @@ -119,11 +119,11 @@ def run(self, context):
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.")
Expand Down
5 changes: 4 additions & 1 deletion tripleo_common/tests/actions/test_config.py
Expand Up @@ -57,13 +57,16 @@ def setUp(self,):

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')
Expand Down
20 changes: 11 additions & 9 deletions tripleo_common/tests/actions/test_plan.py
Expand Up @@ -264,10 +264,12 @@ def test_run_stack_exists(self, get_orchestration_client):
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()
Expand All @@ -283,9 +285,14 @@ def test_run(self, get_orchestration_client, get_obj_client_mock):
{'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(
Expand All @@ -296,16 +303,11 @@ def test_run(self, get_orchestration_client, get_obj_client_mock):
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):

Expand Down
26 changes: 9 additions & 17 deletions tripleo_common/tests/utils/test_swift.py
Expand Up @@ -37,29 +37,21 @@ def setUp(self):
]
)

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()
30 changes: 5 additions & 25 deletions tripleo_common/utils/swift.py
Expand Up @@ -18,7 +18,6 @@
import os
import tempfile

import six
from swiftclient.service import SwiftError
from swiftclient.service import SwiftUploadObject

Expand All @@ -27,30 +26,11 @@
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):
Expand Down

0 comments on commit f689182

Please sign in to comment.