From: Juan Manuel Olle Date: Mon, 28 Apr 2014 18:09:29 +0000 (-0300) Subject: Adding filter options to backup list X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=5b05b372c8eb2dc577b1ddbd4095b9e8e1b3594c;p=openstack-build%2Fcinder-build.git Adding filter options to backup list This patch adds filter options to backup list. Like cinder list, now cinder backup-list accept parameters. This blueprint is complimented with the client part https://blueprints.launchpad.net/python-cinderclient /+spec/add-filter-options-to-backup-list blueprint: add-filter-options-to-backup-list DocImpact Change-Id: I9e095ee44a03744eb3f319323d9130a89196614e --- diff --git a/cinder/api/contrib/backups.py b/cinder/api/contrib/backups.py index 405b87466..1b2970051 100644 --- a/cinder/api/contrib/backups.py +++ b/cinder/api/contrib/backups.py @@ -194,10 +194,25 @@ class BackupsController(wsgi.Controller): """Returns a detailed list of backups.""" return self._get_backups(req, is_detail=True) + @staticmethod + def _get_backup_filter_options(): + """Return volume search options allowed by non-admin.""" + return ('name', 'status', 'volume_id') + def _get_backups(self, req, is_detail): """Returns a list of backups, transformed through view builder.""" context = req.environ['cinder.context'] - backups = self.backup_api.get_all(context) + filters = req.params.copy() + + utils.remove_invalid_filter_options(context, + filters, + self._get_backup_filter_options()) + + if 'name' in filters: + filters['display_name'] = filters['name'] + del filters['name'] + + backups = self.backup_api.get_all(context, search_opts=filters) limited_list = common.limited(backups, req) if is_detail: diff --git a/cinder/api/v1/snapshots.py b/cinder/api/v1/snapshots.py index 1e4411143..54a50b6c5 100644 --- a/cinder/api/v1/snapshots.py +++ b/cinder/api/v1/snapshots.py @@ -20,7 +20,6 @@ from webob import exc from cinder.api import common from cinder.api.openstack import wsgi -from cinder.api.v1 import volumes from cinder.api import xmlutil from cinder import exception from cinder.openstack.common import log as logging @@ -146,8 +145,8 @@ class SnapshotsController(wsgi.Controller): #filter out invalid option allowed_search_options = ('status', 'volume_id', 'display_name') - volumes.remove_invalid_options(context, search_opts, - allowed_search_options) + utils.remove_invalid_filter_options(context, search_opts, + allowed_search_options) snapshots = self.volume_api.get_all_snapshots(context, search_opts=search_opts) diff --git a/cinder/api/v1/volumes.py b/cinder/api/v1/volumes.py index 829237046..6021473d6 100644 --- a/cinder/api/v1/volumes.py +++ b/cinder/api/v1/volumes.py @@ -273,8 +273,9 @@ class VolumeController(wsgi.Controller): search_opts['metadata'] = ast.literal_eval(search_opts['metadata']) context = req.environ['cinder.context'] - remove_invalid_options(context, - search_opts, self._get_volume_search_options()) + utils.remove_invalid_filter_options(context, + search_opts, + self._get_volume_search_options()) volumes = self.volume_api.get_all(context, marker=None, limit=None, sort_key='created_at', @@ -441,19 +442,3 @@ class VolumeController(wsgi.Controller): def create_resource(ext_mgr): return wsgi.Resource(VolumeController(ext_mgr)) - - -def remove_invalid_options(context, search_options, allowed_search_options): - """Remove search options that are not valid for non-admin API/context.""" - if context.is_admin: - # Allow all options - return - # Otherwise, strip out all unknown options - unknown_options = [opt for opt in search_options - if opt not in allowed_search_options] - bad_options = ", ".join(unknown_options) - log_msg = _("Removing options '%(bad_options)s'" - " from query") % {'bad_options': bad_options} - LOG.debug(log_msg) - for opt in unknown_options: - del search_options[opt] diff --git a/cinder/api/v2/snapshots.py b/cinder/api/v2/snapshots.py index bc74d58fd..efff59ccb 100644 --- a/cinder/api/v2/snapshots.py +++ b/cinder/api/v2/snapshots.py @@ -20,7 +20,6 @@ from webob import exc from cinder.api import common from cinder.api.openstack import wsgi -from cinder.api.v2 import volumes from cinder.api import xmlutil from cinder import exception from cinder.openstack.common import log as logging @@ -149,8 +148,8 @@ class SnapshotsController(wsgi.Controller): #filter out invalid option allowed_search_options = ('status', 'volume_id', 'name') - volumes.remove_invalid_options(context, search_opts, - allowed_search_options) + utils.remove_invalid_filter_options(context, search_opts, + allowed_search_options) # NOTE(thingee): v2 API allows name instead of display_name if 'name' in search_opts: diff --git a/cinder/api/v2/volumes.py b/cinder/api/v2/volumes.py index 3434a9bb5..d8b0c19b1 100644 --- a/cinder/api/v2/volumes.py +++ b/cinder/api/v2/volumes.py @@ -214,8 +214,9 @@ class VolumeController(wsgi.Controller): params.pop('offset', None) filters = params - remove_invalid_options(context, - filters, self._get_volume_filter_options()) + utils.remove_invalid_filter_options(context, + filters, + self._get_volume_filter_options()) # NOTE(thingee): v2 API allows name instead of display_name if 'name' in filters: @@ -417,18 +418,3 @@ class VolumeController(wsgi.Controller): def create_resource(ext_mgr): return wsgi.Resource(VolumeController(ext_mgr)) - - -def remove_invalid_options(context, filters, allowed_search_options): - """Remove search options that are not valid for non-admin API/context.""" - if context.is_admin: - # Allow all options - return - # Otherwise, strip out all unknown options - unknown_options = [opt for opt in filters - if opt not in allowed_search_options] - bad_options = ", ".join(unknown_options) - log_msg = _("Removing options '%s' from query") % bad_options - LOG.debug(log_msg) - for opt in unknown_options: - del filters[opt] diff --git a/cinder/backup/api.py b/cinder/backup/api.py index 37851e4bd..e0c9a31de 100644 --- a/cinder/backup/api.py +++ b/cinder/backup/api.py @@ -71,16 +71,16 @@ class API(base.Base): backup['host'], backup['id']) - # TODO(moorehef): Add support for search_opts, discarded atm def get_all(self, context, search_opts=None): if search_opts is None: search_opts = {} check_policy(context, 'get_all') if context.is_admin: - backups = self.db.backup_get_all(context) + backups = self.db.backup_get_all(context, filters=search_opts) else: backups = self.db.backup_get_all_by_project(context, - context.project_id) + context.project_id, + filters=search_opts) return backups diff --git a/cinder/db/api.py b/cinder/db/api.py index b81b04a38..ffb92e180 100644 --- a/cinder/db/api.py +++ b/cinder/db/api.py @@ -722,9 +722,9 @@ def backup_get(context, backup_id): return IMPL.backup_get(context, backup_id) -def backup_get_all(context): +def backup_get_all(context, filters=None): """Get all backups.""" - return IMPL.backup_get_all(context) + return IMPL.backup_get_all(context, filters=filters) def backup_get_all_by_host(context, host): @@ -737,9 +737,10 @@ def backup_create(context, values): return IMPL.backup_create(context, values) -def backup_get_all_by_project(context, project_id): +def backup_get_all_by_project(context, project_id, filters=None): """Get all backups belonging to a project.""" - return IMPL.backup_get_all_by_project(context, project_id) + return IMPL.backup_get_all_by_project(context, project_id, + filters=filters) def backup_update(context, backup_id, values): diff --git a/cinder/db/sqlalchemy/api.py b/cinder/db/sqlalchemy/api.py index 219740b1c..f9bac15c6 100644 --- a/cinder/db/sqlalchemy/api.py +++ b/cinder/db/sqlalchemy/api.py @@ -2567,9 +2567,20 @@ def backup_get(context, backup_id): return result +def _backup_get_all(context, filters=None): + session = get_session() + with session.begin(): + # Generate the query + query = model_query(context, models.Backup) + if filters: + query = query.filter_by(**filters) + + return query.all() + + @require_admin_context -def backup_get_all(context): - return model_query(context, models.Backup).all() +def backup_get_all(context, filters=None): + return _backup_get_all(context, filters) @require_admin_context @@ -2578,11 +2589,17 @@ def backup_get_all_by_host(context, host): @require_context -def backup_get_all_by_project(context, project_id): +def backup_get_all_by_project(context, project_id, filters=None): + authorize_project_context(context, project_id) + if not filters: + filters = {} + else: + filters = filters.copy() + + filters['project_id'] = project_id - return model_query(context, models.Backup).\ - filter_by(project_id=project_id).all() + return _backup_get_all(context, filters) @require_context diff --git a/cinder/tests/api/contrib/test_backups.py b/cinder/tests/api/contrib/test_backups.py index 3cda44a48..6fe3a6f36 100644 --- a/cinder/tests/api/contrib/test_backups.py +++ b/cinder/tests/api/contrib/test_backups.py @@ -251,6 +251,48 @@ class BackupsAPITestCase(test.TestCase): db.backup_destroy(context.get_admin_context(), backup_id2) db.backup_destroy(context.get_admin_context(), backup_id1) + def test_list_backups_detail_using_filters(self): + backup_id1 = self._create_backup(display_name='test2') + backup_id2 = self._create_backup(status='available') + backup_id3 = self._create_backup(volume_id=4321) + + req = webob.Request.blank('/v2/fake/backups/detail?name=test2') + req.method = 'GET' + req.headers['Content-Type'] = 'application/json' + req.headers['Accept'] = 'application/json' + res = req.get_response(fakes.wsgi_app()) + res_dict = json.loads(res.body) + + self.assertEqual(len(res_dict['backups']), 1) + self.assertEqual(res.status_int, 200) + self.assertEqual(res_dict['backups'][0]['id'], backup_id1) + + req = webob.Request.blank('/v2/fake/backups/detail?status=available') + req.method = 'GET' + req.headers['Content-Type'] = 'application/json' + req.headers['Accept'] = 'application/json' + res = req.get_response(fakes.wsgi_app()) + res_dict = json.loads(res.body) + + self.assertEqual(len(res_dict['backups']), 1) + self.assertEqual(res.status_int, 200) + self.assertEqual(res_dict['backups'][0]['id'], backup_id2) + + req = webob.Request.blank('/v2/fake/backups/detail?volume_id=4321') + req.method = 'GET' + req.headers['Content-Type'] = 'application/json' + req.headers['Accept'] = 'application/json' + res = req.get_response(fakes.wsgi_app()) + res_dict = json.loads(res.body) + + self.assertEqual(len(res_dict['backups']), 1) + self.assertEqual(res.status_int, 200) + self.assertEqual(res_dict['backups'][0]['id'], backup_id3) + + db.backup_destroy(context.get_admin_context(), backup_id3) + db.backup_destroy(context.get_admin_context(), backup_id2) + db.backup_destroy(context.get_admin_context(), backup_id1) + def test_list_backups_detail_xml(self): backup_id1 = self._create_backup() backup_id2 = self._create_backup() diff --git a/cinder/tests/test_db_api.py b/cinder/tests/test_db_api.py index 532ba5a78..61a46c0f5 100644 --- a/cinder/tests/test_db_api.py +++ b/cinder/tests/test_db_api.py @@ -1184,6 +1184,19 @@ class DBAPIBackupTestCase(BaseTest): all_backups = db.backup_get_all(self.ctxt) self._assertEqualListsOfObjects(self.created, all_backups) + def tests_backup_get_all_by_filter(self): + filters = {'status': self.created[1]['status']} + filtered_backups = db.backup_get_all(self.ctxt, filters=filters) + self._assertEqualListsOfObjects([self.created[1]], filtered_backups) + + filters = {'display_name': self.created[1]['display_name']} + filtered_backups = db.backup_get_all(self.ctxt, filters=filters) + self._assertEqualListsOfObjects([self.created[1]], filtered_backups) + + filters = {'volume_id': self.created[1]['volume_id']} + filtered_backups = db.backup_get_all(self.ctxt, filters=filters) + self._assertEqualListsOfObjects([self.created[1]], filtered_backups) + def test_backup_get_all_by_host(self): byhost = db.backup_get_all_by_host(self.ctxt, self.created[1]['host']) diff --git a/cinder/utils.py b/cinder/utils.py index 6bc03835e..c9603998c 100644 --- a/cinder/utils.py +++ b/cinder/utils.py @@ -846,3 +846,21 @@ def add_visible_admin_metadata(volume): volume['metadata'].update(visible_admin_meta) else: volume['metadata'] = visible_admin_meta + + +def remove_invalid_filter_options(context, filters, + allowed_search_options): + """Remove search options that are not valid + for non-admin API/context. + """ + if context.is_admin: + # Allow all options + return + # Otherwise, strip out all unknown options + unknown_options = [opt for opt in filters + if opt not in allowed_search_options] + bad_options = ", ".join(unknown_options) + log_msg = "Removing options '%s' from query." % bad_options + LOG.debug(log_msg) + for opt in unknown_options: + del filters[opt]