From 14936b62dced0e4f7b6f95e80cab1027d43d0544 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Sat, 7 Nov 2026 22:07:44 -0500 Subject: [PATCH] refactor: retire the images tarball backup Images and attachments are backed up off-site, one object per file, so the tar+zstd of /images goes: its service and restore, both tasks, the trigger and restore routes, the Keep last setting, the screen's Images section, and the tests. Migration 0122 drops backup_run.tar_path and import_settings.backup_images_keep_last_n and deletes any images runs. The local database dump stays for an install with no off-site destination, and the Backups card now says plainly that images and attachments are backed up only off-site. Co-Authored-By: Claude Opus 5.5 --- README.md | 3 +- alembic/versions/0122_retire_images_backup.py | 30 +++++ backend/app/api/system_backup.py | 47 ++------ backend/app/models/backup_run.py | 6 +- backend/app/models/import_settings.py | 4 - backend/app/services/backup_service.py | 89 +-------------- backend/app/tasks/backup.py | 106 +----------------- backend/app/tasks/maintenance.py | 8 +- .../modal/DestructiveConfirmModal.vue | 2 +- .../src/components/settings/BackupCard.vue | 47 ++------ frontend/src/stores/backup.js | 7 +- tests/test_api_system_backup.py | 25 ++--- tests/test_backup_service.py | 76 +------------ tests/test_tasks_backup.py | 35 ++---- 14 files changed, 92 insertions(+), 393 deletions(-) create mode 100644 alembic/versions/0122_retire_images_backup.py diff --git a/README.md b/README.md index d256596..7e6792b 100644 --- a/README.md +++ b/README.md @@ -182,7 +182,8 @@ Run it for a burst of tagging, stop it to get your card back. It deploys from Backups live under **Settings → Maintenance → Backups**. With nothing else set up, the database is dumped nightly into `./images/_backups`, which is on the -same disk as what it protects. +same disk as what it protects. Images and attachments are backed up only to an +off-site destination. To keep a copy off the machine, set up an off-site destination on the same screen: any S3 bucket (Fabled Archives, MinIO, AWS) and a key that may add and diff --git a/alembic/versions/0122_retire_images_backup.py b/alembic/versions/0122_retire_images_backup.py new file mode 100644 index 0000000..e89044c --- /dev/null +++ b/alembic/versions/0122_retire_images_backup.py @@ -0,0 +1,30 @@ +"""Retire the images tarball backup. + +Images and attachments are backed up off-site, one object per file +(backup_blob). The tarball of /images that this replaces is gone, with its +retention setting, its path column, and any runs it left. + +Revision ID: 0122 +Revises: 0121 +Create Date: 2026-11-08 + +""" +import sqlalchemy as sa +from alembic import op + +revision = "0122" +down_revision = "0121" +branch_labels = None +depends_on = None + + +def upgrade() -> None: + op.execute("DELETE FROM backup_run WHERE kind = 'images'") + op.drop_column("backup_run", "tar_path") + op.drop_column("import_settings", "backup_images_keep_last_n") + + +def downgrade() -> None: + op.add_column("import_settings", sa.Column( + "backup_images_keep_last_n", sa.Integer(), nullable=False, server_default="3")) + op.add_column("backup_run", sa.Column("tar_path", sa.Text(), nullable=True)) diff --git a/backend/app/api/system_backup.py b/backend/app/api/system_backup.py index 3590527..c7d33c7 100644 --- a/backend/app/api/system_backup.py +++ b/backend/app/api/system_backup.py @@ -1,5 +1,5 @@ -"""FC-3h: /api/system/backup — create/list/restore/delete/tag for -DB + image backups. +"""/api/system/backup: database dumps, the off-site destination and its media +backup, restores, test restores, and the run history. Read endpoints are public on FC (operator-facing internal API; same posture as /api/system/activity). Write endpoints take a typed @@ -20,13 +20,12 @@ system_backup_bp = Blueprint( "system_backup", __name__, url_prefix="/api/system/backup", ) -_KINDS = frozenset({"db", "images", "media", "restore", "reconcile", "test"}) +_KINDS = frozenset({"db", "media", "restore", "reconcile", "test"}) _TAG_MAX_LEN = 64 _BACKUP_SETTINGS_FIELDS = ( "backup_db_nightly_enabled", "backup_db_nightly_hour_utc", "backup_db_keep_last_n", - "backup_images_keep_last_n", ) @@ -44,7 +43,6 @@ def _row_to_dict(r: BackupRun) -> dict: if r.finished_at and r.started_at else None ), "sql_path": r.sql_path, - "tar_path": r.tar_path, "size_bytes": r.size_bytes, "error": r.error, "restored_from_id": r.restored_from_id, @@ -81,10 +79,6 @@ def _validate_backup_settings_patch(body: dict): v = body["backup_db_keep_last_n"] if not isinstance(v, int) or isinstance(v, bool) or not (1 <= v <= 365): return _bad("invalid_value", detail="backup_db_keep_last_n must be 1..365") - if "backup_images_keep_last_n" in body: - v = body["backup_images_keep_last_n"] - if not isinstance(v, int) or isinstance(v, bool) or not (1 <= v <= 100): - return _bad("invalid_value", detail="backup_images_keep_last_n must be 1..100") return None @@ -99,17 +93,6 @@ async def trigger_db_backup(): return jsonify({"status": "dispatched"}), 202 -@system_backup_bp.route("/images", methods=["POST"]) -async def trigger_images_backup(): - body = await request.get_json(silent=True) or {} - tag = _validate_tag(body.get("tag")) - if isinstance(tag, tuple): - return tag - from ..tasks.backup import backup_images_task - backup_images_task.delay(tag=tag, triggered_by="manual") - return jsonify({"status": "dispatched"}), 202 - - @system_backup_bp.route("/media", methods=["POST"]) async def trigger_media_backup(): """Send the images and attachments not yet in the off-site bucket. Refused @@ -348,17 +331,17 @@ async def trigger_restore(run_id: int): row = await session.get(BackupRun, run_id) if row is None: return _bad("not_found", status=404) - if row.status != "ok": - return _bad( - "not_restorable", - detail=f"source backup status={row.status!r}; only 'ok' rows are restorable", - ) if (row.manifest or {}).get("offsite"): return _bad( "not_restorable", detail=("this dump is kept in the off-site bucket, not on this server, " "so it can't be restored from here"), ) + if row.kind != "db" or row.status != "ok" or not row.sql_path: + return _bad( + "not_restorable", + detail="only a finished database dump kept on this server can be restored here", + ) expected = f"restore-{row.kind}-{row.id}" if supplied != expected: return _bad( @@ -366,15 +349,10 @@ async def trigger_restore(run_id: int): detail=f"confirm must equal {expected!r}", expected=expected, ) - kind = row.kind - if kind == "db": - from ..tasks.backup import restore_db_task - restore_db_task.delay(source_backup_run_id=run_id) - else: # 'images' (the only other value _KINDS allows via the trigger path) - from ..tasks.backup import restore_images_task - restore_images_task.delay(source_backup_run_id=run_id) - return jsonify({"status": "dispatched", "kind": kind}), 202 + from ..tasks.backup import restore_db_task + restore_db_task.delay(source_backup_run_id=run_id) + return jsonify({"status": "dispatched", "kind": "db"}), 202 @system_backup_bp.route("/runs//test", methods=["POST"]) @@ -414,7 +392,7 @@ async def delete_run(run_id: int): ) from ..services import backup_service backup_service.unlink_artifact_files( - sql_path=row.sql_path, tar_path=row.tar_path, + sql_path=row.sql_path, manifest_path=(row.manifest or {}).get("manifest_path"), ) await session.delete(row) @@ -430,7 +408,6 @@ async def get_settings(): "backup_db_nightly_enabled": row.backup_db_nightly_enabled, "backup_db_nightly_hour_utc": row.backup_db_nightly_hour_utc, "backup_db_keep_last_n": row.backup_db_keep_last_n, - "backup_images_keep_last_n": row.backup_images_keep_last_n, }) diff --git a/backend/app/models/backup_run.py b/backend/app/models/backup_run.py index cac6a04..ed507d4 100644 --- a/backend/app/models/backup_run.py +++ b/backend/app/models/backup_run.py @@ -1,7 +1,8 @@ """FC-3h: backup_run — operator-facing artifact record for a backup run. -One row per backup attempt (kind='db', 'images', or 'media' for the off-site -upload of images and attachments, which spans many task chunks). Lifecycle +One row per run: kind 'db' (a database dump), 'media' (the off-site upload of +images and attachments, which spans many task chunks), 'restore', 'reconcile' +(the weekly bucket check) or 'test' (a test restore). Lifecycle tracking (started_at/finished_at/duration_ms/exception text) lives in task_run from FC-3i — this row records artifact metadata: file paths, sizes, tag (retention protection), and restore lineage via @@ -55,7 +56,6 @@ class BackupRun(Base): DateTime(timezone=True), nullable=True, index=True, ) sql_path: Mapped[str | None] = mapped_column(Text, nullable=True) - tar_path: Mapped[str | None] = mapped_column(Text, nullable=True) size_bytes: Mapped[int | None] = mapped_column(BigInteger, nullable=True) error: Mapped[str | None] = mapped_column(Text, nullable=True) manifest: Mapped[dict] = mapped_column( diff --git a/backend/app/models/import_settings.py b/backend/app/models/import_settings.py index 434a7d8..9cf5937 100644 --- a/backend/app/models/import_settings.py +++ b/backend/app/models/import_settings.py @@ -97,10 +97,6 @@ class ImportSettings(Base): Integer, nullable=False, default=14, server_default="14", ) - backup_images_keep_last_n: Mapped[int] = mapped_column( - Integer, nullable=False, default=3, - server_default="3", - ) # Off-site backup destination: an S3 bucket (services/backup_destination). # The secret key is stored sealed with the credential key, never in clear. backup_s3_endpoint: Mapped[str | None] = mapped_column(Text, nullable=True) diff --git a/backend/app/services/backup_service.py b/backend/app/services/backup_service.py index 5d18771..1a23932 100644 --- a/backend/app/services/backup_service.py +++ b/backend/app/services/backup_service.py @@ -1,14 +1,11 @@ """FC-3h: first-class backup/restore service for FC. -Two independent backup kinds: - - 'db' — pg_dump only; fast; nightly via Beat (settings-gated) - - 'images' — tar+zstd of /images; slow; manual trigger only - -With an off-site destination set up, a database dump streams straight to that -bucket instead (backup_db_offsite). Otherwise files live under -/_backups/. Each local backup writes: - fc__.{sql|tar.zst} — the artifact - fc__.json — manifest (kind/tag/triggered_by) +The database dump (pg_dump), nightly via Beat when the setting is on. With an +off-site destination set up, the dump streams straight to that bucket +(backup_db_offsite), and images and attachments go there too (backup_media). +Without one, the dump is written under /_backups/: + fc_db_.dump — the dump (custom format) + fc_db_.json — manifest (tag, trigger, row counts) Service functions are sync (subprocess-bound). Celery tasks in backend.app.tasks.backup wrap each one with task_run-tracked @@ -31,25 +28,6 @@ log = logging.getLogger(__name__) _BACKUPS_DIRNAME = "_backups" -# Excluded from the images tarball, and each for its own reason (#4233, #4234): -# -# _backups — the archive would otherwise contain every previous archive. -# This is not hypothetical: the 2026-05-23/24 runs, taken before -# this exclude existed, grew 43G -> 107G -> ... -> 2123G as each -# swallowed its predecessors, and cost 4.3T of the images -# filesystem until they were reclaimed on 2026-09-21. -# _quarantine — holds files deliberately pulled OUT of the library. -# secrets — `credential_key.b64`, the key that decrypts the stored -# Patreon/SubscribeStar session credentials. -# cookies — those session cookies themselves. -# -# The last two are the ones worth stating plainly: an images tarball is a media -# archive, and a media archive that carries the key to the operator's accounts -# is a credential leak wearing a backup's name. Encryption at rest buys nothing -# when the key rides along in the same file. A restore therefore does NOT -# re-establish credentials — you sign in again, which is the correct outcome. -_IMAGES_EXCLUDED_DIRNAMES = ("_backups", "_quarantine", "secrets", "cookies") - # Subprocess-level guardrails BEYOND the Celery soft_time_limit. The Celery # soft limit signals the Python process; subprocess.Popen in a blocking syscall # ignores that signal, so these bound the worst case directly. Each sits just @@ -58,9 +36,7 @@ _IMAGES_EXCLUDED_DIRNAMES = ("_backups", "_quarantine", "secrets", "cookies") # matters because an NFS D-state hang defeats even Celery's SIGKILL (the failure # that wedged the maintenance lane for hours, #739). # backup_db_task: soft=1800s / hard=2100s → 1700s -# backup_images_task: soft=21600s / hard=23400s → 21000s _DB_SUBPROCESS_TIMEOUT_S = 1700 # ~28 min, under the 30-min DB soft limit -_IMAGES_SUBPROCESS_TIMEOUT_S = 21000 # ~5.8 hr, under the 6-hr images soft limit # Grace after SIGKILL to reap the child. If it can't be reaped in this window # (an uninterruptible NFS D-state — the failure mode that wedged the # concurrency-1 maintenance lane for hours, operator-flagged 2026-06-07), we @@ -259,7 +235,6 @@ def backup_db( "kind": "db", "ts": ts, "sql_path": str(sql_path), - "tar_path": None, "manifest_path": str(manifest_path), "size_bytes": _file_size_or_none(sql_path), "counts": counts, @@ -348,42 +323,6 @@ def _stream_dump(db_url: str, target, ts: str, limit_mbps: int | None, snapshot: } -def backup_images( - *, images_root: Path, - tag: str | None = None, triggered_by: str = "manual", -) -> dict: - """Run tar --zstd over images_root; write .tar.zst + manifest.""" - ts = _now_ts() - out_dir = _backups_dir(images_root) - tar_path = out_dir / f"fc_images_{ts}.tar.zst" - # No local-temp here (the archive is hundreds of GB — it can't stage in - # /tmp), but bounded-kill still applies so a tar wedged on NFS fails fast - # rather than holding the lane for hours. - _run_bounded( - [ - "tar", "--zstd", "-cf", str(tar_path), - "-C", str(images_root.parent), images_root.name, - *( - f"--exclude={images_root.name}/{name}" - for name in _IMAGES_EXCLUDED_DIRNAMES - ), - ], - _IMAGES_SUBPROCESS_TIMEOUT_S, - ) - manifest_path = _write_manifest( - out_dir, kind="images", ts=ts, tag=tag, triggered_by=triggered_by, - artifact_path=tar_path, - ) - return { - "kind": "images", - "ts": ts, - "sql_path": None, - "tar_path": str(tar_path), - "manifest_path": str(manifest_path), - "size_bytes": _file_size_or_none(tar_path), - } - - class RestoreFailed(Exception): """psql or pg_restore exited with an error; the message is its last line.""" @@ -483,24 +422,9 @@ def restore_db(*, db_url: str, sql_path: Path) -> None: else f"pg_restore exited with {proc.returncode}") -def restore_images(*, images_root: Path, tar_path: Path) -> None: - """Untar over images_root.parent. Additive — files NOT in the - tarball are NOT removed. Caller wipes first if a clean restore - is needed.""" - subprocess.run( - [ - "tar", "--zstd", "-xf", str(tar_path), - "-C", str(images_root.parent), - ], - capture_output=True, check=True, - timeout=_IMAGES_SUBPROCESS_TIMEOUT_S, - ) - - def unlink_artifact_files( *, sql_path: str | None, - tar_path: str | None, manifest_path: str | None, ) -> dict: """Best-effort unlink of all on-disk files for a BackupRun row. @@ -509,7 +433,6 @@ def unlink_artifact_files( deleted: dict = {} for label, p in ( ("sql", sql_path), - ("tar", tar_path), ("manifest", manifest_path), ): if not p: diff --git a/backend/app/tasks/backup.py b/backend/app/tasks/backup.py index 5a2561f..281ca75 100644 --- a/backend/app/tasks/backup.py +++ b/backend/app/tasks/backup.py @@ -125,55 +125,6 @@ def backup_db_task(self, *, tag: str | None = None, return {"backup_run_id": run_id} -@celery.task( - name="backend.app.tasks.backup.backup_images_task", - bind=True, - autoretry_for=(OperationalError, DBAPIError), - retry_backoff=30, retry_backoff_max=300, max_retries=1, - soft_time_limit=21600, time_limit=23400, -) -def backup_images_task(self, *, tag: str | None = None, - triggered_by: str = "manual") -> dict: - """Create one images backup. Same shape as backup_db_task; uses - tar_path instead of sql_path.""" - SessionLocal = _sync_session_factory() - now = datetime.now(UTC) - with SessionLocal() as session: - row = BackupRun( - kind="images", status="running", tag=tag, - triggered_by=triggered_by, started_at=now, manifest={}, - ) - session.add(row) - session.commit() - session.refresh(row) - run_id = row.id - - try: - result = backup_service.backup_images( - images_root=IMAGES_ROOT, - tag=tag, triggered_by=triggered_by, - ) - except (SoftTimeLimitExceeded, Exception) as exc: - with SessionLocal() as session: - row = session.get(BackupRun, run_id) - if row is not None: - _mark_failed(session, row, exc) - raise - - with SessionLocal() as session: - row = session.get(BackupRun, run_id) - row.status = "ok" - row.finished_at = datetime.now(UTC) - row.tar_path = result["tar_path"] - row.size_bytes = result["size_bytes"] - row.manifest = { - "manifest_path": result["manifest_path"], - "ts": result["ts"], - } - session.commit() - return {"backup_run_id": run_id} - - @celery.task( name="backend.app.tasks.backup.restore_db_task", bind=True, @@ -225,52 +176,6 @@ def restore_db_task(self, *, source_backup_run_id: int) -> dict: return {"backup_run_id": marker_id} -@celery.task( - name="backend.app.tasks.backup.restore_images_task", - bind=True, max_retries=0, - soft_time_limit=21600, time_limit=23400, -) -def restore_images_task(self, *, source_backup_run_id: int) -> dict: - """Mirrors restore_db_task; uses backup_service.restore_images.""" - SessionLocal = _sync_session_factory() - now = datetime.now(UTC) - with SessionLocal() as session: - src = session.get(BackupRun, source_backup_run_id) - if src is None or src.kind != "images" or not src.tar_path: - raise ValueError( - f"BackupRun id={source_backup_run_id} is not a valid images backup" - ) - marker = BackupRun( - kind="images", status="restoring", - triggered_by="restore", started_at=now, - restored_from_id=src.id, - manifest={"source_tar_path": src.tar_path}, - ) - session.add(marker) - session.commit() - session.refresh(marker) - marker_id = marker.id - tar_path = src.tar_path - - try: - backup_service.restore_images( - images_root=IMAGES_ROOT, tar_path=Path(tar_path), - ) - except (SoftTimeLimitExceeded, Exception) as exc: - with SessionLocal() as session: - row = session.get(BackupRun, marker_id) - if row is not None: - _mark_failed(session, row, exc) - raise - - with SessionLocal() as session: - row = session.get(BackupRun, marker_id) - row.status = "restored" - row.finished_at = datetime.now(UTC) - session.commit() - return {"backup_run_id": marker_id} - - @celery.task( name="backend.app.tasks.backup.prune_backups", soft_time_limit=300, time_limit=600, @@ -278,27 +183,23 @@ def restore_images_task(self, *, source_backup_run_id: int) -> dict: def prune_backups() -> dict: """Daily Beat. Per-kind retention from ImportSettings. - Returns {"db_deleted": N, "images_deleted": M, "media_deleted": L, - "restore_deleted": R, "reconcile_deleted": C, "test_deleted": T, + Returns {"db_deleted": N, "media_deleted": L, "restore_deleted": R, "reconcile_deleted": C, "test_deleted": T, "files_unlinked": K}. Media, restore, reconcile and test runs leave no files here; only their history is trimmed, to the latest MEDIA_RUNS_KEPT, RESTORE_RUNS_KEPT, RECONCILE_RUNS_KEPT and TEST_RUNS_KEPT. Tagged rows (tag IS NOT NULL) are never pruned. Status='running' / 'restoring' rows are never pruned — the recover_stalled_backup_runs sweep flips truly-stuck ones to - 'error' first. (Earlier docstring claimed the FC-3i TaskRun sweep - handled those, but TaskRun cleanup never touched BackupRun rows. - Audit 2026-06-02 added the dedicated sweep.) + 'error' first. """ SessionLocal = _sync_session_factory() - counts = {"db_deleted": 0, "images_deleted": 0, "media_deleted": 0, + counts = {"db_deleted": 0, "media_deleted": 0, "restore_deleted": 0, "reconcile_deleted": 0, "test_deleted": 0, "files_unlinked": 0} with SessionLocal() as session: s = ImportSettings.load_sync(session) for kind, keep in ( ("db", s.backup_db_keep_last_n), - ("images", s.backup_images_keep_last_n), ("media", MEDIA_RUNS_KEPT), ("restore", RESTORE_RUNS_KEPT), ("reconcile", RECONCILE_RUNS_KEPT), @@ -315,7 +216,6 @@ def prune_backups() -> dict: for row in candidates: result = backup_service.unlink_artifact_files( sql_path=row.sql_path, - tar_path=row.tar_path, manifest_path=(row.manifest or {}).get("manifest_path"), ) counts["files_unlinked"] += sum( diff --git a/backend/app/tasks/maintenance.py b/backend/app/tasks/maintenance.py index d377bec..41d3a13 100644 --- a/backend/app/tasks/maintenance.py +++ b/backend/app/tasks/maintenance.py @@ -168,11 +168,6 @@ QUEUE_STUCK_THRESHOLD_MINUTES: dict[str, int] = { } TASK_STUCK_THRESHOLD_MINUTES: dict[str, int] = { "backend.app.tasks.import_file.import_archive_file": 40, - # Backup images runs hours, not minutes (6.5h hard limit). The - # task-name override beats the queue's 75-min default so a - # legitimately-running backup isn't flagged. - "backend.app.tasks.backup.backup_images_task": 420, - "backend.app.tasks.backup.restore_images_task": 420, # One media-backup chunk stops starting files after 45 min but may finish # a large file in flight, up to its 6.5 h hard limit. "backend.app.tasks.backup.backup_media_task": 420, @@ -849,7 +844,8 @@ def recover_stalled_backup_runs() -> int: result = session.execute( update(BackupRun) .where(BackupRun.status.in_(["running", "restoring"])) - # db backups/restores are fast (40-min window); images run hours (7h). + # db backups/restores are fast (40-min window); a test restore and the + # weekly bucket check get 7 h. # A media run or an off-site restore spans many chunks and hours on # purpose, so its window runs from its last progress, not its start. .where( diff --git a/frontend/src/components/modal/DestructiveConfirmModal.vue b/frontend/src/components/modal/DestructiveConfirmModal.vue index b60490f..5a4d8be 100644 --- a/frontend/src/components/modal/DestructiveConfirmModal.vue +++ b/frontend/src/components/modal/DestructiveConfirmModal.vue @@ -58,7 +58,7 @@ import { computed, ref, watch } from 'vue' const props = defineProps({ modelValue: { type: Boolean, required: true }, action: { type: String, required: true }, // 'restore' | 'delete' - kind: { type: String, required: true }, // 'db' | 'images' | 'artist' | 'tag' | 'images-selection' | 'audit' | 'min-dim' + kind: { type: String, required: true }, // 'db' | 'offsite' | 'artist' | 'tag' | 'images-selection' | 'audit' | 'min-dim' runId: { type: [Number, String], default: '' }, // numeric id or sha8 string description: { type: String, default: '' }, tier: { type: String, default: 'C' }, // 'B' | 'C' diff --git a/frontend/src/components/settings/BackupCard.vue b/frontend/src/components/settings/BackupCard.vue index 495523e..290c0fc 100644 --- a/frontend/src/components/settings/BackupCard.vue +++ b/frontend/src/components/settings/BackupCard.vue @@ -2,7 +2,7 @@

Database dumps stream to the {{ offsite.bucket }} bucket @@ -11,11 +11,10 @@ bucket's media/ folder, below.

- pg_dump for the database (fast, nightly-schedulable); - tar+zstd for the images (slow, manual only). Files live in - /images/_backups/. Tag a backup to protect it - from autoprune. Set up an off-site destination above to send - database dumps off this server instead. + Database dumps are written to /images/_backups/ on this + server; tag one to keep it past Keep last. Images and attachments are + backed up only to an off-site destination: set one up above to back up + the whole library, database included, off this server.

- store.settings || { backup_db_nightly_enabled: false, backup_db_nightly_hour_utc: 3, backup_db_keep_last_n: 14, - backup_images_keep_last_n: 3, }) // The off-site destination, once it is fully set up: database dumps go there. @@ -267,7 +237,6 @@ const runningSent = computed(() => runningMedia.value?.manifest?.sent || 0) const mediaTriggering = ref(false) const dbTriggering = ref(false) -const imagesTriggering = ref(false) const confirmOpen = ref(false) const confirmAction = ref('restore') const confirmKind = ref('db') @@ -356,8 +325,6 @@ function pollOnce() { store.loadMediaCoverage() store.loadRuns('reconcile') } - } else { - store.loadRuns('images') } } @@ -373,7 +340,7 @@ onUnmounted(() => { }) async function onTrigger(kind) { - const triggeringRef = { db: dbTriggering, images: imagesTriggering, media: mediaTriggering }[kind] + const triggeringRef = { db: dbTriggering, media: mediaTriggering }[kind] triggeringRef.value = true try { await store.triggerBackup(kind) @@ -395,7 +362,7 @@ function _openConfirm(action, run) { confirmToken.value = '' confirmDescription.value = ( action === 'restore' - ? `Source artifact: ${run.kind === 'db' ? run.sql_path : run.tar_path}` + ? `Source artifact: ${run.sql_path}` : 'Removes the BackupRun row and unlinks the artifact files.' ) confirmOpen.value = true diff --git a/frontend/src/stores/backup.js b/frontend/src/stores/backup.js index b76ed02..0a38430 100644 --- a/frontend/src/stores/backup.js +++ b/frontend/src/stores/backup.js @@ -7,7 +7,6 @@ export const useBackupStore = defineStore('backup', () => { const api = useApi() const dbRuns = ref([]) - const imagesRuns = ref([]) const mediaRuns = ref([]) const restoreRuns = ref([]) // The weekly check of the bucket against what was sent. @@ -16,12 +15,12 @@ export const useBackupStore = defineStore('backup', () => { const testRuns = ref([]) const settings = ref(null) const loading = ref({ - dbRuns: false, imagesRuns: false, mediaRuns: false, restoreRuns: false, + dbRuns: false, mediaRuns: false, restoreRuns: false, reconcileRuns: false, testRuns: false, settings: false, }) const lastError = ref(null) const RUNS = { - db: dbRuns, images: imagesRuns, media: mediaRuns, restore: restoreRuns, + db: dbRuns, media: mediaRuns, restore: restoreRuns, reconcile: reconcileRuns, test: testRuns, } @@ -200,7 +199,7 @@ export const useBackupStore = defineStore('backup', () => { } return { - dbRuns, imagesRuns, mediaRuns, restoreRuns, reconcileRuns, testRuns, + dbRuns, mediaRuns, restoreRuns, reconcileRuns, testRuns, mediaCoverage, loadMediaCoverage, listOffsiteDumps, databaseBytes, restoreFromOffsite, fetchMissingFiles, testDump, settings, loading, lastError, diff --git a/tests/test_api_system_backup.py b/tests/test_api_system_backup.py index 882bedb..a4c6a37 100644 --- a/tests/test_api_system_backup.py +++ b/tests/test_api_system_backup.py @@ -16,21 +16,20 @@ async def _seed_runs(db): now = datetime.now(UTC) for i in range(4): db.add(BackupRun( - kind="db" if i % 2 == 0 else "images", + kind="db" if i % 2 == 0 else "media", status="ok", tag=None, triggered_by="manual", started_at=now - timedelta(seconds=10 - i), finished_at=now - timedelta(seconds=9 - i), sql_path="/tmp/fake.sql" if i % 2 == 0 else None, - tar_path=None if i % 2 == 0 else "/tmp/fake.tar.zst", size_bytes=100, manifest={}, )) await db.commit() -# --- POST /db, /images ---------------------------------------------- +# --- POST /db --------------------------------------------------------- @pytest.mark.asyncio @@ -72,14 +71,10 @@ async def test_post_db_rejects_tag_too_long(client): @pytest.mark.asyncio -async def test_post_images_dispatches(client, monkeypatch): - dispatched = [] - monkeypatch.setattr( - "backend.app.tasks.backup.backup_images_task.delay", - lambda **kw: dispatched.append(kw), - ) - resp = await client.post("/api/system/backup/images", json={}) - assert resp.status_code == 202 +async def test_the_images_tarball_is_gone(client): + assert (await client.post("/api/system/backup/images", json={})).status_code in (404, 405) + resp = await client.get("/api/system/backup/runs?kind=images") + assert resp.status_code == 400 # --- GET /runs ------------------------------------------------------- @@ -103,10 +98,10 @@ async def test_runs_filter_by_kind_db(client, _seed_runs): @pytest.mark.asyncio -async def test_runs_filter_by_kind_images(client, _seed_runs): - resp = await client.get("/api/system/backup/runs?kind=images") +async def test_runs_filter_by_kind_media(client, _seed_runs): + resp = await client.get("/api/system/backup/runs?kind=media") body = await resp.get_json() - assert all(r["kind"] == "images" for r in body["runs"]) + assert body["runs"] and all(r["kind"] == "media" for r in body["runs"]) @pytest.mark.asyncio @@ -298,7 +293,7 @@ async def test_get_settings_returns_defaults(client): assert body["backup_db_nightly_enabled"] is False assert body["backup_db_nightly_hour_utc"] == 3 assert body["backup_db_keep_last_n"] == 14 - assert body["backup_images_keep_last_n"] == 3 + assert "backup_images_keep_last_n" not in body @pytest.mark.asyncio diff --git a/tests/test_backup_service.py b/tests/test_backup_service.py index 6394b0b..4ab1d9b 100644 --- a/tests/test_backup_service.py +++ b/tests/test_backup_service.py @@ -1,6 +1,6 @@ """FC-3h: backup_service unit tests. -Subprocess calls (pg_dump, tar, psql) are monkeypatched so tests run +Subprocess calls (pg_dump, pg_restore, psql) are monkeypatched so tests run without external binaries. The real subprocess behavior is exercised implicitly via the Celery task tests in test_tasks_backup.py. """ @@ -32,9 +32,6 @@ def fake_subprocess(monkeypatch): if "pg_dump" in cmd: i = cmd.index("-f") Path(cmd[i + 1]).write_bytes(b"-- fake pg_dump\n") - elif cmd[0] == "tar" and "-cf" in cmd: - i = cmd.index("-cf") - Path(cmd[i + 1]).write_bytes(b"fake tar payload") def _fake_run(cmd, **kwargs): calls.append(list(cmd)) @@ -100,7 +97,6 @@ def test_backup_db_writes_sql_and_manifest(tmp_path, fake_subprocess): assert sql.is_file() and sql.suffix == ".dump" assert manifest.is_file() and manifest.suffix == ".json" assert result["kind"] == "db" - assert result["tar_path"] is None assert result["size_bytes"] == len(b"-- fake pg_dump\n") parsed = json.loads(manifest.read_text()) @@ -141,50 +137,6 @@ def test_backup_db_default_tag_is_none(tmp_path, fake_subprocess): assert parsed["tag"] is None -# --- backup_images --------------------------------------------------- - - -def test_backup_images_writes_tar_and_manifest(tmp_path, fake_subprocess): - result = backup_service.backup_images( - images_root=tmp_path, tag="monthly", triggered_by="manual", - ) - tar = Path(result["tar_path"]) - assert tar.is_file() and tar.name.endswith(".tar.zst") - assert result["sql_path"] is None - assert result["size_bytes"] == len(b"fake tar payload") - - -def test_backup_images_excludes_backups_and_quarantine(tmp_path, fake_subprocess): - backup_service.backup_images(images_root=tmp_path) - cmd = fake_subprocess[0] - excludes = [arg for arg in cmd if arg.startswith("--exclude=")] - assert any("_backups" in e for e in excludes) - assert any("_quarantine" in e for e in excludes) - - -def test_backup_images_excludes_credentials(tmp_path, fake_subprocess): - """#4234: the images tarball carried `secrets/credential_key.b64` — the key - that decrypts the stored session cookies — and `cookies/` itself. A media - archive must not be a credential leak; encryption at rest is worth nothing - if the key travels with the data.""" - backup_service.backup_images(images_root=tmp_path) - excludes = [a for a in fake_subprocess[0] if a.startswith("--exclude=")] - assert any(e.endswith("/secrets") for e in excludes) - assert any(e.endswith("/cookies") for e in excludes) - - -def test_backup_images_excludes_are_root_relative(tmp_path, fake_subprocess): - """tar matches --exclude against the archived path, which is prefixed with - the root's own directory name (`-C `). A bare `secrets` - would also match an ARTIST folder called secrets; the prefix is what keeps - the exclusion to the top level.""" - backup_service.backup_images(images_root=tmp_path) - excludes = [a for a in fake_subprocess[0] if a.startswith("--exclude=")] - assert excludes and all( - e.startswith(f"--exclude={tmp_path.name}/") for e in excludes - ) - - # --- restore_db ------------------------------------------------------ @@ -267,41 +219,25 @@ def test_every_dump_leaves_out_account_logins(tmp_path, fake_subprocess): assert "--exclude-table-data=credential" in fake_subprocess[0] -# --- restore_images -------------------------------------------------- - - -def test_restore_images_untar_to_parent(tmp_path, fake_subprocess): - tar_path = tmp_path / "fake.tar.zst" - tar_path.write_bytes(b"fake") - backup_service.restore_images(images_root=tmp_path, tar_path=tar_path) - cmd = fake_subprocess[0] - assert cmd[:3] == ["tar", "--zstd", "-xf"] - assert str(tar_path) in cmd - assert "-C" in cmd - - # --- unlink ---------------------------------------------------------- def test_unlink_removes_present_files_and_reports(tmp_path): sql = tmp_path / "x.sql" sql.write_bytes(b"x") - tar = tmp_path / "x.tar.zst" - tar.write_bytes(b"x") manifest = tmp_path / "x.json" manifest.write_text("{}") result = backup_service.unlink_artifact_files( - sql_path=str(sql), tar_path=str(tar), manifest_path=str(manifest), + sql_path=str(sql), manifest_path=str(manifest), ) - assert result == {"sql": True, "tar": True, "manifest": True} - assert not sql.exists() and not tar.exists() and not manifest.exists() + assert result == {"sql": True, "manifest": True} + assert not sql.exists() and not manifest.exists() def test_unlink_missing_files_returns_true(tmp_path): """missing_ok semantics: a non-existent file isn't an error.""" result = backup_service.unlink_artifact_files( - sql_path=str(tmp_path / "nope.sql"), - tar_path=None, manifest_path=None, + sql_path=str(tmp_path / "nope.sql"), manifest_path=None, ) assert result == {"sql": True} @@ -309,7 +245,7 @@ def test_unlink_missing_files_returns_true(tmp_path): def test_unlink_skips_none_paths(): """A None path is skipped — not added to the result dict.""" result = backup_service.unlink_artifact_files( - sql_path=None, tar_path=None, manifest_path=None, + sql_path=None, manifest_path=None, ) assert result == {} diff --git a/tests/test_tasks_backup.py b/tests/test_tasks_backup.py index 0557a52..35560be 100644 --- a/tests/test_tasks_backup.py +++ b/tests/test_tasks_backup.py @@ -36,9 +36,6 @@ def fake_subprocess_and_images_root(monkeypatch, tmp_path): if "pg_dump" in cmd: i = cmd.index("-f") Path(cmd[i + 1]).write_bytes(b"-- fake pg_dump\n") - elif cmd[0] == "tar" and "-cf" in cmd: - i = cmd.index("-cf") - Path(cmd[i + 1]).write_bytes(b"fake tar payload") def _fake_run(cmd, **kwargs): _sentinel(cmd) @@ -55,7 +52,7 @@ def fake_subprocess_and_images_root(monkeypatch, tmp_path): def kill(self): self.returncode = -9 - # backup_db/backup_images go through _run_bounded (Popen); restore via run. + # backup_db goes through _run_bounded (Popen); restore via run. monkeypatch.setattr("subprocess.run", _fake_run) monkeypatch.setattr("subprocess.Popen", _FakePopen) @@ -67,7 +64,6 @@ def _seed_backup(db_sync, *, kind, status, started_at, tag=None, triggered_by="manual", started_at=started_at, finished_at=finished_at or (started_at + timedelta(seconds=10)), sql_path="/tmp/fake.sql" if kind == "db" else None, - tar_path="/tmp/fake.tar.zst" if kind == "images" else None, manifest={}, ) db_sync.add(row) @@ -185,22 +181,6 @@ async def test_an_off_site_dump_that_fails_is_recorded_with_why(db_sync, monkeyp assert row.error == "DumpFailed: pg_dump: error: connection lost" -# --- backup_images_task --------------------------------------------- - - -@pytest.mark.asyncio -async def test_backup_images_task_creates_backup_run(db_sync): - from backend.app.tasks.backup import backup_images_task - result = backup_images_task.delay().get() - row = db_sync.execute( - select(BackupRun.kind, BackupRun.status, BackupRun.tar_path) - .where(BackupRun.id == result["backup_run_id"]) - ).one() - assert row.kind == "images" - assert row.status == "ok" - assert row.tar_path and row.tar_path.endswith(".tar.zst") - - # --- restore_db_task ------------------------------------------------ @@ -226,11 +206,11 @@ async def test_restore_db_task_rejects_non_db_source(db_sync): from backend.app.tasks.backup import restore_db_task now = datetime.now(UTC) - img_id = _seed_backup(db_sync, kind="images", status="ok", started_at=now) + media_id = _seed_backup(db_sync, kind="media", status="ok", started_at=now) db_sync.commit() with pytest.raises(ValueError): - restore_db_task.delay(source_backup_run_id=img_id).get() + restore_db_task.delay(source_backup_run_id=media_id).get() # --- prune_backups -------------------------------------------------- @@ -243,7 +223,6 @@ def test_prune_backups_keeps_last_n_untagged_per_kind(db_sync): select(ImportSettings).where(ImportSettings.id == 1) ).scalar_one() s.backup_db_keep_last_n = 2 - s.backup_images_keep_last_n = 1 db_sync.commit() now = datetime.now(UTC) @@ -251,13 +230,13 @@ def test_prune_backups_keeps_last_n_untagged_per_kind(db_sync): _seed_backup(db_sync, kind="db", status="ok", started_at=now - timedelta(hours=i)) for i in range(3): - _seed_backup(db_sync, kind="images", status="ok", + _seed_backup(db_sync, kind="media", status="ok", started_at=now - timedelta(hours=i)) db_sync.commit() result = prune_backups.apply().get() assert result["db_deleted"] == 2 - assert result["images_deleted"] == 2 + assert result["media_deleted"] == 0 # 60 media runs are kept def test_prune_backups_protects_tagged_rows(db_sync): @@ -321,9 +300,9 @@ def test_stall_sweep_flips_db_fast_but_spares_running_images(db_sync): db_sync, kind="db", status="running", started_at=now - timedelta(minutes=50), ) - # An images backup running 50 min is still well under the 7h window → spared. + # A test restore running 50 min is still well under the 7h window → spared. img_id = _seed_backup( - db_sync, kind="images", status="running", + db_sync, kind="test", status="running", started_at=now - timedelta(minutes=50), ) db_sync.commit()