compute: Fix 'server * -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. For example, compare before for the 'server show' command:

  $ openstack --os-compute-api-version 2.79 server show test-server -f yaml
  ...
  addresses: private=fdff:77e3:9bb4:0:f816:3eff:fe6d:a944, 10.0.0.44
  flavor: disk='1', ephemeral='0', extra_specs.hw_rng:allowed='True', original_name='m1.tiny',
    ram='512', swap='0', vcpus='1'
  ...

To after:

  $ openstack --os-compute-api-version 2.79 server show test-server -f yaml
  ...
  addresses:
    private:
    - fdff:77e3:9bb4:0:f816:3eff:fe6d:a944
    - 10.0.0.44
  flavor:
    disk: 1
    ephemeral: 0
    extra_specs:
      hw_rng:allowed: 'True'
    original_name: m1.tiny
    ram: 512
    swap: 0
    vcpus: 1
  ...

Similarly, compare before for 'server list':

  $ openstack --os-compute-api-version 2.79 server list -f yaml
  - ...
    Networks: private=fdff:77e3:9bb4:0:f816:3eff:fe6d:a944, 10.0.0.44
    Power State: Running
    Properties: ''
    ...

To after:

  $ openstack --os-compute-api-version 2.79 server list -f yaml
  - ...
    Networks:
      private:
      - fdff:77e3:9bb4:0:f816:3eff:fe6d:a944
      - 10.0.0.44
    Power State: 1
    Properties: {}
    ...

We also fix the human-readable output for the 'tags' field.

Before:

  $ openstack --os-compute-api-version 2.79 server list
  ...
  | tags   | ['bar', 'foo']  |

After:

  $ openstack --os-compute-api-version 2.79 server list
  ...
  | tags   | bar, foo  |

Change-Id: I7a8349106e211c57c4577b75326b39b88bd9ac1e
Signed-off-by: Stephen Finucane <sfinucan@redhat.com>

Author: stephenfin

openstackclient/compute/v2/server.py

@@ -21,6 +21,7 @@
 import logging
 import os
 
+from cliff import columns as cliff_columns
 import iso8601
 from novaclient import api_versions
 from novaclient.v2 import servers
@@ -41,28 +42,9 @@
 IMAGE_STRING_FOR_BFV = 'N/A (booted from volume)'
 
 
-def _format_servers_list_networks(networks):
-    """Return a formatted string of a server's networks
+class PowerStateColumn(cliff_columns.FormattableColumn):
+    """Generate a formatted string of a server's power state."""
 
