mirror of
https://github.com/jmcorgan/fips.git
synced 2026-10-05 11:08:25 +00:00
fix(testing): claim the medium-change lab's subnets instead of pinning them
The suite pinned three fixed /24s — 172.31.60, .61 and .62. Compose
project names are unique per run, so two concurrent runs got distinct
container and network *names*, but the address pools are constants and
both runs asked for the same ones. Whichever created a network first
won; the other died at topology start with
failed to create network ..._mc-far: invalid pool request:
Pool overlaps with other one on this address space
having tested nothing. Pushing maint, master and next within a second of
each other is enough to hit it, and the branch that loses looks broken
when it is fine.
The three `MC_*_PREFIX` overrides existed from the start but nothing
ever set them, so the defaults were the only values ever used.
This does what the nat suite already does. A free /24 per network is
claimed under 10.42.0.0/16 before the lab starts, with docker's own
`network create` as the atomic arbiter of who owns what — the run-id
derived offset is deliberately not used here for the same reason it was
rejected there: it makes a collision unlikely rather than impossible,
and a collision is the failure being removed. Both CI labels are stamped
so ci-cleanup.sh's label sweep recovers the networks when a run is
SIGKILLed, which no inline removal can cover.
Since the claim creates the networks, compose has to attach rather than
create, so `docker-compose.external-net.yml` declares the three
external, applied through a new `MC_EXTRA_COMPOSE` hook. Nothing else
sets it: the GitHub matrix runs one job per runner and invokes
`test.sh` directly, and a bare `docker compose up` is a single lab, so
both keep the fixed defaults and the addresses in the README stay
literal. The hook appends to the whole COMPOSE array rather than to the
`up` alone, so teardown addresses the same project — a `down` without
the overlay would not know the networks are external.
The claim lives in ci-local.sh rather than the suite script for the
reason the nat comment gives: the workflow invokes the script directly
and tears down with the base file only, and does not want a claim.
Release does a `compose down` before removing the networks. That order
is load-bearing rather than tidy: `docker network rm` silently no-ops on
a network that still has endpoints attached and reports success, so
without the `down` the removal fails exactly on the path it exists for.
A network left behind would not merely leak — the next invocation in the
run would hit `network with name ... already exists`, which is not a
pool overlap, so the allocator correctly refuses to advance and fails.
Also gives the suite its own compose project. It set none, so it
inherited whichever COMPOSE_PROJECT_NAME the previous suite exported —
which is why the failure named a medium-change network under the nat
project, `fipsci_<runid>_nat_mc-far`. That is not what caused the
overlap, but filing one suite's resources under another's project is a
teardown hazard: `down --remove-orphans` on either would consider the
other's containers orphans.
The /24 claim loop is now shared rather than copied a third time.
`ci_claim_nat_net` keeps its name, its `[nat]` log tag and its exported
prefix, and becomes a two-line caller.
Verified rather than assumed. The lab runs on the claimed prefixes, not
just alongside them: `node-b re-pinned to 10.42.1.10:2121`. With the
first three candidates occupied by squatter networks — the collision
path itself — the allocator advances to 10.42.3/4/5 and the suite passes
on those. Run standalone with no overlay it still renders 172.31.6x and
passes 8/8, so the workflow path is untouched. `nat-cone` passes and
still claims 10.41.0/1, so the shared loop did not disturb it. Networks
are gone after each run. `ci-local.sh --only medium-change` is 17/17.
Not exercised: the partial-claim rollback, which needs a /16 exhausted
part-way to reach. It mirrors ci_claim_nat_networks' shape.
This commit is contained in:
+106
-11
@@ -817,33 +817,52 @@ run_firewall() {
|
||||
# labels are stamped so ci-cleanup.sh's label sweep can recover the networks
|
||||
# when a run is SIGKILLed, which no inline removal can cover.
|
||||
CI_NAT_NET_BASE="10.41"
|
||||
CI_NAT_NET_CANDIDATES=256
|
||||
# 10.42.0.0/16 for the medium-change lab, on the same reasoning as 10.41 above
|
||||
# and adjacent to it so the two stay legible as a pair. Its three bridges are
|
||||
# claimed per suite invocation exactly as the nat pair are; before that they
|
||||
# were three fixed /24s under 172.31.6x, which two concurrent runs both
|
||||
# requested and the second lost with `Pool overlaps`.
|
||||
CI_MC_NET_BASE="10.42"
|
||||
CI_V4_NET_CANDIDATES=256
|
||||
CI_NAT_CLAIMED_PREFIX=""
|
||||
CI_CLAIMED_V4_PREFIX=""
|
||||
|
||||
# Claim one free /24 for `net` by walking candidates under `base` and letting
|
||||
# docker's own create be the atomic arbiter. Sets CI_CLAIMED_V4_PREFIX.
|
||||
#
|
||||
# Deliberately does NOT discard stderr: only an address-pool conflict is worth
|
||||
# advancing on. Any other failure is real, and burning through 256 candidates
|
||||
# would bury the reason.
|
||||
ci_claim_nat_net() {
|
||||
local net="$1" i err
|
||||
CI_NAT_CLAIMED_PREFIX=""
|
||||
for (( i = 0; i < CI_NAT_NET_CANDIDATES; i++ )); do
|
||||
#
|
||||
# `tag` is the suite word for the log lines only. Both labels are stamped so
|
||||
# ci-cleanup.sh's label sweep can recover the network when a run is SIGKILLed,
|
||||
# which no inline removal can cover.
|
||||
ci_claim_v4_net() {
|
||||
local net="$1" base="$2" tag="$3" i err
|
||||
CI_CLAIMED_V4_PREFIX=""
|
||||
for (( i = 0; i < CI_V4_NET_CANDIDATES; i++ )); do
|
||||
if err=$(docker network create \
|
||||
--subnet "${CI_NAT_NET_BASE}.${i}.0/24" \
|
||||
--subnet "${base}.${i}.0/24" \
|
||||
--label "$CI_LABEL" --label "$CI_LABEL_RUN" \
|
||||
"$net" 2>&1); then
|
||||
CI_NAT_CLAIMED_PREFIX="${CI_NAT_NET_BASE}.${i}"
|
||||
info "[nat] Claimed $net on ${CI_NAT_CLAIMED_PREFIX}.0/24"
|
||||
CI_CLAIMED_V4_PREFIX="${base}.${i}"
|
||||
info "[$tag] Claimed $net on ${CI_CLAIMED_V4_PREFIX}.0/24"
|
||||
return 0
|
||||
fi
|
||||
case "$err" in
|
||||
*"Pool overlaps"*|*"pool overlaps"*) continue ;;
|
||||
*) fail "[nat] docker network create: $err"; return 1 ;;
|
||||
*) fail "[$tag] docker network create: $err"; return 1 ;;
|
||||
esac
|
||||
done
|
||||
fail "[nat] no free /24 in ${CI_NAT_NET_BASE}.0.0/16 after ${CI_NAT_NET_CANDIDATES} attempts"
|
||||
fail "[$tag] no free /24 in ${base}.0.0/16 after ${CI_V4_NET_CANDIDATES} attempts"
|
||||
return 1
|
||||
}
|
||||
|
||||
ci_claim_nat_net() {
|
||||
ci_claim_v4_net "$1" "$CI_NAT_NET_BASE" nat || return 1
|
||||
CI_NAT_CLAIMED_PREFIX="$CI_CLAIMED_V4_PREFIX"
|
||||
}
|
||||
|
||||
# Claim both lab networks and export the prefixes every lab address derives
|
||||
# from. A partial claim is rolled back here, because nothing downstream will
|
||||
# run to release it.
|
||||
@@ -890,6 +909,63 @@ ci_release_nat_networks() {
|
||||
return 0
|
||||
}
|
||||
|
||||
# Claim all three medium-change lab networks and export both the names the
|
||||
# overlay attaches to and the prefixes every lab address derives from. A
|
||||
# partial claim is rolled back here, because nothing downstream will run to
|
||||
# release it.
|
||||
#
|
||||
# Three, not two: the lab's whole design rests on node-b being off-link, so the
|
||||
# route to it follows node-a's default route. A mix of claimed and compose-made
|
||||
# bridges would still come up, with the wrong topology and no error.
|
||||
ci_claim_mc_networks() {
|
||||
unset MC_PRIMARY_PREFIX MC_SECONDARY_PREFIX MC_FAR_PREFIX
|
||||
export FIPS_MC_PRIMARY_NET="fips-mc-primary${FIPS_CI_NAME_SUFFIX:-}"
|
||||
export FIPS_MC_SECONDARY_NET="fips-mc-secondary${FIPS_CI_NAME_SUFFIX:-}"
|
||||
export FIPS_MC_FAR_NET="fips-mc-far${FIPS_CI_NAME_SUFFIX:-}"
|
||||
|
||||
ci_claim_v4_net "$FIPS_MC_PRIMARY_NET" "$CI_MC_NET_BASE" medium-change || return 1
|
||||
local primary="$CI_CLAIMED_V4_PREFIX"
|
||||
|
||||
if ! ci_claim_v4_net "$FIPS_MC_SECONDARY_NET" "$CI_MC_NET_BASE" medium-change; then
|
||||
docker network rm "$FIPS_MC_PRIMARY_NET" >/dev/null 2>&1 || true
|
||||
return 1
|
||||
fi
|
||||
local secondary="$CI_CLAIMED_V4_PREFIX"
|
||||
|
||||
if ! ci_claim_v4_net "$FIPS_MC_FAR_NET" "$CI_MC_NET_BASE" medium-change; then
|
||||
docker network rm "$FIPS_MC_SECONDARY_NET" >/dev/null 2>&1 || true
|
||||
docker network rm "$FIPS_MC_PRIMARY_NET" >/dev/null 2>&1 || true
|
||||
return 1
|
||||
fi
|
||||
|
||||
export MC_PRIMARY_PREFIX="$primary"
|
||||
export MC_SECONDARY_PREFIX="$secondary"
|
||||
export MC_FAR_PREFIX="$CI_CLAIMED_V4_PREFIX"
|
||||
}
|
||||
|
||||
# Release all three claimed networks. Left behind they would not merely leak:
|
||||
# the next medium-change invocation in this run would hit `network with name
|
||||
# ... already exists`, which is not a pool overlap, so the allocator correctly
|
||||
# refuses to advance and fails.
|
||||
#
|
||||
# The `down` before the removals is load-bearing, not tidiness. `docker network
|
||||
# rm` silently no-ops on a network that still has endpoints attached and
|
||||
# reports success, and the suite leaves its containers up on some paths. Without
|
||||
# the `down` the removal fails to remove exactly when it matters. The overlay is
|
||||
# included so the `down` addresses the same project the `up` created.
|
||||
ci_release_mc_networks() {
|
||||
docker compose \
|
||||
-f testing/medium-change/docker-compose.yml \
|
||||
-f testing/medium-change/docker-compose.external-net.yml \
|
||||
down --volumes --remove-orphans >/dev/null 2>&1 || true
|
||||
local net
|
||||
for net in "${FIPS_MC_PRIMARY_NET:-}" "${FIPS_MC_SECONDARY_NET:-}" \
|
||||
"${FIPS_MC_FAR_NET:-}"; do
|
||||
[[ -n "$net" ]] && { docker network rm "$net" >/dev/null 2>&1 || true; }
|
||||
done
|
||||
return 0
|
||||
}
|
||||
|
||||
# The overlay pointing compose at the claimed networks. APPENDED to any
|
||||
# caller-supplied chain rather than replacing it: mesh-lab/run-loop.sh sets
|
||||
# FIPS_NAT_EXTRA_COMPOSE for its trace overlay, and overwriting would silently
|
||||
@@ -997,12 +1073,31 @@ run_native_api() {
|
||||
# Owns its own compose project and its own three bridges, so it neither
|
||||
# shares container names with the NAT lab nor has to run after it.
|
||||
run_medium_change() {
|
||||
# Its own project, like every other suite. Without this the lab inherited
|
||||
# whichever COMPOSE_PROJECT_NAME the previous suite exported, so its
|
||||
# containers and networks were filed under that suite's project — visible
|
||||
# in a failure as `fipsci_<runid>_nat_mc-far`, a medium-change network
|
||||
# under the nat project.
|
||||
local -x COMPOSE_PROJECT_NAME="$(ci_project medium-change)"
|
||||
|
||||
# Claim before generate-configs: the suite renders every address in the
|
||||
# compose file, the node configs and its own assertions from these
|
||||
# prefixes, so all of them must see the claimed values.
|
||||
info "[medium-change] Claiming lab networks"
|
||||
if ! ci_claim_mc_networks; then
|
||||
record "medium-change" 1
|
||||
return
|
||||
fi
|
||||
|
||||
info "[medium-change] Running transport-medium change test"
|
||||
if FIPS_TEST_IMAGE="$CI_IMAGE_TEST" bash testing/medium-change/scripts/test.sh 2>&1; then
|
||||
if MC_EXTRA_COMPOSE="testing/medium-change/docker-compose.external-net.yml" \
|
||||
FIPS_TEST_IMAGE="$CI_IMAGE_TEST" \
|
||||
bash testing/medium-change/scripts/test.sh 2>&1; then
|
||||
record "medium-change" 0
|
||||
else
|
||||
record "medium-change" 1
|
||||
fi
|
||||
ci_release_mc_networks
|
||||
}
|
||||
|
||||
# Run dns-resolver harness (multi-distro + e2e scenarios)
|
||||
|
||||
@@ -69,6 +69,27 @@ claim, not a test, and the claim is cheap to make and expensive to trust.
|
||||
| `MC_PRIMARY_PREFIX` | `172.31.60` | first access path `/24` |
|
||||
| `MC_SECONDARY_PREFIX` | `172.31.61` | second access path `/24` |
|
||||
| `MC_FAR_PREFIX` | `172.31.62` | far segment `/24` |
|
||||
| `MC_EXTRA_COMPOSE` | unset | extra compose overlays, colon-separated |
|
||||
|
||||
The gap budget sits far below the 30 s liveness timeout on purpose: a pass
|
||||
must mean the move was absorbed, not that the reaper was quick.
|
||||
|
||||
## Running two of these at once
|
||||
|
||||
The three prefixes above are fixed defaults, and two runs that both take them
|
||||
do not both get them: docker refuses the second with `Pool overlaps with other
|
||||
one on this address space`, and the suite fails at topology start having tested
|
||||
nothing. The compose project name is unique per run, so the containers and the
|
||||
networks get distinct *names* — it is only the address pools that are shared.
|
||||
|
||||
`testing/ci-local.sh` avoids that by claiming a free `/24` per network under
|
||||
`10.42.0.0/16` before it starts the lab, letting docker's own `network create`
|
||||
be the arbiter of who owns what, and pointing compose at the result with
|
||||
`docker-compose.external-net.yml` via `MC_EXTRA_COMPOSE`. It exports the
|
||||
claimed prefixes as the three `MC_*_PREFIX` variables, so everything the suite
|
||||
renders from them follows.
|
||||
|
||||
Nothing else applies that overlay. The GitHub matrix runs one job per runner
|
||||
and the README invocation above is a single lab, so both keep the fixed
|
||||
defaults and the addresses in this document stay literal. If you want to run
|
||||
two by hand on one host, set the three prefixes yourself.
|
||||
|
||||
@@ -0,0 +1,32 @@
|
||||
# Override: attach the medium-change lab's three bridges to pre-created
|
||||
# external networks instead of letting compose create them from the base
|
||||
# file's fixed pins.
|
||||
#
|
||||
# Applied only by ci-local.sh's run_medium_change, which claims a free /24 per
|
||||
# network per suite invocation (ci_claim_mc_networks) and exports
|
||||
# FIPS_MC_PRIMARY_NET / FIPS_MC_SECONDARY_NET / FIPS_MC_FAR_NET alongside
|
||||
# MC_PRIMARY_PREFIX / MC_SECONDARY_PREFIX / MC_FAR_PREFIX before `up`. That is
|
||||
# what makes two concurrent local runs collision-safe: each claims distinct
|
||||
# ranges, so neither requests address space the other holds.
|
||||
#
|
||||
# The GitHub matrix, the README invocation and any standalone
|
||||
# `docker compose up` do NOT apply this overlay; they use the base file's
|
||||
# normal networks with the `:-` defaults, which render today's exact
|
||||
# addresses. This mirrors nat/docker-compose.external-net.yml,
|
||||
# static/docker-compose.gateway-external-net.yml and
|
||||
# sidecar/docker-compose.external-net.yml.
|
||||
#
|
||||
# One file carries all three: they are claimed and released together, and
|
||||
# splitting them would let the lab attach to a mix of claimed and compose-made
|
||||
# bridges — which is the state that produces a half-addressed topology rather
|
||||
# than a clean failure.
|
||||
networks:
|
||||
mc-primary:
|
||||
external: true
|
||||
name: ${FIPS_MC_PRIMARY_NET:-fips-mc-primary}
|
||||
mc-secondary:
|
||||
external: true
|
||||
name: ${FIPS_MC_SECONDARY_NET:-fips-mc-secondary}
|
||||
mc-far:
|
||||
external: true
|
||||
name: ${FIPS_MC_FAR_NET:-fips-mc-far}
|
||||
@@ -36,6 +36,23 @@ NODE_B="fips-mc-node-b${FIPS_CI_NAME_SUFFIX:-}"
|
||||
|
||||
COMPOSE=(docker compose -f "$MC_DIR/docker-compose.yml")
|
||||
|
||||
# Extra compose overlays, colon-separated, appended in order. ci-local.sh sets
|
||||
# this to docker-compose.external-net.yml so the lab attaches to the /24s it
|
||||
# claimed rather than the base file's fixed pins; nothing else sets it, so the
|
||||
# GitHub matrix and a bare `docker compose up` keep today's addresses. Applied
|
||||
# to the whole COMPOSE array, so teardown matches bring-up — a `down` that
|
||||
# omitted the overlay would not know the networks are external and would try to
|
||||
# delete networks it does not own.
|
||||
if [ -n "${MC_EXTRA_COMPOSE:-}" ]; then
|
||||
IFS=':' read -ra _MC_EXTRA <<< "${MC_EXTRA_COMPOSE}"
|
||||
for _f in "${_MC_EXTRA[@]}"; do
|
||||
case "$_f" in
|
||||
/*) COMPOSE+=(-f "$_f") ;;
|
||||
*) COMPOSE+=(-f "$ROOT_DIR/$_f") ;;
|
||||
esac
|
||||
done
|
||||
fi
|
||||
|
||||
# Ping cadence during a move. 5/s is fast enough to resolve a sub-second gap
|
||||
# without the send loop itself becoming the thing under test.
|
||||
PING_INTERVAL=0.2
|
||||
|
||||
Reference in New Issue
Block a user