openstack
/
metalsmith
Code Issues Proposed changes

Bring formatting in line with other OpenStack CLI 

Change-Id: Ifb299d5d95245e8848785eb2e2a7018fbb4129d6
This commit is contained in:
Dmitry Tantsur 2019-07-22 11:04:52 +02:00
parent 249b2ac0cc
commit 24d006b996
6 changed files with 225 additions and 88 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
lower-constraints.txt
View File

@ -7,6 +7,7 @@ hacking==1.0.0
mock==2.0
openstacksdk==0.29.0
pbr==2.0.0
PrettyTable==0.7.2
Pygments==2.2.0
requests==2.18.4
six==1.10.0

11
metalsmith/_cmd.py
View File

@ -122,9 +122,15 @@ def _parse_args(args, config):
'up to three times')
parser.add_argument('--dry-run', action='store_true',
help='do not take any destructive actions')
parser.add_argument('--format', choices=list(_format.FORMATS),
parser.add_argument('-f', '--format', choices=list(_format.FORMATS),
default=_format.DEFAULT_FORMAT,
help='output format')
parser.add_argument('-c', '--column', action='append', dest='columns',
choices=_format.FIELDS,
help='for table output, specify column(s) to show')
parser.add_argument('--sort-column', choices=_format.FIELDS,
help='for table output, specify a column to use '
'for sorting')
config.register_argparse_arguments(parser, sys.argv[1:])
@ -245,7 +251,8 @@ def main(args=sys.argv[1:]):
if args.quiet:
formatter = _format.NULL_FORMAT
else:
formatter = _format.FORMATS[args.format]
formatter = _format.FORMATS[args.format](columns=args.columns,
sort_column=args.sort_column)
region = config.get_one(argparse=args)
api = _provisioner.Provisioner(cloud_region=region, dry_run=args.dry_run)

82
metalsmith/_format.py
View File

@ -15,9 +15,12 @@
from __future__ import print_function
import collections
import json
import sys
import prettytable
from metalsmith import _utils
@ -31,6 +34,10 @@ class NullFormat(object):
Used implicitly with --quiet.
"""
def __init__(self, columns=None, sort_column=None):
self.columns = columns
self.sort_column = sort_column
def deploy(self, instance):
pass
@ -38,8 +45,12 @@ class NullFormat(object):
pass
class DefaultFormat(object):
"""Human-readable formatter."""
FIELDS = ['UUID', 'Node Name', 'Allocation UUID', 'Hostname',
'State', 'IP Addresses']
class ValueFormat(NullFormat):
""""Simple value formatter."""
def deploy(self, instance):
"""Output result of the deploy."""
@ -54,24 +65,55 @@ class DefaultFormat(object):
_print(message, node=_utils.log_res(node))
def show(self, instances):
def _iter_rows(self, instances):
for instance in instances:
_print("Node %(node)s, current state is %(state)s",
node=_utils.log_res(instance.node),
state=instance.state.name)
if instance.hostname:
_print('* Hostname: %(hostname)s', hostname=instance.hostname)
if instance.is_deployed:
ips = instance.ip_addresses()
if ips:
ips = '; '.join('%s=%s' % (net, ','.join(ips))
for net, ips in ips.items())
_print('* IP addresses: %(ips)s', ips=ips)
ips = '\n'.join('%s=%s' % (net, ','.join(ips))
for net, ips in
instance.ip_addresses().items())
else:
ips = ''
row = [instance.uuid, instance.node.name or '',
instance.allocation.id if instance.allocation else '',
instance.hostname or '', instance.state.name, ips]
yield row
def show(self, instances):
allowed_columns = set(self.columns or FIELDS)
rows = (collections.OrderedDict(zip(FIELDS, row))
for row in self._iter_rows(instances))
if self.sort_column:
rows = sorted(rows, key=lambda row: row.get(self.sort_column))
for row in rows:
_print(' '.join(value if value is not None else ''
for key, value in row.items()
if key in allowed_columns))
class JsonFormat(object):
class DefaultFormat(ValueFormat):
"""Human-readable formatter."""
def show(self, instances):
if not instances:
_print('') # Compatibility with openstackclient - one empty line
return
pt = prettytable.PrettyTable(field_names=FIELDS)
pt.align = 'l'
if self.sort_column:
pt.sortby = self.sort_column
for row in self._iter_rows(instances):
pt.add_row(row)
if self.columns:
value = pt.get_string(fields=self.columns)
else:
value = pt.get_string()
_print(value)
class JsonFormat(NullFormat):
"""JSON formatter."""
def deploy(self, instance):
@ -92,13 +134,15 @@ class JsonFormat(object):
FORMATS = {
'default': DefaultFormat(),
'json': JsonFormat()
'default': DefaultFormat,
'json': JsonFormat,
'table': DefaultFormat,
'value': ValueFormat,
}
"""Available formatters."""
DEFAULT_FORMAT = 'default'
DEFAULT_FORMAT = 'table'
"""Default formatter."""

211
metalsmith/test/test_cmd.py
View File

@ -39,9 +39,10 @@ class TestDeploy(testtools.TestCase):
self.os_conf_fixture = self.useFixture(fixtures.MockPatchObject(
_cmd.os_config, 'OpenStackConfig', autospec=True))
self.mock_os_conf = self.os_conf_fixture.mock
self._init = False
def _check(self, mock_pr, args, reserve_args, provision_args,
dry_run=False):
dry_run=False, formatter='value'):
reserve_defaults = dict(resource_class='compute',
conductor_group=None,
capabilities={},
@ -60,6 +61,11 @@ class TestDeploy(testtools.TestCase):
clean_up_on_failure=True)
provision_defaults.update(provision_args)
if not self._init:
self._init_instance(mock_pr)
if '--format' not in args and formatter:
args = ['--format', formatter] + args
_cmd.main(args)
mock_pr.assert_called_once_with(
@ -71,16 +77,23 @@ class TestDeploy(testtools.TestCase):
mock_pr.return_value.reserve_node.return_value,
**provision_defaults)
@mock.patch.object(_cmd, 'logging', autospec=True)
def test_args_ok(self, mock_log, mock_pr):
def _init_instance(self, mock_pr):
instance = mock_pr.return_value.provision_node.return_value
instance.create_autospec(_instance.Instance)
instance.uuid = '123'
instance.node.name = None
instance.node.id = '123'
instance.allocation.id = '321'
instance.state = _instance.InstanceState.ACTIVE
instance.is_deployed = True
instance.ip_addresses.return_value = {'private': ['1.2.3.4']}
instance.hostname = None
self._init = True
return instance
@mock.patch.object(_cmd, 'logging', autospec=True)
def test_args_ok(self, mock_log, mock_pr):
self._init_instance(mock_pr)
args = ['deploy', '--network', 'mynet', '--image', 'myimg',
'--resource-class', 'compute']
@ -101,21 +114,41 @@ class TestDeploy(testtools.TestCase):
mock_log.getLogger.mock_calls)
self.mock_print.assert_has_calls([
mock.call(mock.ANY, node='123', state='ACTIVE'),
mock.call(mock.ANY, ips='private=1.2.3.4')
mock.call('123 321 ACTIVE private=1.2.3.4'),
])
@mock.patch.object(_cmd, 'logging', autospec=True)
def test_args_default_format(self, mock_log, mock_pr):
self._init_instance(mock_pr)
args = ['deploy', '--network', 'mynet', '--image', 'myimg',
'--resource-class', 'compute']
self._check(mock_pr, args, {}, {}, formatter=None)
config = mock_pr.return_value.provision_node.call_args[1]['config']
self.assertEqual([], config.ssh_keys)
mock_log.basicConfig.assert_called_once_with(level=mock_log.WARNING,
format=mock.ANY)
source = mock_pr.return_value.provision_node.call_args[1]['image']
self.assertIsInstance(source, sources.GlanceImage)
self.assertEqual("myimg", source.image)
self.assertEqual(
mock.call('metalsmith').setLevel(mock_log.WARNING).call_list() +
mock.call(_cmd._URLLIB3_LOGGER).setLevel(
mock_log.CRITICAL).call_list(),
mock_log.getLogger.mock_calls)
@mock.patch.object(_cmd, 'logging', autospec=True)
def test_args_json_format(self, mock_log, mock_pr):
instance = mock_pr.return_value.provision_node.return_value
instance.create_autospec(_instance.Instance)
instance = self._init_instance(mock_pr)
instance.to_dict.return_value = {'node': 'dict'}
args = ['--format', 'json', 'deploy', '--network', 'mynet',
'--image', 'myimg', '--resource-class', 'compute']
args = ['deploy', '--network', 'mynet', '--image', 'myimg',
'--resource-class', 'compute']
fake_io = six.StringIO()
with mock.patch('sys.stdout', fake_io):
self._check(mock_pr, args, {}, {})
self._check(mock_pr, args, {}, {}, formatter='json')
self.assertEqual(json.loads(fake_io.getvalue()),
{'node': 'dict'})
@ -128,42 +161,34 @@ class TestDeploy(testtools.TestCase):
mock_log.getLogger.mock_calls)
def test_no_ips(self, mock_pr):
instance = mock_pr.return_value.provision_node.return_value
instance.create_autospec(_instance.Instance)
instance.is_deployed = True
instance = self._init_instance(mock_pr)
instance.ip_addresses.return_value = {}
instance.node.name = None
instance.node.id = '123'
instance.state = _instance.InstanceState.ACTIVE
instance.hostname = None
args = ['deploy', '--network', 'mynet', '--image', 'myimg',
'--resource-class', 'compute']
self._check(mock_pr, args, {}, {})
self.mock_print.assert_called_once_with(mock.ANY, node='123',
state='ACTIVE'),
self.mock_print.assert_has_calls([
mock.call('123 321 ACTIVE '),
])
def test_not_deployed_no_ips(self, mock_pr):
instance = mock_pr.return_value.provision_node.return_value
instance.create_autospec(_instance.Instance)
instance = self._init_instance(mock_pr)
instance.is_deployed = False
instance.node.name = None
instance.node.id = '123'
instance.state = _instance.InstanceState.DEPLOYING
instance.hostname = None
instance.ip_addresses.return_value = {}
args = ['deploy', '--network', 'mynet', '--image', 'myimg',
'--resource-class', 'compute']
self._check(mock_pr, args, {}, {})
self.mock_print.assert_called_once_with(mock.ANY, node='123',
state='DEPLOYING'),
self.mock_print.assert_has_calls([
mock.call('123 321 DEPLOYING '),
])
@mock.patch.object(_cmd.LOG, 'info', autospec=True)
def test_no_logs_not_deployed(self, mock_log, mock_pr):
instance = mock_pr.return_value.provision_node.return_value
instance.create_autospec(_instance.Instance)
instance = self._init_instance(mock_pr)
instance.is_deployed = False
args = ['deploy', '--network', 'mynet', '--image', 'myimg',
@ -361,23 +386,15 @@ class TestDeploy(testtools.TestCase):
self.assertFalse(mock_pr.return_value.provision_node.called)
def test_args_hostname(self, mock_pr):
instance = mock_pr.return_value.provision_node.return_value
instance.create_autospec(_instance.Instance)
instance.is_deployed = True
instance.node.name = None
instance.node.id = '123'
instance.state = _instance.InstanceState.ACTIVE
instance = self._init_instance(mock_pr)
instance.hostname = 'host'
instance.ip_addresses.return_value = {'private': ['1.2.3.4']}
args = ['deploy', '--network', 'mynet', '--image', 'myimg',
'--hostname', 'host', '--resource-class', 'compute']
self._check(mock_pr, args, {'hostname': 'host'}, {})
self.mock_print.assert_has_calls([
mock.call(mock.ANY, node='123', state='ACTIVE'),
mock.call(mock.ANY, hostname='host'),
mock.call(mock.ANY, ips='private=1.2.3.4')
mock.call('123 321 host ACTIVE private=1.2.3.4'),
])
def test_args_with_candidates(self, mock_pr):
@ -608,59 +625,98 @@ class TestShowWait(testtools.TestCase):
self.mock_print = self.print_fixture.mock
self.instances = [
mock.Mock(
spec=_instance.Instance, hostname=hostname,
uuid=hostname[-1], is_deployed=(hostname[-1] == '1'),
spec=_instance.Instance,
hostname='hostname%d' % i,
uuid=str(i),
is_deployed=(i == 1),
state=_instance.InstanceState.ACTIVE
if hostname[-1] == '1' else _instance.InstanceState.DEPLOYING,
**{'ip_addresses.return_value': {'private': ['1.2.3.4']}})
for hostname in ['hostname1', 'hostname2']
if i == 1 else _instance.InstanceState.DEPLOYING,
allocation=mock.Mock(spec=['id']) if i == 1 else None,
**{'ip_addresses.return_value': {'private': ['1.2.3.4']}}
)
for i in (1, 2)
]
for inst in self.instances:
inst.node.id = inst.uuid
inst.node.name = 'name-%s' % inst.uuid
if inst.allocation:
inst.allocation.id = '%s00' % inst.uuid
inst.to_dict.return_value = {inst.node.id: inst.node.name}
def test_show(self, mock_os_conf, mock_pr):
mock_pr.return_value.show_instances.return_value = self.instances
args = ['show', 'uuid1', 'hostname2']
args = ['--format', 'value', 'show', 'uuid1', 'hostname2']
_cmd.main(args)
self.mock_print.assert_has_calls([
mock.call(mock.ANY, node='name-1 (UUID 1)', state='ACTIVE'),
mock.call(mock.ANY, hostname='hostname1'),
mock.call(mock.ANY, ips='private=1.2.3.4'),
mock.call(mock.ANY, node='name-2 (UUID 2)', state='DEPLOYING'),
mock.call(mock.ANY, hostname='hostname2'),
mock.call('1 name-1 100 hostname1 ACTIVE private=1.2.3.4'),
mock.call('2 name-2 hostname2 DEPLOYING '),
])
mock_pr.return_value.show_instances.assert_called_once_with(
['uuid1', 'hostname2'])
def test_list(self, mock_os_conf, mock_pr):
mock_pr.return_value.list_instances.return_value = self.instances
args = ['list']
args = ['--format', 'value', 'list']
_cmd.main(args)
self.mock_print.assert_has_calls([
mock.call(mock.ANY, node='name-1 (UUID 1)', state='ACTIVE'),
mock.call(mock.ANY, hostname='hostname1'),
mock.call(mock.ANY, ips='private=1.2.3.4'),
mock.call(mock.ANY, node='name-2 (UUID 2)', state='DEPLOYING'),
mock.call(mock.ANY, hostname='hostname2'),
mock.call('1 name-1 100 hostname1 ACTIVE private=1.2.3.4'),
mock.call('2 name-2 hostname2 DEPLOYING '),
])
mock_pr.return_value.list_instances.assert_called_once_with()
def test_list_sort(self, mock_os_conf, mock_pr):
mock_pr.return_value.list_instances.return_value = self.instances
args = ['--format', 'value', '--sort-column', 'IP Addresses', 'list']
_cmd.main(args)
self.mock_print.assert_has_calls([
mock.call('2 name-2 hostname2 DEPLOYING '),
mock.call('1 name-1 100 hostname1 ACTIVE private=1.2.3.4'),
])
mock_pr.return_value.list_instances.assert_called_once_with()
def test_list_one_column(self, mock_os_conf, mock_pr):
mock_pr.return_value.list_instances.return_value = self.instances
args = ['--format', 'value', '--column', 'Node Name', 'list']
_cmd.main(args)
self.mock_print.assert_has_calls([
mock.call('name-1'),
mock.call('name-2'),
])
mock_pr.return_value.list_instances.assert_called_once_with()
def test_list_two_columns(self, mock_os_conf, mock_pr):
mock_pr.return_value.list_instances.return_value = self.instances
args = ['--format', 'value', '--column', 'Node Name',
'--column', 'Allocation UUID', 'list']
_cmd.main(args)
self.mock_print.assert_has_calls([
mock.call('name-1 100'),
mock.call('name-2 '),
])
mock_pr.return_value.list_instances.assert_called_once_with()
def test_list_empty(self, mock_os_conf, mock_pr):
mock_pr.return_value.list_instances.return_value = []
args = ['--format', 'value', 'list']
_cmd.main(args)
self.assertFalse(self.mock_print.called)
mock_pr.return_value.list_instances.assert_called_once_with()
def test_wait(self, mock_os_conf, mock_pr):
mock_pr.return_value.wait_for_provisioning.return_value = (
self.instances)
args = ['wait', 'uuid1', 'hostname2']
args = ['--format', 'value', 'wait', 'uuid1', 'hostname2']
_cmd.main(args)
self.mock_print.assert_has_calls([
mock.call(mock.ANY, node='name-1 (UUID 1)', state='ACTIVE'),
mock.call(mock.ANY, hostname='hostname1'),
mock.call(mock.ANY, ips='private=1.2.3.4'),
mock.call(mock.ANY, node='name-2 (UUID 2)', state='DEPLOYING'),
mock.call(mock.ANY, hostname='hostname2'),
mock.call('1 name-1 100 hostname1 ACTIVE private=1.2.3.4'),
mock.call('2 name-2 hostname2 DEPLOYING '),
])
mock_pr.return_value.wait_for_provisioning.assert_called_once_with(
['uuid1', 'hostname2'], timeout=None)
@ -668,19 +724,25 @@ class TestShowWait(testtools.TestCase):
def test_wait_custom_timeout(self, mock_os_conf, mock_pr):
mock_pr.return_value.wait_for_provisioning.return_value = (
self.instances)
args = ['wait', '--timeout', '42', 'uuid1', 'hostname2']
args = ['--format', 'value', 'wait', '--timeout', '42',
'uuid1', 'hostname2']
_cmd.main(args)
self.mock_print.assert_has_calls([
mock.call(mock.ANY, node='name-1 (UUID 1)', state='ACTIVE'),
mock.call(mock.ANY, hostname='hostname1'),
mock.call(mock.ANY, ips='private=1.2.3.4'),
mock.call(mock.ANY, node='name-2 (UUID 2)', state='DEPLOYING'),
mock.call(mock.ANY, hostname='hostname2'),
mock.call('1 name-1 100 hostname1 ACTIVE private=1.2.3.4'),
mock.call('2 name-2 hostname2 DEPLOYING '),
])
mock_pr.return_value.wait_for_provisioning.assert_called_once_with(
['uuid1', 'hostname2'], timeout=42)
def test_show_table(self, mock_os_conf, mock_pr):
mock_pr.return_value.show_instances.return_value = self.instances
args = ['show', 'uuid1', 'hostname2']
_cmd.main(args)
mock_pr.return_value.show_instances.assert_called_once_with(
['uuid1', 'hostname2'])
def test_show_json(self, mock_os_conf, mock_pr):
mock_pr.return_value.show_instances.return_value = self.instances
args = ['--format', 'json', 'show', 'uuid1', 'hostname2']
@ -692,6 +754,21 @@ class TestShowWait(testtools.TestCase):
{'hostname1': {'1': 'name-1'},
'hostname2': {'2': 'name-2'}})
def test_list_table(self, mock_os_conf, mock_pr):
mock_pr.return_value.list_instances.return_value = self.instances
args = ['list']
_cmd.main(args)
mock_pr.return_value.list_instances.assert_called_once_with()
def test_list_table_empty(self, mock_os_conf, mock_pr):
mock_pr.return_value.list_instances.return_value = []
args = ['list']
_cmd.main(args)
self.mock_print.assert_called_once_with('')
mock_pr.return_value.list_instances.assert_called_once_with()
def test_list_json(self, mock_os_conf, mock_pr):
mock_pr.return_value.list_instances.return_value = self.instances
args = ['--format', 'json', 'list']

7
releasenotes/notes/prettytable-1db78a96c1d921d0.yaml Normal file
View File

@ -0,0 +1,7 @@
---
features:
- |
The ``metalsmith`` CLI now uses table format similar to OpenStack CLI.
- |
The ``metalsmith`` CLI now supports the same ``-f``, ``-c`` and
``--sort-column`` arguments as other OpenStack CLI.

1
requirements.txt
View File

@ -6,3 +6,4 @@ openstacksdk>=0.29.0 # Apache-2.0
requests>=2.18.4 # Apache-2.0
six>=1.10.0 # MIT
enum34>=1.0.4;python_version=='2.7' or python_version=='2.6' or python_version=='3.3' # BSD
PrettyTable<0.8,>=0.7.2 # BSD