Repository: zalando/patroni
Pull request #1806 (2021-01-04 10:51:06)

Exit only if authentication explicitly failed
It could happen that one of etcd servers is not accessible on Patroni start.
In this case Patroni was trying to perform authentication and exiting, while it should exit only if Etcd explicitly responded with the
AuthFailed error.

Close https://github.com/zalando/patroni/issues/1805

FxKu: :+1: 

Patch:
diff --git a/.github/workflows/install_deps.py b/.github/workflows/install_deps.py
index 1a942fe1..d3af36bf 100644
--- a/.github/workflows/install_deps.py
+++ b/.github/workflows/install_deps.py
@@ -44,7 +44,11 @@ def install_packages(what):
 
 
 def get_file(url, name):
-    from six.moves.urllib.request import urlretrieve
+    try:
+        from urllib.request import urlretrieve
+    except ImportError:
+        from urllib import urlretrieve
+
     print('Downloading ' + url)
     urlretrieve(url, name)
 
@@ -119,7 +123,6 @@ def setup_kubernetes():
                      stdout=devnull, stderr=devnull)
     for _ in range(0, 120):
         if subprocess.call(['wget', '-qO', '-', 'http://127.0.0.1:8080/'], stdout=devnull, stderr=devnull) == 0:
-            time.sleep(10)
             break
         time.sleep(1)
     else:
@@ -156,22 +159,22 @@ users:
 
 def main():
     what = os.environ.get('DCS', sys.argv[1] if len(sys.argv) > 1 else 'all')
-    r = install_requirements(what)
-    if what == 'all' or r != 0:
-        return r
 
-    if sys.platform.startswith('linux'):
-        r = install_packages(what)
-    else:
-        r = install_postgres()
-    if r != 0:
-        return r
-
-    if what.startswith('etcd'):
-        return install_etcd()
-    elif what == 'kubernetes':
-        return setup_kubernetes()
-    return 0
+    if what != 'all':
+        if sys.platform.startswith('linux'):
+            r = install_packages(what)
+            if r == 0 and what == 'kubernetes':
+                r = setup_kubernetes()
+        else:
+            r = install_postgres()
+
+        if r == 0 and what.startswith('etcd'):
+            r = install_etcd()
+
+        if r != 0:
+            return r
+
+    return install_requirements(what)
 
 
 if __name__ == '__main__':
diff --git a/patroni/dcs/consul.py b/patroni/dcs/consul.py
index ba6799af..a9acb08b 100644
--- a/patroni/dcs/consul.py
+++ b/patroni/dcs/consul.py
@@ -8,6 +8,7 @@ import ssl
 import time
 import urllib3
 
+from collections import namedtuple
 from consul import ConsulException, NotFound, base
 from urllib3.exceptions import HTTPError
 from six.moves.urllib.parse import urlencode, urlparse, quote
@@ -36,6 +37,9 @@ class InvalidSession(ConsulException):
     """invalid session"""
 
 
+Response = namedtuple('Response', 'code,headers,body,content')
+
+
 class HTTPClient(object):
 
     def __init__(self, host='127.0.0.1', port=8500, token=None, scheme='http', verify=True, cert=None, ca_cert=None):
@@ -71,16 +75,17 @@ class HTTPClient(object):
 
     @staticmethod
     def response(response):
-        data = response.data.decode('utf-8')
+        content = response.data
+        body = content.decode('utf-8')
         if response.status == 500:
-            msg = '{0} {1}'.format(response.status, data)
-            if data.startswith('Invalid Session TTL'):
+            msg = '{0} {1}'.format(response.status, body)
+            if body.startswith('Invalid Session TTL'):
                 raise InvalidSessionTTL(msg)
-            elif data.startswith('invalid session'):
+            elif body.startswith('invalid session'):
                 raise InvalidSession(msg)
             else:
                 raise ConsulInternalError(msg)
-        return base.Response(response.status, response.headers, data)
+        return Response(response.status, response.headers, body, content)
 
     def uri(self, path, params=None):
         return '{0}{1}{2}'.format(self.base_uri, path, params and '?' + urlencode(params) or '')
@@ -89,7 +94,7 @@ class HTTPClient(object):
         if method not in ('get', 'post', 'put', 'delete'):
             raise AttributeError("HTTPClient instance has no attribute '{0}'".format(method))
 
