docs(tests): correct five stale claims in the e2e capture tooling - #3662
Merged
Aleksei Sviridkin (lexfrei) merged 4 commits intoAug 9, 2026
Conversation
Section 8 described two of the three on-failure capture legs and not the third, so the collector that reads forwarding state off the node was absent from the one document that tells a contributor what diagnostics exist. It is also the leg whose two callers disagree about its budget, 600s from the BATS runner and 300s from the Chainsaw catch, which is what decides whether something added to it fits. Its notes contract is described with the carve-out the script itself declares: the two EndpointSlice reads in the LoadBalancer path report nothing either way, so a <none> backend line there is the one place a reader still cannot tell an endpointless Service from a read that never answered. Stating the contract without it would have promised the distinction on the path this collector exists to diagnose. The prose also still said the runner's captures serve "the BATS tests". They serve the e2e ones: the runner keys on an e2e- prefix, on the suite's filename or on its parent directory, because all three legs read cluster state and a unit runner has kubectl on PATH with no cluster behind it. Said once where the captures are introduced, and the two phrases that over-claimed now name the e2e suites. The legs are not gated alike, so the sentence says which fired where: two key on kubectl, and the snapshot leg keys on crust-gather being installed, which on a unit runner it usually is not. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la>
Two numbers in this comment block no longer described the code. The snapshot leg's cost was given as 360s, the deadline passed to timeout. The leg is invoked as `timeout -k 30 360`, so a hung crust-gather is signalled at 360s and killed at 390s, and the trap waits out the later one. The comment 40 lines below already said 390s, as does the op-budget arithmetic in .chainsaw.yaml; the three now agree, and the number names the grace it includes, because the data-plane leg beside it is quoted at its bare deadline. The data-plane leg was still described as having no bound on its first pod list, and billed at the full 600s on that basis. Its reads carry their own ceilings now, 28s for a list and 20s for a single read, so the leg returns in tens of seconds and the paragraph's bottom line overstated the bill it exists to justify. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la>
The note explaining that image-cache readiness is gated at point of use pointed into hack/e2e-apps/, which holds two monitoring suites and no runner. The readiness gate it means is the rollout status wait in hack/e2e-chainsaw/_lib/talos-image-cache.sh, along with both fallbacks to the public factory; run-kubernetes.sh only calls it. Naming the caller would have left the reader one hop short of the code. 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 08:38
Contributor
📝 WalkthroughWalkthroughThe changes update E2E failure-diagnostics guidance, document bounded capture timing and expanded Chainsaw coverage, and point Talos image cache readiness checks to the Chainsaw helper. ChangesE2E diagnostics and readiness
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 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 |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@hack/e2e-install-cozystack.bats`:
- Around line 46-48: Update the comment near the talos image cache reference to
document both fallback gates: rollout failure and failure of the tenant-scoped
Service probe to return HTTP 206. Clarify that the deployment may be Available
yet still fall back to the public factory when the readiness probe cannot reach
the mirror.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: f147c192-3e01-44a4-a0e1-9257d95f0ebb
📒 Files selected for processing (3)
docs/agents/e2e-testing.mdhack/cozytest.shhack/e2e-install-cozystack.bats
Comment on lines
+46
to
+48
| # at point-of-use in hack/e2e-chainsaw/_lib/talos-image-cache.sh, called from | ||
| # run-kubernetes.sh, which falls back to the public factory if the mirror never | ||
| # becomes Available, so this can only help. |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Document both mirror readiness gates.
The referenced helper falls back to the public factory not only when the rollout fails, but also when the tenant-scoped probe cannot reach the Service with HTTP 206. An Available deployment can therefore still use the public factory.
Proposed wording
- # run-kubernetes.sh, which falls back to the public factory if the mirror never
- # becomes Available, so this can only help.
+ # run-kubernetes.sh, which falls back to the public factory if the mirror
+ # rollout or tenant-scoped HTTP 206 reachability check fails, so this can
+ # only help.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| # at point-of-use in hack/e2e-chainsaw/_lib/talos-image-cache.sh, called from | |
| # run-kubernetes.sh, which falls back to the public factory if the mirror never | |
| # becomes Available, so this can only help. | |
| # at point-of-use in hack/e2e-chainsaw/_lib/talos-image-cache.sh, called from | |
| # run-kubernetes.sh, which falls back to the public factory if the mirror | |
| # rollout or tenant-scoped HTTP 206 reachability check fails, so this can | |
| # only help. |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@hack/e2e-install-cozystack.bats` around lines 46 - 48, Update the comment
near the talos image cache reference to document both fallback gates: rollout
failure and failure of the tenant-scoped Service probe to return HTTP 206.
Clarify that the deployment may be Available yet still fall back to the public
factory when the readiness probe cannot reach the mirror.
Two files enumerated the BATS suites and both stopped one short. The global catch in .chainsaw.yaml describes itself as the Chainsaw analog of the EXIT-trap host snapshot in hack/cozytest.sh, and said that runner "only covers the BATS install/openapi tests". The README said cluster bootstrap and the OpenAPI checks remain BATS and named only two files. Both miss e2e-prepare-cluster.bats, which packages/core/testing/Makefile drives through the same runner, and which is the suite whose failures leave the sandbox in the state hardest to reconstruct afterwards. docs/agents/e2e-testing.md already listed all three, so the count was answerable from the tree; it just disagreed with itself depending on which file you opened. The .chainsaw.yaml paragraph is reflowed because the correction is longer than the line it replaces; no other wording changed. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la>
Aleksei Sviridkin (lexfrei)
force-pushed
the
fix/e2e-docs-and-comment-tidyup
branch
from
August 8, 2026 08:49
aaa2610 to
b11066e
Compare
This was referenced Aug 8, 2026
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
Five things in the e2e tooling said something the code no longer does, or did not say it at all. All five are prose; no executable line changes.
docs/agents/e2e-testing.mdsection 8 described two of the three on-failure capture legs. The missing one ishack/e2e-capture-dataplane.sh, the only leg that reads forwarding state off the node rather than object state, and the only one whose two callers bound it differently, 600s from the BATS runner and 300s from the Chainsaw catch. That split is what decides whether something added to it fits, so a contributor who reads the document and not the script sizes a change against the wrong budget. Its notes contract is described with the carve-out the script declares itself: the two EndpointSlice reads in the LoadBalancer path report nothing either way, so a<none>backend line there is the one place a reader still cannot tell an endpointless Service from a read that never answered.The same section said the runner's captures serve "the BATS tests" in two places. They serve the e2e ones. The runner keys on an
e2e-prefix, on the suite's own filename or on its parent directory, because all three legs read cluster state and a unit runner haskubectlon PATH with no cluster behind it. The legs are not gated alike either, so the sentence says which fired where: two key onkubectl, and the snapshot leg keys oncrust-gatherbeing installed, which on a unit runner it usually is not.Two numbers in the capture-gate rationale in
hack/cozytest.shno longer described the code. The snapshot leg's cost was given as 360s, the deadline passed totimeout; the leg is invoked astimeout -k 30 360, so the trap waits out 390s, which is what the comment forty lines below and the op-budget arithmetic in.chainsaw.yamlalready said. And the data-plane leg was still described as having no bound on its first pod list, and billed at the full 600s on that basis, after that read was given a 28s ceiling.Two files enumerated the BATS suites and both stopped one short. The global catch in
hack/e2e-chainsaw/.chainsaw.yamlsaid the runner it names as its analog "only covers the BATS install/openapi tests", andhack/e2e-chainsaw/README.mdnamed the same two files. Both misshack/e2e-prepare-cluster.bats, whichpackages/core/testing/Makefiledrives through that runner, and which is the suite whose failures leave the sandbox in the state hardest to reconstruct afterwards.docs/agents/e2e-testing.mdalready listed all three, so the tree disagreed with itself depending on which file you opened.A comment in
hack/e2e-install-cozystack.batspointed athack/e2e-apps/run-kubernetes.sh, which does not exist; that directory holds two monitoring suites and no runner. The readiness gate it means is therollout statuswait inhack/e2e-chainsaw/_lib/talos-image-cache.sh, along with both fallbacks to the public factory.relates to #3626 and #3642
Screenshots
Downstream repositories
Release note