From 4f55a281ac9812c03799ff4007f1cad0790f0a23 Mon Sep 17 00:00:00 2001 From: Johnathan Corgan Date: Wed, 22 Jul 2026 07:37:48 +0000 Subject: [PATCH] Let a failed chaos scenario fail the run The simulation caught every exception a scenario raised, logged it, and exited 0, so a scenario that died during setup was recorded as a pass. Local CI discarded the exit code in any case: run_chaos ends in a call whose own last statement is an echo, so it returned 0 whatever the scenario did, and the parallel launcher's wait therefore always succeeded and always recorded a pass. Between them no chaos failure of any kind could turn a local run red, and none could since the script was written. Carry the abort out of the run and give it its own exit code, ordered ahead of the two content codes so a run that never produced a mesh cannot report that mesh's panic and assertion counts. Return the scenario's status from run_chaos so the launcher sees it. The documented exit codes were stale before this change and are rewritten in full, including that the argument parser rejects a malformed command line with the same code that reports panics. Expect scenarios that have been reporting green to start reporting red. In the last recorded local run twelve of the thirteen scenarios never got past starting their containers, and all thirteen were counted as passes; those that now execute have not run for months, and their assertions have never been evaluated against a mesh of their own. --- testing/chaos/README.md | 16 +++++++++++++++- testing/chaos/sim/__main__.py | 5 +++++ testing/chaos/sim/runner.py | 10 ++++++++++ testing/ci-local.sh | 11 +++++++++++ 4 files changed, 41 insertions(+), 1 deletion(-) diff --git a/testing/chaos/README.md b/testing/chaos/README.md index 1a52191..7d21a1d 100644 --- a/testing/chaos/README.md +++ b/testing/chaos/README.md @@ -262,7 +262,21 @@ Results written to `sim-results/` (configurable via - `runner.log` -- Orchestration events (topology, netem, churn, traffic) with timestamps - `fips-node-nXX.log` -- Per-node log output -Exit code 0 on success, 2 if panics detected. +Exit codes: + +- `0` -- Ran to completion, no panics, every assertion passed +- `1` -- The scenario file could not be loaded, teardown itself raised, or a + second interrupt arrived while the first was being handled +- `2` -- Panics found in the collected node logs. Also what the argument + parser exits with when it rejects the command line, before any run starts +- `3` -- A post-run assertion failed +- `4` -- Setup, warmup or the simulation loop raised, so the run did not + complete; `runner.log` carries the traceback + +Codes 2 and 3 describe what a mesh that ran did. Code 4 says there is +nothing to describe, and takes precedence over both. Code 2 is dual-use: +a run that never started cannot have panicked, so read it together with +whether `runner.log` exists. ## Creating Custom Scenarios diff --git a/testing/chaos/sim/__main__.py b/testing/chaos/sim/__main__.py index ba4f2b8..ec1867f 100644 --- a/testing/chaos/sim/__main__.py +++ b/testing/chaos/sim/__main__.py @@ -60,6 +60,11 @@ def main(): runner = SimRunner(scenario) result = runner.run() + # Liveness before content. A simulation that raised on its way up, or part + # way through, has no panic count or assertion outcome worth reporting, and + # saying so is more useful than whatever partial content it did produce. + if runner.aborted: + sys.exit(4) if result and result.panics: sys.exit(2) if runner.assertions_failed: diff --git a/testing/chaos/sim/runner.py b/testing/chaos/sim/runner.py index 056f359..61f2ce1 100644 --- a/testing/chaos/sim/runner.py +++ b/testing/chaos/sim/runner.py @@ -40,6 +40,11 @@ class SimRunner: self.output_dir: str = self._resolve_output_dir(scenario) self._interrupted = False + # Set when setup, warmup or the simulation loop raises. The exception + # is logged rather than propagated, so nothing else on the return path + # of run() carries the fact that the simulation did not complete. + self.aborted = False + # Shared set of currently-down node IDs (updated by NodeManager, # read by NetemManager, LinkManager, TrafficManager) self._down_nodes: set[str] = set() @@ -82,7 +87,12 @@ class SimRunner: self._warmup() self._simulation_loop() except Exception: + # Log rather than re-raise: the default excepthook writes to stderr + # and would not reach the file handler installed in _setup(), so a + # re-raise would lose the traceback from runner.log, which is the + # artifact that survives into CI. The flag carries the abort out. log.exception("Simulation failed") + self.aborted = True finally: result = self._teardown() diff --git a/testing/ci-local.sh b/testing/ci-local.sh index 688608d..ffdb60d 100755 --- a/testing/ci-local.sh +++ b/testing/ci-local.sh @@ -566,6 +566,17 @@ run_chaos() { fi record "chaos-$name" $rc + + # record() ends in pass()/fail(), which are echoes, so it returns 0 for any + # rc — and without this return so does run_chaos. On the parallel path the + # function's status is the child subshell's status, and that is the only + # thing the waiting parent can see: the child's own RESULTS entry and its + # OVERALL=1 die with its process, and its FAIL line goes to a logfile only + # the unreachable branch reads. So a bare record() left every scenario + # recorded as a pass whatever the simulation reported. On the --only path + # record() already wrote the true rc into the parent's own RESULTS, and + # returning it as well is inert: this script does not set -e. + return $rc } # Run gateway integration test