Improve GUCs validation (#3130)

Due to postgres --describe-config not showing GUCs defined as GUC_NO_SHOW_ALL | GUC_NOT_IN_SAMPLE | GUC_DISALLOW_IN_FILE, Patroni was always ignoring some GUCs that a user might want to have configured with non-default values.

- remove postgres --describe-config validation.
- define minor versions for availability bounds of some back-patched GUCs

patroni/postgresql/__init__.py

@@ -17,7 +17,7 @@ from psutil import TimeoutExpired
 
 from .. import global_config, psycopg
 from ..async_executor import CriticalTask
-from ..collections import CaseInsensitiveDict, CaseInsensitiveSet, EMPTY_DICT
+from ..collections import CaseInsensitiveDict, EMPTY_DICT
 from ..dcs import Cluster, Leader, Member
 from ..exceptions import PostgresConnectionException
 from ..tags import Tags
@@ -82,10 +82,10 @@ class Postgresql(object):
         self.connection_pool = ConnectionPool()
         self._connection = self.connection_pool.get('heartbeat')
         self.mpp_handler = mpp.get_handler_impl(self)
+        self._bin_dir = config.get('bin_dir') or ''
         self.config = ConfigHandler(self, config)
         self.config.check_directories()
 
-        self._bin_dir = config.get('bin_dir') or ''
         self.bootstrap = Bootstrap(self)
         self.bootstrapping = False
         self.__thread_ident = current_thread().ident
@@ -119,8 +119,6 @@ class Postgresql(object):
         # Last known running process
         self._postmaster_proc = None
 
-        self._available_gucs = None
-
         if self.is_running():
             # If we found postmaster process we need to figure out whether postgres is accepting connections
             self.set_state('starting')
@@ -245,13 +243,6 @@ class Postgresql(object):
 
         return ("SELECT " + self.TL_LSN + ", {3}").format(self.wal_name, self.lsn_name, self.wal_flush, extra)
 
-    @property
-    def available_gucs(self) -> CaseInsensitiveSet:
-        """GUCs available in this Postgres server."""
-        if not self._available_gucs:
-            self._available_gucs = self._get_gucs()
-        return self._available_gucs
-
     def _version_file_exists(self) -> bool:
         return not self.data_directory_empty() and os.path.isfile(self._version_file)
 
@@ -1363,13 +1354,3 @@ class Postgresql(object):
         self.slots_handler.schedule()
         self.mpp_handler.schedule_cache_rebuild()
         self._sysid = ''
-
-    def _get_gucs(self) -> CaseInsensitiveSet:
-        """Get all available GUCs based on ``postgres --describe-config`` output.
-
-        :returns: all available GUCs in the local Postgres server.
-        """
-        cmd = [self.pgcommand('postgres'), '--describe-config']
-        return CaseInsensitiveSet({
-            line.split('\t')[0] for line in subprocess.check_output(cmd).decode('utf-8').strip().split('\n')
-        })

patroni/postgresql/available_parameters/0_postgres.yml

@@ -4,7 +4,22 @@ parameters:
     version_from: 170000
   allow_in_place_tablespaces:
   - type: Bool
-    version_from: 100000
+    version_from: 150000
+  - type: Bool
+    version_from: 140005
+    version_till: 140099
+  - type: Bool
+    version_from: 130008
+    version_till: 130099
+  - type: Bool
+    version_from: 120012
+    version_till: 120099
+  - type: Bool
+    version_from: 110017
+    version_till: 110099
+  - type: Bool
+    version_from: 100022
+    version_till: 100099
   allow_system_table_mods:
   - type: Bool
     version_from: 90300
@@ -1222,6 +1237,24 @@ parameters:
   restart_after_crash:
   - type: Bool
     version_from: 90300
+  restrict_nonsystem_relation_kind:
+  - type: String
+    version_from: 170000
+  - type: String
+    version_from: 160004
+    version_till: 160099
+  - type: String
+    version_from: 150008
+    version_till: 150099
+  - type: String
+    version_from: 140013
+    version_till: 140099
+  - type: String
+    version_from: 130016
+    version_till: 130099
+  - type: String
+    version_from: 120020
+    version_till: 120099
   row_security:
   - type: Bool
     version_from: 90500

patroni/postgresql/config.py

@@ -16,8 +16,9 @@ from ..collections import CaseInsensitiveDict, CaseInsensitiveSet, EMPTY_DICT
 from ..dcs import Leader, Member, RemoteMember, slot_name_from_member_name
 from ..exceptions import PatroniFatalException, PostgresConnectionException
 from ..file_perm import pg_perm
-from ..utils import compare_values, is_subpath, maybe_convert_from_base_unit, \
-    parse_bool, parse_int, split_host_port, uri, validate_directory
+from ..postgresql.misc import get_major_from_minor_version, postgres_version_to_int
+from ..utils import compare_values, get_postgres_version, is_subpath, \
+    maybe_convert_from_base_unit, parse_bool, parse_int, split_host_port, uri, validate_directory
 from ..validator import EnumValidator, IntValidator
 from .validator import recovery_parameters, transform_postgresql_parameter_value, transform_recovery_parameter_value
 
@@ -403,6 +404,24 @@ class ConfigHandler(object):
     def config_dir(self) -> str:
         return self._config_dir
 
+    @property
+    def pg_version(self) -> int:
+        """Current full postgres version if instance is running, major version otherwise.
+
+        We can only use ``postgres --version`` output if major version there equals to the one
+        in data directory. If it is not the case, we should use major version from the ``PG_VERSION``
+        file.
+        """
+        if self._postgresql.state == 'running':
+            try:
+                return self._postgresql.server_version
+            except AttributeError:
+                pass
+        bin_minor = postgres_version_to_int(get_postgres_version(bin_name=self._postgresql.pgcommand('postgres')))
+        bin_major = get_major_from_minor_version(bin_minor)
+        datadir_major = self._postgresql.major_version
+        return datadir_major if bin_major != datadir_major else bin_minor
+
     @property
     def _configuration_to_save(self) -> List[str]:
         configuration = [os.path.basename(self._postgresql_conf)]
@@ -486,9 +505,9 @@ class ConfigHandler(object):
         with self.config_writer(self._postgresql_conf) as f:
             include = self._config.get('custom_conf') or self._postgresql_base_conf_name
             f.writeline("include '{0}'\n".format(ConfigWriter.escape(include)))
+            version = self.pg_version
             for name, value in sorted((configuration).items()):
-                value = transform_postgresql_parameter_value(self._postgresql.major_version, name, value,
-                                                             self._postgresql.available_gucs)
+                value = transform_postgresql_parameter_value(version, name, value)
                 if value is not None and\
                         (name != 'hba_file' or not self._postgresql.bootstrap.running_custom_bootstrap):
                     f.write_param(name, value)
@@ -609,8 +628,7 @@ class ConfigHandler(object):
                     self._passfile_mtime = mtime(self._pgpass)
                 value = self.format_dsn(value)
             else:
-                value = transform_recovery_parameter_value(self._postgresql.major_version, name, value,
-                                                           self._postgresql.available_gucs)
+                value = transform_recovery_parameter_value(self._postgresql.major_version, name, value)
                 if value is None:
                     continue
             fd.write_param(name, value)

patroni/postgresql/misc.py

@@ -57,6 +57,24 @@ def postgres_major_version_to_int(pg_version: str) -> int:
     return postgres_version_to_int(pg_version + '.0')
 
 
+def get_major_from_minor_version(version: int) -> int:
+    """Extract major PostgreSQL version from the provided full version.
+
+    :param version: integer representation of PostgreSQL full version (major + minor).
+
+    :returns: integer representation of the PostgreSQL major version.
+
+    :Example:
+
+        >>> get_major_from_minor_version(100012)
+        100000
+
+        >>> get_major_from_minor_version(90313)
+        90300
+    """
+    return version // 100 * 100
+
+
 def parse_lsn(lsn: str) -> int:
     t = lsn.split('/')
     return int(t[0], 16) * 0x100000000 + int(t[1], 16)

patroni/postgresql/validator.py

@@ -6,7 +6,7 @@ from typing import Any, Dict, Iterator, List, MutableMapping, Optional, Tuple, T
 
 import yaml
 
-from ..collections import CaseInsensitiveDict, CaseInsensitiveSet
+from ..collections import CaseInsensitiveDict
 from ..exceptions import PatroniException
 from ..utils import parse_bool, parse_int, parse_real
 from .available_parameters import get_validator_files, PathLikeObj
@@ -412,9 +412,8 @@ _load_postgres_gucs_validators()
 
 
 def _transform_parameter_value(validators: MutableMapping[str, Tuple[_Transformable, ...]],
-                               version: int, name: str, value: Any,
-                               available_gucs: CaseInsensitiveSet) -> Optional[Any]:
-    """Validate *value* of GUC *name* for Postgres *version* using defined *validators* and *available_gucs*.
+                               version: int, name: str, value: Any) -> Optional[Any]:
+    """Validate *value* of GUC *name* for Postgres *version* using defined *validators*.
 
     :param validators: a dictionary of all GUCs across all Postgres versions. Each key is the name of a Postgres GUC,
         and the corresponding value is a variable length tuple of :class:`_Transformable`. Each item is a validation
@@ -423,8 +422,6 @@ def _transform_parameter_value(validators: MutableMapping[str, Tuple[_Transforma
     :param version: Postgres version to validate the GUC against.
     :param name: name of the Postgres GUC.
     :param value: value of the Postgres GUC.
-    :param available_gucs: a set of all GUCs available in Postgres *version*. Each item is the name of a Postgres
-        GUC. Used for a couple purposes:
 
         * Disallow writing GUCs to ``postgresql.conf`` (or ``recovery.conf``) that does not exist in Postgres *version*;
         * Avoid ignoring GUC *name* if it does not have a validator in *validators*, but is a valid GUC in Postgres
@@ -432,35 +429,23 @@ def _transform_parameter_value(validators: MutableMapping[str, Tuple[_Transforma
 
     :returns: the return value may be one among:
 
-        * *value* transformed to the expected format for GUC *name* in Postgres *version*, if *name* is present
-          in *available_gucs* and has a validator in *validators* for the corresponding Postgres *version*; or
-        * The own *value* if *name* is present in *available_gucs* but not in *validators*; or
-        * ``None`` if *name* is not present in *available_gucs*.
+        * *value* transformed to the expected format for GUC *name* in Postgres *version*, if *name* has a validator
+          in *validators* for the corresponding Postgres *version*; or
+        * ``None`` if *name* does not have a validator in *validators*.
     """
-    if name in available_gucs:
-        for validator in validators.get(name, ()) or ():
-            if version >= validator.version_from and\
-                    (validator.version_till is None or version < validator.version_till):
-                return validator.transform(name, value)
-        # Ideally we should have a validator in *validators*. However, if none is available, we will not discard a
-        # setting that exists in Postgres *version*, but rather allow the value with no validation.
-        return value
+    for validator in validators.get(name, ()) or ():
+        if version >= validator.version_from and\
+                (validator.version_till is None or version < validator.version_till):
+            return validator.transform(name, value)
     logger.warning('Removing unexpected parameter=%s value=%s from the config', name, value)
 
 
-def transform_postgresql_parameter_value(version: int, name: str, value: Any,
-                                         available_gucs: CaseInsensitiveSet) -> Optional[Any]:
-    """Validate *value* of GUC *name* for Postgres *version* using ``parameters`` and *available_gucs*.
+def transform_postgresql_parameter_value(version: int, name: str, value: Any) -> Optional[Any]:
+    """Validate *value* of GUC *name* for Postgres *version* using ``parameters``.
 
     :param version: Postgres version to validate the GUC against.
     :param name: name of the Postgres GUC.
     :param value: value of the Postgres GUC.
-    :param available_gucs: a set of all GUCs available in Postgres *version*. Each item is the name of a Postgres
-        GUC. Used for a couple purposes:
-
-            * Disallow writing GUCs to ``postgresql.conf`` that does not exist in Postgres *version*;
-            * Avoid ignoring GUC *name* if it does not have a validator in ``parameters``, but is a valid GUC in
-                Postgres *version*.
 
     :returns: The return value may be one among:
 
@@ -475,32 +460,17 @@ def transform_postgresql_parameter_value(version: int, name: str, value: Any,
         return value
     if name in recovery_parameters:
         return None
-    return _transform_parameter_value(parameters, version, name, value, available_gucs)
+    return _transform_parameter_value(parameters, version, name, value)
 
 
-def transform_recovery_parameter_value(version: int, name: str, value: Any,
-                                       available_gucs: CaseInsensitiveSet) -> Optional[Any]:
-    """Validate *value* of GUC *name* for Postgres *version* using ``recovery_parameters`` and *available_gucs*.
+def transform_recovery_parameter_value(version: int, name: str, value: Any) -> Optional[Any]:
+    """Validate *value* of GUC *name* for Postgres *version* using ``recovery_parameters``.
 
     :param version: Postgres version to validate the recovery GUC against.
     :param name: name of the Postgres recovery GUC.
     :param value: value of the Postgres recovery GUC.
-    :param available_gucs: a set of all GUCs available in Postgres *version*. Each item is the name of a Postgres
-        GUC. Used for a couple purposes:
-
-            * Disallow writing GUCs to ``recovery.conf`` (or ``postgresql.conf`` depending on *version*), that does not
-                exist in Postgres *version*;
-            * Avoid ignoring recovery GUC *name* if it does not have a validator in ``recovery_parameters``, but is a
-                valid GUC in Postgres *version*.
 
     :returns: *value* transformed to the expected format for recovery GUC *name* in Postgres *version* using validators
         defined in ``recovery_parameters``. It can also return ``None``. See :func:`_transform_parameter_value`.
     """
-    # Recovery settings are not present in ``postgres --describe-config`` output of Postgres <= 11. In that case we
-    # just pass down the list of settings defined in Patroni validators so :func:`_transform_parameter_value` will not
-    # discard the recovery GUCs when running Postgres <= 11.
-    # NOTE: At the moment this change was done Postgres 11 was almost EOL, and had been likely extensively used with
-    # Patroni, so we should be able to rely solely on Patroni validators as the source of truth.
-    return _transform_parameter_value(
-        recovery_parameters, version, name, value,
-        available_gucs if version >= 120000 else CaseInsensitiveSet(recovery_parameters.keys()))
+    return _transform_parameter_value(recovery_parameters, version, name, value)

patroni/utils.py

@@ -1180,10 +1180,48 @@ def unquote(string: str) -> str:
     return ret
 
 
+def get_postgres_version(bin_dir: Optional[str] = None, bin_name: str = 'postgres') -> str:
+    """Get full PostgreSQL version.
+
+    It is based on the output of ``postgres --version``.
+
+    :param bin_dir: path to the PostgreSQL binaries directory. If ``None`` or an empty string, it will use the first
+                    *bin_name* binary that is found by the subprocess in the ``PATH``.
+    :param bin_name: name of the postgres binary to call (``postgres`` by default)
+
+    :returns: the PostgreSQL version.
+
+    :raises:
+        :exc:`~patroni.exceptions.PatroniException`: if the postgres binary call failed due to :exc:`OSError`.
+
+    :Example:
+
+        * Returns `9.6.24` for PostgreSQL 9.6.24
+        * Returns `15.2` for PostgreSQL 15.2
+    """
+    if not bin_dir:
+        binary = bin_name
+    else:
+        binary = os.path.join(bin_dir, bin_name)
+    try:
+        version = subprocess.check_output([binary, '--version']).decode()
+    except OSError as e:
+        raise PatroniException(f'Failed to get postgres version: {e}')
+    version = re.match(r'^[^\s]+ [^\s]+ ((\d+)(\.\d+)*)', version)
+    if TYPE_CHECKING:  # pragma: no cover
+        assert version is not None
+    version = version.groups()  # e.g., ('15.2', '15', '.2')
+    major_version = int(version[1])
+    dot_count = version[0].count('.')
+    if major_version < 10 and dot_count < 2 or major_version >= 10 and dot_count < 1:
+        return '.'.join((version[0], '0'))
+    return version[0]
+
+
 def get_major_version(bin_dir: Optional[str] = None, bin_name: str = 'postgres') -> str:
     """Get the major version of PostgreSQL.
 
-    It is based on the output of ``postgres --version``.
+    Like func:`get_postgres_version` but without minor version.
 
     :param bin_dir: path to the PostgreSQL binaries directory. If ``None`` or an empty string, it will use the first
                     *bin_name* binary that is found by the subprocess in the ``PATH``.
@@ -1199,15 +1237,5 @@ def get_major_version(bin_dir: Optional[str] = None, bin_name: str = 'postgres')
         * Returns `9.6` for PostgreSQL 9.6.24
         * Returns `15` for PostgreSQL 15.2
     """
-    if not bin_dir:
-        binary = bin_name
-    else:
-        binary = os.path.join(bin_dir, bin_name)
-    try:
-        version = subprocess.check_output([binary, '--version']).decode()
-    except OSError as e:
-        raise PatroniException(f'Failed to get postgres version: {e}')
-    version = re.match(r'^[^\s]+ [^\s]+ (\d+)(\.(\d+))?', version)
-    if TYPE_CHECKING:  # pragma: no cover
-        assert version is not None
-    return '.'.join([version.group(1), version.group(3)]) if int(version.group(1)) < 10 else version.group(1)
+    full_version = get_postgres_version(bin_dir, bin_name)
+    return re.sub(r'\.\d+$', '', full_version)

tests/__init__.py

@@ -3,7 +3,7 @@ import os
 import shutil
 import unittest
 
-from unittest.mock import Mock, patch, PropertyMock
+from unittest.mock import Mock, patch
 
 import urllib3
 
@@ -20,15 +20,6 @@ class SleepException(Exception):
     pass
 
 
-mock_available_gucs = PropertyMock(return_value={
-    'cluster_name', 'constraint_exclusion', 'force_parallel_mode', 'hot_standby', 'listen_addresses', 'max_connections',
-    'max_locks_per_transaction', 'max_prepared_transactions', 'max_replication_slots', 'max_stack_depth',
-    'max_wal_senders', 'max_worker_processes', 'port', 'search_path', 'shared_preload_libraries',
-    'stats_temp_directory', 'synchronous_standby_names', 'track_commit_timestamp', 'unix_socket_directories',
-    'vacuum_cost_delay', 'vacuum_cost_limit', 'wal_keep_size', 'wal_level', 'wal_log_hints', 'zero_damaged_pages',
-    'autovacuum', 'wal_segment_size', 'wal_block_size', 'shared_buffers', 'wal_buffers',
-})
-
 GET_PG_SETTINGS_RESULT = [
     ('wal_segment_size', '2048', '8kB', 'integer', 'internal'),
     ('wal_block_size', '8192', None, 'integer', 'internal'),

tests/test_bootstrap.py

@@ -10,13 +10,13 @@ from patroni.postgresql.bootstrap import Bootstrap
 from patroni.postgresql.cancellable import CancellableSubprocess
 from patroni.postgresql.config import ConfigHandler, get_param_diff
 
-from . import BaseTestPostgresql, mock_available_gucs, psycopg_connect
+from . import BaseTestPostgresql, psycopg_connect
 
 
 @patch('subprocess.call', Mock(return_value=0))
+@patch('subprocess.check_output', Mock(return_value=b"postgres (PostgreSQL) 12.1"))
 @patch('patroni.psycopg.connect', psycopg_connect)
 @patch('os.rename', Mock())
-@patch.object(Postgresql, 'available_gucs', mock_available_gucs)
 class TestBootstrap(BaseTestPostgresql):
 
     @patch('patroni.postgresql.CallbackExecutor', Mock())

tests/test_patroni.py

@@ -78,7 +78,6 @@ class TestPatroni(unittest.TestCase):
     @patch.object(etcd.Client, 'read', etcd_read)
     @patch.object(Thread, 'start', Mock())
     @patch.object(AbstractEtcdClientWithFailover, '_get_machines_list', Mock(return_value=['http://remotehost:2379']))
-    @patch.object(Postgresql, '_get_gucs', Mock(return_value={'foo': True, 'bar': True}))
     def setUp(self):
         self._handlers = logging.getLogger().handlers[:]
         RestApiServer._BaseServer__is_shut_down = Mock()
@@ -102,7 +101,6 @@ class TestPatroni(unittest.TestCase):
     @patch.object(etcd.Client, 'delete', Mock())
     @patch.object(AbstractEtcdClientWithFailover, '_get_machines_list', Mock(return_value=['http://remotehost:2379']))
     @patch.object(Thread, 'join', Mock())
-    @patch.object(Postgresql, '_get_gucs', Mock(return_value={'foo': True, 'bar': True}))
     def test_patroni_patroni_main(self):
         with patch('subprocess.call', Mock(return_value=1)):
             with patch.object(Patroni, 'run', Mock(side_effect=SleepException)):

tests/test_postgresql.py

@@ -15,7 +15,7 @@ import patroni.psycopg as psycopg
 
 from patroni import global_config
 from patroni.async_executor import CriticalTask
-from patroni.collections import CaseInsensitiveDict, CaseInsensitiveSet
+from patroni.collections import CaseInsensitiveDict
 from patroni.dcs import RemoteMember
 from patroni.exceptions import PatroniException, PostgresConnectionException
 from patroni.postgresql import Postgresql, STATE_NO_RESPONSE, STATE_REJECT
@@ -24,12 +24,12 @@ from patroni.postgresql.callback_executor import CallbackAction
 from patroni.postgresql.config import _false_validator, get_param_diff
 from patroni.postgresql.postmaster import PostmasterProcess
 from patroni.postgresql.validator import _get_postgres_guc_validators, _load_postgres_gucs_validators, \
-    _read_postgres_gucs_validators_file, Bool, Enum, EnumBool, Integer, InvalidGucValidatorsFile, Real, String, \
-    ValidatorFactory, ValidatorFactoryInvalidSpec, ValidatorFactoryInvalidType, ValidatorFactoryNoType
+    _read_postgres_gucs_validators_file, Bool, Enum, EnumBool, Integer, InvalidGucValidatorsFile, \
+    Real, String, transform_postgresql_parameter_value, ValidatorFactory, ValidatorFactoryInvalidSpec, \
+    ValidatorFactoryInvalidType, ValidatorFactoryNoType
 from patroni.utils import RetryFailedError
 
-from . import BaseTestPostgresql, GET_PG_SETTINGS_RESULT, \
-    mock_available_gucs, MockCursor, MockPostmaster, psycopg_connect
+from . import BaseTestPostgresql, GET_PG_SETTINGS_RESULT, MockCursor, MockPostmaster, psycopg_connect
 
 mtime_ret = {}
 
@@ -98,8 +98,8 @@ Data page checksum version:           0
 
 
 @patch('subprocess.call', Mock(return_value=0))
+@patch('subprocess.check_output', Mock(return_value=b"postgres (PostgreSQL) 12.1"))
 @patch('patroni.psycopg.connect', psycopg_connect)
-@patch.object(Postgresql, 'available_gucs', mock_available_gucs)
 class TestPostgresql(BaseTestPostgresql):
 
     @patch('subprocess.call', Mock(return_value=0))
@@ -107,7 +107,6 @@ class TestPostgresql(BaseTestPostgresql):
     @patch('patroni.postgresql.CallbackExecutor', Mock())
     @patch.object(Postgresql, 'get_major_version', Mock(return_value=140000))
     @patch.object(Postgresql, 'is_running', Mock(return_value=True))
-    @patch.object(Postgresql, 'available_gucs', mock_available_gucs)
     def setUp(self):
         super(TestPostgresql, self).setUp()
         self.p.config.write_postgresql_conf()
@@ -530,9 +529,16 @@ class TestPostgresql(BaseTestPostgresql):
             self.assertEqual(self.p.controldata(), {})
 
     @patch('patroni.postgresql.Postgresql._version_file_exists', Mock(return_value=True))
-    @patch('subprocess.check_output', MagicMock(return_value=0, side_effect=pg_controldata_string))
     def test_sysid(self):
-        self.assertEqual(self.p.sysid, "6200971513092291716")
+        with patch('subprocess.check_output', Mock(return_value=0, side_effect=pg_controldata_string)):
+            self.assertEqual(self.p.sysid, "6200971513092291716")
+
+    def test_pg_version(self):
+        self.assertEqual(self.p.config.pg_version, 99999)  # server_version
+        with patch.object(Postgresql, 'server_version', PropertyMock(side_effect=AttributeError)):
+            self.assertEqual(self.p.config.pg_version, 140000)  # PG_VERSION==14, postgres --version == 12.1
+            with patch('subprocess.check_output', Mock(return_value=b"postgres (PostgreSQL) 14.1")):
+                self.assertEqual(self.p.config.pg_version, 140001)
 
     @patch('os.path.isfile', Mock(return_value=True))
     @patch('shutil.copy', Mock(side_effect=IOError))
@@ -896,6 +902,32 @@ class TestPostgresql(BaseTestPostgresql):
     def test_handle_parameter_change(self):
         self.p.handle_parameter_change()
 
+    @patch('patroni.postgresql.validator.logger.warning')
+    def test_transform_postgresql_parameter_value(self, mock_warning):
+        not_none_values = (
+            ('foo.bar', 'foo', 160003),  # name, value, version
+            ("allow_in_place_tablespaces", 'true', 130008),
+            ("restrict_nonsystem_relation_kind", 'view', 160005)
+        )
+        for i in not_none_values:
+            self.assertIsNotNone(
+                transform_postgresql_parameter_value(i[2], i[0], i[1])
+            )
+
+        none_values = (
+            ("archive_cleanup_command", 'foo', 160003, False),  # name, value, version, unexpected param
+            ("allow_in_place_tablespaces", 'true', 130005, True),
+            ("restrict_nonsystem_relation_kind", 'view', 160001, True),
+        )
+        for i in none_values:
+            self.assertIsNone(
+                transform_postgresql_parameter_value(i[2], i[0], i[1])
+            )
+            if i[3]:
+                mock_warning.assert_called_once_with(
+                    'Removing unexpected parameter=%s value=%s from the config', i[0], i[1])
+            mock_warning.reset_mock()
+
     def test_validator_factory(self):
         # validator with no type
         validator = {
@@ -1115,12 +1147,6 @@ class TestPostgresql2(BaseTestPostgresql):
     def setUp(self):
         super(TestPostgresql2, self).setUp()
 
-    @patch('subprocess.check_output', Mock(return_value='\n'.join(mock_available_gucs.return_value).encode('utf-8')))
-    def test_available_gucs(self):
-        gucs = self.p.available_gucs
-        self.assertIsInstance(gucs, CaseInsensitiveSet)
-        self.assertEqual(gucs, mock_available_gucs.return_value)
-
     def test_cluster_info_query(self):
         self.assertIn('diff(pg_catalog.pg_current_wal_flush_lsn(', self.p.cluster_info_query)
         self.p._major_version = 90600

tests/test_sync.py

@@ -7,12 +7,11 @@ from patroni.collections import CaseInsensitiveSet
 from patroni.dcs import Cluster, ClusterConfig, Status, SyncState
 from patroni.postgresql import Postgresql
 
-from . import BaseTestPostgresql, mock_available_gucs, psycopg_connect
+from . import BaseTestPostgresql, psycopg_connect
 
 
 @patch('subprocess.call', Mock(return_value=0))
 @patch('patroni.psycopg.connect', psycopg_connect)
-@patch.object(Postgresql, 'available_gucs', mock_available_gucs)
 class TestSync(BaseTestPostgresql):
 
     @patch('subprocess.call', Mock(return_value=0))
@@ -20,7 +19,6 @@ class TestSync(BaseTestPostgresql):
     @patch('patroni.postgresql.CallbackExecutor', Mock())
     @patch.object(Postgresql, 'get_major_version', Mock(return_value=140000))
     @patch.object(Postgresql, 'is_running', Mock(return_value=True))
-    @patch.object(Postgresql, 'available_gucs', mock_available_gucs)
     def setUp(self):
         super(TestSync, self).setUp()
         self.p.config.write_postgresql_conf()

tests/test_utils.py

@@ -3,7 +3,8 @@ import unittest
 from unittest.mock import Mock, patch
 
 from patroni.exceptions import PatroniException
-from patroni.utils import enable_keepalive, polling_loop, Retry, RetryFailedError, unquote, validate_directory
+from patroni.utils import enable_keepalive, get_major_version, get_postgres_version, \
+    polling_loop, Retry, RetryFailedError, unquote, validate_directory
 
 
 class TestUtils(unittest.TestCase):
@@ -67,6 +68,47 @@ class TestUtils(unittest.TestCase):
             '\'value with a \'"\'"\' single quote\''),
             'value with a \' single quote')
 
+    def test_get_postgres_version(self):
+        with patch('subprocess.check_output', Mock(return_value=b'postgres (PostgreSQL) 9.6.24\n')):
+            self.assertEqual(get_postgres_version(), '9.6.24')
+        with patch('subprocess.check_output',
+                   Mock(return_value=b'postgres (PostgreSQL) 10.23 (Ubuntu 10.23-4.pgdg22.04+1)\n')):
+            self.assertEqual(get_postgres_version(), '10.23')
+        with patch('subprocess.check_output',
+                   Mock(return_value=b'postgres (PostgreSQL) 17beta3 (Ubuntu 17~beta3-1.pgdg22.04+1)\n')):
+            self.assertEqual(get_postgres_version(), '17.0')
+        with patch('subprocess.check_output',
+                   Mock(return_value=b'postgres (PostgreSQL) 9.6beta3\n')):
+            self.assertEqual(get_postgres_version(), '9.6.0')
+        with patch('subprocess.check_output', Mock(return_value=b'postgres (PostgreSQL) 9.6rc2\n')):
+            self.assertEqual(get_postgres_version(), '9.6.0')
+        # because why not
+        with patch('subprocess.check_output', Mock(return_value=b'postgres (PostgreSQL) 10\n')):
+            self.assertEqual(get_postgres_version(), '10.0')
+        with patch('subprocess.check_output', Mock(return_value=b'postgres (PostgreSQL) 10wow, something new\n')):
+            self.assertEqual(get_postgres_version(), '10.0')
+        with patch('subprocess.check_output', Mock(side_effect=OSError)):
+            self.assertRaises(PatroniException, get_postgres_version, 'postgres')
+
+    def test_get_major_version(self):
+        with patch('subprocess.check_output', Mock(return_value=b'postgres (PostgreSQL) 9.6.24\n')):
+            self.assertEqual(get_major_version(), '9.6')
+        with patch('subprocess.check_output',
+                   Mock(return_value=b'postgres (PostgreSQL) 10.23 (Ubuntu 10.23-4.pgdg22.04+1)\n')):
+            self.assertEqual(get_major_version(), '10')
+        with patch('subprocess.check_output',
+                   Mock(return_value=b'postgres (PostgreSQL) 17beta3 (Ubuntu 17~beta3-1.pgdg22.04+1)\n')):
+            self.assertEqual(get_major_version(), '17')
+        with patch('subprocess.check_output',
+                   Mock(return_value=b'postgres (PostgreSQL) 9.6beta3\n')):
+            self.assertEqual(get_major_version(), '9.6')
+        with patch('subprocess.check_output', Mock(return_value=b'postgres (PostgreSQL) 9.6rc2\n')):
+            self.assertEqual(get_major_version(), '9.6')
+        with patch('subprocess.check_output', Mock(return_value=b'postgres (PostgreSQL) 10\n')):
+            self.assertEqual(get_major_version(), '10')
+        with patch('subprocess.check_output', Mock(side_effect=OSError)):
+            self.assertRaises(PatroniException, get_major_version, 'postgres')
+
 
 @patch('time.sleep', Mock())
 class TestRetrySleeper(unittest.TestCase):