-        def wrapper(callback, path, params=None, data=''):
+        def wrapper(callback, path, params=None, data='', headers=None):
             # python-consul doesn't allow to specify ttl smaller then 10 seconds
             # because session_ttl_min defaults to 10s, so we have to do this ugly dirty hack...
             if method == 'put' and path == '/v1/session/create':
@@ -110,8 +115,9 @@ class HTTPClient(object):
                 kwargs['timeout'] = timeout + max(timeout/15.0, 1)
             else:
                 kwargs['timeout'] = self._read_timeout
+            kwargs['headers'] = (headers or {}).copy()
+            kwargs['headers'].update(urllib3.make_headers(user_agent=USER_AGENT))
             token = params.pop('token', self.token) if isinstance(params, dict) else self.token
-            kwargs['headers'] = urllib3.make_headers(user_agent=USER_AGENT)
             if token:
                 kwargs['headers']['X-Consul-Token'] = token
             return callback(self.response(self.http.request(method.upper(), self.uri(path, params), **kwargs)))
@@ -126,7 +132,7 @@ class ConsulClient(base.Consul):
         self.token = kwargs.get('token')
         super(ConsulClient, self).__init__(*args, **kwargs)
 
-    def connect(self, *args, **kwargs):
+    def http_connect(self, *args, **kwargs):
         kwargs.update(dict(zip(['host', 'port', 'scheme', 'verify'], args)))
         if self._cert:
             kwargs['cert'] = self._cert
@@ -136,6 +142,9 @@ class ConsulClient(base.Consul):
             kwargs['token'] = self.token
         return HTTPClient(**kwargs)
 
+    def connect(self, *args, **kwargs):
+        return self.http_connect(*args, **kwargs)
+
     def reload_config(self, config):
         self.http.token = self.token = config.get('token')
         self.consistency = config.get('consistency', 'default')
diff --git a/patroni/dcs/etcd3.py b/patroni/dcs/etcd3.py
index 1e84d5ce..2d9c0656 100644
--- a/patroni/dcs/etcd3.py
+++ b/patroni/dcs/etcd3.py
@@ -98,6 +98,10 @@ class UserEmpty(InvalidArgument):
     error = "etcdserver: user name is empty"
 
 
+class AuthFailed(InvalidArgument):
+    error = "etcdserver: authentication failed, invalid user ID or password"
+
+
 class PermissionDenied(Etcd3ClientError):
     code = GRPCCode.PermissionDenied
     error = "etcdserver: permission denied"
@@ -186,7 +190,7 @@ class Etcd3Client(AbstractEtcdClientWithFailover):
 
         try:
             self.authenticate()
-        except Exception as e:
+        except AuthFailed as e:
             logger.fatal('Etcd3 authentication failed: %r', e)
             sys.exit(1)
 


Test patch:
diff --git a/.github/workflows/tests.yaml b/.github/workflows/tests.yaml
index aaa19332..ccc5f7ed 100644
--- a/.github/workflows/tests.yaml
+++ b/.github/workflows/tests.yaml
@@ -86,7 +86,7 @@ jobs:
         COVERALLS_FLAG_NAME: unit-${{ matrix.os }}
         COVERALLS_PARALLEL: 'true'
         GITHUB_TOKEN: ${{ secrets.github_token }}
-      run: python -m coveralls
+      run: python -m coveralls --service=github
 
     - name: Run codacy-coverage-reporter
       uses: codacy/codacy-coverage-reporter-action@master
@@ -129,7 +129,7 @@ jobs:
         COVERALLS_FLAG_NAME: behave-${{ matrix.os }}-${{ matrix.dcs }}-${{ matrix.python-version }}
         COVERALLS_PARALLEL: 'true'
         GITHUB_TOKEN: ${{ secrets.github_token }}
-      run: python -m coveralls
+      run: python -m coveralls --service=github
 
   behavem:
     runs-on: ${{ matrix.os }}-latest
@@ -161,7 +161,7 @@ jobs:
         COVERALLS_FLAG_NAME: behave-${{ matrix.os }}-${{ matrix.dcs }}-${{ matrix.python-version }}
         COVERALLS_PARALLEL: 'true'
         GITHUB_TOKEN: ${{ secrets.github_token }}
