Add support for limiting and sorting stacks
This adds support to the stacks index call for limit, sort_keys, sort_dir, and marker query parameters, allowing users of the API to retrieve a subset of stacks. With this commit, stacks are also sorted by created_at DESC by default. blueprint pagination Change-Id: Ib20cfae183475ff42deda82168b8b9c8d043ecd0
This commit is contained in:
parent
7e19123f93
commit
0ab4a74e97
|
@ -76,6 +76,7 @@ class FaultWrapper(wsgi.Middleware):
|
|||
'UserParameterMissing': webob.exc.HTTPBadRequest,
|
||||
'RequestLimitExceeded': webob.exc.HTTPBadRequest,
|
||||
'InvalidTemplateParameter': webob.exc.HTTPBadRequest,
|
||||
'Invalid': webob.exc.HTTPBadRequest,
|
||||
}
|
||||
|
||||
def _error(self, ex):
|
||||
|
|
|
@ -183,8 +183,14 @@ class StackController(object):
|
|||
"""
|
||||
Lists summary information for all stacks
|
||||
"""
|
||||
|
||||
stacks = self.engine.list_stacks(req.context)
|
||||
whitelist = {
|
||||
'limit': 'single',
|
||||
'marker': 'single',
|
||||
'sort_dir': 'single',
|
||||
'sort_keys': 'multi',
|
||||
}
|
||||
params = util.get_allowed_params(req.params, whitelist)
|
||||
stacks = self.engine.list_stacks(req.context, **params)
|
||||
|
||||
summary_keys = (engine_api.STACK_ID,
|
||||
engine_api.STACK_NAME,
|
||||
|
|
|
@ -62,3 +62,31 @@ def make_url(req, identity):
|
|||
def make_link(req, identity, relationship='self'):
|
||||
'''Return a link structure for the supplied identity dictionary.'''
|
||||
return {'href': make_url(req, identity), 'rel': relationship}
|
||||
|
||||
|
||||
def get_allowed_params(params, whitelist):
|
||||
'''Extract from ``params`` all entries listed in ``whitelist``
|
||||
|
||||
The returning dict will contain an entry for a key if, and only if,
|
||||
there's an entry in ``whitelist`` for that key and at least one entry in
|
||||
``params``. If ``params`` contains multiple entries for the same key, it
|
||||
will yield an array of values: ``{key: [v1, v2,...]}``
|
||||
|
||||
:param params: a NestedMultiDict from webob.Request.params
|
||||
:param whitelist: an array of strings to whitelist
|
||||
|
||||
:returns: a dict with {key: value} pairs
|
||||
'''
|
||||
allowed_params = {}
|
||||
|
||||
for key, get_type in whitelist.iteritems():
|
||||
value = None
|
||||
if get_type == 'single':
|
||||
value = params.get(key)
|
||||
elif get_type == 'multi':
|
||||
value = params.getall(key)
|
||||
|
||||
if value:
|
||||
allowed_params[key] = value
|
||||
|
||||
return allowed_params
|
||||
|
|
|
@ -119,8 +119,10 @@ def stack_get_all_by_owner_id(context, owner_id):
|
|||
return IMPL.stack_get_all_by_owner_id(context, owner_id)
|
||||
|
||||
|
||||
def stack_get_all_by_tenant(context):
|
||||
return IMPL.stack_get_all_by_tenant(context)
|
||||
def stack_get_all_by_tenant(context, limit=None, sort_keys=None,
|
||||
marker=None, sort_dir=None):
|
||||
return IMPL.stack_get_all_by_tenant(context, limit, sort_keys,
|
||||
marker, sort_dir)
|
||||
|
||||
|
||||
def stack_count_all_by_tenant(context):
|
||||
|
|
|
@ -31,6 +31,7 @@ from heat.common import exception
|
|||
from heat.db.sqlalchemy import migration
|
||||
from heat.db.sqlalchemy import models
|
||||
from heat.openstack.common.db.sqlalchemy import session as db_session
|
||||
from heat.openstack.common.db.sqlalchemy import utils
|
||||
|
||||
|
||||
get_engine = db_session.get_engine
|
||||
|
@ -246,15 +247,46 @@ def stack_get_all_by_owner_id(context, owner_id):
|
|||
return results
|
||||
|
||||
|
||||
def _paginate_query(context, query, model, limit=None, sort_keys=None,
|
||||
marker=None, sort_dir=None):
|
||||
default_sort_keys = ['created_at']
|
||||
if not sort_keys:
|
||||
sort_keys = default_sort_keys
|
||||
if not sort_dir:
|
||||
sort_dir = 'desc'
|
||||
elif not isinstance(sort_keys, list):
|
||||
sort_keys = [sort_keys]
|
||||
|
||||
# This assures the order of the stacks will always be the same
|
||||
# even for sort_key values that are not unique in the database
|
||||
sort_keys = sort_keys + ['id']
|
||||
|
||||
model_marker = None
|
||||
if marker:
|
||||
model_marker = model_query(context, model).get(marker)
|
||||
|
||||
try:
|
||||
query = utils.paginate_query(query, model, limit, sort_keys,
|
||||
model_marker, sort_dir)
|
||||
except utils.InvalidSortKey as exc:
|
||||
raise exception.Invalid(reason=exc.message)
|
||||
|
||||
return query
|
||||
|
||||
|
||||
def _query_stack_get_all_by_tenant(context):
|
||||
query = soft_delete_aware_query(context, models.Stack).\
|
||||
filter_by(owner_id=None).\
|
||||
filter_by(tenant=context.tenant_id)
|
||||
|
||||
return query
|
||||
|
||||
|
||||
def stack_get_all_by_tenant(context):
|
||||
return _query_stack_get_all_by_tenant(context).all()
|
||||
def stack_get_all_by_tenant(context, limit=None, sort_keys=None, marker=None,
|
||||
sort_dir=None):
|
||||
query = _query_stack_get_all_by_tenant(context)
|
||||
return _paginate_query(context, query, models.Stack, limit, sort_keys,
|
||||
marker, sort_dir).all()
|
||||
|
||||
|
||||
def stack_count_all_by_tenant(context):
|
||||
|
|
|
@ -197,7 +197,8 @@ class EngineService(service.Service):
|
|||
return [format_stack_detail(s) for s in stacks]
|
||||
|
||||
@request_context
|
||||
def list_stacks(self, cnxt):
|
||||
def list_stacks(self, cnxt, limit=None, sort_keys=None, marker=None,
|
||||
sort_dir=None):
|
||||
"""
|
||||
The list_stacks method returns attributes of all stacks.
|
||||
arg1 -> RPC cnxt.
|
||||
|
@ -215,7 +216,8 @@ class EngineService(service.Service):
|
|||
else:
|
||||
yield api.format_stack(stack)
|
||||
|
||||
stacks = db_api.stack_get_all_by_tenant(cnxt) or []
|
||||
stacks = db_api.stack_get_all_by_tenant(cnxt, limit, sort_keys, marker,
|
||||
sort_dir) or []
|
||||
return list(format_stack_details(stacks))
|
||||
|
||||
def _validate_deferred_auth_context(self, cnxt, stack):
|
||||
|
|
|
@ -50,13 +50,16 @@ class EngineClient(heat.openstack.common.rpc.proxy.RpcProxy):
|
|||
return self.call(ctxt, self.make_msg('identify_stack',
|
||||
stack_name=stack_name))
|
||||
|
||||
def list_stacks(self, ctxt):
|
||||
def list_stacks(self, ctxt, limit=None, sort_keys=None, marker=None,
|
||||
sort_dir=None):
|
||||
"""
|
||||
The list_stacks method returns the attributes of all stacks.
|
||||
|
||||
:param ctxt: RPC context.
|
||||
"""
|
||||
return self.call(ctxt, self.make_msg('list_stacks'))
|
||||
return self.call(ctxt, self.make_msg('list_stacks', limit=limit,
|
||||
sort_keys=sort_keys, marker=marker,
|
||||
sort_dir=sort_dir))
|
||||
|
||||
def show_stack(self, ctxt, stack_identity):
|
||||
"""
|
||||
|
|
|
@ -132,10 +132,12 @@ class CfnStackControllerTest(HeatTestCase):
|
|||
u'StackName': u'wordpress',
|
||||
u'StackStatus': u'CREATE_COMPLETE'}]}}}
|
||||
self.assertEqual(result, expected)
|
||||
default_args = {'limit': None, 'sort_keys': None, 'marker': None,
|
||||
'sort_dir': None}
|
||||
mock_call.assert_called_once_with(dummy_req.context, self.topic,
|
||||
{'namespace': None,
|
||||
'method': 'list_stacks',
|
||||
'args': {},
|
||||
'args': default_args,
|
||||
'version': self.api_version},
|
||||
None)
|
||||
|
||||
|
@ -155,7 +157,7 @@ class CfnStackControllerTest(HeatTestCase):
|
|||
mock_call.assert_called_once_with(dummy_req.context, self.topic,
|
||||
{'namespace': None,
|
||||
'method': 'list_stacks',
|
||||
'args': {},
|
||||
'args': mock.ANY,
|
||||
'version': self.api_version},
|
||||
None)
|
||||
|
||||
|
@ -174,7 +176,7 @@ class CfnStackControllerTest(HeatTestCase):
|
|||
mock_call.assert_called_once_with(dummy_req.context, self.topic,
|
||||
{'namespace': None,
|
||||
'method': 'list_stacks',
|
||||
'args': {},
|
||||
'args': mock.ANY,
|
||||
'version': self.api_version},
|
||||
None)
|
||||
|
||||
|
|
|
@ -239,16 +239,20 @@ class ControllerTest(object):
|
|||
'wsgi.url_scheme': 'http',
|
||||
}
|
||||
|
||||
def _simple_request(self, path, method='GET'):
|
||||
def _simple_request(self, path, params=None, method='GET'):
|
||||
environ = self._environ(path)
|
||||
environ['REQUEST_METHOD'] = method
|
||||
|
||||
if params:
|
||||
qs = "&".join(["=".join([k, str(params[k])]) for k in params])
|
||||
environ['QUERY_STRING'] = qs
|
||||
|
||||
req = Request(environ)
|
||||
req.context = utils.dummy_context('api_test_user', self.tenant)
|
||||
return req
|
||||
|
||||
def _get(self, path):
|
||||
return self._simple_request(path)
|
||||
def _get(self, path, params=None):
|
||||
return self._simple_request(path, params=params)
|
||||
|
||||
def _delete(self, path):
|
||||
return self._simple_request(path, method='DELETE')
|
||||
|
@ -336,13 +340,38 @@ class StackControllerTest(ControllerTest, HeatTestCase):
|
|||
]
|
||||
}
|
||||
self.assertEqual(result, expected)
|
||||
default_args = {'limit': None, 'sort_keys': None, 'marker': None,
|
||||
'sort_dir': None}
|
||||
mock_call.assert_called_once_with(req.context, self.topic,
|
||||
{'namespace': None,
|
||||
'method': 'list_stacks',
|
||||
'args': {},
|
||||
'args': default_args,
|
||||
'version': self.api_version},
|
||||
None)
|
||||
|
||||
@mock.patch.object(rpc, 'call')
|
||||
def test_index_whitelists_request_params(self, mock_call):
|
||||
params = {
|
||||
'limit': 'fake limit',
|
||||
'sort_keys': 'fake sort keys',
|
||||
'marker': 'fake marker',
|
||||
'sort_dir': 'fake sort dir',
|
||||
'balrog': 'you shall not pass!'
|
||||
}
|
||||
req = self._get('/stacks', params=params)
|
||||
mock_call.return_value = []
|
||||
|
||||
result = self.controller.index(req, tenant_id='fake_tenant_id')
|
||||
|
||||
rpc_call_args, _ = mock_call.call_args
|
||||
engine_args = rpc_call_args[2]['args']
|
||||
self.assertEqual(4, len(engine_args))
|
||||
self.assertIn('limit', engine_args)
|
||||
self.assertIn('sort_keys', engine_args)
|
||||
self.assertIn('marker', engine_args)
|
||||
self.assertIn('sort_dir', engine_args)
|
||||
self.assertNotIn('balrog', engine_args)
|
||||
|
||||
@mock.patch.object(rpc, 'call')
|
||||
def test_detail(self, mock_call):
|
||||
req = self._get('/stacks/detail')
|
||||
|
@ -396,10 +425,12 @@ class StackControllerTest(ControllerTest, HeatTestCase):
|
|||
}
|
||||
|
||||
self.assertEqual(result, expected)
|
||||
default_args = {'limit': None, 'sort_keys': None, 'marker': None,
|
||||
'sort_dir': None}
|
||||
mock_call.assert_called_once_with(req.context, self.topic,
|
||||
{'namespace': None,
|
||||
'method': 'list_stacks',
|
||||
'args': {},
|
||||
'args': default_args,
|
||||
'version': self.api_version},
|
||||
None)
|
||||
|
||||
|
@ -418,7 +449,7 @@ class StackControllerTest(ControllerTest, HeatTestCase):
|
|||
mock_call.assert_called_once_with(req.context, self.topic,
|
||||
{'namespace': None,
|
||||
'method': 'list_stacks',
|
||||
'args': {},
|
||||
'args': mock.ANY,
|
||||
'version': self.api_version},
|
||||
None)
|
||||
|
||||
|
@ -437,7 +468,7 @@ class StackControllerTest(ControllerTest, HeatTestCase):
|
|||
mock_call.assert_called_once_with(req.context, self.topic,
|
||||
{'namespace': None,
|
||||
'method': 'list_stacks',
|
||||
'args': {},
|
||||
'args': mock.ANY,
|
||||
'version': self.api_version},
|
||||
None)
|
||||
|
||||
|
|
|
@ -0,0 +1,64 @@
|
|||
# vim: tabstop=4 shiftwidth=4 softtabstop=4
|
||||
|
||||
# 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
|
||||
#
|
||||
# http://www.apache.org/licenses/LICENSE-2.0
|
||||
#
|
||||
# Unless required by applicable law or agreed to in writing, software
|
||||
# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
|
||||
# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
|
||||
# License for the specific language governing permissions and limitations
|
||||
# under the License.
|
||||
|
||||
from heat.api.openstack.v1 import util
|
||||
from heat.common.wsgi import Request
|
||||
from heat.tests.common import HeatTestCase
|
||||
|
||||
|
||||
class TestGetAllowedParams(HeatTestCase):
|
||||
def setUp(self):
|
||||
super(TestGetAllowedParams, self).setUp()
|
||||
req = Request({})
|
||||
self.params = req.params.copy()
|
||||
self.params.add('foo', 'foo value')
|
||||
self.whitelist = {'foo': 'single'}
|
||||
|
||||
def test_returns_empty_dict(self):
|
||||
self.whitelist = {}
|
||||
|
||||
result = util.get_allowed_params(self.params, self.whitelist)
|
||||
self.assertEqual({}, result)
|
||||
|
||||
def test_only_adds_whitelisted_params_if_param_exists(self):
|
||||
self.whitelist = {'foo': 'single'}
|
||||
self.params.clear()
|
||||
|
||||
result = util.get_allowed_params(self.params, self.whitelist)
|
||||
self.assertNotIn('foo', result)
|
||||
|
||||
def test_returns_only_whitelisted_params(self):
|
||||
self.params.add('bar', 'bar value')
|
||||
|
||||
result = util.get_allowed_params(self.params, self.whitelist)
|
||||
self.assertIn('foo', result)
|
||||
self.assertNotIn('bar', result)
|
||||
|
||||
def test_handles_single_value_params(self):
|
||||
result = util.get_allowed_params(self.params, self.whitelist)
|
||||
self.assertEqual('foo value', result['foo'])
|
||||
|
||||
def test_handles_multiple_value_params(self):
|
||||
self.whitelist = {'foo': 'multi'}
|
||||
self.params.add('foo', 'foo value 2')
|
||||
|
||||
result = util.get_allowed_params(self.params, self.whitelist)
|
||||
self.assertEqual(2, len(result['foo']))
|
||||
self.assertIn('foo value', result['foo'])
|
||||
self.assertIn('foo value 2', result['foo'])
|
||||
|
||||
def test_ignores_bogus_whitelist_items(self):
|
||||
self.whitelist = {'foo': 'blah'}
|
||||
result = util.get_allowed_params(self.params, self.whitelist)
|
||||
self.assertNotIn('foo', result)
|
|
@ -83,7 +83,13 @@ class EngineRpcAPITestCase(testtools.TestCase):
|
|||
self._test_engine_api('authenticated_to_backend', 'call')
|
||||
|
||||
def test_list_stacks(self):
|
||||
self._test_engine_api('list_stacks', 'call')
|
||||
default_args = {
|
||||
'limit': mock.ANY,
|
||||
'sort_keys': mock.ANY,
|
||||
'marker': mock.ANY,
|
||||
'sort_dir': mock.ANY
|
||||
}
|
||||
self._test_engine_api('list_stacks', 'call', **default_args)
|
||||
|
||||
def test_identify_stack(self):
|
||||
self._test_engine_api('identify_stack', 'call',
|
||||
|
@ -105,7 +111,7 @@ class EngineRpcAPITestCase(testtools.TestCase):
|
|||
template={u'Foo': u'bar'},
|
||||
params={u'InstanceType': u'm1.xlarge'},
|
||||
files={},
|
||||
args={})
|
||||
args=mock.ANY)
|
||||
|
||||
def test_get_template(self):
|
||||
self._test_engine_api('get_template', 'call',
|
||||
|
|
|
@ -217,6 +217,61 @@ class SqlAlchemyTest(HeatTestCase):
|
|||
st_db = db_api.stack_get_all_by_tenant(self.ctx)
|
||||
self.assertEqual(1, len(st_db))
|
||||
|
||||
def test_stack_get_all_by_tenant_default_sort_keys_and_dir(self):
|
||||
stacks = [self._setup_test_stack('stack', x)[1] for x in UUIDs]
|
||||
|
||||
st_db = db_api.stack_get_all_by_tenant(self.ctx)
|
||||
self.assertEqual(3, len(st_db))
|
||||
self.assertEqual(stacks[2].id, st_db[0].id)
|
||||
self.assertEqual(stacks[1].id, st_db[1].id)
|
||||
self.assertEqual(stacks[0].id, st_db[2].id)
|
||||
|
||||
def test_stack_get_all_by_tenant_default_sort_dir(self):
|
||||
stacks = [self._setup_test_stack('stack', x)[1] for x in UUIDs]
|
||||
|
||||
st_db = db_api.stack_get_all_by_tenant(self.ctx, sort_dir='asc')
|
||||
self.assertEqual(3, len(st_db))
|
||||
self.assertEqual(stacks[0].id, st_db[0].id)
|
||||
self.assertEqual(stacks[1].id, st_db[1].id)
|
||||
self.assertEqual(stacks[2].id, st_db[2].id)
|
||||
|
||||
def test_stack_get_all_by_tenant_str_sort_keys(self):
|
||||
stacks = [self._setup_test_stack('stack', x)[1] for x in UUIDs]
|
||||
|
||||
st_db = db_api.stack_get_all_by_tenant(self.ctx,
|
||||
sort_keys='created_at')
|
||||
self.assertEqual(3, len(st_db))
|
||||
self.assertEqual(stacks[0].id, st_db[0].id)
|
||||
self.assertEqual(stacks[1].id, st_db[1].id)
|
||||
self.assertEqual(stacks[2].id, st_db[2].id)
|
||||
|
||||
def test_stack_get_all_by_tenant_marker(self):
|
||||
stacks = [self._setup_test_stack('stack', x)[1] for x in UUIDs]
|
||||
|
||||
st_db = db_api.stack_get_all_by_tenant(self.ctx, marker=stacks[1].id)
|
||||
self.assertEqual(1, len(st_db))
|
||||
self.assertEqual(stacks[0].id, st_db[0].id)
|
||||
|
||||
def test_stack_get_all_by_tenant_non_existing_marker(self):
|
||||
stacks = [self._setup_test_stack('stack', x)[1] for x in UUIDs]
|
||||
|
||||
uuid = 'this stack doesnt exist'
|
||||
st_db = db_api.stack_get_all_by_tenant(self.ctx, marker=uuid)
|
||||
self.assertEqual(3, len(st_db))
|
||||
|
||||
def test_stack_get_all_by_tenant_handles_invalid_sort_key(self):
|
||||
self.assertRaises(exception.Invalid,
|
||||
db_api.stack_get_all_by_tenant,
|
||||
self.ctx,
|
||||
sort_keys=['foo'])
|
||||
|
||||
def test_stack_get_all_by_tenant_doesnt_mutate_sort_keys(self):
|
||||
stacks = [self._setup_test_stack('stack', x)[1] for x in UUIDs]
|
||||
sort_keys = ['id']
|
||||
|
||||
st_db = db_api.stack_get_all_by_tenant(self.ctx, sort_keys=sort_keys)
|
||||
self.assertEqual(['id'], sort_keys)
|
||||
|
||||
def test_stack_count_all_by_tenant(self):
|
||||
stacks = [self._setup_test_stack('stack', x)[1] for x in UUIDs]
|
||||
|
||||
|
|
Loading…
Reference in New Issue