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
This commit is contained in:
		| @@ -116,6 +116,7 @@ Set aggregate properties | ||||
|         [--name <new-name>] | ||||
|         [--zone <availability-zone>] | ||||
|         [--property <key=value> [...] ] | ||||
|         [--no-property] | ||||
|         <aggregate> | ||||
|  | ||||
| .. option:: --name <name> | ||||
| @@ -131,6 +132,12 @@ Set aggregate properties | ||||
|     Property to set on :ref:`\<aggregate\> <aggregate_set-aggregate>` | ||||
|     (repeat option to set multiple properties) | ||||
|  | ||||
| .. option:: --no-property | ||||
|  | ||||
|     Remove all properties from :ref:`\<aggregate\> <aggregate_set-aggregate>` | ||||
|     (specify both --property and --no-property to | ||||
|     overwrite the current properties) | ||||
|  | ||||
| .. _aggregate_set-aggregate: | ||||
| .. describe:: <aggregate> | ||||
|  | ||||
|   | ||||
| @@ -248,6 +248,14 @@ class SetAggregate(command.Command): | ||||
|             help=_("Property to set on <aggregate> " | ||||
|                    "(repeat option to set multiple properties)") | ||||
|         ) | ||||
|         parser.add_argument( | ||||
|             "--no-property", | ||||
|             dest="no_property", | ||||
|             action="store_true", | ||||
|             help=_("Remove all properties from <aggregate> " | ||||
|                    "(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="<key>", | ||||
|             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} | ||||
|         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) | ||||
|   | ||||
| @@ -83,6 +83,7 @@ class FakeAggregate(object): | ||||
|             "id": "aggregate-id-" + uuid.uuid4().hex, | ||||
|             "metadata": { | ||||
|                 "availability_zone": "ag_zone", | ||||
|                 "key1": "value1", | ||||
|             } | ||||
|         } | ||||
|  | ||||
|   | ||||
| @@ -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) | ||||
|   | ||||
| @@ -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 <https://blueprints.launchpad.net/python-openstackclient/+spec/support-no-property-in-aggregate>`_] | ||||
		Reference in New Issue
	
	Block a user
	 Rui Chen
					Rui Chen