-      run: python -m coveralls
+      run: python -m coveralls --service=github
 
   coveralls-finish:
     name: Finalize coveralls.io
@@ -170,6 +170,6 @@ jobs:
     steps:
     - uses: actions/setup-python@v2
     - run: python -m pip install coveralls
-    - run: python -m coveralls --finish
+    - run: python -m coveralls --service=github --finish
       env:
         GITHUB_TOKEN: ${{ secrets.github_token }}
diff --git a/tests/test_consul.py b/tests/test_consul.py
index 6d0c4732..a38a2869 100644
--- a/tests/test_consul.py
+++ b/tests/test_consul.py
@@ -4,7 +4,7 @@ import unittest
 from consul import ConsulException, NotFound
 from mock import Mock, patch
 from patroni.dcs.consul import AbstractDCS, Cluster, Consul, ConsulInternalError, \
-                                ConsulError, HTTPClient, InvalidSessionTTL, InvalidSession
+                                ConsulError, ConsulClient, HTTPClient, InvalidSessionTTL, InvalidSession
 from . import SleepException
 
 
@@ -41,7 +41,8 @@ def kv_get(self, key, **kwargs):
 class TestHTTPClient(unittest.TestCase):
 
     def setUp(self):
-        self.client = HTTPClient('127.0.0.1', '8500', 'http', False)
+        c = ConsulClient()
+        self.client = c.http
         self.client.http.request = Mock()
 
     def test_get(self):
diff --git a/tests/test_etcd3.py b/tests/test_etcd3.py
index 6afef8f1..70f19e1d 100644
--- a/tests/test_etcd3.py
+++ b/tests/test_etcd3.py
@@ -5,7 +5,7 @@ import urllib3
 
 from mock import Mock, patch
 from patroni.dcs.etcd3 import PatroniEtcd3Client, Cluster, Etcd3, Etcd3Error, Etcd3ClientError, RetryFailedError,\
-        InvalidAuthToken, Unavailable, Unknown, UnsupportedEtcdVersion, UserEmpty, base64_encode
+        InvalidAuthToken, Unavailable, Unknown, UnsupportedEtcdVersion, UserEmpty, AuthFailed, base64_encode
 from threading import Thread
 
 from . import SleepException, MockResponse
@@ -90,7 +90,7 @@ class TestKVCache(BaseTestEtcd3):
 
 class TestPatroniEtcd3Client(BaseTestEtcd3):
 
-    @patch('patroni.dcs.etcd3.Etcd3Client.authenticate', Mock(side_effect=Exception))
+    @patch('patroni.dcs.etcd3.Etcd3Client.authenticate', Mock(side_effect=AuthFailed))
     def test__init__(self):
         self.assertRaises(SystemExit, self.setUp)
 
diff --git a/tests/test_raft.py b/tests/test_raft.py
index cd79bfc4..d1e245bc 100644
--- a/tests/test_raft.py
+++ b/tests/test_raft.py
@@ -86,8 +86,6 @@ class TestKVStoreTTL(unittest.TestCase):
         self.so.set('foo', 'bar')
         self.so.set('fooo', 'bar')
         self.assertFalse(self.so.delete('foo', prevValue='buz'))
-        self.assertFalse(self.so.delete('foo', prevValue='bar', timeout=0.00001))
-        self.assertFalse(self.so.delete('foo', prevValue='bar'))
         self.assertTrue(self.so.delete('foo', recursive=True))
         self.assertFalse(self.so.retry(self.so._delete, 'foo', prevValue=''))
 
@@ -99,10 +97,14 @@ class TestKVStoreTTL(unittest.TestCase):
 
     @patch('time.sleep', Mock())
     def test_retry(self):
-        return_values = [FAIL_REASON.QUEUE_FULL, FAIL_REASON.SUCCESS, FAIL_REASON.REQUEST_DENIED]
+        return_values = [FAIL_REASON.QUEUE_FULL] * 2 + [FAIL_REASON.SUCCESS, FAIL_REASON.REQUEST_DENIED]
 
         def test(callback):
             callback(True, return_values.pop(0))
+
+        with patch('time.time', Mock(side_effect=[1, 100])):
+            self.assertFalse(self.so.retry(test))
+
         self.assertTrue(self.so.retry(test))
         self.assertFalse(self.so.retry(test))