openstack
/
cinder
Code Issues Proposed changes

Merge "Add pagination to backups"

This commit is contained in:
Jenkins 2015-09-01 06:51:35 +00:00 committed by Gerrit Code Review
parent e33d1b20da 4a230e04ea
commit 0e1cdfb2e8
9 changed files with 243 additions and 59 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

21
cinder/api/contrib/backups.py
View File

@ -210,6 +210,8 @@ class BackupsController(wsgi.Controller):
"""Returns a list of backups, transformed through view builder."""
context = req.environ['cinder.context']
filters = req.params.copy()
marker, limit, offset = common.get_pagination_params(filters)
sort_keys, sort_dirs = common.get_sort_params(filters)
utils.remove_invalid_filter_options(context,
filters,
@ -219,17 +221,20 @@ class BackupsController(wsgi.Controller):
filters['display_name'] = filters['name']
del filters['name']
backups = self.backup_api.get_all(context, search_opts=filters)
backup_count = len(backups)
limited_list = common.limited(backups.objects, req)
req.cache_db_backups(limited_list)
backups = self.backup_api.get_all(context, search_opts=filters,
marker=marker,
limit=limit,
offset=offset,
sort_keys=sort_keys,
sort_dirs=sort_dirs,
)
req.cache_db_backups(backups.objects)
if is_detail:
backups = self._view_builder.detail_list(req, limited_list,
backup_count)
backups = self._view_builder.detail_list(req, backups.objects)
else:
backups = self._view_builder.summary_list(req, limited_list,
backup_count)
backups = self._view_builder.summary_list(req, backups.objects)
return backups
# TODO(frankm): Add some checks here including

14
cinder/api/views/backups.py
View File

@ -30,15 +30,13 @@ class ViewBuilder(common.ViewBuilder):
"""Initialize view builder."""
super(ViewBuilder, self).__init__()
def summary_list(self, request, backups, origin_backup_count):
def summary_list(self, request, backups, backup_count=None):
"""Show a list of backups without many details."""
return self._list_view(self.summary, request, backups,
origin_backup_count)
return self._list_view(self.summary, request, backups, backup_count)
def detail_list(self, request, backups, origin_backup_count):
def detail_list(self, request, backups, backup_count=None):
"""Detailed view of a list of backups ."""
return self._list_view(self.detail, request, backups,
origin_backup_count)
return self._list_view(self.detail, request, backups, backup_count)
def summary(self, request, backup):
"""Generic, non-detailed view of a backup."""
@ -82,13 +80,13 @@ class ViewBuilder(common.ViewBuilder):
}
}
def _list_view(self, func, request, backups, origin_backup_count):
def _list_view(self, func, request, backups, backup_count):
"""Provide a view for a list of backups."""
backups_list = [func(request, backup)['backup'] for backup in backups]
backups_links = self._get_collection_links(request,
backups,
self._collection_name,
origin_backup_count)
backup_count)
backups_dict = dict(backups=backups_list)
if backups_links:

14
cinder/backup/api.py
View File

@ -90,7 +90,7 @@ class API(base.Base):
# Don't allow backup to be deleted if there are incremental
# backups dependent on it.
deltas = self.get_all(context, {'parent_id': backup.id})
deltas = self.get_all(context, search_opts={'parent_id': backup.id})
if deltas and len(deltas):
msg = _('Incremental backups exist for this backup.')
raise exception.InvalidBackup(reason=msg)
@ -99,7 +99,8 @@ class API(base.Base):
backup.save()
self.backup_rpcapi.delete_backup(context, backup)
def get_all(self, context, search_opts=None):
def get_all(self, context, search_opts=None, marker=None, limit=None,
offset=None, sort_keys=None, sort_dirs=None):
check_policy(context, 'get_all')
search_opts = search_opts or {}
@ -110,12 +111,13 @@ class API(base.Base):
raise exception.InvalidParameterValue(err=msg)
if context.is_admin and strutils.bool_from_string(all_tenants):
backups = objects.BackupList.get_all(context, filters=search_opts)
backups = objects.BackupList.get_all(context, search_opts,
marker, limit, offset,
sort_keys, sort_dirs)
else:
backups = objects.BackupList.get_all_by_project(
context,
context.project_id,
filters=search_opts
context, context.project_id, search_opts,
marker, limit, offset, sort_keys, sort_dirs
)
return backups

