From 8eee7d0eca50c846f380f1ca753cc797d22cad55 Mon Sep 17 00:00:00 2001 From: Nejc Saje Date: Tue, 27 May 2014 10:21:27 +0000 Subject: Check if the alarm has time constraints field before displaying Fixes the bug that broke the alarms CLI if the alarm didn't have a time constraints field. Reduces code duplication of alarm printing code, so now all the alarm printing code is actually tested. Renames some auxiliary methods for more clarity. Change-Id: Ib691b4a5a6cf5ae133cd0a5576f90e4d0d189a92 Closes-bug: #1316390 --- ceilometerclient/tests/v2/test_shell.py | 45 +++++++++++++++++++++++++++---- ceilometerclient/v2/shell.py | 47 +++++++++++++++++---------------- 2 files changed, 64 insertions(+), 28 deletions(-) diff --git a/ceilometerclient/tests/v2/test_shell.py b/ceilometerclient/tests/v2/test_shell.py index 69e6a50..ea362aa 100644 --- a/ceilometerclient/tests/v2/test_shell.py +++ b/ceilometerclient/tests/v2/test_shell.py @@ -516,6 +516,8 @@ class ShellQueryAlarmsCommandTest(utils.BaseTestCase): "state_timestamp": "2014-02-20T10:37:15.589860", "threshold_rule": None, "timestamp": "2014-02-20T10:37:15.589856", + "time_constraints": [{"name": "test", "start": "0 23 * * *", + "duration": 10800}], "type": "combination", "user_id": "c96c887c216949acbdfbd8b494863567"}] @@ -543,19 +545,52 @@ class ShellQueryAlarmsCommandTest(utils.BaseTestCase): self.assertEqual('''\ +--------------------------------------+------------------+-------+---------\ +------------+--------------------------------------------------------------\ -----------------------------------------+ +----------------------------------------+--------------------------------+ | Alarm ID | Name | State | Enabled \ | Continuous | Alarm condition \ - | + | Time constraints | +--------------------------------------+------------------+-------+---------\ +------------+--------------------------------------------------------------\ -----------------------------------------+ +----------------------------------------+--------------------------------+ | 768ff714-8cfb-4db9-9753-d484cb33a1cc | SwiftObjectAlarm | ok | True \ | False | combinated states (OR) of 739e99cb-c2ec-4718-b900-332502355f3\ -8, 153462d0-a9b8-4b5b-8175-9e4b05e9b856 | +8, 153462d0-a9b8-4b5b-8175-9e4b05e9b856 | test at 0 23 * * * for 10800s | +--------------------------------------+------------------+-------+---------\ +------------+--------------------------------------------------------------\ -----------------------------------------+ +----------------------------------------+--------------------------------+ +''', sys.stdout.getvalue()) + + @mock.patch('sys.stdout', new=six.StringIO()) + def test_time_constraints_compatibility(self): + # client should be backwards compatible + alarm_without_tc = dict(self.ALARM[0]) + del alarm_without_tc['time_constraints'] + + # NOTE(nsaje): Since we're accessing a nonexisting key in the resource, + # the resource is looking it up with the manager (which is a mock). + manager_mock = mock.Mock() + del manager_mock.get + ret_alarm = [alarms.Alarm(manager_mock, alarm_without_tc)] + self.cc.query_alarms.query.return_value = ret_alarm + + ceilometer_shell.do_query_alarms(self.cc, self.args) + + self.assertEqual('''\ ++--------------------------------------+------------------+-------+---------\ ++------------+--------------------------------------------------------------\ +----------------------------------------+------------------+ +| Alarm ID | Name | State | Enabled \ +| Continuous | Alarm condition \ + | Time constraints | ++--------------------------------------+------------------+-------+---------\ ++------------+--------------------------------------------------------------\ +----------------------------------------+------------------+ +| 768ff714-8cfb-4db9-9753-d484cb33a1cc | SwiftObjectAlarm | ok | True \ +| False | combinated states (OR) of 739e99cb-c2ec-4718-b900-332502355f3\ +8, 153462d0-a9b8-4b5b-8175-9e4b05e9b856 | None | ++--------------------------------------+------------------+-------+---------\ ++------------+--------------------------------------------------------------\ +----------------------------------------+------------------+ ''', sys.stdout.getvalue()) diff --git a/ceilometerclient/v2/shell.py b/ceilometerclient/v2/shell.py index b13ab20..d0d2f75 100644 --- a/ceilometerclient/v2/shell.py +++ b/ceilometerclient/v2/shell.py @@ -186,6 +186,20 @@ def do_meter_list(cc, args={}): sortby=0) +def _display_alarm_list(alarms, sortby=None): + # omit action initially to keep output width sane + # (can switch over to vertical formatting when available from CLIFF) + field_labels = ['Alarm ID', 'Name', 'State', 'Enabled', 'Continuous', + 'Alarm condition', 'Time constraints'] + fields = ['alarm_id', 'name', 'state', 'enabled', 'repeat_actions', + 'rule', 'time_constraints'] + utils.print_list( + alarms, fields, field_labels, + formatters={'rule': alarm_rule_formatter, + 'time_constraints': time_constraints_formatter_brief}, + sortby=sortby) + + def _display_rule(type, rule): if type == 'threshold': return ('%(meter_name)s %(comparison_operator)s ' @@ -212,7 +226,7 @@ def alarm_rule_formatter(alarm): return _display_rule(alarm.type, alarm.rule) -def _display_time_constraints(time_constraints): +def _display_time_constraints_brief(time_constraints): if time_constraints: return ', '.join('%(name)s at %(start)s %(timezone)s for %(duration)ss' % { @@ -226,8 +240,10 @@ def _display_time_constraints(time_constraints): return 'None' -def time_constraints_formatter(alarm): - return _display_time_constraints(alarm.time_constraints) +def time_constraints_formatter_brief(alarm): + return _display_time_constraints_brief(getattr(alarm, + 'time_constraints', + None)) def _infer_type(detail): @@ -255,7 +271,7 @@ def alarm_change_detail_formatter(change): fields.append('%s: %s' % (k, detail[k])) if 'time_constraints' in detail: fields.append('time_constraints: %s' % - _display_time_constraints( + _display_time_constraints_brief( detail['time_constraints'])) elif change.type == 'rule change': for k, v in six.iteritems(detail): @@ -273,16 +289,7 @@ def alarm_change_detail_formatter(change): def do_alarm_list(cc, args={}): '''List the user's alarms.''' alarms = cc.alarms.list(q=options.cli_to_array(args.query)) - # omit action initially to keep output width sane - # (can switch over to vertical formatting when available from CLIFF) - field_labels = ['Alarm ID', 'Name', 'State', 'Enabled', 'Continuous', - 'Alarm condition', 'Time constraints'] - fields = ['alarm_id', 'name', 'state', 'enabled', 'repeat_actions', - 'rule', 'time_constraints'] - utils.print_list( - alarms, fields, field_labels, - formatters={'rule': alarm_rule_formatter, - 'time_constraints': time_constraints_formatter}, sortby=0) + _display_alarm_list(alarms, sortby=0) def alarm_query_formater(alarm): @@ -293,7 +300,7 @@ def alarm_query_formater(alarm): return r' AND\n'.join(qs) -def alarm_time_constraints_formatter(alarm): +def time_constraints_formatter_full(alarm): time_constraints = [] for tc in alarm.time_constraints: lines = [] @@ -314,7 +321,7 @@ def _display_alarm(alarm): if alarm.type == 'threshold': data['query'] = alarm_query_formater(alarm) if alarm.time_constraints: - data['time_constraints'] = alarm_time_constraints_formatter(alarm) + data['time_constraints'] = time_constraints_formatter_full(alarm) utils.print_dict(data, wrap=72) @@ -793,13 +800,7 @@ def do_query_alarms(cc, args): except exc.HTTPNotFound: raise exc.CommandError('Alarms not found') else: - field_labels = ['Alarm ID', 'Name', 'State', 'Enabled', 'Continuous', - 'Alarm condition'] - fields = ['alarm_id', 'name', 'state', 'enabled', 'repeat_actions', - 'rule'] - utils.print_list(alarms, fields, field_labels, - formatters={'rule': alarm_rule_formatter}, - sortby=None) + _display_alarm_list(alarms, sortby=None) @utils.arg('-f', '--filter', metavar='', -- cgit v1.2.1