From bb90feb393aa8dfaf24c28acaf4519137ca7822e Mon Sep 17 00:00:00 2001 From: Israel Date: Wed, 25 Oct 2023 10:01:08 -0300 Subject: [PATCH] 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 | 25 ++++++++++++++++---- features/backup_restore.py | 1 + features/environment.py | 22 ++++++++++-------- features/steps/standby_cluster.py | 4 +--- patroni/postgresql/bootstrap.py | 38 +++++++++++++++++++++++++++++++ 5 files changed, 72 insertions(+), 18 deletions(-) diff --git a/docs/replica_bootstrap.rst b/docs/replica_bootstrap.rst index 5ae53103..861a0e86 100644 --- a/docs/replica_bootstrap.rst +++ b/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 -one of the ``recovery_target_*`` parameters, together with the ``recovery_target_timeline`` set to ``promote``. +``recovery.conf`` before starting the newly bootstrapped instance (or set the recovery settings on Postgres configuration if +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. -This is useful when bootstrapping from a backup with tools like pgBackRest that generate the appropriate ``recovery.conf`` for you. +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). +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: + : + command: + 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 diff --git a/features/backup_restore.py b/features/backup_restore.py index 5aa973c4..9b926b54 100755 --- a/features/backup_restore.py +++ b/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) diff --git a/features/environment.py b/features/environment.py index 1c3e654b..a0367657 100644 --- a/features/environment.py +++ b/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': { - 'command': (self.BACKUP_RESTORE_SCRIPT + ' --sourcedir=' - + os.path.join(self.patroni_path, 'data', 'basebackup').replace('\\', '/')), + 'backup_restore': self.backup_restore_config({ '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': { - 'command': (self.BACKUP_RESTORE_SCRIPT + ' --sourcedir=' - + os.path.join(self.patroni_path, 'data', 'basebackup').replace('\\', '/')), - 'no_leader': '1' - } + 'no_leader_bootstrap': self.backup_restore_config({'no_leader': '1'}) } } self.start(name, custom_config=custom_config) diff --git a/features/steps/standby_cluster.py b/features/steps/standby_cluster.py index a428130c..1bf3b3fb 100644 --- a/features/steps/standby_cluster.py +++ b/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": { - "command": (context.pctl.PYTHON + " features/backup_restore.py --sourcedir=" - + os.path.join(context.pctl.patroni_path, 'data', 'basebackup').replace('\\', '/'))} + "backup_restore": context.pctl.backup_restore_config() } }) diff --git a/patroni/postgresql/bootstrap.py b/patroni/postgresql/bootstrap.py index 8a9b0fba..751c0797 100644 --- a/patroni/postgresql/bootstrap.py +++ b/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: