Reconcile the Chainsaw e2e migration with ~500 commits of main drift: - keep main's run-kubernetes.sh helpers (kubectl_wait_retry, tenant drain, LINSTOR pool-free wait, SC-fallback probe) under the branch's _lib/ rename with the Chainsaw finally/snapshot deltas re-applied - resolve modify/delete on 10 drifted hack/e2e-apps/*.bats as deleted; their post-branch assertion drift is re-ported into the Chainsaw suites in follow-up commits - drop the 4 new main-side bats (kubernetes-oidc-*, securitygroup, serviceexposure) from _lib/ relocation; ported as Chainsaw suites in follow-up commits - docs/agents/e2e-testing.md: merge main's plan-job TIA wording with the Chainsaw suite phrasing Assisted-By: Claude <noreply@anthropic.com> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
14 KiB
E2E Testing Conventions
Guidance for writing, changing, and reviewing Cozystack's end-to-end (E2E) tests and the CI that runs them. Read this before touching anything under hack/e2e-chainsaw/, the bootstrap/OpenAPI BATS (hack/e2e-*.bats), hack/*.sh test helpers, the E2E CI workflow (.github/workflows/pull-requests.yaml), or packages/core/testing/.
The app suite is Kyverno Chainsaw (hack/e2e-chainsaw/, one directory per app, each with a chainsaw-test.yaml). Cluster bootstrap (hack/e2e-install-cozystack.bats, hack/e2e-prepare-cluster.bats) and the OpenAPI checks (hack/e2e-test-openapi.bats) remain BATS.
The core principle
Retries do not recover flakes — they hide deterministic bugs and triple diagnostic wall-time. An audit of 30 PR runs found that across sampled failures, 25/25 retry attempts failed: the retry loop never once turned a red run green, it only delayed surfacing real bugs (Helm namespace conflicts, seaweedfs/harbor races, a vminstance disk race) that looked like flakes because the retry sometimes coincided with transient state clearing.
Every convention below follows from that finding: fail fast, fail loud, make the failure legible. A test that flakes is a test (or a product) with a real race to fix, not a test to wrap in a retry. Chainsaw's per-assertion polling is the structural expression of this: each assert waits on its own condition to its own timeout, and a failure is reported as a structured diff plus auto-captured events/describe/logs — not a generic non-zero exit.
Conventions
1. No retries on deterministic steps; retry only pure infrastructure
Run E2E testsandInstall Cozystackrun once. On failure they print❌ ... (no retry — see diagnostics below)and dump state. Do not reintroduce a retry loop around them. The E2E step is a singlechainsaw testinvocation — there is no per-app retry loop to bring back, because Chainsaw already polls each assertion to its own timeout.Prepare environmentkeeps its 3× retry — and only it — because it is pure infrastructure (Talos image download, sandbox VM boot, network) where a transient runner hiccup is a genuine flake with no application logic involved.- Rule of thumb: retry is justified only for a step that contains no product or test logic.
2. Prefer declarative asserts over imperative waits
A Chainsaw assert polls until the resource matches its expected shape or the per-operation timeout fires, so it subsumes both "the resource exists" and "the field/condition reached this value" in one operation — the old until kubectl get …; do sleep 2; done existence backstop followed by kubectl wait --for=… is no longer needed.
- assert:
timeout: 6m
resource:
apiVersion: helm.toolkit.fluxcd.io/v2
kind: HelmRelease
metadata:
name: postgres-test
status:
(conditions[?type == 'Ready']):
- status: "True"
- Condition checks use the filter-as-list form
(conditions[?type == 'Ready']). Chainsaw v0.2.15 throws "field not found" on the indexed(...)[0]form. - Imperative checks that no Kubernetes API condition models — S3 reachability through a port-forward, an external LB HTTP path, MetalLB advertisement — stay in a
scriptstep. A baresleep Ninside such a script is allowed only when nothing better models the wait; annotate it with aTODO(e2e-replace-fixed-timeouts):comment. The bootstrap BATS (hack/e2e-install-cozystack.bats,hack/e2e-chainsaw/_lib/run-kubernetes.sh) carries the sanctioneduntil kubectl getexceptions.
3. Let Chainsaw own cleanup; no test-level EXIT/RETURN traps
Trap-based cleanup was the single biggest source of false failures in the BATS suite: an EXIT trap ran cleanup in a context where shell variables were unset, set -u killed it, and a successful test was marked failed — then retried twice into real failures.
- Chainsaw deletes the resources it
apply-ed during its cleanup phase (bounded by thedelete/cleanuptimeouts inhack/e2e-chainsaw/.chainsaw.yaml). Do not hand-roll teardown for resources Chainsaw created. - A self-contained
trap '… ' EXITinside a singlescriptstep — to kill a port-forward or remove a temp dir — is fine, because it runs in a contained subprocess with its variables in scope. Seehack/e2e-chainsaw/bucket/chainsaw-test.yaml. What is banned is test-level trap-based cleanup of the BATS kind.
4. Do not mask cleanup or teardown failures
- Resources a test creates that Chainsaw cannot reclaim — because a controller, not the test, owns them (e.g. the Velero
Backup,BackupStorageLocation, and credentials secret incozy-veleroin the disabledhack/e2e-chainsaw/backup/suite) — must be pruned explicitly. Do not|| trueover a stuck delete and leave stale state for the next run. - For nested tenants, tear down child → parent with a hard wait for deletion between each. Deleting the parent while a child is still uninstalling wedges the parent's cleanup Job on the child namespace, and both stuck uninstalls occupy helm-controller workers past the end of the test — starving whichever suite runs next. The kubernetes suites encode this ordering in
hack/e2e-chainsaw/_lib/run-kubernetes.sh.
5. The install gate must have teeth
The install test fails if any HelmRelease is not Ready. A toothless gate (backgrounded kubectl wait discarding child exit codes + a bare echo instead of exit 1) once shipped a permanently-failing platform HR through green CI for weeks.
- Use a single
kubectl wait hr --all -A, then an outcome-based re-list (covers HRs created after the snapshot), andexit 1on any non-Ready HR. - Dump the full Ready-condition message per non-Ready HR so the real error is in the test output. See
hack/e2e-install-cozystack.bats.
6. Assert the parent HelmRelease did not remediate — via status.history
A parent HelmRelease that hit its wait timeout, uninstalled, and reinstalled is a silent race we want to catch. Do not check .status.installFailures / .status.upgradeFailures: Flux's ClearFailures() zeroes those on every successful reconcile, so checking them after the HR is Ready is vacuous and passes against a reverted fix.
- Inspect
.status.historyinstead — afailedoruninstalledSnapshot survives a later successful reconcile. Use the shared helper inhack/e2e-chainsaw/_lib/remediation-guard.sh(helmrelease_has_remediation_cycle) from ascriptstep.
7. Test-Impact Analysis (TIA), default-on
hack/select-e2e.sh walks the packages/core/platform/sources/*.yaml dependency graph and runs only the Chainsaw suites affected by a diff. A suite is a directory under hack/e2e-chainsaw/ containing a chainsaw-test.yaml.
- Conservative escalation: edits to
packages/library/,packages/core/,api/,cmd/,internal/, top-levelhack/*.sh|*.batshelpers, the sharedhack/e2e-chainsaw/_lib/, the chainsaw confighack/e2e-chainsaw/.chainsaw.yaml, theMakefile, or the E2E workflows escalate to the full suite. A per-suite edit (any file underhack/e2e-chainsaw/<app>/) selects only that suite. - The
full-e2ePR label forces the whole suite. - The selection is passed to
make test-chainsaw CHAINSAW_SUITES="<names>", which runschainsaw test <names>fromhack/e2e-chainsaw/(empty = the whole suite). - The coverage gap TIA opens on PRs is closed at release-cut time: the "Prepare release" commit bakes image digests into
packages/core/(and other packages), which TIA escalates to the full suite — so the release PR exercises the whole suite against the commit the tag will point at.
When adding a new app package, confirm select-e2e.sh maps it correctly (it has a unit test, hack/select-e2e_test.bats). A new app whose suite directory does not yet exist will not be selected — add hack/e2e-chainsaw/<app>/chainsaw-test.yaml first.
What TIA does and does not do
Be precise about TIA's scope — it is narrower than "skip E2E for unrelated PRs," and the wiring has consequences worth knowing before you rely on it or change it.
- It trims only which suites
chainsaw testruns, not the expensive stages.Prepare environmentandInstall Cozystack(the full platform install) run regardless of the selection. TIA only decides theCHAINSAW_SUITESpassed to the final E2E step. A perfect narrow saves the suite tail, not the bulk of wall-time. (Package image builds are scoped separately — byhack/build-matrix.sh, on the PR diff — while the Talos and installer/finalize legs still run for non-docs PRs; that is a distinct mechanism from TIA's suite trimming.) Select E2E testsruns afterInstall Cozystackand has noalways(). GitHub implicitly ANDs a step'sif:withsuccess(), so if install fails the selector (andRun E2E tests) are skipped entirely. On a red install you will not see TIA narrow anything — that is the ordering, not a selector bug.- Most PRs legitimately escalate to the full suite. Platform manifests live under
packages/core/, Go underapi//internal/, helpers underhack/, plus theMakefile— touching any one triggers the full suite. So "all suites selected" is usually correct, not a failure to narrow. - Two skip layers, with a gap. The
planjob computes a coarse docs-only gate (code, true for any change outsidedocs/**) that skips the whole build+E2E pipeline for docs-only PRs, plus a per-package build matrix (hack/build-matrix.sh) that scopes which images build. TIA's finer skip (select-e2e.shignoringdocs/,dashboards/, and*.md) only trims the suite selection. A PR touching onlydashboards/*.jsontherefore still runs install, then skips just the Chainsaw suites — near-zero savings. If you need such PRs to skip the pipeline, widen theplandocs-gate, notselect-e2e.sh.
8. Failure must be self-explanatory — diagnostics are first-class
- On install failure, the CI step dumps
kubectl get hr -A -o wide, adescribeof each non-Ready HR, and sorted events, all under collapsible::group::blocks. - On E2E failure, the CI step dumps
kubectl get hr -A, sorted events, and COSI bucket/claim/access readiness columns (those CRDs ship no printer columns, so plaingetshows only NAME/AGE). Thechainsaw testrun is one step, so this dump is at run level; Chainsaw's own per-Testcatchblocks add scoped events/describe/podLogs for the failing suite. - Inside a suite, attach scoped diagnostics with a top-level
catch:block (events,describe,podLogs,get) rather than letting a bare assert failure through — seehack/e2e-chainsaw/bucket/chainsaw-test.yamlandhack/e2e-chainsaw/harbor/chainsaw-test.yaml. This replaces the BATS... || { echo …; kubectl get/describe …; false; }idiom. make collect-report→hack/cozyreport.shuploads a fullcozyreport.tgz(operator/Flux/cert-manager logs, LINSTOR, Kamaji, Talos dmesg, COSI YAML) on every run, and the Chainsaw JUnit report (chainsaw-report.xml) is uploaded as its own artifact.- Per-failed-test crust-gather snapshots are the richest diagnostic — a
crust-gather serve-able archive of the cluster at the moment of failure, before cleanup. The globalerror.catchinhack/e2e-chainsaw/.chainsaw.yamlcaptures the host cluster into_out/cozyreport/snapshots/<test>/hoston any failure (the Chainsaw analog of thehack/cozytest.shEXIT-trap snapshot used by the BATS tests); the kubernetes suites additionally snapshot their tenant cluster fromhack/e2e-chainsaw/_lib/run-kubernetes.shwhile the tenant API LB is still routable.hack/cozyreport.shfoldssnapshots/into the artifact. Do not remove the global catch — without it the Chainsaw suite uploads a report with no per-test cluster state.
9. Keep the test environment deterministic
- Pre-pull every kubeovn / linstor / cert-manager image onto all nodes before those HelmReleases install (
hack/e2e-prepull-images.sh), so clustered workloads (OVN raft, LINSTOR, the cert-manager webhook) do not fail on per-node image-pull stagger. - Exclude loop devices from host LVM scanning in the Talos machine config so the host does not activate volume groups inside loop-mounted e2e disk images.
- Fail fast on node readiness (≈5m, then bail) rather than marching into LB/NFS tests that will also fail — it saves several minutes per attempt and keeps the real failure at the top of the log.
Reviewer checklist for a new or changed E2E test
- No new retry loop unless the step is pure infra (image pull / VM boot / network).
- Resource readiness uses a Chainsaw
assert(not an imperativeuntil kubectl get …; kubectl wait); condition checks use the filter-as-list form(conditions[?type == 'Ready']). - Imperative-only waits live in a
scriptstep; any baresleepcarries aTODO(e2e-replace-fixed-timeouts):justification. - No test-level
EXIT/RETURNtrap — rely on Chainsaw cleanup; a self-contained trap is allowed only inside a singlescriptstep (port-forward / temp dir). - Controller-created artifacts the test cannot reclaim are pruned explicitly; nested tenants delete child → parent with a wait-for-deletion between.
- Standard HR-Ready assert timeout is 5–6m; longer waits (harbor 10m, NFS 10m, VM image pulls, platform-wide install 15m) are justified in-line.
- Failure path attaches scoped diagnostics via a
catch:block, never a silent pass. - If it touches parent-HR behavior, add the
status.historyremediation guard (hack/e2e-chainsaw/_lib/remediation-guard.sh).
In-flight direction (not yet the merged standard)
These are being explored on branches and may become conventions; do not assume they are the current main behavior:
- Cilium orphaned-endpoint self-heal — an interim CI watchdog that evicts a single confirmed-orphan Cilium endpoint ("IP already in use", cilium/cilium#38313). Explicitly a mitigation to remove once a fixed Cilium ships; it refuses to touch an endpoint backing a live pod so real duplicate-IP bugs stay visible.
- Cluster state snapshot/restore between test groups instead of reinstalling.