From 949821c57be41cf4fb9914226d8bf55bb78615d9 Mon Sep 17 00:00:00 2001
From: Oleksii Kliukin <oleksii.kliukin@zalando.de>
Date: Mon, 1 Aug 2016 16:23:08 +0200
Subject: [PATCH 01/10] Add patronictl scaffold command.

Creates the cluster structure in DCS as long as the initialize
key does not exist.

The structure consists of the leader and member keys. Both are
intentionally set to never expire, in order to support running the
cluster with the master that doesn't run the Patroni (external master).

Changes to the DCS code as well, in order to support non-expiring
leader and member keys.

Some silly default settings had to be applied if Patroni is unable
to find the configuration file. In particular, the connect address
will point to the localhost. Perhaps we should avoid running with
wihtout the valid configuration altogether, but currently there is
a valid use-case for this behavior, namely the replicas that are
running with the inaccessible master and getting up-to-date with
WAL segments only.
---
 patroni/ctl.py           | 57 +++++++++++++++++++++++++++++++++++++---
 patroni/dcs/__init__.py  |  3 ++-
 patroni/dcs/consul.py    |  8 +++---
 patroni/dcs/etcd.py      | 12 ++++++---
 patroni/dcs/zookeeper.py | 11 +++++---
 5 files changed, 76 insertions(+), 15 deletions(-)

diff --git a/patroni/ctl.py b/patroni/ctl.py
index a0f0c565..fae7cba0 100644
--- a/patroni/ctl.py
+++ b/patroni/ctl.py
@@ -20,8 +20,9 @@ import yaml
 from click import ClickException
 from patroni.config import Config
 from patroni.dcs import get_dcs as _get_dcs
-from patroni.exceptions import PatroniException
-from patroni.postgresql import get_conn_kwargs
+from patroni.exceptions import PatroniException, DCSError
+from patroni.postgresql import Postgresql, get_conn_kwargs
+from patroni.api import RestApiServer
 from prettytable import PrettyTable
 from six.moves.urllib_parse import urlparse
 
@@ -76,7 +77,6 @@ def load_config(path, dcs):
         for d in DCS_DEFAULTS:
             config.pop(d, None)
         config.update(dcs)
-
     return config
 
 
@@ -646,3 +646,54 @@ def configure(config_file, dcs, namespace):
     config['dcs_api'] = str(dcs)
     config['namespace'] = str(namespace)
     store_config(config, config_file)
+
+
+def touch_member(config, dcs):
+    ''' Rip of the ha.touch_member without inter-class dependencies '''
+    p = Postgresql(config['postgresql'])
+    p.set_state('running')
+    p.set_role('master')
+
+    api = RestApiServer(None, config['restapi'])
+    data = {
+        'conn_url': p.connection_string,
+        'api_url': api.connection_string,
+        'state': p.state,
+        'role': p.role
+    }
+    try:
+        dcs.touch_member(json.dumps(data, separators=(',', ':')), permanent=True)
+    except DCSError:
+        return False
+    return True
+
+
+def set_defaults(config, cluster_name):
+    ''' fill-in some basic configuration parameters if config file is not set '''
+    config['postgresql']['name'] = config['postgresql'].get('name') or cluster_name
+    config['postgresql']['scope'] = config['postgresql'].get('scope') or cluster_name
+    config['postgresql']['listen'] = config['postgresql'].get('listen') or "127.0.0.1"
+    config['postgresql']['authentication'] = {'replication': None}
+    config['restapi']['listen'] = ':' in config['restapi'].get('listen', ".") or '127.0.0.1:5432'
+
+
+@ctl.command('scaffold', help='Create a structure for the cluster in DCS')
+@click.argument('cluster_name')
+@click.option('--sysid', '-s', help='System ID of the cluster to put into the initialize key', default="")
+@option_config_file
+@option_dcs
+def scaffold(cluster_name, config_file, dcs, sysid):
+    logging.debug("config_file = %s, cluster_name = %s, dcs = %s, sysid = %s", config_file, cluster_name, dcs, sysid)
+    config, dcs, cluster = ctl_load_config(cluster_name, config_file, dcs)
+    if cluster and cluster.initialize:
+        raise PatroniCtlException("This cluster is already initialized")
+    dcs.initialize(create_new=True, sysid=sysid)
+
+    set_defaults(config, cluster_name)
+
+    # make sure the leader key will never expire
+    if not (touch_member(config, dcs) and dcs.attempt_to_acquire_leader(permanent=True)):
+        dcs.delete_leader()
+        dcs.cancel_initialization()
+        return 1
+    return 0
diff --git a/patroni/dcs/__init__.py b/patroni/dcs/__init__.py
index 39049792..a2264a03 100644
--- a/patroni/dcs/__init__.py
+++ b/patroni/dcs/__init__.py
@@ -343,13 +343,14 @@ class AbstractDCS(object):
         """Create or update `/config` key"""
 
     @abc.abstractmethod
