Avoid reset of repeat_actions attribute on alarm update

Fixes bug 1253057

Previously, when an unrelated alarm attribute was updated via the CLI,
the repeat_actions attribute was also set to False as an unwanted side-effect.

Now, we only set this attribute in the update call if explicitly specified
as a CLI arg. Otherwise the value provided in the PUT request body reflects
the pre-existing value for this attribute.

Change-Id: I94468ff649dd4367848c74e94a3feb08494bb223

ceilometerclient/tests/v2/test_shell.py

@@ -138,6 +138,73 @@ def test_alarm_constrained_history(self):
                                     parsed_query=parsed_query)
 
 
+class ShellAlarmCommandTest(utils.BaseTestCase):
+
+    ALARM_ID = '768ff714-8cfb-4db9-9753-d484cb33a1cc'
+    ALARM = {"alarm_actions": ["log://"],
+             "ok_actions": [],
+             "description": "instance running hot",
+             "timestamp": "2013-11-20T10:38:42.206952",
+             "enabled": True,
+             "state_timestamp": "2013-11-19T17:20:44",
+             "threshold_rule": {"meter_name": "cpu_util",
+                                "evaluation_periods": 3,
+                                "period": 600,
+                                "statistic": "avg",
+                                "threshold": 99.0,
+                                "query": [{"field": "resource_id",
+                                           "value": "INSTANCE_ID",
+                                           "op": "eq"}],
+                                "comparison_operator": "gt"},
+             "alarm_id": ALARM_ID,
+             "state": "insufficient data",
+             "insufficient_data_actions": [],
+             "repeat_actions": True,
+             "user_id": "528d9b68fa774689834b5c04b4564f8a",
+             "project_id": "ed9d4e2be2a748bc80108053cf4598f5",
+             "type": "threshold",
+             "name": "cpu_high"}
+
+    def setUp(self):
+        super(ShellAlarmCommandTest, self).setUp()
+        self.cc = mock.Mock()
+        self.cc.alarms = mock.Mock()
+        self.args = mock.Mock()
+        self.args.alarm_id = self.ALARM_ID
+
+    def _do_test_alarm_update_repeat_actions(self, repeat_actions):
+        self.args.threshold = 42.0
+        if repeat_actions is not None:
+            self.args.repeat_actions = repeat_actions
+        orig = sys.stdout
+        sys.stdout = six.StringIO()
+        alarm = [alarms.Alarm(mock.Mock(), self.ALARM)]
+        self.cc.alarms.get.return_value = alarm
+        self.cc.alarms.update.return_value = alarm[0]
+
+        try:
+            ceilometer_shell.do_alarm_update(self.cc, self.args)
+            args, kwargs = self.cc.alarms.update.call_args
+            self.assertEqual(self.ALARM_ID, args[0])
+            self.assertEqual(42.0, kwargs.get('threshold'))
+            if repeat_actions is not None:
+                self.assertEqual(repeat_actions, kwargs.get('repeat_actions'))
+            else:
+                self.assertFalse('repeat_actions' in kwargs)
+        finally:
+            sys.stdout.close()
+            sys.stdout = orig
+
+    def test_alarm_update_repeat_actions_untouched(self):
+        self._do_test_alarm_update_repeat_actions(None)
+
+    def test_alarm_update_repeat_actions_set(self):
+        self._do_test_alarm_update_repeat_actions(True)
+
+    def test_alarm_update_repeat_actions_clear(self):
+        self._do_test_alarm_update_repeat_actions(False)
+
+
 class ShellSampleListCommandTest(utils.BaseTestCase):
 
     METER = 'cpu_util'

ceilometerclient/v2/shell.py

@@ -267,11 +267,6 @@ def _wrapper(func):
                    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)
@@ -296,6 +291,11 @@ def _wrapped(*args, **kwargs):
            metavar='<Matching Metadata>', action='append', default=None,
            help=('A meter should match this resource metadata (key=value) '
                  'additionally to the meter_name'))
+@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'))
 def do_alarm_create(cc, args={}):
     '''Create a new alarm (Deprecated).'''
     fields = dict(filter(lambda x: not (x[1] is None), vars(args).items()))
@@ -327,6 +327,11 @@ def do_alarm_create(cc, args={}):
            dest='threshold_rule/query',
            help='The query to find the data for computing statistics '
            '(key[op]value; list.)')
+@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'))
 def do_alarm_threshold_create(cc, args={}):
     '''Create a new alarm based on computed statistics.'''
     fields = dict(filter(lambda x: not (x[1] is None), vars(args).items()))
@@ -347,6 +352,11 @@ def do_alarm_threshold_create(cc, args={}):
            dest='combination_rule/operator',
            help='Operator to compare with, one of: ' + str(
                ALARM_COMBINATION_OPERATORS))
+@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'))
 def do_alarm_combination_create(cc, args={}):
     '''Create a new alarm based on state of other alarms.'''
     fields = dict(filter(lambda x: not (x[1] is None), vars(args).items()))
@@ -375,6 +385,10 @@ def do_alarm_combination_create(cc, args={}):
            metavar='<Matching Metadata>', action='append', default=None,
            help=('A meter should match this resource metadata (key=value) '
                  'additionally to the meter_name'))
+@utils.arg('--repeat-actions', dest='repeat_actions',
+           metavar='{True|False}', type=utils.string_to_bool,
+           help=('True if actions should be repeatedly notified '
+                'while alarm remains in target state'))
 def do_alarm_update(cc, args={}):
     '''Update an existing alarm.'''
     fields = dict(filter(lambda x: not (x[1] is None), vars(args).items()))
@@ -412,6 +426,10 @@ def do_alarm_update(cc, args={}):
            dest='threshold_rule/query',
            help='The query to find the data for computing statistics '
            '(key[op]value; list.)')
+@utils.arg('--repeat-actions', dest='repeat_actions',
+           metavar='{True|False}', type=utils.string_to_bool,
+           help=('True if actions should be repeatedly notified '
+                'while alarm remains in target state'))
 def do_alarm_threshold_update(cc, args={}):
     '''Update an existing alarm based on computed statistics.'''
     fields = dict(filter(lambda x: not (x[1] is None), vars(args).items()))
@@ -438,6 +456,10 @@ def do_alarm_threshold_update(cc, args={}):
            dest='combination_rule/operator',
            help='Operator to compare with, one of: ' + str(
                ALARM_COMBINATION_OPERATORS))
+@utils.arg('--repeat-actions', dest='repeat_actions',
+           metavar='{True|False}', type=utils.string_to_bool,
+           help=('True if actions should be repeatedly notified '
+                'while alarm remains in target state'))
 def do_alarm_combination_update(cc, args={}):
     '''Update an existing alarm based on state of other alarms.'''
     fields = dict(filter(lambda x: not (x[1] is None), vars(args).items()))