16
cinder/db/api.py
View File

@ -853,9 +853,12 @@ def backup_get(context, backup_id):
return IMPL.backup_get(context, backup_id)
def backup_get_all(context, filters=None):
def backup_get_all(context, filters=None, marker=None, limit=None,
offset=None, sort_keys=None, sort_dirs=None):
"""Get all backups."""
return IMPL.backup_get_all(context, filters=filters)
return IMPL.backup_get_all(context, filters=filters, marker=marker,
limit=limit, offset=offset, sort_keys=sort_keys,
sort_dirs=sort_dirs)
def backup_get_all_by_host(context, host):
@ -868,10 +871,15 @@ def backup_create(context, values):
return IMPL.backup_create(context, values)
def backup_get_all_by_project(context, project_id, filters=None):
def backup_get_all_by_project(context, project_id, filters=None, marker=None,
limit=None, offset=None, sort_keys=None,
sort_dirs=None):
"""Get all backups belonging to a project."""
return IMPL.backup_get_all_by_project(context, project_id,
filters=filters)
filters=filters, marker=marker,
limit=limit, offset=offset,
sort_keys=sort_keys,
sort_dirs=sort_dirs)
def backup_get_all_by_volume(context, volume_id, filters=None):

53
cinder/db/sqlalchemy/api.py
View File

@ -3421,9 +3421,12 @@ def volume_glance_metadata_delete_by_snapshot(context, snapshot_id):
@require_context
def backup_get(context, backup_id):
result = model_query(context, models.Backup, project_only=True).\
filter_by(id=backup_id).\
first()
return _backup_get(context, backup_id)
def _backup_get(context, backup_id, session=None):
result = model_query(context, models.Backup, session=session,
project_only=True).filter_by(id=backup_id).first()
if not result:
raise exception.BackupNotFound(backup_id=backup_id)
@ -3431,23 +3434,41 @@ def backup_get(context, backup_id):
return result
def _backup_get_all(context, filters=None):
def _backup_get_all(context, filters=None, marker=None, limit=None,
offset=None, sort_keys=None, sort_dirs=None):
if filters and not is_valid_model_filters(models.Backup, filters):
return []
session = get_session()
with session.begin():
# Generate the query
query = model_query(context, models.Backup)
if filters:
query = query.filter_by(**filters)
# Generate the paginate query
query = _generate_paginate_query(context, session, marker,
limit, sort_keys, sort_dirs, filters,
offset, models.Backup)
if query is None:
return []
return query.all()
def _backups_get_query(context, session=None, project_only=False):
return model_query(context, models.Backup, session=session,
project_only=project_only)
def _process_backups_filters(query, filters):
if filters:
# Ensure that filters' keys exist on the model
if not is_valid_model_filters(models.Backup, filters):
return
query = query.filter_by(**filters)
return query
@require_admin_context
def backup_get_all(context, filters=None):
return _backup_get_all(context, filters)
def backup_get_all(context, filters=None, marker=None, limit=None,
offset=None, sort_keys=None, sort_dirs=None):
return _backup_get_all(context, filters, marker, limit, offset, sort_keys,
sort_dirs)
@require_admin_context
@ -3456,7 +3477,9 @@ def backup_get_all_by_host(context, host):
@require_context
def backup_get_all_by_project(context, project_id, filters=None):
def backup_get_all_by_project(context, project_id, filters=None, marker=None,
limit=None, offset=None, sort_keys=None,
sort_dirs=None):
authorize_project_context(context, project_id)
if not filters:
@ -3466,7 +3489,8 @@ def backup_get_all_by_project(context, project_id, filters=None):
filters['project_id'] = project_id
return _backup_get_all(context, filters)
return _backup_get_all(context, filters, marker, limit, offset, sort_keys,
sort_dirs)
@require_context
@ -3967,5 +3991,6 @@ def driver_initiator_data_get(context, initiator, namespace):
PAGINATION_HELPERS = {
models.Volume: (_volume_get_query, _process_volume_filters, _volume_get),
models.Snapshot: (_snaps_get_query, _process_snaps_filters, _snapshot_get)
models.Snapshot: (_snaps_get_query, _process_snaps_filters, _snapshot_get),
models.Backup: (_backups_get_query, _process_backups_filters, _backup_get)
}

