From af7ab03693a5708102cf6746563da289e4c1e3b7 Mon Sep 17 00:00:00 2001 From: Huanxuan Ao Date: Mon, 27 Jun 2016 11:04:05 +0800 Subject: [PATCH] Support bulk deletion for delete commands in computev2 Support bulk deletion and error handling for "keypair delete" and "service delete" commands in computev2. Up to now, all the delete commands in computev2 support bulk deletion. Change-Id: I6d5c960e9716188e56615514d0921618a15a88ec Partially-Implements: blueprint multi-argument-compute Partial-Bug: #1592906 --- .../command-objects/compute-service.rst | 6 +- doc/source/command-objects/keypair.rst | 6 +- openstackclient/compute/v2/keypair.py | 27 +++++++- openstackclient/compute/v2/service.py | 20 +++++- openstackclient/tests/compute/v2/fakes.py | 19 ++++++ .../tests/compute/v2/test_keypair.py | 62 +++++++++++++++++-- .../tests/compute/v2/test_service.py | 55 ++++++++++++++-- ...lti-argument-compute-0bc4522f6edca355.yaml | 4 +- 8 files changed, 174 insertions(+), 25 deletions(-) diff --git a/doc/source/command-objects/compute-service.rst b/doc/source/command-objects/compute-service.rst index f43b5fa636..d886533e54 100644 --- a/doc/source/command-objects/compute-service.rst +++ b/doc/source/command-objects/compute-service.rst @@ -7,18 +7,18 @@ Compute v2 compute service delete ---------------------- -Delete service command +Delete compute service(s) .. program:: compute service delete .. code:: bash os compute service delete - + [ ...] .. _compute-service-delete: .. describe:: - Compute service to delete (ID only) + Compute service(s) to delete (ID only) compute service list -------------------- diff --git a/doc/source/command-objects/keypair.rst b/doc/source/command-objects/keypair.rst index 6638b8c93a..64cc20cd4c 100644 --- a/doc/source/command-objects/keypair.rst +++ b/doc/source/command-objects/keypair.rst @@ -30,17 +30,17 @@ Create new public key keypair delete -------------- -Delete public key +Delete public key(s) .. program:: keypair delete .. code:: bash os keypair delete - + [ ...] .. describe:: - Public key to delete (name only) + Public key(s) to delete (name only) keypair list ------------ diff --git a/openstackclient/compute/v2/keypair.py b/openstackclient/compute/v2/keypair.py index 13c0a99ca1..3725a3a889 100644 --- a/openstackclient/compute/v2/keypair.py +++ b/openstackclient/compute/v2/keypair.py @@ -16,6 +16,7 @@ """Keypair action implementations""" import io +import logging import os import sys @@ -27,6 +28,9 @@ import six from openstackclient.i18n import _ +LOG = logging.getLogger(__name__) + + class CreateKeypair(command.ShowOne): """Create new public key""" @@ -78,20 +82,37 @@ class CreateKeypair(command.ShowOne): class DeleteKeypair(command.Command): - """Delete public key""" + """Delete public key(s)""" def get_parser(self, prog_name): parser = super(DeleteKeypair, self).get_parser(prog_name) parser.add_argument( 'name', metavar='', - help=_("Public key to delete (name only)") + nargs='+', + help=_("Public key(s) to delete (name only)") ) return parser def take_action(self, parsed_args): compute_client = self.app.client_manager.compute - compute_client.keypairs.delete(parsed_args.name) + result = 0 + for n in parsed_args.name: + try: + data = utils.find_resource( + compute_client.keypairs, n) + compute_client.keypairs.delete(data.name) + except Exception as e: + result += 1 + LOG.error(_("Failed to delete public key with name " + "'%(name)s': %(e)s") + % {'name': n, 'e': e}) + + if result > 0: + total = len(parsed_args.name) + msg = (_("%(result)s of %(total)s public keys failed " + "to delete.") % {'result': result, 'total': total}) + raise exceptions.CommandError(msg) class ListKeypair(command.Lister): diff --git a/openstackclient/compute/v2/service.py b/openstackclient/compute/v2/service.py index 2e40dd7f47..0b5b1cfbf5 100644 --- a/openstackclient/compute/v2/service.py +++ b/openstackclient/compute/v2/service.py @@ -29,21 +29,35 @@ LOG = logging.getLogger(__name__) class DeleteService(command.Command): - """Delete service command""" + """Delete compute service(s)""" def get_parser(self, prog_name): parser = super(DeleteService, self).get_parser(prog_name) parser.add_argument( "service", metavar="", - help=_("Compute service to delete (ID only)") + nargs='+', + help=_("Compute service(s) to delete (ID only)") ) return parser def take_action(self, parsed_args): compute_client = self.app.client_manager.compute + result = 0 + for s in parsed_args.service: + try: + compute_client.services.delete(s) + except Exception as e: + result += 1 + LOG.error(_("Failed to delete compute service with " + "ID '%(service)s': %(e)s") + % {'service': s, 'e': e}) - compute_client.services.delete(parsed_args.service) + if result > 0: + total = len(parsed_args.service) + msg = (_("%(result)s of %(total)s compute services failed " + "to delete.") % {'result': result, 'total': total}) + raise exceptions.CommandError(msg) class ListService(command.Lister): diff --git a/openstackclient/tests/compute/v2/fakes.py b/openstackclient/tests/compute/v2/fakes.py index b7f17fbc93..882d848001 100644 --- a/openstackclient/tests/compute/v2/fakes.py +++ b/openstackclient/tests/compute/v2/fakes.py @@ -826,6 +826,25 @@ class FakeKeypair(object): return keypairs + @staticmethod + def get_keypairs(keypairs=None, count=2): + """Get an iterable MagicMock object with a list of faked keypairs. + + If keypairs list is provided, then initialize the Mock object with the + list. Otherwise create one. + + :param List keypairs: + A list of FakeResource objects faking keypairs + :param int count: + The number of keypairs to fake + :return: + An iterable Mock object with side_effect set to a list of faked + keypairs + """ + if keypairs is None: + keypairs = FakeKeypair.create_keypairs(count) + return mock.MagicMock(side_effect=keypairs) + class FakeAvailabilityZone(object): """Fake one or more compute availability zones (AZs).""" diff --git a/openstackclient/tests/compute/v2/test_keypair.py b/openstackclient/tests/compute/v2/test_keypair.py index a50a532392..25949e310b 100644 --- a/openstackclient/tests/compute/v2/test_keypair.py +++ b/openstackclient/tests/compute/v2/test_keypair.py @@ -14,6 +14,10 @@ # import mock +from mock import call + +from osc_lib import exceptions +from osc_lib import utils from openstackclient.compute.v2 import keypair from openstackclient.tests.compute.v2 import fakes as compute_fakes @@ -114,22 +118,23 @@ class TestKeypairCreate(TestKeypair): class TestKeypairDelete(TestKeypair): - keypair = compute_fakes.FakeKeypair.create_one_keypair() + keypairs = compute_fakes.FakeKeypair.create_keypairs(count=2) def setUp(self): super(TestKeypairDelete, self).setUp() - self.keypairs_mock.get.return_value = self.keypair + self.keypairs_mock.get = compute_fakes.FakeKeypair.get_keypairs( + self.keypairs) self.keypairs_mock.delete.return_value = None self.cmd = keypair.DeleteKeypair(self.app, None) def test_keypair_delete(self): arglist = [ - self.keypair.name + self.keypairs[0].name ] verifylist = [ - ('name', self.keypair.name), + ('name', [self.keypairs[0].name]), ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) @@ -137,7 +142,54 @@ class TestKeypairDelete(TestKeypair): ret = self.cmd.take_action(parsed_args) self.assertIsNone(ret) - self.keypairs_mock.delete.assert_called_with(self.keypair.name) + self.keypairs_mock.delete.assert_called_with(self.keypairs[0].name) + + def test_delete_multiple_keypairs(self): + arglist = [] + for k in self.keypairs: + arglist.append(k.name) + verifylist = [ + ('name', arglist), + ] + + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + result = self.cmd.take_action(parsed_args) + + calls = [] + for k in self.keypairs: + calls.append(call(k.name)) + self.keypairs_mock.delete.assert_has_calls(calls) + self.assertIsNone(result) + + def test_delete_multiple_keypairs_with_exception(self): + arglist = [ + self.keypairs[0].name, + 'unexist_keypair', + ] + verifylist = [ + ('name', arglist), + ] + + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + + find_mock_result = [self.keypairs[0], exceptions.CommandError] + with mock.patch.object(utils, 'find_resource', + side_effect=find_mock_result) as find_mock: + try: + self.cmd.take_action(parsed_args) + self.fail('CommandError should be raised.') + except exceptions.CommandError as e: + self.assertEqual('1 of 2 public keys failed to delete.', + str(e)) + + find_mock.assert_any_call( + self.keypairs_mock, self.keypairs[0].name) + find_mock.assert_any_call(self.keypairs_mock, 'unexist_keypair') + + self.assertEqual(2, find_mock.call_count) + self.keypairs_mock.delete.assert_called_once_with( + self.keypairs[0].name + ) class TestKeypairList(TestKeypair): diff --git a/openstackclient/tests/compute/v2/test_service.py b/openstackclient/tests/compute/v2/test_service.py index e41d633a62..1599f466b7 100644 --- a/openstackclient/tests/compute/v2/test_service.py +++ b/openstackclient/tests/compute/v2/test_service.py @@ -14,6 +14,7 @@ # import mock +from mock import call from osc_lib import exceptions @@ -33,32 +34,74 @@ class TestService(compute_fakes.TestComputev2): class TestServiceDelete(TestService): + services = compute_fakes.FakeService.create_services(count=2) + def setUp(self): super(TestServiceDelete, self).setUp() - self.service = compute_fakes.FakeService.create_one_service() - self.service_mock.delete.return_value = None # Get the command object to test self.cmd = service.DeleteService(self.app, None) - def test_service_delete_no_options(self): + def test_service_delete(self): arglist = [ - self.service.binary, + self.services[0].binary, ] verifylist = [ - ('service', self.service.binary), + ('service', [self.services[0].binary]), ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) result = self.cmd.take_action(parsed_args) self.service_mock.delete.assert_called_with( - self.service.binary, + self.services[0].binary, ) self.assertIsNone(result) + def test_multi_services_delete(self): + arglist = [] + for s in self.services: + arglist.append(s.binary) + verifylist = [ + ('service', arglist), + ] + + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + result = self.cmd.take_action(parsed_args) + + calls = [] + for s in self.services: + calls.append(call(s.binary)) + self.service_mock.delete.assert_has_calls(calls) + self.assertIsNone(result) + + def test_multi_services_delete_with_exception(self): + arglist = [ + self.services[0].binary, + 'unexist_service', + ] + verifylist = [ + ('service', arglist) + ] + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + + delete_mock_result = [None, exceptions.CommandError] + self.service_mock.delete = ( + mock.MagicMock(side_effect=delete_mock_result) + ) + + try: + self.cmd.take_action(parsed_args) + self.fail('CommandError should be raised.') + except exceptions.CommandError as e: + self.assertEqual( + '1 of 2 compute services failed to delete.', str(e)) + + self.service_mock.delete.assert_any_call(self.services[0].binary) + self.service_mock.delete.assert_any_call('unexist_service') + class TestServiceList(TestService): diff --git a/releasenotes/notes/bp-multi-argument-compute-0bc4522f6edca355.yaml b/releasenotes/notes/bp-multi-argument-compute-0bc4522f6edca355.yaml index 286dc7ea67..2e129dfa02 100644 --- a/releasenotes/notes/bp-multi-argument-compute-0bc4522f6edca355.yaml +++ b/releasenotes/notes/bp-multi-argument-compute-0bc4522f6edca355.yaml @@ -1,5 +1,5 @@ --- features: - - Support bulk deletion and error handling for ``aggregate delete`` and - ``flavor delete`` commands. + - Support bulk deletion and error handling for ``aggregate delete``, + ``flavor delete``, ``keypair delete`` and ``service delete`` commands. [Blueprint `multi-argument-compute `_]