From 95c34d4db0360a1058e26de5bc1f51167a9b534b Mon Sep 17 00:00:00 2001 From: Kevin Veen-Birkenbach Date: Sun, 2 Aug 2026 09:10:20 +0200 Subject: [PATCH] feat(backup): exclude a volume by name, not only by image volume_is_fully_ignored can only skip a volume when every container using it is ignored, so a container holding a derived tree next to state that must be kept cannot express the exclusion at all. The matrix docker-in-docker runner is exactly that: matrix_mdad_docker, matrix_mdad_matrix and matrix_mdad_state all hang off one container, and the derived one is an inner overlay2 store that no rsync in the chain can restore faithfully (none carries -X, so trusted.overlay.* is stripped in both directions). --volumes-no-backup-required names volumes directly. The check runs before containers_using_volume, so an excluded volume costs no docker call and the decision no longer depends on which containers happen to exist at backup time. Co-Authored-By: Claude Opus 5 (1M context) --- src/baudolo/backup/app.py | 8 ++ src/baudolo/backup/cli.py | 7 + ...e_volumes_no_backup_required_early_skip.py | 125 ++++++++++++++++++ .../test_app_volumes_no_backup_required.py | 82 ++++++++++++ 4 files changed, 222 insertions(+) create mode 100644 tests/e2e/test_e2e_volumes_no_backup_required_early_skip.py create mode 100644 tests/unit/backup/test_app_volumes_no_backup_required.py diff --git a/src/baudolo/backup/app.py b/src/baudolo/backup/app.py index 46c53a5..0da3027 100644 --- a/src/baudolo/backup/app.py +++ b/src/baudolo/backup/app.py @@ -54,6 +54,14 @@ def main() -> int: for volume_name in docker_volume_names(): print(f"Start backup routine for volume: {volume_name}", flush=True) + + if volume_name in args.volumes_no_backup_required: + print( + f"Skipping volume '{volume_name}' entirely (declared no-backup).", + flush=True, + ) + continue + containers = containers_using_volume(volume_name) if volume_is_fully_ignored(containers, args.images_no_backup_required): diff --git a/src/baudolo/backup/cli.py b/src/baudolo/backup/cli.py index c72acb8..992ad65 100644 --- a/src/baudolo/backup/cli.py +++ b/src/baudolo/backup/cli.py @@ -68,6 +68,13 @@ def parse_args() -> argparse.Namespace: help="Exact image references (repo:tag, incl. any registry prefix) for which no backup should be performed", ) + p.add_argument( + "--volumes-no-backup-required", + nargs="+", + default=[], + help="Exact volume names that are never backed up, whatever containers use them. For derived trees a restore cannot reproduce, above all a nested docker data root", + ) + p.add_argument( "--everything", action="store_true", diff --git a/tests/e2e/test_e2e_volumes_no_backup_required_early_skip.py b/tests/e2e/test_e2e_volumes_no_backup_required_early_skip.py new file mode 100644 index 0000000..9428f40 --- /dev/null +++ b/tests/e2e/test_e2e_volumes_no_backup_required_early_skip.py @@ -0,0 +1,125 @@ +import unittest + +from .helpers import ( + backup_path, + cleanup_docker, + create_minimal_compose_dir, + ensure_empty_dir, + latest_version_dir, + require_docker, + run, + unique, + write_databases_csv, +) + + +class TestE2EVolumesNoBackupRequiredEarlySkip(unittest.TestCase): + """Both volumes hang off the same container, so an image-level exclusion + could only drop both. Only the named one may disappear.""" + + @classmethod + def setUpClass(cls) -> None: + require_docker() + + cls.prefix = unique("baudolo-e2e-early-skip-no-backup-volume") + cls.backups_dir = f"/tmp/{cls.prefix}/Backups" + ensure_empty_dir(cls.backups_dir) + + cls.compose_dir = create_minimal_compose_dir(f"/tmp/{cls.prefix}") + cls.repo_name = cls.prefix + + cls.container = f"{cls.prefix}-app" + cls.excluded_volume = f"{cls.prefix}-derived-vol" + cls.kept_volume = f"{cls.prefix}-state-vol" + + cls.containers = [cls.container] + cls.volumes = [cls.excluded_volume, cls.kept_volume] + + run(["docker", "volume", "create", cls.excluded_volume]) + run(["docker", "volume", "create", cls.kept_volume]) + + run( + [ + "docker", + "run", + "--rm", + "-v", + f"{cls.excluded_volume}:/derived", + "-v", + f"{cls.kept_volume}:/state", + "alpine:3.20", + "sh", + "-lc", + "echo derived > /derived/derived.txt && echo state > /state/state.txt", + ] + ) + + run( + [ + "docker", + "run", + "-d", + "--name", + cls.container, + "-v", + f"{cls.excluded_volume}:/derived", + "-v", + f"{cls.kept_volume}:/state", + "alpine:3.20", + "sleep", + "600", + ] + ) + + cls.databases_csv = f"/tmp/{cls.prefix}/databases.csv" + write_databases_csv(cls.databases_csv, []) + + cmd = [ + "baudolo", + "--compose-dir", + cls.compose_dir, + "--repo-name", + cls.repo_name, + "--databases-csv", + cls.databases_csv, + "--backups-dir", + cls.backups_dir, + "--images-no-stop-required", + "alpine:3.20", + "--volumes-no-backup-required", + cls.excluded_volume, + ] + cp = run(cmd, capture=True, check=True) + cls.stdout = cp.stdout or "" + cls.stderr = cp.stderr or "" + + cls.hash, cls.version = latest_version_dir(cls.backups_dir, cls.repo_name) + + @classmethod + def tearDownClass(cls) -> None: + cleanup_docker(containers=cls.containers, volumes=cls.volumes) + + def test_excluded_volume_has_no_backup_directory_at_all(self) -> None: + p = backup_path( + self.backups_dir, + self.repo_name, + self.version, + self.excluded_volume, + ) + self.assertFalse( + p.exists(), + f"Expected NO backup directory for the excluded volume, but found: {p}", + ) + + def test_sibling_volume_of_the_same_container_is_still_backed_up(self) -> None: + p = ( + backup_path( + self.backups_dir, + self.repo_name, + self.version, + self.kept_volume, + ) + / "files" + / "state.txt" + ) + self.assertTrue(p.is_file(), f"Expected backed up file at: {p}") diff --git a/tests/unit/backup/test_app_volumes_no_backup_required.py b/tests/unit/backup/test_app_volumes_no_backup_required.py new file mode 100644 index 0000000..aee2908 --- /dev/null +++ b/tests/unit/backup/test_app_volumes_no_backup_required.py @@ -0,0 +1,82 @@ +"""Contract of --volumes-no-backup-required: exclusion is per volume name, +independent of which containers use it.""" + +from __future__ import annotations + +import unittest +from unittest import mock + +from baudolo.backup import app + +ARGV = [ + "baudolo", + "--compose-dir", + "/compose", + "--backups-dir", + "/backups", + "--volumes-no-backup-required", + "derived", +] + + +def drive() -> tuple[list[str], list[str], list[str]]: + backed_up: list[str] = [] + created: list[str] = [] + inspected: list[str] = [] + + def record_backup(versions_dir, volume_name, volume_dir, *, authoritative, source): + backed_up.append(volume_name) + + 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", + side_effect=lambda _version_dir, name: created.append(name) or "/gen/vol", + ), + mock.patch.object(app, "load_databases_df", return_value=None), + mock.patch.object( + app, "docker_volume_names", return_value=["derived", "state"] + ), + mock.patch.object( + app, + "containers_using_volume", + side_effect=lambda name: inspected.append(name) or ["app"], + ), + 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, "get_storage_path", return_value="/data/"), + 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.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), + mock.patch.object(app, "change_containers_status"), + ): + app.main() + return backed_up, created, inspected + + +class TestVolumesNoBackupRequired(unittest.TestCase): + def test_the_named_volume_is_never_backed_up(self) -> None: + backed_up, _created, _inspected = drive() + self.assertNotIn("derived", backed_up) + + def test_a_sibling_volume_of_the_same_container_survives(self) -> None: + backed_up, _created, _inspected = drive() + self.assertEqual(backed_up, ["state"]) + + def test_no_generation_directory_is_created_for_it(self) -> None: + _backed_up, created, _inspected = drive() + self.assertEqual(created, ["state"]) + + def test_the_skip_precedes_the_container_inspection(self) -> None: + _backed_up, _created, inspected = drive() + self.assertEqual(inspected, ["state"]) + + +if __name__ == "__main__": + unittest.main()