diff --git a/cinder/db/api.py b/cinder/db/api.py index ccb40e7bf60..602a3084d08 100644 --- a/cinder/db/api.py +++ b/cinder/db/api.py @@ -205,9 +205,11 @@ def volume_get(context, volume_id): return IMPL.volume_get(context, volume_id) -def volume_get_all(context, marker, limit, sort_key, sort_dir): +def volume_get_all(context, marker, limit, sort_key, sort_dir, + filters=None): """Get all volumes.""" - return IMPL.volume_get_all(context, marker, limit, sort_key, sort_dir) + return IMPL.volume_get_all(context, marker, limit, sort_key, sort_dir, + filters=filters) def volume_get_all_by_host(context, host): @@ -221,10 +223,10 @@ def volume_get_all_by_instance_uuid(context, instance_uuid): def volume_get_all_by_project(context, project_id, marker, limit, sort_key, - sort_dir): + sort_dir, filters=None): """Get all volumes belonging to a project.""" return IMPL.volume_get_all_by_project(context, project_id, marker, limit, - sort_key, sort_dir) + sort_key, sort_dir, filters=filters) def volume_get_iscsi_target_num(context, volume_id): diff --git a/cinder/db/sqlalchemy/api.py b/cinder/db/sqlalchemy/api.py index 41b94791338..e73b7324b0f 100644 --- a/cinder/db/sqlalchemy/api.py +++ b/cinder/db/sqlalchemy/api.py @@ -1,6 +1,7 @@ # Copyright (c) 2011 X.commerce, a business unit of eBay Inc. # Copyright 2010 United States Government as represented by the # Administrator of the National Aeronautics and Space Administration. +# Copyright 2014 IBM Corp. # All Rights Reserved. # # Licensed under the Apache License, Version 2.0 (the "License"); you may @@ -26,6 +27,7 @@ from oslo.config import cfg from sqlalchemy.exc import IntegrityError from sqlalchemy import or_ from sqlalchemy.orm import joinedload, joinedload_all +from sqlalchemy.orm import RelationshipProperty from sqlalchemy.sql.expression import literal_column from sqlalchemy.sql import func @@ -1153,20 +1155,30 @@ def volume_get(context, volume_id): @require_admin_context -def volume_get_all(context, marker, limit, sort_key, sort_dir): +def volume_get_all(context, marker, limit, sort_key, sort_dir, + filters=None): + """Retrieves all volumes. + + :param context: context to query under + :param marker: the last item of the previous page, used to determine the + next page of results to return + :param limit: maximum number of items to return + :param sort_key: single attributes by which results should be sorted + :param sort_dir: direction in which results should be sorted (asc, desc) + :param filters: Filters for the query. A filter key/value of + 'no_migration_targets'=True causes volumes with either + a NULL 'migration_status' or a 'migration_status' that + does not start with 'target:' to be retrieved. + :returns: list of matching volumes + """ session = get_session() with session.begin(): - query = _volume_get_query(context, session=session) - - marker_volume = None - if marker is not None: - marker_volume = _volume_get(context, marker, session=session) - - query = sqlalchemyutils.paginate_query(query, models.Volume, limit, - [sort_key, 'created_at', 'id'], - marker=marker_volume, - sort_dir=sort_dir) - + # Generate the query + query = _generate_paginate_query(context, session, marker, limit, + sort_key, sort_dir, filters) + # No volumes would match, return empty list + if query == None: + return [] return query.all() @@ -1192,25 +1204,136 @@ def volume_get_all_by_instance_uuid(context, instance_uuid): @require_context def volume_get_all_by_project(context, project_id, marker, limit, sort_key, - sort_dir): + sort_dir, filters=None): + """"Retrieves all volumes in a project. + + :param context: context to query under + :param project_id: project for all volumes being retrieved + :param marker: the last item of the previous page, used to determine the + next page of results to return + :param limit: maximum number of items to return + :param sort_key: single attributes by which results should be sorted + :param sort_dir: direction in which results should be sorted (asc, desc) + :param filters: Filters for the query. A filter key/value of + 'no_migration_targets'=True causes volumes with either + a NULL 'migration_status' or a 'migration_status' that + does not start with 'target:' to be retrieved. + :returns: list of matching volumes + """ session = get_session() with session.begin(): authorize_project_context(context, project_id) - query = _volume_get_query(context, session).\ - filter_by(project_id=project_id) - - marker_volume = None - if marker is not None: - marker_volume = _volume_get(context, marker, session) - - query = sqlalchemyutils.paginate_query(query, models.Volume, limit, - [sort_key, 'created_at', 'id'], - marker=marker_volume, - sort_dir=sort_dir) - + # Add in the project filter without modifying the given filters + filters = filters.copy() if filters else {} + filters['project_id'] = project_id + # Generate the query + query = _generate_paginate_query(context, session, marker, limit, + sort_key, sort_dir, filters) + # No volumes would match, return empty list + if query == None: + return [] return query.all() +def _generate_paginate_query(context, session, marker, limit, sort_key, + sort_dir, filters): + """Generate the query to include the filters and the paginate options. + + Returns a query with sorting / pagination criteria added or None + if the given filters will not yield any results. + + :param context: context to query under + :param session: the session to use + :param marker: the last item of the previous page; we returns the next + results after this value. + :param limit: maximum number of items to return + :param sort_key: single attributes by which results should be sorted + :param sort_dir: direction in which results should be sorted (asc, desc) + :param filters: dictionary of filters; values that are lists, + tuples, sets, or frozensets cause an 'IN' test to + be performed, while exact matching ('==' operator) + is used for other values + :returns: updated query or None + """ + query = _volume_get_query(context, session=session) + + if filters: + filters = filters.copy() + + # 'no_migration_targets' is unique, must be either NULL or + # not start with 'target:' + if ('no_migration_targets' in filters and + filters['no_migration_targets'] == True): + filters.pop('no_migration_targets') + try: + column_attr = getattr(models.Volume, 'migration_status') + conditions = [column_attr == None, + column_attr.op('NOT LIKE')('target:%')] + query = query.filter(or_(*conditions)) + except AttributeError: + log_msg = _("'migration_status' column could not be found.") + LOG.debug(log_msg) + return None + + # Apply exact match filters for everything else, ensure that the + # filter value exists on the model + for key in filters.keys(): + # metadata is unique, must be a dict + if key == 'metadata': + if not isinstance(filters[key], dict): + log_msg = _("'metadata' filter value is not valid.") + LOG.debug(log_msg) + return None + continue + try: + column_attr = getattr(models.Volume, key) + # Do not allow relationship properties since those require + # schema specific knowledge + prop = getattr(column_attr, 'property') + if isinstance(prop, RelationshipProperty): + log_msg = (_("'%s' filter key is not valid, " + "it maps to a relationship.")) % key + LOG.debug(log_msg) + return None + except AttributeError: + log_msg = _("'%s' filter key is not valid.") % key + LOG.debug(log_msg) + return None + + # Holds the simple exact matches + filter_dict = {} + + # Iterate over all filters, special case the filter is necessary + for key, value in filters.iteritems(): + if key == 'metadata': + # model.VolumeMetadata defines the backref to Volumes as + # 'volume_metadata', use that column attribute key + key = 'volume_metadata' + column_attr = getattr(models.Volume, key) + for k, v in value.iteritems(): + query = query.filter(column_attr.any(key=k, value=v)) + elif isinstance(value, (list, tuple, set, frozenset)): + # Looking for values in a list; apply to query directly + column_attr = getattr(models.Volume, key) + query = query.filter(column_attr.in_(value)) + else: + # OK, simple exact match; save for later + filter_dict[key] = value + + # Apply simple exact matches + if filter_dict: + query = query.filter_by(**filter_dict) + + marker_volume = None + if marker is not None: + marker_volume = _volume_get(context, marker, session) + + return sqlalchemyutils.paginate_query(query, models.Volume, limit, + [sort_key, 'created_at', 'id'], + marker=marker_volume, + sort_dir=sort_dir) + + @require_admin_context def volume_get_iscsi_target_num(context, volume_id): result = model_query(context, models.IscsiTarget, read_deleted="yes").\ diff --git a/cinder/tests/api/v1/test_volumes.py b/cinder/tests/api/v1/test_volumes.py index d072cd63ee2..1dcf653ecf9 100644 --- a/cinder/tests/api/v1/test_volumes.py +++ b/cinder/tests/api/v1/test_volumes.py @@ -17,7 +17,6 @@ import datetime from lxml import etree from oslo.config import cfg -import urllib import webob from cinder.api import extensions @@ -517,130 +516,6 @@ class VolumeApiTest(test.TestCase): 'size': 1}]} self.assertEqual(res_dict, expected) - def test_volume_list_by_name(self): - def stub_volume_get_all_by_project(context, project_id, marker, limit, - sort_key, sort_dir): - return [ - stubs.stub_volume(1, display_name='vol1'), - stubs.stub_volume(2, display_name='vol2'), - stubs.stub_volume(3, display_name='vol3'), - ] - self.stubs.Set(db, 'volume_get', stubs.stub_volume_get_db) - self.stubs.Set(db, 'volume_get_all_by_project', - stub_volume_get_all_by_project) - - # no display_name filter - req = fakes.HTTPRequest.blank('/v1/volumes') - resp = self.controller.index(req) - self.assertEqual(len(resp['volumes']), 3) - # filter on display_name - req = fakes.HTTPRequest.blank('/v1/volumes?display_name=vol2') - resp = self.controller.index(req) - self.assertEqual(len(resp['volumes']), 1) - self.assertEqual(resp['volumes'][0]['display_name'], 'vol2') - # filter no match - req = fakes.HTTPRequest.blank('/v1/volumes?display_name=vol4') - resp = self.controller.index(req) - self.assertEqual(len(resp['volumes']), 0) - - def test_volume_list_by_metadata(self): - def stub_volume_get_all_by_project(context, project_id, marker, limit, - sort_key, sort_dir): - return [ - stubs.stub_volume(1, display_name='vol1', - status='available', - volume_metadata=[{'key': 'key1', - 'value': 'value1'}]), - stubs.stub_volume(2, display_name='vol2', - status='available', - volume_metadata=[{'key': 'key1', - 'value': 'value2'}]), - stubs.stub_volume(3, display_name='vol3', - status='in-use', - volume_metadata=[{'key': 'key1', - 'value': 'value2'}]), - ] - self.stubs.Set(db, 'volume_get_all_by_project', - stub_volume_get_all_by_project) - - # no metadata filter - req = fakes.HTTPRequest.blank('/v1/volumes', use_admin_context=True) - resp = self.controller.index(req) - self.assertEqual(len(resp['volumes']), 3) - - # single match - qparams = urllib.urlencode({'metadata': {'key1': 'value1'}}) - req = fakes.HTTPRequest.blank('/v1/volumes?%s' % qparams, - use_admin_context=True) - resp = self.controller.index(req) - self.assertEqual(len(resp['volumes']), 1) - self.assertEqual(resp['volumes'][0]['display_name'], 'vol1') - self.assertEqual(resp['volumes'][0]['metadata']['key1'], 'value1') - - # multiple matches - qparams = urllib.urlencode({'metadata': {'key1': 'value2'}}) - req = fakes.HTTPRequest.blank('/v1/volumes?%s' % qparams, - use_admin_context=True) - resp = self.controller.index(req) - self.assertEqual(len(resp['volumes']), 2) - for volume in resp['volumes']: - self.assertEqual(volume['metadata']['key1'], 'value2') - - # multiple filters - qparams = urllib.urlencode({'metadata': {'key1': 'value2'}}) - req = fakes.HTTPRequest.blank('/v1/volumes?status=in-use&%s' % qparams, - use_admin_context=True) - resp = self.controller.index(req) - self.assertEqual(len(resp['volumes']), 1) - self.assertEqual(resp['volumes'][0]['display_name'], 'vol3') - - # no match - qparams = urllib.urlencode({'metadata': {'key1': 'value3'}}) - req = fakes.HTTPRequest.blank('/v1/volumes?%s' % qparams, - use_admin_context=True) - resp = self.controller.index(req) - self.assertEqual(len(resp['volumes']), 0) - - def test_volume_list_by_status(self): - def stub_volume_get_all_by_project(context, project_id, marker, limit, - sort_key, sort_dir): - return [ - stubs.stub_volume(1, display_name='vol1', status='available'), - stubs.stub_volume(2, display_name='vol2', status='available'), - stubs.stub_volume(3, display_name='vol3', status='in-use'), - ] - self.stubs.Set(db, 'volume_get', stubs.stub_volume_get_db) - self.stubs.Set(db, 'volume_get_all_by_project', - stub_volume_get_all_by_project) - - # no status filter - req = fakes.HTTPRequest.blank('/v1/volumes') - resp = self.controller.index(req) - self.assertEqual(len(resp['volumes']), 3) - # single match - req = fakes.HTTPRequest.blank('/v1/volumes?status=in-use') - resp = self.controller.index(req) - self.assertEqual(len(resp['volumes']), 1) - self.assertEqual(resp['volumes'][0]['status'], 'in-use') - # multiple match - req = fakes.HTTPRequest.blank('/v1/volumes?status=available') - resp = self.controller.index(req) - self.assertEqual(len(resp['volumes']), 2) - for volume in resp['volumes']: - self.assertEqual(volume['status'], 'available') - # multiple filters - req = fakes.HTTPRequest.blank('/v1/volumes?status=available&' - 'display_name=vol1') - resp = self.controller.index(req) - self.assertEqual(len(resp['volumes']), 1) - self.assertEqual(resp['volumes'][0]['display_name'], 'vol1') - self.assertEqual(resp['volumes'][0]['status'], 'available') - # no match - req = fakes.HTTPRequest.blank('/v1/volumes?status=in-use&' - 'display_name=vol1') - resp = self.controller.index(req) - self.assertEqual(len(resp['volumes']), 0) - def test_volume_show(self): self.stubs.Set(db, 'volume_get', stubs.stub_volume_get_db) @@ -740,7 +615,8 @@ class VolumeApiTest(test.TestCase): def test_volume_detail_limit_offset(self): def volume_detail_limit_offset(is_admin): def stub_volume_get_all_by_project(context, project_id, marker, - limit, sort_key, sort_dir): + limit, sort_key, sort_dir, + filters=None): return [ stubs.stub_volume(1, display_name='vol1'), stubs.stub_volume(2, display_name='vol2'), diff --git a/cinder/tests/api/v2/stubs.py b/cinder/tests/api/v2/stubs.py index 49c2b80a197..dc269a67d99 100644 --- a/cinder/tests/api/v2/stubs.py +++ b/cinder/tests/api/v2/stubs.py @@ -109,7 +109,7 @@ def stub_volume_get_db(context, volume_id): def stub_volume_get_all(context, search_opts=None, marker=None, limit=None, - sort_key='created_at', sort_dir='desc'): + sort_key='created_at', sort_dir='desc', filters=None): return [stub_volume(100, project_id='fake'), stub_volume(101, project_id='superfake'), stub_volume(102, project_id='superduperfake')] diff --git a/cinder/tests/api/v2/test_volumes.py b/cinder/tests/api/v2/test_volumes.py index efdda4e87c0..fab7d98b6d1 100644 --- a/cinder/tests/api/v2/test_volumes.py +++ b/cinder/tests/api/v2/test_volumes.py @@ -18,7 +18,6 @@ import datetime from lxml import etree from oslo.config import cfg -import urllib import webob from cinder.api import extensions @@ -584,7 +583,7 @@ class VolumeApiTest(test.TestCase): def test_volume_index_with_marker(self): def stub_volume_get_all_by_project(context, project_id, marker, limit, - sort_key, sort_dir): + sort_key, sort_dir, filters=None): return [ stubs.stub_volume(1, display_name='vol1'), stubs.stub_volume(2, display_name='vol2'), @@ -635,7 +634,7 @@ class VolumeApiTest(test.TestCase): def test_volume_index_limit_offset(self): def stub_volume_get_all_by_project(context, project_id, marker, limit, - sort_key, sort_dir): + sort_key, sort_dir, filters=None): return [ stubs.stub_volume(1, display_name='vol1'), stubs.stub_volume(2, display_name='vol2'), @@ -662,7 +661,7 @@ class VolumeApiTest(test.TestCase): def test_volume_detail_with_marker(self): def stub_volume_get_all_by_project(context, project_id, marker, limit, - sort_key, sort_dir): + sort_key, sort_dir, filters=None): return [ stubs.stub_volume(1, display_name='vol1'), stubs.stub_volume(2, display_name='vol2'), @@ -713,7 +712,7 @@ class VolumeApiTest(test.TestCase): def test_volume_detail_limit_offset(self): def stub_volume_get_all_by_project(context, project_id, marker, limit, - sort_key, sort_dir): + sort_key, sort_dir, filters=None): return [ stubs.stub_volume(1, display_name='vol1'), stubs.stub_volume(2, display_name='vol2'), @@ -760,7 +759,8 @@ class VolumeApiTest(test.TestCase): # Number of volumes equals the max, include next link def stub_volume_get_all(context, marker, limit, - sort_key, sort_dir): + sort_key, sort_dir, + filters=None): vols = [stubs.stub_volume(i) for i in xrange(CONF.osapi_max_limit)] if limit == None or limit >= len(vols): @@ -777,7 +777,8 @@ class VolumeApiTest(test.TestCase): # Number of volumes less then max, do not include def stub_volume_get_all2(context, marker, limit, - sort_key, sort_dir): + sort_key, sort_dir, + filters=None): vols = [stubs.stub_volume(i) for i in xrange(100)] if limit == None or limit >= len(vols): @@ -793,7 +794,8 @@ class VolumeApiTest(test.TestCase): # Number of volumes more then the max, include next link def stub_volume_get_all3(context, marker, limit, - sort_key, sort_dir): + sort_key, sort_dir, + filters=None): vols = [stubs.stub_volume(i) for i in xrange(CONF.osapi_max_limit + 100)] if limit == None or limit >= len(vols): @@ -819,130 +821,78 @@ class VolumeApiTest(test.TestCase): volumes_links = res_dict['volumes_links'] self.assertEqual(volumes_links[0]['rel'], 'next') - def test_volume_list_by_name(self): + def test_volume_list_default_filters(self): + """Tests that the default filters from volume.api.API.get_all are set. + + 1. 'no_migration_status'=True for non-admins and get_all_by_project is + invoked. + 2. 'no_migration_status' is not included for admins. + 3. When 'all_tenants' is not specified, then it is removed and + get_all_by_project is invoked for admins. + 3. When 'all_tenants' is specified, then it is removed and get_all + is invoked for admins. + """ + # Non-admin, project function should be called with no_migration_status def stub_volume_get_all_by_project(context, project_id, marker, limit, - sort_key, sort_dir): - return [ - stubs.stub_volume(1, display_name='vol1'), - stubs.stub_volume(2, display_name='vol2'), - stubs.stub_volume(3, display_name='vol3'), - ] + sort_key, sort_dir, filters=None): + self.assertEqual(filters['no_migration_targets'], True) + self.assertFalse('all_tenants' in filters) + return [stubs.stub_volume(1, display_name='vol1')] + + def stub_volume_get_all(context, marker, limit, + sort_key, sort_dir, filters=None): + return [] self.stubs.Set(db, 'volume_get_all_by_project', stub_volume_get_all_by_project) - self.stubs.Set(volume_api.API, 'get', stubs.stub_volume_get) + self.stubs.Set(db, 'volume_get_all', stub_volume_get_all) - # no name filter - req = fakes.HTTPRequest.blank('/v2/volumes') - resp = self.controller.index(req) - self.assertEqual(len(resp['volumes']), 3) - # filter on name - req = fakes.HTTPRequest.blank('/v2/volumes?name=vol2') + # all_tenants does not matter for non-admin + for params in ['', '?all_tenants=1']: + req = fakes.HTTPRequest.blank('/v2/volumes%s' % params) + resp = self.controller.index(req) + self.assertEqual(len(resp['volumes']), 1) + self.assertEqual(resp['volumes'][0]['name'], 'vol1') + + # Admin, all_tenants is not set, project function should be called + # without no_migration_status + def stub_volume_get_all_by_project2(context, project_id, marker, limit, + sort_key, sort_dir, filters=None): + self.assertFalse('no_migration_targets' in filters) + return [stubs.stub_volume(1, display_name='vol2')] + + def stub_volume_get_all2(context, marker, limit, + sort_key, sort_dir, filters=None): + return [] + self.stubs.Set(db, 'volume_get_all_by_project', + stub_volume_get_all_by_project2) + self.stubs.Set(db, 'volume_get_all', stub_volume_get_all2) + + req = fakes.HTTPRequest.blank('/v2/volumes', use_admin_context=True) resp = self.controller.index(req) self.assertEqual(len(resp['volumes']), 1) self.assertEqual(resp['volumes'][0]['name'], 'vol2') - # filter no match - req = fakes.HTTPRequest.blank('/v2/volumes?name=vol4') - resp = self.controller.index(req) - self.assertEqual(len(resp['volumes']), 0) - def test_volume_list_by_metadata(self): - def stub_volume_get_all_by_project(context, project_id, marker, limit, - sort_key, sort_dir): - return [ - stubs.stub_volume(1, display_name='vol1', - status='available', - volume_metadata=[{'key': 'key1', - 'value': 'value1'}]), - stubs.stub_volume(2, display_name='vol2', - status='available', - volume_metadata=[{'key': 'key1', - 'value': 'value2'}]), - stubs.stub_volume(3, display_name='vol3', - status='in-use', - volume_metadata=[{'key': 'key1', - 'value': 'value2'}]), - ] + # Admin, all_tenants is set, get_all function should be called + # without no_migration_status + def stub_volume_get_all_by_project3(context, project_id, marker, limit, + sort_key, sort_dir, filters=None): + return [] + + def stub_volume_get_all3(context, marker, limit, + sort_key, sort_dir, filters=None): + self.assertFalse('no_migration_targets' in filters) + self.assertFalse('all_tenants' in filters) + return [stubs.stub_volume(1, display_name='vol3')] self.stubs.Set(db, 'volume_get_all_by_project', - stub_volume_get_all_by_project) + stub_volume_get_all_by_project3) + self.stubs.Set(db, 'volume_get_all', stub_volume_get_all3) - # no metadata filter - req = fakes.HTTPRequest.blank('/v2/volumes', use_admin_context=True) - resp = self.controller.detail(req) - self.assertEqual(len(resp['volumes']), 3) - - # single match - qparams = urllib.urlencode({'metadata': {'key1': 'value1'}}) - req = fakes.HTTPRequest.blank('/v2/volumes?%s' % qparams, + req = fakes.HTTPRequest.blank('/v2/volumes?all_tenants=1', use_admin_context=True) - resp = self.controller.detail(req) - self.assertEqual(len(resp['volumes']), 1) - self.assertEqual(resp['volumes'][0]['name'], 'vol1') - self.assertEqual(resp['volumes'][0]['metadata']['key1'], 'value1') - - # multiple matches - qparams = urllib.urlencode({'metadata': {'key1': 'value2'}}) - req = fakes.HTTPRequest.blank('/v2/volumes?%s' % qparams, - use_admin_context=True) - resp = self.controller.detail(req) - self.assertEqual(len(resp['volumes']), 2) - for volume in resp['volumes']: - self.assertEqual(volume['metadata']['key1'], 'value2') - - # multiple filters - qparams = urllib.urlencode({'metadata': {'key1': 'value2'}}) - req = fakes.HTTPRequest.blank('/v2/volumes?status=in-use&%s' % qparams, - use_admin_context=True) - resp = self.controller.detail(req) + resp = self.controller.index(req) self.assertEqual(len(resp['volumes']), 1) self.assertEqual(resp['volumes'][0]['name'], 'vol3') - # no match - qparams = urllib.urlencode({'metadata': {'key1': 'value3'}}) - req = fakes.HTTPRequest.blank('/v2/volumes?%s' % qparams, - use_admin_context=True) - resp = self.controller.detail(req) - self.assertEqual(len(resp['volumes']), 0) - - def test_volume_list_by_status(self): - def stub_volume_get_all_by_project(context, project_id, marker, limit, - sort_key, sort_dir): - return [ - stubs.stub_volume(1, display_name='vol1', status='available'), - stubs.stub_volume(2, display_name='vol2', status='available'), - stubs.stub_volume(3, display_name='vol3', status='in-use'), - ] - self.stubs.Set(db, 'volume_get_all_by_project', - stub_volume_get_all_by_project) - self.stubs.Set(volume_api.API, 'get', stubs.stub_volume_get) - - # no status filter - req = fakes.HTTPRequest.blank('/v2/volumes/details') - resp = self.controller.detail(req) - self.assertEqual(len(resp['volumes']), 3) - # single match - req = fakes.HTTPRequest.blank('/v2/volumes/details?status=in-use') - resp = self.controller.detail(req) - self.assertEqual(len(resp['volumes']), 1) - self.assertEqual(resp['volumes'][0]['status'], 'in-use') - # multiple match - req = fakes.HTTPRequest.blank('/v2/volumes/details/?status=available') - resp = self.controller.detail(req) - self.assertEqual(len(resp['volumes']), 2) - for volume in resp['volumes']: - self.assertEqual(volume['status'], 'available') - # multiple filters - req = fakes.HTTPRequest.blank('/v2/volumes/details/?status=available&' - 'name=vol1') - resp = self.controller.detail(req) - self.assertEqual(len(resp['volumes']), 1) - self.assertEqual(resp['volumes'][0]['name'], 'vol1') - self.assertEqual(resp['volumes'][0]['status'], 'available') - # no match - req = fakes.HTTPRequest.blank('/v2/volumes/details?status=in-use&' - 'name=vol1') - resp = self.controller.detail(req) - self.assertEqual(len(resp['volumes']), 0) - def test_volume_show(self): self.stubs.Set(volume_api.API, 'get', stubs.stub_volume_get) diff --git a/cinder/tests/test_db_api.py b/cinder/tests/test_db_api.py index cb1cd7a5c92..0e9d49cb560 100644 --- a/cinder/tests/test_db_api.py +++ b/cinder/tests/test_db_api.py @@ -1,3 +1,4 @@ +# Copyright 2014 IBM Corp. # Licensed under the Apache License, Version 2.0 (the "License"); you may # not use this file except in compliance with the License. You may obtain # a copy of the License at @@ -410,6 +411,257 @@ class DBAPIVolumeTestCase(BaseTest): self.ctxt, 'p%d' % i, None, None, 'host', None)) + def test_volume_get_by_name(self): + db.volume_create(self.ctxt, {'display_name': 'vol1'}) + db.volume_create(self.ctxt, {'display_name': 'vol2'}) + db.volume_create(self.ctxt, {'display_name': 'vol3'}) + + # no name filter + volumes = db.volume_get_all(self.ctxt, None, None, 'created_at', + 'asc') + self.assertEqual(len(volumes), 3) + # filter on name + volumes = db.volume_get_all(self.ctxt, None, None, 'created_at', + 'asc', {'display_name': 'vol2'}) + self.assertEqual(len(volumes), 1) + self.assertEqual(volumes[0]['display_name'], 'vol2') + # filter no match + volumes = db.volume_get_all(self.ctxt, None, None, 'created_at', + 'asc', {'display_name': 'vol4'}) + self.assertEqual(len(volumes), 0) + + def test_volume_list_by_status(self): + db.volume_create(self.ctxt, {'display_name': 'vol1', + 'status': 'available'}) + db.volume_create(self.ctxt, {'display_name': 'vol2', + 'status': 'available'}) + db.volume_create(self.ctxt, {'display_name': 'vol3', + 'status': 'in-use'}) + + # no status filter + volumes = db.volume_get_all(self.ctxt, None, None, 'created_at', + 'asc') + self.assertEqual(len(volumes), 3) + # single match + volumes = db.volume_get_all(self.ctxt, None, None, 'created_at', + 'asc', {'status': 'in-use'}) + self.assertEqual(len(volumes), 1) + self.assertEqual(volumes[0]['status'], 'in-use') + # multiple match + volumes = db.volume_get_all(self.ctxt, None, None, 'created_at', + 'asc', {'status': 'available'}) + self.assertEqual(len(volumes), 2) + for volume in volumes: + self.assertEqual(volume['status'], 'available') + # multiple filters + volumes = db.volume_get_all(self.ctxt, None, None, 'created_at', + 'asc', {'status': 'available', + 'display_name': 'vol1'}) + self.assertEqual(len(volumes), 1) + self.assertEqual(volumes[0]['display_name'], 'vol1') + self.assertEqual(volumes[0]['status'], 'available') + # no match + volumes = db.volume_get_all(self.ctxt, None, None, 'created_at', + 'asc', {'status': 'in-use', + 'display_name': 'vol1'}) + self.assertEqual(len(volumes), 0) + + def _assertEqualsVolumeOrderResult(self, correct_order, limit=None, + sort_key='created_at', sort_dir='asc', + filters=None, project_id=None, + match_keys=['id', 'display_name', + 'volume_metadata', + 'created_at']): + """"Verifies that volumes are returned in the correct order.""" + if project_id: + result = db.volume_get_all_by_project(self.ctxt, project_id, None, + limit, sort_key, + sort_dir, filters=filters) + else: + result = db.volume_get_all(self.ctxt, None, limit, sort_key, + sort_dir, filters=filters) + self.assertEqual(len(correct_order), len(result)) + self.assertEqual(len(result), len(correct_order)) + for vol1, vol2 in zip(result, correct_order): + for key in match_keys: + val1 = vol1.get(key) + val2 = vol2.get(key) + # metadata is a list, compare the 'key' and 'value' of each + if key == 'volume_metadata': + self.assertEqual(len(val1), len(val2)) + for m1, m2 in zip(val1, val2): + self.assertEqual(m1.get('key'), m2.get('key')) + self.assertEqual(m1.get('value'), m2.get('value')) + else: + self.assertEqual(val1, val2) + + def test_volume_get_by_filter(self): + """Verifies that all filtering is done at the DB layer.""" + vols = [] + vols.extend([db.volume_create(self.ctxt, + {'project_id': 'g1', + 'display_name': 'name_%d' % i, + 'size': 1}) + for i in xrange(2)]) + vols.extend([db.volume_create(self.ctxt, + {'project_id': 'g1', + 'display_name': 'name_%d' % i, + 'size': 2}) + for i in xrange(2)]) + vols.extend([db.volume_create(self.ctxt, + {'project_id': 'g1', + 'display_name': 'name_%d' % i}) + for i in xrange(2)]) + vols.extend([db.volume_create(self.ctxt, + {'project_id': 'g2', + 'display_name': 'name_%d' % i, + 'size': 1}) + for i in xrange(2)]) + + # By project, filter on size and name + filters = {'size': '1'} + correct_order = [vols[0], vols[1]] + self._assertEqualsVolumeOrderResult(correct_order, filters=filters, + project_id='g1') + filters = {'size': '1', 'display_name': 'name_1'} + correct_order = [vols[1]] + self._assertEqualsVolumeOrderResult(correct_order, filters=filters, + project_id='g1') + + # Remove project scope + filters = {'size': '1'} + correct_order = [vols[0], vols[1], vols[6], vols[7]] + self._assertEqualsVolumeOrderResult(correct_order, filters=filters) + filters = {'size': '1', 'display_name': 'name_1'} + correct_order = [vols[1], vols[7]] + self._assertEqualsVolumeOrderResult(correct_order, filters=filters) + + # Remove size constraint + filters = {'display_name': 'name_1'} + correct_order = [vols[1], vols[3], vols[5]] + self._assertEqualsVolumeOrderResult(correct_order, filters=filters, + project_id='g1') + correct_order = [vols[1], vols[3], vols[5], vols[7]] + self._assertEqualsVolumeOrderResult(correct_order, filters=filters) + + # Verify bogus values return nothing + filters = {'display_name': 'name_1', 'bogus_value': 'foo'} + self._assertEqualsVolumeOrderResult([], filters=filters, + project_id='g1') + self._assertEqualsVolumeOrderResult([], project_id='bogus') + self._assertEqualsVolumeOrderResult([], filters=filters) + self._assertEqualsVolumeOrderResult([], filters={'metadata': + 'not valid'}) + self._assertEqualsVolumeOrderResult([], filters={'metadata': + ['not', 'valid']}) + + # Verify that relationship property keys return nothing, these + # exist on the Volumes model but are not columns + filters = {'volume_type': 'bogus_type'} + self._assertEqualsVolumeOrderResult([], filters=filters) + + def test_volume_get_all_filters_limit(self): + vol1 = db.volume_create(self.ctxt, {'display_name': 'test1'}) + vol2 = db.volume_create(self.ctxt, {'display_name': 'test2'}) + vol3 = db.volume_create(self.ctxt, {'display_name': 'test2', + 'metadata': {'key1': 'val1'}}) + vol4 = db.volume_create(self.ctxt, {'display_name': 'test3', + 'metadata': {'key1': 'val1', + 'key2': 'val2'}}) + vol5 = db.volume_create(self.ctxt, {'display_name': 'test3', + 'metadata': {'key2': 'val2', + 'key3': 'val3'}, + 'host': 'host5'}) + vols = [vol1, vol2, vol3, vol4, vol5] + + # Ensure we have 5 total instances + self._assertEqualsVolumeOrderResult(vols) + + # No filters, test limit + self._assertEqualsVolumeOrderResult(vols[:1], limit=1) + self._assertEqualsVolumeOrderResult(vols[:4], limit=4) + + # Just the test2 volumes + filters = {'display_name': 'test2'} + self._assertEqualsVolumeOrderResult([vol2, vol3], filters=filters) + self._assertEqualsVolumeOrderResult([vol2], limit=1, + filters=filters) + self._assertEqualsVolumeOrderResult([vol2, vol3], limit=2, + filters=filters) + self._assertEqualsVolumeOrderResult([vol2, vol3], limit=100, + filters=filters) + + # metdata filters + filters = {'metadata': {'key1': 'val1'}} + self._assertEqualsVolumeOrderResult([vol3, vol4], filters=filters) + self._assertEqualsVolumeOrderResult([vol3], limit=1, + filters=filters) + self._assertEqualsVolumeOrderResult([vol3, vol4], limit=10, + filters=filters) + + filters = {'metadata': {'key1': 'val1', + 'key2': 'val2'}} + self._assertEqualsVolumeOrderResult([vol4], filters=filters) + self._assertEqualsVolumeOrderResult([vol4], limit=1, + filters=filters) + + # No match + filters = {'metadata': {'key1': 'val1', + 'key2': 'val2', + 'key3': 'val3'}} + self._assertEqualsVolumeOrderResult([], filters=filters) + filters = {'metadata': {'key1': 'val1', + 'key2': 'bogus'}} + self._assertEqualsVolumeOrderResult([], filters=filters) + filters = {'metadata': {'key1': 'val1', + 'key2': 'val1'}} + self._assertEqualsVolumeOrderResult([], filters=filters) + + # Combination + filters = {'display_name': 'test2', + 'metadata': {'key1': 'val1'}} + self._assertEqualsVolumeOrderResult([vol3], filters=filters) + self._assertEqualsVolumeOrderResult([vol3], limit=1, + filters=filters) + self._assertEqualsVolumeOrderResult([vol3], limit=100, + filters=filters) + filters = {'display_name': 'test3', + 'metadata': {'key2': 'val2', + 'key3': 'val3'}, + 'host': 'host5'} + self._assertEqualsVolumeOrderResult([vol5], filters=filters) + self._assertEqualsVolumeOrderResult([vol5], limit=1, + filters=filters) + + def test_volume_get_no_migration_targets(self): + """Verifies the unique 'no_migration_targets'=True filter. + + This filter returns volumes with either a NULL 'migration_status' + or a non-NULL value that does not start with 'target:'. + """ + vol1 = db.volume_create(self.ctxt, {'display_name': 'test1'}) + vol2 = db.volume_create(self.ctxt, {'display_name': 'test2', + 'migration_status': 'bogus'}) + vol3 = db.volume_create(self.ctxt, {'display_name': 'test3', + 'migration_status': 'btarget:'}) + vol4 = db.volume_create(self.ctxt, {'display_name': 'test4', + 'migration_status': 'target:'}) + vols = [vol1, vol2, vol3, vol4] + + # Ensure we have 4 total instances + self._assertEqualsVolumeOrderResult(vols) + + # Apply the unique filter + filters = {'no_migration_targets': True} + self._assertEqualsVolumeOrderResult([vol1, vol2, vol3], + filters=filters) + self._assertEqualsVolumeOrderResult([vol1, vol2], limit=2, + filters=filters) + + filters = {'no_migration_targets': True, + 'display_name': 'test4'} + self._assertEqualsVolumeOrderResult([], filters=filters) + def test_volume_get_iscsi_target_num(self): target = db.iscsi_target_create_safe(self.ctxt, {'volume_id': 42, 'target_num': 43}) diff --git a/cinder/volume/api.py b/cinder/volume/api.py index 37c3de30d8a..19cdade7cf7 100644 --- a/cinder/volume/api.py +++ b/cinder/volume/api.py @@ -267,8 +267,10 @@ class API(base.Base): return volume def get_all(self, context, marker=None, limit=None, sort_key='created_at', - sort_dir='desc', filters={}): + sort_dir='desc', filters=None): check_policy(context, 'get_all') + if filters == None: + filters = {} try: if limit is not None: @@ -280,60 +282,27 @@ class API(base.Base): msg = _('limit param must be an integer') raise exception.InvalidInput(reason=msg) - if (context.is_admin and 'all_tenants' in filters): - # Need to remove all_tenants to pass the filtering below. - del filters['all_tenants'] - volumes = self.db.volume_get_all(context, marker, limit, sort_key, - sort_dir) - else: - volumes = self.db.volume_get_all_by_project(context, - context.project_id, - marker, limit, - sort_key, sort_dir) - - # Non-admin shouldn't see temporary target of a volume migration + # Non-admin shouldn't see temporary target of a volume migration, add + # unique filter data to reflect that only volumes with a NULL + # 'migration_status' or a 'migration_status' that does not start with + # 'target:' should be returned (processed in db/sqlalchemy/api.py) if not context.is_admin: filters['no_migration_targets'] = True if filters: - LOG.debug(_("Searching by: %s") % filters) + LOG.debug(_("Searching by: %s") % str(filters)) - def _check_metadata_match(volume, searchdict): - volume_metadata = {} - for i in volume.get('volume_metadata'): - volume_metadata[i['key']] = i['value'] - - for k, v in searchdict.iteritems(): - if (k not in volume_metadata.keys() or - volume_metadata[k] != v): - return False - return True - - def _check_migration_target(volume, searchdict): - status = volume['migration_status'] - if status and status.startswith('target:'): - return False - return True - - # search_option to filter_name mapping. - filter_mapping = {'metadata': _check_metadata_match, - 'no_migration_targets': _check_migration_target} - - result = [] - not_found = object() - for volume in volumes: - # go over all filters in the list - for opt, values in filters.iteritems(): - try: - filter_func = filter_mapping[opt] - except KeyError: - def filter_func(volume, value): - return volume.get(opt, not_found) == value - if not filter_func(volume, values): - break # volume doesn't match this filter - else: # did not break out loop - result.append(volume) # volume matches all filters - volumes = result + if (context.is_admin and 'all_tenants' in filters): + # Need to remove all_tenants to pass the filtering below. + del filters['all_tenants'] + volumes = self.db.volume_get_all(context, marker, limit, sort_key, + sort_dir, filters=filters) + else: + volumes = self.db.volume_get_all_by_project(context, + context.project_id, + marker, limit, + sort_key, sort_dir, + filters=filters) return volumes