openstack/aodh
Code Issues Proposed changes

Clean-ups related to alarm history patches

Browse Source 
Some non-blocking clean-ups requested in reviews of the alarm
history patches.

Change-Id: If009166be450f2e1bba0e68ce74699b5a032543e
This commit is contained in:
Eoghan Glynn 2013-09-04 22:17:27 +00:00
parent 0d8c4bbd24
commit a09ef3261c
9 changed files with 215 additions and 20 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

21
ceilometer/api/controllers/v2.py
View File

@ -211,13 +211,14 @@ class Query(_Base):
return converted_value
def _sanitize_query(q, valid_keys):
def _sanitize_query(q, valid_keys, headers=None):
'''Check the query to see if:
1) the request is comming from admin - then allow full visibility
1) the request is coming from admin - then allow full visibility
2) non-admin - make sure that the query includes the requester's
project.
'''
auth_project = acl.get_limited_to_project(pecan.request.headers)
auth_project = acl.get_limited_to_project(headers or
pecan.request.headers)
if auth_project:
proj_q = [i for i in q if i.field == 'project_id']
for i in proj_q:
@ -240,19 +241,11 @@ def _sanitize_query(q, valid_keys):
return q
def _exclude_from(keys, excluded):
if keys and excluded:
for key in excluded:
if key in keys:
keys.remove(key)
def _query_to_kwargs(query, db_func, internal_keys=[]):
# TODO(dhellmann): This function needs tests of its own.
def _query_to_kwargs(query, db_func, internal_keys=[], headers=None):
valid_keys = inspect.getargspec(db_func)[0]
query = _sanitize_query(query, valid_keys)
query = _sanitize_query(query, valid_keys, headers=headers)
internal_keys.append('self')
_exclude_from(valid_keys, internal_keys)
valid_keys = set(valid_keys) - set(internal_keys)
translation = {'user_id': 'user',
'project_id': 'project',
'resource_id': 'resource'}

11
ceilometer/storage/base.py
View File

@ -236,6 +236,17 @@ class Connection(object):
start_timestamp=None, start_timestamp_op=None,
end_timestamp=None, end_timestamp_op=None):
"""Yields list of AlarmChanges describing alarm history
Changes are always sorted in reverse order of occurence, given
the importance of currency.
Segregation for non-administrative users is done on the basis
of the on_behalf_of parameter. This allows such users to have
visibility on both the changes initiated by themselves directly
(generally creation, rule changes, or deletion) and also on those
changes initiated on their behalf by the alarming service (state
transitions after alarm thresholds are crossed).
:param alarm_id: ID of alarm to return changes for
:param on_behalf_of: ID of tenant to scope changes query (None for
administrative user, indicating all projects)

11
ceilometer/storage/impl_db2.py
View File

@ -610,6 +610,17 @@ class Connection(base.Connection):
start_timestamp=None, start_timestamp_op=None,
end_timestamp=None, end_timestamp_op=None):
"""Yields list of AlarmChanges describing alarm history
Changes are always sorted in reverse order of occurence, given
the importance of currency.
Segregation for non-administrative users is done on the basis
of the on_behalf_of parameter. This allows such users to have
visibility on both the changes initiated by themselves directly
(generally creation, rule changes, or deletion) and also on those
changes initiated on their behalf by the alarming service (state
transitions after alarm thresholds are crossed).
:param alarm_id: ID of alarm to return changes for
:param on_behalf_of: ID of tenant to scope changes query (None for
administrative user, indicating all projects)

11
ceilometer/storage/impl_hbase.py
View File

@ -603,6 +603,17 @@ class Connection(base.Connection):
start_timestamp=None, start_timestamp_op=None,
end_timestamp=None, end_timestamp_op=None):
"""Yields list of AlarmChanges describing alarm history
Changes are always sorted in reverse order of occurence, given
the importance of currency.
Segregation for non-administrative users is done on the basis
of the on_behalf_of parameter. This allows such users to have
visibility on both the changes initiated by themselves directly
(generally creation, rule changes, or deletion) and also on those
changes initiated on their behalf by the alarming service (state
transitions after alarm thresholds are crossed).
:param alarm_id: ID of alarm to return changes for
:param on_behalf_of: ID of tenant to scope changes query (None for
administrative user, indicating all projects)

11
ceilometer/storage/impl_log.py
View File

@ -182,6 +182,17 @@ class Connection(base.Connection):
start_timestamp=None, start_timestamp_op=None,
end_timestamp=None, end_timestamp_op=None):
"""Yields list of AlarmChanges describing alarm history
Changes are always sorted in reverse order of occurence, given
the importance of currency.
Segregation for non-administrative users is done on the basis
of the on_behalf_of parameter. This allows such users to have
visibility on both the changes initiated by themselves directly
(generally creation, rule changes, or deletion) and also on those
changes initiated on their behalf by the alarming service (state
transitions after alarm thresholds are crossed).
:param alarm_id: ID of alarm to return changes for
:param on_behalf_of: ID of tenant to scope changes query (None for
administrative user, indicating all projects)

