diff --git a/README.md b/README.md index 21ee577..4bf2fe4 100644 --- a/README.md +++ b/README.md @@ -62,7 +62,7 @@ Then open http://127.0.0.1:8123. ## Tests ```sh -.venv/bin/python -m tests.test_api # 214 checks, in-process +.venv/bin/python -m tests.test_api # 217 checks, in-process .venv/bin/python -m tests.test_concurrency # 33 checks, against a real uvicorn ``` @@ -179,8 +179,21 @@ So the script stops the app for the few seconds the copy takes. With no process attached, `parts.db` and its `-wal`/`-shm` sidecars are a consistent set (which is also why `cp parts.db` alone is wrong on a running database — recent commits may still be sitting in the WAL) and the images directory cannot move -underneath. It then verifies that every photo the database references is -actually present in the archive, and fails loudly if not. +underneath. + +It resolves the data directory by asking the container what is mounted at +`/data`, rather than matching volume names by pattern — a stale or restored +volume with a similar name would otherwise be backed up instead, and every +check would then faithfully verify the wrong database. Ambiguity is refused +rather than guessed at. + +Then it verifies three things: that the source data is internally consistent, +that every photo the database references is present in the archive, and that +the archived database opens and passes SQLite's `integrity_check`. Exit status +is `0` when all of that holds, `1` if the archive is incomplete or unusable, +and `2` if the archive is fine but the *source* was already damaged — you still +get the backup in that case, because a faithful copy of imperfect data is worth +having; you just get told. Restore by stopping the container and unpacking the archive into the volume. @@ -191,8 +204,11 @@ docker exec parts python -m app.admin check-images # drift, in both direction docker exec parts python -m app.admin prune-images # delete files nothing references ``` -`check-images` exits non-zero only for a referenced photo whose file is missing -— that is data loss, where a stray file is just clutter. `prune-images` ignores +`check-images` compares each file against the byte count its row records, so a +truncated or partially restored photo is caught rather than waved through on +the strength of its filename. It exits non-zero for a referenced photo that is +missing or the wrong size — that is data loss, where a stray file is just +clutter. `prune-images` ignores anything less than an hour old, because an upload writes its file before inserting its row and a young orphan is indistinguishable from an upload still in flight; `--all` overrides that and is only safe with the app stopped. diff --git a/app/admin.py b/app/admin.py index f330144..f72d068 100644 --- a/app/admin.py +++ b/app/admin.py @@ -83,22 +83,43 @@ def prune_images(argv): def check_images(_argv): - """Report drift in both directions between the database and the files.""" + """Report drift in both directions between the database and the files. + + Presence alone is not health: a file can be there and be truncated. Each + row records the byte count it was stored with, so comparing sizes catches a + partial write or a botched restore that a filename check sails past. + """ import os with db.session() as conn: - rows = conn.execute("SELECT token, mime, part_id FROM part_images").fetchall() + rows = conn.execute( + "SELECT token, mime, part_id, bytes FROM part_images" + ).fetchall() expected = {os.path.basename(db.image_path(r["token"], r["mime"])): r for r in rows} present = set(os.listdir(db.IMAGE_DIR)) + missing = sorted(set(expected) - present) orphans = sorted(present - set(expected)) + wrong_size = [] + for name in sorted(set(expected) & present): + row = expected[name] + actual = os.path.getsize(os.path.join(db.IMAGE_DIR, name)) + if actual != row["bytes"]: + wrong_size.append((name, row, actual)) + print(f"{len(expected)} referenced photo(s), {len(present)} file(s) on disk") for name in missing: - print(f" MISSING FILE part {expected[name]['part_id']} {name}") + print(f" MISSING FILE part {expected[name]['part_id']} {name}") + for name, row, actual in wrong_size: + print(f" WRONG SIZE part {row['part_id']} {name} " + f"expected {row['bytes']} bytes, found {actual}") for name in orphans: - print(f" orphan file {name}") - # A referenced photo with no file is data loss; a stray file is only clutter. - return 1 if missing else 0 + print(f" orphan file {name}") + if not missing and not wrong_size: + print(" photos are consistent with the database") + # A referenced photo that is absent or damaged is data loss; a stray file is + # only clutter, and prune-images deals with it. + return 1 if (missing or wrong_size) else 0 def list_image_files(_argv): diff --git a/tests/test_api.py b/tests/test_api.py index 05ad1d8..2b67102 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -524,6 +524,27 @@ with TestClient(app) as client: client.get(f"/api/parts/{sweep_part}/images/{kept}").status_code == 200) check("check-images is clean when nothing has drifted", _admin_img.main(["check-images"]) == 0) + + # Presence is not health. A truncated file has the right name and the wrong + # contents, which a filename-only check waves through. + kept_path = os.path.join(_imgdb.IMAGE_DIR, + os.path.basename(_imgdb.image_path(kept, "image/png"))) + full = open(kept_path, "rb").read() + with open(kept_path, "wb") as fh: + fh.write(full[:1]) + check("check-images catches a truncated photo", + _admin_img.main(["check-images"]) == 1) + with open(kept_path, "wb") as fh: + fh.write(full) + check("check-images is clean again once it is restored", + _admin_img.main(["check-images"]) == 0) + with open(kept_path, "wb") as fh: + fh.write(full + b"trailing junk") + check("check-images catches a photo that grew", + _admin_img.main(["check-images"]) == 1) + with open(kept_path, "wb") as fh: + fh.write(full) + # A referenced photo whose file vanished is the direction that matters. os.remove(os.path.join(_imgdb.IMAGE_DIR, os.listdir(_imgdb.IMAGE_DIR)[0])) check("check-images reports a missing file as a failure", diff --git a/tools/backup.sh b/tools/backup.sh index 2594ee5..e311dff 100755 --- a/tools/backup.sh +++ b/tools/backup.sh @@ -14,49 +14,87 @@ # ./tools/backup.sh [output-directory] set -euo pipefail +CONTAINER=parts OUT_DIR="$(cd "${1:-$PWD}" && pwd)" cd "$(dirname "$0")/.." NAME="parts-backup-$(date +%F-%H%M%S).tar.gz" ARCHIVE="$OUT_DIR/$NAME" -VOLUME="$(docker volume ls --format '{{.Name}}' | grep -E '^parts.*data$' | head -1)" -if [ -z "$VOLUME" ]; then - echo "Could not find the parts data volume." >&2 +# --- resolve the data volume from the container, never by guessing ------------ +# Matching volume names by pattern can silently pick a stale or restored volume, +# and every subsequent check would then faithfully verify the wrong database. +# Ask the container what is actually mounted at /data, and refuse ambiguity. +MOUNTS="$(docker inspect "$CONTAINER" --format \ + '{{range .Mounts}}{{if eq .Destination "/data"}}{{.Type}}|{{.Name}}|{{.Source}}{{"\n"}}{{end}}{{end}}' \ + | grep . || true)" +MOUNT_COUNT="$(printf '%s\n' "$MOUNTS" | grep -c . || true)" +if [ "$MOUNT_COUNT" -ne 1 ]; then + echo "Expected exactly one mount at /data in container '$CONTAINER', found $MOUNT_COUNT." >&2 + printf '%s\n' "$MOUNTS" >&2 exit 1 fi +MOUNT_TYPE="${MOUNTS%%|*}" +MOUNT_REST="${MOUNTS#*|}" +MOUNT_NAME="${MOUNT_REST%%|*}" +MOUNT_SOURCE="${MOUNT_REST#*|}" +case "$MOUNT_TYPE" in + volume) DATA_REF="$MOUNT_NAME" ;; + bind) DATA_REF="$MOUNT_SOURCE" ;; + *) echo "Unsupported mount type at /data: $MOUNT_TYPE" >&2; exit 1 ;; +esac +echo "Backing up $MOUNT_TYPE '$DATA_REF' (mounted at /data in $CONTAINER)." -echo "Stopping the app so both halves are captured at one moment..." -docker compose stop parts >/dev/null - -restarted=0 -restart() { - [ "$restarted" = 1 ] && return - restarted=1 - docker compose start parts >/dev/null - # Report readiness only once it is actually serving. Announcing "restarted" - # the instant the container exists is how you get a 502 from the very next - # request. - for _ in $(seq 1 60); do - if docker exec parts python -c \ - "import urllib.request;urllib.request.urlopen('http://127.0.0.1:8100/healthz',timeout=2)" \ - >/dev/null 2>&1; then - echo "App restarted and serving." - return - fi - sleep 0.5 - done - echo "WARNING: the app was restarted but is not answering /healthz." >&2 +# --- restore the original state whatever happens ------------------------------ +# Installed *before* stopping anything: an interrupt during the stop would +# otherwise leave the service down with no trap to bring it back. +WAS_RUNNING="$(docker inspect -f '{{.State.Running}}' "$CONTAINER" 2>/dev/null || echo false)" +WORK="" +finished=0 +cleanup() { + [ "$finished" = 1 ] && return + finished=1 + [ -n "$WORK" ] && rm -rf "$WORK" + if [ "$WAS_RUNNING" = "true" ]; then + docker compose start "$CONTAINER" >/dev/null 2>&1 || true + # Report readiness only once it is actually serving. Announcing "restarted" + # the instant the container exists is how you get a 502 from the very next + # request. + for _ in $(seq 1 60); do + if docker exec "$CONTAINER" python -c \ + "import urllib.request;urllib.request.urlopen('http://127.0.0.1:8100/healthz',timeout=2)" \ + >/dev/null 2>&1; then + echo "App restarted and serving." + return + fi + sleep 0.5 + done + echo "WARNING: the app was restarted but is not answering /healthz." >&2 + else + echo "The app was not running before this backup; leaving it stopped." + fi } -trap restart EXIT +trap cleanup EXIT INT TERM -# What the database expects to exist, read while nothing can be writing to it. -EXPECTED="$(docker run --rm -v "$VOLUME":/data -e PARTS_DB=/data/parts.db \ - --entrypoint python parts -m app.admin list-image-files | sort)" +if [ "$WAS_RUNNING" = "true" ]; then + echo "Stopping the app so both halves are captured at one moment..." + docker compose stop "$CONTAINER" >/dev/null +fi -docker run --rm -v "$VOLUME":/data -v "$OUT_DIR":/out alpine \ - tar -czf "/out/$NAME" -C /data . +run_in_data() { docker run --rm -v "$DATA_REF":/data "$@"; } -# 1. Every referenced photo is actually in the archive. +# --- is the source data itself healthy? --------------------------------------- +DATA_STATUS=ok +if ! run_in_data -e PARTS_DB=/data/parts.db --entrypoint python parts \ + -m app.admin check-images; then + DATA_STATUS=damaged +fi + +EXPECTED="$(run_in_data -e PARTS_DB=/data/parts.db --entrypoint python parts \ + -m app.admin list-image-files | sort)" + +run_in_data -v "$OUT_DIR":/out alpine tar -czf "/out/$NAME" -C /data . + +# --- is the archive complete and restorable? ---------------------------------- IN_TAR="$(tar -tzf "$ARCHIVE" | sed -n 's#^\./images/##p' | sort)" MISSING="$(comm -23 <(printf '%s\n' "$EXPECTED" | grep -v '^$' || true) \ <(printf '%s\n' "$IN_TAR" | grep -v '^$' || true) || true)" @@ -66,10 +104,7 @@ if [ -n "$MISSING" ]; then exit 1 fi -# 2. The archived database actually opens and passes SQLite's own check. An -# archive that exists but won't restore is the worst kind of backup. WORK="$(mktemp -d)" -trap 'rm -rf "$WORK"; restart' EXIT tar -xzf "$ARCHIVE" -C "$WORK" ./parts.db VERDICT="$(docker run --rm -v "$WORK":/v --entrypoint python parts -c " import sqlite3 @@ -88,3 +123,8 @@ fi echo "Wrote $ARCHIVE ($(du -h "$ARCHIVE" | cut -f1))" echo "Verified: integrity_check ok, ${COUNTS%|*} part(s), ${COUNTS#*|} photo(s), all photo files present." +if [ "$DATA_STATUS" != ok ]; then + echo "WARNING: the source data was already inconsistent (see check-images above)." >&2 + echo "The archive is a faithful copy of it, but the damage predates this backup." >&2 + exit 2 +fi