Add retry function for alarm REST notifier2014.2.b2

The alarm REST notifier does not have retry function: notification simply
fails, for example, when the receiver is down. With this patch, retry
function for alarm REST notifier is added.

In some cases, alarm notifier can not get response even if the receiver is
working fine (e.g. temporarily lost connection, failing over).
Currently, alarm notifier give up to notify in that case.
So, this patch introduces ability to retry to alarm notifier.

By setting a unique request-id in HTTP request header (same id on retry),
the receiver can decide if the notification is new or redundant.

Change-Id: I29ee910e5beb5669377baaaa2810044a7c40d9ad
Closes-bug: #1329716

2 files changed, 40 insertions, 13 deletions

diff --git a/ceilometer/alarm/notifier/rest.py b/ceilometer/alarm/notifier/rest.py
index ec10a1a8..1648cde1 100644
--- a/ceilometer/alarm/notifier/rest.py
+++ b/ceilometer/alarm/notifier/rest.py
@@ -22,6 +22,7 @@ import requests
 import six.moves.urllib.parse as urlparse
 
 from ceilometer.alarm import notifier
+from ceilometer.openstack.common import context
 from ceilometer.openstack.common.gettextutils import _
 from ceilometer.openstack.common import jsonutils
 from ceilometer.openstack.common import log
@@ -42,6 +43,10 @@ REST_NOTIFIER_OPTS = [
                 help='Whether to verify the SSL Server certificate when '
                 'calling alarm action.'
                 ),
+    cfg.IntOpt('rest_notifier_max_retries',
+               default=0,
+               help='Number of retries for REST notifier',
+               ),
 
 ]
 
@@ -54,16 +59,21 @@ class RestAlarmNotifier(notifier.AlarmNotifier):
     @staticmethod
     def notify(action, alarm_id, previous, current, reason, reason_data,
                headers=None):
+        headers = headers or {}
+        if not headers.get('x-openstack-request-id'):
+            headers['x-openstack-request-id'] = context.generate_request_id()
+
         LOG.info(_(
             "Notifying alarm %(alarm_id)s from %(previous)s "
             "to %(current)s with action %(action)s because "
-            "%(reason)s") % ({'alarm_id': alarm_id, 'previous': previous,
-                              'current': current, 'action': action,
-                              'reason': reason}))
+            "%(reason)s. request-id: %(request_id)s") %
+            ({'alarm_id': alarm_id, 'previous': previous,
+              'current': current, 'action': action,
+              'reason': reason,
+              'request_id': headers['x-openstack-request-id']}))
         body = {'alarm_id': alarm_id, 'previous': previous,
                 'current': current, 'reason': reason,
                 'reason_data': reason_data}
-        headers = headers or {}
         headers['content-type'] = 'application/json'
         kwargs = {'data': jsonutils.dumps(body),
                   'headers': headers}
@@ -80,4 +90,12 @@ class RestAlarmNotifier(notifier.AlarmNotifier):
             if cert:
                 kwargs['cert'] = (cert, key) if key else cert
 
-        eventlet.spawn_n(requests.post, action.geturl(), **kwargs)
+        # FIXME(rhonjo): Retries are automatically done by urllib3 in requests
+        # library. However, there's no interval between retries in urllib3
+        # implementation. It will be better to put some interval between
+        # retries (future work).
+        max_retries = cfg.CONF.alarm.rest_notifier_max_retries
+        session = requests.Session()
+        session.mount(action.geturl(),
+                      requests.adapters.HTTPAdapter(max_retries=max_retries))
+        eventlet.spawn_n(session.post, action.geturl(), **kwargs)
diff --git a/ceilometer/tests/alarm/test_notifier.py b/ceilometer/tests/alarm/test_notifier.py
index 8c5dcc99..a91366f7 100644
--- a/ceilometer/tests/alarm/test_notifier.py
+++ b/ceilometer/tests/alarm/test_notifier.py
@@ -44,6 +44,9 @@ class TestAlarmNotifier(tests_base.BaseTestCase):
         self.CONF = self.useFixture(config.Config()).conf
         self.setup_messaging(self.CONF)
         self.service = service.AlarmNotifierService()
+        self.useFixture(mockpatch.Patch(
+            'ceilometer.openstack.common.context.generate_request_id',
+            self._fake_generate_request_id))
 
     @mock.patch('ceilometer.pipeline.setup_pipeline', mock.MagicMock())
     def test_init_host(self):
