From e2a9a9607cc84c0afc2fc5524681a3adebdc68ec Mon Sep 17 00:00:00 2001
From: Stephen Finucane <sfinucan@redhat.com>
Date: Thu, 5 Nov 2020 11:29:54 +0000
Subject: [PATCH] compute: Fix 'server group * -f yaml' output

Make use of 'FormattableColumn'-derived formatters, which provide better
output than what we were using before, particularly for the YAML output
format.

Change-Id: Id6d25a0a348596d5a0430ff7afbf87b049a76bc8
Signed-off-by: Stephen Finucane <sfinucan@redhat.com>
---
 openstackclient/compute/v2/server_group.py    | 27 ++++++-----
 .../compute/v2/test_server_group.py           | 12 ++---
 .../unit/compute/v2/test_server_group.py      | 48 ++++++++++---------
 ...proved-server-output-6965b664f6abda8d.yaml |  4 ++
 4 files changed, 51 insertions(+), 40 deletions(-)

diff --git a/openstackclient/compute/v2/server_group.py b/openstackclient/compute/v2/server_group.py
index a33632446c..d245c0927f 100644
--- a/openstackclient/compute/v2/server_group.py
+++ b/openstackclient/compute/v2/server_group.py
@@ -18,6 +18,7 @@
 import logging
 
 from novaclient import api_versions
+from osc_lib.cli import format_columns
 from osc_lib.command import command
 from osc_lib import exceptions
 from osc_lib import utils
@@ -29,8 +30,8 @@ LOG = logging.getLogger(__name__)
 
 
 _formatters = {
-    'policies': utils.format_list,
-    'members': utils.format_list,
+    'policies': format_columns.ListColumn,
+    'members': format_columns.ListColumn,
 }
 
 
@@ -93,8 +94,8 @@ class CreateServerGroup(command.ShowOne):
         info.update(server_group._info)
 
         columns = _get_columns(info)
-        data = utils.get_dict_properties(info, columns,
-                                         formatters=_formatters)
+        data = utils.get_dict_properties(
+            info, columns, formatters=_formatters)
         return columns, data
 
 
@@ -176,14 +177,18 @@ class ListServerGroup(command.Lister):
                 policy_key,
             )
 
-        return (column_headers,
-                (utils.get_item_properties(
+        return (
+            column_headers,
+            (
+                utils.get_item_properties(
                     s, columns,
                     formatters={
-                        'Policies': utils.format_list,
-                        'Members': utils.format_list,
+                        'Policies': format_columns.ListColumn,
+                        'Members': format_columns.ListColumn,
                     }
-                ) for s in data))
+                ) for s in data
+            ),
+        )
 
 
 class ShowServerGroup(command.ShowOne):
@@ -205,6 +210,6 @@ class ShowServerGroup(command.ShowOne):
         info = {}
         info.update(group._info)
         columns = _get_columns(info)
-        data = utils.get_dict_properties(info, columns,
-                                         formatters=_formatters)
+        data = utils.get_dict_properties(
+            info, columns, formatters=_formatters)
         return columns, data
diff --git a/openstackclient/tests/functional/compute/v2/test_server_group.py b/openstackclient/tests/functional/compute/v2/test_server_group.py
index 44ecda1de7..3dff3dcdaf 100644
--- a/openstackclient/tests/functional/compute/v2/test_server_group.py
+++ b/openstackclient/tests/functional/compute/v2/test_server_group.py
@@ -33,7 +33,7 @@ class ServerGroupTests(base.TestCase):
             cmd_output['name']
         )
         self.assertEqual(
-            'affinity',
+            ['affinity'],
             cmd_output['policies']
         )
 
@@ -47,7 +47,7 @@ class ServerGroupTests(base.TestCase):
             cmd_output['name']
         )
         self.assertEqual(
-            'anti-affinity',
+            ['anti-affinity'],
             cmd_output['policies']
         )
 
@@ -74,7 +74,7 @@ class ServerGroupTests(base.TestCase):
             cmd_output['name']
         )
         self.assertEqual(
-            'affinity',
+            ['affinity'],
             cmd_output['policies']
         )
 
