From 161db213cbf41008b7de09bcf7167095e6e3bbca Mon Sep 17 00:00:00 2001 From: Kevin Veen-Birkenbach Date: Wed, 23 Sep 2026 15:34:19 +0200 Subject: [PATCH] fix(backup): let the stopped copy replace a failed live pre-copy A volume whose containers must stop was copied twice: live first, then with the containers stopped and --checksum. A live writer that truncates a file under rsync (read errors mapping ...: No data available (61), exit 23) aborted the run in the first pass, before the containers were stopped, although the second pass rewrites every differing file anyway. Seen in infinito-nexus CI on kix_kix_shared/objecticons. The live copy now only warns when a stopped copy follows. Without a stop it stays the only copy and still fails the run, and a failing stopped copy still fails it. The e2e test drives a writer that shrinks files mid-read; without the fix it reproduces the CI failure verbatim. Co-Authored-By: Claude Opus 5.5 (1M context) --- src/baudolo/backup/app.py | 25 ++-- tests/e2e/helpers/fixtures.py | 4 +- .../e2e/test_e2e_files_live_writer_precopy.py | 110 ++++++++++++++++++ tests/unit/backup/test_app_live_precopy.py | 70 +++++++++++ 4 files changed, 200 insertions(+), 9 deletions(-) create mode 100644 tests/e2e/test_e2e_files_live_writer_precopy.py create mode 100644 tests/unit/backup/test_app_live_precopy.py diff --git a/src/baudolo/backup/app.py b/src/baudolo/backup/app.py index 5b8a55b..4b34a2c 100644 --- a/src/baudolo/backup/app.py +++ b/src/baudolo/backup/app.py @@ -23,6 +23,7 @@ from .layout import ( write_manifest, ) from .policy import requires_stop, volume_is_fully_ignored +from .shell import BackupError from .snapshot import snapshot_source, volume_snapshot from .volume import backup_volume, inspect_backing @@ -125,13 +126,23 @@ def main() -> int: copy(authoritative=False) continue - copy(authoritative=False) - if requires_stop(containers, args.images_no_stop_required): - stoppable = filter_stoppable(containers) - change_containers_status(stoppable, "stop") - copy(authoritative=True) - if not args.shutdown: - change_containers_status(stoppable, "start") + if not requires_stop(containers, args.images_no_stop_required): + copy(authoritative=False) + continue + + try: + copy(authoritative=False) + except BackupError as error: + print( + f"WARNING: live pre-copy of volume '{volume_name}' failed; " + f"the copy with its containers stopped replaces it.\n{error}", + flush=True, + ) + stoppable = filter_stoppable(containers) + change_containers_status(stoppable, "stop") + copy(authoritative=True) + if not args.shutdown: + change_containers_status(stoppable, "start") write_manifest(version_dir, outcomes) stamp_directory(version_dir) diff --git a/tests/e2e/helpers/fixtures.py b/tests/e2e/helpers/fixtures.py index 4d4182b..e96fec7 100644 --- a/tests/e2e/helpers/fixtures.py +++ b/tests/e2e/helpers/fixtures.py @@ -25,7 +25,7 @@ def backup_run( images_no_stop_required: list[str], images_no_backup_required: list[str] | None = None, only_sql: bool = False, -) -> None: +) -> subprocess.CompletedProcess: cmd = [ "baudolo", "--compose-dir", @@ -49,7 +49,7 @@ def backup_run( cmd += ["--only-sql"] try: - run(cmd, capture=True, check=True) + return run(cmd, capture=True, check=True) except subprocess.CalledProcessError as e: print(">>> baudolo failed (exit code:", e.returncode, ")") if e.stdout: diff --git a/tests/e2e/test_e2e_files_live_writer_precopy.py b/tests/e2e/test_e2e_files_live_writer_precopy.py new file mode 100644 index 0000000..ec1a7cf --- /dev/null +++ b/tests/e2e/test_e2e_files_live_writer_precopy.py @@ -0,0 +1,110 @@ +"""A writer that shrinks files mid-read fails the live pre-copy, not the run.""" + +import unittest + +from .helpers import ( + backup_path, + backup_run, + cleanup_docker, + create_minimal_compose_dir, + ensure_empty_dir, + latest_version_dir, + require_docker, + run, + unique, + wait_for_log, + write_databases_csv, +) + +CHURN_FILES = 8 + +WRITER = f""" +trap 'exit 0' TERM +echo hello > /data/hello.txt +for i in $(seq 1 {CHURN_FILES}); do + (while :; do truncate -s 0 /data/churn$i; truncate -s 16M /data/churn$i; done) & +done +echo ready +wait +""" + + +class TestE2EFilesLiveWriterPreCopy(unittest.TestCase): + @classmethod + def setUpClass(cls) -> None: + require_docker() + cls.prefix = unique("baudolo-e2e-live-writer") + 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.volume = f"{cls.prefix}-vol" + cls.writer = f"{cls.prefix}-writer" + cls.containers = [cls.writer] + cls.volumes = [cls.volume] + + run(["docker", "volume", "create", cls.volume]) + run( + [ + "docker", + "run", + "-d", + "--name", + cls.writer, + "-v", + f"{cls.volume}:/data", + "alpine:3.20", + "sh", + "-c", + WRITER, + ] + ) + wait_for_log(cls.writer, "ready") + + cls.databases_csv = f"/tmp/{cls.prefix}/databases.csv" + write_databases_csv(cls.databases_csv, []) + + cls.result = backup_run( + backups_dir=cls.backups_dir, + repo_name=cls.repo_name, + compose_dir=cls.compose_dir, + databases_csv=cls.databases_csv, + database_containers=["dummy-db"], + images_no_stop_required=["dummy-image"], + ) + + 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_the_live_pre_copy_hit_the_writer(self) -> None: + self.assertIn( + f"WARNING: live pre-copy of volume '{self.volume}' failed", + self.result.stdout, + "the writer never shrank a file under rsync, so this run proves nothing", + ) + + def test_the_stopped_copy_captured_the_volume(self) -> None: + files = ( + backup_path(self.backups_dir, self.repo_name, self.version, self.volume) + / "files" + ) + self.assertEqual((files / "hello.txt").read_text().strip(), "hello") + self.assertEqual( + sorted(p.name for p in files.glob("churn*")), + sorted(f"churn{i}" for i in range(1, CHURN_FILES + 1)), + ) + + def test_the_writer_runs_again_after_the_backup(self) -> None: + state = run( + ["docker", "inspect", "-f", "{{.State.Running}}", self.writer] + ).stdout.strip() + self.assertEqual(state, "true") + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/unit/backup/test_app_live_precopy.py b/tests/unit/backup/test_app_live_precopy.py new file mode 100644 index 0000000..392d84f --- /dev/null +++ b/tests/unit/backup/test_app_live_precopy.py @@ -0,0 +1,70 @@ +"""Contract of app.main's live copy when no snapshot is taken.""" + +from __future__ import annotations + +import unittest +from unittest import mock + +from baudolo.backup import app +from baudolo.backup.shell import BackupError +from baudolo.backup.volume import Backing + +from . import BASE_ARGV + + +def drive(*, stop: bool, fail_live: bool) -> list[str]: + events: list[str] = [] + + def record(versions_dir, volume_name, volume_dir, *, authoritative, source): + events.append("authoritative" if authoritative else "live") + if fail_live and not authoritative: + raise BackupError("rsync exit code 23") + + with ( + mock.patch("sys.argv", BASE_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=["vol"]), + mock.patch.object(app, "containers_using_volume", return_value=["c"]), + 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("/var/lib/docker/volumes/vol/_data"), + ), + mock.patch.object(app, "requires_stop", return_value=stop), + mock.patch.object(app, "filter_stoppable", return_value=["c"]), + mock.patch.object( + app, + "change_containers_status", + side_effect=lambda containers, status: events.append(status), + ), + mock.patch.object(app, "write_manifest"), + mock.patch.object(app, "stamp_directory"), + mock.patch.object(app, "handle_docker_compose_services"), + mock.patch.object(app, "backup_volume", side_effect=record), + ): + app.main() + return events + + +class TestLivePreCopy(unittest.TestCase): + def test_a_failed_pre_copy_is_replaced_by_the_stopped_copy(self) -> None: + self.assertEqual( + drive(stop=True, fail_live=True), + ["live", "stop", "authoritative", "start"], + ) + + def test_a_failed_live_copy_without_a_stop_aborts_the_run(self) -> None: + with self.assertRaises(BackupError): + drive(stop=False, fail_live=True) + + def test_a_volume_without_a_stop_is_copied_live_once(self) -> None: + self.assertEqual(drive(stop=False, fail_live=False), ["live"]) + + +if __name__ == "__main__": + unittest.main()