Release 1.4.4 (#683)

bump version and update release notes

README.rst

@@ -29,7 +29,7 @@ For an example of a Docker-based deployment with Patroni, see `Spilo <https://gi
 
 For additional background info, see:
 
-* `Elephants on Automatic: HA Clustered PostgreSQL with Helm <https://www.youtube.com/watch?v=CftcVhFMGSY>`_, talk by Josh Berkus and Oleksii Kliukin at KubeCon Berlin 2017 
+* `Elephants on Automatic: HA Clustered PostgreSQL with Helm <https://www.youtube.com/watch?v=CftcVhFMGSY>`_, talk by Josh Berkus and Oleksii Kliukin at KubeCon Berlin 2017
 * `PostgreSQL HA with Kubernetes and Patroni <https://www.youtube.com/watch?v=iruaCgeG7qs>`__, talk by Josh Berkus at KubeCon 2016 (video)
 * `Feb. 2016 Zalando Tech blog post <https://tech.zalando.de/blog/zalandos-patroni-a-template-for-high-availability-postgresql/>`__
 
@@ -41,6 +41,12 @@ Patroni is in active development and accepts contributions. See our `Contributin
 
 We report new releases information `here <https://github.com/zalando/patroni/releases>`__.
 
+=========
+Community
+=========
+
+There are two places to connect with the Patroni community: `on github <https://github.com/zalando/patroni>`__, via Issues and PRs, and on channel #patroni in the `PostgreSQL Slack <https://postgres-slack.herokuapp.com/>`__.  If you're using Patroni, or just interested, please join us.
+
 ===================================
 Technical Requirements/Installation
 ===================================

docs/CONTRIBUTING.rst

@@ -5,6 +5,11 @@ Contributing guidelines
 
 Wanna contribute to Patroni? Yay - here is how!
 
+Chatting
+--------
+
+Just want to chat with other Patroni users?  Looking for interactive troubleshooting help? Join us on channel #patroni in the `PostgreSQL Slack <https://postgres-slack.herokuapp.com/>`__.
+
 Reporting issues
 ----------------
 

docs/ENVIRONMENT.rst

@@ -11,6 +11,8 @@ Global/Universal
 -  **PATRONI\_NAME**: name of the node where the current instance of Patroni is running. Must be unique for the cluster.
 -  **PATRONI\_NAMESPACE**: path within the configuration store where Patroni will keep information about the cluster. Default value: "/service"
 -  **PATRONI\_SCOPE**: cluster name
+-  **PATRONI\_LOGLEVEL**: sets the general logging level (see `the docs for Python logging <https://docs.python.org/3.6/library/logging.html#levels>`_)
+-  **PATRONI\_REQUESTS_LOGLEVEL**: sets the logging level for all HTTP requests e.g. Kubernetes API calls (see `the docs for Python logging <https://docs.python.org/3.6/library/logging.html#levels>`_)
 
 Bootstrap configuration
 -----------------------
@@ -29,7 +31,7 @@ Consul
 -  **PATRONI\_CONSUL\_SCHEME**: (optional) **http** or **https**, defaults to **http**
 -  **PATRONI\_CONSUL\_TOKEN**: (optional) ACL token
 -  **PATRONI\_CONSUL\_VERIFY**: (optional) whether to verify the SSL certificate for HTTPS requests
--  **PATRONI\_CONSUL\_CACERT**: (optional) The ca certificate. If pressent it will enable validation.
+-  **PATRONI\_CONSUL\_CACERT**: (optional) The ca certificate. If present it will enable validation.
 -  **PATRONI\_CONSUL\_CERT**: (optional) File with the client certificate
 -  **PATRONI\_CONSUL\_KEY**: (optional) File with the client key. Can be empty if the key is part of certificate.
 -  **PATRONI\_CONSUL\_DC**: (optional) Datacenter to communicate with. By default the datacenter of the host is used.
@@ -42,7 +44,7 @@ Etcd
 -  **PATRONI\_ETCD\_URL**: url for the etcd, in format: http(s)://(username:password@)host:port
 -  **PATRONI\_ETCD\_PROXY**: proxy url for the etcd. If you are connecting to the etcd using proxy, use this parameter instead of **PATRONI\_ETCD\_URL**
 -  **PATRONI\_ETCD\_SRV**: Domain to search the SRV record(s) for cluster autodiscovery.
--  **PATRONI\_ETCD\_CACERT**: The ca certificate. If pressent it will enable validation.
+-  **PATRONI\_ETCD\_CACERT**: The ca certificate. If present it will enable validation.
 -  **PATRONI\_ETCD\_CERT**: File with the client certificate
 -  **PATRONI\_ETCD\_KEY**: File with the client key. Can be empty if the key is part of certificate.
 
@@ -61,7 +63,7 @@ Kubernetes
 -  **PATRONI\_KUBERNETES\_ROLE\_LABEL**: (optional) name of the label containing Postgres role (`master` or `replica`). Patroni will set this label on the pod it is running in. Default value is `role`.
 -  **PATRONI\_KUBERNETES\_USE\_ENDPOINTS**: (optional) if set to true, Patroni will use Endpoints instead of ConfigMaps to run leader elections and keep cluster state.
 -  **PATRONI\_KUBERNETES\_POD\_IP**: (optional) IP address of the pod Patroni is running in. This value is required when `PATRONI_KUBERNETES_USE_ENDPOINTS` is enabled and is used to populate the leader endpoint subsets when the pod's PostgreSQL is promoted.
--  **PATRONI\_KUBERNETES\_PORTS**: (optional) if the Service object has the name for the port, the same name must appear in the Endpoint object, otherwise service wont work. For example, if your service is defined as ``{Kind: Service, spec: {ports: [{name: postgresql, port: 5432, targetPort: 5432}]}}``, then you have to set ``PATRONI_KUBERNETES_PORTS='{[{"name": "postgresql", "port": 5432}]}'`` and Patroni will use it for updating subsets of the leader Endpoint. This parameter is used only if `PATRONI_KUBERNETES_USE_ENDPOINTS` is set.
+-  **PATRONI\_KUBERNETES\_PORTS**: (optional) if the Service object has the name for the port, the same name must appear in the Endpoint object, otherwise service won't work. For example, if your service is defined as ``{Kind: Service, spec: {ports: [{name: postgresql, port: 5432, targetPort: 5432}]}}``, then you have to set ``PATRONI_KUBERNETES_PORTS='{[{"name": "postgresql", "port": 5432}]}'`` and Patroni will use it for updating subsets of the leader Endpoint. This parameter is used only if `PATRONI_KUBERNETES_USE_ENDPOINTS` is set.
 
 PostgreSQL
 ----------

docs/README.rst

@@ -62,7 +62,7 @@ For example, the command in order to install Patroni together with dependencies
 
     pip install patroni[etcd,aws]
 
-Note that external tools to call in the replica creation or custom bootstap scripts (i.e. WAL-E) should be installed
+Note that external tools to call in the replica creation or custom bootstrap scripts (i.e. WAL-E) should be installed
 independently of Patroni.
 
 

docs/SETTINGS.rst

@@ -18,13 +18,13 @@ Bootstrap configuration
     -  **retry\_timeout**: timeout for DCS and PostgreSQL operation retries. DCS or network issues shorter than this will not cause Patroni to demote the leader. Default value: 10
     -  **maximum\_lag\_on\_failover**: the maximum bytes a follower may lag to be able to participate in leader election.
     -  **master\_start\_timeout**: the amount of time a master is allowed to recover from failures before failover is triggered. Default is 300 seconds. When set to 0 failover is done immediately after a crash is detected if possible. When using asynchronous replication a failover can cause lost transactions. Best worst case failover time for master failure is: loop\_wait + master\_start\_timeout + loop\_wait, unless master\_start\_timeout is zero, in which case it's just loop\_wait. Set the value according to your durability/availability tradeoff.
-    -  **synchronous\_mode**: turns on synchronous replication mode. In this mode a replica will be chosen as synchronous and only the latest leader and synchronous replica are able to participate in leader election. Synchronous mode makes sure that succesfully committed transactions will not be lost at failover, at the cost of losing availability for writes when Patroni cannot ensure transaction durability. See `replication modes documentation <https://github.com/zalando/patroni/blob/master/docs/replication_modes.rst>`__ for details.
+    -  **synchronous\_mode**: turns on synchronous replication mode. In this mode a replica will be chosen as synchronous and only the latest leader and synchronous replica are able to participate in leader election. Synchronous mode makes sure that successfully committed transactions will not be lost at failover, at the cost of losing availability for writes when Patroni cannot ensure transaction durability. See :ref:`replication modes documentation <replication_modes>` for details.
     -  **postgresql**:
-        -  **use\_pg\_rewind**:whether or not to use pg_rewind
+        -  **use\_pg\_rewind**: whether or not to use pg_rewind
         -  **use\_slots**: whether or not to use replication_slots. Must be False for PostgreSQL 9.3. You should comment out max_replication_slots before it becomes ineligible for leader status.
         -  **recovery\_conf**: additional configuration settings written to recovery.conf when configuring follower. 
         -  **parameters**: list of configuration settings for Postgres. Many of these are required for replication to work.
--  **method**: custom script to use for bootstrpapping this cluster.
+-  **method**: custom script to use for bootstrapping this cluster.
    See :ref:`custom bootstrap methods documentation <custom_bootstrap>` for details.
    When ``initdb`` is specified revert to the default ``initdb`` command. ``initdb`` is also triggered when no ``method``
    parameter is present in the configuration file.
@@ -35,7 +35,7 @@ Bootstrap configuration
 -  **pg\_hba**: list of lines that you should add to pg\_hba.conf.
         -  **- host all all 0.0.0.0/0 md5**.
         -  **- host replication replicator 127.0.0.1/32 md5**: A line like this is required for replication.
--  **users**: Some additional users users which needs to be created after initializing new cluster
+-  **users**: Some additional users which need to be created after initializing new cluster
     -  **admin**: the name of user
         -  **password: zalando**:
         -  **options**: list of options for CREATE USER statement
@@ -54,8 +54,8 @@ Most of the parameters are optional, but you have to specify one of the **host**
 -  **port**: (optional) Consul port
 -  **scheme**: (optional) **http** or **https**, defaults to **http**
 -  **token**: (optional) ACL token
--  **verify** (optional) whether to verify the SSL certificate for HTTPS requests
--  **cacert**: (optional) The ca certificate. If pressent it will enable validation.
+-  **verify**: (optional) whether to verify the SSL certificate for HTTPS requests
+-  **cacert**: (optional) The ca certificate. If present it will enable validation.
 -  **cert**: (optional) file with the client certificate
 -  **key**: (optional) file with the client key. Can be empty if the key is part of **cert**.
 -  **dc**: (optional) Datacenter to communicate with. By default the datacenter of the host is used.
@@ -73,7 +73,7 @@ Most of the parameters are optional, but you have to specify one of the **host**
 -  **protocol**: (optional) http or https, if not specified http is used. If the **url** or **proxy** is specified - will take protocol from them.
 -  **username**: (optional) username for etcd authentication
 -  **password**: (optional) password for etcd authentication.
--  **cacert**: (optional) The ca certificate. If pressent it will enable validation.
+-  **cacert**: (optional) The ca certificate. If present it will enable validation.
 -  **cert**: (optional) file with the client certificate
 -  **key**: (optional) file with the client key. Can be empty if the key is part of **cert**.
 
@@ -93,7 +93,7 @@ Kubernetes
 -  **role\_label**: (optional) name of the label containing role (master or replica). Patroni will set this label on the pod it runs in. Default value is ``role``.
 -  **use\_endpoints**: (optional) if set to true, Patroni will use Endpoints instead of ConfigMaps to run leader elections and keep cluster state.
 -  **pod\_ip**: (optional) IP address of the pod Patroni is running in. This value is required when `use_endpoints` is enabled and is used to populate the leader endpoint subsets when the pod's PostgreSQL is promoted.
--  **ports**: (optional) if the Service object has the name for the port, the same name must appear in the Endpoint object, otherwise service wont work. For example, if your service is defined as ``{Kind: Service, spec: {ports: [{name: postgresql, port: 5432, targetPort: 5432}]}}``, then you have to set ``kubernetes.ports: {[{"name": "postgresql", "port": 5432}]}`` and Patroni will use it for updating subsets of the leader Endpoint. This parameter is used only if `kubernetes.use_endpoints` is set.
+-  **ports**: (optional) if the Service object has the name for the port, the same name must appear in the Endpoint object, otherwise service won't work. For example, if your service is defined as ``{Kind: Service, spec: {ports: [{name: postgresql, port: 5432, targetPort: 5432}]}}``, then you have to set ``kubernetes.ports: {[{"name": "postgresql", "port": 5432}]}`` and Patroni will use it for updating subsets of the leader Endpoint. This parameter is used only if `kubernetes.use_endpoints` is set.
 
 .. _postgresql_settings:
 
@@ -118,12 +118,12 @@ PostgreSQL
    own config item. See :ref:`custom replica creation methods documentation <custom_replica_creation>` for further explanation.
 -  **data\_dir**: The location of the Postgres data directory, either existing or to be initialized by Patroni.
 -  **config\_dir**: The location of the Postgres configuration directory, defaults to the data directory. Must be writable by Patroni.
--  **bin\_dir**: Path to PostgreSQL binaries. (pg_ctl, pg_rewind, pg_basebackup, postgres) The  default value is an empty string meaning that PATH environment variable will be used to find the executables.
+-  **bin\_dir**: Path to PostgreSQL binaries (pg_ctl, pg_rewind, pg_basebackup, postgres). The default value is an empty string meaning that PATH environment variable will be used to find the executables.
 -  **listen**: IP address + port that Postgres listens to; must be accessible from other nodes in the cluster, if you're using streaming replication. Multiple comma-separated addresses are permitted, as long as the port component is appended after to the last one with a colon, i.e. ``listen: 127.0.0.1,127.0.0.2:5432``. Patroni will use the first address from this list to establish local connections to the PostgreSQL node.
 -  **use\_unix\_socket**: specifies that Patroni should prefer to use unix sockets to connect to the cluster. Default value is ``false``. If ``unix_socket_directories`` is definded, Patroni will use first suitable value from it to connect to the cluster and fallback to tcp if nothing is suitable. If ``unix_socket_directories`` is not specified in ``postgresql.parameters``, Patroni will assume that default value should be used and omit ``host`` from connection parameters.
 -  **pgpass**: path to the `.pgpass <https://www.postgresql.org/docs/current/static/libpq-pgpass.html>`__ password file. Patroni creates this file before executing pg\_basebackup, the post_init script and under some other circumstances. The location must be writable by Patroni.
 -  **recovery\_conf**: additional configuration settings written to recovery.conf when configuring follower.
--  **custom\_conf** : path to an optional custom ``postgresql.conf`` file, that will be used in place of ``postgresql.base.conf``. The file must exist on all cluster nodes, be readable by PostgreSQL and will be included from its location on the real ``postgresql.conf``. Note that Patroni will not monitor this file for changes, nor backup it. However, its settings can still be overriden by Patroni's own configuration facilities - see `dynamic configuration <https://github.com/zalando/patroni/blob/master/docs/dynamic_configuration.rst>`__ for details.
+-  **custom\_conf** : path to an optional custom ``postgresql.conf`` file, that will be used in place of ``postgresql.base.conf``. The file must exist on all cluster nodes, be readable by PostgreSQL and will be included from its location on the real ``postgresql.conf``. Note that Patroni will not monitor this file for changes, nor backup it. However, its settings can still be overridden by Patroni's own configuration facilities - see :ref:`dynamic configuration <dynamic_configuration>` for details.
 -  **parameters**: list of configuration settings for Postgres. Many of these are required for replication to work.
 -  **pg\_hba**: list of lines that Patroni will use to generate ``pg_hba.conf``. This parameter has higher priority than ``bootstrap.pg_hba``. Together with :ref:`dynamic configuration <dynamic_configuration>` it simplifies management of ``pg_hba.conf``.
         -  **- host all all 0.0.0.0/0 md5**.
@@ -131,7 +131,7 @@ PostgreSQL
 -  **pg\_ctl\_timeout**: How long should pg_ctl wait when doing ``start``, ``stop`` or ``restart``. Default value is 60 seconds.
 -  **use\_pg\_rewind**: try to use pg\_rewind on the former leader when it joins cluster as a replica.
 -  **remove\_data\_directory\_on\_rewind\_failure**: If this option is enabled, Patroni will remove postgres data directory and recreate replica. Otherwise it will try to follow the new leader. Default value is **false**.
--  **replica\_method** for each create_replica_method other than basebackup, you would add a configuration section of the same name. At a minimum, this should include "command" with a full path to the actual script to be executed.  Other configuration parameters will be passed along to the script in the form "parameter=value".
+-  **replica\_method**: for each create_replica_method other than basebackup, you would add a configuration section of the same name. At a minimum, this should include "command" with a full path to the actual script to be executed. Other configuration parameters will be passed along to the script in the form "parameter=value".
 
 REST API
 -------- 
@@ -151,6 +151,6 @@ ZooKeeper
 
 Watchdog
 --------
-- **mode**: ``off``, ``automatic`` or ``required``. When ``off`` watchdog is disabled. When ``automatic`` watchdog will be used if available, but ignored if it is not. When ``required`` the node will not become a leader unless watchdog can be succesfully enabled.
+- **mode**: ``off``, ``automatic`` or ``required``. When ``off`` watchdog is disabled. When ``automatic`` watchdog will be used if available, but ignored if it is not. When ``required`` the node will not become a leader unless watchdog can be successfully enabled.
 - **device**: Path to watchdog device. Defaults to ``/dev/watchdog``.
 - **safety_margin**: Number of seconds of safety margin between watchdog triggering and leader key expiration.

docs/dynamic_configuration.rst

@@ -16,9 +16,9 @@ Patroni configuration is stored in the DCS (Distributed Configuration Store). Th
 	These options are defined in the configuration file and take precedence over dynamic configuration.
 	patroni.yml could be changed and reload in runtime (without restart of Patroni) by sending SIGHUP to the Patroni process or by performing ``POST /reload`` REST-API request.
 
-- Environment :ref:`configuration <environment>` .
+- Environment :ref:`configuration <environment>`.
 	It is possible to set/override some of the "Local" configuration parameters with environment variables.
-	Environment configuration is very useful when you are running in a dynamic environment and you don't know some of the parameters in advance (for example it's not possible to know you external IP address when you are running inside ``docker``).
+	Environment configuration is very useful when you are running in a dynamic environment and you don't know some of the parameters in advance (for example it's not possible to know your external IP address when you are running inside ``docker``).
 
 Some of the PostgreSQL parameters must hold the same values on the master and the replicas. For those, values set either in the local patroni configuration files or via the environment variables take no effect. To alter or set their values one must change the shared configuration in the DCS. Below is the actual list of such parameters together with the default values:
 
