mirror of
https://github.com/kevinveenbirkenbach/docker-volume-backup.git
synced 2026-08-24 23:04:34 +00:00
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>
144 lines
4.4 KiB
Python
144 lines
4.4 KiB
Python
from __future__ import annotations
|
|
|
|
import logging
|
|
import os
|
|
import pathlib
|
|
import re
|
|
|
|
import pandas
|
|
|
|
from .shell import BackupException, execute_shell_command
|
|
|
|
log = logging.getLogger(__name__)
|
|
|
|
|
|
def get_instance(container: str, database_containers: list[str]) -> str:
|
|
"""
|
|
Derive a stable instance name from the container name.
|
|
"""
|
|
if container in database_containers:
|
|
return container
|
|
return re.split(r"(_|-)(database|db|postgres)", container)[0]
|
|
|
|
|
|
def _validate_database_value(value: str | None, *, instance: str) -> str:
|
|
"""
|
|
Enforce explicit database semantics:
|
|
|
|
- "*" => dump ALL databases (cluster dump for Postgres)
|
|
- "<name>" => dump exactly this database
|
|
- "" => invalid configuration (would previously result in NaN / nan.backup.sql)
|
|
"""
|
|
v = (value or "").strip()
|
|
if v == "":
|
|
raise ValueError(
|
|
f"Invalid databases.csv entry for instance '{instance}': "
|
|
"column 'database' must be '*' or a concrete database name (not empty)."
|
|
)
|
|
return v
|
|
|
|
|
|
def _atomic_write_cmd(cmd: str, out_file: str) -> None:
|
|
"""
|
|
Write dump output atomically:
|
|
- write to <file>.tmp
|
|
- rename to <file> only on success
|
|
|
|
This prevents empty or partial dump files from being treated as valid backups.
|
|
"""
|
|
tmp = f"{out_file}.tmp"
|
|
execute_shell_command(f"{cmd} > {tmp}")
|
|
execute_shell_command(f"mv {tmp} {out_file}")
|
|
|
|
|
|
def fallback_pg_dumpall(
|
|
container: str, username: str, password: str, out_file: str
|
|
) -> None:
|
|
"""
|
|
Perform a full Postgres cluster dump using pg_dumpall.
|
|
"""
|
|
cmd = (
|
|
f"PGPASSWORD={password} docker exec -i {container} "
|
|
f"pg_dumpall -U {username} -h localhost"
|
|
)
|
|
_atomic_write_cmd(cmd, out_file)
|
|
|
|
|
|
def backup_database(
|
|
*,
|
|
container: str,
|
|
volume_dir: str,
|
|
db_type: str,
|
|
databases_df: pandas.DataFrame,
|
|
database_containers: list[str],
|
|
) -> bool:
|
|
"""
|
|
Backup databases for a given DB container.
|
|
|
|
Returns True if at least one dump was produced.
|
|
"""
|
|
instance_name = get_instance(container, database_containers)
|
|
|
|
entries = databases_df[databases_df["instance"] == instance_name]
|
|
if entries.empty:
|
|
log.debug("No database entries for instance '%s'", instance_name)
|
|
return False
|
|
|
|
out_dir = os.path.join(volume_dir, "sql")
|
|
pathlib.Path(out_dir).mkdir(parents=True, exist_ok=True)
|
|
|
|
produced = False
|
|
|
|
for row in entries.itertuples(index=False):
|
|
raw_db = getattr(row, "database", "")
|
|
user = (getattr(row, "username", "") or "").strip()
|
|
password = (getattr(row, "password", "") or "").strip()
|
|
|
|
db_value = _validate_database_value(raw_db, instance=instance_name)
|
|
|
|
if db_value == "*":
|
|
if db_type != "postgres":
|
|
raise ValueError(
|
|
f"databases.csv entry for instance '{instance_name}': "
|
|
"'*' is currently only supported for Postgres."
|
|
)
|
|
|
|
cluster_file = os.path.join(out_dir, f"{instance_name}.cluster.backup.sql")
|
|
fallback_pg_dumpall(container, user, password, cluster_file)
|
|
produced = True
|
|
continue
|
|
|
|
db_name = db_value
|
|
dump_file = os.path.join(out_dir, f"{db_name}.backup.sql")
|
|
|
|
if db_type == "mariadb":
|
|
# Force TCP so auth matches '<user>'@'%' instead of socket -> 'localhost'.
|
|
cmd = (
|
|
f"docker exec {container} /usr/bin/mariadb-dump "
|
|
f"-h 127.0.0.1 --protocol=tcp "
|
|
f"-u {user} -p{password} {db_name}"
|
|
)
|
|
_atomic_write_cmd(cmd, dump_file)
|
|
produced = True
|
|
continue
|
|
|
|
if db_type == "postgres":
|
|
try:
|
|
cmd = (
|
|
f"PGPASSWORD={password} docker exec -i {container} "
|
|
f"pg_dump -U {user} -d {db_name} -h localhost "
|
|
f"--no-owner --no-privileges"
|
|
)
|
|
_atomic_write_cmd(cmd, dump_file)
|
|
produced = True
|
|
except BackupException as e:
|
|
raise BackupException(
|
|
f"Postgres dump failed for instance '{instance_name}', "
|
|
f"database '{db_name}'. This database was explicitly configured "
|
|
"and therefore must succeed.\n"
|
|
f"{e}"
|
|
)
|
|
continue
|
|
|
|
return produced
|