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

Improve formatting of generated config and validation of ints (#2928)

Browse Source 
- order sections similar to sample configs
- add warnings and comments to `bootstrap.dcs` section.
- add `tags` and `log` sections.
- use discovered IPs in `postgresql.connect_address` and `postgresql.listen`
- set `wal_level` to `replica` for PostgreSQL 9.6+
- make unit tests pass with python 3.6
- improve config validator so it doesn't complain when some ints are strings in YAML file.
This commit is contained in:
Alexander Kukushkin
2023-10-25 14:23:57 +02:00
committed by GitHub
parent 6c06f5cc96
commit 3d527f5728
3 changed files with 161 additions and 91 deletions
Download Patch File Download Diff File Expand all files Collapse all files
patroni/config_generator.py
+90 -43
View File
@@ -9,7 +9,7 @@ import yaml
from getpass import getuser, getpass
from contextlib import contextmanager
from typing import Any, Dict, Iterator, List, Optional, Tuple, TYPE_CHECKING, Union
from typing import Any, Dict, Iterator, List, Optional, TextIO, Tuple, TYPE_CHECKING, Union
if TYPE_CHECKING: # pragma: no cover
from psycopg import Cursor
from psycopg2 import cursor
@@ -17,6 +17,7 @@ if TYPE_CHECKING: # pragma: no cover
from . import psycopg
from .config import Config
from .exceptions import PatroniException
from .log import PatroniLogger
from .postgresql.config import ConfigHandler, parse_dsn
from .postgresql.misc import postgres_major_version_to_int
from .utils import get_major_version, parse_bool, patch_config, read_stripped
@@ -93,10 +94,20 @@ class AbstractConfigGenerator(abc.ABC):
template_config: Dict[str, Any] = {
'scope': NO_VALUE_MSG,
'name': cls._HOSTNAME,
'restapi': {
'connect_address': cls._IP + ':8008',
'listen': cls._IP + ':8008'
},
'log': {
'level': PatroniLogger.DEFAULT_LEVEL,
'traceback_level': PatroniLogger.DEFAULT_TRACEBACK_LEVEL,
'format': PatroniLogger.DEFAULT_FORMAT,
'max_queue_size': PatroniLogger.DEFAULT_MAX_QUEUE_SIZE
},
'postgresql': {
'data_dir': NO_VALUE_MSG,
'connect_address': NO_VALUE_MSG + ':5432',
'listen': NO_VALUE_MSG + ':5432',
'connect_address': cls._IP + ':5432',
'listen': cls._IP + ':5432',
'bin_dir': '',
'authentication': {
'superuser': {
@@ -109,9 +120,11 @@ class AbstractConfigGenerator(abc.ABC):
}
}
},
'restapi': {
'connect_address': cls._IP + ':8008',
'listen': cls._IP + ':8008'
'tags': {
'failover_priority': 1,
'noloadbalance': False,
'clonefrom': True,
'nosync': False,
}
}
@@ -130,6 +143,72 @@ class AbstractConfigGenerator(abc.ABC):
def generate(self) -> None:
"""Generate config and store in :attr:`~AbstractConfigGenerator.config`."""
@staticmethod
def _format_block(block: Any, line_prefix: str = '') -> str:
"""Format a single YAML block.
.. note::
Optionally the formatted block could be indented with the *line_prefix*
:param block: the object that should be formatted to YAML.
:param line_prefix: is used for indentation.
:returns: a formatted and indented *block*.
"""
return line_prefix + yaml.safe_dump(block, default_flow_style=False, line_break='\n',
allow_unicode=True, indent=2).strip().replace('\n', '\n' + line_prefix)
def _format_config_section(self, section_name: str) -> Iterator[str]:
"""Format and yield as single section of the current :attr:`~AbstractConfigGenerator.config`.
.. note::
If the section is a :class:`dict` object we put an empty line before it.
:param section_name: a section name in the :attr:`~AbstractConfigGenerator.config`.
:yields: a formatted section in case if it exists in the :attr:`~AbstractConfigGenerator.config`.
"""
if section_name in self.config:
if isinstance(self.config[section_name], dict):
yield ''
yield self._format_block({section_name: self.config[section_name]})
def _format_config(self) -> Iterator[str]:
"""Format current :attr:`~AbstractConfigGenerator.config` and enrich it with some comments.
:yields: formatted lines or blocks that represent a text output of the YAML document.
"""
for name in ('scope', 'namespace', 'name', 'log', 'restapi', 'ctl' 'citus',
'consul', 'etcd', 'etcd3', 'exhibitor', 'kubernetes', 'raft', 'zookeeper'):
yield from self._format_config_section(name)
if 'bootstrap' in self.config:
yield '\n# The bootstrap configuration. Works only when the cluster is not yet initialized.'
yield '# If the cluster is already initialized, all changes in the `bootstrap` section are ignored!'
yield 'bootstrap:'
if 'dcs' in self.config['bootstrap']:
yield ' # This section will be written into <dcs>:/<namespace>/<scope>/config after initializing'
yield ' # new cluster and all other cluster members will use it as a `global configuration`.'
yield ' # WARNING! If you want to change any of the parameters that were set up'
yield ' # via `bootstrap.dcs` section, please use `patronictl edit-config`!'
yield ' dcs:'
for name in ('loop_wait', 'retry_timeout', 'ttl'):
if name in self.config['bootstrap']['dcs']:
yield self._format_block({name: self.config['bootstrap']['dcs'].pop(name)}, ' ')
for name, value in self.config['bootstrap']['dcs'].items():
yield self._format_block({name: value}, ' ')
for name in ('postgresql', 'watchdog', 'tags'):
yield from self._format_config_section(name)
def _write_config_to_fd(self, fd: TextIO) -> None:
"""Format and write current :attr:`~AbstractConfigGenerator.config` to provided file descriptor.
:param fd: where to write the config file. Could be ``sys.stdout`` or the real file.
"""
fd.write('\n'.join(self._format_config()))
def write_config(self) -> None:
"""Write current :attr:`~AbstractConfigGenerator.config` to the output file if provided, to stdout otherwise."""
if self.output_file:
@@ -137,9 +216,9 @@ class AbstractConfigGenerator(abc.ABC):
if dir_path and not os.path.isdir(dir_path):
os.makedirs(dir_path)
with open(self.output_file, 'w', encoding='UTF-8') as output_file:
yaml.safe_dump(self.config, output_file, default_flow_style=False, allow_unicode=True)
self._write_config_to_fd(output_file)
else:
yaml.safe_dump(self.config, sys.stdout, default_flow_style=False, allow_unicode=True)
self._write_config_to_fd(sys.stdout)
class SampleConfigGenerator(AbstractConfigGenerator):
@@ -182,6 +261,9 @@ class SampleConfigGenerator(AbstractConfigGenerator):
self.config['bootstrap']['dcs']['postgresql']['parameters'][wal_keep_param] = \
ConfigHandler.CMDLINE_OPTIONS[wal_keep_param][0]
wal_level = 'hot_standby' if self.pg_major < 90600 else 'replica'
self.config['bootstrap']['dcs']['postgresql']['parameters']['wal_level'] = wal_level
self.config['bootstrap']['dcs']['postgresql']['use_pg_rewind'] = True
if self.pg_major >= 110000:
self.config['postgresql']['authentication'].setdefault(
@@ -411,41 +493,6 @@ class RunningClusterConfigGenerator(AbstractConfigGenerator):
def generate_config(output_file: str, sample: bool, dsn: Optional[str]) -> None:
"""Generate Patroni configuration file.
Gather all the available non-internal GUC values having configuration file, postmaster command line or environment
variable as a source and store them in the appropriate part of Patroni configuration (``postgresql.parameters`` or
``bootstrap.dcs.postgresql.parameters``). Either the provided DSN (takes precedence) or PG ENV vars will be used
for the connection. If password is not provided, it should be entered via prompt.
The created configuration contains:
* ``scope``: ``cluster_name`` GUC value or ``PATRONI_SCOPE ENV`` variable value if available.
* ``name``: ``PATRONI_NAME`` ENV variable value if set, otherwise hostname.
* ``bootstrap.dcs``: section with all the parameters (incl. the majority of PG GUCs) set to their default values
defined by Patroni and adjusted by the source instances's configuration values.
* ``postgresql.parameters``: the source instance's ``archive_command``, ``restore_command``,
``archive_cleanup_command``, ``recovery_end_command``, ``ssl_passphrase_command``, ``hba_file``, ``ident_file``,
``config_file`` GUC values.
* ``postgresql.bin_dir``: path to Postgres binaries gathered from the running instance or, if not available,
the value of ``PATRONI_POSTGRESQL_BIN_DIR`` ENV variable. Otherwise, an empty string.
* ``postgresql.datadir``: the value gathered from the corresponding PG GUC.
* ``postgresql.listen``: source instance's ``listen_addresses`` and port GUC values.
* ``postgresql.connect_address``: if possible, generated from the connection params.
* ``postgresql.authentication``:
* superuser and replication users defined (if possible, usernames are set from the respective Patroni ENV vars,
otherwise the default ``postgres`` and ``replicator`` values are used).
If not a sample config, either DSN or PG ENV vars are used to define superuser authentication parameters.
* rewind user is defined only for sample config, if PG version can be defined and PG version is >=11
(if possible, username is set from the respective Patroni ENV var).
* ``bootstrap.dcs.postgresql.use_pg_rewind`` set to ``True`` for a sample config only.
* ``postgresql.pg_hba`` defaults or the lines gathered from the source instance's ``hba_file``.
* ``postgresql.pg_ident`` the lines gathered from the source instance's ``ident_file``.
:param output_file: Full path to the configuration file to be used. If not provided, result is sent to ``stdout``.
:param sample: Optional flag. If set, no source instance will be used - generate config with some sane defaults.
:param dsn: Optional DSN string for the local instance to get GUC values from.
patroni/validator.py
+30 -29
View File
@@ -814,27 +814,28 @@ def assert_(condition: bool, message: str = "Wrong value") -> None:
class IntValidator(object):
"""Validate an integer setting.
:cvar expected_type: the expected Python type for an integer setting (:class:`int`).
:ivar min: minimum allowed value for the setting, if any.
:ivar max: maximum allowed value for the setting, if any.
:ivar base_unit: the base unit to convert the value to before checking if it's within *min* and *max* range.
:ivar expected_type: the expected Python type.
:ivar raise_assert: if an ``assert`` test should be performed regarding expected type and valid range.
"""
expected_type = int
def __init__(self, min: OptionalType[int] = None, max: OptionalType[int] = None,
base_unit: OptionalType[str] = None, raise_assert: bool = False) -> None:
base_unit: OptionalType[str] = None, expected_type: Any = None, raise_assert: bool = False) -> None:
"""Create an :class:`IntValidator` object with the given rules.
:param min: minimum allowed value for the setting, if any.
:param max: maximum allowed value for the setting, if any.
:param base_unit: the base unit to convert the value to before checking if it's within *min* and *max* range.
:param expected_type: the expected Python type.
:param raise_assert: if an ``assert`` test should be performed regarding expected type and valid range.
"""
self.min = min
self.max = max
self.base_unit = base_unit
if expected_type:
self.expected_type = expected_type
self.raise_assert = raise_assert
def __call__(self, value: Any) -> bool:
@@ -953,36 +954,36 @@ schema = Schema({
Optional("allowlist_include_members"): bool,
Optional("http_extra_headers"): dict,
Optional("https_extra_headers"): dict,
Optional("request_queue_size"): IntValidator(min=0, max=4096, raise_assert=True)
Optional("request_queue_size"): IntValidator(min=0, max=4096, expected_type=int, raise_assert=True)
},
Optional("bootstrap"): {
"dcs": {
Optional("ttl"): int,
Optional("loop_wait"): int,
Optional("retry_timeout"): int,
Optional("maximum_lag_on_failover"): int,
Optional("maximum_lag_on_syncnode"): int,
Optional("ttl"): IntValidator(min=20, raise_assert=True),
Optional("loop_wait"): IntValidator(min=1, raise_assert=True),
Optional("retry_timeout"): IntValidator(min=3, raise_assert=True),
Optional("maximum_lag_on_failover"): IntValidator(min=0, raise_assert=True),
Optional("maximum_lag_on_syncnode"): IntValidator(min=-1, raise_assert=True),
Optional("postgresql"): {
Optional("parameters"): {
Optional("max_connections"): int,
Optional("max_locks_per_transaction"): int,
Optional("max_prepared_transactions"): int,
Optional("max_replication_slots"): int,
Optional("max_wal_senders"): int,
Optional("max_worker_processes"): int
Optional("max_connections"): IntValidator(1, 262143, raise_assert=True),
Optional("max_locks_per_transaction"): IntValidator(10, 2147483647, raise_assert=True),
Optional("max_prepared_transactions"): IntValidator(0, 262143, raise_assert=True),
Optional("max_replication_slots"): IntValidator(0, 262143, raise_assert=True),
Optional("max_wal_senders"): IntValidator(0, 262143, raise_assert=True),
Optional("max_worker_processes"): IntValidator(0, 262143, raise_assert=True),
},
Optional("use_pg_rewind"): bool,
Optional("pg_hba"): [str],
Optional("pg_ident"): [str],
Optional("pg_ctl_timeout"): int,
Optional("pg_ctl_timeout"): IntValidator(min=0, raise_assert=True),
Optional("use_slots"): bool,
},
Optional("primary_start_timeout"): int,
Optional("primary_stop_timeout"): int,
Optional("primary_start_timeout"): IntValidator(min=0, raise_assert=True),
Optional("primary_stop_timeout"): IntValidator(min=0, raise_assert=True),
Optional("standby_cluster"): {
Or("host", "port", "restore_command"): Case({
"host": str,
"port": int,
"port": IntValidator(max=65535, expected_type=int, raise_assert=True),
"restore_command": str
}),
Optional("primary_slot_name"): str,
@@ -992,7 +993,7 @@ schema = Schema({
},
Optional("synchronous_mode"): bool,
Optional("synchronous_mode_strict"): bool,
Optional("synchronous_node_count"): int
Optional("synchronous_node_count"): IntValidator(min=1, raise_assert=True),
},
Optional("initdb"): [Or(str, dict)],
Optional("method"): str
@@ -1003,7 +1004,7 @@ schema = Schema({
"host": validate_host_port,
"url": str
}),
Optional("port"): int,
Optional("port"): IntValidator(max=65535, expected_type=int, raise_assert=True),
Optional("scheme"): str,
Optional("token"): str,
Optional("verify"): bool,
@@ -1023,8 +1024,8 @@ schema = Schema({
"etcd3": validate_etcd,
"exhibitor": {
"hosts": [str],
"port": IntValidator(max=65535, raise_assert=True),
Optional("pool_interval"): int
"port": IntValidator(max=65535, expected_type=int, raise_assert=True),
Optional("poll_interval"): IntValidator(min=1, expected_type=int, raise_assert=True),
},
"raft": {
"self_addr": validate_connect_address,
@@ -1055,14 +1056,14 @@ schema = Schema({
Optional("tmp_role_label"): str,
Optional("use_endpoints"): bool,
Optional("pod_ip"): Or(is_ipv4_address, is_ipv6_address),
Optional("ports"): [{"name": str, "port": int}],
Optional("ports"): [{"name": str, "port": IntValidator(max=65535, expected_type=int, raise_assert=True)}],
Optional("cacert"): str,
Optional("retriable_http_codes"): Or(int, [int]),
},
}),
Optional("citus"): {
"database": str,
"group": int
"group": IntValidator(min=0, expected_type=int, raise_assert=True),
},
"postgresql": {
"listen": validate_host_port_listen_multiple_hosts,
@@ -1089,18 +1090,18 @@ schema = Schema({
},
Optional("pg_hba"): [str],
Optional("pg_ident"): [str],
Optional("pg_ctl_timeout"): int,
Optional("pg_ctl_timeout"): IntValidator(min=0, raise_assert=True),
Optional("use_pg_rewind"): bool
},
Optional("watchdog"): {
Optional("mode"): validate_watchdog_mode,
Optional("device"): str,
Optional("safety_margin"): int
Optional("safety_margin"): IntValidator(min=-1, expected_type=int, raise_assert=True),
},
Optional("tags"): {
AtMostOne("nofailover", "failover_priority"): Case({
"nofailover": bool,
"failover_priority": IntValidator(min=0, raise_assert=True),
"failover_priority": IntValidator(min=0, expected_type=int, raise_assert=True),
}),
Optional("clonefrom"): bool,
Optional("noloadbalance"): bool,
tests/test_config_generator.py
+41 -19
View File
@@ -1,14 +1,16 @@
import os
import psutil
import unittest
import yaml
from . import MockConnect, MockCursor, MockConnectionInfo
from copy import deepcopy
from mock import MagicMock, Mock, PropertyMock, mock_open, patch
from mock import MagicMock, Mock, PropertyMock, mock_open as _mock_open, patch
from patroni.__main__ import main as _main
from patroni.config import Config
from patroni.config_generator import AbstractConfigGenerator, get_address, NO_VALUE_MSG
from patroni.log import PatroniLogger
from patroni.utils import patch_config
from . import psycopg_connect
@@ -17,6 +19,14 @@ HOSTNAME = 'test_hostname'
IP = '1.9.8.4'
def mock_open(*args, **kwargs):
ret = _mock_open(*args, **kwargs)
ret.return_value.__iter__ = lambda o: iter(o.readline, '')
if not kwargs.get('read_data'):
ret.return_value.readline = Mock(return_value=None)
return ret
@patch('patroni.psycopg.connect', psycopg_connect)
@patch('builtins.open', MagicMock())
@patch('subprocess.check_output', Mock(return_value=b"postgres (PostgreSQL) 16.2"))
@@ -51,13 +61,23 @@ class TestGenerateConfig(unittest.TestCase):
self.config = {
'scope': self.environ['PATRONI_SCOPE'],
'name': HOSTNAME,
'log': {
'level': PatroniLogger.DEFAULT_LEVEL,
'traceback_level': PatroniLogger.DEFAULT_TRACEBACK_LEVEL,
'format': PatroniLogger.DEFAULT_FORMAT,
'max_queue_size': PatroniLogger.DEFAULT_MAX_QUEUE_SIZE
},
'restapi': {
'connect_address': self.environ['PATRONI_RESTAPI_CONNECT_ADDRESS'],
'listen': self.environ['PATRONI_RESTAPI_LISTEN']
},
'bootstrap': {
'dcs': dynamic_config
},
'postgresql': {
'connect_address': NO_VALUE_MSG + ':5432',
'connect_address': IP + ':5432',
'data_dir': NO_VALUE_MSG,
'listen': NO_VALUE_MSG + ':5432',
'listen': IP + ':5432',
'pg_hba': ['host all all all md5',
f'host replication {self.environ["PATRONI_REPLICATION_USERNAME"]} all md5'],
'authentication': {'superuser': {'username': self.environ['PATRONI_SUPERUSER_USERNAME'],
@@ -68,10 +88,6 @@ class TestGenerateConfig(unittest.TestCase):
'bin_dir': self.environ['PATRONI_POSTGRESQL_BIN_DIR'],
'bin_name': {'postgres': self.environ['PATRONI_POSTGRESQL_BIN_POSTGRES']},
'parameters': {'password_encryption': 'md5'}
},
'restapi': {
'connect_address': self.environ['PATRONI_RESTAPI_CONNECT_ADDRESS'],
'listen': self.environ['PATRONI_RESTAPI_LISTEN']
}
}
@@ -119,6 +135,12 @@ class TestGenerateConfig(unittest.TestCase):
},
'rewind': None
},
},
'tags': {
'failover_priority': 1,
'noloadbalance': False,
'clonefrom': True,
'nosync': False,
}
}
patch_config(self.config, conf)
@@ -139,22 +161,21 @@ class TestGenerateConfig(unittest.TestCase):
]
@patch('os.makedirs')
@patch('yaml.safe_dump')
def test_generate_sample_config_pre_13_dir_creation(self, mock_config_dump, mock_makedir):
def test_generate_sample_config_pre_13_dir_creation(self, mock_makedir):
with patch('sys.argv', ['patroni.py', '--generate-sample-config', '/foo/bar.yml']), \
patch('subprocess.check_output', Mock(return_value=b"postgres (PostgreSQL) 9.4.3")) as pg_bin_mock, \
patch('builtins.open', _mock_open()) as mocked_file, \
self.assertRaises(SystemExit) as e:
_main()
self.assertEqual(self.config, yaml.safe_load(mocked_file().write.call_args_list[0][0][0]))
self.assertEqual(e.exception.code, 0)
self.assertEqual(self.config, mock_config_dump.call_args[0][0])
mock_makedir.assert_called_once()
pg_bin_mock.assert_called_once_with([os.path.join(self.environ['PATRONI_POSTGRESQL_BIN_DIR'],
self.environ['PATRONI_POSTGRESQL_BIN_POSTGRES']),
'--version'])
@patch('os.makedirs', Mock())
@patch('yaml.safe_dump')
def test_generate_sample_config_16(self, mock_config_dump):
def test_generate_sample_config_16(self):
conf = {
'bootstrap': {
'dcs': {
@@ -182,14 +203,15 @@ class TestGenerateConfig(unittest.TestCase):
patch_config(self.config, conf)
with patch('sys.argv', ['patroni.py', '--generate-sample-config', '/foo/bar.yml']), \
patch('builtins.open', _mock_open()) as mocked_file, \
self.assertRaises(SystemExit) as e:
_main()
self.assertEqual(self.config, yaml.safe_load(mocked_file().write.call_args_list[0][0][0]))
self.assertEqual(e.exception.code, 0)
self.assertEqual(self.config, mock_config_dump.call_args[0][0])
@patch('os.makedirs', Mock())
@patch('yaml.safe_dump')
def test_generate_config_running_instance_16(self, mock_config_dump):
@patch('sys.stdout')
def test_generate_config_running_instance_16(self, mock_sys_stdout):
self._set_running_instance_config_vals()
with patch('builtins.open', Mock(side_effect=self._get_running_instance_open_res())), \
@@ -198,11 +220,11 @@ class TestGenerateConfig(unittest.TestCase):
self.assertRaises(SystemExit) as e:
_main()
self.assertEqual(e.exception.code, 0)
self.assertEqual(self.config, mock_config_dump.call_args[0][0])
self.assertEqual(self.config, yaml.safe_load(mock_sys_stdout.write.call_args_list[0][0][0]))
@patch('os.makedirs', Mock())
@patch('yaml.safe_dump')
def test_generate_config_running_instance_16_connect_from_env(self, mock_config_dump):
@patch('sys.stdout')
def test_generate_config_running_instance_16_connect_from_env(self, mock_sys_stdout):
self._set_running_instance_config_vals()
# su auth params and connect host from env
os.environ['PGCHANNELBINDING'] = \
@@ -245,7 +267,7 @@ class TestGenerateConfig(unittest.TestCase):
self.assertRaises(SystemExit) as e:
_main()
self.assertEqual(e.exception.code, 0)
self.assertEqual(self.config, mock_config_dump.call_args[0][0])
self.assertEqual(self.config, yaml.safe_load(mock_sys_stdout.write.call_args_list[0][0][0]))
def test_generate_config_running_instance_errors(self):
# 1. Wrong DSN format