mirror of
https://github.com/kevinveenbirkenbach/docker-volume-backup.git
synced 2026-09-23 21:23:19 +00:00
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) <noreply@anthropic.com>
This commit is contained in:
@@ -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)
|
||||
|
||||
@@ -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:
|
||||
|
||||
110
tests/e2e/test_e2e_files_live_writer_precopy.py
Normal file
110
tests/e2e/test_e2e_files_live_writer_precopy.py
Normal file
@@ -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()
|
||||
70
tests/unit/backup/test_app_live_precopy.py
Normal file
70
tests/unit/backup/test_app_live_precopy.py
Normal file
@@ -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()
|
||||
Reference in New Issue
Block a user