@@ -42,7 +42,7 @@ There are some other Postgres parameters controlled by Patroni:
 
 - listen_addresses - is set either from ``postgresql.listen`` or from ``PATRONI_POSTGRESQL_LISTEN`` environment variable
 - port - is set either from ``postgresql.listen`` or from ``PATRONI_POSTGRESQL_LISTEN`` environment variable
-- cluster_name - is set either from ``scope`` or from ``PATRRONI_SCOPE`` environment variable
+- cluster_name - is set either from ``scope`` or from ``PATRONI_SCOPE`` environment variable
 - hot_standby: on
 
 To be on the safe side parameters from the above lists are not written into ``postgresql.conf``, but passed as a list of arguments to the ``pg_ctl start`` which gives them the highest precedence, even above `ALTER SYSTEM <https://www.postgresql.org/docs/current/static/sql-altersystem.html>`__

docs/kubernetes.rst

@@ -45,7 +45,7 @@ Examples
 - You can find the full-featured Docker image that can use Persistent Volumes in the
   `Spilo Project <https://github.com/zalando/spilo>`_.
 
-- There is also a `Helm chart <https://github.com/unguiculus/charts/tree/feature/patroni/incubator/patroni>`_
+- There is also a `Helm chart <https://github.com/kubernetes/charts/tree/master/incubator/patroni>`_
   to deploy the Spilo image configured with Patroni running using Kubernetes.
 
 - In order to run your database clusters at scale using Patroni and Spilo, take a look at the

docs/pause.rst

@@ -25,7 +25,7 @@ When Patroni runs in a paused mode, it does not change the state of PostgreSQL,
 
 - If there is no leader lock in the cluster, the running master acquires the lock. If there is more than one master node, then the first master to acquire the lock wins. If there are no masters altogether, Patroni does not try to promote any replicas. There is an exception in this rule: if there is no leader lock because the old master has demoted itself due to the manual promotion, then only the candidate node mentioned in the promotion request may take the leader lock. When the new leader lock is granted (i.e. after promoting a replica manually), Patroni makes sure the replicas that were streaming from the previous leader will switch to the new one.
 
-- When Postgres is stopped, Patroni does not try to start it. When Patroni is stopped, it does not to stop Postgres instance it is managing.
+- When Postgres is stopped, Patroni does not try to start it. When Patroni is stopped, it does not try to stop the Postgres instance it is managing.
 
 User guide
 ----------

docs/releases.rst

@@ -3,6 +3,94 @@
 Release notes
 =============
 
