Support bulk deletion for "flavor/aggregate delete"

Support bulk deletion and error handling for "aggregate delete"
and "flavor delete" commands.

Change-Id: I3f6105cbeeab1c9f8cd571c63ce0e7ac3d4252b3
Partially-Implements: blueprint multi-argument-compute
Partial-Bug: #1592906

doc/source/command-objects/aggregate.rst

@@ -56,17 +56,17 @@ Create a new aggregate
 aggregate delete
 ----------------
 
-Delete an existing aggregate
+Delete existing aggregate(s)
 
 .. program:: aggregate delete
 .. code:: bash
 
     os aggregate delete
-        <aggregate>
+        <aggregate> [<aggregate> ...]
 
 .. describe:: <aggregate>
 
-    Aggregate to delete (name or ID)
+    Aggregate(s) to delete (name or ID)
 
 aggregate list
 --------------

doc/source/command-objects/flavor.rst

@@ -67,18 +67,18 @@ Create new flavor
 flavor delete
 -------------
 
-Delete flavor
+Delete flavor(s)
 
 .. program:: flavor delete
 .. code:: bash
 
     os flavor delete
-        <flavor>
+        <flavor> [<flavor> ...]
 
 .. _flavor_delete-flavor:
 .. describe:: <flavor>
 
-    Flavor to delete (name or ID)
+    Flavor(s) to delete (name or ID)
 
 flavor list
 -----------

openstackclient/compute/v2/aggregate.py

@@ -16,14 +16,20 @@
 
 """Compute v2 Aggregate action implementations"""
 
+import logging
+
 from osc_lib.cli import parseractions
 from osc_lib.command import command
+from osc_lib import exceptions
 from osc_lib import utils
 import six
 
 from openstackclient.i18n import _
 
 
+LOG = logging.getLogger(__name__)
+
+
 class AddAggregateHost(command.ShowOne):
     """Add host to aggregate"""
 
@@ -99,25 +105,37 @@ class CreateAggregate(command.ShowOne):
 
 
 class DeleteAggregate(command.Command):
-    """Delete an existing aggregate"""
+    """Delete existing aggregate(s)"""
 
     def get_parser(self, prog_name):
         parser = super(DeleteAggregate, self).get_parser(prog_name)
         parser.add_argument(
             'aggregate',
             metavar='<aggregate>',
-            help=_("Aggregate to delete (name or ID)")
+            nargs='+',
+            help=_("Aggregate(s) to delete (name or ID)")
         )
         return parser
 
     def take_action(self, parsed_args):
-
         compute_client = self.app.client_manager.compute
+        result = 0
+        for a in parsed_args.aggregate:
+            try:
                 data = utils.find_resource(
-            compute_client.aggregates,
+                    compute_client.aggregates, a)
-            parsed_args.aggregate,
-        )
                 compute_client.aggregates.delete(data.id)
+            except Exception as e:
+                result += 1
+                LOG.error(_("Failed to delete aggregate with name or "
+                          "ID '%(aggregate)s': %(e)s")
+                          % {'aggregate': a, 'e': e})
+
+        if result > 0:
+            total = len(parsed_args.aggregate)
+            msg = (_("%(result)s of %(total)s aggregates failed "
+                   "to delete.") % {'result': result, 'total': total})
+            raise exceptions.CommandError(msg)
 
 
 class ListAggregate(command.Lister):

openstackclient/compute/v2/flavor.py

@@ -15,6 +15,8 @@
 
 """Flavor action implementations"""
 
+import logging
+
 from osc_lib.cli import parseractions
 from osc_lib.command import command
 from osc_lib import exceptions
@@ -25,6 +27,9 @@ from openstackclient.i18n import _
 from openstackclient.identity import common as identity_common
 
 
+LOG = logging.getLogger(__name__)
+
+
 def _find_flavor(compute_client, flavor):
     try:
         return compute_client.flavors.get(flavor)
@@ -140,21 +145,36 @@ class CreateFlavor(command.ShowOne):
 
 
 class DeleteFlavor(command.Command):
-    """Delete flavor"""
+    """Delete flavor(s)"""
 
     def get_parser(self, prog_name):
         parser = super(DeleteFlavor, self).get_parser(prog_name)
         parser.add_argument(
             "flavor",
             metavar="<flavor>",
-            help=_("Flavor to delete (name or ID)")
+            nargs='+',
+            help=_("Flavor(s) to delete (name or ID)")
         )
         return parser
 
     def take_action(self, parsed_args):
         compute_client = self.app.client_manager.compute
