Add optional argument to list subcommand

Cinder list command was intended to show summary of
volumes, but currently the command shows many entries
than we expected.

This patch adds these two fixes to show volume list
more simply.

- Remove 'migration status' and 'multiattach' from
  default list view.

- Add '--field' optional argument to 'list' subcommand.
  This argument allows users to show only specified
  entries. Unavailable fields will be ignored from
  the list result.

DocImpact
Change-Id: Iad4bf76e87f84b6e57ec59d134b9413fcad16ce8
Closes-Bug: #1502639
This commit is contained in:
Mitsuhiro Tanino
2015-11-12 20:57:40 -05:00
parent 2ca2b96723
commit c38b35cb61
3 changed files with 79 additions and 28 deletions

View File

@@ -25,22 +25,10 @@ import cinderclient.tests.unit.utils as utils
from cinderclient.v2 import client from cinderclient.v2 import client
def _stub_volume(**kwargs): def _stub_volume(*args, **kwargs):
volume = { volume = {
'id': '1234', "migration_status": None,
'name': None, "attachments": [{'server_id': 1234}],
'description': None,
"attachments": [],
"bootable": "false",
"availability_zone": "cinder",
"created_at": "2012-08-27T00:00:00.000000",
"id": '00000000-0000-0000-0000-000000000000',
"metadata": {},
"size": 1,
"snapshot_id": None,
"status": "available",
"volume_type": "None",
"multiattach": "false",
"links": [ "links": [
{ {
"href": "http://localhost/v2/fake/volumes/1234", "href": "http://localhost/v2/fake/volumes/1234",
@@ -51,6 +39,30 @@ def _stub_volume(**kwargs):
"rel": "bookmark" "rel": "bookmark"
} }
], ],
"availability_zone": "cinder",
"os-vol-host-attr:host": "ip-192-168-0-2",
"encrypted": "false",
"updated_at": "2013-11-12T21:00:00.000000",
"os-volume-replication:extended_status": "None",
"replication_status": "disabled",
"snapshot_id": None,
'id': 1234,
"size": 1,
"user_id": "1b2d6e8928954ca4ae7c243863404bdc",
"os-vol-tenant-attr:tenant_id": "eb72eb33a0084acf8eb21356c2b021a7",
"os-vol-mig-status-attr:migstat": None,
"metadata": {},
"status": "available",
'description': None,
"multiattach": "false",
"os-volume-replication:driver_data": None,
"source_volid": None,
"consistencygroup_id": None,
"os-vol-mig-status-attr:name_id": None,
"name": "sample-volume",
"bootable": "false",
"created_at": "2012-08-27T00:00:00.000000",
"volume_type": "None",
} }
volume.update(kwargs) volume.update(kwargs)
return volume return volume
@@ -385,13 +397,9 @@ class FakeHTTPClient(base_client.HTTPClient):
{'id': 5678, 'name': 'sample-volume2'} {'id': 5678, 'name': 'sample-volume2'}
]}) ]})
# TODO(jdg): This will need to change
# at the very least it's not complete
def get_volumes_detail(self, **kw): def get_volumes_detail(self, **kw):
return (200, {}, {"volumes": [ return (200, {}, {"volumes": [
{'id': kw.get('id', 1234), _stub_volume(id=kw.get('id', 1234))
'name': 'sample-volume',
'attachments': [{'server_id': 1234}]},
]}) ]})
def get_volumes_1234(self, **kw): def get_volumes_1234(self, **kw):

View File

