Add pagination to snapshots

Currently snapshot list does not provide pagination like volume does.

This patch adds pagination for snapshots in the same way that volume
does, using marker, limit, sort_keys and sort_dirs.

DocImpact
APIImpact: Use marker, limit, sort_keys and sort_dirs
Implements: snapshots-and-bakcup-support-pagination-query
Implements: blueprint extend-limit-implementations
Change-Id: I21edac6d29abf7fc2f446111a7d6888db9ab6eba
This commit is contained in:
Gorka Eguileor 2015-07-31 20:07:30 +02:00
parent 25d90467b1
commit e0e73a534a
11 changed files with 319 additions and 72 deletions

View File

@ -114,8 +114,8 @@ class SnapshotsController(wsgi.Controller):
# Pop out non search_opts and create local variables
search_opts = req.GET.copy()
search_opts.pop('limit', None)
search_opts.pop('offset', None)
sort_keys, sort_dirs = common.get_sort_params(search_opts)
marker, limit, offset = common.get_pagination_params(search_opts)
# Filter out invalid options
allowed_search_options = ('status', 'volume_id', 'name')
@ -128,18 +128,19 @@ class SnapshotsController(wsgi.Controller):
del search_opts['name']
snapshots = self.volume_api.get_all_snapshots(context,
search_opts=search_opts)
search_opts=search_opts,
marker=marker,
limit=limit,
sort_keys=sort_keys,
sort_dirs=sort_dirs,
offset=offset)
limited_list = common.limited(snapshots.objects, req)
req.cache_db_snapshots(limited_list)
snapshot_count = len(snapshots)
req.cache_db_snapshots(snapshots.objects)
if is_detail:
snapshots = self._view_builder.detail_list(req, limited_list,
snapshot_count)
snapshots = self._view_builder.detail_list(req, snapshots.objects)
else:
snapshots = self._view_builder.summary_list(req, limited_list,
snapshot_count)
snapshots = self._view_builder.summary_list(req, snapshots.objects)
return snapshots
@wsgi.response(202)

View File

@ -30,12 +30,12 @@ class ViewBuilder(common.ViewBuilder):
"""Initialize view builder."""
super(ViewBuilder, self).__init__()
def summary_list(self, request, snapshots, snapshot_count):
def summary_list(self, request, snapshots, snapshot_count=None):
"""Show a list of snapshots without many details."""
return self._list_view(self.summary, request, snapshots,
snapshot_count)
def detail_list(self, request, snapshots, snapshot_count):
def detail_list(self, request, snapshots, snapshot_count=None):
"""Detailed view of a list of snapshots."""
return self._list_view(self.detail, request, snapshots, snapshot_count,
coll_name=self._collection_name + '/detail')
@ -70,6 +70,13 @@ class ViewBuilder(common.ViewBuilder):
"""Provide a view for a list of snapshots."""
snapshots_list = [func(request, snapshot)['snapshot']
for snapshot in snapshots]
snapshots_links = self._get_collection_links(request,
snapshots,
coll_name,
snapshot_count)
snapshots_dict = {self._collection_name: snapshots_list}
if snapshots_links:
snapshots_dict[self._collection_name + '_links'] = snapshots_links
return snapshots_dict

View File

@ -292,14 +292,20 @@ def snapshot_get(context, snapshot_id):
return IMPL.snapshot_get(context, snapshot_id)
def snapshot_get_all(context, filters=None):
def snapshot_get_all(context, filters=None, marker=None, limit=None,
sort_keys=None, sort_dirs=None, offset=None):
"""Get all snapshots."""
return IMPL.snapshot_get_all(context, filters)
return IMPL.snapshot_get_all(context, filters, marker, limit, sort_keys,
sort_dirs, offset)
def snapshot_get_all_by_project(context, project_id, filters=None):
def snapshot_get_all_by_project(context, project_id, filters=None, marker=None,
limit=None, sort_keys=None, sort_dirs=None,
offset=None):
"""Get all snapshots belonging to a project."""
return IMPL.snapshot_get_all_by_project(context, project_id, filters)
return IMPL.snapshot_get_all_by_project(context, project_id, filters,
marker, limit, sort_keys,
sort_dirs, offset)
def snapshot_get_by_host(context, host, filters=None):

View File

