Repository: zalando/patroni
Pull request #2650 (2023-04-21 12:25:35)

Update REWIND_USERNAME,PASSWORD optional tag in ENVIRONMENT.rst file
Update REWIND_USERNAME, REWIND_PASSWORD optional tag in ENVIRONMENT.rst file


Patch:
diff --git a/.github/workflows/install_deps.py b/.github/workflows/install_deps.py
index 82f0a6cc..29acb701 100644
--- a/.github/workflows/install_deps.py
+++ b/.github/workflows/install_deps.py
@@ -19,7 +19,7 @@ def install_requirements(what):
     requirements = ['mock>=2.0.0', 'flake8', 'pytest', 'pytest-cov'] if what == 'all' else ['behave']
     requirements += ['coverage']
     # try to split tests between psycopg2 and psycopg3
-    requirements += ['psycopg[binary]'] if sys.version_info > (3, 7, 0) and\
+    requirements += ['psycopg[binary]'] if sys.version_info >= (3, 8, 0) and\
         (sys.platform != 'darwin' or what == 'etcd3') else ['psycopg2-binary']
     for r in read('requirements.txt').split('\n'):
         r = r.strip()
@@ -85,8 +85,8 @@ def unzip_all(archive):
 
 
 def chmod_755(name):
-    os.chmod(name, stat.S_IRUSR | stat.S_IWUSR | stat.S_IXUSR |
-             stat.S_IRGRP | stat.S_IXGRP | stat.S_IROTH | stat.S_IXOTH)
+    os.chmod(name, stat.S_IRUSR | stat.S_IWUSR | stat.S_IXUSR
+             | stat.S_IRGRP | stat.S_IXGRP | stat.S_IROTH | stat.S_IXOTH)
 
 
 def unpack(archive, name):
diff --git a/docs/ENVIRONMENT.rst b/docs/ENVIRONMENT.rst
index 75e195c3..f9df7017 100644
--- a/docs/ENVIRONMENT.rst
+++ b/docs/ENVIRONMENT.rst
@@ -159,8 +159,8 @@ PostgreSQL
 -  **PATRONI\_SUPERUSER\_SSLCRLDIR**: (optional) maps to the `sslcrldir <https://www.postgresql.org/docs/current/libpq-connect.html#LIBPQ-CONNECT-SSLCRLDIR>`__ connection parameter, which specifies the location of a directory with files containing a certificate revocation list. A client will reject connecting to any server that has a certificate present in this list.
 -  **PATRONI\_SUPERUSER\_GSSENCMODE**: (optional) maps to the `gssencmode <https://www.postgresql.org/docs/current/libpq-connect.html#LIBPQ-CONNECT-GSSENCMODE>`__ connection parameter, which determines whether or with what priority a secure GSS TCP/IP connection will be negotiated with the server
 -  **PATRONI\_SUPERUSER\_CHANNEL\_BINDING**: (optional) maps to the `channel_binding <https://www.postgresql.org/docs/current/libpq-connect.html#LIBPQ-CONNECT-CHANNEL-BINDING>`__ connection parameter, which controls the client's use of channel binding.
