gitops/patroni
1
0
Fork 0
mirror of https://github.com/outbackdingo/patroni.git synced 2026-08-25 14:53:37 +00:00
Code Issues Packages Projects Releases Wiki Activity

Validate-config fixes (#2678)

Browse Source 
- fix --validate-config not to error out if bin_dir is an empty string in the yaml config
- mention bin_dir optionality in the docs
- validate bin_dir even if it is not in the yaml config (add optional
  default value for Optional config params in validator)
- make rewind user optional
This commit is contained in:
Polina Bungina
2023-05-15 13:40:22 +02:00
committed by GitHub
parent 44e58a1ba1
commit 506b5bec48
3 changed files with 60 additions and 24 deletions
Download Patch File Download Diff File Expand all files Collapse all files
docs/yaml_configuration.rst
+1 -1
View File
@@ -262,7 +262,7 @@ 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 :ref:`existing <existing_data>` 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**: (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.
- **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.
patroni/validator.py
+26 -12
View File
@@ -341,14 +341,17 @@ class Optional(object):
"""Mark a configuration option as optional.
:ivar name: name of the configuration option.
:ivar default: value to set if the configuration option is not explicitly provided
"""
def __init__(self, name: str) -> None:
def __init__(self, name: str, default: OptionalType[Any] = None) -> None:
"""Create an :class:`Optional` object.
:param name: name of the configuration option.
:param default: value to set if the configuration option is not explicitly provided
"""
self.name = name
self.default = default
class Directory(object):
@@ -370,14 +373,27 @@ class Directory(object):
self.contains = contains
self.contains_executable = contains_executable
def _check_executables(self, path: OptionalType[str] = None) -> Iterator[Result]:
"""Check that all executables from contains_executable list exist within the given directory or within PATH.
:param path: optional path to the base directory against which executables will be validated.
If not provided, check within PATH.
:rtype: Iterator[:class:`Result`] objects with the error message containing the name of the executable,
if any check fails.
"""
for program in self.contains_executable or []:
if not shutil.which(program, path=path):
yield Result(False, f"does not contain '{program}' in '{(path or '$PATH')}'")
def validate(self, name: str) -> Iterator[Result]:
"""Check if the expected paths and executables can be found under *name* directory.
:param name: path to the base directory against which paths and executables will be validated.
Check against PATH if name is not provided.
:rtype: Iterator[:class:`Result`] objects with the error message related to the failure, if any check fails.
"""
if not name:
yield Result(False, "is an empty string")
yield from self._check_executables()
elif not os.path.exists(name):
yield Result(False, "Directory '{}' does not exist.".format(name))
elif not os.path.isdir(name):
@@ -387,10 +403,7 @@ class Directory(object):
for path in self.contains:
if not os.path.exists(os.path.join(name, path)):
yield Result(False, "'{}' does not contain '{}'".format(name, path))
if self.contains_executable:
for program in self.contains_executable:
if not shutil.which(program, path=name):
yield Result(False, "'{}' does not contain '{}'".format(name, program))
yield from self._check_executables(path=name)
class Schema(object):
@@ -472,8 +485,7 @@ class Schema(object):
* It must contain a ``bind.host`` entry which value should be valid as per function ``validate_host``;
* It must contain a ``bind.port`` entry which value should be an :class:`int` instance;
* It must contain a ``aliases`` entry which value should be a :class:`list` of :class:`str` instances;
* It may optionally contain a ``data_directory`` entry. If not given it will assume the value
``/var/lib/myapp``;
* It may optionally contain a ``data_directory`` entry, with a value which should be a string;
* It must contain at least one of ``log_to_file`` or ``log_to_db``, with a value which should be a
:class:`bool` instance;
* It must contain a ``version`` entry which value should be either an :class:`int` or a :class:`float`
@@ -583,9 +595,11 @@ class Schema(object):
for d in self._data_key(key):
if d not in self.data and not isinstance(key, Optional):
yield Result(False, "is not defined.", path=d)
elif d not in self.data and isinstance(key, Optional):
elif d not in self.data and isinstance(key, Optional) and key.default is None:
continue
else:
if d not in self.data and isinstance(key, Optional):
self.data[d] = key.default
validator = self.validator[key]
if isinstance(key, Or) and isinstance(self.validator[key], Case):
validator = self.validator[key]._schema[d]
@@ -807,11 +821,11 @@ schema = Schema({
"authentication": {
"replication": userattributes,
"superuser": userattributes,
"rewind": userattributes
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_dir", ""): Directory(contains_executable=["pg_ctl", "initdb", "pg_controldata", "pg_basebackup",
"postgres", "pg_isready"]),
Optional("parameters"): {
Optional("unix_socket_directories"): str
},
tests/test_validator.py
+33 -11
View File
@@ -7,7 +7,7 @@ import unittest
from io import StringIO
from mock import Mock, patch, mock_open
from patroni.dcs import dcs_modules
from patroni.validator import schema
from patroni.validator import schema, Directory, Schema
available_dcs = [m.split(".")[-1] for m in dcs_modules()]
config = {
@@ -92,6 +92,16 @@ config = {
}
}
config_2 = {
"some_dir": "very_interesting_dir"
}
schema2 = Schema({
"some_dir": Directory(contains=["very_interesting_subdir", "another_interesting_subdir"])
})
required_binaries = ["pg_ctl", "initdb", "pg_controldata", "pg_basebackup", "postgres", "pg_isready"]
directories = []
files = []
binaries = []
@@ -190,6 +200,14 @@ class TestValidator(unittest.TestCase):
self.assertEqual(['consul.host', 'etcd.host', 'postgresql.bin_dir', 'postgresql.data_dir', 'postgresql.listen',
'raft.bind_addr', 'raft.self_addr', 'restapi.connect_address'], parse_output(output))
def test_bin_dir_is_empty_string_excutables_in_path(self, mock_out, mock_err):
binaries.extend(required_binaries)
c = copy.deepcopy(config)
c["postgresql"]["bin_dir"] = ""
errors = schema(c)
output = "\n".join(errors)
self.assertEqual(['raft.bind_addr', 'raft.self_addr'], parse_output(output))
@patch('subprocess.check_output', Mock(return_value=b"postgres (PostgreSQL) 12.1"))
def test_data_dir_contains_pg_version(self, mock_out, mock_err):
directories.append(config["postgresql"]["data_dir"])
@@ -197,14 +215,11 @@ class TestValidator(unittest.TestCase):
directories.append(os.path.join(config["postgresql"]["data_dir"], "pg_wal"))
files.append(os.path.join(config["postgresql"]["data_dir"], "global", "pg_control"))
files.append(os.path.join(config["postgresql"]["data_dir"], "PG_VERSION"))
binaries.append(os.path.join(config["postgresql"]["bin_dir"], "pg_ctl"))
binaries.append(os.path.join(config["postgresql"]["bin_dir"], "initdb"))
binaries.append(os.path.join(config["postgresql"]["bin_dir"], "pg_controldata"))
binaries.append(os.path.join(config["postgresql"]["bin_dir"], "pg_basebackup"))
binaries.append(os.path.join(config["postgresql"]["bin_dir"], "postgres"))
binaries.append(os.path.join(config["postgresql"]["bin_dir"], "pg_isready"))
binaries.extend(required_binaries)
c = copy.deepcopy(config)
c["postgresql"]["bin_dir"] = "" # to cover postgres --version call from PATH
with patch('patroni.validator.open', mock_open(read_data='12')):
errors = schema(config)
errors = schema(c)
output = "\n".join(errors)
self.assertEqual(['raft.bind_addr', 'raft.self_addr'], parse_output(output))
@@ -215,10 +230,10 @@ class TestValidator(unittest.TestCase):
directories.append(os.path.join(config["postgresql"]["data_dir"], "pg_wal"))
files.append(os.path.join(config["postgresql"]["data_dir"], "global", "pg_control"))
files.append(os.path.join(config["postgresql"]["data_dir"], "PG_VERSION"))
binaries.extend([os.path.join(config["postgresql"]["bin_dir"], i) for i in required_binaries])
c = copy.deepcopy(config)
c["etcd"]["hosts"] = []
c["postgresql"]["listen"] = '127.0.0.2,*:543'
del c["postgresql"]["bin_dir"]
with patch('patroni.validator.open', mock_open(read_data='11')):
errors = schema(c)
output = "\n".join(errors)
@@ -227,18 +242,19 @@ class TestValidator(unittest.TestCase):
@patch('subprocess.check_output', Mock(return_value=b"postgres (PostgreSQL) 12.1"))
def test_pg_wal_doesnt_exist(self, mock_out, mock_err):
binaries.extend([os.path.join(config["postgresql"]["bin_dir"], i) for i in required_binaries])
directories.append(config["postgresql"]["data_dir"])
directories.append(config["postgresql"]["bin_dir"])
files.append(os.path.join(config["postgresql"]["data_dir"], "global", "pg_control"))
files.append(os.path.join(config["postgresql"]["data_dir"], "PG_VERSION"))
c = copy.deepcopy(config)
del c["postgresql"]["bin_dir"]
with patch('patroni.validator.open', mock_open(read_data='11')):
errors = schema(c)
output = "\n".join(errors)
self.assertEqual(['postgresql.data_dir', 'raft.bind_addr', 'raft.self_addr'], parse_output(output))
def test_data_dir_is_empty_string(self, mock_out, mock_err):
binaries.extend(required_binaries)
directories.append(config["postgresql"]["data_dir"])
directories.append(config["postgresql"]["bin_dir"])
c = copy.deepcopy(config)
@@ -248,5 +264,11 @@ class TestValidator(unittest.TestCase):
c["postgresql"]["bin_dir"] = ""
errors = schema(c)
output = "\n".join(errors)
self.assertEqual(['kubernetes', 'postgresql.bin_dir', 'postgresql.data_dir',
self.assertEqual(['kubernetes', 'postgresql.data_dir',
'postgresql.pg_hba', 'raft.bind_addr', 'raft.self_addr'], parse_output(output))
def test_directory_contains(self, mock_out, mock_err):
directories.extend([config_2["some_dir"], os.path.join(config_2["some_dir"], "very_interesting_subdir")])
errors = schema2(config_2)
output = "\n".join(errors)
self.assertEqual(['some_dir'], parse_output(output))