@ -1505,7 +1505,8 @@ def volume_get_all_by_project(context, project_id, marker, limit,
def _generate_paginate_query(context, session, marker, limit, sort_keys,
sort_dirs, filters, offset=None):
sort_dirs, filters, offset=None,
paginate_type=models.Volume):
"""Generate the query to include the filters and the paginate options.
Returns a query with sorting / pagination criteria added or None
@ -1525,23 +1526,26 @@ def _generate_paginate_query(context, session, marker, limit, sort_keys,
is used for other values, see _process_volume_filters
function for more information
:param offset: number of items to skip
:param paginate_type: type of pagination to generate
:returns: updated query or None
"""
get_query, process_filters, get = PAGINATION_HELPERS[paginate_type]
sort_keys, sort_dirs = process_sort_params(sort_keys,
sort_dirs,
default_dir='desc')
query = _volume_get_query(context, session=session)
query = get_query(context, session=session)
if filters:
query = _process_volume_filters(query, filters)
query = process_filters(query, filters)
if query is None:
return None
marker_volume = None
if marker is not None:
marker_volume = _volume_get(context, marker, session)
marker_volume = get(context, marker, session)
return sqlalchemyutils.paginate_query(query, models.Volume, limit,
return sqlalchemyutils.paginate_query(query, paginate_type, limit,
sort_keys,
marker=marker_volume,
sort_dirs=sort_dirs,
@ -2057,22 +2061,50 @@ def snapshot_get(context, snapshot_id):
@require_admin_context
def snapshot_get_all(context, filters=None):
# Ensure that the filter value exists on the model
if filters:
for key in filters.keys():
try:
getattr(models.Snapshot, key)
except AttributeError:
LOG.debug("'%s' filter key is not valid.", key)
return []
def snapshot_get_all(context, filters=None, marker=None, limit=None,
sort_keys=None, sort_dirs=None, offset=None):
"""Retrieves all snapshots.
query = model_query(context, models.Snapshot)
If no sorting parameters are specified then returned snapshots are sorted
first by the 'created_at' key and then by the 'id' key in descending
order.
:param context: context to query under
:param filters: dictionary of filters; will do exact matching on values
: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_keys: list of attributes by which results should be sorted,
paired with corresponding item in sort_dirs
:param sort_dirs: list of directions in which results should be sorted,
paired with corresponding item in sort_keys
:returns: list of matching snapshots
"""
session = get_session()
with session.begin():
query = _generate_paginate_query(context, session, marker, limit,
sort_keys, sort_dirs, filters,
offset, models.Snapshot)
# No snapshots would match, return empty list
if not query:
return []
return query.all()
def _snaps_get_query(context, session=None, project_only=False):
return model_query(context, models.Snapshot, session=session,
project_only=project_only).\
options(joinedload('snapshot_metadata'))
def _process_snaps_filters(query, filters):
if filters:
# Ensure that filters' keys exist on the model
if not is_valid_model_filters(models.Snapshot, filters):
return None
query = query.filter_by(**filters)
return query.options(joinedload('snapshot_metadata')).all()
return query
@require_context
@ -2107,15 +2139,43 @@ def snapshot_get_all_for_cgsnapshot(context, cgsnapshot_id):
@require_context
def snapshot_get_all_by_project(context, project_id, filters=None):
def snapshot_get_all_by_project(context, project_id, filters=None, marker=None,
limit=None, sort_keys=None, sort_dirs=None,
offset=None):
""""Retrieves all snapshots in a project.
If no sorting parameters are specified then returned snapshots are sorted
first by the 'created_at' key and then by the 'id' key in descending
order.
:param context: context to query under
:param project_id: project for all snapshots being retrieved
:param filters: dictionary of filters; will do exact matching on values
: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_keys: list of attributes by which results should be sorted,
paired with corresponding item in sort_dirs
:param sort_dirs: list of directions in which results should be sorted,
paired with corresponding item in sort_keys
:returns: list of matching snapshots
"""
authorize_project_context(context, project_id)
query = model_query(context, models.Snapshot)
if filters:
query = query.filter_by(**filters)
# Add project_id to filters
filters = filters.copy() if filters else {}
filters['project_id'] = project_id
return query.filter_by(project_id=project_id).\
options(joinedload('snapshot_metadata')).all()
session = get_session()
with session.begin():
query = _generate_paginate_query(context, session, marker, limit,
sort_keys, sort_dirs, filters,
offset, models.Snapshot)
# No snapshots would match, return empty list
if not query:
return []
return query.all()
@require_context
@ -3854,3 +3914,12 @@ def driver_initiator_data_get(context, initiator, namespace):
filter_by(initiator=initiator).\
filter_by(namespace=namespace).\
all()
###############################
PAGINATION_HELPERS = {
models.Volume: (_volume_get_query, _process_volume_filters, _volume_get),
models.Snapshot: (_snaps_get_query, _process_snaps_filters, _snapshot_get)
}

View File

@ -215,8 +215,10 @@ class SnapshotList(base.ObjectListBase, base.CinderObject):
}
@base.remotable_classmethod
def get_all(cls, context, search_opts):
snapshots = db.snapshot_get_all(context, search_opts)
def get_all(cls, context, search_opts, marker=None, limit=None,
sort_keys=None, sort_dirs=None, offset=None):
snapshots = db.snapshot_get_all(context, search_opts, marker, limit,
sort_keys, sort_dirs, offset)
return base.obj_make_list(context, cls(), objects.Snapshot,
snapshots,
expected_attrs=['metadata'])
@ -228,9 +230,12 @@ class SnapshotList(base.ObjectListBase, base.CinderObject):
snapshots, expected_attrs=['metadata'])
@base.remotable_classmethod
def get_all_by_project(cls, context, project_id, search_opts):
snapshots = db.snapshot_get_all_by_project(context, project_id,
search_opts)
def get_all_by_project(cls, context, project_id, search_opts, marker=None,
limit=None, sort_keys=None, sort_dirs=None,
offset=None):
snapshots = db.snapshot_get_all_by_project(
context, project_id, search_opts, marker, limit, sort_keys,
sort_dirs, offset)
return base.obj_make_list(context, cls(context), objects.Snapshot,
snapshots, expected_attrs=['metadata'])