--  **PATRONI\_REWIND\_USERNAME**: name for the user for ``pg_rewind``; the user will be created during initialization of postgres 11+ and all necessary `permissions <https://www.postgresql.org/docs/11/app-pgrewind.html#id-1.9.5.8.8>`__ will be granted.
--  **PATRONI\_REWIND\_PASSWORD**: password for the user for ``pg_rewind``; the user will be created during initialization.
+-  **PATRONI\_REWIND\_USERNAME**: (optional) name for the user for ``pg_rewind``; the user will be created during initialization of postgres 11+ and all necessary `permissions <https://www.postgresql.org/docs/11/app-pgrewind.html#id-1.9.5.8.8>`__ will be granted.
+-  **PATRONI\_REWIND\_PASSWORD**: (optional) password for the user for ``pg_rewind``; the user will be created during initialization.
 -  **PATRONI\_REWIND\_SSLMODE**: (optional) maps to the `sslmode <https://www.postgresql.org/docs/current/libpq-connect.html#LIBPQ-CONNECT-SSLMODE>`__ connection parameter, which allows a client to specify the type of TLS negotiation mode with the server. For more information on how each mode works, please visit the `PostgreSQL documentation <https://www.postgresql.org/docs/current/libpq-ssl.html#LIBPQ-SSL-SSLMODE-STATEMENTS>`__. The default mode is ``prefer``.
 -  **PATRONI\_REWIND\_SSLKEY**: (optional) maps to the `sslkey <https://www.postgresql.org/docs/current/libpq-connect.html#LIBPQ-CONNECT-SSLKEY>`__ connection parameter, which specifies the location of the secret key used with the client's certificate.
 -  **PATRONI\_REWIND\_SSLPASSWORD**: (optional) maps to the `sslpassword <https://www.postgresql.org/docs/current/libpq-connect.html#LIBPQ-CONNECT-SSLPASSWORD>`__ connection parameter, which specifies the password for the secret key specified in ``PATRONI_REWIND_SSLKEY``.
diff --git a/docs/SETTINGS.rst b/docs/SETTINGS.rst
index 538f99dd..b5ee702f 100644
--- a/docs/SETTINGS.rst
+++ b/docs/SETTINGS.rst
@@ -18,33 +18,41 @@ Dynamic configuration is stored in the DCS (Distributed Configuration Store) and
 -  **maximum\_lag\_on\_syncnode**: the maximum bytes a synchronous follower may lag before it is considered as an unhealthy candidate and swapped by healthy asynchronous follower. Patroni utilize the max replica lsn if there is more than one follower, otherwise it will use leader's current wal lsn. Default is -1, Patroni will not take action to swap synchronous unhealthy follower when the value is set to 0 or below. Please set the value high enough so Patroni won't swap synchrounous follower fequently during high transaction volume.
 -  **max\_timelines\_history**: maximum number of timeline history items kept in DCS.  Default value: 0. When set to 0, it keeps the full history in DCS.
 -  **primary\_start\_timeout**: the amount of time a primary is allowed to recover from failures before failover is triggered (in seconds). 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. Worst case failover time for primary failure is: loop\_wait + primary\_start\_timeout + loop\_wait, unless primary\_start\_timeout is zero, in which case it's just loop\_wait. Set the value according to your durability/availability tradeoff.
-- **primary\_stop\_timeout**: The number of seconds Patroni is allowed to wait when stopping Postgres and effective only when synchronous_mode is enabled. When set to > 0 and the synchronous_mode is enabled, Patroni sends SIGKILL to the postmaster if the stop operation is running for more than the value set by primary\_stop\_timeout. Set the value according to your durability/availability tradeoff. If the parameter is not set or set <= 0, primary\_stop\_timeout does not apply.
+-  **primary\_stop\_timeout**: The number of seconds Patroni is allowed to wait when stopping Postgres and effective only when synchronous_mode is enabled. When set to > 0 and the synchronous_mode is enabled, Patroni sends SIGKILL to the postmaster if the stop operation is running for more than the value set by primary\_stop\_timeout. Set the value according to your durability/availability tradeoff. If the parameter is not set or set <= 0, primary\_stop\_timeout does not apply.
 -  **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.
 -  **synchronous\_mode\_strict**: prevents disabling synchronous replication if no synchronous replicas are available, blocking all client writes to the primary. See :ref:`replication modes documentation <replication_modes>` for details.
 -  **failsafe\_mode**: Enables :ref:`DCS Failsafe Mode <dcs_failsafe_mode>`. Defaults to `false`.
 -  **postgresql**:
