Check that teardown and node stops actually did what they report

Two commands in the chaos harness were trusted without being checked.

docker compose down exits 0 while leaving this run's containers alive, so
a partly-failed bring-up leaked named containers with nothing to detect
it. Teardown now asks whether the containers this run owns are gone. A
survivor that the forced removal clears is a warning, since nothing
outlives the run; one that survives that, or a query that could not run
at all, aborts and writes the names to an artifact. The check is scoped
to this run's own names, so a concurrent run cannot trip it.

Node churn marked a node down whether or not docker stop worked: the
return code was never inspected and the captured output was discarded.
The simulation's model of the mesh then diverged from reality, and
nodes_down, the max_down_nodes cap and the connectivity guard are all
computed from that model, so the guard written to prevent a partition
could cause one. A failed stop now warns, carries the daemon's own
message, and leaves the node out of the down set, which is the part that
matters: the next churn tick simply retries against an honest model.

Detection and remedy are kept separate in the code so a later reader can
tell which is which.

testing/chaos/sim/docker_exec.py

@@ -133,3 +133,54 @@ def is_container_running(container: str) -> bool:
         return result.returncode == 0 and result.stdout.strip() == "true"
     except subprocess.TimeoutExpired:
         return False
+
+
+def existing_containers(names: list[str], timeout: int = 30) -> list[str] | None:
+    """Return which of `names` docker still knows about, running or not.
+
+    Deliberately scoped to the names the caller passes in. Enumerating by
+    compose project label would also sweep up whatever a concurrent run or
+    a different project owns, and under the parallel CI matrix that turns a
+    leak check into a flake generator.
+
+    Returns `None` when docker could not be asked -- a query that failed has
+    observed nothing, and reporting "no survivors" for it would recreate the
+    silence this check exists to break.
+    """
+    try:
+        result = subprocess.run(
+            ["docker", "ps", "-a", "--format", "{{.Names}}"],
+            capture_output=True,
+            text=True,
+            timeout=timeout,
+        )
+    except subprocess.TimeoutExpired:
+        log.error("docker ps timed out after %ds", timeout)
+        return None
+    if result.returncode != 0:
+        log.error("docker ps exited %d\nstderr: %s", result.returncode, _tail(result.stderr))
+        return None
+    present = set(result.stdout.split())
+    return [name for name in names if name in present]
+
+
+def force_remove(names: list[str], timeout: int = 60) -> None:
+    """Remove the named containers outright, whatever state they are in.
+
+    Best effort and never raises: this runs on a teardown path that has
+    already reported a leak, and the report is the part that must survive.
+    """
+    if not names:
+        return
+    try:
+        result = subprocess.run(
+            ["docker", "rm", "-f"] + names,
+            capture_output=True,
+            text=True,
+            timeout=timeout,
+        )
+    except subprocess.TimeoutExpired:
+        log.error("docker rm -f timed out after %ds", timeout)
+        return
+    if result.returncode != 0:
+        log.error("docker rm -f exited %d\nstderr: %s", result.returncode, _tail(result.stderr))

testing/chaos/sim/nodes.py

@@ -104,17 +104,34 @@ class NodeManager:
                 self._start_node(nid)
 
     def _stop_node(self, node_id: str, duration: float):
-        """Stop a container."""
+        """Stop a container, and record it as down only if it stopped.
+
+        A `docker stop` that exits non-zero -- the container already gone,
+        the daemon refusing -- once left the node marked down anyway. Every
+        figure the scenario reasons with is derived from that mark:
+        `down_count` and so the `max_down_nodes` cap, `_would_disconnect`
+        and so the `protect_connectivity` guard, and the shared
+        `down_nodes` set that netem, links and traffic all skip. Returning
+        before the mutation keeps the model equal to the mesh and lets the
+        next churn tick retry the node. See ISSUE-2026-0081.
+        """
         container = self.topology.container_name(node_id)
         docker_exec_quiet(container, "kill 1", timeout=5)  # SIGTERM to PID 1
         # Use docker stop with a short grace period
         import subprocess
 
