Make SetAggregate inherit from cliff.Command

set/unset comamnd classes should inherit from cliff.Command class.

Also, this patch adds functional tests for aggregate.

And also, use utils.format_dict() to format the output of the
properties dict.

Change-Id: Idb50bef8990da95666960e2414dfd7c9be234bba
Partial-bug: #1519503
Closes-Bug: 1546065

Author: Tang Chen

doc/source/backwards-incompatible.rst

@@ -150,6 +150,18 @@ List of Backwards Incompatible Changes
   * Bug: https://bugs.launchpad.net/python-openstackclient/+bug/1546065
   * Commit: https://review.openstack.org/#/c/281088/
 
+13. `aggregate set` commands will no longer return the modified resource
+
+  Previously, modifying an aggregate would result in the new aggregate being
+  displayed to the user. To keep things consistent with other `set` commands,
+  we will no longer be showing the modified resource.
+
+  * In favor of: Use `set` then `show`
+  * As of: NA
+  * Removed in: NA
+  * Bug: https://bugs.launchpad.net/python-openstackclient/+bug/1546065
+  * Commit: https://review.openstack.org/#/c/281089/
+
 For Developers
 ==============
 

functional/tests/compute/v2/test_aggregate.py

@@ -0,0 +1,56 @@
+#    Licensed under the Apache License, Version 2.0 (the "License"); you may
+#    not use this file except in compliance with the License. You may obtain
+#    a copy of the License at
+#
+#         http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#    License for the specific language governing permissions and limitations
+#    under the License.
+
+import uuid
+
+from functional.common import test
+
+
+class AggregateTests(test.TestCase):
+    """Functional tests for aggregate. """
+
+    NAME = uuid.uuid4().hex
+    HEADERS = ['Name']
+    FIELDS = ['name']
+
+    @classmethod
+    def setUpClass(cls):
+        opts = cls.get_show_opts(cls.FIELDS)
+        raw_output = cls.openstack('aggregate create ' + cls.NAME + opts)
+        expected = cls.NAME + '\n'
+        cls.assertOutput(expected, raw_output)
+
+    @classmethod
+    def tearDownClass(cls):
+        raw_output = cls.openstack('aggregate delete ' + cls.NAME)
+        cls.assertOutput('', raw_output)
+
+    def test_aggregate_list(self):
+        opts = self.get_list_opts(self.HEADERS)
+        raw_output = self.openstack('aggregate list' + opts)
+        self.assertIn(self.NAME, raw_output)
+
+    def test_aggregate_show(self):
+        opts = self.get_show_opts(self.FIELDS)
+        raw_output = self.openstack('aggregate show ' + self.NAME + opts)
+        self.assertEqual(self.NAME + "\n", raw_output)
+
+    def test_aggregate_properties(self):
+        opts = self.get_show_opts(['properties'])
+
+        raw_output = self.openstack(
+            'aggregate set --property a=b --property c=d ' + self.NAME
+        )
+        self.assertEqual('', raw_output)
+
+        raw_output = self.openstack('aggregate show ' + self.NAME + opts)
+        self.assertIn("a='b', c='d'\n", raw_output)

openstackclient/compute/v2/aggregate.py

@@ -201,7 +201,7 @@ def take_action(self, parsed_args):
         return zip(*sorted(six.iteritems(info)))
 
 
-class SetAggregate(command.ShowOne):
+class SetAggregate(command.Command):
     """Set aggregate properties"""
 
     def get_parser(self, prog_name):
@@ -238,28 +238,22 @@ def take_action(self, parsed_args):
             parsed_args.aggregate,
         )
 
-        info = {}
         kwargs = {}
         if parsed_args.name:
             kwargs['name'] = parsed_args.name
         if parsed_args.zone:
             kwargs['availability_zone'] = parsed_args.zone
         if kwargs:
-            info.update(compute_client.aggregates.update(
+            compute_client.aggregates.update(
                 aggregate,
                 kwargs
-            )._info)
+            )
 
         if parsed_args.property:
-            info.update(compute_client.aggregates.set_metadata(
+            compute_client.aggregates.set_metadata(
                 aggregate,
-                parsed_args.property,
-            )._info)
-
-        if info:
-            return zip(*sorted(six.iteritems(info)))
-        else:
-            return ({}, {})
+                parsed_args.property
+            )
 
 
 class ShowAggregate(command.ShowOne):
@@ -284,8 +278,14 @@ def take_action(self, parsed_args):
         # Remove availability_zone from metadata because Nova doesn't
         if 'availability_zone' in data.metadata:
             data.metadata.pop('availability_zone')
-        # Map 'metadata' column to 'properties'
-        data._info.update({'properties': data._info.pop('metadata')})
+
+        # Special mapping for columns to make the output easier to read:
+        # 'metadata' --> 'properties'
+        data._info.update(
+            {
+                'properties': utils.format_dict(data._info.pop('metadata')),
+            },
+        )
 
         info = {}
         info.update(data._info)

releasenotes/notes/bug-1546065-41d09ffbd8606513.yaml

@@ -6,3 +6,5 @@ fixes:
     [Bug `1546065 <https://bugs.launchpad.net/python-openstackclient/+bug/1546065>`_]
   - Command ``compute agent set`` now outputs nothing.
     [Bug `1546065 <https://bugs.launchpad.net/python-openstackclient/+bug/1546065>`_]
+  - Command ``aggregate set`` now outputs nothing.
+    [Bug `1546065 <https://bugs.launchpad.net/python-openstackclient/+bug/1546065>`_]