From 3a7609d8fabd999e3ed260601488104ebcb96ce1 Mon Sep 17 00:00:00 2001 From: nidhimittalhada Date: Tue, 3 May 2016 16:16:41 +0530 Subject: [PATCH] manila list --all-tenants should display projectID Both 'manila list --all-tenants' and 'manila list --public' should show column Project ID. Project/Tenant ownership of shares is vital information, e.g. when working with project quota allocations. 'manila list' doesn't show public shares of other projects. Without '--all-tenants' and without '--public' the result is scoped to the project/tenant of the user issuing the command. So, it would be redundant to display a column for this. Same fix has been put for snapshot-list --all-tenants too. DocImpact Change-Id: I7afae542c391041d48c28e268ab59c83447bb940 Closes-Bug: #1570085 --- manilaclient/api_versions.py | 2 +- manilaclient/tests/functional/client.py | 6 ++- .../tests/functional/test_shares_listing.py | 51 ++++++++++++++++--- manilaclient/tests/unit/v2/test_shell.py | 28 +++++++++- manilaclient/v2/shell.py | 26 ++++++---- .../bug_1570085_fix-905786b797379309.yaml | 5 ++ 6 files changed, 97 insertions(+), 21 deletions(-) create mode 100644 releasenotes/notes/bug_1570085_fix-905786b797379309.yaml diff --git a/manilaclient/api_versions.py b/manilaclient/api_versions.py index 66d696a6d..2af5be0d1 100644 --- a/manilaclient/api_versions.py +++ b/manilaclient/api_versions.py @@ -27,7 +27,7 @@ from manilaclient import utils LOG = logging.getLogger(__name__) -MAX_VERSION = '2.15' +MAX_VERSION = '2.17' MIN_VERSION = '2.0' DEPRECATED_VERSION = '1.0' _VERSIONED_METHOD_MAP = {} diff --git a/manilaclient/tests/functional/client.py b/manilaclient/tests/functional/client.py index dab7f2e44..d07d22292 100644 --- a/manilaclient/tests/functional/client.py +++ b/manilaclient/tests/functional/client.py @@ -543,7 +543,7 @@ class ManilaCLIClient(base.CLIClient): return self.manila(cmd, microversion=microversion) def list_shares(self, all_tenants=False, filters=None, columns=None, - microversion=None): + is_public=False, microversion=None): """List shares. :param all_tenants: bool -- whether to list shares that belong @@ -557,10 +557,14 @@ class ManilaCLIClient(base.CLIClient): will be transformed to filter parameter "--project-id=foo" :param columns: comma separated string of columns. Example, "--columns Name,Size" + :param is_public: bool -- should list public shares or not. + Default is False. """ cmd = 'list ' if all_tenants: cmd += '--all-tenants ' + if is_public: + cmd += '--public ' if filters and isinstance(filters, dict): for k, v in filters.items(): cmd += '%(k)s=%(v)s ' % { diff --git a/manilaclient/tests/functional/test_shares_listing.py b/manilaclient/tests/functional/test_shares_listing.py index 8f572c704..31cd2182f 100644 --- a/manilaclient/tests/functional/test_shares_listing.py +++ b/manilaclient/tests/functional/test_shares_listing.py @@ -109,6 +109,19 @@ class SharesListReadWriteTest(base.BaseTestCase): cls.public_description = data_utils.rand_name( 'autotest_public_share_description') + cls.admin_private_name = data_utils.rand_name( + 'autotest_admin_private_share_name') + cls.admin_private_description = data_utils.rand_name( + 'autotest_admin_private_share_description') + + cls.admin_private_share = cls.create_share( + name=cls.admin_private_name, + description=cls.admin_private_description, + public=False, + cleanup_in_class=True, + client=None, + wait_for_creation=False) + cls.private_share = cls.create_share( name=cls.private_name, description=cls.private_description, @@ -124,7 +137,8 @@ class SharesListReadWriteTest(base.BaseTestCase): client=cls.get_user_client(), cleanup_in_class=True) - for share_id in (cls.private_share['id'], cls.public_share['id']): + for share_id in (cls.private_share['id'], cls.public_share['id'], + cls.admin_private_share['id']): cls.get_admin_client().wait_for_share_status(share_id, 'available') def _list_shares(self, filters=None): @@ -153,11 +167,34 @@ class SharesListReadWriteTest(base.BaseTestCase): def test_list_shares(self): self._list_shares() - def test_list_shares_for_all_tenants(self): - shares = self.user_client.list_shares(True) + @ddt.data(1, 0) + def test_list_shares_for_all_tenants(self, all_tenants): + shares = self.admin_client.list_shares(all_tenants=all_tenants) + self.assertTrue(len(shares) >= 1) + + if all_tenants: + self.assertTrue(all('Project ID' in s for s in shares)) + for s_id in (self.private_share['id'], self.public_share['id'], + self.admin_private_share['id']): + self.assertTrue(any(s_id == s['ID'] for s in shares)) + else: + self.assertTrue(all('Project ID' not in s for s in shares)) + self.assertTrue(any(self.admin_private_share['id'] == s['ID'] + for s in shares)) + if self.private_share['project_id'] != ( + self.admin_private_share['project_id']): + for s_id in ( + self.private_share['id'], self.public_share['id']): + self.assertFalse(any(s_id == s['ID'] for s in shares)) + + @ddt.data(True, False) + def test_list_shares_with_public(self, public): + shares = self.user_client.list_shares(is_public=public) self.assertTrue(len(shares) > 1) - for s_id in (self.private_share['id'], self.public_share['id']): - self.assertTrue(any(s_id == s['ID'] for s in shares)) + if public: + self.assertTrue(all('Project ID' in s for s in shares)) + else: + self.assertTrue(all('Project ID' not in s for s in shares)) def test_list_shares_by_name(self): shares = self.user_client.list_shares( @@ -180,8 +217,8 @@ class SharesListReadWriteTest(base.BaseTestCase): self._list_shares({'status': 'available'}) def test_list_shares_by_project_id(self): - project_id = self.admin_client.get_project_id( - self.admin_client.tenant_name) + project_id = self.user_client.get_project_id( + self.user_client.tenant_name) self._list_shares({'project_id': project_id}) @testtools.skipUnless( diff --git a/manilaclient/tests/unit/v2/test_shell.py b/manilaclient/tests/unit/v2/test_shell.py index 5a21170af..4586d03f2 100644 --- a/manilaclient/tests/unit/v2/test_shell.py +++ b/manilaclient/tests/unit/v2/test_shell.py @@ -147,9 +147,30 @@ class ShellTest(test_utils.TestCase): self.run_command('list --name' + separator + '1234') self.assert_called('GET', '/shares/detail?name=1234') + @mock.patch.object(cliutils, 'print_list', mock.Mock()) def test_list_all_tenants_only_key(self): self.run_command('list --all-tenants') self.assert_called('GET', '/shares/detail?all_tenants=1') + cliutils.print_list.assert_called_once_with( + mock.ANY, + ['ID', 'Name', 'Size', 'Share Proto', 'Status', 'Is Public', + 'Share Type Name', 'Host', 'Availability Zone', 'Project ID']) + + @mock.patch.object(cliutils, 'print_list', mock.Mock()) + def test_list_select_column_and_all_tenants(self): + self.run_command('list --columns ID,Name --all-tenants') + self.assert_called('GET', '/shares/detail?all_tenants=1') + cliutils.print_list.assert_called_once_with( + mock.ANY, + ['Id', 'Name']) + + @mock.patch.object(cliutils, 'print_list', mock.Mock()) + def test_list_select_column_and_public(self): + self.run_command('list --columns ID,Name --public') + self.assert_called('GET', '/shares/detail?is_public=True') + cliutils.print_list.assert_called_once_with( + mock.ANY, + ['Id', 'Name']) def test_list_all_tenants_key_and_value_1(self): for separator in self.separators: @@ -452,7 +473,8 @@ class ShellTest(test_utils.TestCase): 'Is Public', 'Share Type Name', 'Host', - 'Availability Zone' + 'Availability Zone', + 'Project ID' ] self.run_command('list --public') self.assert_called('GET', '/shares/detail?is_public=True') @@ -609,9 +631,13 @@ class ShellTest(test_utils.TestCase): mock.ANY, ['Id', 'Name']) + @mock.patch.object(cliutils, 'print_list', mock.Mock()) def test_list_snapshots_all_tenants_only_key(self): self.run_command('snapshot-list --all-tenants') self.assert_called('GET', '/snapshots/detail?all_tenants=1') + cliutils.print_list.assert_called_once_with( + mock.ANY, + ['ID', 'Share ID', 'Status', 'Name', 'Share Size', 'Project ID']) def test_list_snapshots_all_tenants_key_and_value_1(self): for separator in self.separators: diff --git a/manilaclient/v2/shell.py b/manilaclient/v2/shell.py index 02a039478..d906b3cec 100644 --- a/manilaclient/v2/shell.py +++ b/manilaclient/v2/shell.py @@ -1281,16 +1281,18 @@ def do_access_list(cs, args): def do_list(cs, args): """List NAS shares with filters.""" - list_of_keys = [ - 'ID', 'Name', 'Size', 'Share Proto', 'Status', 'Is Public', - 'Share Type Name', 'Host', 'Availability Zone' - ] - columns = args.columns + all_tenants = int(os.environ.get("ALL_TENANTS", args.all_tenants)) if columns is not None: list_of_keys = _split_columns(columns=columns) + else: + list_of_keys = [ + 'ID', 'Name', 'Size', 'Share Proto', 'Status', 'Is Public', + 'Share Type Name', 'Host', 'Availability Zone' + ] + if all_tenants or args.public: + list_of_keys.append('Project ID') - all_tenants = int(os.environ.get("ALL_TENANTS", args.all_tenants)) empty_obj = type('Empty', (object,), {'id': None}) share_type = (_find_share_type(cs, args.share_type) if args.share_type else empty_obj) @@ -1565,14 +1567,16 @@ def do_share_instance_export_location_show(cs, args): 'e.g. --columns "id,name"') def do_snapshot_list(cs, args): """List all the snapshots.""" - list_of_keys = [ - 'ID', 'Share ID', 'Status', 'Name', 'Share Size', - ] - + all_tenants = int(os.environ.get("ALL_TENANTS", args.all_tenants)) if args.columns is not None: list_of_keys = _split_columns(columns=args.columns) + else: + list_of_keys = [ + 'ID', 'Share ID', 'Status', 'Name', 'Share Size', + ] + if all_tenants: + list_of_keys.append('Project ID') - all_tenants = int(os.environ.get("ALL_TENANTS", args.all_tenants)) empty_obj = type('Empty', (object,), {'id': None}) share = _find_share(cs, args.share_id) if args.share_id else empty_obj search_opts = { diff --git a/releasenotes/notes/bug_1570085_fix-905786b797379309.yaml b/releasenotes/notes/bug_1570085_fix-905786b797379309.yaml new file mode 100644 index 000000000..dc3b2c9ae --- /dev/null +++ b/releasenotes/notes/bug_1570085_fix-905786b797379309.yaml @@ -0,0 +1,5 @@ +--- +fixes: + - Both 'manila list --all-tenants' and 'manila list --public' + will show column Project ID. Same is the change in behavior + of snapshot-list --all-tenants.