Merge pull request #223 from living-ip/codex/fix-postgres-volume-leak-20260722

Stop disposable Postgres fixtures leaking Docker volumes

tests/docker_volume_lifecycle.py

@@ -0,0 +1,128 @@
+from __future__ import annotations
+
+import fcntl
+import json
+import subprocess
+import tempfile
+import threading
+from dataclasses import dataclass
+from pathlib import Path
+from typing import TextIO
+
+POSTGRES_DATA_PATH = "/var/lib/postgresql/data"
+POSTGRES_TMPFS_SPEC = f"{POSTGRES_DATA_PATH}:rw,nosuid,nodev,noexec,size=384m"
+VOLUME_GUARD_LOCK = Path(tempfile.gettempdir()) / "livingip-teleo-docker-volume-lifecycle.lock"
+
+
+@dataclass(frozen=True)
+class DockerVolumeSetGuard:
+    before: frozenset[str]
+    context: str
+    owner_thread_id: int
+
+
+_PROCESS_GUARD = threading.RLock()
+_LOCK_FILE: TextIO | None = None
+_GUARD_STACK: list[DockerVolumeSetGuard] = []
+
+
+def _docker(*arguments: str, check: bool = True) -> subprocess.CompletedProcess[str]:
+    completed = subprocess.run(
+        ["docker", *arguments],
+        text=True,
+        capture_output=True,
+        check=False,
+        timeout=120,
+    )
+    if check and completed.returncode != 0:
+        raise AssertionError(completed.stderr or completed.stdout)
+    return completed
+
+
+def docker_volume_names() -> frozenset[str]:
+    completed = _docker("volume", "ls", "--quiet")
+    return frozenset(name for name in completed.stdout.splitlines() if name)
+
+
+def assert_docker_volume_set_unchanged(before: frozenset[str], *, context: str) -> None:
+    after = docker_volume_names()
+    added = sorted(after - before)
+    removed = sorted(before - after)
+    assert after == before, (
+        f"daemon-wide Docker volume drift while running {context}; another Docker job may be responsible; "
+        f"added={added!r}; removed={removed!r}; before_count={len(before)}; after_count={len(after)}"
+    )
+
+
+def acquire_docker_volume_set_guard(*, context: str) -> DockerVolumeSetGuard:
+    global _LOCK_FILE
+
+    _PROCESS_GUARD.acquire()
+    opened_here = False
+    try:
+        if _LOCK_FILE is None:
+            lock_file = VOLUME_GUARD_LOCK.open("a+", encoding="utf-8")
+            try:
+                fcntl.flock(lock_file.fileno(), fcntl.LOCK_EX)
+            except BaseException:
+                lock_file.close()
+                raise
+            _LOCK_FILE = lock_file
+            opened_here = True
+        guard = DockerVolumeSetGuard(
+            before=docker_volume_names(),
+            context=context,
+            owner_thread_id=threading.get_ident(),
+        )
+        _GUARD_STACK.append(guard)
+        return guard
+    except BaseException:
+        if opened_here and _LOCK_FILE is not None:
+            fcntl.flock(_LOCK_FILE.fileno(), fcntl.LOCK_UN)
+            _LOCK_FILE.close()
+            _LOCK_FILE = None
+        _PROCESS_GUARD.release()
+        raise
+
+
+def release_docker_volume_set_guard(guard: DockerVolumeSetGuard) -> None:
+    global _LOCK_FILE
+
+    assert guard.owner_thread_id == threading.get_ident(), "Docker volume guard released from a different thread"
+    assert _GUARD_STACK and _GUARD_STACK[-1] is guard, "Docker volume guards must be released in LIFO order"
+    try:
+        assert_docker_volume_set_unchanged(guard.before, context=guard.context)
+    finally:
+        _GUARD_STACK.pop()
+        if not _GUARD_STACK:
+            assert _LOCK_FILE is not None
+            fcntl.flock(_LOCK_FILE.fileno(), fcntl.LOCK_UN)
+            _LOCK_FILE.close()
+            _LOCK_FILE = None
+        _PROCESS_GUARD.release()
+
+
+def inspect_postgres_tmpfs_container(name: str) -> dict:
+    inspected = json.loads(_docker("inspect", name).stdout)[0]
+    assert inspected["Mounts"] == [], f"{name} unexpectedly uses Docker volumes or bind mounts"
+    tmpfs = inspected["HostConfig"]["Tmpfs"]
+    assert POSTGRES_DATA_PATH in tmpfs, f"{name} does not mount PostgreSQL data on tmpfs"
+    return inspected
+
+
+def remove_container_and_volumes(name: str) -> None:
+    removed = _docker("rm", "--force", "--volumes", name, check=False)
+    if removed.returncode != 0 and "no such container" not in removed.stderr.lower():
+        raise AssertionError(removed.stderr or removed.stdout)
+
+    assert _docker("container", "inspect", name, check=False).returncode != 0
+    names = _docker(
+        "container",
+        "ls",
+        "--all",
+        "--filter",
+        f"name=^/{name}$",
+        "--format",
+        "{{.Names}}",
+    )
+    assert names.stdout.strip() == "", f"container still exists after cleanup: {name}"

