coilyco-flight-deck/infrastructure

Watch

Teable's Postgres needs a pg_dump entry before it holds data, because the namespace list alone gives it a torn copy #928

New issue

Closed

opened 2026-08-25 09:40:51 +00:00 by coilyco-ops · 1 comment

coilyco-ops commented

2026-08-25 09:40:51 +00:00

Owner

Sysadmin prerequisite for coilyco-bridge/deploy#794, which deploys Teable as the connections index. That issue's acceptance includes "backed up by the existing path", and the obvious one-word change does not deliver it.

The trap

scripts/restic-backup.sh discovers every Bound PVC in RESTIC_BACKUP_NAMESPACES, currently forgejo registry. So the obvious change is:

RESTIC_BACKUP_NAMESPACES="${RESTIC_BACKUP_NAMESPACES:-forgejo registry teable}"

That would correctly capture Teable's teable-assets PVC, which is an ordinary filesystem. It would also capture teable-db-data, which is a live Postgres data directory, as a raw file copy taken while the database is writing. Restic snapshots it happily and reports success.

The script already knows this is wrong, and says so in its own words for Forgejo:

echo "restic-backup: creating an application-consistent Forgejo PostgreSQL dump"
"$K3S_BIN" kubectl -n forgejo exec "$RESTIC_BACKUP_FORGEJO_DB_TARGET" -- \
  pg_dump -Fc -U forgejo -d forgejo >"$dump_tmp"

It fails hard on a failed or empty dump, which is the right severity for the only copy of something.

So adding the namespace without a dump entry produces a backup that looks complete and holds a database that 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.

What it needs

create_forgejo_db_dump is hardcoded to one namespace, target, user and database. Teable needs the same treatment against deployment/teable-db, user teable, database teable.

Generalising is the right shape, roughly:

RESTIC_BACKUP_DB_DUMPS="${RESTIC_BACKUP_DB_DUMPS:-forgejo:statefulset/forgejo-db:forgejo:forgejo teable:deployment/teable-db:teable:teable}"

with the dump directory derived per entry rather than the single RESTIC_BACKUP_FORGEJO_DB_DIR.

Why I did not just do it

scripts/test-restic-backup.sh drives the script against a fake k3s whose command dispatch ends in *) exit 64. It asserts the exact string exec statefulset/forgejo-db -- pg_dump -Fc -U forgejo -d forgejo. So generalising means changing the script and its harness together, or the default list and the tested default diverge, which is its own silent-drift hazard.

That is careful surgery on the estate's only working backup path. It is also not on the critical path: Teable cannot start until its Tailscale authkey exists, which needs admin credentials docs/tailscale.md deliberately keeps out of SSM. So there is no data to protect yet, and the change deserves a fresh look rather than being appended to a long session.

Order that matters

Generalise the dump list, update the test harness, just test-restic-backup green.
Add teable to RESTIC_BACKUP_NAMESPACES so the assets PVC is captured.
Only then let Teable hold real rows.

Doing 2 without 1 is the failure this issue exists to prevent.

Worth noting the whole thing lands on kai-server, so it needs the attended sudo run tracked in #923.

Done means

just test-restic-backup passes with a teable dump entry, a real run produces a non-empty teable.pgdump, and RESTIC_BACKUP_NAMESPACES includes teable.

