feat(storage): migrate all data mounts to versioned 16/main subdirectories

refresh_versions.toml

@@ -1,11 +1,14 @@
 charm_major = 1
 workload = "16.13"
+charm = "16/1.999.0" # Unique version for upgrade test
 
 [snap]
 name = "charmed-postgresql"
 
 [snap.revisions]
 # amd64
-x86_64 = "289"
-# arm64
-aarch64 = "288"
+x86_64 = "329"
+# arm64 (Linux)
+aarch64 = "330"
+# arm64 (macOS / Apple Silicon - same snap revision)
+arm64 = "330"

src/backups.py

@@ -30,9 +30,11 @@
 from tenacity import RetryError, Retrying, stop_after_attempt, wait_fixed
 
 from constants import (
+    ARCHIVE_DATA_DIR,
     BACKUP_ID_FORMAT,
     BACKUP_TYPE_OVERRIDES,
     BACKUP_USER,
+    LOGS_DATA_DIR,
     PATRONI_CONF_PATH,
     PGBACKREST_ARCHIVE_TIMEOUT_ERROR_CODE,
     PGBACKREST_BACKUP_ID_FORMAT,
@@ -42,7 +44,8 @@
     PGBACKREST_LOG_LEVEL_STDERR,
     PGBACKREST_LOGROTATE_FILE,
     PGBACKREST_LOGS_PATH,
-    POSTGRESQL_DATA_PATH,
+    POSTGRESQL_DATA_DIR,
+    TEMP_DATA_DIR,
     UNIT_SCOPE,
 )
 from relations.async_replication import REPLICATION_CONSUMER_RELATION, REPLICATION_OFFER_RELATION
@@ -245,7 +248,7 @@ def can_use_s3_repository(self) -> tuple[bool, str]:
 
             return_code, system_identifier_from_instance, error = self._execute_command([
                 f"/snap/charmed-postgresql/current/usr/lib/postgresql/{self.charm._patroni.get_postgresql_version().split('.')[0]}/bin/pg_controldata",
-                POSTGRESQL_DATA_PATH,
+                POSTGRESQL_DATA_DIR,
             ])
             if return_code != 0:
                 raise Exception(error)
@@ -353,10 +356,10 @@ def _create_bucket_if_not_exists(self) -> None:
     def _empty_data_files(self) -> bool:
         """Empty the PostgreSQL data directory in preparation of backup restore."""
         paths = [
-            "/var/snap/charmed-postgresql/common/data/archive",
-            POSTGRESQL_DATA_PATH,
-            "/var/snap/charmed-postgresql/common/data/logs",
-            "/var/snap/charmed-postgresql/common/data/temp",
+            ARCHIVE_DATA_DIR,
+            POSTGRESQL_DATA_DIR,
+            LOGS_DATA_DIR,
+            TEMP_DATA_DIR,
         ]
         path = None
         try:
@@ -1379,7 +1382,7 @@ def _render_pgbackrest_conf_file(self) -> bool:
             enable_tls=len(self.charm._peer_members_ips) > 0,
             peer_endpoints=self.charm._peer_members_ips,
             path=s3_parameters["path"],
-            data_path=f"{POSTGRESQL_DATA_PATH}",
+            data_path=POSTGRESQL_DATA_DIR,
             log_path=f"{PGBACKREST_LOGS_PATH}",
             region=s3_parameters.get("region"),
             endpoint=s3_parameters["endpoint"],

src/charm.py

@@ -11,6 +11,7 @@
 import pathlib
 import platform
 import re
+import shutil
 import subprocess
 import sys
 import time
@@ -113,7 +114,7 @@
     PGBACKREST_METRICS_PORT,
     PGBACKREST_MONITORING_SNAP_SERVICE,
     PLUGIN_OVERRIDES,
-    POSTGRESQL_DATA_PATH,
+    POSTGRESQL_DATA_DIR,
     RAFT_PASSWORD_KEY,
     RAFT_PORT,
     REPLICATION_CONSUMER_RELATION,
@@ -123,7 +124,10 @@
     SECRET_DELETED_LABEL,
     SECRET_INTERNAL_LABEL,
     SECRET_KEY_OVERRIDES,
+    SNAP_DAEMON_USER,
     SPI_MODULE,