tests/test_kb_apply_prereqs.py

@@ -12,6 +12,14 @@ from pathlib import Path
 
 import pytest
 
+from tests.docker_volume_lifecycle import (
+    POSTGRES_TMPFS_SPEC,
+    acquire_docker_volume_set_guard,
+    inspect_postgres_tmpfs_container,
+    release_docker_volume_set_guard,
+    remove_container_and_volumes,
+)
+
 REPO_ROOT = Path(__file__).resolve().parents[1]
 MIGRATION = REPO_ROOT / "scripts" / "kb_apply_prereqs.sql"
 POSTGRES_IMAGE = os.environ.get("LEOCLEAN_RUNTIME_POSTGRES_IMAGE", "postgres:16-alpine")
@@ -197,16 +205,21 @@ def _postgres_clone() -> Iterator[str]:
     if shutil.which("docker") is None:
         pytest.skip("Docker is required for the disposable PostgreSQL migration canary")
 
+    volume_guard = acquire_docker_volume_set_guard(
+        context="kb_apply_prereqs PostgreSQL fixture",
+    )
     container = f"kb-apply-prereqs-{uuid.uuid4().hex[:12]}"
+    try:
         started = subprocess.run(
             [
                 "docker",
                 "run",
                 "--pull=never",
-            "--rm",
                 "-d",
                 "--name",
                 container,
+                "--tmpfs",
+                POSTGRES_TMPFS_SPEC,
                 "-e",
                 "POSTGRES_PASSWORD=postgres",
                 "-e",
@@ -219,8 +232,8 @@ def _postgres_clone() -> Iterator[str]:
         )
         if started.returncode != 0:
             pytest.fail(f"could not start disposable PostgreSQL: {started.stderr}")
+        inspect_postgres_tmpfs_container(container)
 
-    try:
         for _ in range(120):
             logs = subprocess.run(
                 ["docker", "logs", container],
@@ -239,11 +252,10 @@ def _postgres_clone() -> Iterator[str]:
         _psql(container, BOOTSTRAP_SQL)
         yield container
     finally:
-        subprocess.run(
-            ["docker", "rm", "-f", container],
-            capture_output=True,
-            check=False,
-        )
+        try:
+            remove_container_and_volumes(container)
+        finally:
+            release_docker_volume_set_guard(volume_guard)
 
 
 @pytest.fixture(scope="module")

tests/test_run_local_genesis_ledger_rebuild.py

@@ -17,6 +17,13 @@ import pytest
 
 from ops import run_local_genesis_ledger_rebuild as rebuild
 from ops.verify_postgres_parity_manifest import compare_manifests, load_manifest
+from tests.docker_volume_lifecycle import (
+    POSTGRES_TMPFS_SPEC,
+    acquire_docker_volume_set_guard,
+    inspect_postgres_tmpfs_container,
+    release_docker_volume_set_guard,
+    remove_container_and_volumes,
+)
 
 CLAIM_A = "aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa"
 CLAIM_B = "bbbbbbbb-bbbb-4bbb-8bbb-bbbbbbbbbbbb"
@@ -1472,13 +1479,16 @@ def capture_manifest(container: str, database: str, destination: Path) -> None:
 def source_postgres() -> Iterator[tuple[str, str]]:
     if shutil.which("docker") is None:
         pytest.skip("Docker is required for the genesis-plus-ledger lifecycle test")
+    volume_guard = acquire_docker_volume_set_guard(
+        context="genesis ledger source PostgreSQL fixture",
+    )
     container = f"genesis-ledger-source-{uuid.uuid4().hex[:12]}"
     database = "teleo_fixture"
+    try:
         started = subprocess.run(
             [
                 "docker",
                 "run",
-            "--rm",
                 "--detach",
                 "--network",
                 "none",
@@ -1486,6 +1496,8 @@ def source_postgres() -> Iterator[tuple[str, str]]:
                 container,
                 "--label",
                 f"{rebuild.canonical_rebuild.CANARY_LABEL}={container}",
+                "--tmpfs",
+                POSTGRES_TMPFS_SPEC,
                 "--env",
                 f"POSTGRES_DB={database}",
                 "--env",
@@ -1498,7 +1510,8 @@ def source_postgres() -> Iterator[tuple[str, str]]:
         )
         if started.returncode != 0:
             pytest.fail(f"could not start fixture PostgreSQL: {started.stderr}")
-    try:
+        inspect_postgres_tmpfs_container(container)
+
         wait_for_postgres(container, database)
         docker_psql(container, database, BOOTSTRAP_SQL)
         copy_result = subprocess.run(
@@ -1544,20 +1557,10 @@ def source_postgres() -> Iterator[tuple[str, str]]:
         )
         yield container, database
     finally:
-        subprocess.run(
-            ["docker", "rm", "--force", container],
-            text=True,
-            capture_output=True,
-            check=False,
-        )
-        inspected = subprocess.run(
-            ["docker", "container", "inspect", container],
-            text=True,
-            capture_output=True,
-            check=False,
-        )
-        if inspected.returncode == 0:
-            pytest.fail(f"fixture PostgreSQL container was not removed: {container}")
+        try:
+            remove_container_and_volumes(container)
+        finally:
+            release_docker_volume_set_guard(volume_guard)
 
 
 def test_live_manifest_normalizes_physical_database_authorization_identity(

tests/test_v3_source_rebuild_lifecycle_postgres.py

@@ -8,6 +8,7 @@ import subprocess
 import sys
 import time
 import uuid
+from collections.abc import Iterator
 from pathlib import Path
 
 import pytest
@@ -18,6 +19,13 @@ from ops import verify_teleo_v3_epistemic_contract as v3_contract
 from scripts import compile_kb_source_packet as compiler
 from tests import test_run_local_genesis_ledger_rebuild as genesis_test
 from tests import test_v3_apply_worker_postgres as worker_test
+from tests.docker_volume_lifecycle import (
+    POSTGRES_TMPFS_SPEC,
+    acquire_docker_volume_set_guard,
+    inspect_postgres_tmpfs_container,
+    release_docker_volume_set_guard,
+    remove_container_and_volumes,
+)
 
 REPO_ROOT = Path(__file__).resolve().parents[1]
 CORPUS_REBUILD = REPO_ROOT / "ops" / "run_local_corpus_knowledge_rebuild.py"
@@ -84,7 +92,6 @@ def _start_postgres(name: str, database: str, run_label: str, *, staging_target:
     command = [
         "run",
         "--detach",
-        "--rm",
         "--network",
         "none",
         "--name",
@@ -92,7 +99,7 @@ def _start_postgres(name: str, database: str, run_label: str, *, staging_target:
         "--label",
         f"{SOURCE_CONTAINER_LABEL}={run_label}",
         "--tmpfs",
-        "/var/lib/postgresql/data:rw,nosuid,nodev,noexec,size=384m",
+        POSTGRES_TMPFS_SPEC,
         "--env",
         f"POSTGRES_DB={database}",
         "--env",
@@ -101,6 +108,7 @@ def _start_postgres(name: str, database: str, run_label: str, *, staging_target:
     if staging_target:
         command.extend(("--label", STAGING_LABEL))
     command.append(v3_contract.POSTGRES_IMAGE)
+    try:
         container_id = _docker(*command).stdout.strip()
         assert container_id
 
@@ -124,19 +132,20 @@ def _start_postgres(name: str, database: str, run_label: str, *, staging_target:
         else:
             pytest.fail(f"disposable PostgreSQL did not become ready: {name}")
 
-    inspected = json.loads(_docker("inspect", name).stdout)[0]
+        inspected = inspect_postgres_tmpfs_container(name)
         assert inspected["HostConfig"]["NetworkMode"] == "none"
         assert inspected["NetworkSettings"]["Ports"] == {}
-    assert inspected["Mounts"] == []
-    assert "/var/lib/postgresql/data" in inspected["HostConfig"]["Tmpfs"]
         assert inspected["Config"]["Labels"][SOURCE_CONTAINER_LABEL] == run_label
         if staging_target:
             assert inspected["Config"]["Labels"]["com.livingip.leo.checkpoint"] == "disposable"
         return container_id
+    except BaseException:
+        remove_container_and_volumes(name)
+        raise
 
 
 def _remove_container(name: str) -> None:
-    _docker("rm", "--force", name, check=False)
+    remove_container_and_volumes(name)
 
 
 def _assert_container_absent(name: str) -> None:
@@ -188,8 +197,44 @@ def _assert_private_files(root: Path) -> None:
         assert stat.S_IMODE(path.stat().st_mode) == 0o600, path
 
 
+@pytest.fixture
+def unchanged_docker_volume_set() -> Iterator[None]:
+    volume_guard = acquire_docker_volume_set_guard(
+        context="v3 source rebuild lifecycle PostgreSQL fixture",
+    )
+    try:
+        yield
+    finally:
+        release_docker_volume_set_guard(volume_guard)
+
+
 @pytest.mark.skipif(not v3_contract.docker_available(), reason="Docker daemon is required")
-def test_v3_source_rebuild_lifecycle_is_exact_networkless_and_cleanup_proven(tmp_path: Path) -> None:
+def test_start_postgres_cleans_up_when_post_start_validation_fails(
+    monkeypatch: pytest.MonkeyPatch,
+    unchanged_docker_volume_set: None,
+) -> None:
+    if _docker("image", "inspect", v3_contract.POSTGRES_IMAGE, check=False).returncode != 0:
+        pytest.skip("the pinned PostgreSQL image must already exist; this test never pulls over the network")
+    run_id = uuid.uuid4().hex[:12]
+    container = f"teleo-v3-startup-failure-{run_id}"
+
+    def fail_inspection(_name: str) -> dict:
+        raise AssertionError("forced post-start validation failure")
+
+    monkeypatch.setattr(sys.modules[__name__], "inspect_postgres_tmpfs_container", fail_inspection)
+    try:
+        with pytest.raises(AssertionError, match="forced post-start validation failure"):
+            _start_postgres(container, DATABASE, run_id, staging_target=True)
+        _assert_container_absent(container)
+    finally:
+        remove_container_and_volumes(container)
+
+
+@pytest.mark.skipif(not v3_contract.docker_available(), reason="Docker daemon is required")
+def test_v3_source_rebuild_lifecycle_is_exact_networkless_and_cleanup_proven(
+    tmp_path: Path,
+    unchanged_docker_volume_set: None,
+) -> None:
     if _docker("image", "inspect", v3_contract.POSTGRES_IMAGE, check=False).returncode != 0:
         pytest.skip("the pinned PostgreSQL image must already exist; this test never pulls over the network")
     run_id = uuid.uuid4().hex[:12]