14
cinder/objects/backup.py
View File

@ -165,8 +165,10 @@ class BackupList(base.ObjectListBase, base.CinderObject):
}
@base.remotable_classmethod
def get_all(cls, context, filters=None):
backups = db.backup_get_all(context, filters)
def get_all(cls, context, filters=None, marker=None, limit=None,
offset=None, sort_keys=None, sort_dirs=None):
backups = db.backup_get_all(context, filters, marker, limit, offset,
sort_keys, sort_dirs)
return base.obj_make_list(context, cls(context), objects.Backup,
backups)
@ -177,8 +179,12 @@ class BackupList(base.ObjectListBase, base.CinderObject):
backups)
@base.remotable_classmethod
def get_all_by_project(cls, context, project_id, filters=None):
backups = db.backup_get_all_by_project(context, project_id, filters)
def get_all_by_project(cls, context, project_id, filters=None,
marker=None, limit=None, offset=None,
sort_keys=None, sort_dirs=None):
backups = db.backup_get_all_by_project(context, project_id, filters,
marker, limit, offset,
sort_keys, sort_dirs)
return base.obj_make_list(context, cls(context), objects.Backup,
backups)

156
cinder/tests/unit/api/contrib/test_backups.py
View File

@ -156,13 +156,13 @@ class BackupsAPITestCase(test.TestCase):
self.assertEqual(200, res.status_int)
self.assertEqual(3, len(res_dict['backups'][0]))
self.assertEqual(backup_id1, res_dict['backups'][0]['id'])
self.assertEqual(backup_id3, res_dict['backups'][0]['id'])
self.assertEqual('test_backup', res_dict['backups'][0]['name'])
self.assertEqual(3, len(res_dict['backups'][1]))
self.assertEqual(backup_id2, res_dict['backups'][1]['id'])
self.assertEqual('test_backup', res_dict['backups'][1]['name'])
self.assertEqual(3, len(res_dict['backups'][2]))
self.assertEqual(backup_id3, res_dict['backups'][2]['id'])
self.assertEqual(backup_id1, res_dict['backups'][2]['id'])
self.assertEqual('test_backup', res_dict['backups'][2]['name'])
db.backup_destroy(context.get_admin_context(), backup_id3)
@ -185,19 +185,89 @@ class BackupsAPITestCase(test.TestCase):
backup_list = dom.getElementsByTagName('backup')
self.assertEqual(2, backup_list.item(0).attributes.length)
self.assertEqual(backup_id1,
self.assertEqual(backup_id3,
backup_list.item(0).getAttribute('id'))
self.assertEqual(2, backup_list.item(1).attributes.length)
self.assertEqual(backup_id2,
backup_list.item(1).getAttribute('id'))
self.assertEqual(2, backup_list.item(2).attributes.length)
self.assertEqual(backup_id3,
self.assertEqual(backup_id1,
backup_list.item(2).getAttribute('id'))
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_with_limit(self):
backup_id1 = self._create_backup()
backup_id2 = self._create_backup()
backup_id3 = self._create_backup()
req = webob.Request.blank('/v2/fake/backups?limit=2')
req.method = 'GET'
req.headers['Content-Type'] = 'application/json'
res = req.get_response(fakes.wsgi_app())
res_dict = json.loads(res.body)
self.assertEqual(200, res.status_int)
self.assertEqual(2, len(res_dict['backups']))
self.assertEqual(3, len(res_dict['backups'][0]))
self.assertEqual(backup_id3, res_dict['backups'][0]['id'])
self.assertEqual('test_backup', res_dict['backups'][0]['name'])
self.assertEqual(3, len(res_dict['backups'][1]))
self.assertEqual(backup_id2, res_dict['backups'][1]['id'])
self.assertEqual('test_backup', res_dict['backups'][1]['name'])
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_with_marker(self):
backup_id1 = self._create_backup()
backup_id2 = self._create_backup()
backup_id3 = self._create_backup()
url = ('/v2/fake/backups?marker=%s' % backup_id3)
req = webob.Request.blank(url)
req.method = 'GET'
req.headers['Content-Type'] = 'application/json'
res = req.get_response(fakes.wsgi_app())
res_dict = json.loads(res.body)
self.assertEqual(200, res.status_int)
self.assertEqual(2, len(res_dict['backups']))
self.assertEqual(3, len(res_dict['backups'][0]))
self.assertEqual(backup_id2, res_dict['backups'][0]['id'])
self.assertEqual('test_backup', res_dict['backups'][0]['name'])
self.assertEqual(3, len(res_dict['backups'][1]))
self.assertEqual(backup_id1, res_dict['backups'][1]['id'])
self.assertEqual('test_backup', res_dict['backups'][1]['name'])
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_with_limit_and_marker(self):
backup_id1 = self._create_backup()
backup_id2 = self._create_backup()
backup_id3 = self._create_backup()
url = ('/v2/fake/backups?limit=1&marker=%s' % backup_id3)
req = webob.Request.blank(url)
req.method = 'GET'
req.headers['Content-Type'] = 'application/json'
res = req.get_response(fakes.wsgi_app())
res_dict = json.loads(res.body)
self.assertEqual(200, res.status_int)
self.assertEqual(1, len(res_dict['backups']))
self.assertEqual(3, len(res_dict['backups'][0]))
self.assertEqual(backup_id2, res_dict['backups'][0]['id'])
self.assertEqual('test_backup', res_dict['backups'][0]['name'])
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_json(self):
backup_id1 = self._create_backup()
backup_id2 = self._create_backup()
@ -219,7 +289,7 @@ class BackupsAPITestCase(test.TestCase):
res_dict['backups'][0]['description'])
self.assertEqual('test_backup',
res_dict['backups'][0]['name'])
self.assertEqual(backup_id1, res_dict['backups'][0]['id'])
self.assertEqual(backup_id3, res_dict['backups'][0]['id'])
self.assertEqual(0, res_dict['backups'][0]['object_count'])
self.assertEqual(0, res_dict['backups'][0]['size'])
self.assertEqual('creating', res_dict['backups'][0]['status'])
@ -241,13 +311,12 @@ class BackupsAPITestCase(test.TestCase):
self.assertEqual(14, len(res_dict['backups'][2]))
self.assertEqual('az1', res_dict['backups'][2]['availability_zone'])
self.assertEqual('volumebackups',
res_dict['backups'][2]['container'])
self.assertEqual('volumebackups', res_dict['backups'][2]['container'])
self.assertEqual('this is a test backup',
res_dict['backups'][2]['description'])
self.assertEqual('test_backup',
res_dict['backups'][2]['name'])
self.assertEqual(backup_id3, res_dict['backups'][2]['id'])
self.assertEqual(backup_id1, res_dict['backups'][2]['id'])
self.assertEqual(0, res_dict['backups'][2]['object_count'])
self.assertEqual(0, res_dict['backups'][2]['size'])
self.assertEqual('creating', res_dict['backups'][2]['status'])
@ -325,7 +394,7 @@ class BackupsAPITestCase(test.TestCase):
self.assertEqual(
'test_backup', backup_detail.item(0).getAttribute('name'))
self.assertEqual(
backup_id1, backup_detail.item(0).getAttribute('id'))
backup_id3, backup_detail.item(0).getAttribute('id'))
self.assertEqual(
0, int(backup_detail.item(0).getAttribute('object_count')))
self.assertEqual(
@ -367,7 +436,7 @@ class BackupsAPITestCase(test.TestCase):
self.assertEqual(
'test_backup', backup_detail.item(2).getAttribute('name'))
self.assertEqual(
backup_id3, backup_detail.item(2).getAttribute('id'))
backup_id1, backup_detail.item(2).getAttribute('id'))
self.assertEqual(
0, int(backup_detail.item(2).getAttribute('object_count')))
self.assertEqual(
@ -381,6 +450,73 @@ 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_with_limit_and_sort_args(self):
backup_id1 = self._create_backup()
backup_id2 = self._create_backup()
backup_id3 = self._create_backup()
url = ('/v2/fake/backups/detail?limit=2&sort_key=created_at'
'&sort_dir=desc')
req = webob.Request.blank(url)
req.method = 'GET'
req.headers['Content-Type'] = 'application/json'
res = req.get_response(fakes.wsgi_app())
res_dict = json.loads(res.body)
self.assertEqual(200, res.status_int)
self.assertEqual(2, len(res_dict['backups']))
self.assertEqual(14, len(res_dict['backups'][0]))
self.assertEqual(backup_id3, res_dict['backups'][0]['id'])
self.assertEqual(14, len(res_dict['backups'][1]))
self.assertEqual(backup_id2, res_dict['backups'][1]['id'])
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_with_marker(self):
backup_id1 = self._create_backup()
backup_id2 = self._create_backup()
backup_id3 = self._create_backup()
url = ('/v2/fake/backups/detail?marker=%s' % backup_id3)
req = webob.Request.blank(url)
req.method = 'GET'
req.headers['Content-Type'] = 'application/json'
res = req.get_response(fakes.wsgi_app())
res_dict = json.loads(res.body)
self.assertEqual(200, res.status_int)
self.assertEqual(2, len(res_dict['backups']))
self.assertEqual(14, len(res_dict['backups'][0]))
self.assertEqual(backup_id2, res_dict['backups'][0]['id'])
self.assertEqual(14, len(res_dict['backups'][1]))
self.assertEqual(backup_id1, res_dict['backups'][1]['id'])
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_with_limit_and_marker(self):
backup_id1 = self._create_backup()
backup_id2 = self._create_backup()
backup_id3 = self._create_backup()
url = ('/v2/fake/backups/detail?limit=1&marker=%s' % backup_id3)
req = webob.Request.blank(url)
req.method = 'GET'
req.headers['Content-Type'] = 'application/json'
res = req.get_response(fakes.wsgi_app())
res_dict = json.loads(res.body)
self.assertEqual(200, res.status_int)
self.assertEqual(1, len(res_dict['backups']))
self.assertEqual(14, len(res_dict['backups'][0]))
self.assertEqual(backup_id2, res_dict['backups'][0]['id'])
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)
@mock.patch('cinder.db.service_get_all_by_topic')
@mock.patch(
'cinder.api.openstack.wsgi.Controller.validate_name_and_description')

