Two defects kept dedicated Postgres databases out of the backup.
docker exec never forwarded PGPASSWORD. execute_to_file set it on baudolo's
own process, but nothing carried it across the container boundary, so an
engine whose pg_hba demands a password on TCP loopback refused the dump.
forward_env passes a bare `-e NAME`, letting docker copy the value out of
this process's environment instead of spelling it into argv, where the
host's process list would publish it.
get_instance returned the container name unchanged when that name carried no
database token, claiming an instance it had never derived. An application
container therefore answered the same databases.csv row as its own dedicated
engine, and application images often ship the engine's client tools, so the
dump command started and wrote a file that looked like a backup and held
none of the data. Discourse is the live case: its launcher names the
container `discourse`, and the image ships pg_dumpall.
The regex stays a normaliser - `<app>-database` from compose and
`<app>_database.1.<task>` from swarm still resolve to the same instance.
Only the fallthrough changes.
BREAKING CHANGE: a database container whose name carries no `database`, `db`
or `postgres` token must now be named in --database-containers. Without that
declaration its rows no longer match and no dump is written.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A finished generation cannot show whether a volume held a database, nor whether a dump was produced for it: under --only-sql a failed dump falls back to a file copy, and the resulting files/ tree looks like any other copy. The run knows both and threw the knowledge away as a printed warning, leaving every reader to guess from file names.
Each generation now carries a manifest.json stating its layout and, per volume, database / dumped / engine. baudolo.generation is the single place those names are spelled; restore/paths.py, backup/db.py and backup/volume.py stop repeating them. It is deliberately import-free so a consumer can read the manifest with nothing but json, on hosts where this package is not installed.
BREAKING CHANGE: BackupException is renamed BackupError. The rename is atomic across the ten modules that define or import it, three of which also carry the manifest change, so it lands in this commit rather than a separate one that could not import.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The backup built command strings and handed them to shell=True, so four interpolated values per dump - user, password, container, database - were each a way out of the command. validate_database covered one of them since the previous commit; now there is nothing to cover: every command is an argv list, and a value can only ever be an argument.
execute_to_file absorbs the atomic dump write. The shell redirect into <file>.tmp and the separate mv process become a Python file handle and os.replace, and a failing dump deletes its partial file instead of leaving it. PGPASSWORD moves out of the command string into the child's environment, where a process listing does not show it.
docker exec is built in one place, docker_exec_argv; db.py's three hand-built copies and the probe use it. The dead docker_volume_exists goes - never called, and the restore side owns the living twin. The rsync quoting in --link-dest falls away: inside an argv it would have become part of the path.
The snapshot module's injected runner changes type with it, which the three e2e drivers implement - the first conversion missed them, btrfs ran with no arguments, and the e2e caught it. Marked breaking for that contract: any external runner injected into volume_snapshot must now accept a list.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
36b2336 matched postgres and mariadb against the image's repository path. That name is not a property of the software: a dedicated Postgres inside an app's own stack is tagged <app>-database or postgis/postgis and carries no engine token at all, so it was never recognised, never dumped, and its data directory was copied as files without a single warning - the fallback notice hangs on found_db, which stayed false.
The container is now asked what it can run. pg_dumpall, mariadb-dump and mysqldump are probed by executing them, not by asking a shell for them, because a distroless image has no shell and would deny every tool it ships. The verdict is cached per image ID rather than per container, so replicas of one image cost a single probe.
Because the probe names the tool it found, the dump uses it instead of the hardcoded /usr/bin/mariadb-dump, which makes an image that ships only mysqldump dumpable rather than silently file-copied.
image_name and has_image are gone with their registry-host and tag stripping; the trap they worked around cannot occur when nothing reads the name. get_image_info stays, since the --images-* lists match exact references.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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>
has_image tested the pattern against the raw .Config.Image, so anything in the reference could decide which dump tool runs -- including the registry host and the tag. A swarm node that hosts the local registry prefixes every pull with its own name, and that node is named after the app under test, so a Postgres container reads as svc-db-mariadb-swarm-mgr-01:5000/postgres_custom:17-3.5. dumps.py tries mariadb before postgres, matched on the hostname, and dumped Postgres with mariadb-dump: exit 127, the image does not ship it. The BackupException took the backup unit down with it.
image_name strips digest, tag and registry host and matches on the repository path, so the decision rests on the image alone. Same intent as the exact --images-* matching from f9776ac, applied to the one place that commit did not reach. Tags stop deciding too: xwiki_custom:lts-postgres-tomcat no longer reads as Postgres.
The e2e reproduces the shape without a registry -- a docker tag is enough for .Config.Image to carry the reference verbatim -- and asserts a real pg_dump lands. Under the old code mariadb-dump aborts and no dump file exists.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Treating every failed swarm-task inspect as skippable opened a false-green
window: a transient inspect failure on a still-running, non-whitelisted
container skipped the stop and backed the volume up hot while the run
reported success. Re-check whether the container is still listed; only a
genuinely vanished container skips, an existing one re-raises so a broken
daemon keeps failing the run loudly. Covered by unit tests for both paths.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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>
Restore fixes, both hit by the infinito svc-bkp e2e drill:
- mariadb --empty dropped tables one docker exec at a time with SET
FOREIGN_KEY_CHECKS=0 issued in a separate client session, so the
session-scoped toggle never applied and any FK-referenced parent table
(mailu.users) died with ERROR 1451. Issue the toggle and all DROPs in
one session.
- postgres --empty now drops only current_user-owned objects (extension
members like pg_trgm's set_limit are superuser-owned) with IF EXISTS
absorbing CASCADE fallout, and the replay skips superuser-only dump
lines (COMMENT ON EXTENSION, ALTER DEFAULT PRIVILEGES) that abort an
app-user psql run under ON_ERROR_STOP.
Backup fixes:
- pg_dump now runs with --no-owner --no-privileges so future dumps are
replayable by the owning app user in the first place.
- Swarm task containers are never stopped or started manually: the
orchestrator replaces a stopped task and a later docker start fails on
the detached overlay network. filter_stoppable skips them visibly and
the whitelist stop check ignores them.
Validated end to end against a live infinito compose stack: the full
svc-bkp-volume-2-local drill (verify, restore cycle, sql replay for
mailu, keycloak and one more db) passes with these patches applied.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