-        flavor = _find_flavor(compute_client, parsed_args.flavor)
+        result = 0
+        for f in parsed_args.flavor:
+            try:
+                flavor = _find_flavor(compute_client, f)
                 compute_client.flavors.delete(flavor.id)
+            except Exception as e:
+                result += 1
+                LOG.error(_("Failed to delete flavor with name or "
+                          "ID '%(flavor)s': %(e)s")
+                          % {'flavor': f, 'e': e})
+
+        if result > 0:
+            total = len(parsed_args.flavor)
+            msg = (_("%(result)s of %(total)s flavors failed "
+                   "to delete.") % {'result': result, 'total': total})
+            raise exceptions.CommandError(msg)
 
 
 class ListFlavor(command.Lister):

openstackclient/tests/compute/v2/fakes.py

@@ -87,6 +87,42 @@ class FakeAggregate(object):
             loaded=True)
         return aggregate
 
+    @staticmethod
+    def create_aggregates(attrs=None, count=2):
+        """Create multiple fake aggregates.
+
+        :param Dictionary attrs:
+            A dictionary with all attributes
+        :param int count:
+            The number of aggregates to fake
+        :return:
+            A list of FakeResource objects faking the aggregates
+        """
+        aggregates = []
+        for i in range(0, count):
+            aggregates.append(FakeAggregate.create_one_aggregate(attrs))
+
+        return aggregates
+
+    @staticmethod
+    def get_aggregates(aggregates=None, count=2):
+        """Get an iterable MagicMock object with a list of faked aggregates.
+
+        If aggregates list is provided, then initialize the Mock object
+        with the list. Otherwise create one.
+
+        :param List aggregates:
+            A list of FakeResource objects faking aggregates
+        :param int count:
+            The number of aggregates to fake
+        :return:
+            An iterable Mock object with side_effect set to a list of faked
+            aggregates
+        """
+        if aggregates is None:
+            aggregates = FakeAggregate.create_aggregates(count)
+        return mock.MagicMock(side_effect=aggregates)
+
 
 class FakeComputev2Client(object):
 