-    -  **use\_pg\_rewind**: whether or not to use pg_rewind. Defaults to `false`.
-    -  **use\_slots**: whether or not to use replication slots. Defaults to `true` on PostgreSQL 9.4+.
-    -  **recovery\_conf**: additional configuration settings written to recovery.conf when configuring follower. There is no recovery.conf anymore in PostgreSQL 12, but you may continue using this section, because Patroni handles it transparently.
-    -  **parameters**: list of configuration settings for Postgres.
+
+   -  **use\_pg\_rewind**: whether or not to use pg_rewind. Defaults to `false`.
+   -  **use\_slots**: whether or not to use replication slots. Defaults to `true` on PostgreSQL 9.4+.
+   -  **recovery\_conf**: additional configuration settings written to recovery.conf when configuring follower. There is no recovery.conf anymore in PostgreSQL 12, but you may continue using this section, because Patroni handles it transparently.
+   -  **parameters**: list of configuration settings for Postgres.
+
 -  **standby\_cluster**: if this section is defined, we want to bootstrap a standby cluster.
-    -  **host**: an address of remote node
-    -  **port**: a port of remote node
-    -  **primary\_slot\_name**: which slot on the remote node to use for replication. This parameter is optional, the default value is derived from the instance name (see function `slot_name_from_member_name`).
-    -  **create\_replica\_methods**: an ordered list of methods that can be used to bootstrap standby leader from the remote primary, can be different from the list defined in :ref:`postgresql_settings`
-    -  **restore\_command**: command to restore WAL records from the remote primary to nodes in a standby cluster, can be different from the list defined in :ref:`postgresql_settings`
-    -  **archive\_cleanup\_command**: cleanup command for standby leader
-    -  **recovery\_min\_apply\_delay**: how long to wait before actually apply WAL records on a standby leader
+
+   -  **host**: an address of remote node
+   -  **port**: a port of remote node
+   -  **primary\_slot\_name**: which slot on the remote node to use for replication. This parameter is optional, the default value is derived from the instance name (see function `slot_name_from_member_name`).
+   -  **create\_replica\_methods**: an ordered list of methods that can be used to bootstrap standby leader from the remote primary, can be different from the list defined in :ref:`postgresql_settings`
+   -  **restore\_command**: command to restore WAL records from the remote primary to nodes in a standby cluster, can be different from the list defined in :ref:`postgresql_settings`
+   -  **archive\_cleanup\_command**: cleanup command for standby leader
+   -  **recovery\_min\_apply\_delay**: how long to wait before actually apply WAL records on a standby leader
+
 -  **slots**: define permanent replication slots. These slots will be preserved during switchover/failover. The logical slots are copied from the primary to a standby with restart, and after that their position advanced every **loop_wait** seconds (if necessary). Copying logical slot files performed via ``libpq`` connection and using either rewind or superuser credentials (see **postgresql.authentication** section). There is always a chance that the logical slot position on the replica is a bit behind the former primary, therefore application should be prepared that some messages could be received the second time after the failover. The easiest way of doing so - tracking ``confirmed_flush_lsn``. Enabling permanent logical replication slots requires **postgresql.use_slots** to be set and will also automatically enable the ``hot_standby_feedback``. Since the failover of logical replication slots is unsafe on PostgreSQL 9.6 and older and PostgreSQL version 10 is missing some important functions, the feature only works with PostgreSQL 11+.
