Add support for additional parameters on custom bootstrap (#2927)

Previous to this commit, if a user would ever like to add parameters to the custom bootstrap script call, they would need to configure Patroni like this:

```
bootstrap:
  method: custom_method_name
  custom_method_name:
    command: /path/to/my/custom_script --arg1=value1 --arg2=value2 ...
```

This commit extends that so we achieve a similar behavior that is seen when using `create_replica_methods`, i.e., we also allow the following syntax:

```
bootstrap:
  method: custom_method_name
  custom_method_name:
    command: /path/to/my/custom_script
    arg1: value1
    arg2: value2
```

All keys in the mapping which are not recognized by Patroni, will be dealt with as if they were additional named arguments to be passed down to the `command` call.

References: PAT-218.

docs/replica_bootstrap.rst

@@ -43,16 +43,31 @@ in the configuration files, Patroni supplies two cluster-specific ones:
 
 Passing these two additional flags can be disabled by setting a special ``no_params`` parameter to ``True``.
 
-If the bootstrap script returns 0, Patroni tries to configure and start the PostgreSQL instance produced by it. If any
+If the bootstrap script returns ``0``, Patroni tries to configure and start the PostgreSQL instance produced by it. If any
 of the intermediate steps fail, or the script returns a non-zero value, Patroni assumes that the bootstrap has failed,
 cleans up after itself and releases the initialize lock to give another node the opportunity to bootstrap.
 
 If a ``recovery_conf`` block is defined in the same section as the custom bootstrap method, Patroni will generate a
-``recovery.conf`` before starting the newly bootstrapped instance. Typically, such recovery.conf should contain at least
+``recovery.conf`` before starting the newly bootstrapped instance (or set the recovery settings on Postgres configuration if
-one of the ``recovery_target_*`` parameters, together with the ``recovery_target_timeline`` set to ``promote``.
+running PostgreSQL >= 12).
+Typically, such recovery configuration 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.
+If ``keep_existing_recovery_conf`` is defined and set to ``True``, Patroni will not remove the existing ``recovery.conf`` file if it exists (PostgreSQL <= 11).
-This is useful when bootstrapping from a backup with tools like pgBackRest that generate the appropriate ``recovery.conf`` for you.
+Similarly, in that case Patroni will not remove the existing ``recovery.signal`` or ``standby.signal`` if either exists, nor will it override the configured recovery settings (PostgreSQL >= 12).
+This is useful when bootstrapping from a backup with tools like pgBackRest that generate the appropriate recovery configuration for you.
+
+Besides that, any additional key/value pairs informed in the custom bootstrap method configuration will be passed as arguments to ``command`` in the format ``--name=value``. For example:
+
+.. code:: YAML
+
+    bootstrap:
+        method: <custom_bootstrap_method_name>
+        <custom_bootstrap_method_name>:
+            command: <path_to_custom_bootstrap_script>
+            arg1: value1
+            arg2: value2
+
+Makes the configured ``command`` to be called additionally with ``--arg1=value1 --arg2=value2`` command-line arguments.
 
  .. note:: Bootstrap methods are neither chained, nor fallen-back to the default one in case the primary one fails
 

features/backup_restore.py

@@ -6,6 +6,7 @@ if __name__ == "__main__":
     parser = argparse.ArgumentParser()
     parser.add_argument("--datadir", required=True)
     parser.add_argument("--sourcedir", required=True)
+    parser.add_argument("--test-argument", required=True)
     args, _ = parser.parse_known_args()
 
     shutil.copytree(args.sourcedir, args.datadir)

features/environment.py

@@ -892,22 +892,28 @@ class PatroniPoolController(object):
         }
         self.start(to_name, custom_config=custom_config)
 