View File

@ -119,13 +119,16 @@ def stub_snapshot(id, **kwargs):
return snapshot
def stub_snapshot_get_all(self, search_opts=None):
def stub_snapshot_get_all(context, filters=None, marker=None, limit=None,
sort_keys=None, sort_dirs=None, offset=None):
return [stub_snapshot(100, project_id='fake'),
stub_snapshot(101, project_id='superfake'),
stub_snapshot(102, project_id='superduperfake')]
def stub_snapshot_get_all_by_project(self, context, search_opts=None):
def stub_snapshot_get_all_by_project(context, project_id, filters=None,
marker=None, limit=None, sort_keys=None,
sort_dirs=None, offset=None):
return [stub_snapshot(1)]

View File

@ -323,7 +323,9 @@ class SnapshotApiTest(test.TestCase):
snapshot_metadata_get):
def list_snapshots_with_limit_and_offset(is_admin):
def stub_snapshot_get_all_by_project(context, project_id,
search_opts):
filters=None, marker=None,
limit=None, sort_keys=None,
sort_dirs=None, offset=None):
return [
stubs.stub_snapshot(1, display_name='backup1'),
stubs.stub_snapshot(2, display_name='backup2'),

View File

@ -177,13 +177,16 @@ def stub_snapshot(id, **kwargs):
return snapshot
def stub_snapshot_get_all(self, search_opts=None):
def stub_snapshot_get_all(context, filters=None, marker=None, limit=None,
sort_keys=None, sort_dirs=None, offset=None):
return [stub_snapshot(100, project_id='fake'),
stub_snapshot(101, project_id='superfake'),
stub_snapshot(102, project_id='superduperfake')]
def stub_snapshot_get_all_by_project(self, context, search_opts=None):
def stub_snapshot_get_all_by_project(context, project_id, filters=None,
marker=None, limit=None, sort_keys=None,
sort_dirs=None, offset=None):
return [stub_snapshot(1)]

View File

@ -15,9 +15,12 @@
from lxml import etree
import mock
from oslo_config import cfg
from oslo_utils import timeutils
from six.moves.urllib import parse as urllib
import webob
from cinder.api import common
from cinder.api.v2 import snapshots
from cinder import context
from cinder import db
@ -32,6 +35,8 @@ from cinder.tests.unit import utils
from cinder import volume
CONF = cfg.CONF
UUID = '00000000-0000-0000-0000-000000000001'
INVALID_UUID = '00000000-0000-0000-0000-000000000002'
@ -79,6 +84,7 @@ class SnapshotApiTest(test.TestCase):
stubs.stub_snapshot_get_all_by_project)
self.stubs.Set(db, 'snapshot_get_all',
stubs.stub_snapshot_get_all)
self.ctx = context.RequestContext('admin', 'fakeproject', True)
@mock.patch(
@ -334,18 +340,7 @@ class SnapshotApiTest(test.TestCase):
@mock.patch('cinder.db.snapshot_metadata_get', return_value=dict())
def test_list_snapshots_with_limit_and_offset(self,
snapshot_metadata_get):
def list_snapshots_with_limit_and_offset(is_admin):
def stub_snapshot_get_all_by_project(context, project_id,
search_opts):
return [
stubs.stub_snapshot(1, display_name='backup1'),
stubs.stub_snapshot(2, display_name='backup2'),
stubs.stub_snapshot(3, display_name='backup3'),
]
self.stubs.Set(db, 'snapshot_get_all_by_project',
stub_snapshot_get_all_by_project)
def list_snapshots_with_limit_and_offset(snaps, is_admin):
req = fakes.HTTPRequest.blank('/v2/fake/snapshots?limit=1\
&offset=1',
use_admin_context=is_admin)
@ -353,12 +348,161 @@ class SnapshotApiTest(test.TestCase):
self.assertIn('snapshots', res)
self.assertEqual(1, len(res['snapshots']))
self.assertEqual('2', res['snapshots'][0]['id'])
self.assertEqual(snaps[1].id, res['snapshots'][0]['id'])
# Test that we get an empty list with an offset greater than the
# number of items
req = fakes.HTTPRequest.blank('/v2/snapshots?limit=1&offset=3')
self.assertEqual({'snapshots': []}, self.controller.index(req))
self.stubs.UnsetAll()
volume, snaps = self._create_db_snapshots(3)
# admin case
list_snapshots_with_limit_and_offset(is_admin=True)
list_snapshots_with_limit_and_offset(snaps, is_admin=True)
# non-admin case
list_snapshots_with_limit_and_offset(is_admin=False)
list_snapshots_with_limit_and_offset(snaps, is_admin=False)
@mock.patch.object(db, 'snapshot_get_all_by_project')
@mock.patch('cinder.db.snapshot_metadata_get', return_value=dict())
def test_list_snpashots_with_wrong_limit_and_offset(self,
mock_metadata_get,
mock_snapshot_get_all):
"""Test list with negative and non numeric limit and offset."""
mock_snapshot_get_all.return_value = []
# Negative limit
req = fakes.HTTPRequest.blank('/v2/snapshots?limit=-1&offset=1')
self.assertRaises(webob.exc.HTTPBadRequest,
self.controller.index,
req)
# Non numeric limit
req = fakes.HTTPRequest.blank('/v2/snapshots?limit=a&offset=1')
self.assertRaises(webob.exc.HTTPBadRequest,
self.controller.index,
req)
# Negative offset
req = fakes.HTTPRequest.blank('/v2/snapshots?limit=1&offset=-1')
self.assertRaises(webob.exc.HTTPBadRequest,
self.controller.index,
req)
# Non numeric offset
req = fakes.HTTPRequest.blank('/v2/snapshots?limit=1&offset=a')
self.assertRaises(webob.exc.HTTPBadRequest,
self.controller.index,
req)
def _assert_list_next(self, expected_query=None, project='fakeproject',
**kwargs):
"""Check a page of snapshots list."""
# Since we are accessing v2 api directly we don't need to specify
# v2 in the request path, if we did, we'd get /v2/v2 links back
request_path = '/%s/snapshots' % project
expected_path = '/v2' + request_path
# Construct the query if there are kwargs
if kwargs:
request_str = request_path + '?' + urllib.urlencode(kwargs)
else:
request_str = request_path
# Make the request
req = fakes.HTTPRequest.blank(request_str)
res = self.controller.index(req)
# We only expect to have a next link if there is an actual expected
# query.
if expected_query:
# We must have the links
self.assertIn('snapshots_links', res)
links = res['snapshots_links']
# Must be a list of links, even if we only get 1 back
self.assertTrue(list, type(links))
next_link = links[0]
# rel entry must be next
self.assertIn('rel', next_link)
self.assertIn('next', next_link['rel'])
# href entry must have the right path
self.assertIn('href', next_link)
href_parts = urllib.urlparse(next_link['href'])
self.assertEqual(expected_path, href_parts.path)
# And the query from the next link must match what we were
# expecting
params = urllib.parse_qs(href_parts.query)
self.assertDictEqual(expected_query, params)
# Make sure we don't have links if we were not expecting them
else:
self.assertNotIn('snapshots_links', res)
def _create_db_snapshots(self, num_snaps):
volume = utils.create_volume(self.ctx)
snaps = [utils.create_snapshot(self.ctx,
volume.id,
display_name='snap' + str(i))
for i in range(num_snaps)]
self.addCleanup(db.volume_destroy, self.ctx, volume.id)
for snap in snaps:
self.addCleanup(db.snapshot_destroy, self.ctx, snap.id)
snaps.reverse()
return volume, snaps
def test_list_snapshots_next_link_default_limit(self):
"""Test that snapshot list pagination is limited by osapi_max_limit."""
self.stubs.UnsetAll()
volume, snaps = self._create_db_snapshots(3)
# NOTE(geguileo): Since cinder.api.common.limited has already been
# imported his argument max_limit already has a default value of 1000
# so it doesn't matter that we change it to 2. That's why we need to
# mock it and send it current value. We still need to set the default
# value because other sections of the code use it, for example
# _get_collection_links
CONF.set_default('osapi_max_limit', 2)
def get_pagination_params(params, max_limit=CONF.osapi_max_limit,
original_call=common.get_pagination_params):
return original_call(params, max_limit)
def _get_limit_param(params, max_limit=CONF.osapi_max_limit,
original_call=common._get_limit_param):
return original_call(params, max_limit)
with mock.patch.object(common, 'get_pagination_params',
get_pagination_params), \
mock.patch.object(common, '_get_limit_param',
_get_limit_param):
# The link from the first page should link to the second
self._assert_list_next({'marker': [snaps[1].id]})
# Second page should have no next link
self._assert_list_next(marker=snaps[1].id)
def test_list_snapshots_next_link_with_limit(self):
"""Test snapshot list pagination with specific limit."""
self.stubs.UnsetAll()
volume, snaps = self._create_db_snapshots(2)
# The link from the first page should link to the second
self._assert_list_next({'limit': ['1'], 'marker': [snaps[0].id]},
limit=1)
# Even though there are no more elements, we should get a next element
# per specification.
expected = {'limit': ['1'], 'marker': [snaps[1].id]}
self._assert_list_next(expected, limit=1, marker=snaps[0].id)
# When we go beyond the number of elements there should be no more
# next links
self._assert_list_next(limit=1, marker=snaps[1].id)
@mock.patch('cinder.db.snapshot_metadata_get', return_value=dict())
def test_admin_list_snapshots_all_tenants(self, snapshot_metadata_get):

View File

@ -167,7 +167,8 @@ class TestSnapshotList(test_objects.BaseObjectsTestCase):
self.context, search_opts)
self.assertEqual(1, len(snapshots))
TestSnapshot._compare(self, fake_snapshot_obj, snapshots[0])
snapshot_get_all.assert_called_once_with(self.context, search_opts)
snapshot_get_all.assert_called_once_with(self.context, search_opts,
None, None, None, None, None)
@mock.patch('cinder.objects.Volume.get_by_id')
@mock.patch('cinder.db.snapshot_get_by_host',
@ -195,7 +196,8 @@ class TestSnapshotList(test_objects.BaseObjectsTestCase):
TestSnapshot._compare(self, fake_snapshot_obj, snapshots[0])
get_all_by_project.assert_called_once_with(self.context,
self.project_id,
search_opts)
search_opts, None, None,
None, None, None)
@mock.patch('cinder.objects.volume.Volume.get_by_id')
@mock.patch('cinder.db.snapshot_get_all_for_volume',
@ -270,4 +272,5 @@ class TestSnapshotList(test_objects.BaseObjectsTestCase):
snapshot_obj = copy.deepcopy(fake_snapshot_obj)
snapshot_obj['metadata'] = {'fake_key': 'fake_value'}
TestSnapshot._compare(self, snapshot_obj, snapshots[0])
snapshot_get_all.assert_called_once_with(self.context, search_opts)
snapshot_get_all.assert_called_once_with(self.context, search_opts,
None, None, None, None, None)

View File

@ -508,19 +508,23 @@ class API(base.Base):
LOG.info(_LI("Volume retrieved successfully."), resource=vref)
return dict(vref)
def get_all_snapshots(self, context, search_opts=None):
def get_all_snapshots(self, context, search_opts=None, marker=None,
limit=None, sort_keys=None, sort_dirs=None,
offset=None):
check_policy(context, 'get_all_snapshots')
search_opts = search_opts or {}
if (context.is_admin and 'all_tenants' in search_opts):
if context.is_admin and 'all_tenants' in search_opts:
# Need to remove all_tenants to pass the filtering below.
del search_opts['all_tenants']
snapshots = objects.SnapshotList.get_all(context,
search_opts)
snapshots = objects.SnapshotList.get_all(
context, search_opts, marker, limit, sort_keys, sort_dirs,
offset)
else:
snapshots = objects.SnapshotList.get_all_by_project(
context, context.project_id, search_opts)
context, context.project_id, search_opts, marker, limit,
sort_keys, sort_dirs, offset)
LOG.info(_LI("Get all snaphsots completed successfully."))
return snapshots