-    -  **my_slot_name**: the name of replication slot. If the permanent slot name matches with the name of the current primary it will not be created. Everything else is the responsibility of the operator to make sure that there are no clashes in names between replication slots automatically created by Patroni for members and permanent replication slots.
-        -  **type**: slot type. Could be ``physical`` or ``logical``. If the slot is logical, you have to additionally define ``database`` and ``plugin``.
-        -  **database**: the database name where logical slots should be created.
-        -  **plugin**: the plugin name for the logical slot.
--  **ignore_slots**: list of sets of replication slot properties for which Patroni should ignore matching slots. This configuration/feature/etc. is useful when some replication slots are managed outside of Patroni. Any subset of matching properties will cause a slot to be ignored.
-    -  **name**: the name of the replication slot.
-    -  **type**: slot type. Can be ``physical`` or ``logical``. If the slot is logical, you may additionally define ``database`` and/or ``plugin``.
-    -  **database**: the database name (when matching a ``logical`` slot).
-    -  **plugin**: the logical decoding plugin (when matching a ``logical`` slot).
+
+   -  **my\_slot\_name**: the name of replication slot. If the permanent slot name matches with the name of the current primary it will not be created. Everything else is the responsibility of the operator to make sure that there are no clashes in names between replication slots automatically created by Patroni for members and permanent replication slots.
+
+      -  **type**: slot type. Could be ``physical`` or ``logical``. If the slot is logical, you have to additionally define ``database`` and ``plugin``.
+      -  **database**: the database name where logical slots should be created.
+      -  **plugin**: the plugin name for the logical slot.
+
+-  **ignore\_slots**: list of sets of replication slot properties for which Patroni should ignore matching slots. This configuration/feature/etc. is useful when some replication slots are managed outside of Patroni. Any subset of matching properties will cause a slot to be ignored.
+
+   -  **name**: the name of the replication slot.
+   -  **type**: slot type. Can be ``physical`` or ``logical``. If the slot is logical, you may additionally define ``database`` and/or ``plugin``.
+   -  **database**: the database name (when matching a ``logical`` slot).
+   -  **plugin**: the logical decoding plugin (when matching a ``logical`` slot).
 
 Note: **slots** is a hashmap while **ignore_slots** is an array. For example:
 
@@ -285,8 +293,8 @@ PostgreSQL
             -  **gssencmode**: (optional) maps to the `gssencmode <https://www.postgresql.org/docs/current/libpq-connect.html#LIBPQ-CONNECT-GSSENCMODE>`__ connection parameter, which determines whether or with what priority a secure GSS TCP/IP connection will be negotiated with the server
             -  **channel_binding**: (optional) maps to the `channel_binding <https://www.postgresql.org/docs/current/libpq-connect.html#LIBPQ-CONNECT-CHANNEL-BINDING>`__ connection parameter, which controls the client's use of channel binding.
         -  **rewind**:
-            -  **username**: name for the user for ``pg_rewind``; the user will be created during initialization of postgres 11+ and all necessary `permissions <https://www.postgresql.org/docs/11/app-pgrewind.html#id-1.9.5.8.8>`__ will be granted.
-            -  **password**: password for the user for ``pg_rewind``; the user will be created during initialization.
+            -  **username**: (optional) name for the user for ``pg_rewind``; the user will be created during initialization of postgres 11+ and all necessary `permissions <https://www.postgresql.org/docs/11/app-pgrewind.html#id-1.9.5.8.8>`__ will be granted.
+            -  **password**: (optional) password for the user for ``pg_rewind``; the user will be created during initialization.
             -  **sslmode**: (optional) maps to the `sslmode <https://www.postgresql.org/docs/current/libpq-connect.html#LIBPQ-CONNECT-SSLMODE>`__ connection parameter, which allows a client to specify the type of TLS negotiation mode with the server. For more information on how each mode works, please visit the `PostgreSQL documentation <https://www.postgresql.org/docs/current/libpq-ssl.html#LIBPQ-SSL-SSLMODE-STATEMENTS>`__. The default mode is ``prefer``.
             -  **sslkey**: (optional) maps to the `sslkey <https://www.postgresql.org/docs/current/libpq-connect.html#LIBPQ-CONNECT-SSLKEY>`__ connection parameter, which specifies the location of the secret key used with the client's certificate.
             -  **sslpassword**: (optional) maps to the `sslpassword <https://www.postgresql.org/docs/current/libpq-connect.html#LIBPQ-CONNECT-SSLPASSWORD>`__ connection parameter, which specifies the password for the secret key specified in ``sslkey``.
