[DPE-9356] Update PostgreSQL mount paths to /var/lib/pg with 16/main subdirectory

docs/explanation/logs.md

@@ -78,7 +78,7 @@ The naming convention of the Pgbackrest logs is `<model name>.patroni-<postgresq
 2023-10-11 13:07:45.146 P00   INFO: expire command end: completed successfully (353ms)
 root@pg-0:/# cat /var/log/pgbackrest/discourse.patroni-pg-backup.log 
 -------------------PROCESS START-------------------
-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/postgresql/data/pgdata --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=<redacted> --repo1-s3-key-secret=<redacted> --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=<redacted> --repo1-s3-key-secret=<redacted> --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

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 

metadata.yaml

@@ -29,19 +29,19 @@ 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:
     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: ghcr.io/canonical/charmed-postgresql@sha256:840100acce8597fe94f1ba5a4d8599a50d4acc9b351d9ec146d9ebb6099e5aa9  # renovate: oci-image tag: 16.11-24.04_edge
 
 peers:
   database-peers:
@@ -109,19 +109,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

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"

src/backups.py

@@ -31,9 +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,
 )
@@ -195,7 +199,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",
+                POSTGRESQL_DATA_PATH,
             ])
             if error != "":
                 raise Exception(error)
@@ -272,15 +276,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:
-            self.container.exec(["rm", "-r", "/var/lib/postgresql/data/pgdata"]).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 [
+            ARCHIVE_PATH,
+            self.charm._actual_pgdata_path,
+            LOGS_STORAGE_PATH,
+            TEMP_STORAGE_PATH,
+        ]:
+            try:
+                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):
+                    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."""
@@ -1212,6 +1225,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),
@@ -1234,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
 

src/charm.py

@@ -134,6 +134,7 @@
     SECRET_KEY_OVERRIDES,
     SPI_MODULE,
     SYSTEM_USERS,
+    TEMP_STORAGE_PATH,
     TLS_CA_BUNDLE_FILE,
     TLS_CA_FILE,
     TLS_CERT_FILE,
@@ -249,7 +250,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}/pgdata"
+        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)
@@ -1109,17 +1111,108 @@ 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 directory."""
-        if not container.exists(self.pgdata_path):
+        """Create the PostgreSQL data directories."""
+        logs_path = str(self.meta.storages["logs"].location)
+        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/pgsql_tmp"
+
+        # 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.
+        # 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:
+                        container.exec(["find", path, "-mindepth", "1", "-delete"]).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}")
+
+        # 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._actual_pgdata_path,
+                permissions=0o700,
+                user=WORKLOAD_OS_USER,
+                group=WORKLOAD_OS_GROUP,
+                make_parents=True,
+            )
+        # Create the WAL directory (e.g., /var/lib/pg/logs/16/main/pg_wal)
+        if not container.exists(waldir_path):
             container.make_dir(
-                self.pgdata_path, permissions=0o700, user=WORKLOAD_OS_USER, group=WORKLOAD_OS_GROUP
+                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/pgsql_tmp)
+        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,
+            )
+        # 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.
+        # 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",
             f"{WORKLOAD_OS_USER}:{WORKLOAD_OS_GROUP}",
-            "/var/lib/postgresql/archive",
+            str(self.meta.storages["archive"].location),
         ]).wait()
         container.exec([
             "chown",
@@ -1129,12 +1222,12 @@ def _create_pgdata(self, container: Container):
         container.exec([
             "chown",
             f"{WORKLOAD_OS_USER}:{WORKLOAD_OS_GROUP}",
-            "/var/lib/postgresql/logs",
+            logs_path,
         ]).wait()
         container.exec([
             "chown",
             f"{WORKLOAD_OS_USER}:{WORKLOAD_OS_GROUP}",
-            "/var/lib/postgresql/temp",
+            temp_path,
         ]).wait()
 
     def _on_start(self, _) -> None:
@@ -1170,15 +1263,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
 
@@ -1337,7 +1429,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=f"{TEMP_STORAGE_PATH}/16/main/pgsql_tmp")
 
         access_groups = self.postgresql.list_access_groups()
         if access_groups != set(ACCESS_GROUPS):
@@ -1587,6 +1679,7 @@ 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()
+
         if self.refresh is not None and not self.refresh.in_progress:
             try:
                 self._create_services()
@@ -1733,9 +1826,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(
@@ -1870,6 +1963,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),
@@ -2582,7 +2676,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

src/constants.py

@@ -19,8 +19,15 @@
 WORKLOAD_OS_USER = "postgres"
 METRICS_PORT = "9187"
 PGBACKREST_METRICS_PORT = "9854"
-POSTGRESQL_DATA_PATH = "/var/lib/postgresql/data/pgdata"
+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/*",