feat(config): complete update_config with API apply, restart engine, and bridges

marceloneppel · marceloneppel · commit 08f599367771 · 2026-07-10T21:38:03.000-03:00
The library now owns the full config-update flow (steps 3-10) instead of the
charm. update_config renders, applies Patroni-controlled parameters via the API,
runs the TLS/pending-restart decision engine, honors the VM snap gate, and
persists the config/user hashes -- all in the lib.

The substrate-tangled pieces of the restart trigger, endpoint refresh, and
monitoring/LDAP service restarts ride charm-side through three injected bridge
callables (request_restart, refresh_endpoints, restart_services) until their own
migration phases, so the two HIGH-risk substrate diffs in the restart trigger
(VM pops postgresql_restarted; K8s updates the metrics scrape job) stay out of
the library. is_tls_enabled and generate_config_hash internalize; user_hash
stays injected. The config hash is byte-compatible with the charm's, so charm
adoption does not force a spurious restart.

Signed-off-by: Marcelo Henrique Neppel &lt;marcelo.neppel@canonical.com&gt;
diff --git a/single_kernel_postgresql/charms/abstract_charm.py b/single_kernel_postgresql/charms/abstract_charm.py
@@ -42,7 +42,15 @@ def __init__(self, *args):
         )
         self.patroni_manager = PatroniManager(state=self.state, workload=self.workload)
         self.cluster_manager = ClusterManager(state=self.state, workload=self.workload)
-        self.config_manager = ConfigManager(state=self.state, workload=self.workload)
+        self.config_manager = ConfigManager(
+            state=self.state,
+            workload=self.workload,
+            tls_manager=self.tls_manager,
+            patroni_manager=self.patroni_manager,
+            request_restart=self.request_restart,
+            refresh_endpoints=self.refresh_endpoints,
+            restart_services=self.restart_services,
+        )
 
         # Events Handler
         self.postgresql_events_handler = PostgreSQLEventsHandler(
@@ -84,3 +92,21 @@ def workload(self) -> BaseWorkload:
     def substrate(self) -> Substrates:
         """Access current substrate."""
         pass
+
+    # Config-update bridges: charm-side callables the ConfigManager invokes for the
+    # substrate-tangled restart trigger, endpoint refresh and monitoring/ldap service
+    # restarts. They stay in the charm until their own migration phases.
+    @abstractmethod
+    def request_restart(self) -> None:
+        """Run the substrate pre-restart side effect and acquire the restart lock."""
+        pass
+
+    @abstractmethod
+    def refresh_endpoints(self) -> None:
+        """Refresh the client relation endpoints."""
+        pass
+
+    @abstractmethod
+    def restart_services(self) -> None:
+        """Restart the monitoring and LDAP-sync sidecar services."""
+        pass
diff --git a/single_kernel_postgresql/charms/k8s_charm.py b/single_kernel_postgresql/charms/k8s_charm.py
@@ -66,3 +66,15 @@ def substrate(self) -> Substrates:
             Substrates: always Substrates.K8S for this charm
         """
         return Substrates.K8S
+
+    # The concrete production charm owns these bridges (update_scrape_job_spec +
+    # acquire_lock, update_endpoints, pebble metrics/ldap restarts). This abstract charm
+    # is unit-test only, so they are no-ops here.
+    def request_restart(self) -> None:
+        """Run the substrate pre-restart side effect and acquire the restart lock."""
+
+    def refresh_endpoints(self) -> None:
+        """Refresh the client relation endpoints."""
+
+    def restart_services(self) -> None:
+        """Restart the monitoring and LDAP-sync sidecar services."""
diff --git a/single_kernel_postgresql/charms/vm_charm.py b/single_kernel_postgresql/charms/vm_charm.py
@@ -56,3 +56,15 @@ def substrate(self) -> Substrates:
             Substrates: always Substrates.VM for this charm
         """
         return Substrates.VM
+
+    # The concrete production charm owns these bridges (pops postgresql_restarted +
+    # acquire_lock, update_endpoints, snap metrics/ldap restarts). This abstract charm
+    # is unit-test only, so they are no-ops here.
+    def request_restart(self) -> None:
+        """Run the substrate pre-restart side effect and acquire the restart lock."""
+
+    def refresh_endpoints(self) -> None:
+        """Refresh the client relation endpoints."""
+
+    def restart_services(self) -> None:
+        """Restart the monitoring and LDAP-sync sidecar services."""
diff --git a/single_kernel_postgresql/managers/config.py b/single_kernel_postgresql/managers/config.py
@@ -9,14 +9,19 @@
 
 import importlib.resources
 import logging
+from collections.abc import Callable
+from hashlib import shake_128
 from typing import Any
 
 import charm_refresh
+import psycopg2
 from data_platform_helpers.advanced_statuses import StatusObject
 from data_platform_helpers.advanced_statuses.types import Scope as AdvancedStatusesScope
 from jinja2 import Template
+from tenacity import RetryError, Retrying, stop_after_attempt, stop_after_delay, wait_fixed
 
 from single_kernel_postgresql.config.enums import Substrates
+from single_kernel_postgresql.config.exceptions import PostgreSQLCannotConnectError
 from single_kernel_postgresql.config.literals import (
     PGBACKREST_CONF_FILE,
     POSTGRESQL_STORAGE_PERMISSIONS,
@@ -26,6 +31,8 @@
 from single_kernel_postgresql.config.statuses import GeneralStatuses
 from single_kernel_postgresql.core.state import CharmState
 from single_kernel_postgresql.managers.base import BaseManager
+from single_kernel_postgresql.managers.patroni import PatroniManager
+from single_kernel_postgresql.managers.tls import TLSManager
 from single_kernel_postgresql.utils import _change_owner, render_file
 from single_kernel_postgresql.utils.postgresql import PostgreSQL as PostgreSQLClient
 from single_kernel_postgresql.workload.base import BaseWorkload
@@ -39,8 +46,24 @@ class ConfigManager(BaseManager):
     This manager is responsible for handling configuration operations.
     """
 
-    def __init__(self, state: CharmState, workload: BaseWorkload):
+    def __init__(
+        self,
+        state: CharmState,
+        workload: BaseWorkload,
+        tls_manager: TLSManager,
+        patroni_manager: PatroniManager,
+        request_restart: Callable[[], None],
+        refresh_endpoints: Callable[[], None],
+        restart_services: Callable[[], None],
+    ):
         super().__init__(state, workload, "config_manager")
+        self.tls_manager = tls_manager
+        self.patroni_manager = patroni_manager
+        # Charm-side bridges: the substrate-tangled restart trigger, endpoint refresh and
+        # monitoring/ldap service restarts stay in the charm until their own migration phases.
+        self.request_restart = request_restart
+        self.refresh_endpoints = refresh_endpoints
+        self.restart_services = restart_services
 
     @staticmethod
     def _dict_to_hba_string(_dict: dict[str, Any]) -> str:
@@ -266,13 +289,154 @@ def _build_postgresql_parameters(
 
         return pg_parameters
 
+    @property
+    def is_tls_enabled(self) -> bool:
+        """Return whether client TLS is enabled (all client TLS files are present)."""
+        return all(self.tls_manager.get_client_tls_files())
+
+    @property
+    def generate_config_hash(self) -> str:
+        """Generate current configuration hash."""
+        return shake_128(str(self.state.config.model_dump()).encode()).hexdigest(16)
+
+    def _can_connect_to_postgresql(self, postgresql_client: PostgreSQLClient) -> bool:
+        if self.state.substrate == Substrates.VM and (
+            not postgresql_client.password or not postgresql_client.current_host
+        ):
+            return False
+        try:
+            for attempt in Retrying(stop=stop_after_delay(10), wait=wait_fixed(3)):
+                with attempt:
+                    if not postgresql_client.get_postgresql_timezones():
+                        logger.debug("Cannot connect to database (CannotConnectError)")
+                        raise PostgreSQLCannotConnectError
+        except RetryError:
+            logger.debug("Cannot connect to database (RetryError)")
+            return False
+        return True
+
+    def is_restart_pending(self, postgresql_client: PostgreSQLClient) -> bool:
+        """Query pg_settings for pending restart."""
+        connection = None
+        try:
+            with (
+                postgresql_client._connect_to_database(
+                    database_host=postgresql_client.current_host
+                ) as connection,
+                connection.cursor() as cursor,
+            ):
+                cursor.execute("SELECT COUNT(*) FROM pg_settings WHERE pending_restart=True;")
+                result = cursor.fetchone()
+                if result is not None:
+                    return result[0] > 0
+                else:
+                    return False
+        except psycopg2.OperationalError:
+            logger.warning("Failed to connect to PostgreSQL.")
+            return False
+        except psycopg2.Error as e:
+            logger.error(f"Failed to check if restart is pending: {e}")
+            return False
+        finally:
+            if connection:
+                connection.close()
+
+    def apply_api_config(
+        self,
+        postgresql_client: PostgreSQLClient,
+        async_primary_cluster_endpoint: str | None = None,
+    ) -> bool:
+        """Update the parameters controlled by Patroni via its API."""
+        cpu_cores = self.workload.get_available_resources()[0]
+        # Use config value if set, calculate otherwise
+        max_connections = (
+            self.state.config.experimental_max_connections
+            if self.state.config.experimental_max_connections
+            else max(4 * cpu_cores, 100)
+        )
+        cfg_patch: dict[str, int | str | None] = {
+            "max_connections": max_connections,
+            "max_prepared_transactions": self.state.config.memory_max_prepared_transactions,
+            "max_replication_slots": 25,
+            "max_wal_senders": 25,
+            "shared_buffers": self.state.config.memory_shared_buffers,
+            "wal_keep_size": self.state.config.durability_wal_keep_size,
+        }
+
+        # Add restart-required worker process parameters via Patroni API
+        worker_configs = self._calculate_worker_process_config(cpu_cores)
+        if "max_worker_processes" in worker_configs:
+            cfg_patch["max_worker_processes"] = worker_configs["max_worker_processes"]
+        if "max_logical_replication_workers" in worker_configs:
+            cfg_patch["max_logical_replication_workers"] = worker_configs[
+                "max_logical_replication_workers"
+            ]
+
+        base_patch = {
+            **self.state.synchronous_configuration,
+            "maximum_lag_on_failover": self.state.config.durability_maximum_lag_on_failover,
+        }
+        if async_primary_cluster_endpoint:
+            base_patch["standby_cluster"] = {"host": async_primary_cluster_endpoint}
+        try:
+            self.patroni_manager.bulk_update_parameters_controller_by_patroni(
+                cfg_patch, base_patch
+            )
+        except RetryError:
+            return False
+        return True
+
+    def handle_restart_need(
+        self, postgresql_client: PostgreSQLClient, config_changed: bool
+    ) -> None:
+        """Handle PostgreSQL restart need based on the TLS configuration and configuration changes."""
+        if self._can_connect_to_postgresql(postgresql_client):
+            # check_current_host is a VM-only precision in the live TLS probe.
+            check_current_host = (
+                {"check_current_host": True} if (self.state.substrate == Substrates.VM) else {}
+            )
+            restart_postgresql = self.is_tls_enabled != postgresql_client.is_tls_enabled(
+                **check_current_host
+            )
+        else:
+            restart_postgresql = False
+
+        try:
+            self.patroni_manager.reload_patroni_configuration()
+        except Exception as e:
+            logger.error(f"Reload patroni call failed! error: {e!s}")
+
+        if config_changed and not restart_postgresql:
+            # Wait for some more time than the Patroni's loop_wait default value (10 seconds),
+            # which tells how much time Patroni will wait before checking the configuration
+            # file again to reload it.
+            try:
+                for attempt in Retrying(stop=stop_after_attempt(5), wait=wait_fixed(3)):
+                    with attempt:
+                        restart_postgresql = restart_postgresql or self.is_restart_pending(
+                            postgresql_client
+                        )
+                        if not restart_postgresql:
+                            raise Exception
+            except RetryError:
+                # Ignore the error, as it happens only to indicate that the configuration has not changed.
+                pass
+
+        self.state.peer.tls = self.is_tls_enabled
+        self.refresh_endpoints()
+
+        # Restart PostgreSQL if TLS configuration has changed
+        # (so the both old and new connections use the configuration).
+        if restart_postgresql:
+            logger.info("PostgreSQL restart required")
+            self.request_restart()
+
     def update_config(
         self,
         postgresql_client: PostgreSQLClient,
+        user_hash: str,
         is_creating_backup: bool = False,
         # TODO add rel handler
-        is_tls_enabled: bool = False,
-        # TODO add rel handler
         relations_user_databases_map: dict[str, Any] | None = None,
         # TODO add rel handler
         ldap_parameters: dict[str, Any] | None = None,
@@ -286,7 +450,12 @@ def update_config(
         *,
         refresh: charm_refresh.Machines | None = None,
     ) -> bool:
-        """Updates Patroni config file based on the existence of the TLS files."""
+        """Updates Patroni config file based on the existence of the TLS files.
+
+        Raises:
+            DeployedWithoutTrustError: on K8s when the app lacks cluster trust; the caller
+                is expected to catch this.
+        """
         # Build PostgreSQL parameters
         pg_parameters = self._build_postgresql_parameters(postgresql_client)
 
@@ -302,7 +471,7 @@ def update_config(
             connectivity=self.state.peer.is_connectivity_enabled,
             is_creating_backup=is_creating_backup,
             enable_ldap=self.state.application.is_ldap_enabled,
-            enable_tls=is_tls_enabled,
+            enable_tls=self.is_tls_enabled,
             backup_id=self.state.application.data.get("restoring-backup"),
             pitr_target=self.state.application.data.get("restore-to-time"),
             restore_timeline=self.state.application.data.get("restore-timeline"),
@@ -319,6 +488,67 @@ def update_config(
             watcher_raft_address=watcher_raft_address,
             no_peers=no_peers,
         )
+        if no_peers:
+            return True
+
+        if not self.workload.is_patroni_running():
+            # If Patroni/PostgreSQL has not started yet and TLS relations was initialised,
+            # then mark TLS as enabled. This commonly happens when the charm is deployed
+            # in a bundle together with the TLS certificates operator. This flag is used to
+            # know when to call the Patroni API using HTTP or HTTPS.
+            self.state.peer.tls = self.is_tls_enabled
+            self.refresh_endpoints()
+            logger.debug("Early exit update_config: Workload not started yet")
+            return True
+
+        if not self.patroni_manager.member_started:
+            if self.is_tls_enabled:
+                logger.debug(
+                    "Early exit update_config: patroni not responding but TLS is enabled."
+                )
+                self.handle_restart_need(postgresql_client, True)
+                return True
+            logger.debug("Early exit update_config: Patroni not started yet")
+            return False
+
+        # Try to connect. Patroni's REST API patch (below) doesn't need the PG-client
+        # connection, so this standalone gate is VM-only; K8s proceeds straight to it.
+        if self.state.substrate == Substrates.VM and not self._can_connect_to_postgresql(
+            postgresql_client
+        ):
+            logger.warning("Early exit update_config: Cannot connect to Postgresql")
+            return False
+
+        if not self.apply_api_config(postgresql_client, async_primary_cluster_endpoint):
+            logger.warning("Early exit update_config: Unable to patch Patroni API")
+            return False
+
+        if self.state.substrate == Substrates.K8S and not (
+            self.patroni_manager.ensure_slots_controller_by_patroni(replication_slots)
+        ):
+            logger.warning(
+                "Failed to sync replication slots with Patroni — will retry on next config update"
+            )
+
+        self.handle_restart_need(
+            postgresql_client, self.state.peer.config_hash != self.generate_config_hash
+        )
+
+        # TODO handle case of scale up while refresh in progress & `refresh` is None
+        if (
+            self.state.substrate == Substrates.VM
+            and refresh is not None
+            and self.workload.get_snap_revision() != refresh.pinned_snap_revision
+        ):
+            logger.debug("Early exit: snap was not refreshed to the right version yet")
+            return True
+
+        self.restart_services()
+
+        self.state.peer.user_hash = user_hash
+        self.state.peer.config_hash = self.generate_config_hash
+        if self.state.peer.is_app_leader:
+            self.state.application.user_hash = user_hash
         return True
 
     def render_patroni_yml_file(
diff --git a/single_kernel_postgresql/workload/base.py b/single_kernel_postgresql/workload/base.py
@@ -278,3 +278,7 @@ def get_available_memory(self) -> int:
     def get_available_resources(self) -> tuple[int, int]:
         """Returns the available (cpu_cores, memory_bytes) for the workload."""
         pass
+
+    def get_snap_revision(self) -> str:
+        """Returns the installed workload snap revision (VM only)."""
+        raise NotImplementedError
diff --git a/single_kernel_postgresql/workload/vm.py b/single_kernel_postgresql/workload/vm.py
@@ -238,3 +238,8 @@ def get_available_memory(self) -> int:
     def get_available_resources(self) -> tuple[int, int]:
         """Returns the available (cpu_cores, memory_bytes) for the workload."""
         return os.cpu_count() or 1, self.get_available_memory()
+
+    def get_snap_revision(self) -> str:
+        """Returns the installed workload snap revision."""
+        cache = snap.SnapCache()
+        return cache[charm_refresh.snap_name()].revision
diff --git a/tests/unit/test_config_manager.py b/tests/unit/test_config_manager.py
