fix: backups with non-default tablespaces (#17) + make demo actually run (#15)#18
Merged
Merged
Conversation
…#15) #17 — non-default tablespace backups PG streams the base/default tablespace archive (which carries backup_label and tablespace_map) LAST when user tablespaces exist. tarsink only intercepted those special files in archive index 0, so any cluster with a user tablespace produced a manifest with an empty backup_label and was refused at commit with backup.manifest_invalid. Capture the special files by name in whichever archive holds them (the exact-name match can only fire for the base tar; user-tablespace entries are nested under PG_<ver>_<cat>/...). Replaced the test that encoded the wrong "only in tablespace 0" assumption with a realistic regression test (base streamed last, after a user tablespace). #15 — `demo` was a stub `demo` printed a description and exited 0 without touching Docker. It now runs the documented end-to-end flow against a throwaway PG in Docker (init -> backup -> restore -> verify, automatic cleanup), driving the `docker` CLI so a custom DOCKER_HOST (Lima/Colima/Podman) is honoured and surfacing a clear demo.docker_unavailable error when Docker is unreachable instead of a fake success. The orchestration is behind a commandRunner seam and fully unit-tested (happy path, docker-unavailable, cleanup on mid-flow failure, port parsing); the real Docker run is exercised by CI. Docs/changelog updated.
This was referenced Jun 26, 2026
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
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 two reported bugs.
#17 — backups fail when a non-default tablespace exists
Creating any user tablespace made every backup fail to commit:
backup.manifest_invalid: backup_label is empty (required for restore).Cause: PG streams the base/default tablespace archive — the one carrying
backup_label+tablespace_map— last when user tablespaces exist. The tar sink only intercepted those special files in archive index 0, so they were never captured and the manifest'sbackup_labelcame out empty (failing its own invariant check).Fix: capture
backup_label/tablespace_mapby exact name in whichever archive holds them. The match can only fire for the base tar (user-tablespace entries are nested underPG_<ver>_<cat>/…), so it's safe and order-independent.Test: replaced the prior test that baked in the wrong "special files only in tablespace 0" assumption with a realistic regression — a user tablespace at index 0, the base tablespace streamed last at index 1 — asserting
backup_label/tablespace_mapare captured. Fails before the fix, passes after.#15 —
pg_hardstorage demodid nothingThe command printed a one-line description and exited 0 without ever touching Docker.
Fix: it now runs the documented end-to-end flow — start a throwaway PostgreSQL in Docker, init a repo, back up, restore, verify, then clean up. It drives the
dockerCLI so a non-default daemon set viaDOCKER_HOST(Lima/Colima/Podman — the reporter's case) is honoured, and returns a cleardemo.docker_unavailableerror instead of a fake success when Docker isn't reachable.Test: the orchestration sits behind a
commandRunnerseam and is fully unit-tested without Docker — happy path (full verb sequence + DSN wiring + teardown), Docker-unavailable, cleanup-on-mid-flow-failure, and port parsing. The real container run is covered by CI (which has Docker).Verified locally
go build ./...,go vet,internal/backup/tarsinkandinternal/clipackages all green. I couldn't run a Docker daemon in my dev sandbox, so the demo's real container run is validated by CI rather than locally; the orchestration logic itself is covered by the unit tests above.Closes #15. Closes #17.