fix(e2e): distinguish a HelmRelease teardown from a retried failure and cover the tenant addons - #3594

What this PR does

The remediation guard read a failed and an uninstalled Snapshot in .status.history as one signal. That holds under Flux's default RemediateOnFailure, where a failed install is remediated by uninstalling the release, and it is wrong under RetryOnFailure, where the failed attempt keeps its manifests applied and is retried as an upgrade: there nothing was removed. The aggregated apiserver stamps RetryOnFailure on the HelmRelease of every Application it serves (pkg/registry/apps/application/rest.go), on install and upgrade alike, and the tenant cilium and csi releases set it in the chart, so the reading reported a cycle on releases that recover by design, and it would have done the same to every addon the guard was about to cover.

helmrelease_has_teardown is the narrow reading: an uninstalled Snapshot, which is proof the release was removed and put back. helmrelease_has_remediation_cycle keeps its meaning and its breadth, since it also catches the upgrade remediation that rolls back rather than uninstalls, at the price of not proving anything was removed. Neither helper takes the release's configuration at all; the caller reads it and decides what the evidence is worth, which keeps each helper answering one question about one list of statuses.

With that distinction in place the coverage reaches past the parent release. cozy_guard_helmrelease judges one release; cozy_guard_addon_helmreleases runs it over the addon HelmReleases the parent installs, because the parent's own history says nothing about them: a teardown of the tenant CNI or CSI otherwise surfaces only as whatever downstream deadline it eventually breaks. The parent block goes through the same function, so its reads are covered by the same unit tests instead of sitting inline in run_kubernetes_test, where a revert of them stays green. The addons are read from the cluster rather than from a copy of the chart's addon set, so a release added later is covered without touching the script, and a prefix that matches nothing is an error rather than a quiet pass over nothing.

What fails a run

One rule, and it takes two fields to state: the release was removed although nothing in its own configuration would remove it to recover. An uninstalled Snapshot is proof of removal. The install strategy is the first field, because RetryOnFailure retries in place and never uninstalls. The retry budget is the second, and it reaches the releases the strategy does not: the default strategy uninstalls only between install attempts it is allowed to take, since InstallRemediation.RetriesExhausted is true once the failure count passes .spec.install.remediation.retries and MustRemediateLastFailure defaults to false, so at a budget of zero no attempt and therefore no uninstall is ever made. An uninstalled Snapshot on such a release is exactly as unexplained as one on RetryOnFailure. The field is omitempty on an int, so a release configured with 0 and a release with no remediation block are the same bytes on the wire and mean the same thing here. No release of this chart is in that shape today, since all twenty addon releases set retries: -1, and the budget is read anyway because the addons are enumerated from the cluster precisely so a release added later is covered without editing the script.

Everything else is printed and named without failing: an uninstalled Snapshot on a release whose default strategy and non-zero budget explain it, and a failed Snapshot anywhere. Nothing in the suite measures how often those addons take their configured recovery path, and failing a 25-minute bringup on a release doing what it is configured to do is how a guard gets switched off. Those addons are configured differently from cilium and csi, which were moved to RetryOnFailure because their teardown removed something the cluster depended on; bringing the rest in line is a separate change from this one.

One read fails the run on the parent alone. .spec.install.strategy.name is read with kubectl -o jsonpath, which prints nothing and exits 0 for a path that stops resolving, so a rename of that field upstream would fail soft into the lenient branch and report every teardown, a real removal of the tenant CNI included, as an install remediation on a green run. An empty value is not by itself evidence of that, because eighteen of the chart's twenty addon releases set no strategy, but the parent always carries one, so an empty strategy there is the read rather than the object. Only the parent is asked for it, and that is enough: the same rename empties the field on every release at once.

The paths where the guard cannot run

