From 640014fa91b939e802f261346473d3ec025f2acb Mon Sep 17 00:00:00 2001 From: Huanxuan Ao Date: Mon, 20 Jun 2016 15:42:40 +0800 Subject: [PATCH] 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 | 6 +- doc/source/command-objects/flavor.rst | 6 +- openstackclient/compute/v2/aggregate.py | 34 +++++++--- openstackclient/compute/v2/flavor.py | 28 ++++++-- openstackclient/tests/compute/v2/fakes.py | 38 ++++++++++- .../tests/compute/v2/test_aggregate.py | 65 +++++++++++++++++-- .../tests/compute/v2/test_flavor.py | 62 +++++++++++++----- ...lti-argument-compute-0bc4522f6edca355.yaml | 5 ++ 8 files changed, 203 insertions(+), 41 deletions(-) create mode 100644 releasenotes/notes/bp-multi-argument-compute-0bc4522f6edca355.yaml diff --git a/doc/source/command-objects/aggregate.rst b/doc/source/command-objects/aggregate.rst index 7616c4965..25c7041a4 100644 --- a/doc/source/command-objects/aggregate.rst +++ b/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 - + [ ...] .. describe:: - Aggregate to delete (name or ID) + Aggregate(s) to delete (name or ID) aggregate list -------------- diff --git a/doc/source/command-objects/flavor.rst b/doc/source/command-objects/flavor.rst index a30bedecb..dd25f4428 100644 --- a/doc/source/command-objects/flavor.rst +++ b/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_delete-flavor: .. describe:: - Flavor to delete (name or ID) + Flavor(s) to delete (name or ID) flavor list ----------- diff --git a/openstackclient/compute/v2/aggregate.py b/openstackclient/compute/v2/aggregate.py index 8000c93a0..2e2838c54 100644 --- a/openstackclient/compute/v2/aggregate.py +++ b/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='', - 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 - data = utils.find_resource( - compute_client.aggregates, - parsed_args.aggregate, - ) - compute_client.aggregates.delete(data.id) + result = 0 + for a in parsed_args.aggregate: + try: + data = utils.find_resource( + compute_client.aggregates, a) + 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): diff --git a/openstackclient/compute/v2/flavor.py b/openstackclient/compute/v2/flavor.py index ab2bc85d0..dc276109f 100644 --- a/openstackclient/compute/v2/flavor.py +++ b/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="", - 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) - compute_client.flavors.delete(flavor.id) + 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): diff --git a/openstackclient/tests/compute/v2/fakes.py b/openstackclient/tests/compute/v2/fakes.py index 60abb8ef1..8416b630b 100644 --- a/openstackclient/tests/compute/v2/fakes.py +++ b/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) diff --git a/openstackclient/tests/compute/v2/test_aggregate.py b/openstackclient/tests/compute/v2/test_aggregate.py index 58dd77552..3ebca35f9 100644 --- a/openstackclient/tests/compute/v2/test_aggregate.py +++ b/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.delete.assert_called_once_with(self.fake_ag.id) + self.aggregate_mock.get.assert_called_once_with(self.fake_ags[0].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): diff --git a/openstackclient/tests/compute/v2/test_flavor.py b/openstackclient/tests/compute/v2/test_flavor.py index 4365a540b..d8d02e111 100644 --- a/openstackclient/tests/compute/v2/test_flavor.py +++ b/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): - self.flavors_mock.get.side_effect = exceptions.NotFound(None) - self.flavors_mock.find.side_effect = exceptions.NotFound(None) - - arglist = [ - 'unexist_flavor' - ] + def test_delete_multiple_flavors(self): + arglist = [] + for f in self.flavors: + arglist.append(f.id) verifylist = [ - ('flavor', 'unexist_flavor'), + ('flavor', arglist), ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) + result = self.cmd.take_action(parsed_args) - self.assertRaises( - exceptions.CommandError, - self.cmd.take_action, - parsed_args) + calls = [] + for f in self.flavors: + calls.append(call(f.id)) + 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): diff --git a/releasenotes/notes/bp-multi-argument-compute-0bc4522f6edca355.yaml b/releasenotes/notes/bp-multi-argument-compute-0bc4522f6edca355.yaml new file mode 100644 index 000000000..286dc7ea6 --- /dev/null +++ b/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 `_]