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