-    def touch_member(self, data, ttl=None):
+    def touch_member(self, data, ttl=None, permanent=False):
         """Update member key in DCS.
         This method should create or update key with the name = '/members/' + `~self._name`
         and value = data in a given DCS.
 
         :param data: json serialized information about instance (including connection strings)
         :param ttl: ttl for member key, optional parameter. If it is None `~self.member_ttl will be used`
+        :permanent: if set to `!True`, the member key will never expire. Used in patronictl for the external master.
         :returns: `!True` on success otherwise `!False`
         """
 
diff --git a/patroni/dcs/consul.py b/patroni/dcs/consul.py
index ffa0ab12..53ab1b1b 100644
--- a/patroni/dcs/consul.py
+++ b/patroni/dcs/consul.py
@@ -191,7 +191,8 @@ class Consul(AbstractDCS):
             return True
 
         try:
-            self._client.kv.put(self.member_path, data, acquire=self._session)
+            args = {'cas': None} if kwargs.get('permanent') else {}
+            self._client.kv.put(self.member_path, data, acquire=self._session, **args)
             self._my_member_data = data
             return True
         except Exception:
@@ -199,8 +200,9 @@ class Consul(AbstractDCS):
         return False
 
     @catch_consul_errors
-    def attempt_to_acquire_leader(self):
-        ret = self._client.kv.put(self.leader_path, self._name, acquire=self._session)
+    def attempt_to_acquire_leader(self, permanent=False):
+        args = {'cas': None} if permanent else {}
+        ret = self._client.kv.put(self.leader_path, self._name, acquire=self._session, **args)
         if not ret:
             logger.info('Could not take out TTL lock')
         return ret
diff --git a/patroni/dcs/etcd.py b/patroni/dcs/etcd.py
index c376ce8b..3f9f2c3c 100644
--- a/patroni/dcs/etcd.py
+++ b/patroni/dcs/etcd.py
@@ -281,16 +281,20 @@ class Etcd(AbstractDCS):
             raise EtcdError('Etcd is not responding properly')
 
     @catch_etcd_errors
-    def touch_member(self, data, ttl=None):
-        return self.retry(self._client.set, self.member_path, data, ttl or self._ttl)
+    def touch_member(self, data, ttl=None, permanent=False):
+        return self.retry(self._client.set, self.member_path, data, (ttl or self._ttl) if not permanent else None)
 
     @catch_etcd_errors
     def take_leader(self):
         return self.retry(self._client.set, self.leader_path, self._name, self._ttl)
 
-    def attempt_to_acquire_leader(self):
+    def attempt_to_acquire_leader(self, permanent=False):
         try:
-            return bool(self.retry(self._client.write, self.leader_path, self._name, ttl=self._ttl, prevExist=False))
+            return bool(self.retry(self._client.write,
+                                   self.leader_path,
+                                   self._name,
+                                   ttl=self._ttl if permanent else None,
+                                   prevExist=False))
         except etcd.EtcdAlreadyExist:
             logger.info('Could not take out TTL lock')
         except (RetryFailedError, etcd.EtcdException):
diff --git a/patroni/dcs/zookeeper.py b/patroni/dcs/zookeeper.py
index be6fe45a..3da9d1f1 100644
--- a/patroni/dcs/zookeeper.py
+++ b/patroni/dcs/zookeeper.py
@@ -162,8 +162,11 @@ class ZooKeeper(AbstractDCS):
         except:
             return False
 
-    def attempt_to_acquire_leader(self):
-        ret = self._create(self.leader_path, self._name, makepath=True, ephemeral=True)
+    def attempt_to_acquire_leader(self, permanent=False):
+        ret = self._create(self.leader_path,
+                           self._name,
+                           makepath=True,
+                           ephemeral=not permanent)
         if not ret:
             logger.info('Could not take out TTL lock')
         return ret
@@ -192,7 +195,7 @@ class ZooKeeper(AbstractDCS):
         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):
+    def touch_member(self, data, ttl=None, permanent=False):
         cluster = self.cluster
         member = cluster and ([m for m in cluster.members if m.name == self._name] or [None])[0]
         path = self.member_path
@@ -213,7 +216,7 @@ class ZooKeeper(AbstractDCS):
             if member:
                 self._client.retry(self._client.set, path, data)
             else:
-                self._client.retry(self._client.create, path, data, makepath=True, ephemeral=True)
+                self._client.retry(self._client.create, path, data, makepath=True, ephemeral=not permanent)
             self._my_member_data = data
             return True
         except NodeExistsError:

From 113ab6379a1782c4f167045d211c1f872d94e1b4 Mon Sep 17 00:00:00 2001
From: Oleksii Kliukin <oleksii.kliukin@zalando.de>
Date: Thu, 4 Aug 2016 15:59:33 +0200
Subject: [PATCH 02/10] Address code review

- Add a new param to the abstract DCS attempt_to_take_leader
- Make sure the cluster is wiped-out properly if we created the
  initialize key, but failed to populate it with leader and member.
  This actually means that we may wipe out the running cluster without
  the intialization key, but that is a very unlikely case in practice.
---
 patroni/ctl.py          | 20 +++++++++++++-------
 patroni/dcs/__init__.py |  3 ++-
 2 files changed, 15 insertions(+), 8 deletions(-)

diff --git a/patroni/ctl.py b/patroni/ctl.py
index fae7cba0..b11b6049 100644
--- a/patroni/ctl.py
+++ b/patroni/ctl.py
@@ -687,13 +687,19 @@ def scaffold(cluster_name, config_file, dcs, sysid):
     config, dcs, cluster = ctl_load_config(cluster_name, config_file, dcs)
     if cluster and cluster.initialize:
         raise PatroniCtlException("This cluster is already initialized")
-    dcs.initialize(create_new=True, sysid=sysid)
+
+    if not dcs.initialize(create_new=True, sysid=sysid):
+        # initialize key already exists, don't touch this cluster
+        raise PatroniCtlException("Initialize key for cluster {0} already exists".format(cluster_name))
 
     set_defaults(config, cluster_name)
 
-    # make sure the leader key will never expire
-    if not (touch_member(config, dcs) and dcs.attempt_to_acquire_leader(permanent=True)):
-        dcs.delete_leader()
-        dcs.cancel_initialization()
-        return 1
-    return 0
+    try:
+        # make sure the leader keys will never expire
+        if not (touch_member(config, dcs) and dcs.attempt_to_acquire_leader(permanent=True)):
+            # we did initialize this cluster, but failed to write the leader or member keys, wipe it down completely.
+            raise PatroniCtlException("Unable to install permanent leader for cluster {0}".format(cluster_name))
+    except:
+        dcs.delete_cluster()
+        raise
+    click.echo("Cluster {0} has been created successfully".format(cluster_name, sysid))
diff --git a/patroni/dcs/__init__.py b/patroni/dcs/__init__.py
index a2264a03..480c85bb 100644
--- a/patroni/dcs/__init__.py
+++ b/patroni/dcs/__init__.py
@@ -313,9 +313,10 @@ class AbstractDCS(object):
         for example for etcd `prevValue` parameter must be used."""
 
     @abc.abstractmethod
-    def attempt_to_acquire_leader(self):
+    def attempt_to_acquire_leader(self, permanent=False):
         """Attempt to acquire leader lock
         This method should create `/leader` key with value=`~self._name`
+        :param permanent: if set to `!True`, the leader key will never expie. Used in patronictl for the external master
         :returns: `!True` if key has been created successfully.
 
         Key must be created atomically. In case if key already exists it should not be

From e3cdeb3244968309597736dd691c3fe4b9481687 Mon Sep 17 00:00:00 2001
From: Oleksii Kliukin <oleksii.kliukin@zalando.de>
Date: Fri, 5 Aug 2016 10:55:38 +0200
Subject: [PATCH 03/10] Address code review.

---
 patroni/ctl.py        | 8 ++++----
 patroni/dcs/consul.py | 8 ++++----
 patroni/dcs/etcd.py   | 4 ++--
 3 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/patroni/ctl.py b/patroni/ctl.py
index 26cead8a..ba8eca01 100644
--- a/patroni/ctl.py
+++ b/patroni/ctl.py
@@ -670,9 +670,9 @@ def touch_member(config, dcs):
 
 def set_defaults(config, cluster_name):
     ''' fill-in some basic configuration parameters if config file is not set '''
-    config['postgresql']['name'] = config['postgresql'].get('name') or cluster_name
-    config['postgresql']['scope'] = config['postgresql'].get('scope') or cluster_name
-    config['postgresql']['listen'] = config['postgresql'].get('listen') or "127.0.0.1"
+    config['postgresql'].setdefault('name', cluster_name)
+    config['postgresql'].setdefault('scope', cluster_name)
+    config['postgresql'].setdefault('listen', '127.0.0.1')
     config['postgresql']['authentication'] = {'replication': None}
     config['restapi']['listen'] = ':' in config['restapi'].get('listen', ".") or '127.0.0.1:5432'
 
@@ -702,4 +702,4 @@ def scaffold(cluster_name, config_file, dcs, sysid):
     except:
         dcs.delete_cluster()
         raise
-    click.echo("Cluster {0} has been created successfully".format(cluster_name, sysid))
+    click.echo("Cluster {0} has been created successfully".format(cluster_name))
diff --git a/patroni/dcs/consul.py b/patroni/dcs/consul.py
index 53ab1b1b..b2e15f76 100644
--- a/patroni/dcs/consul.py
+++ b/patroni/dcs/consul.py
@@ -191,8 +191,8 @@ class Consul(AbstractDCS):
             return True
 
         try:
