Updated _get_alarm_state function

- Get the state of an alarm based on the severity of the collectd
   notification that is sent and not the message.
 - Added a bug fix reno for this.
 - Reduced the probability that the state will return "insufficient
   data".

Change-Id: Ic993337aa49aed1d189708f5d8cd3935e6d1bbe1
Closes-bug: #1681392
Closes-bug: #1672301

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)

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,
+                result = self._update_or_create_alarm(alarm_name, auth_token,
-                                                      auth_token, severity,
+                                                      severity, metername)
-                                                      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,
+            result = self._update_or_create_alarm(alarm_name, auth_token,
-                                                  auth_token, severity,
+                                                  severity, metername)
-                                                  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,
+                self._create_alarm(endpoint, severity, metername, alarm_name)
-                                   metername, alarm_name, message)
         return result
 
-    def _create_alarm(self, endpoint, severity, metername,
+    def _create_alarm(self, endpoint, severity, metername, alarm_name):
-                      alarm_name, message):
         """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 severity in ('critical', 'moderate'):
-        if 'above' in message:
             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

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)

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.
+