mirror of
https://github.com/jmcorgan/fips.git
synced 2026-07-30 19:46:15 +00:00
Close the gaps an adversarial review found in the new CI guards
Four of these are places the parity guard could stop covering something without saying so, which is the failure mode it exists to prevent. Removing the deb-install suite array left it green, because the dispatch cross-check compared against a hardcoded name rather than the array it was meant to read. An arm-shaped line at an unexpected indent, or one written in quotes, was dropped silently by the arm pattern; unexpected indents are now a hard error rather than a quiet skip, and quoted arms parse. A matrix leg with a type of chaos or deb-install but no scenario key raised a Python traceback instead of reporting a problem, and a leg carrying a scenario but no suite key was skipped entirely even though scenario is now the identity for those legs. The e2e builder no longer folds docker create's stderr into the container id. Docker prints warnings on success as well as failure, so a platform-mismatch warning would have become part of the id and made every later reference to that container fail, turning a working extraction into a spurious red. Also corrects the comment about registering a new chaos assertion type, which named only one of the two key sets that actually need it.
This commit is contained in:
@@ -249,8 +249,9 @@ class Scenario:
|
|||||||
# link_swap.policies and topology.transport_mix. Two sub-trees are passed through
|
# link_swap.policies and topology.transport_mix. Two sub-trees are passed through
|
||||||
# whole and are likewise not checked: fips_overrides and topology.params.
|
# whole and are likewise not checked: fips_overrides and topology.params.
|
||||||
#
|
#
|
||||||
# NOTE: adding a new assertion type means adding it to _ASSERTION_KEYS below and
|
# NOTE: adding a new assertion type means registering it in TWO places below —
|
||||||
# giving it an entry here, or scenarios using it will be rejected at load.
|
# _SECTION_KEYS["assertions"] (so the block accepts its name) and _ASSERTION_KEYS
|
||||||
|
# (so its own members are checked) — or scenarios using it are rejected at load.
|
||||||
_TOP_KEYS = {
|
_TOP_KEYS = {
|
||||||
"scenario", "topology", "netem", "link_flaps", "traffic", "node_churn",
|
"scenario", "topology", "netem", "link_flaps", "traffic", "node_churn",
|
||||||
"peer_churn", "bandwidth", "ingress", "link_swap", "assertions", "logging",
|
"peer_churn", "bandwidth", "ingress", "link_swap", "assertions", "logging",
|
||||||
|
|||||||
@@ -136,16 +136,26 @@ with open(ci_yml_path, encoding="utf-8") as fh:
|
|||||||
|
|
||||||
include = doc["jobs"]["integration"]["strategy"]["matrix"]["include"]
|
include = doc["jobs"]["integration"]["strategy"]["matrix"]["include"]
|
||||||
github_chaos, github_deb, github = {}, set(), set()
|
github_chaos, github_deb, github = {}, set(), set()
|
||||||
|
malformed = []
|
||||||
for leg in include:
|
for leg in include:
|
||||||
if "suite" not in leg:
|
if "suite" not in leg and "scenario" not in leg:
|
||||||
continue
|
continue
|
||||||
kind = str(leg.get("type", ""))
|
kind = str(leg.get("type", ""))
|
||||||
if kind == "chaos":
|
if kind in ("chaos", "deb-install"):
|
||||||
github_chaos[str(leg["scenario"])] = str(leg.get("chaos_flags", ""))
|
# scenario: is the identity for these; suite: is cosmetic.
|
||||||
elif kind == "deb-install":
|
if "scenario" not in leg:
|
||||||
github_deb.add(str(leg["scenario"]))
|
malformed.append(f"{leg.get('suite', '(unnamed leg)')} has type "
|
||||||
else:
|
f"{kind} but no scenario:")
|
||||||
|
continue
|
||||||
|
if kind == "chaos":
|
||||||
|
github_chaos[str(leg["scenario"])] = str(leg.get("chaos_flags", ""))
|
||||||
|
else:
|
||||||
|
github_deb.add(str(leg["scenario"]))
|
||||||
|
elif "suite" in leg:
|
||||||
github.add(str(leg["suite"]))
|
github.add(str(leg["suite"]))
|
||||||
|
else:
|
||||||
|
malformed.append(f"leg with scenario {leg['scenario']} has no suite: "
|
||||||
|
f"and no chaos/deb-install type")
|
||||||
|
|
||||||
# ── Dispatch cross-check: every run_suite arm needs a backing array ──────────
|
# ── Dispatch cross-check: every run_suite arm needs a backing array ──────────
|
||||||
# A suite dispatched without an array is invisible to the sweep above, so the
|
# A suite dispatched without an array is invisible to the sweep above, so the
|
||||||
@@ -159,14 +169,26 @@ if body is None:
|
|||||||
# Arms sit at one fixed indentation inside the case block. Pin to it, taken from
|
# Arms sit at one fixed indentation inside the case block. Pin to it, taken from
|
||||||
# the first arm rather than assumed, so a body line that happens to end in ')'
|
# the first arm rather than assumed, so a body line that happens to end in ')'
|
||||||
# cannot be read as an arm.
|
# cannot be read as an arm.
|
||||||
arm_re = re.compile(r"^([ \t]+)([a-z0-9|*_-]+)\)", re.MULTILINE)
|
arm_re = re.compile(r"^([ \t]+)['\"]?([a-z0-9|*_.-]+)['\"]?\)", re.MULTILINE)
|
||||||
first = arm_re.search(body.group(0))
|
first = arm_re.search(body.group(0))
|
||||||
if first is None:
|
if first is None:
|
||||||
print("check-ci-parity: no dispatch arms found in run_suite()", file=sys.stderr)
|
print("check-ci-parity: no dispatch arms found in run_suite()", file=sys.stderr)
|
||||||
sys.exit(2)
|
sys.exit(2)
|
||||||
|
|
||||||
indent = first.group(1)
|
indent = first.group(1)
|
||||||
known = set(local) | set(local_chaos) | local_deb | {"deb-install"}
|
# An arm-shaped line at a different indent is not skipped silently: it would
|
||||||
|
# make this check quietly stop covering a suite, which is the failure mode the
|
||||||
|
# check exists to prevent.
|
||||||
|
odd_arms = [
|
||||||
|
m.group(2) for m in arm_re.finditer(body.group(0)) if m.group(1) != indent
|
||||||
|
]
|
||||||
|
if odd_arms:
|
||||||
|
print("check-ci-parity: run_suite has arm-shaped lines at an unexpected "
|
||||||
|
f"indent, so the dispatch check cannot be trusted: {', '.join(odd_arms)}",
|
||||||
|
file=sys.stderr)
|
||||||
|
sys.exit(2)
|
||||||
|
known = (set(local) | set(local_chaos) | local_deb
|
||||||
|
| {e.split()[0] for e in arrays.get("DEB_INSTALL_SUITES", [])})
|
||||||
for m in arm_re.finditer(body.group(0)):
|
for m in arm_re.finditer(body.group(0)):
|
||||||
if m.group(1) != indent:
|
if m.group(1) != indent:
|
||||||
continue
|
continue
|
||||||
@@ -197,7 +219,7 @@ deb_github_only = sorted(github_deb - local_deb)
|
|||||||
|
|
||||||
problems = (local_only or github_only or chaos_local_only or chaos_github_only
|
problems = (local_only or github_only or chaos_local_only or chaos_github_only
|
||||||
or chaos_flag_drift or deb_local_only or deb_github_only
|
or chaos_flag_drift or deb_local_only or deb_github_only
|
||||||
or dispatch_uncovered)
|
or dispatch_uncovered or malformed)
|
||||||
|
|
||||||
if problems:
|
if problems:
|
||||||
print("CI parity FAILED: the two runners do not cover the same work.\n")
|
print("CI parity FAILED: the two runners do not cover the same work.\n")
|
||||||
@@ -230,6 +252,10 @@ if problems:
|
|||||||
print(" deb-install distros GitHub-only:")
|
print(" deb-install distros GitHub-only:")
|
||||||
for n in deb_github_only:
|
for n in deb_github_only:
|
||||||
print(f" - {n}")
|
print(f" - {n}")
|
||||||
|
if malformed:
|
||||||
|
print(" Matrix legs this guard cannot identify:")
|
||||||
|
for n in malformed:
|
||||||
|
print(f" - {n}")
|
||||||
if dispatch_uncovered:
|
if dispatch_uncovered:
|
||||||
print(" run_suite dispatches these with no backing *_SUITES array, so "
|
print(" run_suite dispatches these with no backing *_SUITES array, so "
|
||||||
"this guard\n cannot see them in the local set:")
|
"this guard\n cannot see them in the local set:")
|
||||||
|
|||||||
@@ -275,11 +275,17 @@ DOCKERFILE
|
|||||||
# and the e2e scenarios silently exercise the previous commit's code.
|
# and the e2e scenarios silently exercise the previous commit's code.
|
||||||
rm -f "$FIPS_BIN_CACHE" "$FIPS_GATEWAY_BIN_CACHE"
|
rm -f "$FIPS_BIN_CACHE" "$FIPS_GATEWAY_BIN_CACHE"
|
||||||
|
|
||||||
local cid err
|
# stderr goes to its own file rather than into $cid: docker prints
|
||||||
if ! cid=$(docker create "$builder_tag" 2>&1); then
|
# warnings (a platform mismatch, say) on success too, and folding them
|
||||||
echo " ERROR: docker create failed: $cid"
|
# into the id would leave every later reference pointing at nothing.
|
||||||
|
local cid err errfile
|
||||||
|
errfile=$(mktemp)
|
||||||
|
if ! cid=$(docker create "$builder_tag" 2>"$errfile"); then
|
||||||
|
echo " ERROR: docker create failed: $(cat "$errfile")"
|
||||||
|
rm -f "$errfile"
|
||||||
return 1
|
return 1
|
||||||
fi
|
fi
|
||||||
|
rm -f "$errfile"
|
||||||
|
|
||||||
local rc=0 spec bin dest
|
local rc=0 spec bin dest
|
||||||
for spec in "fips:$FIPS_BIN_CACHE" "fips-gateway:$FIPS_GATEWAY_BIN_CACHE"; do
|
for spec in "fips:$FIPS_BIN_CACHE" "fips-gateway:$FIPS_GATEWAY_BIN_CACHE"; do
|
||||||
|
|||||||
Reference in New Issue
Block a user