11
ceilometer/storage/impl_mongodb.py
View File

@ -846,6 +846,17 @@ class Connection(base.Connection):
start_timestamp=None, start_timestamp_op=None,
end_timestamp=None, end_timestamp_op=None):
"""Yields list of AlarmChanges describing alarm history
Changes are always sorted in reverse order of occurence, given
the importance of currency.
Segregation for non-administrative users is done on the basis
of the on_behalf_of parameter. This allows such users to have
visibility on both the changes initiated by themselves directly
(generally creation, rule changes, or deletion) and also on those
changes initiated on their behalf by the alarming service (state
transitions after alarm thresholds are crossed).
:param alarm_id: ID of alarm to return changes for
:param on_behalf_of: ID of tenant to scope changes query (None for
administrative user, indicating all projects)

11
ceilometer/storage/impl_sqlalchemy.py
View File

@ -634,6 +634,17 @@ class Connection(base.Connection):
start_timestamp=None, start_timestamp_op=None,
end_timestamp=None, end_timestamp_op=None):
"""Yields list of AlarmChanges describing alarm history
Changes are always sorted in reverse order of occurence, given
the importance of currency.
Segregation for non-administrative users is done on the basis
of the on_behalf_of parameter. This allows such users to have
visibility on both the changes initiated by themselves directly
(generally creation, rule changes, or deletion) and also on those
changes initiated on their behalf by the alarming service (state
transitions after alarm thresholds are crossed).
:param alarm_id: ID of alarm to return changes for
:param on_behalf_of: ID of tenant to scope changes query (None for
administrative user, indicating all projects)

43
tests/api/v2/test_alarm_scenarios.py
View File

@ -206,6 +206,11 @@ class TestAlarms(FunctionalTest,
params=data,
headers=auth_headers or self.auth_headers)
def _delete_alarm(self, alarm, auth_headers=None):
self.delete('/alarms/%s' % alarm['alarm_id'],
headers=auth_headers or self.auth_headers,
status=200)
def _assert_is_subset(self, expected, actual):
for k, v in expected.iteritems():
self.assertEqual(v, actual.get(k), 'mismatched field: %s' % k)
@ -370,12 +375,38 @@ class TestAlarms(FunctionalTest,
user_id=alarm['user_id']),
history[0])
self._assert_in_json(alarm, history[0]['detail'])
self._assert_is_subset(dict(alarm_id=alarm['alarm_id'],
detail='{"name": "renamed"}',
on_behalf_of=alarm['project_id'],
project_id=alarm['project_id'],
type='rule change',
user_id=alarm['user_id']),
history[1])
def test_get_constrained_alarm_history(self):
def test_get_alarm_history_ordered_by_recentness(self):
alarm = self._get_alarm('a')
for i in xrange(10):
self._update_alarm(alarm, dict(name='%s' % i))
alarm = self._get_alarm('a')
self._delete_alarm(alarm)
history = self._get_alarm_history(alarm)
self.assertEqual(11, len(history), 'hist: %s' % history)
self._assert_is_subset(dict(alarm_id=alarm['alarm_id'],
type='deletion'),
history[0])
self._assert_in_json(alarm, history[0]['detail'])
for i in xrange(1, 10):
detail = '{"name": "%s"}' % (10 - i)
self._assert_is_subset(dict(alarm_id=alarm['alarm_id'],
detail=detail,
type='rule change'),
history[i])
def test_get_alarm_history_constrained_by_timestamp(self):
alarm = self._get_alarm('a')
self._update_alarm(alarm, dict(name='renamed'))
now = datetime.datetime.utcnow().isoformat()
query = dict(field='timestamp', op='gt', value=now)
after = datetime.datetime.utcnow().isoformat()
query = dict(field='timestamp', op='gt', value=after)
history = self._get_alarm_history(alarm, query=query)
self.assertEqual(0, len(history))
query['op'] = 'le'
@ -389,10 +420,10 @@ class TestAlarms(FunctionalTest,
type='rule change',
user_id=alarm['user_id']),
history[0])
def test_get_alarm_history_constrained_by_type(self):
alarm = self._get_alarm('a')
self.delete('/alarms/%s' % alarm['alarm_id'],
headers=self.auth_headers,
status=200)
self._delete_alarm(alarm)
query = dict(field='type', op='eq', value='deletion')
history = self._get_alarm_history(alarm, query=query)
self.assertEqual(1, len(history))

