diff --git a/scripts/test-e2e.sh b/scripts/test-e2e.sh index 02a50e7..86cb761 100755 --- a/scripts/test-e2e.sh +++ b/scripts/test-e2e.sh @@ -16,9 +16,16 @@ DIND="${E2E_DIND_NAME:-baudolo-e2e-dind}" DIND_VOL="${E2E_DIND_VOL:-baudolo-e2e-dind-data}" E2E_TMP_VOL="${E2E_TMP_VOL:-baudolo-e2e-tmp}" -DIND_HOST="${E2E_DIND_HOST:-tcp://127.0.0.1:2375}" +# Host-side access to the DinD daemon goes through `docker exec` (dind() +# below) instead of a host-published port: port publishing is not reachable +# from every environment (sandboxed runners, hosts with broken loopback +# publishing), while exec only needs the outer docker socket. The TCP +# listener stays for the test container inside the dedicated network. DIND_HOST_IN_NET="${E2E_DIND_HOST_IN_NET:-tcp://${DIND}:2375}" +dind() { docker exec "${DIND}" docker "$@"; } +dind_stdin() { docker exec -i "${DIND}" docker "$@"; } + IMG="${E2E_IMAGE:-baudolo:local}" RSYNC_IMG="${E2E_RSYNC_IMAGE:-ghcr.io/kevinveenbirkenbach/alpine-rsync}" @@ -48,8 +55,8 @@ dump_debug() { echo "=== Host docker info ===" docker info || true echo - echo "=== DinD reachable? (docker -H ${DIND_HOST} version) ===" - docker -H "${DIND_HOST}" version || true + echo "=== DinD reachable? (docker exec ${DIND} docker version) ===" + dind version || true echo } > "${ARTIFACTS_DIR}/debug-host-${TS}.txt" 2>&1 || true @@ -58,45 +65,31 @@ dump_debug() { # DinD state { - echo "=== docker -H ps -a ===" - docker -H "${DIND_HOST}" ps -a || true + echo "=== dind ps -a ===" + dind ps -a || true echo - echo "=== docker -H images ===" - docker -H "${DIND_HOST}" images || true + echo "=== dind images ===" + dind images || true echo - echo "=== docker -H network ls ===" - docker -H "${DIND_HOST}" network ls || true + echo "=== dind network ls ===" + dind network ls || true echo - echo "=== docker -H volume ls ===" - docker -H "${DIND_HOST}" volume ls || true + echo "=== dind volume ls ===" + dind volume ls || true echo - echo "=== docker -H system df ===" - docker -H "${DIND_HOST}" system df || true + echo "=== dind system df ===" + dind system df || true } > "${ARTIFACTS_DIR}/debug-dind-${TS}.txt" 2>&1 || true # Try to capture recent events (best effort; might be noisy) - docker -H "${DIND_HOST}" events --since 10m --until 0s \ + dind events --since 10m --until 0s \ > "${ARTIFACTS_DIR}/dind-events-${TS}.txt" 2>&1 || true - # Dump shared /tmp content from the tmp volume: - # We create a temporary container that mounts the volume, then tar its content. - # (Does not rely on host filesystem paths.) + # The shared tmp volume is mounted at /tmp inside the DinD container + # itself, so tar it there and copy it out with the outer daemon. log "DEBUG: archiving shared /tmp (volume ${E2E_TMP_VOL})" - docker -H "${DIND_HOST}" run --rm \ - -v "${E2E_TMP_VOL}:/tmp" \ - alpine:3.20 \ - bash -lc 'cd /tmp && tar -czf /out.tar.gz . || true' \ - >/dev/null 2>&1 || true - - # The above writes inside the container FS, not to host. So do it properly: - # Use "docker cp" from a temp container. - local tmpc="baudolo-e2e-tmpdump-${TS}" - docker -H "${DIND_HOST}" rm -f "${tmpc}" >/dev/null 2>&1 || true - docker -H "${DIND_HOST}" create --name "${tmpc}" -v "${E2E_TMP_VOL}:/tmp" alpine:3.20 \ - bash -lc 'cd /tmp && tar -czf /tmpdump.tar.gz . || true' >/dev/null - docker -H "${DIND_HOST}" start -a "${tmpc}" >/dev/null 2>&1 || true - docker -H "${DIND_HOST}" cp "${tmpc}:/tmpdump.tar.gz" "${ARTIFACTS_DIR}/e2e-tmp-${TS}.tar.gz" >/dev/null 2>&1 || true - docker -H "${DIND_HOST}" rm -f "${tmpc}" >/dev/null 2>&1 || true + docker exec "${DIND}" tar -czf "/tmpdump-${TS}.tar.gz" -C /tmp . >/dev/null 2>&1 || true + docker cp "${DIND}:/tmpdump-${TS}.tar.gz" "${ARTIFACTS_DIR}/e2e-tmp-${TS}.tar.gz" >/dev/null 2>&1 || true log "DEBUG: artifacts written:" find "${ARTIFACTS_DIR}" -maxdepth 1 -mindepth 1 -print | sed 's/^/ /' || true @@ -107,9 +100,9 @@ cleanup() { log "KEEP_ON_FAIL=1 and failure detected -> skipping cleanup." log "Next steps:" echo " - Inspect DinD logs: docker logs ${DIND} | less" - echo " - Use DinD daemon: docker -H ${DIND_HOST} ps -a" - echo " - Shared tmp vol: docker -H ${DIND_HOST} run --rm -v ${E2E_TMP_VOL}:/tmp alpine:3.20 ls -la /tmp" - echo " - DinD docker root: docker -H ${DIND_HOST} run --rm -v ${DIND_VOL}:/var/lib/docker alpine:3.20 ls -la /var/lib/docker/volumes" + echo " - Use DinD daemon: docker exec ${DIND} docker ps -a" + echo " - Shared tmp vol: docker exec ${DIND} ls -la /tmp" + echo " - DinD docker root: docker exec ${DIND} ls -la /var/lib/docker/volumes" return 0 fi @@ -150,7 +143,6 @@ docker run -d --privileged \ -e DOCKER_TLS_CERTDIR="" \ -v "${DIND_VOL}:/var/lib/docker" \ -v "${E2E_TMP_VOL}:/tmp" \ - -p 2375:2375 \ docker:dind \ --host=tcp://0.0.0.0:2375 \ --tls=false \ @@ -158,7 +150,7 @@ docker run -d --privileged \ log "Waiting for DinD to be ready..." for i in $(seq 1 "${READY_TIMEOUT_SECONDS}"); do - if docker -H "${DIND_HOST}" version >/dev/null 2>&1; then + if dind version >/dev/null 2>&1; then log "DinD is ready." break fi @@ -174,13 +166,13 @@ done log "Pre-pulling helper images in DinD..." log " - Pulling: ${RSYNC_IMG}" -docker -H "${DIND_HOST}" pull "${RSYNC_IMG}" +dind pull "${RSYNC_IMG}" log "Ensuring alpine exists in DinD (for debug helpers)" -docker -H "${DIND_HOST}" pull alpine:3.20 >/dev/null +dind pull alpine:3.20 >/dev/null log "Loading ${IMG} image into DinD..." -docker save "${IMG}" | docker -H "${DIND_HOST}" load >/dev/null +docker save "${IMG}" | dind_stdin load >/dev/null log "Running E2E tests inside DinD" set +e diff --git a/src/baudolo/backup/docker.py b/src/baudolo/backup/docker.py index 3997152..ed52065 100644 --- a/src/baudolo/backup/docker.py +++ b/src/baudolo/backup/docker.py @@ -1,6 +1,6 @@ from __future__ import annotations -from .shell import execute_shell_command +from .shell import BackupException, execute_shell_command def get_image_info(container: str) -> str: @@ -27,11 +27,16 @@ def containers_using_volume(volume_name: str) -> list[str]: def is_swarm_task(container: str) -> bool: """Swarm-managed task containers must never be stopped or started manually: the orchestrator replaces the stopped task and a later - `docker start` fails on the detached overlay network.""" - out = execute_shell_command( - "docker inspect --format " - f"'{{{{index .Config.Labels \"com.docker.swarm.task.id\"}}}}' {container}" - ) + `docker start` fails on the detached overlay network. A container that + vanished between listing and inspect (--rm one-shots, task-history GC) + counts as not stoppable instead of aborting the whole backup run.""" + try: + out = execute_shell_command( + "docker inspect --format " + f"'{{{{index .Config.Labels \"com.docker.swarm.task.id\"}}}}' {container}" + ) + except BackupException: + return True return bool(out and out[0].strip()) diff --git a/src/baudolo/restore/db/postgres.py b/src/baudolo/restore/db/postgres.py index 4132045..74000d7 100644 --- a/src/baudolo/restore/db/postgres.py +++ b/src/baudolo/restore/db/postgres.py @@ -1,9 +1,41 @@ from __future__ import annotations import os +import tempfile +from collections.abc import Iterable, Iterator from ..run import docker_exec +_SUPERUSER_ONLY_PREFIXES = (b"COMMENT ON EXTENSION", b"ALTER DEFAULT PRIVILEGES") + + +def filter_superuser_only_lines(lines: Iterable[bytes]) -> Iterator[bytes]: + """Drop superuser-only statements an app-level psql replay cannot run. + + Args: + lines: dump lines including their trailing newlines. + + Yields: + Every line except top-level statements starting with a superuser-only + prefix. Lines inside COPY ... FROM stdin data blocks are passed + through untouched: a data row may legally start with the same bytes, + and dropping it would silently corrupt the restored table. + """ + in_copy = False + for line in lines: + if in_copy: + yield line + if line.rstrip(b"\r\n") == b"\\.": + in_copy = False + continue + if line.startswith(b"COPY ") and line.rstrip(b"\r\n").endswith(b"FROM stdin;"): + in_copy = True + yield line + continue + if line.startswith(_SUPERUSER_ONLY_PREFIXES): + continue + yield line + def restore_postgres_sql( *, @@ -68,19 +100,18 @@ END $$; docker_env=docker_env, ) - with open(sql_path, "rb") as f: - raw_sql = f.read() - # COMMENT ON EXTENSION and ALTER DEFAULT PRIVILEGES are superuser-only; - # app-level restores must skip them or ON_ERROR_STOP aborts the replay. - superuser_only = (b"COMMENT ON EXTENSION", b"ALTER DEFAULT PRIVILEGES") - sql = b"\n".join( - line for line in raw_sql.splitlines() if not line.startswith(superuser_only) - ) - docker_exec( - container, - ["psql", "-v", "ON_ERROR_STOP=1", "-U", user, "-d", db_name], - stdin=sql, - docker_env=docker_env, - ) + # Filter into a spooled temp file instead of building the whole dump in + # memory: production dumps reach many GB and the previous read/splitlines/ + # join needed roughly three times the dump size in RSS. + with open(sql_path, "rb") as src, tempfile.TemporaryFile() as filtered: + for line in filter_superuser_only_lines(src): + filtered.write(line) + filtered.seek(0) + docker_exec( + container, + ["psql", "-v", "ON_ERROR_STOP=1", "-U", user, "-d", db_name], + stdin=filtered, + docker_env=docker_env, + ) print(f"PostgreSQL restore complete for db '{db_name}'.") diff --git a/tests/e2e/test_e2e_swarm_task_skip.py b/tests/e2e/test_e2e_swarm_task_skip.py new file mode 100644 index 0000000..9df3bb9 --- /dev/null +++ b/tests/e2e/test_e2e_swarm_task_skip.py @@ -0,0 +1,196 @@ +# tests/e2e/test_e2e_swarm_task_skip.py +# +# Reproduces the swarm flake fixed on this branch: baudolo used to stop a +# swarm task container around the volume file backup because its image was +# not whitelisted; the orchestrator immediately replaced the stopped task and +# the later `docker start` failed on the detached overlay network, killing +# the backup run. With the fix the task container is skipped (backed up hot): +# the backup succeeds, the very same container instance keeps running, and +# the service never has to replace a task. +import time +import unittest + +from .helpers import ( + backup_path, + backup_run, + create_minimal_compose_dir, + ensure_empty_dir, + latest_version_dir, + require_docker, + run, + unique, + write_databases_csv, +) + + +def _swarm_state() -> str: + return run( + ["docker", "info", "--format", "{{.Swarm.LocalNodeState}}"] + ).stdout.strip() + + +def _task_container_id(service: str, timeout_s: int = 60) -> str: + deadline = time.time() + timeout_s + while time.time() < deadline: + out = run( + [ + "docker", + "ps", + "--filter", + f"label=com.docker.swarm.service.name={service}", + "--format", + "{{.ID}}", + ] + ).stdout.strip() + if out: + return out.splitlines()[0] + time.sleep(2) + raise RuntimeError(f"No running task container for service {service}") + + +def _started_at(container_id: str) -> str: + return run( + ["docker", "inspect", "--format", "{{.State.StartedAt}}", container_id] + ).stdout.strip() + + +class TestE2ESwarmTaskSkip(unittest.TestCase): + @classmethod + def setUpClass(cls) -> None: + require_docker() + cls.prefix = unique("baudolo-e2e-swarm-skip") + 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.swarm_initted = False + if _swarm_state() != "active": + run(["docker", "swarm", "init", "--advertise-addr", "127.0.0.1"]) + cls.swarm_initted = True + + cls.volume = f"{cls.prefix}-vol" + cls.service = f"{cls.prefix}-svc" + cls.volumes = [cls.volume] + + run(["docker", "volume", "create", cls.volume]) + run( + [ + "docker", + "run", + "--rm", + "-v", + f"{cls.volume}:/data", + "alpine:3.20", + "sh", + "-lc", + "echo 'swarm-payload' > /data/payload.txt", + ] + ) + + run( + [ + "docker", + "service", + "create", + "--name", + cls.service, + "--replicas", + "1", + "--mount", + f"type=volume,source={cls.volume},target=/data", + "alpine:3.20", + "sleep", + "3600", + ] + ) + cls.task_cid = _task_container_id(cls.service) + cls.task_started_at = _started_at(cls.task_cid) + + cls.databases_csv = f"/tmp/{cls.prefix}/databases.csv" + write_databases_csv(cls.databases_csv, []) + + # Whitelist that matches nothing: on main this forces a stop of every + # container at the volume, i.e. exactly the flake; on this branch the + # swarm task must be skipped instead. (An empty list would leave the + # --images-no-stop-required flag without arguments and argparse-fail.) + 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=["image-that-matches-nothing"], + ) + cls.hash, cls.version = latest_version_dir(cls.backups_dir, cls.repo_name) + + @classmethod + def tearDownClass(cls) -> None: + run(["docker", "service", "rm", cls.service], check=False) + deadline = time.time() + 30 + while time.time() < deadline: + out = run( + [ + "docker", + "ps", + "-aq", + "--filter", + f"label=com.docker.swarm.service.name={cls.service}", + ], + check=False, + ).stdout.strip() + if not out: + break + time.sleep(2) + for v in cls.volumes: + run(["docker", "volume", "rm", "-f", v], check=False) + if cls.swarm_initted: + run(["docker", "swarm", "leave", "--force"], check=False) + + def test_volume_backed_up_hot(self) -> None: + p = ( + backup_path( + self.backups_dir, + self.repo_name, + self.version, + self.volume, + ) + / "files" + / "payload.txt" + ) + self.assertTrue(p.is_file(), f"Expected backed up file at: {p}") + + def test_task_container_never_stopped(self) -> None: + out = run( + ["docker", "ps", "-q", "--no-trunc", "--filter", f"id={self.task_cid}"] + ).stdout.strip() + self.assertTrue( + out.startswith(self.task_cid) or self.task_cid.startswith(out.strip()[:12]), + f"Task container {self.task_cid} is no longer running", + ) + self.assertEqual( + self.task_started_at, + _started_at(self.task_cid), + "Task container was restarted during the backup", + ) + + def test_service_never_replaced_the_task(self) -> None: + states = run( + [ + "docker", + "service", + "ps", + self.service, + "--format", + "{{.DesiredState}} {{.CurrentState}}", + ] + ).stdout.strip() + lines = [line for line in states.splitlines() if line.strip()] + self.assertEqual( + len(lines), 1, f"Service task history shows replacements:\n{states}" + ) + self.assertIn("Running", lines[0]) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/unit/backup/test_docker_swarm.py b/tests/unit/backup/test_docker_swarm.py new file mode 100644 index 0000000..fc5869a --- /dev/null +++ b/tests/unit/backup/test_docker_swarm.py @@ -0,0 +1,41 @@ +import unittest +from unittest.mock import patch + +from baudolo.backup import docker as docker_mod +from baudolo.backup.shell import BackupException + + +class TestIsSwarmTask(unittest.TestCase): + @patch.object(docker_mod, "execute_shell_command", return_value=["task-id-123"]) + def test_true_when_task_label_present(self, _mock) -> None: + self.assertTrue(docker_mod.is_swarm_task("c1")) + + @patch.object(docker_mod, "execute_shell_command", return_value=[""]) + def test_false_when_label_empty(self, _mock) -> None: + self.assertFalse(docker_mod.is_swarm_task("c1")) + + @patch.object(docker_mod, "execute_shell_command", return_value=[]) + def test_false_when_no_output(self, _mock) -> None: + self.assertFalse(docker_mod.is_swarm_task("c1")) + + @patch.object( + docker_mod, + "execute_shell_command", + side_effect=BackupException("gone"), + ) + def test_vanished_container_counts_as_not_stoppable(self, _mock) -> None: + # A container removed between listing and inspect must not abort the + # whole backup run; treating it as a swarm task keeps it out of every + # stop/start and image-inspect path. + self.assertTrue(docker_mod.is_swarm_task("gone-container")) + + +class TestFilterStoppable(unittest.TestCase): + @patch.object(docker_mod, "is_swarm_task", side_effect=[False, True, False]) + def test_mixed_list_keeps_order_and_drops_tasks(self, _mock) -> None: + result = docker_mod.filter_stoppable(["plain-1", "swarm-task", "plain-2"]) + self.assertEqual(result, ["plain-1", "plain-2"]) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/unit/restore/__init__.py b/tests/unit/restore/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/tests/unit/restore/test_mariadb_empty_drop.py b/tests/unit/restore/test_mariadb_empty_drop.py new file mode 100644 index 0000000..9e26e2b --- /dev/null +++ b/tests/unit/restore/test_mariadb_empty_drop.py @@ -0,0 +1,44 @@ +import tempfile +import unittest +from unittest.mock import MagicMock, patch + +from baudolo.restore.db import mariadb as mariadb_mod + + +class TestMariadbEmptyDrop(unittest.TestCase): + def test_drops_run_in_one_session_with_fk_checks_off(self) -> None: + calls = [] + + def _capture(container, argv, **kwargs): + calls.append(argv) + result = MagicMock() + result.stdout = b"users\nfetches\n" + return result + + with tempfile.NamedTemporaryFile(suffix=".sql") as sql: + sql.write(b"CREATE TABLE t (id int);\n") + sql.flush() + with ( + patch.object(mariadb_mod, "docker_exec", side_effect=_capture), + patch.object(mariadb_mod, "_pick_client", return_value="mariadb"), + ): + mariadb_mod.restore_mariadb_sql( + container="db", + db_name="mailu", + user="mailu", + password="pw", + sql_path=sql.name, + empty=True, + ) + + drop_calls = [argv for argv in calls if any("DROP TABLE" in a for a in argv)] + self.assertEqual(len(drop_calls), 1, f"expected ONE drop session: {calls}") + drop_sql = drop_calls[0][-1] + self.assertTrue(drop_sql.startswith("SET FOREIGN_KEY_CHECKS=0; ")) + self.assertIn("DROP TABLE IF EXISTS `mailu`.`users`;", drop_sql) + self.assertIn("DROP TABLE IF EXISTS `mailu`.`fetches`;", drop_sql) + self.assertTrue(drop_sql.endswith("SET FOREIGN_KEY_CHECKS=1;")) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/unit/restore/test_postgres_filter.py b/tests/unit/restore/test_postgres_filter.py new file mode 100644 index 0000000..dcb4ec9 --- /dev/null +++ b/tests/unit/restore/test_postgres_filter.py @@ -0,0 +1,63 @@ +import unittest + +from baudolo.restore.db.postgres import filter_superuser_only_lines + + +def _filter(raw: bytes) -> bytes: + return b"".join(filter_superuser_only_lines(raw.splitlines(keepends=True))) + + +class TestFilterSuperuserOnlyLines(unittest.TestCase): + def test_drops_superuser_only_statements(self) -> None: + raw = ( + b"CREATE TABLE t (id int);\n" + b"COMMENT ON EXTENSION pg_trgm IS 'trigram';\n" + b"ALTER DEFAULT PRIVILEGES IN SCHEMA public GRANT ALL ON TABLES TO x;\n" + b"INSERT INTO t VALUES (1);\n" + ) + self.assertEqual( + _filter(raw), + b"CREATE TABLE t (id int);\nINSERT INTO t VALUES (1);\n", + ) + + def test_copy_data_rows_are_never_filtered(self) -> None: + raw = ( + b"COPY public.snippets (body) FROM stdin;\n" + b"COMMENT ON EXTENSION looks like sql but is data\n" + b"ALTER DEFAULT PRIVILEGES stored as text\n" + b"\\.\n" + b"ALTER DEFAULT PRIVILEGES IN SCHEMA public REVOKE ALL ON TABLES FROM y;\n" + ) + self.assertEqual( + _filter(raw), + b"COPY public.snippets (body) FROM stdin;\n" + b"COMMENT ON EXTENSION looks like sql but is data\n" + b"ALTER DEFAULT PRIVILEGES stored as text\n" + b"\\.\n", + ) + + def test_consecutive_copy_blocks_keep_state(self) -> None: + raw = ( + b"COPY public.a (v) FROM stdin;\n" + b"row-a\n" + b"\\.\n" + b"COMMENT ON EXTENSION dropme IS 'x';\n" + b"COPY public.b (v) FROM stdin;\n" + b"COMMENT ON EXTENSION kept-as-data\n" + b"\\.\n" + ) + out = _filter(raw) + self.assertNotIn(b"dropme", out) + self.assertIn(b"COMMENT ON EXTENSION kept-as-data\n", out) + + def test_everything_else_passes_through_verbatim(self) -> None: + raw = ( + b"SET statement_timeout = 0;\n" + b"CREATE EXTENSION IF NOT EXISTS pg_trgm;\n" + b"GRANT ALL ON SCHEMA public TO app;\n" + ) + self.assertEqual(_filter(raw), raw) + + +if __name__ == "__main__": + unittest.main()