From 76be2e23a5b9a986ff9c3a67a283c0a2ac315d1e Mon Sep 17 00:00:00 2001 From: Marcelo Henrique Neppel Date: Sat, 11 Jul 2026 12:14:43 -0300 Subject: [PATCH 1/2] refactor(config): adopt single-kernel update_config, drop charm copy The single-kernel PostgreSQL library now owns the full update_config subsystem (parameter building, Patroni API patching, K8s slot sync, restart handling, and hash persistence), matching the already-adopted VM charm. Duplicating this logic in both places would let the two substrates drift out of sync on every future config change, so the charm delegates to the lib's ConfigManager instead of keeping its own copy. The charm now constructs a 7-arg ConfigManager wired with 3 bridge callbacks (request_restart, refresh_endpoints, restart_services) so the lib can trigger K8s-specific pebble/metrics side effects without depending on charm internals directly. update_config becomes a thin wrapper that also catches DeployedWithoutTrustError, since the lib's K8s workload now raises this on a 403 instead of a raw ApiError. Signed-off-by: Marcelo Henrique Neppel --- poetry.lock | 72 +++--- pyproject.toml | 4 +- src/charm.py | 429 +++----------------------------- templates/patroni.yml.j2 | 217 ---------------- tests/unit/test_charm.py | 525 ++++++--------------------------------- 5 files changed, 151 insertions(+), 1096 deletions(-) delete mode 100644 templates/patroni.yml.j2 diff --git a/poetry.lock b/poetry.lock index 04802703f52..3ee1be3f2db 100644 --- a/poetry.lock +++ b/poetry.lock @@ -54,7 +54,7 @@ version = "0.7.0" description = "Reusable constraint types to use with typing.Annotated" optional = false python-versions = ">=3.8" -groups = ["main", "charm-libs"] +groups = ["main", "charm-libs", "integration"] files = [ {file = "annotated_types-0.7.0-py3-none-any.whl", hash = "sha256:1f02e8b43a8fbbc3f3e0d4f0f4bfc8131bcb4eebe8849b8e5c773f3a1c582a53"}, {file = "annotated_types-0.7.0.tar.gz", hash = "sha256:aff07c09a53a08bc8cfccb9c85b05f1aa9a2a6f23728d790723543408344ce89"}, @@ -413,7 +413,7 @@ version = "0.1.3" description = "" optional = false python-versions = ">=3.8" -groups = ["main"] +groups = ["main", "integration"] files = [ {file = "charm_api-0.1.3-py3-none-any.whl", hash = "sha256:4dba74e33f3de4608e88db2e08e41b0681221e7d3c2cd6913e0ebeb3dbdf8d92"}, {file = "charm_api-0.1.3.tar.gz", hash = "sha256:a90a926b89fce834e9fb2184b96e07ec46f4dd56f3cd50e0209c0fd0ff35a29e"}, @@ -425,7 +425,7 @@ version = "0.1.2" description = "" optional = false python-versions = ">=3.8" -groups = ["main"] +groups = ["main", "integration"] files = [ {file = "charm_json-0.1.2-py3-none-any.whl", hash = "sha256:1cad028a92831c61d6dcb99127c5216c333f90ec58d06b9f73fd0fd7355d1b3c"}, {file = "charm_json-0.1.2.tar.gz", hash = "sha256:f91255858cd6721ec491d09450f37a0cdecf1addbfcfbd41f3e27c58dd95092e"}, @@ -440,7 +440,7 @@ version = "3.1.1.6" description = "In-place rolling refreshes (upgrades) of stateful charmed applications" optional = false python-versions = ">=3.8" -groups = ["main"] +groups = ["main", "integration"] files = [ {file = "charm_refresh-3.1.1.6-py3-none-any.whl", hash = "sha256:36fa18cadd77b6c1b27e440ae78ca29bb2da7621a2a928665f880c095043332f"}, {file = "charm_refresh-3.1.1.6.tar.gz", hash = "sha256:a74ab1b7aedf8169cd68162a6ab46e2543b6fd6f2c1b188ca779a25234342dae"}, @@ -478,7 +478,7 @@ version = "1.8.3" description = "The charmlibs.interfaces.tls_certificates package." optional = false python-versions = ">=3.10" -groups = ["main"] +groups = ["main", "integration"] files = [ {file = "charmlibs_interfaces_tls_certificates-1.8.3-py3-none-any.whl", hash = "sha256:fa0aa150b4e4513d439c7afb7eb0ceaf00363671143b27b09382758deea8a0cc"}, {file = "charmlibs_interfaces_tls_certificates-1.8.3.tar.gz", hash = "sha256:6bc35ee330681f2e0c58cf67e41133b144f3281f73d27293714c9ca918ee7cbc"}, @@ -495,7 +495,7 @@ version = "1.3.0.post0" description = "A pathlib-like interface for Juju K8s charms to interact with files in their workload container." optional = false python-versions = ">=3.10" -groups = ["main"] +groups = ["main", "integration"] files = [ {file = "charmlibs_pathops-1.3.0.post0-py3-none-any.whl", hash = "sha256:c6fc3e3460ffa03a7d4273cc7784c9d74378609a098c728cd9bbb2ed90bdfa5b"}, {file = "charmlibs_pathops-1.3.0.post0.tar.gz", hash = "sha256:36ac11b0c7c55b71341a549a1803f5c8d61c2279204862b8d50736041b782f59"}, @@ -510,7 +510,7 @@ version = "1.1.1" description = "The charmlibs.rollingops package." optional = false python-versions = ">=3.12" -groups = ["main"] +groups = ["main", "integration"] files = [ {file = "charmlibs_rollingops-1.1.1-py3-none-any.whl", hash = "sha256:1e61347c7792ec11afefa60d36f77a5afccf867832854d5aeef2da0b536afec4"}, {file = "charmlibs_rollingops-1.1.1.tar.gz", hash = "sha256:831457d2ef12117794f89a483ee9a5c347cb209a87dafe82907e9dede7d4641c"}, @@ -853,7 +853,7 @@ version = "1.1.0" description = "" optional = false python-versions = "<4.0,>=3.10" -groups = ["main"] +groups = ["main", "integration"] files = [ {file = "data_platform_helpers-1.1.0-py3-none-any.whl", hash = "sha256:71b2c2ba31d1bc01c55c9da82d02b0cb654b99231172916722ecaf4c469d27c4"}, {file = "data_platform_helpers-1.1.0.tar.gz", hash = "sha256:7539ef1c1047fec36ee8298aa4c75fe2bb36419544193b691805c12ca407a797"}, @@ -886,7 +886,7 @@ version = "1.0.0" description = "The dpcharmlibs.interfaces package." optional = false python-versions = ">=3.10" -groups = ["main"] +groups = ["main", "integration"] files = [ {file = "dpcharmlibs_interfaces-1.0.0-py3-none-any.whl", hash = "sha256:afff80be30e3ff8c31f68557946e9d5959247f1973cbc00e96da55b095f95a21"}, {file = "dpcharmlibs_interfaces-1.0.0.tar.gz", hash = "sha256:a177653019781a7a165be52cf4293134ee58912cd93686082cdd82da347a3bed"}, @@ -1411,7 +1411,7 @@ version = "4.2.0" description = "Python port of markdown-it. Markdown parsing, done right!" optional = false python-versions = ">=3.10" -groups = ["main"] +groups = ["main", "integration"] files = [ {file = "markdown_it_py-4.2.0-py3-none-any.whl", hash = "sha256:9f7ebbcd14fe59494226453aed97c1070d83f8d24b6fc3a3bcf9a38092641c4a"}, {file = "markdown_it_py-4.2.0.tar.gz", hash = "sha256:04a21681d6fbb623de53f6f364d352309d4094dd4194040a10fd51833e418d49"}, @@ -1552,7 +1552,7 @@ version = "0.1.2" description = "Markdown URL utilities" optional = false python-versions = ">=3.7" -groups = ["main"] +groups = ["main", "integration"] files = [ {file = "mdurl-0.1.2-py3-none-any.whl", hash = "sha256:84008a41e51615a49fc9966191ff91509e3c40b939176e643fd50a5c2196b8f8"}, {file = "mdurl-0.1.2.tar.gz", hash = "sha256:bb413d29f5eea38f31dd4754dd7377d4465116fb207585f97bf925588687c1ba"}, @@ -1782,34 +1782,40 @@ files = [ [[package]] name = "postgresql-charms-single-kernel" -version = "16.3.4" +version = "16.3.3" description = "Shared and reusable code for PostgreSQL-related charms" optional = false -python-versions = "<4.0,>=3.8" +python-versions = ">=3.8,<4.0" groups = ["main", "integration"] files = [ - {file = "postgresql_charms_single_kernel-16.3.4-py3-none-any.whl", hash = "sha256:2a8bd3a789b25df39e8e69a58fa00c943eb309644abf320df3364c5a4520361f"}, - {file = "postgresql_charms_single_kernel-16.3.4.tar.gz", hash = "sha256:6df3fd4208e1b91869a9a00819b997b6d55213239c6ecb4549bb74010a2cc7e7"}, + {file = "912b1a44a1158f93e17fcb1e36719a878ff48810.tar.gz", hash = "sha256:6af3973d1b8c5ada5be238527e86fc223657c1ec2b7d9ada58cea7ec43c9f34a"}, ] [package.dependencies] -charm-refresh = {version = "*", optional = true, markers = "python_full_version >= \"3.12.0\" and extra == \"postgresql\""} -charmlibs-interfaces-tls-certificates = {version = ">=1.8.3", optional = true, markers = "python_full_version >= \"3.12.0\" and extra == \"postgresql\""} -charmlibs-pathops = {version = ">=1.0.1", optional = true, markers = "python_full_version >= \"3.12.0\" and extra == \"postgresql\""} -charmlibs-rollingops = {version = ">=1.1.1", optional = true, markers = "python_full_version >= \"3.12.0\" and extra == \"postgresql\""} -data-platform-helpers = {version = ">=0.1.7", optional = true, markers = "python_full_version >= \"3.12.0\" and extra == \"postgresql\""} -httpx = {version = "*", optional = true, markers = "python_full_version >= \"3.12.0\" and extra == \"postgresql\""} -jinja2 = {version = ">=3.1.6", optional = true, markers = "python_full_version >= \"3.12.0\" and extra == \"postgresql\""} +charm-refresh = {version = "*", optional = true, markers = "python_version >= \"3.12\" and extra == \"postgresql\""} +charmlibs-interfaces-tls-certificates = {version = ">=1.8.3", optional = true, markers = "python_version >= \"3.12\" and extra == \"postgresql\""} +charmlibs-pathops = {version = ">=1.0.1", optional = true, markers = "python_version >= \"3.12\" and extra == \"postgresql\""} +charmlibs-rollingops = {version = ">=1.1.1", optional = true, markers = "python_version >= \"3.12\" and extra == \"postgresql\""} +data-platform-helpers = {version = ">=0.1.7", optional = true, markers = "python_version >= \"3.12\" and extra == \"postgresql\""} +httpx = {version = "*", optional = true, markers = "python_version >= \"3.12\" and extra == \"postgresql\""} +jinja2 = {version = ">=3.1.6", optional = true, markers = "python_version >= \"3.12\" and extra == \"postgresql\""} +lightkube = {version = ">=0.21.0", optional = true, markers = "python_version >= \"3.12\" and extra == \"k8s\""} +lightkube-models = {version = ">=1.28.1.4", optional = true, markers = "python_version >= \"3.12\" and extra == \"k8s\""} ops = ">=2.0.0" -pydantic = {version = ">=2.0", optional = true, markers = "python_full_version >= \"3.12.0\" and extra == \"postgresql\""} -requests = {version = "*", optional = true, markers = "python_full_version >= \"3.12.0\" and extra == \"postgresql\""} +pydantic = {version = ">=2.0", optional = true, markers = "python_version >= \"3.12\" and extra == \"postgresql\""} +requests = {version = "*", optional = true, markers = "python_version >= \"3.12\" and extra == \"postgresql\""} tenacity = ">=9.0.0" -tomli = {version = "*", optional = true, markers = "python_full_version >= \"3.12.0\" and extra == \"postgresql\""} +tomli = {version = "*", optional = true, markers = "python_version >= \"3.12\" and extra == \"postgresql\""} [package.extras] db-driver = ["psycopg2 (>=2.9.10)"] -postgresql = ["charm-refresh ; python_full_version >= \"3.12.0\"", "charmlibs-interfaces-tls-certificates (>=1.8.3) ; python_full_version >= \"3.12.0\"", "charmlibs-pathops (>=1.0.1) ; python_full_version >= \"3.12.0\"", "charmlibs-rollingops (>=1.1.1) ; python_full_version >= \"3.12.0\"", "data-platform-helpers (>=0.1.7) ; python_full_version >= \"3.12.0\"", "httpx ; python_full_version >= \"3.12.0\"", "jinja2 (>=3.1.6) ; python_full_version >= \"3.12.0\"", "pydantic (>=2.0) ; python_full_version >= \"3.12.0\"", "requests ; python_full_version >= \"3.12.0\"", "tomli ; python_full_version >= \"3.12.0\""] -vm = ["charmlibs-snap (>=1.0.1) ; python_full_version >= \"3.12.0\"", "charmlibs-systemd (>=1.0.0.post0) ; python_full_version >= \"3.12.0\"", "psutil (>=7.2.2) ; python_full_version >= \"3.12.0\"", "pysyncobj (>=0.3.15) ; python_full_version >= \"3.12.0\""] +k8s = ["lightkube (>=0.21.0) ; python_version >= \"3.12\"", "lightkube-models (>=1.28.1.4) ; python_version >= \"3.12\""] +postgresql = ["charm-refresh ; python_version >= \"3.12\"", "charmlibs-interfaces-tls-certificates (>=1.8.3) ; python_version >= \"3.12\"", "charmlibs-pathops (>=1.0.1) ; python_version >= \"3.12\"", "charmlibs-rollingops (>=1.1.1) ; python_version >= \"3.12\"", "data-platform-helpers (>=0.1.7) ; python_version >= \"3.12\"", "httpx ; python_version >= \"3.12\"", "jinja2 (>=3.1.6) ; python_version >= \"3.12\"", "pydantic (>=2.0) ; python_version >= \"3.12\"", "requests ; python_version >= \"3.12\"", "tomli ; python_version >= \"3.12\""] +vm = ["charmlibs-snap (>=1.0.1) ; python_version >= \"3.12\"", "charmlibs-systemd (>=1.0.0.post0) ; python_version >= \"3.12\"", "psutil (>=7.2.2) ; python_version >= \"3.12\"", "pysyncobj (>=0.3.15) ; python_version >= \"3.12\""] + +[package.source] +type = "url" +url = "https://github.com/canonical/postgresql-single-kernel-library/archive/912b1a44a1158f93e17fcb1e36719a878ff48810.tar.gz" [[package]] name = "prompt-toolkit" @@ -2048,7 +2054,7 @@ version = "2.13.4" description = "Data validation using Python type hints" optional = false python-versions = ">=3.9" -groups = ["main", "charm-libs"] +groups = ["main", "charm-libs", "integration"] files = [ {file = "pydantic-2.13.4-py3-none-any.whl", hash = "sha256:45a282cde31d808236fd7ea9d919b128653c8b38b393d1c4ab335c62924d9aba"}, {file = "pydantic-2.13.4.tar.gz", hash = "sha256:c40756b57adaa8b1efeeced5c196f3f3b7c435f90e84ea7f443901bec8099ef6"}, @@ -2070,7 +2076,7 @@ version = "2.46.4" description = "Core functionality for Pydantic validation and serialization" optional = false python-versions = ">=3.9" -groups = ["main", "charm-libs"] +groups = ["main", "charm-libs", "integration"] files = [ {file = "pydantic_core-2.46.4-cp310-cp310-macosx_10_12_x86_64.whl", hash = "sha256:a396dcc17e5a0b164dbe026896245a4fa9ff402edca1dff0be3d53a517f74de4"}, {file = "pydantic_core-2.46.4-cp310-cp310-macosx_11_0_arm64.whl", hash = "sha256:da4b951fe36dc7c3a1ccb4e3cd1747c3542b8c9ceede8fc86cae054e764485f5"}, @@ -2520,7 +2526,7 @@ version = "15.0.0" description = "Render rich text, tables, progress bars, syntax highlighting, markdown and more to the terminal" optional = false python-versions = ">=3.9.0" -groups = ["main"] +groups = ["main", "integration"] files = [ {file = "rich-15.0.0-py3-none-any.whl", hash = "sha256:33bd4ef74232fb73fe9279a257718407f169c09b78a87ad3d296f548e27de0bb"}, {file = "rich-15.0.0.tar.gz", hash = "sha256:edd07a4824c6b40189fb7ac9bc4c52536e9780fbbfbddf6f1e2502c31b068c36"}, @@ -2711,7 +2717,7 @@ version = "1.0.13" description = "A generator library for concise, unambiguous and URL-safe UUIDs." optional = false python-versions = ">=3.6" -groups = ["main"] +groups = ["main", "integration"] files = [ {file = "shortuuid-1.0.13-py3-none-any.whl", hash = "sha256:a482a497300b49b4953e15108a7913244e1bb0d41f9d332f5e9925dba33a3c5a"}, {file = "shortuuid-1.0.13.tar.gz", hash = "sha256:3bb9cf07f606260584b1df46399c0b87dd84773e7b25912b7e391e30797c5e72"}, @@ -2936,7 +2942,7 @@ version = "0.4.2" description = "Runtime typing introspection tools" optional = false python-versions = ">=3.9" -groups = ["main", "charm-libs"] +groups = ["main", "charm-libs", "integration"] files = [ {file = "typing_inspection-0.4.2-py3-none-any.whl", hash = "sha256:4ed1cacbdc298c220f1bd249ed5287caa16f34d44ef4e9c3d0cbad5b521545e7"}, {file = "typing_inspection-0.4.2.tar.gz", hash = "sha256:ba561c48a67c5958007083d386c3295464928b01faa735ab8547c5692e87f464"}, @@ -3129,4 +3135,4 @@ h11 = ">=0.16.0,<1" [metadata] lock-version = "2.1" python-versions = "^3.12" -content-hash = "d113fc50e37c361cba0fbb18adb17e4d2dc4f0d58cff9ecf7bf98faf0ecb221a" +content-hash = "bb1e4bf19a765abb3dbe69d2db99b281b7f54a3db3d7e6e1fa24d3ae37737e4f" diff --git a/pyproject.toml b/pyproject.toml index 00144f3a9ef..21e7e78917c 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -19,7 +19,7 @@ lightkube-models = "^1.28.1.4" psycopg2 = "^2.9.12" charmlibs-interfaces-tls-certificates = "^1.8.3" charm-refresh = "^3.1.1.2" -postgresql-charms-single-kernel = {extras = ["postgresql"], version = "16.3.4"} +postgresql-charms-single-kernel = {url = "https://github.com/canonical/postgresql-single-kernel-library/archive/912b1a44a1158f93e17fcb1e36719a878ff48810.tar.gz", extras = ["postgresql", "k8s"]} [tool.poetry.group.charm-libs.dependencies] # data_platform_libs/v0/data_interfaces.py @@ -71,7 +71,7 @@ allure-pytest = "^2.16.0" jubilant = "^1.11.0" tomli-w = "^1.2.0" tomli = "^2.4.1" -postgresql-charms-single-kernel = "*" +postgresql-charms-single-kernel = {url = "https://github.com/canonical/postgresql-single-kernel-library/archive/912b1a44a1158f93e17fcb1e36719a878ff48810.tar.gz", extras = ["postgresql", "k8s"]} [tool.poetry.group.build-refresh-version] optional = true diff --git a/src/charm.py b/src/charm.py index 6d9cf620d5d..769ea24342d 100755 --- a/src/charm.py +++ b/src/charm.py @@ -99,6 +99,7 @@ from requests import ConnectionError as RequestsConnectionError from single_kernel_postgresql.config.enums import Substrates from single_kernel_postgresql.config.exceptions import ( + DeployedWithoutTrustError, NotReadyError, SwitchoverFailedError, SwitchoverNotSyncError, @@ -250,7 +251,6 @@ def __init__(self, *args): # Managers 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.postgresql_service = "postgresql" self.rotate_logs_service = "rotate-logs" @@ -303,6 +303,15 @@ def __init__(self, *args): client_certificate=self.tls.client_certificate, peer_certificate=self.tls.peer_certificate, ) + 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, + ) # Bridge the lib TLS handler's requirer events back into a PostgreSQL reload: # the lib handler stores+pushes certs on certificate_available, then we refresh # the K8s trust store + charm-local CA bundle and reload. Also fires on @@ -396,7 +405,10 @@ def workload(self) -> K8sWorkload: BaseWorkload: The K8sWorkload instance for this charm """ return K8sWorkload( - charm_dir=self.charm_dir, container=self.unit.get_container(CONTAINER_NAME) + charm_dir=self.charm_dir, + container=self.unit.get_container(CONTAINER_NAME), + unit_name=self.unit.name, + namespace=self.model.name, ) @property @@ -2591,337 +2603,37 @@ def _can_connect_to_postgresql(self) -> bool: return False return True - def _calculate_max_worker_processes(self, cpu_cores: int) -> str | None: - """Calculate cpu_max_worker_processes configuration value.""" - if self.config.cpu_max_worker_processes == "auto": - # auto = minimum(8, 2 * vCores) - return str(min(8, 2 * cpu_cores)) - elif self.config.cpu_max_worker_processes is not None: - value = self.config.cpu_max_worker_processes - cap = 10 * cpu_cores - if value > cap: - raise ValueError( - f"cpu-max-worker-processes value {value} exceeds maximum allowed " - f"of {cap} (10 * vCores). Please set a value <= {cap}." - ) - return str(value) - return None - - def _validate_worker_config_value(self, param_name: str, value: int, cpu_cores: int) -> str: - """Shared validation logic for worker process parameters. - - Args: - param_name: The configuration parameter name (for error messages) - value: The integer value to validate - cpu_cores: The number of available CPU cores - - Returns: - String representation of the validated value - - Raises: - ValueError: If value is less than 2 or exceeds 10 * vCores - """ - cap = 10 * cpu_cores - if value > cap: - raise ValueError( - f"{param_name} value {value} exceeds maximum allowed " - f"of {cap} (10 * vCores). Please set a value <= {cap}." - ) - return str(value) - - def _calculate_max_parallel_workers(self, base_max_workers: int, cpu_cores: int) -> str | None: - """Calculate cpu_max_parallel_workers configuration value.""" - if self.config.cpu_max_parallel_workers == "auto": - return str(base_max_workers) - elif self.config.cpu_max_parallel_workers is not None: - # Validate the value first - validated_value_str = self._validate_worker_config_value( - "cpu-max-parallel-workers", self.config.cpu_max_parallel_workers, cpu_cores - ) - # Apply the min constraint with base_max_workers - return str(min(int(validated_value_str), base_max_workers)) - return None - - def _calculate_max_parallel_maintenance_workers( - self, base_max_workers: int, cpu_cores: int - ) -> str | None: - """Calculate cpu_max_parallel_maintenance_workers configuration value.""" - if self.config.cpu_max_parallel_maintenance_workers == "auto": - return str(base_max_workers) - elif self.config.cpu_max_parallel_maintenance_workers is not None: - return self._validate_worker_config_value( - "cpu-max-parallel-maintenance-workers", - self.config.cpu_max_parallel_maintenance_workers, - cpu_cores, - ) - return None - - def _calculate_max_logical_replication_workers( - self, base_max_workers: int, cpu_cores: int - ) -> str | None: - """Calculate cpu_max_logical_replication_workers configuration value.""" - if self.config.cpu_max_logical_replication_workers == "auto": - return str(base_max_workers) - elif self.config.cpu_max_logical_replication_workers is not None: - return self._validate_worker_config_value( - "cpu-max-logical-replication-workers", - self.config.cpu_max_logical_replication_workers, - cpu_cores, - ) - return None - - def _calculate_max_sync_workers_per_subscription( - self, base_max_workers: int, cpu_cores: int - ) -> str | None: - """Calculate cpu_max_sync_workers_per_subscription configuration value.""" - if self.config.cpu_max_sync_workers_per_subscription == "auto": - return str(base_max_workers) - elif self.config.cpu_max_sync_workers_per_subscription is not None: - return self._validate_worker_config_value( - "cpu-max-sync-workers-per-subscription", - self.config.cpu_max_sync_workers_per_subscription, - cpu_cores, - ) - return None - - def _calculate_max_parallel_apply_workers_per_subscription( - self, base_max_workers: int, cpu_cores: int - ) -> str | None: - """Calculate cpu_max_parallel_apply_workers_per_subscription configuration value.""" - if self.config.cpu_max_parallel_apply_workers_per_subscription == "auto": - return str(base_max_workers) - elif self.config.cpu_max_parallel_apply_workers_per_subscription is not None: - return self._validate_worker_config_value( - "cpu-max-parallel-apply-workers-per-subscription", - self.config.cpu_max_parallel_apply_workers_per_subscription, - cpu_cores, - ) - return None - - def _calculate_worker_process_config(self, cpu_cores: int) -> dict[str, str]: - """Calculate worker process configuration values. - - Handles 'auto' values and capping logic for worker process parameters. - Returns a dictionary with the calculated values ready for PostgreSQL. - """ - result: dict[str, str] = {} - - # Calculate cpu_max_worker_processes (baseline for other worker configs) - cpu_max_worker_processes_value = self._calculate_max_worker_processes(cpu_cores) - if cpu_max_worker_processes_value is not None: - result["max_worker_processes"] = cpu_max_worker_processes_value - - # Get the effective cpu_max_worker_processes for dependent configs - # Use the calculated value, or fall back to PostgreSQL default (8) - base_max_workers = int(result.get("max_worker_processes", "8")) - - # Calculate other worker parameters - cpu_max_parallel_workers_value = self._calculate_max_parallel_workers( - base_max_workers, cpu_cores - ) - if cpu_max_parallel_workers_value is not None: - result["max_parallel_workers"] = cpu_max_parallel_workers_value - - cpu_max_parallel_maintenance_workers_value = ( - self._calculate_max_parallel_maintenance_workers(base_max_workers, cpu_cores) - ) - if cpu_max_parallel_maintenance_workers_value is not None: - result["max_parallel_maintenance_workers"] = cpu_max_parallel_maintenance_workers_value - - cpu_max_logical_replication_workers_value = ( - self._calculate_max_logical_replication_workers(base_max_workers, cpu_cores) - ) - if cpu_max_logical_replication_workers_value is not None: - result["max_logical_replication_workers"] = cpu_max_logical_replication_workers_value - - cpu_max_sync_workers_per_subscription_value = ( - self._calculate_max_sync_workers_per_subscription(base_max_workers, cpu_cores) - ) - if cpu_max_sync_workers_per_subscription_value is not None: - result["max_sync_workers_per_subscription"] = ( - cpu_max_sync_workers_per_subscription_value - ) - - cpu_max_parallel_apply_workers_per_subscription_value = ( - self._calculate_max_parallel_apply_workers_per_subscription( - base_max_workers, cpu_cores - ) - ) - if cpu_max_parallel_apply_workers_per_subscription_value is not None: - result["max_parallel_apply_workers_per_subscription"] = ( - cpu_max_parallel_apply_workers_per_subscription_value - ) - - return result - - def _api_update_config(self, available_cpu_cores: int) -> bool: - # Use config value if set, calculate otherwise - if self.config.experimental_max_connections: - max_connections = self.config.experimental_max_connections - else: - max_connections = max(4 * available_cpu_cores, 100) - - cfg_patch: dict[str, int | str | None] = { - "max_connections": max_connections, - "max_prepared_transactions": self.config.memory_max_prepared_transactions, - "max_replication_slots": 25, - "max_wal_senders": 25, - "shared_buffers": self.config.memory_shared_buffers, - "wal_keep_size": self.config.durability_wal_keep_size, - } - - # Add restart-required worker process parameters via Patroni API - worker_configs = self._calculate_worker_process_config(available_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.config.durability_maximum_lag_on_failover, - } - if primary_endpoint := self.async_replication.get_primary_cluster_endpoint(): - base_patch["standby_cluster"] = {"host": primary_endpoint} - try: - self.patroni_manager.bulk_update_parameters_controller_by_patroni( - cfg_patch, base_patch - ) - except RetryError: - return False - return True - - def _build_postgresql_parameters( - self, available_cpu_cores: int, available_memory: int - ) -> dict | None: - """Build PostgreSQL configuration parameters. - - Args: - available_cpu_cores: Number of available CPU cores - available_memory: Available memory in bytes - - Returns: - Dictionary of PostgreSQL parameters or None if base parameters couldn't be built. - """ - limit_memory = None - if self.config.profile_limit_memory: - limit_memory = self.config.profile_limit_memory * 10**6 - - # Build PostgreSQL parameters. - pg_parameters = self.postgresql.build_postgresql_parameters( - self.model.config, available_memory, limit_memory + def request_restart(self) -> None: + """Bridge for the lib ConfigManager: refresh metrics scrape jobs and request a restart.""" + self.metrics_endpoint.update_scrape_job_spec( + self._generate_metrics_jobs(self.is_tls_enabled) ) + self.on[str(self.restart_manager.name)].acquire_lock.emit() - # Calculate and merge worker process configurations - worker_configs = self._calculate_worker_process_config(available_cpu_cores) - - # Add cpu_wal_compression configuration (separate from worker processes) - if self.config.cpu_wal_compression is not None: - cpu_wal_compression = "on" if self.config.cpu_wal_compression else "off" - else: - # Use config.yaml default when unset (default: true) - cpu_wal_compression = "on" - - if pg_parameters is not None: - pg_parameters.update(worker_configs) - pg_parameters["wal_compression"] = cpu_wal_compression - else: - pg_parameters = dict(worker_configs) - pg_parameters["wal_compression"] = cpu_wal_compression - logger.debug(f"pg_parameters set to worker_configs = {pg_parameters}") + def refresh_endpoints(self) -> None: + """Bridge for the lib ConfigManager: refresh client-relation endpoints.""" + self.postgresql_client_relation.update_endpoints() - return pg_parameters + def restart_services(self) -> None: + """Bridge for the lib ConfigManager: restart the metrics and LDAP sync services.""" + self._restart_metrics_service() + self._restart_ldap_sync_service() def update_config(self, is_creating_backup: bool = False) -> bool: """Updates Patroni config file based on the existence of the TLS files.""" - # Retrieve PostgreSQL parameters. try: - available_cpu_cores, available_memory = self.get_available_resources() - except ApiError as e: - if e.status.code == 403: - self.on_deployed_without_trust() - return False - raise e - - # Build PostgreSQL parameters - postgresql_parameters = self._build_postgresql_parameters( - available_cpu_cores, available_memory - ) - - # replication_slots = self.logical_replication.replication_slots() - replication_slots = {} - - logger.info("Updating Patroni config file") - # Update and reload configuration based on TLS files availability. - # TODO move to config manager's update config - self.config_manager.render_patroni_yml_file( - connectivity=self.state.peer.is_connectivity_enabled, - is_creating_backup=is_creating_backup, - enable_ldap=self.state.application.is_ldap_enabled, - # TODO add rel handler - 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"), - restore_to_latest=self.state.application.data.get("restore-to-time", None) == "latest", - stanza=self.state.application.data.get("stanza", self.state.peer.data.get("stanza")), - restore_stanza=self.state.application.data.get("restore-stanza"), - parameters=postgresql_parameters, - # TODO add rel handler - user_databases_map=self.relations_user_databases_map, - # TODO add rel handler - ldap_parameters=self.get_ldap_parameters(), - # TODO add rel handler - async_primary_cluster_endpoint=self.async_replication.get_primary_cluster_endpoint(), - async_standby_endpoints=self.async_replication.get_standby_endpoints(), - slots=replication_slots, - ) - - if not self._is_workload_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.unit_peer_data.update({"tls": "enabled" if self.is_tls_enabled else ""}) - self.postgresql_client_relation.update_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_postgresql_restart_need(True) - return True - logger.debug("Early exit update_config: Patroni not started yet") - return False - - if not self._api_update_config(available_cpu_cores): - logger.warning("Early exit update_config: Unable to patch Patroni API") - return False - - if 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" + return self.config_manager.update_config( + self.postgresql, + self.generate_user_hash, + is_creating_backup=is_creating_backup, + relations_user_databases_map=self.relations_user_databases_map, + ldap_parameters=self.get_ldap_parameters(), + async_primary_cluster_endpoint=self.async_replication.get_primary_cluster_endpoint(), + async_standby_endpoints=self.async_replication.get_standby_endpoints(), ) - - self._handle_postgresql_restart_need( - self.unit_peer_data.get("config_hash") != self.generate_config_hash - ) - self._restart_metrics_service() - self._restart_ldap_sync_service() - - self.unit_peer_data.update({ - "user_hash": self.generate_user_hash, - "config_hash": self.generate_config_hash, - }) - if self.unit.is_leader(): - self.app_peer_data.update({"user_hash": self.generate_user_hash}) - return True + except DeployedWithoutTrustError: + self.on_deployed_without_trust() + return False def _validate_config_options(self) -> None: """Validates specific config options that need access to the database or to the TLS status.""" @@ -2952,42 +2664,6 @@ def _validate_config_options(self) -> None: "storage-default-table-access-method config option has an invalid value" ) - def _handle_postgresql_restart_need(self, config_changed: bool): - """Handle PostgreSQL restart need based on the TLS configuration and configuration changes.""" - if self._can_connect_to_postgresql: - restart_postgresql = self.is_tls_enabled != self.postgresql.is_tls_enabled() - 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() - 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.unit_peer_data.update({"tls": "enabled" if self.is_tls_enabled else ""}) - self.postgresql_client_relation.update_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.metrics_endpoint.update_scrape_job_spec( - self._generate_metrics_jobs(self.is_tls_enabled) - ) - self.on[str(self.restart_manager.name)].acquire_lock.emit() - def _update_pebble_layers(self, replan: bool = True) -> None: """Update the pebble layers to keep the health check URL up-to-date.""" # Get the current layer. @@ -3155,11 +2831,6 @@ def generate_user_hash(self) -> str: """Generate expected user and database hash.""" return shake_128(str(self._collect_user_relations()).encode()).hexdigest(16) - @cached_property - def generate_config_hash(self) -> str: - """Generate current configuration hash.""" - return shake_128(str(self.config.model_dump()).encode()).hexdigest(16) - def override_patroni_on_failure_condition( self, new_condition: str, repeat_cause: str | None ) -> bool: @@ -3345,32 +3016,6 @@ def get_ldap_parameters(self) -> dict: "ldapsearchfilter": self.config.ldap_search_filter, } - def is_restart_pending(self) -> bool: - """Query pg_settings for pending restart.""" - connection = None - try: - with ( - self.postgresql._connect_to_database( - database_host=self.postgresql.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() - if __name__ == "__main__": main(PostgresqlOperatorCharm, use_juju_for_storage=True) diff --git a/templates/patroni.yml.j2 b/templates/patroni.yml.j2 deleted file mode 100644 index c1da1015c14..00000000000 --- a/templates/patroni.yml.j2 +++ /dev/null @@ -1,217 +0,0 @@ -bootstrap: - dcs: - failsafe_mode: true - maximum_lag_on_failover: {{ maximum_lag_on_failover }} - synchronous_mode: true - synchronous_mode_strict: false - synchronous_node_count: {{ synchronous_node_count }} - max_timelines_history: 50 - postgresql: - use_pg_rewind: true - remove_data_directory_on_rewind_failure: false - remove_data_directory_on_diverged_timelines: false - bin_dir: /usr/lib/postgresql/{{ version }}/bin - parameters: - synchronous_standby_names: "*" - {%- if enable_pgbackrest_archiving %} - archive_command: 'pgbackrest --stanza={{ stanza }} archive-push %p' - {% else %} - archive_command: /bin/true - {%- endif %} - archive_mode: on - autovacuum: true - debug_print_plan: 'off' - debug_print_parse: 'off' - debug_print_rewritten: 'off' - fsync: true - full_page_writes: true - lc_messages: 'en_US.UTF8' - log_autovacuum_min_duration: 60000 - log_checkpoints: 'on' - log_destination: 'stderr' - log_directory: '{{ logs_storage_path }}/16/main/pg_logs' - log_error_verbosity: 'verbose' - log_file_mode: '0600' - log_filename: 'postgresql-%w_%H%M.log' - log_hostname: 'off' - log_line_prefix: '%t [%p]: user=%u,db=%d,app=%a,client=%h,line=%l ' - log_min_duration_sample: -1 - log_min_error_statement: 'warning' - log_recovery_conflict_waits: 'on' - log_replication_commands: 'on' - log_rotation_age: 1 - log_rotation_size: 0 - log_statement: ddl - log_statement_sample_rate: 1 - log_statement_stats: 'off' - log_temp_files: 1 - log_timezone: 'UTC' - log_truncate_on_rotation: 'on' - logging_collector: 'on' - wal_level: logical - shared_preload_libraries: 'timescaledb,pgaudit,set_user,pg_stat_statements' - session_preload_libraries: 'login_hook' - set_user.block_log_statement: 'on' - set_user.exit_on_error: 'on' - set_user.superuser_allowlist: '+charmed_dba' - {%- if slots %} - slots: - {%- for slot, database in slots.items() %} - {{slot}}: - database: {{database}} - plugin: pgoutput - type: logical - {%- endfor -%} - {% endif %} - - {%- if restoring_backup %} - method: pgbackrest - pgbackrest: - command: > - pgbackrest --stanza={{ restore_stanza }} --pg1-path={{ pgdata_path }} - {%- if backup_id %} --set={{ backup_id }} {%- endif %} - {%- if restore_timeline %} --target-timeline="0x{{ restore_timeline }}" {% endif %} - {%- if restore_to_latest %} --type=default {%- else %} - --target-action=promote {%- if pitr_target %} --target="{{ pitr_target }}" --type=time {%- else %} --type=immediate {%- endif %} - {%- endif %} - restore - no_params: True - keep_existing_recovery_conf: True - {% elif primary_cluster_endpoint %} - standby_cluster: - host: {{ primary_cluster_endpoint }} - port: 5432 - create_replica_methods: ["basebackup"] - {% else %} - initdb: - - auth-host: scram-sha-256 - - auth-local: trust - - encoding: UTF8 - - data-checksums - - waldir: {{ logs_storage_path }}/16/main/pg_wal - {%- endif %} - pg_hba: - - {{ 'hostssl' if enable_tls else 'host' }} all all 0.0.0.0/0 {{ instance_password_encryption }} - - {{ 'hostssl' if enable_tls else 'host' }} replication replication 127.0.0.1/32 scram-sha-256 - {%- for endpoint in extra_replication_endpoints %} - - {{ 'hostssl' if enable_tls else 'host' }} replication replication {{ endpoint }}/32 scram-sha-256 - {%- endfor %} -bypass_api_service: true -log: - dir: {{ logs_storage_path }}/16/main/patroni_logs - dateformat: "%Y-%m-%d %H:%M:%S %Z" - format: "%(asctime)s [%(process)d]: %(levelname)s: %(message)s " - file_num: 10080 - file_size: 600 -restapi: - connect_address: '{{ endpoint }}:8008' - listen: 0.0.0.0:8008 - {%- if patroni_password %} - authentication: - username: patroni - password: {{ patroni_password }} - {%- endif %} - cafile: {{ storage_path }}/peer_ca.pem - certfile: {{ storage_path }}/peer_cert.pem - keyfile: {{ storage_path }}/peer_key.pem -ctl: - cacert: {{ storage_path }}/peer_ca.pem -pod_ip: '{{ endpoint }}' -postgresql: - basebackup: - - waldir: {{ logs_storage_path }}/16/main/pg_wal - connect_address: '{{ endpoint }}:5432' - data_dir: {{ pgdata_path }} - bin_dir: /usr/lib/postgresql/{{ version }}/bin - listen: 0.0.0.0:5432 - parameters: - shared_preload_libraries: 'timescaledb,pgaudit,set_user,pg_stat_statements' - session_preload_libraries: 'login_hook' - set_user.block_log_statement: 'on' - set_user.exit_on_error: 'on' - set_user.superuser_allowlist: '+charmed_dba' - {%- if enable_pgbackrest_archiving %} - archive_command: 'pgbackrest --stanza={{ stanza }} archive-push %p' - {% else %} - archive_command: /bin/true - {%- endif %} - {%- if enable_tls %} - ssl: on - ssl_ca_file: {{ storage_path }}/ca.pem - ssl_cert_file: {{ storage_path }}/cert.pem - ssl_key_file: {{ storage_path }}/key.pem - {%- endif %} - temp_tablespaces: temp - pg_stat_statements.max: 10000 - {%- if pg_parameters %} - {%- for key, value in pg_parameters.items() %} - {{key}}: {{value}} - {%- endfor -%} - {% endif %} - pgpass: /tmp/pgpass - pg_hba: - - local all backup peer map=operator - - local all monitoring password - - {{ 'hostssl' if enable_tls else 'host' }} all +charmed_stats 0.0.0.0/0 scram-sha-256 - - {{ 'hostssl' if enable_tls else 'host' }} all +charmed_read 0.0.0.0/0 scram-sha-256 - - {{ 'hostssl' if enable_tls else 'host' }} all +charmed_dml 0.0.0.0/0 scram-sha-256 - - {{ 'hostssl' if enable_tls else 'host' }} all +charmed_backup 0.0.0.0/0 scram-sha-256 - - {{ 'hostssl' if enable_tls else 'host' }} all +charmed_dba 0.0.0.0/0 scram-sha-256 - - {{ 'hostssl' if enable_tls else 'host' }} all +charmed_admin 0.0.0.0/0 scram-sha-256 - - {{ 'hostssl' if enable_tls else 'host' }} all +charmed_databases_owner 0.0.0.0/0 scram-sha-256 - {%- if not connectivity %} - - {{ 'hostssl' if enable_tls else 'host' }} all all {{ endpoint }}.{{ namespace }}.svc.cluster.local {{ instance_password_encryption }} - - {{ 'hostssl' if enable_tls else 'host' }} all all 0.0.0.0/0 reject - {%- elif enable_ldap %} - - {{ 'hostssl' if enable_tls else 'host' }} all +identity_access 0.0.0.0/0 ldap {{ ldap_parameters }} - - {{ 'hostssl' if enable_tls else 'host' }} all +internal_access 0.0.0.0/0 {{ instance_password_encryption }} - {%- for user, databases in user_databases_map.items() %} - - {{ 'hostssl' if enable_tls else 'host' }} {{ databases }} {{ user }} 0.0.0.0/0 {{ instance_password_encryption }} - {%- endfor %} - {%- else %} - - {{ 'hostssl' if enable_tls else 'host' }} all +internal_access 0.0.0.0/0 {{ instance_password_encryption }} - {%- for user, databases in user_databases_map.items() %} - {%- if 'pgbouncer_auth_relation_' in user %} - - {{ 'hostssl' if enable_tls else 'host' }} {{ databases }} {{ user }} 0.0.0.0/0 md5 - {%- else %} - - {{ 'hostssl' if enable_tls else 'host' }} {{ databases }} {{ user }} 0.0.0.0/0 {{ instance_password_encryption }} - {%- endif %} - {%- endfor %} - {%- endif %} - - {{ 'hostssl' if enable_tls else 'host' }} replication replication 127.0.0.1/32 scram-sha-256 - - {{ 'hostssl' if enable_tls else 'host' }} replication replication 127.0.0.6/32 scram-sha-256 - {%- for endpoint in extra_replication_endpoints %} - - {{ 'hostssl' if enable_tls else 'host' }} replication replication {{ endpoint }}/32 scram-sha-256 - {%- endfor %} - {%- for endpoint in endpoints %} - - {{ 'hostssl' if enable_tls else 'host' }} replication replication {{ endpoint }}.{{ namespace }}.svc.cluster.local scram-sha-256 - {%- endfor %} - pg_ident: - - operator postgres backup - authentication: - replication: - password: {{ replication_password }} - {%- if enable_tls %} - sslcert: {{ conf_path }}/nonexistent_cert.pem - sslkey: {{ conf_path }}/nonexistent_key.pem - {%- endif %} - rewind: - username: {{ rewind_user }} - password: {{ rewind_password }} - {%- if enable_tls %} - sslcert: {{ conf_path }}/nonexistent_cert.pem - sslkey: {{ conf_path }}/nonexistent_key.pem - {%- endif %} - superuser: - password: {{ superuser_password }} - {%- if enable_tls %} - sslcert: {{ conf_path }}/nonexistent_cert.pem - sslkey: {{ conf_path }}/nonexistent_key.pem - {%- endif %} -use_endpoints: true -use_unix_socket: true -{%- if is_no_sync_member or is_creating_backup %} -tags: - nosync: {{ is_no_sync_member }} - is_creating_backup: {{ is_creating_backup }} -{%- endif %} diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py index 3b2670a9877..9178b37c926 100644 --- a/tests/unit/test_charm.py +++ b/tests/unit/test_charm.py @@ -25,12 +25,13 @@ from ops.testing import Harness from requests import ConnectionError as RequestsConnectionError from single_kernel_postgresql.config.exceptions import ( + DeployedWithoutTrustError, NotReadyError, SwitchoverFailedError, SwitchoverNotSyncError, ) from single_kernel_postgresql.config.literals import PEER_RELATION, SECRET_INTERNAL_LABEL -from tenacity import RetryError, wait_fixed +from tenacity import RetryError from charm import EXTENSION_OBJECT_MESSAGE, PostgresqlOperatorCharm from tests.unit.helpers import _FakeApiError @@ -1361,179 +1362,93 @@ def test_clean_ca_file_from_workload(harness): assert ca_exists is False -def test_update_config(harness): +def test_request_restart(harness): + """The request_restart bridge refreshes the metrics scrape job and acquires the restart lock.""" + with ( + patch("charm.PostgresqlOperatorCharm._generate_metrics_jobs") as _generate_metrics_jobs, + patch("charm.PostgresqlOperatorCharm.is_tls_enabled", new_callable=PropertyMock), + patch("charm.MetricsEndpointProvider.update_scrape_job_spec") as _update_scrape_job_spec, + patch("charms.rolling_ops.v0.rollingops.RollingOpsManager._on_acquire_lock") as _restart, + ): + harness.charm.request_restart() + + _update_scrape_job_spec.assert_called_once_with(_generate_metrics_jobs.return_value) + _restart.assert_called_once() + + +def test_refresh_endpoints(harness): + """The refresh_endpoints bridge refreshes the client-relation endpoints.""" + with patch("charm.PostgreSQLProvider.update_endpoints") as _update_endpoints: + harness.charm.refresh_endpoints() + + _update_endpoints.assert_called_once_with() + + +def test_restart_services(harness): + """The restart_services bridge restarts both the metrics and LDAP sync pebble services.""" with ( - patch("ops.model.Container.get_plan") as _get_plan, - patch("charm.PostgresqlOperatorCharm.get_available_resources", return_value=(4, 8000000)), - patch( - "charm.PostgresqlOperatorCharm._handle_postgresql_restart_need" - ) as _handle_postgresql_restart_need, - patch("charm.PatroniManager.ensure_slots_controller_by_patroni"), patch( "charm.PostgresqlOperatorCharm._restart_metrics_service" ) as _restart_metrics_service, patch( "charm.PostgresqlOperatorCharm._restart_ldap_sync_service" ) as _restart_ldap_sync_service, - patch("charm.PatroniManager.bulk_update_parameters_controller_by_patroni"), - patch("charm.PatroniManager.member_started", new_callable=PropertyMock) as _member_started, - patch( - "charm.PostgresqlOperatorCharm._is_workload_running", new_callable=PropertyMock - ) as _is_workload_running, - patch("charm.ConfigManager.render_patroni_yml_file") as _render_patroni_yml_file, - patch("charm.PostgresqlOperatorCharm.is_primary", return_value=False), - patch( - "charm.PostgresqlOperatorCharm.is_tls_enabled", new_callable=PropertyMock - ) as _is_tls_enabled, - patch.object(PostgresqlOperatorCharm, "postgresql", Mock()) as postgresql_mock, ): - rel_id = harness.model.get_relation(PEER_RELATION).id - # Mock some properties. - harness.set_can_connect(POSTGRESQL_CONTAINER, True) - postgresql_mock.is_tls_enabled = PropertyMock(side_effect=[False, False, False, False]) - _is_workload_running.side_effect = [True, True, False, True] - _member_started.side_effect = [True, True, False] - postgresql_mock.build_postgresql_parameters.return_value = {"test": "test"} + harness.charm.restart_services() - # Test without TLS files available. - with harness.hooks_disabled(): - harness.update_relation_data(rel_id, harness.charm.unit.name, {"tls": ""}) - _is_tls_enabled.return_value = False - harness.charm.update_config() - # Expected parameters include base "test": "test" plus worker configs (auto = 8, since min(8, 2*4) = 8) - expected_parameters = { - "test": "test", - "max_worker_processes": "8", - "max_parallel_workers": "8", - "max_parallel_maintenance_workers": "8", - "max_logical_replication_workers": "8", - "max_sync_workers_per_subscription": "8", - "max_parallel_apply_workers_per_subscription": "8", - "wal_compression": "on", - } - _render_patroni_yml_file.assert_called_once_with( - connectivity=True, - is_creating_backup=False, - enable_ldap=False, - enable_tls=False, - # is_no_sync_member=False, - backup_id=None, - pitr_target=None, - restore_timeline=None, - restore_to_latest=False, - stanza=None, - restore_stanza=None, - parameters=expected_parameters, - user_databases_map={"operator": "all", "replication": "all", "rewind": "all"}, - ldap_parameters={}, - async_primary_cluster_endpoint=None, - async_standby_endpoints=[], - slots={}, - ) - _handle_postgresql_restart_need.assert_called_once() - _restart_metrics_service.assert_called_once() - _restart_ldap_sync_service.assert_called_once() - assert "tls" not in harness.get_relation_data(rel_id, harness.charm.unit.name) - - # Test with TLS files available. - _handle_postgresql_restart_need.reset_mock() - _restart_metrics_service.reset_mock() - _restart_ldap_sync_service.reset_mock() - harness.update_relation_data( - rel_id, harness.charm.unit.name, {"tls": ""} - ) # Mock some data in the relation to test that it change. - _is_tls_enabled.return_value = True - _render_patroni_yml_file.reset_mock() - harness.charm.update_config() - _render_patroni_yml_file.assert_called_once_with( - connectivity=True, - is_creating_backup=False, - enable_ldap=False, - enable_tls=True, - # is_no_sync_member=False, - backup_id=None, - pitr_target=None, - restore_timeline=None, - restore_to_latest=False, - stanza=None, - restore_stanza=None, - parameters=expected_parameters, - user_databases_map={"operator": "all", "replication": "all", "rewind": "all"}, - ldap_parameters={}, - async_primary_cluster_endpoint=None, - async_standby_endpoints=[], - slots={}, - ) - _handle_postgresql_restart_need.assert_called_once() - _restart_metrics_service.assert_called_once() - _restart_ldap_sync_service.assert_called_once() - assert "tls" not in harness.get_relation_data( - rel_id, harness.charm.unit.name - ) # The "tls" flag is set in handle_postgresql_restart_need. - - # Test with workload not running yet. - harness.update_relation_data( - rel_id, harness.charm.unit.name, {"tls": ""} - ) # Mock some data in the relation to test that it change. - _handle_postgresql_restart_need.reset_mock() - _restart_metrics_service.reset_mock() - _restart_ldap_sync_service.reset_mock() - harness.charm.update_config() - _handle_postgresql_restart_need.assert_not_called() - _restart_metrics_service.assert_not_called() - _restart_ldap_sync_service.assert_not_called() - assert harness.get_relation_data(rel_id, harness.charm.unit.name)["tls"] == "enabled" - - # Test with member not started yet. - harness.update_relation_data( - rel_id, harness.charm.unit.name, {"tls": ""} - ) # Mock some data in the relation to test that it doesn't change. - _is_tls_enabled.return_value = False - harness.charm.update_config() - _handle_postgresql_restart_need.assert_not_called() - _restart_metrics_service.assert_not_called() - _restart_ldap_sync_service.assert_not_called() - assert "tls" not in harness.get_relation_data(rel_id, harness.charm.unit.name) - - -def test_handle_postgresql_restart_need(harness): + _restart_metrics_service.assert_called_once_with() + _restart_ldap_sync_service.assert_called_once_with() + + +def test_update_config_delegates_to_config_manager(harness): + """update_config delegates to the lib ConfigManager with the charm-sourced parameters.""" with ( + patch("charm.ConfigManager.update_config") as _update_config, patch.object(PostgresqlOperatorCharm, "postgresql", Mock()) as postgresql_mock, - patch("charms.rolling_ops.v0.rollingops.RollingOpsManager._on_acquire_lock") as _restart, - patch("charm.PostgresqlOperatorCharm._generate_metrics_jobs") as _generate_metrics_jobs, - patch("charm.wait_fixed", return_value=wait_fixed(0)), patch( - "charm.PatroniManager.reload_patroni_configuration" - ) as _reload_patroni_configuration, - patch("charm.PostgresqlOperatorCharm.is_restart_pending") as _is_restart_pending, + "charm.PostgresqlOperatorCharm.relations_user_databases_map", + new_callable=PropertyMock, + ) as _relations_user_databases_map, + patch("charm.PostgresqlOperatorCharm.get_ldap_parameters") as _get_ldap_parameters, patch( - "charm.PostgresqlOperatorCharm.is_tls_enabled", new_callable=PropertyMock - ) as _is_tls_enabled, + "charm.PostgreSQLAsyncReplication.get_primary_cluster_endpoint" + ) as _get_primary_cluster_endpoint, + patch("charm.PostgreSQLAsyncReplication.get_standby_endpoints") as _get_standby_endpoints, ): - rel_id = harness.model.get_relation(PEER_RELATION).id - for values in itertools.product([True, False], [True, False], [True, False]): - _reload_patroni_configuration.reset_mock() - _generate_metrics_jobs.reset_mock() - _restart.reset_mock() - with harness.hooks_disabled(): - harness.update_relation_data(rel_id, harness.charm.unit.name, {"tls": ""}) + _update_config.return_value = True + _relations_user_databases_map.return_value = {"operator": "all"} + _get_ldap_parameters.return_value = {"ldap": "params"} + _get_primary_cluster_endpoint.return_value = "primary-endpoint" + _get_standby_endpoints.return_value = ["standby-endpoint"] + + result = harness.charm.update_config(is_creating_backup=True) + + assert result is True + _update_config.assert_called_once_with( + postgresql_mock, + harness.charm.generate_user_hash, + is_creating_backup=True, + relations_user_databases_map={"operator": "all"}, + ldap_parameters={"ldap": "params"}, + async_primary_cluster_endpoint="primary-endpoint", + async_standby_endpoints=["standby-endpoint"], + ) - _is_tls_enabled.return_value = values[0] - postgresql_mock.is_tls_enabled = PropertyMock(return_value=values[1]) - _is_restart_pending.return_value = values[2] - harness.charm._handle_postgresql_restart_need(True) - _reload_patroni_configuration.assert_called_once() - if values[0]: - assert "tls" in harness.get_relation_data(rel_id, harness.charm.unit) - else: - assert "tls" not in harness.get_relation_data(rel_id, harness.charm.unit) - if (values[0] != values[1]) or values[2]: - _generate_metrics_jobs.assert_called_once_with(values[0]) - _restart.assert_called_once() - else: - _generate_metrics_jobs.assert_not_called() - _restart.assert_not_called() +def test_update_config_catches_deployed_without_trust(harness): + """update_config catches DeployedWithoutTrustError, blocks the unit, and returns False.""" + with ( + patch("charm.ConfigManager.update_config", side_effect=DeployedWithoutTrustError), + patch.object(PostgresqlOperatorCharm, "postgresql", Mock()), + patch("charm.PostgresqlOperatorCharm.get_ldap_parameters"), + patch("charm.PostgreSQLAsyncReplication.get_primary_cluster_endpoint"), + patch("charm.PostgreSQLAsyncReplication.get_standby_endpoints"), + ): + result = harness.charm.update_config() + + assert result is False + assert isinstance(harness.charm.unit.status, BlockedStatus) + assert "juju trust" in harness.charm.unit.status.message def test_set_active_status(harness): @@ -1938,297 +1853,3 @@ def test_on_secret_remove(harness): event.secret.label = None harness.charm._on_secret_remove(event) assert not event.remove_revision.called - - -def test_calculate_worker_process_config_auto_values(harness): - """Test worker process config calculation with 'auto' values.""" - with patch.object( - PostgresqlOperatorCharm, "get_available_resources", return_value=(2, 8000000) - ): - with harness.hooks_disabled(): - harness.update_config({ - "cpu-max-worker-processes": "auto", - "cpu-max-parallel-workers": "auto", - "cpu-max-parallel-maintenance-workers": "auto", - "cpu-max-logical-replication-workers": "auto", - "cpu-max-sync-workers-per-subscription": "auto", - "cpu-max-parallel-apply-workers-per-subscription": "auto", - }) - - result = harness.charm._calculate_worker_process_config(2) - - # All auto values should resolve to 4 (min(8, 2*2)) - assert result["max_worker_processes"] == "4" - assert result["max_parallel_workers"] == "4" - assert result["max_parallel_maintenance_workers"] == "4" - assert result["max_logical_replication_workers"] == "4" - assert result["max_sync_workers_per_subscription"] == "4" - assert result["max_parallel_apply_workers_per_subscription"] == "4" - - # Test with 8 vCPUs (auto should be min(8, 2*8) = 8) - result = harness.charm._calculate_worker_process_config(8) - assert result["max_worker_processes"] == "8" - assert result["max_parallel_workers"] == "8" - - # Test with 1 vCPU (auto should be min(8, 2*1) = 2) - result = harness.charm._calculate_worker_process_config(1) - assert result["max_worker_processes"] == "2" - - -def test_calculate_worker_process_config_numeric_values(harness): - """Test worker process config calculation with numeric values.""" - with harness.hooks_disabled(): - harness.update_config({ - "cpu-max-worker-processes": "10", - "cpu-max-parallel-workers": "8", - "cpu-max-parallel-maintenance-workers": "8", - }) - - result = harness.charm._calculate_worker_process_config(4) - - assert result["max_worker_processes"] == "10" - assert result["max_parallel_workers"] == "8" - assert result["max_parallel_maintenance_workers"] == "8" - - -def test_calculate_worker_process_config_all_workers_numeric(harness): - """Test all worker configs with numeric values within limits (not capped).""" - with harness.hooks_disabled(): - harness.update_config({ - "cpu-max-worker-processes": "20", - "cpu-max-parallel-workers": "15", - "cpu-max-parallel-maintenance-workers": "10", - "cpu-max-logical-replication-workers": "12", - "cpu-max-sync-workers-per-subscription": "8", - "cpu-max-parallel-apply-workers-per-subscription": "8", - }) - - result = harness.charm._calculate_worker_process_config(4) # Cap would be 40 - - # All values should pass through unchanged (within cap of 40) - assert result["max_worker_processes"] == "20" - assert result["max_parallel_workers"] == "15" - assert result["max_parallel_maintenance_workers"] == "10" - assert result["max_logical_replication_workers"] == "12" - assert result["max_sync_workers_per_subscription"] == "8" - assert result["max_parallel_apply_workers_per_subscription"] == "8" - - -def test_calculate_worker_process_config_validation_blocking_max_worker_processes(harness): - """Test worker process config validation blocks max_worker_processes exceeding values instead of capping.""" - with harness.hooks_disabled(): - harness.update_config({"cpu-max-worker-processes": "50"}) # Exceeds cap of 20 (2*10) - - with pytest.raises( - ValueError, match="cpu-max-worker-processes value 50 exceeds maximum allowed of 20" - ): - harness.charm._calculate_worker_process_config(2) - - -def test_calculate_worker_process_config_validation_blocking_max_parallel_workers(harness): - """Test worker process config validation blocks max_parallel_workers exceeding values instead of capping.""" - with harness.hooks_disabled(): - harness.update_config({"cpu-max-parallel-workers": "30"}) # Exceeds cap of 20 (2*10) - - with pytest.raises( - ValueError, match="cpu-max-parallel-workers value 30 exceeds maximum allowed of 20" - ): - harness.charm._calculate_worker_process_config(2) - - -def test_calculate_worker_process_config_validation_blocking_valid_value(harness): - """Test worker process config validation accepts valid values within cap.""" - with harness.hooks_disabled(): - harness.update_config({"cpu-max-parallel-maintenance-workers": "15"}) # Within cap of 20 - - result = harness.charm._calculate_worker_process_config(2) - assert result["max_parallel_maintenance_workers"] == "15" # Should accept valid value - - -def test_calculate_worker_process_config_edge_cases(harness): - """Test worker process config edge cases.""" - # Test with no worker config set (but wal_compression has default) - with harness.hooks_disabled(): - harness.update_config( - unset=[ - "cpu-max-worker-processes", - "cpu-max-parallel-workers", - "cpu-max-parallel-maintenance-workers", - "cpu-max-logical-replication-workers", - "cpu-max-sync-workers-per-subscription", - "cpu-max-parallel-apply-workers-per-subscription", - "cpu-wal-compression", - ] - ) - - result = harness.charm._calculate_worker_process_config(4) - # All worker parameters default to auto, so they get calculated - expected = { - "max_worker_processes": "8", # min(8, 2 * 4) = 8 - "max_parallel_workers": "8", - "max_parallel_maintenance_workers": "8", - "max_logical_replication_workers": "8", - "max_sync_workers_per_subscription": "8", - "max_parallel_apply_workers_per_subscription": "8", - } - assert result == expected - - # Test with only one worker config set - with harness.hooks_disabled(): - harness.update_config({"cpu-max-worker-processes": "10"}) - - result = harness.charm._calculate_worker_process_config(4) - # Other worker parameters still get auto-calculated since they default to "auto" - expected = { - "max_worker_processes": "10", - "max_parallel_workers": "10", # auto = base_max_workers (10) - "max_parallel_maintenance_workers": "10", - "max_logical_replication_workers": "10", - "max_sync_workers_per_subscription": "10", - "max_parallel_apply_workers_per_subscription": "10", - } - assert result == expected - - # Test with very high vCPU count (reset config to defaults first) - with harness.hooks_disabled(): - harness.update_config( - unset=[ - "cpu-max-worker-processes", - "cpu-max-parallel-workers", - "cpu-max-parallel-maintenance-workers", - "cpu-max-logical-replication-workers", - "cpu-max-sync-workers-per-subscription", - "cpu-max-parallel-apply-workers-per-subscription", - ] - ) - - result = harness.charm._calculate_worker_process_config(128) - # Should be min(8, 2*128) = 8 - expected = { - "max_worker_processes": "8", - "max_parallel_workers": "8", - "max_parallel_maintenance_workers": "8", - "max_logical_replication_workers": "8", - "max_sync_workers_per_subscription": "8", - "max_parallel_apply_workers_per_subscription": "8", - } - assert result == expected - - -def test_calculate_worker_process_config_dependencies(harness): - """Test that dependent worker configs properly use max_worker_processes as base.""" - with harness.hooks_disabled(): - harness.update_config({ - "cpu-max-worker-processes": "12", - "cpu-max-parallel-workers": "auto", - "cpu-max-parallel-maintenance-workers": "auto", - }) - - result = harness.charm._calculate_worker_process_config(4) - - # Auto values should use max_worker_processes (12) as base - assert result["max_worker_processes"] == "12" - assert result["max_parallel_workers"] == "12" - assert result["max_parallel_maintenance_workers"] == "12" - - -def test_calculate_worker_process_config_wal_compression(harness): - """Test that wal_compression is NOT included in worker process config.""" - # Test with wal_compression enabled - should NOT be in worker config result - with harness.hooks_disabled(): - harness.update_config({"cpu-wal-compression": True}) - - result = harness.charm._calculate_worker_process_config(4) - assert "wal_compression" not in result # wal_compression is not part of worker configs - - # Test with wal_compression disabled - should NOT be in worker config result - with harness.hooks_disabled(): - harness.update_config({"cpu-wal-compression": False}) - - result = harness.charm._calculate_worker_process_config(4) - assert "wal_compression" not in result # wal_compression is not part of worker configs - - # Test with wal_compression not explicitly set - should NOT be in worker config result - with harness.hooks_disabled(): - harness.update_config(unset=["cpu_wal_compression"]) - - result = harness.charm._calculate_worker_process_config(4) - assert "wal_compression" not in result # wal_compression is not part of worker configs - - -def test_calculate_worker_process_config_mixed_auto_and_numeric(harness): - """Test worker process config with mixed auto and numeric values.""" - with harness.hooks_disabled(): - harness.update_config({ - "cpu-max-worker-processes": "16", - "cpu-max-parallel-workers": "auto", # Should become 16 - "cpu-max-parallel-maintenance-workers": "8", # Explicit value - "cpu-max-logical-replication-workers": "auto", # Should become 16 - }) - - result = harness.charm._calculate_worker_process_config(4) - - assert result["max_worker_processes"] == "16" - assert result["max_parallel_workers"] == "16" # Auto resolved to base - assert result["max_parallel_maintenance_workers"] == "8" # Explicit value - assert result["max_logical_replication_workers"] == "16" # Auto resolved to base - - -def test_calculate_worker_process_config_zero_cpu_count(harness): - """Test worker process config when cpu_count is 0 (edge case).""" - with harness.hooks_disabled(): - harness.update_config({"cpu-max-worker-processes": "auto"}) - - result = harness.charm._calculate_worker_process_config(0) - - # min(8, 2*0) = 0 - assert result["max_worker_processes"] == "0" - - -def test_calculate_worker_process_config_all_workers_validation_blocking(harness): - """Test that worker configs exceeding limits raise ValueError instead of capping.""" - cpu_cores = 2 # Cap = 20 - - # Test each exceeding parameter individually (since first exception stops execution) - exceeding_configs = [ - ("cpu-max-worker-processes", "25"), - ("cpu-max-parallel-maintenance-workers", "22"), - ("cpu-max-logical-replication-workers", "30"), - ("cpu-max-sync-workers-per-subscription", "25"), - ("cpu-max-parallel-apply-workers-per-subscription", "100"), - ] - - for param_name, invalid_value in exceeding_configs: - # Reset all configs to auto first, then set the specific one we want to test - reset_config = { - "cpu-max-worker-processes": "auto", - "cpu-max-parallel-workers": "auto", - "cpu-max-parallel-maintenance-workers": "auto", - "cpu-max-logical-replication-workers": "auto", - "cpu-max-sync-workers-per-subscription": "auto", - "cpu-max-parallel-apply-workers-per-subscription": "auto", - } - reset_config[param_name] = invalid_value - - with harness.hooks_disabled(): - harness.update_config(reset_config) - - with pytest.raises( - ValueError, - match=f"{param_name} value {invalid_value} exceeds maximum allowed of 20", - ): - harness.charm._calculate_worker_process_config(cpu_cores) - - # Test valid value within cap works - need to set max_worker_processes high enough - with harness.hooks_disabled(): - harness.update_config({ - "cpu-max-worker-processes": "20", # Set high enough to allow max_parallel_workers = 18 - "cpu-max-parallel-workers": "18", # Within both CPU cap (20) and max_worker_processes (20) - "cpu-max-parallel-maintenance-workers": "auto", - "cpu-max-logical-replication-workers": "auto", - "cpu-max-sync-workers-per-subscription": "auto", - "cpu-max-parallel-apply-workers-per-subscription": "auto", - }) - - result = harness.charm._calculate_worker_process_config(cpu_cores) - assert result["max_parallel_workers"] == "18" # Should accept valid value From 71f3879aac437bf28f8d865534847493fc2ab8c3 Mon Sep 17 00:00:00 2001 From: Marcelo Henrique Neppel Date: Mon, 20 Jul 2026 14:21:50 -0300 Subject: [PATCH 2/2] refactor(tls): reload on tls_files_pushed instead of certificate_available The lib TLS handler now emits tls_files_pushed after a successful cert push. Observe that single event instead of certificate_available/relation_broken on both requirers, and drop the two guards (internal-ca present, cert files on disk) the bridge duplicated from the handler: the event fires only once the push has landed, so a deferred push no longer lets the bridge reload against stale files. This also removes the ops observer-order dependency the wiring tests asserted. Re-pin the lib to the commit that adds the event. Signed-off-by: Marcelo Henrique Neppel --- poetry.lock | 6 +- pyproject.toml | 4 +- src/charm.py | 39 +++-------- tests/unit/test_tls.py | 145 ++--------------------------------------- 4 files changed, 21 insertions(+), 173 deletions(-) diff --git a/poetry.lock b/poetry.lock index 3ee1be3f2db..8b9e39a512e 100644 --- a/poetry.lock +++ b/poetry.lock @@ -1788,7 +1788,7 @@ optional = false python-versions = ">=3.8,<4.0" groups = ["main", "integration"] files = [ - {file = "912b1a44a1158f93e17fcb1e36719a878ff48810.tar.gz", hash = "sha256:6af3973d1b8c5ada5be238527e86fc223657c1ec2b7d9ada58cea7ec43c9f34a"}, + {file = "f188608e1859e3f1212d5f1d8c77397caa05fbbc.tar.gz", hash = "sha256:0863c731d66d95fd089b16495ea37420975e0e0c84a885edbde631d75e8c8e4d"}, ] [package.dependencies] @@ -1815,7 +1815,7 @@ vm = ["charmlibs-snap (>=1.0.1) ; python_version >= \"3.12\"", "charmlibs-system [package.source] type = "url" -url = "https://github.com/canonical/postgresql-single-kernel-library/archive/912b1a44a1158f93e17fcb1e36719a878ff48810.tar.gz" +url = "https://github.com/canonical/postgresql-single-kernel-library/archive/f188608e1859e3f1212d5f1d8c77397caa05fbbc.tar.gz" [[package]] name = "prompt-toolkit" @@ -3135,4 +3135,4 @@ h11 = ">=0.16.0,<1" [metadata] lock-version = "2.1" python-versions = "^3.12" -content-hash = "bb1e4bf19a765abb3dbe69d2db99b281b7f54a3db3d7e6e1fa24d3ae37737e4f" +content-hash = "f02b8ce22dec81f343b5a86f44d45981fe25d61210fddb1d0c9032a3afcb8bf3" diff --git a/pyproject.toml b/pyproject.toml index 21e7e78917c..2588a3c3243 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -19,7 +19,7 @@ lightkube-models = "^1.28.1.4" psycopg2 = "^2.9.12" charmlibs-interfaces-tls-certificates = "^1.8.3" charm-refresh = "^3.1.1.2" -postgresql-charms-single-kernel = {url = "https://github.com/canonical/postgresql-single-kernel-library/archive/912b1a44a1158f93e17fcb1e36719a878ff48810.tar.gz", extras = ["postgresql", "k8s"]} +postgresql-charms-single-kernel = {url = "https://github.com/canonical/postgresql-single-kernel-library/archive/f188608e1859e3f1212d5f1d8c77397caa05fbbc.tar.gz", extras = ["postgresql", "k8s"]} [tool.poetry.group.charm-libs.dependencies] # data_platform_libs/v0/data_interfaces.py @@ -71,7 +71,7 @@ allure-pytest = "^2.16.0" jubilant = "^1.11.0" tomli-w = "^1.2.0" tomli = "^2.4.1" -postgresql-charms-single-kernel = {url = "https://github.com/canonical/postgresql-single-kernel-library/archive/912b1a44a1158f93e17fcb1e36719a878ff48810.tar.gz", extras = ["postgresql", "k8s"]} +postgresql-charms-single-kernel = {url = "https://github.com/canonical/postgresql-single-kernel-library/archive/f188608e1859e3f1212d5f1d8c77397caa05fbbc.tar.gz", extras = ["postgresql", "k8s"]} [tool.poetry.group.build-refresh-version] optional = true diff --git a/src/charm.py b/src/charm.py index 769ea24342d..1946dc5c8b0 100755 --- a/src/charm.py +++ b/src/charm.py @@ -129,8 +129,6 @@ SPI_MODULE, SYSTEM_USERS, TLS_CA_BUNDLE_FILE, - TLS_CLIENT_RELATION, - TLS_PEER_RELATION, TRACING_RELATION_NAME, UNIT_SCOPE, USER, @@ -312,22 +310,12 @@ def __init__(self, *args): refresh_endpoints=self.refresh_endpoints, restart_services=self.restart_services, ) - # Bridge the lib TLS handler's requirer events back into a PostgreSQL reload: - # the lib handler stores+pushes certs on certificate_available, then we refresh - # the K8s trust store + charm-local CA bundle and reload. Also fires on - # relation_broken so detaching the TLS operator re-renders Patroni with TLS off. - self.framework.observe( - self.tls.client_certificate.on.certificate_available, self._reload_tls_after_push - ) - self.framework.observe( - self.tls.peer_certificate.on.certificate_available, self._reload_tls_after_push - ) - self.framework.observe( - self.on[TLS_CLIENT_RELATION].relation_broken, self._reload_tls_after_push - ) - self.framework.observe( - self.on[TLS_PEER_RELATION].relation_broken, self._reload_tls_after_push - ) + # Reload PostgreSQL after the lib TLS handler has actually pushed the cert files. + # tls_files_pushed fires only on a completed push (the handler routes both + # certificate_available and relation_broken through it); we then refresh the K8s + # trust store + charm-local CA bundle and reload. A deferred push never emits, so it + # never triggers a reload against files that were never written. + self.framework.observe(self.tls.tls_files_pushed, self._reload_tls_after_push) self.tls_transfer = TLSTransfer(self, PEER_RELATION) self.async_replication = PostgreSQLAsyncReplication(self) # self.logical_replication = PostgreSQLLogicalReplication(self) @@ -2456,18 +2444,11 @@ def _reload_tls_after_push(self, event) -> None: charm-local CA bundle that this unit's Patroni REST client verifies against. Refresh those, then reload so PostgreSQL/Patroni pick up the new material. - Mirror the handler's readiness guard: when the internal CA is absent the - handler defers its push (no files on disk), so skip the reload to avoid - rendering ssl:on against missing TLS files on an already-running unit. + Observes the handler's ``tls_files_pushed`` event, which fires only after a + successful push, so the files are on disk and the internal CA is present by the + time this runs -- no local readiness guard is needed. A transient config-apply + failure defers and retries rather than leaving stale TLS state or failing the hook. """ - if not self.get_secret(APP_SCOPE, "internal-ca"): - return - # Don't enable TLS in the config until the lib has written the cert files to - # disk (its Pebble push can defer while this local render would still succeed, - # which would start Patroni ssl:on against missing files). - if self.is_tls_enabled and not self.tls_manager.client_tls_files_on_disk(): - event.defer() - return self._sync_tls_trust_store_and_bundle() try: if not self.update_config(): diff --git a/tests/unit/test_tls.py b/tests/unit/test_tls.py index 2b069b8d595..27fb68d4c0b 100644 --- a/tests/unit/test_tls.py +++ b/tests/unit/test_tls.py @@ -18,8 +18,6 @@ from single_kernel_postgresql.config.literals import ( PEER_RELATION, TLS_CA_BUNDLE_FILE, - TLS_CLIENT_RELATION, - TLS_PEER_RELATION, ) from single_kernel_postgresql.events.tls import TLS from single_kernel_postgresql.managers.tls import TLSManager @@ -60,74 +58,22 @@ def test_is_tls_enabled_reflects_tls_manager(harness): assert harness.charm.is_tls_enabled is True -def _observers_for(harness, bound_event): - """Return method names observing the given bound event, in registration order.""" - emitter_path = bound_event.emitter.handle.path - event_kind = bound_event.event_kind - return [ - method - for (_obs_path, method, e_path, e_kind) in harness.framework._observers - if e_path == emitter_path and e_kind == event_kind - ] +def test_reload_bridge_observes_tls_files_pushed(harness): + """The reload bridge fires on the lib's tls_files_pushed event, not certificate_available. - -def test_reload_bridge_wired_after_handler_on_client_certificate(harness): - """The reload bridge observes the same certificate_available event as the handler. - - The lib handler's store+push observer must be registered BEFORE the charm's - reload bridge so that, when the event fires, certs are stored+pushed first and - the reload (update_config) runs afterwards (ops calls observers in order). + The lib emits tls_files_pushed only after a successful push, so the sync+reload runs once + the files are on disk; a deferred push never emits and never triggers a stale reload. This + also removes the ops observer-order dependency the bridge previously relied on. """ - methods = _observers_for( - harness, harness.charm.tls.client_certificate.on.certificate_available - ) - assert "_on_certificate_available" in methods, methods - assert "_reload_tls_after_push" in methods, methods - # Handler (store+push) before bridge (reload). - assert methods.index("_on_certificate_available") < methods.index("_reload_tls_after_push") - - -def test_reload_bridge_wired_after_handler_on_peer_certificate(harness): - """The reload bridge also observes the peer certificate_available event.""" - methods = _observers_for(harness, harness.charm.tls.peer_certificate.on.certificate_available) - assert "_on_peer_certificate_available" in methods, methods - assert "_reload_tls_after_push" in methods, methods - assert methods.index("_on_peer_certificate_available") < methods.index( - "_reload_tls_after_push" - ) - - -def test_reload_bridge_calls_update_config(harness): - """_reload_tls_after_push syncs CA artifacts and reloads when internal-ca is present.""" - with harness.hooks_disabled(): - harness.set_leader(True) - harness.charm.set_secret("app", "internal-ca", "ca-content") - with ( patch("charm.PostgresqlOperatorCharm.update_config") as _update_config, patch("charm.PostgresqlOperatorCharm._sync_tls_trust_store_and_bundle") as _sync, ): - harness.charm._reload_tls_after_push(Mock()) + harness.charm.tls.tls_files_pushed.emit() _sync.assert_called_once_with() _update_config.assert_called_once_with() -def test_reload_bridge_skips_without_internal_ca(harness): - """_reload_tls_after_push is a no-op when internal-ca is absent (defer path). - - The lib TLS handler defers its push when the internal CA isn't present yet - (no files written to disk). The bridge must not sync or reload in that case, - or it would render ssl:on against TLS files that don't exist yet. - """ - with ( - patch("charm.PostgresqlOperatorCharm.update_config") as _update_config, - patch("charm.PostgresqlOperatorCharm._sync_tls_trust_store_and_bundle") as _sync, - ): - harness.charm._reload_tls_after_push(Mock()) - _update_config.assert_not_called() - _sync.assert_not_called() - - def test_sync_tls_trust_store_and_bundle_writes_ca_artifacts(harness, tmp_path): """Sync pushes the CA into the container trust store and writes the bundle. @@ -182,59 +128,6 @@ def test_sync_tls_trust_store_skips_container_push_when_no_ca(harness): _open.assert_called_once() -def _relation_broken_observers(harness, relation_name): - """Return method names observing relation_broken for the given relation, in order. - - Relation events are dispatched from the charm's ``on`` handle, so e_path is - ``/on`` and e_kind is ``_relation_broken``. - """ - on_path = f"{harness.charm.handle.path}/on" - event_kind = f"{relation_name.replace('-', '_')}_relation_broken" - return [ - method - for (_obs_path, method, e_path, e_kind) in harness.framework._observers - if e_path == on_path and e_kind == event_kind - ] - - -def test_reload_bridge_wired_on_client_relation_broken(harness): - """On client-certificate relation_broken the lib handler fires before the charm bridge. - - When the TLS operator detaches, the lib clears + pushes TLS files (dropping the - operator CA from state), then the bridge refreshes the /tmp bundle + reloads - (update_config). The lib's observer must come first so cleared state is visible - when the bridge runs. - """ - methods = _relation_broken_observers(harness, TLS_CLIENT_RELATION) - assert "_on_certificate_available" in methods, ( - f"lib's _on_certificate_available not observing {TLS_CLIENT_RELATION} relation_broken: {methods}" - ) - assert "_reload_tls_after_push" in methods, ( - f"charm's _reload_tls_after_push not observing {TLS_CLIENT_RELATION} relation_broken: {methods}" - ) - assert methods.index("_on_certificate_available") < methods.index("_reload_tls_after_push"), ( - "lib handler must fire before charm bridge" - ) - - -def test_reload_bridge_wired_on_peer_relation_broken(harness): - """On peer-certificate relation_broken the lib handler fires before the charm bridge. - - Same ordering constraint as the client case: the lib clears peer state first, - the bridge sees the cleared state when computing the updated bundle. - """ - methods = _relation_broken_observers(harness, TLS_PEER_RELATION) - assert "_on_peer_certificate_available" in methods, ( - f"lib's _on_peer_certificate_available not observing {TLS_PEER_RELATION} relation_broken: {methods}" - ) - assert "_reload_tls_after_push" in methods, ( - f"charm's _reload_tls_after_push not observing {TLS_PEER_RELATION} relation_broken: {methods}" - ) - assert methods.index("_on_peer_certificate_available") < methods.index( - "_reload_tls_after_push" - ), "lib handler must fire before charm bridge" - - def test_pebble_ready_internal_cert_path_calls_update_config(harness): """G3: pebble-ready internal-cert path calls update_config eagerly. @@ -321,29 +214,3 @@ def test_reload_bridge_no_defer_on_success(harness): _update_config.assert_called_once_with() event.defer.assert_not_called() - - -def test_reload_bridge_defers_until_tls_files_on_disk(harness): - """_reload_tls_after_push defers instead of rendering ssl:on before files exist. - - With TLS enabled (client cert material assigned) but the lib's file push not - yet landed in the container (e.g. its Pebble push deferred), reloading would - render ssl:on against missing files. The bridge must defer and retry, and - must not sync the trust store against not-yet-pushed material either. - """ - with harness.hooks_disabled(): - harness.set_leader(True) - harness.charm.set_secret("app", "internal-ca", "ca-content") - - event = Mock() - with ( - patch("charm.TLSManager.get_client_tls_files", return_value=("key", "ca", "cert")), - patch("charm.TLSManager.client_tls_files_on_disk", return_value=False), - patch("charm.PostgresqlOperatorCharm.update_config") as _update_config, - patch("charm.PostgresqlOperatorCharm._sync_tls_trust_store_and_bundle") as _sync, - ): - harness.charm._reload_tls_after_push(event) - - event.defer.assert_called_once_with() - _update_config.assert_not_called() - _sync.assert_not_called()