From c38b35cb61baff1f9efbd61508890cdf632e188f Mon Sep 17 00:00:00 2001 From: Mitsuhiro Tanino Date: Thu, 12 Nov 2015 20:57:40 -0500 Subject: [PATCH] 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 --- cinderclient/tests/unit/v2/fakes.py | 50 ++++++++++++++---------- cinderclient/tests/unit/v2/test_shell.py | 27 +++++++++++++ cinderclient/v2/shell.py | 30 ++++++++++---- 3 files changed, 79 insertions(+), 28 deletions(-) diff --git a/cinderclient/tests/unit/v2/fakes.py b/cinderclient/tests/unit/v2/fakes.py index 4093d109a..f9dd2e22b 100644 --- a/cinderclient/tests/unit/v2/fakes.py +++ b/cinderclient/tests/unit/v2/fakes.py @@ -25,22 +25,10 @@ import cinderclient.tests.unit.utils as utils from cinderclient.v2 import client -def _stub_volume(**kwargs): +def _stub_volume(*args, **kwargs): volume = { - 'id': '1234', - 'name': None, - '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", + "migration_status": None, + "attachments": [{'server_id': 1234}], "links": [ { "href": "http://localhost/v2/fake/volumes/1234", @@ -51,7 +39,31 @@ def _stub_volume(**kwargs): "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) return volume @@ -385,13 +397,9 @@ class FakeHTTPClient(base_client.HTTPClient): {'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): return (200, {}, {"volumes": [ - {'id': kw.get('id', 1234), - 'name': 'sample-volume', - 'attachments': [{'server_id': 1234}]}, + _stub_volume(id=kw.get('id', 1234)) ]}) def get_volumes_1234(self, **kw): diff --git a/cinderclient/tests/unit/v2/test_shell.py b/cinderclient/tests/unit/v2/test_shell.py index 0775f0c4d..0df043720 100644 --- a/cinderclient/tests/unit/v2/test_shell.py +++ b/cinderclient/tests/unit/v2/test_shell.py @@ -210,6 +210,33 @@ class ShellTest(utils.TestCase): self.run_command('list --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): self.run_command('list --sort_key=id --sort_dir=asc') self.assert_called('GET', '/volumes/detail?sort_dir=asc&sort_key=id') diff --git a/cinderclient/v2/shell.py b/cinderclient/v2/shell.py index 9940d2495..f86966054 100644 --- a/cinderclient/v2/shell.py +++ b/cinderclient/v2/shell.py @@ -187,6 +187,13 @@ def _extract_metadata(args): metavar='', default=None, help='Maximum number of volumes to return. Default=None.') +@utils.arg('--fields', + default=None, + metavar='', + 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', metavar='', default=None, @@ -226,6 +233,13 @@ def do_list(cs, args): '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 # with --sort 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] setattr(vol, 'attached_to', ','.join(map(str, servers))) - if all_tenants: - key_list = ['ID', 'Tenant ID', 'Status', 'Migration Status', 'Name', - 'Size', 'Volume Type', 'Bootable', 'Multiattach', - 'Attached to'] + if field_titles: + key_list = ['ID'] + field_titles else: - key_list = ['ID', 'Status', 'Migration Status', 'Name', - 'Size', 'Volume Type', 'Bootable', - 'Multiattach', 'Attached to'] + key_list = ['ID', 'Status', 'Name', 'Size', 'Volume Type', + 'Bootable', '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: sortby_index = None else: