velero: exclude transient subtrees from restore-test parity, move sweep off backup window #418
No Assignees
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: konairius/homelab#418
Reference in New Issue
Block a user
Why
The 2026-07-26
restore-testrun failed 1 of 40 checks:This was a false positive — the backup is fine.
audiobookshelf-daily-20260725034518isphase=Completedwith zero errors and zero warnings, and all four of its DataUploads finished withbytesDone == totalBytes.The parity check compares a backup taken up to ~24 h ago against the live volume now, counting files with an unfiltered
find. Between the backup (07-25 03:45) and the source probe (07-26 02:12), a single Audiobookshelf playback session wrote 1531 HLS transcode files (~153 MiB) intometadata/streams/.2339 - 1531 = 808 persistent vs 803 restored -> 0.6% delta, well inside ±10%. The next day's metadata DataUpload was +157 MB, independently confirming the stream segments were the only thing that changed.
What changed
1.
volatile_paths_for()- a per-<namespace>/<pvc>list of transient subtrees pruned from both the source and restored counts (keyed off source identity, since the PVC name survives the restore but the namespace does not).Only
streams/andtmp/are excluded foraudiobookshelf-metadata.cache/is deliberately left counted: it is regenerable, but stable enough (190 files) not to threaten the tolerance, and dropping it would cost real coverage. Every entry is a blind spot, so the list is kept minimal and justified.Volumes with no entry keep the exact previous behaviour -
build_count_cmdemits the original unfilteredfindwhen the exclusion list is empty.2. CronJob moved from Sunday 02:00 -> Sunday 08:00 UTC. The old slot was chosen as "well before Monday's daily backups", which missed that the daily schedules also fire on Sunday. A ~5 h sweep starting at 02:00 sat on top of the entire 03:15-07:15 backup window and starved it of node-agent DataUpload concurrency:
On 07-26 the
booksDataUpload did not start until 06:18. Both dates are Sundays; no other day shows it. 08:00 + the 6 hactiveDeadlineSecondscap still lands well clear of Monday's 03:15.Verification
audiobookshelf-metadatavolume with busyboxfind(same family as thealpine:3.21probe image): returns775, matching 2339 - 1531 - 33.bash -nclean;kubectl kustomizebuilds clean, withschedule: 0 8 * * 0and the new script present in therestore-test-scriptConfigMap.Note
lastSuccessfulTimeon the CronJob is2026-07-12- both the 07-19 and 07-26 runs failed. This PR only explains 07-26. The 07-19 root cause remains unconfirmed: per #380 the Job and pod were deleted before triage ran, and the diagnostics ServiceAccount has no Loki access, so it is unlikely to ever be confirmed. It may or may not have been the same transient-directory cause.Refs #380.
🤖 Generated with Claude Code
The 2026-07-26 restore-test run failed 1 of 40 checks: audiobookshelf: PVC audiobookshelf-metadata file count parity (±10%) -- source=2339 restored=803 This was a false positive. The backup was complete: backup audiobookshelf-daily-20260725034518 is phase=Completed with no errors or warnings, and all four of its DataUploads finished with bytesDone == totalBytes. The parity check compares a backup taken up to ~24 h ago against the live volume *now*, counting files with an unfiltered `find`. Between the backup (07-25 03:45) and the source probe (07-26 02:12), one Audiobookshelf playback session wrote 1531 HLS transcode files (~153 MiB) into metadata/streams/. Excluding that directory: 2339 - 1531 = 808 persistent files vs 803 restored, a 0.6% delta — comfortably inside the tolerance. The next day's metadata DataUpload was +157 MB, independently confirming the stream segments were the only thing that changed. Fixes: 1. Add volatile_paths_for(), a per-"<namespace>/<pvc>" list of transient subtrees pruned from BOTH the source and restored counts. Only streams/ and tmp/ are excluded for audiobookshelf-metadata; cache/ is left counted deliberately, since it is stable enough not to threaten the tolerance and dropping it would cost real coverage. Every entry is a blind spot, so the list is kept minimal. Volumes with no entry keep the exact previous behaviour. 2. Move the CronJob from Sunday 02:00 to Sunday 08:00 UTC. The old slot was picked as "well before Monday's daily backups", which missed that the daily schedules also fire on Sunday — a ~5 h sweep starting at 02:00 sat on top of the whole 03:15-07:15 backup window and starved it of node-agent DataUpload concurrency. audiobookshelf-daily took 2 h 18 m (07-19) and 2 h 33 m (07-26) against a ~1 min baseline, with the books DataUpload not starting until 06:18. Both Sundays; no other day shows it. The pruning find expression was verified against the live audiobookshelf-metadata volume (busybox find, same as the alpine probe image): it returns 775, matching 2339 - 1531 - 33. Refs #380. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>🤖 Automated review — model
claude-sonnet-5(complexity: normal), commite41730e8bb578640d88e63b6c78955818dda899e.Summary
This PR does two things to
kubernetes/infrastructure/velero/: (1) adds a per-<namespace>/<pvc>exclusion list (volatile_paths_for) that prunes known-transient subtrees from both sides of the restore-test's file-count parity check, currently populated only foraudiobookshelf/audiobookshelf-metadata(streams,tmp); and (2) moves the weeklyrestore-testCronJob from Sunday 02:00 UTC to 08:00 UTC so it no longer overlaps Sunday's own daily backup window. Both changes are well-justified in the PR body with concrete data (file counts, DataUpload timing) and I traced the shell logic by hand against the checked-out script — it holds up. No blocking issues found.🟡 Suggestions
kubernetes/infrastructure/velero/restore-test.sh:117-123(volatile_paths_for) — the exclusion is keyed on"${src_ns}/${pvc}"via acaseliteral match, confirmed correct againstkubernetes/apps/audiobookshelf/pvcs.yaml(namespaceaudiobookshelf, PVCaudiobookshelf-metadata) anddeployment.yaml(PVC mounted at/metadata, sostreams/tmp/cacheare volume-root siblings as the PR body's directory table implies). No functional concern, just noting I verified rather than assumed it.find, same family as thealpine:3.21probe image") is reasonable evidence but is not the exact image the probe pod runs (alpine:3.21's busyboxfindbuild). If-prune/-path/parenthesized-ogrouping ever behaves differently in that specific busybox build,build_count_cmd's excluding branch (restore-test.sh:129-139) would silently degrade — worth a mental note to watch the first real run's(excluded: streams tmp)output rather than assuming success frombash -n/kubectl kustomize buildalone (per the PR's own "Verification" section, that's already the plan).git revertof one without the other.🟢 Nits
restore-test.sh:130: the local variable nameexprshadows theexprcommand available in the script's shell (harmless here since it's only ever used as data, not invoked), but a name likefind_exprwould avoid any doubt for a future reader skimming forexprinvocations.Tests
No unit/integration test harness exists for this script in-repo (confirmed — no
*.batsor shellcheck config forkubernetes/infrastructure/velero/, and.woodpecker/ci-scripts-test.ymlonly coversscripts/ci/), so this is consistent with existing practice for this file. The PR's verification section (bash -n,kubectl kustomize build, and an offlinefindtest against a live volume with a busybox binary) matches the level of diligence the file has always had. The real end-to-end validation only happens on the next scheduled Sunday run — reasonable given this is a live-cluster operational script that can't be meaningfully unit-tested without a cluster.Recommendation
Looks reasonable — no blocking issues; comments above are optional.
Thanks — all four points addressed below. Two acted on in code (
e8c2e400), two answered.🟡 1 — exclusion key verified against the manifests
Acknowledged, no action needed. Agreed on the conclusion, and thanks for checking the mount path rather than assuming it:
/metadatabeing the volume root is exactly what makesstreams/tmpvolume-root siblings, and thecasekeyaudiobookshelf/audiobookshelf-metadatais a whole-string match so it cannot collide with another namespace's identically-named PVC.🟡 2 — busybox build of the probe image: now verified, not assumed
This was the one real gap and it was testable, so I tested it rather than leaving a mental note.
Ran a pod with the same image, securityContext and read-only mount as
count_files_in_pvc's probe (alpine:3.21,runAsUser: 65532,readOnly: true), against the liveaudiobookshelf-metadatavolume:775 == 2339 − 1531 (
streams) − 33 (tmp). So-prune/-path/ the parenthesised-ogrouping behave identically in the probe image's own busybox build. The silent-degradation risk you identified is closed.Two robustness properties confirmed in the same image while I was there:
/d/nopeis also listed. This matters more than it looks: a stale entry left behind after an app reorganises its volume layout degrades gracefully instead of erroring out or zeroing the count and faking a catastrophic-loss FAIL.tmpalone gives 2306.Both are now recorded in the comment above
build_count_cmdso the next reader doesn't have to re-derive them. The probe pod was deleted afterwards; nothing left behind.🟡 3 — two fixes in one PR
Fair flag; keeping them together deliberately. They're independent in code but not in effect: the exclusion list is what stops the spurious FAIL, and the schedule move is what stops the sweep starving the backups it exists to verify. Shipping only the first would leave next Sunday's run still contending with the 03:15–07:15 window; shipping only the second would leave the parity check still firing on transient files. The next scheduled run is the first real end-to-end validation of either, so I'd rather it exercise both.
On revertability: each change is confined to its own file (
restore-test.shvsrestore-test-cronjob.yaml), so a targetedgit revert -n <sha> && git checkout <sha>^ -- <one file>recovers either independently without untangling a shared hunk.🟢 4 —
exprshadowingFixed in
e8c2e400— renamed tofind_expr. Harmless as you noted, but it costs nothing to remove the ambiguity.Correction to my own "Verification" section
The original PR body said the expression was tested "with busybox
find(same family as thealpine:3.21probe image)". That was accurate but weaker than it should have been — it was the audiobookshelf container's busybox, not the probe image's. The run above replaces it with a test in the actual image. Treat the 🟡 2 block as the authoritative verification evidence for this PR.🤖 Automated review — model
claude-sonnet-5(complexity: normal), commite8c2e400e5fe0016137df601b441ef104fe5b871.Summary
This PR does two things to the weekly Velero restore-test: (1) adds a per-PVC
volatile_paths_for()exclusion list so transient/regenerable subtrees (currentlystreamsandtmponaudiobookshelf/audiobookshelf-metadata) don't count toward the ±10% file-count parity check, and (2) moves the restore-test CronJob from Sunday 02:00 UTC to Sunday 08:00 UTC so the ~5h sweep no longer overlaps and starves Sunday's own daily backup window. Both changes are well-justified in the PR body with concrete data, and I traced the shell logic (quoting, escaping, remotefind/sh -cconstruction) line by line — it is correct. No blocking issues found.🟡 Suggestions
kubernetes/infrastructure/velero/restore-test.sh:113-123(volatile_paths_for) — the exclusion keys are matched with a plaincaseglob on the literal string"${src_ns}/${pvc}". That's fine today with one entry, but if this list grows, consider a short comment noting thatcasepatterns here are glob-matched (so a bare path segment containing*/?/[would behave unexpectedly) — not an issue now since Kubernetes namespace/PVC names can't contain those characters, but worth a one-line guard note for future editors adding entries.kubernetes/infrastructure/velero/restore-test.sh:294-306(verify_pvc_file_count_parity) — the exclusion list is looked up once fromsrc_ns/pvcand applied to both the destination and source probes. This is correct per the PR's stated rationale (namespace doesn't survive the restore, PVC name does), but it's worth double-checking that no two different services ever share the same PVC name across namespaces in a way that would pull in an exclusion meant for a different app. Currently the only entry is audiobookshelf-specific, so this isn't a live risk, just something to keep in mind as the table grows.🟢 Nits
kubernetes/infrastructure/velero/restore-test-cronjob.yaml:4-16— the schedule-change rationale assumes the (unmodified)Schedule.velero.iodaily backups run in the same wall-clock frame as the explicittimeZone: Etc/UTCset on this CronJob. The daily schedules invelero-schedules.yamlhave notimeZonefield and their comments say "local," so the UTC alignment is an assumption rather than something enforced in code. The PR's empirical observation (books DataUpload starting at 06:18) is solid corroborating evidence, so this isn't a reason to block, just a note that the safety margin rests partly on that assumption holding.Tests
No automated tests exist for this Bash script (consistent with the rest of the repo —
restore-test.shhas no test harness), and this PR doesn't add any. Given the change is a shell script whose logic is easy to get subtly wrong around quoting (escaped parens, single-quoted paths inside a remotely-executedsh -cstring), the PR compensates with strong manual verification instead: the author reports testing the pruningfindexpression directly against the liveaudiobookshelf-metadatavolume with busyboxfind(matching the probe image'salpine:3.21), confirming the exact count (775 == 2339 - 1531 - 33), plusbash -nandkubectl kustomizebuild checks. I independently tracedbuild_count_cmd()'s quoting/escaping (outer double-quoted command substitution → single kubectl-exec argument → remotesh -cparse of\(/\)and single-quoted paths) and confirmed it reduces to the standardfind dir \( -path a -o -path b \) -prune -o -type f -printidiom, and that the empty-excludes case reproduces the original unfilteredfindverbatim. All call sites ofcount_files_in_pvc/count_files_in_source_pvcwere updated consistently for the new parameter. This level of manual verification is reasonable for a homelab shell script of this kind; no gaps found.Recommendation
Looks reasonable — no blocking issues; comments above are optional.