11
cinder/tests/unit/test_backup.py
View File

@ -942,7 +942,8 @@ class BackupAPITestCase(BaseBackupTest):
self.assertEqual(mock_backuplist.get_all_by_project.return_value,
result)
mock_backuplist.get_all_by_project.assert_called_once_with(
self.ctxt, self.ctxt.project_id, filters={'key': 'value'})
self.ctxt, self.ctxt.project_id, {'key': 'value'}, None, None,
None, None, None)
@mock.patch.object(objects, 'BackupList')
@ddt.data(False, 'false', '0', 0, 'no')
@ -954,7 +955,8 @@ class BackupAPITestCase(BaseBackupTest):
self.assertEqual(mock_backuplist.get_all_by_project.return_value,
result)
mock_backuplist.get_all_by_project.assert_called_once_with(
self.ctxt, self.ctxt.project_id, filters={'key': 'value'})
self.ctxt, self.ctxt.project_id, {'key': 'value'}, None, None,
None, None, None)
@mock.patch.object(objects, 'BackupList')
@ddt.data(True, 'true', '1', 1, 'yes')
@ -966,7 +968,7 @@ class BackupAPITestCase(BaseBackupTest):
self.assertEqual(mock_backuplist.get_all.return_value,
result)
mock_backuplist.get_all.assert_called_once_with(
self.ctxt, filters={'key': 'value'})
self.ctxt, {'key': 'value'}, None, None, None, None, None)
@mock.patch.object(objects, 'BackupList')
def test_get_all_true_value_all_tenants_non_admin(self, mock_backuplist):
@ -977,4 +979,5 @@ class BackupAPITestCase(BaseBackupTest):
self.assertEqual(mock_backuplist.get_all_by_project.return_value,
result)
mock_backuplist.get_all_by_project.assert_called_once_with(
ctxt, ctxt.project_id, filters={'key': 'value'})
ctxt, ctxt.project_id, {'key': 'value'}, None, None, None, None,
None)

3
cinder/tests/unit/test_cmd.py
View File

@ -614,7 +614,8 @@ class TestCinderManageCmd(test.TestCase):
backup_cmds.list()
get_admin_context.assert_called_once_with()
backup_get_all.assert_called_once_with(ctxt, None)
backup_get_all.assert_called_once_with(ctxt, None, None, None,
None, None, None)
self.assertEqual(expected_out, fake_out.getvalue())
@mock.patch('cinder.utils.service_is_up')