+    TEMP_DATA_DIR,
+    TEMP_STORAGE_PATH,
     TLS_CA_BUNDLE_FILE,
     TLS_CA_FILE,
     TLS_CERT_FILE,
@@ -248,6 +252,12 @@ def refresh_snap(
         self._charm.set_unit_status(MaintenanceStatus("updating configuration"), refresh=refresh)
         self._charm.update_config(refresh=refresh)
 
+        # Ensure the temp tablespace directory exists before the snap refresh.
+        # Data migration between storage roots and versioned paths is handled
+        # by the snap's post-refresh hook (forward) and pre-refresh hook
+        # (reverse), which start a one-shot daemon running as _daemon_.
+        self._charm._ensure_storage_layout()
+
         # TODO add graceful shutdown before refreshing snap?
         # TODO future improvement: if snap refresh fails (i.e. same snap revision installed) after
         # graceful shutdown, restart workload
@@ -411,11 +421,8 @@ def __init__(self, *args):
         self.tracing = Tracing(self, tracing_relation_name=TRACING_RELATION_NAME)
         charm_tracing_config(self._grafana_agent)
 
-    def _post_snap_refresh(self, refresh: charm_refresh.Machines):
-        """Start PostgreSQL, check if this app and unit are healthy, and allow next unit to refresh.
-
-        Called after snap refresh
-        """
+    def _check_and_update_internal_cert(self) -> None:
+        """Check if the internal cert CN matches the unit IP and regenerate if needed."""
         try:
             if (
                 (raw_cert := self.get_secret(UNIT_SCOPE, "internal-cert"))
@@ -429,6 +436,14 @@ def _post_snap_refresh(self, refresh: charm_refresh.Machines):
         except Exception:
             logger.exception("Unable to check or update internal cert")
 
+    def _post_snap_refresh(self, refresh: charm_refresh.Machines):
+        """Start PostgreSQL, check if this app and unit are healthy, and allow next unit to refresh.
+
+        Called after snap refresh
+        """
+        self._check_and_update_internal_cert()
+        self._ensure_storage_layout()
+
         if not self._patroni.start_patroni():
             self.set_unit_status(BlockedStatus("Failed to start PostgreSQL"), refresh=refresh)
             return
@@ -440,7 +455,7 @@ def _post_snap_refresh(self, refresh: charm_refresh.Machines):
         # Wait until the database initialise.
         self.set_unit_status(WaitingStatus("waiting for database initialisation"), refresh=refresh)
         try:
-            for attempt in Retrying(stop=stop_after_attempt(6), wait=wait_fixed(10)):
+            for attempt in Retrying(stop=stop_after_attempt(30), wait=wait_fixed(10)):
                 with attempt:
                     # Check if the member hasn't started or hasn't joined the cluster yet.
                     if (
@@ -458,6 +473,7 @@ def _post_snap_refresh(self, refresh: charm_refresh.Machines):
                 "Did not allow next unit to refresh: member not ready or not joined the cluster yet"
             )
         else:
+            self._migrate_temp_tablespace_location()
             try:
                 self._patroni.set_max_timelines_history()
             except Exception:
@@ -652,6 +668,130 @@ def is_unit_stopped(self) -> bool:
         """Returns whether the unit is stopped."""
         return "stopped" in self.unit_peer_data
 
+    def _ensure_storage_layout(self) -> None:
+        """Ensure the temp tablespace directory exists.
+
+        Data migration between storage roots and versioned 16/main
+        subdirectories is handled by the snap hooks (pre-refresh for
+        reverse, post-refresh for forward).  TEMP_DATA_DIR may live on
+        a tmpfs mount that is wiped on reboot, so we recreate it
+        unconditionally.  CREATE TABLESPACE requires the directory to
+        be writable by the PostgreSQL _daemon_ user, so we chown it.
+
+        The 16/ parent dir must also be _daemon_-owned: the snap daemon
+        runs as _daemon_ and needs write permission to clean up the
+        versioned subdirectory and run DROP/CREATE TABLESPACE during
+        rollback (handled by the snap's pre-refresh hook).
+        """
+        temp_dir = Path(TEMP_DATA_DIR)
+        temp_dir.mkdir(parents=True, exist_ok=True)
+        shutil.chown(temp_dir, user=SNAP_DAEMON_USER, group=SNAP_DAEMON_USER)
+        if temp_dir.parent.exists():
+            shutil.chown(temp_dir.parent, user=SNAP_DAEMON_USER, group=SNAP_DAEMON_USER)
+
+    @staticmethod
+    def _clear_pg_version_dirs(path: Path) -> None:
+        """Remove PostgreSQL version subdirectories (PG_<ver>_<catalog>) from a directory.
+
+        These directories are created by PostgreSQL when a tablespace is created and must not
+        exist at a target path before CREATE TABLESPACE is called.  Temp tablespace data is
+        ephemeral, so removal is safe.
+        """
+        if path.exists():
+            for entry in path.iterdir():
+                if entry.name.startswith("PG_"):
+                    shutil.rmtree(entry)
+
+    def _migrate_temp_tablespace_location(self) -> bool:
+        """One-shot migration of the temp tablespace to the versioned directory.
+
+        During a snap upgrade, the post-refresh hook migrates temp data from the
+        old non-versioned storage root (TEMP_STORAGE_PATH) to the versioned
+        subdirectory (TEMP_DATA_DIR).  This method updates the PostgreSQL catalog
+        entry to match.
+
+        During a snap downgrade (rollback), the pre-refresh hook handles both
+        file migration and catalog migration (DROP/CREATE TABLESPACE) back to
+        the non-versioned root.  This method only handles the forward case.
+
+        Other temp tablespace recovery scenarios (missing catalog entry after a
+        partially-failed migration, empty directory after a tmpfs wipe) are
+        handled by the single-kernel library's set_up_database during the
+        leader-elected event.
+
+        DROP TABLESPACE and CREATE TABLESPACE cannot run inside a transaction
+        block, so this method avoids using the connection as a context manager
+        (which would create one in psycopg2).  Instead it uses plain assignments
+        and explicit close(), mirroring the pattern in the single_kernel_postgresql
+        set_up_database helper.
+        """
+        if not self.is_primary:
+            return True
+
+        if not self.primary_endpoint:
+            logger.debug("Primary endpoint not yet available; skipping temp tablespace check")
+            return True
+
+        # Do not migrate the temp tablespace while cross-cluster async replication is
+        # active.  The DROP/CREATE TABLESPACE generates WAL that is streamed to the
+        # standby cluster.  If the standby has not been upgraded to the versioned
+        # storage layout yet, it will not have the TEMP_DATA_DIR directory, causing
+        # PostgreSQL to crash with "FATAL: directory does not exist" during WAL replay.
+        if self.async_replication._relation is not None:
+            logger.debug("Skipping temp tablespace migration while async replication is active")
+            return True
+
+        connection = None
+        cursor = None
+        try:
+            connection = self.postgresql._connect_to_database()
+            connection.autocommit = True  # DROP/CREATE TABLESPACE cannot run inside a transaction
+            cursor = connection.cursor()
+
+            cursor.execute(
+                "SELECT pg_tablespace_location(oid) FROM pg_tablespace WHERE spcname='temp';"
+            )
+            row = cursor.fetchone()
+            if row is None:
+                # Tablespace was already dropped by a previous migration or was
+                # never created (e.g. fresh deploy).  Nothing to migrate.
+                return True
+
+            current_location = row[0]
+            if current_location == TEMP_DATA_DIR:
+                # Already at the versioned path.  Nothing to migrate.
+                return True
+
+            if current_location != TEMP_STORAGE_PATH:
+                logger.warning(
+                    "Skipping temp tablespace migration: unexpected location %s "
+                    "(expected %s or %s)",
+                    current_location,
+                    TEMP_STORAGE_PATH,
+                    TEMP_DATA_DIR,
+                )
+                return True
+
+            logger.info(
+                "Migrating temp tablespace location from %s to %s",
+                TEMP_STORAGE_PATH,
+                TEMP_DATA_DIR,
+            )
+            cursor.execute("DROP TABLESPACE temp;")
+            self._clear_pg_version_dirs(Path(TEMP_DATA_DIR))
+            cursor.execute(f"CREATE TABLESPACE temp LOCATION '{TEMP_DATA_DIR}';")
+            cursor.execute("GRANT CREATE ON TABLESPACE temp TO public;")
+        except psycopg2.Error:
+            logger.exception("Failed to migrate temp tablespace location")
+            return False
+        finally:
+            if cursor is not None:
+                cursor.close()
+            if connection is not None:
+                connection.close()
+
+        return True
+
     @cached_property
     def postgresql(self) -> PostgreSQL:
         """Returns an instance of the object used to interact with the database."""
@@ -1000,15 +1140,7 @@ def _on_peer_relation_changed(self, event: HookEvent):
             event.defer()
             return
 
-        # In Raft mode with a watcher, ensure this member is properly registered in the DCS.
-        # A new member may be running but not registered if it was added to Raft after starting.
-        if (
-            self.watcher_offer.is_watcher_connected
-            and not self._patroni.is_member_registered_in_cluster()
-        ):
-            logger.info("Member running but not registered in Raft cluster - restarting Patroni")
-            self._patroni.restart_patroni()
-            event.defer()
+        if not self._check_member_registration(event):
             return
 
         self._start_stop_pgbackrest_service(event)
@@ -1024,6 +1156,23 @@ def _on_peer_relation_changed(self, event: HookEvent):
 
         self._update_new_unit_status()
 
+    # Split off into separate function, because of complexity _on_peer_relation_changed
+    def _check_member_registration(self, event: HookEvent) -> bool:
+        """Check and ensure the member is registered in the Raft/replication cluster.
+
+        Returns:
+            True if processing should continue, False if we should return early.
+        """
+        if (
+            self.watcher_offer.is_watcher_connected
+            and not self._patroni.is_member_registered_in_cluster()
+        ):
+            logger.info("Member running but not registered in Raft cluster - restarting Patroni")
+            self._patroni.restart_patroni()
+            event.defer()
+            return False
+        return True
+
     # Split off into separate function, because of complexity _on_peer_relation_changed
     def _handle_s3_initialization(self, event: HookEvent) -> bool:
         """Handle S3 initialization during peer relation changes.
@@ -1670,6 +1819,7 @@ def _on_start(self, event: StartEvent) -> None:
             self.tls.generate_internal_peer_cert()
 
         self.unit_peer_data.update({"ip": self._unit_ip})
+        self._ensure_storage_layout()
 
         # Open port
         try:
@@ -1813,9 +1963,7 @@ def _setup_users(self) -> None:
                 extra_user_roles=[ROLE_STATS],
             )
 
-        self.postgresql.set_up_database(
-            temp_location="/var/snap/charmed-postgresql/common/data/temp"
-        )
+        self.postgresql.set_up_database(temp_location=TEMP_DATA_DIR)
 
         access_groups = self.postgresql.list_access_groups()
         if access_groups != set(ACCESS_GROUPS):
@@ -2001,7 +2149,7 @@ def promote_primary_unit(self, event: ActionEvent) -> None:
             except SwitchoverFailedError:
                 event.fail("Switchover failed or timed out, check the logs for details")
 
-    def _on_update_status(self, _) -> None:
+    def _on_update_status(self, event) -> None:
         """Update the unit status message and users list in the database."""
         if not self._can_run_on_update_status():
             return
@@ -2014,6 +2162,9 @@ def _on_update_status(self, _) -> None:
         if self._handle_processes_failures():
             return
 
+        if self.unit.is_leader() and not self._reconfigure_cluster(event):
+            return
+
         self.postgresql_client_relation.oversee_users()
         if self.primary_endpoint:
             self._update_relation_endpoints()
@@ -2143,11 +2294,11 @@ def _handle_processes_failures(self) -> bool:
         # Restart the PostgreSQL process if it was frozen (in that case, the Patroni
         # process is running by the PostgreSQL process not).
         if self._unit_ip in self.members_ips and self._patroni.member_inactive:
-            data_directory_contents = os.listdir(POSTGRESQL_DATA_PATH)
+            data_directory_contents = os.listdir(POSTGRESQL_DATA_DIR)
             if len(data_directory_contents) == 1 and data_directory_contents[0] == "pg_wal":
                 os.rename(
-                    os.path.join(POSTGRESQL_DATA_PATH, "pg_wal"),
-                    os.path.join(POSTGRESQL_DATA_PATH, f"pg_wal-{datetime.now(UTC).isoformat()}"),
+                    os.path.join(POSTGRESQL_DATA_DIR, "pg_wal"),
+                    os.path.join(POSTGRESQL_DATA_DIR, f"pg_wal-{datetime.now(UTC).isoformat()}"),
                 )
                 logger.info("PostgreSQL data directory was not empty. Moved pg_wal")
                 return True