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

Author: Huanxuan Ao

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
-        <service>
+        <service> [<service> ...]
 
 .. _compute-service-delete:
 .. describe:: <service>
 
-    Compute service to delete (ID only)
+    Compute service(s) to delete (ID only)
 
 compute service list
 --------------------

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
-        <key>
+        <key> [<key> ...]
 
 .. describe:: <key>
 
-    Public key to delete (name only)
+    Public key(s) to delete (name only)
 
 keypair list
 ------------

openstackclient/compute/v2/keypair.py

@@ -16,6 +16,7 @@
 """Keypair action implementations"""
 
 import io
+import logging
 import os
 import sys
 
@@ -27,6 +28,9 @@
 from openstackclient.i18n import _
 
 
+LOG = logging.getLogger(__name__)
+
+
 class CreateKeypair(command.ShowOne):
     """Create new public key"""
 
@@ -78,20 +82,37 @@ def take_action(self, parsed_args):
 
 
 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='<key>',
-            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):

openstackclient/compute/v2/service.py

@@ -29,21 +29,35 @@
 
 
 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="<service>",
-            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):

openstackclient/tests/compute/v2/fakes.py

@@ -826,6 +826,25 @@ def create_keypairs(attrs=None, count=2):
 
         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)."""

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,30 +118,78 @@ def test_keypair_create_public_key(self):
 
 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)
 
         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):

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 @@ def setUp(self):
 
 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):
 

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 <https://blueprints.launchpad.net/python-openstackclient/+spec/multi-argument-compute>`_]