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

5 files changed, 129 insertions, 68 deletions

diff --git a/ceilometerclient/common/utils.py b/ceilometerclient/common/utils.py
index ef76dcc..f28a3ea 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 0e209b0..e0626af 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 82279f5..f64ded1 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 d1ce7c6..71cc0ac 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 bcd5657..60fa107 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='<NAME>', required=True,
-               help='Name of the alarm (must be unique per tenant)')
-    @utils.arg('--project-id', metavar='<PROJECT_ID>',
-               help='Tenant to associate with alarm '
-               '(only settable by admin users)')
-    @utils.arg('--user-id', metavar='<USER_ID>',
-               help='User to associate with alarm '
-               '(only settable by admin users)')
-    @utils.arg('--description', metavar='<DESCRIPTION>',
-               help='Free text description of the alarm')
-    @utils.arg('--state', metavar='<STATE>',
-               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='<Webhook URL>', 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='<Webhook URL>', 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='<Webhook URL>', 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='<NAME>', required=create,
+                   help='Name of the alarm (must be unique per tenant)')
+        @utils.arg('--project-id', metavar='<PROJECT_ID>',
+                   help='Tenant to associate with alarm '
+                   '(only settable by admin users)')
+        @utils.arg('--user-id', metavar='<USER_ID>',
+                   help='User to associate with alarm '
+                   '(only settable by admin users)')
+        @utils.arg('--description', metavar='<DESCRIPTION>',
+                   help='Free text description of the alarm')
+        @utils.arg('--state', metavar='<STATE>',
+                   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='<Webhook URL>', 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='<Webhook URL>', 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='<Webhook URL>', 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='<PERIOD>',
            help='Length of each period (seconds) to evaluate over')
 @utils.arg('--evaluation-periods', type=int, metavar='<COUNT>',
@@ -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='<METRIC>', 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='<ALARM IDS>',
            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='<ALARM_ID>', required=True,
            help='ID of the alarm to update.')
-@common_alarm_arguments
+@common_alarm_arguments()
 @utils.arg('--period', type=int, metavar='<PERIOD>',
            help='Length of each period (seconds) to evaluate over')
 @utils.arg('--evaluation-periods', type=int, metavar='<COUNT>',
            help='Number of periods to evaluate over')
-@utils.arg('--meter-name', metavar='<METRIC>', required=True,
+@utils.arg('--meter-name', metavar='<METRIC>',
            help='Metric to evaluate against')
 @utils.arg('--statistic', metavar='<STATISTIC>',
            help='Statistic to evaluate, one of: ' + str(STATISTICS))
 @utils.arg('--comparison-operator', metavar='<OPERATOR>',
            help='Operator to compare with, one of: ' + str(ALARM_OPERATORS))
-@utils.arg('--threshold', type=float, metavar='<THRESHOLD>', required=True,
+@utils.arg('--threshold', type=float, metavar='<THRESHOLD>',
            help='Threshold to evaluate against')
 @utils.arg('--matching-metadata', dest='matching_metadata',
            metavar='<Matching Metadata>', action='append', default=None,
@@ -344,9 +348,9 @@ def do_alarm_update(cc, args={}):
 
 @utils.arg('-a', '--alarm_id', metavar='<ALARM_ID>', required=True,
            help='ID of the alarm to update.')
-@common_alarm_arguments
+@common_alarm_arguments()
 @utils.arg('--meter-name', metavar='<METRIC>',
-           dest='threshold_rule/meter_name', required=True,
+           dest='threshold_rule/meter_name',
            help='Metric to evaluate against')
 @utils.arg('--period', type=int, metavar='<PERIOD>',
            dest='threshold_rule/period',
@@ -360,7 +364,7 @@ def do_alarm_update(cc, args={}):
 @utils.arg('--comparison-operator', metavar='<OPERATOR>',
            dest='threshold_rule/comparison_operator',
            help='Operator to compare with, one of: ' + str(ALARM_OPERATORS))
-@utils.arg('--threshold', type=float, metavar='<THRESHOLD>', required=True,
+@utils.arg('--threshold', type=float, metavar='<THRESHOLD>',
            dest='threshold_rule/threshold',
            help='Threshold to evaluate against')
 @utils.arg('-q', '--query', metavar='<QUERY>',
@@ -385,9 +389,9 @@ def do_alarm_threshold_update(cc, args={}):
 
 @utils.arg('-a', '--alarm_id', metavar='<ALARM_ID>', required=True,
            help='ID of the alarm to update.')
-@common_alarm_arguments
+@common_alarm_arguments()
 @utils.arg('--alarm_ids', action='append', metavar='<ALARM IDS>',
-           dest='combination_rule/alarm_ids', required=True,
+           dest='combination_rule/alarm_ids',
            help='List of alarm id')
 @utils.arg('---operator', metavar='<OPERATOR>',
            dest='combination_rule/operator',