From d11328020de740523b42865c629e35323af43ae5 Mon Sep 17 00:00:00 2001 From: Israel Date: Tue, 30 May 2023 08:57:57 -0300 Subject: [PATCH] Add support for custom Postgres binary names (#2692) When using a custom Postgres distribution it may be the case that the Postgres binaries are compiled with different names other than the ones used by the community Postgres distribution. With that in mind we implemented a new set of settings for Patroni, so the user is able to override the default binary names with custom binary names through the new section postgresql.bin_name in the local configuration. References: PAT-17. --- docs/ENVIRONMENT.rst | 2 +- docs/yaml_configuration.rst | 11 ++++- patroni/postgresql/__init__.py | 13 ++++- patroni/validator.py | 89 ++++++++++++++++++++++++++++++++-- tests/test_validator.py | 35 +++++++++++++ 5 files changed, 142 insertions(+), 8 deletions(-) diff --git a/docs/ENVIRONMENT.rst b/docs/ENVIRONMENT.rst index d7e9670b..d2f8a49b 100644 --- a/docs/ENVIRONMENT.rst +++ b/docs/ENVIRONMENT.rst @@ -135,7 +135,7 @@ PostgreSQL - **PATRONI\_POSTGRESQL\_PROXY\_ADDRESS**: IP address + port through which a connection pool (e.g. pgbouncer) running next to Postgres is accessible. The value is written to the member key in DCS as ``proxy_url`` and could be used/useful for service discovery. - **PATRONI\_POSTGRESQL\_DATA\_DIR**: The location of the Postgres data directory, either existing or to be initialized by Patroni. - **PATRONI\_POSTGRESQL\_CONFIG\_DIR**: The location of the Postgres configuration directory, defaults to the data directory. Must be writable by Patroni. -- **PATRONI\_POSTGRESQL\_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. +- **PATRONI\_POSTGRESQL\_BIN_DIR**: Path to PostgreSQL binaries. (pg_ctl, initdb, pg_controldata, pg_basebackup, postgres, pg_isready, pg_rewind) The default value is an empty string meaning that PATH environment variable will be used to find the executables. - **PATRONI\_POSTGRESQL\_PGPASS**: path to the `.pgpass `__ password file. Patroni creates this file before executing pg\_basebackup and under some other circumstances. The location must be writable by Patroni. - **PATRONI\_REPLICATION\_USERNAME**: replication username; the user will be created during initialization. Replicas will use this user to access the replication source via streaming replication - **PATRONI\_REPLICATION\_PASSWORD**: replication password; the user will be created during initialization. diff --git a/docs/yaml_configuration.rst b/docs/yaml_configuration.rst index 598143a7..f58c9b0a 100644 --- a/docs/yaml_configuration.rst +++ b/docs/yaml_configuration.rst @@ -258,7 +258,16 @@ PostgreSQL own config item. See :ref:`custom replica creation methods documentation ` for further explanation. - **data\_dir**: The location of the Postgres data directory, either :ref:`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**: (optional) Path to PostgreSQL binaries (pg_ctl, pg_rewind, pg_basebackup, postgres). If not provided or is an empty string, PATH environment variable will be used to find the executables. + - **bin\_dir**: (optional) Path to PostgreSQL binaries (pg_ctl, initdb, pg_controldata, pg_basebackup, postgres, pg_isready, pg_rewind). If not provided or is an empty string, PATH environment variable will be used to find the executables. + - **bin\_name**: (optional) Make it possible to override Postgres binary names, if you are using a custom Postgres distribution: + + - **pg\_ctl**: (optional) Custom name for ``pg_ctl`` binary. + - **initdb**: (optional) Custom name for ``initdb`` binary. + - **pg\controldata**: (optional) Custom name for ``pg_controldata`` binary. + - **pg\_basebackup**: (optional) Custom name for ``pg_basebackup`` binary. + - **postgres**: (optional) Custom name for ``postgres`` binary. + - **pg\_isready**: (optional) Custom name for ``pg_isready`` binary. + - **pg\_rewind**: (optional) Custom name for ``pg_rewind`` binary. - **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 defined, Patroni will use the 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 the default value should be used and omit ``host`` from the connection parameters. - **use\_unix\_socket\_repl**: specifies that Patroni should prefer to use unix sockets for replication user cluster connection. Default value is ``false``. If ``unix_socket_directories`` is defined, Patroni will use the 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 the default value should be used and omit ``host`` from the connection parameters. diff --git a/patroni/postgresql/__init__.py b/patroni/postgresql/__init__.py index 305aa4ce..6d2d18af 100644 --- a/patroni/postgresql/__init__.py +++ b/patroni/postgresql/__init__.py @@ -227,8 +227,17 @@ class Postgresql(object): return 0 def pgcommand(self, cmd: str) -> str: - """Returns path to the specified PostgreSQL command""" - return os.path.join(self._bin_dir, cmd) + """Return path to the specified PostgreSQL command. + + .. note:: + If ``postgresql.bin_name.*cmd*`` was configured by the user then that binary name is used, otherwise the + default binary name *cmd* is used. + + :param cmd: the Postgres binary name to get path to. + + :returns: path to Postgres binary named *cmd*. + """ + return os.path.join(self._bin_dir, (self.config.get('bin_name', {}) or {}).get(cmd, cmd)) def pg_ctl(self, cmd: str, *args: str, **kwargs: Any) -> bool: """Builds and executes pg_ctl command diff --git a/patroni/validator.py b/patroni/validator.py index 9a4b117c..f780f9e0 100644 --- a/patroni/validator.py +++ b/patroni/validator.py @@ -176,6 +176,16 @@ def is_ipv6_address(ip: str) -> bool: return True +def get_bin_name(bin_name: str) -> str: + """Get the value of ``postgresql.bin_name[*bin_name*]`` configuration option. + + :param bin_name: a key to be retrieved from ``postgresql.bin_name`` configuration. + + :returns: value of ``postgresql.bin_name[*bin_name*]``, if present, otherwise *bin_name*. + """ + return (schema.data.get('postgresql', {}).get('bin_name', {}) or {}).get(bin_name, bin_name) + + def get_major_version(bin_dir: OptionalType[str] = None) -> str: """Get the major version of PostgreSQL. @@ -191,9 +201,9 @@ def get_major_version(bin_dir: OptionalType[str] = None) -> str: * Returns `15` for PostgreSQL 15.2 """ if not bin_dir: - binary = 'postgres' + binary = get_bin_name('postgres') else: - binary = os.path.join(bin_dir, 'postgres') + binary = os.path.join(bin_dir, get_bin_name('postgres')) version = subprocess.check_output([binary, '--version']).decode() version = re.match(r'^[^\s]+ [^\s]+ (\d+)(\.(\d+))?', version) if TYPE_CHECKING: # pragma: no cover @@ -242,6 +252,38 @@ def validate_data_dir(data_dir: str) -> bool: return True +def validate_binary_name(bin_name: str) -> bool: + """Validate the value of ``postgresql.binary_name[*bin_name*]`` configuration option. + + If ``postgresql.bin_dir`` is set and the value of the *bin_name* meets these conditions: + + * The path join of ``postgresql.bin_dir`` plus the *bin_name* value exists; and + * The path join as above is executable + + If ``postgresql.bin_dir`` is not set, then validate that the value of *bin_name* meets this + condition: + + * Is found in the system PATH using ``which`` + + :param bin_name: the value of the ``postgresql.bin_name[*bin_name*]`` + + :returns: ``True`` if the conditions are true + + :raises :class:`patroni.exceptions.ConfigParserError`: if: + * *bin_name* is not set; or + * the path join of the ``postgresql.bin_dir`` plus *bin_name* does not exist; or + * the path join as above is not executable; or + * the *bin_name* cannot be found in the system PATH + + """ + if not bin_name: + raise ConfigParseError("is an empty string") + bin_dir = schema.data.get('postgresql', {}).get('bin_dir', None) + if not shutil.which(bin_name, path=bin_dir): + raise ConfigParseError(f"does not contain '{bin_name}' in '{bin_dir or '$PATH'}'") + return True + + class Result(object): """Represent the result of a given validation that was performed. @@ -406,6 +448,31 @@ class Directory(object): yield from self._check_executables(path=name) +class BinDirectory(Directory): + """Check if a Postgres binary directory contains the expected files. + + It is a subclass of :class:`Directory` with an extended capability: translating ``BINARIES`` according to configured + ``postgresql.bin_name``, if any. + + :cvar BINARIES: list of executable files that should exist directly under a given Postgres binary directory. + """ + + # ``pg_rewind`` is not in the list because its usage by Patroni is optional. Also, it is not available by default on + # Postgres 9.3 and 9.4, versions which Patroni supports. + BINARIES = ["pg_ctl", "initdb", "pg_controldata", "pg_basebackup", "postgres", "pg_isready"] + + def validate(self, name: str) -> Iterator[Result]: + """Check if the expected executables can be found under *name* binary directory. + + :param name: path to the base directory against which executables will be validated. Check against PATH if + *name* is not provided. + + :yields: objects with the error message related to the failure, if any check fails. + """ + self.contains_executable: List[str] = [get_bin_name(binary) for binary in self.BINARIES] + yield from super().validate(name) + + class Schema(object): """Define a configuration schema. @@ -700,6 +767,7 @@ class IntValidator(object): :ivar base_unit: the base unit to convert the value to before checking if it's within `min` and `max` range. :ivar raise_assert: if an ``assert`` call should be performed regarding expected type and valid range. """ + expected_type = int def __init__(self, min: OptionalType[int] = None, max: OptionalType[int] = None, @@ -736,6 +804,10 @@ class IntValidator(object): def validate_watchdog_mode(value: Any) -> None: + """Validate ``watchdog.mode`` configuration option. + + :param value: value of ``watchdog.mode`` to be validated. + """ assert_(isinstance(value, (str, bool)), "expected type is not a string") assert_(value in (False, "off", "automatic", "required")) @@ -748,6 +820,7 @@ setattr(validate_connect_address, 'expected_type', str) setattr(validate_host_port_listen, 'expected_type', str) setattr(validate_host_port_listen_multiple_hosts, 'expected_type', str) setattr(validate_data_dir, 'expected_type', str) +setattr(validate_binary_name, 'expected_type', str) validate_etcd = { Or("host", "hosts", "srv", "srv_suffix", "url", "proxy"): Case({ "host": validate_host_port, @@ -823,8 +896,16 @@ schema = Schema({ Optional("rewind"): userattributes }, "data_dir": validate_data_dir, - Optional("bin_dir", ""): Directory(contains_executable=["pg_ctl", "initdb", "pg_controldata", "pg_basebackup", - "postgres", "pg_isready"]), + Optional("bin_name"): { + Optional("pg_ctl"): validate_binary_name, + Optional("initdb"): validate_binary_name, + Optional("pg_controldata"): validate_binary_name, + Optional("pg_basebackup"): validate_binary_name, + Optional("postgres"): validate_binary_name, + Optional("pg_isready"): validate_binary_name, + Optional("pg_rewind"): validate_binary_name, + }, + Optional("bin_dir", ""): BinDirectory(), Optional("parameters"): { Optional("unix_socket_directories"): str }, diff --git a/tests/test_validator.py b/tests/test_validator.py index cec1f140..4c195a0e 100644 --- a/tests/test_validator.py +++ b/tests/test_validator.py @@ -271,3 +271,38 @@ class TestValidator(unittest.TestCase): errors = schema2(config_2) output = "\n".join(errors) self.assertEqual(['some_dir'], parse_output(output)) + + def test_validate_binary_name(self, mock_out, mock_err): + r = copy.copy(required_binaries) + r.remove('postgres') + r.append('fake-postgres') + binaries.extend(r) + c = copy.deepcopy(config) + c["postgresql"]["bin_name"] = {"postgres": "fake-postgres"} + del c["postgresql"]["bin_dir"] + errors = schema(c) + output = "\n".join(errors) + self.assertEqual(['raft.bind_addr', 'raft.self_addr'], parse_output(output)) + + def test_validate_binary_name_missing(self, mock_out, mock_err): + r = copy.copy(required_binaries) + r.remove('postgres') + binaries.extend(r) + c = copy.deepcopy(config) + c["postgresql"]["bin_name"] = {"postgres": "fake-postgres"} + del c["postgresql"]["bin_dir"] + errors = schema(c) + output = "\n".join(errors) + self.assertEqual(['postgresql.bin_dir', 'postgresql.bin_name.postgres', 'raft.bind_addr', 'raft.self_addr'], + parse_output(output)) + + def test_validate_binary_name_empty_string(self, mock_out, mock_err): + r = copy.copy(required_binaries) + binaries.extend(r) + c = copy.deepcopy(config) + c["postgresql"]["bin_name"] = {"postgres": ""} + del c["postgresql"]["bin_dir"] + errors = schema(c) + output = "\n".join(errors) + self.assertEqual(['postgresql.bin_dir', 'postgresql.bin_name.postgres', 'raft.bind_addr', 'raft.self_addr'], + parse_output(output))