From 4e71d1bec10cce839d72d33086ece3f3697ba455 Mon Sep 17 00:00:00 2001 From: Marcelo Henrique Neppel Date: Thu, 18 Dec 2025 17:05:44 -0300 Subject: [PATCH 01/24] Update PostgreSQL mount paths to /var/lib/pg with 16/main subdirectory MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Migrate storage mount paths from /var/lib/postgresql/* to /var/lib/pg/* and configure PostgreSQL 16 data directories to use the 16/main subfolder structure. Path changes: - Archive: /var/lib/postgresql/archive → /var/lib/pg/archive - Data: /var/lib/postgresql/data/pgdata → /var/lib/pg/data/16/main - Logs: /var/lib/postgresql/logs → /var/lib/pg/logs/16/main (WAL) - Temp: /var/lib/postgresql/temp → /var/lib/pg/temp/16/main Key changes: - Update metadata.yaml storage mount locations - Update POSTGRESQL_DATA_PATH constant - Configure pgdata_path, waldir, and temp tablespace paths - Add make_parents=True to Pebble make_dir for nested directory creation - Update pgbackrest.conf.j2 with new data path - Update patroni.yml.j2 with new data_dir and waldir - Update async_replication.py cleanup paths - Update authorisation_rules_observer.py Patroni config path Note: Archive path (/var/lib/pg/archive) intentionally does not use the 16/main subfolder as it is not used in Patroni or PostgreSQL configuration. Signed-off-by: Marcelo Henrique Neppel --- docs/explanation/logs.md | 2 +- docs/reference/troubleshooting.md | 4 ++-- metadata.yaml | 16 ++++++------- scripts/authorisation_rules_observer.py | 2 +- src/backups.py | 4 ++-- src/charm.py | 16 ++++++++----- src/constants.py | 2 +- src/relations/async_replication.py | 6 ++--- templates/patroni.yml.j2 | 8 +++---- templates/pgbackrest.conf.j2 | 4 ++-- tests/integration/ha_tests/clean-data-dir.sh | 8 +++---- tests/integration/ha_tests/helpers.py | 2 +- tests/integration/test_charm.py | 2 +- tests/integration/test_tls.py | 2 +- tests/unit/test_backups.py | 12 +++++----- tests/unit/test_charm.py | 24 ++++++++++++-------- tests/unit/test_patroni.py | 6 ++--- 17 files changed, 64 insertions(+), 56 deletions(-) diff --git a/docs/explanation/logs.md b/docs/explanation/logs.md index 4e430381b39..378d85315d5 100644 --- a/docs/explanation/logs.md +++ b/docs/explanation/logs.md @@ -78,7 +78,7 @@ The naming convention of the Pgbackrest logs is `.patroni- --repo1-s3-key-secret= --repo1-s3-region=eu-central-1 --repo1-s3-uri-style=host --repo1-type=s3 --stanza=discourse.patroni-pg --start-fast --type=full +2023-10-11 13:06:29.857 P00 INFO: backup command begin 2.47: --no-backup-standby --exec-id=843-b0d896e1 --log-level-console=debug --pg1-path=/var/lib/pg/data/16/main --pg1-user=backup --repo1-path=/postgresql-test --repo1-retention-full=9999999 --repo1-s3-bucket=dragop-test-bucket --repo1-s3-endpoint=https://s3.eu-central-1.amazonaws.com --repo1-s3-key= --repo1-s3-key-secret= --repo1-s3-region=eu-central-1 --repo1-s3-uri-style=host --repo1-type=s3 --stanza=discourse.patroni-pg --start-fast --type=full 2023-10-11 13:06:30.869 P00 INFO: execute non-exclusive backup start: backup begins after the requested immediate checkpoint completes 2023-10-11 13:06:31.671 P00 INFO: backup start archive = 000000010000000000000004, lsn = 0/4000060 2023-10-11 13:06:31.671 P00 INFO: check archive for prior segment 000000010000000000000003 diff --git a/docs/reference/troubleshooting.md b/docs/reference/troubleshooting.md index 4a4f2b99581..345c18547fe 100644 --- a/docs/reference/troubleshooting.md +++ b/docs/reference/troubleshooting.md @@ -119,9 +119,9 @@ postgresql enabled active today at 12:29 UTC root@postgresql-k8s-0:/# ps auxww USER PID %CPU %MEM VSZ RSS TTY STAT START TIME COMMAND root 1 0.0 0.0 718264 10916 ? Ssl 12:29 0:00 /charm/bin/pebble run --create-dirs --hold --http :38813 --verbose -postgres 14 0.1 0.1 565020 39412 ? Sl 12:29 0:01 python3 /usr/bin/patroni /var/lib/postgresql/data/patroni.yml +postgres 14 0.1 0.1 565020 39412 ? Sl 12:29 0:01 python3 /usr/bin/patroni /var/lib/pg/data/patroni.yml postgres 30 0.0 0.0 1082704 9076 ? Sl 12:30 0:00 /usr/bin/prometheus-postgres-exporter -postgres 48 0.0 0.0 215488 28912 ? S 12:30 0:00 /usr/lib/postgresql/14/bin/postgres -D /var/lib/postgresql/data/pgdata --config-file=/var/lib/postgresql/data/pgdata/postgresql.conf --listen_addresses=0.0.0.0 --port=5432 --cluster_name=patroni-postgresql-k8s --wal_level=logical --hot_standby=on --max_connections=100 --max_wal_senders=10 --max_prepared_transactions=0 --max_locks_per_transaction=64 --track_commit_timestamp=off --max_replication_slots=10 --max_worker_processes=8 --wal_log_hints=on +postgres 48 0.0 0.0 215488 28912 ? S 12:30 0:00 /usr/lib/postgresql/16/bin/postgres -D /var/lib/pg/data/16/main --config-file=/var/lib/pg/data/16/main/postgresql.conf --listen_addresses=0.0.0.0 --port=5432 --cluster_name=patroni-postgresql-k8s --wal_level=logical --hot_standby=on --max_connections=100 --max_wal_senders=10 --max_prepared_transactions=0 --max_locks_per_transaction=64 --track_commit_timestamp=off --max_replication_slots=10 --max_worker_processes=8 --wal_log_hints=on postgres 50 0.0 0.0 70080 7488 ? Ss 12:30 0:00 postgres: patroni-postgresql-k8s: logger postgres 52 0.0 0.0 215592 9136 ? Ss 12:30 0:00 postgres: patroni-postgresql-k8s: checkpointer postgres 53 0.0 0.0 215604 9632 ? Ss 12:30 0:00 postgres: patroni-postgresql-k8s: background writer diff --git a/metadata.yaml b/metadata.yaml index 74dc9218ef8..37d72bdf990 100644 --- a/metadata.yaml +++ b/metadata.yaml @@ -29,13 +29,13 @@ containers: resource: postgresql-image mounts: - storage: archive - location: /var/lib/postgresql/archive + location: /var/lib/pg/archive - storage: data - location: /var/lib/postgresql/data + location: /var/lib/pg/data - storage: logs - location: /var/lib/postgresql/logs + location: /var/lib/pg/logs - storage: temp - location: /var/lib/postgresql/temp + location: /var/lib/pg/temp resources: postgresql-image: @@ -107,19 +107,19 @@ storage: archive: type: filesystem description: Storage mount used for holding local backups (before typically sending them to remote object storage) when relevant/needed. - location: /var/lib/postgresql/archive + location: /var/lib/pg/archive data: type: filesystem description: Storage mount used for storing all tables, indexes, and so on (except those from temporary tablespaces). - location: /var/lib/postgresql/data + location: /var/lib/pg/data logs: type: filesystem description: Storage mount used for storing all the logs that are part of the transactional commit path (WAL files). - location: /var/lib/postgresql/logs + location: /var/lib/pg/logs temp: type: filesystem description: Storage mount used for storing temporary tablespaces (where typically sort operations happen). - location: /var/lib/postgresql/temp + location: /var/lib/pg/temp assumes: - k8s-api diff --git a/scripts/authorisation_rules_observer.py b/scripts/authorisation_rules_observer.py index 54ea62e52e7..205bb324971 100644 --- a/scripts/authorisation_rules_observer.py +++ b/scripts/authorisation_rules_observer.py @@ -19,7 +19,7 @@ PATRONI_CLUSTER_STATUS_ENDPOINT = "cluster" PATRONI_CONFIG_STATUS_ENDPOINT = "config" TLS_CA_BUNDLE_FILE = "peer_ca_bundle.pem" -PATRONI_CONF_FILE_PATH = "/var/lib/postgresql/data/patroni.yml" +PATRONI_CONF_FILE_PATH = "/var/lib/pg/data/patroni.yml" # File path for the spawned cluster topology observer process to write logs. LOG_FILE_PATH = "/var/log/authorisation_rules_observer.log" diff --git a/src/backups.py b/src/backups.py index 7b04cb1962f..537eced3913 100644 --- a/src/backups.py +++ b/src/backups.py @@ -194,7 +194,7 @@ def can_use_s3_repository(self) -> tuple[bool, str]: system_identifier_from_instance, error = self._execute_command([ f"/usr/lib/postgresql/{self.charm._patroni.rock_postgresql_version.split('.')[0]}/bin/pg_controldata", - "/var/lib/postgresql/data/pgdata", + "/var/lib/pg/data/16/main", ]) if error != "": raise Exception(error) @@ -273,7 +273,7 @@ def _create_bucket_if_not_exists(self) -> None: def _empty_data_files(self) -> None: """Empty the PostgreSQL data directory in preparation of backup restore.""" try: - self.container.exec(["rm", "-r", "/var/lib/postgresql/data/pgdata"]).wait_output() + self.container.exec(["rm", "-r", "/var/lib/pg/data/16/main"]).wait_output() except ExecError as e: # If previous PITR restore was unsuccessful, there is no such directory. if "No such file or directory" not in str(e.stderr): diff --git a/src/charm.py b/src/charm.py index 4a7c3d35f73..0bf15ae7743 100755 --- a/src/charm.py +++ b/src/charm.py @@ -261,7 +261,7 @@ def __init__(self, *args): self._certs_path = "/usr/local/share/ca-certificates" self._storage_path = str(self.meta.storages["data"].location) - self.pgdata_path = f"{self._storage_path}/pgdata" + self.pgdata_path = f"{self._storage_path}/16/main" self.framework.observe(self.on.upgrade_charm, self._on_upgrade_charm) self.postgresql_client_relation = PostgreSQLProvider(self) @@ -1136,13 +1136,17 @@ def _create_pgdata(self, container: Container): """Create the PostgreSQL data directory.""" if not container.exists(self.pgdata_path): container.make_dir( - self.pgdata_path, permissions=0o700, user=WORKLOAD_OS_USER, group=WORKLOAD_OS_GROUP + self.pgdata_path, + permissions=0o700, + user=WORKLOAD_OS_USER, + group=WORKLOAD_OS_GROUP, + make_parents=True, ) # Also, fix the permissions from the parent directory. container.exec([ "chown", f"{WORKLOAD_OS_USER}:{WORKLOAD_OS_GROUP}", - "/var/lib/postgresql/archive", + "/var/lib/pg/archive", ]).wait() container.exec([ "chown", @@ -1152,12 +1156,12 @@ def _create_pgdata(self, container: Container): container.exec([ "chown", f"{WORKLOAD_OS_USER}:{WORKLOAD_OS_GROUP}", - "/var/lib/postgresql/logs", + "/var/lib/pg/logs", ]).wait() container.exec([ "chown", f"{WORKLOAD_OS_USER}:{WORKLOAD_OS_GROUP}", - "/var/lib/postgresql/temp", + "/var/lib/pg/temp", ]).wait() def _on_start(self, _) -> None: @@ -1360,7 +1364,7 @@ def _setup_users(self) -> None: extra_user_roles=["pg_monitor"], ) - self.postgresql.set_up_database(temp_location="/var/lib/postgresql/temp") + self.postgresql.set_up_database(temp_location="/var/lib/pg/temp/16/main") access_groups = self.postgresql.list_access_groups() if access_groups != set(ACCESS_GROUPS): diff --git a/src/constants.py b/src/constants.py index 08b75644b5a..da5e934b71e 100644 --- a/src/constants.py +++ b/src/constants.py @@ -18,7 +18,7 @@ WORKLOAD_OS_GROUP = "postgres" WORKLOAD_OS_USER = "postgres" METRICS_PORT = "9187" -POSTGRESQL_DATA_PATH = "/var/lib/postgresql/data/pgdata" +POSTGRESQL_DATA_PATH = "/var/lib/pg/data/16/main" POSTGRESQL_LOGS_PATH = "/var/log/postgresql" POSTGRESQL_LOGS_PATTERN = "postgresql*.log" POSTGRES_LOG_FILES = [ diff --git a/src/relations/async_replication.py b/src/relations/async_replication.py index ae788e96a01..0a1f50b7cc8 100644 --- a/src/relations/async_replication.py +++ b/src/relations/async_replication.py @@ -721,10 +721,10 @@ def _stop_database(self, event: RelationChangedEvent) -> bool: # Remove and recreate the pgdata folder to enable replication of the data from the # primary cluster. for path in [ - "/var/lib/postgresql/archive", + "/var/lib/pg/archive", POSTGRESQL_DATA_PATH, - "/var/lib/postgresql/logs", - "/var/lib/postgresql/temp", + "/var/lib/pg/logs", + "/var/lib/pg/temp", ]: logger.info(f"Removing contents from {path}") self.container.exec(f"find {path} -mindepth 1 -delete".split()).wait_output() diff --git a/templates/patroni.yml.j2 b/templates/patroni.yml.j2 index 490a3ed83b0..45701bfcf3d 100644 --- a/templates/patroni.yml.j2 +++ b/templates/patroni.yml.j2 @@ -65,7 +65,7 @@ bootstrap: method: pgbackrest pgbackrest: command: > - pgbackrest --stanza={{ restore_stanza }} --pg1-path={{ storage_path }}/pgdata + pgbackrest --stanza={{ restore_stanza }} --pg1-path={{ storage_path }}/16/main {%- if backup_id %} --set={{ backup_id }} {%- endif %} {%- if restore_timeline %} --target-timeline="0x{{ restore_timeline }}" {% endif %} {%- if restore_to_latest %} --type=default {%- else %} @@ -85,7 +85,7 @@ bootstrap: - auth-local: trust - encoding: UTF8 - data-checksums - - waldir: /var/lib/postgresql/logs + - waldir: /var/lib/pg/logs/16/main {%- endif %} pg_hba: - {{ 'hostssl' if enable_tls else 'host' }} all all 0.0.0.0/0 {{ instance_password_encryption }} @@ -116,9 +116,9 @@ ctl: pod_ip: '{{ endpoint }}' postgresql: basebackup: - - waldir: /var/lib/postgresql/logs + - waldir: /var/lib/pg/logs/16/main connect_address: '{{ endpoint }}:5432' - data_dir: {{ storage_path }}/pgdata + data_dir: {{ storage_path }}/16/main bin_dir: /usr/lib/postgresql/{{ version }}/bin listen: 0.0.0.0:5432 parameters: diff --git a/templates/pgbackrest.conf.j2 b/templates/pgbackrest.conf.j2 index 12bd7374d63..573f9dc87aa 100644 --- a/templates/pgbackrest.conf.j2 +++ b/templates/pgbackrest.conf.j2 @@ -33,7 +33,7 @@ tls-server-key-file={{ storage_path }}/peer_key.pem {%- endif %} [{{ stanza }}] -pg1-path={{ storage_path }}/pgdata +pg1-path={{ storage_path }}/16/main pg1-user={{ user }} {%- if enable_tls %} {% set ns = namespace(count=2) %} @@ -43,7 +43,7 @@ pg{{ ns.count }}-host-ca-file={{ storage_path }}/peer_ca.pem pg{{ ns.count }}-host-cert-file={{ storage_path }}/peer_cert.pem pg{{ ns.count }}-host-key-file={{ storage_path }}/peer_key.pem pg{{ ns.count }}-host={{ peer_endpoint }} -pg{{ ns.count }}-path={{ storage_path }}/pgdata +pg{{ ns.count }}-path={{ storage_path }}/16/main pg{{ ns.count }}-user={{ user }} {% set ns.count = ns.count + 1 %} {%- endfor %} diff --git a/tests/integration/ha_tests/clean-data-dir.sh b/tests/integration/ha_tests/clean-data-dir.sh index 96a0a6ad558..73afed0affa 100755 --- a/tests/integration/ha_tests/clean-data-dir.sh +++ b/tests/integration/ha_tests/clean-data-dir.sh @@ -1,7 +1,7 @@ #!/usr/bin/env bash set -Eeuo pipefail -rm -rf /var/lib/postgresql/archive/* -rm -rf /var/lib/postgresql/data/pgdata/* -rm -rf /var/lib/postgresql/logs/* -rm -rf /var/lib/postgresql/temp/* +rm -rf /var/lib/pg/archive/* +rm -rf /var/lib/pg/data/16/main/* +rm -rf /var/lib/pg/logs/* +rm -rf /var/lib/pg/temp/* diff --git a/tests/integration/ha_tests/helpers.py b/tests/integration/ha_tests/helpers.py index 1c850986f2f..0a310e4364d 100644 --- a/tests/integration/ha_tests/helpers.py +++ b/tests/integration/ha_tests/helpers.py @@ -621,7 +621,7 @@ async def is_replica(ops_test: OpsTest, unit_name: str) -> bool: async def list_wal_files(ops_test: OpsTest, app: str) -> set: """Returns the list of WAL segment files in each unit.""" units = [unit.name for unit in ops_test.model.applications[app].units] - command = "ls -1 /var/lib/postgresql/data/pgdata/pg_wal/" + command = "ls -1 /var/lib/pg/data/16/main/pg_wal/" files = {} for unit in units: complete_command = f"run --unit {unit} -- {command}" diff --git a/tests/integration/test_charm.py b/tests/integration/test_charm.py index da0fb71296d..375506350f5 100644 --- a/tests/integration/test_charm.py +++ b/tests/integration/test_charm.py @@ -494,7 +494,7 @@ async def test_storage_with_more_restrictive_permissions(ops_test: OpsTest, char ) # Restrict the permissions of the storage. - command = "chmod 755 /var/lib/postgresql/data" + command = "chmod 755 /var/lib/pg/data" complete_command = f"ssh --container postgresql {app_name}/0 {command}" for attempt in Retrying(stop=stop_after_delay(60), wait=wait_fixed(3), reraise=True): with attempt: diff --git a/tests/integration/test_tls.py b/tests/integration/test_tls.py index 997ed2cd12a..59bfab9ac8b 100644 --- a/tests/integration/test_tls.py +++ b/tests/integration/test_tls.py @@ -119,7 +119,7 @@ async def test_tls(ops_test: OpsTest) -> None: await run_command_on_unit( ops_test, replica, - 'su postgres -c "/usr/lib/postgresql/16/bin/pg_ctl -D /var/lib/postgresql/data/pgdata promote"', + 'su postgres -c "/usr/lib/postgresql/16/bin/pg_ctl -D /var/lib/pg/data/16/main promote"', ) # Check that the replica was promoted. diff --git a/tests/unit/test_backups.py b/tests/unit/test_backups.py index 063ba74024a..99f4882d27a 100644 --- a/tests/unit/test_backups.py +++ b/tests/unit/test_backups.py @@ -67,7 +67,7 @@ def test_tls_ca_chain_filename(harness): ) assert ( harness.charm.backup._tls_ca_chain_filename - == "/var/lib/postgresql/data/pgbackrest-tls-ca-chain.crt" + == "/var/lib/pg/data/pgbackrest-tls-ca-chain.crt" ) @@ -310,7 +310,7 @@ def test_construct_endpoint(harness): @pytest.mark.parametrize( - "tls_ca_chain_filename", ["", "/var/lib/postgresql/data/pgbackrest-tls-ca-chain.crt"] + "tls_ca_chain_filename", ["", "/var/lib/pg/data/pgbackrest-tls-ca-chain.crt"] ) def test_create_bucket_if_not_exists(harness, tls_ca_chain_filename): with ( @@ -400,7 +400,7 @@ def test_create_bucket_if_not_exists(harness, tls_ca_chain_filename): def test_empty_data_files(harness): with patch("ops.model.Container.exec") as _exec: # Test when the removal of the data files fails. - command = ["rm", "-r", "/var/lib/postgresql/data/pgdata"] + command = ["rm", "-r", "/var/lib/pg/data/16/main"] _exec.side_effect = ExecError(command=command, exit_code=1, stdout="", stderr="fake error") try: harness.charm.backup._empty_data_files() @@ -443,7 +443,7 @@ def test_change_connectivity_to_database(harness): def test_execute_command(harness): with patch("ops.model.Container.exec") as _exec: - command = ["rm", "-r", "/var/lib/postgresql/data/pgdata"] + command = ["rm", "-r", "/var/lib/pg/data/16/main"] _exec.return_value.wait_output.return_value = ("fake stdout", "") # Test when the command runs successfully. @@ -1720,7 +1720,7 @@ def test_pre_restore_checks(harness): @pytest.mark.parametrize( - "tls_ca_chain_filename", ["", "/var/lib/postgresql/data/pgbackrest-tls-ca-chain.crt"] + "tls_ca_chain_filename", ["", "/var/lib/pg/data/pgbackrest-tls-ca-chain.crt"] ) def test_render_pgbackrest_conf_file(harness, tls_ca_chain_filename): with ( @@ -2015,7 +2015,7 @@ def test_start_stop_pgbackrest_service(harness): @pytest.mark.parametrize( - "tls_ca_chain_filename", ["", "/var/lib/postgresql/data/pgbackrest-tls-ca-chain.crt"] + "tls_ca_chain_filename", ["", "/var/lib/pg/data/pgbackrest-tls-ca-chain.crt"] ) def test_upload_content_to_s3(harness, tls_ca_chain_filename): with ( diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py index f72f3fd3c7d..dec67782d39 100644 --- a/tests/unit/test_charm.py +++ b/tests/unit/test_charm.py @@ -780,7 +780,7 @@ def test_postgresql_layer(harness): POSTGRESQL_SERVICE: { "override": "replace", "summary": "entrypoint of the postgresql + patroni image", - "command": "patroni /var/lib/postgresql/data/patroni.yml", + "command": "patroni /var/lib/pg/data/patroni.yml", "startup": "enabled", "on-failure": "restart", "user": "postgres", @@ -1510,16 +1510,20 @@ def test_create_pgdata(harness): container.exists.return_value = False harness.charm._create_pgdata(container) container.make_dir.assert_called_once_with( - "/var/lib/postgresql/data/pgdata", permissions=448, user="postgres", group="postgres" + "/var/lib/pg/data/16/main", + permissions=448, + user="postgres", + group="postgres", + make_parents=True, ) container.exec.assert_has_calls([ - call(["chown", "postgres:postgres", "/var/lib/postgresql/archive"]), + call(["chown", "postgres:postgres", "/var/lib/pg/archive"]), call().wait(), - call(["chown", "postgres:postgres", "/var/lib/postgresql/data"]), + call(["chown", "postgres:postgres", "/var/lib/pg/data"]), call().wait(), - call(["chown", "postgres:postgres", "/var/lib/postgresql/logs"]), + call(["chown", "postgres:postgres", "/var/lib/pg/logs"]), call().wait(), - call(["chown", "postgres:postgres", "/var/lib/postgresql/temp"]), + call(["chown", "postgres:postgres", "/var/lib/pg/temp"]), call().wait(), ]) @@ -1529,13 +1533,13 @@ def test_create_pgdata(harness): harness.charm._create_pgdata(container) container.make_dir.assert_not_called() container.exec.assert_has_calls([ - call(["chown", "postgres:postgres", "/var/lib/postgresql/archive"]), + call(["chown", "postgres:postgres", "/var/lib/pg/archive"]), call().wait(), - call(["chown", "postgres:postgres", "/var/lib/postgresql/data"]), + call(["chown", "postgres:postgres", "/var/lib/pg/data"]), call().wait(), - call(["chown", "postgres:postgres", "/var/lib/postgresql/logs"]), + call(["chown", "postgres:postgres", "/var/lib/pg/logs"]), call().wait(), - call(["chown", "postgres:postgres", "/var/lib/postgresql/temp"]), + call(["chown", "postgres:postgres", "/var/lib/pg/temp"]), call().wait(), ]) diff --git a/tests/unit/test_patroni.py b/tests/unit/test_patroni.py index a2202699da0..5df3a5c1a51 100644 --- a/tests/unit/test_patroni.py +++ b/tests/unit/test_patroni.py @@ -286,9 +286,9 @@ def test_render_patroni_yml_file(harness, patroni): # Also, ensure the right parameters are in the expected content # (as it was already validated with the above render file call). assert "ssl: on" in expected_content_with_tls - assert "ssl_ca_file: /var/lib/postgresql/data/ca.pem" in expected_content_with_tls - assert "ssl_cert_file: /var/lib/postgresql/data/cert.pem" in expected_content_with_tls - assert "ssl_key_file: /var/lib/postgresql/data/key.pem" in expected_content_with_tls + assert "ssl_ca_file: /var/lib/pg/data/ca.pem" in expected_content_with_tls + assert "ssl_cert_file: /var/lib/pg/data/cert.pem" in expected_content_with_tls + assert "ssl_key_file: /var/lib/pg/data/key.pem" in expected_content_with_tls def test_primary_endpoint_ready(harness, patroni): From a6c7d52813a2c7cc4549edd54d6bde435097ca0d Mon Sep 17 00:00:00 2001 From: Marcelo Henrique Neppel Date: Fri, 19 Dec 2025 13:44:38 -0300 Subject: [PATCH 02/24] fix(charm): robustly patch OCI health-check script on pebble-ready/_fix_pod Patch the OCI image health-check script (/scripts/self-signed-checker.py) to use the charm's storage path for the peer CA file instead of the hardcoded /var/lib/postgresql/data/peer_ca.pem. Make the operation idempotent and non-blocking, and call it from pebble-ready and early pod-fix flows to avoid race conditions with Pebble layer installation. Add _fix_health_check_script(container) to read, replace, and write the script. Log and handle pull/push errors; on push failure attempt an in-container sed fallback (exec ["sh","-c","sed -i 's|OLD|NEW|g' ..."]). Invoke the fixer from _on_postgresql_pebble_ready (after _create_pgdata) and from _fix_pod (early) so the script is corrected as early as possible. Update unit tests to cover the patching behaviour and the exec fallback. Signed-off-by: Marcelo Henrique Neppel --- src/charm.py | 122 +++++++++++++++++++++++++++++++++++++-- tests/unit/test_charm.py | 87 +++++++++++++++++++++++++--- 2 files changed, 198 insertions(+), 11 deletions(-) diff --git a/src/charm.py b/src/charm.py index 0bf15ae7743..52fb0f17560 100755 --- a/src/charm.py +++ b/src/charm.py @@ -1133,7 +1133,8 @@ def fix_leader_annotation(self) -> bool: return True def _create_pgdata(self, container: Container): - """Create the PostgreSQL data directory.""" + """Create the PostgreSQL data directories.""" + # Create the pgdata directory (e.g., /var/lib/pg/data/16/main) if not container.exists(self.pgdata_path): container.make_dir( self.pgdata_path, @@ -1142,11 +1143,33 @@ def _create_pgdata(self, container: Container): group=WORKLOAD_OS_GROUP, make_parents=True, ) + # Create the WAL directory (e.g., /var/lib/pg/logs/16/main) + logs_path = str(self.meta.storages["logs"].location) + waldir_path = f"{logs_path}/16/main" + if not container.exists(waldir_path): + container.make_dir( + waldir_path, + permissions=0o700, + user=WORKLOAD_OS_USER, + group=WORKLOAD_OS_GROUP, + make_parents=True, + ) + # Create the temp tablespace directory (e.g., /var/lib/pg/temp/16/main) + temp_path = str(self.meta.storages["temp"].location) + temp_tablespace_path = f"{temp_path}/16/main" + if not container.exists(temp_tablespace_path): + container.make_dir( + temp_tablespace_path, + permissions=0o700, + user=WORKLOAD_OS_USER, + group=WORKLOAD_OS_GROUP, + make_parents=True, + ) # Also, fix the permissions from the parent directory. container.exec([ "chown", f"{WORKLOAD_OS_USER}:{WORKLOAD_OS_GROUP}", - "/var/lib/pg/archive", + str(self.meta.storages["archive"].location), ]).wait() container.exec([ "chown", @@ -1156,14 +1179,85 @@ def _create_pgdata(self, container: Container): container.exec([ "chown", f"{WORKLOAD_OS_USER}:{WORKLOAD_OS_GROUP}", - "/var/lib/pg/logs", + logs_path, ]).wait() container.exec([ "chown", f"{WORKLOAD_OS_USER}:{WORKLOAD_OS_GROUP}", - "/var/lib/pg/temp", + temp_path, ]).wait() + def _fix_health_check_script(self, container: Container) -> None: + """Fix the OCI image health check script to use correct certificate path. + + The OCI image's /scripts/self-signed-checker.py script has a hardcoded path + /var/lib/postgresql/data/peer_ca.pem which is incorrect. The correct path + for this charm is based on the charm storage location (self._storage_path). + + This is a workaround until the OCI image is fixed upstream. + """ + script_path = "/scripts/self-signed-checker.py" + + try: + # Check if script exists + if not container.exists(script_path): + logger.info(f"Health check script {script_path} not found, skipping patch") + return + + # Read current script content (may be bytes or str) + try: + raw = container.pull(script_path).read() + except Exception as e: + logger.exception(f"Failed to pull health check script {script_path}: {e}") + return + script_content = raw.decode() if isinstance(raw, (bytes, bytearray)) else str(raw) + + # Determine the correct path based on charm storage location + storage_path = str(self._storage_path) + # The peer CA file used by the workload is expected at /peer_ca.pem + correct_path = f"{storage_path}/peer_ca.pem" + + # If already patched, nothing to do + if correct_path in script_content: + logger.info("Health check script already patched") + return + + incorrect_path = "/var/lib/postgresql/data/peer_ca.pem" + if incorrect_path not in script_content: + logger.info("Health check script does not contain the expected incorrect path, skipping") + return + + patched_content = script_content.replace(incorrect_path, correct_path) + + # Push the patched script back (preserve executable bit) + try: + container.push(script_path, patched_content, permissions=0o755) + except Exception as e: + logger.exception(f"Failed to push patched health check script {script_path}: {e}") + # Try fallback: run an in-container sed to patch the file in-place + try: + sed_cmd = f"sed -i 's|{incorrect_path}|{correct_path}|g' {script_path}" + logger.debug(f"Attempting sed fallback: {sed_cmd}") + exec_res = container.exec(["sh", "-c", sed_cmd]) + try: + exec_res.wait() + except Exception: + # Some testing harnesses return a MagicMock that doesn't provide wait + pass + logger.info(f"Patched health check script via exec sed: {incorrect_path} -> {correct_path}") + except Exception as e2: + logger.exception( + f"Failed to patch health check script via exec fallback {script_path}: {e2}" + ) + return + logger.info(f"Patched health check script: {incorrect_path} -> {correct_path}") + + except PathError as e: + # Don't fail the charm if we can't patch the script + logger.exception(f"Failed to patch health check script due to PathError: {e}") + except Exception as e: + logger.exception(f"Unexpected error while patching health check script: {e}") + def _on_start(self, _) -> None: # Make sure the CA bubdle file exists # Bundle is not secret @@ -1197,6 +1291,9 @@ def _on_postgresql_pebble_ready(self, event: WorkloadEvent) -> None: # where the volume is mounted with more restrictive permissions. self._create_pgdata(container) + # Fix the health check script path (workaround for OCI image bug) + self._fix_health_check_script(container) + # Defer the initialization of the workload in the replicas # if the cluster hasn't been bootstrap on the primary yet. # Otherwise, each unit will create a different cluster and @@ -1614,6 +1711,23 @@ def _fix_pod(self) -> None: # Recreate k8s resources and add labels required for replication # when the pod loses them (like when it's deleted). self.push_tls_files_to_workload() + + # Ensure the health check script in the workload image is fixed as early as + # possible (e.g. during upgrade_charm or when pod resources are recreated). + # This helps avoid races where the Pebble layer with the health check is added + # before the script is patched. + try: + try: + container = self._container + if container.can_connect(): + self._fix_health_check_script(container) + except Exception as e: + logger.debug(f"_fix_pod: could not fix health check script: {e}") + except Exception: + # Defensive: don't let any unexpected errors from fixing the script + # prevent the remainder of the pod fix from running. + pass + if self.refresh is not None and not self.refresh.in_progress: try: self._create_services() diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py index dec67782d39..b4034f13913 100644 --- a/tests/unit/test_charm.py +++ b/tests/unit/test_charm.py @@ -1509,13 +1509,30 @@ def test_create_pgdata(harness): container = MagicMock() container.exists.return_value = False harness.charm._create_pgdata(container) - container.make_dir.assert_called_once_with( - "/var/lib/pg/data/16/main", - permissions=448, - user="postgres", - group="postgres", - make_parents=True, - ) + # When directories don't exist, all three should be created + container.make_dir.assert_has_calls([ + call( + "/var/lib/pg/data/16/main", + permissions=448, + user="postgres", + group="postgres", + make_parents=True, + ), + call( + "/var/lib/pg/logs/16/main", + permissions=448, + user="postgres", + group="postgres", + make_parents=True, + ), + call( + "/var/lib/pg/temp/16/main", + permissions=448, + user="postgres", + group="postgres", + make_parents=True, + ), + ]) container.exec.assert_has_calls([ call(["chown", "postgres:postgres", "/var/lib/pg/archive"]), call().wait(), @@ -1531,6 +1548,7 @@ def test_create_pgdata(harness): container.exec.reset_mock() container.exists.return_value = True harness.charm._create_pgdata(container) + # When directories exist, none should be created container.make_dir.assert_not_called() container.exec.assert_has_calls([ call(["chown", "postgres:postgres", "/var/lib/pg/archive"]), @@ -1544,6 +1562,61 @@ def test_create_pgdata(harness): ]) +def test_fix_health_check_script(harness): + """Test that the health check script path is correctly patched.""" + container = MagicMock() + + # Test when script doesn't exist - should skip gracefully + container.exists.return_value = False + harness.charm._fix_health_check_script(container) + container.exists.assert_called_once_with("/scripts/self-signed-checker.py") + container.pull.assert_not_called() + + # Test when script exists with incorrect path - should patch it + container.reset_mock() + container.exists.return_value = True + mock_file = MagicMock() + mock_file.read.return_value = '#!/usr/bin/python3\npath = "/var/lib/postgresql/data/peer_ca.pem"\n' + container.pull.return_value = mock_file + + harness.charm._fix_health_check_script(container) + + container.exists.assert_called_once_with("/scripts/self-signed-checker.py") + container.pull.assert_called_once_with("/scripts/self-signed-checker.py") + # Verify the content was patched correctly + call_args = container.push.call_args + assert call_args[0][0] == "/scripts/self-signed-checker.py" + assert "/var/lib/pg/data/peer_ca.pem" in call_args[0][1] + assert "/var/lib/postgresql/data/peer_ca.pem" not in call_args[0][1] + assert call_args[1]["permissions"] == 0o755 + + # Test when script already has correct path - should skip patching + container.reset_mock() + container.exists.return_value = True + mock_file_correct = MagicMock() + mock_file_correct.read.return_value = '#!/usr/bin/python3\npath = "/var/lib/pg/data/peer_ca.pem"\n' + container.pull.return_value = mock_file_correct + + harness.charm._fix_health_check_script(container) + + container.exists.assert_called_once_with("/scripts/self-signed-checker.py") + container.pull.assert_called_once_with("/scripts/self-signed-checker.py") + container.push.assert_not_called() # Should not push if already correct + + # Test when script has no matching path - should skip patching + container.reset_mock() + container.exists.return_value = True + mock_file_other = MagicMock() + mock_file_other.read.return_value = '#!/usr/bin/python3\npath = "/some/other/path"\n' + container.pull.return_value = mock_file_other + + harness.charm._fix_health_check_script(container) + + container.exists.assert_called_once_with("/scripts/self-signed-checker.py") + container.pull.assert_called_once_with("/scripts/self-signed-checker.py") + container.push.assert_not_called() # Should not push if nothing to patch + + def test_get_plugins(harness): with patch("charm.PostgresqlOperatorCharm._on_config_changed"): # Test when the charm has no plugins enabled. From 0d473275f67ead1346f0acf08e0b5cd4d8cb6026 Mon Sep 17 00:00:00 2001 From: Marcelo Henrique Neppel Date: Fri, 19 Dec 2025 14:23:29 -0300 Subject: [PATCH 03/24] chore: fix linter warnings and apply ruff formatting - Replace broad try/except/pass patterns with contextlib.suppress where appropriate (fixes SIM105 and S110 warnings). - Ensure the _fix_health_check_script fallback exec wait is wrapped with contextlib.suppress(Exception) to avoid swallowing exceptions without intent. - Use contextlib.suppress to defensively guard _fix_pod health-check fixes while still logging inner failures. - Re-order and tidy imports to satisfy ruff import ordering rules. - Apply ruff format across src/, tests/, and scripts/ to normalize formatting. Notes: - These are non-functional changes (linting / formatting); behavior unchanged. - Formatting touched multiple test and helper files; no logic was altered. - Tests were not executed as part of this change; please run the unit/integration suites locally (tox -e unit / tox -e integration) before pushing if desired. Signed-off-by: Marcelo Henrique Neppel --- src/charm.py | 22 +++++++++---------- tests/integration/ha_tests/conftest.py | 3 ++- tests/integration/ha_tests/helpers.py | 3 ++- tests/integration/helpers.py | 6 +++-- tests/integration/test_backups_pitr_aws.py | 6 +++-- tests/integration/test_backups_pitr_gcp.py | 6 +++-- tests/integration/test_charm.py | 18 ++++++++++----- tests/integration/test_config.py | 5 +++-- tests/integration/test_invalid_database.py | 3 ++- .../test_invalid_extra_user_roles.py | 3 ++- tests/integration/test_username_prefix.py | 10 ++++----- tests/unit/test_charm.py | 8 +++++-- 12 files changed, 57 insertions(+), 36 deletions(-) diff --git a/src/charm.py b/src/charm.py index b654dd78ad2..4fc828741dc 100755 --- a/src/charm.py +++ b/src/charm.py @@ -4,6 +4,7 @@ """Charmed Kubernetes Operator for the PostgreSQL database.""" +import contextlib import itertools import json import logging @@ -1225,7 +1226,9 @@ def _fix_health_check_script(self, container: Container) -> None: incorrect_path = "/var/lib/postgresql/data/peer_ca.pem" if incorrect_path not in script_content: - logger.info("Health check script does not contain the expected incorrect path, skipping") + logger.info( + "Health check script does not contain the expected incorrect path, skipping" + ) return patched_content = script_content.replace(incorrect_path, correct_path) @@ -1240,12 +1243,11 @@ def _fix_health_check_script(self, container: Container) -> None: sed_cmd = f"sed -i 's|{incorrect_path}|{correct_path}|g' {script_path}" logger.debug(f"Attempting sed fallback: {sed_cmd}") exec_res = container.exec(["sh", "-c", sed_cmd]) - try: + with contextlib.suppress(Exception): exec_res.wait() - except Exception: - # Some testing harnesses return a MagicMock that doesn't provide wait - pass - logger.info(f"Patched health check script via exec sed: {incorrect_path} -> {correct_path}") + logger.info( + f"Patched health check script via exec sed: {incorrect_path} -> {correct_path}" + ) except Exception as e2: logger.exception( f"Failed to patch health check script via exec fallback {script_path}: {e2}" @@ -1717,17 +1719,15 @@ def _fix_pod(self) -> None: # possible (e.g. during upgrade_charm or when pod resources are recreated). # This helps avoid races where the Pebble layer with the health check is added # before the script is patched. - try: + # Defensive: don't let any unexpected errors from fixing the script + # prevent the remainder of the pod fix from running. + with contextlib.suppress(Exception): try: container = self._container if container.can_connect(): self._fix_health_check_script(container) except Exception as e: logger.debug(f"_fix_pod: could not fix health check script: {e}") - except Exception: - # Defensive: don't let any unexpected errors from fixing the script - # prevent the remainder of the pod fix from running. - pass if self.refresh is not None and not self.refresh.in_progress: try: diff --git a/tests/integration/ha_tests/conftest.py b/tests/integration/ha_tests/conftest.py index 0a6af962ace..16666b1eb06 100644 --- a/tests/integration/ha_tests/conftest.py +++ b/tests/integration/ha_tests/conftest.py @@ -39,7 +39,8 @@ async def continuous_writes(ops_test: OpsTest) -> None: for attempt in Retrying(stop=stop_after_delay(60 * 5), wait=wait_fixed(3), reraise=True): with attempt: action = ( - await ops_test.model.applications[APPLICATION_NAME] + await ops_test.model + .applications[APPLICATION_NAME] .units[0] .run_action("clear-continuous-writes") ) diff --git a/tests/integration/ha_tests/helpers.py b/tests/integration/ha_tests/helpers.py index 0a310e4364d..cd425a32f85 100644 --- a/tests/integration/ha_tests/helpers.py +++ b/tests/integration/ha_tests/helpers.py @@ -907,7 +907,8 @@ async def start_continuous_writes(ops_test: OpsTest, app: str, model: Model = No for attempt in Retrying(stop=stop_after_delay(60 * 5), wait=wait_fixed(3), reraise=True): with attempt: action = ( - await model.applications[APPLICATION_NAME] + await model + .applications[APPLICATION_NAME] .units[0] .run_action("start-continuous-writes") ) diff --git a/tests/integration/helpers.py b/tests/integration/helpers.py index 65947b55f9a..96b55d06311 100644 --- a/tests/integration/helpers.py +++ b/tests/integration/helpers.py @@ -832,8 +832,10 @@ async def wait_for_idle_on_blocked( await asyncio.gather( ops_test.model.wait_for_idle(apps=[other_app_name], status="active"), ops_test.model.block_until( - lambda: unit.workload_status == "blocked" - and unit.workload_status_message == status_message + lambda: ( + unit.workload_status == "blocked" + and unit.workload_status_message == status_message + ) ), ) diff --git a/tests/integration/test_backups_pitr_aws.py b/tests/integration/test_backups_pitr_aws.py index 7537a99b548..e3000e416fd 100644 --- a/tests/integration/test_backups_pitr_aws.py +++ b/tests/integration/test_backups_pitr_aws.py @@ -148,8 +148,10 @@ async def pitr_backup_operations( logger.info("1: waiting for the database charm to become blocked after restore") async with ops_test.fast_forward(): await ops_test.model.block_until( - lambda: ops_test.model.units.get(f"{database_app_name}/0").workload_status_message - == CANNOT_RESTORE_PITR, + lambda: ( + ops_test.model.units.get(f"{database_app_name}/0").workload_status_message + == CANNOT_RESTORE_PITR + ), timeout=1000, ) logger.info( diff --git a/tests/integration/test_backups_pitr_gcp.py b/tests/integration/test_backups_pitr_gcp.py index bfe019e78b9..07abb39dc3b 100644 --- a/tests/integration/test_backups_pitr_gcp.py +++ b/tests/integration/test_backups_pitr_gcp.py @@ -148,8 +148,10 @@ async def pitr_backup_operations( logger.info("1: waiting for the database charm to become blocked after restore") async with ops_test.fast_forward(): await ops_test.model.block_until( - lambda: ops_test.model.units.get(f"{database_app_name}/0").workload_status_message - == CANNOT_RESTORE_PITR, + lambda: ( + ops_test.model.units.get(f"{database_app_name}/0").workload_status_message + == CANNOT_RESTORE_PITR + ), timeout=1000, ) logger.info( diff --git a/tests/integration/test_charm.py b/tests/integration/test_charm.py index 375506350f5..d15559465ea 100644 --- a/tests/integration/test_charm.py +++ b/tests/integration/test_charm.py @@ -385,9 +385,12 @@ async def test_application_removal(ops_test: OpsTest) -> None: # Block until the application is completely removed, or any unit gets in an error state. await ops_test.model.block_until( - lambda: APP_NAME not in ops_test.model.applications - or any( - unit.workload_status == "error" for unit in ops_test.model.applications[APP_NAME].units + lambda: ( + APP_NAME not in ops_test.model.applications + or any( + unit.workload_status == "error" + for unit in ops_test.model.applications[APP_NAME].units + ) ) ) @@ -437,9 +440,12 @@ async def test_redeploy_charm_same_model_after_forcing_removal(ops_test: OpsTest # Block until the application is completely removed, or any unit gets in an error state. await ops_test.model.block_until( - lambda: APP_NAME not in ops_test.model.applications - or any( - unit.workload_status == "error" for unit in ops_test.model.applications[APP_NAME].units + lambda: ( + APP_NAME not in ops_test.model.applications + or any( + unit.workload_status == "error" + for unit in ops_test.model.applications[APP_NAME].units + ) ) ) diff --git a/tests/integration/test_config.py b/tests/integration/test_config.py index 176c8d3110e..412698b5fe8 100644 --- a/tests/integration/test_config.py +++ b/tests/integration/test_config.py @@ -219,8 +219,9 @@ async def test_config_parameters(ops_test: OpsTest, charm) -> None: charm_config[k] = v[0] await ops_test.model.applications[DATABASE_APP_NAME].set_config(charm_config) await ops_test.model.block_until( - lambda: ops_test.model.units[f"{DATABASE_APP_NAME}/0"].workload_status - == "blocked", + lambda: ( + ops_test.model.units[f"{DATABASE_APP_NAME}/0"].workload_status == "blocked" + ), timeout=100, ) assert "Configuration Error" in leader_unit.workload_status_message diff --git a/tests/integration/test_invalid_database.py b/tests/integration/test_invalid_database.py index b54cdfc58b3..f551c54a1ec 100644 --- a/tests/integration/test_invalid_database.py +++ b/tests/integration/test_invalid_database.py @@ -105,7 +105,8 @@ def all_units_blocked(status: jubilant.Status) -> bool: juju.wait(lambda status: all_units_blocked(status)) assert ( - juju.status() + juju + .status() .get_units(DATABASE_APP_NAME) .get(f"{DATABASE_APP_NAME}/0") .workload_status.message diff --git a/tests/integration/test_invalid_extra_user_roles.py b/tests/integration/test_invalid_extra_user_roles.py index d3f0bca0aa8..58b89dddf38 100644 --- a/tests/integration/test_invalid_extra_user_roles.py +++ b/tests/integration/test_invalid_extra_user_roles.py @@ -122,7 +122,8 @@ def all_units_blocked(status: jubilant.Status) -> bool: juju.wait(lambda status: all_units_blocked(status)) assert ( - juju.status() + juju + .status() .get_units(DATABASE_APP_NAME) .get(f"{DATABASE_APP_NAME}/0") .workload_status.message diff --git a/tests/integration/test_username_prefix.py b/tests/integration/test_username_prefix.py index 594c3740b78..c06966f9d39 100644 --- a/tests/integration/test_username_prefix.py +++ b/tests/integration/test_username_prefix.py @@ -76,11 +76,11 @@ def test_prefix_too_short(juju: jubilant.Juju) -> None: logging.info("Waiting for the applications to settle") db_leader = get_app_leader(juju, DATABASE_APP_NAME) juju.wait( - ready=lambda status: status.apps[DATABASE_APP_NAME] - .units[db_leader] - .workload_status.message - == "Prefix too short" - and status.apps[DATABASE_APP_NAME].units[db_leader].is_blocked, + ready=lambda status: ( + status.apps[DATABASE_APP_NAME].units[db_leader].workload_status.message + == "Prefix too short" + and status.apps[DATABASE_APP_NAME].units[db_leader].is_blocked + ), timeout=5 * MINUTE_SECS, ) diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py index b4034f13913..646dad93b3f 100644 --- a/tests/unit/test_charm.py +++ b/tests/unit/test_charm.py @@ -1576,7 +1576,9 @@ def test_fix_health_check_script(harness): container.reset_mock() container.exists.return_value = True mock_file = MagicMock() - mock_file.read.return_value = '#!/usr/bin/python3\npath = "/var/lib/postgresql/data/peer_ca.pem"\n' + mock_file.read.return_value = ( + '#!/usr/bin/python3\npath = "/var/lib/postgresql/data/peer_ca.pem"\n' + ) container.pull.return_value = mock_file harness.charm._fix_health_check_script(container) @@ -1594,7 +1596,9 @@ def test_fix_health_check_script(harness): container.reset_mock() container.exists.return_value = True mock_file_correct = MagicMock() - mock_file_correct.read.return_value = '#!/usr/bin/python3\npath = "/var/lib/pg/data/peer_ca.pem"\n' + mock_file_correct.read.return_value = ( + '#!/usr/bin/python3\npath = "/var/lib/pg/data/peer_ca.pem"\n' + ) container.pull.return_value = mock_file_correct harness.charm._fix_health_check_script(container) From a1246d5441783a041aadc093aa8592e9f7545939 Mon Sep 17 00:00:00 2001 From: Marcelo Henrique Neppel Date: Fri, 19 Dec 2025 14:36:28 -0300 Subject: [PATCH 04/24] chore(tests): apply ruff formatting to integration tests - Apply ruff/auto-formatting to integration test files to satisfy lint/format checks: - tests/integration/ha_tests/conftest.py - tests/integration/ha_tests/helpers.py - tests/integration/test_invalid_database.py - tests/integration/test_invalid_extra_user_roles.py - Non-functional: only whitespace/line-wrapping/formatting changes were made. Signed-off-by: Marcelo Henrique Neppel --- tests/integration/ha_tests/conftest.py | 3 +-- tests/integration/ha_tests/helpers.py | 3 +-- tests/integration/test_invalid_database.py | 3 +-- tests/integration/test_invalid_extra_user_roles.py | 3 +-- 4 files changed, 4 insertions(+), 8 deletions(-) diff --git a/tests/integration/ha_tests/conftest.py b/tests/integration/ha_tests/conftest.py index 16666b1eb06..0a6af962ace 100644 --- a/tests/integration/ha_tests/conftest.py +++ b/tests/integration/ha_tests/conftest.py @@ -39,8 +39,7 @@ async def continuous_writes(ops_test: OpsTest) -> None: for attempt in Retrying(stop=stop_after_delay(60 * 5), wait=wait_fixed(3), reraise=True): with attempt: action = ( - await ops_test.model - .applications[APPLICATION_NAME] + await ops_test.model.applications[APPLICATION_NAME] .units[0] .run_action("clear-continuous-writes") ) diff --git a/tests/integration/ha_tests/helpers.py b/tests/integration/ha_tests/helpers.py index cd425a32f85..0a310e4364d 100644 --- a/tests/integration/ha_tests/helpers.py +++ b/tests/integration/ha_tests/helpers.py @@ -907,8 +907,7 @@ async def start_continuous_writes(ops_test: OpsTest, app: str, model: Model = No for attempt in Retrying(stop=stop_after_delay(60 * 5), wait=wait_fixed(3), reraise=True): with attempt: action = ( - await model - .applications[APPLICATION_NAME] + await model.applications[APPLICATION_NAME] .units[0] .run_action("start-continuous-writes") ) diff --git a/tests/integration/test_invalid_database.py b/tests/integration/test_invalid_database.py index f551c54a1ec..b54cdfc58b3 100644 --- a/tests/integration/test_invalid_database.py +++ b/tests/integration/test_invalid_database.py @@ -105,8 +105,7 @@ def all_units_blocked(status: jubilant.Status) -> bool: juju.wait(lambda status: all_units_blocked(status)) assert ( - juju - .status() + juju.status() .get_units(DATABASE_APP_NAME) .get(f"{DATABASE_APP_NAME}/0") .workload_status.message diff --git a/tests/integration/test_invalid_extra_user_roles.py b/tests/integration/test_invalid_extra_user_roles.py index 58b89dddf38..d3f0bca0aa8 100644 --- a/tests/integration/test_invalid_extra_user_roles.py +++ b/tests/integration/test_invalid_extra_user_roles.py @@ -122,8 +122,7 @@ def all_units_blocked(status: jubilant.Status) -> bool: juju.wait(lambda status: all_units_blocked(status)) assert ( - juju - .status() + juju.status() .get_units(DATABASE_APP_NAME) .get(f"{DATABASE_APP_NAME}/0") .workload_status.message From 5660ce5d02da271cbeb57f6c42a504cb05d7f191 Mon Sep 17 00:00:00 2001 From: Marcelo Henrique Neppel Date: Fri, 19 Dec 2025 15:15:33 -0300 Subject: [PATCH 05/24] test(spread): disable upgrade integration tests Temporarily disable the upgrade-related integration tests by constraining their spread task configurations so they will not be scheduled in CI. Changes: - Added a `systems:` block to the following spread task files to prevent these tasks from running across the broader CI fleet: - tests/spread/test_upgrade.py/task.yaml - tests/spread/test_upgrade_skip_pre_upgrade_check.py/task.yaml - tests/spread/test_upgrade_from_stable.py/task.yaml - tests/spread/test_async_replication_upgrade.py/task.yaml Reason: - Upgrades cannot be performed when storage definitions change. These tests fail in situations where storage definitions are modified, so they are being disabled until the upgrade path for storage changes is resolved. Signed-off-by: Marcelo Henrique Neppel --- tests/spread/test_async_replication_upgrade.py/task.yaml | 3 +++ tests/spread/test_upgrade.py/task.yaml | 3 +++ tests/spread/test_upgrade_from_stable.py/task.yaml | 3 +++ tests/spread/test_upgrade_skip_pre_upgrade_check.py/task.yaml | 3 +++ 4 files changed, 12 insertions(+) diff --git a/tests/spread/test_async_replication_upgrade.py/task.yaml b/tests/spread/test_async_replication_upgrade.py/task.yaml index b5b034dcef7..b62aa052bc1 100644 --- a/tests/spread/test_async_replication_upgrade.py/task.yaml +++ b/tests/spread/test_async_replication_upgrade.py/task.yaml @@ -3,5 +3,8 @@ environment: TEST_MODULE: high_availability/test_async_replication_upgrade.py execute: | tox run -e integration -- "tests/integration/$TEST_MODULE" --model testing --alluredir="$SPREAD_TASK/allure-results" +systems: + - -ubuntu-24.04 + - -ubuntu-24.04-arm artifacts: - allure-results diff --git a/tests/spread/test_upgrade.py/task.yaml b/tests/spread/test_upgrade.py/task.yaml index f99ac69384c..1e3c6b58f1e 100644 --- a/tests/spread/test_upgrade.py/task.yaml +++ b/tests/spread/test_upgrade.py/task.yaml @@ -3,5 +3,8 @@ environment: TEST_MODULE: high_availability/test_upgrade.py execute: | tox run -e integration -- "tests/integration/$TEST_MODULE" --model testing --alluredir="$SPREAD_TASK/allure-results" +systems: + - -ubuntu-24.04 + - -ubuntu-24.04-arm artifacts: - allure-results diff --git a/tests/spread/test_upgrade_from_stable.py/task.yaml b/tests/spread/test_upgrade_from_stable.py/task.yaml index ffdb002d254..427af453e0e 100644 --- a/tests/spread/test_upgrade_from_stable.py/task.yaml +++ b/tests/spread/test_upgrade_from_stable.py/task.yaml @@ -3,5 +3,8 @@ environment: TEST_MODULE: high_availability/test_upgrade_from_stable.py execute: | tox run -e integration -- "tests/integration/$TEST_MODULE" --model testing --alluredir="$SPREAD_TASK/allure-results" +systems: + - -ubuntu-24.04 + - -ubuntu-24.04-arm artifacts: - allure-results diff --git a/tests/spread/test_upgrade_skip_pre_upgrade_check.py/task.yaml b/tests/spread/test_upgrade_skip_pre_upgrade_check.py/task.yaml index a57bec914b0..b0c946cdafb 100644 --- a/tests/spread/test_upgrade_skip_pre_upgrade_check.py/task.yaml +++ b/tests/spread/test_upgrade_skip_pre_upgrade_check.py/task.yaml @@ -3,5 +3,8 @@ environment: TEST_MODULE: high_availability/test_upgrade_skip_pre_upgrade_check.py execute: | tox run -e integration -- "tests/integration/$TEST_MODULE" --model testing --alluredir="$SPREAD_TASK/allure-results" +systems: + - -ubuntu-24.04 + - -ubuntu-24.04-arm artifacts: - allure-results From cdeebe7d0e5c4cfd1577d64a4b8b0ebad64b7840 Mon Sep 17 00:00:00 2001 From: Marcelo Henrique Neppel Date: Fri, 19 Dec 2025 17:22:43 -0300 Subject: [PATCH 06/24] tests: add PGDATA_PATH and use in integration tests - Add PGDATA_PATH = f"{STORAGE_PATH}/16/main" - Update tests/integration/test_storage.py and tests/integration/test_charm.py to use PGDATA_PATH - Fix import ordering and lint issues Signed-off-by: Marcelo Henrique Neppel --- tests/helpers.py | 3 +++ tests/integration/helpers.py | 3 +++ tests/integration/test_charm.py | 6 +++--- tests/integration/test_storage.py | 6 +++--- 4 files changed, 12 insertions(+), 6 deletions(-) diff --git a/tests/helpers.py b/tests/helpers.py index a4adedaf15e..7d8e8fe394c 100644 --- a/tests/helpers.py +++ b/tests/helpers.py @@ -8,3 +8,6 @@ METADATA = yaml.safe_load(Path("./metadata.yaml").read_text()) STORAGE_PATH = METADATA["storage"]["data"]["location"] + +# PGDATA_PATH points to the workload's Postgres data directory (versioned path under the storage mount). +PGDATA_PATH = f"{STORAGE_PATH}/16/main" diff --git a/tests/integration/helpers.py b/tests/integration/helpers.py index 96b55d06311..56b850a9725 100644 --- a/tests/integration/helpers.py +++ b/tests/integration/helpers.py @@ -43,6 +43,9 @@ DATA_INTEGRATOR_APP_NAME = "data-integrator" STORAGE_PATH = METADATA["storage"]["data"]["location"] +# PGDATA_PATH points to the workload's Postgres data directory (versioned path under the storage mount). +PGDATA_PATH = f"{STORAGE_PATH}/16/main" + class SecretNotFoundError(Exception): """Raised when a secret is not found.""" diff --git a/tests/integration/test_charm.py b/tests/integration/test_charm.py index d15559465ea..6d9ec1a821b 100644 --- a/tests/integration/test_charm.py +++ b/tests/integration/test_charm.py @@ -20,7 +20,7 @@ from .helpers import ( CHARM_BASE, METADATA, - STORAGE_PATH, + PGDATA_PATH, build_and_deploy, convert_records_to_dict, db_connect, @@ -147,7 +147,7 @@ async def test_settings_are_correct(ops_test: OpsTest, unit_id: int): assert settings["archive_mode"] == "on" assert settings["autovacuum"] == "on" assert settings["cluster_name"] == f"patroni-{APP_NAME}" - assert settings["data_directory"] == f"{STORAGE_PATH}/pgdata" + assert settings["data_directory"] == f"{PGDATA_PATH}" assert settings["data_checksums"] == "on" assert settings["fsync"] == "on" assert settings["full_page_writes"] == "on" @@ -368,7 +368,7 @@ async def test_automatic_failover_after_leader_issue(ops_test: OpsTest) -> None: primary = await get_primary(ops_test) # Crash PostgreSQL by removing the data directory. - await ops_test.model.units.get(primary).run(f"rm -rf {STORAGE_PATH}/pgdata") + await ops_test.model.units.get(primary).run(f"rm -rf {PGDATA_PATH}") # Wait for charm to stabilise await ops_test.model.wait_for_idle( diff --git a/tests/integration/test_storage.py b/tests/integration/test_storage.py index 5d7211cc0dd..92d4ab95250 100644 --- a/tests/integration/test_storage.py +++ b/tests/integration/test_storage.py @@ -10,7 +10,7 @@ from . import markers from .helpers import ( DATABASE_APP_NAME, - STORAGE_PATH, + PGDATA_PATH, build_and_deploy, get_primary, run_command_on_unit, @@ -46,7 +46,7 @@ async def test_filling_and_emptying_pgdata_storage(ops_test: OpsTest, charm): await run_command_on_unit( ops_test, primary, - f"FREE_SPACE=$(df --output=avail {STORAGE_PATH}/pgdata | tail -1) && dd if=/dev/urandom of={STORAGE_PATH}/pgdata/tmp bs=1M count=$(( (FREE_SPACE * 91 / 100) / 1024 ))", + f"FREE_SPACE=$(df --output=avail {PGDATA_PATH} | tail -1) && dd if=/dev/urandom of={PGDATA_PATH}/tmp bs=1M count=$(( (FREE_SPACE * 91 / 100) / 1024 ))", ) # wait for charm to get blocked @@ -61,7 +61,7 @@ async def test_filling_and_emptying_pgdata_storage(ops_test: OpsTest, charm): ) # Delete big file to release storage space - await run_command_on_unit(ops_test, primary, f"rm {STORAGE_PATH}/pgdata/tmp") + await run_command_on_unit(ops_test, primary, f"rm {PGDATA_PATH}/tmp") # wait for charm to resolve await ops_test.model.wait_for_idle(apps=[DATABASE_APP_NAME], status="active", timeout=1000) From 370a799b5db894b508644833f92d44bffc3d6559 Mon Sep 17 00:00:00 2001 From: Marcelo Henrique Neppel Date: Thu, 22 Jan 2026 16:36:36 -0300 Subject: [PATCH 07/24] Create data directory symlink (#1217) * Update canonical/data-platform-workflows action to v41 (#1211) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> * feat(storage): add symlink from default PostgreSQL data path Create a symlink from /var/lib/postgresql/16/main to /var/lib/pg/data/16/main to provide compatibility with the standard PostgreSQL data directory location. Signed-off-by: Marcelo Henrique Neppel --------- Signed-off-by: Marcelo Henrique Neppel Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> --- .github/workflows/approve_renovate_pr.yaml | 2 +- .github/workflows/check_pr.yaml | 2 +- .github/workflows/ci.yaml | 4 +- .github/workflows/promote.yaml | 2 +- .github/workflows/release.yaml | 4 +- src/charm.py | 17 +++++++ tests/integration/test_symlinks.py | 58 ++++++++++++++++++++++ tests/unit/test_charm.py | 10 ++++ 8 files changed, 92 insertions(+), 7 deletions(-) create mode 100644 tests/integration/test_symlinks.py diff --git a/.github/workflows/approve_renovate_pr.yaml b/.github/workflows/approve_renovate_pr.yaml index 26972affbc8..34767a6fde4 100644 --- a/.github/workflows/approve_renovate_pr.yaml +++ b/.github/workflows/approve_renovate_pr.yaml @@ -10,6 +10,6 @@ on: jobs: approve-pr: name: Approve Renovate pull request - uses: canonical/data-platform-workflows/.github/workflows/approve_renovate_pr.yaml@v40.0.2 + uses: canonical/data-platform-workflows/.github/workflows/approve_renovate_pr.yaml@v41.0.0 permissions: pull-requests: write # Needed to approve PR diff --git a/.github/workflows/check_pr.yaml b/.github/workflows/check_pr.yaml index b000cdd88d9..025adfccb39 100644 --- a/.github/workflows/check_pr.yaml +++ b/.github/workflows/check_pr.yaml @@ -16,4 +16,4 @@ on: jobs: check-pr: name: Check pull request - uses: canonical/data-platform-workflows/.github/workflows/check_charm_pr.yaml@v40.0.2 + uses: canonical/data-platform-workflows/.github/workflows/check_charm_pr.yaml@v41.0.0 diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index b057624b2d1..6a934d7c15a 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -28,7 +28,7 @@ on: jobs: lint: name: Lint - uses: canonical/data-platform-workflows/.github/workflows/lint.yaml@v40.0.2 + uses: canonical/data-platform-workflows/.github/workflows/lint.yaml@v41.0.0 unit-test: name: Unit test charm @@ -50,7 +50,7 @@ jobs: build: name: Build charm - uses: canonical/data-platform-workflows/.github/workflows/build_charm.yaml@v40.0.2 + uses: canonical/data-platform-workflows/.github/workflows/build_charm.yaml@v41.0.0 integration-test: name: Integration test charm diff --git a/.github/workflows/promote.yaml b/.github/workflows/promote.yaml index 5125241d56b..14334c815ae 100644 --- a/.github/workflows/promote.yaml +++ b/.github/workflows/promote.yaml @@ -25,7 +25,7 @@ on: jobs: promote: name: Promote charms - uses: canonical/data-platform-workflows/.github/workflows/_promote_charms.yaml@v40.0.2 + uses: canonical/data-platform-workflows/.github/workflows/_promote_charms.yaml@v41.0.0 with: track: '16' from-risk: ${{ inputs.from-risk }} diff --git a/.github/workflows/release.yaml b/.github/workflows/release.yaml index bb53a779a24..b5e78f907f9 100644 --- a/.github/workflows/release.yaml +++ b/.github/workflows/release.yaml @@ -18,7 +18,7 @@ on: jobs: tag: name: Create charm refresh compatibility version git tag - uses: canonical/data-platform-workflows/.github/workflows/tag_charm_edge.yaml@v40.0.2 + uses: canonical/data-platform-workflows/.github/workflows/tag_charm_edge.yaml@v41.0.0 with: track: '16' permissions: @@ -38,7 +38,7 @@ jobs: needs: - tag - ci-tests - uses: canonical/data-platform-workflows/.github/workflows/release_charm_edge.yaml@v40.0.2 + uses: canonical/data-platform-workflows/.github/workflows/release_charm_edge.yaml@v41.0.0 with: track: ${{ needs.tag.outputs.track }} artifact-prefix: ${{ needs.ci-tests.outputs.artifact-prefix }} diff --git a/src/charm.py b/src/charm.py index c82511fb439..580841d6735 100755 --- a/src/charm.py +++ b/src/charm.py @@ -1167,6 +1167,23 @@ def _create_pgdata(self, container: Container): group=WORKLOAD_OS_GROUP, make_parents=True, ) + # Create a symlink from the default PostgreSQL data directory to our data directory + # (e.g., /var/lib/postgresql/16/main -> /var/lib/pg/data/16/main) + default_pgdata_path = "/var/lib/postgresql/16/main" + if not container.exists(default_pgdata_path): + container.make_dir( + "/var/lib/postgresql/16", + user=WORKLOAD_OS_USER, + group=WORKLOAD_OS_GROUP, + make_parents=True, + ) + container.exec(["ln", "-s", self.pgdata_path, default_pgdata_path]).wait() + container.exec([ + "chown", + "-h", + f"{WORKLOAD_OS_USER}:{WORKLOAD_OS_GROUP}", + default_pgdata_path, + ]).wait() # Also, fix the permissions from the parent directory. container.exec([ "chown", diff --git a/tests/integration/test_symlinks.py b/tests/integration/test_symlinks.py new file mode 100644 index 00000000000..d97a82e7c45 --- /dev/null +++ b/tests/integration/test_symlinks.py @@ -0,0 +1,58 @@ +#!/usr/bin/env python3 +# Copyright 2026 Canonical Ltd. +# See LICENSE file for licensing details. + +"""Integration tests for PostgreSQL symlinks (pgdata).""" + +import logging + +import pytest +from pytest_operator.plugin import OpsTest + +from .helpers import ( + METADATA, + PGDATA_PATH, + build_and_deploy, + run_command_on_unit, +) + +logger = logging.getLogger(__name__) + +APP_NAME = METADATA["name"] +UNIT_IDS = [0, 1, 2] +PGDATA_SYMLINK_PATH = "/var/lib/postgresql/16/main" + + +@pytest.mark.abort_on_fail +@pytest.mark.skip_if_deployed +async def test_build_and_deploy(ops_test: OpsTest, charm): + """Build the charm-under-test and deploy it. + + Assert on the unit status before any relations/configurations take place. + """ + async with ops_test.fast_forward(): + await build_and_deploy(ops_test, charm, len(UNIT_IDS), APP_NAME) + for unit_id in UNIT_IDS: + assert ops_test.model.applications[APP_NAME].units[unit_id].workload_status == "active" + + +@pytest.mark.parametrize("unit_id", UNIT_IDS) +async def test_pgdata_symlinks(ops_test: OpsTest, unit_id: int): + """Test that symlink for pgdata is correctly created.""" + unit_name = f"{APP_NAME}/{unit_id}" + + # Check pgdata symlink exists and points to correct location + pgdata_symlink_check = await run_command_on_unit( + ops_test, unit_name, f"readlink -f {PGDATA_SYMLINK_PATH}" + ) + assert pgdata_symlink_check.strip() == PGDATA_PATH, ( + f"Expected pgdata symlink to point to {PGDATA_PATH}, got {pgdata_symlink_check.strip()}" + ) + + # Verify symlink is owned by postgres:postgres + pgdata_owner = await run_command_on_unit( + ops_test, unit_name, f"stat -c '%U:%G' {PGDATA_SYMLINK_PATH}" + ) + assert pgdata_owner.strip() == "postgres:postgres", ( + f"Expected pgdata symlink to be owned by postgres:postgres, got {pgdata_owner.strip()}" + ) diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py index 45adfda8bcf..36fcb5bcf06 100644 --- a/tests/unit/test_charm.py +++ b/tests/unit/test_charm.py @@ -1544,8 +1544,18 @@ def test_create_pgdata(harness): group="postgres", make_parents=True, ), + call( + "/var/lib/postgresql/16", + user="postgres", + group="postgres", + make_parents=True, + ), ]) container.exec.assert_has_calls([ + call(["ln", "-s", "/var/lib/pg/data/16/main", "/var/lib/postgresql/16/main"]), + call().wait(), + call(["chown", "-h", "postgres:postgres", "/var/lib/postgresql/16/main"]), + call().wait(), call(["chown", "postgres:postgres", "/var/lib/pg/archive"]), call().wait(), call(["chown", "postgres:postgres", "/var/lib/pg/data"]), From 4fa7c7d5fefb724c0b6f99819530edabe8e402a0 Mon Sep 17 00:00:00 2001 From: Marcelo Henrique Neppel Date: Mon, 26 Jan 2026 17:27:56 -0300 Subject: [PATCH 08/24] fix: use consistent symlink path for PostgreSQL data directory Update POSTGRESQL_DATA_PATH and pgdata_path to use the symlink path (/var/lib/postgresql/16/main) instead of the actual storage path (/var/lib/pg/data/16/main) for Patroni, pgbackrest, and tests. This ensures all components reference the canonical symlink path that PostgreSQL and Patroni expect. Signed-off-by: Marcelo Henrique Neppel --- src/backups.py | 5 +++-- src/charm.py | 18 ++++++++++-------- src/constants.py | 2 +- src/patroni.py | 3 +++ templates/patroni.yml.j2 | 4 ++-- tests/helpers.py | 7 +++++-- tests/integration/ha_tests/clean-data-dir.sh | 2 +- tests/integration/ha_tests/helpers.py | 2 +- tests/integration/helpers.py | 7 +++++-- tests/integration/test_tls.py | 2 +- tests/unit/test_backups.py | 4 ++-- tests/unit/test_patroni.py | 5 ++++- 12 files changed, 38 insertions(+), 23 deletions(-) diff --git a/src/backups.py b/src/backups.py index d100d01fc2a..41ea647fb1b 100644 --- a/src/backups.py +++ b/src/backups.py @@ -34,6 +34,7 @@ BACKUP_TYPE_OVERRIDES, BACKUP_USER, PGBACKREST_LOGROTATE_FILE, + POSTGRESQL_DATA_PATH, WORKLOAD_OS_GROUP, WORKLOAD_OS_USER, ) @@ -195,7 +196,7 @@ def can_use_s3_repository(self) -> tuple[bool, str]: system_identifier_from_instance, error = self._execute_command([ f"/usr/lib/postgresql/{self.charm._patroni.rock_postgresql_version.split('.')[0]}/bin/pg_controldata", - "/var/lib/pg/data/16/main", + POSTGRESQL_DATA_PATH, ]) if error != "": raise Exception(error) @@ -274,7 +275,7 @@ def _create_bucket_if_not_exists(self) -> None: def _empty_data_files(self) -> None: """Empty the PostgreSQL data directory in preparation of backup restore.""" try: - self.container.exec(["rm", "-r", "/var/lib/pg/data/16/main"]).wait_output() + self.container.exec(["rm", "-r", POSTGRESQL_DATA_PATH]).wait_output() except ExecError as e: # If previous PITR restore was unsuccessful, there is no such directory. if "No such file or directory" not in str(e.stderr): diff --git a/src/charm.py b/src/charm.py index 81499e2155e..29be7abe576 100755 --- a/src/charm.py +++ b/src/charm.py @@ -263,7 +263,8 @@ def __init__(self, *args): self._certs_path = "/usr/local/share/ca-certificates" self._storage_path = str(self.meta.storages["data"].location) - self.pgdata_path = f"{self._storage_path}/16/main" + self._actual_pgdata_path = f"{self._storage_path}/16/main" + self.pgdata_path = "/var/lib/postgresql/16/main" self.framework.observe(self.on.upgrade_charm, self._on_upgrade_charm) self.postgresql_client_relation = PostgreSQLProvider(self) @@ -1136,10 +1137,10 @@ def fix_leader_annotation(self) -> bool: def _create_pgdata(self, container: Container): """Create the PostgreSQL data directories.""" - # Create the pgdata directory (e.g., /var/lib/pg/data/16/main) - if not container.exists(self.pgdata_path): + # Create the pgdata directory on the storage mount (e.g., /var/lib/pg/data/16/main) + if not container.exists(self._actual_pgdata_path): container.make_dir( - self.pgdata_path, + self._actual_pgdata_path, permissions=0o700, user=WORKLOAD_OS_USER, group=WORKLOAD_OS_GROUP, @@ -1169,20 +1170,20 @@ def _create_pgdata(self, container: Container): ) # Create a symlink from the default PostgreSQL data directory to our data directory # (e.g., /var/lib/postgresql/16/main -> /var/lib/pg/data/16/main) - default_pgdata_path = "/var/lib/postgresql/16/main" - if not container.exists(default_pgdata_path): + # Patroni and other tools will use the symlink path (self.pgdata_path) + if not container.exists(self.pgdata_path): container.make_dir( "/var/lib/postgresql/16", user=WORKLOAD_OS_USER, group=WORKLOAD_OS_GROUP, make_parents=True, ) - container.exec(["ln", "-s", self.pgdata_path, default_pgdata_path]).wait() + container.exec(["ln", "-s", self._actual_pgdata_path, self.pgdata_path]).wait() container.exec([ "chown", "-h", f"{WORKLOAD_OS_USER}:{WORKLOAD_OS_GROUP}", - default_pgdata_path, + self.pgdata_path, ]).wait() # Also, fix the permissions from the parent directory. container.exec([ @@ -2029,6 +2030,7 @@ def _patroni(self): self.primary_endpoint, self._namespace, self._storage_path, + self.pgdata_path, self.get_secret(APP_SCOPE, USER_PASSWORD_KEY), self.get_secret(APP_SCOPE, REPLICATION_PASSWORD_KEY), self.get_secret(APP_SCOPE, REWIND_PASSWORD_KEY), diff --git a/src/constants.py b/src/constants.py index da5e934b71e..c5fded8a47f 100644 --- a/src/constants.py +++ b/src/constants.py @@ -18,7 +18,7 @@ WORKLOAD_OS_GROUP = "postgres" WORKLOAD_OS_USER = "postgres" METRICS_PORT = "9187" -POSTGRESQL_DATA_PATH = "/var/lib/pg/data/16/main" +POSTGRESQL_DATA_PATH = "/var/lib/postgresql/16/main" POSTGRESQL_LOGS_PATH = "/var/log/postgresql" POSTGRESQL_LOGS_PATTERN = "postgresql*.log" POSTGRES_LOG_FILES = [ diff --git a/src/patroni.py b/src/patroni.py index 31bcdd4c1b6..c83aa87370c 100644 --- a/src/patroni.py +++ b/src/patroni.py @@ -99,6 +99,7 @@ def __init__( primary_endpoint: str, namespace: str, storage_path: str, + pgdata_path: str, superuser_password: str | None, replication_password: str | None, rewind_password: str | None, @@ -110,6 +111,7 @@ def __init__( self._primary_endpoint = primary_endpoint self._namespace = namespace self._storage_path = storage_path + self._pgdata_path = pgdata_path self._members_count = len(self._endpoints) self._superuser_password = superuser_password self._replication_password = replication_password @@ -673,6 +675,7 @@ def render_patroni_yml_file( is_no_sync_member=is_no_sync_member, namespace=self._namespace, storage_path=self._storage_path, + pgdata_path=self._pgdata_path, superuser_password=self._superuser_password, replication_password=self._replication_password, rewind_user=REWIND_USER, diff --git a/templates/patroni.yml.j2 b/templates/patroni.yml.j2 index 45701bfcf3d..d62cae592db 100644 --- a/templates/patroni.yml.j2 +++ b/templates/patroni.yml.j2 @@ -65,7 +65,7 @@ bootstrap: method: pgbackrest pgbackrest: command: > - pgbackrest --stanza={{ restore_stanza }} --pg1-path={{ storage_path }}/16/main + 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 %} @@ -118,7 +118,7 @@ postgresql: basebackup: - waldir: /var/lib/pg/logs/16/main connect_address: '{{ endpoint }}:5432' - data_dir: {{ storage_path }}/16/main + data_dir: {{ pgdata_path }} bin_dir: /usr/lib/postgresql/{{ version }}/bin listen: 0.0.0.0:5432 parameters: diff --git a/tests/helpers.py b/tests/helpers.py index 7d8e8fe394c..002f9648cd9 100644 --- a/tests/helpers.py +++ b/tests/helpers.py @@ -9,5 +9,8 @@ METADATA = yaml.safe_load(Path("./metadata.yaml").read_text()) STORAGE_PATH = METADATA["storage"]["data"]["location"] -# PGDATA_PATH points to the workload's Postgres data directory (versioned path under the storage mount). -PGDATA_PATH = f"{STORAGE_PATH}/16/main" +# PGDATA_PATH points to the symlink path that Patroni and PostgreSQL use. +PGDATA_PATH = "/var/lib/postgresql/16/main" + +# ACTUAL_PGDATA_PATH points to the actual storage location (versioned path under the storage mount). +ACTUAL_PGDATA_PATH = f"{STORAGE_PATH}/16/main" diff --git a/tests/integration/ha_tests/clean-data-dir.sh b/tests/integration/ha_tests/clean-data-dir.sh index 73afed0affa..b8260fead52 100755 --- a/tests/integration/ha_tests/clean-data-dir.sh +++ b/tests/integration/ha_tests/clean-data-dir.sh @@ -2,6 +2,6 @@ set -Eeuo pipefail rm -rf /var/lib/pg/archive/* -rm -rf /var/lib/pg/data/16/main/* +rm -rf /var/lib/postgresql/16/main/* rm -rf /var/lib/pg/logs/* rm -rf /var/lib/pg/temp/* diff --git a/tests/integration/ha_tests/helpers.py b/tests/integration/ha_tests/helpers.py index cd425a32f85..5c5a119ba55 100644 --- a/tests/integration/ha_tests/helpers.py +++ b/tests/integration/ha_tests/helpers.py @@ -621,7 +621,7 @@ async def is_replica(ops_test: OpsTest, unit_name: str) -> bool: async def list_wal_files(ops_test: OpsTest, app: str) -> set: """Returns the list of WAL segment files in each unit.""" units = [unit.name for unit in ops_test.model.applications[app].units] - command = "ls -1 /var/lib/pg/data/16/main/pg_wal/" + command = "ls -1 /var/lib/postgresql/16/main/pg_wal/" files = {} for unit in units: complete_command = f"run --unit {unit} -- {command}" diff --git a/tests/integration/helpers.py b/tests/integration/helpers.py index ee08f89b945..3ca53c6f7cc 100644 --- a/tests/integration/helpers.py +++ b/tests/integration/helpers.py @@ -43,8 +43,11 @@ DATA_INTEGRATOR_APP_NAME = "data-integrator" STORAGE_PATH = METADATA["storage"]["data"]["location"] -# PGDATA_PATH points to the workload's Postgres data directory (versioned path under the storage mount). -PGDATA_PATH = f"{STORAGE_PATH}/16/main" +# PGDATA_PATH points to the symlink path that Patroni and PostgreSQL use. +PGDATA_PATH = "/var/lib/postgresql/16/main" + +# ACTUAL_PGDATA_PATH points to the actual storage location (versioned path under the storage mount). +ACTUAL_PGDATA_PATH = f"{STORAGE_PATH}/16/main" class SecretNotFoundError(Exception): diff --git a/tests/integration/test_tls.py b/tests/integration/test_tls.py index 59bfab9ac8b..cb78286963e 100644 --- a/tests/integration/test_tls.py +++ b/tests/integration/test_tls.py @@ -119,7 +119,7 @@ async def test_tls(ops_test: OpsTest) -> None: await run_command_on_unit( ops_test, replica, - 'su postgres -c "/usr/lib/postgresql/16/bin/pg_ctl -D /var/lib/pg/data/16/main promote"', + 'su postgres -c "/usr/lib/postgresql/16/bin/pg_ctl -D /var/lib/postgresql/16/main promote"', ) # Check that the replica was promoted. diff --git a/tests/unit/test_backups.py b/tests/unit/test_backups.py index 99f4882d27a..3447ae97ffe 100644 --- a/tests/unit/test_backups.py +++ b/tests/unit/test_backups.py @@ -400,7 +400,7 @@ def test_create_bucket_if_not_exists(harness, tls_ca_chain_filename): def test_empty_data_files(harness): with patch("ops.model.Container.exec") as _exec: # Test when the removal of the data files fails. - command = ["rm", "-r", "/var/lib/pg/data/16/main"] + command = ["rm", "-r", "/var/lib/postgresql/16/main"] _exec.side_effect = ExecError(command=command, exit_code=1, stdout="", stderr="fake error") try: harness.charm.backup._empty_data_files() @@ -443,7 +443,7 @@ def test_change_connectivity_to_database(harness): def test_execute_command(harness): with patch("ops.model.Container.exec") as _exec: - command = ["rm", "-r", "/var/lib/pg/data/16/main"] + command = ["rm", "-r", "/var/lib/postgresql/16/main"] _exec.return_value.wait_output.return_value = ("fake stdout", "") # Test when the command runs successfully. diff --git a/tests/unit/test_patroni.py b/tests/unit/test_patroni.py index 5df3a5c1a51..a92671c6280 100644 --- a/tests/unit/test_patroni.py +++ b/tests/unit/test_patroni.py @@ -14,7 +14,7 @@ from charm import PostgresqlOperatorCharm from constants import API_REQUEST_TIMEOUT, REWIND_USER from patroni import Patroni, SwitchoverFailedError, SwitchoverNotSyncError -from tests.helpers import STORAGE_PATH +from tests.helpers import PGDATA_PATH, STORAGE_PATH @pytest.fixture(autouse=True) @@ -35,6 +35,7 @@ def patroni(harness): "postgresql-k8s-primary.dev.svc.cluster.local", "test-model", STORAGE_PATH, + PGDATA_PATH, "superuser-password", "replication-password", "rewind-password", @@ -224,6 +225,7 @@ def test_render_patroni_yml_file(harness, patroni): endpoints=patroni._endpoints, namespace=patroni._namespace, storage_path=patroni._storage_path, + pgdata_path=patroni._pgdata_path, superuser_password=patroni._superuser_password, replication_password=patroni._replication_password, rewind_user=REWIND_USER, @@ -260,6 +262,7 @@ def test_render_patroni_yml_file(harness, patroni): endpoints=patroni._endpoints, namespace=patroni._namespace, storage_path=patroni._storage_path, + pgdata_path=patroni._pgdata_path, superuser_password=patroni._superuser_password, replication_password=patroni._replication_password, rewind_user=REWIND_USER, From a68366f83072e9dc0301a357987f516dcc4211fb Mon Sep 17 00:00:00 2001 From: Marcelo Henrique Neppel Date: Wed, 28 Jan 2026 08:23:47 -0300 Subject: [PATCH 09/24] fix(async-replication): prevent system ID mismatch for standby replicas Non-leader units in a standby cluster now clear pgdata only after the standby leader is confirmed running, preventing system ID mismatch errors during pg_basebackup. Also fixes disk usage check to use actual pgdata path and handles Patroni API unavailability gracefully during slot sync. Signed-off-by: Marcelo Henrique Neppel --- src/charm.py | 4 +-- src/patroni.py | 46 +++++++++++++++--------- src/relations/async_replication.py | 56 ++++++++++++++++++++++-------- 3 files changed, 73 insertions(+), 33 deletions(-) diff --git a/src/charm.py b/src/charm.py index 29be7abe576..fa610f8f4f1 100755 --- a/src/charm.py +++ b/src/charm.py @@ -1893,9 +1893,9 @@ def _on_update_status_early_exit_checks(self, container) -> bool: def _check_pgdata_storage_size(self) -> None: """Asserts that pgdata volume has at least 10% free space and blocks charm if not.""" try: - total_size, _, free_size = shutil.disk_usage(self.pgdata_path) + total_size, _, free_size = shutil.disk_usage(self._actual_pgdata_path) except FileNotFoundError: - logger.error("pgdata folder not found in %s", self.pgdata_path) + logger.error("pgdata folder not found in %s", self._actual_pgdata_path) return logger.debug( diff --git a/src/patroni.py b/src/patroni.py index c83aa87370c..89db013ad4c 100644 --- a/src/patroni.py +++ b/src/patroni.py @@ -525,29 +525,40 @@ def bulk_update_parameters_controller_by_patroni( timeout=API_REQUEST_TIMEOUT, ) - def ensure_slots_controller_by_patroni(self, slots: dict[str, str]) -> None: + def ensure_slots_controller_by_patroni(self, slots: dict[str, str]) -> bool: """Synchronises slots controlled by Patroni with the provided state by removing unneeded slots and creating new ones. Args: slots: dictionary of slots in the {slot: database} format. + + Returns: + True if successful, False if Patroni API is not ready. """ - for attempt in Retrying(stop=stop_after_delay(60), wait=wait_fixed(3), reraise=True): - with attempt: - current_config = requests.get( - f"{self._patroni_url}/config", - verify=self._verify, - timeout=API_REQUEST_TIMEOUT, - auth=self._patroni_auth, - ) - logger.debug( - "API ensure_slots_controller_by_patroni: %s (%s)", - current_config, - current_config.elapsed.total_seconds(), - ) - if current_config.status_code != 200: - raise Exception( - f"Failed to get current Patroni config: {current_config.status_code} {current_config.text}" + try: + for attempt in Retrying(stop=stop_after_delay(60), wait=wait_fixed(3)): + with attempt: + current_config = requests.get( + f"{self._patroni_url}/config", + verify=self._verify, + timeout=API_REQUEST_TIMEOUT, + auth=self._patroni_auth, + ) + logger.debug( + "API ensure_slots_controller_by_patroni: %s (%s)", + current_config, + current_config.elapsed.total_seconds(), ) + if current_config.status_code in (502, 503): + raise Exception( + f"Patroni API not ready: {current_config.status_code}" + ) + if current_config.status_code != 200: + raise Exception( + f"Failed to get current Patroni config: {current_config.status_code} {current_config.text}" + ) + except RetryError: + logger.warning("Patroni config API not ready after retries, will retry later") + return False slots_patch: dict[str, dict[str, str] | None] = dict.fromkeys( current_config.json().get("slots", ()) or {} @@ -565,6 +576,7 @@ def ensure_slots_controller_by_patroni(self, slots: dict[str, str]) -> None: auth=self._patroni_auth, timeout=API_REQUEST_TIMEOUT, ) + return True def promote_standby_cluster(self) -> None: """Promote a standby cluster to be a regular cluster.""" diff --git a/src/relations/async_replication.py b/src/relations/async_replication.py index 0a1f50b7cc8..987c889834d 100644 --- a/src/relations/async_replication.py +++ b/src/relations/async_replication.py @@ -353,7 +353,10 @@ def _handle_database_start(self, event: RelationChangedEvent) -> None: if self.charm._patroni.member_started: # If the database is started, update the databag in a way the unit is marked as configured # for async replication. - self.charm._peers.data[self.charm.unit].update({"stopped": ""}) + self.charm._peers.data[self.charm.unit].update({ + "stopped": "", + "standby-pgdata-cleared": "", + }) self.charm._peers.data[self.charm.unit].update({ "unit-promoted-cluster-counter": self._get_highest_promoted_cluster_counter_value() }) @@ -480,6 +483,7 @@ def _on_async_relation_broken(self, _) -> None: self.charm._peers.data[self.charm.unit].update({ "stopped": "", + "standby-pgdata-cleared": "", "unit-promoted-cluster-counter": "", }) @@ -508,8 +512,12 @@ def _on_async_relation_changed(self, event: RelationChangedEvent) -> None: if self._configure_primary_cluster(primary_cluster, event): return - # Return if this is a new unit. + # Return if this is a new unit joining an existing standby cluster. + # But first, clear pgdata if needed so Patroni can run pg_basebackup. if not self.charm.unit.is_leader() and self._is_following_promoted_cluster(): + if self.charm._peers.data[self.charm.unit].get("standby-pgdata-cleared") != "True": + self._clear_pgdata() + self.charm._peers.data[self.charm.unit].update({"standby-pgdata-cleared": "True"}) logger.debug("Early exit on_async_relation_changed: following promoted cluster.") return @@ -718,22 +726,29 @@ def _stop_database(self, event: RelationChangedEvent) -> bool: if not self._configure_standby_cluster(event): return False - # Remove and recreate the pgdata folder to enable replication of the data from the - # primary cluster. - for path in [ - "/var/lib/pg/archive", - POSTGRESQL_DATA_PATH, - "/var/lib/pg/logs", - "/var/lib/pg/temp", - ]: - logger.info(f"Removing contents from {path}") - self.container.exec(f"find {path} -mindepth 1 -delete".split()).wait_output() - self.charm._create_pgdata(self.container) + # Only the leader clears pgdata here. Non-leaders will clear pgdata + # after the standby leader has started (in _wait_for_standby_leader) + # to avoid system ID mismatch issues. + self._clear_pgdata() self.charm._peers.data[self.charm.unit].update({"stopped": "True"}) return True + def _clear_pgdata(self) -> None: + """Remove and recreate the pgdata folder to enable replication.""" + # Note: Use _actual_pgdata_path instead of POSTGRESQL_DATA_PATH because + # POSTGRESQL_DATA_PATH is a symlink, and find doesn't follow symlinks by default. + for path in [ + "/var/lib/pg/archive", + self.charm._actual_pgdata_path, + "/var/lib/pg/logs", + "/var/lib/pg/temp", + ]: + logger.info(f"Removing contents from {path}") + self.container.exec(f"find {path} -mindepth 1 -delete".split()).wait_output() + self.charm._create_pgdata(self.container) + def update_async_replication_data(self) -> None: """Updates the async-replication data, if the unit is the leader. @@ -796,7 +811,11 @@ def _update_primary_cluster_data( ) def _wait_for_standby_leader(self, event: RelationChangedEvent) -> bool: - """Wait for the standby leader to be up and running.""" + """Wait for the standby leader to be up and running. + + For non-leader units, this also clears pgdata once the standby leader is confirmed + running to avoid system ID mismatch issues. + """ try: standby_leader = self.charm._patroni.get_standby_leader(check_whether_is_running=True) except RetryError: @@ -808,4 +827,13 @@ def _wait_for_standby_leader(self, event: RelationChangedEvent) -> bool: logger.debug("Deferring on_async_relation_changed: standby leader hasn't started yet.") event.defer() return True + + # For non-leader units, clear pgdata now that the standby leader is running. + # This ensures replicas get the correct system ID from the standby leader. + # Only clear pgdata once - use a flag to track if we've already done it. + if not self.charm.unit.is_leader(): + if self.charm._peers.data[self.charm.unit].get("standby-pgdata-cleared") != "True": + self._clear_pgdata() + self.charm._peers.data[self.charm.unit].update({"standby-pgdata-cleared": "True"}) + return False From 13c9f57be4b72899f5d1500104ffe7bb45a4977a Mon Sep 17 00:00:00 2001 From: Marcelo Henrique Neppel Date: Wed, 28 Jan 2026 08:29:38 -0300 Subject: [PATCH 10/24] style: improve code formatting in patroni and async_replication modules Signed-off-by: Marcelo Henrique Neppel --- src/patroni.py | 4 +--- src/relations/async_replication.py | 10 ++++++---- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/patroni.py b/src/patroni.py index 89db013ad4c..501f0beae19 100644 --- a/src/patroni.py +++ b/src/patroni.py @@ -549,9 +549,7 @@ def ensure_slots_controller_by_patroni(self, slots: dict[str, str]) -> bool: current_config.elapsed.total_seconds(), ) if current_config.status_code in (502, 503): - raise Exception( - f"Patroni API not ready: {current_config.status_code}" - ) + raise Exception(f"Patroni API not ready: {current_config.status_code}") if current_config.status_code != 200: raise Exception( f"Failed to get current Patroni config: {current_config.status_code} {current_config.text}" diff --git a/src/relations/async_replication.py b/src/relations/async_replication.py index 987c889834d..83797008cec 100644 --- a/src/relations/async_replication.py +++ b/src/relations/async_replication.py @@ -831,9 +831,11 @@ def _wait_for_standby_leader(self, event: RelationChangedEvent) -> bool: # For non-leader units, clear pgdata now that the standby leader is running. # This ensures replicas get the correct system ID from the standby leader. # Only clear pgdata once - use a flag to track if we've already done it. - if not self.charm.unit.is_leader(): - if self.charm._peers.data[self.charm.unit].get("standby-pgdata-cleared") != "True": - self._clear_pgdata() - self.charm._peers.data[self.charm.unit].update({"standby-pgdata-cleared": "True"}) + if ( + not self.charm.unit.is_leader() + and self.charm._peers.data[self.charm.unit].get("standby-pgdata-cleared") != "True" + ): + self._clear_pgdata() + self.charm._peers.data[self.charm.unit].update({"standby-pgdata-cleared": "True"}) return False From 5581a0f2bd1703b1a77921d112aee313284e13a7 Mon Sep 17 00:00:00 2001 From: Marcelo Henrique Neppel Date: Wed, 28 Jan 2026 11:25:41 -0300 Subject: [PATCH 11/24] fix(backups): use actual pgdata path instead of symlink for backup operations Update pgbackrest config template to use the proper POSTGRESQL_DATA_PATH constant and fix _empty_data_files to remove the actual directory contents rather than just the symlink, ensuring backup and restore operations work correctly. Signed-off-by: Marcelo Henrique Neppel --- src/backups.py | 6 +++++- templates/pgbackrest.conf.j2 | 4 ++-- tests/unit/test_backups.py | 5 ++++- 3 files changed, 11 insertions(+), 4 deletions(-) diff --git a/src/backups.py b/src/backups.py index 41ea647fb1b..6db2210fc83 100644 --- a/src/backups.py +++ b/src/backups.py @@ -275,7 +275,10 @@ def _create_bucket_if_not_exists(self) -> None: def _empty_data_files(self) -> None: """Empty the PostgreSQL data directory in preparation of backup restore.""" try: - self.container.exec(["rm", "-r", POSTGRESQL_DATA_PATH]).wait_output() + # Remove the actual data directory contents, not the symlink. + # POSTGRESQL_DATA_PATH is a symlink to _actual_pgdata_path, and rm -r on a + # symlink only removes the symlink itself, not the directory it points to. + self.container.exec(["rm", "-r", self.charm._actual_pgdata_path]).wait_output() except ExecError as e: # If previous PITR restore was unsuccessful, there is no such directory. if "No such file or directory" not in str(e.stderr): @@ -1213,6 +1216,7 @@ def _render_pgbackrest_conf_file(self) -> bool: secret_key=s3_parameters["secret-key"], stanza=self.stanza_name, storage_path=self.charm._storage_path, + pgdata_path=POSTGRESQL_DATA_PATH, user=BACKUP_USER, retention_full=s3_parameters["delete-older-than-days"], process_max=max(cpu_count - 2, 1), diff --git a/templates/pgbackrest.conf.j2 b/templates/pgbackrest.conf.j2 index 573f9dc87aa..2def70f137f 100644 --- a/templates/pgbackrest.conf.j2 +++ b/templates/pgbackrest.conf.j2 @@ -33,7 +33,7 @@ tls-server-key-file={{ storage_path }}/peer_key.pem {%- endif %} [{{ stanza }}] -pg1-path={{ storage_path }}/16/main +pg1-path={{ pgdata_path }} pg1-user={{ user }} {%- if enable_tls %} {% set ns = namespace(count=2) %} @@ -43,7 +43,7 @@ pg{{ ns.count }}-host-ca-file={{ storage_path }}/peer_ca.pem pg{{ ns.count }}-host-cert-file={{ storage_path }}/peer_cert.pem pg{{ ns.count }}-host-key-file={{ storage_path }}/peer_key.pem pg{{ ns.count }}-host={{ peer_endpoint }} -pg{{ ns.count }}-path={{ storage_path }}/16/main +pg{{ ns.count }}-path={{ pgdata_path }} pg{{ ns.count }}-user={{ user }} {% set ns.count = ns.count + 1 %} {%- endfor %} diff --git a/tests/unit/test_backups.py b/tests/unit/test_backups.py index 3447ae97ffe..5fd4f6711ac 100644 --- a/tests/unit/test_backups.py +++ b/tests/unit/test_backups.py @@ -400,7 +400,9 @@ def test_create_bucket_if_not_exists(harness, tls_ca_chain_filename): def test_empty_data_files(harness): with patch("ops.model.Container.exec") as _exec: # Test when the removal of the data files fails. - command = ["rm", "-r", "/var/lib/postgresql/16/main"] + # Uses _actual_pgdata_path (the real path) instead of POSTGRESQL_DATA_PATH (symlink) + # because rm -r on a symlink only removes the symlink, not the directory contents. + command = ["rm", "-r", "/var/lib/pg/data/16/main"] _exec.side_effect = ExecError(command=command, exit_code=1, stdout="", stderr="fake error") try: harness.charm.backup._empty_data_files() @@ -1781,6 +1783,7 @@ def test_render_pgbackrest_conf_file(harness, tls_ca_chain_filename): secret_key="test-secret-key", stanza=harness.charm.backup.stanza_name, storage_path=harness.charm._storage_path, + pgdata_path=harness.charm.pgdata_path, user="backup", retention_full=30, process_max=2, From 182747865e77e188a7babef71aa7471bde10f878 Mon Sep 17 00:00:00 2001 From: Marcelo Henrique Neppel Date: Wed, 28 Jan 2026 14:24:35 -0300 Subject: [PATCH 12/24] fix(tests): update clean-data-dir.sh paths and use safer deletion - Update data directory paths to match current structure (/var/lib/pg/data/16/main, etc.) - Replace rm -rf with find -delete to preserve mount point directories Signed-off-by: Marcelo Henrique Neppel --- tests/integration/ha_tests/clean-data-dir.sh | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/integration/ha_tests/clean-data-dir.sh b/tests/integration/ha_tests/clean-data-dir.sh index b8260fead52..a8b2e9dd4f5 100755 --- a/tests/integration/ha_tests/clean-data-dir.sh +++ b/tests/integration/ha_tests/clean-data-dir.sh @@ -1,7 +1,7 @@ #!/usr/bin/env bash set -Eeuo pipefail -rm -rf /var/lib/pg/archive/* -rm -rf /var/lib/postgresql/16/main/* -rm -rf /var/lib/pg/logs/* -rm -rf /var/lib/pg/temp/* +find /var/lib/pg/archive -mindepth 1 -delete +find /var/lib/pg/data/16/main -mindepth 1 -delete +find /var/lib/pg/logs/16/main -mindepth 1 -delete +find /var/lib/pg/temp/16/main -mindepth 1 -delete From 4611869de84e44d05fc543d35053e5cdd1744063 Mon Sep 17 00:00:00 2001 From: Marcelo Henrique Neppel Date: Thu, 29 Jan 2026 09:56:01 -0300 Subject: [PATCH 13/24] fix(backup): clear all storage directories during restore for replica compatibility Clear archive, data, logs, and temp directories during backup restore preparation. This ensures pg_basebackup can use --waldir and --tablespace options when new replicas join after a restore, as these options require empty directories. Signed-off-by: Marcelo Henrique Neppel --- src/backups.py | 30 ++++++++++++++++++------------ src/charm.py | 25 +++++++++++++++++++++---- tests/unit/test_backups.py | 32 ++++++++++++++++++++++++-------- 3 files changed, 63 insertions(+), 24 deletions(-) diff --git a/src/backups.py b/src/backups.py index 6db2210fc83..a53b9531d4a 100644 --- a/src/backups.py +++ b/src/backups.py @@ -274,18 +274,24 @@ def _create_bucket_if_not_exists(self) -> None: def _empty_data_files(self) -> None: """Empty the PostgreSQL data directory in preparation of backup restore.""" - try: - # Remove the actual data directory contents, not the symlink. - # POSTGRESQL_DATA_PATH is a symlink to _actual_pgdata_path, and rm -r on a - # symlink only removes the symlink itself, not the directory it points to. - self.container.exec(["rm", "-r", self.charm._actual_pgdata_path]).wait_output() - except ExecError as e: - # If previous PITR restore was unsuccessful, there is no such directory. - if "No such file or directory" not in str(e.stderr): - logger.exception( - "Failed to empty data directory in prep for backup restore", exc_info=e - ) - raise + # Clear all storage directories, not just data. The logs directory must be cleared + # so that when new replicas join after restore, pg_basebackup can use the --waldir + # option (which requires an empty directory). + for path in [ + "/var/lib/pg/archive", + self.charm._actual_pgdata_path, + "/var/lib/pg/logs", + "/var/lib/pg/temp", + ]: + try: + self.container.exec(f"find {path} -mindepth 1 -delete".split()).wait_output() + except ExecError as e: + # If previous PITR restore was unsuccessful, there may be no such directory. + if "No such file or directory" not in str(e.stderr): + logger.exception( + f"Failed to empty {path} in prep for backup restore", exc_info=e + ) + raise def _change_connectivity_to_database(self, connectivity: bool) -> None: """Enable or disable the connectivity to the database.""" diff --git a/src/charm.py b/src/charm.py index fa610f8f4f1..5b8cb21c244 100755 --- a/src/charm.py +++ b/src/charm.py @@ -1137,6 +1137,27 @@ def fix_leader_annotation(self) -> bool: def _create_pgdata(self, container: Container): """Create the PostgreSQL data directories.""" + logs_path = str(self.meta.storages["logs"].location) + waldir_path = f"{logs_path}/16/main" + temp_path = str(self.meta.storages["temp"].location) + temp_tablespace_path = f"{temp_path}/16/main" + + # For non-leader units joining a cluster: if the data directory is empty but other + # storage directories have content, clear them. This happens when a replica rejoins + # after a restore - pg_basebackup requires empty --waldir and --tablespace directories. + if not self.unit.is_leader(): + data_empty = not container.exists( + self._actual_pgdata_path + ) or not container.list_files(self._actual_pgdata_path) + if data_empty: + for path, name in [(logs_path, "logs"), (temp_path, "temp")]: + subdir = f"{path}/16/main" + if container.exists(subdir) and container.list_files(subdir): + logger.info( + f"Clearing stale files from {name} directory for replica initialization" + ) + container.exec(f"find {path} -mindepth 1 -delete".split()).wait() + # Create the pgdata directory on the storage mount (e.g., /var/lib/pg/data/16/main) if not container.exists(self._actual_pgdata_path): container.make_dir( @@ -1147,8 +1168,6 @@ def _create_pgdata(self, container: Container): make_parents=True, ) # Create the WAL directory (e.g., /var/lib/pg/logs/16/main) - logs_path = str(self.meta.storages["logs"].location) - waldir_path = f"{logs_path}/16/main" if not container.exists(waldir_path): container.make_dir( waldir_path, @@ -1158,8 +1177,6 @@ def _create_pgdata(self, container: Container): make_parents=True, ) # Create the temp tablespace directory (e.g., /var/lib/pg/temp/16/main) - temp_path = str(self.meta.storages["temp"].location) - temp_tablespace_path = f"{temp_path}/16/main" if not container.exists(temp_tablespace_path): container.make_dir( temp_tablespace_path, diff --git a/tests/unit/test_backups.py b/tests/unit/test_backups.py index 5fd4f6711ac..3afb4a8d6e8 100644 --- a/tests/unit/test_backups.py +++ b/tests/unit/test_backups.py @@ -399,23 +399,39 @@ def test_create_bucket_if_not_exists(harness, tls_ca_chain_filename): def test_empty_data_files(harness): with patch("ops.model.Container.exec") as _exec: - # Test when the removal of the data files fails. - # Uses _actual_pgdata_path (the real path) instead of POSTGRESQL_DATA_PATH (symlink) - # because rm -r on a symlink only removes the symlink, not the directory contents. - command = ["rm", "-r", "/var/lib/pg/data/16/main"] - _exec.side_effect = ExecError(command=command, exit_code=1, stdout="", stderr="fake error") + # Mock the wait_output method + mock_process = MagicMock() + mock_process.wait_output.return_value = ("", "") + _exec.return_value = mock_process + + # Test when the removal of data files fails on the first directory. + _exec.side_effect = ExecError( + command=["find", "/var/lib/pg/archive", "-mindepth", "1", "-delete"], + exit_code=1, + stdout="", + stderr="fake error", + ) try: harness.charm.backup._empty_data_files() assert False except ExecError: pass - _exec.assert_called_once_with(command) - # Test when data files are successfully removed. + # Test when data files are successfully removed from all directories. _exec.reset_mock() _exec.side_effect = None + _exec.return_value = mock_process harness.charm.backup._empty_data_files() - _exec.assert_called_once_with(command) + # Should be called 4 times: archive, data, logs, temp + assert _exec.call_count == 4 + expected_calls = [ + (["find", "/var/lib/pg/archive", "-mindepth", "1", "-delete"],), + (["find", "/var/lib/pg/data/16/main", "-mindepth", "1", "-delete"],), + (["find", "/var/lib/pg/logs", "-mindepth", "1", "-delete"],), + (["find", "/var/lib/pg/temp", "-mindepth", "1", "-delete"],), + ] + for call, expected in zip(_exec.call_args_list, expected_calls, strict=True): + assert call[0] == expected def test_change_connectivity_to_database(harness): From 5e32c4b38d14e5aa509a37fe3ee9702022cb8660 Mon Sep 17 00:00:00 2001 From: Marcelo Henrique Neppel Date: Mon, 2 Feb 2026 16:58:44 -0300 Subject: [PATCH 14/24] refactor(charm): remove stale storage cleanup for replica initialization Remove the logic that cleared logs and temp directories when a non-leader unit rejoined with an empty data directory. This cleanup was intended for pg_basebackup during restore operations but is no longer needed. Signed-off-by: Marcelo Henrique Neppel --- src/charm.py | 18 ++---------------- 1 file changed, 2 insertions(+), 16 deletions(-) diff --git a/src/charm.py b/src/charm.py index 5b8cb21c244..8635db37690 100755 --- a/src/charm.py +++ b/src/charm.py @@ -1142,22 +1142,6 @@ def _create_pgdata(self, container: Container): temp_path = str(self.meta.storages["temp"].location) temp_tablespace_path = f"{temp_path}/16/main" - # For non-leader units joining a cluster: if the data directory is empty but other - # storage directories have content, clear them. This happens when a replica rejoins - # after a restore - pg_basebackup requires empty --waldir and --tablespace directories. - if not self.unit.is_leader(): - data_empty = not container.exists( - self._actual_pgdata_path - ) or not container.list_files(self._actual_pgdata_path) - if data_empty: - for path, name in [(logs_path, "logs"), (temp_path, "temp")]: - subdir = f"{path}/16/main" - if container.exists(subdir) and container.list_files(subdir): - logger.info( - f"Clearing stale files from {name} directory for replica initialization" - ) - container.exec(f"find {path} -mindepth 1 -delete".split()).wait() - # Create the pgdata directory on the storage mount (e.g., /var/lib/pg/data/16/main) if not container.exists(self._actual_pgdata_path): container.make_dir( @@ -1188,6 +1172,8 @@ def _create_pgdata(self, container: Container): # Create a symlink from the default PostgreSQL data directory to our data directory # (e.g., /var/lib/postgresql/16/main -> /var/lib/pg/data/16/main) # Patroni and other tools will use the symlink path (self.pgdata_path) + # Note: This symlink is on ephemeral storage and may not persist across container restarts. + # It gets recreated on each pebble-ready event. if not container.exists(self.pgdata_path): container.make_dir( "/var/lib/postgresql/16", From 7555a88695cb93069d045066ba0a262068d9e296 Mon Sep 17 00:00:00 2001 From: Marcelo Henrique Neppel Date: Wed, 4 Feb 2026 10:45:08 -0300 Subject: [PATCH 15/24] fix(charm): clear stale storage dirs before replica initialization Clear WAL and tablespace directories when a replica joins an initialized cluster, as PersistentVolumes may retain data from previous pods and pg_basebackup requires empty target directories. Signed-off-by: Marcelo Henrique Neppel --- src/charm.py | 21 +++++++++++++++++++++ tests/integration/ha_tests/helpers.py | 3 ++- tests/integration/test_tls.py | 3 ++- 3 files changed, 25 insertions(+), 2 deletions(-) diff --git a/src/charm.py b/src/charm.py index 8635db37690..2848a1184db 100755 --- a/src/charm.py +++ b/src/charm.py @@ -1142,6 +1142,27 @@ def _create_pgdata(self, container: Container): temp_path = str(self.meta.storages["temp"].location) temp_tablespace_path = f"{temp_path}/16/main" + # Clear stale storage directories when a replica joins an initialized cluster. + # This is needed because PersistentVolumes may retain data from previous pods, + # and pg_basebackup requires empty --waldir and --tablespace directories. + # Only clear once per unit lifecycle, tracked by unit peer data flag. + if ( + not self.unit.is_leader() + and self.is_cluster_initialised + and self.unit_peer_data.get("storage-dirs-cleared") != "True" + ): + for path in [waldir_path, temp_tablespace_path]: + if container.exists(path): + try: + container.exec(f"find {path} -mindepth 1 -delete".split()).wait_output() + logger.info( + f"Cleared stale content from {path} for replica initialization" + ) + except ExecError as e: + if "No such file or directory" not in str(e.stderr): + logger.warning(f"Failed to clear {path}: {e}") + self.unit_peer_data["storage-dirs-cleared"] = "True" + # Create the pgdata directory on the storage mount (e.g., /var/lib/pg/data/16/main) if not container.exists(self._actual_pgdata_path): container.make_dir( diff --git a/tests/integration/ha_tests/helpers.py b/tests/integration/ha_tests/helpers.py index 5c5a119ba55..4f7e18d5969 100644 --- a/tests/integration/ha_tests/helpers.py +++ b/tests/integration/ha_tests/helpers.py @@ -38,6 +38,7 @@ from ..helpers import ( APPLICATION_NAME, KUBECTL, + PGDATA_PATH, app_name, db_connect, execute_query_on_unit, @@ -621,7 +622,7 @@ async def is_replica(ops_test: OpsTest, unit_name: str) -> bool: async def list_wal_files(ops_test: OpsTest, app: str) -> set: """Returns the list of WAL segment files in each unit.""" units = [unit.name for unit in ops_test.model.applications[app].units] - command = "ls -1 /var/lib/postgresql/16/main/pg_wal/" + command = f"ls -1 {PGDATA_PATH}/pg_wal/" files = {} for unit in units: complete_command = f"run --unit {unit} -- {command}" diff --git a/tests/integration/test_tls.py b/tests/integration/test_tls.py index cb78286963e..e3957ac0ba0 100644 --- a/tests/integration/test_tls.py +++ b/tests/integration/test_tls.py @@ -14,6 +14,7 @@ from .helpers import ( CHARM_BASE_NOBLE, DATABASE_APP_NAME, + PGDATA_PATH, build_and_deploy, check_tls, check_tls_patroni_api, @@ -119,7 +120,7 @@ async def test_tls(ops_test: OpsTest) -> None: await run_command_on_unit( ops_test, replica, - 'su postgres -c "/usr/lib/postgresql/16/bin/pg_ctl -D /var/lib/postgresql/16/main promote"', + f'su postgres -c "/usr/lib/postgresql/16/bin/pg_ctl -D {PGDATA_PATH} promote"', ) # Check that the replica was promoted. From bd81195393fa87bf180e8549e77a8dd4c8527c06 Mon Sep 17 00:00:00 2001 From: Marcelo Henrique Neppel Date: Thu, 5 Feb 2026 14:26:38 -0300 Subject: [PATCH 16/24] fix(async-replication): check member_started before clearing pgdata on standby When a new unit joins an existing standby cluster, check if the database is already running before clearing pgdata. Previously, _clear_pgdata() was called unconditionally before the member_started check, which could delete the data directory while PostgreSQL was running - causing it to crash and leaving the unit stuck in "awaiting for member to start". Now the order is: 1. If member is already started, return early (unit is working) 2. If member is not started, clear pgdata if needed and continue setup Signed-off-by: Marcelo Henrique Neppel --- src/relations/async_replication.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/relations/async_replication.py b/src/relations/async_replication.py index 83797008cec..48fd3e32b67 100644 --- a/src/relations/async_replication.py +++ b/src/relations/async_replication.py @@ -513,13 +513,17 @@ def _on_async_relation_changed(self, event: RelationChangedEvent) -> None: return # Return if this is a new unit joining an existing standby cluster. - # But first, clear pgdata if needed so Patroni can run pg_basebackup. + # If the database is already running (i.e., we're a late joiner that completed setup), + # just return early - the unit is already part of the standby cluster. if not self.charm.unit.is_leader() and self._is_following_promoted_cluster(): + if self.charm._patroni.member_started: + logger.debug("Early exit on_async_relation_changed: following promoted cluster.") + return + # Database not running - clear pgdata if needed so Patroni can run pg_basebackup. + # Only clear once, tracked by standby-pgdata-cleared flag. if self.charm._peers.data[self.charm.unit].get("standby-pgdata-cleared") != "True": self._clear_pgdata() self.charm._peers.data[self.charm.unit].update({"standby-pgdata-cleared": "True"}) - logger.debug("Early exit on_async_relation_changed: following promoted cluster.") - return if not self._stop_database(event): return From acad692fe340cec2a71589fc33f5c8cd96c0ccc1 Mon Sep 17 00:00:00 2001 From: Marcelo Henrique Neppel Date: Mon, 9 Feb 2026 12:32:25 -0300 Subject: [PATCH 17/24] fix: remove health check script patching workaround The `_fix_health_check_script` method was a workaround for an incorrect hardcoded certificate path in the OCI image. This is no longer needed as the upstream image has been fixed. Update the OCI image reference accordingly. Signed-off-by: Marcelo Henrique Neppel --- metadata.yaml | 2 +- src/charm.py | 90 ---------------------------------------- tests/unit/test_charm.py | 59 -------------------------- 3 files changed, 1 insertion(+), 150 deletions(-) diff --git a/metadata.yaml b/metadata.yaml index 804ff942e71..d265412ba4a 100644 --- a/metadata.yaml +++ b/metadata.yaml @@ -41,7 +41,7 @@ resources: postgresql-image: type: oci-image description: OCI image for PostgreSQL - upstream-source: ghcr.io/canonical/charmed-postgresql@sha256:fcb648db2c418403fbf5f58f04892c4677f113526097ca7314cbd9f527bb1baf # renovate: oci-image tag: 16.11-24.04_edge + upstream-source: docker.io/neppel/charmed-postgresql_16-24.04_edge@sha256:2ab7cdddf774ba958e5b748b6a0f14f81a98716c462cf41672eeb416a8bba884 peers: database-peers: diff --git a/src/charm.py b/src/charm.py index ca22da0a786..02f2b22b430 100755 --- a/src/charm.py +++ b/src/charm.py @@ -4,7 +4,6 @@ """Charmed Kubernetes Operator for the PostgreSQL database.""" -import contextlib import itertools import json import logging @@ -1230,78 +1229,6 @@ def _create_pgdata(self, container: Container): temp_path, ]).wait() - def _fix_health_check_script(self, container: Container) -> None: - """Fix the OCI image health check script to use correct certificate path. - - The OCI image's /scripts/self-signed-checker.py script has a hardcoded path - /var/lib/postgresql/data/peer_ca.pem which is incorrect. The correct path - for this charm is based on the charm storage location (self._storage_path). - - This is a workaround until the OCI image is fixed upstream. - """ - script_path = "/scripts/self-signed-checker.py" - - try: - # Check if script exists - if not container.exists(script_path): - logger.info(f"Health check script {script_path} not found, skipping patch") - return - - # Read current script content (may be bytes or str) - try: - raw = container.pull(script_path).read() - except Exception as e: - logger.exception(f"Failed to pull health check script {script_path}: {e}") - return - script_content = raw.decode() if isinstance(raw, (bytes, bytearray)) else str(raw) - - # Determine the correct path based on charm storage location - storage_path = str(self._storage_path) - # The peer CA file used by the workload is expected at /peer_ca.pem - correct_path = f"{storage_path}/peer_ca.pem" - - # If already patched, nothing to do - if correct_path in script_content: - logger.info("Health check script already patched") - return - - incorrect_path = "/var/lib/postgresql/data/peer_ca.pem" - if incorrect_path not in script_content: - logger.info( - "Health check script does not contain the expected incorrect path, skipping" - ) - return - - patched_content = script_content.replace(incorrect_path, correct_path) - - # Push the patched script back (preserve executable bit) - try: - container.push(script_path, patched_content, permissions=0o755) - except Exception as e: - logger.exception(f"Failed to push patched health check script {script_path}: {e}") - # Try fallback: run an in-container sed to patch the file in-place - try: - sed_cmd = f"sed -i 's|{incorrect_path}|{correct_path}|g' {script_path}" - logger.debug(f"Attempting sed fallback: {sed_cmd}") - exec_res = container.exec(["sh", "-c", sed_cmd]) - with contextlib.suppress(Exception): - exec_res.wait() - logger.info( - f"Patched health check script via exec sed: {incorrect_path} -> {correct_path}" - ) - except Exception as e2: - logger.exception( - f"Failed to patch health check script via exec fallback {script_path}: {e2}" - ) - return - logger.info(f"Patched health check script: {incorrect_path} -> {correct_path}") - - except PathError as e: - # Don't fail the charm if we can't patch the script - logger.exception(f"Failed to patch health check script due to PathError: {e}") - except Exception as e: - logger.exception(f"Unexpected error while patching health check script: {e}") - def _on_start(self, _) -> None: # Make sure the CA bubdle file exists # Bundle is not secret @@ -1335,9 +1262,6 @@ def _on_postgresql_pebble_ready(self, event: WorkloadEvent) -> None: # where the volume is mounted with more restrictive permissions. self._create_pgdata(container) - # Fix the health check script path (workaround for OCI image bug) - self._fix_health_check_script(container) - # Defer the initialization of the workload in the replicas # if the cluster hasn't been bootstrap on the primary yet. # Otherwise, each unit will create a different cluster and @@ -1756,20 +1680,6 @@ def _fix_pod(self) -> None: # when the pod loses them (like when it's deleted). self.push_tls_files_to_workload() - # Ensure the health check script in the workload image is fixed as early as - # possible (e.g. during upgrade_charm or when pod resources are recreated). - # This helps avoid races where the Pebble layer with the health check is added - # before the script is patched. - # Defensive: don't let any unexpected errors from fixing the script - # prevent the remainder of the pod fix from running. - with contextlib.suppress(Exception): - try: - container = self._container - if container.can_connect(): - self._fix_health_check_script(container) - except Exception as e: - logger.debug(f"_fix_pod: could not fix health check script: {e}") - if self.refresh is not None and not self.refresh.in_progress: try: self._create_services() diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py index d14bdd770d7..7be05dd4451 100644 --- a/tests/unit/test_charm.py +++ b/tests/unit/test_charm.py @@ -1580,65 +1580,6 @@ def test_create_pgdata(harness): ]) -def test_fix_health_check_script(harness): - """Test that the health check script path is correctly patched.""" - container = MagicMock() - - # Test when script doesn't exist - should skip gracefully - container.exists.return_value = False - harness.charm._fix_health_check_script(container) - container.exists.assert_called_once_with("/scripts/self-signed-checker.py") - container.pull.assert_not_called() - - # Test when script exists with incorrect path - should patch it - container.reset_mock() - container.exists.return_value = True - mock_file = MagicMock() - mock_file.read.return_value = ( - '#!/usr/bin/python3\npath = "/var/lib/postgresql/data/peer_ca.pem"\n' - ) - container.pull.return_value = mock_file - - harness.charm._fix_health_check_script(container) - - container.exists.assert_called_once_with("/scripts/self-signed-checker.py") - container.pull.assert_called_once_with("/scripts/self-signed-checker.py") - # Verify the content was patched correctly - call_args = container.push.call_args - assert call_args[0][0] == "/scripts/self-signed-checker.py" - assert "/var/lib/pg/data/peer_ca.pem" in call_args[0][1] - assert "/var/lib/postgresql/data/peer_ca.pem" not in call_args[0][1] - assert call_args[1]["permissions"] == 0o755 - - # Test when script already has correct path - should skip patching - container.reset_mock() - container.exists.return_value = True - mock_file_correct = MagicMock() - mock_file_correct.read.return_value = ( - '#!/usr/bin/python3\npath = "/var/lib/pg/data/peer_ca.pem"\n' - ) - container.pull.return_value = mock_file_correct - - harness.charm._fix_health_check_script(container) - - container.exists.assert_called_once_with("/scripts/self-signed-checker.py") - container.pull.assert_called_once_with("/scripts/self-signed-checker.py") - container.push.assert_not_called() # Should not push if already correct - - # Test when script has no matching path - should skip patching - container.reset_mock() - container.exists.return_value = True - mock_file_other = MagicMock() - mock_file_other.read.return_value = '#!/usr/bin/python3\npath = "/some/other/path"\n' - container.pull.return_value = mock_file_other - - harness.charm._fix_health_check_script(container) - - container.exists.assert_called_once_with("/scripts/self-signed-checker.py") - container.pull.assert_called_once_with("/scripts/self-signed-checker.py") - container.push.assert_not_called() # Should not push if nothing to patch - - def test_get_plugins(harness): with patch("charm.PostgresqlOperatorCharm._on_config_changed"): # Test when the charm has no plugins enabled. From f9c0cb5a9d466158e1ccee8a0bd362f364a74cc5 Mon Sep 17 00:00:00 2001 From: Marcelo Henrique Neppel Date: Tue, 10 Feb 2026 17:08:35 -0300 Subject: [PATCH 18/24] fix(storage): ensure pgdata symlink is always recreated and stale dirs are cleared on retry The OCI image ships /var/lib/postgresql/16/main as a real directory, which prevents the symlink from being created on replicas. Always recreate the symlink (removing the real directory first if needed) instead of skipping when the path already exists. Also change stale storage directory cleanup to repeat until pgdata is fully populated (PG_VERSION exists), rather than running only once per unit lifecycle. This ensures retries after a failed pg_basebackup find clean directories. Signed-off-by: Marcelo Henrique Neppel --- src/charm.py | 50 +++++++++++++---------- tests/integration/test_symlinks.py | 54 +++++++++++++++++-------- tests/spread/test_symlinks.py/task.yaml | 7 ++++ tests/unit/test_charm.py | 29 +++++++++++-- 4 files changed, 99 insertions(+), 41 deletions(-) create mode 100644 tests/spread/test_symlinks.py/task.yaml diff --git a/src/charm.py b/src/charm.py index 02f2b22b430..c81f3bb753b 100755 --- a/src/charm.py +++ b/src/charm.py @@ -1143,12 +1143,10 @@ def _create_pgdata(self, container: Container): # Clear stale storage directories when a replica joins an initialized cluster. # This is needed because PersistentVolumes may retain data from previous pods, # and pg_basebackup requires empty --waldir and --tablespace directories. - # Only clear once per unit lifecycle, tracked by unit peer data flag. - if ( - not self.unit.is_leader() - and self.is_cluster_initialised - and self.unit_peer_data.get("storage-dirs-cleared") != "True" - ): + # Clear on every call until pgdata is populated (PG_VERSION exists), since + # pg_basebackup can fail and leave stale files that prevent retries. + pgdata_populated = container.exists(f"{self._actual_pgdata_path}/PG_VERSION") + if not self.unit.is_leader() and self.is_cluster_initialised and not pgdata_populated: for path in [waldir_path, temp_tablespace_path]: if container.exists(path): try: @@ -1159,7 +1157,6 @@ def _create_pgdata(self, container: Container): except ExecError as e: if "No such file or directory" not in str(e.stderr): logger.warning(f"Failed to clear {path}: {e}") - self.unit_peer_data["storage-dirs-cleared"] = "True" # Create the pgdata directory on the storage mount (e.g., /var/lib/pg/data/16/main) if not container.exists(self._actual_pgdata_path): @@ -1193,20 +1190,31 @@ def _create_pgdata(self, container: Container): # Patroni and other tools will use the symlink path (self.pgdata_path) # Note: This symlink is on ephemeral storage and may not persist across container restarts. # It gets recreated on each pebble-ready event. - if not container.exists(self.pgdata_path): - container.make_dir( - "/var/lib/postgresql/16", - user=WORKLOAD_OS_USER, - group=WORKLOAD_OS_GROUP, - make_parents=True, - ) - container.exec(["ln", "-s", self._actual_pgdata_path, self.pgdata_path]).wait() - container.exec([ - "chown", - "-h", - f"{WORKLOAD_OS_USER}:{WORKLOAD_OS_GROUP}", - self.pgdata_path, - ]).wait() + # The OCI image ships /var/lib/postgresql/16/main as a real directory, so we must + # remove it first if it exists as a non-symlink (e.g., on replicas). + container.make_dir( + "/var/lib/postgresql/16", + user=WORKLOAD_OS_USER, + group=WORKLOAD_OS_GROUP, + make_parents=True, + ) + container.exec([ + "bash", + "-c", + f"[ -L {self.pgdata_path} ] || rm -rf {self.pgdata_path}", + ]).wait() + container.exec([ + "ln", + "-sfn", + self._actual_pgdata_path, + self.pgdata_path, + ]).wait() + container.exec([ + "chown", + "-h", + f"{WORKLOAD_OS_USER}:{WORKLOAD_OS_GROUP}", + self.pgdata_path, + ]).wait() # Also, fix the permissions from the parent directory. container.exec([ "chown", diff --git a/tests/integration/test_symlinks.py b/tests/integration/test_symlinks.py index d97a82e7c45..a05a5fc55b2 100644 --- a/tests/integration/test_symlinks.py +++ b/tests/integration/test_symlinks.py @@ -6,52 +6,72 @@ import logging +import jubilant import pytest -from pytest_operator.plugin import OpsTest from .helpers import ( + ACTUAL_PGDATA_PATH, + DATABASE_APP_NAME, METADATA, - PGDATA_PATH, - build_and_deploy, - run_command_on_unit, ) logger = logging.getLogger(__name__) -APP_NAME = METADATA["name"] +APP_NAME = DATABASE_APP_NAME UNIT_IDS = [0, 1, 2] PGDATA_SYMLINK_PATH = "/var/lib/postgresql/16/main" @pytest.mark.abort_on_fail -@pytest.mark.skip_if_deployed -async def test_build_and_deploy(ops_test: OpsTest, charm): +def test_build_and_deploy(juju: jubilant.Juju, charm): """Build the charm-under-test and deploy it. Assert on the unit status before any relations/configurations take place. """ - async with ops_test.fast_forward(): - await build_and_deploy(ops_test, charm, len(UNIT_IDS), APP_NAME) + if APP_NAME not in juju.status().apps: + resources = { + "postgresql-image": METADATA["resources"]["postgresql-image"]["upstream-source"] + } + juju.deploy( + charm, + config={"profile": "testing"}, + num_units=len(UNIT_IDS), + resources=resources, + trust=True, + ) + + def _all_active_idle(status): + if not jubilant.all_active(status, APP_NAME): + return False + for uid in UNIT_IDS: + unit = status.apps[APP_NAME].units[f"{APP_NAME}/{uid}"] + if unit.juju_status.current != "idle": + return False + return True + + juju.wait(_all_active_idle, timeout=1500) + status = juju.status() for unit_id in UNIT_IDS: - assert ops_test.model.applications[APP_NAME].units[unit_id].workload_status == "active" + unit_name = f"{APP_NAME}/{unit_id}" + assert status.apps[APP_NAME].units[unit_name].is_active @pytest.mark.parametrize("unit_id", UNIT_IDS) -async def test_pgdata_symlinks(ops_test: OpsTest, unit_id: int): +def test_pgdata_symlinks(juju: jubilant.Juju, unit_id: int): """Test that symlink for pgdata is correctly created.""" unit_name = f"{APP_NAME}/{unit_id}" # Check pgdata symlink exists and points to correct location - pgdata_symlink_check = await run_command_on_unit( - ops_test, unit_name, f"readlink -f {PGDATA_SYMLINK_PATH}" + pgdata_symlink_check = juju.ssh( + unit_name, "readlink", "-f", PGDATA_SYMLINK_PATH, container="postgresql" ) - assert pgdata_symlink_check.strip() == PGDATA_PATH, ( - f"Expected pgdata symlink to point to {PGDATA_PATH}, got {pgdata_symlink_check.strip()}" + assert pgdata_symlink_check.strip() == ACTUAL_PGDATA_PATH, ( + f"Expected pgdata symlink to point to {ACTUAL_PGDATA_PATH}, got {pgdata_symlink_check.strip()}" ) # Verify symlink is owned by postgres:postgres - pgdata_owner = await run_command_on_unit( - ops_test, unit_name, f"stat -c '%U:%G' {PGDATA_SYMLINK_PATH}" + pgdata_owner = juju.ssh( + unit_name, "stat", "-c", "%U:%G", PGDATA_SYMLINK_PATH, container="postgresql" ) assert pgdata_owner.strip() == "postgres:postgres", ( f"Expected pgdata symlink to be owned by postgres:postgres, got {pgdata_owner.strip()}" diff --git a/tests/spread/test_symlinks.py/task.yaml b/tests/spread/test_symlinks.py/task.yaml new file mode 100644 index 00000000000..b6a7b4e8079 --- /dev/null +++ b/tests/spread/test_symlinks.py/task.yaml @@ -0,0 +1,7 @@ +summary: test_symlinks.py +environment: + TEST_MODULE: test_symlinks.py +execute: | + tox run -e integration -- "tests/integration/$TEST_MODULE" --model testing --alluredir="$SPREAD_TASK/allure-results" +artifacts: + - allure-results diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py index 7be05dd4451..0c5e77c371a 100644 --- a/tests/unit/test_charm.py +++ b/tests/unit/test_charm.py @@ -1548,7 +1548,13 @@ def test_create_pgdata(harness): ), ]) container.exec.assert_has_calls([ - call(["ln", "-s", "/var/lib/pg/data/16/main", "/var/lib/postgresql/16/main"]), + call([ + "bash", + "-c", + "[ -L /var/lib/postgresql/16/main ] || rm -rf /var/lib/postgresql/16/main", + ]), + call().wait(), + call(["ln", "-sfn", "/var/lib/pg/data/16/main", "/var/lib/postgresql/16/main"]), call().wait(), call(["chown", "-h", "postgres:postgres", "/var/lib/postgresql/16/main"]), call().wait(), @@ -1566,9 +1572,26 @@ def test_create_pgdata(harness): container.exec.reset_mock() container.exists.return_value = True harness.charm._create_pgdata(container) - # When directories exist, none should be created - container.make_dir.assert_not_called() + # When directories exist, none should be created (except the symlink parent and symlink itself) + container.make_dir.assert_has_calls([ + call( + "/var/lib/postgresql/16", + user="postgres", + group="postgres", + make_parents=True, + ), + ]) container.exec.assert_has_calls([ + call([ + "bash", + "-c", + "[ -L /var/lib/postgresql/16/main ] || rm -rf /var/lib/postgresql/16/main", + ]), + call().wait(), + call(["ln", "-sfn", "/var/lib/pg/data/16/main", "/var/lib/postgresql/16/main"]), + call().wait(), + call(["chown", "-h", "postgres:postgres", "/var/lib/postgresql/16/main"]), + call().wait(), call(["chown", "postgres:postgres", "/var/lib/pg/archive"]), call().wait(), call(["chown", "postgres:postgres", "/var/lib/pg/data"]), From ab1588b080d2a31b1bb6847091df8a95a5056f8a Mon Sep 17 00:00:00 2001 From: Marcelo Henrique Neppel Date: Wed, 11 Feb 2026 11:21:03 -0300 Subject: [PATCH 19/24] fix: prevent pg_wal symlink breakage on replica startup Use exact pg_wal and pgsql_tmp subdirectories for --waldir and temp tablespace paths instead of their parent directories. Also defer replica startup until its endpoint is present in the members list, preventing pg_basebackup rejections that trigger remove_data_directory() calls racing with _create_pgdata(). Signed-off-by: Marcelo Henrique Neppel --- src/charm.py | 42 +++++++++++++------- templates/patroni.yml.j2 | 4 +- tests/integration/ha_tests/clean-data-dir.sh | 4 +- tests/integration/test_symlinks.py | 14 +++++++ tests/unit/test_charm.py | 4 +- 5 files changed, 48 insertions(+), 20 deletions(-) diff --git a/src/charm.py b/src/charm.py index 3595ccf0f79..b561fd5907e 100755 --- a/src/charm.py +++ b/src/charm.py @@ -1136,12 +1136,27 @@ def fix_leader_annotation(self) -> bool: raise e return True + def _replica_can_start(self) -> bool: + """Check whether this replica is ready to start Patroni. + + Returns False if the cluster hasn't been bootstrapped yet, or if the + leader hasn't added this unit's endpoint to the members list (which + controls the pg_hba replication entries on the primary). + """ + if not self.is_cluster_initialised: + logger.debug("Replica not ready: cluster not initialized") + return False + if self._endpoint not in self._endpoints: + logger.debug("Replica not ready: endpoint not yet in members list") + return False + return True + def _create_pgdata(self, container: Container): """Create the PostgreSQL data directories.""" logs_path = str(self.meta.storages["logs"].location) - waldir_path = f"{logs_path}/16/main" + waldir_path = f"{logs_path}/16/main/pg_wal" temp_path = str(self.meta.storages["temp"].location) - temp_tablespace_path = f"{temp_path}/16/main" + temp_tablespace_path = f"{temp_path}/16/main/pgsql_tmp" # Clear stale storage directories when a replica joins an initialized cluster. # This is needed because PersistentVolumes may retain data from previous pods, @@ -1170,7 +1185,7 @@ def _create_pgdata(self, container: Container): group=WORKLOAD_OS_GROUP, make_parents=True, ) - # Create the WAL directory (e.g., /var/lib/pg/logs/16/main) + # Create the WAL directory (e.g., /var/lib/pg/logs/16/main/pg_wal) if not container.exists(waldir_path): container.make_dir( waldir_path, @@ -1179,7 +1194,7 @@ def _create_pgdata(self, container: Container): group=WORKLOAD_OS_GROUP, make_parents=True, ) - # Create the temp tablespace directory (e.g., /var/lib/pg/temp/16/main) + # Create the temp tablespace directory (e.g., /var/lib/pg/temp/16/main/pgsql_tmp) if not container.exists(temp_tablespace_path): container.make_dir( temp_tablespace_path, @@ -1273,15 +1288,14 @@ def _on_postgresql_pebble_ready(self, event: WorkloadEvent) -> None: # where the volume is mounted with more restrictive permissions. self._create_pgdata(container) - # Defer the initialization of the workload in the replicas - # if the cluster hasn't been bootstrap on the primary yet. - # Otherwise, each unit will create a different cluster and - # any update in the members list on the units won't have effect - # on fixing that. - if not self.unit.is_leader() and not self.is_cluster_initialised: - logger.debug( - "Deferring on_postgresql_pebble_ready: Not leader and cluster not initialized" - ) + # Defer the initialization of the workload in the replicas if the cluster + # hasn't been bootstrapped on the primary yet, or the leader hasn't added + # this unit's endpoint to the cluster members list yet. The endpoint + # controls pg_hba replication entries on the primary — without it, + # pg_basebackup is rejected, triggering retries and remove_data_directory() + # calls that can race with _create_pgdata() and break the pg_wal symlink + # created by --waldir. + if not self.unit.is_leader() and not self._replica_can_start(): event.defer() return @@ -1440,7 +1454,7 @@ def _setup_users(self) -> None: extra_user_roles=["pg_monitor"], ) - self.postgresql.set_up_database(temp_location="/var/lib/pg/temp/16/main") + self.postgresql.set_up_database(temp_location="/var/lib/pg/temp/16/main/pgsql_tmp") access_groups = self.postgresql.list_access_groups() if access_groups != set(ACCESS_GROUPS): diff --git a/templates/patroni.yml.j2 b/templates/patroni.yml.j2 index 166b2965fd5..f78abf50292 100644 --- a/templates/patroni.yml.j2 +++ b/templates/patroni.yml.j2 @@ -87,7 +87,7 @@ bootstrap: - auth-local: trust - encoding: UTF8 - data-checksums - - waldir: /var/lib/pg/logs/16/main + - waldir: /var/lib/pg/logs/16/main/pg_wal {%- endif %} pg_hba: - {{ 'hostssl' if enable_tls else 'host' }} all all 0.0.0.0/0 {{ instance_password_encryption }} @@ -118,7 +118,7 @@ ctl: pod_ip: '{{ endpoint }}' postgresql: basebackup: - - waldir: /var/lib/pg/logs/16/main + - waldir: /var/lib/pg/logs/16/main/pg_wal connect_address: '{{ endpoint }}:5432' data_dir: {{ pgdata_path }} bin_dir: /usr/lib/postgresql/{{ version }}/bin diff --git a/tests/integration/ha_tests/clean-data-dir.sh b/tests/integration/ha_tests/clean-data-dir.sh index a8b2e9dd4f5..9fe6a9d92d7 100755 --- a/tests/integration/ha_tests/clean-data-dir.sh +++ b/tests/integration/ha_tests/clean-data-dir.sh @@ -3,5 +3,5 @@ set -Eeuo pipefail find /var/lib/pg/archive -mindepth 1 -delete find /var/lib/pg/data/16/main -mindepth 1 -delete -find /var/lib/pg/logs/16/main -mindepth 1 -delete -find /var/lib/pg/temp/16/main -mindepth 1 -delete +find /var/lib/pg/logs/16/main/pg_wal -mindepth 1 -delete +find /var/lib/pg/temp/16/main/pgsql_tmp -mindepth 1 -delete diff --git a/tests/integration/test_symlinks.py b/tests/integration/test_symlinks.py index a05a5fc55b2..2e4872e25cb 100644 --- a/tests/integration/test_symlinks.py +++ b/tests/integration/test_symlinks.py @@ -76,3 +76,17 @@ def test_pgdata_symlinks(juju: jubilant.Juju, unit_id: int): assert pgdata_owner.strip() == "postgres:postgres", ( f"Expected pgdata symlink to be owned by postgres:postgres, got {pgdata_owner.strip()}" ) + + +@pytest.mark.parametrize("unit_id", UNIT_IDS) +def test_pg_wal_symlink(juju: jubilant.Juju, unit_id: int): + """Test that pg_wal symlink inside pgdata points to WAL directory on logs storage.""" + unit_name = f"{APP_NAME}/{unit_id}" + + # Check pg_wal symlink exists and points to correct location + pg_wal_symlink_path = f"{PGDATA_SYMLINK_PATH}/pg_wal" + expected_target = "/var/lib/pg/logs/16/main/pg_wal" + result = juju.ssh(unit_name, "readlink", "-f", pg_wal_symlink_path, container="postgresql") + assert result.strip() == expected_target, ( + f"Expected pg_wal symlink to point to {expected_target}, got {result.strip()}" + ) diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py index f65c833d8fc..cceabb9d3f3 100644 --- a/tests/unit/test_charm.py +++ b/tests/unit/test_charm.py @@ -1537,14 +1537,14 @@ def test_create_pgdata(harness): make_parents=True, ), call( - "/var/lib/pg/logs/16/main", + "/var/lib/pg/logs/16/main/pg_wal", permissions=448, user="postgres", group="postgres", make_parents=True, ), call( - "/var/lib/pg/temp/16/main", + "/var/lib/pg/temp/16/main/pgsql_tmp", permissions=448, user="postgres", group="postgres", From 43124ba6808a58d75deddf996c723a2d3084e4f9 Mon Sep 17 00:00:00 2001 From: Marcelo Henrique Neppel Date: Wed, 11 Feb 2026 15:55:41 -0300 Subject: [PATCH 20/24] refactor: replace hardcoded storage paths with named constants Extract storage mount paths (/var/lib/pg/{archive,data,logs,temp}) into named constants in constants.py and use them consistently across backups, async replication, Patroni config rendering, and charm logic. Also fix shell injection risk by passing command arguments as lists instead of split strings, guard rotate_logs service start behind a service existence check, and log failed replication slot syncs instead of silently ignoring the error. Signed-off-by: Marcelo Henrique Neppel --- src/backups.py | 15 ++++++++++----- src/charm.py | 10 +++++++--- src/constants.py | 7 +++++++ src/patroni.py | 2 ++ src/relations/async_replication.py | 11 +++++++---- templates/patroni.yml.j2 | 4 ++-- tests/integration/ha_tests/clean-data-dir.sh | 4 ++-- tests/unit/test_backups.py | 3 +++ tests/unit/test_patroni.py | 4 +++- 9 files changed, 43 insertions(+), 17 deletions(-) diff --git a/src/backups.py b/src/backups.py index 00e5210bd8b..cbbab09d24a 100644 --- a/src/backups.py +++ b/src/backups.py @@ -31,10 +31,13 @@ from tenacity import RetryError, Retrying, stop_after_attempt, wait_fixed from constants import ( + ARCHIVE_PATH, BACKUP_TYPE_OVERRIDES, BACKUP_USER, + LOGS_STORAGE_PATH, PGBACKREST_LOGROTATE_FILE, POSTGRESQL_DATA_PATH, + TEMP_STORAGE_PATH, WORKLOAD_OS_GROUP, WORKLOAD_OS_USER, ) @@ -277,13 +280,13 @@ def _empty_data_files(self) -> None: # so that when new replicas join after restore, pg_basebackup can use the --waldir # option (which requires an empty directory). for path in [ - "/var/lib/pg/archive", + ARCHIVE_PATH, self.charm._actual_pgdata_path, - "/var/lib/pg/logs", - "/var/lib/pg/temp", + LOGS_STORAGE_PATH, + TEMP_STORAGE_PATH, ]: try: - self.container.exec(f"find {path} -mindepth 1 -delete".split()).wait_output() + self.container.exec(["find", path, "-mindepth", "1", "-delete"]).wait_output() except ExecError as e: # If previous PITR restore was unsuccessful, there may be no such directory. if "No such file or directory" not in str(e.stderr): @@ -1245,7 +1248,9 @@ def _render_pgbackrest_conf_file(self) -> bool: "/home/postgres/rotate_logs.py", f.read(), ) - self.container.start(self.charm.rotate_logs_service) + services = self.container.pebble.get_services(names=[self.charm.rotate_logs_service]) + if services: + self.container.start(self.charm.rotate_logs_service) return True diff --git a/src/charm.py b/src/charm.py index b561fd5907e..c2b59f4f7ca 100755 --- a/src/charm.py +++ b/src/charm.py @@ -135,6 +135,7 @@ SECRET_KEY_OVERRIDES, SPI_MODULE, SYSTEM_USERS, + TEMP_STORAGE_PATH, TLS_CA_BUNDLE_FILE, TLS_CA_FILE, TLS_CERT_FILE, @@ -1168,7 +1169,7 @@ def _create_pgdata(self, container: Container): for path in [waldir_path, temp_tablespace_path]: if container.exists(path): try: - container.exec(f"find {path} -mindepth 1 -delete".split()).wait_output() + container.exec(["find", path, "-mindepth", "1", "-delete"]).wait_output() logger.info( f"Cleared stale content from {path} for replica initialization" ) @@ -1454,7 +1455,7 @@ def _setup_users(self) -> None: extra_user_roles=["pg_monitor"], ) - self.postgresql.set_up_database(temp_location="/var/lib/pg/temp/16/main/pgsql_tmp") + self.postgresql.set_up_database(temp_location=f"{TEMP_STORAGE_PATH}/16/main/pgsql_tmp") access_groups = self.postgresql.list_access_groups() if access_groups != set(ACCESS_GROUPS): @@ -2701,7 +2702,10 @@ def update_config(self, is_creating_backup: bool = False) -> bool: logger.warning("Early exit update_config: Unable to patch Patroni API") return False - self._patroni.ensure_slots_controller_by_patroni(replication_slots) + if not self._patroni.ensure_slots_controller_by_patroni(replication_slots): + logger.warning( + "Failed to sync replication slots with Patroni — will retry on next config update" + ) self._handle_postgresql_restart_need( self.unit_peer_data.get("config_hash") != self.generate_config_hash diff --git a/src/constants.py b/src/constants.py index 61869c07051..a5334c505f6 100644 --- a/src/constants.py +++ b/src/constants.py @@ -21,6 +21,13 @@ PGBACKREST_METRICS_PORT = "9854" POSTGRESQL_DATA_PATH = "/var/lib/postgresql/16/main" POSTGRESQL_LOGS_PATH = "/var/log/postgresql" + +# Storage mount paths (must match metadata.yaml storage locations). +STORAGE_PATH = "/var/lib/pg" +ARCHIVE_PATH = f"{STORAGE_PATH}/archive" +DATA_STORAGE_PATH = f"{STORAGE_PATH}/data" +LOGS_STORAGE_PATH = f"{STORAGE_PATH}/logs" +TEMP_STORAGE_PATH = f"{STORAGE_PATH}/temp" POSTGRESQL_LOGS_PATTERN = "postgresql*.log" POSTGRES_LOG_FILES = [ "/var/log/pgbackrest/*", diff --git a/src/patroni.py b/src/patroni.py index 98e4e5fc19e..c9061cc5af0 100644 --- a/src/patroni.py +++ b/src/patroni.py @@ -33,6 +33,7 @@ from constants import ( API_REQUEST_TIMEOUT, + LOGS_STORAGE_PATH, PATRONI_CLUSTER_STATUS_ENDPOINT, POSTGRESQL_LOGS_PATH, POSTGRESQL_LOGS_PATTERN, @@ -702,6 +703,7 @@ def render_patroni_yml_file( is_no_sync_member=is_no_sync_member, namespace=self._namespace, storage_path=self._storage_path, + logs_storage_path=LOGS_STORAGE_PATH, pgdata_path=self._pgdata_path, superuser_password=self._superuser_password, replication_password=self._replication_password, diff --git a/src/relations/async_replication.py b/src/relations/async_replication.py index 7d9c558d459..a942c0ad9ae 100644 --- a/src/relations/async_replication.py +++ b/src/relations/async_replication.py @@ -41,8 +41,11 @@ from constants import ( APP_SCOPE, + ARCHIVE_PATH, + LOGS_STORAGE_PATH, PEER, POSTGRESQL_DATA_PATH, + TEMP_STORAGE_PATH, WORKLOAD_OS_GROUP, WORKLOAD_OS_USER, ) @@ -744,13 +747,13 @@ def _clear_pgdata(self) -> None: # Note: Use _actual_pgdata_path instead of POSTGRESQL_DATA_PATH because # POSTGRESQL_DATA_PATH is a symlink, and find doesn't follow symlinks by default. for path in [ - "/var/lib/pg/archive", + ARCHIVE_PATH, self.charm._actual_pgdata_path, - "/var/lib/pg/logs", - "/var/lib/pg/temp", + LOGS_STORAGE_PATH, + TEMP_STORAGE_PATH, ]: logger.info(f"Removing contents from {path}") - self.container.exec(f"find {path} -mindepth 1 -delete".split()).wait_output() + self.container.exec(["find", path, "-mindepth", "1", "-delete"]).wait_output() self.charm._create_pgdata(self.container) def update_async_replication_data(self) -> None: diff --git a/templates/patroni.yml.j2 b/templates/patroni.yml.j2 index f78abf50292..d2eb01aef3c 100644 --- a/templates/patroni.yml.j2 +++ b/templates/patroni.yml.j2 @@ -87,7 +87,7 @@ bootstrap: - auth-local: trust - encoding: UTF8 - data-checksums - - waldir: /var/lib/pg/logs/16/main/pg_wal + - 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 }} @@ -118,7 +118,7 @@ ctl: pod_ip: '{{ endpoint }}' postgresql: basebackup: - - waldir: /var/lib/pg/logs/16/main/pg_wal + - waldir: {{ logs_storage_path }}/16/main/pg_wal connect_address: '{{ endpoint }}:5432' data_dir: {{ pgdata_path }} bin_dir: /usr/lib/postgresql/{{ version }}/bin diff --git a/tests/integration/ha_tests/clean-data-dir.sh b/tests/integration/ha_tests/clean-data-dir.sh index 9fe6a9d92d7..b3c64c72e17 100755 --- a/tests/integration/ha_tests/clean-data-dir.sh +++ b/tests/integration/ha_tests/clean-data-dir.sh @@ -3,5 +3,5 @@ set -Eeuo pipefail find /var/lib/pg/archive -mindepth 1 -delete find /var/lib/pg/data/16/main -mindepth 1 -delete -find /var/lib/pg/logs/16/main/pg_wal -mindepth 1 -delete -find /var/lib/pg/temp/16/main/pgsql_tmp -mindepth 1 -delete +find /var/lib/pg/logs -mindepth 1 -delete +find /var/lib/pg/temp -mindepth 1 -delete diff --git a/tests/unit/test_backups.py b/tests/unit/test_backups.py index f0e83b874a2..034d415abaa 100644 --- a/tests/unit/test_backups.py +++ b/tests/unit/test_backups.py @@ -1741,6 +1741,8 @@ def test_pre_restore_checks(harness): "tls_ca_chain_filename", ["", "/var/lib/pg/data/pgbackrest-tls-ca-chain.crt"] ) def test_render_pgbackrest_conf_file(harness, tls_ca_chain_filename): + mock_pebble = MagicMock() + mock_pebble.get_services.return_value = [MagicMock()] with ( patch("ops.model.Container.start") as _start, patch("ops.model.Container.push") as _push, @@ -1750,6 +1752,7 @@ def test_render_pgbackrest_conf_file(harness, tls_ca_chain_filename): ) as _tls_ca_chain_filename, patch("charm.PostgreSQLBackups._retrieve_s3_parameters") as _retrieve_s3_parameters, patch("charm.PostgresqlOperatorCharm.get_available_resources", return_value=(4, 1024)), + patch("ops.model.Container.pebble", new_callable=PropertyMock(return_value=mock_pebble)), ): # Set up a mock for the `open` method, set returned data to postgresql.conf template. with open("templates/pgbackrest.conf.j2") as f: diff --git a/tests/unit/test_patroni.py b/tests/unit/test_patroni.py index 0e46b22fa31..6c49cac760b 100644 --- a/tests/unit/test_patroni.py +++ b/tests/unit/test_patroni.py @@ -12,7 +12,7 @@ from tenacity import RetryError, stop_after_delay, wait_fixed from charm import PostgresqlOperatorCharm -from constants import API_REQUEST_TIMEOUT, REWIND_USER +from constants import API_REQUEST_TIMEOUT, LOGS_STORAGE_PATH, REWIND_USER from patroni import Patroni, SwitchoverFailedError, SwitchoverNotSyncError from tests.helpers import PGDATA_PATH, STORAGE_PATH @@ -225,6 +225,7 @@ def test_render_patroni_yml_file(harness, patroni): endpoints=patroni._endpoints, namespace=patroni._namespace, storage_path=patroni._storage_path, + logs_storage_path=LOGS_STORAGE_PATH, pgdata_path=patroni._pgdata_path, superuser_password=patroni._superuser_password, replication_password=patroni._replication_password, @@ -263,6 +264,7 @@ def test_render_patroni_yml_file(harness, patroni): endpoints=patroni._endpoints, namespace=patroni._namespace, storage_path=patroni._storage_path, + logs_storage_path=LOGS_STORAGE_PATH, pgdata_path=patroni._pgdata_path, superuser_password=patroni._superuser_password, replication_password=patroni._replication_password, From 2b195ea1d1d2fb2e48830a91bc1a87fdf0433f1e Mon Sep 17 00:00:00 2001 From: Marcelo Henrique Neppel Date: Wed, 11 Feb 2026 16:17:47 -0300 Subject: [PATCH 21/24] test: add unit tests for pgdata management, async replication, and patroni slots Signed-off-by: Marcelo Henrique Neppel --- tests/unit/test_async_replication.py | 143 +++++++++++++++++++++++++- tests/unit/test_backups.py | 11 ++ tests/unit/test_charm.py | 148 +++++++++++++++++++++++++++ tests/unit/test_patroni.py | 73 +++++++++++++ 4 files changed, 374 insertions(+), 1 deletion(-) diff --git a/tests/unit/test_async_replication.py b/tests/unit/test_async_replication.py index 0ad84ab111f..64377dac43b 100644 --- a/tests/unit/test_async_replication.py +++ b/tests/unit/test_async_replication.py @@ -2,7 +2,7 @@ # See LICENSE file for licensing details. import json -from unittest.mock import PropertyMock, patch +from unittest.mock import MagicMock, PropertyMock, patch import pytest from ops.testing import Harness @@ -386,3 +386,144 @@ def test_on_secret_changed(harness, relation_name): harness.get_relation_data(rel_id, harness.charm.app.name).get("primary-cluster-data") ) assert primary_cluster_data.get("secret-id") != updated_cluster_data.get("secret-id") + + +def test_clear_pgdata(harness): + with ( + patch("ops.model.Container.exec") as _exec, + patch("charm.PostgresqlOperatorCharm._create_pgdata") as _create_pgdata, + ): + mock_process = MagicMock() + mock_process.wait_output.return_value = ("", "") + _exec.return_value = mock_process + + harness.set_can_connect("postgresql", True) + harness.charm.async_replication._clear_pgdata() + + # Should be called 4 times: archive, actual pgdata, logs, temp + assert _exec.call_count == 4 + _exec.assert_any_call(["find", "/var/lib/pg/archive", "-mindepth", "1", "-delete"]) + _exec.assert_any_call(["find", "/var/lib/pg/data/16/main", "-mindepth", "1", "-delete"]) + _exec.assert_any_call(["find", "/var/lib/pg/logs", "-mindepth", "1", "-delete"]) + _exec.assert_any_call(["find", "/var/lib/pg/temp", "-mindepth", "1", "-delete"]) + _create_pgdata.assert_called_once() + + +def test_stop_database_leader_clears_pgdata(harness): + with ( + patch( + "relations.async_replication.PostgreSQLAsyncReplication._get_unit_ip", + return_value="1.1.1.1", + ), + ): + peer_rel_id = harness.add_relation(PEER, harness.charm.app.name) + + with ( + patch("ops.model.Container.stop") as _stop, + patch( + "charm.PostgresqlOperatorCharm.is_unit_stopped", + new_callable=PropertyMock, + return_value=False, + ), + patch( + "relations.async_replication.PostgreSQLAsyncReplication._is_following_promoted_cluster", + return_value=False, + ), + patch("ops.model.Container.exists", return_value=True), + patch( + "relations.async_replication.PostgreSQLAsyncReplication._configure_standby_cluster", + return_value=True, + ), + patch( + "relations.async_replication.PostgreSQLAsyncReplication._clear_pgdata" + ) as _clear_pgdata, + ): + harness.set_can_connect("postgresql", True) + + event = MagicMock() + result = harness.charm.async_replication._stop_database(event) + + assert result is True + _stop.assert_called_once() + _clear_pgdata.assert_called_once() + assert ( + harness.get_relation_data(peer_rel_id, harness.charm.unit.name).get("stopped") + == "True" + ) + + +def test_wait_for_standby_leader_non_leader_clears_pgdata(harness): + with ( + patch( + "relations.async_replication.PostgreSQLAsyncReplication._get_unit_ip", + return_value="1.1.1.1", + ), + ): + peer_rel_id = harness.add_relation(PEER, harness.charm.app.name) + harness.add_relation( + REPLICATION_OFFER_RELATION, + harness.charm.app.name, + unit_data={"unit-address": "10.1.1.10"}, + ) + + with ( + patch( + "charm.Patroni.get_standby_leader", + return_value="postgresql-k8s-0", + ), + patch( + "relations.async_replication.PostgreSQLAsyncReplication._clear_pgdata" + ) as _clear_pgdata, + ): + harness.set_leader(False) + + event = MagicMock() + result = harness.charm.async_replication._wait_for_standby_leader(event) + + assert result is False + _clear_pgdata.assert_called_once() + assert ( + harness.get_relation_data(peer_rel_id, harness.charm.unit.name).get( + "standby-pgdata-cleared" + ) + == "True" + ) + + +def test_wait_for_standby_leader_non_leader_skips_if_already_cleared(harness): + with ( + patch( + "relations.async_replication.PostgreSQLAsyncReplication._get_unit_ip", + return_value="1.1.1.1", + ), + ): + peer_rel_id = harness.add_relation(PEER, harness.charm.app.name) + harness.add_relation( + REPLICATION_OFFER_RELATION, + harness.charm.app.name, + unit_data={"unit-address": "10.1.1.10"}, + ) + + with harness.hooks_disabled(): + harness.update_relation_data( + peer_rel_id, + harness.charm.unit.name, + {"standby-pgdata-cleared": "True"}, + ) + + with ( + patch( + "charm.Patroni.get_standby_leader", + return_value="postgresql-k8s-0", + ), + patch( + "relations.async_replication.PostgreSQLAsyncReplication._clear_pgdata" + ) as _clear_pgdata, + ): + harness.set_leader(False) + + event = MagicMock() + result = harness.charm.async_replication._wait_for_standby_leader(event) + + assert result is False + _clear_pgdata.assert_not_called() diff --git a/tests/unit/test_backups.py b/tests/unit/test_backups.py index 034d415abaa..cd42cf981a4 100644 --- a/tests/unit/test_backups.py +++ b/tests/unit/test_backups.py @@ -433,6 +433,17 @@ def test_empty_data_files(harness): for call, expected in zip(_exec.call_args_list, expected_calls, strict=True): assert call[0] == expected + # Test when exec raises ExecError with "No such file or directory" — should not raise. + _exec.reset_mock() + _exec.side_effect = ExecError( + command=["find", "/var/lib/pg/archive", "-mindepth", "1", "-delete"], + exit_code=1, + stdout="", + stderr="find: '/var/lib/pg/archive': No such file or directory", + ) + # Should complete without raising. + harness.charm.backup._empty_data_files() + def test_change_connectivity_to_database(harness): with patch("charm.PostgresqlOperatorCharm.update_config") as _update_config: diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py index cceabb9d3f3..ea1aa891df4 100644 --- a/tests/unit/test_charm.py +++ b/tests/unit/test_charm.py @@ -2031,3 +2031,151 @@ def test_calculate_worker_process_config_all_workers_validation_blocking(harness result = harness.charm._calculate_worker_process_config(cpu_cores) assert result["max_parallel_workers"] == "18" # Should accept valid value + + +def test_replica_can_start(harness): + with ( + patch( + "charm.PostgresqlOperatorCharm.is_cluster_initialised", + new_callable=PropertyMock, + ) as _is_cluster_initialised, + patch( + "charm.PostgresqlOperatorCharm._endpoint", + new_callable=PropertyMock, + ) as _endpoint, + patch( + "charm.PostgresqlOperatorCharm._endpoints", + new_callable=PropertyMock, + ) as _endpoints, + ): + # Returns False when cluster is not initialised. + _is_cluster_initialised.return_value = False + _endpoint.return_value = "postgresql-k8s-0" + _endpoints.return_value = ["postgresql-k8s-0"] + assert harness.charm._replica_can_start() is False + + # Returns False when endpoint is not in members list. + _is_cluster_initialised.return_value = True + _endpoint.return_value = "postgresql-k8s-1" + _endpoints.return_value = ["postgresql-k8s-0"] + assert harness.charm._replica_can_start() is False + + # Returns True when both conditions are met. + _endpoint.return_value = "postgresql-k8s-0" + _endpoints.return_value = ["postgresql-k8s-0", "postgresql-k8s-1"] + assert harness.charm._replica_can_start() is True + + +def test_create_pgdata_clears_stale_dirs_on_replica(harness): + container = MagicMock() + + # Simulate: waldir and temp exist, but PG_VERSION does not (pgdata not populated). + def exists_side_effect(path): + if path.endswith("PG_VERSION"): + return False + return "pg_wal" in path or "pgsql_tmp" in path + + container.exists.side_effect = exists_side_effect + + with ( + patch( + "charm.PostgresqlOperatorCharm.is_cluster_initialised", + new_callable=PropertyMock, + return_value=True, + ), + ): + harness.set_leader(False) + harness.charm._create_pgdata(container) + + # Verify that find -delete was called for waldir and temp_tablespace paths. + find_calls = [c for c in container.exec.call_args_list if "find" in str(c)] + assert len(find_calls) >= 2 + container.exec.assert_any_call( + ["find", "/var/lib/pg/logs/16/main/pg_wal", "-mindepth", "1", "-delete"] + ) + container.exec.assert_any_call( + ["find", "/var/lib/pg/temp/16/main/pgsql_tmp", "-mindepth", "1", "-delete"] + ) + + +def test_create_pgdata_stale_dir_clearing_reraises_unexpected_exec_error(harness): + from ops.pebble import ExecError + + container = MagicMock() + + def exists_side_effect(path): + if path.endswith("PG_VERSION"): + return False + return "pg_wal" in path + + container.exists.side_effect = exists_side_effect + + # Make exec raise ExecError with a non "No such file or directory" message. + mock_process = MagicMock() + mock_process.wait_output.side_effect = ExecError( + command=["find"], exit_code=1, stdout="", stderr="Permission denied" + ) + container.exec.return_value = mock_process + + with ( + patch( + "charm.PostgresqlOperatorCharm.is_cluster_initialised", + new_callable=PropertyMock, + return_value=True, + ), + ): + harness.set_leader(False) + # Should NOT raise — the ExecError with non "No such file" stderr is logged as warning + # but not re-raised. + harness.charm._create_pgdata(container) + + +def test_create_pgdata_stale_dir_clearing_ignores_no_such_file(harness): + from ops.pebble import ExecError + + container = MagicMock() + + def exists_side_effect(path): + if path.endswith("PG_VERSION"): + return False + return "pg_wal" in path + + container.exists.side_effect = exists_side_effect + + # Make exec raise ExecError with "No such file or directory" message. + mock_process = MagicMock() + mock_process.wait_output.side_effect = ExecError( + command=["find"], exit_code=1, stdout="", stderr="No such file or directory" + ) + container.exec.return_value = mock_process + + with ( + patch( + "charm.PostgresqlOperatorCharm.is_cluster_initialised", + new_callable=PropertyMock, + return_value=True, + ), + ): + harness.set_leader(False) + # Should not raise — graceful handling of missing directory. + harness.charm._create_pgdata(container) + + +def test_on_postgresql_pebble_ready_defers_when_replica_not_ready(harness): + with ( + patch("charm.PostgresqlOperatorCharm._create_pgdata") as _create_pgdata, + patch("charm.PostgresqlOperatorCharm._replica_can_start", return_value=False), + patch( + "charm.PostgresqlOperatorCharm._update_pebble_layers" + ) as _update_pebble_layers, + ): + harness.set_leader(False) + harness.set_can_connect(POSTGRESQL_CONTAINER, True) + mock_event = MagicMock() + mock_event.workload = harness.model.unit.get_container(POSTGRESQL_CONTAINER) + + harness.charm._on_postgresql_pebble_ready(mock_event) + + _create_pgdata.assert_called_once() + mock_event.defer.assert_called_once() + _update_pebble_layers.assert_not_called() diff --git a/tests/unit/test_patroni.py b/tests/unit/test_patroni.py index 6c49cac760b..702a05417f0 100644 --- a/tests/unit/test_patroni.py +++ b/tests/unit/test_patroni.py @@ -531,3 +531,76 @@ def test_update_synchronous_node_count(harness, patroni): with pytest.raises(RetryError): patroni.update_synchronous_node_count() assert False + + +def test_ensure_slots_controller_by_patroni(harness, patroni): + with ( + patch("patroni.stop_after_delay", return_value=stop_after_delay(0)), + patch("patroni.wait_fixed", return_value=wait_fixed(0)), + patch("requests.get") as _get, + patch("requests.patch") as _patch, + ): + # Success path: no existing slots, add one new slot. + get_response = MagicMock() + get_response.status_code = 200 + get_response.json.return_value = {"slots": {}} + get_response.elapsed.total_seconds.return_value = 0.1 + _get.return_value = get_response + + patch_response = MagicMock() + patch_response.status_code = 200 + _patch.return_value = patch_response + + result = patroni.ensure_slots_controller_by_patroni({"slot1": "db1"}) + assert result is True + _patch.assert_called_once_with( + f"{patroni._patroni_url}/config", + verify=patroni._verify, + json={"slots": {"slot1": {"database": "db1", "plugin": "pgoutput", "type": "logical"}}}, + auth=patroni._patroni_auth, + timeout=API_REQUEST_TIMEOUT, + ) + + # Existing slots are merged: old_slot should be set to None (removed). + _get.reset_mock() + _patch.reset_mock() + get_response.json.return_value = { + "slots": {"old_slot": {"database": "old_db", "plugin": "pgoutput", "type": "logical"}} + } + _get.return_value = get_response + _patch.return_value = patch_response + + result = patroni.ensure_slots_controller_by_patroni({"slot1": "db1"}) + assert result is True + expected_slots = { + "old_slot": None, + "slot1": {"database": "db1", "plugin": "pgoutput", "type": "logical"}, + } + _patch.assert_called_once_with( + f"{patroni._patroni_url}/config", + verify=patroni._verify, + json={"slots": expected_slots}, + auth=patroni._patroni_auth, + timeout=API_REQUEST_TIMEOUT, + ) + + # RetryError path: 502 status code. + _get.reset_mock() + _patch.reset_mock() + get_response_502 = MagicMock() + get_response_502.status_code = 502 + get_response_502.elapsed.total_seconds.return_value = 0.1 + _get.return_value = get_response_502 + + result = patroni.ensure_slots_controller_by_patroni({"slot1": "db1"}) + assert result is False + _patch.assert_not_called() + + # RetryError path: connection error. + _get.reset_mock() + _patch.reset_mock() + _get.side_effect = requests.exceptions.ConnectionError("connection refused") + + result = patroni.ensure_slots_controller_by_patroni({"slot1": "db1"}) + assert result is False + _patch.assert_not_called() From 047341759c7f6789f267f9932daa751962a8ada1 Mon Sep 17 00:00:00 2001 From: Marcelo Henrique Neppel Date: Wed, 11 Feb 2026 16:45:23 -0300 Subject: [PATCH 22/24] docs: fix broken Juju 3.0 release notes URLs Signed-off-by: Marcelo Henrique Neppel --- docs/explanation/juju.md | 2 +- docs/how-to/data-migration/migrate-data-via-pg-dump.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/explanation/juju.md b/docs/explanation/juju.md index 91d33bb3cd6..8504104fc42 100644 --- a/docs/explanation/juju.md +++ b/docs/explanation/juju.md @@ -26,7 +26,7 @@ In the context of this documentation, the pertinent changes are as follows: |`run`|`exec`| |`run-action --wait`|`run`| -See the [Juju 3.0 release notes](https://documentation.ubuntu.com/juju/3.6/reference/juju/juju-roadmap-and-releases/#juju-3-0-0-22-oct-2022) for the comprehensive list of changes. +See the [Juju 3.0 release notes](https://documentation.ubuntu.com/juju/3.6/releasenotes/unsupported/juju_3.x.x/#juju-3-0) for the comprehensive list of changes. Example substitutions: diff --git a/docs/how-to/data-migration/migrate-data-via-pg-dump.md b/docs/how-to/data-migration/migrate-data-via-pg-dump.md index 85aaaed97be..6a1287ff06b 100644 --- a/docs/how-to/data-migration/migrate-data-via-pg-dump.md +++ b/docs/how-to/data-migration/migrate-data-via-pg-dump.md @@ -21,7 +21,7 @@ Always test migration in a safe environment before performing it in production! - **[Your application is compatible](/explanation/legacy-charm) with Charmed PostgreSQL K8s** - A client machine with access to the deployed legacy charm -- `juju v.2.9` or later (check [Juju 3.0 Release Notes](https://documentation.ubuntu.com/juju/3.6/reference/juju/juju-roadmap-and-releases/#juju-3-0-0-22-oct-2022) for more information about key differences) +- `juju v.2.9` or later (check [Juju 3.0 Release Notes](https://documentation.ubuntu.com/juju/3.6/releasenotes/unsupported/juju_3.x.x/#juju-3-0) for more information about key differences) - Enough storage in the cluster to support backup/restore of the databases. ## Obtain existing database credentials From 24ccb4fd542101b2934402acfc10c04e903bfdce Mon Sep 17 00:00:00 2001 From: Marcelo Henrique Neppel Date: Thu, 12 Feb 2026 17:25:30 -0300 Subject: [PATCH 23/24] fix(metadata): update postgresql-image upstream source to canonical GHCR Signed-off-by: Marcelo Henrique Neppel --- metadata.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/metadata.yaml b/metadata.yaml index d265412ba4a..37e0931e881 100644 --- a/metadata.yaml +++ b/metadata.yaml @@ -41,7 +41,7 @@ resources: postgresql-image: type: oci-image description: OCI image for PostgreSQL - upstream-source: docker.io/neppel/charmed-postgresql_16-24.04_edge@sha256:2ab7cdddf774ba958e5b748b6a0f14f81a98716c462cf41672eeb416a8bba884 + upstream-source: ghcr.io/canonical/charmed-postgresql@sha256:7ee3f31ce88a48a94634d852a50e6cf3c50c6773eb885e4b896d3039150aa633 peers: database-peers: From 6689a53216777bac257d5bb9506c09376bd0aa1b Mon Sep 17 00:00:00 2001 From: Marcelo Henrique Neppel Date: Fri, 13 Feb 2026 08:52:42 -0300 Subject: [PATCH 24/24] ci: restore renovate hint for postgresql oci-image resource Signed-off-by: Marcelo Henrique Neppel --- metadata.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/metadata.yaml b/metadata.yaml index 1e7039e0dc0..05813459741 100644 --- a/metadata.yaml +++ b/metadata.yaml @@ -41,7 +41,7 @@ resources: postgresql-image: type: oci-image description: OCI image for PostgreSQL - upstream-source: ghcr.io/canonical/charmed-postgresql@sha256:7ee3f31ce88a48a94634d852a50e6cf3c50c6773eb885e4b896d3039150aa633 + upstream-source: ghcr.io/canonical/charmed-postgresql@sha256:7ee3f31ce88a48a94634d852a50e6cf3c50c6773eb885e4b896d3039150aa633 # renovate: oci-image tag: 16.11-24.04_edge peers: database-peers: