mirror of
https://github.com/cozystack/cozystack.git
synced 2026-11-08 21:14:56 +09:00
Aleksei Sviridkin
db590b493c
<!-- 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 -->