+Version 1.4.4
+-------------
+
+**Stability improvements**
+
+- Fix race condition in poll_failover_result (Alexander Kukushkin)
+
+  It didn't affect directly neither failover nor switchover, but in some rare cases it was reporting success too early, when the former leader released the lock, producing a 'Failed over to "None"' instead of 'Failed over to "desired-node"' message.
+
+- Treat Postgres parameter names as case insensitive (Alexander)
+
+  Most of the Postgres parameters have snake_case names, but there are three exceptions from this rule: DateStyle, IntervalStyle and TimeZone. Postgres accepts those parameters when written in a different case (e.g. timezone = 'some/tzn'); however, Patroni was unable to find case-insensitive matches of those parameter names in pg_settings and ignored such parameters as a result.
+
+- Abort start if attaching to running postgres and cluster not initialized (Alexander)
+
+  Patroni can attach itself to an already running Postgres instance. It is imperative to start running Patroni on the master node before getting to the replicas.
+
+- Fix behavior of patronictl scaffold (Alexander)
+
+  Pass dict object to touch_member instead of json encoded string, DCS implementation will take care of encoding it.
+
+- Don't demote master if failed to update leader key in pause (Alexander)
+
+  During maintenance a DCS may start failing write requests while continuing to responds to read ones. In that case, Patroni used to put the Postgres master node to a read-only mode after failing to update the leader lock in DCS.
+
+- Sync replication slots when Patroni notices a new postmaster process (Alexander)
+
+  If Postgres has been restarted, Patroni has to make sure that list of replication slots matches its expectations.
+
+- Verify sysid and sync replication slots after coming out of pause (Alexander)
+
+  During the `maintenance` mode it may happen that data directory was completely rewritten and therefore we have to make sure that `Database system identifier` still belongs to our cluster and replication slots are in sync with Patroni expectations.
+
+- Fix a possible failure to start not running Postgres on a data directory with postmaster lock file present (Alexander)
+
+  Detect reuse of PID from the postmaster lock file. More likely to hit such problem if you run Patroni and Postgres in the docker container.
+
+- Improve protection of DCS being accidentally wiped (Alexander)
+
+  Patroni has a lot of logic in place to prevent failover in such case; it can also restore all keys back; however, until this change an accidental removal of /config key was switching off pause mode for 1 cycle of HA loop.
+
+- Do not exit when encountering invalid system ID (Oleksii Kliukin)
+
+  Do not exit when the cluster system ID is empty or the one that doesn't pass the validation check. In that case, the cluster most likely needs a reinit; mention it in the result message. Avoid terminating Patroni, as otherwise reinit cannot happen.
+
+**Compatibility with Kubernetes 1.10+**
+
+- Added check for empty subsets (Cody Coons)
+
+  Kubernetes 1.10.0+ started returning `Endpoints.subsets` set to `None` instead of `[]`.
+
+**Bootstrap improvements**
+
+- Make deleting recovery.conf optional (Brad Nicholson)
+
+  If `bootstrap.<custom_bootstrap_method_name>.keep_existing_recovery_conf` is defined and set to ``True``, Patroni will not remove the existing ``recovery.conf`` file. This is useful when bootstrapping from a backup with tools like pgBackRest that generate the appropriate `recovery.conf` for you.
+
+- Allow options to the basebackup built-in method (Oleksii)
+
+  It is now possible to supply options to the built-in basebackup method by defining the `basebackup` section in the configuration, similar to how those are defined for custom replica creation methods. The difference is in the format accepted by the `basebackup` section: since pg_basebackup accepts both `--key=value` and `--key` options, the contents of the section could be either a dictionary of key-value pairs, or a list of either one-element dictionaries or just keys (for the options that don't accept values). See :ref:`replica creation method <custom_replica_creation>` section for additional examples.
+
+
+Version 1.4.3
+-------------
+
+**Improvements in logging**
+
+- Make log level configurable from environment variables (Andy Newton, Keyvan Hedayati)
+
+  `PATRONI_LOGLEVEL` - sets the general logging level
+  `PATRONI_REQUESTS_LOGLEVEL` - sets the logging level for all HTTP requests e.g. Kubernetes API calls
+  See `the docs for Python logging <https://docs.python.org/3.6/library/logging.html#levels>` to get the names of possible log levels
+
+**Stability improvements and bug fixes**
+
+- Don't rediscover etcd cluster topology when watch timed out (Alexander Kukushkin)
+
+  If we have only one host in etcd configuration and exactly this host is not accessible, Patroni was starting discovery of cluster topology and never succeeding. Instead it should just switch to the next available node.
+
+- Write content of bootstrap.pg_hba into a pg_hba.conf after custom bootstrap (Alexander)
+
+  Now it behaves similarly to the usual bootstrap with `initdb`
+
+- Single user mode was waiting for user input and never finish (Alexander)
+
+  Regression was introduced in https://github.com/zalando/patroni/pull/576
+
+
 Version 1.4.2
 -------------
 

docs/replica_bootstrap.rst

@@ -3,7 +3,7 @@ Replica imaging and bootstrap
 
 Patroni allows customizing creation of a new replica. It also supports defining what happens when the new empty cluster
 is being bootstrapped. The distinction between two is well defined: Patroni creates replicas only if the ``initialize``
-key is present in Etcd for the cluster. If there is no ``initialize`` key - Patroni calls bootstrap exclusively on the
+key is present in DCS for the cluster. If there is no ``initialize`` key - Patroni calls bootstrap exclusively on the
 first node that takes the initialize key lock.
 
 .. _custom_bootstrap:
@@ -23,6 +23,7 @@ arguments to them, i.e. the name of the cluster and the path to the data directo
         method: <custom_bootstrap_method_name>
         <custom_bootstrap_method_name>:
             command: <path_to_custom_bootstrap_script> [param1 [, ...]]
+            keep_existing_recovery_conf: False
             recovery_conf:
                 recovery_target_action: promote
                 recovery_target_timeline: latest
@@ -47,6 +48,9 @@ If a ``recovery_conf`` block is defined in the same section as the custom bootst
 ``recovery.conf`` before starting the newly bootstrapped instance. Typically, such recovery.conf should contain at least
 one of the ``recovery_target_*`` parameters, together with the ``recovery_target_timeline`` set to ``promote``.
 
+If ``keep_existing_recovery_conf`` is defined and set to ``True``, Patroni will not remove the existing ``recovery.conf`` file if it exists.
+This is useful when bootstrapping from a backup with tools like pgBackRest that generate the appropriate ``recovery.conf`` for you.
+
  .. note:: Bootstrap methods are neither chained, nor fallen-back to the default one in case the primary one fails
 
 
@@ -72,13 +76,14 @@ scripts to clone a new replica. Those are configured in the ``postgresql`` confi
             no_master: 1
             envdir: {{WALE_ENV_DIR}}
             use_iam: 1
+        basebackup:
+            max-rate: '100M'
 
 
 The ``create_replica_method`` defines available replica creation methods and the order of executing them. Patroni will
-stop on the first one that returns 0. The basebackup is the built-in method and doesn't require any configuration. The
-rest of the methods should define a separate section in the configuration file, listing the command to execute and any
-custom parameters that should be passed to that command. All parameters will be passed in a ``--name=value`` format.
-Besides user-defined parameters, Patroni supplies a couple of cluster-specific ones:
+stop on the first one that returns 0. Each method should define a separate section in the configuration file, listing the command
+to execute and any custom parameters that should be passed to that command. All parameters will be passed in a
+``--name=value`` format. Besides user-defined parameters, Patroni supplies a couple of cluster-specific ones:
 
 --scope
     Which cluster this replica belongs to
@@ -94,4 +99,32 @@ A special ``no_master`` parameter, if defined, allows Patroni to call the replic
 running master or replicas. In that case, an empty string will be passed in a connection string. This is useful for
 restoring the formerly running cluster from the binary backup.
 
+A ``basebackup`` method is a special case: it will be used if ``create_replica_method`` is empty, although it is possible
+to list it explicitly among the ``create_replica_method`` methods. This method initializes a new replica with the
+``pg_basebackup``, the base backup is taken from the master unless there are replicas with ``clonefrom`` tag, in which case one
+of such replicas will be used as the origin for pg_basebackup. It works without any configuration; however, it is
+possible to specify a ``basebackup`` configuration section. Same rules as with the other method configuration apply,
+namely, only long (with --) options should be specified there. Not all parameters make sense, if you override a connection
+string or provide an option to created tar-ed or compressed base backups, patroni won't be able to make a replica out
+of it. There is no validation performed on the names or values of the parameters passed to the ``basebackup`` section.
+You can specify basebackup parameters as either a map (key-value pairs) or a list of elements, where each element
+could be either a key-value pair or a single key (for options that does not receive any values, for instance, ``--verbose``).
+Consider those 2 examples:
+
+.. code:: YAML
+
+    postgresql:
+        basebackup:
+            max-rate: '100M'
+            checkpoint: 'fast'
+
+and
+
+.. code:: YAML
+
+    postgresql:
+        basebackup:
+            - verbose
+            - max-rate: '100M'
+
 If all replica creation methods fail, Patroni will try again all methods in order during the next event loop cycle.

docs/replication_modes.rst

@@ -9,9 +9,9 @@ Patroni uses PostgreSQL streaming replication. For more information about stream
 Asynchronous mode durability
 ----------------------------
 
-In asynchronous mode the cluster is allowed to lose some committed transactions to ensure availability. When master server fails or becomes unavailable for any other reason Patroni will automatically promote a sufficiently healthy standby to master. Any transactions that have not been replicated to that standby remain in a "forked timeline" on the master, and are effectively unrecoverable [1]_.
+In asynchronous mode the cluster is allowed to lose some committed transactions to ensure availability. When the primary server fails or becomes unavailable for any other reason Patroni will automatically promote a sufficiently healthy standby to primary. Any transactions that have not been replicated to that standby remain in a "forked timeline" on the primary, and are effectively unrecoverable [1]_.
 
-The amount of transactions that can be lost is controlled via ``maximum_lag_on_failover`` parameter. Because master transaction log position is not sampled in real time, in reality the amount of lost data on failover is worst case bounded by  ``maximum_lag_on_failover`` bytes of transaction log plus the amount that is written in the last ``ttl`` seconds (``loop_wait``/2 seconds in the average case). However typical steady state replication delay is well under a second.
+The amount of transactions that can be lost is controlled via ``maximum_lag_on_failover`` parameter. Because the primary transaction log position is not sampled in real time, in reality the amount of lost data on failover is worst case bounded by  ``maximum_lag_on_failover`` bytes of transaction log plus the amount that is written in the last ``ttl`` seconds (``loop_wait``/2 seconds in the average case). However typical steady state replication delay is well under a second.
 
 PostgreSQL synchronous replication
 ----------------------------------
@@ -20,7 +20,7 @@ You can use Postgres's `synchronous replication <http://www.postgresql.org/docs/
 
 In hosted datacenter environments (like AWS, Rackspace, or any network you do not control), synchronous replication significantly increases the variability of write performance. If followers become inaccessible from the leader, the leader effectively becomes read-only.
 
-To enable a simple synchronous replication test, add the follow lines to the ``parameters`` section of your YAML configuration files:
+To enable a simple synchronous replication test, add the following lines to the ``parameters`` section of your YAML configuration files:
 
 .. code:: YAML
 
@@ -29,18 +29,18 @@ To enable a simple synchronous replication test, add the follow lines to the ``p
 
 When using PostgreSQL synchronous replication, use at least three Postgres data nodes to ensure write availability if one host fails.
 
-Using PostgreSQL synchronous replication does not guarantee zero lost transactions under all circumstances. When master and standby that is currently acting as synchronous fail simultaneously a third node that might not contain all transactions will be promoted.
+Using PostgreSQL synchronous replication does not guarantee zero lost transactions under all circumstances. When the primary and the secondary that is currently acting as a synchronous replica fail simultaneously a third node that might not contain all transactions will be promoted.
 
 .. _synchronous_mode:
 
 Synchronous mode
 ----------------
 
-For use cases where losing committed transactions is not permissible you can turn on Patronis ``synchronous_mode``. When ``synchronous_mode`` is turned on Patroni will not promote a standby unless it is certain that the standby contains all transactions that may have returned a successful commit status to client [2]_. This means that the system may be unavailable for writes even though some servers are available. System administrators can still use manual failover commmands to promote a standby even if it results in transaction loss.
+For use cases where losing committed transactions is not permissible you can turn on Patroni's ``synchronous_mode``. When ``synchronous_mode`` is turned on Patroni will not promote a standby unless it is certain that the standby contains all transactions that may have returned a successful commit status to client [2]_. This means that the system may be unavailable for writes even though some servers are available. System administrators can still use manual failover commands to promote a standby even if it results in transaction loss.
 
-Turning on ``synchronous_mode`` does not guarantee multi node durability of commits under all circumstances. When no suitable standby is available, master server will still accept writes, but does not guarantee their replication. When the master fails in this mode no standby will be promote.  When the host that used to be master comes back it will get promoted automatically, unless system administrator performed a manual failover. This behavior makes synchronous mode usable with 2 node clusters.
+Turning on ``synchronous_mode`` does not guarantee multi node durability of commits under all circumstances. When no suitable standby is available, primary server will still accept writes, but does not guarantee their replication. When the primary fails in this mode no standby will be promoted. When the host that used to be the primary comes back it will get promoted automatically, unless system administrator performed a manual failover. This behavior makes synchronous mode usable with 2 node clusters.
 
-When ``synchronous_mode`` is on and a standby crashes, commits will block until next iteration of Patroni runs and switches master to standalone mode (worst case delay for writes ``ttl`` seconds, average case ``loop_wait``/2 seconds). Manually shutting down or restarting a standby will not cause a commit service interruption. Standby will signal the master to release itself from synchronous standby duties before PostgreSQL shutdown is initiated.
+When ``synchronous_mode`` is on and a standby crashes, commits will block until next iteration of Patroni runs and switches the primary to standalone mode (worst case delay for writes ``ttl`` seconds, average case ``loop_wait``/2 seconds). Manually shutting down or restarting a standby will not cause a commit service interruption. Standby will signal the primary to release itself from synchronous standby duties before PostgreSQL shutdown is initiated.
 
 You can ensure that a standby never becomes the synchronous standby by setting ``nosync`` tag to true. This is recommended to set for standbys that are behind slow network connections and would cause performance degradation when becoming a synchronous standby.
 
@@ -50,7 +50,7 @@ Synchronous mode can be switched on and off via Patroni REST interface. See :ref
 Synchronous mode implementation
 -------------------------------
 
-When in synchronous mode Patroni maintains synchronization state in the DCS, containing the latest master and current synchronous standby. This state is updated with strict ordering constraints to ensure the following invariants:
+When in synchronous mode Patroni maintains synchronization state in the DCS, containing the latest primary and current synchronous standby. This state is updated with strict ordering constraints to ensure the following invariants:
 
 - A node must be marked as the latest leader whenever it can accept write transactions. Patroni crashing or PostgreSQL not shutting down can cause violations of this invariant.
 
@@ -60,9 +60,9 @@ When in synchronous mode Patroni maintains synchronization state in the DCS, con
 
 Patroni will only ever assign one standby to ``synchronous_standby_names`` because with multiple candidates it is not possible to know which node was acting as synchronous during the failure.
 
-On each HA loop iteration Patroni re-evaluates synchronous standby choice. If the current synchronous standby is connected and has not requested its synchronous status to be removed it remains picked. Otherwise the cluster member avaiable for sync that is furthest ahead in replication is picked.
+On each HA loop iteration Patroni re-evaluates synchronous standby choice. If the current synchronous standby is connected and has not requested its synchronous status to be removed it remains picked. Otherwise the cluster member available for sync that is furthest ahead in replication is picked.
 
 
-.. [1] The data is still there, but recovering it requires a manual recovery effort by data recovery specialists. When Patroni is allowed to rewind with ``use_pg_rewind`` the forked timeline will be automatically erased to rejoin the failed master with the cluster.
+.. [1] The data is still there, but recovering it requires a manual recovery effort by data recovery specialists. When Patroni is allowed to rewind with ``use_pg_rewind`` the forked timeline will be automatically erased to rejoin the failed primary with the cluster.
 
 .. [2] Clients can change the behavior per transaction using PostgreSQL's ``synchronous_commit`` setting. Transactions with ``synchronous_commit`` values of ``off`` and ``local`` may be lost on fail over, but will not be blocked by replication delays.

docs/watchdog.rst

@@ -9,7 +9,7 @@ Having multiple PostgreSQL servers running as master can result in transactions
 
 - Shutting down PostgreSQL is too slow.
 
-- Patroni does not get to run due to high load on the system, th VM being paused by the hypervisor, or other infrastructure issues.
+- Patroni does not get to run due to high load on the system, the VM being paused by the hypervisor, or other infrastructure issues.
 
 To guarantee correct behavior under these conditions Patroni supports watchdog devices. Watchdog devices are software or hardware mechanisms that will reset the whole system when they do not get a keepalive heartbeat within a specified timeframe. This adds an additional layer of fail safe in case usual Patroni split-brain protection mechanisms fail.
 

patroni/__init__.py

@@ -135,8 +135,10 @@ class Patroni(object):
 
 def patroni_main():
     logformat = os.environ.get('PATRONI_LOGFORMAT', '%(asctime)s %(levelname)s: %(message)s')
-    logging.basicConfig(format=logformat, level=logging.INFO)
-    logging.getLogger('requests').setLevel(logging.WARNING)
+    loglevel = os.environ.get('PATRONI_LOGLEVEL', 'INFO')
+    requests_loglevel = os.environ.get('PATRONI_REQUESTS_LOGLEVEL', 'WARNING')
+    logging.basicConfig(format=logformat, level=loglevel)
+    logging.getLogger('requests').setLevel(requests_loglevel)
 
     patroni = Patroni()
     try:

patroni/api.py

@@ -295,22 +295,23 @@ class RestApiHandler(BaseHTTPRequestHandler):
             status_code = 503
         self._write_response(status_code, data)
 
-    def poll_failover_result(self, leader, candidate):
+    def poll_failover_result(self, leader, candidate, action):
         timeout = max(10, self.server.patroni.dcs.loop_wait)
         for _ in range(0, timeout*2):
             time.sleep(1)
             try:
                 cluster = self.server.patroni.dcs.get_cluster()
-                if cluster.leader and cluster.leader.name != leader:
+                if not cluster.is_unlocked() and cluster.leader.name != leader:
                     if not candidate or candidate == cluster.leader.name:
-                        return 200, 'Successfully failed over to "{0}"'.format(cluster.leader.name)
+                        return 200, 'Successfully {0}ed over to "{1}"'.format(action[:-4], cluster.leader.name)
                     else:
-                        return 200, 'Failed over to "{0}" instead of "{1}"'.format(cluster.leader.name, candidate)
+                        return 200, '{0}ed over to "{1}" instead of "{2}"'.format(action[:-4].title(),
+                                                                                  cluster.leader.name, candidate)
                 if not cluster.failover:
-                    return 503, 'Failover failed'
+                    return 503, action.title() + ' failed'
             except Exception as e:
-                logger.debug('Exception occured during polling failover result: %s', e)
-        return 503, 'Failover status unknown'
+                logger.debug('Exception occured during polling %s result: %s', action, e)
+        return 503, action.title() + ' status unknown'
 
     def is_failover_possible(self, cluster, leader, candidate, action):
         if leader and (not cluster.leader or cluster.leader.name != leader):
@@ -377,7 +378,8 @@ class RestApiHandler(BaseHTTPRequestHandler):
                     data = action.title() + ' scheduled'
                     status_code = 202
                 else:
-                    status_code, data = self.poll_failover_result(cluster.leader and cluster.leader.name, candidate)
+                    status_code, data = self.poll_failover_result(cluster.leader and cluster.leader.name,
+                                                                  candidate, action)
             else:
                 data = 'failed to write {0} key into DCS'.format(action)
                 status_code = 503

patroni/config.py

@@ -9,7 +9,8 @@ from collections import defaultdict
 from copy import deepcopy
 from patroni.dcs import ClusterConfig
 from patroni.postgresql import Postgresql
-from patroni.utils import deep_compare, parse_int, patch_config
+from patroni.utils import deep_compare, parse_bool, parse_int, patch_config
+from requests.structures import CaseInsensitiveDict
 
 logger = logging.getLogger(__name__)
 
@@ -47,7 +48,7 @@ class Config(object):
         'postgresql': {
             'bin_dir': '',
             'use_slots': True,
-            'parameters': {p: v[0] for p, v in Postgresql.CMDLINE_OPTIONS.items()}
+            'parameters': CaseInsensitiveDict({p: v[0] for p, v in Postgresql.CMDLINE_OPTIONS.items()})
         },
         'watchdog': {
             'mode': 'automatic',
@@ -87,6 +88,9 @@ class Config(object):
     def dynamic_configuration(self):
         return deepcopy(self._dynamic_configuration)
 
+    def check_mode(self, mode):
+        return bool(parse_bool(self._dynamic_configuration.get(mode)))
+
     def _load_config_file(self):
         """Loads config.yaml from filesystem and applies some values which were set via ENV"""
         with open(self._config_file) as f:

patroni/ctl.py

@@ -786,7 +786,7 @@ def touch_member(config, dcs):
         'role': p.role
     }
 
-    return dcs.touch_member(json.dumps(data, separators=(',', ':')), permanent=True)
+    return dcs.touch_member(data, permanent=True)
 
 
 def set_defaults(config, cluster_name):

patroni/dcs/__init__.py

@@ -11,6 +11,7 @@ import sys
 
 from collections import namedtuple
 from patroni.exceptions import PatroniException
+from patroni.utils import parse_bool
 from random import randint
 from six.moves.urllib_parse import urlparse, urlunparse, parse_qsl
 from threading import Event, Lock
@@ -349,14 +350,14 @@ class Cluster(namedtuple('Cluster', 'initialize,config,leader,last_leader_operat
         candidates = [m for m in self.members if m.clonefrom and m.is_running and m.name not in exclude]
         return candidates[randint(0, len(candidates) - 1)] if candidates else self.leader
 
+    def check_mode(self, mode):
+        return bool(self.config and parse_bool(self.config.data.get(mode)))
+
     def is_paused(self):
-        return self.config and self.config.data.get('pause', False) or False
+        return self.check_mode('pause')
 
     def is_synchronous_mode(self):
-        return bool(self.config and self.config.data.get('synchronous_mode'))
-
-    def is_synchronous_mode_strict(self):
-        return bool(self.config and self.config.data.get('synchronous_mode_strict'))
+        return self.check_mode('synchronous_mode')
 
 
 @six.add_metaclass(abc.ABCMeta)

patroni/dcs/etcd.py

@@ -210,8 +210,11 @@ class Client(etcd.Client):
                 self._machines_cache = self.machines
                 if self._base_uri in self._machines_cache:
                     self._machines_cache.remove(self._base_uri)
-        except etcd.EtcdConnectionFailed:
-            self._update_machines_cache = True
+        except etcd.EtcdConnectionFailed as e:
+            if isinstance(e, etcd.EtcdWatchTimedOut) and self._machines_cache:
+                self._base_uri = self._next_server()
+            else:
+                self._update_machines_cache = True
             if not response:
                 raise
         return self._handle_server_response(response)

patroni/dcs/kubernetes.py

@@ -161,7 +161,7 @@ class Kubernetes(AbstractDCS):
             leader = nodes.get(self.leader_path)
             metadata = leader and leader.metadata
             self._leader_resource_version = metadata.resource_version if metadata else None
-            self._leader_observed_subsets = leader.subsets if self.__subsets and leader else []
+            self._leader_observed_subsets = leader.subsets if self.__subsets and leader and leader.subsets else []
             annotations = metadata and metadata.annotations or {}
 
             # get last leader operation

patroni/ha.py

@@ -57,6 +57,7 @@ class Ha(object):
         self.dcs = patroni.dcs
         self.cluster = None
         self.old_cluster = None
+        self._was_paused = False
         self._leader_timeline = None
         self.recovering = False
         self._post_bootstrap_task = None
@@ -72,8 +73,19 @@ class Ha(object):
         # standby. Changes protected by _member_state_lock.
         self._disable_sync = 0
 
+        # We need following property to avoid shutdown of postgres when join of Patroni to the postgres
+        # already running as replica was aborted due to cluster not beeing initialized in DCS.
+        self._join_aborted = False
+
+    def check_mode(self, mode):
+        # Try to protect from the case when DCS was wiped out during pause
+        if self.cluster and self.cluster.config and self.cluster.config.modify_index:
+            return self.cluster.check_mode(mode)
+        else:
+            return self.patroni.config.check_mode(mode)
+
     def is_paused(self):
-        return self.cluster and self.cluster.is_paused()
+        return self.check_mode('pause')
 
     def load_cluster_from_dcs(self):
         cluster = self.dcs.get_cluster()
@@ -284,10 +296,10 @@ class Ha(object):
         return follow_reason
 
     def is_synchronous_mode(self):
-        return bool(self.cluster and self.cluster.is_synchronous_mode())
+        return self.check_mode('synchronous_mode')
 
     def is_synchronous_mode_strict(self):
-        return bool(self.cluster and self.cluster.is_synchronous_mode_strict())
+        return self.check_mode('synchronous_mode_strict')
 
     def process_sync_replication(self):
         """Process synchronous standby beahvior.
@@ -760,6 +772,8 @@ class Ha(object):
                 # Either there is no connection to DCS or someone else acquired the lock
                 logger.error('failed to update leader lock')
                 if self.state_handler.is_leader():
+                    if self.is_paused():
+                        return 'continue to run as master after failing to update leader lock in DCS'
                     self.demote('immediate-nolock')
                     return 'demoted self because failed to update leader lock in DCS'
                 else:
@@ -1034,6 +1048,11 @@ class Ha(object):
 
             if self.is_paused():
                 self.watchdog.disable()
+                self._was_paused = True
+            else:
+                if self._was_paused:
+                    self.state_handler.schedule_sanity_checks_after_pause()
+                self._was_paused = False
 
             if not self.cluster.has_member(self.state_handler.name):
                 self.touch_member()
@@ -1079,9 +1098,20 @@ class Ha(object):
                 return self.bootstrap()  # new node
             # "bootstrap", but data directory is not empty
             elif not self.sysid_valid(self.cluster.initialize) and self.cluster.is_unlocked() and not self.is_paused():
+                if not self.state_handler.cb_called and self.state_handler.is_running() \
+                        and not self.state_handler.is_leader():
+                    self._join_aborted = True
+                    logger.error('No initialize key in DCS and PostgreSQL is running as replica, aborting start')
+                    logger.error('Please first start Patroni on the node running as master')
+                    sys.exit(1)
                 self.dcs.initialize(create_new=(self.cluster.initialize is None), sysid=self.state_handler.sysid)
             else:
                 # check if we are allowed to join
+                data_sysid = self.state_handler.sysid
+                if not self.sysid_valid(data_sysid):
+                    # data directory is not empty, but no valid sysid, cluster must be broken, suggest reinit
+                    return "data dir for the cluster is not empty, but system ID is invalid; consider doing reinitalize"
+
                 if self.sysid_valid(self.cluster.initialize) and self.cluster.initialize != self.state_handler.sysid:
                     logger.fatal("system ID mismatch, node %s belongs to a different cluster: %s != %s",
                                  self.state_handler.name, self.cluster.initialize, self.state_handler.sysid)
@@ -1138,7 +1168,7 @@ class Ha(object):
         if self.is_paused():
             logger.info('Leader key is not deleted and Postgresql is not stopped due paused state')
             self.watchdog.disable()
-        else:
+        elif not self._join_aborted:
             # FIXME: If stop doesn't reach safepoint quickly enough keepalive is triggered. If shutdown checkpoint
             # takes longer than ttl, then leader key is lost and replication might not have sent out all xlog.
             # This might not be the desired behavior of users, as a graceful shutdown of the host can mean lost data.

patroni/postgresql.py

@@ -15,6 +15,7 @@ from patroni.callback_executor import CallbackExecutor
 from patroni.exceptions import PostgresConnectionException, PostgresException
 from patroni.utils import compare_values, parse_bool, parse_int, Retry, RetryFailedError, polling_loop, split_host_port
 from patroni.postmaster import PostmasterProcess
+from requests.structures import CaseInsensitiveDict
 from six import string_types
 from six.moves.urllib.parse import quote_plus
 from threading import current_thread, Lock
@@ -86,7 +87,7 @@ class Postgresql(object):
     #    default_value -- some sane default value
     #    check_function -- if the new value is not correct must return `!False`
     #    min_version -- major version of PostgreSQL when parameter was introduced
-    CMDLINE_OPTIONS = {
+    CMDLINE_OPTIONS = CaseInsensitiveDict({
         'listen_addresses': (None, lambda _: False, 90100),
         'port': (None, lambda _: False, 90100),
         'cluster_name': (None, lambda _: False, 90500),
@@ -101,7 +102,7 @@ class Postgresql(object):
         'max_replication_slots': (10, lambda v: int(v) >= 10, 90400),
         'max_worker_processes': (8, lambda v: int(v) >= 8, 90400),
         'wal_log_hints': ('on', lambda _: False, 90400)
-    }
+    })
 
     _CONFIG_WARNING_HEADER = '# Do not edit this file manually!\n# It will be overwritten by Patroni!\n'
 
@@ -140,7 +141,6 @@ class Postgresql(object):
         self._postgresql_base_conf = os.path.join(self._config_dir, self._postgresql_base_conf_name)
         self._pg_hba_conf = os.path.join(self._config_dir, 'pg_hba.conf')
         self._recovery_conf = os.path.join(self._data_dir, 'recovery.conf')
-        self._postmaster_pid = os.path.join(self._data_dir, 'postmaster.pid')
         self._trigger_file = config.get('recovery_conf', {}).get('trigger_file') or 'promote'
         self._trigger_file = os.path.abspath(os.path.join(self._data_dir, self._trigger_file))
 
@@ -241,11 +241,9 @@ class Postgresql(object):
                 parameters['synchronous_standby_names'] = self._synchronous_standby_names
         if self._major_version >= 90600 and parameters['wal_level'] == 'hot_standby':
             parameters['wal_level'] = 'replica'
-        ret = {k: v for k, v in parameters.items() if not self._major_version or
-               self._major_version >= self.CMDLINE_OPTIONS.get(k, (0, 1, 90100))[2]}
-        for k in ('hba_file', 'ident_file'):
-            if k in ret:
-                ret[k] = os.path.join(self._config_dir, ret[k])
+        ret = CaseInsensitiveDict({k: v for k, v in parameters.items() if not self._major_version or
+                                   self._major_version >= self.CMDLINE_OPTIONS.get(k, (0, 1, 90100))[2]})
+        ret.update({k: os.path.join(self._config_dir, ret[k]) for k in ('hba_file', 'ident_file') if k in ret})
         return ret
 
     def resolve_connection_addresses(self):
@@ -310,16 +308,16 @@ class Postgresql(object):
 
         conf_changed = hba_changed = local_connection_address_changed = pending_restart = False
         if self.state == 'running':
-            changes = {p: v for p, v in server_parameters.items() if '.' not in p}
-            changes.update({p: None for p, v in self._server_parameters.items() if not ('.' in p or p in changes)})
+            changes = CaseInsensitiveDict({p: v for p, v in server_parameters.items() if '.' not in p})
+            changes.update({p: None for p in self._server_parameters.keys() if not ('.' in p or p in changes)})
             if changes:
                 if 'wal_segment_size' not in changes:
                     changes['wal_segment_size'] = '16384kB'
                 # XXX: query can raise an exception
                 for r in self.query("""SELECT name, setting, unit, vartype, context
                                          FROM pg_settings
-                                        WHERE name IN (""" + ', '.join(['%s'] * len(changes)) + """)
-                                        ORDER BY 1 DESC""", *(list(changes.keys()))):
+                                        WHERE LOWER(name) IN (""" + ', '.join(['%s'] * len(changes)) + """)
+                                        ORDER BY 1 DESC""", *(k.lower() for k in changes.keys())):
                     if r[4] == 'internal':
                         if r[0] == 'wal_segment_size':
                             server_parameters.pop(r[0], None)
@@ -500,28 +498,44 @@ class Postgresql(object):
         return not os.path.exists(self._data_dir) or os.listdir(self._data_dir) == []
 
     @staticmethod
-    def initdb_allowed_option(name):
-        if name in ['pgdata', 'nosync', 'pwfile', 'sync-only']:
-            raise Exception('{0} option for initdb is not allowed'.format(name))
-        return True
+    def process_user_options(tool, options, not_allowed_options, error_handler):
+        user_options = []
 
-    def get_initdb_options(self, config):
-        options = []
-        for o in config:
-            if isinstance(o, string_types) and self.initdb_allowed_option(o):
-                options.append('--{0}'.format(o))
-            elif isinstance(o, dict):
-                keys = list(o.keys())
-                if len(keys) != 1 or not isinstance(keys[0], string_types) or not self.initdb_allowed_option(keys[0]):
-                    raise Exception('Invalid option: {0}'.format(o))
-                options.append('--{0}={1}'.format(keys[0], o[keys[0]]))
-            else:
-                raise Exception('Unknown type of initdb option: {0}'.format(o))
-        return options
+        def option_is_allowed(name):
+            ret = name not in not_allowed_options
+            if not ret:
+                error_handler('{0} option for {1} is not allowed'.format(name, tool))
+            return ret
+
+        if isinstance(options, dict):
+            for k, v in options.items():
+                if k and v:
+                    user_options.append('--{0}={1}'.format(k, v))
+        elif isinstance(options, list):
+            for opt in options:
+                if isinstance(opt, string_types) and option_is_allowed(opt):
+                    user_options.append('--{0}'.format(opt))
+                elif isinstance(opt, dict):
+                    keys = list(opt.keys())
+                    if len(keys) != 1 or not isinstance(opt[keys[0]], string_types) or not option_is_allowed(keys[0]):
+                        error_handler('Error when parsing {0} key-value option {1}: only one key-value is allowed'
+                                      ' and value should be a string'.format(tool, opt[keys[0]]))
+                    user_options.append('--{0}={1}'.format(keys[0], opt[keys[0]]))
+                else:
+                    error_handler('Error when parsing {0} option {1}: value should be string value'
+                                  ' or a single key-value pair'.format(tool, opt))
+        else:
+            error_handler('{0} options must be list ot dict'.format(tool))
+        return user_options
 
     def _initdb(self, config):
         self.set_state('initalizing new cluster')
-        options = self.get_initdb_options(config.get('initdb') or [])
+        not_allowed_options = ('pgdata', 'nosync', 'pwfile', 'sync-only', 'version')
+
+        def error_handler(e):
+            raise Exception(e)
+
+        options = self.process_user_options('initdb', config.get('initdb') or [], not_allowed_options, error_handler)
         pwfile = None
 
         if self._superuser:
@@ -537,12 +551,7 @@ class Postgresql(object):
         ret = self.pg_ctl('initdb', *options)
         if pwfile:
             os.remove(pwfile)
-        if ret:
-            if not self._server_parameters.get('hba_file') and not self.config.get('pg_hba'):
-                self.write_pg_hba(config.get('pg_hba', []))
-            self._major_version = self.get_major_version()
-            self._server_parameters = self.get_server_parameters(self.config)
-        else:
+        if not ret:
             self.set_state('initdb failed')
         return ret
 
@@ -558,11 +567,11 @@ class Postgresql(object):
             logger.exception('Exception during custom bootstrap')
             return False
         self._post_restore()
-        self.save_configuration_files()
 
         if 'recovery_conf' in config:
             self.write_recovery_conf(config['recovery_conf'])
-        elif os.path.isfile(self._recovery_conf) or os.path.islink(self._recovery_conf):
+        elif (os.path.isfile(self._recovery_conf) or os.path.islink(self._recovery_conf)) and \
+                not config.get('keep_existing_recovery_conf'):
             os.unlink(self._recovery_conf)
         return True
 
@@ -666,7 +675,7 @@ class Postgresql(object):
                     break
             # if the method is basebackup, then use the built-in
             if replica_method == "basebackup":
-                ret = self.basebackup(connstring, env)
+                ret = self.basebackup(connstring, env, self.config.get(replica_method, {}))
                 if ret == 0:
                     logger.info("replica has been created using basebackup")
                     # if basebackup succeeds, exit with success
@@ -679,7 +688,7 @@ class Postgresql(object):
                 method_config = {}
                 # user-defined method; check for configuration
                 # not required, actually
-                if replica_method in self.config:
+                if self.config.get(replica_method, {}):
                     method_config = self.config[replica_method].copy()
                     # look to see if the user has supplied a full command path
                     # if not, use the method name as the command
@@ -732,27 +741,18 @@ class Postgresql(object):
 
     def is_running(self):
         """Returns PostmasterProcess if one is running on the data directory or None. If most recently seen process
-        is running udpates the cached process based on pid file."""
+        is running updates the cached process based on pid file."""
         if self._postmaster_proc:
             if self._postmaster_proc.is_running():
                 return self._postmaster_proc
             self._postmaster_proc = None
 
-        self._postmaster_proc = PostmasterProcess.from_pidfile(self._read_pid_file())
+        # we noticed that postgres was restarted, force syncing of replication
+        self._schedule_load_slots = self.use_slots
+
+        self._postmaster_proc = PostmasterProcess.from_pidfile(self._data_dir)
         return self._postmaster_proc
 
-    def _read_pid_file(self):
-        """Reads and parses postmaster.pid from the data directory
-
-        :returns dictionary of values if successful, empty dictionary otherwise
-        """
-        pid_line_names = ['pid', 'data_dir', 'start_time', 'port', 'socket_dir', 'listen_addr', 'shmem_key']
-        try:
-            with open(self._postmaster_pid) as f:
-                return {name: line.rstrip("\n") for name, line in zip(pid_line_names, f)}
-        except IOError:
-            return {}
-
     @property
     def cb_called(self):
         return self.__cb_called
@@ -990,7 +990,7 @@ class Postgresql(object):
 
         Should only be called when state == 'starting'
 
-        :returns: True iff state was changed from 'starting'
+        :returns: True if state was changed from 'starting'
         """
         ready = self.pg_isready()
 
@@ -1080,8 +1080,10 @@ class Postgresql(object):
         return True
 
     def write_pg_hba(self, config):
-        with open(self._pg_hba_conf, 'a') as f:
-            f.write('\n{}\n'.format('\n'.join(config)))
+        if not self._server_parameters.get('hba_file') and not self.config.get('pg_hba'):
+            with open(self._pg_hba_conf, 'a') as f:
+                f.write('\n{}\n'.format('\n'.join(config)))
+        return True
 
     def _replace_pg_hba(self):
         """
@@ -1398,6 +1400,7 @@ class Postgresql(object):
                     shutil.copy(config_file, backup_file)
         except IOError:
             logger.exception('unable to create backup copies of configuration files')
+        return True
 
     def restore_configuration_files(self):
         """ restore a previously saved postgresql.conf """
@@ -1547,6 +1550,7 @@ $$""".format(name, ' '.join(options)), name, password, password)
 
     def bootstrap(self, config):
         """ Initialize a new node from scratch and start it. """
+        pg_hba = config.get('pg_hba', [])
         method = config.get('method') or 'initdb'
         self._running_custom_bootstrap = method != 'initdb' and method in config and 'command' in config[method]
         if self._running_custom_bootstrap:
@@ -1554,7 +1558,8 @@ $$""".format(name, ' '.join(options)), name, password, password)
             config = config[method]
         else:
             do_initialize = self._initdb
-        return do_initialize(config) and self._configure_server_parameters() and self.start()
+        return do_initialize(config) and self.write_pg_hba(pg_hba) and self.save_configuration_files() \
+            and self._configure_server_parameters() and self.start()
 
     def post_bootstrap(self, config, task):
         try:
@@ -1615,23 +1620,27 @@ $$""".format(name, ' '.join(options)), name, password, password)
             logger.exception('Could not remove data directory %s', self._data_dir)
             self.move_data_directory()
 
