mirror of
https://github.com/cozystack/cozystack.git
synced 2026-10-28 23:57:08 +09:00
docs/agents-dev-principles
25
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Myasnikov Daniil
|
cc942fed68 |
test(tests): run unit suites with Bats and retain POSIX coverage
The shell translator rewrites test bodies and loses Bats reporting. The unit lane needs complete TAP results and JUnit reports without changing how live-cluster suites run. Sourced production helpers still need a dash pass, and unit test bodies need nounset without exporting shell options into subprocesses. Assisted-by: LLM Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Aleksei Sviridkin
|
c4e6d169a0 |
fix(e2e): give the cozytest caller a dataplane budget that fits its backstop
The collector's default budget is sized under the 300s Chainsaw backstop. The cozytest.sh caller allows 600s, and on the default it would give up more than half of that window, so it passes a budget matching its own backstop. The budget test reads each caller's override and fails when a budget leaves more of its backstop unused than the backstop's kill grace. The Chainsaw call-site comment still described the fan-out caps as the bound, and the e2e testing guide described an EndpointSlice exception to the notes contract that the code does not have; both now describe what the collector does. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Myasnikov Daniil
|
c55ab268e2 |
test(monitoring): remove orphaned OIDC BATS suites
The two pre-Chainsaw monitoring OIDC suites have had no invocation site since the app suite migrated to Chainsaw. Remove them instead of carrying dead tests that force the selector to run every live suite. Keep the existing selector and capture guards for the historical path shape, and update their comments now that the directory is empty. Retain a named chart assertion for the no-KeycloakRealmGroup ownership invariant; the existing count and kind assertions already enforce the same behavior. Assisted-By: LLM Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Alexey Artamonov
|
0473079448 |
fix(hack): parse non-ASCII @test titles under BSD awk
cozytest.sh rebuilds each test's function name by walking its @test title one character at a time with awk substr(). BSD awk aborts that walk on the first multibyte byte instead of returning it, so a file carrying a non-ASCII title never parses and none of its tests run -- silently, since the suite only reports that the runner errored. Four hack/*.bats files carry such a title today, and the gap is invisible to CI because GNU awk does not abort on the same input. Pin LC_ALL=C on all three awk passes -- the title parse, the run selection, and the exit-code scan of the captured log -- so each reads the input byte-wise. The parse and selection passes must share the ctype or they would derive mismatched function names for one title. Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io> |
||
|
Myasnikov Daniil
|
c88da18139 |
test(tests): keep trace switch suite ASCII-only
Assisted-By: GPT-5 <noreply@openai.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Myasnikov Daniil
|
76d397523b |
docs(tests): record why an INT/TERM trap cannot cover a killed test
The comment added with the trace switch pointed at a signal trap as the fix for the one case quiet mode does not cover: a test killed rather than failed never reaches the fail handler, so its trace is gone rather than already printed. That advice is wrong, and wrong in a way that costs whoever follows it. POSIX sh defers a trap until the current foreground command returns, so installing an INT/TERM handler stops the runner dying on the signal at all. Measured: a suite hung on `sleep 30` exits in 30s with the trap installed against 2s under the default disposition. GitHub's cancellation escalates INT, TERM, then KILL, so the deferred handler gets SIGKILLed before it runs -- no dump, and the grace period spent waiting for a hang. Built it, measured it, reverted it. Making it work means running the test pipeline in the background and waiting on it, because `wait` is the interruptible one. That is surgery on the path every suite in this repo takes, unit and e2e, to recover a partial trace in a rare case that already has a 48-second escape hatch. Not taken; recorded so the next reader does not spend the afternoon finding out the same way. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Myasnikov Daniil
|
a5c90d984e |
fix(tests): guard the finishedAt leaf and make three guards actually bite
Findings from two independent adversarial reviews of the preceding two
commits. Each was reproduced locally before being fixed.
The ordering inverted itself on a shape I had not probed. `finishedAt`
is a metav1.Time behind `json:",omitempty"`, which cannot omit a
struct, and its zero value marshals to `null`; the kubelet does
synthesize a terminated state with no time (ContainerStatusUnknown on a
pod deleted mid-run, which also bumps restartCount and so clears the
restart filter). Two guards deep left the leaf exposed, and kubectl
renders it as the literal `<no value>` -- which sorts ABOVE every digit,
so that row took the front of the queue from the most recent real
restart, and the header printed `previous instance ended <no value>`, a
claim about a time nothing observed. Verified with a text/template probe
over seven lastState shapes: `null` and a missing key both produced it.
Fixed at both layers, because they fail independently. The template now
guards the leaf, so the string cannot be produced. And the sort
normalises any field-6 value not shaped like a timestamp to empty, so
the ordering stays correct and the header stays quiet even if some other
producer emits one. Earlier verification against a live cluster showed
no `<no value>` and I read that as the guard being right; it only meant
no container on that cluster was in that state.
Three guards did not bite:
- both stability fixtures were pre-sorted ascending, which is exactly
what sort's last-resort whole-line compare reproduces with no -s at
all, so the tests written for -s passed without it. Reversed; they
now fail when -s is dropped.
- no test fed a six-field row through the whole script, so the
timestamp reached the dump header uncovered -- deleting the read
variable left the suite green at 39/39. One integration test now
asserts the header names the time.
- the e2e guard matched any mention of COZYTEST_TRACE, so documenting
the design over there would have turned it red. It now matches an
assignment, and tolerating a comment is verified too.
Also: a failed sort no longer discards a whole namespace group. The
caller compares row counts to report the cap, so losing a group silently
became "0 of N captured, N more not captured" and sent the reader after
a cap that never fired. It falls back to discovery order and says so.
LC_ALL=C on the sort, since the key is fixed-format ASCII.
And the claim itself was too strong. "Nothing diagnostic is lost" holds
only for a test that RETURNS: the fail handler runs after the pipeline,
and there is no INT/TERM trap, so a test killed rather than failed -- a
hang reaped by the job timeout -- never dumps its log, and under quiet
mode its trace is gone rather than already printed. The comments in
hack/cozytest.sh and the Makefile now say so and name the escape hatch.
A trap that flushed the tail of the log would close it; left out
deliberately rather than bundled in here.
Every fix is mutation-proven: reverting the leaf guard, removing the
key normalisation, dropping the read variable, dropping -s, and
silencing an e2e recipe are each caught. Unit lane 1513 passed, 1 failed
(hack/ghcr-mirror_test.bats, which reads a live cluster and fails the
same way under main's runner).
Assisted-By: Claude <noreply@anthropic.com>
Assisted-By: GPT-5 <noreply@openai.com>
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
|
||
|
Myasnikov Daniil
|
fdb2e89fe7 |
test(unit): make the bats trace stream opt-in, quiet on the unit lane
hack/cozytest.sh runs every @test body under `set -x` and streams the trace live. Across the ~40 hack/*.bats unit suites that is 233,444 of 238,794 lines - 98% of a 31 MB job log - and the single failing assertion sits at line 238,573 of it. The web log viewer is unusable at that size, and anyone (or anything) reading the job to find out what broke pays for the whole file first. Nothing in that stream is load-bearing on the unit lane. run_one already tees the raw output to $log and the fail handler dumps all of it for whichever test fails, so suppressing the live stream drops the output of PASSING tests only. Add COZYTEST_TRACE, default 1 so the e2e call sites in packages/core/testing/Makefile keep the live stream - there it is the only progress signal, and the only record at all when a step timeout or a dead runner kills the job before the fail handler can run. The bats-unit-tests target sets 0; running the runner by hand stays verbose, which is the case where a human is actually watching. Measured on 25 suites locally, quiet vs verbose: 29,528 -> 951 lines, 3.9 MB -> 94 KB, and 71s -> 48s wall, the last because the per-line formatter forks date twice for every line it prints. Quiet output is byte-for-byte the non-trace subset of verbose, and a hanging test still shows as a ╭ with no matching ╰. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Aleksei Sviridkin
|
ad62a8d62b |
test(e2e): stop blocking merges on the tenant node-join deadline
The tenant node-join wait no longer discriminates. Nested virtualisation on the shared runners degrades far enough that a guest which registers in minutes on real hardware can miss any deadline this test can afford, so a red on that one wait says as much about the machine the run landed on as about the product, and it teaches a reader to re-run the job. The suite still fails on it. That is what keeps Chainsaw's catch running: the previous-instance container logs, the host snapshot, the data-plane capture and the event dump are all keyed on the test failing, and the report stays honest. What changes is one layer up. The wait writes a marker beside its diagnostics, and a new lane-side gate reads that marker to decide whether the job blocks. It opens only when every failed test in the run carries one, and blocks on anything it cannot attribute: a report it could not read, a failed test the report does not name, a failure with no marker. Only the lanes that block a merge ask it. The two pull-request lanes do; nightly and the tag lane run the same suites and stay hard. A soft pass on the tag lane would ship a release candidate whose tenant clusters brought up no worker nodes, and nightly is the only place an e2e failure reaches anyone who did not open a pull request, since no lane in this tree notifies anywhere. A softened red finishes the job green, so the job's result stops answering whether the suite passed, and both kinds of consumer are handed the decision instead of inferring it. A step gated on the job failing would silently stop running: the SSH breakpoint is that case, and it still opens for a pull request marked for debugging. A job that reports the result would silently start reporting a pass: the required commit status now says the deadline was missed and that the merge is not blocked on it, rather than that the suite passed. The deadline goes to 29m. That is the test's own budget and nothing more: its clock starts after the machinedeployment wait, the LoadBalancer assignment and the healthz probe, while the chart's autoscaler and health-check timers have been running since the Machine was created, so no ordering against them is claimed. The chart's 20m and 30m give a scale to pick against, and a budget over the first leaves room for a remediation cycle without promising that one fits. The operation ceiling both suites give the script rises with the deadline, and the diagnostics phase budget rises inside it, because the collectors gated ahead of the serial console cost more between them than the old budget covered. Both budget guards read their terms from the sources that set them. The e2e job cap rises with them, and its guard holds what a guard can hold: the pair's operation and catch ceilings, and one figure across every lane that runs the suite. It also records that the suites' full summed ceilings have never fit any cap in that file, so the margin above the bound is taken from a measured run instead. The prose that restates the cap elsewhere moves with it, and the sweep that holds the deadline's restatements now reads the figure spelled in words as well as in minutes. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
86fb92abf5 |
docs(tests): correct five stale claims in the e2e capture tooling (#3662)
<!-- Thank you for making a contribution! Here are some tips for you: - Use Conventional Commits for the PR title: `type(scope): description` - Types: feat, fix, docs, style, refactor, perf, test, build, ci, chore - Scopes are not an exhaustive list — pick the most specific scope for the change and extend the list when a genuinely new area appears. Examples: - System components: dashboard, platform, operator, cilium, kube-ovn, linstor, fluxcd, cluster-api - Managed apps: postgres, mariadb, redis, kafka, clickhouse, virtual-machine, kubernetes - Development and maintenance: api, hack, tests, ci, docs, maintenance - Breaking changes: append `!` after type/scope (`feat(api)!: ...`) or add a `BREAKING CHANGE:` footer - If it's a work in progress, consider creating this PR as a draft. - Don't hesistate to ask for opinion and review in the community chats, even if it's still a draft. - Add the label `backport` if it's a bugfix that needs to be backported to a previous version. --> ## 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.md` section 8 described two of the three on-failure capture legs. The missing one is `hack/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 has `kubectl` on PATH with no cluster behind it. The legs are not gated alike either, 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. Two numbers in the capture-gate rationale in `hack/cozytest.sh` 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 the trap waits out 390s, which is what the comment forty lines below and the op-budget arithmetic in `.chainsaw.yaml` already 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.yaml` said the runner it names as its analog "only covers the BATS install/openapi tests", and `hack/e2e-chainsaw/README.md` named the same two files. Both miss `hack/e2e-prepare-cluster.bats`, which `packages/core/testing/Makefile` drives through that 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 tree disagreed with itself depending on which file you opened. A comment in `hack/e2e-install-cozystack.bats` pointed at `hack/e2e-apps/run-kubernetes.sh`, which does not exist; that directory 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. relates to #3626 and #3642 ### Screenshots <!-- REQUIRED for UI changes: attach screenshots or screen recordings demonstrating the visual impact of your changes. PRs with UI changes without screenshots will not be merged. --> ### Downstream repositories <!-- Cozystack is upstream for repositories that are NOT kept in sync with it automatically, and nothing in CI compares the two sides, so a change here can break them silently. Which change forces what, and which file to touch, is the trigger map in docs/agents/contributing.md under "Downstream Repositories". Walk it before you tick anything. IF YOU ARE AN AI AGENT OR AN LLM, THIS IS PART OF THE TASK, NOT A FORM TO FILL IN: 1. Walk the trigger map against the actual diff of this PR, file by file. Do not decide from the PR title or from your memory of what you changed. 2. For every repository you tick, open the follow-up PR there and link it on the same line. A ticked box with no link claims work that does not exist, which is worse than an empty box. Search that repository for an open PR or issue covering it first, and link that instead of filing a duplicate. 3. When the follow-up is out of scope, or needs a decision that is not yours to make, open an issue in that repository instead, link that, and say so in the PR body. Do not push a speculative PR into another repository just to fill a line here. 4. Do not tick "No downstream repository is affected" to make the checklist go away. If you are unsure, leave every box empty and say so in the PR body, so a human decides. --> - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: ### Release note <!-- Write a release note: - Explain what has changed internally and for users. - Start with the same `type(scope):` prefix as in the PR title - Follow the guidelines at https://github.com/kubernetes/community/blob/master/contributors/guide/release-notes.md. --> ```release-note NONE ``` |
||
|
Aleksei Sviridkin
|
c6961541f3 |
fix(tests): report a data-plane capture cut short by its backstop
Both callers of the data-plane collector wrap it in a wall-clock backstop and threw the result away with `|| true`, while the previous-instance logs leg sitting beside each of them records the exit status and prints "INCOMPLETE (exit N); kept what landed in ...". When a backstop killed the collector, nothing said so, and a truncated dataplane/ directory read exactly like a complete one that found little. The collector itself now names every read of its own it could not finish, which left these two call sites as the only remaining way for that leg to die without a word. The Chainsaw one is the tighter of the pair -- 300s against 600s, because it shares an op envelope with the snapshot leg -- so it is both likelier to fire and more expensive to leave silent. In each place the status is read, never propagated: a diagnostic still cannot change a test's outcome. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
2147e12d79 |
docs(hack): correct both budget claims in the capture-gate rationale
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> |
||
|
Aleksei Sviridkin
|
db590b493c |
fix(hack): arm cozytest cluster captures for e2e suites only (#3645)
<!-- Thank you for making a contribution! Here are some tips for you: - Use Conventional Commits for the PR title: `type(scope): description` - Types: feat, fix, docs, style, refactor, perf, test, build, ci, chore - Scopes are not an exhaustive list — pick the most specific scope for the change and extend the list when a genuinely new area appears. Examples: - System components: dashboard, platform, operator, cilium, kube-ovn, linstor, fluxcd, cluster-api - Managed apps: postgres, mariadb, redis, kafka, clickhouse, virtual-machine, kubernetes - Development and maintenance: api, hack, tests, ci, docs, maintenance - Breaking changes: append `!` after type/scope (`feat(api)!: ...`) or add a `BREAKING CHANGE:` footer - If it's a work in progress, consider creating this PR as a draft. - Don't hesistate to ask for opinion and review in the community chats, even if it's still a draft. - Add the label `backport` if it's a bugfix that needs to be backported to a previous version. --> ## What this PR does `hack/cozytest.sh` arms 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 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 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 <!-- REQUIRED for UI changes: attach screenshots or screen recordings demonstrating the visual impact of your changes. PRs with UI changes without screenshots will not be merged. --> ### Downstream repositories <!-- Cozystack is upstream for repositories that are NOT kept in sync with it automatically, and nothing in CI compares the two sides, so a change here can break them silently. Which change forces what, and which file to touch, is the trigger map in docs/agents/contributing.md under "Downstream Repositories". Walk it before you tick anything. IF YOU ARE AN AI AGENT OR AN LLM, THIS IS PART OF THE TASK, NOT A FORM TO FILL IN: 1. Walk the trigger map against the actual diff of this PR, file by file. Do not decide from the PR title or from your memory of what you changed. 2. For every repository you tick, open the follow-up PR there and link it on the same line. A ticked box with no link claims work that does not exist, which is worse than an empty box. Search that repository for an open PR or issue covering it first, and link that instead of filing a duplicate. 3. When the follow-up is out of scope, or needs a decision that is not yours to make, open an issue in that repository instead, link that, and say so in the PR body. Do not push a speculative PR into another repository just to fill a line here. 4. Do not tick "No downstream repository is affected" to make the checklist go away. If you are unsure, leave every box empty and say so in the PR body, so a human decides. --> - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: ### Release note <!-- Write a release note: - Explain what has changed internally and for users. - Start with the same `type(scope):` prefix as in the PR title - Follow the guidelines at https://github.com/kubernetes/community/blob/master/contributors/guide/release-notes.md. --> ```release-note fix(hack): cozytest.sh no longer arms its cluster-capture hooks for unit BATS suites, so a failing unit test fails fast instead of waiting out capture timeouts against a cluster that is not there ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Cluster diagnostics are now collected only for end-to-end test suites. * Unit tests no longer trigger unnecessary Kubernetes or cluster-capture operations. * End-to-end suites nested under application directories are correctly recognized. * Diagnostic capture remains subject to existing tool availability and timeout checks. * **Tests** * Added coverage verifying diagnostic artifacts are collected for end-to-end failures and omitted for unit-test failures. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
8d5ab4cfee |
fix(hack): arm cozytest cluster captures for e2e suites only
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
|
8a44b3e408 |
fix(e2e): skip crust-gather host-log collection rejected by PodSecurity
The diagnostic snapshot captured on a failed e2e run spawns a privileged debug pod per node for its host-log leg. Baseline PodSecurity rejects every such pod with a 403, and crust-gather's retry loop keeps trying until it burns the entire --duration budget, then exits 1 with a deadline error. The snapshot verdict therefore printed FAILED on every failure capture even though the namespaced object state had been collected, and the host-log leg itself gathered nothing usable here. Pass --disable-additional-logs at all three collect sites: the host snapshot from the BATS runner, the host snapshot from the Chainsaw error.catch, and the nested tenant snapshot. The object collection now finishes cleanly, the happy path reports complete, and FAILED once again means a genuine collection failure. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
76f7267ff2 |
test(e2e): describe --previous as the preceding instance, not the first
kubectl logs --previous returns the immediately preceding terminated container instance, not the original first one — after multiple restarts the first instance is already gone. Reword the helper, the BATS and Chainsaw failure handlers, and the diagnostics doc to match the actual --previous semantics. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
07c67a8913 |
test(e2e): capture previous container logs and stop killing the snapshot
Root-causing a crash-loop failure was blocked twice over by the diagnostics collected on failure. `kubectl logs` shows only the current container instance, so for a crash-looping pod the suites' `podLogs` collector captured the Nth restart -- a fresh replay of the same startup path -- while the evidence that explained the failure sat in the first, interrupted instance. Chainsaw's podLogs collector has no `previous` option, so add e2e-capture-previous-logs.sh and drive it from the global catch and from the BATS failure hook. It discovers containers with restartCount > 0 at runtime (init containers included, where an interrupted bootstrap lives), orders the app suites' namespace first, and dumps each to the job log and the snapshot tree. Containers that never restarted are skipped, so a healthy run emits one line rather than a page of "previous terminated container not found". Every way that capture can come up short is named rather than inferred, because kubectl exits 1 for a garbage-collected container, an RBAC denial and an unreachable node alike: the container cap, a pod list that never returned (which is not the same as nothing having restarted), a read that timed out versus one kubectl refused and the reason it gave, and a previous instance that produced no output. A partial capture is kept and marked truncated rather than deleted -- it is the only copy of a log already gone from the cluster. The crust-gather snapshot was killed by the Chainsaw op containing it. A script op is bounded by timeouts.exec (5m), and the catch's own wall-clock wrappers summed to 960s, so the script was SIGKILLed partway through -- taking out the process group mid-write and skipping the data-plane capture entirely. Give each catch op an explicit timeout above the budgets it contains, so an inner timeout fires instead and leaves a partial, still-servable snapshot behind. The data-plane leg drops from 600s to 300s at that call site so both legs fit one envelope; the BATS hook keeps the wider budget, having no containing op. Set crust-gather --duration explicitly as well. It is the collection phase's own budget, it defaults to 60s, and on elapse it abandons the collection and skips its finish step, leaving a truncated tree with no indication anything is missing. The outer wall-clock has to exceed it because the API discovery that runs first is not covered by that budget at all -- with an unhealthy aggregated APIService, the state a failed run is usually in, discovery alone can consume the whole timeout before a single object is written. Send each collector's output to a log beside its snapshot instead of /dev/null, with a complete/truncated/failed verdict, so that question is answerable from the artifact. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
079e4079d3 |
test(e2e): capture host->pod data-plane on failure
The E2E install intermittently fails on a CNI host->local-pod data-plane transient: kubelet on a node reaches a local pod's readiness/startup probe with "connection refused" for several minutes while overlay pod->pod traffic works, then it self-heals. This is rooted in the cilium+kube-ovn chaining config (forced enable-host-legacy-routing, CNI InstallEndpointRoute:false), which delegates host->local-pod routing to kube-ovn/ovn0. The crust-gather failure snapshot records Kubernetes object state but not the node's L3 forwarding state, so the mechanism cannot be root-caused after the fact. On test failure, for each NotReady pod that is scheduled, has a podIP, and is not terminal (Succeeded/Failed excluded), collect the host->pod data-plane state from the pod's node into the crust-gather snapshot dir (so it ships in the uploaded report): - host netns via the kube-ovn cni-server (hostNetwork): ip route get <podIP>, ip neigh, ip rule, ip addr show ovn0, and the KERNEL conntrack entries for the podIP (conntrack -L, falling back to /proc/net/nf_conntrack). Under enable-host-legacy-routing the host->local-pod path traverses the kernel netfilter stack, not cilium BPF, so this is the authoritative conntrack table and route/rule state for the transient; - cilium-agent on the node: endpoint list, bpf CT entries for the podIP, a bounded monitor --type drop, and hubble dropped verdicts when present (complementary: confirms the path is not handled in BPF); - OVS/OVN on the node: ovs-ofctl dump-flows br-int, the OVN Port_Binding / Logical_Switch_Port for the pod, and a bounded ovn-sbctl lflow-list. Diagnostic-only: it runs after the existing crust-gather collect and only on an already-failed test, never mutates the cluster, and cannot change the pass/fail outcome. Every live capture is time-boxed, every command is || true, the whole run has a wall-clock backstop, node-global captures are deduped per node, and the pod count is capped. It no-ops cleanly when no pods are affected or when a tool/pod is absent. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Myasnikov DaniilandClaude Opus 4.8
|
5653c30799 |
fix(e2e): bound per-test crust-gather snapshots with a timeout
The per-test failure snapshots (host in cozytest.sh, tenant in run-kubernetes.sh) ran `crust-gather collect` unbounded. On a contended/degraded cluster the collect can hang indefinitely — observed wedging the E2E install step for ~2h35m after a test failure until the job-level cancel, with no output after the capture line. `|| true` only catches a bad exit code, never a hang. Wrap both invocations in `timeout -k 30 300`: a 5-minute cap (hard SIGKILL 30s later if the collect ignores SIGTERM) bounds the hang; a partial snapshot still beats a multi-hour wedge, and timeout's exit 124 is already swallowed by `|| true`. Assisted-By: Claude <noreply@anthropic.com> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Myasnikov Daniil
|
51496832e7 |
fix(e2e): capture tenant cluster snapshot from an in-subshell trap
crust-gather can only reach a tenant Kamaji cluster through the test's localhost port-forward (it connects directly to the kubeconfig server URL — there is no apiserver-proxy mode — and the in-cluster URL is unreachable from the runner). cozytest.sh's generic EXIT trap runs in the parent shell AFTER the failing test subshell has exited and its port-forward is reaped, so a collect there hits a dead socket and crust-gather bails before writing anything (every tenant snapshot so far came back empty). Capture the tenant from an EXIT trap registered INSIDE run_kubernetes_test instead: it fires during THAT subshell's exit, while the supervised port-forward is still alive. Verified end-to-end against the real hack/cozytest.sh under dash (CI's /bin/sh) in an ubuntu:22.04 container — the in-subshell trap fires on failure with the PF alive, the tenant snapshot lands in the same snapshots/<test>/ dir as the host snapshot, the generic cozy_cleanup still runs, and there is no pipeline deadlock. Drop the now-dead tenant loop from cozytest.sh's generic trap (it could only ever see a reaped PF); it keeps the host snapshot. Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Myasnikov DaniilandClaude Opus 4.8
|
50e5f94132 |
test(e2e): per-test crust-gather snapshots + cleanup-on-exit via cozytest.sh
cozytest.sh runs each .bats as one invocation, exits on the first failing @test, and never invokes bats setup/teardown. Add a single EXIT-trap hook: on failure, snapshot the host AND every nested tenant cluster (tenantkubeconfig-*) with crust-gather into per-app dirs BEFORE cleanup — so each failure is independently inspectable (crust-gather serve) instead of one end-of-suite host-only dump; then ALWAYS run the file's cozy_cleanup() if defined. run-kubernetes.sh gains cozy_cleanup() (delete tenant clusters, wait for PVCs to free): a failed kubernetes test otherwise leaves tens of GiB of worker-VM PVCs in tenant-test, exhausting the shared tenant-quota and cascade-failing every storage-heavy app after it. The kubernetes .bats now source run-kubernetes.sh at file scope so the parent-shell trap can reach the hook. Ship pinned crust-gather 0.15.2 in the e2e-sandbox image; fold per-test snapshots into cozyreport.tgz. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Aleksei Sviridkin
|
18961de36b |
[tests] Add pre-cleanup, fix port-forward race, fix temp leak
- Add pre-cleanup of stale resources to all app E2E tests so retries start fresh instead of patching stuck state - Wait for port-forward to be ready before using it in kubernetes tests (fixes race condition) - Reduce version check sleep from 5s to 1s for faster retries - Clean up temp directory on test failure in cozytest.sh Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Myasnikov Daniil
|
3bf43312aa |
(ci) Removed cozytest output trimming in non-tty run
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Andrei Kvapil
|
168a24ffdf |
[tests] Add more space for e2e tests
Signed-off-by: Andrei Kvapil <kvapss@gmail.com> |
||
|
Andrei Kvapil
|
2bf4032d5b |
[tests] Introduce cozytest - a new testing framework
Signed-off-by: Andrei Kvapil <kvapss@gmail.com> |