The guard sits after the assertions, and every deadline whose breach it exists to explain, a node that never turns Ready, a HelmRelease wait that expires, a PVC that never binds, ends the script under errexit well before it. So on the runs where a teardown of the tenant CNI or CSI was the cause, the guard had nothing to say: the failure surfaced as the downstream deadline and the removal that preceded it was named nowhere. The node-join collector prints each release's Ready message rather than its .status.history, so that path was as blind as before the guard existed.

cozy_report_helmrelease_remediation classifies the same releases on those paths, prints the verdicts and fails nothing. It runs from the node-join collector and from the EXIT handler that covers every other failure. It fails nothing deliberately: returning non-zero from a collector on a failing path would replace the caller's own exit status, and the tenant crust-gather snapshot hangs off that status. It runs once per run, so the two call sites and the guard itself cannot classify one release twice. The enumeration is shared with the guard rather than copied, so a release the chart adds later appears in both readings or in neither.

Inside the node-join collector the classification runs after the guest serial console rather than before it, and the ordering rule is the evidence rather than the price. The console is the only artefact covering a worker that never reached apid, which is the dominant shape of that failure and one nothing else in the block describes; its measured worst case takes most of the phase budget, so anything ahead of it competes with it on exactly the slow runs it exists for. The classification reads objects on the management cluster that outlive the run, so a walk the budget declines can still be answered from those objects afterwards. Both suites' spend-order comments list it in that position, and the two guards over that order match calls by position inside the block rather than by name, since the same collector is also called from the EXIT handler, which belongs to no phase.

The handler is armed at the Kubernetes CR apply rather than once the tenant answers. Everything between those two points is a management-cluster wait on the parent HelmRelease or on what it installs: the KamajiControlPlane, the control-plane Deployments, the MachineDeployment, the admin-kubeconfig Secret. Those waits carry the largest budgets in the run, and that stretch is the window the guard was written for, since a helm-wait budget expiring while admin-kubeconfig is still being provisioned is what triggers the uninstall remediation the guard reads. Armed after those waits, the handler was absent from exactly the window it exists to cover. The parent HelmRelease exists from the apply onwards, rendered from the CR by the aggregated apiserver, so there is something to read the whole way, and arming this early costs the snapshot nothing: _tenant_snapshot_on_fail returns on an unset CURRENT_TENANT_KC, which is still assigned where the kubeconfig becomes usable.

The parent readiness wait the guard depends on now names what it timed out on. On elapse kubectl printed its own timeout line and nothing about the release, and the guard below it never ran, so the run reported a readiness timeout with no statement of what the release was stuck on. That branch collects the HelmRelease table, the parent's own detail and the namespace events, bounded like every other read on a failure path, before the exit that triggers the tenant snapshot. The exit handler classifies .status.history from there as well, but a release still mid-install has a Ready message and events that no history carries.

How the reads are made

Every probe keeps its exit status, because a read that failed and a release with no history both print nothing on stdout, and folding them together lets an API timeout pass for the latter. Neither folds kubectl's stderr into the value it captures, because kubectl writes warnings there while exiting 0: such a line inside the history capture turns an empty history into a populated one and skips the readiness branch, and inside the strategy capture it stops the value matching RetryOnFailure and downgrades a real teardown to a note.

Every read is bounded, by a --request-timeout and by a wall clock, and both are needed. The guard runs around minute 25 of the bringup on the passing path, and against a wedged apiserver an unbounded read there does not lose only itself: it holds the Chainsaw op until the op is killed, so the tenant snapshot the caller's exit triggers is lost rather than truncated. --request-timeout bounds one HTTP request, while a client retrying against an apiserver that keeps answering slowly stays inside it indefinitely, which is what the wall clock catches. Neither adds a retry. The read is issued once, and one that did not finish stays a failed read, which the callers report and fail on rather than folding into "no history". The failure-path classification carries a budget of its own on top, checked before the addon listing and before each release, because the listing is the wrong read to spend past a deadline: it buys a set of names for a walk with no time left to make. A release the budget cut off is named rather than dropped.

Readiness travels in the same read as the strategy and the retry budget, and all three are read before the history, because the other order observes them the wrong way round. The controller writes the Snapshot and flips Ready in one status patch, so a release is never Ready before its first Snapshot exists; read the history first and an addon that finishes installing in the round-trip between the two reads presents as Ready with an empty history, which the guard fails the run on as a status shape it can no longer read. Nothing waits on most of these releases, so that window would be open on every run. What the order buys is the truncation half of that: neither truncation variant in helm-controller/api can empty a non-empty history, since Truncate returns early below two Snapshots and TruncateIgnoringPreviousSnapshots cuts only above five, so which of them the controller picks does not matter here. It does not close the window entirely. HelmReleaseStatus.ClearHistory() drops history outright, and where the controller calls it is in a module this tree does not carry, so a call landing between the two reads would still present as Ready with an empty history. The claim is about truncation and not about the read order being safe. It also drops the guard from three reads per release to two.

Each note says only what was read. The retry budget is quoted in the teardown note, which is the note it explains, and not in the rollback one. Readiness decides the tense of the failed-Snapshot note, because a release that is not Ready carries its failure live rather than behind it, and a note claiming a recovery would send a reader after a second cause while the first one is still the answer.

An empty history is judged against that release's own Ready condition rather than the parent's, because the parent cannot stand in for it: the Kubernetes ApplicationDefinition sets release.cozystack.io/helm-install-disable-wait and the apiserver turns that into DisableWait on both actions, so the parent reaches Ready without waiting for anything it applied, and the suite waits on some addons by name while never naming others. Ready with no history is a status shape the helper can no longer read and fails the run; not-Ready with no history is a release that never completed an action, and it is reported as not inspected rather than failed or passed over in silence.

The guard adds no wait and no retry, and it runs before the tenant-snapshot trap is disarmed, so a teardown it finds still captures the tenant cluster. A test pins that wiring by reading the script, because with the guards behind functions, deleting the calls or moving them past the trap would otherwise leave every case in the file green while the run asserted nothing. cozy_guard_all_helmreleases collects the parent's verdict and the addons', because as two bare commands under errexit a failing parent ended the script before any addon was read, while the addon loop one level down keeps inspecting after a failure since the other notes explain it.

Two smaller corrections ride along in the same library. Both helpers assigned statuses without scoping it, and the library is sourced rather than executed, so calling either one left the argument behind in the caller's scope; nothing in this tree keeps release history under that name, which makes this hardening rather than the repair of an observed clobber, and is also why the scoping gets a case of its own instead of riding on the existing ones. And the comment said a failed or uninstalled Snapshot survives a later successful reconcile and stopped there, which reads as unconditional: the controller truncates history on every in-sync reconcile, keeping the newest five Snapshots on a release whose strategy is a retry and, on any other, those down to the previous deployed or superseded one. Nothing in the guard changes, because a single run applies its release once and never upgrades it afterwards, but a caller reading history further from the action is reading a window and the text says so now.

Tests and docs

hack/remediation-guard.bats gains cases for the two readings and their scoping. hack/run-kubernetes-remediation_test.bats is new, with kubectl answered from fixtures, covering the enumeration, the literal prefix match, the read order, the readiness branch, both fatal conditions and the wording of every note, the probe-failure paths, the arming point of the EXIT handler and the status it hands to the snapshot, the failure-path classification and its budget, the one-classification-per-run flag, the diagnostics on the parent readiness wait, and the wiring of the guard calls. The classification budget is a knob, so it joins the two pins the file's other knobs already have: the loop in hack/run-kubernetes-node-join_test.bats that rejects a unit suffix and a leading zero, and a case of its own for the zero that would decline the walk rather than bound it, at both the environment and the post-source path. The wiring pin requires the matched line to end at the call, because the verdict reaches the run through errexit rather than an exit of its own and a substring match would stay green against an appended || true; indentation is left free so that wrapping the call in a block stays a refactor. The ordering pin in hack/run-kubernetes-serial-console_test.bats moves onto the call it anchors on, off the text of its arguments, which name library locals this change renames, and onto the control-plane wait by name rather than the file's first kubectl_wait_retry, since a file-global match would compare two unrelated line numbers as soon as a wait is added to any function defined above.