@@ -91,7 +91,7 @@ class ServerGroupTests(base.TestCase):
             cmd_output['name']
         )
         self.assertEqual(
-            'anti-affinity',
+            ['anti-affinity'],
             cmd_output['policies']
         )
 
@@ -102,5 +102,5 @@ class ServerGroupTests(base.TestCase):
         self.assertIn(name1, names)
         self.assertIn(name2, names)
         policies = [x["Policies"] for x in cmd_output]
-        self.assertIn('affinity', policies)
-        self.assertIn('anti-affinity', policies)
+        self.assertIn(['affinity'], policies)
+        self.assertIn(['anti-affinity'], policies)
diff --git a/openstackclient/tests/unit/compute/v2/test_server_group.py b/openstackclient/tests/unit/compute/v2/test_server_group.py
index bf0ea0ba45..8de7492c79 100644
--- a/openstackclient/tests/unit/compute/v2/test_server_group.py
+++ b/openstackclient/tests/unit/compute/v2/test_server_group.py
@@ -16,6 +16,7 @@
 from unittest import mock
 
 from novaclient import api_versions
+from osc_lib.cli import format_columns
 from osc_lib import exceptions
 from osc_lib import utils
 
@@ -39,9 +40,9 @@ class TestServerGroup(compute_fakes.TestComputev2):
 
     data = (
         fake_server_group.id,
-        utils.format_list(fake_server_group.members),
+        format_columns.ListColumn(fake_server_group.members),
         fake_server_group.name,
-        utils.format_list(fake_server_group.policies),
+        format_columns.ListColumn(fake_server_group.policies),
         fake_server_group.project_id,
         fake_server_group.user_id,
     )
@@ -70,7 +71,7 @@ class TestServerGroupV264(TestServerGroup):
 
     data = (
         fake_server_group.id,
-        utils.format_list(fake_server_group.members),
+        format_columns.ListColumn(fake_server_group.members),
         fake_server_group.name,
         fake_server_group.policy,
         fake_server_group.project_id,
@@ -105,8 +106,8 @@ class TestServerGroupCreate(TestServerGroup):
             policies=[parsed_args.policy],
         )
 
-        self.assertEqual(self.columns, columns)
-        self.assertEqual(self.data, data)
+        self.assertCountEqual(self.columns, columns)
+        self.assertCountEqual(self.data, data)
 
     def test_server_group_create_with_soft_policies(self):
         self.app.client_manager.compute.api_version = api_versions.APIVersion(
@@ -127,8 +128,8 @@ class TestServerGroupCreate(TestServerGroup):
             policies=[parsed_args.policy],
         )
 
-        self.assertEqual(self.columns, columns)
-        self.assertEqual(self.data, data)
+        self.assertCountEqual(self.columns, columns)
+        self.assertCountEqual(self.data, data)
 
     def test_server_group_create_with_soft_policies_pre_v215(self):
         self.app.client_manager.compute.api_version = api_versions.APIVersion(
@@ -170,8 +171,8 @@ class TestServerGroupCreate(TestServerGroup):
             policy=parsed_args.policy,
         )
 
-        self.assertEqual(self.columns, columns)
-        self.assertEqual(self.data, data)
+        self.assertCountEqual(self.columns, columns)
+        self.assertCountEqual(self.data, data)
 
 
 class TestServerGroupDelete(TestServerGroup):
@@ -275,14 +276,14 @@ class TestServerGroupList(TestServerGroup):
     list_data = ((
         TestServerGroup.fake_server_group.id,
         TestServerGroup.fake_server_group.name,
-        utils.format_list(TestServerGroup.fake_server_group.policies),
+        format_columns.ListColumn(TestServerGroup.fake_server_group.policies),
     ),)
 
     list_data_long = ((
         TestServerGroup.fake_server_group.id,
         TestServerGroup.fake_server_group.name,
-        utils.format_list(TestServerGroup.fake_server_group.policies),
-        utils.format_list(TestServerGroup.fake_server_group.members),
+        format_columns.ListColumn(TestServerGroup.fake_server_group.policies),
+        format_columns.ListColumn(TestServerGroup.fake_server_group.members),
         TestServerGroup.fake_server_group.project_id,
         TestServerGroup.fake_server_group.user_id,
     ),)
