From 7ecdbde6b3749a61ddfdbf52f159542416f8e089 Mon Sep 17 00:00:00 2001 From: Your Name Date: Fri, 10 Oct 2014 13:47:12 -0400 Subject: [PATCH] Improve snapshots list API filtering 1) Added new params for filtering by: - source share - usage of snapshot* - limit and offset a la pagination - direction (asc, desc)* - key of snapshot* * requires server-side update 2) Added aliases for params, mostly in perspective to avoid mess of underscore and dash symbols. Implements blueprint improve-snapshot-list-filtering Change-Id: I3727911f179ea01dd4cb68149fa7f4d79ca75a5c --- manilaclient/common/constants.py | 11 ++ manilaclient/tests/unit/v1/fakes.py | 13 ++- .../tests/unit/v1/test_share_snapshots.py | 32 +++++- manilaclient/tests/unit/v1/test_shell.py | 102 ++++++++++++++++-- manilaclient/v1/share_snapshots.py | 28 ++++- manilaclient/v1/shell.py | 68 ++++++++++-- 6 files changed, 235 insertions(+), 19 deletions(-) diff --git a/manilaclient/common/constants.py b/manilaclient/common/constants.py index 9bb1d7bda..2ac268ccc 100644 --- a/manilaclient/common/constants.py +++ b/manilaclient/common/constants.py @@ -27,3 +27,14 @@ SHARE_SORT_KEY_VALUES = ( 'share_network_id', 'share_network', 'snapshot_id', 'snapshot', ) + +SNAPSHOT_SORT_KEY_VALUES = ( + 'id', + 'status', + 'size', + 'share_id', + 'user_id', + 'project_id', + 'progress', + 'name', 'display_name', +) diff --git a/manilaclient/tests/unit/v1/fakes.py b/manilaclient/tests/unit/v1/fakes.py index 6df2f5203..61988574b 100644 --- a/manilaclient/tests/unit/v1/fakes.py +++ b/manilaclient/tests/unit/v1/fakes.py @@ -90,8 +90,19 @@ class FakeHTTPClient(fakes.FakeHTTPClient): raise AssertionError("Unexpected action: %s" % action) return (resp, {}, _body) + def get_snapshots(self, **kw): + snapshots = { + 'snapshots': [ + { + 'id': 1234, + 'status': 'available', + 'name': 'sharename', + } + ] + } + return (200, {}, snapshots) + def get_snapshots_detail(self, **kw): - print(kw) snapshots = {'snapshots': [{ 'id': 1234, 'created_at': '2012-08-27T00:00:00.000000', diff --git a/manilaclient/tests/unit/v1/test_share_snapshots.py b/manilaclient/tests/unit/v1/test_share_snapshots.py index 2f3d7f3d5..fa57c170c 100644 --- a/manilaclient/tests/unit/v1/test_share_snapshots.py +++ b/manilaclient/tests/unit/v1/test_share_snapshots.py @@ -1,6 +1,6 @@ # Copyright 2010 Jacob Kaplan-Moss - # Copyright 2011 OpenStack LLC. +# Copyright 2014 Mirantis, Inc. # All Rights Reserved. # # Licensed under the Apache License, Version 2.0 (the "License"); you may @@ -44,6 +44,32 @@ class ShareSnapshotsTest(utils.TestCase): cs.assert_called('POST', '/snapshots/1234/action', {'os-force_delete': None}) - def test_list_share_snapshots(self): - cs.share_snapshots.list() + def test_list_share_snapshots_index(self): + cs.share_snapshots.list(detailed=False) + cs.assert_called('GET', '/snapshots') + + def test_list_share_snapshots_index_with_search_opts(self): + search_opts = {'fake_str': 'fake_str_value', 'fake_int': 1} + cs.share_snapshots.list(detailed=False, search_opts=search_opts) + cs.assert_called( + 'GET', '/snapshots?fake_int=1&fake_str=fake_str_value') + + def test_list_share_snapshots_sort_by_asc_and_share_id(self): + cs.share_snapshots.list( + detailed=False, sort_key='share_id', sort_dir='asc') + cs.assert_called('GET', '/snapshots?sort_dir=asc&sort_key=share_id') + + def test_list_share_snapshots_sort_by_desc_and_status(self): + cs.share_snapshots.list( + detailed=False, sort_key='status', sort_dir='desc') + cs.assert_called('GET', '/snapshots?sort_dir=desc&sort_key=status') + + def test_list_share_snapshots_by_improper_direction(self): + self.assertRaises(ValueError, cs.share_snapshots.list, sort_dir='fake') + + def test_list_share_snapshots_by_improper_key(self): + self.assertRaises(ValueError, cs.share_snapshots.list, sort_key='fake') + + def test_list_share_snapshots_detail(self): + cs.share_snapshots.list(detailed=True) cs.assert_called('GET', '/snapshots/detail') diff --git a/manilaclient/tests/unit/v1/test_shell.py b/manilaclient/tests/unit/v1/test_shell.py index 73cdad9ea..e39b161ff 100644 --- a/manilaclient/tests/unit/v1/test_shell.py +++ b/manilaclient/tests/unit/v1/test_shell.py @@ -267,14 +267,102 @@ class ShellTest(test_utils.TestCase): 'delete fake-not-found' ) - def test_snapshot_list_filter_share_id(self): - self.run_command('snapshot-list --share-id=1234') - self.assert_called('GET', '/snapshots/detail?share_id=1234') + def test_list_snapshots(self): + self.run_command('snapshot-list') + self.assert_called('GET', '/snapshots/detail?usage=any') - def test_snapshot_list_filter_status_and_share_id(self): - self.run_command('snapshot-list --status=available --share-id=1234') - self.assert_called('GET', '/snapshots/detail?' - 'share_id=1234&status=available') + 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&usage=any') + + def test_list_snapshots_all_tenants_key_and_value_1(self): + for separator in self.separators: + self.run_command('snapshot-list --all-tenants' + separator + '1') + self.assert_called( + 'GET', '/snapshots/detail?all_tenants=1&usage=any') + + def test_list_snapshots_all_tenants_key_and_value_0(self): + for separator in self.separators: + self.run_command('snapshot-list --all-tenants' + separator + '0') + self.assert_called('GET', '/snapshots/detail?usage=any') + + def test_list_snapshots_filter_by_name(self): + for separator in self.separators: + self.run_command('snapshot-list --name' + separator + '1234') + self.assert_called( + 'GET', '/snapshots/detail?name=1234&usage=any') + + def test_list_snapshots_filter_by_status(self): + for separator in self.separators: + self.run_command('snapshot-list --status' + separator + '1234') + self.assert_called( + 'GET', '/snapshots/detail?status=1234&usage=any') + + def test_list_snapshots_filter_by_share_id(self): + aliases = ['--share_id', '--share-id'] + for alias in aliases: + for separator in self.separators: + self.run_command('snapshot-list ' + alias + separator + '1234') + self.assert_called( + 'GET', '/snapshots/detail?share_id=1234&usage=any') + + def test_list_snapshots_only_used(self): + for separator in self.separators: + self.run_command('snapshot-list --usage' + separator + 'used') + self.assert_called('GET', '/snapshots/detail?usage=used') + + def test_list_snapshots_only_unused(self): + for separator in self.separators: + self.run_command('snapshot-list --usage' + separator + 'unused') + self.assert_called('GET', '/snapshots/detail?usage=unused') + + def test_list_snapshots_with_limit(self): + for separator in self.separators: + self.run_command('snapshot-list --limit' + separator + '50') + self.assert_called( + 'GET', '/snapshots/detail?limit=50&usage=any') + + def test_list_snapshots_with_offset(self): + for separator in self.separators: + self.run_command('snapshot-list --offset' + separator + '50') + self.assert_called( + 'GET', '/snapshots/detail?offset=50&usage=any') + + def test_list_snapshots_with_sort_dir_verify_keys(self): + aliases = ['--sort_dir', '--sort-dir'] + for alias in aliases: + for key in constants.SORT_DIR_VALUES: + for separator in self.separators: + self.run_command( + 'snapshot-list ' + alias + separator + key) + self.assert_called( + 'GET', + '/snapshots/detail?sort_dir=' + key + '&usage=any') + + def test_list_snapshots_with_fake_sort_dir(self): + self.assertRaises( + ValueError, + self.run_command, + 'snapshot-list --sort-dir fake_sort_dir', + ) + + def test_list_snapshots_with_sort_key_verify_keys(self): + aliases = ['--sort_key', '--sort-key'] + for alias in aliases: + for key in constants.SNAPSHOT_SORT_KEY_VALUES: + for separator in self.separators: + self.run_command( + 'snapshot-list ' + alias + separator + key) + self.assert_called( + 'GET', + '/snapshots/detail?sort_key=' + key + '&usage=any') + + def test_list_snapshots_with_fake_sort_key(self): + self.assertRaises( + ValueError, + self.run_command, + 'snapshot-list --sort-key fake_sort_key', + ) def test_rename(self): # basic rename with positional agruments diff --git a/manilaclient/v1/share_snapshots.py b/manilaclient/v1/share_snapshots.py index 91e86d8f6..ccfde7547 100644 --- a/manilaclient/v1/share_snapshots.py +++ b/manilaclient/v1/share_snapshots.py @@ -20,6 +20,7 @@ except ImportError: from urllib.parse import urlencode # noqa from manilaclient import base +from manilaclient.common import constants from manilaclient.openstack.common.apiclient import base as common_base @@ -74,11 +75,34 @@ class ShareSnapshotManager(base.ManagerWithFind): """ return self._get('/snapshots/%s' % snapshot_id, 'snapshot') - def list(self, detailed=True, search_opts=None): - """Get a list of all snapshots of shares. + def list(self, detailed=True, search_opts=None, sort_key=None, + sort_dir=None): + """Get a list of snapshots of shares. + :param search_opts: Search options to filter out shares. + :param sort_key: Key to be sorted. + :param sort_dir: Sort direction, should be 'desc' or 'asc'. :rtype: list of :class:`ShareSnapshot` """ + if search_opts is None: + search_opts = {} + + if sort_key is not None: + if sort_key in constants.SNAPSHOT_SORT_KEY_VALUES: + search_opts['sort_key'] = sort_key + else: + raise ValueError( + 'sort_key must be one of the following: %s.' + % ', '.join(constants.SNAPSHOT_SORT_KEY_VALUES)) + + if sort_dir is not None: + if sort_dir in constants.SORT_DIR_VALUES: + search_opts['sort_dir'] = sort_dir + else: + raise ValueError( + 'sort_dir must be one of the following: %s.' + % ', '.join(constants.SORT_DIR_VALUES)) + if search_opts: query_string = urlencode( sorted([(k, v) for (k, v) in list(search_opts.items()) if v])) diff --git a/manilaclient/v1/shell.py b/manilaclient/v1/shell.py index 6a36afa35..553500696 100644 --- a/manilaclient/v1/shell.py +++ b/manilaclient/v1/shell.py @@ -726,22 +726,78 @@ def do_list(cs, args): help='Filter results by status.') @cliutils.arg( '--share-id', - metavar='', + '--share_id', # alias + metavar='', default=None, - help='Filter results by share ID.') + action='single_alias', + help='Filter results by source share ID.') +@cliutils.arg( + '--usage', + dest='usage', + metavar='any|used|unused', + nargs='?', + type=str, + const='any', + default='any', + choices=['any', 'used', 'unused', ], + help='Either filter or not snapshots by its usage. OPTIONAL: Default=any.') +@cliutils.arg( + '--limit', + metavar='', + type=int, + default=None, + help='Maximum number of share snapshots to return. ' + 'OPTIONAL: Default=None.') +@cliutils.arg( + '--offset', + metavar='', + type=int, + default=None, + help='Set offset to define start point of share snapshots listing. ' + 'OPTIONAL: Default=None.') +@cliutils.arg( + '--sort-key', + '--sort_key', # alias + metavar='', + type=str, + default=None, + action='single_alias', + help='Key to be sorted, available keys are %(keys)s. ' + 'OPTIONAL: Default=None.' % { + 'keys': constants.SNAPSHOT_SORT_KEY_VALUES}) +@cliutils.arg( + '--sort-dir', + '--sort_dir', # alias + metavar='', + type=str, + default=None, + action='single_alias', + help='Sort direction, available values are %(values)s. ' + 'OPTIONAL: Default=None.' % {'values': constants.SORT_DIR_VALUES}) @cliutils.service_type('share') 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)) + empty_obj = type('Empty', (object,), {'id': None}) + share = _find_share(cs, args.share_id) if args.share_id else empty_obj search_opts = { + 'offset': args.offset, + 'limit': args.limit, 'all_tenants': all_tenants, 'name': args.name, 'status': args.status, - 'share_id': args.share_id, + 'share_id': share.id, + 'usage': args.usage, } - snapshots = cs.share_snapshots.list(search_opts=search_opts) - utils.print_list(snapshots, - ['ID', 'Share ID', 'Status', 'Name', 'Share Size']) + snapshots = cs.share_snapshots.list( + search_opts=search_opts, + sort_key=args.sort_key, + sort_dir=args.sort_dir, + ) + utils.print_list(snapshots, list_of_keys) @cliutils.arg(