mirror of
https://github.com/jmcorgan/fips.git
synced 2026-07-30 19:46:15 +00:00
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.
This commit is contained in:
@@ -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()
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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)
|
||||
@@ -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
|
||||
|
||||
|
||||
@@ -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
|
||||
|
||||
+9
-9
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user