openstack/python-openstackclient
Code Issues Proposed changes

compute: Fix 'server * -f yaml' output

Browse Source 
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>
This commit is contained in:
Stephen Finucane 2020-11-04 10:14:00 +00:00
parent f2c49142f0
commit 03776d82e5
4 changed files with 99 additions and 115 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

52
openstackclient/compute/v2/server.py
View File

@ -21,6 +21,7 @@ import io
import logging import logging
import os import os
from cliff import columns as cliff_columns
import iso8601 import iso8601
from novaclient import api_versions from novaclient import api_versions
from novaclient.v2 import servers from novaclient.v2 import servers
@ -41,28 +42,9 @@ LOG = logging.getLogger(__name__)
IMAGE_STRING_FOR_BFV = 'N/A (booted from volume)' IMAGE_STRING_FOR_BFV = 'N/A (booted from volume)'
def _format_servers_list_networks(networks): class PowerStateColumn(cliff_columns.FormattableColumn):
"""Return a formatted string of a server's networks """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 = [ power_states = [
'NOSTATE', # 0x00 'NOSTATE', # 0x00
'Running', # 0x01 'Running', # 0x01
@ -74,8 +56,9 @@ def _format_servers_list_power_state(state):
'Suspended' # 0x07 'Suspended' # 0x07
] ]
def human_readable(self):
try: try:
return power_states[state] return self.power_states[self._value]
except Exception: except Exception:
return 'N/A' return 'N/A'
@ -169,7 +152,7 @@ def _prep_server_detail(compute_client, image_client, server, refresh=True):
except Exception: except Exception:
info['flavor'] = flavor_id info['flavor'] = flavor_id
else: else:
info['flavor'] = utils.format_dict(flavor_info) info['flavor'] = format_columns.DictColumn(flavor_info)
if 'os-extended-volumes:volumes_attached' in info: if 'os-extended-volumes:volumes_attached' in info:
info.update( info.update(
@ -185,19 +168,15 @@ def _prep_server_detail(compute_client, image_client, server, refresh=True):
info.pop('security_groups')) 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... # NOTE(dtroyer): novaclient splits these into separate entries...
# Format addresses in a useful way # 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' # Map 'metadata' field to 'properties'
if not info['metadata']: info['properties'] = format_columns.DictColumn(info.pop('metadata'))
info.update(
{'properties': utils.format_dict(info.pop('metadata'))}
)
else:
info.update(
{'properties': format_columns.DictColumn(info.pop('metadata'))}
)
# Migrate tenant_id to project_id naming # Migrate tenant_id to project_id naming
if 'tenant_id' in info: if 'tenant_id' in info:
@ -205,7 +184,7 @@ def _prep_server_detail(compute_client, image_client, server, refresh=True):
# Map power state num to meaningful string # Map power state num to meaningful string
if 'OS-EXT-STS:power_state' in info: 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']) info['OS-EXT-STS:power_state'])
# Remove values that are long and not too useful # Remove values that are long and not too useful
@ -1873,10 +1852,9 @@ class ListServer(command.Lister):
s, columns, s, columns,
mixed_case_fields=mixed_case_fields, mixed_case_fields=mixed_case_fields,
formatters={ formatters={
'OS-EXT-STS:power_state': 'OS-EXT-STS:power_state': PowerStateColumn,
_format_servers_list_power_state, 'Networks': format_columns.DictListColumn,
'Networks': _format_servers_list_networks, 'Metadata': format_columns.DictColumn,
'Metadata': utils.format_dict,
}, },
) for s in data ) for s in data
), ),

20
openstackclient/tests/functional/compute/v2/test_server.py
View File