-            args = {'cas': None} if kwargs.get('permanent') else {}
-            self._client.kv.put(self.member_path, data, acquire=self._session, **args)
+            args = {} if kwargs.get('permanent') else {'acquire': self._session}
+            self._client.kv.put(self.member_path, data, **args)
             self._my_member_data = data
             return True
         except Exception:
@@ -201,8 +201,8 @@ class Consul(AbstractDCS):
 
     @catch_consul_errors
     def attempt_to_acquire_leader(self, permanent=False):
-        args = {'cas': None} if permanent else {}
-        ret = self._client.kv.put(self.leader_path, self._name, acquire=self._session, **args)
+        args = {} if permanent else {'acquire': self._session}
+        ret = self._client.kv.put(self.leader_path, self._name, **args)
         if not ret:
             logger.info('Could not take out TTL lock')
         return ret
diff --git a/patroni/dcs/etcd.py b/patroni/dcs/etcd.py
index 3f9f2c3c..0116aeab 100644
--- a/patroni/dcs/etcd.py
+++ b/patroni/dcs/etcd.py
@@ -282,7 +282,7 @@ class Etcd(AbstractDCS):
 
     @catch_etcd_errors
     def touch_member(self, data, ttl=None, permanent=False):
-        return self.retry(self._client.set, self.member_path, data, (ttl or self._ttl) if not permanent else None)
+        return self.retry(self._client.set, self.member_path, data, None if permanent else ttl or self._ttl)
 
     @catch_etcd_errors
     def take_leader(self):
@@ -293,7 +293,7 @@ class Etcd(AbstractDCS):
             return bool(self.retry(self._client.write,
                                    self.leader_path,
                                    self._name,
-                                   ttl=self._ttl if permanent else None,
+                                   ttl=None if permanent else self._ttl,
                                    prevExist=False))
         except etcd.EtcdAlreadyExist:
             logger.info('Could not take out TTL lock')

From eeb8f1b694c2bec08550ab488977baa60d941bfb Mon Sep 17 00:00:00 2001
From: Oleksii Kliukin <oleksii.kliukin@zalando.de>
Date: Mon, 8 Aug 2016 12:21:01 +0200
Subject: [PATCH 04/10] Further address code reviews.

- Fix the issue in ctl that would result in setting the  listen_address to True.
- Minor stylistic issues.
- Add unit-tests.
---
 patroni/ctl.py        |  9 ++++-----
 patroni/dcs/consul.py |  2 +-
 tests/test_ctl.py     | 42 +++++++++++++++++++++++++++++++++++++++---
 3 files changed, 44 insertions(+), 9 deletions(-)

diff --git a/patroni/ctl.py b/patroni/ctl.py
index ba8eca01..cc0bca7e 100644
--- a/patroni/ctl.py
+++ b/patroni/ctl.py
@@ -649,7 +649,7 @@ def configure(config_file, dcs, namespace):
 
 
 def touch_member(config, dcs):
-    ''' Rip of the ha.touch_member without inter-class dependencies '''
+    ''' Rip-off of the ha.touch_member without inter-class dependencies '''
     p = Postgresql(config['postgresql'])
     p.set_state('running')
     p.set_role('master')
@@ -674,7 +674,8 @@ def set_defaults(config, cluster_name):
     config['postgresql'].setdefault('scope', cluster_name)
     config['postgresql'].setdefault('listen', '127.0.0.1')
     config['postgresql']['authentication'] = {'replication': None}
-    config['restapi']['listen'] = ':' in config['restapi'].get('listen', ".") or '127.0.0.1:5432'
+    config['restapi']['listen'] = (config['restapi']['listen']
+                                   if ':' in config['restapi'].get('listen', ".") else '127.0.0.1:5432')
 
 
 @ctl.command('scaffold', help='Create a structure for the cluster in DCS')
@@ -683,9 +684,8 @@ def set_defaults(config, cluster_name):
 @option_config_file
 @option_dcs
 def scaffold(cluster_name, config_file, dcs, sysid):
-    logging.debug("config_file = %s, cluster_name = %s, dcs = %s, sysid = %s", config_file, cluster_name, dcs, sysid)
     config, dcs, cluster = ctl_load_config(cluster_name, config_file, dcs)
-    if cluster and cluster.initialize:
+    if cluster and cluster.initialize is not None:
         raise PatroniCtlException("This cluster is already initialized")
 
     if not dcs.initialize(create_new=True, sysid=sysid):
@@ -700,6 +700,5 @@ def scaffold(cluster_name, config_file, dcs, sysid):
             # we did initialize this cluster, but failed to write the leader or member keys, wipe it down completely.
             raise PatroniCtlException("Unable to install permanent leader for cluster {0}".format(cluster_name))
     except:
-        dcs.delete_cluster()
         raise
     click.echo("Cluster {0} has been created successfully".format(cluster_name))
diff --git a/patroni/dcs/consul.py b/patroni/dcs/consul.py
index b2e15f76..36a93536 100644
--- a/patroni/dcs/consul.py
+++ b/patroni/dcs/consul.py
@@ -191,7 +191,7 @@ class Consul(AbstractDCS):
             return True
 
         try:
-            args = {} if kwargs.get('permanent') else {'acquire': self._session}
+            args = {} if kwargs.get('permanent', False) else {'acquire': self._session}
             self._client.kv.put(self.member_path, data, **args)
             self._my_member_data = data
             return True
diff --git a/tests/test_ctl.py b/tests/test_ctl.py
index 42929a43..de07ff44 100644
--- a/tests/test_ctl.py
+++ b/tests/test_ctl.py
@@ -4,14 +4,20 @@ import requests
 import sys
 import unittest
 
+from patroni.api import RestApiServer
+from six.moves import BaseHTTPServer
+
 from click.testing import CliRunner
 from mock import patch, Mock
 from patroni.ctl import ctl, members, store_config, load_config, output_members, post_patroni, get_dcs, parse_dcs, \
-    wait_for_leader, get_all_members, get_any_member, get_cursor, query_member, configure, PatroniCtlException
+    wait_for_leader, get_all_members, get_any_member, get_cursor, query_member, configure, touch_member, set_defaults,\
+    PatroniCtlException
+
+from patroni.exceptions import DCSError
 from psycopg2 import OperationalError
 from test_etcd import etcd_read, requests_get, socket_getaddrinfo, MockResponse
 from test_ha import get_cluster_initialized_without_leader, get_cluster_initialized_with_leader, \
-    get_cluster_initialized_with_only_leader
+    get_cluster_initialized_with_only_leader, get_cluster_not_initialized_without_leader
 from test_postgresql import MockConnect, psycopg2_connect
 
 CONFIG_FILE_PATH = './test-ctl.yaml'
@@ -28,7 +34,9 @@ def test_rw_config():
         os.rmdir(CONFIG_FILE_PATH)
 
 
-@patch('patroni.ctl.load_config', Mock(return_value={'restapi': {'auth': 'u:p'}, 'etcd': {'host': 'localhost:4001'}}))
+@patch('patroni.ctl.load_config', Mock(return_value={'postgresql': {'data_dir': '.', 'parameters': {}, 'retry_timeout': 5},
+                                                     'restapi': {'auth': 'u:p', 'listen': ''},
+                                                     'etcd': {'host': 'localhost:4001'}}))
 class TestCtl(unittest.TestCase):
 
     @patch('socket.getaddrinfo', socket_getaddrinfo)
@@ -292,3 +300,31 @@ class TestCtl(unittest.TestCase):
     def test_configure(self):
         result = self.runner.invoke(configure, ['--dcs', 'abc', '-c', 'dummy', '-n', 'bla'])
         assert result.exit_code == 0
+
+    @patch('patroni.ctl.get_dcs')
+    @patch.object(BaseHTTPServer.HTTPServer, '__init__', Mock())
+    def test_scaffold(self, mock_get_dcs):
+        mock_get_dcs.return_value = self.e
+        mock_get_dcs.return_value.get_cluster = get_cluster_not_initialized_without_leader
+        mock_get_dcs.return_value.initialize = Mock(return_value=True)
+        mock_get_dcs.return_value.touch_member = Mock(return_value=True)
+        mock_get_dcs.return_value.attempt_to_acquire_leader = Mock(return_value=True)
+
+        RestApiServer._BaseServer__is_shut_down = Mock()
+        RestApiServer._BaseServer__shutdown_request = True
+        RestApiServer.socket = 0
+
+        with patch.object(self.e, 'initialize', return_value=False):
+            result = self.runner.invoke(ctl, ['scaffold', 'alpha'])
+            assert result.exit_code == 1
+
+        with patch.object(mock_get_dcs.return_value, 'touch_member', Mock(side_effect=DCSError("foo"))):
+            result = self.runner.invoke(ctl, ['scaffold', 'alpha'])
+            assert result.exception
+
+        result = self.runner.invoke(ctl, ['scaffold', 'alpha'])
+        assert result.exit_code == 0
+
+        mock_get_dcs.return_value.get_cluster = get_cluster_initialized_with_leader
+        result = self.runner.invoke(ctl, ['scaffold', 'alpha'])
+        assert result.exit_code == 1

