From 9b7a27f2194f7b0b07bbf3181d22cf68d871f947 Mon Sep 17 00:00:00 2001 From: Johnathan Corgan Date: Thu, 23 Jul 2026 17:09:07 +0000 Subject: [PATCH] Claim the chaos network range instead of deriving it, ending the collision Two concurrent ci-local runs could not both run chaos. Each child's subnet came from its position in the suite list, so both runs walked 10.30.0 through 10.30.12 and requested identical ranges; whichever reached the daemon first won and the other died with a pool overlap. A run index or hashed offset would only make that unlikely, and it is precisely the failure being removed. The simulator now claims its range: attempt-create on a candidate, advance on docker's own overlap error, fail loudly on anything else. Docker's address pool becomes the arbiter, so an overlap is impossible rather than improbable. The claim lives in the sim rather than in ci-local because only the process that creates the network can advance on conflict, and because it fixes the bare chaos.sh path too, which a ci-local-only fix would have left broken. The generated compose now declares the network external over the claimed one, and teardown releases the range from both paths, including the setup-failed path where a run that fell over after claiming still holds one. Ordering matters here and is not obvious: node IPs derive from the subnet during topology generation, and traffic shaping keys its filters on those addresses, so the claim happens before the topology exists. Claiming later would give a network on one range and filters on another, which does not fail at bring-up and instead leaves the shaping matching nothing. --subnet survives as an explicit pin for when a known range is wanted, and now fails loudly if that range is taken rather than silently overlapping. ci-local no longer passes it. Validated by running two instances of the same scenario deliberately concurrently: they claimed 10.30.1.0/24 and 10.30.0.0/24, both exited 0 with their assertions passing, and both released their networks. The negative control holds too: with a squatter on a pinned range the run aborts with a message naming the range rather than proceeding. --- testing/chaos/sim/__main__.py | 12 ++-- testing/chaos/sim/compose.py | 22 ++++--- testing/chaos/sim/netclaim.py | 108 ++++++++++++++++++++++++++++++++++ testing/chaos/sim/runner.py | 43 +++++++++++++- testing/chaos/sim/scenario.py | 4 ++ testing/ci-local.sh | 18 +++--- 6 files changed, 185 insertions(+), 22 deletions(-) create mode 100644 testing/chaos/sim/netclaim.py diff --git a/testing/chaos/sim/__main__.py b/testing/chaos/sim/__main__.py index ec1867f..92bbe5a 100644 --- a/testing/chaos/sim/__main__.py +++ b/testing/chaos/sim/__main__.py @@ -27,9 +27,10 @@ def main(): ) parser.add_argument( "--subnet", type=str, default=None, - help="Override topology subnet CIDR (e.g. 10.30.0.0/24); node IPs " - "derive from it. Used by CI to give each parallel run a " - "non-overlapping network.", + help="Pin the topology subnet CIDR (e.g. 10.30.0.0/24) instead of " + "claiming a free one. The sim normally claims a range so " + "concurrent runs cannot collide; pin only when you need a known " + "range, and expect a hard failure if it is already taken.", ) args = parser.parse_args() @@ -55,7 +56,10 @@ def main(): sys.exit(1) scenario.duration_secs = args.duration if args.subnet is not None: - scenario.topology.subnet = args.subnet + # Pinning opts out of claiming. The runner still *creates* the network, + # so a range already in use fails loudly here rather than silently + # overlapping a concurrent run. + scenario.topology.pinned_subnet = args.subnet runner = SimRunner(scenario) result = runner.run() diff --git a/testing/chaos/sim/compose.py b/testing/chaos/sim/compose.py index c27a153..3594ead 100644 --- a/testing/chaos/sim/compose.py +++ b/testing/chaos/sim/compose.py @@ -19,12 +19,12 @@ _COMPOSE_TEMPLATE = Template( """\ networks: fips-net: - driver: bridge - labels: - - "com.corganlabs.fips-ci=1" - ipam: - config: - - subnet: {{ subnet }} + # External, and created by the sim rather than by compose. The range has to + # be *claimed* -- attempt-create, advance on docker's own overlap error -- + # so that two concurrent runs cannot select the same one, and only the + # process that creates the network can do that. See sim/netclaim.py. + external: true + name: {{ network_name }} x-fips-common: &fips-common image: {{ image }} @@ -64,8 +64,14 @@ def generate_compose( topology: SimTopology, scenario: Scenario, output_dir: str, + network_name: str, ) -> str: - """Render docker-compose.yml and write to output_dir. Returns the file path.""" + """Render docker-compose.yml and write to output_dir. Returns the file path. + + ``network_name`` is the docker network the sim has already claimed; the + compose file refers to it as external rather than declaring a subnet, so + that the claim and the creation are the same operation. + """ os.makedirs(output_dir, exist_ok=True) nodes = [topology.nodes[nid] for nid in sorted(topology.nodes)] @@ -77,7 +83,7 @@ def generate_compose( ) content = _COMPOSE_TEMPLATE.render( - subnet=scenario.topology.subnet, + network_name=network_name, rust_log=scenario.logging.rust_log, image=FIPS_SIM_IMAGE, nodes=nodes, diff --git a/testing/chaos/sim/netclaim.py b/testing/chaos/sim/netclaim.py new file mode 100644 index 0000000..ab4294d --- /dev/null +++ b/testing/chaos/sim/netclaim.py @@ -0,0 +1,108 @@ +"""Claim a free /24 for a scenario's network, atomically. + +Two concurrent chaos runs used to request identical ranges: `ci-local.sh` +derived each child's subnet from its position in the suite list, so both runs +walked `10.30.0` through `10.30.12` and collided on every one of them. A run +index or a hashed offset only makes a collision unlikely, and a collision is +precisely the failure being removed, so this claims instead. + +Claim-and-advance: attempt to create the network on a candidate range and, on +docker's own "Pool overlaps" error, move to the next. Docker's address pool is +then the arbiter and an overlap between two concurrent runs is *impossible* +rather than improbable. The pattern is taken from `testing/sidecar/scripts/ +test-sidecar.sh:75-98`, which has been carrying it since 2026-07-19. + +The claim has to happen before the topology is generated, not before the +compose file is written: node IPs derive from the subnet inside +`generate_topology`, and traffic shaping keys its filters on those addresses. +Claiming later yields a network on one range and `tc` filters on another, which +does not fail at bring-up and instead leaves the shaping matching nothing. +""" + +from __future__ import annotations + +import logging +import subprocess + +log = logging.getLogger(__name__) + +# Stay inside 10.30.0.0/16, which ci-local.sh already documents as clear of +# docker's default pool (172.17-31, 192.168) and of the fixed-subnet suites in +# 172.x. Sidecar has claimed 10.40.0.0/16; do not overlap it. +NET_BASE = "10.30" +NET_CANDIDATES = 200 + + +class NetworkClaimError(RuntimeError): + """No range could be claimed, or docker refused for another reason.""" + + +def claim_network( + name: str, + labels: dict[str, str] | None = None, + candidates: list[str] | None = None, +) -> str: + """Create `name` on the first free /24 and return its CIDR. + + `candidates` pins the search to an explicit list, which is how `--subnet` + opts out of claiming. A single pinned candidate that is already taken + raises rather than advancing, so pinning fails loudly instead of silently + overlapping a concurrent run. + + Raises NetworkClaimError if every candidate is taken, or immediately if + docker fails for any reason other than an address-pool conflict. Retrying + a real error 200 times would bury the reason for it. + """ + ranges = candidates or [ + f"{NET_BASE}.{i}.0/24" for i in range(NET_CANDIDATES) + ] + label_args: list[str] = [] + for key, value in (labels or {}).items(): + label_args += ["--label", f"{key}={value}"] + + for subnet in ranges: + proc = subprocess.run( + ["docker", "network", "create", "--subnet", subnet] + + label_args + + [name], + capture_output=True, + text=True, + ) + if proc.returncode == 0: + log.info("Claimed network %s on %s", name, subnet) + return subnet + err = (proc.stderr or "") + (proc.stdout or "") + if "ool overlaps" in err: + continue + raise NetworkClaimError( + f"docker network create {name} on {subnet} failed: {err.strip()}" + ) + + if candidates: + raise NetworkClaimError( + f"pinned subnet(s) {', '.join(candidates)} already in use; " + f"another run holds the range" + ) + raise NetworkClaimError( + f"no free /24 in {NET_BASE}.0.0/16 after {NET_CANDIDATES} attempts" + ) + + +def remove_network(name: str) -> None: + """Remove a claimed network, tolerating its absence. + + The network is `external:` to the generated compose file, so `compose down` + leaves it behind and this is the only thing that reclaims the range. + Failures are logged rather than raised: teardown runs on the failure path + too, and losing a /24 is a leak, not a reason to mask the original error. + """ + proc = subprocess.run( + ["docker", "network", "rm", name], + capture_output=True, + text=True, + ) + if proc.returncode != 0: + err = ((proc.stderr or "") + (proc.stdout or "")).strip() + if "not found" in err or "No such network" in err: + return + log.warning("Could not remove network %s: %s", name, err) diff --git a/testing/chaos/sim/runner.py b/testing/chaos/sim/runner.py index 182a252..ee9c8c6 100644 --- a/testing/chaos/sim/runner.py +++ b/testing/chaos/sim/runner.py @@ -30,6 +30,7 @@ from .link_swap import LinkSwapManager from .links import LinkManager from .logs import AnalysisResult, analyze_logs, collect_logs, write_sim_metadata from .naming import name_suffix +from .netclaim import claim_network, remove_network from .netem import NetemManager from .nodes import NodeManager from .peer_churn import PeerChurnManager @@ -47,6 +48,9 @@ class SimRunner: self.rng = random.Random(scenario.seed) self.topology: SimTopology | None = None self.compose_file: str | None = None + # Claimed in _setup; the compose file refers to it as external, so + # `compose down` does not remove it and teardown must. + self.network_name: str | None = None self.output_dir: str = self._resolve_output_dir(scenario) self._interrupted = False @@ -185,6 +189,28 @@ class SimRunner: logging.getLogger().addHandler(fh) log.info("Runner log: %s", runner_log_path) + # 0. Claim this run's network range, before anything derives from it. + # + # The ordering is not obvious and it matters: node IPs are computed + # from the subnet inside generate_topology, and traffic shaping keys + # its filters on those addresses. Claiming after the topology exists + # would give a network on one range and `tc` filters on another, which + # does not fail at bring-up — it silently leaves the shaping matching + # nothing. + self.network_name = f"fips-sim{name_suffix()}-net" + s.topology.subnet = claim_network( + self.network_name, + labels={ + "com.corganlabs.fips-ci": "1", + "com.corganlabs.fips-ci.run": os.environ.get( + "FIPS_CI_RUN_ID", "manual" + ), + }, + candidates=( + [s.topology.pinned_subnet] if s.topology.pinned_subnet else None + ), + ) + # 1. Generate topology log.info( "Generating %d-node %s topology (seed=%d)...", @@ -237,7 +263,9 @@ class SimRunner: log.info("Wrote node configs to %s", config_dir) # 3. Generate docker-compose.yml - self.compose_file = generate_compose(self.topology, self.scenario, config_dir) + self.compose_file = generate_compose( + self.topology, self.scenario, config_dir, self.network_name + ) log.info("Wrote %s", self.compose_file) # 4. Build the test image once (avoids per-service build at scale) @@ -521,6 +549,17 @@ class SimRunner: except Exception: log.exception("Could not write status file") + def _release_network(self) -> None: + """Give this run's claimed /24 back. + + The compose file declares the network `external:`, so `compose down` + leaves it alone and nothing else reclaims the range. Called from both + teardown paths, including the setup-failed one, because a run that + fell over after claiming still holds a range. + """ + if self.network_name: + remove_network(self.network_name) + def _teardown(self) -> AnalysisResult | None: """Stop dynamic elements, collect logs, analyze, stop containers.""" if not self._containers_started: @@ -536,6 +575,7 @@ class SimRunner: # containers or a network even with no mesh to speak of. log.info("Stopping containers...") docker_compose(self.compose_file, ["down"], check=False) + self._release_network() return None result = None @@ -701,6 +741,7 @@ class SimRunner: ["down"], check=False, ) + self._release_network() return result diff --git a/testing/chaos/sim/scenario.py b/testing/chaos/sim/scenario.py index 8f03f83..eb2e66e 100644 --- a/testing/chaos/sim/scenario.py +++ b/testing/chaos/sim/scenario.py @@ -52,6 +52,10 @@ class TopologyConfig: # unreachable. Set false where election from an arbitrary key distribution # is itself the subject. pin_root: bool = True + # Set by --subnet to opt out of claiming a free range. Not a scenario-file + # key: a scenario that hardcoded its range would reintroduce the collision + # claiming exists to remove. + pinned_subnet: str | None = None @dataclass diff --git a/testing/ci-local.sh b/testing/ci-local.sh index a6c52b7..5bd9c99 100755 --- a/testing/ci-local.sh +++ b/testing/ci-local.sh @@ -905,7 +905,6 @@ run_integration() { local pids=() local suite_names=() local running=0 - local chaos_idx=0 for entry in "${CHAOS_SUITES[@]}"; do # Parse: "display-name scenario [flags...]" @@ -913,14 +912,15 @@ run_integration() { local name="${parts[0]}" local args=("${parts[@]:1}") - # Give each chaos child a unique, non-overlapping /24 in 10.30.x so - # parallel children never collide with each other, and so a chaos - # net can never swallow a fixed-subnet suite (sidecar/static/nat in - # 172.x). 10.30.x sits outside docker's default-address-pool range - # (172.17-31 / 192.168), so auto-assigned nets can't land on it - # either. Node IPs derive from this subnet inside the sim. - args+=("--subnet" "10.30.${chaos_idx}.0/24") - chaos_idx=$((chaos_idx + 1)) + # No --subnet: the sim claims a free /24 itself (sim/netclaim.py). + # This used to pass 10.30.${chaos_idx}.0/24, which uniquified the + # children of ONE run against each other and was byte-identical + # between runs -- so two concurrent ci-local runs both walked + # 10.30.0 through 10.30.12 and collided on every one. A claim makes + # the daemon the arbiter, so an overlap is impossible rather than + # merely unlikely. The range still sits in 10.30.0.0/16, clear of + # docker's default pool (172.17-31 / 192.168) and of the + # fixed-subnet suites in 172.x. # Throttle: wait for a slot while [[ $running -ge $PARALLEL_JOBS ]]; do