diff --git a/src/baudolo/backup/app.py b/src/baudolo/backup/app.py index 356352e..5b8a55b 100644 --- a/src/baudolo/backup/app.py +++ b/src/baudolo/backup/app.py @@ -2,9 +2,9 @@ from __future__ import annotations -import os from contextlib import ExitStack from datetime import datetime +from pathlib import Path from .cli import parse_args from .compose import handle_docker_compose_services @@ -14,12 +14,13 @@ from .docker import ( docker_volume_names, filter_stoppable, ) -from .dumps import backup_dumps_for_volume, load_databases_df +from .dumps import VolumeOutcome, backup_dumps_for_volume, load_databases_df from .layout import ( create_version_directory, create_volume_directory, get_machine_id, stamp_directory, + write_manifest, ) from .policy import requires_stop, volume_is_fully_ignored from .snapshot import snapshot_source, volume_snapshot @@ -34,13 +35,15 @@ def main() -> int: # order new ones before the existing ones wherever the offset is positive. backup_time = datetime.now().strftime("%Y%m%d%H%M%S") # noqa: DTZ005 - versions_dir = os.path.join(args.backups_dir, machine_id, args.repo_name) + versions_dir = str(Path(args.backups_dir) / machine_id / args.repo_name) version_dir = create_version_directory(versions_dir, backup_time) databases_df = None if args.only_files else load_databases_df(args.databases_csv) print("💾 Start volume backups...", flush=True) + outcomes: dict[str, VolumeOutcome] = {} + with ExitStack() as stack: resolve_source = None if args.snapshot: @@ -69,17 +72,18 @@ def main() -> int: vol_dir = create_volume_directory(version_dir, volume_name) - found_db = dumped_any = False + outcome = VolumeOutcome(database=False, dumped=False) if not args.only_files: - found_db, dumped_any = backup_dumps_for_volume( + outcome = backup_dumps_for_volume( containers=containers, vol_dir=vol_dir, databases_df=databases_df, database_containers=args.database_containers, ) + outcomes[volume_name] = outcome - if args.only_sql and found_db: - if not dumped_any: + if args.only_sql and outcome.database: + if not outcome.dumped: print( f"WARNING: only-sql requested but no DB dump was produced for DB volume '{volume_name}'. " "Falling back to file backup.", @@ -129,6 +133,7 @@ def main() -> int: if not args.shutdown: change_containers_status(stoppable, "start") + write_manifest(version_dir, outcomes) stamp_directory(version_dir) print("Finished volume backups.", flush=True) diff --git a/src/baudolo/backup/db.py b/src/baudolo/backup/db.py index 2045996..96971fa 100644 --- a/src/baudolo/backup/db.py +++ b/src/baudolo/backup/db.py @@ -1,16 +1,18 @@ from __future__ import annotations import logging -import os import pathlib import re - -import pandas +from typing import TYPE_CHECKING from baudolo.databases import CLUSTER_ROW, validate_database +from baudolo.generation import CLUSTER_SUFFIX, DUMP_SUFFIX, SQL_DIR from .docker import docker_exec_argv -from .shell import BackupException, execute_to_file +from .shell import BackupError, execute_to_file + +if TYPE_CHECKING: + import pandas as pd log = logging.getLogger(__name__) @@ -47,7 +49,7 @@ def backup_database( volume_dir: str, db_type: str, dump_tool: str, - databases_df: pandas.DataFrame, + databases_df: pd.DataFrame, database_containers: list[str], ) -> bool: """ @@ -66,8 +68,8 @@ def backup_database( 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) + out_dir = pathlib.Path(volume_dir) / SQL_DIR + out_dir.mkdir(parents=True, exist_ok=True) produced = False @@ -85,13 +87,13 @@ def backup_database( f"'{CLUSTER_ROW}' is currently only supported for Postgres." ) - cluster_file = os.path.join(out_dir, f"{instance_name}.cluster.backup.sql") + cluster_file = str(out_dir / f"{instance_name}{CLUSTER_SUFFIX}") 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") + dump_file = str(out_dir / f"{db_name}{DUMP_SUFFIX}") if db_type == "mariadb": # Force TCP so auth matches ''@'%' instead of socket -> 'localhost'. @@ -136,13 +138,13 @@ def backup_database( env={"PGPASSWORD": password}, ) produced = True - except BackupException as e: - raise BackupException( + except BackupError as e: + raise BackupError( f"Postgres dump failed for instance '{instance_name}', " f"database '{db_name}'. This database was explicitly configured " "and therefore must succeed.\n" f"{e}" - ) + ) from e continue return produced diff --git a/src/baudolo/backup/docker.py b/src/baudolo/backup/docker.py index 825e35a..5ff62f8 100644 --- a/src/baudolo/backup/docker.py +++ b/src/baudolo/backup/docker.py @@ -1,8 +1,11 @@ from __future__ import annotations -from collections.abc import Sequence +from typing import TYPE_CHECKING -from .shell import BackupException, execute_shell_command +from .shell import BackupError, execute_shell_command + +if TYPE_CHECKING: + from collections.abc import Sequence def docker_exec_argv( @@ -34,7 +37,7 @@ def has_tool(container: str, tool: str) -> bool: """ try: execute_shell_command(docker_exec_argv(container, [tool, "--version"])) - except BackupException: + except BackupError: return False return True @@ -74,7 +77,7 @@ def is_swarm_task(container: str) -> bool: container, ] ) - except BackupException: + except BackupError: still_listed = execute_shell_command( [ "docker", diff --git a/src/baudolo/backup/dumps.py b/src/baudolo/backup/dumps.py index bac7836..dabe38e 100644 --- a/src/baudolo/backup/dumps.py +++ b/src/baudolo/backup/dumps.py @@ -3,8 +3,9 @@ from __future__ import annotations import sys +from typing import NamedTuple -import pandas +import pandas as pd from pandas.errors import EmptyDataError from baudolo.databases import COLUMNS, DELIMITER @@ -21,6 +22,19 @@ DUMP_TOOLS: tuple[tuple[str, str], ...] = ( _ENGINE_BY_IMAGE: dict[str, tuple[str, str] | None] = {} +class VolumeOutcome(NamedTuple): + """What a dump attempt established about one volume. + + ``database`` says a container serving the volume speaks an engine this + tool can dump; ``dumped`` says a dump was actually written. ``engine`` is + the engine that was detected, or None when none was. + """ + + database: bool + dumped: bool + engine: str | None = None + + def container_engine(container: str) -> tuple[str, str] | None: """The (engine, dump tool) a container can serve, or None for neither. @@ -55,15 +69,13 @@ def backup_mariadb_or_postgres( *, container: str, volume_dir: str, - databases_df: pandas.DataFrame, + databases_df: pd.DataFrame, database_containers: list[str], -) -> tuple[bool, bool]: - """ - Returns (is_db_container, dumped_any) - """ +) -> VolumeOutcome: + """What this container contributes to its volume's outcome.""" engine = container_engine(container) if engine is None: - return False, False + return VolumeOutcome(database=False, dumped=False) db_type, dump_tool = engine dumped = backup_database( container=container, @@ -73,20 +85,20 @@ def backup_mariadb_or_postgres( databases_df=databases_df, database_containers=database_containers, ) - return True, dumped + return VolumeOutcome(database=True, dumped=dumped, engine=db_type) -def _empty_databases_df() -> pandas.DataFrame: +def _empty_databases_df() -> pd.DataFrame: """ Create an empty DataFrame with the expected schema for databases.csv. This allows the backup to continue without DB dumps when the CSV is missing or empty (pandas EmptyDataError). """ - return pandas.DataFrame(columns=list(COLUMNS)) + return pd.DataFrame(columns=list(COLUMNS)) -def load_databases_df(csv_path: str) -> pandas.DataFrame: +def load_databases_df(csv_path: str) -> pd.DataFrame: """ Load databases.csv robustly. @@ -95,9 +107,7 @@ def load_databases_df(csv_path: str) -> pandas.DataFrame: - Valid CSV -> return dataframe """ try: - return pandas.read_csv( - csv_path, sep=DELIMITER, keep_default_na=False, dtype=str - ) + return pd.read_csv(csv_path, sep=DELIMITER, keep_default_na=False, dtype=str) except FileNotFoundError: print( f"WARNING: databases.csv not found: {csv_path}. Continuing without database dumps.", @@ -118,25 +128,26 @@ def backup_dumps_for_volume( *, containers: list[str], vol_dir: str, - databases_df: pandas.DataFrame, + databases_df: pd.DataFrame, database_containers: list[str], -) -> tuple[bool, bool]: - """ - Returns (found_db_container, dumped_any) - """ +) -> VolumeOutcome: + """The volume's outcome across every container that mounts it.""" found_db = False dumped_any = False + engine: str | None = None for c in containers: - is_db, dumped = backup_mariadb_or_postgres( + outcome = backup_mariadb_or_postgres( container=c, volume_dir=vol_dir, databases_df=databases_df, database_containers=database_containers, ) - if is_db: + if outcome.database: found_db = True - if dumped: + if outcome.dumped: dumped_any = True + if engine is None: + engine = outcome.engine - return found_db, dumped_any + return VolumeOutcome(database=found_db, dumped=dumped_any, engine=engine) diff --git a/src/baudolo/backup/layout.py b/src/baudolo/backup/layout.py index 0374134..7b65002 100644 --- a/src/baudolo/backup/layout.py +++ b/src/baudolo/backup/layout.py @@ -2,12 +2,14 @@ from __future__ import annotations -import os +import json import pathlib from dirval import create_stamp_file -from .shell import BackupException, execute_shell_command +from baudolo.generation import MANIFEST_FILE, manifest_document + +from .shell import BackupError, execute_shell_command def get_machine_id() -> str: @@ -22,11 +24,11 @@ def stamp_directory(version_dir: str) -> None: def create_version_directory(versions_dir: str, backup_time: str) -> str: - version_dir = os.path.join(versions_dir, backup_time) + version_dir = str(pathlib.Path(versions_dir) / backup_time) try: pathlib.Path(version_dir).mkdir(parents=True) except FileExistsError: - raise BackupException( + raise BackupError( f"generation {backup_time} already exists at {version_dir}; " "another run claimed this second - refusing to write into it, " "since rsync --delete would overwrite that generation" @@ -35,6 +37,25 @@ def create_version_directory(versions_dir: str, backup_time: str) -> str: def create_volume_directory(version_dir: str, volume_name: str) -> str: - path = os.path.join(version_dir, volume_name) - pathlib.Path(path).mkdir(parents=True, exist_ok=True) - return path + path = pathlib.Path(version_dir) / volume_name + path.mkdir(parents=True, exist_ok=True) + return str(path) + + +def write_manifest(version_dir: str, volumes: dict[str, dict[str, bool]]) -> str: + """Record the generation's layout and per-volume outcome. + + Written before the directory is stamped, so the stamp covers it. + + Args: + version_dir: the generation directory. + volumes: per volume name, ``database`` and ``dumped``. + + Returns: + The path written. + """ + path = pathlib.Path(version_dir) / MANIFEST_FILE + with path.open("w", encoding="utf-8") as handle: + json.dump(manifest_document(volumes), handle, indent=2, sort_keys=True) + handle.write("\n") + return str(path) diff --git a/src/baudolo/backup/shell.py b/src/baudolo/backup/shell.py index 018eb6f..400a436 100644 --- a/src/baudolo/backup/shell.py +++ b/src/baudolo/backup/shell.py @@ -9,10 +9,14 @@ from __future__ import annotations import os import subprocess -from collections.abc import Mapping, Sequence +from pathlib import Path +from typing import TYPE_CHECKING + +if TYPE_CHECKING: + from collections.abc import Mapping, Sequence -class BackupException(Exception): +class BackupError(Exception): """Generic exception for backup errors.""" @@ -21,7 +25,7 @@ def _child_env(env: Mapping[str, str] | None) -> dict[str, str] | None: def _fail(command: Sequence[str], returncode: int, out: bytes, err: bytes) -> None: - raise BackupException( + raise BackupError( f"Error in command: {' '.join(command)}\n" f"Output: {out}\nError: {err}\n" f"Exit code: {returncode}" @@ -59,13 +63,13 @@ def execute_to_file( """ command = list(command) print(" ".join(command), flush=True) - tmp = f"{out_file}.tmp" - with open(tmp, "wb") as handle: + tmp = Path(f"{out_file}.tmp") + with tmp.open("wb") as handle: process = subprocess.Popen( command, stdout=handle, stderr=subprocess.PIPE, env=_child_env(env) ) _, err = process.communicate() if process.returncode != 0: - os.unlink(tmp) + tmp.unlink() _fail(command, process.returncode, b"", err) - os.replace(tmp, out_file) + tmp.replace(out_file) diff --git a/src/baudolo/backup/snapshot.py b/src/baudolo/backup/snapshot.py index 5a6c104..401e224 100644 --- a/src/baudolo/backup/snapshot.py +++ b/src/baudolo/backup/snapshot.py @@ -21,11 +21,16 @@ keeps its snapshot. from __future__ import annotations import os -from collections.abc import Callable, Iterator from contextlib import contextmanager +from pathlib import Path +from typing import TYPE_CHECKING -from .shell import BackupException, execute_shell_command -from .volume import Backing +from .shell import BackupError, execute_shell_command + +if TYPE_CHECKING: + from collections.abc import Callable, Iterator + + from .volume import Backing KINDS = ("btrfs", "zfs") @@ -36,10 +41,15 @@ class SnapshotError(RuntimeError): def _resolver(subject: str, root: str) -> Callable[[str], str]: def resolve(path: str) -> str: - relative = os.path.relpath(os.path.abspath(path), os.path.abspath(subject)) + # Exception: abspath, not Path.resolve() - resolve() follows symlinks, + # which would let a symlinked volume test as inside the subject. + relative = os.path.relpath( + os.path.abspath(path), # noqa: PTH100 + os.path.abspath(subject), # noqa: PTH100 + ) if relative.startswith(".."): raise SnapshotError(f"{path} lies outside the snapshot subject {subject}") - resolved = root if relative == "." else os.path.join(root, relative) + resolved = root if relative == "." else str(Path(root) / relative) # abspath drops a trailing separator, and rsync reads "dir/" as its # contents where "dir" means the directory itself. @@ -54,7 +64,7 @@ def _btrfs( # The snapshot goes inside the subject, never beside it: the kernel rejects # a snapshot whose destination is on another filesystem, which is exactly # what the parent directory is when the subject is a mountpoint of its own. - target = os.path.join(os.path.abspath(subject), f".{name}") + target = str(Path(os.path.abspath(subject)) / f".{name}") # noqa: PTH100 - see _resolver run(["btrfs", "subvolume", "snapshot", "-r", subject, target]) return target, ["btrfs", "subvolume", "delete", target] @@ -67,7 +77,7 @@ def _zfs( if not dataset: raise SnapshotError(f"no zfs dataset is mounted at {subject}") run(["zfs", "snapshot", f"{dataset}@{name}"]) - root = os.path.join(subject, ".zfs", "snapshot", name) + root = str(Path(subject) / ".zfs" / "snapshot" / name) return root, ["zfs", "destroy", f"{dataset}@{name}"] @@ -99,7 +109,9 @@ def unsnapshotted(backing: Backing, subject: str) -> str | None: if os.path.ismount(real): return f"its mountpoint {backing.mountpoint} sits on its own mount" try: - crosses = os.stat(real).st_dev != os.stat(os.path.realpath(subject)).st_dev + crosses = ( + Path(real).stat().st_dev != Path(os.path.realpath(subject)).stat().st_dev + ) except OSError as error: return f"its mountpoint {backing.mountpoint} could not be read: {error}" if crosses: @@ -123,7 +135,7 @@ def snapshot_source( source = resolve(backing.source) except SnapshotError as error: return None, str(error) - if not os.path.isdir(source): + if not Path(source).is_dir(): return None, "it was created after the snapshot was taken" return source, "" @@ -159,6 +171,6 @@ def volume_snapshot( finally: try: run(remove) - except BackupException as error: + except BackupError as error: # Raising here would also mask whatever the body raised. print(f"WARNING: {root} could not be removed: {error}", flush=True) diff --git a/src/baudolo/backup/volume.py b/src/baudolo/backup/volume.py index dc72cac..5500e94 100644 --- a/src/baudolo/backup/volume.py +++ b/src/baudolo/backup/volume.py @@ -5,7 +5,9 @@ import os import pathlib from dataclasses import dataclass, field -from .shell import BackupException, execute_shell_command +from baudolo.generation import FILES_DIR + +from .shell import BackupError, execute_shell_command @dataclass(frozen=True) @@ -45,8 +47,8 @@ def get_last_backup_dir( ) -> str | None: versions = sorted(os.listdir(versions_dir), reverse=True) for version in versions: - candidate = os.path.join(versions_dir, version, volume_name, "files", "") - if candidate != current_backup_dir and os.path.isdir(candidate): + candidate = f"{pathlib.Path(versions_dir) / version / volume_name / FILES_DIR}/" + if candidate != current_backup_dir and pathlib.Path(candidate).is_dir(): return candidate return None @@ -69,7 +71,7 @@ def backup_volume( source: directory to read from - the volume's mountpoint, or its path inside a snapshot. """ - dest = os.path.join(volume_dir, "files") + "/" + dest = f"{pathlib.Path(volume_dir) / FILES_DIR}/" pathlib.Path(dest).mkdir(parents=True, exist_ok=True) last = get_last_backup_dir(versions_dir, volume_name, dest) @@ -82,7 +84,7 @@ def backup_volume( try: execute_shell_command(cmd) - except BackupException as e: + except BackupError as e: if "file has vanished" in str(e): print( "Warning: Some files vanished before transfer. Continuing.", flush=True diff --git a/src/baudolo/generation.py b/src/baudolo/generation.py new file mode 100644 index 0000000..65af72f --- /dev/null +++ b/src/baudolo/generation.py @@ -0,0 +1,54 @@ +"""The on-disk shape of a generation, and the manifest that states it. + +Every name a reader needs to find payload in a generation is declared here +once, and written into each generation's own manifest. A consumer therefore +never has to hardcode the layout or match this package's version: it reads +what the run that produced the tree recorded. + +The manifest also carries what only the run itself can know: per volume, +``database`` (it held one), ``dumped`` (a dump was produced for it) and +``engine`` (which one was detected). Both flags true is a replayable dump; +``database`` without ``dumped`` is a raw copy of live engine files. + +Kept import-free: consumers read the manifest with nothing but ``json``, on +hosts that do not have this package installed. +""" + +from __future__ import annotations + +FILES_DIR = "files" +SQL_DIR = "sql" +DUMP_SUFFIX = ".backup.sql" +CLUSTER_SUFFIX = ".cluster.backup.sql" + +MANIFEST_FILE = "manifest.json" +MANIFEST_SCHEMA = 1 + + +def manifest_document(volumes: dict[str, object]) -> dict[str, object]: + """The manifest a finished run writes. + + Args: + volumes: per volume name, an object carrying ``database``, ``dumped`` + and ``engine`` -- a ``baudolo.backup.dumps.VolumeOutcome``. + + Returns: + The document, ready for ``json.dump``. + """ + return { + "schema": MANIFEST_SCHEMA, + "layout": { + "files_dir": FILES_DIR, + "sql_dir": SQL_DIR, + "dump_suffix": DUMP_SUFFIX, + "cluster_suffix": CLUSTER_SUFFIX, + }, + "volumes": { + name: { + "database": bool(outcome.database), + "dumped": bool(outcome.dumped), + "engine": outcome.engine, + } + for name, outcome in sorted(volumes.items()) + }, + } diff --git a/src/baudolo/restore/paths.py b/src/baudolo/restore/paths.py index 536b11e..5c6f343 100644 --- a/src/baudolo/restore/paths.py +++ b/src/baudolo/restore/paths.py @@ -1,7 +1,9 @@ from __future__ import annotations -import os from dataclasses import dataclass +from pathlib import Path + +from baudolo.generation import CLUSTER_SUFFIX, DUMP_SUFFIX, FILES_DIR, SQL_DIR @dataclass(frozen=True) @@ -14,20 +16,20 @@ class BackupPaths: def root(self) -> str: # Always build an absolute path under backups_dir - return os.path.join( - self.backups_dir, - self.backup_hash, - self.repo_name, - self.version, - self.volume_name, + return str( + Path(self.backups_dir) + / self.backup_hash + / self.repo_name + / self.version + / self.volume_name ) def files_dir(self) -> str: - return os.path.join(self.root(), "files") + return str(Path(self.root()) / FILES_DIR) def sql_file(self, db_name: str) -> str: - return os.path.join(self.root(), "sql", f"{db_name}.backup.sql") + return str(Path(self.root()) / SQL_DIR / f"{db_name}{DUMP_SUFFIX}") def cluster_file(self, instance: str) -> str: """The pg_dumpall stream a `database = '*'` row produces.""" - return os.path.join(self.root(), "sql", f"{instance}.cluster.backup.sql") + return str(Path(self.root()) / SQL_DIR / f"{instance}{CLUSTER_SUFFIX}") diff --git a/tests/e2e/test_e2e_only_sql_fallback_to_files.py b/tests/e2e/test_e2e_only_sql_fallback_to_files.py index 8c6cb63..37ecd4b 100644 --- a/tests/e2e/test_e2e_only_sql_fallback_to_files.py +++ b/tests/e2e/test_e2e_only_sql_fallback_to_files.py @@ -1,5 +1,8 @@ +import json import unittest +from baudolo.generation import FILES_DIR, MANIFEST_FILE, MANIFEST_SCHEMA, SQL_DIR + from .helpers import ( POSTGRES_DATA_DIR, POSTGRES_IMAGE, @@ -159,6 +162,31 @@ class TestE2EOnlySqlFallbackToFiles(unittest.TestCase): f"Did not expect SQL dump files, found: {dumps}", ) + def manifest(self) -> dict: + generation = backup_path( + self.backups_dir, self.repo_name, self.version, self.pg_volume + ).parent + return json.loads((generation / MANIFEST_FILE).read_text(encoding="utf-8")) + + def test_the_manifest_records_the_volume_as_a_database_left_undumped(self) -> None: + """The fallback is invisible in the tree: files/ looks like any copy.""" + self.assertEqual( + self.manifest()["volumes"][self.pg_volume], + {"database": True, "dumped": False, "engine": "postgres"}, + ) + + def test_the_manifest_layout_names_where_the_payload_really_landed(self) -> None: + layout = self.manifest()["layout"] + volume_dir = backup_path( + self.backups_dir, self.repo_name, self.version, self.pg_volume + ) + self.assertTrue((volume_dir / layout["files_dir"]).is_dir()) + self.assertEqual(layout["files_dir"], FILES_DIR) + self.assertEqual(layout["sql_dir"], SQL_DIR) + + def test_the_manifest_states_a_schema_a_reader_can_check(self) -> None: + self.assertEqual(self.manifest()["schema"], MANIFEST_SCHEMA) + def test_restored_files_contain_marker(self) -> None: p = run( [ diff --git a/tests/e2e/test_e2e_only_sql_mixed_run.py b/tests/e2e/test_e2e_only_sql_mixed_run.py index 153f086..fef6d52 100644 --- a/tests/e2e/test_e2e_only_sql_mixed_run.py +++ b/tests/e2e/test_e2e_only_sql_mixed_run.py @@ -1,5 +1,8 @@ +import json import unittest +from baudolo.generation import MANIFEST_FILE + from .helpers import ( POSTGRES_DATA_DIR, POSTGRES_IMAGE, @@ -179,3 +182,21 @@ class TestE2EOnlySqlMixedRun(unittest.TestCase): (base / "files").exists(), f"Expected non-DB volume files backup to exist at: {base / 'files'}", ) + + def manifest(self) -> dict: + generation = backup_path( + self.backups_dir, self.repo_name, self.version, self.db_volume + ).parent + return json.loads((generation / MANIFEST_FILE).read_text(encoding="utf-8")) + + def test_the_manifest_records_the_dumped_volume_as_dumped(self) -> None: + self.assertEqual( + self.manifest()["volumes"][self.db_volume], + {"database": True, "dumped": True, "engine": "postgres"}, + ) + + def test_the_manifest_records_the_plain_volume_as_no_database(self) -> None: + self.assertEqual( + self.manifest()["volumes"][self.files_volume], + {"database": False, "dumped": False, "engine": None}, + ) diff --git a/tests/unit/backup/test_app_manifest.py b/tests/unit/backup/test_app_manifest.py new file mode 100644 index 0000000..ee7ccbb --- /dev/null +++ b/tests/unit/backup/test_app_manifest.py @@ -0,0 +1,84 @@ +"""What main() records in the manifest for each volume it touched.""" + +from __future__ import annotations + +import unittest +from unittest import mock + +from baudolo.backup import app +from baudolo.backup.dumps import VolumeOutcome +from baudolo.backup.volume import Backing + +from . import REQUIRED_PAIRS + +ARGV = ["baudolo", *[arg for pair in REQUIRED_PAIRS for arg in pair]] + + +def drive(argv: list[str], dump_result: VolumeOutcome) -> dict: + """Run main() over one volume and return the manifest's volume section. + + Args: + argv: the command line under test. + dump_result: what backup_dumps_for_volume reports. + """ + with ( + mock.patch("sys.argv", argv), + mock.patch.object(app, "get_machine_id", return_value="machine"), + mock.patch.object(app, "create_version_directory", return_value="/gen"), + mock.patch.object(app, "create_volume_directory", return_value="/gen/vol"), + mock.patch.object(app, "load_databases_df", return_value=None), + mock.patch.object(app, "docker_volume_names", return_value=["pgdata"]), + mock.patch.object(app, "containers_using_volume", return_value=["db"]), + mock.patch.object(app, "volume_is_fully_ignored", return_value=False), + mock.patch.object(app, "backup_dumps_for_volume", return_value=dump_result), + mock.patch.object(app, "inspect_backing", return_value=Backing("/data")), + mock.patch.object(app, "write_manifest") as manifest, + mock.patch.object(app, "stamp_directory"), + mock.patch.object(app, "handle_docker_compose_services"), + mock.patch("os.path.isdir", return_value=True), + mock.patch.object(app, "backup_volume"), + mock.patch.object(app, "filter_stoppable", return_value=[]), + mock.patch.object(app, "requires_stop", return_value=False), + mock.patch.object(app, "change_containers_status"), + ): + app.main() + return manifest.call_args.args[1] + + +class TestManifest(unittest.TestCase): + def test_a_database_volume_without_a_dump_is_recorded_as_undumped(self) -> None: + volumes = drive( + [*ARGV, "--only-sql"], + VolumeOutcome(database=True, dumped=False, engine="postgres"), + ) + self.assertEqual( + volumes["pgdata"], + VolumeOutcome(database=True, dumped=False, engine="postgres"), + ) + + def test_a_dumped_database_volume_is_recorded_as_dumped(self) -> None: + volumes = drive( + [*ARGV, "--only-sql"], + VolumeOutcome(database=True, dumped=True, engine="mariadb"), + ) + self.assertEqual(volumes["pgdata"].dumped, True) + self.assertEqual(volumes["pgdata"].engine, "mariadb") + + def test_a_plain_volume_is_recorded_as_no_database(self) -> None: + volumes = drive(ARGV, VolumeOutcome(database=False, dumped=False)) + self.assertEqual(volumes["pgdata"].database, False) + self.assertIsNone(volumes["pgdata"].engine) + + def test_the_dumped_volume_is_recorded_even_though_the_copy_is_skipped( + self, + ) -> None: + """--only-sql returns to the loop head on success, before the copy.""" + volumes = drive( + [*ARGV, "--only-sql"], + VolumeOutcome(database=True, dumped=True, engine="postgres"), + ) + self.assertIn("pgdata", volumes) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/unit/backup/test_app_only_files.py b/tests/unit/backup/test_app_only_files.py index 4da2ff5..409b597 100644 --- a/tests/unit/backup/test_app_only_files.py +++ b/tests/unit/backup/test_app_only_files.py @@ -34,9 +34,10 @@ def drive(argv: list[str]) -> tuple[list[str], list, list]: mock.patch.object(app, "volume_is_fully_ignored", return_value=False), mock.patch.object(app, "backup_dumps_for_volume") as dumps, mock.patch.object(app, "inspect_backing", return_value=Backing("/data")), + mock.patch.object(app, "write_manifest"), mock.patch.object(app, "stamp_directory"), mock.patch.object(app, "handle_docker_compose_services"), - mock.patch.object(app.os.path, "isdir", return_value=True), + mock.patch("os.path.isdir", return_value=True), mock.patch.object(app, "backup_volume", side_effect=record_backup), mock.patch.object(app, "filter_stoppable", return_value=[]), mock.patch.object(app, "requires_stop", return_value=False), diff --git a/tests/unit/backup/test_app_snapshot.py b/tests/unit/backup/test_app_snapshot.py index dda8749..f38e992 100644 --- a/tests/unit/backup/test_app_snapshot.py +++ b/tests/unit/backup/test_app_snapshot.py @@ -50,9 +50,10 @@ def drive(*, present: bool = True, reason: str | None = None) -> list[dict]: return_value=Backing("/var/lib/docker/volumes/vol/_data"), ), mock.patch.object(snapshot_mod, "unsnapshotted", return_value=reason), + mock.patch.object(app, "write_manifest"), mock.patch.object(app, "stamp_directory"), mock.patch.object(app, "handle_docker_compose_services"), - mock.patch.object(app.os.path, "isdir", return_value=present), + mock.patch("os.path.isdir", return_value=present), mock.patch.object(app, "backup_volume", side_effect=record), mock.patch.object(app, "volume_snapshot", stubbed_snapshot), ): diff --git a/tests/unit/backup/test_app_volumes_no_backup_required.py b/tests/unit/backup/test_app_volumes_no_backup_required.py index cf64570..59199ac 100644 --- a/tests/unit/backup/test_app_volumes_no_backup_required.py +++ b/tests/unit/backup/test_app_volumes_no_backup_required.py @@ -47,9 +47,10 @@ def drive() -> tuple[list[str], list[str], list[str]]: mock.patch.object(app, "volume_is_fully_ignored", return_value=False), mock.patch.object(app, "backup_dumps_for_volume", return_value=(False, False)), mock.patch.object(app, "inspect_backing", return_value=Backing("/data")), + mock.patch.object(app, "write_manifest"), mock.patch.object(app, "stamp_directory"), mock.patch.object(app, "handle_docker_compose_services"), - mock.patch.object(app.os.path, "isdir", return_value=True), + mock.patch("os.path.isdir", return_value=True), mock.patch.object(app, "backup_volume", side_effect=record_backup), mock.patch.object(app, "filter_stoppable", return_value=[]), mock.patch.object(app, "requires_stop", return_value=False), diff --git a/tests/unit/backup/test_docker_swarm.py b/tests/unit/backup/test_docker_swarm.py index f694274..29dfb94 100644 --- a/tests/unit/backup/test_docker_swarm.py +++ b/tests/unit/backup/test_docker_swarm.py @@ -2,7 +2,7 @@ import unittest from unittest.mock import patch from baudolo.backup import docker as docker_mod -from baudolo.backup.shell import BackupException +from baudolo.backup.shell import BackupError class TestIsSwarmTask(unittest.TestCase): @@ -21,7 +21,7 @@ class TestIsSwarmTask(unittest.TestCase): @patch.object( docker_mod, "execute_shell_command", - side_effect=[BackupException("gone"), []], + side_effect=[BackupError("gone"), []], ) def test_vanished_container_counts_as_not_stoppable(self, _mock) -> None: # A container removed between listing and inspect must not abort the @@ -32,13 +32,13 @@ class TestIsSwarmTask(unittest.TestCase): @patch.object( docker_mod, "execute_shell_command", - side_effect=[BackupException("daemon hiccup"), ["still-here"]], + side_effect=[BackupError("daemon hiccup"), ["still-here"]], ) def test_inspect_failure_on_existing_container_still_fails(self, _mock) -> None: # If the container still exists, an inspect failure must keep failing # the run: silently skipping the stop would back up a hot volume and # report green without the stop guarantee. - with self.assertRaises(BackupException): + with self.assertRaises(BackupError): docker_mod.is_swarm_task("still-here") diff --git a/tests/unit/backup/test_docker_tool_probe.py b/tests/unit/backup/test_docker_tool_probe.py index 53021a0..56b7ee9 100644 --- a/tests/unit/backup/test_docker_tool_probe.py +++ b/tests/unit/backup/test_docker_tool_probe.py @@ -2,7 +2,7 @@ import unittest from unittest.mock import patch from baudolo.backup import docker as docker_mod -from baudolo.backup.shell import BackupException +from baudolo.backup.shell import BackupError class TestImageId(unittest.TestCase): @@ -20,7 +20,7 @@ class TestHasTool(unittest.TestCase): def test_a_tool_that_exits_non_zero_is_absent(self) -> None: with patch.object( - docker_mod, "execute_shell_command", side_effect=BackupException("127") + docker_mod, "execute_shell_command", side_effect=BackupError("127") ): self.assertFalse(docker_mod.has_tool("c1", "mariadb-dump")) diff --git a/tests/unit/backup/test_dumps_engine_detection.py b/tests/unit/backup/test_dumps_engine_detection.py index 1e479ff..2feb66a 100644 --- a/tests/unit/backup/test_dumps_engine_detection.py +++ b/tests/unit/backup/test_dumps_engine_detection.py @@ -1,15 +1,13 @@ import unittest from unittest.mock import patch -import pandas +import pandas as pd from baudolo.backup import dumps as dumps_mod def _df(rows): - return pandas.DataFrame( - rows, columns=["instance", "database", "username", "password"] - ) + return pd.DataFrame(rows, columns=["instance", "database", "username", "password"]) class _Probe: @@ -104,15 +102,16 @@ class TestBackupDispatch(unittest.TestCase): patch.object(dumps_mod, "image_id", probe.image_id), patch.object(dumps_mod, "backup_database", _fake_backup_database), ): - is_db, dumped = dumps_mod.backup_mariadb_or_postgres( + outcome = dumps_mod.backup_mariadb_or_postgres( container="c1", volume_dir="/tmp", databases_df=_df([("c1", "appdb", "u", "p")]), database_containers=["c1"], ) - self.assertTrue(is_db) - self.assertTrue(dumped) + self.assertTrue(outcome.database) + self.assertTrue(outcome.dumped) + self.assertEqual(outcome.engine, "mariadb") self.assertEqual(seen["db_type"], "mariadb") self.assertEqual(seen["dump_tool"], "mysqldump") @@ -130,7 +129,7 @@ class TestBackupDispatch(unittest.TestCase): databases_df=_df([]), database_containers=[], ), - (False, False), + dumps_mod.VolumeOutcome(database=False, dumped=False, engine=None), ) diff --git a/tests/unit/backup/test_layout.py b/tests/unit/backup/test_layout.py index ce1e892..b2e891c 100644 --- a/tests/unit/backup/test_layout.py +++ b/tests/unit/backup/test_layout.py @@ -8,7 +8,7 @@ from pathlib import Path from unittest import mock from baudolo.backup import layout as mod -from baudolo.backup.shell import BackupException +from baudolo.backup.shell import BackupError class TestVersionDirectory(unittest.TestCase): @@ -21,7 +21,7 @@ class TestVersionDirectory(unittest.TestCase): def test_it_refuses_a_generation_another_run_already_claimed(self) -> None: with tempfile.TemporaryDirectory() as tmp: mod.create_version_directory(tmp, "20260731") - with self.assertRaises(BackupException) as caught: + with self.assertRaises(BackupError) as caught: mod.create_version_directory(tmp, "20260731") self.assertIn("20260731", str(caught.exception)) diff --git a/tests/unit/backup/test_snapshot.py b/tests/unit/backup/test_snapshot.py index 8a83513..ffd6797 100644 --- a/tests/unit/backup/test_snapshot.py +++ b/tests/unit/backup/test_snapshot.py @@ -4,7 +4,7 @@ from __future__ import annotations import unittest -from baudolo.backup.shell import BackupException +from baudolo.backup.shell import BackupError from baudolo.backup.snapshot import SnapshotError, volume_snapshot @@ -143,7 +143,7 @@ class TestRejections(unittest.TestCase): class Busy(Runner): def __call__(self, command: list[str]) -> list[str]: if command[:3] == ["btrfs", "subvolume", "delete"]: - raise BackupException("target is busy") + raise BackupError("target is busy") return super().__call__(command) diff --git a/tests/unit/test_generation.py b/tests/unit/test_generation.py new file mode 100644 index 0000000..96366d1 --- /dev/null +++ b/tests/unit/test_generation.py @@ -0,0 +1,65 @@ +"""Contract of the generation manifest document and the file it lands in.""" + +from __future__ import annotations + +import json +import tempfile +import unittest +from pathlib import Path + +from baudolo.backup.dumps import VolumeOutcome +from baudolo.backup.layout import write_manifest +from baudolo.generation import ( + CLUSTER_SUFFIX, + DUMP_SUFFIX, + FILES_DIR, + MANIFEST_FILE, + MANIFEST_SCHEMA, + SQL_DIR, + manifest_document, +) + + +class TestManifestDocument(unittest.TestCase): + def test_it_states_the_layout_a_reader_needs(self) -> None: + document = manifest_document({}) + self.assertEqual( + document["layout"], + { + "files_dir": FILES_DIR, + "sql_dir": SQL_DIR, + "dump_suffix": DUMP_SUFFIX, + "cluster_suffix": CLUSTER_SUFFIX, + }, + ) + + def test_it_carries_a_schema_so_a_reader_can_refuse_a_newer_one(self) -> None: + self.assertEqual(manifest_document({})["schema"], MANIFEST_SCHEMA) + + def test_it_sorts_volumes_so_two_runs_produce_the_same_bytes(self) -> None: + state = VolumeOutcome(database=False, dumped=False) + document = manifest_document({"b": state, "a": state}) + self.assertEqual(list(document["volumes"]), ["a", "b"]) + + +class TestWriteManifest(unittest.TestCase): + def test_it_writes_readable_json_next_to_the_volumes(self) -> None: + with tempfile.TemporaryDirectory() as version_dir: + path = write_manifest( + version_dir, + { + "pgdata": VolumeOutcome( + database=True, dumped=False, engine="postgres" + ) + }, + ) + self.assertEqual(Path(path).name, MANIFEST_FILE) + document = json.loads(Path(path).read_text(encoding="utf-8")) + self.assertEqual( + document["volumes"]["pgdata"], + {"database": True, "dumped": False, "engine": "postgres"}, + ) + + +if __name__ == "__main__": + unittest.main()