@@ -94,13 +97,17 @@ class TestAlarmNotifier(tests_base.BaseTestCase):
         notification['actions'] = [action]
         return notification
 
-    HTTP_HEADERS = {'content-type': 'application/json'}
+    HTTP_HEADERS = {'x-openstack-request-id': 'fake_request_id',
+                    'content-type': 'application/json'}
+
+    def _fake_generate_request_id(self):
+        return self.HTTP_HEADERS['x-openstack-request-id']
 
     def test_notify_alarm_rest_action_ok(self):
         action = 'http://host/action'
 
         with mock.patch('eventlet.spawn_n', self._fake_spawn_n):
-            with mock.patch.object(requests, 'post') as poster:
+            with mock.patch.object(requests.Session, 'post') as poster:
                 self.service.notify_alarm(context.get_admin_context(),
                                           self._notification(action))
                 poster.assert_called_with(action, data=DATA_JSON,
@@ -114,7 +121,7 @@ class TestAlarmNotifier(tests_base.BaseTestCase):
                                group='alarm')
 
         with mock.patch('eventlet.spawn_n', self._fake_spawn_n):
-            with mock.patch.object(requests, 'post') as poster:
+            with mock.patch.object(requests.Session, 'post') as poster:
                 self.service.notify_alarm(context.get_admin_context(),
                                           self._notification(action))
                 poster.assert_called_with(action, data=DATA_JSON,
@@ -132,7 +139,7 @@ class TestAlarmNotifier(tests_base.BaseTestCase):
                                group='alarm')
 
         with mock.patch('eventlet.spawn_n', self._fake_spawn_n):
-            with mock.patch.object(requests, 'post') as poster:
+            with mock.patch.object(requests.Session, 'post') as poster:
                 self.service.notify_alarm(context.get_admin_context(),
                                           self._notification(action))
                 poster.assert_called_with(action, data=DATA_JSON,
@@ -146,7 +153,7 @@ class TestAlarmNotifier(tests_base.BaseTestCase):
                                group='alarm')
 
         with mock.patch('eventlet.spawn_n', self._fake_spawn_n):
-            with mock.patch.object(requests, 'post') as poster:
+            with mock.patch.object(requests.Session, 'post') as poster:
                 self.service.notify_alarm(context.get_admin_context(),
                                           self._notification(action))
                 poster.assert_called_with(action, data=DATA_JSON,
@@ -157,7 +164,7 @@ class TestAlarmNotifier(tests_base.BaseTestCase):
         action = 'https://host/action?ceilometer-alarm-ssl-verify=0'
 
         with mock.patch('eventlet.spawn_n', self._fake_spawn_n):
-            with mock.patch.object(requests, 'post') as poster:
+            with mock.patch.object(requests.Session, 'post') as poster:
                 self.service.notify_alarm(context.get_admin_context(),
                                           self._notification(action))
                 poster.assert_called_with(action, data=DATA_JSON,
@@ -171,7 +178,7 @@ class TestAlarmNotifier(tests_base.BaseTestCase):
                                group='alarm')
 
         with mock.patch('eventlet.spawn_n', self._fake_spawn_n):
-            with mock.patch.object(requests, 'post') as poster:
+            with mock.patch.object(requests.Session, 'post') as poster:
                 self.service.notify_alarm(context.get_admin_context(),
                                           self._notification(action))
                 poster.assert_called_with(action, data=DATA_JSON,
@@ -214,12 +221,14 @@ class TestAlarmNotifier(tests_base.BaseTestCase):
 
         client = mock.MagicMock()
         client.auth_token = 'token_1234'
+        headers = {'X-Auth-Token': 'token_1234'}
+        headers.update(self.HTTP_HEADERS)
 
         self.useFixture(mockpatch.Patch('keystoneclient.v3.client.Client',
                                         lambda **kwargs: client))
 
         with mock.patch('eventlet.spawn_n', self._fake_spawn_n):
-            with mock.patch.object(requests, 'post') as poster:
+            with mock.patch.object(requests.Session, 'post') as poster:
                 self.service.notify_alarm(context.get_admin_context(),
                                           self._notification(action))
                 headers = {'X-Auth-Token': 'token_1234'}