-    :param networks: a Server.networks field
-    :rtype: a string of formatted network addresses
-    """
-    output = []
-    for (network, addresses) in networks.items():
-        if not addresses:
-            continue
-        addresses_csv = ', '.join(addresses)
-        group = "%s=%s" % (network, addresses_csv)
-        output.append(group)
-    return '; '.join(output)
-
-
-def _format_servers_list_power_state(state):
-    """Return a formatted string of a server's power state
-
-    :param state: the power state number of a server
-    :rtype: a string mapped to the power state number
-    """
     power_states = [
         'NOSTATE',      # 0x00
         'Running',      # 0x01
@@ -74,10 +56,11 @@ def _format_servers_list_power_state(state):
         'Suspended'     # 0x07
     ]
 
-    try:
-        return power_states[state]
-    except Exception:
-        return 'N/A'
+    def human_readable(self):
+        try:
+            return self.power_states[self._value]
+        except Exception:
+            return 'N/A'
 
 
 def _get_ip_address(addresses, address_type, ip_address_family):
@@ -169,7 +152,7 @@ def _prep_server_detail(compute_client, image_client, server, refresh=True):
         except Exception:
             info['flavor'] = flavor_id
     else:
-        info['flavor'] = utils.format_dict(flavor_info)
+        info['flavor'] = format_columns.DictColumn(flavor_info)
 
     if 'os-extended-volumes:volumes_attached' in info:
         info.update(
@@ -185,27 +168,23 @@ def _prep_server_detail(compute_client, image_client, server, refresh=True):
                     info.pop('security_groups'))
             }
         )
+    if 'tags' in info:
+        info.update({'tags': format_columns.ListColumn(info.pop('tags'))})
+
     # NOTE(dtroyer): novaclient splits these into separate entries...
     # Format addresses in a useful way
-    info['addresses'] = _format_servers_list_networks(server.networks)
+    info['addresses'] = format_columns.DictListColumn(server.networks)
 
     # Map 'metadata' field to 'properties'
-    if not info['metadata']:
-        info.update(
-            {'properties': utils.format_dict(info.pop('metadata'))}
-        )
-    else:
-        info.update(
-            {'properties': format_columns.DictColumn(info.pop('metadata'))}
-        )
+    info['properties'] = format_columns.DictColumn(info.pop('metadata'))
 
     # Migrate tenant_id to project_id naming
     if 'tenant_id' in info:
         info['project_id'] = info.pop('tenant_id')
 
     # Map power state num to meaningful string
     if 'OS-EXT-STS:power_state' in info:
-        info['OS-EXT-STS:power_state'] = _format_servers_list_power_state(
+        info['OS-EXT-STS:power_state'] = PowerStateColumn(
             info['OS-EXT-STS:power_state'])
 
     # Remove values that are long and not too useful
@@ -1873,10 +1852,9 @@ def take_action(self, parsed_args):
                     s, columns,
                     mixed_case_fields=mixed_case_fields,
                     formatters={
-                        'OS-EXT-STS:power_state':
-                            _format_servers_list_power_state,
-                        'Networks': _format_servers_list_networks,
-                        'Metadata': utils.format_dict,
+                        'OS-EXT-STS:power_state': PowerStateColumn,
+                        'Networks': format_columns.DictListColumn,
+                        'Metadata': format_columns.DictColumn,
                     },
                 ) for s in data
             ),

openstackclient/tests/functional/compute/v2/test_server.py

@@ -10,6 +10,7 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+import itertools
 import json
 import time
 import uuid
@@ -346,6 +347,14 @@ def test_server_attach_detach_floating_ip(self):
             #                DevStack without cells.
             self.skipTest("No Network service present")
 
+        def _chain_addresses(addresses):
+            # Flatten a dict of lists mapping network names to IP addresses,
+            # returning only the IP addresses
+            #
+            # >>> _chain_addresses({'private': ['10.1.0.32', '172.24.5.41']})
+            # ['10.1.0.32', '172.24.5.41']
+            return itertools.chain(*[*addresses.values()])
+
         cmd_output = self.server_create()
         name = cmd_output['name']
         self.wait_for_status(name, "ACTIVE")
@@ -387,7 +396,7 @@ def test_server_attach_detach_floating_ip(self):
                 'server show -f json ' +
                 name
             ))
-            if floating_ip not in cmd_output['addresses']:
+            if floating_ip not in _chain_addresses(cmd_output['addresses']):
                 # Hang out for a bit and try again
                 print('retrying floating IP check')
                 wait_time += 10
@@ -397,7 +406,7 @@ def test_server_attach_detach_floating_ip(self):
 
         self.assertIn(
             floating_ip,
-            cmd_output['addresses'],
+            _chain_addresses(cmd_output['addresses']),
         )
 
         # detach ip
@@ -417,7 +426,7 @@ def test_server_attach_detach_floating_ip(self):
                 'server show -f json ' +
                 name
             ))
-            if floating_ip in cmd_output['addresses']:
+            if floating_ip in _chain_addresses(cmd_output['addresses']):
                 # Hang out for a bit and try again
                 print('retrying floating IP check')
                 wait_time += 10
@@ -431,7 +440,7 @@ def test_server_attach_detach_floating_ip(self):
         ))
         self.assertNotIn(
             floating_ip,
-            cmd_output['addresses'],
+            _chain_addresses(cmd_output['addresses']),
         )
 
     def test_server_reboot(self):
@@ -856,8 +865,7 @@ def test_server_create_with_none_network(self):
         server = json.loads(self.openstack(
             'server show -f json ' + server_name
         ))
-        self.assertIsNotNone(server['addresses'])
-        self.assertEqual('', server['addresses'])
+        self.assertEqual({}, server['addresses'])
 
     def test_server_create_with_security_group(self):
         """Test server create with security group ID and name"""