@ -10,6 +10,7 @@
# License for the specific language governing permissions and limitations # License for the specific language governing permissions and limitations
# under the License. # under the License.
import itertools
import json import json
import time import time
import uuid import uuid
@ -346,6 +347,14 @@ class ServerTests(common.ComputeTestCase):
# DevStack without cells. # DevStack without cells.
self.skipTest("No Network service present") 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() cmd_output = self.server_create()
name = cmd_output['name'] name = cmd_output['name']
self.wait_for_status(name, "ACTIVE") self.wait_for_status(name, "ACTIVE")
@ -387,7 +396,7 @@ class ServerTests(common.ComputeTestCase):
'server show -f json ' + 'server show -f json ' +
name 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 # Hang out for a bit and try again
print('retrying floating IP check') print('retrying floating IP check')
wait_time += 10 wait_time += 10
@ -397,7 +406,7 @@ class ServerTests(common.ComputeTestCase):
self.assertIn( self.assertIn(
floating_ip, floating_ip,
cmd_output['addresses'], _chain_addresses(cmd_output['addresses']),
) )
# detach ip # detach ip
@ -417,7 +426,7 @@ class ServerTests(common.ComputeTestCase):
'server show -f json ' + 'server show -f json ' +
name 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 # Hang out for a bit and try again
print('retrying floating IP check') print('retrying floating IP check')
wait_time += 10 wait_time += 10
@ -431,7 +440,7 @@ class ServerTests(common.ComputeTestCase):
)) ))
self.assertNotIn( self.assertNotIn(
floating_ip, floating_ip,
cmd_output['addresses'], _chain_addresses(cmd_output['addresses']),
) )
def test_server_reboot(self): def test_server_reboot(self):
@ -856,8 +865,7 @@ class ServerTests(common.ComputeTestCase):
server = json.loads(self.openstack( server = json.loads(self.openstack(
'server show -f json ' + server_name '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): def test_server_create_with_security_group(self):
"""Test server create with security group ID and name""" """Test server create with security group ID and name"""

126
openstackclient/tests/unit/compute/v2/test_server.py
View File

@ -22,6 +22,7 @@ from unittest.mock import call
import iso8601 import iso8601
from novaclient import api_versions from novaclient import api_versions
from openstack import exceptions as sdk_exceptions from openstack import exceptions as sdk_exceptions
from osc_lib.cli import format_columns
from osc_lib import exceptions from osc_lib import exceptions
from osc_lib import utils as common_utils from osc_lib import utils as common_utils
@ -33,6 +34,29 @@ from openstackclient.tests.unit import utils
from openstackclient.tests.unit.volume.v2 import fakes as volume_fakes from openstackclient.tests.unit.volume.v2 import fakes as volume_fakes
class TestPowerStateColumn(utils.TestCase):
def test_human_readable(self):
self.assertEqual(
'NOSTATE', server.PowerStateColumn(0x00).human_readable())
self.assertEqual(
'Running', server.PowerStateColumn(0x01).human_readable())
self.assertEqual(
'', server.PowerStateColumn(0x02).human_readable())
self.assertEqual(
'Paused', server.PowerStateColumn(0x03).human_readable())
self.assertEqual(
'Shutdown', server.PowerStateColumn(0x04).human_readable())
self.assertEqual(
'', server.PowerStateColumn(0x05).human_readable())
self.assertEqual(
'Crashed', server.PowerStateColumn(0x06).human_readable())
self.assertEqual(
'Suspended', server.PowerStateColumn(0x07).human_readable())
self.assertEqual(
'N/A', server.PowerStateColumn(0x08).human_readable())
class TestServer(compute_fakes.TestComputev2): class TestServer(compute_fakes.TestComputev2):
def setUp(self): def setUp(self):
@ -1015,15 +1039,15 @@ class TestServerCreate(TestServer):
def datalist(self): def datalist(self):
datalist = ( datalist = (
server._format_servers_list_power_state( server.PowerStateColumn(
getattr(self.new_server, 'OS-EXT-STS:power_state')), getattr(self.new_server, 'OS-EXT-STS:power_state')),
'', format_columns.DictListColumn({}),
self.flavor.name + ' (' + self.new_server.flavor.get('id') + ')', self.flavor.name + ' (' + self.new_server.flavor.get('id') + ')',
self.new_server.id, self.new_server.id,
self.image.name + ' (' + self.new_server.image.get('id') + ')', self.image.name + ' (' + self.new_server.image.get('id') + ')',
self.new_server.name, self.new_server.name,
self.new_server.networks, self.new_server.networks,
'', format_columns.DictColumn(self.new_server.metadata),
) )
return datalist return datalist
@ -3041,7 +3065,7 @@ class TestServerList(TestServer):
s.id, s.id,
s.name, s.name,
s.status, s.status,
server._format_servers_list_networks(s.networks), format_columns.DictListColumn(s.networks),
# Image will be an empty string if boot-from-volume # Image will be an empty string if boot-from-volume
self.image.name if s.image else server.IMAGE_STRING_FOR_BFV, self.image.name if s.image else server.IMAGE_STRING_FOR_BFV,
self.flavor.name, self.flavor.name,
@ -3051,10 +3075,10 @@ class TestServerList(TestServer):
s.name, s.name,
s.status, s.status,
getattr(s, 'OS-EXT-STS:task_state'), getattr(s, 'OS-EXT-STS:task_state'),
server._format_servers_list_power_state( server.PowerStateColumn(
getattr(s, 'OS-EXT-STS:power_state') getattr(s, 'OS-EXT-STS:power_state')
), ),
server._format_servers_list_networks(s.networks), format_columns.DictListColumn(s.networks),
# Image will be an empty string if boot-from-volume # Image will be an empty string if boot-from-volume
self.image.name if s.image else server.IMAGE_STRING_FOR_BFV, self.image.name if s.image else server.IMAGE_STRING_FOR_BFV,
s.image['id'] if s.image else server.IMAGE_STRING_FOR_BFV, s.image['id'] if s.image else server.IMAGE_STRING_FOR_BFV,
@ -3062,13 +3086,13 @@ class TestServerList(TestServer):
s.flavor['id'], s.flavor['id'],
getattr(s, 'OS-EXT-AZ:availability_zone'), getattr(s, 'OS-EXT-AZ:availability_zone'),
getattr(s, 'OS-EXT-SRV-ATTR:host'), getattr(s, 'OS-EXT-SRV-ATTR:host'),
s.Metadata, format_columns.DictColumn({}),
)) ))
self.data_no_name_lookup.append(( self.data_no_name_lookup.append((
s.id, s.id,
s.name, s.name,
s.status, s.status,
server._format_servers_list_networks(s.networks), format_columns.DictListColumn(s.networks),
# Image will be an empty string if boot-from-volume # Image will be an empty string if boot-from-volume
s.image['id'] if s.image else server.IMAGE_STRING_FOR_BFV, s.image['id'] if s.image else server.IMAGE_STRING_FOR_BFV,
s.flavor['id'] s.flavor['id']
@ -3128,7 +3152,7 @@ class TestServerList(TestServer):
self.servers_mock.list.assert_called_with(**self.kwargs) self.servers_mock.list.assert_called_with(**self.kwargs)
self.assertEqual(self.columns_long, columns) self.assertEqual(self.columns_long, columns)
self.assertEqual(tuple(self.data_long), tuple(data)) self.assertCountEqual(tuple(self.data_long), tuple(data))
def test_server_list_column_option(self): def test_server_list_column_option(self):
arglist = [ arglist = [
@ -3429,9 +3453,11 @@ class TestServerList(TestServer):
def test_server_list_v269_with_partial_constructs(self): def test_server_list_v269_with_partial_constructs(self):
self.app.client_manager.compute.api_version = \ self.app.client_manager.compute.api_version = \
api_versions.APIVersion('2.69') api_versions.APIVersion('2.69')
arglist = [] arglist = []
verifylist = [] verifylist = []
parsed_args = self.check_parser(self.cmd, arglist, verifylist) parsed_args = self.check_parser(self.cmd, arglist, verifylist)
# include "partial results" from non-responsive part of # include "partial results" from non-responsive part of
# infrastructure. # infrastructure.
server_dict = { server_dict = {
@ -3454,10 +3480,10 @@ class TestServerList(TestServer):
# it will fail at formatting the networks info later on. # it will fail at formatting the networks info later on.
"networks": {} "networks": {}
} }
server = compute_fakes.fakes.FakeResource( _server = compute_fakes.fakes.FakeResource(
info=server_dict, info=server_dict,
) )
self.servers.append(server) self.servers.append(_server)
columns, data = self.cmd.take_action(parsed_args) columns, data = self.cmd.take_action(parsed_args)
# get the first three servers out since our interest is in the partial # get the first three servers out since our interest is in the partial
# server. # server.
@ -3466,8 +3492,13 @@ class TestServerList(TestServer):
next(data) next(data)
partial_server = next(data) partial_server = next(data)
expected_row = ( expected_row = (
'server-id-95a56bfc4xxxxxx28d7e418bfd97813a', '', 'server-id-95a56bfc4xxxxxx28d7e418bfd97813a',
'UNKNOWN', '', '', '') '',
'UNKNOWN',
format_columns.DictListColumn({}),
'',
'',
)
self.assertEqual(expected_row, partial_server) self.assertEqual(expected_row, partial_server)
def test_server_list_with_tag(self): def test_server_list_with_tag(self):
@ -6292,15 +6323,16 @@ class TestServerShow(TestServer):
) )
self.data = ( self.data = (
'Running', server.PowerStateColumn(
'public=10.20.30.40, 2001:db8::f', getattr(self.server, 'OS-EXT-STS:power_state')),
format_columns.DictListColumn(self.server.networks),
self.flavor.name + " (" + self.flavor.id + ")", self.flavor.name + " (" + self.flavor.id + ")",
self.server.id, self.server.id,
self.image.name + " (" + self.image.id + ")", self.image.name + " (" + self.image.id + ")",
self.server.name, self.server.name,
{'public': ['10.20.30.40', '2001:db8::f']}, {'public': ['10.20.30.40', '2001:db8::f']},
'tenant-id-xxx', 'tenant-id-xxx',
'', format_columns.DictColumn({}),
) )
def test_show_no_options(self): def test_show_no_options(self):
@ -6323,7 +6355,7 @@ class TestServerShow(TestServer):
columns, data = self.cmd.take_action(parsed_args) columns, data = self.cmd.take_action(parsed_args)
self.assertEqual(self.columns, columns) self.assertEqual(self.columns, columns)
self.assertEqual(self.data, data) self.assertCountEqual(self.data, data)
def test_show_embedded_flavor(self): def test_show_embedded_flavor(self):
# Tests using --os-compute-api-version >= 2.47 where the flavor # Tests using --os-compute-api-version >= 2.47 where the flavor
@ -6350,7 +6382,7 @@ class TestServerShow(TestServer):
self.assertEqual(self.columns, columns) self.assertEqual(self.columns, columns)
# Since the flavor details are in a dict we can't be sure of the # Since the flavor details are in a dict we can't be sure of the
# ordering so just assert that one of the keys is in the output. # ordering so just assert that one of the keys is in the output.
self.assertIn('original_name', data[2]) self.assertIn('original_name', data[2]._value)
def test_show_diagnostics(self): def test_show_diagnostics(self):
arglist = [ arglist = [
@ -6719,50 +6751,6 @@ class TestServerGeneral(TestServer):
self.assertRaises(exceptions.CommandError, self.assertRaises(exceptions.CommandError,
server._get_ip_address, self.OLD, 'private', [6]) server._get_ip_address, self.OLD, 'private', [6])
def test_format_servers_list_power_state(self):
self.assertEqual("NOSTATE",
server._format_servers_list_power_state(0x00))
self.assertEqual("Running",
server._format_servers_list_power_state(0x01))
self.assertEqual("",
server._format_servers_list_power_state(0x02))
self.assertEqual("Paused",
server._format_servers_list_power_state(0x03))
self.assertEqual("Shutdown",
server._format_servers_list_power_state(0x04))
self.assertEqual("",
server._format_servers_list_power_state(0x05))
self.assertEqual("Crashed",
server._format_servers_list_power_state(0x06))
self.assertEqual("Suspended",
server._format_servers_list_power_state(0x07))
self.assertEqual("N/A",
server._format_servers_list_power_state(0x08))
def test_format_servers_list_networks(self):
# Setup network info to test.
networks = {
u'public': [u'10.20.30.40', u'2001:db8::f'],
u'private': [u'2001:db8::f', u'10.20.30.40'],
}
# Prepare expected data.
# Since networks is a dict, whose items are in random order, there
# could be two results after formatted.
data_1 = (u'private=2001:db8::f, 10.20.30.40; '
u'public=10.20.30.40, 2001:db8::f')
data_2 = (u'public=10.20.30.40, 2001:db8::f; '
u'private=2001:db8::f, 10.20.30.40')
# Call _format_servers_list_networks().
networks_format = server._format_servers_list_networks(networks)
msg = ('Network string is not formatted correctly.\n'
'reference = %s or %s\n'
'actual = %s\n' %
(data_1, data_2, networks_format))
self.assertIn(networks_format, (data_1, data_2), msg)
@mock.patch('osc_lib.utils.find_resource') @mock.patch('osc_lib.utils.find_resource')
def test_prep_server_detail(self, find_resource): def test_prep_server_detail(self, find_resource):
# Setup mock method return value. utils.find_resource() will be called # Setup mock method return value. utils.find_resource() will be called
@ -6789,14 +6777,14 @@ class TestServerGeneral(TestServer):
info = { info = {
'id': _server.id, 'id': _server.id,
'name': _server.name, 'name': _server.name,
'addresses': u'public=10.20.30.40, 2001:db8::f', 'image': '%s (%s)' % (_image.name, _image.id),
'flavor': u'%s (%s)' % (_flavor.name, _flavor.id), 'flavor': '%s (%s)' % (_flavor.name, _flavor.id),
'image': u'%s (%s)' % (_image.name, _image.id), 'OS-EXT-STS:power_state': server.PowerStateColumn(
'project_id': u'tenant-id-xxx',
'properties': '',
'OS-EXT-STS:power_state': server._format_servers_list_power_state(
getattr(_server, 'OS-EXT-STS:power_state')), getattr(_server, 'OS-EXT-STS:power_state')),
'properties': '',
'volumes_attached': [{"id": "6344fe9d-ef20-45b2-91a6"}], 'volumes_attached': [{"id": "6344fe9d-ef20-45b2-91a6"}],
'addresses': format_columns.DictListColumn(_server.networks),
'project_id': 'tenant-id-xxx',
} }
# Call _prep_server_detail(). # Call _prep_server_detail().
@ -6809,4 +6797,4 @@ class TestServerGeneral(TestServer):
server_detail.pop('networks') server_detail.pop('networks')
# Check the results. # Check the results.
self.assertEqual(info, server_detail) self.assertCountEqual(info, server_detail)

10
releasenotes/notes/improved-server-output-6965b664f6abda8d.yaml Normal file
View File

@ -0,0 +1,10 @@
---
fixes:
- |
The ``addresses`` and ``flavor`` fields of the ``server show`` command will
now be correctly rendered as a list of objects and an object, respectively.
- |
The ``networks`` and ``properties`` fields of the ``server list`` command
will now be rendered as objects. In addition, the ``power_state`` field
will now be humanized and rendered as a string value when using the table
formatter.