From 63715569160785ffeac05e34b604136440a9f865 Mon Sep 17 00:00:00 2001 From: Rui Chen Date: Tue, 26 Apr 2016 19:34:13 +0800 Subject: [PATCH] Support "--no" option in aggregate set Supporting "--no-property" option will apply user a convenience way to clean all properties of aggregate in a short command, and this kind of behavior is the recommended way to devref. The patch add "--no-property" option in "aggregate set" command, and update related test cases and devref document. Change-Id: I7614a23c0db05144562330dc600dbab7d003d5d8 Implements: blueprint support-no-property-in-aggregate --- doc/source/command-objects/aggregate.rst | 7 ++ openstackclient/compute/v2/aggregate.py | 33 ++++++- .../tests/unit/compute/v2/fakes.py | 1 + .../tests/unit/compute/v2/test_aggregate.py | 98 +++++++++++++++++-- ...roperty-in-aggregate-b74a42e00a65d14a.yaml | 7 ++ 5 files changed, 131 insertions(+), 15 deletions(-) create mode 100644 releasenotes/notes/support-no-property-in-aggregate-b74a42e00a65d14a.yaml diff --git a/doc/source/command-objects/aggregate.rst b/doc/source/command-objects/aggregate.rst index 25c7041a48..642942d457 100644 --- a/doc/source/command-objects/aggregate.rst +++ b/doc/source/command-objects/aggregate.rst @@ -116,6 +116,7 @@ Set aggregate properties [--name ] [--zone ] [--property [...] ] + [--no-property] .. option:: --name @@ -131,6 +132,12 @@ Set aggregate properties Property to set on :ref:`\ ` (repeat option to set multiple properties) +.. option:: --no-property + + Remove all properties from :ref:`\ ` + (specify both --property and --no-property to + overwrite the current properties) + .. _aggregate_set-aggregate: .. describe:: diff --git a/openstackclient/compute/v2/aggregate.py b/openstackclient/compute/v2/aggregate.py index 2e2838c54c..58d529e9f8 100644 --- a/openstackclient/compute/v2/aggregate.py +++ b/openstackclient/compute/v2/aggregate.py @@ -248,6 +248,14 @@ class SetAggregate(command.Command): help=_("Property to set on " "(repeat option to set multiple properties)") ) + parser.add_argument( + "--no-property", + dest="no_property", + action="store_true", + help=_("Remove all properties from " + "(specify both --property and --no-property to " + "overwrite the current properties)"), + ) return parser def take_action(self, parsed_args): @@ -269,10 +277,23 @@ class SetAggregate(command.Command): kwargs ) + set_property = {} + if parsed_args.no_property: + # NOTE(RuiChen): "availability_zone" is removed from response of + # aggregate show and create commands, don't see it + # anywhere, so pop it, avoid the unexpected server + # exception(can't unset the availability zone from + # aggregate metadata in nova). + set_property.update({key: None + for key in aggregate.metadata.keys() + if key != 'availability_zone'}) if parsed_args.property: + set_property.update(parsed_args.property) + + if set_property: compute_client.aggregates.set_metadata( aggregate, - parsed_args.property + set_property ) @@ -326,7 +347,6 @@ class UnsetAggregate(command.Command): "--property", metavar="", action='append', - required=True, help=_("Property to remove from aggregate " "(repeat option to remove multiple properties)") ) @@ -338,6 +358,9 @@ class UnsetAggregate(command.Command): compute_client.aggregates, parsed_args.aggregate) - unset_property = {key: None for key in parsed_args.property} - compute_client.aggregates.set_metadata(aggregate, - unset_property) + unset_property = {} + if parsed_args.property: + unset_property.update({key: None for key in parsed_args.property}) + if unset_property: + compute_client.aggregates.set_metadata(aggregate, + unset_property) diff --git a/openstackclient/tests/unit/compute/v2/fakes.py b/openstackclient/tests/unit/compute/v2/fakes.py index 3c82977333..985ce5e2ed 100644 --- a/openstackclient/tests/unit/compute/v2/fakes.py +++ b/openstackclient/tests/unit/compute/v2/fakes.py @@ -83,6 +83,7 @@ class FakeAggregate(object): "id": "aggregate-id-" + uuid.uuid4().hex, "metadata": { "availability_zone": "ag_zone", + "key1": "value1", } } diff --git a/openstackclient/tests/unit/compute/v2/test_aggregate.py b/openstackclient/tests/unit/compute/v2/test_aggregate.py index c636d3de2f..3efe0dbd36 100644 --- a/openstackclient/tests/unit/compute/v2/test_aggregate.py +++ b/openstackclient/tests/unit/compute/v2/test_aggregate.py @@ -21,7 +21,6 @@ from osc_lib import utils from openstackclient.compute.v2 import aggregate from openstackclient.tests.unit.compute.v2 import fakes as compute_fakes -from openstackclient.tests.unit import utils as tests_utils class TestAggregate(compute_fakes.TestComputev2): @@ -235,7 +234,8 @@ class TestAggregateList(TestAggregate): TestAggregate.fake_ag.id, TestAggregate.fake_ag.name, TestAggregate.fake_ag.availability_zone, - {}, + {key: value for key, value in TestAggregate.fake_ag.metadata.items() + if key != 'availability_zone'}, ), ) def setUp(self): @@ -371,6 +371,62 @@ class TestAggregateSet(TestAggregate): self.fake_ag, parsed_args.property) self.assertIsNone(result) + def test_aggregate_set_with_no_property_and_property(self): + arglist = [ + '--no-property', + '--property', 'key2=value2', + 'ag1', + ] + verifylist = [ + ('no_property', True), + ('property', {'key2': 'value2'}), + ('aggregate', 'ag1'), + ] + 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.assertNotCalled(self.aggregate_mock.update) + self.aggregate_mock.set_metadata.assert_called_once_with( + self.fake_ag, {'key1': None, 'key2': 'value2'}) + self.assertIsNone(result) + + def test_aggregate_set_with_no_property(self): + arglist = [ + '--no-property', + 'ag1', + ] + verifylist = [ + ('no_property', True), + ('aggregate', 'ag1'), + ] + 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.assertNotCalled(self.aggregate_mock.update) + self.aggregate_mock.set_metadata.assert_called_once_with( + self.fake_ag, {'key1': None}) + self.assertIsNone(result) + + def test_aggregate_set_with_zone_and_no_property(self): + arglist = [ + '--zone', 'new_zone', + '--no-property', + 'ag1', + ] + verifylist = [ + ('zone', 'new_zone'), + ('no_property', True), + ('aggregate', 'ag1'), + ] + 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.update.assert_called_once_with( + self.fake_ag, {'availability_zone': parsed_args.zone}) + self.aggregate_mock.set_metadata.assert_called_once_with( + self.fake_ag, {'key1': None}) + self.assertIsNone(result) + class TestAggregateShow(TestAggregate): @@ -387,7 +443,10 @@ class TestAggregateShow(TestAggregate): TestAggregate.fake_ag.hosts, TestAggregate.fake_ag.id, TestAggregate.fake_ag.name, - '', + utils.format_dict( + {key: value + for key, value in TestAggregate.fake_ag.metadata.items() + if key != 'availability_zone'}), ) def setUp(self): @@ -435,13 +494,32 @@ class TestAggregateUnset(TestAggregate): self.fake_ag, {'unset_key': None}) self.assertIsNone(result) - def test_aggregate_unset_no_property(self): + def test_aggregate_unset_multiple_properties(self): + arglist = [ + '--property', 'unset_key1', + '--property', 'unset_key2', + 'ag1', + ] + verifylist = [ + ('property', ['unset_key1', 'unset_key2']), + ('aggregate', 'ag1'), + ] + + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + result = self.cmd.take_action(parsed_args) + self.aggregate_mock.set_metadata.assert_called_once_with( + self.fake_ag, {'unset_key1': None, 'unset_key2': None}) + self.assertIsNone(result) + + def test_aggregate_unset_no_option(self): arglist = [ 'ag1', ] - verifylist = None - self.assertRaises(tests_utils.ParserException, - self.check_parser, - self.cmd, - arglist, - verifylist) + verifylist = [ + ('property', None), + ('aggregate', 'ag1'), + ] + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + result = self.cmd.take_action(parsed_args) + self.assertNotCalled(self.aggregate_mock.set_metadata) + self.assertIsNone(result) diff --git a/releasenotes/notes/support-no-property-in-aggregate-b74a42e00a65d14a.yaml b/releasenotes/notes/support-no-property-in-aggregate-b74a42e00a65d14a.yaml new file mode 100644 index 0000000000..5a785e4ae6 --- /dev/null +++ b/releasenotes/notes/support-no-property-in-aggregate-b74a42e00a65d14a.yaml @@ -0,0 +1,7 @@ +--- +features: + - | + Add ``--no-property`` option in ``aggregate set``. + Supporting ``--no-property`` option will apply user a convenience way to + clean all properties of aggregate in a short command. + [Blueprint `support-no-property-in-aggregate `_]