diff --git a/src/charm.py b/src/charm.py index e0d5606dc3..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) @@ -2434,6 +2438,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..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,88 @@ 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. + + 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 +1231,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(