From 66ef360c1480899bcdf6ad7af8f2d581b532c5e6 Mon Sep 17 00:00:00 2001 From: Mehdi Abaakouk Date: Thu, 26 Sep 2013 09:50:37 +0200 Subject: [PATCH] Allow to update an alarm partially The patch allow to only modify a part of an alarm instead of force to set the full alarm description. This permit to an application that have been written code around alarm with ceilometerclient 1.0.4 and ceilometer pre havana-3. To update alarm without any change with this and ceilometer >= havana-3 (ie: heat). Fixes bug #1231303 Change-Id: I20250131d05d20bfadbca450dfe6b8237f4b7183 --- ceilometerclient/common/utils.py | 9 +++ ceilometerclient/tests/test_utils.py | 31 ++++++++ ceilometerclient/tests/v2/test_alarms.py | 24 +++++- ceilometerclient/v2/alarms.py | 37 ++++----- ceilometerclient/v2/shell.py | 96 ++++++++++++------------ 5 files changed, 129 insertions(+), 68 deletions(-) diff --git a/ceilometerclient/common/utils.py b/ceilometerclient/common/utils.py index ef76dcc8..f28a3ea5 100644 --- a/ceilometerclient/common/utils.py +++ b/ceilometerclient/common/utils.py @@ -160,6 +160,15 @@ def key_with_slash_to_nested_dict(kwargs): return kwargs +def merge_nested_dict(dest, source, depth=0): + for (key, value) in source.iteritems(): + if isinstance(value, dict) and depth: + merge_nested_dict(dest[key], value, + depth=(depth - 1)) + else: + dest[key] = value + + def exit(msg=''): if msg: print >> sys.stderr, msg diff --git a/ceilometerclient/tests/test_utils.py b/ceilometerclient/tests/test_utils.py index 0e209b0a..e0626af7 100644 --- a/ceilometerclient/tests/test_utils.py +++ b/ceilometerclient/tests/test_utils.py @@ -93,3 +93,34 @@ class UtilsTest(test_utils.BaseTestCase): pass _, args = not_required_default.__dict__['arguments'][0] self.assertEqual(args['help'], "not_required_default. Defaults to 42.") + + def test_merge_nested_dict(self): + dest = {'key': 'value', + 'nested': {'key2': 'value2', + 'key3': 'value3', + 'nested2': {'key': 'value', + 'some': 'thing'}}} + source = {'key': 'modified', + 'nested': {'key3': 'modified3', + 'nested2': {'key5': 'value5'}}} + utils.merge_nested_dict(dest, source, depth=1) + + self.assertEqual(dest, {'key': 'modified', + 'nested': {'key2': 'value2', + 'key3': 'modified3', + 'nested2': {'key5': 'value5'}}}) + + def test_merge_nested_dict_no_depth(self): + dest = {'key': 'value', + 'nested': {'key2': 'value2', + 'key3': 'value3', + 'nested2': {'key': 'value', + 'some': 'thing'}}} + source = {'key': 'modified', + 'nested': {'key3': 'modified3', + 'nested2': {'key5': 'value5'}}} + utils.merge_nested_dict(dest, source) + + self.assertEqual(dest, {'key': 'modified', + 'nested': {'key3': 'modified3', + 'nested2': {'key5': 'value5'}}}) diff --git a/ceilometerclient/tests/v2/test_alarms.py b/ceilometerclient/tests/v2/test_alarms.py index 82279f50..f64ded19 100644 --- a/ceilometerclient/tests/v2/test_alarms.py +++ b/ceilometerclient/tests/v2/test_alarms.py @@ -61,6 +61,7 @@ DELTA_ALARM_RULE = {u'comparison_operator': u'lt', UPDATED_ALARM = copy.deepcopy(AN_ALARM) UPDATED_ALARM.update(DELTA_ALARM) UPDATED_ALARM['threshold_rule'].update(DELTA_ALARM_RULE) +DELTA_ALARM['threshold_rule'] = DELTA_ALARM_RULE UPDATE_ALARM = copy.deepcopy(UPDATED_ALARM) del UPDATE_ALARM['user_id'] del UPDATE_ALARM['project_id'] @@ -215,7 +216,20 @@ class AlarmManagerTest(testtools.TestCase): def test_update(self): alarm = self.mgr.update(alarm_id='alarm-id', **UPDATE_ALARM) expect = [ - ('PUT', '/v2/alarms/alarm-id', {}, UPDATE_ALARM), + ('GET', '/v2/alarms/alarm-id', {}, None), + ('PUT', '/v2/alarms/alarm-id', {}, UPDATED_ALARM), + ] + self.assertEqual(self.api.calls, expect) + self.assertTrue(alarm) + self.assertEqual(alarm.alarm_id, 'alarm-id') + for (key, value) in UPDATED_ALARM.iteritems(): + self.assertEqual(getattr(alarm, key), value) + + def test_update_delta(self): + alarm = self.mgr.update(alarm_id='alarm-id', **DELTA_ALARM) + expect = [ + ('GET', '/v2/alarms/alarm-id', {}, None), + ('PUT', '/v2/alarms/alarm-id', {}, UPDATED_ALARM), ] self.assertEqual(self.api.calls, expect) self.assertTrue(alarm) @@ -276,9 +290,10 @@ class AlarmLegacyManagerTest(testtools.TestCase): self.assertTrue(alarm) def test_update(self): - alarm = self.mgr.update(alarm_id='alarm-id', **UPDATE_LEGACY_ALARM) + alarm = self.mgr.update(alarm_id='alarm-id', **DELTA_LEGACY_ALARM) expect = [ - ('PUT', '/v2/alarms/alarm-id', {}, UPDATE_ALARM), + ('GET', '/v2/alarms/alarm-id', {}, None), + ('PUT', '/v2/alarms/alarm-id', {}, UPDATED_ALARM), ] self.assertEqual(self.api.calls, expect) self.assertTrue(alarm) @@ -293,7 +308,8 @@ class AlarmLegacyManagerTest(testtools.TestCase): del updated['meter_name'] alarm = self.mgr.update(alarm_id='alarm-id', **updated) expect = [ - ('PUT', '/v2/alarms/alarm-id', {}, UPDATE_ALARM), + ('GET', '/v2/alarms/alarm-id', {}, None), + ('PUT', '/v2/alarms/alarm-id', {}, UPDATED_ALARM), ] self.assertEqual(self.api.calls, expect) self.assertTrue(alarm) diff --git a/ceilometerclient/v2/alarms.py b/ceilometerclient/v2/alarms.py index d1ce7c68..71cc0acb 100644 --- a/ceilometerclient/v2/alarms.py +++ b/ceilometerclient/v2/alarms.py @@ -19,6 +19,7 @@ import warnings from ceilometerclient.common import base +from ceilometerclient.common import utils from ceilometerclient.v2 import options @@ -34,7 +35,7 @@ UPDATABLE_ATTRIBUTES = [ 'repeat_actions', 'threshold_rule', 'combination_rule', - ] +] CREATION_ATTRIBUTES = UPDATABLE_ATTRIBUTES + ['project_id', 'user_id'] @@ -67,12 +68,12 @@ class AlarmManager(base.Manager): return None @classmethod - def _compat_legacy_alarm_kwargs(cls, kwargs): - cls._compat_counter_rename_kwargs(kwargs) - cls._compat_alarm_before_rule_type_kwargs(kwargs) + def _compat_legacy_alarm_kwargs(cls, kwargs, create=False): + cls._compat_counter_rename_kwargs(kwargs, create) + cls._compat_alarm_before_rule_type_kwargs(kwargs, create) @staticmethod - def _compat_counter_rename_kwargs(kwargs): + def _compat_counter_rename_kwargs(kwargs, create=False): # NOTE(jd) Compatibility with Havana-2 API if 'counter_name' in kwargs: warnings.warn("counter_name has been renamed to meter_name", @@ -80,40 +81,40 @@ class AlarmManager(base.Manager): kwargs['meter_name'] = kwargs['counter_name'] @staticmethod - def _compat_alarm_before_rule_type_kwargs(kwargs): + def _compat_alarm_before_rule_type_kwargs(kwargs, create=False): # NOTE(sileht) Compatibility with Havana-3 API - if kwargs.get('type'): - return - warnings.warn("alarm without type set is deprecated", - DeprecationWarning) + if create and 'type' not in kwargs: + warnings.warn("alarm without type set is deprecated", + DeprecationWarning) + kwargs['type'] = 'threshold' - kwargs['type'] = 'threshold' - kwargs['threshold_rule'] = {} for field in ['period', 'evaluation_periods', 'threshold', 'statistic', 'comparison_operator', 'meter_name']: if field in kwargs: - kwargs['threshold_rule'][field] = kwargs[field] + kwargs.setdefault('threshold_rule', {})[field] = kwargs[field] del kwargs[field] - query = [] if 'matching_metadata' in kwargs: + query = [] for key in kwargs['matching_metadata']: query.append({'field': key, 'op': 'eq', 'value': kwargs['matching_metadata'][key]}) del kwargs['matching_metadata'] - kwargs['threshold_rule']['query'] = query + kwargs['threshold_rule']['query'] = query def create(self, **kwargs): - self._compat_legacy_alarm_kwargs(kwargs) + self._compat_legacy_alarm_kwargs(kwargs, create=True) new = dict((key, value) for (key, value) in kwargs.items() if key in CREATION_ATTRIBUTES) return self._create(self._path(), new) def update(self, alarm_id, **kwargs): self._compat_legacy_alarm_kwargs(kwargs) - updated = dict((key, value) for (key, value) in kwargs.items() - if key in UPDATABLE_ATTRIBUTES) + updated = self.get(alarm_id).to_dict() + kwargs = dict((k, v) for k, v in kwargs.items() + if k in updated and k in UPDATABLE_ATTRIBUTES) + utils.merge_nested_dict(updated, kwargs, depth=1) return self._update(self._path(alarm_id), updated) def delete(self, alarm_id): diff --git a/ceilometerclient/v2/shell.py b/ceilometerclient/v2/shell.py index bcd56579..60fa1079 100644 --- a/ceilometerclient/v2/shell.py +++ b/ceilometerclient/v2/shell.py @@ -196,45 +196,49 @@ def do_alarm_show(cc, args={}): _display_alarm(alarm) -def common_alarm_arguments(func): - @utils.arg('--name', metavar='', required=True, - help='Name of the alarm (must be unique per tenant)') - @utils.arg('--project-id', metavar='', - help='Tenant to associate with alarm ' - '(only settable by admin users)') - @utils.arg('--user-id', metavar='', - help='User to associate with alarm ' - '(only settable by admin users)') - @utils.arg('--description', metavar='', - help='Free text description of the alarm') - @utils.arg('--state', metavar='', - help='State of the alarm, one of: ' + str(ALARM_STATES)) - @utils.arg('--enabled', type=utils.string_to_bool, metavar='{True|False}', - help='True if alarm evaluation/actioning is enabled') - @utils.arg('--alarm-action', dest='alarm_actions', - metavar='', action='append', default=None, - help=('URL to invoke when state transitions to alarm. ' - 'May be used multiple times.')) - @utils.arg('--ok-action', dest='ok_actions', - metavar='', action='append', default=None, - help=('URL to invoke when state transitions to OK. ' - 'May be used multiple times.')) - @utils.arg('--insufficient-data-action', dest='insufficient_data_actions', - metavar='', action='append', default=None, - help=('URL to invoke when state transitions to unkown. ' - 'May be used multiple times.')) - @utils.arg('--repeat-actions', dest='repeat_actions', - metavar='{True|False}', type=utils.string_to_bool, - default=False, - help=('True if actions should be repeatedly notified ' - 'while alarm remains in target state')) - @functools.wraps(func) - def _wrapper(*args, **kwargs): - return func(*args, **kwargs) +def common_alarm_arguments(create=False): + def _wrapper(func): + @utils.arg('--name', metavar='', required=create, + help='Name of the alarm (must be unique per tenant)') + @utils.arg('--project-id', metavar='', + help='Tenant to associate with alarm ' + '(only settable by admin users)') + @utils.arg('--user-id', metavar='', + help='User to associate with alarm ' + '(only settable by admin users)') + @utils.arg('--description', metavar='', + help='Free text description of the alarm') + @utils.arg('--state', metavar='', + help='State of the alarm, one of: ' + str(ALARM_STATES)) + @utils.arg('--enabled', type=utils.string_to_bool, + metavar='{True|False}', + help='True if alarm evaluation/actioning is enabled') + @utils.arg('--alarm-action', dest='alarm_actions', + metavar='', action='append', default=None, + help=('URL to invoke when state transitions to alarm. ' + 'May be used multiple times.')) + @utils.arg('--ok-action', dest='ok_actions', + metavar='', action='append', default=None, + help=('URL to invoke when state transitions to OK. ' + 'May be used multiple times.')) + @utils.arg('--insufficient-data-action', + dest='insufficient_data_actions', + metavar='', action='append', default=None, + help=('URL to invoke when state transitions to unkown. ' + 'May be used multiple times.')) + @utils.arg('--repeat-actions', dest='repeat_actions', + metavar='{True|False}', type=utils.string_to_bool, + default=False, + help=('True if actions should be repeatedly notified ' + 'while alarm remains in target state')) + @functools.wraps(func) + def _wrapped(*args, **kwargs): + return func(*args, **kwargs) + return _wrapped return _wrapper -@common_alarm_arguments +@common_alarm_arguments(create=True) @utils.arg('--period', type=int, metavar='', help='Length of each period (seconds) to evaluate over') @utils.arg('--evaluation-periods', type=int, metavar='', @@ -259,7 +263,7 @@ def do_alarm_create(cc, args={}): _display_alarm(alarm) -@common_alarm_arguments +@common_alarm_arguments(create=True) @utils.arg('--meter-name', metavar='', required=True, dest='threshold_rule/meter_name', help='Metric to evaluate against') @@ -294,7 +298,7 @@ def do_alarm_threshold_create(cc, args={}): _display_alarm(alarm) -@common_alarm_arguments +@common_alarm_arguments(create=True) @utils.arg('--alarm_ids', action='append', metavar='', required=True, dest='combination_rule/alarm_ids', help='List of alarm id') @@ -313,18 +317,18 @@ def do_alarm_combination_create(cc, args={}): @utils.arg('-a', '--alarm_id', metavar='', required=True, help='ID of the alarm to update.') -@common_alarm_arguments +@common_alarm_arguments() @utils.arg('--period', type=int, metavar='', help='Length of each period (seconds) to evaluate over') @utils.arg('--evaluation-periods', type=int, metavar='', help='Number of periods to evaluate over') -@utils.arg('--meter-name', metavar='', required=True, +@utils.arg('--meter-name', metavar='', help='Metric to evaluate against') @utils.arg('--statistic', metavar='', help='Statistic to evaluate, one of: ' + str(STATISTICS)) @utils.arg('--comparison-operator', metavar='', help='Operator to compare with, one of: ' + str(ALARM_OPERATORS)) -@utils.arg('--threshold', type=float, metavar='', required=True, +@utils.arg('--threshold', type=float, metavar='', help='Threshold to evaluate against') @utils.arg('--matching-metadata', dest='matching_metadata', metavar='', action='append', default=None, @@ -344,9 +348,9 @@ def do_alarm_update(cc, args={}): @utils.arg('-a', '--alarm_id', metavar='', required=True, help='ID of the alarm to update.') -@common_alarm_arguments +@common_alarm_arguments() @utils.arg('--meter-name', metavar='', - dest='threshold_rule/meter_name', required=True, + dest='threshold_rule/meter_name', help='Metric to evaluate against') @utils.arg('--period', type=int, metavar='', dest='threshold_rule/period', @@ -360,7 +364,7 @@ def do_alarm_update(cc, args={}): @utils.arg('--comparison-operator', metavar='', dest='threshold_rule/comparison_operator', help='Operator to compare with, one of: ' + str(ALARM_OPERATORS)) -@utils.arg('--threshold', type=float, metavar='', required=True, +@utils.arg('--threshold', type=float, metavar='', dest='threshold_rule/threshold', help='Threshold to evaluate against') @utils.arg('-q', '--query', metavar='', @@ -385,9 +389,9 @@ def do_alarm_threshold_update(cc, args={}): @utils.arg('-a', '--alarm_id', metavar='', required=True, help='ID of the alarm to update.') -@common_alarm_arguments +@common_alarm_arguments() @utils.arg('--alarm_ids', action='append', metavar='', - dest='combination_rule/alarm_ids', required=True, + dest='combination_rule/alarm_ids', help='List of alarm id') @utils.arg('---operator', metavar='', dest='combination_rule/operator',