+    def backup_restore_config(self, params=None):
+        return {
+            'command': (self.BACKUP_RESTORE_SCRIPT
+                        + ' --sourcedir=' + os.path.join(self.patroni_path, 'data', 'basebackup')).replace('\\', '/'),
+            'test-argument': 'test-value',  # test config mapping approach on custom bootstrap/replica creation
+            **(params or {}),
+        }
+
     def bootstrap_from_backup(self, name, cluster_name):
         custom_config = {
             'scope': cluster_name,
             'bootstrap': {
                 'method': 'backup_restore',
-                'backup_restore': {
+                'backup_restore': self.backup_restore_config({
-                    'command': (self.BACKUP_RESTORE_SCRIPT + ' --sourcedir='
-                                + os.path.join(self.patroni_path, 'data', 'basebackup').replace('\\', '/')),
                     'recovery_conf': {
                         'recovery_target_action': 'promote',
                         'recovery_target_timeline': 'latest',
                         'restore_command': (self.ARCHIVE_RESTORE_SCRIPT + ' --mode restore '
                                             + '--dirname {} --filename %f --pathname %p').format(
                             os.path.join(self.patroni_path, 'data', 'wal_archive_clone').replace('\\', '/'))
-                    }
+                    },
-                }
+                })
             },
             'postgresql': {
                 'authentication': {
@@ -928,11 +934,7 @@ class PatroniPoolController(object):
                     .format(os.path.join(self.patroni_path, 'data', 'wal_archive').replace('\\', '/'))
                 },
                 'create_replica_methods': ['no_leader_bootstrap'],
-                'no_leader_bootstrap': {
+                'no_leader_bootstrap': self.backup_restore_config({'no_leader': '1'})
-                    'command': (self.BACKUP_RESTORE_SCRIPT + ' --sourcedir='
-                                + os.path.join(self.patroni_path, 'data', 'basebackup').replace('\\', '/')),
-                    'no_leader': '1'
-                }
             }
         }
         self.start(name, custom_config=custom_config)

features/steps/standby_cluster.py

@@ -15,9 +15,7 @@ def start_patroni(context, name, cluster_name):
         "scope": cluster_name,
         "postgresql": {
             "callbacks": callbacks(context, name),
-            "backup_restore": {
+            "backup_restore": context.pctl.backup_restore_config()
-                "command": (context.pctl.PYTHON + " features/backup_restore.py --sourcedir="
-                            + os.path.join(context.pctl.patroni_path, 'data', 'basebackup').replace('\\', '/'))}
         }
     })
 

patroni/postgresql/bootstrap.py

@@ -151,9 +151,47 @@ class Bootstrap(object):
             os.unlink(trigger_file)
 
     def _custom_bootstrap(self, config: Any) -> bool:
+        """Bootstrap a fresh Patroni cluster using a custom method provided by the user.
+
+        :param config: configuration used for running a custom bootstrap method. It comes from the Patroni YAML file,
+            so it is expected to be a :class:`dict`.
+
+        .. note::
+            *config* must contain a ``command`` key, which value is the command or script to perform the custom
+            bootstrap procedure. The exit code of the ``command`` dictates if the bootstrap succeeded or failed.
+
+            When calling ``command``, Patroni will pass the following arguments to the ``command`` call:
+
+                * ``--scope``: contains the value of ``scope`` configuration;
+                * ``--data_dir``: contains the value of the ``postgresql.data_dir`` configuration.
+
+            You can avoid that behavior by filling the optional key ``no_params`` with the value ``False`` in the
+            configuration file, which will instruct Patroni to not pass these parameters to the ``command`` call.
+
+            Besides that, a couple more keys are supported in *config*, but optional:
+
+                * ``keep_existing_recovery_conf``: if ``True``, instruct Patroni to not remove the existing
+                  ``recovery.conf`` (PostgreSQL <= 11), to not discard recovery parameters from the configuration
+                  (PostgreSQL >= 12), and to not remove the files ``recovery.signal`` or ``standby.signal``
+                  (PostgreSQL >= 12). This is specially useful when you are restoring backups through tools like
+                  pgBackRest and Barman, in which case they generated the appropriate recovery settings for you;
+                * ``recovery_conf``: a section containing a map, where each key is the name of a recovery related
+                  setting, and the value is the value of the corresponding setting.
+
+            Any key/value other than the ones that were described above will be interpreted as additional arguments for
+            the ``command`` call. They will all be added to the call in the format ``--key=value``.
+
+        :returns: ``True`` if the bootstrap was successful, i.e. the execution of the custom ``command`` from *config*
+            exited with code ``0``, ``False`` otherwise.
+        """
         self._postgresql.set_state('running custom bootstrap script')
         params = [] if config.get('no_params') else ['--scope=' + self._postgresql.scope,
                                                      '--datadir=' + self._postgresql.data_dir]
+        # Add custom parameters specified by the user
+        reserved_args = {'no_params', 'keep_existing_recovery_conf', 'recovery_conf', 'scope', 'datadir'}
+        for arg, val in config.items():
+            if arg not in reserved_args:
+                params.append(f"--{arg}={val}")
         try:
             logger.info('Running custom bootstrap script: %s', config['command'])
             if self._postgresql.cancellable.call(shlex.split(config['command']) + params) != 0: