Avoid pending_restart=True flag flickering

The current Patroni 3.2.2 has wired/flickering  behaviour:
it temporary flag pending_restart=True on many changes to REST API,
which is gone within a second but long enough to be cougth by charm.
Sleepping a bit is a necessary evil, until Patroni 3.3.0 upgrade.

The previous code sleept for 15 seconds waiting for pg_settings update.

Also, the unnecessary restarts could be triggered by missmatch of
Patroni config file and in-memory changes coming from REST API,
e.g. the slots were undefined in yaml file but set as an empty JSON {} => None.
Updating the default template to match the default API PATCHes and avoid restarts.

Signed-off-by: Marcelo Henrique Neppel <[email protected]>

Author: marceloneppel

src/charm.py

@@ -2137,7 +2137,7 @@ def update_config(self, is_creating_backup: bool = False) -> bool:
             restore_stanza=self.app_peer_data.get("restore-stanza"),
             parameters=postgresql_parameters,
             user_databases_map=self.relations_user_databases_map,
-            slots=replication_slots or None,
+            slots=replication_slots,
         )
 
         if not self._is_workload_running:

src/patroni.py

@@ -7,6 +7,7 @@
 import logging
 import os
 import pwd
+from time import sleep
 from typing import Any, TypedDict
 
 import requests
@@ -366,18 +367,32 @@ def is_replication_healthy(self) -> bool:
 
     def is_restart_pending(self) -> bool:
         """Returns whether the Patroni/PostgreSQL restart pending."""
-        patroni_status = requests.get(
+        pending_restart = self._get_patroni_restart_pending()
+        if pending_restart:
+            # The current Patroni 3.2.2 has wired behaviour: it temporarily flags pending_restart=True
+            # on any changes to REST API, which is gone within a second but long enough to be
+            # caught by charm. Sleep 2 seconds as a protection here until Patroni 3.3.0 upgrade.
+            # Repeat the request to make sure pending_restart flag is still here
+            logger.debug("Enduring restart is pending (to avoid unnecessary rolling restarts)")
+            sleep(2)
+            pending_restart = self._get_patroni_restart_pending()
+
+        return pending_restart
+
+    def _get_patroni_restart_pending(self) -> bool:
+        """Returns whether the Patroni flag pending_restart on REST API."""
+        r = requests.get(
             f"{self._patroni_url}/patroni",
             verify=self._verify,
             timeout=PATRONI_TIMEOUT,
             auth=self._patroni_auth,
         )
-        try:
-            pending_restart = patroni_status.json()["pending_restart"]
-        except KeyError:
-            pending_restart = False
-            pass
-        logger.debug(f"Patroni API is_restart_pending: {pending_restart}")
+        pending_restart = r.json().get("pending_restart", False)
+        logger.debug(
+            f"API _get_patroni_restart_pending ({pending_restart}): %s (%s)",
+            r,
+            r.elapsed.total_seconds(),
+        )
 
         return pending_restart
 
@@ -509,7 +524,7 @@ def ensure_slots_controller_by_patroni(self, slots: dict[str, str]) -> None:
             auth=self._patroni_auth,
         )
         slots_patch: dict[str, dict[str, str] | None] = dict.fromkeys(
-            current_config.json().get("slots", ())
+            current_config.json().get("slots") or {}
         )
         for slot, database in slots.items():
             slots_patch[slot] = {
@@ -617,6 +632,8 @@ def render_patroni_yml_file(
             user_databases_map: map of databases to be accessible by each user.
             slots: replication slots (keys) with assigned database name (values).
         """
+        slots = slots or {}
+
         # Open the template patroni.yml file.
         with open("templates/patroni.yml.j2") as file:
             template = Template(file.read())

templates/patroni.yml.j2

@@ -47,15 +47,13 @@ bootstrap:
         logging_collector: 'on'
         wal_level: logical
         shared_preload_libraries: 'timescaledb,pgaudit'
-    {%- if slots %}
     slots:
     {%- for slot, database in slots.items() %}
       {{slot}}:
         database: {{database}}
         plugin: pgoutput
         type: logical
     {%- endfor -%}
-    {% endif %}
 
   {%- if restoring_backup %}
   method: pgbackrest

tests/unit/test_charm.py

@@ -1504,7 +1504,7 @@ def test_update_config(harness):
             restore_to_latest=False,
             parameters={"test": "test"},
             user_databases_map={"operator": "all", "replication": "all", "rewind": "all"},
-            slots=None,
+            slots={},
         )
         _handle_postgresql_restart_need.assert_called_once()
         _restart_metrics_service.assert_called_once()
@@ -1535,7 +1535,7 @@ def test_update_config(harness):
             restore_to_latest=False,
             parameters={"test": "test"},
             user_databases_map={"operator": "all", "replication": "all", "rewind": "all"},
-            slots=None,
+            slots={},
         )
         _handle_postgresql_restart_need.assert_called_once()
         _restart_metrics_service.assert_called_once()

tests/unit/test_patroni.py

@@ -224,6 +224,7 @@ def test_render_patroni_yml_file(harness, patroni):
         with open("templates/patroni.yml.j2") as file:
             template = Template(file.read())
         expected_content = template.render(
+            slots={},
             endpoint=patroni._endpoint,
             endpoints=patroni._endpoints,
             namespace=patroni._namespace,
@@ -259,6 +260,7 @@ def test_render_patroni_yml_file(harness, patroni):
         # Then test the rendering of the file with TLS enabled.
         _render_file.reset_mock()
         expected_content_with_tls = template.render(
+            slots={},
             enable_tls=True,
             endpoint=patroni._endpoint,
             endpoints=patroni._endpoints,