Merge "Support bulk deletion for delete commands in computev2"

5 files changed, 165 insertions, 16 deletions

diff --git a/openstackclient/compute/v2/keypair.py b/openstackclient/compute/v2/keypair.py
index 13c0a99c..3725a3a8 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='<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):
diff --git a/openstackclient/compute/v2/service.py b/openstackclient/compute/v2/service.py
index 1441114b..53624f55 100644
--- a/openstackclient/compute/v2/service.py
+++ b/openstackclient/compute/v2/service.py
@@ -36,14 +36,28 @@ class DeleteService(command.Command):
         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):
diff --git a/openstackclient/tests/compute/v2/fakes.py b/openstackclient/tests/compute/v2/fakes.py
index b7f17fbc..882d8480 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 a50a5323..25949e31 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 e41d633a..1599f466 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):