docs/agents/e2e-testing.md is updated where it described the guard as parent-only and the footprint as surviving indefinitely, where it stated the teardown rule as the strategy alone rather than the two fields it is, and where it placed the EXIT handler at the point it used to be armed. The bullet answering which releases can red a run now names every condition that does, not the teardown rule alone. Item 8 names where the policy lives as well as where the predicates do.

Two open issues affected without being closed

The global assignment reported in #3774 goes away with the local added to both helpers here; the sweep that issue also asks for over the other _lib/ helpers is not done, and etcd-cleanup.sh, etcd-probe.sh, ghcr-mirror.sh, pod-label-census.sh and talos-image-cache.sh are not touched by this change at all.

#3773 has two halves. The self-refuting comment it quotes is rewritten in full, dropping the opening sentence claiming a non-zero installFailures/upgradeFailures means something is gone, and what replaces it says those counters are exactly what the guard does not use. Of the other half, the kubectl wait calls in the file with no event-driven backstop, this change reaches one: the parent HelmRelease readiness wait the guard depends on, which now dumps the release state before exiting. Six such waits remain, and the issue stays open for them.

Closes #3553.

Open questions

Two things the reviews disagreed on. Not decided in this PR.

The parent's failed Snapshot is a note now instead of a red run, with no measurement behind the change. The mechanism is right: under RetryOnFailure a failed Snapshot is a retried attempt that kept its manifests. What nobody showed is that the old check ever fired falsely. The apiserver has stamped RetryOnFailure on Application HelmReleases since 5e148f7 (2026-05-06) and the fatal-on-failed guard landed in the Chainsaw suite in e22d84f (2026-06-08); no run in those two months was named where it fired. That is the same unmeasured pass condition this PR refuses for the addons. One review suggested splitting the rule by release class: failed fatal on the Application-served parent, a note on the chart-rendered addons. Not done here.

Fail-fast API reads went from one to one listing plus two per release. Each kubectl get reds the whole run on any non-zero exit, and they land around minute 25 of the bringup. They are bounded now, by a request timeout and a wall clock, but no retry is added: docs/agents/e2e-testing.md limits retry to steps with no product or test logic, and a HelmRelease status read is not that.

Screenshots

Not a UI change.

Downstream repositories

Walked the trigger map against the diff: it touches hack/e2e-chainsaw/_lib/run-kubernetes.sh and remediation-guard.sh, adds one hack/*.bats, extends two others, re-anchors a pin in a fourth, adds one line to each of the two kubernetes suites' spend-order comments, and edits docs/agents/e2e-testing.md. Nothing under hack/ is moved or renamed, no make target changes what it does, and no package, schema, CRD or user-facing document is touched, so none of the listed repositories is reached.

• No downstream repository is affected by this change
• cozystack/website - follow-up:
• cozystack/terraform-provider-cozystack - follow-up:
• cozystack/ansible-cozystack - follow-up:
• cozystack/ccp - follow-up:
• cozystack/talm - follow-up:
• cozystack/cozyhr - follow-up:
• cozystack/cozy-proxy - follow-up:
• cozystack/cozystack-telemetry-server - follow-up:
• cozystack/external-apps-example - follow-up:
• cozystack/examples - follow-up:

Release note

fix(e2e): the remediation guard now covers the tenant addon HelmReleases as well as the parent release, and classifies them on the failure paths where the guard itself cannot run.

It fails a run only when a release was uninstalled with nothing in its own configuration to explain the removal, and reports every other remediation footprint without failing.