105
tests/api/v2/test_query.py
View File

@ -15,10 +15,13 @@
# under the License.
"""Test the methods related to query."""
import datetime
import wsme
from ceilometer.api.controllers import v2 as api
from ceilometer.api.controllers.v2 import Query
from ceilometer.api.controllers.v2 import _query_to_kwargs
from ceilometer.tests import base as tests_base
@ -98,6 +101,108 @@ class TestQuery(tests_base.TestCase):
type='integer')
self.assertRaises(wsme.exc.ClientSideError, query._get_value_as_type)
def _fake_db_func(self, resource, on_behalf_of, x, y,
metaquery={}, user=None, project=None,
start_timestamp=None, start_timestamp_op=None,
end_timestamp=None, end_timestamp_op=None, **kwargs):
pass
def test_query_to_kwargs_exclude_internal(self):
queries = [Query(field=f,
op='eq',
value='fake',
type='string') for f in ['y', 'on_behalf_of', 'x']]
self.assertRaises(wsme.exc.ClientSideError,
_query_to_kwargs,
queries,
self._fake_db_func,
headers={'X-ProjectId': 'foobar'},
internal_keys=['on_behalf_of'])
def test_query_to_kwargs_self_always_excluded(self):
queries = [Query(field=f,
op='eq',
value='fake',
type='string') for f in ['x', 'y']]
kwargs = _query_to_kwargs(queries,
self._fake_db_func,
headers={'X-ProjectId': 'foobar'})
self.assertFalse('self' in kwargs)
def test_query_to_kwargs_timestamp_mapping(self):
start = datetime.datetime.utcnow()
end = datetime.datetime.utcnow()
queries = [Query(field='timestamp',
op='gt',
value=start.isoformat(),
type='string'),
Query(field='timestamp',
op='le',
value=end.isoformat(),
type='string')]
kwargs = _query_to_kwargs(queries,
self._fake_db_func,
headers={'X-ProjectId': 'foobar'})
self.assertEqual(kwargs.get('start_timestamp'), start)
self.assertEqual(kwargs.get('start_timestamp_op'), 'gt')
self.assertEqual(kwargs.get('end_timestamp'), end)
self.assertEqual(kwargs.get('end_timestamp_op'), 'le')
def test_query_to_kwargs_non_equality_on_metadata(self):
queries = [Query(field='resource_metadata.image_id',
op='gt',
value='image',
type='string'),
Query(field='metadata.ramdisk_id',
op='le',
value='ramdisk',
type='string')]
kwargs = _query_to_kwargs(queries,
self._fake_db_func,
headers={'X-ProjectId': 'foobar'})
self.assertFalse('metaquery' in kwargs)
def test_query_to_kwargs_equality_on_metadata(self):
queries = [Query(field='resource_metadata.image_id',
op='eq',
value='image',
type='string'),
Query(field='metadata.ramdisk_id',
op='eq',
value='ramdisk',
type='string')]
kwargs = _query_to_kwargs(queries,
self._fake_db_func,
headers={'X-ProjectId': 'foobar'})
self.assertTrue('metaquery' in kwargs)
metaquery = kwargs['metaquery']
self.assertEqual(metaquery.get('metadata.image_id'), 'image')
self.assertEqual(metaquery.get('metadata.ramdisk_id'), 'ramdisk')
def test_query_to_kwargs_translation(self):
queries = [Query(field=f,
op='eq',
value='fake_%s' % f,
type='string') for f in ['user_id',
'project_id',
'resource_id']]
kwargs = _query_to_kwargs(queries,
self._fake_db_func,
headers={'X-ProjectId': 'foobar'})
for o in ['user', 'project', 'resource']:
self.assertEqual(kwargs.get(o), 'fake_%s_id' % o)
def test_query_to_kwargs_unrecognized(self):
queries = [Query(field=f,
op='eq',
value='fake',
type='string') for f in ['y', 'z', 'x']]
self.assertRaises(wsme.exc.ClientSideError,
_query_to_kwargs,
queries,
self._fake_db_func,
headers={'X-ProjectId': 'foobar'})
class TestValidateGroupByFields(tests_base.TestCase):