diff --git a/collectd_ceilometer/aodh/notifier.py b/collectd_ceilometer/aodh/notifier.py index 746d094..19ece0a 100644 --- a/collectd_ceilometer/aodh/notifier.py +++ b/collectd_ceilometer/aodh/notifier.py @@ -48,9 +48,9 @@ class Notifier(object): 'Writing: plugin="%s", message="%s", severity="%s", time="%s', vl.plugin, message, severity, timestamp) - self._send_data(metername, severity, resource_id, message) + self._send_data(metername, severity, resource_id) - def _send_data(self, metername, severity, resource_id, message): + def _send_data(self, metername, severity, resource_id): """Send data to Aodh.""" LOGGER.debug('Sending alarm for %s', metername) - self._sender.send(metername, severity, resource_id, message) + self._sender.send(metername, severity, resource_id) diff --git a/collectd_ceilometer/aodh/sender.py b/collectd_ceilometer/aodh/sender.py index e093de9..d92de7c 100644 --- a/collectd_ceilometer/aodh/sender.py +++ b/collectd_ceilometer/aodh/sender.py @@ -110,7 +110,7 @@ class Sender(object): return self._auth_token - def send(self, metername, severity, resource_id, message): + def send(self, metername, severity, resource_id): """Send the payload to Aodh.""" # get the auth_token auth_token = self._authenticate() @@ -131,7 +131,7 @@ class Sender(object): alarm_name = self._get_alarm_name(metername, resource_id) # Update or create this alarm - result = self._update_or_create_alarm(alarm_name, message, auth_token, + result = self._update_or_create_alarm(alarm_name, auth_token, severity, metername) if result is None: @@ -154,9 +154,8 @@ class Sender(object): auth_token = self._authenticate() if auth_token is not None: - result = self._update_or_create_alarm(alarm_name, message, - auth_token, severity, - metername) + result = self._update_or_create_alarm(alarm_name, auth_token, + severity, metername) if result.status_code == HTTP_NOT_FOUND: LOGGER.debug("Received 404 error when submitting %s notification, \ @@ -164,9 +163,8 @@ class Sender(object): alarm_name) # check for and/or get alarm_id - result = self._update_or_create_alarm(alarm_name, message, - auth_token, severity, - metername) + result = self._update_or_create_alarm(alarm_name, auth_token, + severity, metername) if result.status_code == HTTP_CREATED: LOGGER.debug('Result: %s', HTTP_CREATED) @@ -182,12 +180,12 @@ class Sender(object): Config.instance().CEILOMETER_URL_TYPE) return endpoint - def _update_or_create_alarm(self, alarm_name, message, auth_token, + def _update_or_create_alarm(self, alarm_name, auth_token, severity, metername): # check for an alarm and update try: alarm_id = self._get_alarm_id(alarm_name) - result = self._update_alarm(alarm_id, message, auth_token) + result = self._update_alarm(alarm_id, severity, auth_token) # or create a new alarm except KeyError as ke: @@ -196,17 +194,15 @@ class Sender(object): endpoint = self._get_endpoint("aodh") LOGGER.warn('No known ID for %s', alarm_name) result, self._alarm_ids[alarm_name] = \ - self._create_alarm(endpoint, severity, - metername, alarm_name, message) + self._create_alarm(endpoint, severity, metername, alarm_name) return result - def _create_alarm(self, endpoint, severity, metername, - alarm_name, message): + def _create_alarm(self, endpoint, severity, metername, alarm_name): """Create a new alarm with a new alarm_id.""" url = "{}/v2/alarms/".format(endpoint) rule = {'event_type': metername, } - payload = json.dumps({'state': self._get_alarm_state(message), + payload = json.dumps({'state': self._get_alarm_state(severity), 'name': alarm_name, 'severity': severity, 'type': "event", @@ -222,12 +218,11 @@ class Sender(object): """Try and return an alarm_id for an collectd notification""" return self._alarm_ids[alarm_name] - def _get_alarm_state(self, message): + def _get_alarm_state(self, severity): """Get the state of the alarm.""" - message = message.split() - if 'above' in message: + if severity in ('critical', 'moderate'): alarm_state = "alarm" - elif 'within' in message: + elif severity == 'low': alarm_state = "ok" else: alarm_state = "insufficient data" @@ -238,11 +233,11 @@ class Sender(object): alarm_name = metername + "(" + resource_id + ")" return alarm_name - def _update_alarm(self, alarm_id, message, auth_token): + def _update_alarm(self, alarm_id, severity, auth_token): """Perform the alarm update.""" url = self._url_base % (alarm_id) # create the payload and update the state of the alarm - payload = json.dumps(self._get_alarm_state(message)) + payload = json.dumps(self._get_alarm_state(severity)) return self._perform_update_request(url, auth_token, payload) @classmethod diff --git a/collectd_ceilometer/tests/aodh/test_plugin.py b/collectd_ceilometer/tests/aodh/test_plugin.py index fb9a627..ebfb744 100644 --- a/collectd_ceilometer/tests/aodh/test_plugin.py +++ b/collectd_ceilometer/tests/aodh/test_plugin.py @@ -159,16 +159,15 @@ class TestPlugin(unittest.TestCase): _get_alarm_name.return_value = 'my-alarm' meter_name = meter.meter_name.return_value - message = meter.message.return_value severity = meter.severity.return_value resource_id = meter.resource_id.return_value # send the values - instance.send(meter_name, severity, resource_id, message) + instance.send(meter_name, severity, resource_id) # check that the function is called _update_or_create_alarm.assert_called_once_with( - instance, 'my-alarm', message, auth_client.auth_token, + instance, 'my-alarm', auth_client.auth_token, severity, meter_name) # reset function @@ -177,11 +176,11 @@ class TestPlugin(unittest.TestCase): # run test again for failed attempt _update_or_create_alarm.return_value = None - instance.send(meter_name, severity, resource_id, message) + instance.send(meter_name, severity, resource_id) # and values that have been sent _update_or_create_alarm.assert_called_once_with( - instance, 'my-alarm', message, auth_client.auth_token, + instance, 'my-alarm', auth_client.auth_token, severity, meter_name) # reset post method @@ -211,13 +210,12 @@ class TestPlugin(unittest.TestCase): # init values to send _get_alarm_id.return_value = 'my-alarm-id' - message = meter.message.return_value metername = meter.meter_name.return_value severity = meter.severity.return_value rid = meter.resource_id.return_value # send the values - instance.send(metername, severity, rid, message) + instance.send(metername, severity, rid) # update the alarm put.assert_called_once_with( @@ -255,13 +253,12 @@ class TestPlugin(unittest.TestCase): _get_alarm_id.return_value = None _get_alarm_id.side_effect = KeyError() _create_alarm.return_value = requests.Response(), 'my-alarm-id' - message = meter.message.return_value metername = meter.meter_name.return_value severity = meter.severity.return_value rid = meter.resource_id.return_value # send the values again - instance.send(metername, severity, rid, message) + instance.send(metername, severity, rid) put.assert_not_called() @@ -298,6 +295,69 @@ class TestPlugin(unittest.TestCase): # no requests method has been called put.assert_not_called() + @mock.patch.object(base.Meter, 'severity', spec=callable) + def test_get_alarm_state_severity_low(self, severity): + """Test _get_alarm_state if severity is 'low'. + + Set-up: create a sender instance, set severity to low + Test: call _get_alarm_state method with severity=low + Expected-behaviour: returned state value should equal 'ok' + and won't equal 'alarm' or insufficient data' + """ + instance = sender.Sender() + + # run test for moderate severity + severity.return_value = 'low' + + self.assertEqual(instance._get_alarm_state('low'), 'ok') + + self.assertNotEqual(instance._get_alarm_state('low'), 'alarm') + + self.assertNotEqual(instance._get_alarm_state('low'), + 'insufficient data') + + @mock.patch.object(base.Meter, 'severity', spec=callable) + def test_get_alarm_state_severity_moderate(self, severity): + """Test _get_alarm_state if severity is 'moderate'. + + Set-up: create a sender instance, set severity to moderate + Test: call _get_alarm_state method with severity=moderate + Expected-behaviour: returned state value should equal 'alarm' + and won't equal 'ok' or insufficient data' + """ + instance = sender.Sender() + + # run test for moderate severity + severity.return_value = 'moderate' + + self.assertEqual(instance._get_alarm_state('moderate'), 'alarm') + + self.assertNotEqual(instance._get_alarm_state('moderate'), 'ok') + + self.assertNotEqual(instance._get_alarm_state('moderate'), + 'insufficient data') + + @mock.patch.object(base.Meter, 'severity', spec=callable) + def test_get_alarm_state_severity_critical(self, severity): + """Test _get_alarm_state if severity is 'critical'. + + Set-up: create a sender instance, set severity to critical + Test: call _get_alarm_state method with severity=critical + Expected-behaviour: returned state value should equal 'alarm' + and won't equal 'ok' or 'insufficient data' + """ + instance = sender.Sender() + + # run test for moderate severity + severity.return_value = 'critical' + + self.assertEqual(instance._get_alarm_state('critical'), 'alarm') + + self.assertNotEqual(instance._get_alarm_state('critical'), 'ok') + + self.assertNotEqual(instance._get_alarm_state('critical'), + 'insufficient data') + @mock.patch.object(sender.Sender, '_perform_post_request', spec=callable) @mock.patch.object(sender, 'ClientV3', autospec=True) @mock_collectd() @@ -345,15 +405,14 @@ class TestPlugin(unittest.TestCase): alarm_name = _get_alarm_name.return_value meter_name = meter.meter_name.return_value - message = meter.message.return_value severity = meter.severity.return_value resource_id = meter.resource_id.return_value # send the data - instance.send(meter_name, severity, resource_id, message) + instance.send(meter_name, severity, resource_id) _update_or_create_alarm.assert_called_once_with( - instance, alarm_name, message, client.auth_token, + instance, alarm_name, client.auth_token, severity, meter_name) # de-assert the request @@ -373,10 +432,10 @@ class TestPlugin(unittest.TestCase): client.auth_token = 'Test auth token' # send the data - instance.send(meter_name, severity, resource_id, message) + instance.send(meter_name, severity, resource_id) _update_or_create_alarm.assert_called_once_with( - instance, alarm_name, message, client.auth_token, + instance, alarm_name, client.auth_token, severity, meter_name) # update/create response is unauthorized -> new token needs @@ -388,7 +447,7 @@ class TestPlugin(unittest.TestCase): client.auth_token = 'New test auth token' # send the data again - instance.send(meter_name, severity, resource_id, message) + instance.send(meter_name, severity, resource_id) @mock.patch.object(sender, 'ClientV3', autospec=True) @mock.patch.object(plugin, 'Notifier', autospec=True) diff --git a/releasenotes/notes/bug-1681392-983ccc16bf0f2f69.yaml b/releasenotes/notes/bug-1681392-983ccc16bf0f2f69.yaml new file mode 100644 index 0000000..7e00123 --- /dev/null +++ b/releasenotes/notes/bug-1681392-983ccc16bf0f2f69.yaml @@ -0,0 +1,13 @@ +--- +fixes: + - | + Fixes 'bug 1681392 https://bugs.launchpad.net/collectd-ceilometer-plugin/+bug/1681392'. + Changed the way the state of the alarm is set to depend on the severity of + the notification sent. Messages are subject to change based on the collectd + plugin that sent the notification. This is a more reliable method to set the + state of an alarm. + - | + Fixes 'bug 1672301 https://bugs.launchpad.net/collectd-ceilometer-plugin/+bug/1672301'. + Changed the why the state of an alarm is set thus reducing the possibility + that it will result in an "insufficient data" state. +