Resolve the backup volume from the container, and verify photo sizes
Three audit items on the backup path. The script matched volume names by pattern and took the first hit, so a stale or restored volume could be backed up instead of the live one — and every verification step would then faithfully confirm the wrong database. It now asks the container what is mounted at /data and refuses ambiguity. Tested against a decoy volume that the old pattern would have matched first. check-images treated a photo as healthy if a file with the right name existed, so a truncated or partially restored file passed. It compares each file against the byte count its row records now; a one-byte stand-in for a 123KB photo is reported as WRONG SIZE and exits non-zero. The backup runs the same check against the stopped volume and exits 2 when the source was already damaged — still writing the archive, because a faithful copy of imperfect data is worth having, but saying so. The restart trap was installed after the app had already been stopped, so an interrupt in between could leave the service down with nothing to bring it back. The trap goes in first now, covers INT and TERM as well as EXIT, and records whether the container was running beforehand so a backup of an already-stopped app leaves it stopped. Verified on the live host: healthy source exits 0, damaged source exits 2 with the archive still written and verified, decoy volume correctly ignored, service answering immediately afterwards, and the test rows removed. Checks go from 290 to 294. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -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.
|
||||
|
||||
+25
-4
@@ -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}")
|
||||
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
|
||||
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):
|
||||
|
||||
@@ -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",
|
||||
|
||||
+62
-22
@@ -14,30 +14,53 @@
|
||||
# ./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
|
||||
# --- 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 parts python -c \
|
||||
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."
|
||||
@@ -46,17 +69,32 @@ restart() {
|
||||
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
|
||||
|
||||
Reference in New Issue
Block a user