fix(restic): dump every database, not just Forgejo's (#928) #959

Merged

coilyco-ops merged 1 commit from ops/928-db-dumps into main

2026-08-27 04:12:28 +00:00

coilyco-ops commented

2026-08-27 04:10:51 +00:00

Owner

Closes #928. Deploy-side counterpart is coilyco-bridge/deploy#794, where Teable is now running, so this is ordered before it holds real rows.

The half-change this fixes

infrastructure#951 added teable to RESTIC_BACKUP_NAMESPACES without a dump entry. That is precisely the failure #928 was filed to prevent, and it is live today: PVC discovery hands Restic teable-db-data, a live PostgreSQL data directory, as a raw file copy taken while the database is writing. Restic snapshots it happily and reports success, and it may not restore.

teable-assets was correctly captured by that change. The database was not, and the backup would have looked complete.

What changed

create_forgejo_db_dump was hardcoded to one namespace, target, user, and database. It is now create_db_dump driven by:

RESTIC_BACKUP_DB_DUMPS="forgejo:statefulset/forgejo-db:forgejo:forgejo teable:deployment/teable-db:teable:teable"

staging at <root>/<namespace>/<database>.pgdump. That is where forgejo's dump already lived, so the on-disk layout and existing snapshots are unchanged, and nothing outside the script sets the old RESTIC_BACKUP_FORGEJO_DB_* variables.

Behaviour preserved: a failed or empty dump aborts before Restic runs. Behaviour added: a namespace that does not exist is skipped rather than failed, matching what PVC discovery already does for an undeployed service.

teable's user and database are teable, and the target is deployment/teable-db, read from services/teable/deploy/teable-datastore.yml rather than assumed.

The harness moved with it

#928 named this as the reason not to do the change casually: the old harness asserted the exact string exec statefulset/forgejo-db -- pg_dump -Fc -U forgejo -d forgejo, so generalising the script alone would have left the tested default and the real default free to diverge.

It now drives both namespaces through the script's own default list rather than a second copy, and adds a drift guard: a namespace in the default backup set with no dump entry fails the test unless it is in namespaces_without_databases, today registry alone.

Both controls were verified to fire, not assumed

Added a fourth namespace with no dump entry, and the guard failed with namespace somedb is in the backup set with no dump entry.
Pointed teable's dump at a nonexistent target, and the run failed closed.

Restored and re-ran clean after each. just restic-backup-test passes, and pre-commit run over the changed files is clean.

Not done here

The real run is attended and lands on kai-server via #923. just restic-backup-test is offline by construction and touches neither k3s nor restic, so #928's done-condition of "a real run produces a non-empty teable.pgdump" is still outstanding and needs that attended run.

Closes #928. Deploy-side counterpart is coilyco-bridge/deploy#794, where Teable is now running, so this is ordered before it holds real rows. ## The half-change this fixes infrastructure#951 added `teable` to `RESTIC_BACKUP_NAMESPACES` without a dump entry. That is precisely the failure #928 was filed to prevent, and it is live today: PVC discovery hands Restic `teable-db-data`, a live PostgreSQL data directory, as a raw file copy taken while the database is writing. Restic snapshots it happily and reports success, and it may not restore. `teable-assets` was correctly captured by that change. The database was not, and the backup would have looked complete. ## What changed `create_forgejo_db_dump` was hardcoded to one namespace, target, user, and database. It is now `create_db_dump` driven by: ``` RESTIC_BACKUP_DB_DUMPS="forgejo:statefulset/forgejo-db:forgejo:forgejo teable:deployment/teable-db:teable:teable" ``` staging at `<root>/<namespace>/<database>.pgdump`. That is where forgejo's dump already lived, so the on-disk layout and existing snapshots are unchanged, and nothing outside the script sets the old `RESTIC_BACKUP_FORGEJO_DB_*` variables. Behaviour preserved: a failed or empty dump aborts before Restic runs. Behaviour added: a namespace that does not exist is skipped rather than failed, matching what PVC discovery already does for an undeployed service. teable's user and database are `teable`, and the target is `deployment/teable-db`, read from `services/teable/deploy/teable-datastore.yml` rather than assumed. ## The harness moved with it #928 named this as the reason not to do the change casually: the old harness asserted the exact string `exec statefulset/forgejo-db -- pg_dump -Fc -U forgejo -d forgejo`, so generalising the script alone would have left the tested default and the real default free to diverge. It now drives **both** namespaces through the script's own default list rather than a second copy, and adds a drift guard: a namespace in the default backup set with no dump entry fails the test unless it is in `namespaces_without_databases`, today `registry` alone. ## Both controls were verified to fire, not assumed * Added a fourth namespace with no dump entry, and the guard failed with `namespace somedb is in the backup set with no dump entry`. * Pointed teable's dump at a nonexistent target, and the run failed closed. Restored and re-ran clean after each. `just restic-backup-test` passes, and `pre-commit run` over the changed files is clean. ## Not done here The real run is attended and lands on kai-server via #923. `just restic-backup-test` is offline by construction and touches neither k3s nor restic, so #928's done-condition of "a real run produces a non-empty `teable.pgdump`" is still outstanding and needs that attended run.

coilyco-ops added 1 commit

2026-08-27 04:10:51 +00:00

fix(restic): dump every database, not just Forgejo's (#928 )

TruffleHog / Scan for secrets (pull_request) Successful in 5s

Details

CI / lint (pull_request) Successful in 53s

Details

359abd9da1

teable went into RESTIC_BACKUP_NAMESPACES in #951 without a dump entry,
which is the exact half-change #928 was filed to prevent. PVC discovery
hands Restic teable-db-data, a live PostgreSQL data directory, as a raw
file copy taken while the database is writing. Restic snapshots it and
reports success, and it may not restore. On an index whose whole
justification is that rows are deletable and git is not the store, that
is worse than knowing it is unprotected.

create_forgejo_db_dump was hardcoded to one namespace, target, user and
database. It is now create_db_dump over RESTIC_BACKUP_DB_DUMPS, entries
of namespace:target:user:database, staging at
<root>/<namespace>/<database>.pgdump. That is where forgejo's dump
already lived, so the on-disk layout and existing snapshots are
unchanged. A failed or empty dump still aborts the run before Restic
starts. A namespace that does not exist is skipped rather than failed,
matching what PVC discovery already does for an undeployed service.

The harness moves with it, which is the part #928 flagged as the reason
not to do this casually. It now drives both namespaces through the
script's own default list rather than a second copy, and adds the drift
guard: a namespace in the default backup set with no dump entry fails
the test unless it is in namespaces_without_databases, today registry
alone. Both controls were verified to fire rather than assumed.

Deploy-side counterpart is coilyco-bridge/deploy#794. Teable is running
now, so this is ordered before it holds real rows.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-authored-by: Kai Siren <coilysiren@gmail.com>
Co-authored-by: Claude <noreply@anthropic.com>
Agent-Role: sysadmin