mirror of
https://github.com/kevinveenbirkenbach/docker-volume-backup.git
synced 2026-08-20 13:12:48 +00:00
fix(backup): detect the engine by its dump tool, not by the image name
36b2336 matched postgres and mariadb against the image's repository path. That name is not a property of the software: a dedicated Postgres inside an app's own stack is tagged <app>-database or postgis/postgis and carries no engine token at all, so it was never recognised, never dumped, and its data directory was copied as files without a single warning - the fallback notice hangs on found_db, which stayed false.
The container is now asked what it can run. pg_dumpall, mariadb-dump and mysqldump are probed by executing them, not by asking a shell for them, because a distroless image has no shell and would deny every tool it ships. The verdict is cached per image ID rather than per container, so replicas of one image cost a single probe.
Because the probe names the tool it found, the dump uses it instead of the hardcoded /usr/bin/mariadb-dump, which makes an image that ships only mysqldump dumpable rather than silently file-copied.
image_name and has_image are gone with their registry-host and tag stripping; the trap they worked around cannot occur when nothing reads the name. get_image_info stays, since the --images-* lists match exact references.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
177
tests/e2e/test_e2e_engine_detection_by_tool.py
Normal file
177
tests/e2e/test_e2e_engine_detection_by_tool.py
Normal file
@@ -0,0 +1,177 @@
|
||||
"""The engine comes from the tools a container ships, not from its image name.
|
||||
|
||||
Two containers in one backup run, each lying in one direction:
|
||||
|
||||
* a real Postgres tagged `<prefix>-database`, the way a dedicated database is
|
||||
built inside an app's own stack - no engine token anywhere in the name;
|
||||
* an Alpine tagged `postgres:<prefix>`, carrying the token without shipping a
|
||||
single Postgres binary.
|
||||
|
||||
Reading the name gets both wrong, and the second one fatally: pg_dump exits 127
|
||||
inside Alpine and takes the whole run with it.
|
||||
"""
|
||||
|
||||
import unittest
|
||||
|
||||
from .helpers import (
|
||||
POSTGRES_DATA_DIR,
|
||||
POSTGRES_IMAGE,
|
||||
backup_path,
|
||||
backup_run,
|
||||
cleanup_docker,
|
||||
create_minimal_compose_dir,
|
||||
ensure_empty_dir,
|
||||
latest_version_dir,
|
||||
require_docker,
|
||||
run,
|
||||
unique,
|
||||
wait_for_postgres,
|
||||
write_databases_csv,
|
||||
)
|
||||
|
||||
IMPOSTOR_BASE_IMAGE = "alpine:3.20"
|
||||
MARKER = "engine-detection-by-tool"
|
||||
|
||||
|
||||
class TestE2EEngineDetectionByTool(unittest.TestCase):
|
||||
@classmethod
|
||||
def setUpClass(cls) -> None:
|
||||
require_docker()
|
||||
cls.prefix = unique("baudolo-e2e-engine-by-tool")
|
||||
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.engine_image = f"{cls.prefix}-database:17"
|
||||
cls.impostor_image = f"postgres:{cls.prefix}"
|
||||
|
||||
cls.engine_container = f"{cls.prefix}-engine"
|
||||
cls.impostor_container = f"{cls.prefix}-impostor"
|
||||
cls.engine_volume = f"{cls.prefix}-engine-vol"
|
||||
cls.impostor_volume = f"{cls.prefix}-impostor-vol"
|
||||
|
||||
cls.containers = [cls.engine_container, cls.impostor_container]
|
||||
cls.volumes = [cls.engine_volume, cls.impostor_volume]
|
||||
|
||||
run(["docker", "pull", POSTGRES_IMAGE])
|
||||
run(["docker", "pull", IMPOSTOR_BASE_IMAGE])
|
||||
run(["docker", "tag", POSTGRES_IMAGE, cls.engine_image])
|
||||
run(["docker", "tag", IMPOSTOR_BASE_IMAGE, cls.impostor_image])
|
||||
run(["docker", "volume", "create", cls.engine_volume])
|
||||
run(["docker", "volume", "create", cls.impostor_volume])
|
||||
|
||||
run(
|
||||
[
|
||||
"docker",
|
||||
"run",
|
||||
"-d",
|
||||
"--name",
|
||||
cls.engine_container,
|
||||
"-e",
|
||||
"POSTGRES_PASSWORD=pgpw",
|
||||
"-e",
|
||||
"POSTGRES_DB=appdb",
|
||||
"-e",
|
||||
"POSTGRES_USER=postgres",
|
||||
"-v",
|
||||
f"{cls.engine_volume}:{POSTGRES_DATA_DIR}",
|
||||
cls.engine_image,
|
||||
]
|
||||
)
|
||||
wait_for_postgres(cls.engine_container, user="postgres", timeout_s=90)
|
||||
run(
|
||||
[
|
||||
"docker",
|
||||
"exec",
|
||||
cls.engine_container,
|
||||
"sh",
|
||||
"-lc",
|
||||
(
|
||||
"psql -U postgres -d appdb -c "
|
||||
'"CREATE TABLE t (id int primary key, v text); '
|
||||
"INSERT INTO t VALUES (1,'ok');\""
|
||||
),
|
||||
]
|
||||
)
|
||||
|
||||
run(
|
||||
[
|
||||
"docker",
|
||||
"run",
|
||||
"-d",
|
||||
"--name",
|
||||
cls.impostor_container,
|
||||
"-v",
|
||||
f"{cls.impostor_volume}:/data",
|
||||
cls.impostor_image,
|
||||
"sh",
|
||||
"-lc",
|
||||
f"echo '{MARKER}' > /data/marker.txt && sleep 3600",
|
||||
]
|
||||
)
|
||||
|
||||
cls.databases_csv = f"/tmp/{cls.prefix}/databases.csv"
|
||||
write_databases_csv(
|
||||
cls.databases_csv,
|
||||
[
|
||||
(cls.engine_container, "appdb", "postgres", "pgpw"),
|
||||
(cls.impostor_container, "appdb", "postgres", "pgpw"),
|
||||
],
|
||||
)
|
||||
|
||||
backup_run(
|
||||
backups_dir=cls.backups_dir,
|
||||
repo_name=cls.repo_name,
|
||||
compose_dir=cls.compose_dir,
|
||||
databases_csv=cls.databases_csv,
|
||||
database_containers=[cls.engine_container, cls.impostor_container],
|
||||
images_no_stop_required=[cls.engine_image, cls.impostor_image],
|
||||
only_sql=True,
|
||||
)
|
||||
|
||||
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)
|
||||
run(["docker", "rmi", cls.engine_image], check=False)
|
||||
run(["docker", "rmi", cls.impostor_image], check=False)
|
||||
|
||||
def _volume_dir(self, volume: str):
|
||||
return backup_path(self.backups_dir, self.repo_name, self.version, volume)
|
||||
|
||||
def test_an_engine_without_an_engine_name_is_still_dumped(self) -> None:
|
||||
dump = self._volume_dir(self.engine_volume) / "sql" / "appdb.backup.sql"
|
||||
self.assertTrue(
|
||||
dump.is_file(),
|
||||
f"a Postgres tagged '{self.engine_image}' produced no dump at {dump}",
|
||||
)
|
||||
self.assertIn("Dumped by pg_dump", dump.read_text(encoding="utf-8"))
|
||||
|
||||
def test_an_engine_name_without_an_engine_is_not_dumped(self) -> None:
|
||||
sql_dir = self._volume_dir(self.impostor_volume) / "sql"
|
||||
dumps = list(sql_dir.glob("*.sql")) if sql_dir.exists() else []
|
||||
self.assertEqual(
|
||||
dumps,
|
||||
[],
|
||||
f"'{self.impostor_image}' ships no Postgres yet was dumped: {dumps}",
|
||||
)
|
||||
|
||||
def test_the_recognised_engine_is_dumped_instead_of_copied(self) -> None:
|
||||
files = self._volume_dir(self.engine_volume) / "files"
|
||||
self.assertFalse(
|
||||
files.exists(),
|
||||
f"--only-sql still copied the engine's files to {files}",
|
||||
)
|
||||
|
||||
def test_the_impostor_falls_through_to_a_file_backup(self) -> None:
|
||||
files = self._volume_dir(self.impostor_volume) / "files"
|
||||
self.assertTrue(files.is_dir(), f"expected a file backup at {files}")
|
||||
self.assertEqual(
|
||||
(files / "marker.txt").read_text(encoding="utf-8").strip(), MARKER
|
||||
)
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
@@ -148,6 +148,7 @@ class TestE2EMariaDBAnonymousPreemption(unittest.TestCase):
|
||||
container=self.db_container,
|
||||
volume_dir=volume_dir,
|
||||
db_type="mariadb",
|
||||
dump_tool="mariadb-dump",
|
||||
databases_df=df,
|
||||
database_containers=[self.db_container],
|
||||
)
|
||||
|
||||
@@ -13,7 +13,7 @@ def _df(rows):
|
||||
)
|
||||
|
||||
|
||||
def _capture_commands(*, db_type, rows, container):
|
||||
def _capture_commands(*, db_type, rows, container, dump_tool="mariadb-dump"):
|
||||
captured = []
|
||||
|
||||
def _capture(cmd):
|
||||
@@ -28,6 +28,7 @@ def _capture_commands(*, db_type, rows, container):
|
||||
container=container,
|
||||
volume_dir=td,
|
||||
db_type=db_type,
|
||||
dump_tool=dump_tool,
|
||||
databases_df=_df(rows),
|
||||
database_containers=[container],
|
||||
)
|
||||
@@ -57,6 +58,19 @@ class TestMariaDBDumpUsesTCP(unittest.TestCase):
|
||||
self.assertIn("-ps3cret", cmd)
|
||||
self.assertIn(" appdb", cmd)
|
||||
|
||||
def test_the_probed_client_is_the_one_invoked(self):
|
||||
captured = _capture_commands(
|
||||
db_type="mariadb",
|
||||
rows=[("mariadb", "appdb", "appuser", "s3cret")],
|
||||
container="mariadb",
|
||||
dump_tool="mysqldump",
|
||||
)
|
||||
dump_cmds = [c for c in captured if "mysqldump" in c]
|
||||
self.assertEqual(
|
||||
len(dump_cmds), 1, f"expected one dump command, got: {captured}"
|
||||
)
|
||||
self.assertNotIn("mariadb-dump", dump_cmds[0])
|
||||
|
||||
def test_postgres_dump_unaffected(self):
|
||||
captured = _capture_commands(
|
||||
db_type="postgres",
|
||||
|
||||
@@ -1,50 +0,0 @@
|
||||
import unittest
|
||||
from unittest.mock import patch
|
||||
|
||||
from baudolo.backup import docker as docker_mod
|
||||
|
||||
|
||||
def _with_image(reference: str):
|
||||
return patch.object(docker_mod, "execute_shell_command", return_value=[reference])
|
||||
|
||||
|
||||
class TestImageName(unittest.TestCase):
|
||||
def test_plain_reference(self) -> None:
|
||||
with _with_image("postgres:16"):
|
||||
self.assertEqual(docker_mod.image_name("c1"), "postgres")
|
||||
|
||||
def test_registry_host_is_dropped(self) -> None:
|
||||
with _with_image("svc-db-mariadb-swarm-mgr-01:5000/postgres_custom:17-3.5"):
|
||||
self.assertEqual(docker_mod.image_name("c1"), "postgres_custom")
|
||||
|
||||
def test_pull_through_path_is_kept(self) -> None:
|
||||
with _with_image(
|
||||
"svc-db-mariadb-swarm-mgr-01:5000/ghcr.io/x/mirror/docker.io/postgres:16"
|
||||
):
|
||||
self.assertEqual(
|
||||
docker_mod.image_name("c1"), "ghcr.io/x/mirror/docker.io/postgres"
|
||||
)
|
||||
|
||||
def test_digest_is_dropped(self) -> None:
|
||||
with _with_image("registry:5000/postgres@sha256:" + "0" * 64):
|
||||
self.assertEqual(docker_mod.image_name("c1"), "postgres")
|
||||
|
||||
|
||||
class TestHasImage(unittest.TestCase):
|
||||
def test_registry_hostname_does_not_decide_the_engine(self) -> None:
|
||||
with _with_image("svc-db-mariadb-swarm-mgr-01:5000/postgres_custom:17-3.5"):
|
||||
self.assertFalse(docker_mod.has_image("c1", "mariadb"))
|
||||
with _with_image("svc-db-mariadb-swarm-mgr-01:5000/postgres_custom:17-3.5"):
|
||||
self.assertTrue(docker_mod.has_image("c1", "postgres"))
|
||||
|
||||
def test_tag_does_not_decide_the_engine(self) -> None:
|
||||
with _with_image("registry:5000/xwiki_custom:lts-postgres-tomcat"):
|
||||
self.assertFalse(docker_mod.has_image("c1", "postgres"))
|
||||
|
||||
def test_mirrored_mariadb_still_matches(self) -> None:
|
||||
with _with_image("registry:5000/ghcr.io/x/mirror/docker.io/mariadb:11"):
|
||||
self.assertTrue(docker_mod.has_image("c1", "mariadb"))
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
42
tests/unit/backup/test_docker_tool_probe.py
Normal file
42
tests/unit/backup/test_docker_tool_probe.py
Normal file
@@ -0,0 +1,42 @@
|
||||
import unittest
|
||||
from unittest.mock import patch
|
||||
|
||||
from baudolo.backup import docker as docker_mod
|
||||
from baudolo.backup.shell import BackupException
|
||||
|
||||
|
||||
class TestImageId(unittest.TestCase):
|
||||
def test_the_id_is_returned_without_surrounding_whitespace(self) -> None:
|
||||
with patch.object(
|
||||
docker_mod, "execute_shell_command", return_value=["sha256:abc \n"]
|
||||
):
|
||||
self.assertEqual(docker_mod.image_id("c1"), "sha256:abc")
|
||||
|
||||
|
||||
class TestHasTool(unittest.TestCase):
|
||||
def test_a_tool_that_runs_is_present(self) -> None:
|
||||
with patch.object(docker_mod, "execute_shell_command", return_value=[]):
|
||||
self.assertTrue(docker_mod.has_tool("c1", "pg_dumpall"))
|
||||
|
||||
def test_a_tool_that_exits_non_zero_is_absent(self) -> None:
|
||||
with patch.object(
|
||||
docker_mod, "execute_shell_command", side_effect=BackupException("127")
|
||||
):
|
||||
self.assertFalse(docker_mod.has_tool("c1", "mariadb-dump"))
|
||||
|
||||
def test_the_probe_needs_no_shell_in_the_image(self) -> None:
|
||||
"""A distroless database ships no shell; `sh -c` would deny every tool."""
|
||||
captured = []
|
||||
|
||||
def _capture(cmd):
|
||||
captured.append(cmd)
|
||||
return []
|
||||
|
||||
with patch.object(docker_mod, "execute_shell_command", side_effect=_capture):
|
||||
docker_mod.has_tool("c1", "pg_dumpall")
|
||||
|
||||
self.assertEqual(captured, ["docker exec c1 pg_dumpall --version"])
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
138
tests/unit/backup/test_dumps_engine_detection.py
Normal file
138
tests/unit/backup/test_dumps_engine_detection.py
Normal file
@@ -0,0 +1,138 @@
|
||||
import unittest
|
||||
from unittest.mock import patch
|
||||
|
||||
import pandas
|
||||
|
||||
from baudolo.backup import dumps as dumps_mod
|
||||
|
||||
|
||||
def _df(rows):
|
||||
return pandas.DataFrame(
|
||||
rows, columns=["instance", "database", "username", "password"]
|
||||
)
|
||||
|
||||
|
||||
class _Probe:
|
||||
def __init__(self, available, image="sha256:aaa"):
|
||||
self.available = set(available)
|
||||
self.image = image
|
||||
self.calls = []
|
||||
|
||||
def has_tool(self, container, tool):
|
||||
self.calls.append((container, tool))
|
||||
return tool in self.available
|
||||
|
||||
def image_id(self, container):
|
||||
return self.image if isinstance(self.image, str) else self.image[container]
|
||||
|
||||
|
||||
def _detect(probe, container="c1"):
|
||||
dumps_mod._ENGINE_BY_IMAGE.clear()
|
||||
with (
|
||||
patch.object(dumps_mod, "has_tool", probe.has_tool),
|
||||
patch.object(dumps_mod, "image_id", probe.image_id),
|
||||
):
|
||||
return dumps_mod.container_engine(container)
|
||||
|
||||
|
||||
class TestContainerEngine(unittest.TestCase):
|
||||
def test_a_postgres_is_found_by_its_dump_tool(self):
|
||||
self.assertEqual(_detect(_Probe(["pg_dumpall"])), ("postgres", "pg_dumpall"))
|
||||
|
||||
def test_a_mariadb_is_found_by_its_dump_tool(self):
|
||||
self.assertEqual(_detect(_Probe(["mariadb-dump"])), ("mariadb", "mariadb-dump"))
|
||||
|
||||
def test_an_image_with_only_mysqldump_is_dumped_with_mysqldump(self):
|
||||
self.assertEqual(_detect(_Probe(["mysqldump"])), ("mariadb", "mysqldump"))
|
||||
|
||||
def test_a_container_without_either_tool_is_no_database(self):
|
||||
self.assertIsNone(_detect(_Probe([])))
|
||||
|
||||
def test_the_probe_stops_at_the_first_tool_it_finds(self):
|
||||
probe = _Probe(["pg_dumpall", "mariadb-dump"])
|
||||
_detect(probe)
|
||||
self.assertEqual(probe.calls, [("c1", "pg_dumpall")])
|
||||
|
||||
def test_the_image_name_does_not_decide_the_engine(self):
|
||||
"""The trap the old substring test fell into, from both directions."""
|
||||
probe = _Probe(["pg_dumpall"], image="svc-db-mariadb-mgr-01:5000/pg_custom")
|
||||
self.assertEqual(_detect(probe), ("postgres", "pg_dumpall"))
|
||||
|
||||
probe = _Probe(["mariadb-dump"], image="discourse-database:17")
|
||||
self.assertEqual(_detect(probe), ("mariadb", "mariadb-dump"))
|
||||
|
||||
|
||||
class TestProbeCache(unittest.TestCase):
|
||||
def test_replicas_of_one_image_are_probed_once(self):
|
||||
probe = _Probe(["pg_dumpall"])
|
||||
dumps_mod._ENGINE_BY_IMAGE.clear()
|
||||
with (
|
||||
patch.object(dumps_mod, "has_tool", probe.has_tool),
|
||||
patch.object(dumps_mod, "image_id", probe.image_id),
|
||||
):
|
||||
first = dumps_mod.container_engine("replica-1")
|
||||
second = dumps_mod.container_engine("replica-2")
|
||||
self.assertEqual(first, second)
|
||||
self.assertEqual(len(probe.calls), 1)
|
||||
|
||||
def test_a_second_image_is_probed_separately(self):
|
||||
probe = _Probe(["pg_dumpall"], image={"pg": "sha256:aaa", "app": "sha256:bbb"})
|
||||
dumps_mod._ENGINE_BY_IMAGE.clear()
|
||||
with (
|
||||
patch.object(dumps_mod, "has_tool", probe.has_tool),
|
||||
patch.object(dumps_mod, "image_id", probe.image_id),
|
||||
):
|
||||
self.assertEqual(
|
||||
dumps_mod.container_engine("pg"), ("postgres", "pg_dumpall")
|
||||
)
|
||||
probe.available = set()
|
||||
self.assertIsNone(dumps_mod.container_engine("app"))
|
||||
|
||||
|
||||
class TestBackupDispatch(unittest.TestCase):
|
||||
def test_the_probed_tool_reaches_the_dump(self):
|
||||
probe = _Probe(["mysqldump"])
|
||||
seen = {}
|
||||
|
||||
def _fake_backup_database(**kwargs):
|
||||
seen.update(kwargs)
|
||||
return True
|
||||
|
||||
dumps_mod._ENGINE_BY_IMAGE.clear()
|
||||
with (
|
||||
patch.object(dumps_mod, "has_tool", probe.has_tool),
|
||||
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(
|
||||
container="c1",
|
||||
volume_dir="/tmp",
|
||||
databases_df=_df([("c1", "appdb", "u", "p")]),
|
||||
database_containers=["c1"],
|
||||
)
|
||||
|
||||
self.assertTrue(is_db)
|
||||
self.assertTrue(dumped)
|
||||
self.assertEqual(seen["db_type"], "mariadb")
|
||||
self.assertEqual(seen["dump_tool"], "mysqldump")
|
||||
|
||||
def test_a_non_database_container_is_left_to_the_file_backup(self):
|
||||
probe = _Probe([])
|
||||
dumps_mod._ENGINE_BY_IMAGE.clear()
|
||||
with (
|
||||
patch.object(dumps_mod, "has_tool", probe.has_tool),
|
||||
patch.object(dumps_mod, "image_id", probe.image_id),
|
||||
):
|
||||
self.assertEqual(
|
||||
dumps_mod.backup_mariadb_or_postgres(
|
||||
container="c1",
|
||||
volume_dir="/tmp",
|
||||
databases_df=_df([]),
|
||||
database_containers=[],
|
||||
),
|
||||
(False, False),
|
||||
)
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
Reference in New Issue
Block a user