From 9ac185d26ebd26e9b5b020b5964e58e16d0efb93 Mon Sep 17 00:00:00 2001 From: zhongjun Date: Fri, 26 May 2017 14:20:32 +0800 Subject: [PATCH] Add export-location filter in share and share instance list Share and share instance list will accept new query string parameter 'export_location'. It can pass path and id of export_location to retrieve shares filtered. Change-Id: Ibd56fdbc294432003849607ddf72d824151ea3c4 Partly-implement: BP support-filter-share-by-export-location --- manilaclient/tests/functional/client.py | 14 +++++- .../tests/functional/test_shares_listing.py | 50 +++++++++++++++++++ .../tests/unit/v2/test_share_instances.py | 9 ++++ manilaclient/tests/unit/v2/test_shares.py | 20 ++++++++ manilaclient/tests/unit/v2/test_shell.py | 34 +++++++++++++ manilaclient/v2/share_instances.py | 22 +++++++- manilaclient/v2/shares.py | 23 +++++++++ manilaclient/v2/shell.py | 34 ++++++++++++- ...port-location-filter-4cf3114doe40k598.yaml | 4 ++ 9 files changed, 206 insertions(+), 4 deletions(-) create mode 100644 releasenotes/notes/add-export-location-filter-4cf3114doe40k598.yaml diff --git a/manilaclient/tests/functional/client.py b/manilaclient/tests/functional/client.py index 9c1b90a17..0dcec6e8f 100644 --- a/manilaclient/tests/functional/client.py +++ b/manilaclient/tests/functional/client.py @@ -665,15 +665,27 @@ class ManilaCLIClient(base.CLIClient): shares = utils.listing(shares_raw) return shares - def list_share_instances(self, share_id=None, microversion=None): + def list_share_instances(self, share_id=None, filters=None, + microversion=None): """List share instances. :param share_id: ID of a share to filter by. + :param filters: dict -- filters for listing of shares. + Example, input: + {'project_id': 'foo'} + {-'project_id': 'foo'} + {--'project_id': 'foo'} + {'project-id': 'foo'} + will be transformed to filter parameter "--export-location=foo" :param microversion: API microversion to be used for request. """ cmd = 'share-instance-list ' if share_id: cmd += '--share-id %s' % share_id + if filters and isinstance(filters, dict): + for k, v in filters.items(): + cmd += '%(k)s=%(v)s ' % { + 'k': self._stranslate_to_cli_optional_param(k), 'v': v} share_instances_raw = self.manila(cmd, microversion=microversion) share_instances = utils.listing(share_instances_raw) return share_instances diff --git a/manilaclient/tests/functional/test_shares_listing.py b/manilaclient/tests/functional/test_shares_listing.py index 0dabdc234..b42f1a35e 100644 --- a/manilaclient/tests/functional/test_shares_listing.py +++ b/manilaclient/tests/functional/test_shares_listing.py @@ -43,6 +43,10 @@ class SharesListReadOnlyTest(base.BaseTestCase): def test_shares_list_filter_by_name(self, role): self.clients[role].manila('list', params='--name name') + @ddt.data('admin', 'user') + def test_shares_list_filter_by_export_location(self, role): + self.clients[role].manila('list', params='--export_location fake') + @ddt.data('admin', 'user') def test_shares_list_filter_by_status(self, role): self.clients[role].manila('list', params='--status status') @@ -240,3 +244,49 @@ class SharesListReadWriteTest(base.BaseTestCase): self.assertTrue(any(s['Name'] is not None for s in shares)) self.assertTrue(any(s['Size'] is not None for s in shares)) self.assertTrue(all('Description' not in s for s in shares)) + + @ddt.data('ID', 'Path') + def test_list_shares_by_export_location(self, option): + export_locations = self.admin_client.list_share_export_locations( + self.public_share['id']) + shares = self.admin_client.list_shares( + filters={'export_location': export_locations[0][option]}) + + self.assertEqual(1, len(shares)) + self.assertTrue( + any(self.public_share['id'] == s['ID'] for s in shares)) + for share in shares: + get = self.admin_client.get_share(share['ID']) + self.assertEqual(self.public_name, get['name']) + + @ddt.data('ID', 'Path') + def test_list_share_instances_by_export_location(self, option): + export_locations = self.admin_client.list_share_export_locations( + self.public_share['id']) + share_instances = self.admin_client.list_share_instances( + filters={'export_location': export_locations[0][option]}) + + self.assertEqual(1, len(share_instances)) + + share_instance_id = share_instances[0]['ID'] + except_export_locations = ( + self.admin_client.list_share_instance_export_locations( + share_instance_id)) + self.assertGreater(len(except_export_locations), 0) + self.assertTrue( + any(export_locations[0][option] == e[option] for e in + except_export_locations)) + + def test_list_share_by_export_location_with_invalid_version(self): + self.assertRaises( + exceptions.CommandFailed, + self.admin_client.list_shares, + filters={'export_location': 'fake'}, + microversion='2.34') + + def test_list_share_instance_by_export_location_invalid_version(self): + self.assertRaises( + exceptions.CommandFailed, + self.admin_client.list_share_instances, + filters={'export_location': 'fake'}, + microversion='2.34') diff --git a/manilaclient/tests/unit/v2/test_share_instances.py b/manilaclient/tests/unit/v2/test_share_instances.py index ba27e14c2..890e70836 100644 --- a/manilaclient/tests/unit/v2/test_share_instances.py +++ b/manilaclient/tests/unit/v2/test_share_instances.py @@ -41,6 +41,15 @@ class ShareInstancesTest(utils.TestCase): cs.share_instances.list() cs.assert_called('GET', '/share_instances') + @ddt.data(('id', 'b4991315-eb7d-43ec-979e-5715d4399827'), + ('path', '//0.0.0.0/fake_path')) + @ddt.unpack + def test_list_by_export_location(self, filter_type, value): + cs.share_instances.list(export_location=value) + cs.assert_called( + 'GET', '/share_instances?export_location_' + + filter_type + '=' + value) + def test_get(self): instance = type('None', (object, ), {'id': '1234'}) cs.share_instances.get(instance) diff --git a/manilaclient/tests/unit/v2/test_shares.py b/manilaclient/tests/unit/v2/test_shares.py index 4e0f8ee07..c5f09a7b9 100644 --- a/manilaclient/tests/unit/v2/test_shares.py +++ b/manilaclient/tests/unit/v2/test_shares.py @@ -297,6 +297,26 @@ class SharesTest(utils.TestCase): 'GET', '/shares?fake_int=1&fake_str=fake_str_value&is_public=True') + @ddt.data(('id', 'b4991315-eb7d-43ec-979e-5715d4399827', True), + ('id', 'b4991315-eb7d-43ec-979e-5715d4399827', False), + ('path', 'fake_path', False), + ('path', 'fake_path', True)) + @ddt.unpack + def test_list_shares_index_with_export_location(self, filter_type, + value, detailed): + search_opts = { + 'export_location': value, + } + cs.shares.list(detailed=detailed, search_opts=search_opts) + if detailed: + cs.assert_called( + 'GET', ('/shares/detail?export_location_' + filter_type + + '=' + value + '&is_public=True')) + else: + cs.assert_called( + 'GET', ('/shares?export_location_' + filter_type + '=' + + value + '&is_public=True')) + def test_list_shares_detailed(self): cs.shares.list(detailed=True) cs.assert_called('GET', '/shares/detail?is_public=True') diff --git a/manilaclient/tests/unit/v2/test_shell.py b/manilaclient/tests/unit/v2/test_shell.py index ec11b23f5..ab66a797f 100644 --- a/manilaclient/tests/unit/v2/test_shell.py +++ b/manilaclient/tests/unit/v2/test_shell.py @@ -323,6 +323,26 @@ class ShellTest(test_utils.TestCase): self.run_command('list --host' + separator + 'fake_host') self.assert_called('GET', '/shares/detail?host=fake_host') + @ddt.data(('id', 'b4991315-eb7d-43ec-979e-5715d4399827'), + ('path', 'fake_path')) + @ddt.unpack + def test_share_list_filter_by_export_location(self, filter_type, value): + for separator in self.separators: + self.run_command('list --export_location' + separator + value) + self.assert_called( + 'GET', + '/shares/detail?export_location_' + filter_type + '=' + value) + + @ddt.data('list', 'share-instance-list') + def test_share_or_instance_list_filter_by_export_location_version_invalid( + self, cmd): + self.assertRaises( + exceptions.CommandError, + self.run_command, + cmd + ' --export_location=fake', + '2.34' + ) + def test_list_filter_by_share_network(self): aliases = ['--share-network', '--share_network', ] fake_sn = type('Empty', (object,), {'id': 'fake_share_network_id'}) @@ -363,6 +383,20 @@ class ShellTest(test_utils.TestCase): mock.ANY, ['Id', 'Host', 'Status']) + @mock.patch.object(cliutils, 'print_list', mock.Mock()) + @ddt.data(('id', 'b4991315-eb7d-43ec-979e-5715d4399827'), + ('path', 'fake_path')) + @ddt.unpack + def test_share_instance_list_filter_by_export_location(self, filter_type, + value): + for separator in self.separators: + self.run_command('share-instance-list --export_location' + + separator + value) + self.assert_called( + 'GET', + ('/share_instances?export_location_' + + filter_type + '=' + value)) + @mock.patch.object(apiclient_utils, 'find_resource', mock.Mock(return_value='fake')) def test_share_instance_list_with_share(self): diff --git a/manilaclient/v2/share_instances.py b/manilaclient/v2/share_instances.py index c0773ef15..914c59cca 100644 --- a/manilaclient/v2/share_instances.py +++ b/manilaclient/v2/share_instances.py @@ -13,6 +13,8 @@ # License for the specific language governing permissions and limitations # under the License. +from oslo_utils import uuidutils + from manilaclient import api_versions from manilaclient import base from manilaclient.common.apiclient import base as common_base @@ -46,10 +48,26 @@ class ShareInstanceManager(base.ManagerWithFind): share_id = common_base.getid(instance) return self._get("/share_instances/%s" % share_id, "share_instance") - @api_versions.wraps("2.3") + @api_versions.wraps("2.3", "2.34") def list(self): """List all share instances.""" - return self._list('/share_instances', 'share_instances') + return self.do_list() + + @api_versions.wraps("2.35") # noqa + def list(self, export_location=None): + """List all share instances.""" + return self.do_list(export_location) + + def do_list(self, export_location=None): + """List all share instances.""" + path = '/share_instances' + if export_location: + if uuidutils.is_uuid_like(export_location): + path += '?export_location_id=' + export_location + else: + path += '?export_location_path=' + export_location + + return self._list(path, 'share_instances') def _action(self, action, instance, info=None, **kwargs): """Perform a share instance 'action'. diff --git a/manilaclient/v2/shares.py b/manilaclient/v2/shares.py index 704a5b4a8..df78eef3d 100644 --- a/manilaclient/v2/shares.py +++ b/manilaclient/v2/shares.py @@ -15,6 +15,7 @@ """Interface for shares extension.""" import collections +from oslo_utils import uuidutils import re import string try: @@ -318,8 +319,23 @@ class ShareManager(base.ManagerWithFind): share_id = common_base.getid(share) return self._update("/shares/%s" % share_id, body) + @api_versions.wraps("1.0", "2.34") def list(self, detailed=True, search_opts=None, sort_key=None, sort_dir=None): + """Get a list of all shares.""" + search_opts.pop("export_location", None) + return self.do_list(detailed=detailed, search_opts=search_opts, + sort_key=sort_key, sort_dir=sort_dir) + + @api_versions.wraps("2.35") # noqa + def list(self, detailed=True, search_opts=None, + sort_key=None, sort_dir=None): + """Get a list of all shares.""" + return self.do_list(detailed=detailed, search_opts=search_opts, + sort_key=sort_key, sort_dir=sort_dir) + + def do_list(self, detailed=True, search_opts=None, + sort_key=None, sort_dir=None): """Get a list of all shares. :param detailed: Whether to return detailed share info or not. @@ -373,6 +389,13 @@ class ShareManager(base.ManagerWithFind): if 'is_public' not in search_opts: search_opts['is_public'] = True + export_location = search_opts.pop('export_location', None) + if export_location: + if uuidutils.is_uuid_like(export_location): + search_opts['export_location_id'] = export_location + else: + search_opts['export_location_path'] = export_location + if search_opts: query_string = urlencode( sorted([(k, v) for (k, v) in list(search_opts.items()) if v])) diff --git a/manilaclient/v2/shell.py b/manilaclient/v2/shell.py index 04e482157..b42d13834 100644 --- a/manilaclient/v2/shell.py +++ b/manilaclient/v2/shell.py @@ -1513,6 +1513,14 @@ def do_snapshot_access_list(cs, args): default=None, help='Comma separated list of columns to be displayed ' 'example --columns "export_location,is public".') +@cliutils.arg( + '--export-location', '--export_location', + metavar='', + type=str, + default=None, + action='single_alias', + help='ID or path of the share export location. ' + 'Available only for microversion >= 2.35.') @cliutils.service_type('sharev2') def do_list(cs, args): """List NAS shares with filters.""" @@ -1560,6 +1568,14 @@ def do_list(cs, args): 'is_public': args.public, } + if cs.api_version.matches(api_versions.APIVersion("2.35"), + api_versions.APIVersion()): + search_opts['export_location'] = args.export_location + elif args.export_location: + raise exceptions.CommandError( + "Filtering by export location is only " + "available with manila API version >= 2.35") + if share_group: search_opts['share_group_id'] = share_group.id @@ -1596,6 +1612,14 @@ def do_list(cs, args): default=None, help='Comma separated list of columns to be displayed ' 'example --columns "id,host,status".') +@cliutils.arg( + '--export-location', '--export_location', + metavar='', + type=str, + default=None, + action='single_alias', + help='ID or path of the share instance export location. ' + 'Available only for microversion >= 2.35.') @api_versions.wraps("2.3") def do_share_instance_list(cs, args): """List share instances (Admin only).""" @@ -1612,7 +1636,15 @@ def do_share_instance_list(cs, args): if share: instances = cs.shares.list_instances(share) else: - instances = cs.share_instances.list() + if cs.api_version.matches( + api_versions.APIVersion("2.35"), api_versions.APIVersion()): + instances = cs.share_instances.list(args.export_location) + else: + if args.export_location: + raise exceptions.CommandError( + "Filtering by export location is only " + "available with manila API version >= 2.35") + instances = cs.share_instances.list() cliutils.print_list(instances, list_of_keys) diff --git a/releasenotes/notes/add-export-location-filter-4cf3114doe40k598.yaml b/releasenotes/notes/add-export-location-filter-4cf3114doe40k598.yaml new file mode 100644 index 000000000..0427d1a1f --- /dev/null +++ b/releasenotes/notes/add-export-location-filter-4cf3114doe40k598.yaml @@ -0,0 +1,4 @@ +--- +features: + - Shares and share instances can now be filtered with their + export location IDs or paths.