@@ -329,6 +337,7 @@ PostgreSQL
     -  **remove\_data\_directory\_on\_diverged\_timelines**: Patroni will remove the PostgreSQL data directory and recreate the replica if it notices that timelines are diverging and the former primary can not start streaming from the new primary. This option is useful when ``pg_rewind`` can not be used. While performing timelines divergence check on PostgreSQL v10 and older Patroni will try to connect with replication credential to the "postgres" database. Hence, such access should be allowed in the  pg_hba.conf. Default value is **false**.
     -  **replica\_method**: for each create_replica_methods 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".
     -  **pre\_promote**: a fencing script that executes during a failover after acquiring the leader lock but before promoting the replica. If the script exits with a non-zero code, Patroni does not promote the replica and removes the leader key from DCS.
+    -  **before\_stop**: a script that executes immediately prior to stopping postgres. As opposed to a callback, this script runs synchronously, blocking shutdown until it has completed. The return code of this script does not impact whether shutdown proceeds afterwards.
 
 .. _restapi_settings:
 
diff --git a/features/environment.py b/features/environment.py
index bb677026..b915fd99 100644
--- a/features/environment.py
+++ b/features/environment.py
@@ -1060,7 +1060,8 @@ def before_all(context):
     try:
         with open(os.devnull, 'w') as null:
             ret = subprocess.call(['openssl', 'req', '-nodes', '-new', '-x509', '-subj', '/CN=batman.patroni',
-                                   '-keyout', context.keyfile, '-out', context.certfile], stdout=null, stderr=null)
+                                   '--addext', 'subjectAltName=IP:127.0.0.1', '-keyout', context.keyfile,
+                                   '-out', context.certfile], stdout=null, stderr=null)
             if ret != 0:
                 raise Exception
     except Exception:
diff --git a/patroni/postgresql/__init__.py b/patroni/postgresql/__init__.py
index a80b76e3..1ecf111a 100644
--- a/patroni/postgresql/__init__.py
+++ b/patroni/postgresql/__init__.py
@@ -233,6 +233,17 @@ class Postgresql(object):
         pg_ctl = [self.pgcommand('pg_ctl'), cmd]
         return subprocess.call(pg_ctl + ['-D', self._data_dir] + list(args), **kwargs) == 0
 
