From 4725f12f9a24ee5ad6b3ee0c1792de61f7a3a7cc Mon Sep 17 00:00:00 2001 From: Feike Steenbergen Date: Mon, 10 Jul 2023 13:44:54 +0200 Subject: [PATCH] Allow integer gucs without units in validation (#2734) Previously, integer gucs, for example `max_connections` would not pass the validation, as these settings have no unit, if and only if they were specified as a string. This causes problems if the `max_connections` is configured in `patroni.yaml` as a string, for example, the following configuration would not result in the right `max_connections` settings, as `max_connections` is configured as a string: bootstrap: dcs: postgresql: parameters: log_checkpoints: "on" log_connections: "off" max_connections: "57" Allowing a user to specify *all* parameters as a string was accepted before in Patroni and also seems very useful, as many of us will be using Ansible/Helm/Golang to build a Patroni configuration, in which creating a `map[string]string` is easier than having to deal with data types. Attemps to address issue #2735 Regression was introduced in https://github.com/zalando/patroni/commit/76b3b99de2f2bfaa8ab2df9e47dbfc3749d14e84 --- features/patroni_api.feature | 12 ++++++------ patroni/utils.py | 15 +++++++++++++++ patroni/validator.py | 3 +-- 3 files changed, 22 insertions(+), 8 deletions(-) diff --git a/features/patroni_api.feature b/features/patroni_api.feature index 0c47fe6e..624d3271 100644 --- a/features/patroni_api.feature +++ b/features/patroni_api.feature @@ -35,21 +35,21 @@ Scenario: check local configuration reload Then I receive a response code 202 Scenario: check dynamic configuration change via DCS - Given I run patronictl.py edit-config -s 'ttl=10' -p 'max_connections=101' --force batman - Then I receive a response returncode 0 - And I receive a response output "+ttl: 10" + Given I issue a PATCH request to http://127.0.0.1:8008/config with {"ttl": 20, "postgresql": {"parameters": {"max_connections": "101"}}} + Then I receive a response code 200 And Response on GET http://127.0.0.1:8008/patroni contains pending_restart after 11 seconds When I issue a GET request to http://127.0.0.1:8008/config Then I receive a response code 200 - And I receive a response ttl 10 + And I receive a response ttl 20 When I issue a GET request to http://127.0.0.1:8008/patroni Then I receive a response code 200 And I receive a response tags {'new_tag': 'new_value'} And I sleep for 4 seconds Scenario: check the scheduled restart - Given I issue a PATCH request to http://127.0.0.1:8008/config with {"postgresql": {"parameters": {"superuser_reserved_connections": "6"}}} - Then I receive a response code 200 + Given I run patronictl.py edit-config -p 'superuser_reserved_connections=6' --force batman + Then I receive a response returncode 0 + And I receive a response output "+ superuser_reserved_connections: 6" And Response on GET http://127.0.0.1:8008/patroni contains pending_restart after 5 seconds Given I issue a scheduled restart at http://127.0.0.1:8008 in 5 seconds with {"role": "replica"} Then I receive a response code 202 diff --git a/patroni/utils.py b/patroni/utils.py index eb02c561..ee9a3d21 100644 --- a/patroni/utils.py +++ b/patroni/utils.py @@ -326,6 +326,21 @@ def parse_int(value: Any, base_unit: Optional[str] = None) -> Optional[int]: >>> parse_int('1TB', 'GB') is None True + >>> parse_int(50, None) == 50 + True + + >>> parse_int("51", None) == 51 + True + + >>> parse_int("nonsense", None) == None + True + + >>> parse_int("nonsense", "kB") == None + True + + >>> parse_int("nonsense") == None + True + >>> parse_int(0) == 0 True diff --git a/patroni/validator.py b/patroni/validator.py index f780f9e0..fc6cdde5 100644 --- a/patroni/validator.py +++ b/patroni/validator.py @@ -792,8 +792,7 @@ class IntValidator(object): :param value: value to be checked against the rules defined for this :class:`IntValidator` instance. :returns: ``True`` if *value* is valid and within the expected range. """ - if self.base_unit: - value = parse_int(value, self.base_unit) or "" + value = parse_int(value, self.base_unit) or "" ret = isinstance(value, int)\ and (self.min is None or value >= self.min)\ and (self.max is None or value <= self.max)