-    def basebackup(self, conn_url, env):
+    def basebackup(self, conn_url, env, options):
         # creates a replica data dir using pg_basebackup.
         # this is the default, built-in create_replica_method
         # tries twice, then returns failure (as 1)
         # uses "stream" as the xlog-method to avoid sync issues
+        # supports additional user-supplied options, those are not validated
         maxfailures = 2
         ret = 1
+        not_allowed_options = ('pgdata', 'format', 'wal-method', 'xlog-method', 'gzip',
+                               'version', 'compress', 'dbname', 'host', 'port', 'username', 'password')
+        user_options = self.process_user_options('basebackup', options, not_allowed_options, logger.error)
+
         for bbfailures in range(0, maxfailures):
             with self._cancellable_lock:
                 if self._is_cancelled:
                     break
             if not self.data_directory_empty():
                 self.remove_data_directory()
-
             try:
                 ret = self.cancellable_subprocess_call([self._pgcommand('pg_basebackup'), '--pgdata=' + self._data_dir,
-                                                       '-X', 'stream', '--dbname=' + conn_url], env=env)
+                                                       '-X', 'stream', '--dbname=' + conn_url] + user_options, env=env)
                 if ret == 0:
                     break
                 else:
@@ -1791,10 +1800,20 @@ $$""".format(name, ' '.join(options)), name, password, password)
         return self.single_user_mode(options=opts) == 0 or None
 
     def cancellable_subprocess_call(self, *args, **kwargs):
-        communicate_input = kwargs.pop('communicate_input', None)
         for s in ('stdin', 'stdout', 'stderr'):
             kwargs.pop(s, None)
 
+        communicate_input = 'communicate_input' in kwargs
+        if communicate_input:
+            input_data = kwargs.pop('communicate_input', None)
+            if not isinstance(input_data, string_types):
+                input_data = ''
+            if input_data and input_data[-1] != '\n':
+                input_data += '\n'
+            kwargs['stdin'] = subprocess.PIPE
+            kwargs['stdout'] = open(os.devnull, 'w')
+            kwargs['stderr'] = subprocess.STDOUT
+
         try:
             with self._cancellable_lock:
                 if self._is_cancelled:
@@ -1804,10 +1823,8 @@ $$""".format(name, ' '.join(options)), name, password, password)
                 self._cancellable = subprocess.Popen(*args, **kwargs)
 
             if communicate_input:
-                kwargs['stdin'] = subprocess.PIPE
-                if communicate_input[-1] != '\n':
-                    communicate_input += '\n'
-                self._cancellable.communicate(communicate_input + '\n')
+                if input_data:
+                    self._cancellable.communicate(input_data)
                 self._cancellable.stdin.close()
 
             return self._cancellable.wait()
@@ -1834,3 +1851,12 @@ $$""".format(name, ' '.join(options)), name, password, password)
         with self._cancellable_lock:
             if self._cancellable is not None and self._cancellable.returncode is None:
                 self._cancellable.kill()
+
+    def schedule_sanity_checks_after_pause(self):
+        """
+            After coming out of pause we have to:
+            1. sync replication slots, because it might happen that slots were removed
+            2. get new 'Database system identifier' to make sure that it wasn't changed
+        """
+        self._schedule_load_slots = self.use_slots
+        self._sysid = None

patroni/postmaster.py

@@ -17,6 +17,7 @@ STOP_SIGNALS = {
 
 
 class PostmasterProcess(psutil.Process):
+
     def __init__(self, pid):
         self.is_single_user = False
         if pid < 0:
@@ -24,32 +25,55 @@ class PostmasterProcess(psutil.Process):
             self.is_single_user = True
         super(PostmasterProcess, self).__init__(pid)
 
-    @classmethod
-    def from_pidfile(cls, pidfile):
-        try:
-            pid = int(pidfile.get('pid', 0))
-            if not pid:
-                return None
-        except ValueError:
-            return None
+    @staticmethod
+    def _read_postmaster_pidfile(data_dir):
+        """Reads and parses postmaster.pid from the data directory
 