From 53f991df0f46f6b806adb6663ab45afca6fe2d46 Mon Sep 17 00:00:00 2001
From: Oleksii Kliukin <oleksii.kliukin@zalando.de>
Date: Mon, 8 Aug 2016 15:30:33 +0200
Subject: [PATCH 05/10] More code-review related fixes

- Add missing delete_cluster.
- Simplify parts of the code by removing exception handlers where
  they are not needed.
- Fix typos.
---
 patroni/ctl.py          | 19 +++++++------------
 patroni/dcs/__init__.py |  2 +-
 tests/test_ctl.py       |  9 ++++-----
 3 files changed, 12 insertions(+), 18 deletions(-)

diff --git a/patroni/ctl.py b/patroni/ctl.py
index cc0bca7e..f6fedfc3 100644
--- a/patroni/ctl.py
+++ b/patroni/ctl.py
@@ -661,11 +661,8 @@ def touch_member(config, dcs):
         'state': p.state,
         'role': p.role
     }
-    try:
-        dcs.touch_member(json.dumps(data, separators=(',', ':')), permanent=True)
-    except DCSError:
-        return False
-    return True
+
+    return dcs.touch_member(json.dumps(data, separators=(',', ':')), permanent=True)
 
 
 def set_defaults(config, cluster_name):
@@ -694,11 +691,9 @@ def scaffold(cluster_name, config_file, dcs, sysid):
 
     set_defaults(config, cluster_name)
 
-    try:
-        # make sure the leader keys will never expire
-        if not (touch_member(config, dcs) and dcs.attempt_to_acquire_leader(permanent=True)):
-            # we did initialize this cluster, but failed to write the leader or member keys, wipe it down completely.
-            raise PatroniCtlException("Unable to install permanent leader for cluster {0}".format(cluster_name))
-    except:
-        raise
+    # make sure the leader keys will never expire
+    if not (touch_member(config, dcs) and dcs.attempt_to_acquire_leader(permanent=True)):
+        # we did initialize this cluster, but failed to write the leader or member keys, wipe it down completely.
+        dcs.delete_cluster()
+        raise PatroniCtlException("Unable to install permanent leader for cluster {0}".format(cluster_name))
     click.echo("Cluster {0} has been created successfully".format(cluster_name))