@@ -303,8 +304,8 @@ class TestServerGroupList(TestServerGroup):
         columns, data = self.cmd.take_action(parsed_args)
         self.server_groups_mock.list.assert_called_once_with(False)
 
-        self.assertEqual(self.list_columns, columns)
-        self.assertEqual(self.list_data, tuple(data))
+        self.assertCountEqual(self.list_columns, columns)
+        self.assertCountEqual(self.list_data, tuple(data))
 
     def test_server_group_list_with_all_projects_and_long(self):
         arglist = [
@@ -319,8 +320,8 @@ class TestServerGroupList(TestServerGroup):
         columns, data = self.cmd.take_action(parsed_args)
         self.server_groups_mock.list.assert_called_once_with(True)
 
-        self.assertEqual(self.list_columns_long, columns)
-        self.assertEqual(self.list_data_long, tuple(data))
+        self.assertCountEqual(self.list_columns_long, columns)
+        self.assertCountEqual(self.list_data_long, tuple(data))
 
 
 class TestServerGroupListV264(TestServerGroupV264):
@@ -350,7 +351,8 @@ class TestServerGroupListV264(TestServerGroupV264):
         TestServerGroupV264.fake_server_group.id,
         TestServerGroupV264.fake_server_group.name,
         TestServerGroupV264.fake_server_group.policy,
-        utils.format_list(TestServerGroupV264.fake_server_group.members),
+        format_columns.ListColumn(
+            TestServerGroupV264.fake_server_group.members),
         TestServerGroupV264.fake_server_group.project_id,
         TestServerGroupV264.fake_server_group.user_id,
     ),)
@@ -373,8 +375,8 @@ class TestServerGroupListV264(TestServerGroupV264):
         columns, data = self.cmd.take_action(parsed_args)
         self.server_groups_mock.list.assert_called_once_with(False)
 
-        self.assertEqual(self.list_columns, columns)
-        self.assertEqual(self.list_data, tuple(data))
+        self.assertCountEqual(self.list_columns, columns)
+        self.assertCountEqual(self.list_data, tuple(data))
 
     def test_server_group_list_with_all_projects_and_long(self):
         arglist = [
@@ -389,8 +391,8 @@ class TestServerGroupListV264(TestServerGroupV264):
         columns, data = self.cmd.take_action(parsed_args)
         self.server_groups_mock.list.assert_called_once_with(True)
 
-        self.assertEqual(self.list_columns_long, columns)
-        self.assertEqual(self.list_data_long, tuple(data))
+        self.assertCountEqual(self.list_columns_long, columns)
+        self.assertCountEqual(self.list_data_long, tuple(data))
 
 
 class TestServerGroupShow(TestServerGroup):
@@ -412,5 +414,5 @@ class TestServerGroupShow(TestServerGroup):
         parsed_args = self.check_parser(self.cmd, arglist, verifylist)
         columns, data = self.cmd.take_action(parsed_args)
 
-        self.assertEqual(self.columns, columns)
-        self.assertEqual(self.data, data)
+        self.assertCountEqual(self.columns, columns)
+        self.assertCountEqual(self.data, data)
diff --git a/releasenotes/notes/improved-server-output-6965b664f6abda8d.yaml b/releasenotes/notes/improved-server-output-6965b664f6abda8d.yaml
index 7a42fa899d..5dc2980c77 100644
--- a/releasenotes/notes/improved-server-output-6965b664f6abda8d.yaml
+++ b/releasenotes/notes/improved-server-output-6965b664f6abda8d.yaml
@@ -14,3 +14,7 @@ fixes:
     formatter. In addition, the ``server_usages``, ``total_memory_mb_usage``,
     ``total_vcpus_usage`` and ``total_local_gb_usage`` values will only be
     humanized when using the table formatter.
+  - |
+    The ``policies`` (or ``policy``, on newer microversions) and ``members``
+    fields of the ``server group list`` and ``server group show`` commands
+    will now be rendered correctly as lists.