+        :returns dictionary of values if successful, empty dictionary otherwise
+        """
+        pid_line_names = ['pid', 'data_dir', 'start_time', 'port', 'socket_dir', 'listen_addr', 'shmem_key']
         try:
-            proc = cls(pid)
-        except psutil.NoSuchProcess:
-            return None
+            with open(os.path.join(data_dir, 'postmaster.pid')) as f:
+                return {name: line.rstrip('\n') for name, line in zip(pid_line_names, f)}
+        except IOError:
+            return {}
 
+    def _is_postmaster_process(self):
         try:
-            start_time = int(pidfile.get('start_time', 0))
-            if start_time and abs(proc.create_time() - start_time) > 3:
-                return None
+            start_time = int(self._postmaster_pid.get('start_time', 0))
+            if start_time and abs(self.create_time() - start_time) > 3:
+                logger.info('Too much difference between %s and %s', self.create_time(), start_time)
+                return False
         except ValueError:
-            logger.warning("Garbage start time value in pid file: %r", pidfile.get('start_time'))
+            logger.warning('Garbage start time value in pid file: %r', self._postmaster_pid.get('start_time'))
 
         # Extra safety check. The process can't be ourselves, our parent or our direct child.
-        if proc.pid == os.getpid() or proc.pid == os.getppid() or proc.parent() == os.getpid():
-            return None
+        if self.pid == os.getpid() or self.pid == os.getppid() or self.ppid() == os.getpid():
+            logger.info('Patroni (pid=%s, ppid=%s), "fake postmaster" (pid=%s, ppid=%s)',
+                        os.getpid(), os.getppid(), self.pid, self.ppid())
+            return False
 
-        return proc
+        return True
+
+    @classmethod
+    def _from_pidfile(cls, data_dir):
+        postmaster_pid = PostmasterProcess._read_postmaster_pidfile(data_dir)
+        try:
+            pid = int(postmaster_pid.get('pid', 0))
+            if pid:
+                proc = cls(pid)
+                proc._postmaster_pid = postmaster_pid
+                return proc
+        except ValueError:
+            pass
+
+    @staticmethod
+    def from_pidfile(data_dir):
+        try:
+            proc = PostmasterProcess._from_pidfile(data_dir)
+            return proc if proc and proc._is_postmaster_process() else None
+        except psutil.NoSuchProcess:
+            return None
 
     @classmethod
     def from_pid(cls, pid):
@@ -100,8 +124,8 @@ class PostmasterProcess(psutil.Process):
         except psutil.Error:
             logger.exception('wait_for_user_backends_to_close')
 
-    @classmethod
-    def start(cls, pgcommand, data_dir, conf, options):
+    @staticmethod
+    def start(pgcommand, data_dir, conf, options):
         # Unfortunately `pg_ctl start` does not return postmaster pid to us. Without this information
         # it is hard to know the current state of postgres startup, so we had to reimplement pg_ctl start
         # in python. It will start postgres, wait for port to be open and wait until postgres will start
@@ -113,10 +137,24 @@ class PostmasterProcess(psutil.Process):
         # of init process to take care about postmaster.
         # In order to make everything portable we can't use fork&exec approach here, so  we will call
         # ourselves and pass list of arguments which must be used to start postgres.
+        env = {p: os.environ[p] for p in ('PATH', 'LC_ALL', 'LANG') if p in os.environ}
+        try:
+            proc = PostmasterProcess._from_pidfile(data_dir)
+            if proc and not proc._is_postmaster_process():
+                # Upon start postmaster process performs various safety checks if there is a postmaster.pid
+                # file in the data directory. Although Patroni already detected that the running process
+                # corresponding to the postmaster.pid is not a postmaster, the new postmaster might fail
+                # to start, because it thinks that postmaster.pid is already locked.
+                # Important!!! Unlink of postmaster.pid isn't an option, because it has a lot of nasty race conditions.
+                # Luckily there is a workaround to this problem, we can pass the pid from postmaster.pid
+                # in the `PG_GRANDPARENT_PID` environment variable and postmaster will ignore it.
+                env['PG_GRANDPARENT_PID'] = str(proc.pid)
+        except psutil.NoSuchProcess:
+            pass
+
         proc = call_self(['pg_ctl_start', pgcommand, '-D', data_dir,
                           '--config-file={}'.format(conf)] + options, close_fds=True,
-                         preexec_fn=os.setsid, stdout=subprocess.PIPE,
-                         env={p: os.environ[p] for p in ('PATH', 'LC_ALL', 'LANG') if p in os.environ})
+                         preexec_fn=os.setsid, stdout=subprocess.PIPE, env=env)
         pid = int(proc.stdout.readline().strip())
         proc.wait()
         logger.info('postmaster pid=%s', pid)

patroni/version.py

@@ -1 +1 @@
-__version__ = '1.4.2'
+__version__ = '1.4.4'

postgres1.yml

@@ -75,6 +75,9 @@ postgresql:
       password: zalando
   parameters:
     unix_socket_directories: '.'
+  basebackup:
+      - verbose
+      - max-rate: 100M
 tags:
     nofailover: false
     noloadbalance: false

tests/test_api.py

@@ -339,6 +339,7 @@ class TestRestApiHandler(unittest.TestCase):
 
         cluster2 = cluster.copy()
         cluster2.leader.name = 'postgresql0'
+        cluster2.is_unlocked.return_value = False
         dcs.get_cluster.side_effect = [cluster, cluster2]
         MockRestApiServer(RestApiHandler, request)
 

tests/test_etcd.py

@@ -90,7 +90,7 @@ def etcd_read(self, key, **kwargs):
         raise etcd.EtcdKeyNotFound
 
     response = {"action": "get", "node": {"key": "/service/batman5", "dir": True, "nodes": [
-                {"key": "/service/batman5/config", "value": '{"foo": "bar"}',
+                {"key": "/service/batman5/config", "value": '{"synchronous_mode": 0}',
                  "modifiedIndex": 1582, "createdIndex": 1582},
                 {"key": "/service/batman5/failover", "value": "",
                  "modifiedIndex": 1582, "createdIndex": 1582},
@@ -207,6 +207,7 @@ class TestClient(unittest.TestCase):
         mock_machines.__get__ = Mock(return_value=['http://localhost:2379'])
         self.client._machines_cache_updated = 0
         self.client.api_execute('/', 'POST', timeout=0)
+        self.client._machines_cache = [self.client._base_uri]
         self.assertRaises(etcd.EtcdWatchTimedOut, self.client.api_execute, '/timeout', 'POST', params={'wait': 'true'})
         self.assertRaises(etcd.EtcdException, self.client.api_execute, '/', '')
         self.client._update_machines_cache = True
@@ -275,7 +276,9 @@ class TestEtcd(unittest.TestCase):
                                   {'hosts': 'foo:4001,bar', 'retry_timeout': 10})
 
     def test_get_cluster(self):
-        self.assertIsInstance(self.etcd.get_cluster(), Cluster)
+        cluster = self.etcd.get_cluster()
+        self.assertIsInstance(cluster, Cluster)
+        self.assertFalse(cluster.is_synchronous_mode())
         self.etcd._base_path = '/service/nocluster'
         cluster = self.etcd.get_cluster()
         self.assertIsInstance(cluster, Cluster)

tests/test_ha.py

@@ -16,6 +16,8 @@ from test_etcd import socket_getaddrinfo, etcd_read, etcd_write, requests_get
 from test_postgresql import psycopg2_connect, MockPostmaster
 
 
+SYSID = '12345678901'
+
 def true(*args, **kwargs):
     return True
 
@@ -45,7 +47,7 @@ def get_cluster_initialized_without_leader(leader=False, failover=None, sync=Non
                                  'scheduled_restart': {'schedule': "2100-01-01 10:53:07.560445+00:00",
                                                        'postgres_version': '99.0.0'}})
     syncstate = SyncState(0 if sync else None, sync and sync[0], sync and sync[1])
-    return get_cluster(True, leader, [m1, m2], failover, syncstate)
+    return get_cluster(SYSID, leader, [m1, m2], failover, syncstate)
 
 
 def get_cluster_initialized_with_leader(failover=None, sync=None):
@@ -120,7 +122,7 @@ def run_async(self, func, args=()):
 @patch.object(Postgresql, '_cluster_info_state_get', Mock(return_value=3))
 @patch.object(Postgresql, 'call_nowait', Mock(return_value=True))
 @patch.object(Postgresql, 'data_directory_empty', Mock(return_value=False))
-@patch.object(Postgresql, 'controldata', Mock(return_value={'Database system identifier': '1234567890'}))
+@patch.object(Postgresql, 'controldata', Mock(return_value={'Database system identifier': SYSID}))
 @patch.object(Postgresql, 'sync_replication_slots', Mock())
 @patch.object(Postgresql, 'write_pg_hba', Mock())
 @patch.object(Postgresql, 'write_pgpass', Mock(return_value={}))
@@ -161,7 +163,7 @@ class TestHa(unittest.TestCase):
                                        'name': 'foo', 'retry_timeout': 10}})
             self.ha = Ha(MockPatroni(self.p, self.e))
             self.ha.old_cluster = self.e.get_cluster()
-            self.ha.cluster = get_cluster_not_initialized_without_leader()
+            self.ha.cluster = get_cluster_initialized_without_leader()
             self.ha.load_cluster_from_dcs = Mock()
 
     def test_update_lock(self):
@@ -178,7 +180,7 @@ class TestHa(unittest.TestCase):
         self.assertEquals(self.ha.run_cycle(), 'starting as a secondary')
 
     def test_recover_replica_failed(self):
-        self.p.controldata = lambda: {'Database cluster state': 'in recovery'}
+        self.p.controldata = lambda: {'Database cluster state': 'in recovery', 'Database system identifier': SYSID}
         self.p.is_running = false
         self.p.follow = false
         self.assertEquals(self.ha.run_cycle(), 'starting as a secondary')
@@ -189,14 +191,14 @@ class TestHa(unittest.TestCase):
         self.p.is_running = false
         self.p.name = 'leader'
         self.p.set_role('master')
-        self.p.controldata = lambda: {'Database cluster state': 'shut down'}
+        self.p.controldata = lambda: {'Database cluster state': 'shut down', 'Database system identifier': SYSID}
         self.ha.cluster = get_cluster_initialized_with_leader()
         self.assertEquals(self.ha.run_cycle(), 'starting as readonly because i had the session lock')
 
     @patch.object(Postgresql, 'fix_cluster_state', Mock())
     def test_crash_recovery(self):
         self.p.is_running = false
-        self.p.controldata = lambda: {'Database cluster state': 'in production'}
+        self.p.controldata = lambda: {'Database cluster state': 'in production', 'Database system identifier': SYSID}
         self.assertEquals(self.ha.run_cycle(), 'doing crash recovery in a single user mode')
 
     @patch.object(Postgresql, 'rewind_needed_and_possible', Mock(return_value=True))
@@ -208,7 +210,7 @@ class TestHa(unittest.TestCase):
     @patch.object(Postgresql, 'can_rewind', PropertyMock(return_value=True))
     @patch.object(Postgresql, 'fix_cluster_state', Mock())
     def test_single_user_after_recover_failed(self):
-        self.p.controldata = lambda: {'Database cluster state': 'in recovery'}
+        self.p.controldata = lambda: {'Database cluster state': 'in recovery', 'Database system identifier': SYSID}
         self.p.is_running = false
         self.p.follow = false
         self.assertEquals(self.ha.run_cycle(), 'starting as a secondary')
@@ -217,6 +219,7 @@ class TestHa(unittest.TestCase):
     @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.p.controldata = lambda: {'Database cluster state': 'in recovery', 'Database system identifier': '123'}
         self.ha.run_cycle()
         exit_mock.assert_called_once_with(1)
 
@@ -225,7 +228,7 @@ class TestHa(unittest.TestCase):
         self.p.is_leader = false
         self.p.is_healthy = true
         self.ha.has_lock = true
-        self.p.controldata = lambda: {'Database cluster state': 'in production'}
+        self.p.controldata = lambda: {'Database cluster state': 'in production', 'Database system identifier': SYSID}
         self.assertEquals(self.ha.run_cycle(), 'promoted self to leader because i had the session lock')
 
     @patch('psycopg2.connect', psycopg2_connect)
@@ -279,6 +282,7 @@ class TestHa(unittest.TestCase):
             self.assertEquals(self.ha.run_cycle(), 'Not promoting self because watchdog could not be activated')
 
     def test_leader_with_lock(self):
+        self.ha.cluster = get_cluster_not_initialized_without_leader()
         self.ha.cluster.is_unlocked = false
         self.ha.has_lock = true
         self.assertEquals(self.ha.run_cycle(), 'no action.  i am the leader with the lock')
@@ -618,6 +622,14 @@ class TestHa(unittest.TestCase):
         self.ha.cluster = get_cluster_initialized_with_leader(Failover(0, '', self.p.name, None))
         self.assertEquals(self.ha.run_cycle(), 'PAUSE: waiting to become master after promote...')
 
+    def test_failed_to_update_lock_in_pause(self):
+        self.ha.update_lock = false
+        self.ha.is_paused = true
+        self.p.name = 'leader'
+        self.ha.cluster = get_cluster_initialized_with_leader()
+        self.assertEquals(self.ha.run_cycle(),
+                          'PAUSE: continue to run as master after failing to update leader lock in DCS')
+
     def test_postgres_unhealthy_in_pause(self):
         self.ha.is_paused = true
         self.p.is_healthy = false
@@ -896,3 +908,18 @@ class TestHa(unittest.TestCase):
         self.ha.has_lock = true
         self.ha.cluster.is_unlocked = false
         self.assertEquals(self.ha.run_cycle(), 'no action.  i am the leader with the lock')
+
+    @patch('sys.exit', return_value=1)
+    def test_abort_join(self, exit_mock):
+        self.ha.cluster = get_cluster_not_initialized_without_leader()
+        self.p.is_leader = false
+        self.ha.run_cycle()
+        exit_mock.assert_called_once_with(1)
+
+    def test_after_pause(self):
+        self.ha.has_lock = true
+        self.ha.cluster.is_unlocked = false
+        self.ha.is_paused = true
+        self.assertEquals(self.ha.run_cycle(), 'PAUSE: no action.  i am the leader with the lock')
+        self.ha.is_paused = false
+        self.assertEquals(self.ha.run_cycle(), 'no action.  i am the leader with the lock')

tests/test_postgresql.py

@@ -210,12 +210,11 @@ class TestPostgresql(unittest.TestCase):
     def tearDown(self):
         shutil.rmtree('data')
 
-    def test_get_initdb_options(self):
-        self.assertEquals(self.p.get_initdb_options([{'encoding': 'UTF8'}, 'data-checksums']),
-                          ['--encoding=UTF8', '--data-checksums'])
-        self.assertRaises(Exception, self.p.get_initdb_options, [{'pgdata': 'bar'}])
-        self.assertRaises(Exception, self.p.get_initdb_options, [{'foo': 'bar', 1: 2}])
-        self.assertRaises(Exception, self.p.get_initdb_options, [1])
+    def test__initdb(self):
+        self.assertRaises(Exception, self.p.bootstrap, {'initdb': [{'pgdata': 'bar'}]})
+        self.assertRaises(Exception, self.p.bootstrap, {'initdb': [{'foo': 'bar', 1: 2}]})
+        self.assertRaises(Exception, self.p.bootstrap, {'initdb': [1]})
+        self.assertRaises(Exception, self.p.bootstrap, {'initdb': 1})
 
     @patch('os.path.exists', Mock(return_value=True))
     @patch('os.unlink', Mock())
@@ -428,6 +427,29 @@ class TestPostgresql(unittest.TestCase):
         del self.p.config['wale']
         self.assertEquals(self.p.create_replica(self.leader), 0)
 
+        self.p.config['create_replica_method'] = ['basebackup']
+        self.p.config['basebackup'] = [{'max_rate': '100M'}, 'no-sync']
+        self.assertEquals(self.p.create_replica(self.leader), 0)
+
+        self.p.config['basebackup'] = [{'max_rate': '100M', 'compress': '9'}]
+        with mock.patch('patroni.postgresql.logger.error', new_callable=Mock()) as mock_logger:
+            self.p.create_replica(self.leader)
+            mock_logger.assert_called_once()
+            self.assertTrue("only one key-value is allowed and value should be a string" in mock_logger.call_args[0][0],
+                            "not matching {0}".format(mock_logger.call_args[0][0]))
+
+        self.p.config['basebackup'] = [42]
+        with mock.patch('patroni.postgresql.logger.error', new_callable=Mock()) as mock_logger:
+            self.p.create_replica(self.leader)
+            mock_logger.assert_called_once()
+            self.assertTrue("value should be string value or a single key-value pair" in mock_logger.call_args[0][0],
+                            "not matching {0}".format(mock_logger.call_args[0][0]))
+
+        self.p.config['basebackup'] = {"foo": "bar"}
+        self.assertEquals(self.p.create_replica(self.leader), 0)
+
+        self.p.config['create_replica_method'] = ['wale', 'basebackup']
+        del self.p.config['basebackup']
         mock_cancellable_subprocess_call.return_value = 1
         self.assertEquals(self.p.create_replica(self.leader), 1)
 
@@ -445,7 +467,7 @@ class TestPostgresql(unittest.TestCase):
 
     def test_basebackup(self):
         self.p.cancel()
-        self.p.basebackup(None, None)
+        self.p.basebackup(None, None, {'foo': 'bar'})
 
     @patch.object(Postgresql, 'is_running', Mock(return_value=True))
     def test_sync_replication_slots(self):
@@ -463,8 +485,10 @@ class TestPostgresql(unittest.TestCase):
             cluster.members.extend([alias1, alias2])
             self.p.sync_replication_slots(cluster)
             errorlog_mock.assert_called_once()
-            assert "test-3" in errorlog_mock.call_args[0][1]
-            assert "test.3" in errorlog_mock.call_args[0][1]
+            self.assertTrue("test-3" in errorlog_mock.call_args[0][1],
+                            "non matching {0}".format(errorlog_mock.call_args[0][1]))
+            self.assertTrue("test.3" in errorlog_mock.call_args[0][1],
+                            "non matching {0}".format(errorlog_mock.call_args[0][1]))
 
     @patch.object(MockCursor, 'execute', Mock(side_effect=psycopg2.OperationalError))
     def test__query(self):
@@ -567,6 +591,7 @@ class TestPostgresql(unittest.TestCase):
 
     @patch.object(Postgresql, 'cancellable_subprocess_call')
     def test_custom_bootstrap(self, mock_cancellable_subprocess_call):
+        self.p.config.pop('pg_hba')
         config = {'method': 'foo', 'foo': {'command': 'bar'}}
 
         mock_cancellable_subprocess_call.return_value = 1
@@ -807,16 +832,6 @@ class TestPostgresql(unittest.TestCase):
             self.p._state = 'starting'
             self.assertIsNone(self.p.wait_for_startup())
 
-    def test_read_pid_file(self):
-        pidfile = os.path.join(self.data_dir, 'postmaster.pid')
-        if os.path.exists(pidfile):
-            os.remove(pidfile)
-        self.assertEquals(self.p._read_pid_file(), {})
-        with open(pidfile, 'w') as fd:
-            fd.write("123\n/foo/bar\n123456789\n5432")
-        self.assertEquals(self.p._read_pid_file(), {"pid": "123", "data_dir": "/foo/bar",
-                                                    "start_time": "123456789", "port": "5432"})
-
     def test_pick_sync_standby(self):
         cluster = Cluster(True, None, self.leader, 0, [self.me, self.other, self.leadermem], None,
                           SyncState(0, self.me.name, self.leadermem.name), None)
@@ -945,7 +960,7 @@ class TestPostgresql(unittest.TestCase):
 
     def test_cancellable_subprocess_call(self):
         self.p.cancel()
-        self.assertRaises(PostgresException, self.p.cancellable_subprocess_call)
+        self.assertRaises(PostgresException, self.p.cancellable_subprocess_call, communicate_input=None)
 
     @patch('patroni.postgresql.polling_loop', Mock(return_value=[0, 0]))
     def test_cancel(self):

tests/test_postmaster.py

@@ -1,8 +1,9 @@
+import psutil
 import unittest
 
-from mock import Mock, patch
+from mock import Mock, patch, mock_open
 from patroni.postmaster import PostmasterProcess
-import psutil
+from six.moves import builtins
 
 
 class TestPostmasterProcess(unittest.TestCase):
@@ -13,26 +14,34 @@ class TestPostmasterProcess(unittest.TestCase):
 
     @patch('psutil.Process.create_time')
     @patch('psutil.Process.__init__')
-    def test_from_pidfile(self, mock_init, mock_create_time):
+    @patch('patroni.postmaster.PostmasterProcess._read_postmaster_pidfile')
+    def test_from_pidfile(self, mock_read, mock_init, mock_create_time):
         mock_init.side_effect = psutil.NoSuchProcess(123)
-        self.assertEquals(PostmasterProcess.from_pidfile({}), None)
-        self.assertEquals(PostmasterProcess.from_pidfile({"pid": "foo"}), None)
-        self.assertEquals(PostmasterProcess.from_pidfile({"pid": "123"}), None)
+        mock_read.return_value = {}
+        self.assertIsNone(PostmasterProcess.from_pidfile(''))
+        mock_read.return_value = {"pid": "foo"}
+        self.assertIsNone(PostmasterProcess.from_pidfile(''))
+        mock_read.return_value = {"pid": "123"}
+        self.assertIsNone(PostmasterProcess.from_pidfile(''))
 
         mock_init.side_effect = None
         with patch.object(psutil.Process, 'pid', 123), \
-                patch.object(psutil.Process, 'parent', return_value=124), \
+                patch.object(psutil.Process, 'ppid', return_value=124), \
                 patch('os.getpid', return_value=125) as mock_ospid, \
                 patch('os.getppid', return_value=126):
 
-            self.assertNotEquals(PostmasterProcess.from_pidfile({"pid": "123"}), None)
+            self.assertIsNotNone(PostmasterProcess.from_pidfile(''))
 
             mock_create_time.return_value = 100000
-            self.assertEquals(PostmasterProcess.from_pidfile({"pid": "123", "start_time": "200000"}), None)
-            self.assertNotEquals(PostmasterProcess.from_pidfile({"pid": "123", "start_time": "foobar"}), None)
+            mock_read.return_value = {"pid": "123", "start_time": "200000"}
+            self.assertIsNone(PostmasterProcess.from_pidfile(''))
+
+            mock_read.return_value = {"pid": "123", "start_time": "foobar"}
+            self.assertIsNotNone(PostmasterProcess.from_pidfile(''))
 
             mock_ospid.return_value = 123
-            self.assertEquals(PostmasterProcess.from_pidfile({"pid": "123", "start_time": "100000"}), None)
+            mock_read.return_value = {"pid": "123", "start_time": "100000"}
+            self.assertIsNone(PostmasterProcess.from_pidfile(''))
 
     @patch('psutil.Process.__init__')
     def test_from_pid(self, mock_init):
@@ -75,11 +84,20 @@ class TestPostmasterProcess(unittest.TestCase):
 
     @patch('subprocess.Popen')
     @patch.object(PostmasterProcess, 'from_pid')
-    def test_start(self, mock_frompid, mock_popen):
+    @patch.object(PostmasterProcess, '_from_pidfile')
+    def test_start(self, mock_frompidfile, mock_frompid, mock_popen):
+        mock_frompidfile.return_value._is_postmaster_process.return_value = False
         mock_frompid.return_value = "proc 123"
         mock_popen.return_value.stdout.readline.return_value = '123'
-        self.assertEquals(
-            PostmasterProcess.start('/bin/true', '/tmp/', '/tmp/test.conf', ['--foo=bar', '--bar=baz']),
-            "proc 123"
-        )
+        self.assertEquals(PostmasterProcess.start('true', '/tmp', '/tmp/test.conf', []), "proc 123")
         mock_frompid.assert_called_with(123)
+
+        mock_frompidfile.side_effect = psutil.NoSuchProcess(123)
+        self.assertEquals(PostmasterProcess.start('true', '/tmp', '/tmp/test.conf', []), "proc 123")
+
+    @patch('psutil.Process.__init__', Mock(side_effect=psutil.NoSuchProcess(123)))
+    def test_read_postmaster_pidfile(self):
+        with patch.object(builtins, 'open', Mock(side_effect=IOError)):
+            self.assertIsNone(PostmasterProcess.from_pidfile(''))
+        with patch.object(builtins, 'open', mock_open(read_data='123\n')):
+            self.assertIsNone(PostmasterProcess.from_pidfile(''))