Commit Graph

9 Commits

Author SHA1 Message Date
e80f11d5e4 fix(restore): refuse to write files into an unmounted backing store
Writing into a volume's mountpoint only restores it when the mountpoint is the storage. A volume with driver options - NFS, a bind device, tmpfs - keeps the same /var/lib/docker/volumes/<name>/_data path, but docker mounts the real backing store over it on demand and unmounts it again when the last consumer stops. Restoring while nothing holds it lands in the empty directory underneath, is hidden by the next mount, and rsync reports success.

The declaration decides, not the mount table: the driver and its options are true at every moment, where the mount table is only true while a container happens to hold the volume.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-17 05:25:54 +02:00
bb647c66ec feat(restore): refuse a dump the target engine cannot read
A restore with --empty destroys before it replays: the pre-clean drops the schema in one session and the dump goes in the next, with no rollback across the two. A dump the engine cannot parse therefore does not fail harmlessly, it leaves an emptied database behind. The version each side is on decides that up front, so the refusal lands before the first session opens.

Both engines state their origin in the dump's own header and spell it differently. Postgres names the source server; MariaDB opens with mariadb-dump's own version and names the server further down, so matching the first number would read the tool on one engine and the server on the other. A pg_dumpall stream carries no version line of its own at all - the first belongs to the first database's embedded pg_dump output, arbitrarily far down - hence the deep scan.

Restoring forward across a major version stays allowed; that is the upgrade path. Only backward is refused, with --no-version-check as the way out.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-17 05:25:29 +02:00
2129c5e362 build(lint): gate make test on a clean ruff run
ruff was never wired into this repository: no target, no CI step, no pin.
It reported 45 findings across sources and tests, so nothing enforced
what the codebase already mostly followed.

Adds `make ruff` (check + format --check), `make ruff-fix`, and `make
lint` as its alias, and makes `make test` run lint as a fourth parallel
spur. The CI workflow calls `make test`, so it is covered there too. The
linter is pinned in a `lint` extra: a ruff minor bump changes which rules
fire, and with the suite gating on a clean run an unpinned linter would
fail it on an unrelated day.

The 45 findings are fixed rather than configured away. Three needed a
decision instead of the mechanical fix:

- The generation timestamp keeps its local wall clock (DTZ005 waived).
  Generations sort by that name, and UTC would order new ones before the
  existing ones wherever the offset is positive - "newest generation" is
  what every restore path selects on.
- The per-volume `copy` closure now binds volume_name and vol_dir as
  default arguments (B023). It only worked because it is called inside
  the same iteration.
- The two CLI top-level handlers keep their blind except (BLE001
  waived): turning any failure into exit 1 is what a CLI boundary is
  for. The two in run.py did not need it and were narrowed to what they
  actually catch.

Also drops the comments that restate the code: the section banners in
restore/__main__.py, the filename repeated as line 1 of nine test files,
step narration above the statement it narrates, and a block in app.py
documenting parameters that had moved to another module. What names a
trip-wire stays - the snapshot destination rule, the mysql-binary
absence in MariaDB 11 images, the session-scoped FOREIGN_KEY_CHECKS, the
spooled temp file for multi-GB dumps, and the negative control that
loses its discriminating power if it ever passes.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-17 04:35:50 +02:00
a0204fd3ea feat(restore): replay pg_dumpall cluster dumps
A databases.csv row asking for every database of an instance
(database = '*') makes the backup side write <instance>.cluster.backup.sql
via pg_dumpall, and nothing could read it back: the restore CLI knew
files, postgres and mariadb. That dump was stored and unrestorable - a
format whose producer had no consumer.

Adds `baudolo-restore cluster`. Three properties of a cluster stream
shape it, and each one bit during development:

- It recreates databases, and CREATE DATABASE cannot run inside a
  transaction block. So unlike the single-database replay this one must
  NOT be wrapped in --single-transaction. The unit tests now pin both
  contracts against each other.
- It recreates every role including the one the replay connects as, and
  the pre-clean cannot drop the role holding its own session. That
  single CREATE ROLE is filtered out of the stream while its ALTER ROLE
  is kept, because that is what carries the attributes and the password.
  Found by running it: the first replay died on `role "postgres"
  already exists`.
- --empty means more than for one database: the cluster's databases go
  first, then DROP OWNED BY releases what a role still holds in the
  control database, then the roles themselves. The order is pinned by a
  phase column because \gexec would otherwise emit them interleaved, and
  a role cannot be dropped while it still owns a database.

Without --empty the replay stops at the first object that already
exists. Recreating a cluster over a populated one is a decision, not a
default.

The e2e test drills the real thing: two databases and their owning role
are dropped outright and have to come back with their payload and their
ownership intact.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-17 04:19:03 +02:00
8409843ff9 fix(restore): wrap the postgres dump replay in a single transaction
The dump replay ran statement-by-statement with autocommit. When restore --empty runs against a LIVE database, the pre-clean drops a table, the replay recreates it and autocommits, and a concurrent writer (discourse's mini_scheduler upserting scheduler_stats(id=1)) inserts the same primary key into the empty table before the dump's COPY loads it. The COPY then aborts with a duplicate-key violation under ON_ERROR_STOP and the whole restore fails. Running the replay with --single-transaction keeps the recreated table invisible to other sessions until commit, so the writer can never insert the racing row.

The --empty pre-clean stays multi-statement (\gexec, one DROP per statement): running every DROP in one transaction exhausts max_locks_per_transaction on large schemas (e.g. gitlab).

Extract the pre-clean SQL from the inline string into restore/db/empty_preclean.sql (loaded via dirname(__file__)) and declare it as package-data so it ships in the wheel. Add a unit test guarding the single-transaction/multi-statement split and an e2e that reproduces the live-writer race.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-20 20:00:31 +02:00
bd267cc280 fix(restore): drop text search objects in the postgres --empty pre-clean
A schema shipping a custom text search dictionary (taiga's english_stem_nostop) survived the pre-clean and aborted the dump replay with a duplicate pg_ts_dict_dictname_index violation under ON_ERROR_STOP. The discovery SELECT now also enumerates user-owned pg_ts_config and pg_ts_dict entries. The string-assertion unit test is replaced by real scenario data in the e2e: the seeded schema now contains an overloaded f()/f(int) pair and the nostop dictionary plus configuration, and the restored database is queried to prove each survives the backup, pre-clean and replay cycle exactly once.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-07-18 00:34:53 +02:00
01a00dd791 fix(restore): drop overloaded functions by identity signature in the --empty pre-clean
A signature-less DROP FUNCTION aborts with 'function name is not unique' as soon as a schema overloads a name, killing the whole --empty replay under ON_ERROR_STOP. The function branch now emits name(identity args) via pg_get_function_identity_arguments; because that compound must not be identifier-quoted as a whole, quoting moves from the outer format into each branch, guarded by a unit test that pins the per-branch %I quoting.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-07-17 17:27:55 +02:00
7a7ec57b54 fix(restore): drop user-owned collations in the --empty pre-clean
The drop_sql loop covered relations, routines, sequences and types but
not pg_collation, so a dump's CREATE COLLATION (OpenProject's ICU
public.versions_name) aborted the ON_ERROR_STOP replay with 'collation
already exists'. Add the fifth UNION ALL branch; DROP COLLATION IF
EXISTS public.<name> CASCADE rides the existing loop, and the loop
already drops every user table, so CASCADE fallout is absorbed by the
IF EXISTS no-ops.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-12 02:55:29 +02:00
96e6b3ea93 fix(backup,restore): harden the branch fixes and prove them with tests
Backup: a container that vanishes between the docker ps listing and the
swarm-task inspect (--rm one-shots, task-history GC) no longer aborts the
whole backup run; it counts as not stoppable and is skipped.

Restore: the postgres replay streams the dump through a spooled temp file
instead of buffering it three times in memory (multi-GB dumps OOMed the
restore mid-replay), and the superuser-only line filter is COPY-aware: data
rows inside COPY ... FROM stdin blocks pass through untouched, so a row
that happens to start with COMMENT ON EXTENSION or ALTER DEFAULT PRIVILEGES
is no longer silently dropped.

The e2e runner talks to the DinD daemon through docker exec instead of a
host-published tcp://127.0.0.1:2375: port publishing is unreachable from
sandboxed runners and from hosts with broken loopback publishing, and the
unencrypted root API port disappears from the host. The debug tmp dump
shrinks to tar plus docker cp against the DinD container itself.

New coverage: an e2e reproducing the swarm flake end to end (service task
on the volume, nothing whitelisted: the backup must succeed, the very same
task container must keep running, and the service must never replace a
task), unit tests for the COPY-aware filter, the swarm-task probe including
the vanished-container path, filter_stoppable ordering, and the one-session
FOREIGN_KEY_CHECKS drop assembly. Full suite: 35 unit, 9 integration,
30 e2e green.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-11 09:19:11 +02:00