Repository: zalando/patroni
Pull request #2928 (2023-10-24 15:22:04)

Improve formatting of generated config
- 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.

coveralls: ## Pull Request Test Coverage Report for [Build 6629000864](https://coveralls.io/builds/63505902)

* **35** of **35**   **(100.0%)**  changed or added relevant lines in **1** file are covered.
* No unchanged relevant lines lost coverage.
* Overall coverage increased (+**0.001%**) to **99.864%**

---



|  Totals | [![Coverage Status](https://coveralls.io/builds/63505902/badge)](https://coveralls.io/builds/63505902) |
| :-- | --: |
| Change from base [Build 6625529639](https://coveralls.io/builds/63497701): |  0.001% |
| Covered Lines: | 13198 |
| Relevant Lines: | 13216 |

---
##### 💛  - [Coveralls](https://coveralls.io)

hughcapet: 👍 

Patch:
diff --git a/docs/existing_data.rst b/docs/existing_data.rst
index 442b0b1c..2aa3ad26 100644
--- a/docs/existing_data.rst
+++ b/docs/existing_data.rst
@@ -36,7 +36,7 @@ You can find below an overview of steps for converting an existing Postgres clus
 
    #. If you are running Postgres through systemd, then disable the Postgres systemd unit. This is performed as Patroni manages starting and stopping the Postgres daemon.
 
-   #. Create a YAML configuration file for Patroni.
+   #. Create a YAML configuration file for Patroni. You can use :ref:`Patroni configuration generation and validation tooling <validate_generate_config>` for that.
 
       * **Note (specific for the primary node):** If you have replication slots being used for replication between cluster members, then it is recommended that you enable ``use_slots`` and configure the existing replication slots as permanent via the ``slots`` configuration item. Be aware that Patroni automatically creates replication slots for replication between members, and drops replication slots that it does not recognize, when ``use_slots`` is enabled. The idea of using permanent slots here is to allow your existing slots to persist while the migration to Patroni is in progress. See :ref:`YAML Configuration Settings <yaml_configuration>` for details.
 
diff --git a/docs/patroni_configuration.rst b/docs/patroni_configuration.rst
index e18c118e..1d6c0777 100644
--- a/docs/patroni_configuration.rst
+++ b/docs/patroni_configuration.rst
@@ -143,3 +143,107 @@ Also the following Patroni configuration options **can be changed only dynamical
 Upon changing these options, Patroni will read the relevant section of the configuration stored in DCS and change its run-time values.
 
 Patroni nodes are dumping the state of the DCS options to disk upon for every change of the configuration into the file ``patroni.dynamic.json`` located in the Postgres data directory. Only the leader is allowed to restore these options from the on-disk dump if these are completely absent from the DCS or if they are invalid.
+
+
+.. _validate_generate_config:
+
+Configuration generation and validation
+---------------------------------------
+
+Patroni provides command-line interfaces for a Patroni :ref:`local configuration <yaml_configuration>` generation and validation. Using the ``patroni`` executable you can:
+
+- Create a sample local Patroni configuration;
+- Create a Patroni configuration file for the locally running PostgreSQL instance (e.g. as a preparation step for the :ref:`Patroni integration <existing_data>`);
+- Validate a given Patroni configuration file.
+
+
+Sample Patroni configuration
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+.. code:: text
+
+   patroni --generate-sample-config [configfile]
+
+Description
+"""""""""""
+
+Generate a sample Patroni configuration file in ``yaml`` format.
+Parameter values are defined using the :ref:`Environment configuration <environment>`, otherwise, if not set, the defaults used in Patroni or the ``#FIXME`` string for the values that should be later defined by the user.
+
+Some default values are defined based on the local setup:
+
+   -  **postgresql.listen**: the IP address returned by ``gethostname`` call for the current machine's hostname and the standard ``5432`` port.
+   -  **postgresql.connect_address**: the IP address returned by ``gethostname`` call for the current machine's hostname and the standard ``5432`` port.
+   -  **postgresql.authentication.rewind**: is only defined if the PostgreSQL version can be defined from the binary and the version is 11 or later.
+   -  **restapi.listen**: IP address returned by ``gethostname`` call for the current machine's hostname and the standard ``8008`` port.
+   -  **restapi.connect_address**: IP address returned by ``gethostname`` call for the current machine's hostname and the standard ``8008`` port.
+
+Parameters
+""""""""""
+
+``configfile`` - full path to the configuration file used to store the result. If not provided, the result is sent to ``stdout``.
+
+
+Patroni configuration for a running instance
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+.. code:: text
+
+   patroni --generate-config [--dsn DSN] [configfile]
+
+Description
+"""""""""""
+
+Generate a Patroni configuration in ``yaml`` format for the locally running PostgreSQL instance. 
+Either the provided DSN (takes precedence) or PostgreSQL `environment variables <https://www.postgresql.org/docs/current/libpq-envars.html>`__ will be used for the PostgreSQL connection. If the password is not provided, it should be entered via prompt.
+
+All the non-internal GUCs defined in the source Postgres instance, independently if they were set through a configuration file, through the postmaster command-line, or through environment variables, will be used as the source for the following Patroni configuration parameters:
+
+   -  **scope**: ``cluster_name`` GUC value;
+   -  **postgresql.listen**: ``listen_addresses`` and ``port`` GUC values;
+   -  **postgresql.datadir**: ``data_directory`` GUC value;
+   -  **postgresql.parameters**: ``archive_command``, ``restore_command``, ``archive_cleanup_command``, ``recovery_end_command``, ``ssl_passphrase_command``, ``hba_file``, ``ident_file``, ``config_file`` GUC values;
+   -  **bootstrap.dcs**: all other gathered PostgreSQL GUCs.
+
+If ``scope``, ``postgresql.listen`` or ``postgresql.datadir`` is not set from the Postgres GUCs, the respective :ref:`Environment configuration <environment>` value is used.
+
+Other rules applied for the values definition:
+
+   -  **name**: ``PATRONI_NAME`` environment variable value if set, otherwise the current machine's hostname.
+   -  **postgresql.bin_dir**: path to the Postgres binaries gathered from the running instance.
+   -  **postgresql.connect_address**: the IP address returned by ``gethostname`` call for the current machine's hostname and the port used for the instance connection or the ``port`` GUC value.
+   -  **postgresql.authentication.superuser**: the configuration used for the instance connection;
+   -  **postgresql.pg_hba**: the lines gathered from the source instance's ``hba_file``.
+   -  **postgresql.pg_ident**: the lines gathered from the source instance's ``ident_file``.
+   -  **restapi.listen**: IP address returned by ``gethostname`` call for the current machine's hostname and the standard ``8008`` port.
+   -  **restapi.connect_address**: IP address returned by ``gethostname`` call for the current machine's hostname and the standard ``8008`` port.
+
+Other parameters defined using :ref:`Environment configuration <environment>` are also included into the configuration.
+
+Parameters
+""""""""""
+
+``configfile``
+    Full path to the configuration file used to store the result. If not provided, result is sent to ``stdout``.
+
+``dsn``
+    Optional DSN string for the local PostgreSQL instance to get GUC values from.
+
+
+Validate Patroni configuration
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+.. code:: text
+
+   patroni --validate-config [configfile]
+
+Description
+"""""""""""
+
+Validate the given Patroni configuration and print the information about the failed checks.
+
+Parameters
+""""""""""
+
+``configfile``
+    Full path to the configuration file to check. If not given or file does not exist, will try to read from the ``PATRONI_CONFIG_VARIABLE`` environment variable or, if not set, from the :ref:`Patroni environment variables <environment>`.
diff --git a/patroni/config_generator.py b/patroni/config_generator.py
index 4bcb2819..c2b133c6 100644
--- a/patroni/config_generator.py
+++ b/patroni/config_generator.py
@@ -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.
diff --git a/patroni/validator.py b/patroni/validator.py
index 122b795e..10bc7b2a 100644
--- a/patroni/validator.py
+++ b/patroni/validator.py
@@ -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,


Test patch:
diff --git a/tests/test_config_generator.py b/tests/test_config_generator.py
index e702f957..1b436bb0 100644
--- a/tests/test_config_generator.py
+++ b/tests/test_config_generator.py
@@ -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