fix(e2e): bound every read in the report collector and the step that runs it - #3596

What this PR does

A single hung read in hack/cozyreport.sh did not truncate cozyreport.tgz, it lost the artifact outright. The collector writes its tarball on its last line, the Collect report step carried no timeout-minutes of its own, and Upload cozyreport.tgz is the step directly after it — so a read that never returned spent whatever was left of the job budget, the job was cancelled, and the upload never ran. The runs where the report is the only evidence left are the runs that produced none. || true guards against a non-zero exit, not against a read that does not return.

Two halves, independent of each other.

The step gets a ceiling of its own, 30 minutes, in all three workflows that run it. The number is measured against the step rather than divided out of the job: pulling the Collect report timings out of recent e2e runs gives 3 to 42 seconds, on failed jobs as much as on green ones, and 30 minutes leaves five sixths of the job budget for the steps that upload what this one produced. It carries continue-on-error: true beside it, because the step's || true is its standing statement that collecting diagnostics is never fatal and a step killed at its ceiling never reaches that || true — without the tolerance, a passing run whose collection ran long would be published as failed, and every later failure() in the job would start meaning "the tests failed or the report was slow". That tolerance also renders the overrun green, so a following step turns the collect step's outcome into a ::warning:: annotation: a bound that fires is recorded, and the outer bound is not exempt from the rule the inner ones follow.

Every read in the collector is bounded, against four sections before, and gates and host commands are reads too. An if kubectl get deploy … that decides whether a module runs costs more unbounded than anything it guards, because the module below it never starts; df -h blocks indefinitely on a stale mount and takes the tarball with it by exactly the mechanism above. A gate cut off short is recorded, since skipping the module leaves the same empty tree as a cluster that never ran the component, while a gate answering "not installed" is recorded nowhere — that is the cluster answering correctly, and a marker firing on every install without KubeVirt is one a reader learns to skip.

The six LINSTOR reads run a command inside a pod and go through a reader of their own. They are the only reads whose duration a workload sets rather than the apiserver, and kubectl exec returns the remote command's exit status — a 124 there is either the collector's deadline or a command that itself exited 124, so the existing classifier would have named this script's own timeout as the cause of something that may have happened inside a pod. The new note states what the status leaves open, and sits beside the output rather than inside it, since two of the six stream a gzipped tarball. A cut-off bundle is kept rather than dropped, and the note says so: the emptiness check at those call sites asks whether the archive lists a first member, which a truncated gzip does.

What this does not do is bound the report. The pod, VM, VMI and services walks carry a count cap and a time budget; every other per-object walk carries neither, so their cost is the cluster's to set, and a failed install with a hundred not-Ready HelmReleases spends four hundred bounded reads in one of them. A per-read ceiling removes the read that never returns, not the thousand reads that each return slowly. The file header says so where it previously implied the opposite, and the step ceiling is what covers the rest.

Two frozen lists in the tests are gone, replaced by one invariant derived from the code: no read below the library guard runs without a ceiling. Both lists were already wrong. The header named four bounded sections while the file had ten, and the per-section counts of unbounded reads only ever matched a redirect into $VAR/, which no gate, selector or command substitution has — so they described the scan rather than the file.

The guard that held the old exec assumption was satisfied by leaving every exec unbounded, which is the state the file was in. It now also requires each exec to be bounded, and requires that reader not to borrow the classification written for reads whose 124 is unambiguous. Matching on the verb rather than on the string kubectl exec is part of it: half of these calls spell it kubectl -n cozy-linstor exec, which that string never saw.

Two properties worth knowing before this lands. kubectl get events -A --sort-by=.lastTimestamp sorts client-side, so kubectl buffers the whole event list and writes nothing until it finishes — under a flat per-read ceiling a cut-off there leaves a file holding only the marker, not a short tail. That read had no ceiling at all before, where the same slowness could take the whole tarball, so this is a narrower failure rather than a new one. And kubectl logs -n cozy-kamaji deployment/kamaji gains --tail=2000, matching every other controller log in the file: it was the only unbounded-in-size log, which under a wall-clock ceiling is the one most likely to be cut at its end, where a control-plane bootstrap failure sits. Longer kamaji history than 2000 lines is no longer collected.

Two accepted imperfections in how the change is split across commits, named rather than left to be found. docs/agents/e2e-testing.md keeps this material in a single shared paragraph that all three commits touch, so in the middle commit one sentence about the module gates lands a commit ahead of the gates themselves. And two corrections to the exec reader — it now keeps the remote command's whole message in the listing rather than quoting the line kubectl exec always appends last, and its cut-off note names both of the two things that can happen to a partial bundle instead of asserting one — sit in the last commit rather than in the one that introduced that reader. Both are true of the branch tip, where every statement is correct and appears once; the cost of moving them is another rebase across the one shared paragraph, which is not worth what it buys.

Refs #3568.

Screenshots

Downstream repositories

• 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): every read in the diagnostic report collector now carries a wall-clock ceiling — module gates, the reads that run a command inside a LINSTOR pod, and the host commands included — and a ceiling that fires is recorded in the artifact rather than only in the job log. The workflow step that runs the collector carries a ceiling of its own, so a read that does not return costs one file and a warning annotation instead of consuming the job budget and losing cozyreport.tgz entirely.

Summary by CodeRabbit

• New Features

  • Added bounded diagnostic collection across Kubernetes, Talos, LINSTOR, host, controller, and command outputs.
  • Reports preserve partial results and identify timeouts, failures, missing output, and truncated artifacts.
  • Added warnings when report collection fails or exceeds its time limit.

• Bug Fixes

  • Corrected resource targets, selectors, deployment extraction, and command matching.
  • Improved handling for empty selections, unavailable resources, and missing timeout tools.

• Documentation

  • Expanded guidance for diagnostic markers, collection limits, timeouts, and incomplete output.

• Tests

  • Added coverage for bounded reads, failure reporting, markers, selectors, logs, and subprocess mocking.