fix(hack): arm cozytest cluster captures for e2e suites only - #3645
Merged
Aleksei Sviridkin (lexfrei) merged 1 commit intoAug 8, 2026
Merged
Conversation
The runner's on-failure EXIT trap gates each cluster capture on a binary being installed rather than on there being a cluster to read. A unit runner has kubectl and no cluster, so a red unit test walked into those legs on its way out. What that cost depends on how kubectl fails. Against an endpoint that refuses immediately it is a second; against one that hangs, measured with a stub that never returns, it is ~28s for the previous-logs leg, which self-bounds its pod list and so never reaches the 300s ceiling above it, plus the full 600s for the data-plane leg, whose first pod list carries no bound of its own. Ten minutes of dead waiting for a failure that should cost seconds. Where crust-gather is installed too the bill stops being empty and starts being wrong: that leg needs no reachable cluster to be pointless, only an ambient KUBECONFIG, and it will spend up to 360s snapshotting whatever cluster the current context names. Gate the three legs on the e2e- prefix instead, read from the suite's own name or from the directory holding it. At the top level that prefix is what the project already sorts on: the Makefile builds BATS_UNIT_FILES as $(filter-out hack/e2e-%.bats,$(wildcard hack/*.bats)), so a top-level e2e-* suite is exactly the one `make unit-tests` refuses to run and packages/core/testing/Makefile runs against a live cluster. That wildcard is not recursive, and hack/e2e-apps/ holds live-cluster suites whose own filenames carry no prefix, so the directory is checked too: matching the basename alone would take the captures away from suites that need them, which is the opposite of this change. The e2e path is otherwise unchanged, every leg, bound and message as it was. Not a reachability probe, though that would also tell a cluster from a bare binary. A failing e2e run is frequently one whose apiserver is degraded, and a probe answering "no" there would skip the snapshot in precisely the case the snapshot exists for. The test drives the runner as a subprocess over generated fixtures that differ only in where they sit, with stub kubectl and crust-gather on PATH. Stubbing both matters: the runner calls crust-gather directly rather than through a script that needs kubectl output first, so a kubectl-only stub would leave that leg reaching for the real binary against the ambient KUBECONFIG. Each leg is pinned by the artefact it leaves in the report tree, not only by the one leg that echoes, so no single leg can be ungated without a test going red; each was checked by ungating it alone. All three directions are asserted, and the two armed cases are the positive control for the disarmed one, whose assertion is negative and would otherwise pass just as well if the harness ran nothing; that case also pins that the fixture did fail. Two shell traps are handled rather than left to be rediscovered. The awk transform rewrites any line beginning `@test "` and any line that is exactly `}`, and knows nothing of heredocs, so a fixture written as a heredoc has its own `@test` line harvested into the outer suite as a phantom test that fails first and ends the run before the real tests; the fixtures are built with printf instead. And `set -e` ignores a command whose status is inverted with `!`, so the negative assertion is spelled as an `if`. Both were checked by building the broken variant, not reasoned about. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la>
Aleksei Sviridkin (lexfrei)
requested a review
from myasnikovdaniil
as a code owner
August 8, 2026 01:01
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe test runner now detects e2e suites before collecting cluster diagnostics. New Bats tests verify capture artifacts for failing e2e suites and their absence for failing unit suites. ChangesCluster capture gate
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related issues
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Aleksei Sviridkin (lexfrei)
deleted the
fix/cozytest-gate-captures-on-e2e-context
branch
August 8, 2026 07:46
11 tasks
8 tasks
1 task
11 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this PR does
hack/cozytest.sharms three cluster captures from its on-failure EXIT trap, and gates each on a binary being installed rather than on there being a cluster to read. A unit runner has kubectl and no cluster, so a red unit test walked into those legs on its way out and paid for snapshots of nothing.What that costs turns on how kubectl fails, which is why the defect survived: against an endpoint that refuses it is a second, and locally that is what happens. Against one that hangs, which is what a runner with a context pointing at a torn-down cluster has, I measured ~28s for the previous-logs leg and the full 600s for the data-plane one. The 28s is the leg self-bounding its own pod list, so it never reaches the 300s ceiling above it; the 600s is real, because that leg's first pod list carries no bound of its own. Ten minutes of dead waiting for a failure that should have cost seconds.
Where crust-gather is installed too, the bill stops being empty and starts being wrong. That leg needs no reachable cluster to fire, only an ambient KUBECONFIG, and it will spend up to 360s snapshotting whatever cluster the current context happens to name.
The gate now keys on the
e2e-prefix, read from the suite's own name or from the directory holding it. At the top level that is the split the project already sorts on: the Makefile buildsBATS_UNIT_FILESas$(filter-out hack/e2e-%.bats,$(wildcard hack/*.bats)), so a top-levele2e-*suite is exactly the onemake unit-testsrefuses to run andpackages/core/testing/Makefileruns against a live cluster. That wildcard is not recursive, andhack/e2e-apps/holds live-cluster suites whose own filenames carry no prefix, so the directory is checked too. Matching the basename alone would take the captures away from suites that need them, which is the opposite of the point. Every leg, bound and message on the e2e path is unchanged.It is deliberately not a reachability probe, though a probe would also tell a cluster from a bare binary. A failing e2e run is frequently one whose apiserver is degraded, and a probe answering "no" there would skip the snapshot in precisely the case the snapshot exists for. A diagnostic should not condition itself on the health of the thing it diagnoses.
The test drives the runner as a subprocess over fixtures that differ only in where they sit, with stub kubectl and crust-gather on PATH so neither direction is decided by a binary being absent. Each leg is pinned by the artefact it leaves in the report tree rather than by the one leg that echoes, so no single leg can be ungated without a test going red; that was checked by ungating each in turn. The two armed cases are the positive control for the disarmed one, whose assertion is negative and would otherwise pass just as well against a harness that ran nothing.
Relates to #3626.
Screenshots
Downstream repositories
Release note
Summary by CodeRabbit
Bug Fixes
Tests