+    def initdb(self, *args: str, **kwargs: Any) -> bool:
+        """Builds and executes the initdb command.
+
+        :param args: List of arguments to be joined into the initdb command.
+        :param kwargs: Keyword arguments to pass to ``subprocess.call``.
+
+        :returns: ``True`` if the result of ``subprocess.call`, the exit code, is ``0``.
+        """
+        initdb = [self.pgcommand('initdb')] + list(args) + [self.data_dir]
+        return subprocess.call(initdb, **kwargs) == 0
+
     def pg_isready(self):
         """Runs pg_isready to see if PostgreSQL is accepting connections.
 
@@ -718,6 +729,9 @@ class Postgresql(object):
         if not block_callbacks:
             self.set_state('stopping')
 
+        # invoke user-directed before stop script
+        self._before_stop()
+
         if before_shutdown:
             before_shutdown()
 
@@ -1024,6 +1038,21 @@ class Postgresql(object):
             logger.info('pre_promote script `%s` exited with %s', cmd, ret)
         return ret == 0
 
+    def _before_stop(self) -> None:
+        """Synchronously run a script prior to stopping postgres."""
+
+        cmd = self.config.get('before_stop')
+        if cmd:
+            self._do_before_stop(cmd)
+
+    def _do_before_stop(self, cmd: str) -> None:
+        try:
+            ret = self.cancellable.call(shlex.split(cmd))
+            if ret is not None:
+                logger.info('before_stop script `%s` exited with %s', cmd, ret)
+        except Exception as e:
+            logger.error('Exception when calling `%s`: %r', cmd, e)
+
     def promote(self, wait_seconds, task, before_promote=None, on_success=None):
         if self.role in ('promoted', 'master', 'primary'):
             return True
diff --git a/patroni/postgresql/bootstrap.py b/patroni/postgresql/bootstrap.py
index 42c24c9c..e21e1202 100644
--- a/patroni/postgresql/bootstrap.py
+++ b/patroni/postgresql/bootstrap.py
@@ -3,10 +3,11 @@ import os
 import shlex
 import tempfile
 import time
+from typing import List, Dict, Union, Callable, Tuple
 
 from ..dcs import RemoteMember
 from ..psycopg import quote_ident, quote_literal
-from ..utils import deep_compare
+from ..utils import deep_compare, unquote
 
 logger = logging.getLogger(__name__)
 
@@ -26,7 +27,56 @@ class Bootstrap(object):
         return self._running_custom_bootstrap and self._keep_existing_recovery_conf
 
     @staticmethod
-    def process_user_options(tool, options, not_allowed_options, error_handler):
+    def process_user_options(tool: str,
+                             options: Union[Dict[str, str], List[Union[str, Dict[str, str]]]],
+                             not_allowed_options: Tuple[str, ...],
+                             error_handler: Callable[[str], None]) -> List:
+        """Format *options* in a list or dictionary format into command line long form arguments.
+
+        .. note::
+            The format of the output of this method is to prepare arguments for use in the ``initdb``
+            method of `self._postgres`.
+
+        :Example:
+
+            The *options* can be defined as a dictionary of key, values to be converted into arguments:
+            >>> Bootstrap.process_user_options('foo', {'foo': 'bar'}, (), print)
+            ['--foo=bar']
+
+            Or as a list of single string arguments
+            >>> Bootstrap.process_user_options('foo', ['yes'], (), print)
+            ['--yes']
+
+            Or as a list of key, value options
+            >>> Bootstrap.process_user_options('foo', [{'foo': 'bar'}], (), print)
+            ['--foo=bar']
+
+            Or a combination of single and key, values
+            >>> Bootstrap.process_user_options('foo', ['yes', {'foo': 'bar'}], (), print)
+            ['--yes', '--foo=bar']
+
+            Options that contain spaces are passed as is to ``subprocess.call``
+            >>> Bootstrap.process_user_options('foo', [{'foo': 'bar baz'}], (), print)
+            ['--foo=bar baz']
+
+            Options that are quoted will be unquoted, so the quotes aren't interpreted
+            literally by the postgres command
+            >>> Bootstrap.process_user_options('foo', [{'foo': '"bar baz"'}], (), print)
+            ['--foo=bar baz']
+
+        .. note::
+            The *error_handler* is called when any of these conditions are met:
+
+            * Key, value dictionaries in the list form contains multiple keys.
+            * If a key is listed in *not_allowed_options*.
+            * If the options list is not in the required structure.
+
+        :param tool: The name of the tool used in error reports to *error_handler*
+        :param options: Options to parse as a list of key, values or single values, or a dictionary
+        :param not_allowed_options: List of keys that cannot be used in the list of key, value formatted options
+        :param error_handler: A function which will be called when an error condition is encountered
+        :returns: List of long form arguments to pass to the named tool
+        """
         user_options = []
 
         def option_is_allowed(name):
@@ -36,9 +86,9 @@ class Bootstrap(object):
             return ret
 
         if isinstance(options, dict):
-            for k, v in options.items():
-                if k and v:
-                    user_options.append('--{0}={1}'.format(k, v))
+            for key, val in options.items():
+                if key and val:
+                    user_options.append('--{0}={1}'.format(key, unquote(val)))
         elif isinstance(options, list):
             for opt in options:
                 if isinstance(opt, str) and option_is_allowed(opt):
@@ -48,7 +98,7 @@ class Bootstrap(object):
                     if len(keys) != 1 or not isinstance(opt[keys[0]], str) 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]]))
+                    user_options.append('--{0}={1}'.format(keys[0], unquote(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))
@@ -74,9 +124,8 @@ class Bootstrap(object):
                 os.write(fd, self._postgresql.config.superuser['password'].encode('utf-8'))
                 os.close(fd)
                 options.append('--pwfile={0}'.format(pwfile))
-        options = ['-o', ' '.join(options)] if options else []
 
-        ret = self._postgresql.pg_ctl('initdb', *options)
+        ret = self._postgresql.initdb(*options)
         if pwfile:
             os.remove(pwfile)
         if ret:
diff --git a/patroni/utils.py b/patroni/utils.py
index 1939185a..f8f9e2fd 100644
--- a/patroni/utils.py
+++ b/patroni/utils.py
@@ -19,6 +19,7 @@ import socket
 import sys
 import tempfile
 import time
+from shlex import split
 
 from typing import Any, Callable, Dict, Iterator, List, Optional, Tuple, Union, TYPE_CHECKING
 
@@ -911,3 +912,32 @@ def enable_keepalive(sock: socket.socket, timeout: int, idle: int, cnt: Optional
 
     for opt in keepalive_socket_options(timeout, idle, cnt):
         sock.setsockopt(*opt)
+
+
+def unquote(string: str) -> str:
+    """Unquote a fully quoted *string*.
+
+    :Examples:
+
+        A *string* with quotes will have those quotes removed
+        >>> unquote('"a quoted string"')
+        'a quoted string'
+
+        A *string* with multiple quotes will be returned as is
+        >>> unquote('"a multi" "quoted string"')
+        '"a multi" "quoted string"'
+
+        So will a *string* with unbalanced quotes
+        >>> unquote('unbalanced "quoted string')
+        'unbalanced "quoted string'
+
+    :param string: The string to be checked for quoting.
+    :returns: The string with quotes removed, if it is a fully quoted single string,
+              or the original string if quoting is not detected, or unquoting was not possible.
+    """
+    try:
+        ret = split(string)
+        ret = ret[0] if len(ret) == 1 else string
+    except ValueError:
+        ret = string
+    return ret


Test patch:
diff --git a/tests/test_bootstrap.py b/tests/test_bootstrap.py
index 1cce58b9..bdc284e5 100644
--- a/tests/test_bootstrap.py
+++ b/tests/test_bootstrap.py
@@ -1,4 +1,5 @@
 import os
+import sys
 
 from mock import Mock, PropertyMock, patch
 
@@ -99,6 +100,48 @@ class TestBootstrap(BaseTestPostgresql):
         self.assertRaises(Exception, self.b.bootstrap, {'initdb': [1]})
         self.assertRaises(Exception, self.b.bootstrap, {'initdb': 1})
 
+    def test__process_user_options(self):
+        def error_handler(msg):
+            raise Exception(msg)
+
+        self.assertEqual(self.b.process_user_options('initdb', ['string'], (), error_handler), ['--string'])
+        self.assertEqual(
+            self.b.process_user_options(
+                'initdb',
+                [{'key': 'value'}],
+                (), error_handler
+            ),
+            ['--key=value'])
+        if sys.platform != 'win32':
+            self.assertEqual(
+                self.b.process_user_options(
+                    'initdb',
+                    [{'key': 'value with spaces'}],
+                    (), error_handler
+                ),
+                ["--key=value with spaces"])
+            self.assertEqual(
+                self.b.process_user_options(
+                    'initdb',
+                    [{'key': "'value with spaces'"}],
+                    (), error_handler
+                ),
+                ["--key=value with spaces"])
+            self.assertEqual(
+                self.b.process_user_options(
+                    'initdb',
+                    {'key': 'value with spaces'},
+                    (), error_handler
+                ),
+                ["--key=value with spaces"])
+            self.assertEqual(
+                self.b.process_user_options(
+                    'initdb',
+                    {'key': "'value with spaces'"},
+                    (), error_handler
+                ),
+                ["--key=value with spaces"])
+
     @patch.object(CancellableSubprocess, 'call', Mock())
     @patch.object(Postgresql, 'is_running', Mock(return_value=True))
     @patch.object(Postgresql, 'data_directory_empty', Mock(return_value=False))
diff --git a/tests/test_postgresql.py b/tests/test_postgresql.py
index 31a807f5..49e58c7a 100644
--- a/tests/test_postgresql.py
+++ b/tests/test_postgresql.py
@@ -170,7 +170,8 @@ class TestPostgresql(BaseTestPostgresql):
     @patch('time.sleep', Mock())
     @patch.object(Postgresql, 'is_running')
     @patch.object(Postgresql, '_wait_for_connection_close', Mock())
-    def test_stop(self, mock_is_running):
+    @patch('patroni.postgresql.cancellable.CancellableSubprocess.call')
+    def test_stop(self, mock_cancellable_call, mock_is_running):
         # Postmaster is not running
         mock_callback = Mock()
         mock_is_running.return_value = None
@@ -195,6 +196,19 @@ class TestPostgresql(BaseTestPostgresql):
         mock_postmaster.wait.side_effect = [psutil.TimeoutExpired(30), Mock()]
         self.assertTrue(self.p.stop(on_safepoint=mock_callback, stop_timeout=30))
 
+        # Ensure before_stop script is called when configured to
+        self.p.config._config['before_stop'] = ':'
+        mock_postmaster.wait.side_effect = [psutil.TimeoutExpired(30), Mock()]
+        mock_cancellable_call.return_value = 0
+        with patch('patroni.postgresql.logger.info') as mock_logger:
+            self.p.stop(on_safepoint=mock_callback, stop_timeout=30)
+            self.assertEqual(mock_logger.call_args[0], ('before_stop script `%s` exited with %s', ':', 0))
+        mock_postmaster.wait.side_effect = [psutil.TimeoutExpired(30), Mock()]
+        mock_cancellable_call.side_effect = Exception
+        with patch('patroni.postgresql.logger.error') as mock_logger:
+            self.p.stop(on_safepoint=mock_callback, stop_timeout=30)
+            self.assertEqual(mock_logger.call_args_list[1][0][0], 'Exception when calling `%s`: %r')
+
         # Stop signal failed
         mock_postmaster.signal_stop.return_value = False
         self.assertFalse(self.p.stop())
diff --git a/tests/test_utils.py b/tests/test_utils.py
index 0c6b21e4..36925041 100644
--- a/tests/test_utils.py
+++ b/tests/test_utils.py
@@ -1,8 +1,9 @@
 import unittest
 
 from mock import Mock, patch
+
 from patroni.exceptions import PatroniException
-from patroni.utils import Retry, RetryFailedError, enable_keepalive, polling_loop, validate_directory
+from patroni.utils import Retry, RetryFailedError, enable_keepalive, polling_loop, validate_directory, unquote
 
 
 class TestUtils(unittest.TestCase):
@@ -41,6 +42,31 @@ class TestUtils(unittest.TestCase):
                 with patch('sys.platform', platform):
                     self.assertIsNone(enable_keepalive(Mock(), 10, 5))
 
+    def test_unquote(self):
+        self.assertEqual(unquote('value'), 'value')
+        self.assertEqual(unquote('value with spaces'), "value with spaces")
+        self.assertEqual(unquote(
+            '"double quoted value"'),
+            'double quoted value')
+        self.assertEqual(unquote(
+            '\'single quoted value\''),
+            'single quoted value')
+        self.assertEqual(unquote(
+            'value "with" double quotes'),
+            'value "with" double quotes')
+        self.assertEqual(unquote(
+            '"value starting with" double quotes'),
+            '"value starting with" double quotes')
+        self.assertEqual(unquote(
+            '\'value starting with\' single quotes'),
+            '\'value starting with\' single quotes')
+        self.assertEqual(unquote(
+            'value with a \' single quote'),
+            'value with a \' single quote')
+        self.assertEqual(unquote(
+            '\'value with a \'"\'"\' single quote\''),
+            'value with a \' single quote')
+
 
 @patch('time.sleep', Mock())
 class TestRetrySleeper(unittest.TestCase):