diff --git a/patroni/dcs/__init__.py b/patroni/dcs/__init__.py
index 480c85bb..a37ac6c9 100644
--- a/patroni/dcs/__init__.py
+++ b/patroni/dcs/__init__.py
@@ -316,7 +316,7 @@ class AbstractDCS(object):
     def attempt_to_acquire_leader(self, permanent=False):
         """Attempt to acquire leader lock
         This method should create `/leader` key with value=`~self._name`
-        :param permanent: if set to `!True`, the leader key will never expie. Used in patronictl for the external master
+        :param permanent: if set to `!True`, the leader key will never expire. Used in patronictl for the external master
         :returns: `!True` if key has been created successfully.
 
         Key must be created atomically. In case if key already exists it should not be
diff --git a/tests/test_ctl.py b/tests/test_ctl.py
index de07ff44..270dff1f 100644
--- a/tests/test_ctl.py
+++ b/tests/test_ctl.py
@@ -10,8 +10,7 @@ from six.moves import BaseHTTPServer
 from click.testing import CliRunner
 from mock import patch, Mock
 from patroni.ctl import ctl, members, store_config, load_config, output_members, post_patroni, get_dcs, parse_dcs, \
-    wait_for_leader, get_all_members, get_any_member, get_cursor, query_member, configure, touch_member, set_defaults,\
-    PatroniCtlException
+    wait_for_leader, get_all_members, get_any_member, get_cursor, query_member, configure, PatroniCtlException
 
 from patroni.exceptions import DCSError
 from psycopg2 import OperationalError
@@ -316,9 +315,9 @@ class TestCtl(unittest.TestCase):
 
         with patch.object(self.e, 'initialize', return_value=False):
             result = self.runner.invoke(ctl, ['scaffold', 'alpha'])
-            assert result.exit_code == 1
+            assert result.exception
 
-        with patch.object(mock_get_dcs.return_value, 'touch_member', Mock(side_effect=DCSError("foo"))):
+        with patch.object(mock_get_dcs.return_value, 'touch_member', Mock(return_value=False)):
             result = self.runner.invoke(ctl, ['scaffold', 'alpha'])
             assert result.exception
 
@@ -327,4 +326,4 @@ class TestCtl(unittest.TestCase):
 
         mock_get_dcs.return_value.get_cluster = get_cluster_initialized_with_leader
         result = self.runner.invoke(ctl, ['scaffold', 'alpha'])
-        assert result.exit_code == 1
+        assert result.exception

From d9102d27038536eeda38c85af03ff31ea64fedc8 Mon Sep 17 00:00:00 2001
From: Oleksii Kliukin <oleksii.kliukin@zalando.de>
Date: Mon, 8 Aug 2016 16:15:57 +0200
Subject: [PATCH 06/10] Remove the necessity of creating a RESTAPI object.

- We don't want to export RestApi object, since it initializes the
  socket and listens on it.
- Change get_dcs, so that the explicit scope passed to it will take
  priority over the one in the configuration file.
---
 patroni/ctl.py    | 16 ++++++++++------
 tests/test_ctl.py |  4 ----
 2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/patroni/ctl.py b/patroni/ctl.py
index f6fedfc3..e8797989 100644
--- a/patroni/ctl.py
+++ b/patroni/ctl.py
@@ -20,7 +20,7 @@ import yaml
 from click import ClickException
 from patroni.config import Config
 from patroni.dcs import get_dcs as _get_dcs
-from patroni.exceptions import PatroniException, DCSError
+from patroni.exceptions import PatroniException
 from patroni.postgresql import Postgresql, get_conn_kwargs
 from patroni.api import RestApiServer
 from prettytable import PrettyTable
@@ -106,7 +106,7 @@ def ctl(ctx):
 
 
 def get_dcs(config, scope):
-    config.setdefault('scope', scope)
+    config['scope'] = scope
     config.setdefault('name', scope)
     try:
         return _get_dcs(config)
@@ -654,10 +654,15 @@ def touch_member(config, dcs):
     p.set_state('running')
     p.set_role('master')
 
-    api = RestApiServer(None, config['restapi'])
+    def restapi_connection_string(config):
+        protocol = 'https' if config.get('certfile') else 'http'
+        connect_address = config.get('connect_address')
+        listen = config['listen']
+        return '{0}://{1}/patroni'.format(protocol, connect_address or listen)
+
     data = {
         'conn_url': p.connection_string,
-        'api_url': api.connection_string,
+        'api_url': restapi_connection_string(config['restapi']),
         'state': p.state,
         'role': p.role
     }
@@ -671,8 +676,7 @@ def set_defaults(config, cluster_name):
     config['postgresql'].setdefault('scope', cluster_name)
     config['postgresql'].setdefault('listen', '127.0.0.1')
     config['postgresql']['authentication'] = {'replication': None}
-    config['restapi']['listen'] = (config['restapi']['listen']
-                                   if ':' in config['restapi'].get('listen', ".") else '127.0.0.1:5432')
+    config['restapi']['listen'] = ':' in config['restapi']['listen'] and config['restapi']['listen'] or '127.0.0.1:8008'
 
 
 @ctl.command('scaffold', help='Create a structure for the cluster in DCS')
diff --git a/tests/test_ctl.py b/tests/test_ctl.py
index 270dff1f..3c6cf2c0 100644
--- a/tests/test_ctl.py
+++ b/tests/test_ctl.py
@@ -309,10 +309,6 @@ class TestCtl(unittest.TestCase):
         mock_get_dcs.return_value.touch_member = Mock(return_value=True)
         mock_get_dcs.return_value.attempt_to_acquire_leader = Mock(return_value=True)
 
-        RestApiServer._BaseServer__is_shut_down = Mock()
-        RestApiServer._BaseServer__shutdown_request = True
-        RestApiServer.socket = 0
-
         with patch.object(self.e, 'initialize', return_value=False):
             result = self.runner.invoke(ctl, ['scaffold', 'alpha'])
             assert result.exception

From 9fd01f6af4f363b0d4c1068cd4fc20b25cf5ba7f Mon Sep 17 00:00:00 2001
From: Oleksii Kliukin <oleksii.kliukin@zalando.de>
Date: Mon, 8 Aug 2016 16:48:14 +0200
Subject: [PATCH 07/10] Remove unused imports.

---
 tests/test_ctl.py | 2 --
 1 file changed, 2 deletions(-)

diff --git a/tests/test_ctl.py b/tests/test_ctl.py
index 3c6cf2c0..b167f764 100644
--- a/tests/test_ctl.py
+++ b/tests/test_ctl.py
@@ -4,7 +4,6 @@ import requests
 import sys
 import unittest
 
-from patroni.api import RestApiServer
 from six.moves import BaseHTTPServer
 
 from click.testing import CliRunner
@@ -12,7 +11,6 @@ from mock import patch, Mock
 from patroni.ctl import ctl, members, store_config, load_config, output_members, post_patroni, get_dcs, parse_dcs, \
     wait_for_leader, get_all_members, get_any_member, get_cursor, query_member, configure, PatroniCtlException
 
-from patroni.exceptions import DCSError
 from psycopg2 import OperationalError
 from test_etcd import etcd_read, requests_get, socket_getaddrinfo, MockResponse
 from test_ha import get_cluster_initialized_without_leader, get_cluster_initialized_with_leader, \

From 8416fecfd844f68760f560395f22d8621ad774d8 Mon Sep 17 00:00:00 2001
From: Oleksii Kliukin <oleksii.kliukin@zalando.de>
Date: Mon, 8 Aug 2016 17:15:45 +0200
Subject: [PATCH 08/10] Mix more flake8 warnings.

---
 patroni/ctl.py | 1 -
 1 file changed, 1 deletion(-)

diff --git a/patroni/ctl.py b/patroni/ctl.py
index e8797989..5c98cf28 100644
--- a/patroni/ctl.py
+++ b/patroni/ctl.py
@@ -22,7 +22,6 @@ from patroni.config import Config
 from patroni.dcs import get_dcs as _get_dcs
 from patroni.exceptions import PatroniException
 from patroni.postgresql import Postgresql, get_conn_kwargs
-from patroni.api import RestApiServer
 from prettytable import PrettyTable
 from six.moves.urllib_parse import urlparse
 

From 595598533aa63388db77931f682ff91c7c5dd016 Mon Sep 17 00:00:00 2001
From: Oleksii Kliukin <oleksii.kliukin@zalando.de>
Date: Mon, 8 Aug 2016 17:44:32 +0200
Subject: [PATCH 09/10] Add missing file.

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

diff --git a/patroni/dcs/__init__.py b/patroni/dcs/__init__.py
index a37ac6c9..4c9e27f0 100644
--- a/patroni/dcs/__init__.py
+++ b/patroni/dcs/__init__.py
@@ -316,7 +316,8 @@ class AbstractDCS(object):
     def attempt_to_acquire_leader(self, permanent=False):
         """Attempt to acquire leader lock
         This method should create `/leader` key with value=`~self._name`