@@ -210,6 +210,33 @@ class ShellTest(utils.TestCase):
self.run_command('list --limit=10') self.run_command('list --limit=10')
self.assert_called('GET', '/volumes/detail?limit=10') self.assert_called('GET', '/volumes/detail?limit=10')
@mock.patch("cinderclient.utils.print_list")
def test_list_field(self, mock_print):
self.run_command('list --field Status,Name,Size,Bootable')
self.assert_called('GET', '/volumes/detail')
key_list = ['ID', 'Status', 'Name', 'Size', 'Bootable']
mock_print.assert_called_once_with(mock.ANY, key_list,
exclude_unavailable=True, sortby_index=0)
@mock.patch("cinderclient.utils.print_list")
def test_list_field_with_all_tenants(self, mock_print):
self.run_command('list --field Status,Name,Size,Bootable '
'--all-tenants 1')
self.assert_called('GET', '/volumes/detail?all_tenants=1')
key_list = ['ID', 'Status', 'Name', 'Size', 'Bootable']
mock_print.assert_called_once_with(mock.ANY, key_list,
exclude_unavailable=True, sortby_index=0)
@mock.patch("cinderclient.utils.print_list")
def test_list_field_with_tenant(self, mock_print):
self.run_command('list --field Status,Name,Size,Bootable '
'--tenant 123')
self.assert_called('GET',
'/volumes/detail?all_tenants=1&project_id=123')
key_list = ['ID', 'Status', 'Name', 'Size', 'Bootable']
mock_print.assert_called_once_with(mock.ANY, key_list,
exclude_unavailable=True, sortby_index=0)
def test_list_sort_valid(self): def test_list_sort_valid(self):
self.run_command('list --sort_key=id --sort_dir=asc') self.run_command('list --sort_key=id --sort_dir=asc')
self.assert_called('GET', '/volumes/detail?sort_dir=asc&sort_key=id') self.assert_called('GET', '/volumes/detail?sort_dir=asc&sort_key=id')

View File

@@ -187,6 +187,13 @@ def _extract_metadata(args):
metavar='<limit>', metavar='<limit>',
default=None, default=None,
help='Maximum number of volumes to return. Default=None.') help='Maximum number of volumes to return. Default=None.')
@utils.arg('--fields',
default=None,
metavar='<fields>',
help='Comma-separated list of fields to display. '
'Use the show command to see which fields are available. '
'Unavailable/non-existent fields will be ignored. '
'Default=None.')
@utils.arg('--sort_key', @utils.arg('--sort_key',
metavar='<sort_key>', metavar='<sort_key>',
default=None, default=None,
@@ -226,6 +233,13 @@ def do_list(cs, args):
'metadata': _extract_metadata(args) if args.metadata else None, 'metadata': _extract_metadata(args) if args.metadata else None,
} }
# If unavailable/non-existent fields are specified, these fields will
# be removed from key_list at the print_list() during key validation.
field_titles = []
if args.fields:
for field_title in args.fields.split(','):
field_titles.append(field_title)
# --sort_key and --sort_dir deprecated in kilo and is not supported # --sort_key and --sort_dir deprecated in kilo and is not supported
# with --sort # with --sort
if args.sort and (args.sort_key or args.sort_dir): if args.sort and (args.sort_key or args.sort_dir):
@@ -243,14 +257,16 @@ def do_list(cs, args):
servers = [s.get('server_id') for s in vol.attachments] servers = [s.get('server_id') for s in vol.attachments]
setattr(vol, 'attached_to', ','.join(map(str, servers))) setattr(vol, 'attached_to', ','.join(map(str, servers)))
if all_tenants: if field_titles:
key_list = ['ID', 'Tenant ID', 'Status', 'Migration Status', 'Name', key_list = ['ID'] + field_titles
'Size', 'Volume Type', 'Bootable', 'Multiattach',
'Attached to']
else: else:
key_list = ['ID', 'Status', 'Migration Status', 'Name', key_list = ['ID', 'Status', 'Name', 'Size', 'Volume Type',
'Size', 'Volume Type', 'Bootable', 'Bootable', 'Attached to']
'Multiattach', 'Attached to'] # If all_tenants is specified, print
# Tenant ID as well.
if search_opts['all_tenants']:
key_list.insert(1, 'Tenant ID')
if args.sort_key or args.sort_dir or args.sort: if args.sort_key or args.sort_dir or args.sort:
sortby_index = None sortby_index = None
else: else: