diff --git a/src/baudolo/backup/app.py b/src/baudolo/backup/app.py index ef9e137..5a699e0 100644 --- a/src/baudolo/backup/app.py +++ b/src/baudolo/backup/app.py @@ -22,8 +22,8 @@ from .layout import ( stamp_directory, ) from .policy import requires_stop, volume_is_fully_ignored -from .snapshot import volume_snapshot -from .volume import backup_volume, get_storage_path +from .snapshot import snapshot_source, volume_snapshot +from .volume import backup_volume, inspect_backing def main() -> int: @@ -86,7 +86,8 @@ def main() -> int: else: continue - live_source = get_storage_path(volume_name) + backing = inspect_backing(volume_name) + live_source = backing.source def copy( *, @@ -104,13 +105,15 @@ def main() -> int: ) if resolve_source is not None: - snapshot_source = resolve_source(live_source) - if os.path.isdir(snapshot_source): - copy(authoritative=True, source=snapshot_source) + source, reason = snapshot_source( + resolve_source, backing, args.snapshot_subject + ) + if source is not None: + copy(authoritative=True, source=source) else: print( f"WARNING: volume '{volume_name}' is not in the snapshot " - "(created after it was taken); copying it live instead.", + f"({reason}); copying it live instead.", flush=True, ) copy(authoritative=False) diff --git a/src/baudolo/backup/snapshot.py b/src/baudolo/backup/snapshot.py index b3bfec4..8412d66 100644 --- a/src/baudolo/backup/snapshot.py +++ b/src/baudolo/backup/snapshot.py @@ -9,6 +9,13 @@ built for. That also removes the reason to stop containers at all. The snapshot kind is stated by the caller rather than probed, because falling back to a live copy when a probe is inconclusive would hand out backups that look consistent and are not. + +Which volumes a snapshot of the subject contains is a different question, and +it is decided per volume: a volume with a backing store of its own appears +inside the snapshot as an existing empty directory, so copying from there +succeeds and stores nothing. Such a volume is copied live instead - correct +data without the point in time - while every other volume of the same run +keeps its snapshot. """ from __future__ import annotations @@ -18,6 +25,7 @@ from collections.abc import Callable, Iterator from contextlib import contextmanager from .shell import BackupException, execute_shell_command +from .volume import Backing KINDS = ("btrfs", "zfs") @@ -62,6 +70,60 @@ def _zfs(subject: str, name: str, run: Callable[[str], list[str]]) -> tuple[str, _CREATE = {"btrfs": _btrfs, "zfs": _zfs} +def unsnapshotted(backing: Backing, subject: str) -> str | None: + """Return why a snapshot of ``subject`` does not hold this volume's data. + + Docker mounts a volume's own backing store lazily and unmounts it when the + last consumer stops, so the declaration is what gets checked: it is true at + every moment, where the mount table is only true while a container happens + to hold the volume. + + Args: + backing: the volume as the daemon describes it. + subject: the snapshot subject, e.g. ``/var/lib/docker``. + + Returns: + The reason, or None when the snapshot holds the volume. + """ + if backing.driver != "local": + return f"it uses the {backing.driver} driver" + if backing.options: + return f"it declares its own backing store {backing.options}" + if not backing.mountpoint: + return "it reports no mountpoint" + real = os.path.realpath(backing.mountpoint) + 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 + except OSError as error: + return f"its mountpoint {backing.mountpoint} could not be read: {error}" + if crosses: + return f"its mountpoint {backing.mountpoint} crosses a filesystem boundary" + return None + + +def snapshot_source( + resolve: Callable[[str], str], backing: Backing, subject: str +) -> tuple[str | None, str]: + """Resolve where to read a volume from, and why if not from the snapshot. + + Returns: + ``(path, "")`` to copy from the snapshot, or ``(None, reason)`` to copy + it live. + """ + reason = unsnapshotted(backing, subject) + if reason: + return None, reason + try: + source = resolve(backing.source) + except SnapshotError as error: + return None, str(error) + if not os.path.isdir(source): + return None, "it was created after the snapshot was taken" + return source, "" + + @contextmanager def volume_snapshot( kind: str, diff --git a/src/baudolo/backup/volume.py b/src/baudolo/backup/volume.py index e9d4285..fb7b5a2 100644 --- a/src/baudolo/backup/volume.py +++ b/src/baudolo/backup/volume.py @@ -1,16 +1,43 @@ from __future__ import annotations +import json import os import pathlib +from dataclasses import dataclass, field from .shell import BackupException, execute_shell_command -def get_storage_path(volume_name: str) -> str: - path = execute_shell_command( - f"docker volume inspect --format '{{{{ .Mountpoint }}}}' {volume_name}" +@dataclass(frozen=True) +class Backing: + """Where a docker volume actually keeps its data. + + Args: + mountpoint: the path the daemon reports. + driver: the volume driver, ``local`` for the built-in one. + options: the driver options; a non-empty map means the mountpoint is a + mount target rather than the storage itself. + """ + + mountpoint: str + driver: str = "local" + options: dict = field(default_factory=dict) + + @property + def source(self) -> str: + return f"{self.mountpoint}/" + + +def inspect_backing(volume_name: str) -> Backing: + reported = execute_shell_command( + f"docker volume inspect --format '{{{{json .}}}}' {volume_name}" )[0] - return f"{path}/" + data = json.loads(reported) + return Backing( + data.get("Mountpoint") or "", + data.get("Driver") or "", + data.get("Options") or {}, + ) def get_last_backup_dir( diff --git a/tests/e2e/faithful_driver.py b/tests/e2e/faithful_driver.py new file mode 100644 index 0000000..55b3614 --- /dev/null +++ b/tests/e2e/faithful_driver.py @@ -0,0 +1,92 @@ +"""Show what a real snapshot holds for a volume that has its own storage. + +Runs inside the privileged container that built the btrfs subject. Prints one +PASS/FAIL line per assertion and exits non-zero on the first failure. +""" + +from __future__ import annotations + +import subprocess +import sys +from pathlib import Path + +sys.path.insert(0, "/src") + +from baudolo.backup.snapshot import ( + SnapshotError, + snapshot_source, + unsnapshotted, + volume_snapshot, +) +from baudolo.backup.volume import Backing + +SUBJECT = sys.argv[1] + + +def shell(command: str) -> list[str]: + proc = subprocess.run( + command, shell=True, capture_output=True, text=True, check=False + ) + if proc.returncode != 0: + raise SnapshotError( + f"{command} exited {proc.returncode}: {proc.stderr.strip()}" + ) + return proc.stdout.splitlines() + + +def check(label: str, condition: bool) -> None: + print(f"{'PASS' if condition else 'FAIL'} {label}", flush=True) + if not condition: + sys.exit(1) + + +def volume(name: str, payload: str) -> Path: + path = Path(SUBJECT) / "volumes" / name / "_data" + path.mkdir(parents=True, exist_ok=True) + (path / "state").write_text(payload) + return path + + +plain = volume("plain", "plain-payload") +own = Path(SUBJECT) / "volumes" / "own" / "_data" +own.mkdir(parents=True, exist_ok=True) +shell(f"mount -t tmpfs tmpfs {own}") +(own / "state").write_text("own-payload") + +check("a plain volume is captured", unsnapshotted(Backing(str(plain)), SUBJECT) is None) +check( + "a volume on a mount of its own is not", + unsnapshotted(Backing(str(own)), SUBJECT) is not None, +) +check( + "a declared backing store is not, mounted or not", + unsnapshotted(Backing(str(plain), options={"type": "nfs"}), SUBJECT) is not None, +) +check( + "a foreign driver is not", + unsnapshotted(Backing(str(plain), driver="rexray"), SUBJECT) is not None, +) + +with volume_snapshot("btrfs", SUBJECT, "e2e", run=shell) as resolve: + frozen_plain = Path(resolve(str(plain))) + frozen_own = Path(resolve(str(own))) + + check( + "the snapshot carries the plain volume", + (frozen_plain / "state").read_text() == "plain-payload", + ) + check( + "the snapshot shows the other volume as an empty directory", + frozen_own.is_dir() and not any(frozen_own.iterdir()), + ) + + source, reason = snapshot_source(resolve, Backing(str(plain)), SUBJECT) + check( + "the plain volume is read from the snapshot", + source is not None and source.rstrip("/") == str(frozen_plain), + ) + + source, reason = snapshot_source(resolve, Backing(str(own)), SUBJECT) + check(f"the other volume degrades to live: {reason[:60]}", source is None) + +print("ALL OK", flush=True) diff --git a/tests/e2e/test_e2e_snapshot.py b/tests/e2e/test_e2e_snapshot.py index d8a4342..ef9be4a 100644 --- a/tests/e2e/test_e2e_snapshot.py +++ b/tests/e2e/test_e2e_snapshot.py @@ -17,6 +17,7 @@ from .helpers import require_docker, run, unique REPO_SRC = Path(__file__).resolve().parents[2] / "src" DRIVER = Path(__file__).resolve().parent / "snapshot_driver.py" +FAITHFUL_DRIVER = Path(__file__).resolve().parent / "faithful_driver.py" IMAGE = "alpine:3.20" PACKAGES = "apk add -q btrfs-progs e2fsprogs zfs python3 util-linux" ATTACH = ( @@ -76,20 +77,17 @@ def required(fstype: str) -> bool: return fstype in demanded.replace(",", " ").split() -def stage() -> Path: +def stage(driver: Path) -> Path: """Copy source and driver under /tmp, the only path the DinD daemon shares.""" staged = Path("/tmp") / unique("baudolo-e2e-snapshot") shutil.copytree(REPO_SRC, staged / "src") - shutil.copy(DRIVER, staged / "driver.py") + shutil.copy(driver, staged / "driver.py") return staged -def drive(fstype: str, kind: str, expect: str) -> str: - staged = stage() - script = ( - f"set -e; {mount_script(fstype)}; " - f"python3 /driver.py {kind} /subject/docker {expect}" - ) +def drive(fstype: str, arguments: str, *, driver: Path = DRIVER) -> str: + staged = stage(driver) + script = f"set -e; {mount_script(fstype)}; python3 /driver.py {arguments}" try: proc = run( [ @@ -115,7 +113,7 @@ def drive(fstype: str, kind: str, expect: str) -> str: shutil.rmtree(staged, ignore_errors=True) if proc.returncode != 0: raise AssertionError( - f"{fstype}/{kind} driver failed:\n{proc.stdout}\n{proc.stderr}" + f"{fstype} driver failed on {arguments}:\n{proc.stdout}\n{proc.stderr}" ) return proc.stdout @@ -126,7 +124,7 @@ class TestE2ESnapshot(unittest.TestCase): require_docker() def assert_freezes(self, fstype: str) -> None: - output = drive(fstype, fstype, "supported") + output = drive(fstype, f"{fstype} /subject/docker supported") self.assertIn("PASS the snapshot exposes the volume", output) self.assertIn("PASS a later write does not reach the snapshot", output) self.assertIn("PASS the snapshot is removed afterwards", output) @@ -148,13 +146,24 @@ class TestE2ESnapshot(unittest.TestCase): self.assert_freezes("zfs") def test_ext4_has_no_snapshot_and_says_so(self) -> None: - output = drive("ext4", "btrfs", "unsupported") + output = drive("ext4", "btrfs /subject/docker unsupported") self.assertIn("PASS refused loudly", output) def test_an_unknown_kind_is_refused_before_touching_the_filesystem(self) -> None: - output = drive("ext4", "lvm", "unsupported") + output = drive("ext4", "lvm /subject/docker unsupported") self.assertIn("PASS refused loudly", output) + def test_a_volume_with_its_own_storage_is_copied_live_not_from_the_snapshot( + self, + ) -> None: + output = drive("btrfs", "/subject/docker", driver=FAITHFUL_DRIVER) + self.assertIn( + "PASS the snapshot shows the other volume as an empty directory", output + ) + self.assertIn("PASS the other volume degrades to live", output) + self.assertIn("PASS the plain volume is read from the snapshot", output) + self.assertIn("ALL OK", output) + if __name__ == "__main__": unittest.main() diff --git a/tests/unit/backup/test_app_snapshot.py b/tests/unit/backup/test_app_snapshot.py index 5e21c7f..19c25dd 100644 --- a/tests/unit/backup/test_app_snapshot.py +++ b/tests/unit/backup/test_app_snapshot.py @@ -6,7 +6,9 @@ import unittest from unittest import mock from baudolo.backup import app +from baudolo.backup import snapshot as snapshot_mod from baudolo.backup.snapshot import volume_snapshot +from baudolo.backup.volume import Backing def stubbed_snapshot(kind: str, subject: str, tag: str): @@ -26,7 +28,7 @@ ARGV = [ ] -def drive(*, present: bool) -> list[dict]: +def drive(*, present: bool = True, reason: str | None = None) -> list[dict]: calls: list[dict] = [] def record(versions_dir, volume_name, volume_dir, *, authoritative, source): @@ -45,8 +47,11 @@ def drive(*, present: bool) -> list[dict]: 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="/var/lib/docker/volumes/vol/_data/" + app, + "inspect_backing", + return_value=Backing("/var/lib/docker/volumes/vol/_data"), ), + mock.patch.object(snapshot_mod, "unsnapshotted", return_value=reason), mock.patch.object(app, "stamp_directory"), mock.patch.object(app, "handle_docker_compose_services"), mock.patch.object(app.os.path, "isdir", return_value=present), @@ -75,6 +80,14 @@ class TestSnapshotBranch(unittest.TestCase): self.assertEqual(call["source"], "/var/lib/docker/volumes/vol/_data/") self.assertFalse(call["authoritative"]) + def test_a_volume_with_its_own_backing_store_is_copied_live(self) -> None: + call = drive(reason="it declares its own backing store")[0] + self.assertEqual(call["source"], "/var/lib/docker/volumes/vol/_data/") + self.assertFalse( + call["authoritative"], + "the snapshot holds an empty directory for it, not its data", + ) + if __name__ == "__main__": unittest.main() 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 aee2908..93a8971 100644 --- a/tests/unit/backup/test_app_volumes_no_backup_required.py +++ b/tests/unit/backup/test_app_volumes_no_backup_required.py @@ -7,6 +7,7 @@ import unittest from unittest import mock from baudolo.backup import app +from baudolo.backup.volume import Backing ARGV = [ "baudolo", @@ -47,7 +48,7 @@ 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, "get_storage_path", return_value="/data/"), + mock.patch.object(app, "inspect_backing", return_value=Backing("/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), diff --git a/tests/unit/backup/test_snapshot_faithfulness.py b/tests/unit/backup/test_snapshot_faithfulness.py new file mode 100644 index 0000000..e301dcd --- /dev/null +++ b/tests/unit/backup/test_snapshot_faithfulness.py @@ -0,0 +1,126 @@ +"""Which volumes a snapshot of the subject actually contains. + +The failure this guards against is silent: a volume with a backing store of +its own is present inside the snapshot as an empty directory, so rsync +succeeds, the generation is stamped complete, and the volume is empty in it. +""" + +from __future__ import annotations + +import os +import tempfile +import unittest +from unittest import mock + +from baudolo.backup.snapshot import SnapshotError, snapshot_source, unsnapshotted +from baudolo.backup.volume import Backing + + +class TestUnsnapshotted(unittest.TestCase): + def setUp(self) -> None: + self.subject = tempfile.mkdtemp() + self.mountpoint = os.path.join(self.subject, "volumes", "app", "_data") + os.makedirs(self.mountpoint) + + def backing(self, **kwargs) -> Backing: + return Backing(kwargs.pop("mountpoint", self.mountpoint), **kwargs) + + def test_a_plain_local_volume_is_captured(self) -> None: + self.assertIsNone(unsnapshotted(self.backing(), self.subject)) + + def test_a_foreign_driver_is_not(self) -> None: + reason = unsnapshotted(self.backing(driver="rexray"), self.subject) + self.assertIn("rexray", reason) + + def test_declared_driver_options_are_not(self) -> None: + reason = unsnapshotted( + self.backing(options={"type": "nfs", "device": ":/exports/app"}), + self.subject, + ) + self.assertIn("backing store", reason) + + def test_the_declaration_decides_not_the_mount_table(self) -> None: + """Docker unmounts an NFS volume when its last container stops.""" + with mock.patch.object(os.path, "ismount", return_value=False): + reason = unsnapshotted(self.backing(options={"type": "nfs"}), self.subject) + self.assertIsNotNone(reason) + + def test_a_volume_without_a_mountpoint_is_not(self) -> None: + reason = unsnapshotted(Backing(""), self.subject) + self.assertIn("no mountpoint", reason) + + def test_an_own_mount_is_not(self) -> None: + with mock.patch.object(os.path, "ismount", return_value=True): + reason = unsnapshotted(self.backing(), self.subject) + self.assertIn("own mount", reason) + + def test_a_filesystem_boundary_is_not(self) -> None: + real = os.stat + + def crossing(path, *args, **kwargs): + info = real(path, *args, **kwargs) + if os.path.realpath(path) == os.path.realpath(self.mountpoint): + return os.stat_result( + (info.st_mode, info.st_ino, info.st_dev + 1, *tuple(info)[3:]) + ) + return info + + with mock.patch.object(os, "stat", side_effect=crossing): + reason = unsnapshotted(self.backing(), self.subject) + self.assertIn("filesystem boundary", reason) + + def test_an_unreadable_mountpoint_is_not(self) -> None: + reason = unsnapshotted( + self.backing(mountpoint=os.path.join(self.subject, "gone")), self.subject + ) + self.assertIn("could not be read", reason) + + +class TestSnapshotSource(unittest.TestCase): + def setUp(self) -> None: + self.subject = tempfile.mkdtemp() + self.mountpoint = os.path.join(self.subject, "volumes", "app", "_data") + os.makedirs(self.mountpoint) + self.snapshot = os.path.join( + self.subject, ".baudolo-tag", "volumes", "app", "_data" + ) + os.makedirs(self.snapshot) + self.backing = Backing(self.mountpoint) + + def test_a_captured_volume_reads_from_the_snapshot(self) -> None: + source, reason = snapshot_source( + lambda path: self.snapshot + "/", self.backing, self.subject + ) + self.assertEqual(source, self.snapshot + "/") + self.assertEqual(reason, "") + + def test_an_uncaptured_volume_is_refused_before_the_resolver_runs(self) -> None: + def resolve(path): + raise AssertionError("must not resolve a volume the snapshot misses") + + source, reason = snapshot_source( + resolve, Backing(self.mountpoint, options={"type": "nfs"}), self.subject + ) + self.assertIsNone(source) + self.assertIn("backing store", reason) + + def test_a_volume_outside_the_subject_degrades_instead_of_raising(self) -> None: + def resolve(path): + raise SnapshotError(f"{path} lies outside the snapshot subject") + + source, reason = snapshot_source(resolve, self.backing, self.subject) + self.assertIsNone(source) + self.assertIn("lies outside", reason) + + def test_a_volume_created_after_the_snapshot_degrades(self) -> None: + source, reason = snapshot_source( + lambda path: os.path.join(self.subject, "absent") + "/", + self.backing, + self.subject, + ) + self.assertIsNone(source) + self.assertIn("created after", reason) + + +if __name__ == "__main__": + unittest.main()