Sysadmin prerequisite for `coilyco-bridge/deploy#794`, which deploys Teable as the connections index. That issue's acceptance includes "backed up by the existing path", and the obvious one-word change does not deliver it. ## The trap `scripts/restic-backup.sh` discovers every Bound PVC in `RESTIC_BACKUP_NAMESPACES`, currently `forgejo registry`. So the obvious change is: ```sh RESTIC_BACKUP_NAMESPACES="${RESTIC_BACKUP_NAMESPACES:-forgejo registry teable}" ``` That would correctly capture Teable's `teable-assets` PVC, which is an ordinary filesystem. It would also capture `teable-db-data`, which is a **live Postgres data directory**, as a raw file copy taken while the database is writing. Restic snapshots it happily and reports success. The script already knows this is wrong, and says so in its own words for Forgejo: ```sh echo "restic-backup: creating an application-consistent Forgejo PostgreSQL dump" "$K3S_BIN" kubectl -n forgejo exec "$RESTIC_BACKUP_FORGEJO_DB_TARGET" -- \ pg_dump -Fc -U forgejo -d forgejo >"$dump_tmp" ``` It fails hard on a failed or empty dump, which is the right severity for the only copy of something. So adding the namespace without a dump entry produces a backup that **looks** complete and holds a database that 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. ## What it needs `create_forgejo_db_dump` is hardcoded to one namespace, target, user and database. Teable needs the same treatment against `deployment/teable-db`, user `teable`, database `teable`. Generalising is the right shape, roughly: ```sh RESTIC_BACKUP_DB_DUMPS="${RESTIC_BACKUP_DB_DUMPS:-forgejo:statefulset/forgejo-db:forgejo:forgejo teable:deployment/teable-db:teable:teable}" ``` with the dump directory derived per entry rather than the single `RESTIC_BACKUP_FORGEJO_DB_DIR`. ## Why I did not just do it `scripts/test-restic-backup.sh` drives the script against a fake `k3s` whose command dispatch ends in `*) exit 64`. It asserts the exact string `exec statefulset/forgejo-db -- pg_dump -Fc -U forgejo -d forgejo`. So generalising means changing the script and its harness together, or the default list and the tested default diverge, which is its own silent-drift hazard. That is careful surgery on the estate's **only** working backup path. It is also not on the critical path: Teable cannot start until its Tailscale authkey exists, which needs admin credentials `docs/tailscale.md` deliberately keeps out of SSM. So there is no data to protect yet, and the change deserves a fresh look rather than being appended to a long session. ## Order that matters 1. Generalise the dump list, update the test harness, `just test-restic-backup` green. 2. Add `teable` to `RESTIC_BACKUP_NAMESPACES` so the assets PVC is captured. 3. Only then let Teable hold real rows. Doing 2 without 1 is the failure this issue exists to prevent. Worth noting the whole thing lands on kai-server, so it needs the attended sudo run tracked in #923. ## Done means `just test-restic-backup` passes with a teable dump entry, a real run produces a non-empty `teable.pgdump`, and `RESTIC_BACKUP_NAMESPACES` includes `teable`.

coilyco-ops referenced this issue from a pull request that will close it,

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

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

coilyco-ops referenced this issue from a commit

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

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

coilyco-ops closed this issue

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

coilyco-ops referenced this issue from a commit

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

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

coilyco-ops commented

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

Author

Owner

Steps 1 and 2 done and merged, #959. Step 3 still needs an attended run.