@@ -732,7 +768,7 @@ class FakeFlavor(object):
             flavors
         """
         if flavors is None:
-            flavors = FakeServer.create_flavors(count)
+            flavors = FakeFlavor.create_flavors(count)
         return mock.MagicMock(side_effect=flavors)
 
 

openstackclient/tests/compute/v2/test_aggregate.py

@@ -13,6 +13,12 @@
 #   under the License.
 #
 
+import mock
+from mock import call
+
+from osc_lib import exceptions
+from osc_lib import utils
+
 from openstackclient.compute.v2 import aggregate
 from openstackclient.tests.compute.v2 import fakes as compute_fakes
 from openstackclient.tests import utils as tests_utils
@@ -135,25 +141,74 @@ class TestAggregateCreate(TestAggregate):
 
 class TestAggregateDelete(TestAggregate):
 
+    fake_ags = compute_fakes.FakeAggregate.create_aggregates(count=2)
+
     def setUp(self):
         super(TestAggregateDelete, self).setUp()
 
-        self.aggregate_mock.get.return_value = self.fake_ag
+        self.aggregate_mock.get = (
+            compute_fakes.FakeAggregate.get_aggregates(self.fake_ags))
         self.cmd = aggregate.DeleteAggregate(self.app, None)
 
     def test_aggregate_delete(self):
         arglist = [
-            'ag1',
+            self.fake_ags[0].id
         ]
         verifylist = [
-            ('aggregate', 'ag1'),
+            ('aggregate', [self.fake_ags[0].id]),
         ]
         parsed_args = self.check_parser(self.cmd, arglist, verifylist)
         result = self.cmd.take_action(parsed_args)
-        self.aggregate_mock.get.assert_called_once_with(parsed_args.aggregate)
+        self.aggregate_mock.get.assert_called_once_with(self.fake_ags[0].id)
-        self.aggregate_mock.delete.assert_called_once_with(self.fake_ag.id)
+        self.aggregate_mock.delete.assert_called_once_with(self.fake_ags[0].id)
         self.assertIsNone(result)
 
+    def test_delete_multiple_aggregates(self):
+        arglist = []
+        for a in self.fake_ags:
+            arglist.append(a.id)
+        verifylist = [
+            ('aggregate', arglist),
+        ]
+
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+        result = self.cmd.take_action(parsed_args)
+
+        calls = []
+        for a in self.fake_ags:
+            calls.append(call(a.id))
+        self.aggregate_mock.delete.assert_has_calls(calls)
+        self.assertIsNone(result)
+
+    def test_delete_multiple_agggregates_with_exception(self):
+        arglist = [
+            self.fake_ags[0].id,
+            'unexist_aggregate',
+        ]
+        verifylist = [
+            ('aggregate', arglist),
+        ]
+
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+
+        find_mock_result = [self.fake_ags[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 aggregates failed to delete.',
+                                 str(e))
+
+            find_mock.assert_any_call(self.aggregate_mock, self.fake_ags[0].id)
+            find_mock.assert_any_call(self.aggregate_mock, 'unexist_aggregate')
+
+            self.assertEqual(2, find_mock.call_count)
+            self.aggregate_mock.delete.assert_called_once_with(
+                self.fake_ags[0].id
+            )
+
 
 class TestAggregateList(TestAggregate):
 

openstackclient/tests/compute/v2/test_flavor.py

@@ -14,6 +14,8 @@
 #
 
 import copy
+import mock
+from mock import call
 
 from osc_lib import exceptions
 from osc_lib import utils
@@ -204,47 +206,73 @@ class TestFlavorCreate(TestFlavor):
 
 class TestFlavorDelete(TestFlavor):
 
-    flavor = compute_fakes.FakeFlavor.create_one_flavor()
+    flavors = compute_fakes.FakeFlavor.create_flavors(count=2)
 
     def setUp(self):
         super(TestFlavorDelete, self).setUp()
 
-        self.flavors_mock.get.return_value = self.flavor
+        self.flavors_mock.get = (
+            compute_fakes.FakeFlavor.get_flavors(self.flavors))
         self.flavors_mock.delete.return_value = None
 
         self.cmd = flavor.DeleteFlavor(self.app, None)
 
     def test_flavor_delete(self):
         arglist = [
-            self.flavor.id
+            self.flavors[0].id
         ]
         verifylist = [
-            ('flavor', self.flavor.id),
+            ('flavor', [self.flavors[0].id]),
         ]
         parsed_args = self.check_parser(self.cmd, arglist, verifylist)
 
         result = self.cmd.take_action(parsed_args)
 
-        self.flavors_mock.delete.assert_called_with(self.flavor.id)
+        self.flavors_mock.delete.assert_called_with(self.flavors[0].id)
         self.assertIsNone(result)
 
-    def test_flavor_delete_with_unexist_flavor(self):
+    def test_delete_multiple_flavors(self):
-        self.flavors_mock.get.side_effect = exceptions.NotFound(None)
+        arglist = []
-        self.flavors_mock.find.side_effect = exceptions.NotFound(None)
+        for f in self.flavors:
-
+            arglist.append(f.id)
-        arglist = [
-            'unexist_flavor'
-        ]
         verifylist = [
-            ('flavor', 'unexist_flavor'),
+            ('flavor', arglist),
         ]
 
         parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+        result = self.cmd.take_action(parsed_args)
 
-        self.assertRaises(
+        calls = []
-            exceptions.CommandError,
+        for f in self.flavors:
-            self.cmd.take_action,
+            calls.append(call(f.id))
-            parsed_args)
+        self.flavors_mock.delete.assert_has_calls(calls)
+        self.assertIsNone(result)
+
+    def test_multi_flavors_delete_with_exception(self):
+        arglist = [
+            self.flavors[0].id,
+            'unexist_flavor',
+        ]
+        verifylist = [
+            ('flavor', [self.flavors[0].id, 'unexist_flavor'])
+        ]
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+
+        find_mock_result = [self.flavors[0], exceptions.CommandError]
+        self.flavors_mock.get = (
+            mock.MagicMock(side_effect=find_mock_result)
+        )
+        self.flavors_mock.find.side_effect = exceptions.NotFound(None)
+
+        try:
+            self.cmd.take_action(parsed_args)
+            self.fail('CommandError should be raised.')
+        except exceptions.CommandError as e:
+            self.assertEqual('1 of 2 flavors failed to delete.', str(e))
+
+        self.flavors_mock.get.assert_any_call(self.flavors[0].id)
+        self.flavors_mock.get.assert_any_call('unexist_flavor')
+        self.flavors_mock.delete.assert_called_once_with(self.flavors[0].id)
 
 
 class TestFlavorList(TestFlavor):

releasenotes/notes/bp-multi-argument-compute-0bc4522f6edca355.yaml

@@ -0,0 +1,5 @@
+---
+features:
+  - Support bulk deletion and error handling for ``aggregate delete`` and
+    ``flavor delete`` commands.
+    [Blueprint `multi-argument-compute <https://blueprints.launchpad.net/python-openstackclient/+spec/multi-argument-compute>`_]