From 62661a2659365973476d6a110cbe1ded3e2ac6fd Mon Sep 17 00:00:00 2001 From: Marcelo Henrique Neppel Date: Wed, 22 Jul 2026 20:35:04 -0300 Subject: [PATCH 1/2] fix(charm): migrate temp tablespace to versioned path on restore MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A backup taken on a pre-versioned-storage revision restores the temp tablespace at the old non-versioned path. Install and refresh already migrate it to the versioned path, but restore did not — so after a cross-revision restore the tablespace stayed at the old location. A new replica's pg_basebackup then streams into the charm-created versioned directory nested under it, hitting a non-empty target and failing, and pgBackRest rejects the nested tablespace links with error [070]. Run the existing temp-tablespace migration in the restore-success path, before clearing the restoring-backup flags, so the restored primary is on the versioned layout before any replica streams from it or a backup runs. Signed-off-by: Marcelo Henrique Neppel --- src/charm.py | 6 ++++ tests/unit/test_charm.py | 69 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 75 insertions(+) diff --git a/src/charm.py b/src/charm.py index e0d5606dc3..e4c796d6b7 100755 --- a/src/charm.py +++ b/src/charm.py @@ -2434,6 +2434,12 @@ def _was_restore_successful(self) -> bool: self.enable_disable_extensions() + # Migrate an old-layout temp tablespace to the versioned path before replicas + # stream it, or their pg_basebackup hits a non-empty target directory. + if not self._migrate_temp_tablespace_location(required=True): + logger.debug("Restore check early exit: temp tablespace migration not complete") + return False + # Remove the restoring backup flag and the restore stanza name. self.app_peer_data.update({ "restoring-backup": "", diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py index 29e0d84100..e46c943f68 100644 --- a/tests/unit/test_charm.py +++ b/tests/unit/test_charm.py @@ -1029,6 +1029,72 @@ def test_ensure_storage_layout_recreates_temp_dir_on_reboot(harness, tmp_path): assert temp_root.is_dir() +def test_was_restore_successful_migrates_temp_tablespace_before_clearing_flags(harness): + """A restore migrates an old-layout temp tablespace to the versioned path. + + A backup taken on a pre-versioned-storage revision restores the temp tablespace + at the non-versioned path; the restore must migrate it to the versioned path + before replicas stream it, or their pg_basebackup hits a non-empty target dir. + """ + harness.disable_hooks() + harness.set_leader(True) + harness.charm.app_peer_data["restoring-backup"] = "20260722-130654F" + with ( + patch("charm.PatroniManager.get_member_status", return_value="running"), + patch( + "charm.PatroniManager.member_started", + new_callable=PropertyMock, + return_value=True, + ), + patch("charm.PostgresqlOperatorCharm._setup_users"), + patch("charm.PostgreSQL.get_current_timeline", return_value="2"), + patch("charm.PostgresqlOperatorCharm.enable_disable_extensions"), + patch("charm.PostgresqlOperatorCharm._migrate_temp_tablespace_location") as _migrate, + patch("charm.PostgresqlOperatorCharm.update_config"), + patch("charm.PostgresqlOperatorCharm.restore_patroni_restart_condition"), + patch("charm.PostgreSQLBackups.can_use_s3_repository", return_value=(True, None)), + ): + _migrate.return_value = True + + assert harness.charm._was_restore_successful() + + _migrate.assert_called_once_with(required=True) + # Flags are cleared only after a successful migration. + assert not harness.charm.app_peer_data.get("restoring-backup") + + +def test_was_restore_successful_defers_when_temp_tablespace_migration_incomplete(harness): + """When the temp tablespace migration is not complete, the restore check defers. + + The restoring-backup flag must not be cleared until the migration finishes, so + the check is retried on the next update-status. + """ + harness.disable_hooks() + harness.set_leader(True) + harness.charm.app_peer_data["restoring-backup"] = "20260722-130654F" + with ( + patch("charm.PatroniManager.get_member_status", return_value="running"), + patch( + "charm.PatroniManager.member_started", + new_callable=PropertyMock, + return_value=True, + ), + patch("charm.PostgresqlOperatorCharm._setup_users"), + patch("charm.PostgreSQL.get_current_timeline", return_value="2"), + patch("charm.PostgresqlOperatorCharm.enable_disable_extensions"), + patch("charm.PostgresqlOperatorCharm._migrate_temp_tablespace_location") as _migrate, + patch("charm.PostgresqlOperatorCharm.update_config") as _update_config, + ): + _migrate.return_value = False + + assert not harness.charm._was_restore_successful() + + _migrate.assert_called_once_with(required=True) + # The restoring-backup flag is preserved so the check retries. + assert harness.charm.app_peer_data["restoring-backup"] == "20260722-130654F" + _update_config.assert_not_called() + + def test_on_update_status(harness): with ( patch("charm.ClusterTopologyObserver.start_observer") as _start_observer, @@ -1144,6 +1210,9 @@ def test_on_update_status_after_restore_operation(harness): ) as _get_current_timeline, patch("charm.PostgresqlOperatorCharm._setup_users") as _setup_users, patch("charm.PostgresqlOperatorCharm.update_config") as _update_config, + patch( + "charm.PostgresqlOperatorCharm._migrate_temp_tablespace_location", return_value=True + ) as _migrate_temp_tablespace_location, patch("charm.PatroniManager.member_started", new_callable=PropertyMock) as _member_started, patch("charm.PatroniManager.get_member_status") as _get_member_status, patch( From 23b78d7da4d2647ce35bf35f303ddb8455a3c107 Mon Sep 17 00:00:00 2001 From: Marcelo Henrique Neppel Date: Wed, 22 Jul 2026 20:36:36 -0300 Subject: [PATCH 2/2] fix(charm): create temp tablespace dir with expected permissions On a tmpfs reboot the temp tablespace directory is recreated, and set_up_database renames the existing temp tablespace to a timestamped leftover (temp_) whenever the directory's mode differs from the expected storage permissions. _ensure_storage_layout created the directory without setting its mode, so the default umask left it at a mode that never matched, triggering the rename on every reboot and accumulating leftover tablespaces the operator has to clean up. Set the directory mode to the expected storage permissions on creation so the rename-and-leave path is not taken. Signed-off-by: Marcelo Henrique Neppel --- src/charm.py | 4 ++++ tests/unit/test_charm.py | 23 ++++++++++++++++++++++- 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/src/charm.py b/src/charm.py index e4c796d6b7..1f66e70ea9 100755 --- a/src/charm.py +++ b/src/charm.py @@ -96,6 +96,7 @@ PEER_RELATION, PGBACKREST_METRICS_PORT, PLUGIN_OVERRIDES, + POSTGRESQL_STORAGE_PERMISSIONS, RAFT_PASSWORD_KEY, REPLICATION_CONSUMER_RELATION, REPLICATION_OFFER_RELATION, @@ -856,6 +857,9 @@ def _ensure_storage_layout(self) -> None: """ temp_dir = Path(TEMP_DATA_DIR) temp_dir.mkdir(parents=True, exist_ok=True) + # Match the mode set_up_database expects, so it does not rename-and-leave a + # temp_ tablespace on every reboot when the mode would otherwise differ. + temp_dir.chmod(POSTGRESQL_STORAGE_PERMISSIONS) shutil.chown(temp_dir, user=SNAP_USER, group=SNAP_USER) if temp_dir.parent.exists(): shutil.chown(temp_dir.parent, user=SNAP_USER, group=SNAP_USER) diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py index e46c943f68..2955d73dd6 100644 --- a/tests/unit/test_charm.py +++ b/tests/unit/test_charm.py @@ -7,6 +7,7 @@ import os import pathlib import platform +import stat import subprocess from datetime import UTC, datetime from typing import ClassVar @@ -39,7 +40,11 @@ SwitchoverFailedError, SwitchoverNotSyncError, ) -from single_kernel_postgresql.config.literals import PEER_RELATION, SECRET_INTERNAL_LABEL +from single_kernel_postgresql.config.literals import ( + PEER_RELATION, + POSTGRESQL_STORAGE_PERMISSIONS, + SECRET_INTERNAL_LABEL, +) from single_kernel_postgresql.utils.postgresql import ( PostgreSQLCreateUserError, PostgreSQLEnableDisableExtensionError, @@ -1029,6 +1034,22 @@ def test_ensure_storage_layout_recreates_temp_dir_on_reboot(harness, tmp_path): assert temp_root.is_dir() +def test_ensure_storage_layout_sets_temp_dir_permissions(harness, tmp_path): + """TEMP_DATA_DIR is created with POSTGRESQL_STORAGE_PERMISSIONS (0o700). + + set_up_database renames the temp tablespace and leaves a leftover when the + dir's mode differs from POSTGRESQL_STORAGE_PERMISSIONS after a reboot. Creating + it with the expected mode avoids triggering that rename-and-leave on every reboot. + """ + temp_root = tmp_path / "temp" / "16" / "main" + with ( + patch("charm.TEMP_DATA_DIR", str(temp_root)), + patch("charm.shutil"), + ): + harness.charm._ensure_storage_layout() + assert stat.S_IMODE(temp_root.stat().st_mode) == POSTGRESQL_STORAGE_PERMISSIONS + + def test_was_restore_successful_migrates_temp_tablespace_before_clearing_flags(harness): """A restore migrates an old-layout temp tablespace to the versioned path.