admin/cozystack
1
0 Fork 0
mirror of https://github.com/cozystack/cozystack.git synced 2026-10-30 00:27:04 +09:00
Code Issues Packages Projects Releases Wiki Activity
5,365 Commits 537 Branches 294 Tags
cc522e7def80a1852fb2b6e976c5af6817f04708
Commit Graph
749 Commits
Author SHA1 Message Date
Aleksei Sviridkin cc522e7def fix(ci): classify the last path when the list has no trailing newline 
The classifier this branch introduces is only total if every line
reaches the loop that applies it. POSIX read assigns the final line and
then returns non-zero when the input does not end in a newline, so a
plain `while read` drops it. The fall-through that escalates an
unrecognised path lives inside the loop body, so the dropped line is
never classified at all and the selection comes back empty. Both lanes
read empty as "skip Chainsaw" and post the required status green.

The shape that matters is a list whose only unclassified path is last:
`docs/x.md` followed by an unrecognised path with no terminator selects
nothing, where the same list with a terminator escalates to all 21
suites. The safety net this branch is built around is bypassed by
exactly the input it exists to catch.

No caller triggers it today, because `git diff --name-only` terminates
its output and the fork lane appends a newline explicitly. That makes it
caller discipline documented in one of two callers, which the next
caller will not read. Guard it where the assumption lives.

Both new cases are red without the guard and the twenty-five existing
ones stay green, so they pin this behaviour rather than the loop's
general shape. They clean up inline instead of through an EXIT trap:
that trap replaces the one the bats binary installs for its own
bookkeeping, and a test failing under it can print no TAP line at all,
which is the opposite of what a regression pin is for.

Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
 2026-08-08 04:47:27 +03:00
Myasnikov Daniil ae4e9e55cb fix(ci): classify every TIA path and escalate the unclassified 
`select-e2e.sh` silently selected nothing for a changed path it did not
recognise outside `packages/`. Both e2e lanes read an empty selection as
"skip Chainsaw" and then post the required "E2E Tests" status green, so
an unrecognised path produced a green merge gate with no suite run. The
worked example is `hack/common-envs.mk`, which supplies the tag, push
and output flags of every image in the tree and matched nothing:
`full_suite_pattern` covered `hack/*.sh` and `hack/*.bats` but not
`hack/*.mk`. `pkg/`, `tools/`, `go.mod`, `hack/lib/` and
`hack/buildkitd.toml` were in the same position, as was
`.github/workflows/e2e-fork.yaml` while `pull-requests.yaml` was
already escalated.

A blanket "empty means full suite" fallback would be the wrong fix: it
escalates the inert paths the design deliberately skips. Instead every
path now lands in one of three classes by an explicit rule — the
full-suite list for shared build inputs and the e2e harness, an inert
list for paths that cannot affect what e2e exercises, and the
PackageSource graph for `packages/(apps|system|extra)/`. A path
matching none of them escalates and logs why, so the next path someone
adds fails safe instead of failing open.

The two lists are checked in that order, so a specific escalation beats
a broad inert directory: `.github/` is inert, the workflows that run
the suite are not. `*.md` and `docs/`/`dashboards/` stay ahead of both,
so a README under `packages/core/` does not inherit that tree's
escalation.

Ten tests cover both sides of the classification. Mutation-checked:
reverting the fall-through fails the unclassified-path test, dropping
`.github/` from the inert list fails the non-e2e-workflow test, and
swapping the two list checks fails the existing pull-requests.yaml
test. Note the per-entry escalations are belt-and-braces — removing
`hack/*.mk` from the pattern still escalates via the fall-through,
which is the point of having it.

Closes #3392

Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
 2026-08-08 04:28:27 +03:00
myasnikovdaniil 7dca642894 fix(ci): restore e2e for fork PRs and close the merge-gate bypass (#3262) 
## What this PR does

Fixes #3257 — fork PRs never ran e2e, and the skipped required check let
them merge anyway.

Fork PRs get no secrets on `pull_request`, so `make image` pushed
anonymously and died with `denied`. That failed every build, collapsed
the `build → finalize → e2e` chain, and left the required **E2E Tests**
job *skipped* — which branch protection treats as satisfied, so an
approved fork PR could merge with e2e never having run.

This implements **Option A** from the issue (privileged `workflow_run`
split), pushing fork e2e images to the existing CI registry (OCIR):

**Build primitive (`hack/common-envs.mk`)**
- New `OCI_EXPORT_DIR` mode: build each image to a per-image OCI archive
instead of pushing, and force `PUSH`/`LOAD` off. The digest is captured
via `--metadata-file` regardless of output type, so the refs baked into
`pr.patch` match what the privileged run later pushes. Threaded through
the `image-tags` macro every package already uses — no per-package
Makefile change.

**`pull-requests.yaml` (unprivileged, fork branch)**
- `build` / `build-talos` / `finalize` export images to OCI archives and
upload them as artifacts instead of pushing → the red `denied` build
wall disappears and the chain no longer collapses.
- The in-tree e2e job is renamed off `"E2E Tests"` and guarded to
same-repo PRs. A new `e2e-report` job concludes the required `"E2E
Tests"` context as an **explicit check-run** (success on a docs-only PR
or when e2e passed, failure otherwise). A skipped job can no longer
satisfy the required check — this also closes the same hole for
same-repo PRs whose build fails.

**`e2e-fork.yaml` (new, privileged `workflow_run` from the default
branch)**
- Resolves the fork PR, opens a pending `"E2E Tests"` check-run, and
fails-closed if the fork build failed.
- Pushes the pre-built image archives to OCIR **by digest** and
flux-pushes the digest-pinned packages tree. The OCIR token is live only
for trusted binaries (`git`/`skopeo`/`flux`/`yq`) over data — never for
`make` or any fork-authored script.
- Runs the e2e suite against the pushed closure with **no push token**
and concludes the check-run on the PR head SHA.

Fork build code never receives registry credentials; the fork's images
and test scripts run only in the credential-less, ephemeral e2e job — a
stronger posture than a `pull_request_target` run that would hand fork
code the secrets directly.

### Validation

Local: buildx OCI-archive output + digest capture, macro expansion for
single- and multi-image packages, non-fork push path unchanged,
`actionlint` clean.

Needs live validation (why this is a **draft**): `workflow_run` artifact
download across runs; that the `"E2E Tests"` commit status satisfies the
required context and keeps a fork PR non-mergeable until the privileged
run concludes it; `refs/pull/<N>/merge` fetch + `git apply --3way`;
`skopeo --preserve-digests` end-to-end. `workflow_run` only runs from
the default branch, so the privileged half is exercised by mirroring the
workflow onto a personal fork's default branch and opening a throwaway
fork PR into it — not by merging first.

_Since review_: the fork `e2e` job ran the pre-#2826 BATS app loop that
the merged Chainsaw migration deleted (already broken); it now runs the
same Chainsaw + TIA path as the in-tree job, with suite selection and
the `full-e2e` override sourced from `resolve`'s trusted base-repo file
list / labels (not a merge-ref diff). Publish job hardened per the
non-blocking follow-ups: base-tree image allowlist, `skopeo copy --all`,
pinned flux CLI. The required `E2E Tests` gate is now published as a
commit **status** (default token + `statuses:write`), not a COZYSTACK_CI
app check-run — the app lacked `checks:write` (`checks.create` 403'd)
and a default-token check-run floated under a labeler suite; a commit
status has neither problem, and forks can't post one (their
`pull_request` token is read-only). Two review-bot findings fixed too:
fork `OCI_EXPORT_DIR` is now absolute (a relative path misplaced
archives under `make -C <pkg>` and every fork build would fail at
upload), and the "≥1 archive" guard no longer no-ops under `nullglob`.

### Repo-admin prerequisites

One repository setting lives outside this diff:

1. **Branch protection: keep `E2E Tests` required.** The gate is now a
commit **status** named `E2E Tests` (posted with the default
`GITHUB_TOKEN` — see `docs/agents/e2e-testing.md` §10), not a job or an
app check-run. Branch protection already requires `E2E Tests` (currently
the e2e job's name, renamed here to `E2E (in-tree)`), so the required
context now resolves to the status — **no GitHub App, no `checks:write`
grant, no app-id pin needed**. Just don't pin the context to a specific
app.
2. **Actions → "Require approval for all outside collaborators".** Each
fork run must be a deliberate maintainer click before it executes, since
the fork's unprivileged run produces the artifacts the privileged
`e2e-fork.yaml` consumes.

### Downstream repositories

Walked the trigger map in `docs/agents/contributing.md` against the diff
(7 files: two CI workflows, `docs/agents/e2e-testing.md`,
`hack/common-envs.mk` + its bats suite, and two package `Makefile`s).

The one trigger worth naming is "change developer tooling
(`cozyvalues-gen`, `cozypkg`, the package `Makefile`s) →
`development.md`" on the website. It does not fire here:
`OCI_EXPORT_DIR` is a CI-only build mode that defaults off, and the
local developer workflow is unchanged — `make image` still builds and
pushes exactly as before, which the new default-path case in
`hack/common-envs_test.bats` now pins. No chart values, no API surface,
no app list, no platform variant, no Talos version bump and no release
asset rename are touched.

- [x] No downstream repository is affected by this change

### Release note

```release-note
fix(ci): fork pull requests now run e2e via a privileged workflow, and a fork PR can no longer merge on a skipped (never-run) required E2E Tests check
```

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

- **New Features**
- Added a dedicated fork E2E workflow that runs after the base PR and
reports results via the required **E2E Tests** commit status.
- Fork PRs now export build images as OCI archive artifacts for later
publishing.
- Added a same-repo E2E reporting job and renamed the in-tree E2E flow
to **E2E (in-tree)**.
- **Bug Fixes**
- Added tighter, fail-closed gating to ensure merge blocking when **E2E
Tests** isn’t correctly finalized.
- **Documentation**
- Documented fork vs same-repo E2E split, OCI export behavior, and
required fork security settings.
- **Tests**
- Added Bats coverage validating `OCI_EXPORT_DIR` and preventing
unintended registry pushes.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
 2026-08-08 04:26:25 +03:00
Aleksei Sviridkin bc6d5e67b7 fix(e2e): keep failure snapshots complete and gate tenant patch on aggregated-API auth (#3444) 
## What this PR does

Two unrelated defects made the sandbox install job's failure handling
unreliable. This fixes both.

The first hit every red run. When a test fails, the diagnostic snapshot
runs `crust-gather collect`, and its host-log leg spawns a privileged
debug pod per node. The sandbox runs baseline PodSecurity, which rejects
each of those pods with a 403, so crust-gather keeps retrying until it
burns its whole collection budget and exits 1. The snapshot verdict then
printed `FAILED` on every failed run even though the namespaced object
state was already captured, so the line that says whether the uploaded
snapshot is trustworthy was always wrong. The host-log leg collects
nothing usable here anyway, so this passes `--disable-additional-logs`
at all three collect sites: the host snapshot from the BATS runner, the
host snapshot from the Chainsaw error handler, and the nested tenant
snapshot. Object collection now finishes and the verdict again separates
a real collection failure from a complete snapshot.

The second is a race in the install test. The "Configure Tenant" step
issues its first `kubectl patch` of the root tenant as soon as an
earlier test sees the aggregated APIService go `Available`. That
condition only tracks endpoint availability, not the serving pod's auth
state. A freshly rolled cozystack-api pod can start answering before it
has loaded the requestheader client-CA, and until it does it drops the
front-proxy identity and answers as `system:anonymous`, so the patch is
rejected with a 403 on the test's very first command. This adds a short
authenticated GET against the same resource before the patch, so the
step proceeds only once the aggregated API both serves and authenticates
the request. If the aggregation layer is actually wedged past two
minutes the gate fails loud instead of hiding a real regression.

Both were traced from a recent sandbox failure (run 30004030331, during
PR #3441's CI).

Out of scope: the broader convergence flakiness of the install step is
not touched here. That covers the long reconcile windows for the storage
and platform releases and the app-level races in the Chainsaw suite. The
durable product-side fix for the second defect, where cozystack-api
should not report ready before the requestheader client-CA is loaded, is
a separate follow-up.

This partially addresses #3368, its crust-gather diagnostics item, and
does not close it.

### 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. -->

No UI changes.

### 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.
-->

The diff touches only the e2e test harness (`hack/cozytest.sh`,
`hack/e2e-chainsaw/.chainsaw.yaml`,
`hack/e2e-chainsaw/_lib/run-kubernetes.sh`,
`hack/e2e-install-cozystack.bats`). The trigger map's `hack/` couplings
are `hack/e2e-prepare-cluster.bats` (node prerequisites for
ansible-cozystack and talm) and `hack/package.mk`,
`hack/common-envs.mk`, `hack/update-crd.sh` plus the package layout
(ccp, external-apps-example, cozyhr). None of those are moved, renamed,
or edited here, and nothing else in the map matches.

- [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(e2e): keep failure snapshots complete when PodSecurity blocks crust-gather's host-log leg, and wait for the aggregated API to authenticate before patching the root tenant so the install step no longer flakes on a system:anonymous 403
```


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

- **Bug Fixes**
- Improved failure diagnostics during automated Kubernetes and tenant
tests by avoiding unsupported privileged log collection steps.
- Ensured failure snapshots complete more reliably instead of timing out
or returning incomplete results.
- Added a readiness check before configuring the root tenant, preventing
intermittent authorization failures during installation tests.

- **Tests**
- Increased reliability of installation and end-to-end test workflows
under baseline security policies.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
 2026-08-08 04:24:48 +03:00
Aleksei Sviridkin cc05c92e47 test(ci): require the discarded-label guard per job, not just agreement 
The check collected every comparison against the label name and required
them to agree on one string. Agreement is not presence: deleting
`e2e-report`'s guard outright leaves the other two agreeing with each
other, so the assertion stayed green while a discarded label event began
publishing again and clobbering the verdict of the run that had actually
tested the SHA. That clobber is the defect two earlier commits were
written to stop.

Require the guard inside each of the two jobs that must carry it, and
keep the agreement check for the rename it does catch. Deleting either
copy now fails.

Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
 2026-08-08 02:26:19 +03:00
Aleksei Sviridkin cc8b01d5cd docs(ci): say when each lane's status opener actually lands 
The reference section claimed both lanes open the required context when
their run starts, and enumerated the triggers it protects against: a
re-run, a reopen, a `full-e2e` label. Every one of those starts a Pull
Request run, and on a fork that run cannot open the status at all. The
opener is guarded same-repo only, because a fork's `pull_request` token
is read-only, and the fork's opener lives in the privileged workflow,
which starts only once the unprivileged build has finished. So on the
lane the sentence was most likely to be read about, an already-green
head SHA keeps reading green for the whole build.

The gap is structural. A read-only token cannot post a status at any
point in the run, and that same read-only token is what makes publishing
the gate with the default token safe in the first place. It is bounded
by the build rather than open-ended. What was wrong was only the claim
that it does not exist.

Say so in all three places that carried the overstatement: the section
itself, the opener's own comment, and the contract test, which pins that
each lane has an opener and cannot express when it lands.

Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
 2026-08-08 02:10:35 +03:00
Aleksei Sviridkin 9c68b1f83e fix(ci): stop a cancelled fork run from publishing a verdict 
The invariant that a cancelled run publishes nothing was written down in
three places and held in two of them.

The fork lane's terminal reporter ran under `always()`, which includes
cancellation. When a second privileged run supersedes the first, the
dying one still posted `failure` on a head SHA it no longer owned, for a
suite that never finished. It self-corrected once the superseding run
reported, but that can be two hours later, and until then a legitimate
pull request reads red. The same-repo lane was moved off `always()` for
exactly this reason. Move this one too: a failed publish or e2e still
reports, since neither is a cancellation, and a cancelled run now leaves
the `pending` it opened standing until its successor concludes.

The same-repo guard's rationale cited a scenario that no longer exists.
It described a label event cancelling the run that opened the pull
request, which the concurrency split made impossible for every label the
workflow discards. The guard is still needed, for a push, for a
`full-e2e` label superseding the head run, and for a manual cancel, so
the comment now names those instead of an extinct one.

The reference section listed the pieces this invariant rests on and
omitted the reporters, which is the half a reader is most likely to
touch, and counted them, which was already wrong and would go wrong
again on the next addition. It now enumerates without counting.

The contract pins both reporters, because both lanes have had this
defect at different times in the same shape.

Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
 2026-08-08 01:26:09 +03:00
Aleksei Sviridkin 8a21085324 fix(ci): decide docs-only from both sides of a rename 
The two lanes answered "is this docs-only" from different sources and
could disagree. `plan` reads `git diff --name-only`, which applies rename
detection and reports a rename as its new path alone, so moving a code
file into docs/ read as docs-only. The fork lane reads the PR file list,
where a rename carries `previous_filename`, and reads the same change as
code.

Disagreement is not a near-miss here. The same-repo lane builds nothing
and uploads no patch artifact; the fork lane concludes e2e is required
and then fails closed because that artifact is missing, reporting that
the fork build did not complete. It completed — it correctly built
nothing. There is no push the contributor can make to clear it and no
label that overrides it, so a legitimate contribution is permanently
red.

Point the docs-only decision at the rename-blind list, which the job
already computes for the downstream-trigger-map flag and whose comment
names this exact trap. Both lanes now agree, and the same-repo side
stops shipping a removed code path untested — until now such a change
skipped every build and reported e2e as not required. The build matrix
keeps reading the rename-detected list: it picks packages to rebuild,
and a path that no longer exists has nothing to build.

The contract file gains the agreement as a pin, and grows a header to
match what it now covers. The fork half is pinned on the docs-only
expression rather than on `previous_filename`, which also appears in the
file list a few lines down and would have kept the assertion satisfied
with the check deleted.

Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
 2026-08-08 01:11:30 +03:00
Aleksei Sviridkin 7fc568db4f test(ci): pin each status publisher by its call, not by a shared string 
The case asserting that both lanes open the required status before they
conclude it counted occurrences of the context string per file and
required two. That measured nothing on the fork lane: the two
occurrences there are the shared `setStatus` helper and the terminal
report, while the opener calls the helper and contributes no occurrence
of its own. Deleting the opener outright left all six tests green, under
a comment promising that dropping either half would be caught — and the
window it guards is the wider of the two, since a privileged run holds
the head SHA for up to three hours.

Scope every assertion to the job that owns it and pin the call rather
than a string that happens to appear twice: `plan` opens with `pending`
under a same-repo guard, `e2e-report` concludes, `resolve` opens through
the helper, `report` concludes. Removing any one of the four now fails
this test.

Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
 2026-08-08 00:54:08 +03:00
Aleksei Sviridkin ee844c98a5 fix(ci): open the "E2E Tests" status when the same-repo run starts 
Moving the gate from a job to a commit status lost a property the job
had for free. A commit status is latest-wins per (sha, context) and
never expires, so a head SHA that already went green keeps reading green
from the moment a new run starts until `e2e-report` posts at the end.
Re-running all jobs, reopening, or adding full-e2e therefore leaves the
pull request mergeable for the whole of a suite that may be about to
fail. While the gate was a job, GitHub created a fresh non-success check
run each time and the window did not exist.

Claim the context at the start of `plan`, the way the fork lane already
claims it in `resolve`, and let `e2e-report` conclude it as before. The
call is same-repo only: a fork's `pull_request` token is read-only, so
posting from there would 403 and fail the job on every fork PR — and
that read-only token is what makes the whole commit-status design safe.
Fork PRs get their opening status from the privileged workflow instead.

The contract test grows a case pinning that each lane writes the context
from two points, one of them pending. Both assertions on the same-repo
opener are scoped to the `plan` job: the bare fork guard occurs on
several steps in other jobs, so a whole-file "at least one" passed with
the opener's guard deleted, pinning unrelated guards and reporting that
as coverage.

Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
 2026-08-08 00:40:49 +03:00
Aleksei Sviridkin 8068619732 test(ci): state the scope note's reasoning as transitive, not direct 
The header claims the third silent path is unreachable because every job
either carries the discarded-label guard or needs `plan`'s outputs. The
`e2e` job does neither literally: it gates on the results of `finalize`,
`build-talos` and `resolve_assets`, which are the jobs that read those
outputs. The conclusion survives — a skipped `plan` still skips it, one
hop further along — but transitivity is the part the claim rests on, and
a reader checking the stated reason against `e2e` finds it false and has
no way to tell whether the conclusion failed with it.

Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
 2026-08-08 00:23:18 +03:00
Aleksei Sviridkin 7696d2b98c test(ci): record which silent path the concurrency pin does not cover 
The contract asserts that the conclusions routed into a per-run group
match the ones `resolve` posts nothing for, which reads as covering
every case where a run stays silent. It does not. `resolve` also
returns silently when the triggering run succeeded but its `Plan build`
job skipped, and such a run sits in the shared group where it can still
cancel the live publisher.

That path is unreachable today, because every job in pull-requests.yaml
either carries the discarded-label guard or needs `plan`'s outputs, so a
skipped `plan` yields a skipped run and the conclusion check catches it
first. Adding a job that runs independently of `plan` would reopen it,
and the extraction here matches on the conclusion constant and would not
notice. Say so in the header rather than leaving the next reader to
infer coverage the pin does not provide.

Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
 2026-08-08 00:04:39 +03:00
Aleksei Sviridkin 999aee5638 test(ci): count occurrences in the gate contract, not matching lines 
Six assertions ran `grep -c` against a variable holding a single line,
so `-eq 1` only asked whether the pattern appeared at all. A key naming
the same field twice — two run-id interpolations, two branch
components — satisfied the pin exactly as the correct key does.
Demonstrated on the run-id assertion: duplicating the interpolation
leaves `grep -c` reporting 1 and passing, while counting occurrences
reports 2 and fails.

Use `grep -o … | wc -l` for the single-line pins. The two whole-file
`cancel-in-progress` counts stay on `grep -c`, where counting matching
lines is the intended question. This is the same correction made once
already in the sibling contract suite; the form did not survive being
copied.

Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
 2026-08-07 23:29:52 +03:00
Aleksei Sviridkin e2643ef4b9 docs(ci): explain the fork image allowlist and fix a test comment 
Two statements that were not true of the code next to them.

The fork `publish` job accepts only archive names it can derive from the
base parent, so a fork cannot name an artifact into an arbitrary
registry repository. The consequence is that a fork PR adding a new
image fails on `image '<name>' is not in the base-tree allowlist` with
no way to pass, and nothing said so — including the E2E section that
AGENTS.md points a reader at when a fork run fails. Record the
constraint, the exact log line, and the two ways around it, so the
failure reads as the guard working rather than as a broken PR.

The concurrency contract's comment on `code_lines` claimed a real grep
error propagates out of the helper. It does under the bats binary, and
not under `hack/cozytest.sh`, whose translator appends `return 0` to
every line that is exactly `}` — file-level helpers included — so a
helper's last command never decides its status there. Both runners
still fail closed, because each test asserts its extraction is
non-empty before comparing, so a swallowed error surfaces as empty
input rather than as a pass. Say that instead.

Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
 2026-08-07 23:10:19 +03:00
Aleksei Sviridkin 7c5e96947e fix(ci): stop two runs from publishing "E2E Tests" on one SHA 
Giving every `labeled` event its own concurrency group stopped a
discarded label from cancelling a live run, but it also stopped a
`full-e2e` label from doing so — and that label is not discarded. `plan`
proceeds for it and `e2e-report` publishes with it, so the labelled run
and the head run it was meant to widen both post the `E2E Tests`
context, on the same head SHA, with no ordering between them.

That was a deliberate trade at the time: commit statuses are latest-wins
per context, so the reasoning went, the full-suite verdict lands last
whenever its suite finishes last, and the label is an explicit opt-in to
a longer run. This commit reverses that decision, because the premise
only holds while the full run succeeds. The full suite is the superset,
so a full-e2e run that fails early — a build flake, a suite failing at
minute twenty — finishes before the narrower head run it was meant to
widen, and that run's later green erases the failure. The required
status then reads success for a SHA whose suite did not pass. One
publisher per head SHA is worth more than the minutes of runner time a
cancelled narrow run costs.

Move aside only the label events `plan` discards, by making the
concurrency condition the exact complement of `plan`'s guard. A
`full-e2e` run stays in the main group and supersedes the head run the
way a push does, so exactly one publisher of the context is alive per
head SHA. Opting into the full suite now costs one suite instead of two,
and a second label landing afterwards can no longer cancel the full-e2e
run, because the two are no longer grouped together.

The contract test gains the complement — the group key must exclude the
publishing label rather than every label — and a check that the label
name is one and the same string in all three guards that read it, since
renaming it in one place splits the decision without touching a line
that reads load-bearing.

Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
 2026-08-07 23:07:33 +03:00
Aleksei Sviridkin 3c50a09457 fix(ci): keep a discarded run out of the fork e2e gate's group 
The privileged fork e2e run groups by fork repo + branch with
cancel-in-progress, so every completed Pull Request run for that branch
lands in the same group -- including a run that built nothing.
Labelling an open fork PR produces exactly that: pull-requests.yaml
triggers on `labeled` and `plan` discards every label other than
full-e2e, so all jobs skip. The privileged run for that event cancels
the one that is mid-suite on the same head SHA, then `resolve`
recognises the discarded trigger and posts no status at all. The
required "E2E Tests" context is a commit status, so the head SHA is
left at "Expected" with nothing left to flip it -- the run that
replaced the owner is the very one that decided to stay silent -- and
the PR stays unmergeable until the contributor pushes again.

Key the group by the triggering run's id when that run concluded
`cancelled` or `skipped`, so a run that will post nothing can cancel
only itself. Real runs keep superseding each other on fork repo +
branch.

The contract test ties the conclusions in the key to the ones `resolve`
stays silent for. Those two lists sit in different languages in
different halves of the file, and a third conclusion added to the guard
alone would restore the wedge without touching a line a reviewer reads
as load-bearing.

Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
 2026-08-07 22:40:34 +03:00
Aleksei Sviridkin 0ba567dd73 fix(tenant): bound the pre-delete cleanup waits and the hook Job (#3598) 
## What this PR does

The tenant pre-delete hook deletes the tenant's HelmReleases in two
waves with `kubectl delete --wait=true` and no `--timeout`. kubectl
reads `--timeout=0` as "wait forever" and substitutes a week, so a wave
hangs for as long as any child HelmRelease cannot finalize. helm then
returns from the uninstall without deleting anything: the release stays
in storage, flux keeps the finalizer, and the same teardown gets
retried. The delete policy carries no `hook-failed`, so the waiting Job
survives between attempts too.

Each wave now gets 90s. Neither is guarded, on purpose. Nothing stands
behind these deletes, so a wave that did not finish ends the run instead
of reporting a cleanup that never happened. That also keeps the order
the two waves exist for, since the tenant modules are what the
applications run on. Whatever got deleted stays deleted, so the next
attempt resumes from there.

The Job gets `activeDeadlineSeconds: 270`. It bounds what the waits do
not, since the DELETE calls carry kubectl's default `--request-timeout`
of 0, and it sits under the 300s uninstall budget flux applies when the
HelmRelease has no timeout of its own. That is the budget for tenants
cozystack-api generates; `tenant-root` is the exception, a static
release in `cozystack-basics` carrying `timeout: 15m0s`, and the
ordering holds there too since 280 is under 900. The 30s gap is not
cosmetic. The deadline runs from the Job's `.status.startTime` while
helm's budget runs from before the Job is created, so at equal values
helm gives up first, on a Job that is still running. That is the state
this change is meant to end, and it would look fixed, because a number
would be there.

`terminationGracePeriodSeconds: 10` is part of the same arithmetic.
Since 1.31 the Job controller holds back the terminal condition until
every pod has finished terminating, so what helm waits for is the
deadline plus the termination. At the 30s default that lands on 270+30,
level with the budget, which is the gap the deadline was set to buy.
That period gets spent in full, every time. The SIGTERM goes to PID 1,
which is the shell: the script runs several commands, so the shell never
execs into kubectl and stays PID 1 itself. A signal carrying its default
disposition is discarded by the kernel when the target is PID 1, and a
non-interactive shell installs no SIGTERM handler, so nothing acts on
it. kubectl is a child and was not signalled at all. The pod ends on
SIGKILL. Nothing in that script needs to finish gracefully, the DELETE
calls are already sent and the process is only watching.

The pod also gets resource requests, matching the sibling hook in
`packages/apps/kubernetes`. Without them it is BestEffort and first to
go under node pressure, which mattered less when a retry inside the Job
absorbed an eviction. With a single attempt it does not.

`backoffLimit: 0` and `restartPolicy: Never` go together, and neither
implies the other. At the default limit of 6 the Job does not reach
`Failed` inside the budget, so helm falls back on its own timeout with
the same nameless "timed out waiting for the condition" as before. Under
`OnFailure` the pod survives its own failure and the kubelet re-runs the
container in place, and the Job controller finds out afterwards by
summing `restartCount`. The script starts over, re-enters the
applications wave that just expired, and gets cut down mid-run when the
controller catches up. `Never` puts a failed pod in front of the
controller directly.

## What this PR does not do

No remedy behind the bounded waits. The `packages/apps/kubernetes` hook
force-clears child finalizers when its own wait expires. That needs
`patch` on HelmReleases in the Role, and it is only safe there because
the whole tenant control plane is going away in the same run. Here a
wave that expires ends the attempt.
`packages/apps/kubernetes/templates/delete.yaml` is untouched.

It does not make a doomed teardown converge. A tenant whose children
cannot finalize still gets its uninstall retried. What changes is that
each attempt ends in bounded time, instead of eating the whole uninstall
budget and leaving a pod behind.

90s is a guess, and it is tight. The first wave deletes every
application HelmRelease in the namespace, including kinds that are slow
to tear down: a nested Tenant runs this same hook one level down before
its release finalizes, and an app of kind Kubernetes tears a control
plane down. Either one can outrun 90s. The bound is what is left once
the Job deadline sits under helm's budget instead of level with it,
split between the two waves.

The 90s the deadline keeps back for scheduling and the image pull is not
calibrated either, and I would rather say so than imply it was.
`docker.io/clastix/kubectl:v1.32` comes from Docker Hub with no mirror
in front of it, and this repo has measured Docker Hub pulls of a smaller
image at about 1m42s in CI. So that remainder is roughly the observed
worst case rather than comfortably above it. A cold pull that slow lands
on the `activeDeadlineSeconds` path, which is the one that deletes the
pod and leaves nothing to read.

There is a band that pays for this, and I would rather name it than
round it off. Neither shape above fits inside 300s today, so for them
the bound costs nothing that works. But every generated child
HelmRelease carries a wait strategy, so a wave inherits its slowest
child, and a wave that settles in something like 95s does complete
inside the budget today. After this change it expires instead, and that
tenant trades one clean attempt for an extra flux retry. In wall clock
that retry is about five minutes, since `helmrelease-interval` defaults
to `5m`, so a tenant in that band takes noticeably longer to delete than
it does now. I think that is the right side of the trade: an unbounded
wave does not fail, it spends the whole budget and leaves helm reporting
a timeout that names nothing. If the band turns out to be populated, the
waits and the deadline move together, and the bats suite pins their
ordering rather than their values.

One caveat on the diagnostics, since the comments in the chart used to
promise more than they deliver. The failing wave leaves its output in
the failed pod, which `ttlSecondsAfterFinished: 300` then reaps while
flux's uninstall backoff is still climbing. And on the
`activeDeadlineSeconds` path there is no log at all, because the Job
controller deletes the active pods when the deadline expires. That path
is the backstop for a kubectl that never returns; the expected path is
the wave's own `--timeout`, where the script exits on its own and the
pod survives to be read.

## Tests

`hack/tenant-pre-delete-hook.bats` renders the chart, pulls the hook
script out of the Job and runs it against a stub `kubectl` on `PATH`. It
covers a clean run reporting success, every delete reaching kubectl with
a non-zero bound, the ordering of the four numbers, and a wave that gave
up ending the run without going on to the next one. `--timeout` is
implemented inside kubectl, so no stub can make that deadline actually
elapse. The tests read the arguments that reached kubectl and inject the
non-zero exit an expired delete produces. No EXIT traps.

The ordering test reads the numbers out of the rendered Job instead of
pinning them, so moving a value keeps it honest while a value that
breaks the ordering still fails. An absent deadline or termination
period is rejected explicitly, otherwise `null` satisfies the numeric
comparison and a Job with no bound at all passes.

The Job's own shape is pinned in the chart's helm-unittest suite, since
the script cannot observe that about itself.

Both suites are green locally, and I mutated each pinned value to
confirm they fail for the right reason: dropping either `--timeout`,
dropping `activeDeadlineSeconds`, dropping
`terminationGracePeriodSeconds`, dropping the requests, `backoffLimit`
back to 6, `restartPolicy` back to `OnFailure`, and a `|| echo` appended
to the applications wave. Each one goes red in the test that claims that
ground.

Worth flagging so the check marks are not read as more than they are:
`make bats-unit-tests` is red at the merge base, unrelated to this
branch, and the loop it runs exits on the first failing file.
`hack/cozyreport.bats` sorts ahead of this suite, so on CI the new bats
file is not reached at all right now. It is not failing, it is not
running. The helm-unittest cases are unaffected and do execute.

### Screenshots

None. This changes a Job manifest and its tests, with no user-visible
surface.

### Downstream repositories

Walked the trigger map against the diff. The diff is one Job manifest
inside the tenant chart, its helm-unittest suite, and a new
`hack/*.bats` file. No package is added, renamed or removed; no
`values.schema.json`, `values.yaml` default or version enum changes; no
`ApplicationDefinition` semantics, namespace, variant or release asset
changes; nothing under `hack/` is moved or renamed and no make target
changes behaviour, since `bats-unit-tests` already globs `hack/*.bats`.

- [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

```release-note
fix(tenant): bound the pre-delete cleanup hook's HelmRelease deletions and give its Job a wall-clock deadline, so a tenant whose children cannot finalize no longer leaves the hook waiting indefinitely and blocking the uninstall
```


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

* **Reliability Improvements**
* Tenant cleanup jobs now enforce execution deadlines and terminate
gracefully.
* Cleanup operations have bounded deletion timeouts and no automatic
retries.
* Failed cleanup steps now stop subsequent operations and correctly
report failure.
  * Cleanup jobs now specify CPU and memory resource requirements.

* **Tests**
* Added comprehensive coverage for cleanup behavior, timeouts, failure
handling, and job lifecycle settings.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
 2026-08-07 17:48:02 +03:00
Aleksei Sviridkin 954be15341 fix(e2e): bound every read in the report collector and the step that runs it (#3596) 
<!-- 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

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

<!-- 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(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.
```


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## 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.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
 2026-08-07 17:47:51 +03:00
Aleksei Sviridkin 0b7a8f67d1 fix(apps/kubernetes): report the pre-delete cleanup outcome honestly (#3592) 
## What this PR does

The pre-delete cleanup Job for the `kubernetes` app ended with an
unconditional `echo "Cleanup completed successfully"`. Several of its
steps swallow a failure behind `|| echo "WARNING: ..."` so that `set -e`
cannot end the run, which means that line was printed in exactly the
situation those warnings exist to report: child HelmReleases still
holding Flux finalizers, or a Kamaji datastore Secret still pinning its
namespace. Someone reading the Job log to find out why a namespace will
not delete was told the cleanup had worked.

The Job now records what a step left behind and closes with either the
success line or the list of steps that left something.

Only residue gets recorded. A bounded delete that times out has a remedy
right behind it (Step 2 falls through to force-clearing finalizers, Step
4 falls through to Step 4b), and whether anything survived is decided by
that remedy, which reports its own outcome. A tenant with no working
nodes takes that route as a matter of course, which is the reason the
wait is bounded at all, so recording the timeout itself would report a
finished teardown as incomplete on the commonest path there is and send
an operator hunting for objects that are not there. That is the same
defect as the one being fixed, pointing the other way.

A read that fails looks exactly like one that found nothing. An empty
list and a failed list both give a loop nothing to iterate; an absent
object and an unreachable apiserver both make a probe non-zero. So the
whole script was walked for that shape instead of patching the instances
as they were reported. Four reads reach the closing line. Step 2's
force-clear re-list now keeps its status and reports when it cannot list
what it was about to clear. Step 4b's two probes became three-way,
present, absent or unknown, so a Secret whose existence could not be
established is no longer announced as absent. Step 1's list is the one
left swallowing its status, and that is deliberate: Step 2 deletes by
label rather than from that list, so a suspend that never happened
leaves nothing of its own behind, and reporting it would name residue
that does not exist.

Making the TenantControlPlane probe three-way also repairs the gate
above it, and that is intended. The gate is there so the datastore
Secret's finalizer is not stripped while Kamaji may still own it, and a
two-way probe defeated it in the one direction that does damage: any
error read as "confirmed absent" and the strip went ahead. Its likeliest
cause, a throttled or briefly unreachable apiserver, is also likeliest
exactly when a broken tenant is being torn down.

Step 4b declining to strip a finalizer while the TenantControlPlane is
still alive is the one residue that no failing command marks: every call
in that branch succeeds, and the namespace stays blocked regardless. It
now reports, and names the TenantControlPlane as the thing holding it
up.

What this PR does not do: the hook still exits zero, even with every
backstop engaged. That is the difference between a main path and a
backstop. `packages/apps/tenant/templates/cleanup-job.yaml` is the same
kind of hook and handles it the opposite way, correctly: deleting the
HelmReleases is that hook's whole purpose, so a failure there means the
cleanup did not happen, and `set -e` ending the run before the success
line is honest. The swallowed sites here are backstops. They run only
after the normal deletion has already timed out, and they exist to push
teardown further along. Failing there aborts the Helm uninstall before
the release is removed, so the release stays in storage and the same
doomed teardown is retried forever, at precisely the moment the backstop
was written for. The steps that are not backstopped still end the run
under `set -e` and never reach the closing message at all.

This makes the message truthful. It does not make it reachable. The Job
carries `helm.sh/hook-delete-policy: hook-succeeded`, and the hook exits
zero by design on every backstopped run, so Helm reaps the Job, and its
pod log, right after the hook completes. Whether the closing line
survives long enough to be read is a question about the delivery
channel, not about its wording, and changing that channel (an Event on
the CR, or keeping the Job when something was left behind) has a
different blast radius. Left for a separate change.

The step comment and the runtime warning both cited the tracking issue
for the Kamaji datastore-secret leak as though it were live. It is
closed, and closed by this step rather than by a fix in Kamaji, so
following the pointer showed a resolved report and no reason for the
code to exist. The comment now says the ordering it covers is still
reachable, and the warning names the finalizer and the object to clear
by hand instead of an issue number. The workaround itself is untouched:
nothing indicates the upstream cause has gone away.

`hack/kubernetes-pre-delete-hook.bats` renders the Job, extracts the
script and runs it against a stub `kubectl`, covering a clean run, a
fallback route that succeeded, a TenantControlPlane outliving Step 4,
each probe that could not answer, every remedy failing at once, an
unbackstopped step ending the run, and the zero-exit invariant. The
helm-unittest cases match the script's source text and cannot tell
whether the closing message is reachable. Every case was checked by
mutation, including reinstating the unconditional success line and
reinstating the over-reporting the fallback case guards against.

### Screenshots

Not a UI change.

### Downstream repositories

- [x] No downstream repository is affected by this change

### Release note

```release-note
fix(kubernetes): the pre-delete cleanup hook no longer reports success when a cleanup step left something behind, and no longer reports a completed teardown as incomplete when it merely fell back to force-clearing finalizers. The Job log now names the steps that left residue. The hook still exits zero so the Helm uninstall can remove the release.
```


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

* **Bug Fixes**
  * Improved Kubernetes cleanup handling during application deletion.
* Cleanup now distinguishes unavailable resource checks from confirmed
absence, avoiding unsafe finalizer removal.
* Reports leftover resources when recoverable cleanup steps fail while
still completing the deletion process.
* Improved handling of child releases, control-plane resources, and
datastore secrets.

* **Tests**
* Added comprehensive coverage for successful cleanup, partial failures,
resource residue, probe errors, and unrecoverable steps.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
 2026-08-07 17:47:43 +03:00
Aleksei Sviridkin 7e2db4da7e fix(e2e): bound every remaining read in the report collector 
Four sections of hack/cozyreport.sh read with a wall-clock ceiling and
the rest -- nodes, namespaces, events, HelmReleases, packages,
packagesources, PVCs, cert-manager, COSI, kamaji, the Talos node reads
and every module gate -- read with none. The tarball is written on the
collector's last line, so a single read that never returns does not
truncate the report, it loses it, on the run where it is the only
evidence left.

Each read now goes through the shape that fits it: the existing bounded
reader for a read whose output is a file, the existing selector for one
that feeds a loop, and a new probe for the `if kubectl get deploy ...`
gates. A gate is a read like any other, and an unbounded one is worse
than the reads it guards, since the module below it never starts. Its
cut-off is recorded, because skipping the module leaves the same empty
tree as a cluster that never had the component; a gate that answers "not
installed" is recorded nowhere, since that is the cluster answering
correctly and a marker firing on every install without KubeVirt is one a
reader learns to skip. The reads that stay on the bare wrapper say so
where they sit; what they share is that the output is not the reader's
to write, going into a variable or through a pipeline that transforms
it, so there is no moment at which a marker could attach.

The header claimed a per-section list of what was bounded, and the tests
froze that list plus a per-section count of the unbounded reads. Both are
gone. The list had already gone stale, naming four sections while the
file had ten, and the counts were an undercount by construction -- they
matched a redirect to `$VAR/`, which no gate, selector or command
substitution has. One test replaces both: no read below the library guard
runs without a ceiling, checked by looking for a counterexample.

A ceiling per read is not a ceiling on the report, and the header now
says so where it used to imply the opposite. The pod, VM, VMI and
services walks carry a count cap and a time budget; the walks over nodes,
namespaces, HelmReleases, packages, PVCs and certificates carry neither,
so their cost is the cluster's to set. What this removes is the read that
never returns.

The bounded reader now names the tool it ran and describes what that tool
returned. It served kubectl alone until this change gave it talosctl and
the four host commands, so a `df` that hit a stale mount left a marker
reading "kubectl exited 1 ... not because the object ended" -- two false
statements in one line, in an artifact whose premise is that its notes
can be believed.

The exec reader keeps the remote command's whole message in the listing
it belongs to. `kubectl exec` appends its own `command terminated with
exit code N` after whatever the command said, so quoting the last line
there drops the cause and keeps the trailer, and sending the message
away from the file leaves a zero-byte listing -- the "returned nothing,
or never ran" question this collector exists to answer. Its cut-off note
also stops asserting that the archive is beside it: a stream that got
members across is kept and truncated, one killed before its first byte
leaves zero bytes and the call site's emptiness check removes it, and
the reader runs before that check, so it names both.

Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
 2026-08-07 14:38:29 +03:00
Aleksei Sviridkin 920ad3e30d fix(e2e): bound the report reads that run inside a pod 
The LINSTOR module runs six reads through `kubectl exec`: three
listings, the error-report index, and the controller and per-satellite
ErrorReport bundles. They are the only reads in the collector whose
duration is set by a workload rather than by the apiserver -- a wedged
controller, a satellite that stops answering, or a broken SPDY upgrade
holds the stream open with no clock anywhere in the path. The tarball is
written on the collector's last line, so one of those never returning
does not truncate the report, it loses it.

They go through a reader of their own rather than the existing one.
`kubectl exec` returns the remote command's exit status, so a 124 there
is either this collector's deadline expiring or a command that itself
exited 124, and the status does not separate them; describing it the way
every `get`, `logs` and `describe` is described would state a mechanism
about the machine writing the report for something that may have
happened inside a pod. The note says what is open instead of resolving
it, and goes beside the output rather than into it, since two of the six
stream a gzipped tarball that a line of prose turns into an archive
nothing can open. kubectl's own message travels with the note for the
same reason -- until now it was merged into the listings, so every run
put the "Defaulted container" line at the top of a table.

The guard that held the old assumption asserted only that no bounded
read was an exec, which leaving every exec unbounded satisfies. It now
also requires each exec to be bounded and requires the 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
the fix: half of these calls spell it `kubectl -n cozy-linstor exec`,
which that string never saw.

Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
 2026-08-07 14:38:29 +03:00
Aleksei Sviridkin 96e33fe656 ci(e2e): give the report collection step a ceiling of its own 
The step that runs hack/cozyreport.sh carried no timeout-minutes in any
of the three workflows that run it, so it inherited only the job's 180.
The collector writes its tarball on its last line and the upload is the
next step, so a read that never returns spends what is left of the job
budget, the job is cancelled, and the upload never runs -- the artifact
is lost entirely rather than truncated, on exactly the run where it is
the only evidence left. `|| true` guards against a non-zero exit, not
against a read that does not return.

30 minutes is measured against the step rather than divided out of the
job: it takes 3-42s across sampled runs, on failed e2e jobs as well as
green ones, and 30 leaves five sixths of the job budget for the steps
that upload what this one produced.

A test pins the ceiling in all three workflows and requires it to stay
under the job's own, since a step ceiling at or above the job's cap is
the same as no ceiling at all.

Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
 2026-08-07 14:38:29 +03:00
Aleksei Sviridkin cf85716444 fix(tenant): bound the pre-delete cleanup waits and the hook Job 
kubectl reads --timeout=0 as "wait forever" and substitutes a week,
so the two `kubectl delete helmreleases --wait=true` calls in the
tenant pre-delete hook hang for as long as any child HelmRelease
cannot finalize. helm returns from Uninstall.Run before deleting
anything, so the release stays in storage and flux retries the same
teardown; helm.sh/hook-delete-policy carries no hook-failed, so the
waiting Job survives between attempts as well.

Bound each wave at 90s. Neither is guarded: nothing stands behind
these deletes, so a wave that did not finish ends the run rather than
report a cleanup that did not happen, and ending it keeps the tenant
modules from being torn out from under applications that are still
deleting. Whatever was deleted stays deleted, so the next attempt
resumes from there.

The bound has a price and it is not zero. A nested Tenant, or an app
of kind Kubernetes whose own hook spends two 120s waits first, misses
90s and converges across uninstall retries instead; neither fits the
300s budget as it stands either. Between those and the fast case sits
a band that does pay: a wave inherits its slowest child, so one that
settles around 95s completes inside the budget today and expires here
instead, trading a clean attempt for one more retry. An unbounded
wave does not fail, it spends the whole budget and reports a timeout
naming nothing, so the trade is worth taking.

Give the Job an activeDeadlineSeconds of 270. It bounds what the
waits do not — the DELETE calls carry kubectl's default
--request-timeout of 0 — and sits under the 300s uninstall budget
flux applies when the HelmRelease carries no timeout of its own,
because the deadline runs from the Job's startTime while that budget
runs from just before the Job is created. At equal values helm gives
up first, on a Job still running.

Cap the pod's termination at 10s, which is part of that arithmetic
rather than a detail of it. Since 1.31 the Job controller withholds
the terminal condition until every pod has finished terminating, so
helm learns of the failure at the deadline plus this period; the 30s
default would land it at 270+30, level with the budget the deadline
is set under. The period is spent in full: the SIGTERM goes to PID 1,
which is a shell running a multi-command script, and a default
disposition sent to PID 1 is discarded by the kernel with no handler
installed to catch it. kubectl is a child and is never signalled.

Set backoffLimit to 0 and restartPolicy to Never. The limit is needed
under either policy, since the default of 6 keeps the failure from
reaching helm inside the budget. The policy matters because under
OnFailure the pod survives its own failure and the kubelet re-runs
the container in place, with the controller applying the cap only
afterwards by summing restartCount: the script re-enters the wave
that just expired and is cut down mid-run. Never leaves a failed pod
the controller sees directly.

Add resource requests to match the sibling hook in
packages/apps/kubernetes. Without them the pod is BestEffort and
first out under node pressure, which a retry inside the Job used to
absorb and a single attempt does not.

Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
 2026-08-07 14:38:06 +03:00
Aleksei Sviridkin c78326281e fix(tests): freeze the EXIT-trap count for run-kubernetes-talos-diagnostics_test.bats 
hack/run-kubernetes-talos-diagnostics_test.bats arrived carrying eight
EXIT traps and was not added to the frozen set, so `found` and `frozen`
disagree by exactly that one entry and the guard fails on every branch
cut from main.

Updating the count is what the comment above the guard prescribes for a
file that did not exist when the freeze was written. The guard did its
job: it noticed.

The cost is wider than one red check. hack/cozytest.sh stops at the
first failing test in a file, and the guard is test 83 of 166 in
cozyreport.bats, so the remaining 83 do not run. The Makefile loop is
`for f in $(BATS_UNIT_FILES); do hack/cozytest.sh "$f" || exit 1; done`,
which aborts the whole loop, and cozyreport.bats sorts fourteenth of
forty-two, so twenty-eight files do not run either. Downstream of that,
finalize needs checks, and the e2e job needs finalize, so E2E Tests is
skipped rather than executed.

Verified by running the file to completion with bats, which walks past a
failure unlike cozytest.sh: 166 of 166 pass with this entry present.

Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
 2026-08-07 14:09:43 +03:00
Aleksei Sviridkin 6c82988570 fix(tests): freeze the EXIT-trap count for multus-install-cni-plugins.bats (#3584) 
<!-- 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/multus-install-cni-plugins.bats` arrived on main in 7739d1e29
carrying 12 EXIT traps, one day before the EXIT-trap ratchet in
`hack/cozyreport.bats` was written, so the frozen list never learned
about it. The guard therefore fails on main itself, and on every PR
whose CI builds a merge with main — `make bats-unit-tests` exits 1 with:

```
FAIL: the set of unconverted EXIT-trap files changed.
  frozen: ... nightly-mirror_test.bats=5 ...
  found:  ... multus-install-cni-plugins.bats=12 nightly-mirror_test.bats=5 ...
```

This records the count, which is what the guard's own comment prescribes
for exactly this case: *"a file that did not exist when this guard was
written arrives carrying its own \[traps\] ... Counts are updated rather
than the files converted: all of them are owned by other branches, and a
conflict there costs more than an uncovered trap."* Converting the file
is not in scope here and belongs to whoever owns the multus branch.

Verified by reproducing the guard's logic against a pristine checkout of
`main` at 879d0f6db: the `found` set differs from `frozen` by exactly
this one entry before the change, and matches after it. Found while
investigating a red `Unit & controller tests` on #3571, which carries
none of this.

### Screenshots

No UI changes.

### 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
```


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

* **Tests**
* Updated the expected EXIT-trap inventory to include
`multus-install-cni-plugins.bats`.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
 2026-08-07 10:42:43 +03:00
IvanHunters 318be935ca fix(e2e): capture Talos logs for tenant worker join failures (#3548) 
## What this PR does

Addresses #3513.

When tenant Kubernetes workers miss the 18-minute Ready deadline, the
E2E runner now mints a one-hour `os:reader` Talos client certificate
from the tenant's existing issuer and runs authenticated `talosctl` from
a hardened same-namespace diagnostics Pod. It captures bounded `dmesg`
and kubelet output for every worker VMI in the cozyreport snapshot,
records missing IPs and command exit codes, continues across unreachable
workers, and preserves the original test failure.

The diagnostics Pod has no service-account token or Secret volume;
credentials are streamed into an `emptyDir`, and the Certificate,
Secret, and Pod are labeled for cleanup. Workers that have not reached
Talos `apid` can only yield a bounded connection error, while workers
that reached `apid` provide guest logs. This adds the evidence needed to
distinguish the remaining guest boot failure but does not claim to
resolve its underlying cause. The readiness budget and success path are
unchanged.

Because this changes a shared E2E helper, test-impact analysis selects
the full E2E suite for this PR.

### Screenshots

Not applicable; no UI changes.

### Testing

- `hack/cozytest.sh hack/run-kubernetes-talos-diagnostics_test.bats` (10
tests)
- `hack/cozytest.sh hack/run-kubernetes-drain_test.bats`
- `hack/cozytest.sh hack/cozyreport-talos.bats`
- `bash -n hack/e2e-chainsaw/_lib/run-kubernetes.sh`
- `git diff --check origin/main...HEAD`

The full `make bats-unit-tests` run reached the unrelated
`hack/migration-seaweedfs-db-adopt.bats` suite and could not continue
because Docker is unavailable in the development environment. A live
failure-path run was not performed because the available development
environment has no Talos tenant fixture; this PR is intended to exercise
that path in Cozystack CI.

### Downstream repositories

- [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

```release-note
fix(e2e): capture authenticated Talos kernel and kubelet diagnostics when tenant Kubernetes workers miss the Ready deadline
```



<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

* **Bug Fixes**
* Improved troubleshooting when worker nodes fail to join by collecting
per-node operating system and kubelet diagnostics.
* Diagnostics now continue across partial failures and clean up
temporary resources afterward.
* Extended Kubernetes test operation timeouts to allow failure
diagnostics to complete reliably.

* **Tests**
* Added regression coverage for diagnostic collection, timeout handling,
multi-worker scenarios, missing node addresses, and cleanup.

* **Chores**
  * Ensured the required diagnostic image is preloaded on nodes.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
 2026-08-07 10:40:03 +03:00
Aleksei Sviridkin 0827bcb300 fix(apps/kubernetes): report the pre-delete cleanup outcome honestly 
The pre-delete cleanup Job ended with an unconditional "Cleanup
completed successfully". Several of its steps swallow a failure behind
`|| echo "WARNING: ..."` so that `set -e` cannot end the run, so that
line was printed in exactly the situation the warnings exist to report:
child HelmReleases still holding finalizers, or a Kamaji datastore
Secret still pinning its namespace in Terminating.

Record what a step leaves behind and close with either the success line
or the list of steps that left something. Residue is the thing worth
recording, not every fallback: a bounded delete that times out has a
remedy right behind it -- Step 2 falls through to force-clearing
finalizers, Step 4 falls through to Step 4b -- and whether anything
survived is decided by that remedy, which reports its own outcome. A
tenant with no working nodes takes that route as a matter of course,
which is why the wait is bounded in the first place, and calling such a
run incomplete would send an operator hunting for objects that are not
there.

A read that fails looks exactly like one that found nothing: an empty
list and a failed list both give a loop nothing to iterate, and an
absent object and an unreachable apiserver both make a probe non-zero.
Every read the summary rests on now keeps its status. Step 2's
force-clear re-list reports when it cannot list what it was about to
clear, because everything it could not return is still holding its
finalizer. Step 4b's two probes became three-way -- present, absent or
unknown -- so a Secret whose existence could not be established is no
longer announced as absent. Step 1's list is the one read still
swallowing its status, and that is a decision rather than an omission:
Step 2 deletes by label rather than from that list, so a suspend that
never happened leaves nothing of its own behind and reporting it would
name residue that does not exist.

Making the TenantControlPlane probe three-way also repairs the gate
above it, and that is intended rather than incidental. The gate exists
so the datastore Secret's finalizer is not stripped while Kamaji may
still own it, and a two-way probe defeated it in the one direction that
does damage: any error reads as "confirmed absent" and the strip goes
ahead. Its likeliest cause, a throttled or briefly unreachable
apiserver, is also likeliest exactly when a broken tenant is being torn
down.

Step 4b declining to strip a finalizer while the TenantControlPlane is
still alive is the one residue that no failing command marks: every
call in that branch succeeds and the namespace stays in Terminating
regardless. It now reports, and names the TenantControlPlane as the
thing holding the namespace up.

The exit code is deliberately unchanged and still zero. These are
backstops, not the main path: they run after the normal deletion has
already timed out, and they exist to push teardown further along, so
failing there would abort the Helm uninstall at the very moment the
backstop was written for, leaving the release in storage and the same
teardown retried forever. That makes the closing message the only
outcome report an operator gets, which is why it has to be accurate.
The steps that are not backstopped still end the run under `set -e` and
never reach that message.

Rework the runtime warning and the RBAC comment that pointed at the
tracking issue for the Kamaji datastore-secret leak. That issue is
closed, and closed by this very step rather than by a fix in Kamaji, so
a reader who followed the pointer found a resolved report and no reason
for the code to exist. The step comment now says the ordering it covers
is still reachable, and the warning names the finalizer and the object
to clear by hand.

Cover the behaviour with a bats file that renders the Job, extracts the
script and runs it against a stub kubectl, since the helm-unittest cases
match the script's source text and cannot tell whether the closing
message is reachable.

Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
 2026-08-07 06:49:56 +03:00
Aleksei Sviridkin 0dae3ca95a fix(e2e): budget tenant scheduling separately from workload readiness (#3579) 
## What this PR does

The tenant backend Deployment in the kubernetes suites had one 300s
budget to reach `condition=Available`, and two unrelated variable costs
shared it.

The first is scheduling. What the suite establishes before that point is
two tenant nodes Ready, which is weaker than schedulable: a Ready node
still carries `node.cilium.io/agent-not-ready` until the tenant cilium
agent claims it, and a node the bringup has not finished with is
`SchedulingDisabled`. In the run recorded in #3577 scheduling took 2m18s
and 1m57s in the two suites, which left the image pull to finish inside
what remained. It did not, and both suites reported the same `timed out
waiting for the condition` for what were two different shortfalls.

So this waits for a node that actually accepts a Pod before creating the
workload, on its own budget and its own failure message. The gate
encodes the scheduler's rule for a Pod that tolerates nothing (Ready,
not unschedulable, no `NoSchedule` or `NoExecute` taint) over a
`custom-columns` probe, prints the node table on both outcomes so a
timeout names the taint that held it, and treats a failed probe as
not-schedulable so an API blip cannot release it. On the happy path it
adds no wall time: it spends the seconds the Pod would otherwise spend
Pending, plus at most one 5s poll interval. On a failing run the two
budgets stack, so a run that exhausts both now gives up at ~600s where
it used to give up at 300s, inside the 40m Chainsaw script op the suites
document as a ~25m bringup. The readiness wait keeps its 300s, now
starting from a schedulable node, and dumps deployment, pod and event
state when it runs out.

The workload image is also pinned by digest. The tenant workers reach no
Docker Hub mirror (`hack/e2e-talos-image-cache.yaml` serves the Talos
worker OS disk image over HTTP and is not a registry mirror), so nginx
is pulled from Docker Hub on every run either way. The digest does not
take the pull off the critical path; it fixes what that pull returns
instead of leaving a floating tag free to change size and content under
a fixed deadline. Preloading the image would need infrastructure this
tree does not have, and is not attempted here. Nothing will bump the
pin: no renovate manager reads `hack/`, and the comment says so, because
for a throwaway test workload the freeze is the point.

What this does not claim: it removes two measured consumers from a fixed
budget, both taken from the events of a single run. Whether that budget
was the only thing making those suites red is not established from one
run, so this is not offered as the fix for a red pipeline.

`hack/run-kubernetes-schedulable_test.bats` covers the new logic: every
branch of the predicate, the multi-node scan, the poll-again path, the
deadline, and a probe that fails. Each test was verified by mutating the
helper and checking that the intended test, and no other, went red.

Note for whoever reviews alongside #3575: that branch mirrors `ghcr.io`
for tenant worker pulls, not `docker.io`, so it does not change the
Docker Hub pull described here. `git merge-tree` reports no conflict
between the two, nor with #3548.

Observed in #3577.

### Screenshots

Not a UI change.

### Downstream repositories

- [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:

The trigger map was walked against the diff. The change is confined to
`hack/e2e-chainsaw/_lib/run-kubernetes.sh` and one new `hack/*.bats`
unit test. It moves and renames nothing under `hack/`, changes no make
target, and does not touch `hack/e2e-prepare-cluster.bats`, any package,
any CRD or any namespace name, so none of the listed repositories are
reached.

### Release note

```release-note
NONE
```


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

* **Bug Fixes**
* Improved Kubernetes deployment readiness by waiting for a suitable,
schedulable node before timing backend startup.
* Added clearer diagnostics when nodes are unavailable or backend
readiness fails.
* Replaced the floating backend container image tag with a fixed,
verified version.

* **Tests**
* Added coverage for node readiness, cordoning, taints, polling,
timeouts, probe failures, and deployment sequencing.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
 2026-08-06 23:46:50 +03:00
Mattia Eleuteri 3aef42b993 fix(tests): freeze the EXIT-trap count for multus-install-cni-plugins.bats 
hack/multus-install-cni-plugins.bats arrived on main in 7739d1e29 carrying
12 EXIT traps, one day before the ratchet in hack/cozyreport.bats was
written, so the frozen list never learned about it. The guard therefore
fails on main itself, and on every PR whose CI builds a merge with it:

  FAIL: the set of unconverted EXIT-trap files changed.
    frozen: ... nightly-mirror_test.bats=5 ...
    found:  ... multus-install-cni-plugins.bats=12 nightly-mirror_test.bats=5 ...

Record the count, which is what the guard's own comment prescribes for a
file that did not exist when it was written: "Counts are updated rather
than the files converted: all of them are owned by other branches, and a
conflict there costs more than an uncovered trap."

Verified by reproducing the guard's logic against a pristine checkout of
main: the difference is exactly this one entry, before the change and
none after it.

Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Mattia Eleuteri <mattia@hidora.io>
 2026-08-06 16:58:08 +02:00
Aleksei Sviridkin 879d0f6db6 fix(installer): make operator readiness honest in the chart and the install gate (#3466) 
## What this PR does

The operator Deployment ran `replicas: 1` with `maxUnavailable: 1`, so
the Deployment controller took the minimum available replica count as `1
- 1 = 0` and the `Available` condition reported `True` with no running
pod at all. Measured against an operator whose image tag does not exist:
the condition was `True` with `reason: MinimumReplicasAvailable` while
the only pod sat in `ImagePullBackOff`, `unavailableReplicas: 1`, and no
`availableReplicas` key in the status.

That broke two separate things, so this fixes both.

The E2E install gate asked `kubectl wait --for=condition=Available` and
passed in 156 ms against that dead operator. The install then carried on
and died about two minutes later on the `packages.cozystack.io` CRD wait
further down the file, which sends you after CRD installation when the
operator never started. It now uses `kubectl rollout status`, which
reads updated and available replicas directly, and dumps the deployment,
pods, operator logs and namespace events when it fails. This half is
deliberately independent of the chart: `rollout status` is correct under
any rollout strategy, so a later change to that strategy cannot quietly
make the gate vacuous again.

The chart is the other half, and fixing the gate does not cover it.
`Available` is read by more than this test, and until the strategy
changes an alert on that condition, a health dashboard, or a person
reading `kubectl get deploy` all show green while the operator is dead.
The strategy moves to `maxSurge: 1` / `maxUnavailable: 0`, which
restores the ordinary meaning of the condition. Kubernetes refuses
`maxSurge: 0` and `maxUnavailable: 0` together, so the surge has to move
for the unavailability to reach zero.

That pair is worth reading twice, because it is not a configuration I
picked. At `replicas: 1` the Kubernetes default of `25%` / `25%`
resolves to exactly `maxSurge: 1` and `maxUnavailable: 0`, since the
surge rounds up and the unavailability rounds down. What this PR does is
stop overriding the default, not introduce a tuning of its own, and the
risk of the change should be judged on that basis.

Moving the surge is safe here, and I checked rather than assumed,
because the pair came from somewhere. It was carried over from the
earlier `cozystack` Deployment, where a sidecar published host port 8123
and `hostNetwork` left the surge pod unschedulable on a single-node
cluster. It got here in two steps, which is worth knowing if you go
looking for the reason: by the commit before this file existed the
sidecar was already gone and no port was declared, yet the pair stayed.
It had outlived its own reason before it was ever copied, which is why
there is no justification to find in this file's own history. On a
single-node cluster the surge pod is scheduled next to the outgoing one
and the rollout completes; adding a declared container port reproduces
the original `didn't have free ports for the requested pod ports`
failure exactly. Nothing else in the pod holds a port either: metrics
and health probes are bound to `0`, and the configured webhook server
never starts because the operator registers no webhook and never asks
the manager for the server.

The surge makes leader election load-bearing, though it covers less of
the overlap than it first appears. While `maxSurge: 0` capped total pods
at `replicas` a second manager process could not exist, so
`--leader-elect=true` was defence in depth. With a surge pod the lease
is what keeps the incoming manager's reconcile loops off the cluster
while the outgoing one is still leader, and because there is no
readiness probe the incoming pod counts as available as soon as its
container starts, so that overlap covers the whole image pull. It does
not cover everything: the operator installs CRDs, installs Flux and
creates the platform PackageSource before `mgr.Start()`, and the lease
is only taken inside `mgr.Start()`, so those three privileged installs
run outside leader election entirely. They are safe because both pods
write the same objects through server-side apply under the same field
manager, which is a different guarantee from the lease. The
`k8s-await-election` wrapper this operator replaced did hold the whole
process until it won leadership, so if that property sounds familiar it
belongs to the predecessor rather than to this binary.

The sibling `opensearch-operator` chart turns leader election off at one
replica for a real reason: a transient lease-renewal blip makes
controller-runtime exit the process, which at one replica reads as a
crashlooping Deployment. This operator installs first, when the
apiserver is at its shakiest, so that scenario applies here too. I am
accepting the risk knowingly, because after `maxSurge: 1` the
alternative is two managers reconciling the same cluster, which is
worse. The flag is pinned in the unit test so it cannot be turned off
without the test going red.

One consequence is permanent, so I am writing it down now instead of
leaving it to be found in a year. Because any bound port breaks the
surge pod under `hostNetwork`, an `httpGet` readiness probe is off the
table for this Deployment for good and only an `exec` probe could ever
work. So `Available` still answers "a container started", not "the
operator is healthy": an operator that starts and then dies partway
through installing CRDs is caught by the CRD waits below the gate, not
by the gate itself.

The surge also opens one new window. The operator installs CRDs and Flux
before the manager starts, which puts that work outside leader election,
so the incoming pod runs those applies while the outgoing pod is still
leader. Both apply the same objects server-side and the outgoing pod
exits moments later.

`--wait` is dropped from the helm step in the same change. Helm decides
a Deployment is ready with the same `replicas - maxUnavailable`
arithmetic, so on the old strategy it was satisfied at zero ready pods
and gated nothing, which is why the broken install got past it as well.
Once the strategy is honest it would gate for real, but on its own 2m
budget, and it would fail the install before the rollout gate ran and
hide the diagnostics behind a bare Helm timeout. Hooks are waited for
with or without the flag, and the release otherwise carries only the
Deployment and its RBAC objects, so the rollout gate is the better
single place to decide the operator is up.

A helm unittest suite pins the strategy and all four preconditions the
surge makes load-bearing: no container or initContainer declares a port,
both listener arguments stay disabled, leader election stays on, and no
probe uses `httpGet`. That last one follows from the two disabled
listeners leaving no port for a probe to reach, and it is the worst of
the four to get wrong, because the others wedge an upgrade while a
`livenessProbe` against a disabled listener crashloops the pod on every
cluster, fresh install included. Only the `httpGet` field is forbidden
rather than the probes themselves, so an `exec` probe stays available.
All four run against all three chart variants, because the template
branches on variant inside the container spec and a port added under one
branch would render for that variant alone.

I checked the suite fails for the right reasons instead of trusting it.
Reverting the strategy, declaring a `containerPort` on the operator
container, declaring one under a single variant branch, declaring one on
an initContainer, pointing a bind-address at a real address, turning
leader election off, and adding a readiness, liveness or startup probe
with `httpGet` each turns it red on its own assert. Inserting a
container ahead of the operator trips the container-name pin before the
argument asserts can start describing the wrong container. An `exec`
probe correctly stays green.

The rationale is split by audience. What an operator applying the
rendered manifest has to respect stays in plain comments and travels
into `_out/assets/cozystack-operator-*.yaml`: why the strategy is what
it is, that no port may be declared or bound, and that leader election
has to stay on. Everything that is only about this repository moves into
a `{{/* */}}` template comment that stops at the chart boundary. That
covers the pointer to the test and which assert pins what, where the
strategy was copied from, why the webhook server on 9443 is not a third
port binder, and why `opensearch-operator` reaches the opposite
conclusion on leader election. The other templates in this chart already
keep maintainer notes that way.

### Screenshots

Not applicable, no UI changes.

### Downstream repositories

I left the boxes empty on purpose rather than to skip the question,
because the honest answer is not one of them and someone else should
make the call.

Walking the trigger map in `docs/agents/contributing.md`, nothing fires.
The `hack/` triggers name `hack/e2e-prepare-cluster.bats`,
`hack/package.mk`, `hack/update-crd.sh` and moves or renames under
`hack/`, and this PR only edits the body of
`hack/e2e-install-cozystack.bats`. The `ansible-cozystack` trigger is
renaming an object the role waits on, and
`deployment/cozystack-operator` keeps its name.

The behaviour that role sees does change, though. It runs `kubectl wait
deployment/cozystack-operator --for=condition=Available`, the same
vacuous check this PR replaces here, so the chart change turns its wait
from one that returns instantly into one that means something. Its
`cozystack_operator_wait_timeout` defaults to 300 seconds, the same
budget I gave the gate here, so a cold image pull fits and I do not
think anything needs to change over there. No follow-up PR seemed
warranted for a change that only makes an existing check work, but that
is a judgement call rather than a trigger, which is why the boxes are
empty.

- [ ] 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

```release-note
fix(installer): the cozystack-operator Deployment no longer reports Available with zero running pods. It ran replicas: 1 with maxUnavailable: 1, which made the minimum available replica count 0, so the condition stayed True while the operator was dead and anything reading it, including the E2E install gate, saw green. The rollout strategy is now maxSurge: 1 / maxUnavailable: 0 and the install gate waits for the rollout instead of the condition.
```



<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

* **Bug Fixes**
* Improved operator installation checks by monitoring deployment
rollouts and providing deployment, pod, log, and event details when a
rollout fails.
* Updated operator upgrades to maintain availability during rollout,
including in single-node environments.
* Documented rollout requirements and behavior across supported
installation configurations.

* **Tests**
* Added coverage for rollout settings and upgrade compatibility across
supported installation variants.
  * Clarified Helm unit-test directory coverage documentation.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
 2026-08-06 15:25:39 +03:00
Aleksei Sviridkin 2527e915bd fix(e2e): collect diagnostics for unready pods and record every truncated capture (#3567) 
## What this PR does

The e2e diagnostic tarball exists so a red run can be explained from the
uploaded artifact alone. Several of its collectors answered "nothing is
wrong" instead — some because they selected on a column that never
carries the value they filtered for, others because they stopped early
and left nothing saying they had. Both failure modes look identical to a
healthy cluster from the outside, which is the worst possible shape for
a diagnostic.

Four selectors in `hack/cozyreport.sh` and `hack/cozyreport-summary.sh`
were reading the wrong thing:

- Per-pod evidence was chosen by the STATUS column, so a pod left at
`0/1 Running` by a readiness probe that never passes was skipped. That
is a common way for a suite to time out, and the artifact carried no
trace of the pod the run died on. Selection now compares the two halves
of the READY column; terminal `Succeeded`/`Completed` pods stay
excluded.
- The pending-LoadBalancer section filtered `$4`, which is CLUSTER-IP.
CLUSTER-IP holds an address, `None` or `<none>` and never `<pending>`,
so that loop had never executed. `<pending>` only ever appears in
EXTERNAL-IP, `$5`.
- The cozystack-apps section gated all three of its kinds on `kubectl
get crd <kind>.apps.cozystack.io`. ApplicationDefinition is a CRD but in
a different group, and Tenant comes from an aggregated APIService with
no CRD to ask about, so the probe failed and the section never ran.
- The kamaji section was gated on `cozy-linstor`'s `linstor-controller`
deployment rather than kamaji's own, so a cluster without LINSTOR got no
kamaji logs and no TenantControlPlanes — on exactly the runs that need
them.

The summary had three of its own: the HelmRelease listing printed `$5`
where STATUS is the Ready condition's whole message, reducing every
failed release to one word like "Helm"; the Flux sources listing used a
jsonpath with a nested filter that kubectl rejects outright, so it
printed nothing on every report ever taken; and pod AGE was read as
`$6`, which shifts whenever RESTARTS prints the `7 (4m12s ago)` form —
that is, for exactly the rows worth reading.

The second half is bounding. The pod, VM, VMI and services walks now run
under a count cap and a wall-clock budget, and every read is
individually time-boxed. The point is not the bounds themselves but that
each one names itself when it fires: a walk stopped by the cap or the
clock leaves `COLLECTION-TRUNCATED.txt`, a pod list that never returned
leaves `COLLECTION-FAILED.txt` instead of the empty tree a healthy
cluster produces, a log cut short says so on its own last line, and a
`--tail` bound is recorded beside the logs rather than inside them,
since the platform's controllers log JSON per line and a trailing prose
line costs a parser the whole file. A knob value the collector could not
use is reported too, so a truncation note quoting `600` is never
mistaken for the `10m` somebody actually set. Where `timeout` is absent
the reads run unbounded rather than not at all, and the section says
which of the two happened, because a missing local binary is not a
statement about the cluster.

One behaviour change worth stating plainly, because it is a trade and
not a pure gain: the per-pod section used to be uncapped and read
container logs whole, and it is now bounded by `COZY_REPORT_PODS_MAX`
(40), `COZY_REPORT_PODS_BUDGET` (600s) and `COZY_REPORT_POD_TAIL`
(2000). Below those thresholds the new tree is a strict superset of the
old one — the old selector missed `Running 0/1` entirely and skipped
logs for `Pending` pods. Above them the artifact is smaller than before,
every drop is named in the tree, and `COZY_REPORT_POD_TAIL=-1` with a
larger `COZY_REPORT_PODS_MAX` restores the old reach. Because widening
the selection and capping the walk in one change could otherwise let a
newly-in-scope pod evict one the uncapped walk always collected, the
ordering splits by evidence class before namespace: pods that are not
`Running` — the class the collector took before this — all rank ahead of
pods that are `Running` but unready, so the cap can only ever drop a pod
the old collector did not keep either.

`hack/e2e-capture-previous-logs.sh` gets the same treatment: its notes
now go to `capture-notes.txt` in the artifact as well as to the job log,
and a pod list that never returned is reported as such rather than as
"nothing restarted". That script now creates its own output directory
before its first line of output, which makes the comment in
`hack/e2e-chainsaw/.chainsaw.yaml` wrong about why the once-per-test
guard there holds — the only change to that file is the corrected
comment, saying the `-d` check keys on whether the catch block already
ran rather than on what the helper found.

The rest is test coverage — a new `hack/cozyreport.bats`, an extended
`hack/capture-previous-logs.bats` — and the removal of `EXIT`-trap
cleanup from the bats files those tests now cover. That trap is worth
its own note: inside an `@test` body it replaces the one the `bats`
binary installs for its own bookkeeping, so a test that then fails
prints no TAP line at all. It does not appear as `not ok`; it
disappears, and only the `# bats warning: Executed N instead of expected
M tests` line at the end distinguishes the run from a green one. A guard
test enforces the ban over a named list of files that grows as the
remaining ones are converted.

### Testing

Every collector this touches is now covered by unit tests that run
without a cluster: a new `hack/cozyreport.bats` (166 tests) and an
extended `hack/capture-previous-logs.bats` (34), driven through
`hack/cozytest.sh`. Ten `hack/*.bats` files run green locally, 265 tests
in total, with each file's `@test` count reconciled against its passing
count rather than trusted to the exit status — that reconciliation is
the point of the trap change, since an exit code answers "did anything
fail" and never "did anything run".

The guard test that bans `EXIT`-trap cleanup was checked by committing
the violation rather than by reading it: a folded `trap 'rm -rf "$tmp"'
\` + `EXIT` added to a file on its list turns the guard red and names
that file, and removing it turns the guard green again. `shellcheck
--shell=sh` over the three changed scripts reports strictly fewer
findings than before the change (`hack/cozyreport.sh` SC2086 170 to 116,
SC2162 15 to 13; the three SC2102 in `hack/cozyreport-summary.sh` are
gone), and `hack/e2e-capture-previous-logs.sh` is clean.

The parts that need a cluster — that the bounds fire, and that each one
lands in the artifact — are exercised by the e2e run itself, since these
collectors run on every failed suite.

### Screenshots

Not a UI change.

### Downstream repositories

Walked the trigger map in `docs/agents/contributing.md` against the
diff. This PR touches `hack/cozyreport.sh`,
`hack/cozyreport-summary.sh`, `hack/e2e-capture-previous-logs.sh`,
`hack/e2e-chainsaw/.chainsaw.yaml`, ten `hack/*.bats` files and
`docs/agents/e2e-testing.md`. Nothing is moved or renamed under `hack/`,
no make target changes what it does, and the files downstream consumers
key on (`hack/package.mk`, `hack/common-envs.mk`) are untouched. No
package, values key, CRD, variant, namespace or release asset is
involved.

- [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

```release-note
fix(e2e): the diagnostic report now collects per-pod evidence for pods that are Running but not Ready, which it previously skipped, and several collector sections that silently produced nothing now report correctly. Collection that stops early on a bound now says so inside the artifact, so a truncated report can be told apart from a healthy cluster.
```


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

- **New Features**
- Diagnostic reports now prioritize relevant resources and collect
bounded pod, VM, service, storage, certificate, and Flux data.
- Reports preserve partial results and clearly identify timeouts,
unavailable resources, truncation, warnings, and collection failures.
- Previous-container logs now include bounded capture, namespace
prioritization, failure details, and partial-read evidence.
- Reports target the operator workload and safely handle missing
dependencies or temporary storage failures.

- **Documentation**
- Expanded guidance for reliable end-to-end testing and diagnostic
report collection.

- **Tests**
- Added comprehensive coverage for report generation, log capture,
failure handling, limits, and portability.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
 2026-08-06 15:25:20 +03:00
Aleksei Sviridkin a20f3620b1 fix(e2e): select catch pod logs by the nested release instance label (#3564) 
## What this PR does

The harbor, openbao and qdrant catch blocks selected pod logs on the app
release name. Each of these charts deploys its workload through a nested
HelmRelease named `<release>-system` and passes `fullnameOverride:
<release>` down to it, so the inner chart stamps
`app.kubernetes.io/instance` from its own release name, with the suffix,
while object names come from the override, without it. The selector
matched no pods, and a failed run collected nothing.

- points the three `podLogs` catch collectors at the `-system` instance
label
- adds a `podLogs` collector for Harbor's CNPG database, which no
collector reached before, selected on `cnpg.io/cluster` rather than on
the instance label CNPG merely derives from the Cluster name
- pins the instance label on a resource in an assert each suite already
runs, so a chart renaming that label fails a step instead of quietly
emptying a capture
- prints the instance label of every pod in the namespace from each
test-wide catch, because `podLogs` cannot report that it matched nothing
- keeps the unsuffixed value as a second collector in qdrant only,
because the parent chart's PVC-cleanup Job labels its pod template with
the parent release name and that suite deletes the parent inside a
`try:`

Harbor renders the same hook and deliberately does not get the same
collector: it has no `delete` op, so its parent release is uninstalled
only in Chainsaw's cleanup phase, which no catch can observe: catch is a
defer inside the step, while the cleaner runs from `t.Cleanup`, after
the test function returns. A comment records that asymmetry so the next
reader does not restore the collector for symmetry.

Each comment states what the pin does and does not cover: it does not
catch a wrong value typed into a selector, and it catches nothing when a
run fails before reaching the assert. The qdrant comment is equally
explicit about how narrow its collector's window is, because the hook
always exits 0, so Helm calls it succeeded and `hook-succeeded` takes
the pod away; what remains is an uninstall that hangs with Helm still
waiting.

Telling a lookup's outcomes apart is one helper under
`hack/e2e-chainsaw/_lib/` rather than a copy per call site, with the
outcome logic split from the kubectl call so it can be pinned without a
cluster — the shape `_lib/etcd-probe.sh` already uses. Both the census
and the qdrant cleanup listing report through it, differing only in
wording. Each lookup keeps stderr apart from the value it captures:
kubectl exits 0 while logging that it could not reach an API group, and
an aggregated APIService in that state is what a failed run usually
looks like, so folding the streams would make the "nothing found" branch
unreachable and print that log line as though it were a result.

Note what the extraction costs in CI: touching the shared `_lib/` and
adding a top-level `hack/*.bats` escalates test-impact analysis from the
three suites this change edits to the full Chainsaw suite, so this PR
runs everything and is exposed to the whole spread of known flakes. A
red suite unrelated to harbor, openbao or qdrant is that exposure, not a
defect in this diff.

## Testing

- `hack/pod-label-census_test.bats`, 9 cases over the helper's outcomes,
run via `hack/cozytest.sh` and picked up by `make unit-tests` through
the existing `BATS_UNIT_FILES` glob
- mutation-checked rather than assumed: folding stderr back into the
captured value, propagating kubectl's exit code out of the helper, and
collapsing the failed/empty distinction each turn the suite red, and
each is green again on revert
- `chainsaw lint test --file` on each of the three suites, and `chainsaw
test --no-cluster --config hack/e2e-chainsaw/.chainsaw.yaml
--include-test-regex '^$'` over all three, both with v0.2.15, the
version the sandbox image pins
- `shellcheck --shell=sh` on the helper and on the catch scripts
extracted from the YAML
- the qdrant step catch driven against a stub kubectl that fails: it
reports the failed lookup rather than dying on it
- every selector, pin and comment claim checked against the chart that
stamps it

### Screenshots

Not applicable.

### Downstream repositories

- [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

```release-note
fix(e2e): collect harbor, openbao and qdrant pod logs on a failed run
```


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

* **New Features**
* Added pod-label census diagnostics to improve visibility into workload
labels during test failures.
* Enhanced failure diagnostics for Harbor, OpenBao, and Qdrant by
collecting relevant workload and database logs.
* Added bounded reporting for missing pods, lookup failures, and
incomplete cleanup resources.

* **Bug Fixes**
* Updated diagnostic selectors and readiness checks to match
chart-generated instance labels.
* Improved PVC cleanup diagnostics with clearer reporting of remaining
resources.

* **Tests**
* Added coverage for successful, empty, failed, and partial diagnostic
results.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
 2026-08-06 15:25:09 +03:00
Aleksei Sviridkin 5542ae52b7 fix(e2e): budget tenant scheduling separately from workload readiness 
The tenant backend Deployment had a single 300s budget to reach
condition=Available, and two unrelated variable costs shared it. The
first is scheduling. What the suite establishes before that point is two
tenant nodes Ready, which is weaker than schedulable: a Ready node still
carries node.cilium.io/agent-not-ready until the tenant cilium agent
claims it, and a node the bringup has not finished with is
SchedulingDisabled. Scheduling took 2m18s and 1m57s in the two tenant
suites of one run, leaving the image pull to finish inside what was
left. It did not, and both suites failed with the same message for two
different shortfalls.

Wait for a node that actually accepts a toleration-free Pod before
creating the workload, on its own budget and its own failure message.
The gate encodes the scheduler's rule for such a Pod (Ready, not
unschedulable, no NoSchedule or NoExecute taint) over a custom-columns
probe, prints the node table on both outcomes so a timeout names the
taint that held it, and treats a failed probe as not-schedulable so an
API blip cannot release it. The readiness wait keeps its 300s, now
starting from a schedulable node, and dumps deployment, pod and event
state when it runs out.

Pin the workload image by digest. The tenant workers reach no registry
mirror, so nginx is pulled from Docker Hub on every run either way; the
digest fixes what that pull returns instead of leaving a floating tag
free to change size and content under a fixed deadline.

Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
 2026-08-06 12:20:50 +03:00
Aleksei Sviridkin 115d9bbaaf fix(installer): let the operator Deployment report Available honestly 
At replicas: 1 with maxUnavailable: 1 the Deployment controller takes
the minimum available replica count as 1 - 1 = 0, so the Available
condition reports True with no running pod at all. That reading is what
every consumer of the status sees: an alert on Available, a health
dashboard, or a person reading kubectl get deploy all show green while
the operator is dead.

The replacement is not a tuned pair, it is the platform default written
down. At replicas: 1 the default 25% / 25% resolves to maxSurge 1 and
maxUnavailable 0, because the surge rounds up and the unavailability
rounds down. So this change stops overriding the default rather than
introducing a configuration of its own, and Kubernetes rejects
maxUnavailable: 0 together with maxSurge: 0, so the surge could not
have gone the other way regardless.

The overridden pair was carried over from the earlier cozystack
Deployment, where a darkhttpd sidecar published a host port and
hostNetwork left a surge pod unschedulable on a single-node cluster. It
arrived here in two steps: by the commit before this file was created
the sidecar was already gone and no port was declared, yet the pair
stayed, so it had outlived its own reason before it was ever copied.
That is why looking for a justification in this file's history finds
none. With no port declared anywhere in the pod there is no hostNetwork
port conflict left, so the surge pod schedules beside the outgoing one;
declaring one reintroduces exactly the scheduling failure the pair was
added for. Not exercised on a live cluster in this rework.

Stay on RollingUpdate rather than move to Recreate, which would fix the
condition too, and by a shorter route: MaxUnavailable() reports 0 for
any strategy that is not RollingUpdate, so the minimum available
replica count becomes replicas - 0 = 1 and the condition is honest
without any surge pod at all.
The transition was not attempted, and the failure this project fixed in
ouroboros 0.8.1 does not transfer to it. There the rollingUpdate block
was the one the apiserver defaults onto a Deployment created without
one, so it belonged to no applier and outlived the switch to Recreate.
This Deployment has carried an explicit block since the commit that
created it, and nothing applies it server-side, so whether Recreate
would upgrade cleanly is an open question about the field-ownership
history of the live object rather than a known rejection. What keeps
RollingUpdate here is availability rather than apply semantics:
maxUnavailable: 0 leaves the outgoing operator running when the
replacement never starts, where Recreate deletes it first and leaves
none. Editing the numbers inside the block is
upgrade-safe from every state the field can already be in.

The surge introduces new windows, and leader election covers less of
them than it looks. The operator runs four privileged install steps
before mgr.Start(), and the lease is only acquired inside mgr.Start():
the CRD install, the Flux install, the platform source resource and the
platform PackageSource. All four therefore run in the incoming pod
outside leader election entirely. What the lease does cover is the
reconcile loops, the part that would otherwise conflict continuously
instead of once at startup.

That leaves three seams, and only the first two happen on a healthy
upgrade. The routine one is the incoming pod's install steps against
the outgoing pod's live reconcile loops, since the outgoing pod ran its
own installs at its startup and has been inside mgr.Start() ever since.
Those do not contend for the same fields: the installs write as field
manager cozystack-operator, while the reconcilers write as
cozystack-packagesource-controller and cozystack-package-controller and
go mostly to status subresources. Different owners, different objects,
so this seam is safe by separation rather than by any shared manager.

The second is version skew rather than field contention, and maxSurge:
1 widens it rather than opening it. Under maxSurge: 0 the surge budget
counts desired replicas rather than live pods, so the incoming pod is
created while the outgoing one may still be Terminating - the overlap
existed. What it did not have was a live counterparty: scale-down
sends SIGTERM before the new pod is created, and that cancels the
manager context, so the old loops were already unwinding. Here the
outgoing pod is not signalled until the incoming one is available, so
the overlap runs against a fully live reconciler for a termination
grace period instead of a vestigial one. The incoming pod reports
Ready the instant its
container starts, because there is no readiness probe, so its CRD
install runs while the outgoing binary's reconcile loops are still live
and now writing against a schema the incoming pod has already moved.
Old code against a new schema, which no field manager separates. It is
bounded rather than benign: the outgoing pod exits on SIGTERM within
seconds, a rejected write is transient and the next reconcile repairs
it, and no object is left corrupt. Widening the skew, by adding a probe
that delays Ready or by a migration the old code cannot round-trip, is
what would make this seam matter.

The third seam is install against install, and it needs the outgoing
pod to be restarting, since a crashlooping pod re-runs its four steps
on every restart, which is exactly the broken operator this gate exists
to catch. Three of the four steps use server-side apply under the same
field manager, so they never conflict and resolve last-writer-wins
whenever the two pods carry different manifests. That ordering matters
in one direction, and it is accepted here rather than guarded against:
the CRD install and the Flux install both apply with Force: true, so an
outgoing pod that writes last silently overwrites what the incoming pod
has just applied, with no conflict and no error to exit on. The
incoming pod is inside mgr.Start() by then and will not apply again
until it restarts, so that revert of the upgrade's install phase
persists rather than healing itself. Reaching it needs a restart
landing in the seconds between the incoming pod's installs and the
outgoing pod's SIGTERM. installPlatformSourceResource is the remaining
exception, a read-modify-write of a Get followed by Create or by Update
with the observed resourceVersion, so two pods running it at once can
collide on AlreadyExists or on a conflict; that failure is loud and
self-healing, since any error there exits the process and the pod
restarts, and the object is not left corrupt.

Pin the strategy and all four preconditions the surge makes
load-bearing in a unit test: no container or initContainer declares a
port, both listener arguments stay disabled, leader election stays on,
and no probe uses httpGet. They fail differently and none is visible to
the others, since a declared port stops the scheduler, a bound one only
fails once the incoming process starts, and a second manager without a
lease fails nothing at all until it writes. The httpGet precondition
follows from the two disabled listeners leaving no port to target, and
it is the worst of the four to get wrong: the other three wedge an
upgrade, while a livenessProbe against a disabled listener crashloops
the pod on every cluster, including a fresh install where no surge
happens. Only the httpGet field is forbidden rather than the probes
themselves, because an exec probe stays legitimate. All four are
checked against all three variants, because the template branches on
variant inside the container spec and a port added under one branch
renders for that variant alone. The strategy type is pinned alongside
its two values, since maxSurge and maxUnavailable say nothing on their
own once the type is no longer RollingUpdate.

The listener preconditions are pinned less tightly than the other two,
and the test says so rather than implying coverage it does not have. A
membership assert has no notion of position while Go's flag package
keeps the last occurrence, so a duplicate after the disabled one wins
and the assert still passes, while a duplicate before it loses and the
assert passes just the same - the test cannot tell those two apart.
Moving the disabled flag last fixes the process without making the
assert catch anything. Rejecting the two addresses this
binary defaults to closes the shape a values-gated block to enable
metrics would take, and nothing available here closes an arbitrary
address, since helm-unittest cannot express that a flag appears at most
once.

Note in hack/helm-unit-tests.sh that the loop covers packages/core,
which its comment omitted. The suite added here lives there, so a later
edit reconciling the loop with the stale comment would have silently
stopped running it.

Split the rationale by audience rather than keeping it all in shipped
comments. What an operator applying the rendered manifest has to respect
stays in plain comments and travels into the install artifacts: that the
pair is the default, why maxUnavailable must stay 0, that no port may be
declared or bound, that no probe may use httpGet, and that leader
election has to stay on. Most of what stays in the template comment is
what the chart's own suite pins, so it cannot drift out of step with the
chart without a test going red: the four preconditions and which assert
covers each, and why the variant loop is needed. Three things there are
deliberate exceptions, kept because anyone editing the strategy has to
know them and no assert can carry them - that the webhook server
configured on 9443 is not a port binder, since controller-runtime adds
it to the runnables only inside GetWebhookServer() and this binary never
calls it; that two overlaps between the pods sit outside the lease; and
why RollingUpdate is kept rather than Recreate. Those name what exists.
The seam analysis above, which is how each one works, deliberately stays
here instead. It describes another file's internals, nothing pins it,
and a commit message is fixed in time, so it cannot quietly fall out of
date the way a comment can.

Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
 2026-08-05 18:15:41 +03:00
Aleksei Sviridkin e80b8e370f fix(e2e): collect logs for unready pods and record truncated captures 
The failure snapshot selected pods by their STATUS column, so a pod
whose container runs while its readiness probe never passes -- STATUS
Running, READY 0/1 -- was dropped from the report entirely. That is a
common way for a suite to time out: the replica never joins, its owning
HelmRelease sits in InProgress, and the artifact carries no trace of the
pod the run died on. Select on the READY column instead, which also
covers the partially ready multi-container case, and keep every phase
that was collected before. On one degraded cluster that is four of the
seven broken pods the old filter reported nothing about. The VMI and VM
sections had the same blind spot and now share one selector that reads
READY as the last field, because a Pending object prints fewer columns
than a Running one and the two kinds differ only in how many columns
precede READY. READY counts ready containers rather than the pod's Ready
condition, so a pod held back by an unsatisfied readiness gate is out of
scope; a test pins that rather than leaving it to be found in an empty
directory.

Logs are requested for every container of a selected pod rather than
skipped for Pending ones, and the per-pod collection moved into a helper
so it can be driven against a stub kubectl. kubectl's refusal is the
finding, and an empty result is ambiguous on its own: a read against an
unscheduled pod exits 0 having printed nothing, which is what a
container that started and stayed quiet also leaves. Every site that
classifies a failed read now orders its branches the same way, naming
the timeout first where both a cutoff and an uncapturable stderr apply.
The CRD gate in the summary had no such branch at all: it returned in
silence, which is byte-identical to the CRD being absent, so every
section behind that gate vanished from a report whose only real problem
was that the machine writing it had nowhere to put a temp file.

Collection used to stop silently. A walk that hit its cap said so in the
job log and nowhere in the artifact. Now the pod walk leaves
COLLECTION-TRUNCATED.txt when it stops at COZY_REPORT_PODS_MAX or
COZY_REPORT_PODS_BUDGET, CONTAINER-LOGS-TRUNCATED.txt on a pod cut off
part way through, COLLECTION-FAILED.txt when the pod list never came
back rather than the empty tree a healthy cluster produces, and
COLLECTION-UNBOUNDED.txt when timeout is missing locally, which is a
fact about the machine writing the report and not about the cluster.
Tenant namespaces are collected first, so a cap that only bites on a
broadly degraded cluster is not spent on platform pods while the pod the
suite died on falls off the end.

Container logs are read with --tail=2000 where they were previously read
whole, and every bounded file states the bound it was written under,
because --tail drops the oldest lines and nothing else in the output
would say so. COZY_REPORT_POD_TAIL=-1 restores the complete log.

The bounded whole-object read no longer merges stderr into pod.yaml,
vm.yaml and vmi.yaml, which are read by yq rather than by eye: a
warning on an otherwise complete read left an unparseable file. stderr
goes to a scratch file and is copied in only when kubectl returned no
object, and the truncation markers those files carry are written as YAML
comments, since a bare marker line after a complete mapping is a second
top-level node.

Six latent faults surfaced while making these reads report. The kamaji
module was gated on the LINSTOR controller, so a cluster without LINSTOR
got no kamaji evidence at all, which is what a tenant-kubernetes
bootstrap timeout needs most. The Flux sources section nested one
jsonpath filter inside another, which kubectl rejects outright, so it
printed nothing on every report ever taken and read as "all sources
Ready". The HelmRelease listing took a single field as STATUS where
STATUS is the Ready condition's whole message, so every failed release
rendered as one word. The services section filtered for a pending
LoadBalancer address on the CLUSTER-IP column, which never carries that
value. The cozystack module read its image from a Deployment the
installer stopped shipping when it moved to the operator, putting
kubectl's NotFound in the first file of the report. And the cozystack
apps section gated all three of its kinds on a CRD in an aggregated API
group, so it never ran once; two of those names were wrong, and both
are collected again.

The tests avoid EXIT-trap cleanup, and e2e-testing.md says why: a bats
test that installs one and then fails prints no TAP line at all, so
output grepped for `not ok` reads as green, while hack/cozytest.sh
reports the same failure correctly. A guard enforces that over a named
list of files rather than a glob with exceptions, because a list of
exceptions is the same claim written so that forgetting to shorten it
makes it false.

Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
 2026-08-05 17:37:04 +03:00
Aleksei Sviridkin 3fc8e524b2 fix(e2e): select catch pod logs by the nested release instance label 
The harbor, openbao and qdrant app charts deploy their workload through
a nested HelmRelease named `<release>-system`, and pass
`fullnameOverride: <release>` down to it. The inner chart therefore
stamps app.kubernetes.io/instance from its own release name, with the
suffix, while object names come from the override, without it. The
catch blocks selected on the outer release name, so podLogs matched no
pods and every failed run collected an empty result.

A pod name says nothing about its instance label; the two come from
different sources on purpose. In qdrant the unsuffixed value is
therefore not retired but kept as a collector of its own: the parent
chart's PVC-cleanup Job labels its pod template with the parent release
name, and that suite deletes the parent inside a `try:`, so the Job is
in scope while the test still runs. The comment there is explicit about
how narrow that window is -- the hook always exits 0, so Helm calls it
succeeded and `hook-succeeded` takes the pod away; what is left is an
uninstall that hangs with Helm still waiting on it.

Harbor renders the same hook and does not get the same collector. It
has no `delete` op, so its parent release is uninstalled only in
Chainsaw's cleanup phase, which no catch can observe: catch is a defer
inside the step, while the cleaner runs from t.Cleanup, after the test
function returns. That collector would be the same permanently-empty
capture this commit removes. A comment records the asymmetry so the
next reader does not restore it for symmetry. openbao renders no such
hook at all.

Each workload value is now also pinned on a resource in an assert the
suite already runs, so the chart renaming a label fails a step instead
of quietly emptying a capture. The comments state what that does and
does not cover: it does not catch a wrong value typed into a selector,
and it catches nothing when the run fails before the assert.

Harbor's database was reached by no collector at all. It is selected on
`cnpg.io/cluster`, CNPG's own primary label, rather than on the
instance label CNPG merely derives from the Cluster name -- the primary
label has nothing to drift from, and it is what the postgres suite and
the WorkloadMonitors already use.

Each lookup keeps stderr apart from the value it captures. kubectl exits
0 while logging a klog line about an API group it could not reach, and
an aggregated APIService in that state is what a failed run usually
looks like; folded into the value, that line makes the "nothing found"
branch unreachable and prints as though it were a result.

Telling those outcomes apart is one helper in hack/e2e-chainsaw/_lib/
rather than a copy per call site, separated from the kubectl call so it
can be pinned without a cluster, the way etcd-probe.sh already is. Both
the census and the qdrant cleanup listing report through it, differing
only in wording, so the discrimination is implemented and tested once.
The tests cover each outcome for both wordings, including the zero exit
with a klog line on stderr -- the case a folded stream turned into a
row.

A collector that matches nothing cannot say so: podLogs is silent, and
`kubectl get -l` exits zero on an empty match. Each test-wide catch
prints the instance label of every pod in the namespace, and the qdrant
cleanup catch reports its three outcomes apart from a single call --
printing what that call returned rather than running it again, which
could put a healthy listing under a failure heading.

That census runs on a degraded cluster, where kubectl otherwise retries
discovery against an unreachable apiserver until something kills the
script, so every lookup carries `--request-timeout`. Measured against an
unreachable apiserver, a bounded call costs about five times the flag,
since kubectl retries discovery five times before it gives up. Each
script then carries an explicit op timeout sized above its own calls --
2m for the single-call censuses, 5m for the qdrant cleanup listing,
which makes three. An inner budget is what should fire: a SIGKILL at the
op boundary yields nothing, where a kubectl that gives up leaves an
error to print.

Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
 2026-08-05 16:46:50 +03:00
Aleksei Sviridkin d3ae21b7f0 fix(e2e): name every way the previous-instance capture falls short 
The capture wrote its reasons to the job log only. A reader holding the
uploaded artifact saw N logs in a directory with no way to tell N from
all of them: the container cap that dropped the rest, a pod list that
never returned, a previous instance that produced nothing, and a read
the outer backstop cut short all looked alike from outside. Every one
of those lines now goes to capture-notes.txt as well, and the directory
is created before the first of them, so a capture that found nothing is
distinguishable from one that never ran.

A pod list that failed is no longer reported as nothing having
restarted. That is a claim about the cluster which the script never
observed, and it is the claim most likely to talk a triager out of the
crash-loop hypothesis on exactly the run where it is right.

Exit 137 no longer asserts whose signal ended the read. 124 comes from
timeout and from nothing else, but 137 is 128+SIGKILL, which the kill
grace produces and so does the OOM killer or a teardown signalling the
process group. Both belong in the cut-off branch, since the capture
ends early either way, but reported flatly as the script's own timeout
a read killed at second two reads as one that waited the full eighteen.

COZY_PREVLOG_TAIL rejects a value kubectl cannot use and says so. A
zero tail asks for no lines while kubectl still exits 0, which the
empty-output branch would otherwise report as the container having
logged nothing, discarding the only remaining copy of a log the kubelet
has already collected. A leading zero is rejected because `$(( ))`
reads it as octal while `[` reads it as decimal, so one string means
two numbers. Each kept log names the bound it was written under, since
--tail drops the oldest lines and nothing else in the output says so,
and -1 suppresses that note because no bound was applied.

An output directory that cannot be written is reported as the local
condition it is. It arrives as an argument, so it can be a file, or
missing, or unwritable; the redirection then fails before kubectl runs
and hands the shell's own status to the branch that names kubectl,
which reported "no previous instance retrieved (kubectl exit 2)" once
per container.

Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
 2026-08-05 16:03:57 +03:00
Aleksei Sviridkin ceccac27b0 test(hack): clean up bats temp dirs without EXIT traps 
A `trap ... EXIT` inside an `@test` replaces the one the bats binary
installs for its own bookkeeping, so a test that then fails prints no
TAP line at all. Not `not ok`, nothing: the run ends with a warning
that N of M tests executed, and anyone judging it by grepping for
`not ok` reads a green suite. Cleanup moves to the last line of each
body instead. Both runners set -e, so a failed test leaves its scratch
directory behind, which is what you want to look at anyway.

hack/promote-retag_test.bats was failing this way already, reporting
five passes and no failures while six of its eleven tests printed
nothing. Those six invoke the script for real, and it requires
sha256sum; they pin PATH to the stub directory plus /usr/bin and /bin,
which is where coreutils installs that binary and where macOS does not,
so the failure was of the platform rather than of the code and CI never
saw it. The stub directory already supplied yq and skopeo for the same
reason and now supplies sha256sum too, resolved from absolute
candidates because the stub itself is first on PATH.

Two file headers described the trap arrangement they no longer use and
are corrected rather than left to outlive it.

Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
 2026-08-05 16:03:57 +03:00
IvanHunters 0612a538a3 fix(e2e): bound the Talos-capture failure-branch forensic kubectl calls 
The describe/get-events/get-certificaterequests calls that run when the
reader Certificate or the diagnostics Pod misses its readiness wait were
unbounded, yet those branches fire precisely when the management apiserver is
wedged. A stalled forensic call there can burn minutes of the 50m step budget
before the pre-existing (a2)/(c) diagnostics and the EXIT-trap tenant
snapshot run. Add --request-timeout=30s to them, matching the happy-path
capture calls.

Signed-off-by: IvanHunters <xorokhotnikov@gmail.com>
 2026-08-05 15:13:56 +03:00
IvanHunters cfbfd770b8 fix(e2e): bound the tenant Talos capture kubectl calls 
The in-guest diagnostics chain runs only when the sandbox is already
degraded enough to miss the 18m node-join deadline (the documented
management-etcd fsync-storm regime), yet its kubectl legs were unbounded
while every talosctl leg is wrapped in timeout. A stalled apiserver call in
the (b) capture could consume the remaining step budget before (a2), (c) and
the EXIT-trap tenant crust-gather run, so a diagnostics feature could reduce
total diagnostic yield under the exact conditions it targets.

Add --request-timeout=30s to the raw Certificate/Pod apply, the three
reader-Secret reads and the worker VMI list, and route the Certificate Ready
wait through kubectl_wait_retry so a single transient etcd-leader flap
retries instead of aborting the whole capture. Update the bats assertion for
the reworded wait call. Also correct a stale comment: the chainsaw step
timeout is 50m, not 40m.

Signed-off-by: IvanHunters <xorokhotnikov@gmail.com>
 2026-08-05 14:47:49 +03:00
IvanHunters 5e3c286e20 fix(e2e): read files with cat in the Talos diagnostics bats helper 
assert_file_contains used $(<file), a bash-only construct. cozytest.sh is
run under /bin/sh (dash on the Ubuntu CI runner via Makefile), where
$(<file) expands to an empty string instead of the file contents, so every
assertion after it failed and reddened the unit-tests job, which in turn
skips the e2e gate and leaves the diagnostics path unexercised. Read the
file with $(cat ...), which is POSIX and behaves identically under bash and
dash.

Signed-off-by: IvanHunters <xorokhotnikov@gmail.com>
 2026-08-05 14:19:50 +03:00
Aleksei Sviridkin 7739d1e294 feat(multus)!: stage reference CNI plugins from the multus image 
Multus-backed NetworkAttachmentDefinitions delegate to the upstream
containernetworking reference plugins (bridge, macvlan, ipvlan, ...),
and nothing guarantees a node has them in the directory Multus
searches.  On k3s they live under /var/lib/rancher/k3s/data/cni while
Multus and Cilium install into /opt/cni/bin, so a bridge-type NAD fails
there with `failed to find plugin "bridge" in path [/opt/cni/bin]`.

Bundle 14 of the release's plugins into the multus image and add an
install-cni-plugins init container that installs them into the host
/opt/cni/bin, mirroring the existing install-multus-binary container.
loopback, dhcp, tap and dummy are left out; the Dockerfile records why.

Gate it on a new chart value, stageCniPlugins, resolved per bundle and
on for both.  Talos bundles only bridge, firewall, flannel, host-local,
loopback and portmap, so the other ten are missing there exactly as on
generic Linux; the four that overlap are rewritten with the same
upstream version, v1.9.1 on both sides, rather than an older one.  A
marker in the image Dockerfile records the Talos release that pin was
compared against, and a test holds it to the release the platform
ships.  multus therefore moves out of system.common-packages into the
two variant branches, the way linstor already does for talos.enabled.
networking.stageCniPlugins overrides the variant default in either
direction.

Where staging is on the install is unconditional: a plugin is replaced
on every pod recreation, not only when the destination is missing.  A
destination that is already a directory is reported and left alone,
since replacing it is not something this can do safely.  Turning the
value off afterwards does not undo it; nothing here removes or
restores a plugin.

The container is not privileged and takes no mount propagation.  It
runs in the spc_t SELinux domain, without which an enforcing node
refuses the write.  Each plugin is installed under a temporary name and
swapped in with a rename: /opt/cni/bin is shared with cilium and
kube-ovn.  The plugin tarball is checksummed before extraction.

A plugin that cannot be installed is reported on stderr and to
/dev/termination-log, and the container still exits 0: Multus stays the
node's primary CNI and nothing removes its config, so an init container
that never completes would leave the shim with no daemon behind it.

The manifest change lives in customize-deployment.patch so it survives
make update.

BREAKING CHANGE: enabling stageCniPlugins replaces reference CNI
plugins in /opt/cni/bin on existing nodes, Talos included -- four of the
names it writes are ones the Talos node image also carries.  Opt out
before upgrading: declining afterwards does not restore what was
replaced.

Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
 2026-08-05 14:19:47 +03:00
Aleksei Sviridkin 517dd11d1a fix(tests): wait for operator rollout instead of condition=Available 
condition=Available answers whether the Deployment sits within the
slack its rollout strategy allows, not whether the operator came up.
That is the wrong question for an install gate, and it stays wrong
whatever the strategy is: any maxUnavailable equal to replicas drives
the minimum available replica count to 0 and the condition then reports
True with no running pod. Measured against an operator stuck in
ImagePullBackOff, the old check passed in 156 ms, and the install went
on to fail two minutes later on the CRD wait, which pointed the
investigation at the wrong component.

rollout status reads updated and available replicas directly and only
succeeds once the updated pod is available, which holds under any
rollout strategy. Dump the deployment, pods, operator logs and events
on failure so the cause is legible where it is detected.

Drop --wait from the helm step. Helm 3 decides a Deployment is ready
with the same replicas - maxUnavailable arithmetic, so on the current
strategy it is satisfied at zero ready pods and gates nothing; were the
strategy tightened it would instead fail the install on its own 2m
budget before this gate runs, hiding the diagnostics behind a bare Helm
timeout. That arithmetic is specific to Helm 3, since Helm 4 reworked
--wait into a wait strategy with a different default, so treat the
reason as version-bound. The conclusion is not: one gate that reports
what it saw is better than two that can disagree about whose budget
expired first, whichever major is installed. Hooks are waited for with
or without the flag. The 5m budget here covers scheduling and a cold
pull of an image that is not in the prepull set.

Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
 2026-08-05 13:13:26 +03:00
Myasnikov Daniil 60e44b24f1 fix(e2e): make tenant Talos diagnostics reliable 
Fix admission, capture setup, timeout headroom, and portability.

Assisted-By: GPT-5 <noreply@openai.com>
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
 2026-08-05 14:52:31 +05:00
Myasnikov Daniil c8ec60cfe3 test(e2e): remove optional search tool dependencies 
Use Bash and awk assertions in the Talos diagnostics regression tests so
they run in the standard E2E sandbox without optional search utilities.

Assisted-By: GPT-5 <noreply@openai.com>
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
 2026-08-05 14:20:33 +05:00
Myasnikov Daniil 80152b1a07 fix(e2e): capture Talos logs for tenant worker join failures 
Tenant Kubernetes worker join failures currently stop at host-side VMI
and CSR state because the worker-only topology does not materialize a
tenant talosconfig.

Mint a short-lived os:reader client certificate and run talosctl from a
hardened same-namespace Pod after the Ready deadline. Preserve bounded
dmesg and kubelet captures for every worker without masking the original
test failure.

Assisted-By: GPT-5 <noreply@openai.com>
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
 2026-08-05 13:18:20 +05:00
Myasnikov Daniil 83fce03839 test(release): scope the freeze pins to their steps and cover the guard 
Three assertions extracted a step block, checked it was non-empty, and
then ran their grep over the whole workflow. An unrelated step carrying
the same condition would keep them green after the protected step lost
its own guard, which is precisely the slip they exist to catch. Each now
greps its own block, and `id: freeze` moved into the freeze step's test
where it belongs.

The force-detection greps looked for `+refs/heads/` and `+refs/tags/`,
which is not how git spells a forced refspec: the `+` leads the refspec,
so `git push origin "+HEAD:refs/heads/$BRANCH"` force-moves the branch
and passed both checks. Match a leading `+` on any push refspec instead.

Adds a test for the new build-release dispatch guard: the pattern
anchored at both ends, the ref_type comparison and not merely its env
wiring, a hard failure rather than a skip, and the guard ordered before
the checkout.

Every pin mutation-tested — forcing either push with a `+` refspec,
parking the freeze condition on another step, dropping `id: freeze`,
removing or unanchoring the dispatch guard, demoting it to exit 0, and
relocating it after the checkout all turn the suite red.

Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
 2026-08-04 14:23:51 +05:00
Myasnikov Daniil e8675dd321 docs(release): reword the changelog-backstop comments for the freeze model 
Three comments still described the changelog reaching main by default,
with a release-X.Y-targeted promote PR as a patch-release corner case.
After the freeze that is inverted: promote-rc.yaml bases the promote PR
on release-X.Y whenever the branch exists, and cutting vX.Y.0-rc.1
creates it, so every release merges its changelog onto the maintenance
line and none reach main on the way. The tags.yaml backstop is now the
normal route, and its self-skip on `exists` is the exception.

These comments are the first thing anyone debugging a missing changelog
reads, so pointing at the wrong model of which path is normal costs real
time. Reworded in place: the generate-changelog job header, the
three-paths comment above check_changelog, its inline at_tag note,
finalize's explanation of why update-releasenotes.yaml is not trusted
with the release body, and the contract test that pins the porting
behaviour. Prose only, no behaviour change.

Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
 2026-08-04 14:23:32 +05:00
Myasnikov Daniil e532844007 test(release): pin the freeze and backport-target contracts 
Neither cut-prerelease.yaml nor backport.yaml had any test coverage, and
neither can be exercised by a PR lane: the first runs only on
workflow_dispatch and its whole purpose is a one-way write-once side
effect, the second only on a merged main-targeted PR. Every invariant
here is one that would otherwise first report a slip at a real release.

Pinned, following promote-gate-contract.bats so a guard demoted to a
comment cannot satisfy a pin:

  - the freeze is gated to kind == 'rc' && patch == '0'. Widening either
    half re-opens a frozen line with no error surfacing.
  - neither the branch create nor the tag push is forced, in any spelling
    including a `+refs/` refspec. A force-push on the branch would drag
    in everything merged since the freeze.
  - an existing release-X.Y is left untouched rather than reused.
  - the refuse gate is gated on a main dispatch and runs BEFORE the tag
    push, so a mistaken dispatch cannot burn a write-once tag name; the
    freeze runs after it, at the commit that was actually tagged.
  - backport targets come from enumerated branches, with no
    getLatestRelease and no min-1 arithmetic, and `previous` is the
    second-newest existing line.
  - the comparator sorts numerically descending, which is the only reason
    release-1.10 outranks release-1.9. A slip to the default
    lexicographic sort aims every backport at a stale line silently.
  - freezing a line dispatches its first build-release run.

backport.yaml's logic is a github-script block whose comments name
getLatestRelease and min-1, so the assertions over it strip `//`
comments; the YAML `#` filter alone would let a comment satisfy a pin.

The Makefile discovers hack/*.bats, so this runs under `make unit-tests`
with no wiring.

Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
 2026-08-03 21:38:41 +05:00
myasnikovdaniil f1f383626e fix(ci): overlay images from the PR base branch, and publish per-line artifacts (#3471) 
## What this PR does

The PR finalize job overlays image refs for every package a PR did not
rebuild, so e2e and the installer do not test last-release images for
everything outside the PR's build matrix. It could only ever read
`cozystack-packages:main`, which meant a **release-line PR was handed
main's binaries to run against its own line's charts**.

cozystack/cozystack#3437 is the demonstration: a one-line change on
`release-1.6` deactivating an app failed install deterministically,
twice, with

```
helmrelease/backupstrategy-controller: Helm install failed …
error validating data: SchemaError(github.com/cozystack/cozystack/pkg/apis/core/v1alpha1.Option.spec):
unknown model in reference: "github.com~1cozystack~1cozystack~1pkg~1apis~1core~1v1alpha1.OptionSpec"
```

main's `cozystack-controller` served an aggregated OpenAPI that
`release-1.6`'s charts could not validate against. Nothing in that PR
was broken; the lane was. Both branches carry `option_types.go` and key
`OptionSpec` identically in the committed generated OpenAPI, so this is
a generation mismatch at runtime, not a codegen drift.

This PR fixes it by giving every base branch its own artifact to overlay
from, rather than by turning the overlay off.

### The change

1. **`pull-requests.yaml`** reads `cozystack-packages:${BASE_REF}`
(`github.base_ref`) instead of a hardcoded `:main`.
2. **`build-release.yaml`** (new) publishes that artifact for maintained
`release-<major>.<minor>` branches exactly as `build-main.yaml` does for
main: every image tagged with the branch, and the whole packages tree
pushed with each reference digest-pinned to what the run just built.

An earlier revision of this branch simply skipped the overlay for
non-main bases. That fixed the wrong-images problem but left
release-line PRs testing their line's *last release*: for any package
the PR did not rebuild, the committed ref is the released digest, so a
component changed by an earlier backport was still exercised as its
pre-backport binary until the next rc. Per-line artifacts remove that
gap too, which is why the guard was replaced rather than kept.

### Three deliberate choices

**The trigger matches line branches only** (`release-[0-9]+.[0-9]+`).
The per-release and rc staging branches `promote-rc.yaml` and
`tags.yaml` create — `release-1.6.1`, `release-1.6.0-rc.4` — must not
trigger a full rebuild; their images come from the tag build and
rebuilding them is waste.

**`WRITE_CACHE` stays `0`.** `CACHE_REGISTRY/<img>:buildcache` is a
single ref per image and `build-main.yaml` is deliberately its only,
serialized writer so concurrent builds cannot race on the cache manifest
— the 409 class #2711 fixed for image tags. A line build can overlap a
main build, so writing here would reintroduce that race. Line builds
read the cache.

**A missing artifact still degrades to committed refs, but says so.** On
a release line it emits a `::warning::` naming the branch. Silent
degradation is indistinguishable from a working overlay, which is how a
mis-specified branch filter could hide for a whole release cycle.

### Cost

One `make build` per push to a maintained line — in practice per merged
backport. That is the price of release-line PRs testing their line's tip
instead of its last release.

### Verification

`hack/overlay-main-images_test.bats` pins the artifact tag to the base
branch, rejects a hardcoded `:main` in either overlay step, and pins
`build-release.yaml`'s branch filter, image tag and `WRITE_CACHE: '0'`.
Mutation-checked: restoring `:main`, setting `WRITE_CACHE: '1'`, and
broadening the filter to `release-*` each fail a test. 13/13 green;
`actionlint` and `zizmor` clean.

Worth an explicit ack in review: the branch-filter pattern is the one
thing no local test can prove, since only GitHub evaluates it. If it
does not match, `build-release` never runs and the new `::warning::` is
what surfaces it on the next release-line PR.

### Backport

`release-1.6` needs this too — for `pull_request` events GitHub builds
the workflow from the merge ref, so a release-line PR only stops
receiving main's images once the change is on its base branch.
cozystack/cozystack#3472 carried the interim guard and is closed in
favour of backporting this instead.

### Release note

```release-note
NONE
```


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

* **New Features**
* Added a “Build release line” workflow that builds and publishes images
and the packages artifact for maintained `release-<major>.<minor>`
branches.

* **Bug Fixes**
* Updated PR workflow finalization to use base-branch–specific package
overlays, avoiding incorrect main-branch image references when targeting
release branches.
* Improved fallback behavior when the base-branch packages artifact is
unavailable.

* **Tests**
* Added workflow wiring tests to verify base-branch artifact usage and
that each maintained release line publishes its own packages artifact.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
 2026-08-03 18:22:47 +05:00