-        :param permanent: if set to `!True`, the leader key will never expire. Used in patronictl for the external master
+        :param permanent: if set to `!True`, the leader key will never expire.
+         Used in patronictl for the external master
         :returns: `!True` if key has been created successfully.
 
         Key must be created atomically. In case if key already exists it should not be

From ac7abfdd7449e83c50e1cfe73e53e729354617b2 Mon Sep 17 00:00:00 2001
From: Oleksii Kliukin <oleksii.kliukin@zalando.de>
Date: Tue, 9 Aug 2016 10:00:46 +0200
Subject: [PATCH 10/10] Minor fixes, address final rounds of code review.

---
 patroni/dcs/__init__.py  | 3 ++-
 patroni/dcs/zookeeper.py | 5 +----
 tests/test_ctl.py        | 3 ---
 3 files changed, 3 insertions(+), 8 deletions(-)

diff --git a/patroni/dcs/__init__.py b/patroni/dcs/__init__.py
index 4c9e27f0..b26162f9 100644
--- a/patroni/dcs/__init__.py
+++ b/patroni/dcs/__init__.py
@@ -352,7 +352,8 @@ class AbstractDCS(object):
 
         :param data: json serialized information about instance (including connection strings)
         :param ttl: ttl for member key, optional parameter. If it is None `~self.member_ttl will be used`
-        :permanent: if set to `!True`, the member key will never expire. Used in patronictl for the external master.
+        :param permanent: if set to `!True`, the member key will never expire.
+         Used in patronictl for the external master.
         :returns: `!True` on success otherwise `!False`
         """
 
diff --git a/patroni/dcs/zookeeper.py b/patroni/dcs/zookeeper.py
index 3da9d1f1..9e018816 100644
--- a/patroni/dcs/zookeeper.py
+++ b/patroni/dcs/zookeeper.py
@@ -163,10 +163,7 @@ class ZooKeeper(AbstractDCS):
             return False
 
     def attempt_to_acquire_leader(self, permanent=False):
-        ret = self._create(self.leader_path,
-                           self._name,
-                           makepath=True,
-                           ephemeral=not permanent)
+        ret = self._create(self.leader_path, self._name, makepath=True, ephemeral=not permanent)
         if not ret:
             logger.info('Could not take out TTL lock')
         return ret
diff --git a/tests/test_ctl.py b/tests/test_ctl.py
index b167f764..4d54fc0f 100644
--- a/tests/test_ctl.py
+++ b/tests/test_ctl.py
@@ -4,8 +4,6 @@ import requests
 import sys
 import unittest
 
-from six.moves import BaseHTTPServer
-
 from click.testing import CliRunner
 from mock import patch, Mock
 from patroni.ctl import ctl, members, store_config, load_config, output_members, post_patroni, get_dcs, parse_dcs, \
@@ -299,7 +297,6 @@ class TestCtl(unittest.TestCase):
         assert result.exit_code == 0
 
     @patch('patroni.ctl.get_dcs')
-    @patch.object(BaseHTTPServer.HTTPServer, '__init__', Mock())
     def test_scaffold(self, mock_get_dcs):
         mock_get_dcs.return_value = self.e
         mock_get_dcs.return_value.get_cluster = get_cluster_not_initialized_without_leader