From 3fee62c39b5300bbea1d0a9d8e092fd4bc119c88 Mon Sep 17 00:00:00 2001
From: Alexander Kukushkin <cyberdemn@gmail.com>
Date: Fri, 16 Jun 2017 10:27:03 +0200
Subject: [PATCH] BUGFIX: retry on boto exceptions never worked (#450)

because `boto.exception` is not an excpetion, but a python module.

+ increase retry timeout to 5 minutes
+ refactor unit-tests to cover the case with retries.
---
 patroni/scripts/aws.py | 14 +++++-----
 tests/test_aws.py      | 60 ++++++++++++++++--------------------------
 2 files changed, 30 insertions(+), 44 deletions(-)

diff --git a/patroni/scripts/aws.py b/patroni/scripts/aws.py
index c3be3ca5..ae2b4d09 100755
--- a/patroni/scripts/aws.py
+++ b/patroni/scripts/aws.py
@@ -10,27 +10,27 @@ from patroni.utils import Retry, RetryFailedError
 
 logger = logging.getLogger(__name__)
 
-retry_timeout = 15
-
 
 class AWSConnection(object):
+
     def __init__(self, cluster_name):
         self.available = False
         self.cluster_name = cluster_name if cluster_name is not None else 'unknown'
-        self._retry = Retry(deadline=retry_timeout, max_delay=5, max_tries=-1, retry_exceptions=(boto.exception,))
+        self._retry = Retry(deadline=300, max_delay=30, max_tries=-1, retry_exceptions=(boto.exception.StandardError,))
         try:
             # get the instance id
-            r = requests.get('http://169.254.169.254/latest/dynamic/instance-identity/document', timeout=0.1)
+            r = requests.get('http://169.254.169.254/latest/dynamic/instance-identity/document', timeout=2.1)
         except RequestException:
-            logger.info("cannot query AWS meta-data")
+            logger.error('cannot query AWS meta-data')
             return
+
         if r.ok:
             try:
                 content = r.json()
                 self.instance_id = content['instanceId']
                 self.region = content['region']
-            except Exception as e:
-                logger.info('unable to fetch instance id and region from AWS meta-data: {}'.format(e))
+            except Exception:
+                logger.exception('unable to fetch instance id and region from AWS meta-data')
                 return
             self.available = True
 
diff --git a/tests/test_aws.py b/tests/test_aws.py
index 457aa1ad..dc297f08 100644
--- a/tests/test_aws.py
+++ b/tests/test_aws.py
@@ -1,83 +1,69 @@
 import boto.ec2
-import requests
 import sys
 import unittest
 
 from mock import Mock, patch
 from collections import namedtuple
 from patroni.scripts.aws import AWSConnection, main as _main
-from patroni.utils import RetryFailedError
 from requests.exceptions import RequestException
 
 
 class MockEc2Connection(object):
 
-    def __init__(self, error=False):
-        self.error = error
-
-    def get_all_volumes(self, filters):
-        if self.error:
-            raise boto.exception("get_all_volumes")
+    @staticmethod
+    def get_all_volumes(*args, **kwargs):
         oid = namedtuple('Volume', 'id')
         return [oid(id='a'), oid(id='b')]
 
-    def create_tags(self, objects, tags):
-        if self.error or len(objects) == 0:
-            raise boto.exception("create_tags")
+    @staticmethod
+    def create_tags(objects, *args, **kwargs):
+        if len(objects) == 0:
+            raise boto.exception.BotoServerError(503, 'Service Unavailable', 'Request limit exceeded')
         return True
 
 
 class MockResponse(object):
+    ok = True
 
     def __init__(self, content):
         self.content = content
-        self.ok = True
 
     def json(self):
         return self.content
 
 
+def requests_get(url, **kwargs):
+    if url.split('/')[-1] == 'document':
+        result = {"instanceId": "012345", "region": "eu-west-1"}
+    else:
+        result = 'foo'
+    return MockResponse(result)
+
+
+@patch('boto.ec2.connect_to_region', Mock(return_value=MockEc2Connection()))
 class TestAWSConnection(unittest.TestCase):
 
-    def boto_ec2_connect_to_region(self, region):
-        return MockEc2Connection(self.error)
-
-    def requests_get(self, url, **kwargs):
-        if self.error:
-            raise RequestException("foo")
-        result = namedtuple('Request', 'ok content')
-        result.ok = True
-        if url.split('/')[-1] == 'document' and not self.json_error:
-            result = {"instanceId": "012345", "region": "eu-west-1"}
-        else:
-            result = 'foo'
-        return MockResponse(result)
-
+    @patch('requests.get', requests_get)
     def setUp(self):
-        self.error = False
-        self.json_error = False
-        requests.get = self.requests_get
-        boto.ec2.connect_to_region = self.boto_ec2_connect_to_region
         self.conn = AWSConnection('test')
 
-    def test_aws_available(self):
-        self.assertTrue(self.conn.aws_available())
-
     def test_on_role_change(self):
         self.assertTrue(self.conn.on_role_change('master'))
-        self.conn.retry = Mock(side_effect=RetryFailedError("retry failed"))
-        self.assertFalse(self.conn.on_role_change('master'))
+        with patch.object(MockEc2Connection, 'get_all_volumes', Mock(return_value=[])):
+            self.conn._retry.max_tries = 1
+            self.assertFalse(self.conn.on_role_change('master'))
 
+    @patch('requests.get', Mock(side_effect=RequestException('foo')))
     def test_non_aws(self):
-        self.error = True
         conn = AWSConnection('test')
         self.assertFalse(conn.on_role_change("master"))
 
+    @patch('requests.get', Mock(return_value=MockResponse('foo')))
     def test_aws_bizare_response(self):
-        self.json_error = True
         conn = AWSConnection('test')
         self.assertFalse(conn.aws_available())
 
+    @patch('requests.get', requests_get)
     @patch('sys.exit', Mock())
     def test_main(self):
         self.assertIsNone(_main())