restore: ask the repository about a snapshot by id, not by app tag
storageSnapshotSourcePath resolved a snapshot's source path with
engineSnapshotsJson "$idx" "$snapshot_id"
but that function's second parameter is an app TAG filter. So it ran
`restic snapshots --tag app=<snapshot-id>`, matched nothing, and returned 1 —
every time, for every snapshot, since the file was written.
Nothing broke loudly, because both callers have a fallback:
* storageRestoreAppTo fell through to "restoring in place", reinstating the
exact cross-root bug the file exists to fix — restoring onto a host whose
containers root differs from the source's matched no include path and
restored nothing, silently
* the first-run preflight never read a manifest, so every app reported size
"?" and its fit and location checks passed unconditionally. Thirteen green
ticks that had checked nothing.
Add engineSnapshotPaths: restic answers it with a positional snapshot id, kopia
by filtering its list. borg has no adapter on purpose — it rebuilds its listing
from archive metadata that carries no paths — so a missing adapter is a quiet
"no" and those callers keep their in-place fallback.
Add scripts/dev/lp-preflight-test, which pins the cases that must say NO: an
app too big for the disk, one this version no longer ships, one whose storage
location is gone, and a resolver that reaches for the app-tag filter again.
Verified against both historical bugs — reintroducing either fails the test.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
librelad
parent
f8c9e87643
commit
647b19cf4a
@ -92,6 +92,18 @@ engineForgetSystem() { local i="$1"; shift; engineDispatch "$(engineForL
|
||||
engineCheckLocation() { local i="$1"; shift; engineDispatch "$(engineForLocation "$i")CheckLocation" "$i" "$@"; }
|
||||
engineDumpFile() { local i="$1"; shift; engineDispatch "$(engineForLocation "$i")DumpFile" "$i" "$@"; }
|
||||
|
||||
# The paths a single snapshot was taken from. Not every engine can answer: borg
|
||||
# reconstructs its listing from archive metadata that carries no paths, so it
|
||||
# has no adapter on purpose. A missing adapter is therefore a quiet "no" rather
|
||||
# than engineDispatch's error — callers fall back to restoring in place, which
|
||||
# is correct for borg and merely conservative elsewhere.
|
||||
engineSnapshotPaths() {
|
||||
local i="$1"; shift
|
||||
local fn; fn="$(engineForLocation "$i")SnapshotPaths"
|
||||
declare -f "$fn" >/dev/null 2>&1 || return 1
|
||||
"$fn" "$i" "$@"
|
||||
}
|
||||
|
||||
# ---- Aggregate helpers (iterate enabled locations) ---------------------------
|
||||
|
||||
engineInstallAll()
|
||||
|
||||
@ -122,3 +122,23 @@ kopiaSnapshotListFiles()
|
||||
kopiaEnvUnset
|
||||
return $rc
|
||||
}
|
||||
|
||||
# The path recorded on one snapshot. kopia has no "describe this id" listing, so
|
||||
# filter the full list; ids are matched whole or by the 8-char short form that
|
||||
# kopiaSnapshotsJson publishes as short_id.
|
||||
kopiaSnapshotPaths()
|
||||
{
|
||||
local idx="$1"
|
||||
local snapshot_id="$2"
|
||||
|
||||
command -v jq >/dev/null 2>&1 || return 1
|
||||
kopiaEnvExport "$idx" || return 1
|
||||
local raw
|
||||
raw=$(runBackupOp kopia snapshot list --all --json 2>/dev/null)
|
||||
local rc=$?
|
||||
kopiaEnvUnset
|
||||
[[ $rc -eq 0 && -n "$raw" ]] || return 1
|
||||
|
||||
printf '%s' "$raw" | jq -r --arg id "$snapshot_id" \
|
||||
'.[] | select(.id == $id or .id[0:8] == $id) | .source.path' 2>/dev/null
|
||||
}
|
||||
|
||||
@ -61,3 +61,28 @@ resticSnapshotListFiles()
|
||||
resticEnvUnset
|
||||
return $rc
|
||||
}
|
||||
|
||||
# The paths recorded ON one snapshot, one per line.
|
||||
#
|
||||
# Deliberately its own call rather than a filter on resticSnapshotsJson: that
|
||||
# function's second parameter is an app TAG filter, so asking it for a snapshot
|
||||
# id runs `--tag app=<id>` and matches nothing. storageSnapshotSourcePath did
|
||||
# exactly that, so it always returned 1 and every restore silently took the
|
||||
# "restore in place" fallback — the very bug storage_restore_path.sh exists to
|
||||
# fix. Restore accepts a snapshot id positionally; use that.
|
||||
resticSnapshotPaths()
|
||||
{
|
||||
local idx="$1"
|
||||
local snapshot_id="$2"
|
||||
|
||||
resticEnvExport "$idx" || return 1
|
||||
local json
|
||||
json=$(runBackupOp restic snapshots "$snapshot_id" --json --no-lock 2>/dev/null)
|
||||
local rc=$?
|
||||
resticEnvUnset
|
||||
[[ $rc -eq 0 && -n "$json" ]] || return 1
|
||||
|
||||
printf '%s' "$json" \
|
||||
| grep -o '"paths":\[[^]]*\]' | head -1 \
|
||||
| grep -o '"/[^"]*"' | tr -d '"'
|
||||
}
|
||||
|
||||
104
scripts/dev/lp-preflight-test
Executable file
104
scripts/dev/lp-preflight-test
Executable file
@ -0,0 +1,104 @@
|
||||
#!/bin/bash
|
||||
# Self-contained unit test for the first-run restore preflight
|
||||
# (scripts/restore/restore_preflight.sh) and the snapshot path resolver it
|
||||
# leans on (scripts/storage/storage_restore_path.sh). Runs against a throwaway
|
||||
# tree in $TMPDIR — never touches a real install. Exits non-zero on failure.
|
||||
#
|
||||
# scripts/dev/lp-preflight-test
|
||||
#
|
||||
# It exists because this preflight has twice shipped checks that could not fail:
|
||||
#
|
||||
# * the manifest is pretty-printed, so `"size_bytes": 123` carries a space
|
||||
# that a `"key":[0-9]*` pattern misses — size came back empty, and the fit
|
||||
# check was skipped for every app
|
||||
# * storageSnapshotSourcePath asked engineSnapshotsJson for a snapshot id,
|
||||
# but that argument is an app TAG filter — so it matched nothing, returned
|
||||
# 1 every time, and the manifest was never read at all
|
||||
#
|
||||
# Both failed silently and *looked* like a clean report: thirteen green ticks.
|
||||
# So the cases worth keeping are the ones that must say NO.
|
||||
|
||||
REPO="$(cd "$(dirname "$0")/../.." && pwd)"
|
||||
BASE="$(mktemp -d "${TMPDIR:-/tmp}/lp-preflight-test-XXXXXX")"
|
||||
trap 'rm -rf "$BASE"' EXIT
|
||||
|
||||
fail=0
|
||||
chk(){ if [[ "$2" == "$3" ]]; then echo " ok $1"; else echo " FAIL $1: got '$2' want '$3'"; fail=1; fi; }
|
||||
has(){ if [[ "$2" == *"$3"* ]]; then echo " ok $1"; else echo " FAIL $1: '$2' lacks '$3'"; fail=1; fi; }
|
||||
|
||||
# --- a machine to check against ----------------------------------------------
|
||||
install_containers_dir="$BASE/templates/"
|
||||
mkdir -p "$BASE/templates/bookstack" "$BASE/templates/nextcloud" "$BASE/templates/jellyfin" "$BASE/data"
|
||||
: > "$BASE/templates/bookstack/bookstack.config"
|
||||
: > "$BASE/templates/nextcloud/nextcloud.config"
|
||||
: > "$BASE/templates/jellyfin/jellyfin.config"
|
||||
# note: no template for 'obsolete' — that is case 2
|
||||
|
||||
isNotice(){ :; }; isError(){ :; }; isSuccessful(){ :; }; isQuestion(){ :; }
|
||||
appDir(){ printf '%s/data/%s' "$BASE" "$1"; }
|
||||
storageLocationName(){ printf 'primary'; }
|
||||
storageLocationPath(){ [[ "$1" == "here" ]] && { printf '%s/data' "$BASE"; return 0; }; return 1; }
|
||||
migrateDiscoverApps(){ :; }
|
||||
engineSnapshotLatestId(){ printf 'abc123'; }
|
||||
|
||||
source "$REPO/scripts/restore/restore_preflight.sh"
|
||||
|
||||
# Stubbed AFTER the source, or the real definition wins. Manifests are
|
||||
# pretty-printed exactly as the real ones are — size_bytes at the top level,
|
||||
# location under "storage" — because the spaces after those colons are the
|
||||
# point of this fixture, not an accident.
|
||||
_manifest(){
|
||||
printf '{\n "app": "%s",\n "size_bytes": %s,\n "storage": {\n "location": "%s"\n }\n}\n' \
|
||||
"$1" "$2" "$3"
|
||||
}
|
||||
restorePreflightManifest(){ _manifest "$2" "$MANIFEST_SIZE" "$MANIFEST_LOC"; }
|
||||
|
||||
echo "--- an app that restores as-is ---"
|
||||
MANIFEST_LOC="default"; MANIFEST_SIZE=147483648
|
||||
rec=$(restorePreflightApp 1 bookstack somehost)
|
||||
chk "verdict" "$(cut -f1 <<< "$rec")" "ok"
|
||||
chk "size parsed" "$(cut -f3 <<< "$rec")" "140M"
|
||||
|
||||
echo "--- an app this version no longer ships ---"
|
||||
rec=$(restorePreflightApp 1 obsolete somehost)
|
||||
chk "verdict" "$(cut -f1 <<< "$rec")" "skip"
|
||||
has "reason" "$rec" "no longer ships"
|
||||
|
||||
echo "--- an app too big for the disk (the check that could not fail) ---"
|
||||
MANIFEST_LOC="default"; MANIFEST_SIZE=9007199254740992 # 8 PiB
|
||||
rec=$(restorePreflightApp 1 jellyfin somehost)
|
||||
chk "verdict" "$(cut -f1 <<< "$rec")" "skip"
|
||||
has "reason" "$rec" "needs 8192T"
|
||||
|
||||
echo "--- an app whose old storage location is gone ---"
|
||||
MANIFEST_LOC="ssd"; MANIFEST_SIZE=1048576
|
||||
rec=$(restorePreflightApp 1 nextcloud somehost)
|
||||
chk "verdict" "$(cut -f1 <<< "$rec")" "move"
|
||||
has "reason" "$rec" "not on this machine"
|
||||
|
||||
echo "--- a location this machine still has is NOT a move ---"
|
||||
MANIFEST_LOC="here"; MANIFEST_SIZE=1048576
|
||||
rec=$(restorePreflightApp 1 nextcloud somehost)
|
||||
chk "verdict" "$(cut -f1 <<< "$rec")" "ok"
|
||||
|
||||
echo "--- storageSnapshotSourcePath picks the app's path from the snapshot ---"
|
||||
# If this ever calls engineSnapshotsJson again it is passing a snapshot id into
|
||||
# an app-tag filter, which is the bug. Make that loud rather than silent.
|
||||
engineSnapshotsJson(){ echo " FAIL resolver used the app-tag filter again" >&2; fail=1; return 1; }
|
||||
engineSnapshotPaths(){ printf '/mnt/ssd/apps/nextcloud\n/libreportal-containers/bookstack\n'; }
|
||||
source "$REPO/scripts/storage/storage_restore_path.sh"
|
||||
chk "matching app" "$(storageSnapshotSourcePath 1 abc123 nextcloud)" "/mnt/ssd/apps/nextcloud"
|
||||
chk "other app" "$(storageSnapshotSourcePath 1 abc123 bookstack)" "/libreportal-containers/bookstack"
|
||||
storageSnapshotSourcePath 1 abc123 grafana >/dev/null 2>&1 \
|
||||
&& { echo " FAIL absent app should not resolve"; fail=1; } \
|
||||
|| echo " ok absent app does not resolve"
|
||||
|
||||
# An engine with no adapter (borg) must fall back quietly, not error.
|
||||
engineSnapshotPaths(){ return 1; }
|
||||
storageSnapshotSourcePath 1 abc123 bookstack >/dev/null 2>&1 \
|
||||
&& { echo " FAIL engine without paths should not resolve"; fail=1; } \
|
||||
|| echo " ok engine without paths falls back"
|
||||
|
||||
echo ""
|
||||
if (( fail )); then echo "FAILED"; exit 1; fi
|
||||
echo "All preflight checks passed."
|
||||
@ -504,6 +504,7 @@ declare -gA LP_FN_MAP=(
|
||||
[engineRestoreSystemLatest]="backup/engine/engine_dispatch.sh"
|
||||
[engineSnapshotLatestId]="backup/engine/engine_dispatch.sh"
|
||||
[engineSnapshotListFiles]="backup/engine/engine_dispatch.sh"
|
||||
[engineSnapshotPaths]="backup/engine/engine_dispatch.sh"
|
||||
[engineSnapshotsJson]="backup/engine/engine_dispatch.sh"
|
||||
[engineSystemSnapshotsJson]="backup/engine/engine_dispatch.sh"
|
||||
[exitScript]="start/start_exit.sh"
|
||||
@ -651,6 +652,7 @@ declare -gA LP_FN_MAP=(
|
||||
[kopiaRestoreSystemLatest]="backup/engine/kopia_snapshots.sh"
|
||||
[kopiaSnapshotLatestId]="backup/engine/kopia_snapshots.sh"
|
||||
[kopiaSnapshotListFiles]="backup/engine/kopia_snapshots.sh"
|
||||
[kopiaSnapshotPaths]="backup/engine/kopia_snapshots.sh"
|
||||
[kopiaSnapshotsJson]="backup/engine/kopia_snapshots.sh"
|
||||
[kopiaSystemSnapshotsJson]="backup/engine/kopia_snapshots.sh"
|
||||
[libreportal_catalog_install_post_setup]="libreportal_catalog/scripts/libreportal_catalog_install_hooks.sh"
|
||||
@ -898,6 +900,7 @@ declare -gA LP_FN_MAP=(
|
||||
[resticRetentionFor]="backup/engine/restic_forget.sh"
|
||||
[resticSnapshotLatestId]="backup/engine/restic_snapshots.sh"
|
||||
[resticSnapshotListFiles]="backup/engine/restic_snapshots.sh"
|
||||
[resticSnapshotPaths]="backup/engine/restic_snapshots.sh"
|
||||
[resticSnapshotsJson]="backup/engine/restic_snapshots.sh"
|
||||
[resticSystemSnapshotsJson]="backup/engine/restic_snapshots.sh"
|
||||
[_resticUsernsPrefix]="backup/engine/restic_restore.sh"
|
||||
@ -1739,6 +1742,7 @@ declare -gA LP_FN_ROOT=(
|
||||
[engineRestoreSystemLatest]="scripts"
|
||||
[engineSnapshotLatestId]="scripts"
|
||||
[engineSnapshotListFiles]="scripts"
|
||||
[engineSnapshotPaths]="scripts"
|
||||
[engineSnapshotsJson]="scripts"
|
||||
[engineSystemSnapshotsJson]="scripts"
|
||||
[exitScript]="scripts"
|
||||
@ -1886,6 +1890,7 @@ declare -gA LP_FN_ROOT=(
|
||||
[kopiaRestoreSystemLatest]="scripts"
|
||||
[kopiaSnapshotLatestId]="scripts"
|
||||
[kopiaSnapshotListFiles]="scripts"
|
||||
[kopiaSnapshotPaths]="scripts"
|
||||
[kopiaSnapshotsJson]="scripts"
|
||||
[kopiaSystemSnapshotsJson]="scripts"
|
||||
[libreportal_catalog_install_post_setup]="containers"
|
||||
@ -2133,6 +2138,7 @@ declare -gA LP_FN_ROOT=(
|
||||
[resticRetentionFor]="scripts"
|
||||
[resticSnapshotLatestId]="scripts"
|
||||
[resticSnapshotListFiles]="scripts"
|
||||
[resticSnapshotPaths]="scripts"
|
||||
[resticSnapshotsJson]="scripts"
|
||||
[resticSystemSnapshotsJson]="scripts"
|
||||
[_resticUsernsPrefix]="scripts"
|
||||
@ -3012,6 +3018,7 @@ engineRestoreSnapshot() { unset -f engineRestoreSnapshot; __lpAutoload "${instal
|
||||
engineRestoreSystemLatest() { unset -f engineRestoreSystemLatest; __lpAutoload "${install_scripts_dir}backup/engine/engine_dispatch.sh"; engineRestoreSystemLatest "$@"; }
|
||||
engineSnapshotLatestId() { unset -f engineSnapshotLatestId; __lpAutoload "${install_scripts_dir}backup/engine/engine_dispatch.sh"; engineSnapshotLatestId "$@"; }
|
||||
engineSnapshotListFiles() { unset -f engineSnapshotListFiles; __lpAutoload "${install_scripts_dir}backup/engine/engine_dispatch.sh"; engineSnapshotListFiles "$@"; }
|
||||
engineSnapshotPaths() { unset -f engineSnapshotPaths; __lpAutoload "${install_scripts_dir}backup/engine/engine_dispatch.sh"; engineSnapshotPaths "$@"; }
|
||||
engineSnapshotsJson() { unset -f engineSnapshotsJson; __lpAutoload "${install_scripts_dir}backup/engine/engine_dispatch.sh"; engineSnapshotsJson "$@"; }
|
||||
engineSystemSnapshotsJson() { unset -f engineSystemSnapshotsJson; __lpAutoload "${install_scripts_dir}backup/engine/engine_dispatch.sh"; engineSystemSnapshotsJson "$@"; }
|
||||
exitScript() { unset -f exitScript; __lpAutoload "${install_scripts_dir}start/start_exit.sh"; exitScript "$@"; }
|
||||
@ -3159,6 +3166,7 @@ kopiaRestoreSnapshot() { unset -f kopiaRestoreSnapshot; __lpAutoload "${install_
|
||||
kopiaRestoreSystemLatest() { unset -f kopiaRestoreSystemLatest; __lpAutoload "${install_scripts_dir}backup/engine/kopia_snapshots.sh"; kopiaRestoreSystemLatest "$@"; }
|
||||
kopiaSnapshotLatestId() { unset -f kopiaSnapshotLatestId; __lpAutoload "${install_scripts_dir}backup/engine/kopia_snapshots.sh"; kopiaSnapshotLatestId "$@"; }
|
||||
kopiaSnapshotListFiles() { unset -f kopiaSnapshotListFiles; __lpAutoload "${install_scripts_dir}backup/engine/kopia_snapshots.sh"; kopiaSnapshotListFiles "$@"; }
|
||||
kopiaSnapshotPaths() { unset -f kopiaSnapshotPaths; __lpAutoload "${install_scripts_dir}backup/engine/kopia_snapshots.sh"; kopiaSnapshotPaths "$@"; }
|
||||
kopiaSnapshotsJson() { unset -f kopiaSnapshotsJson; __lpAutoload "${install_scripts_dir}backup/engine/kopia_snapshots.sh"; kopiaSnapshotsJson "$@"; }
|
||||
kopiaSystemSnapshotsJson() { unset -f kopiaSystemSnapshotsJson; __lpAutoload "${install_scripts_dir}backup/engine/kopia_snapshots.sh"; kopiaSystemSnapshotsJson "$@"; }
|
||||
libreportal_catalog_install_post_setup() { unset -f libreportal_catalog_install_post_setup; __lpAutoload "${install_containers_dir}libreportal_catalog/scripts/libreportal_catalog_install_hooks.sh"; libreportal_catalog_install_post_setup "$@"; }
|
||||
@ -3406,6 +3414,7 @@ resticRestoreSystemLatest() { unset -f resticRestoreSystemLatest; __lpAutoload "
|
||||
resticRetentionFor() { unset -f resticRetentionFor; __lpAutoload "${install_scripts_dir}backup/engine/restic_forget.sh"; resticRetentionFor "$@"; }
|
||||
resticSnapshotLatestId() { unset -f resticSnapshotLatestId; __lpAutoload "${install_scripts_dir}backup/engine/restic_snapshots.sh"; resticSnapshotLatestId "$@"; }
|
||||
resticSnapshotListFiles() { unset -f resticSnapshotListFiles; __lpAutoload "${install_scripts_dir}backup/engine/restic_snapshots.sh"; resticSnapshotListFiles "$@"; }
|
||||
resticSnapshotPaths() { unset -f resticSnapshotPaths; __lpAutoload "${install_scripts_dir}backup/engine/restic_snapshots.sh"; resticSnapshotPaths "$@"; }
|
||||
resticSnapshotsJson() { unset -f resticSnapshotsJson; __lpAutoload "${install_scripts_dir}backup/engine/restic_snapshots.sh"; resticSnapshotsJson "$@"; }
|
||||
resticSystemSnapshotsJson() { unset -f resticSystemSnapshotsJson; __lpAutoload "${install_scripts_dir}backup/engine/restic_snapshots.sh"; resticSystemSnapshotsJson "$@"; }
|
||||
_resticUsernsPrefix() { unset -f _resticUsernsPrefix; __lpAutoload "${install_scripts_dir}backup/engine/restic_restore.sh"; _resticUsernsPrefix "$@"; }
|
||||
|
||||
@ -23,17 +23,18 @@
|
||||
storageSnapshotSourcePath()
|
||||
{
|
||||
local idx="$1" snapshot_id="$2" app="$3"
|
||||
local json path
|
||||
local path
|
||||
|
||||
json=$(engineSnapshotsJson "$idx" "$snapshot_id" 2>/dev/null) || return 1
|
||||
# "paths":["/libreportal-containers/bookstack"]
|
||||
path=$(printf '%s' "$json" \
|
||||
| grep -o '"paths":\[[^]]*\]' \
|
||||
| head -1 \
|
||||
| grep -o '"/[^"]*"' \
|
||||
| tr -d '"' \
|
||||
| grep -E "/${app}(/|$)" \
|
||||
| head -1)
|
||||
# Ask about this ONE snapshot. The first version passed the id to
|
||||
# engineSnapshotsJson, whose second parameter is an app TAG filter — so it
|
||||
# searched for `--tag app=<snapshot-id>`, matched nothing, and returned 1
|
||||
# every single time. Nothing broke loudly, because both callers have a
|
||||
# fallback: storageRestoreAppTo restored in place (reinstating the
|
||||
# cross-root bug this file was written to fix) and the restore preflight
|
||||
# reported every app's size as "?" while its fit and location checks passed
|
||||
# unconditionally.
|
||||
path=$(engineSnapshotPaths "$idx" "$snapshot_id" 2>/dev/null \
|
||||
| grep -E "/${app}(/|$)" | head -1)
|
||||
[[ -n "$path" ]] || return 1
|
||||
printf '%s' "${path%/}"
|
||||
}
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user