fix(tra-1059): retarget restore-proof recipes at cluster-per-env CNPG - #163
Merged
Conversation
Both restore-proof recipes target the shared-cluster CNPG topology that TRA-849 superseded. Records three independent defects (topology, GCS path, tofu backend init), the live-cluster resolution approach that drops the tofu dependency, and the decision to keep the PITR scratch cluster in trakrf-system so no terraform apply is needed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Seven tasks: cnpg_primary_pod helper (TDD against test-ops-lib.sh), psql refactor, the three recipe retargets, docs, and the four live restore runs that are the actual acceptance criterion. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Selects on cnpg.io/instanceRole=primary so it survives a failover, unlike the cnpg.io/cluster=<name>,role=primary pair the restore recipes use today. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Resolves ns trakrf-<env> / cluster trakrf-db-<env>, reads the bucket from the live Cluster spec instead of an uninitialized tofu backend, and fixes the GCS prefix to <cluster>/dump/ — the old <env>/ prefix has aged out of the bucket, so the path was broken independently of the topology. ENV is now required and prod is gated behind confirm_prod. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Extend the EXIT trap so it also drops the scratch database on a mid-run failure (pg_restore, timescaledb_post_restore, or the sanity query), not just on the success path. Previously only rm -rf on the local temp dir ran on failure, leaving the scratch DB behind on the live primary — including prod, where it silently consumes PVC space on a single-instance cluster. The trap is installed only after CREATE DATABASE succeeds, so an earlier abort never tries to drop a database that doesn't exist. The drop uses DROP DATABASE IF EXISTS ... WITH (FORCE) and is best-effort (a failed drop only warns), so cleanup never masks the recipe's real exit status, which is captured up front and re-asserted via exit. Also adds the missing -v ON_ERROR_STOP=1 to the success-path DROP for consistency with every other psql call in the recipe. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Scratch cluster stays trakrf-restore-test in trakrf-system to keep the static WI binding valid; only serverName, bucket and GSA become per-env. Sanity check now targets the single trakrf DB each per-env cluster holds. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
db-restore-pitr-test left the trakrf-restore-test scratch Cluster running if the Ready wait timed out or a psql exec failed, leaking a PVC and node capacity in trakrf-system. Install an EXIT trap right after `kubectl apply` succeeds (never before, so an earlier bucket/GSA-resolution abort has nothing to clean up), guarded by a scratch_applied flag so the normal success-path teardown doesn't double-delete or double-print. The trap deletes with --wait=false (fire-and-forget) since a blocking delete could itself hang for minutes on an already-failed run; the success path keeps its existing --wait=true teardown. Cleanup is best-effort — a failed delete only warns — and the trap re-asserts the recipe's real exit status via `exit "$status"`, so a failed PITR proof still exits non-zero. Mirrors the trap pattern already used in the sibling db-restore-test recipe. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Same superseded-topology defect as the restore recipes; prod is gated behind confirm_prod since it schedules work on the live primary. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Removes the ops.md §9 broken-recipe caveat and fixes backups.md's GCS path layout, trakrf-system references, and the two-database PITR check. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…Ready wait Production run against preview hit ENOSPC: the scratch Cluster hardcoded storage.size: 5Gi, but preview's trakrf DB is ~6GB and its base backup expands to ~6GB on restore. The recovery job crashlooped for the full 10-minute wait with no visible cause once the cleanup trap removed the evidence. Resolve storage_size from the source cluster's live spec (spec.storage.size), the same pattern already used for bucket and GSA, guarded the same way. This tracks the source automatically so it cannot silently drift undersized again as a database grows. Also raise the Ready wait from 10m to 20m (a real restore + WAL replay measured ~18m once storage is sized correctly) and print an operator hint pointing at the recovery job's logs (cnpg.io/jobRole=full-recovery) before the cleanup trap deletes the cluster on a wait failure, since losing that evidence to the trap cost real debugging time diagnosing this. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…e-pitr-test A real preview run completed end to end after the storage-size fix, but every table reported n_live_tup=0 and the recipe still exited 0 — a restore proof that prints all zeros and calls itself a success. Root cause: pg_stat_user_tables.n_live_tup is a stats-collector counter, not part of the physical backup. It is reset by recovery and reads 0 until autovacuum/ANALYZE repopulates it — that recipe was checking the one thing physical PITR cannot preserve. The 1.22 GiB compressed base backup, correctly restored schema, and correct ownership were strong independent evidence the data itself was fine; only the check was broken. Replace it with signals that DO survive a physical restore: pg_class.reltuples (a regular catalog column, written by ANALYZE via normal WAL-logged UPDATEs) for a full per-table overview, plus exact count(*) on two representative tables present in both envs today (organizations, users) for operator-readable confidence. Add an explicit emptiness gate judged on the trakrf schema AS A WHOLE — total tables and total estimated rows — so it can exit non-zero on a genuinely hollow restore without false-alarming on tables that are legitimately empty today (tag_scans/asset_scans, TRA-900). Verified against real preview data (28 tables, ~242k rows, passes) and against two constructed empty cases on a throwaway preview database — no schema at all, and schema-with-zero-row-tables — both correctly fail. db-restore-test's own pg_stat_user_tables query is unaffected and correct for its context (logical restore into an already-running cluster, where the collector does count the inserted rows) — out of scope, not touched. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The emptiness gate captured its result via
`read -r a b <<< "$(kubectl ... psql ...)"`. `set -e` does not fire for a
command substitution feeding a here-string, and `read` succeeds against an
empty here-string, so a failed query left both fields empty, `${var:-0}`
defaulted them to 0, and the recipe reported a healthy restore as
"restored trakrf schema looks EMPTY". Exit code right, diagnosis wrong —
during a recovery incident that sends the operator down the wrong path.
Capture the query output first and observe its exit status, then require
both fields to be numeric before judging emptiness. Three outcomes are now
distinct: ERROR: INCONCLUSIVE (query could not run), ERROR: INCONCLUSIVE
(ran but returned nothing usable, raw output echoed), and FAIL (the check
ran and the schema is genuinely empty). All still exit non-zero through the
cleanup trap; the genuine-empty path is exactly as strict as before and the
aggregate SQL is unchanged.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…tiness Four review findings on the restore-proof recipes, all in the same "resolve from live cluster state rather than hardcoding" spirit that this branch adopted after a hardcoded 5Gi scratch volume rotted into a real ENOSPC failure. - db-restore-pitr-test: derive the scratch Cluster's imageName from the source cluster's .spec.imageName instead of pinning ghcr.io/clevyr/cloudnativepg-timescale:17.2-ts2.18 two lines above the size we deliberately stopped hardcoding. A physical base backup can only be replayed by the PG major version that wrote it, so a helm bump to a PG 18 image would leave the scratch cluster unable to start, burning the full 20m Ready timeout before the trap deletes the evidence. Guarded the same way as storage_size, bucket and gsa. - db-restore-pitr-test: document why storageClass stays the literal premium-rwo (Delete reclaim) rather than the source's premium-rwo-retain — a throwaway cluster torn down every run must not retain a PV. - db-restore-test: add an emptiness gate, closing the asymmetry with the sibling PITR recipe. It printed \dn plus a rowcount table and then unconditionally reported success, so a schema-only dump or a dump of the wrong database produced a green proof. The gate uses pg_stat_user_tables.n_live_tup, which is correct HERE (pg_restore INSERTs into a running cluster, so the collector genuinely counts) and wrong in the PITR recipe (physical restore carries no collector state) — the asymmetry is deliberate. Judged on the trakrf schema as a whole, so the 10 legitimately empty tables in preview cannot false-alarm, and schema-agnostic so it works for preview (28 tables) and prod (15). "Could not run" stays distinct from "genuinely empty", following the PITR gate's capture-then-validate pattern. Runs before the DROP, so a failure still routes through the cleanup trap. - db-restore-test: make the "no dumps found" guard reachable. gcloud storage ls exits 1 on a non-matching prefix and pipefail propagates it, so the bare assignment aborted first and the message naming the bucket and prefix never printed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The documented escape hatch ("comment out the final kubectl delete
cluster line") stopped being reliable when the EXIT cleanup trap was
added mid-branch: `scratch_applied=0` sits on the line AFTER the
teardown, so an operator who comments out the whole teardown block
leaves the flag at 1 and the trap deletes the cluster on normal exit —
losing the cluster they wanted to inspect during a DR drill.
Spell out both deletion paths and exactly which line disarms the trap,
verified against the recipe as written: commenting out the kubectl
delete line works only if `scratch_applied=0` still runs; commenting out
the block requires also commenting out `trap cleanup EXIT`. Adds the
manual cleanup command. The trap itself is unchanged.
Also documents the new db-restore-test emptiness gate.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…gates Both restore proofs undercounted the restored database by ~98% because they only measured parent tables. trakrf.asset_scans and trakrf.tag_scans are TimescaleDB hypertables whose rows live in chunk relations under _timescaledb_internal, so the hypertable parent reports 0 in BOTH pg_stat_user_tables.n_live_tup (db-restore-test) and pg_class.reltuples (db-restore-pitr-test). Measured on preview: both gates scored 242,322 rows against a database actually holding 16,820,563 — about 1.4% of the truth. A restore that lost EVERY hypertable chunk would have passed both gates, and the output printed "asset_scans 0" directly above a PASS. That is the same false-green class these gates were written to prevent, one layer deeper, and the data it hid is the product's core time-series data. Both gates now sum plain tables and hypertables separately and judge the total, keeping their deliberately different sources: db-restore-test (logical restore into a running cluster) keeps n_live_tup for plain tables and uses exact count(*) per hypertable via query_to_xml, measured at 1.2s for 16.6M rows. Summing chunk n_live_tup was rejected: tag_scans is compressed on preview (5 of 8 chunks) and a compressed chunk stores ~1 row per 1000 source rows. db-restore-pitr-test (physical recovery, no collector state) keeps pg_class.reltuples for plain tables and adds approximate_row_count() per hypertable — derived purely from chunk reltuples plus _timescaledb_catalog.compression_chunk_size, all permanent WAL-logged relations, so it is as PITR-safe as reltuples and never scans the heap. Hypertables are discovered from timescaledb_information.hypertables, never hardcoded, and are excluded from the plain-table aggregate by oid so nothing is double counted. Row counts are reported split (plain vs hypertable) so an operator can see the time-series data came back, and the per-table listings now show hypertables with their real counts instead of 0. Verified read-only against both live clusters: preview logical 28 tables (26 plain + 2 ht), 16,820,563 rows (1.20s) preview pitr 28 tables (26 plain + 2 ht), ~16,291,280 rows (0.39s) prod logical 15 tables (13 plain + 2 ht), 766 rows (0.48s) prod pitr 15 tables (13 plain + 2 ht), ~675 rows (0.38s) Verified against throwaway scratch databases on preview that all three verdicts still separate: FAIL-empty on an empty database and on a schema with tables but no rows, INCONCLUSIVE-could-not-run when the query itself fails, PASS with real data. Prod's legitimately-empty tag_scans (0 rows, 0 chunks) does not trip a failure. Both cleanup traps still fire on a gate failure with exit status 1 preserved. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…able preview accumulates ~1 GB of WAL/hour from live MQTT ingestion (measured 2026-07-29: 5.14 GB / 1,193 segments in 5h11m since the last base backup), so a recover-to-latest run late in the day can need far more than the previous fixed 20m wait — the run this ticket investigates timed out mid-replay, not stuck. Base backups run daily at 09:30 UTC, so the worst case (just before the next one) is on the order of ~24 GB of WAL, and the new 120m default is sized with headroom above a straight-line extrapolation from the measured case. RESTORE_READY_TIMEOUT is an env var override (validated, falls back to the default with a warning on a malformed value) rather than a third positional arg, so it can't collide with ENV/TARGET_TIME ordering. The pre-wait message now states the real numbers and both ways to make a preview run fast: run shortly after the daily base backup, or pass a TARGET_TIME close to it to bound replay instead of chasing latest. No change to the recovery logic, emptiness gate, or the rendered Cluster manifest — only the kubectl wait timeout and operator-facing messaging. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…w proofs fast Mirrors the justfile's reasoning in docs/backups.md: recovery time scales with WAL accumulated since the source's last base backup (not dataset size), preview measured ~1 GB/hour from live MQTT ingestion, and the two ways to make a preview db-restore-pitr-test run fast are running it shortly after the 09:30 UTC base backup or passing a TARGET_TIME close to it — confirmed against CNPG's recovery docs that targetTime both selects the closest prior backup and stops WAL replay at the target, so it genuinely bounds replay rather than just being a different label for the same latest-WAL chase. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Running the logical proof (db-restore-test) writes real data to the live primary and is WAL-logged; running it before db-restore-pitr-test on the same env makes the PITR proof replay that extra WAL, slowing it markedly. Documents the preview measurements from today's session and recommends running the PITR proof first when both are needed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The RESTORE_READY_TIMEOUT commit attributed preview's WAL accumulation to live MQTT ingestion at ~1 GB/hour, extrapolating a ~24 GB worst case. That was wrong, and it landed a false causal claim in an operator runbook. Measured: steady-state WAL on preview is ~21 MB/hour via pg_current_wal_lsn() deltas, corroborated by ~6 archived WAL objects per 30-minute bucket — and that baseline already includes the MQTT scan soak test, ambient tag reads, continuous BLE reads, and the continuous-aggregate refresh. A full day is ~0.5 GB, not 24 GB. The 5.14 GB that exhausted the old 20m wait came from three `db-restore-test preview` runs earlier in the same session, each bursting ~1.4-1.7 GB by writing a full logical restore into a scratch DB on the live primary. WAL archival is spiky and tracks proof runs, not the clock. Corrects the justfile comment, the operator-facing pre-wait message, and docs/backups.md, and rewrites the "make a preview proof fast" guidance to lead with ordering (run the PITR proof before the logical one) rather than time of day, which barely matters at 21 MB/hour. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
Author
All four proofs green on the final commit
No leftover scratch clusters, PVCs, or databases in any namespace, prod included. Five commits added since the review aboveThe last stretch found three more defects, all by running the recipes rather than reading them. 1. Both gates measured ~1.4% of the data. Note the naive fix would also have been wrong: summing chunk-level stats undercounts by ~2.9M rows, because 5 of preview's 8 2. The 20m Ready timeout was too small. Now 120m, overridable via 3. A false causal claim shipped into the runbook and is now corrected. The timeout commit attributed preview's WAL accumulation to live ingestion at "~1 GB/hour" and extrapolated a ~24 GB worst case. Measured reality: steady-state WAL is ~21 MB/hour, and that baseline already includes the MQTT scan soak test, ambient tag reads, continuous BLE reads, and the continuous-aggregate refresh. A full day is ~0.5 GB. The 5.14 GB that exhausted the old timeout came from three That interaction is now documented: run DeferredTRA-1068 carries the gate-hardening items (total chunk loss still PASSes on the aggregate; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes the two restore-proof recipes, which still targeted the shared-cluster CNPG
topology that the per-env split superseded. Both were broken; neither had been run.
What was wrong
The ticket named three defects. Executing the recipes found three more.
From the ticket:
trakrf-system,cluster
trakrf-db, selectorcnpg.io/cluster=trakrf-db,role=primary. Livetopology is cluster-per-env:
trakrf-db-<env>intrakrf-<env>.db-restore-testlooked undergs://<bucket>/<env>/. The CronJobwrites
gs://<bucket>/<cluster>/dump/YYYY/MM/DD/HHMM.pgdump, and the old<env>/prefixes have since aged out of the bucket entirely. Fixing the namespace alone
was not sufficient.
tofu output, which fails without an initializedR2 backend.
Found by running them, after code review had passed the same code:
5Givolume; preview's database is 6,084 MB. Every run died with[Errno 28] No space left on deviceabout 30 seconds in, crashlooped, and burnedthe full Ready timeout. Prod (16 MB) would have passed by luck — a prod-only proof
would have reported the PITR path healthy.
n_live_tup = 0for all28 tables and still exited 0 with "restore proof complete".
pg_stat_user_tablesis statistics-collector state, which is not part of a physical base backup and
resets on recovery. The check could not distinguish a good restore from an empty
one, and had no failure path at all.
db-restore-testprinted results but never gated on them — the same class ofdefect, in the recipe that serves as the daily backup proof.
What changed
ENV(preview|prod), validated byrequire_env,resolving
trakrf-<env>/trakrf-db-<env>.cnpg_primary_podhelper inscripts/ops-lib.sh, keyed oncnpg.io/instanceRole=primary, which survives afailover.
just psqlmoves onto it too. Covered byscripts/test-ops-lib.sh.tofu. The bucket, the backup GSA, the scratch cluster'sstorage size, and its image are all read from live cluster state. This is the
branch's guiding decision: a hardcoded
5Giis what caused defect 4, so anythingthat can be derived is derived. Recipes now need no
.env.localand no R2reachability, and run from a worktree.
run" from "the restore is genuinely empty". The two recipes deliberately use
different sources —
pg_stat_user_tablesfor the logical restore (pg_restoreINSERTs into a running cluster, so the collector counts) and
pg_class.reltuplesfor PITR (physical recovery resets collector stats). Both gates are schema-agnostic,
which matters because preview and prod currently carry different schemas.
primary, nor a scratch cluster holding a PVC.
confirm_prodgatesdb-restore-testanddb-pitr-trigger-base. It isdeliberately absent from
db-restore-pitr-test, which never writes to the livecluster.
db-pitr-trigger-basecarried the same topology defect and is fixed with them.docs/ops.md§9 loses its "Currently broken" caveat;docs/backups.mdis correctedfor the path layout, the
trakrf-systemreferences, the singletrakrfdatabase,and an inspect-before-teardown escape hatch that our own cleanup trap had broken.
Proof
The point of these recipes is that they have been executed. All four ran to
completion on the shipped commit:
just db-restore-test previewjust db-restore-test prodjust db-restore-pitr-test previewstorage 20Giderivedjust db-restore-pitr-test prodstorage 10GiderivedCross-check: the PITR proof's exact counts (
organizations1023,users764 onpreview; 46 and 44 on prod) match the logical restore's numbers, from an entirely
different backup mechanism.
just db-pitr-trigger-base previewalso submitted a real Backup CR that reachedcompletedin 18 minutes — worth knowing, since the docs previously implied a coupleof minutes and an operator would reasonably have given up.
Afterwards: no leftover scratch clusters, PVCs, or databases in any namespace, prod
included.
scripts/test-ops-lib.sh14/14.Notes for the reviewer
the mechanism — credentials, Workload Identity, Timescale bracketing, the recipe
wiring — not restore at scale. Preview is the load-bearing proof today.
migrations behind preview (
schema_migrations10 vs 38; 15 tables vs 28).trakrf-system(a
trakrf-db-baseScheduledBackup pointing at a deleted cluster, a failedtrakrf-db-init-grantsJob, a staletrakrf-db/GCS prefix), the deadtrakrf-system/*WI bindings and stale comments interraform/gcp/cnpg_backups.tf,and a
trakrf-db-preview-basebackup that failed 20 days ago withexit status 4.🤖 Generated with Claude Code