-        subprocess.run(
+        result = subprocess.run(
             ["docker", "stop", "-t", "2", container],
             capture_output=True,
+            text=True,
             timeout=15,
         )
+        if result.returncode != 0:
+            log.warning(
+                "Failed to stop %s: %s; leaving %s marked up",
+                container, result.stderr.strip(), node_id,
+            )
+            return
 
         now = time.time()
         state = self.node_states[node_id]

testing/chaos/sim/runner.py

@@ -25,7 +25,7 @@ from .assertions import (
 from .compose import generate_compose
 from .config_gen import write_configs
 from .control import snapshot_all_congestion, snapshot_all_mmp, snapshot_all_trees
-from .docker_exec import docker_compose
+from .docker_exec import docker_compose, existing_containers, force_remove
 from .link_swap import LinkSwapManager
 from .links import LinkManager
 from .logs import AnalysisResult, analyze_logs, collect_logs, write_sim_metadata
@@ -569,6 +569,85 @@ class SimRunner:
         except Exception:
             log.exception("Could not write status file")
 
+    def _own_containers(self) -> list[str]:
+        """Name every container this run asked compose to create.
+
+        Empty before the topology exists, which is the only window in which
+        a teardown can run with nothing of its own on the host.
+        """
+        if not self.topology:
+            return []
+        return [self.topology.container_name(nid) for nid in self.topology.nodes]
+
+    def _stop_mesh(self) -> None:
+        """Take the containers down, then check that they actually went.
+
+        `docker compose down` exits 0 while leaving containers behind when it
+        races an `up -d` that failed part way through: compose enumerates the
+        project before the stragglers have registered, finds nothing to
+        remove, and says so successfully. Nothing downstream looks at what
+        survived, so the leak is invisible at the one moment it is cheap to
+        see. See ISSUE-2026-0079.
+        """
+        log.info("Stopping containers...")
+        docker_compose(self.compose_file, ["down"], check=False)
+        self._check_teardown()
+
+    def _check_teardown(self) -> None:
+        """Report, then clear, any of this run's containers that outlived `down`.
+
+        Scoped to names this run generated. A check that reasoned about the
+        compose project label, or about container names in general, would
+        answer for whatever a concurrent scenario happens to own, and the CI
+        matrix runs plenty of those at once.
+        """
+        wanted = self._own_containers()
+        if not wanted:
+            return
+
+        survivors = existing_containers(wanted)
+        if survivors is None:
+            # Detection failed, which is not the same as detecting nothing.
+            log.error("Could not ask docker what survived `down`; leak undetectable")
+            return
+        if not survivors:
+            return
+        log.warning(
+            "`compose down` exited 0 but left %d of this run's %d containers: %s",
+            len(survivors), len(wanted), ", ".join(survivors),
+        )
+
+        # Remedy, kept apart from the detection above on purpose: deleting
+        # everything from here down leaves the report intact.
+        force_remove(survivors)
+        leaked = existing_containers(survivors)
+        if not leaked:
+            return
+        # Either docker could not be asked a second time, or the containers
+        # are still there after a forced removal. Neither is the transient
+        # race, and neither clears itself, so this is the run's verdict.
+        detail = "unknown" if leaked is None else ", ".join(leaked)
+        log.error("Containers survived forced removal, leaked to the host: %s", detail)
+        self._write_leak(leaked if leaked is not None else wanted)
+        self.aborted = True
+
+    def _write_leak(self, names: list[str]) -> None:
+        """Record the leaked names beside the run's other artifacts.
+
+        Never raises, for the same reason `_write_status` does not: this runs
+        on a teardown path that has already gone wrong. Written alongside
+        status.txt rather than into it: the status is the simulation's own
+        outcome, and a leak on the way out does not retract it.
+        """
+        try:
+            os.makedirs(self.output_dir, exist_ok=True)
+            path = os.path.join(self.output_dir, "leaked-containers.txt")
+            with open(path, "w") as f:
+                for name in names:
+                    f.write(name + "\n")
+        except Exception:
+            log.exception("Could not write leaked-containers file")
+
     def _release_network(self) -> None:
         """Give this run's claimed /24 back.
 
@@ -593,8 +672,7 @@ class SimRunner:
             if self.compose_file:
                 # `up -d` can fail part way through, so this run may own
                 # containers or a network even with no mesh to speak of.
-                log.info("Stopping containers...")
-                docker_compose(self.compose_file, ["down"], check=False)
+                self._stop_mesh()
             self._release_network()
             return None
 
@@ -755,12 +833,7 @@ class SimRunner:
             # Status first: it is the one artifact that must exist whatever
             # else happens, and stopping containers can still time out.
             self._write_status(status)
-            log.info("Stopping containers...")
-            docker_compose(
-                self.compose_file,
-                ["down"],
-                check=False,
-            )
+            self._stop_mesh()
             self._release_network()
 
         return result