From f35d1098102f484846f7eb10a15678e61646e822 Mon Sep 17 00:00:00 2001
From: Alexander Kukushkin <alexander.kukushkin@zalando.de>
Date: Thu, 15 Oct 2015 16:17:11 +0200
Subject: [PATCH 01/28] Bugfix: do not try to double encode data

---
 patroni/zookeeper.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/patroni/zookeeper.py b/patroni/zookeeper.py
index 6f8ab981..9a2ee8bb 100644
--- a/patroni/zookeeper.py
+++ b/patroni/zookeeper.py
@@ -198,7 +198,7 @@ class ZooKeeper(AbstractDCS):
             self.client.retry(self.client.set, self.failover_path, value.encode('utf-8'), version=index or -1)
             return True
         except NoNodeError:
-            return value == '' or (not index and self._create(self.failover_path, value.encode('utf-8')))
+            return value == '' or (not index and self._create(self.failover_path, value))
         except:
             logging.exception('set_failover_value')
             return False

From 3ed82ae22c5f3ec7c22ad944338e4ea5c09ace3e Mon Sep 17 00:00:00 2001
From: Alexander Kukushkin <alexander.kukushkin@zalando.de>
Date: Thu, 15 Oct 2015 16:18:28 +0200
Subject: [PATCH 02/28] Manual failover via rest api

curl -XPOST --data '{"leader": "leader_name", "member": "member_name"}' http://127.0.0.1:8008/failover

It will execute some preliminary checks and write failover key into DCS.
Afterward it will wait until new leader key will appear in a DCS.

It's better to execute this request on the master node. It will send a
signal to the main HA loop which makes possible to release leader key
immidiately even if you are working with etcd.
---
 patroni/api.py    | 49 +++++++++++++++++++++++++++++++++++++++++++++++
 tests/test_api.py | 26 +++++++++++++++++++++++++
 2 files changed, 75 insertions(+)

diff --git a/patroni/api.py b/patroni/api.py
index dc83249f..f673d1c0 100644
--- a/patroni/api.py
+++ b/patroni/api.py
@@ -3,6 +3,7 @@ import fcntl
 import json
 import logging
 import psycopg2
+import time
 
 from patroni.exceptions import PostgresConnectionException
 from patroni.utils import Retry, RetryFailedError
@@ -121,6 +122,54 @@ class RestApiHandler(BaseHTTPRequestHandler):
         self.end_headers()
         self.wfile.write(data)
 
+    def poll_failover_result(self, leader, member):
+        for a in range(0, 15):
+            time.sleep(1)
+            try:
+                cluster = self.server.patroni.dcs.get_cluster()
+                if cluster.leader and cluster.leader.name != leader:
+                    return 200, ('Successfully failed over to ' + cluster.leader.name).encode('utf-8')
+            except:
+                pass
+        return 503, b'Failover failed'
+
+    def is_failover_possible(self, cluster, leader, member):
+        if leader and not cluster.leader or cluster.leader.name != leader:
+            return b'leader name does not match'
+        if member:
+            members = [m for m in cluster.members if m.name == member]
+            if not members:
+                return b'member does not exists'
+        else:
+            members = [m for m in cluster.members if m.name != cluster.leader.name and m.api_url]
+            if not members:
+                return b'failover is not possible: cluster does not have members except leader'
+        for member, reachable, in_recovery, xlog_location in self.server.patroni.ha.fetch_nodes_statuses(members):
+            if reachable:
+                return None
+        return b'failover is not possible: no good candidates have been found'
+
+    @check_auth
+    def do_POST_failover(self):
+        content_length = int(self.headers.get('content-length', 0))
+        request = json.loads(self.rfile.read(content_length).decode('utf-8'))
+        leader = request.get('leader', None)
+        member = request.get('member', None)
+        cluster = self.server.patroni.ha.dcs.get_cluster()
+        status_code = 503
+        data = self.is_failover_possible(cluster, leader, member)
+        if not data:
+            if not self.server.patroni.dcs.manual_failover(leader, member):
+                data = b'failed to write failover key into DCS'
+            else:
+                self.server.patroni.dcs.event.set()
+                status_code, data = self.poll_failover_result(cluster.leader and cluster.leader.name, member)
+
+        self.send_response(status_code)
+        self.send_header('Content-Type', 'text/html')
+        self.end_headers()
+        self.wfile.write(data)
+
     def parse_request(self):
         """Override parse_request method to enrich basic functionality of `BaseHTTPRequestHandler` class
 
diff --git a/tests/test_api.py b/tests/test_api.py
index e4b8f87e..aa4608c4 100644
--- a/tests/test_api.py
+++ b/tests/test_api.py
@@ -3,6 +3,7 @@ import unittest
 
 from mock import Mock, patch
 from patroni.api import RestApiHandler, RestApiServer
+from patroni.dcs import Member
 from six import BytesIO as IO
 from six.moves import BaseHTTPServer
 from test_postgresql import psycopg2_connect, MockCursor
@@ -38,6 +39,9 @@ class MockHa(Mock):
     def restart_scheduled(self):
         return False
 
+    def fetch_nodes_statuses(self, members):
+        return [[None, True, None, None]]
+
 
 class MockPatroni:
 
@@ -117,3 +121,25 @@ class TestRestApiHandler(unittest.TestCase):
             MockRestApiServer(RestApiHandler, b'GET /patroni')
         with patch.object(MockPostgresql, 'connection', Mock(side_effect=psycopg2.OperationalError)):
             MockRestApiServer(RestApiHandler, b'GET /patroni')
+
+    @patch('time.sleep', Mock())
+    @patch.object(MockHa, 'dcs')
+    def test_do_POST_failover(self, dcs):
+        cluster = dcs.get_cluster.return_value
+        request = b'POST /failover HTTP/1.0\nAuthorization: Basic dGVzdDp0ZXN0\n' +\
+                  b'Content-Length: 25\n\n{"leader": "postgresql1"}'
+        MockRestApiServer(RestApiHandler, request)
+        cluster.leader.name = 'postgresql1'
+        MockRestApiServer(RestApiHandler, request)
+        cluster.members = [Member(0, 'postgresql0', 30, {'api_url': 'http'})]
+        MockRestApiServer(RestApiHandler, request)
+        with patch.object(MockPatroni, 'dcs') as d:
+            d.get_cluster = Mock(side_effect=Exception())
+            MockRestApiServer(RestApiHandler, request)
+            d.manual_failover.return_value = False
+            MockRestApiServer(RestApiHandler, request)
+        with patch.object(MockHa, 'fetch_nodes_statuses', Mock(return_value=[])):
+            MockRestApiServer(RestApiHandler, request)
+        request = b'POST /failover HTTP/1.0\nAuthorization: Basic dGVzdDp0ZXN0\n' +\
+                  b'Content-Length: 50\n\n{"leader": "postgresql1", "member": "postgresql2"}'
+        MockRestApiServer(RestApiHandler, request)

From a844920489425496c4553fd174df6472e028859b Mon Sep 17 00:00:00 2001
From: Oleksii Kliukin <oleksii.kliukin@zalando.de>
Date: Fri, 16 Oct 2015 16:14:45 +0200
Subject: [PATCH 03/28] Store the cluster sysid in the initialize flag.

Make sure that the new PostgreSQL node will only
join the cluster if its sysid matches the one
stored in DCS.
---
 patroni/dcs.py        |  5 ++++-
 patroni/etcd.py       |  7 ++++---
 patroni/ha.py         | 20 +++++++++++++++-----
 patroni/postgresql.py |  8 ++++++++
 patroni/zookeeper.py  |  7 ++++---
 5 files changed, 35 insertions(+), 12 deletions(-)

diff --git a/patroni/dcs.py b/patroni/dcs.py
index 25e44cd1..48d49cf5 100644
--- a/patroni/dcs.py
+++ b/patroni/dcs.py
@@ -240,8 +240,11 @@ class AbstractDCS:
         overwriting the key if necessary."""
 
     @abc.abstractmethod
-    def initialize(self):
+    def initialize(self, create_new=True, sysid=None):
         """Race for cluster initialization.
+
+        :param create_new: False if the key should already exist (in the case we are setting the system_id)
+        :param sysid: PostgreSQL cluster system identifier, if specified, is written to the key
         :returns: `!True` if key has been created successfully.
 
         this method should create atomically initialize key and return `!True`
diff --git a/patroni/etcd.py b/patroni/etcd.py
index 79379700..c20d73ab 100644
--- a/patroni/etcd.py
+++ b/patroni/etcd.py
@@ -177,7 +177,8 @@ class Etcd(AbstractDCS):
             nodes = {os.path.relpath(node.key, result.key): node for node in result.leaves}
 
             # get initialize flag
-            initialize = bool(nodes.get(self._INITIALIZE, False))
+            initialize = nodes.get(self._INITIALIZE, None)
+            initialize = initialize and initialize.value
 
             # get last leader operation
             last_leader_operation = nodes.get(self._LEADER_OPTIME, None)
@@ -235,8 +236,8 @@ class Etcd(AbstractDCS):
         return self.retry(self.client.test_and_set, self.leader_path, self._name, self._name, self.ttl)
 
     @catch_etcd_errors
-    def initialize(self):
-        return self.retry(self.client.write, self.initialize_path, self._name, prevExist=False)
+    def initialize(self, create_new=True, sysid=None):
+        return self.retry(self.client.write, self.initialize_path, sysid or "", prevExist=(not create_new))
 
     @catch_etcd_errors
     def delete_leader(self):
diff --git a/patroni/ha.py b/patroni/ha.py
index 0019253d..82921a74 100644
--- a/patroni/ha.py
+++ b/patroni/ha.py
@@ -73,9 +73,10 @@ class Ha:
             self._async_executor.run_async(self.copy_backup_from_leader, args=(self.cluster.leader, ))
             return 'trying to bootstrap from leader'
         elif not self.cluster.initialize:  # no initialize key
-            if self.dcs.initialize():  # race for initialization
+            if self.dcs.initialize(create_new=True):  # race for initialization
                 try:
                     self.state_handler.bootstrap()
+                    self.dcs.initialize(create_new=False, sysid=self.state_handler.sysid)
                 except:  # initdb or start failed
                     # remove initialization key and give a chance to other members
                     logger.info("removing initialize key after failed attempt to initialize the cluster")
@@ -350,6 +351,11 @@ class Ha:
         else:
             return self._async_executor.scheduled_action + ' in progress'
 
+    def sysid_valid(self, sysid):
+        # sysid does tv_sec << 32, where tv_sec is the number of seconds sine 1970,
+        # so even 1 << 32 would have 10 digits.
+        return str(sysid) and len(str(sysid)) >= 10 and str(sysid).isdigit()
+
     def _run_cycle(self):
         try:
             self.load_cluster_from_dcs()
@@ -357,8 +363,8 @@ class Ha:
             self.touch_member()
 
             # cluster has leader key but not initialize key
-            if not self.cluster.is_unlocked() and not self.cluster.initialize:
-                self.dcs.initialize()  # fix it
+            if not self.cluster.is_unlocked() and not self.sysid_valid(self.cluster.initialize) and self.has_lock():
+                self.dcs.initialize(create_new=(self.cluster.initialize is None), sysid=self.state_handler.sysid)
 
             if self._async_executor.busy:
                 return self.handle_long_action_in_progress()
@@ -372,8 +378,12 @@ class Ha:
             if self.state_handler.data_directory_empty():
                 return self.bootstrap()  # new node
             # "bootstrap", but data directory is not empty
-            elif not self.cluster.initialize and self.cluster.is_unlocked():
-                self.dcs.initialize()
+            elif not self.sysid_valid(self.cluster.initialize) and self.cluster.is_unlocked():
+                self.dcs.initialize(create_new=(self.cluster.initialize is None), sysid=self.state_handler.sysid)
+            else:
+                # check if we are allowed to join
+                if self.sysid_valid(self.cluster.initialize) and self.cluster.initialize != self.state_handler.sysid:
+                    return "system ID mismatch, node {0} belongs to a different cluster".format(self.state_handler.name)
 
             # try to start dead postgres
             if not self.state_handler.is_healthy():
diff --git a/patroni/postgresql.py b/patroni/postgresql.py
index fc7956e9..07f62fdf 100644
--- a/patroni/postgresql.py
+++ b/patroni/postgresql.py
@@ -69,6 +69,7 @@ class Postgresql:
         self._connection = None
         self._cursor_holder = None
         self._need_rewind = False
+        self._sysid = None
         self.replication_slots = []  # list of already existing replication slots
         self.retry = Retry(max_tries=-1, deadline=5, max_delay=1, retry_exceptions=PostgresConnectionException)
 
@@ -105,6 +106,13 @@ class Postgresql:
                 data.get('Data page checksum version', '0') != '0'
         return False
 
+    @property
+    def sysid(self):
+        if not self._sysid:
+            data = self.controldata()
+            self._sysid = data and data.get('Database system identifier', None)
+        return self._sysid
+
     def require_rewind(self):
         self._need_rewind = True
 
diff --git a/patroni/zookeeper.py b/patroni/zookeeper.py
index 6f8ab981..d3e4fd57 100644
--- a/patroni/zookeeper.py
+++ b/patroni/zookeeper.py
@@ -139,7 +139,7 @@ class ZooKeeper(AbstractDCS):
             self.fetch_cluster = True
 
         # get initialize flag
-        initialize = self._INITIALIZE in nodes
+        initialize = self.get_node(self._INITIALIZE)[0] if self._INITIALIZE in nodes else None
 
         # get list of members
         members = self.load_members() if self._MEMBERS[:-1] in nodes else []
@@ -203,8 +203,9 @@ class ZooKeeper(AbstractDCS):
             logging.exception('set_failover_value')
             return False
 
-    def initialize(self):
-        return self._create(self.initialize_path, self._name, makepath=True)
+    def initialize(self, create_new=True, sysid=None):
+        return self._create(self.initialize_path, sysid if sysid else "", makepath=True) if create_new \
+            else self.client.retry(self.client.set, self.initialize_path,  sysid.encode("utf-8") if sysid else "")
 
     def touch_member(self, data, ttl=None):
         cluster = self.cluster

From 83662f71cba6c3af3fe7e6bdcbf1346585a3fc24 Mon Sep 17 00:00:00 2001
From: Oleksii Kliukin <oleksii.kliukin@zalando.de>
Date: Fri, 16 Oct 2015 16:38:05 +0200
Subject: [PATCH 04/28] Exit right away if the node sysid is different from the
 cluster's one

---
 patroni/ha.py | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/patroni/ha.py b/patroni/ha.py
index 82921a74..283d3678 100644
--- a/patroni/ha.py
+++ b/patroni/ha.py
@@ -2,6 +2,7 @@ import json
 import logging
 import psycopg2
 import requests
+import sys
 
 from patroni.async_executor import AsyncExecutor
 from patroni.exceptions import DCSError, PostgresConnectionException
@@ -383,7 +384,8 @@ class Ha:
             else:
                 # check if we are allowed to join
                 if self.sysid_valid(self.cluster.initialize) and self.cluster.initialize != self.state_handler.sysid:
-                    return "system ID mismatch, node {0} belongs to a different cluster".format(self.state_handler.name)
+                    logger.fatal("system ID mismatch, node {0} belongs to a different cluster".format(self.state_handler.name))
+                    sys.exit(1)
 
             # try to start dead postgres
             if not self.state_handler.is_healthy():

From a10b7248a6e92956b5d3201ae744f9f038bab60c Mon Sep 17 00:00:00 2001
From: Oleksii Kliukin <oleksii.kliukin@zalando.de>
Date: Mon, 19 Oct 2015 09:19:25 +0200
Subject: [PATCH 05/28] Fix a flake8 warning

---
 patroni/ha.py | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/patroni/ha.py b/patroni/ha.py
index 283d3678..8ed18b16 100644
--- a/patroni/ha.py
+++ b/patroni/ha.py
@@ -384,7 +384,8 @@ class Ha:
             else:
                 # check if we are allowed to join
                 if self.sysid_valid(self.cluster.initialize) and self.cluster.initialize != self.state_handler.sysid:
-                    logger.fatal("system ID mismatch, node {0} belongs to a different cluster".format(self.state_handler.name))
+                    logger.fatal("system ID mismatch, node {0} belongs to a different cluster".
+                                 format(self.state_handler.name))
                     sys.exit(1)
 
             # try to start dead postgres

From 4e448015f3fda0c80a5e59dde38633a82ca880d1 Mon Sep 17 00:00:00 2001
From: Oleksii Kliukin <oleksii.kliukin@zalando.de>
Date: Mon, 19 Oct 2015 10:13:14 +0200
Subject: [PATCH 06/28] Increase the test coverage.

---
 tests/test_ha.py         | 8 +++++++-
 tests/test_postgresql.py | 4 ++++
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/tests/test_ha.py b/tests/test_ha.py
index a5a816da..e34f9b8e 100644
--- a/tests/test_ha.py
+++ b/tests/test_ha.py
@@ -1,7 +1,7 @@
 import etcd
 import unittest
 
-from mock import Mock, patch
+from mock import Mock, MagicMock, patch
 from patroni.dcs import Cluster, Failover, Leader, Member
 from patroni.etcd import Client, Etcd
 from patroni.exceptions import DCSError, PostgresException
@@ -130,6 +130,12 @@ class TestHa(unittest.TestCase):
         self.ha.has_lock = true
         self.assertEquals(self.ha.run_cycle(), 'removed leader key after trying and failing to start postgres')
 
+    @patch('sys.exit', return_value=1)
+    @patch('patroni.ha.Ha.sysid_valid', MagicMock(return_value=True))
+    def test_sysid_no_match(self, exit_mock):
+        self.ha.run_cycle()
+        exit_mock.assert_called_once_with(1)
+
     @patch.object(Cluster, 'is_unlocked', Mock(return_value=False))
     def test_start_as_readonly(self):
         self.p.is_leader = self.p.is_healthy = false
diff --git a/tests/test_postgresql.py b/tests/test_postgresql.py
index 9a02ece6..544f8afd 100644
--- a/tests/test_postgresql.py
+++ b/tests/test_postgresql.py
@@ -429,3 +429,7 @@ class TestPostgresql(unittest.TestCase):
         self.p.cleanup_archive_status()
         mock_unlink.assert_not_called()
         mock_remove.assert_not_called()
+
+    @patch('subprocess.check_output', MagicMock(return_value=0, side_effect=pg_controldata_string))
+    def test_sysid(self):
+        self.assertEqual(self.p.sysid, "6200971513092291716")

From 18eebdadaa7ef40613d129981e3c9c532d3ef25c Mon Sep 17 00:00:00 2001
From: Alexander Kukushkin <alexander.kukushkin@zalando.de>
Date: Mon, 19 Oct 2015 15:00:06 +0200
Subject: [PATCH 07/28] Watch for change of failover key.

If the value is empty and leader didn't changed, this probably means
that failover failed.

After 15 seconds timeout we will consider failover status = unknown
---
 patroni/api.py    | 4 +++-
 tests/test_api.py | 4 ++++
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/patroni/api.py b/patroni/api.py
index f673d1c0..1fa5ca62 100644
--- a/patroni/api.py
+++ b/patroni/api.py
@@ -129,9 +129,11 @@ class RestApiHandler(BaseHTTPRequestHandler):
                 cluster = self.server.patroni.dcs.get_cluster()
                 if cluster.leader and cluster.leader.name != leader:
                     return 200, ('Successfully failed over to ' + cluster.leader.name).encode('utf-8')
+                if not cluster.failover:
+                    return 503, b'Failover failed'
             except:
                 pass
-        return 503, b'Failover failed'
+        return 503, b'Failover status unknown'
 
     def is_failover_possible(self, cluster, leader, member):
         if leader and not cluster.leader or cluster.leader.name != leader:
diff --git a/tests/test_api.py b/tests/test_api.py
index aa4608c4..ae25964d 100644
--- a/tests/test_api.py
+++ b/tests/test_api.py
@@ -134,6 +134,10 @@ class TestRestApiHandler(unittest.TestCase):
         cluster.members = [Member(0, 'postgresql0', 30, {'api_url': 'http'})]
         MockRestApiServer(RestApiHandler, request)
         with patch.object(MockPatroni, 'dcs') as d:
+            cluster = d.get_cluster.return_value
+            cluster.leader.name = 'postgresql1'
+            cluster.failover = None
+            MockRestApiServer(RestApiHandler, request)
             d.get_cluster = Mock(side_effect=Exception())
             MockRestApiServer(RestApiHandler, request)
             d.manual_failover.return_value = False

From 8f606e4ff9a85f6f6d76db744a48880ec940f827 Mon Sep 17 00:00:00 2001
From: Oleksii Kliukin <oleksii.kliukin@zalando.de>
Date: Mon, 19 Oct 2015 15:13:24 +0200
Subject: [PATCH 08/28] Add a missing call to restore_configuration_files.

I accidentially removed the call when moving the backup functions
to the external script. It is intended to save the configuration,
so that at the restore phase one can just copy backup files.
Its primary intention was to save configuration files in the WAL-E
case (WAL-E just omits everything with .conf), but it is also
useful in the pg_basebackup case, which omits all symlinks, leaving
the cluster with .conf files symlinked in the broken state.
---
 patroni/postgresql.py    | 17 +++++++++++------
 tests/test_postgresql.py | 17 +++++++++++++++++
 2 files changed, 28 insertions(+), 6 deletions(-)

diff --git a/patroni/postgresql.py b/patroni/postgresql.py
index f75324e7..b4e8ff94 100644
--- a/patroni/postgresql.py
+++ b/patroni/postgresql.py
@@ -485,19 +485,23 @@ recovery_target_timeline = 'latest'
 
     def save_configuration_files(self):
         """
-            copy postgresql.conf to postgresql.conf.backup to preserve it in the WAL-e backup.
-            see http://comments.gmane.org/gmane.comp.db.postgresql.wal-e/239
+            copy postgresql.conf to postgresql.conf.backup to be able to retrive configuration files
+            - originally stored as symlinks, those are normally skipped by pg_basebackup
+            - in case of WAL-E basebackup (see http://comments.gmane.org/gmane.comp.db.postgresql.wal-e/239)
         """
-        for f in self.configuration_to_save:
-            shutil.copy(f, f + '.backup')
+        try:
+            for f in self.configuration_to_save:
+                os.path.isfile(f) and shutil.copy(f, f + '.backup')
+        except:
+            logger.exception('unable to create backup copies of configuration files')
 
     def restore_configuration_files(self):
         """ restore a previously saved postgresql.conf """
         try:
             for f in self.configuration_to_save:
-                shutil.copy(f + '.backup', f)
+                not os.path.isfile(f) and os.path.isfile(f+'.backup') and shutil.copy(f + '.backup', f)
         except:
-            logger.exception('unable to restore configuration from WAL-E backup')
+            logger.exception('unable to restore configuration files from backup')
 
     def promote(self):
         if self.role == 'master':
@@ -585,6 +589,7 @@ recovery_target_timeline = 'latest'
                 raise PostgresException("Could not bootstrap master PostgreSQL")
         else:
             if self.sync_from_leader(current_leader):
+                self.restore_configuration_files()
                 self.write_recovery_conf(current_leader)
                 ret = self.start()
         return ret
diff --git a/tests/test_postgresql.py b/tests/test_postgresql.py
index 9a02ece6..ca60760f 100644
--- a/tests/test_postgresql.py
+++ b/tests/test_postgresql.py
@@ -19,6 +19,11 @@ from test_ha import false
 import subprocess
 
 
+def is_file_raise_on_backup(*args, **kwargs):
+    if args[0].endswith('.backup'):
+        raise Exception("foo")
+
+
 class MockCursor:
 
     def __init__(self, connection):
@@ -429,3 +434,15 @@ class TestPostgresql(unittest.TestCase):
         self.p.cleanup_archive_status()
         mock_unlink.assert_not_called()
         mock_remove.assert_not_called()
+
+    @patch('os.path.isfile', MagicMock(return_value=True))
+    @patch('shutil.copy', side_effect=Exception)
+    def test_save_configuration_files(self, mock_copy):
+        shutil.copy = mock_copy
+        self.p.save_configuration_files()
+
+    @patch('os.path.isfile', MagicMock(side_effect=is_file_raise_on_backup))
+    @patch('shutil.copy', side_effect=Exception)
+    def test_restore_configuration_files(self, mock_copy):
+        shutil.copy = mock_copy
+        self.p.restore_configuration_files()

From 90c738d83a4897f1292d38c59e99a4e80945b578 Mon Sep 17 00:00:00 2001
From: Oleksii Kliukin <oleksii.kliukin@zalando.de>
Date: Mon, 19 Oct 2015 16:03:21 +0200
Subject: [PATCH 09/28] Address the code review by Alex.

---
 patroni/etcd.py          | 4 ++--
 patroni/postgresql.py    | 9 +++------
 patroni/zookeeper.py     | 8 ++++----
 tests/test_postgresql.py | 2 +-
 4 files changed, 10 insertions(+), 13 deletions(-)

diff --git a/patroni/etcd.py b/patroni/etcd.py
index c20d73ab..93ef2a2c 100644
--- a/patroni/etcd.py
+++ b/patroni/etcd.py
@@ -236,8 +236,8 @@ class Etcd(AbstractDCS):
         return self.retry(self.client.test_and_set, self.leader_path, self._name, self._name, self.ttl)
 
     @catch_etcd_errors
-    def initialize(self, create_new=True, sysid=None):
-        return self.retry(self.client.write, self.initialize_path, sysid or "", prevExist=(not create_new))
+    def initialize(self, create_new=True, sysid=""):
+        return self.retry(self.client.write, self.initialize_path, sysid, prevExist=(not create_new))
 
     @catch_etcd_errors
     def delete_leader(self):
diff --git a/patroni/postgresql.py b/patroni/postgresql.py
index 165057e6..9ac0d69a 100644
--- a/patroni/postgresql.py
+++ b/patroni/postgresql.py
@@ -101,16 +101,13 @@ class Postgresql:
             return False
         # check if the cluster's configuration permits pg_rewind
         data = self.controldata()
-        if data:
-            return data.get('wal_log_hints setting', 'off') == 'on' or\
-                data.get('Data page checksum version', '0') != '0'
-        return False
+        return data.get('wal_log_hints setting', 'off') == 'on' or data.get('Data page checksum version', '0') != '0'
 
     @property
     def sysid(self):
         if not self._sysid:
             data = self.controldata()
-            self._sysid = data and data.get('Database system identifier', None)
+            self._sysid = data.get('Database system identifier', "")
         return self._sysid
 
     def require_rewind(self):
@@ -399,7 +396,7 @@ recovery_target_timeline = 'latest'
         try:
             data = subprocess.check_output(['pg_controldata', self.data_dir])
             if data:
-                data = data.splitlines()
+                data = data.decode().splitlines()
                 result = {l.split(':')[0].replace('Current ', '', 1): l.split(':')[1].strip() for l in data if l}
         except subprocess.CalledProcessError:
             logger.exception("Error when calling pg_controldata")
diff --git a/patroni/zookeeper.py b/patroni/zookeeper.py
index d3e4fd57..ba31e756 100644
--- a/patroni/zookeeper.py
+++ b/patroni/zookeeper.py
@@ -139,7 +139,7 @@ class ZooKeeper(AbstractDCS):
             self.fetch_cluster = True
 
         # get initialize flag
-        initialize = self.get_node(self._INITIALIZE)[0] if self._INITIALIZE in nodes else None
+        initialize = self.get_node(self.initialize_path)[0] if self._INITIALIZE in nodes else None
 
         # get list of members
         members = self.load_members() if self._MEMBERS[:-1] in nodes else []
@@ -203,9 +203,9 @@ class ZooKeeper(AbstractDCS):
             logging.exception('set_failover_value')
             return False
 
-    def initialize(self, create_new=True, sysid=None):
-        return self._create(self.initialize_path, sysid if sysid else "", makepath=True) if create_new \
-            else self.client.retry(self.client.set, self.initialize_path,  sysid.encode("utf-8") if sysid else "")
+    def initialize(self, create_new=True, sysid=""):
+        return self._create(self.initialize_path, sysid, makepath=True) if create_new \
+            else self.client.retry(self.client.set, self.initialize_path,  sysid.encode("utf-8"))
 
     def touch_member(self, data, ttl=None):
         cluster = self.cluster
diff --git a/tests/test_postgresql.py b/tests/test_postgresql.py
index 544f8afd..c4628135 100644
--- a/tests/test_postgresql.py
+++ b/tests/test_postgresql.py
@@ -86,7 +86,7 @@ class MockConnect(Mock):
 
 
 def pg_controldata_string(*args, **kwargs):
-    return """
+    return b"""
 pg_control version number:            942
 Catalog version number:               201509161
 Database system identifier:           6200971513092291716

From 40c5d5e3516b225ffddb3cfd14bc6673004d960e Mon Sep 17 00:00:00 2001
From: Oleksii Kliukin <oleksii.kliukin@zalando.de>
Date: Mon, 19 Oct 2015 16:08:52 +0200
Subject: [PATCH 10/28] Match default param in the abstract class definition
 with those from the implementation.

---
 patroni/dcs.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/patroni/dcs.py b/patroni/dcs.py
index 48d49cf5..a8afda06 100644
--- a/patroni/dcs.py
+++ b/patroni/dcs.py
@@ -240,7 +240,7 @@ class AbstractDCS:
         overwriting the key if necessary."""
 
     @abc.abstractmethod
-    def initialize(self, create_new=True, sysid=None):
+    def initialize(self, create_new=True, sysid=""):
         """Race for cluster initialization.
 
         :param create_new: False if the key should already exist (in the case we are setting the system_id)

From 92fe6a1de9c05dfc9946ec66691529a55af1571f Mon Sep 17 00:00:00 2001
From: Oleksii Kliukin <oleksii.kliukin@zalando.de>
Date: Tue, 20 Oct 2015 11:28:26 +0200
Subject: [PATCH 11/28] Make pgpass location configurable.

One can use pgpass configuration parameter in the postgres
subsection of Patroni. By default pgpass is written in ~/.
Mock actual writes to pgpass in the tests.
---
 patroni/postgresql.py    | 9 ++++++---
 tests/test_postgresql.py | 8 ++++++++
 2 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/patroni/postgresql.py b/patroni/postgresql.py
index f75324e7..81758909 100644
--- a/patroni/postgresql.py
+++ b/patroni/postgresql.py
@@ -48,6 +48,7 @@ class Postgresql:
         self.replication = config['replication']
         self.superuser = config['superuser']
         self.admin = config['admin']
+        self.pgpass = config.get('pgpass', None)
         self.pg_rewind = config.get('pg_rewind', {})
         self.callback = config.get('callbacks', {})
         self.use_slots = config.get('use_slots', True)
@@ -171,12 +172,14 @@ class Postgresql:
         os.path.exists(self.trigger_file) and os.unlink(self.trigger_file)
 
     def write_pgpass(self, record):
-        pgpass = 'pgpass'
-        with open(pgpass, 'w') as f:
+        self.pgpass = self.pgpass or os.path.join(os.path.expanduser('~'), 'pgpass')
+
+        with open(self.pgpass, 'w') as f:
             os.fchmod(f.fileno(), 0o600)
             f.write('{host}:{port}:*:{user}:{password}\n'.format(**record))
+
         env = os.environ.copy()
-        env['PGPASSFILE'] = pgpass
+        env['PGPASSFILE'] = self.pgpass
         return env
 
     def sync_from_leader(self, leader):
diff --git a/tests/test_postgresql.py b/tests/test_postgresql.py
index 9a02ece6..5bd09e13 100644
--- a/tests/test_postgresql.py
+++ b/tests/test_postgresql.py
@@ -210,10 +210,16 @@ class TestPostgresql(unittest.TestCase):
         self.assertFalse(self.p.restart())
         self.assertEquals(self.p.state, 'restart failed (restarting)')
 
+    @patch.object(builtins, 'open', MagicMock())
+    def test_write_pgpass(self):
+        self.p.write_pgpass({'host': 'localhost', 'port': '5432', 'user': 'foo', 'password': 'bar'})
+
+    @patch('patroni.postgresql.Postgresql.write_pgpass', MagicMock(return_value=dict()))
     def test_sync_from_leader(self):
         self.assertTrue(self.p.sync_from_leader(self.leader))
 
     @patch('subprocess.call', side_effect=Exception("Test"))
+    @patch('patroni.postgresql.Postgresql.write_pgpass', MagicMock(return_value=dict()))
     def test_pg_rewind(self, mock_call):
         self.assertTrue(self.p.rewind(self.leader))
         subprocess.call = mock_call
@@ -222,6 +228,7 @@ class TestPostgresql(unittest.TestCase):
     @patch('patroni.postgresql.Postgresql.rewind', return_value=False)
     @patch('patroni.postgresql.Postgresql.remove_data_directory', MagicMock(return_value=True))
     @patch('patroni.postgresql.Postgresql.single_user_mode', MagicMock(return_value=1))
+    @patch('patroni.postgresql.Postgresql.write_pgpass', MagicMock(return_value=dict()))
     def test_follow_the_leader(self, mock_pg_rewind):
         self.p.demote()
         self.p.follow_the_leader(None)
@@ -327,6 +334,7 @@ class TestPostgresql(unittest.TestCase):
         with patch('os.rename', Mock(side_effect=OSError())):
             self.p.move_data_directory()
 
+    @patch('patroni.postgresql.Postgresql.write_pgpass', MagicMock(return_value=dict()))
     def test_bootstrap(self):
         with patch('subprocess.call', Mock(return_value=1)):
             self.assertRaises(PostgresException, self.p.bootstrap)

From 35641ac0727f04f9527333a45a76e7ded1f55734 Mon Sep 17 00:00:00 2001
From: Oleksii Kliukin <oleksii.kliukin@zalando.de>
Date: Tue, 20 Oct 2015 11:40:52 +0200
Subject: [PATCH 12/28] Use distinct paths for pgpass from test nodes.

---
 postgres0.yml | 1 +
 postgres1.yml | 1 +
 2 files changed, 2 insertions(+)

diff --git a/postgres0.yml b/postgres0.yml
index a155b1cd..8747a3af 100644
--- a/postgres0.yml
+++ b/postgres0.yml
@@ -34,6 +34,7 @@ postgresql:
   data_dir: data/postgresql0
   maximum_lag_on_failover: 1048576 # 1 megabyte in bytes
   use_slots: True
+  pgpass: /tmp/pgpass0
   pg_rewind:
     username: postgres
     password: zalando
diff --git a/postgres1.yml b/postgres1.yml
index 94e33a42..dcf2f0cf 100644
--- a/postgres1.yml
+++ b/postgres1.yml
@@ -34,6 +34,7 @@ postgresql:
   data_dir: data/postgresql1
   maximum_lag_on_failover: 1048576 # 1 megabyte in bytes
   use_slots: True
+  pgpass: /tmp/pgpass1
   pg_rewind:
     username: postgres
     password: zalando

From f53c968d8b17fc526c0883af70dfe99797dbb1ca Mon Sep 17 00:00:00 2001
From: Alexander Kukushkin <alexander.kukushkin@zalando.de>
Date: Tue, 20 Oct 2015 14:36:49 +0200
Subject: [PATCH 13/28] Improve tests

---
 tests/test_api.py | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tests/test_api.py b/tests/test_api.py
index ae25964d..72d5c2b0 100644
--- a/tests/test_api.py
+++ b/tests/test_api.py
@@ -135,6 +135,8 @@ class TestRestApiHandler(unittest.TestCase):
         MockRestApiServer(RestApiHandler, request)
         with patch.object(MockPatroni, 'dcs') as d:
             cluster = d.get_cluster.return_value
+            cluster.leader.name = 'postgresql0'
+            MockRestApiServer(RestApiHandler, request)
             cluster.leader.name = 'postgresql1'
             cluster.failover = None
             MockRestApiServer(RestApiHandler, request)

From 5d7e4fe90afd03bc9be1a92328890fa16dc8ea3d Mon Sep 17 00:00:00 2001
From: Dr Nic Williams <drnicwilliams@gmail.com>
Date: Tue, 20 Oct 2015 14:32:59 -0500
Subject: [PATCH 14/28] allow $PATRONI_SCOPE to be set via 'docker run -e
 PATRONI_SCOPE=ironman'

---
 docker/entrypoint.sh | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/docker/entrypoint.sh b/docker/entrypoint.sh
index f4851a36..d94757b3 100755
--- a/docker/entrypoint.sh
+++ b/docker/entrypoint.sh
@@ -3,25 +3,25 @@
 function usage()
 {
     cat <<__EOF__
-Usage: $0 
+Usage: $0
 
 Options:
 
     --etcd      ETCD    Provide an external etcd to connect to
-    --name      NAME    Give the cluster a specific name 
+    --name      NAME    Give the cluster a specific name
     --etcd-only         Do not run Patroni, run a standalone etcd
 
 Examples:
 
     $0 --etcd=127.17.0.84:4001
     $0 --etcd-only
-    $0 
+    $0
     $0 --name=true_scotsman
 __EOF__
 }
 
 DOCKER_IP=$(hostname --ip-address)
-PATRONI_SCOPE=batman
+PATRONI_SCOPE=${PATRONI_SCOPE:-batman}
 
 optspec=":vh-:"
 while getopts "$optspec" optchar; do
@@ -32,7 +32,7 @@ while getopts "$optspec" optchar; do
                     exec etcd --data-dir /tmp/etcd.data \
                         -advertise-client-urls=http://${DOCKER_IP}:4001 \
                         -listen-client-urls=http://0.0.0.0:4001 \
-                        -listen-peer-urls=http://0.0.0.0:2380 
+                        -listen-peer-urls=http://0.0.0.0:2380
                     exit 0
                     ;;
                 cheat)

From 0096b6b06fdb76d9b616fe08dcfa7e00f79b2c57 Mon Sep 17 00:00:00 2001
From: Alexander Kukushkin <alexander.kukushkin@zalando.de>
Date: Wed, 21 Oct 2015 10:56:43 +0200
Subject: [PATCH 15/28] Schedule update of machines cache when api_execute call
 has failed

Such situation could happen if we replaced all etcd nodes except one
which was used by patroni. After replacing the last node patroni will
try to execute request on all other nodes from machines_cache but non of
them are available. Michines cache would became empty and patroni will
stick to the latest node which was available in the machines_cache and
will never try to refresh machines_cache from dns for example.

Currently machines cache is refreshed only when one request to the etcd
cluster has failed, but probably it should be done periodically, for
example every minute...
---
 patroni/etcd.py    |  6 +++++-
 tests/test_etcd.py | 14 ++++++++++----
 2 files changed, 15 insertions(+), 5 deletions(-)

diff --git a/patroni/etcd.py b/patroni/etcd.py
index 79379700..622eb747 100644
--- a/patroni/etcd.py
+++ b/patroni/etcd.py
@@ -52,7 +52,11 @@ class Client(etcd.Client):
     def api_execute(self, path, method, **kwargs):
         # Update machines_cache if previous attempt of update has failed
         self._update_machines_cache and self._load_machines_cache()
-        return super(Client, self).api_execute(path, method, **kwargs)
+        try:
+            return super(Client, self).api_execute(path, method, **kwargs)
+        except etcd.EtcdConnectionFailed:
+            self._update_machines_cache = True
+            raise
 
     @staticmethod
     def get_srv_record(host):
diff --git a/tests/test_etcd.py b/tests/test_etcd.py
index 53c054e5..6cd5441e 100644
--- a/tests/test_etcd.py
+++ b/tests/test_etcd.py
@@ -106,13 +106,13 @@ def etcd_read(key, **kwargs):
                  "modifiedIndex": 20437, "createdIndex": 20437},
                 {"key": "/service/batman5/members", "dir": True, "nodes": [
                     {"key": "/service/batman5/members/postgresql1",
-                     "value": "postgres://replicator:rep-pass@127.0.0.1:5434/postgres"
-                        + "?application_name=http://127.0.0.1:8009/patroni",
+                     "value": "postgres://replicator:rep-pass@127.0.0.1:5434/postgres" +
+                        "?application_name=http://127.0.0.1:8009/patroni",
                      "expiration": "2015-05-15T09:10:59.949384522Z", "ttl": 21,
                      "modifiedIndex": 20727, "createdIndex": 20727},
                     {"key": "/service/batman5/members/postgresql0",
-                     "value": "postgres://replicator:rep-pass@127.0.0.1:5433/postgres"
-                        + "?application_name=http://127.0.0.1:8008/patroni",
+                     "value": "postgres://replicator:rep-pass@127.0.0.1:5433/postgres" +
+                        "?application_name=http://127.0.0.1:8008/patroni",
                      "expiration": "2015-05-15T09:11:09.611860899Z", "ttl": 30,
                      "modifiedIndex": 20730, "createdIndex": 20730}],
                  "modifiedIndex": 1581, "createdIndex": 1581}], "modifiedIndex": 1581, "createdIndex": 1581}}
@@ -143,6 +143,7 @@ def socket_getaddrinfo(*args):
 
 
 def http_request(method, url, **kwargs):
+    print('http_request', method, url, kwargs)
     if url == 'http://localhost:2379/':
         return MockResponse()
     raise socket.error
@@ -165,6 +166,11 @@ class TestClient(unittest.TestCase):
         self.client._base_uri = 'http://localhost:4001'
         self.client._machines_cache = ['http://localhost:2379']
         self.client.api_execute('/', 'GET')
+        self.client._update_machines_cache = False
+        self.client._base_uri = 'http://localhost:4001'
+        self.client._machines_cache = []
+        self.assertRaises(etcd.EtcdConnectionFailed, self.client.api_execute, '/', 'GET')
+        self.assertTrue(self.client._update_machines_cache)
 
     def test_get_srv_record(self):
         self.assertEquals(self.client.get_srv_record('blabla'), [])

From 8bd28507a93310cb8b37d830822900ce1d1417f2 Mon Sep 17 00:00:00 2001
From: Alexander Kukushkin <alexander.kukushkin@zalando.de>
Date: Wed, 21 Oct 2015 11:08:06 +0200
Subject: [PATCH 16/28] format tests according to the latest pep8 standards

---
 tests/test_postgresql.py | 15 +++++----------
 tests/test_utils.py      |  4 +++-
 2 files changed, 8 insertions(+), 11 deletions(-)

diff --git a/tests/test_postgresql.py b/tests/test_postgresql.py
index 9a02ece6..cc781fc8 100644
--- a/tests/test_postgresql.py
+++ b/tests/test_postgresql.py
@@ -4,12 +4,7 @@ import psycopg2
 import shutil
 import unittest
 
-from sys import version_info
-if version_info.major == 2:
-    import __builtin__ as builtins
-else:
-    import builtins
-
+from six.moves import builtins
 from mock import Mock, MagicMock, PropertyMock, patch, mock_open
 from patroni.dcs import Cluster, Leader, Member
 from patroni.exceptions import PostgresException, PostgresConnectionException
@@ -141,10 +136,10 @@ Data page checksum version:           0
 
 
 def postmaster_opts_string(*args, **kwargs):
-    return '/usr/local/pgsql/bin/postgres "-D" "data/postgresql0" "--listen_addresses=127.0.0.1" "--port=5432"'\
-        ' "--hot_standby=on" "--wal_keep_segments=8" "--wal_level=hot_standby" "--archive_command=mkdir -p ../wal_archive \n'\
-        '&& cp %p ../wal_archive/%f" "--wal_log_hints=on" "--max_wal_senders=5" "--archive_timeout=1800s" "--archive_mode=on"'\
-        ' "--max_replication_slots=5"\n'
+    return '/usr/local/pgsql/bin/postgres "-D" "data/postgresql0" "--listen_addresses=127.0.0.1" \
+"--port=5432" "--hot_standby=on" "--wal_keep_segments=8" "--wal_level=hot_standby" \
+"--archive_command=mkdir -p ../wal_archive && cp %p ../wal_archive/%f" "--wal_log_hints=on" \
+"--max_wal_senders=5" "--archive_timeout=1800s" "--archive_mode=on" "--max_replication_slots=5"\n'
 
 
 def psycopg2_connect(*args, **kwargs):
diff --git a/tests/test_utils.py b/tests/test_utils.py
index 45b194dc..265f98ab 100644
--- a/tests/test_utils.py
+++ b/tests/test_utils.py
@@ -67,7 +67,9 @@ class TestRetrySleeper(unittest.TestCase):
         self.assertRaises(RetryFailedError, retry, self._fail(times=100))
 
     def test_copy(self):
-        _sleep = lambda t: None
+        def _sleep(t):
+            None
+
         retry = self._makeOne(sleep_func=_sleep)
         rcopy = retry.copy()
         self.assertTrue(rcopy.sleep_func is _sleep)

From c4a6dd48d34b490971edc77e87bd466ad95d8988 Mon Sep 17 00:00:00 2001
From: Alexander Kukushkin <alexander.kukushkin@zalando.de>
Date: Wed, 21 Oct 2015 11:09:37 +0200
Subject: [PATCH 17/28] remove debug print statement

---
 tests/test_etcd.py | 1 -
 1 file changed, 1 deletion(-)

diff --git a/tests/test_etcd.py b/tests/test_etcd.py
index 6cd5441e..d0d01d71 100644
--- a/tests/test_etcd.py
+++ b/tests/test_etcd.py
@@ -143,7 +143,6 @@ def socket_getaddrinfo(*args):
 
 
 def http_request(method, url, **kwargs):
-    print('http_request', method, url, kwargs)
     if url == 'http://localhost:2379/':
         return MockResponse()
     raise socket.error

From 44a73982d4dda64618345142f0a3381aaafa539a Mon Sep 17 00:00:00 2001
From: Oleksii Kliukin <oleksii.kliukin@zalando.de>
Date: Wed, 21 Oct 2015 12:00:03 +0200
Subject: [PATCH 18/28] Do not try to fetch the element from the get_node
 result if the node is not there.

---
 patroni/zookeeper.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/patroni/zookeeper.py b/patroni/zookeeper.py
index ba31e756..4cb73e0f 100644
--- a/patroni/zookeeper.py
+++ b/patroni/zookeeper.py
@@ -139,7 +139,7 @@ class ZooKeeper(AbstractDCS):
             self.fetch_cluster = True
 
         # get initialize flag
-        initialize = self.get_node(self.initialize_path)[0] if self._INITIALIZE in nodes else None
+        initialize = (self.get_node(self.initialize_path) or [None])[0] if self._INITIALIZE in nodes else None
 
         # get list of members
         members = self.load_members() if self._MEMBERS[:-1] in nodes else []

From 9130891029076f3bf0aadb3e54bdf829bcc4deec Mon Sep 17 00:00:00 2001
From: Oleksii Kliukin <oleksii.kliukin@zalando.de>
Date: Wed, 21 Oct 2015 13:06:54 +0200
Subject: [PATCH 19/28] Move calculation of pgpass to the class constructor:
 better to fail fast in case of issues.

---
 patroni/postgresql.py | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/patroni/postgresql.py b/patroni/postgresql.py
index 81758909..44406fe2 100644
--- a/patroni/postgresql.py
+++ b/patroni/postgresql.py
@@ -48,7 +48,7 @@ class Postgresql:
         self.replication = config['replication']
         self.superuser = config['superuser']
         self.admin = config['admin']
-        self.pgpass = config.get('pgpass', None)
+        self.pgpass = config.get('pgpass', None) or os.path.join(os.path.expanduser('~'), 'pgpass')
         self.pg_rewind = config.get('pg_rewind', {})
         self.callback = config.get('callbacks', {})
         self.use_slots = config.get('use_slots', True)
@@ -172,8 +172,6 @@ class Postgresql:
         os.path.exists(self.trigger_file) and os.unlink(self.trigger_file)
 
     def write_pgpass(self, record):
-        self.pgpass = self.pgpass or os.path.join(os.path.expanduser('~'), 'pgpass')
-
         with open(self.pgpass, 'w') as f:
             os.fchmod(f.fileno(), 0o600)
             f.write('{host}:{port}:*:{user}:{password}\n'.format(**record))

From 2c7e3f60cc249e8c34f92e85ab3829e433ca5fbc Mon Sep 17 00:00:00 2001
From: Alexander Kukushkin <alexander.kukushkin@zalando.de>
Date: Wed, 21 Oct 2015 15:34:55 +0200
Subject: [PATCH 20/28] Make possible to override default namespace (/service/)
 from a config file

If the namespace is not specified in a config file /service/ would be
used.
Also it's possible to use just '/' as a namespace. It means we would
have following structure:
  /scope1
  /scope2
  ...
---
 patroni/dcs.py     | 4 ++--
 tests/test_etcd.py | 7 +++++--
 2 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/patroni/dcs.py b/patroni/dcs.py
index a8afda06..bb56fecc 100644
--- a/patroni/dcs.py
+++ b/patroni/dcs.py
@@ -122,8 +122,8 @@ class AbstractDCS:
             i.e.: `zookeeper` for zookeeper, `etcd` for etcd, etc...
         """
         self._name = name
-        self._scope = config['scope']
-        self._base_path = '/service/' + self._scope
+        self._namespace = '/{}'.format(config.get('namespace', '/service/').strip('/'))
+        self._base_path = '/'.join([self._namespace, config['scope']])
 
         self._cluster = None
         self._cluster_thread_lock = Lock()
diff --git a/tests/test_etcd.py b/tests/test_etcd.py
index d0d01d71..6ffa59af 100644
--- a/tests/test_etcd.py
+++ b/tests/test_etcd.py
@@ -80,7 +80,7 @@ def etcd_watch(key, index=None, timeout=None, recursive=None):
 def etcd_write(key, value, **kwargs):
     if key == '/service/exists/leader':
         raise etcd.EtcdAlreadyExist
-    if key == '/service/test/leader':
+    if key == '/service/test/leader' or key == '/patroni/test/leader':
         if kwargs.get('prevValue', None) == 'foo' or not kwargs.get('prevExist', True):
             return True
     raise etcd.EtcdException
@@ -204,11 +204,14 @@ class TestEtcd(unittest.TestCase):
     def setUp(self):
         with patch.object(Client, 'machines') as mock_machines:
             mock_machines.__get__ = Mock(return_value=['http://localhost:2379', 'http://localhost:4001'])
-            self.etcd = Etcd('foo', {'ttl': 30, 'host': 'localhost:2379', 'scope': 'test'})
+            self.etcd = Etcd('foo', {'namespace': '/patroni/', 'ttl': 30, 'host': 'localhost:2379', 'scope': 'test'})
             self.etcd.client.write = etcd_write
             self.etcd.client.read = etcd_read
             self.etcd.client.delete = Mock(side_effect=etcd.EtcdException())
 
+    def test_base_path(self):
+        self.assertEquals(self.etcd._base_path, '/patroni/test')
+
     @patch('dns.resolver.query', dns_query)
     def test_get_etcd_client(self):
         with patch.object(etcd.Client, 'machines') as mock_machines:

From deaaf8ad1aa1ecae2c5bef9768c3e647a447b647 Mon Sep 17 00:00:00 2001
From: Alexander Kukushkin <alexander.kukushkin@zalando.de>
Date: Wed, 21 Oct 2015 15:38:51 +0200
Subject: [PATCH 21/28] Fix unit-test for Postgresql.controldata()

---
 tests/test_postgresql.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/test_postgresql.py b/tests/test_postgresql.py
index 83e93ec0..70446533 100644
--- a/tests/test_postgresql.py
+++ b/tests/test_postgresql.py
@@ -346,7 +346,7 @@ class TestPostgresql(unittest.TestCase):
         self.p.remove_data_directory()
 
     @patch('subprocess.check_output', MagicMock(return_value=0, side_effect=pg_controldata_string))
-    @patch('subprocess.check_output', side_effect=subprocess.CalledProcessError)
+    @patch('subprocess.check_output', side_effect=subprocess.CalledProcessError(1, ''))
     @patch('subprocess.check_output', side_effect=Exception('Failed'))
     def test_controldata(self, check_output_call_error, check_output_generic_exception):
         data = self.p.controldata()

From e0e4789b8a1d96e94627a050b17f57729fe64b13 Mon Sep 17 00:00:00 2001
From: Feike Steenbergen <feikesteenbergen@gmail.com>
Date: Wed, 21 Oct 2015 15:49:20 +0200
Subject: [PATCH 22/28] Explicitly cast scope to string.

Fixes issue #74
---
 patroni/dcs.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/patroni/dcs.py b/patroni/dcs.py
index bb56fecc..cd1a6512 100644
--- a/patroni/dcs.py
+++ b/patroni/dcs.py
@@ -123,7 +123,7 @@ class AbstractDCS:
         """
         self._name = name
         self._namespace = '/{}'.format(config.get('namespace', '/service/').strip('/'))
-        self._base_path = '/'.join([self._namespace, config['scope']])
+        self._base_path = '/'.join([self._namespace, str(config['scope']]))
 
         self._cluster = None
         self._cluster_thread_lock = Lock()

From c751dfdebfe3c044c3b22790c98eaa04722a16e2 Mon Sep 17 00:00:00 2001
From: Feike Steenbergen <feike.steenbergen@zalando.de>
Date: Thu, 22 Oct 2015 08:50:53 +0200
Subject: [PATCH 23/28] Typo in joining namespace to scope

---
 patroni/dcs.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/patroni/dcs.py b/patroni/dcs.py
index cd1a6512..f406456d 100644
--- a/patroni/dcs.py
+++ b/patroni/dcs.py
@@ -123,7 +123,7 @@ class AbstractDCS:
         """
         self._name = name
         self._namespace = '/{}'.format(config.get('namespace', '/service/').strip('/'))
-        self._base_path = '/'.join([self._namespace, str(config['scope']]))
+        self._base_path = '/'.join([self._namespace, str(config['scope'])])
 
         self._cluster = None
         self._cluster_thread_lock = Lock()

From 857caa13977273bc1a6677d14234e8a67b3acc4b Mon Sep 17 00:00:00 2001
From: Feike Steenbergen <feikesteenbergen@gmail.com>
Date: Thu, 22 Oct 2015 09:24:31 +0200
Subject: [PATCH 24/28] Revert casting to string

---
 patroni/dcs.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/patroni/dcs.py b/patroni/dcs.py
index f406456d..bb56fecc 100644
--- a/patroni/dcs.py
+++ b/patroni/dcs.py
@@ -123,7 +123,7 @@ class AbstractDCS:
         """
         self._name = name
         self._namespace = '/{}'.format(config.get('namespace', '/service/').strip('/'))
-        self._base_path = '/'.join([self._namespace, str(config['scope'])])
+        self._base_path = '/'.join([self._namespace, config['scope']])
 
         self._cluster = None
         self._cluster_thread_lock = Lock()

From eaf63db886da9c2930b54e546547bfc6a831bc4b Mon Sep 17 00:00:00 2001
From: Feike Steenbergen <feikesteenbergen@gmail.com>
Date: Thu, 22 Oct 2015 09:28:00 +0200
Subject: [PATCH 25/28] Use a different namespace in the Docker container.

Also bugfix: Patroni should advertise Docker ip as connect address
---
 docker/entrypoint.sh | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/docker/entrypoint.sh b/docker/entrypoint.sh
index d94757b3..7afdf9c5 100755
--- a/docker/entrypoint.sh
+++ b/docker/entrypoint.sh
@@ -83,10 +83,11 @@ cat > /patroni/postgres.yml <<__EOF__
 
 ttl: &ttl 30
 loop_wait: &loop_wait 10
-scope: &scope ${PATRONI_SCOPE}
+scope: &scope '${PATRONI_SCOPE}'
+namespace: 'patroni'
 restapi:
-  listen: 127.0.0.1:8008
-  connect_address: 127.0.0.1:8008
+  listen: 0.0.0.0:8008
+  connect_address: ${DOCKER_IP}:8008
 etcd:
   scope: *scope
   ttl: *ttl

From 5ae6f3a56c2d073d5c9543c6afcc8772e1e606a4 Mon Sep 17 00:00:00 2001
From: Feike Steenbergen <feikesteenbergen@gmail.com>
Date: Thu, 22 Oct 2015 09:30:12 +0200
Subject: [PATCH 26/28] Change Docker registry

---
 docker/dev_patroni_cluster.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/docker/dev_patroni_cluster.sh b/docker/dev_patroni_cluster.sh
index e9a253dc..dcc18f87 100755
--- a/docker/dev_patroni_cluster.sh
+++ b/docker/dev_patroni_cluster.sh
@@ -1,6 +1,6 @@
 #!/bin/bash
 
-DOCKER_IMAGE="os-registry.stups.zalan.do/acid/patroni:1.0-SNAPSHOT"
+DOCKER_IMAGE="registry.opensource.zalan.do/acid/patroni:1.0-SNAPSHOT"
 MEMBERS=3
 
 

From 0c5a21e57d106f42028f41ea906b5091c17ed37b Mon Sep 17 00:00:00 2001
From: Oleksii Kliukin <oleksii.kliukin@zalando.de>
Date: Fri, 23 Oct 2015 10:46:55 +0200
Subject: [PATCH 27/28] Fix removal of keys on failed initialization.

The initialize key was checked against the value
of the node name before removal, but it was changed
recently to contain either an empty string, or cluster
sysid. To fix this, the check for the previous value
was simply removed: we can guarantee that the code path
that removes the key is the one that created it.
---
 patroni/etcd.py      | 2 +-
 patroni/zookeeper.py | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/patroni/etcd.py b/patroni/etcd.py
index d84f5a50..4a82f2d7 100644
--- a/patroni/etcd.py
+++ b/patroni/etcd.py
@@ -249,7 +249,7 @@ class Etcd(AbstractDCS):
 
     @catch_etcd_errors
     def cancel_initialization(self):
-        return self.retry(self.client.delete, self.initialize_path, prevValue=self._name)
+        return self.retry(self.client.delete, self.initialize_path)
 
     def watch(self, timeout):
         cluster = self.cluster
diff --git a/patroni/zookeeper.py b/patroni/zookeeper.py
index 1fa2cca3..bc9b83c4 100644
--- a/patroni/zookeeper.py
+++ b/patroni/zookeeper.py
@@ -271,7 +271,7 @@ class ZooKeeper(AbstractDCS):
 
     def _cancel_initialization(self):
         node = self.get_node(self.initialize_path)
-        if node and node[0] == self._name:
+        if node:
             self.client.delete(self.initialize_path, version=node[1].version)
 
     def cancel_initialization(self):

From 553129a981d84a0f96bc83337002e44c8255f7ed Mon Sep 17 00:00:00 2001
From: Alexander Kukushkin <alexander.kukushkin@zalando.de>
Date: Fri, 23 Oct 2015 15:59:20 +0200
Subject: [PATCH 28/28] Revert "Fix unit-test for Postgresql.controldata()"

This reverts commit deaaf8ad1aa1ecae2c5bef9768c3e647a447b647.
---
 tests/test_postgresql.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/test_postgresql.py b/tests/test_postgresql.py
index 9eb7c1d3..0ed04a15 100644
--- a/tests/test_postgresql.py
+++ b/tests/test_postgresql.py
@@ -354,7 +354,7 @@ class TestPostgresql(unittest.TestCase):
         self.p.remove_data_directory()
 
     @patch('subprocess.check_output', MagicMock(return_value=0, side_effect=pg_controldata_string))
-    @patch('subprocess.check_output', side_effect=subprocess.CalledProcessError(1, ''))
+    @patch('subprocess.check_output', side_effect=subprocess.CalledProcessError)
     @patch('subprocess.check_output', side_effect=Exception('Failed'))
     def test_controldata(self, check_output_call_error, check_output_generic_exception):
         data = self.p.controldata()