Taken on now rather than later because Teable is running as of tonight (deploy#794), so the window before it holds real rows is the window this issue asked for.

The trap was already sprung

Worth recording: #951 added teable to RESTIC_BACKUP_NAMESPACES without a dump entry, which is the exact half-change this issue exists to prevent. Since then, PVC discovery would have handed Restic teable-db-data as a raw copy of a live PostgreSQL data directory. teable-assets was correctly captured. The database was not, and the snapshot would have reported success.

What landed

create_forgejo_db_dump is now create_db_dump over the shape this issue proposed:

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

Staging is <root>/<namespace>/<database>.pgdump, which is where forgejo's dump already lived, so the on-disk layout and existing snapshots are untouched. Nothing outside the script ever set the old RESTIC_BACKUP_FORGEJO_DB_* variables, so there was no external consumer to break.

Fail-closed on a failed or empty dump is preserved. New: a namespace that does not exist is skipped rather than failed, matching what PVC discovery already does, so an undeployed service cannot break Forgejo's backup.

teable's deployment/teable-db, user teable, database teable were read out of services/teable/deploy/teable-datastore.yml rather than assumed.

The harness, which is the part you flagged

You were right that this was the reason not to append it to a long session. The old harness asserted the literal string exec statefulset/forgejo-db -- pg_dump -Fc -U forgejo -d forgejo, so generalising the script alone would have let the tested default and the real default drift apart silently.

It now drives both namespaces through the script's own default list rather than a second copy of it, and adds the guard that makes the ordering enforceable instead of remembered: 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 verified to fire

A guard that cannot fail is not a guard, so I checked rather than assumed:

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

Restored and re-ran clean after each. just restic-backup-test passes, pre-commit run over the changed files is clean, and CI on #959 was green on both jobs.

Small correction to this issue's text: the verb is just restic-backup-test, not just test-restic-backup.

Still open, which is why this issue is not closed

Step 3, the real run. just restic-backup-test is offline by construction and touches neither k3s nor restic, so the done-condition "a real run produces a non-empty teable.pgdump" is unproven. That is the attended sudo run on kai-server tracked in #923. Until it happens, teable's Postgres has a correct dump path in the repository and no evidence it works against the live database.

## Steps 1 and 2 done and merged, #959. Step 3 still needs an attended run. Taken on now rather than later because Teable is running as of tonight (deploy#794), so the window before it holds real rows is the window this issue asked for. ### The trap was already sprung Worth recording: #951 added `teable` to `RESTIC_BACKUP_NAMESPACES` **without** a dump entry, which is the exact half-change this issue exists to prevent. Since then, PVC discovery would have handed Restic `teable-db-data` as a raw copy of a live PostgreSQL data directory. `teable-assets` was correctly captured. The database was not, and the snapshot would have reported success. ### What landed `create_forgejo_db_dump` is now `create_db_dump` over the shape this issue proposed: ``` RESTIC_BACKUP_DB_DUMPS="forgejo:statefulset/forgejo-db:forgejo:forgejo teable:deployment/teable-db:teable:teable" ``` Staging is `<root>/<namespace>/<database>.pgdump`, which is where forgejo's dump already lived, so the on-disk layout and existing snapshots are untouched. Nothing outside the script ever set the old `RESTIC_BACKUP_FORGEJO_DB_*` variables, so there was no external consumer to break. Fail-closed on a failed or empty dump is preserved. New: a namespace that does not exist is skipped rather than failed, matching what PVC discovery already does, so an undeployed service cannot break Forgejo's backup. teable's `deployment/teable-db`, user `teable`, database `teable` were read out of `services/teable/deploy/teable-datastore.yml` rather than assumed. ### The harness, which is the part you flagged You were right that this was the reason not to append it to a long session. The old harness asserted the literal string `exec statefulset/forgejo-db -- pg_dump -Fc -U forgejo -d forgejo`, so generalising the script alone would have let the tested default and the real default drift apart silently. It now drives both namespaces through the script's **own** default list rather than a second copy of it, and adds the guard that makes the ordering enforceable instead of remembered: 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 verified to fire A guard that cannot fail is not a guard, so I checked rather than assumed: * Added a fourth namespace with no dump entry. Failed with `namespace somedb is in the backup set with no dump entry`. * Pointed teable's dump at a nonexistent target. Run failed closed, and Restic did not run. Restored and re-ran clean after each. `just restic-backup-test` passes, `pre-commit run` over the changed files is clean, and CI on #959 was green on both jobs. Small correction to this issue's text: the verb is `just restic-backup-test`, not `just test-restic-backup`. ### Still open, which is why this issue is not closed Step 3, the real run. `just restic-backup-test` is offline by construction and touches neither k3s nor restic, so the done-condition "a real run produces a non-empty `teable.pgdump`" is unproven. That is the attended sudo run on kai-server tracked in #923. Until it happens, teable's Postgres has a correct dump path in the repository and no evidence it works against the live database.

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference

coilyco-flight-deck/infrastructure#928

No description provided.

Rows
Columns