Commit Graph
5550 Commits
Author SHA1 Message Date
IvanHunters b1ce856c47 test(platform): cover the 53-char child-name overflow skip branch
Every warn-pin-skip hardening branch in migration 54 had a regression fixture
except the >53-char child-name overflow guard. Add one: a 33-char cluster name
plus pool md0 yields a 54-char child release name, so the overflow branch must
warn, pin the live pool objects with resource-policy=keep, skip adoption, and
let the run stamp. Without the guard the flow reaches kubectl apply of an
over-long HelmRelease name, the apiserver rejects it, and set -e exits before
the version stamp, deadlocking every tenant's platform upgrade. Verified
non-vacuous: neutralising the length guard reddens this fixture.

Signed-off-by: IvanHunters <xorokhotnikov@gmail.com>
2026-08-14 17:37:51 +03:00
IvanHunters c7d3d40810 test(platform): assert the non-scalar-field skip branch pins live pool objects
The existing bad-scalar test ran with the pool objects absent, so it pinned
the warn+skip+no-child-HR contract but not the pin itself. Add a companion that
presents the live MachineDeployment, MachineHealthCheck, WorkloadMonitor and
KubevirtMachineTemplate and asserts each is annotated resource-policy=keep with
no ownership transfer, mirroring the non-object-nodeGroups companion. Verified
non-vacuous: removing the branch's pin_pool_objects call reddens it.

Signed-off-by: IvanHunters <xorokhotnikov@gmail.com>
2026-08-14 17:21:14 +03:00
IvanHunters 8046144494 docs(kubernetes): scope the adoption guard comment to the etcd-present branch
The guard lives in the else of 'if not $etcd', so it does not run during the
awaiting-etcd window, where the chart renders only the status beacon. Note that
an owned+unpinned MachineDeployment is not refused in that transient window.
This is a pre-existing exposure (the pre-split chart also rendered pools only in
this branch) that closes once etcd resolves and a real upgrade re-renders; the
comment previously claimed an unconditional refuse-to-render.

Signed-off-by: IvanHunters <xorokhotnikov@gmail.com>
2026-08-14 17:21:14 +03:00
IvanHunters f9065a9455 fix(migrations): correct the non-scalar-field skip remedy
The non-scalar scalar-field skip branch told the operator to restore the field
and 're-run adoption', but migration 54 stamps the version unconditionally and
does not re-run, so that remedy is a dead end. Align it with the sibling skip
branches: restore the scalar, then recreate the pool, and state plainly that
the migration has already stamped and will not re-run.

Signed-off-by: IvanHunters <xorokhotnikov@gmail.com>
2026-08-14 17:21:14 +03:00
IvanHunters e68784b3b6 test(kubernetes): cover the release-name predicate in the worker adoption guard
The adoption guard in cluster.yaml refuses a control-plane-only render only
when a MachineDeployment matches all three predicates: spec.clusterName,
meta.helm.sh/release-name owned by this parent release, and not pinned with
resource-policy=keep. The suite left the release-name predicate untested: the
adopted-pool case carried both a child release-name and a keep pin, so the keep
predicate alone exempted it and deleting the release-name predicate kept every
case green.

Add a natively-created (greenfield) fixture: child release-name, no keep pin.
Its clusterName and unpinned state are both true, so the release-name predicate
is the sole reason it renders. This is the routine post-split parent upgrade of
every cluster, the guard's highest-blast-radius path.

Per-conjunct mutation now: dropping release-name reddens the greenfield case,
dropping keep reddens the pinned case, dropping clusterName reddens nothing (an
MD from another cluster cannot carry this release name, so clusterName is
belt-and-braces). Baseline 6 of 6 green.

Signed-off-by: IvanHunters <xorokhotnikov@gmail.com>
2026-08-14 17:17:30 +03:00
IvanHunters 8815468c9b fix(migrations): route non-scalar nodeGroup scalar-fields through warn+pin+skip
Migration 54 copied per-group scalar fields (storageClass, instanceType,
diskSize, minReplicas, maxReplicas, logSerialConsole, maxUnhealthy,
nodeStartupTimeout) verbatim into the child HelmRelease values. The nodeGroups
map value was never fully schema-constrained, so a kubectl/dashboard edit could
store one of these as an object or array; copied through, it fails the
KubernetesNodes schema on the child install and leaves that child release stuck
Failed.

Validate the scalar fields before building the child values and route a
non-scalar value through the same warn + pin + skip contract as the invalid
name, overflow and non-object branches: pin the pool's objects prune-proof and
skip adoption, so the pool keeps running unmanaged until the operator restores
the stored value. Container-typed fields (roles, resources, gpus, kubelet) are
left untouched.

Signed-off-by: IvanHunters <xorokhotnikov@gmail.com>
2026-08-13 18:44:07 +03:00
IvanHunters 5999807aa6 fix(kubernetes-nodes): guard against silent clusterName drift on pool adoption
The release name kubernetes-nodes-<cluster>-<pool> does not encode where the
cluster segment ends, so several (.Values.cluster, pool) splits reconstruct the
same object name kubernetes-<cluster>-<pool>. An operator who edits spec.cluster
to another such value (immutability is enforced only by the dashboard, not the
aggregated apiserver, per docs/storage-immutability.md) prunes no worker VM but
silently flips spec.clusterName and the pool WorkloadMonitor selector; CAPI
rejects the immutable MachineDeployment.spec.clusterName Update loudly while the
WorkloadMonitor drift stays silent.

Extend assertNoForeignPool to refuse the render when the live MachineDeployment
of the reconstructed name carries a spec.clusterName that disagrees with the
clusterName this pool renders, naming the value to restore. Inert offline.

Signed-off-by: IvanHunters <xorokhotnikov@gmail.com>
2026-08-13 18:39:38 +03:00
IvanHunters 17e27e459f test(kubernetes): cover the adoption guard live-fail path
The previous suite only pinned that the guard is inert on an empty offline
lookup; reverting the guard kept it green. Seed a live lookup through
kubernetesProvider (as parent_version_skew_test does) and pin the real
contract: an owned, unpinned MachineDeployment fails the render; an owned MD
pinned with resource-policy=keep renders; an MD adopted onto its child release
renders; an MD from a different cluster renders.

Signed-off-by: IvanHunters <xorokhotnikov@gmail.com>
2026-08-13 17:15:27 +03:00
IvanHunters 836e1b887c fix(kubernetes): exempt keep-pinned worker pools from the adoption guard
The Phase 2 worker-pool adoption guard flagged any worker MachineDeployment
still owned by the parent release, without checking helm.sh/resource-policy.
Migration 54 deliberately pins keep on pools it cannot adopt (non-object
nodeGroups, invalid rfc-1123 child name, child name over 53 chars, non-object
group value) and leaves the parent release-name in place, so keep is its
processed-and-made-prune-proof signature. The guard therefore converted that
documented degraded-but-running state into a permanent upgrade deadlock for
the tenant.

Skip MachineDeployments carrying helm.sh/resource-policy=keep: a pinned pool
is already safe from Helm prune, so the only remaining data-loss case is an MD
owned by this release and unpinned, which means the migration never processed
the pool. Reword the failure message accordingly.

Signed-off-by: IvanHunters <xorokhotnikov@gmail.com>
2026-08-13 17:15:19 +03:00
IvanHunters 64815cf12b Merge remote-tracking branch 'origin/main' into pr-3315-lgtm 2026-08-12 11:01:04 +03:00
IvanHunters 5cd2502d01 feat(kubernetes): guard against pruning un-adopted worker pools on upgrade
The Phase 2 split makes the parent chart render control-plane-only and relies
on the platform migration (a pre-upgrade hook) having adopted each worker pool
onto its child kubernetes-nodes release first. If that migration does not run
(its version slot was already stamped, or it was renumbered out on a
merge/backport), the live worker MachineDeployment is still owned by this
release and Helm's control-plane-only re-render would prune it, cascade-deleting
the tenant's worker VMs.

Add a render-time guard: on an upgrade, when the render can see the cluster
(release-namespace canary lookup non-nil, so client-side renders stay a no-op),
look up worker MachineDeployments whose spec.clusterName is this cluster and
whose meta.helm.sh/release-name is still this parent release (un-adopted) and
fail with a legible message instead of rendering. The upgrade blocks with a
reason and the workers keep running on the prior release until the migration
runs. Discriminates on the adoption annotation, not mere existence, so it does
not deadlock the routine upgrade (the MD always exists at render time); mirrors
the fail-closed canary pattern of system/seaweedfs naming-guard. Inert offline;
the live-fail path is covered by the upgrade-adoption e2e.

Signed-off-by: IvanHunters <xorokhotnikov@gmail.com>
2026-08-12 11:00:34 +03:00
scooby87 a242f34593 fix(clickhouse): assign DB_LIST so backup retention cleanup runs (#3760)
## What this PR does

The legacy per-instance ClickHouse backup CronJob (rendered when
`backup.enabled=true`, `backup.schedule` is set and
`backup.useSystemBucket=false`) never applied its retention policy.
`packages/apps/clickhouse/templates/backup-script.yaml` printed `echo
DB_LIST=$(...)` instead of assigning `DB_LIST`, so the variable stayed
unset and the cleanup loop `for db in $DB_LIST` ran zero iterations —
`restic forget` and `restic prune` were never invoked, snapshots
accumulated in object storage indefinitely despite the configured
`cleanupStrategy`, and the Job still exited 0 so nothing surfaced the
failure. This drops the stray `echo` so `DB_LIST` is assigned from the
table list and the retention cleanup runs as configured. MariaDB's
equivalent backup script already assigns `DB_LIST` correctly; this
brings ClickHouse in line. A helm-unittest regression over the rendered
`backup.sh` asserts the assignment is present and the `echo DB_LIST=`
typo is gone. Verified with `make -C packages/apps/clickhouse test`
(18/18 pass). Fixes #3758.

### Screenshots

<!-- No UI changes. -->

### Downstream repositories

Walked the trigger map in `docs/agents/contributing.md` against the
diff. The change is a one-line shell fix inside a Helm template plus a
helm-unittest case — it touches no `values.yaml`, `values.schema.json`,
`README.md`, `ApplicationDefinition`, version enum, variant/bundle,
release asset, `hack/` layout or namespace. No downstream repository
restates any of this.

- [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(clickhouse): ClickHouse backup retention now prunes old snapshots. The scheduled backup job's cleanup step never ran because `DB_LIST` was left unset, so snapshots accumulated in object storage indefinitely; it now honors the configured retention (`cleanupStrategy` / `--keep-last=N`).
```


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

## Summary by CodeRabbit

* **Bug Fixes**
* Fixed backup processing so table lists are assigned correctly,
allowing retention cleanup to run as expected.
  * Removed unintended output of the shuffled table list during backups.

* **Tests**
* Added regression coverage to verify correct backup list assignment and
prevent the previous malformed assignment from recurring.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
2026-08-12 00:08:43 +03:00
Alexey Artamonov 2b5c85cde7 fix(clickhouse): assign DB_LIST so backup retention cleanup runs
The legacy per-instance backup script printed `echo DB_LIST=$(...)`
instead of assigning DB_LIST, leaving it unset. The cleanup loop
`for db in $DB_LIST` then ran zero iterations, so `restic forget` and
`restic prune` never executed and snapshots accumulated in object
storage indefinitely despite the configured cleanupStrategy. The Job
still reported success, hiding the failure.

Assign DB_LIST from the shuffled table list so the retention cleanup
loop runs. Add a helm-unittest regression asserting the rendered
backup.sh assigns DB_LIST and no longer contains the `echo DB_LIST=` typo.

Fixes #3758

Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io>
2026-08-11 22:03:51 +03:00
myasnikovdaniil d04231829a docs(hack): fix a wrong claim about grep-error propagation in two contract tests (#3620)
<!-- 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/promote-gate-contract.bats` and
`hack/release-freeze-contract.bats` both carried the same wrong comment
above their `code_lines` helper: it said a real grep error (exit 2)
fails a pin through the helper's own exit status, distinct from an empty
match (exit 1).

That was never actually the mechanism. `code_lines` is always piped into
a further `grep` in both files, never the last stage of a pipeline, so
ordinary (no `pipefail`) pipe semantics discard whatever it returns on
its own, regardless of runner. `hack/cozytest.sh`, the runner these
tests actually execute under in CI, also appends a `return 0` to every
line that is exactly `}`, `code_lines`' closing brace included, making
that exit status even less trustworthy as a signal.

What catches a real grep error is the output, and only for one class of
pin. The error empties `code_lines`' stream, so a pin that requires
something to be present in that stream fails on it. A pin demanding an
absence stays blind: an exact count of zero is what an emptied stream
produces anyway, and what the pin was written to assert. Both files hold
pins of that shape. Nor is the `[ -n "$block" ]` guard at the top of
each test the backstop: a block here is a bare awk extraction, upstream
of `code_lines`. The new comment says both things and stops there.

Comment-only: `code_lines`'s body is untouched in both files, and all
tests pass under both `bats` and `hack/cozytest.sh` before and after.

Known limitation of the wording: the comment says the top-of-test `[ -n
"$block" ]` guard cannot see a filter failure, which is true, but both
files also carry non-empty guards on values that did pass through
`code_lines` (`promote-gate-contract.bats:72,73,165` and
`release-freeze-contract.bats:166,236,327` via `step_line`), and those
do fail on an emptied stream. The discriminator is whether the guarded
value came through the filter, not where the guard sits in the test.

### Screenshots

Not applicable, no UI change.

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

* **Documentation**
* Clarified comments describing how code-line filtering errors are
detected.
* Documented pipeline status behavior and why output-based assertions
are used instead of relying solely on command status codes.
  * No user-facing behavior or executable logic changed.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
2026-08-11 19:40:01 +05:00
Aleksei Sviridkin cf6bf2e379 fix(ci): stop labeled events from cancelling and re-running backports (#3569)
## What this PR does

A run joins its concurrency group before any job-level `if` is
evaluated, so `prepare`'s guard in `backport.yaml` is too late to help.
By the time it works out that a label event is irrelevant, that run has
already cancelled whatever was in flight for the same PR. Labels set
with the default `GITHUB_TOKEN` start no run, which is why the in-repo
labelers never showed this. Third-party GitHub Apps do start runs, and
they arrive in bursts.

The delivery survived, which is why this went unnoticed for so long. The
old guard admitted a run on the PR's cumulative label set, so the last
label of a burst, the one nothing cancels, requalified and redid the
work. In the bursts on record the kill lands within seconds of the merge
run starting, well before the cherry-pick, so the cost was a restart
rather than a lost backport, recorded as nothing more than a cancelled
run sitting next to a green one.

Two changes to the concurrency key, and both are needed. A label event
that requests no backport now goes into a group of its own, so it can
neither cancel nor displace the run doing the work. A label event that
does request one queues behind that run instead of killing it, because
`backport` and `backport-previous` are separate requests and both have
to finish. Splitting alone would let `backport` cancel the
merge-triggered run. Queuing alone would let an unrelated label evict a
pending request, since the group holds a single pending run and a newer
one replaces it.

`prepare`'s condition is scoped in the same commit. It read the PR's
cumulative label set, so once a merged PR carried `backport`, any later
unrelated label re-entered the job to redo a backport already delivered.
That run used to get cancelled by the next label in the burst. After the
change above it runs to completion, so the scoping has to land here
rather than as a follow-up.

The coupling runs the other way too, and it is the sharper half. That
redundant run is exactly what redelivered the backport the burst had
just killed, so narrowing the guard on its own would convert a noisy
delivery into a missing one. Neither change is safe to land without the
other.

Both jobs also gain a `timeout-minutes` ceiling, which is a consequence
of the queuing rather than housekeeping. A cancelling key disposed of a
stuck run by killing it; queuing makes the next genuine request wait
behind it instead, on the six-hour job default, which turns one wedged
job into a six-hour hole in the release line. That ceiling reaches a
stuck job and nothing else: a run can also wedge at the run level with
every job already finished, and `timeout-minutes` has no run-level
equivalent, so nothing in the workflow reaches that state.
`docs/release.md` now tells an operator to look for a run still in
flight and clear it before re-applying the label, because the retry
queues behind it rather than replacing it. The ceiling bounds execution,
not the wait for a runner, and thirty minutes is generous rather than
calculated. Execution on run 29844315422 was six seconds for `prepare`
and two and sixteen for the matrix legs, against 9m24s, 5m28s and 5m45s
of runner queue for the same three jobs. That queue is a third unbounded
case: GitHub's limits page says "job execution time" for the six-hour
job cap while spelling out that the 35-day run cap "includes execution
duration, and time spent on waiting and approval", and
[orgs/community#50926](https://github.com/orgs/community/discussions/50926)
shows a job sitting six hours on "Waiting for a runner to pick up this
job" under `timeout-minutes: 5`. So the value is picked to leave room
for a cherry-pick far larger than any on record while replacing the
six-hour default, and nothing here is sized against the queue, because
nothing here can be.

This removes a mitigation, and the trade is worth naming on both sides.
Under an unconditional cancel a run wedged at the run level cleared
itself, because the next event killed whatever was sitting in the group.
That is the same behaviour that killed live backports, so the mitigation
is inseparable from the bug being fixed: keeping it means keeping label
events that cancel a backport in flight. What it buys is a change of
failure mode rather than one fewer failure. A backport lost silently,
discovered weeks later as a fix missing from a release line, becomes a
delay the operator can see in the run list, with the recovery written
into `docs/release.md`. There is no lossless alternative here: `queue:
max` removes the eviction but only by dropping the conditional cancel
entirely, and the group key cannot tell a manual retry from a second
genuine request.

The run cited above, 31232645133, is still live and retained on purpose:
it is the only observed instance of the run-level wedge, so clearing it
would remove the evidence for the paragraph that describes it. Its
exposure is one pull request, #3262, which carries no backport label, so
nothing is queued behind it.

The old condition also carried a disjunct that could never decide
anything. A `labeled` payload already lists the new label in
`pull_request.labels`, so `contains` was true whenever the explicit
`github.event.label.name` check was, and the name check only ever agreed
with a `contains` that had already matched. The restructure drops it by
construction rather than by deletion: the name check is now the whole of
what a `labeled` event may match on.

### Why this no longer touches `pull-requests.yaml`

`main` fixed it there by splitting the group key, then refined that so a
`full-e2e` label stays in the main group instead of publishing a second
`E2E Tests` status on one SHA. That is the better mechanism, it neither
queues nor delays, and `gate-concurrency-contract.bats` now pins
`cancel-in-progress: true` in that file, so what this PR originally did
would turn it red. Nothing was left to add there, so the hunk is dropped
and its pin with it. The split-key idea is applied in `backport.yaml`
instead, where it did not exist.

One claim from the earlier description is withdrawn. It said the
leftover risk was a duplicate backport PR, opened when the original had
merged and its branch had been deleted before the queued run pushed.
`main` has since gained a guard that lists closed PRs on the
`backport-<n>-to-<target>` branch and skips the target when one of them
merged. Branch deletion does not defeat it: `GET
/branches/backport-3510-to-release-1.6` is 404 while the same head
filter on `/pulls?state=closed` still returns the merged PR. So a
redundant run only cost runner minutes, and the scoping above removes
the run.

### Screenshots

Not applicable, no UI change.

### Downstream repositories

Walked the trigger map in `docs/agents/contributing.md` against the
diff. The only workflow it cites is `tags.yaml`, for the release-prep
behaviour `cozystack/ccp` documents. Every `hack/` trigger it lists
either names a specific file another repository mirrors or gates on, or
covers moving and renaming under `hack/`. This PR appends tests to a
contract file the map does not name, and moves nothing. No package,
chart, values file, CRD, image reference, namespace, variant or node
prerequisite in the diff.

- [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(ci): stop `labeled` pull request events from cancelling the automatic backport run already in flight. A burst of labels from a third-party app could previously kill the backport the merge had just started, leaving the work to be redone by whichever label arrived last.
```
2026-08-11 16:50:50 +03:00
Aleksei Sviridkin 4ac02f59d5 fix(e2e): stop the sandbox cluster CIDR overlapping the pod range (#3750)
## What this PR does

The sandbox Talos config and the platform install step both used
`10.244.0.0/16`, one as the Kubernetes cluster CIDR and the other as the
kube-ovn `POD_CIDR`, so two allocators drew from one range.

kube-ovn owns pod IPAM and allocates flat from `POD_CIDR`. Cilium is
chained behind it (`generic-veth`, target `kube-ovn`) and allocates no
pod addresses at all, but it still carves its own per-node router IP and
Ingress IP out of `Node.spec.podCIDR`, which kube-controller-manager
slices from the cluster CIDR. Sooner or later kube-ovn hands a pod an
address cilium already holds. When that address is the Ingress IP the
agent refuses the sandbox with `IP ipv4:<addr> is already in use`, and
cilium never relents on it, so the pod proceeds only if something
reschedules it onto a different address; in the run this was filed from,
the same address was refused seven times across six pod UIDs before the
suite ran out of budget. When it is the router IP nothing complains and
only host-to-pod traffic breaks.

This moves the sandbox cluster CIDR to `100.65.0.0/16`, which no other
range in the tree uses, and adds a guard so the two cannot be brought
back together by accident.

Pod addresses do not change. They still come from kube-ovn out of
`10.244.0.0/16`, and cilium does not require a chained endpoint's
address to sit inside its own allocation CIDR, so only cilium's two
per-node infrastructure addresses move.

The quiet variant gets fixed twice over, which is worth spelling out
because only half of it was obvious. Under `ipam.mode: kubernetes`
cilium also installs a route for the node's own pod CIDR via
`cilium_host`. On the current default that route covers `10.244.a.0/24`
on a node whose pods actually live behind `ovn0`, so it shadows real pod
addresses. Moving the cluster CIDR both stops the router IP from ever
being handed to a pod and retargets that route at `100.65.a.0/24`, where
it shadows nothing.

Excluding the addresses on the kube-ovn side instead is not an option:
cilium picks its offsets by random scan over the node's range, so there
is no static set to exclude.

The collision shows up in two shapes, and both have been seen. Loudly,
on the Ingress IP, where the agent names the address in the event, which
is the failure https://github.com/cozystack/cozystack/issues/3736 was
filed for. Quietly, on the router IP, where the pod comes up and the
only symptom is `connection refused` on a kubelet probe, with `ip route
get <podIP>` on the node resolving to `local ... dev lo`; that variant
is documented in https://github.com/cozystack/cozystack/issues/3445 and
https://github.com/cozystack/cozystack/issues/3615. They were being
tracked as unrelated flakes in different suites before the shared cause
was identified.

## What the guard covers, and what it does not

`hack/sandbox-cidr-disjoint.bats` pins the configuration, not the
behaviour. It reads the values out of the tree and fails when they
overlap. It does not reproduce the collision, which needs a running
cluster.

It checks the invariant rather than the literals, so either value may
move as long as they stay disjoint, and it checks the cluster CIDR
against the service and join ranges too, because landing on one of those
instead is the next way to get this wrong. An extraction that comes up
empty, duplicated or misshapen fails the test instead of shrinking the
comparison, and comments and blank lines inside the list are skipped
rather than read as the end of it. It is not a YAML parser, though:
anything it does not read as a bare dotted quad fails the shape
validation rather than being understood, which is the safe direction but
not the same promise.

Those three are the allocator ranges and nothing more. The sandbox node
network, the load-balancer pool and the transit range of a component
that is off by default are not read, so the guard is not a complete map
of what the cluster CIDR has to avoid. An IPv6 entry fails the shape
validation rather than getting an overlap check, so dual-stack means
extending the guard rather than passing through it.

## Residual risk

I did not prove from source that cilium is the only consumer of
`Node.spec.podCIDR` in this topology. What I did check is that
kube-proxy is disabled and that `local-ccm` runs the node-lifecycle
controller only, with no route controller.

This diff selects the full e2e suite, 21 Chainsaw suites with `gateway`
and `mongodb` among them, so a consumer I missed surfaces as a red run
on this PR rather than staying an assumption.

Split of what covers what: the static checks and the unit guard cover
the configuration, and the e2e run covers the behaviour. Nothing short
of a green suite answers whether cilium's router and Ingress addresses
behave once they sit outside the range kube-ovn allocates from.

## A check that is not "the suite went green"

`hack/e2e-cilium-endpoint-leak-healer.sh` already prints a line of its
own for this exact collision, so there is a signal to read that does not
depend on the overall verdict. In the `cozyreport` artifact it lands at
`snapshots/<suite>/host/namespaces/kube-system/v1/pod/cilium-leak-healer-*/healer/current.log`,
and the lines to grep for are `IP is this node's Cilium ingress IP`. In
the run this was filed from there are 42 of them, all naming the same
address.

After this change that line cannot be produced by an address inside the
kube-ovn range, because there is no longer an address that is in both.
So the expectation is zero from the first run, not zero on average: one
occurrence contradicts the change rather than being noise, and is worth
stopping on. A green suite can be green for many reasons; this line
disappearing has only one.

## Scope

This fixes the sandbox and nothing else. For a real installation the
cluster CIDR lives in the operator's own machine configuration, outside
this repository, so the same collision is still reachable there.
https://github.com/cozystack/cozystack/issues/3740 tracks the product
side, including the finding that the install documentation does not
merely allow the overlap but instructs operators to create it, by
requiring `podCIDR` to match the cluster's pod CIDR.

There is a tradeoff in this worth naming rather than leaving to be
discovered. Talos defaults `podSubnets` to `10.244.0.0/16`
(`DefaultIPv4PodNet`, `pkg/machinery/constants/constants.go:888` at
v1.13.6, the version this tree ships), and the platform defaults
`networking.podCIDR` to the same value, so an operator who takes both
defaults gets the overlap. Until now the sandbox shared that topology
and was the one place the bug surfaced where anyone would see it. After
this change it no longer reproduces the operator-facing form, and
nothing else does either. That is the right call for CI, which should
not be wedged by a defect it did not introduce, but it does mean the
production exposure loses its only accidental detector.

Closes https://github.com/cozystack/cozystack/issues/3736

To be explicit about what closing that issue does and does not mean:
this removes the reproduction in the sandbox and leaves the product
configuration untouched. The collision is still reachable for users, and
https://github.com/cozystack/cozystack/issues/3740 is where that lives.
The issue text describes the root mechanism, which is wider than its
title, so the closure covers the e2e failure it was filed for and not
the platform default.

### Screenshots

Not a UI change.

### Downstream repositories

No box is ticked deliberately, and no follow-up PR is opened, so a human
decides.

Walking the trigger map against this diff: both
`hack/e2e-prepare-cluster.bats` entries in it are scoped to node
prerequisites, the kernel modules a node must load, the containerd
settings, the LVM `global_filter` and the cluster domain. This diff
changes none of those, it changes a network range, so no entry fires
literally.

The underlying constraint does reach several of them all the same.
`cozystack/talm` sets `podSubnets: [10.244.0.0/16]` in
`charts/cozystack/values.yaml`, the same range the platform uses for
`networking.podCIDR`, so the documented Talos bootstrap produces the
overlap this PR removes from the sandbox. `cozystack/ansible-cozystack`
reaches it from the other side: `roles/cozystack/defaults/main.yml` sets
`cozystack_pod_cidr: "10.42.0.0/16"` and renders it into
`networking.podCIDR`, while its inventories set `cluster-cidr:
10.42.0.0/16`, so those two are identical as well. And
`cozystack/website` prescribes the equality outright,
`install/kubernetes/generic.md` stating that `podCIDR` and `serviceCIDR`
must match the Kubernetes cluster configuration.

No issue is filed there yet on purpose: which side moves is not decided,
and asking talm to change `podSubnets` would be the wrong request if the
platform ends up moving instead. That decision belongs to
https://github.com/cozystack/cozystack/issues/3740.

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


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

## Summary by CodeRabbit

- **Bug Fixes**
- Updated cluster networking ranges to avoid conflicts with Kubernetes,
kube-ovn, kilo, and Cilium networks.
- Added validation to detect missing, invalid, or overlapping cluster
and platform CIDR ranges.
- Preserved safeguards for environments where network ranges overlap,
with clearer diagnostic logging.

- **Documentation**
- Documented network separation requirements and clarified expected
sandbox networking behavior.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
2026-08-11 16:50:35 +03:00
IvanHunters 9ec5f8ceca fix(migration): carry logSerialConsole through worker-pool adoption
Migration 54's adopt pick-list copied every worker NodeGroup field that feeds
the content-hashed KubevirtMachineTemplate except logSerialConsole. That field
renders into the same hashed template (kubernetes-nodes nodegroup.yaml), so an
adopted pool that set logSerialConsole would render a KMT without it, change the
content-hash, rename the KMT, and roll every worker VM of the pool on upgrade
(and silently drop the setting). Add logSerialConsole to the pick-list (guarded
by has(), so pools without it are unaffected) and assert it carries through.

Signed-off-by: IvanHunters <xorokhotnikov@gmail.com>
2026-08-11 10:56:07 +03:00
Aleksei Sviridkin e0d6c6be92 fix(ci): stop labeled events from cancelling and re-running backports
A run joins its concurrency group before any job-level `if` is
evaluated, so `prepare`'s guard cannot stop a label event from
disturbing the run already backporting a PR. Labels set with the
default GITHUB_TOKEN start no run, but labels set by third-party
GitHub Apps do, and they arrive in bursts, so each one cancelled the
backport the merge had just started.

The delivery survived anyway, which is why this went unnoticed: the
old guard admitted a run on the PR's cumulative labels, so the last
label of a burst requalified and redid the work. In the bursts on
record the kill lands within seconds, well before the cherry-pick,
so the cost was a restart rather than a lost backport.

Move a label event that requests no backport into a group of its own,
and let one that does request a backport queue rather than cancel.
Both halves are needed. `backport` and `backport-previous` are
separate requests that both have to complete, so they share a group
and must not cancel each other, while every other label
does no work here and only has to stay out of the way.

Scope `prepare` to the labels each trigger is entitled to read at the
same time. Reading the cumulative label set on a `labeled` event let
any later unrelated label on a merged PR still carrying `backport`
re-enter the job to redo a backport already delivered. That redo is
also what redelivered a killed backport, so narrowing the guard alone
would turn a noisy delivery into a missing one: neither change is
safe to land without the other.

Bound both jobs with `timeout-minutes` for the same reason, since a
queued request now waits on the run ahead of it rather than replacing
it. The ceiling bounds execution and not the wait for a runner, which
no job-level key reaches, so 30 is generous rather than calculated:
every execution measured on this workflow has been seconds, against
minutes of queue that nothing here can bound.

Document the recovery path the narrowing removes: with no accidental
re-run left, re-applying whichever backport label the PR already
carries is how an operator retries a merge-triggered run that died.

Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <3811295@gmail.com>
2026-08-11 03:14:09 +03:00
Aleksei Sviridkin ec6a835251 fix(manifestutil): report one message when the CRD wait is cancelled (#3742)
<!-- 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

`WaitForCRDsEstablished` reported a cancelled context two different
ways. The check at the top of the poll loop returned a message with no
CRD name, the `select` at the bottom named the CRD the poll had stopped
on. When a tick and the deadline come ready in the same instant,
`select` picks between the ready cases at random, so one cancellation
reached the operator either way.

Both callers wrap this as `CRDs not established after apply`, so the CRD
name is what makes that log line actionable, and whether it is there was
a coin toss. `TestWaitForCRDsEstablished_timeout` asserts on the name,
which is where this surfaced: its 2s deadline is an exact multiple of
the 500ms ticker, so the two channels come ready together on the fourth
tick and the assertion failed at random.

The message is now built in one place and returned from both exits, so
the two are indistinguishable by construction rather than by inspection.
The name is carried across iterations, because a cancellation noticed at
the top of the loop has no poll of its own to take one from, and it
starts as the first CRD of the set for a wait cancelled before any poll
observed anything. That start value is a stand-in rather than an
observation, and the comment on it says so; the alternative, an empty
name and a second phrasing for it, would leave the loop-top message with
nothing to assert against and no test able to catch its return to the
unnamed form.

Deleting the top check instead, so there is literally one exit, is the
smaller diff and it moves the name to the head of the list in the very
interleaving this PR is about. A client refuses `Get` on a dead context
at the first name in the list whichever CRD is actually lagging, so a
poll entered on a cancelled context renames the report to that first
name. That premise is about the production client, `client.New` at
`cmd/cozystack-operator/main.go:176`, uncached, so the request is
rejected before it is issued, and it is worth saying that nothing in the
test surface pins it: replacing the `ctx` passed to `Get` with
`context.Background()` survives the whole package, and the fixtures that
need a context-honouring client check `ctx.Err()` by hand. That line
predates this change and is untouched by it. Measured with a client that
honours the context: with the check, a wait that re-enters the loop
after the deadline keeps the name its last poll settled on; without it,
the report moves to the head of the list. The check closes that window
and no more than that. A context that dies part-way through a poll still
records the first `Get` to fail, exactly as merge base does, because
this branch cannot tell a refused request from a CRD status. That is
unchanged here and written up separately.

Four tests cover the ways this wait can end on a cancelled context:
before any poll has observed the set, cut off inside a poll, cancelled
while waiting for the next tick, and re-entering the loop on a context
that died during that wait. Two of them are red on merge base and are
the regression pins for the defect above, both deterministically rather
than by repetition: merge base answers a wait that starts cancelled, and
a wait that re-enters the loop cancelled, with the unnamed message every
time. Its loop-top exit is a `select` whose `Done` case is ready, and a
ready case always beats `default`. The middle two are green on merge
base, which gets those cases right; they are here because each catches a
specific way of breaking the fix. Every one of the four is the sole
killer of at least one mutation, which is the property worth having: it
says none of them is redundant. They compare the whole message rather
than searching it for a name, because a second exit that happens to
carry a name is still the defect being fixed here, and a test that only
looks for the name cannot see it.

The re-entry test needs the bottom `select` to take the tick while
cancellation is already pending, which against a real context is the
same coin toss the bug rides on. The double it uses hands out an open
`Done` for the single read that `select` makes in that iteration and a
closed one for every read after, so the tick wins by construction and
the next iteration sees a context cancelled through both `Done` and
`Err`, the way a real one is. That pairing is guaranteed rather than
incidental: `cancelCtx.Err` reads `<-c.Done()` before returning a
non-nil error, so a double that reported cancellation through `Err`
alone would be modelling a state no context can be in, and would pin the
shape of the check rather than its behaviour. One property of that
double is worth knowing before editing the loop: it hands out its open
and closed channels per read, so a `Done()` read added inside the poll
would consume the open one and leave the bottom `select` a closed
channel, quietly turning this into a second bottom-exit test that still
passes. The failure direction there is false-green, which is the worse
half, and the cheap guard is a lower bound on elapsed time inside that
test, a run that leaves through the loop top spends a ticker interval
getting there, one that leaves at the bottom of the first iteration does
not. Not done here, because the test is correct today and provably kills
the mutation it exists for, so the hazard is conditional on a future
edit.

The timeout test does reproduce locally under load: 6 failures across
400 runs on the unfixed code while I was measuring, spread over five
batches, the last of which saw none. #3719 reports that it does not
reproduce locally, so this is worth writing down before someone spends
an evening re-testing that. That spread is also why the number is an
order of magnitude and not a rate.

`TestInstall_crdNotEstablished` in `internal/crdinstall` has the same 2s
deadline against the same 500ms ticker. It asserts on the `CRDs not
established` wrapper the caller adds, and both messages carried that
wrapper, so this coin flip could not reach it and nothing here touches
it.

I left the 2s deadline where it is. Moving it off the tick boundary
would have hidden the coin flip and left the second message in place.
The new tests catch a reintroduced second message deterministically,
which is what that job now rests on rather than the timeout test's
roughly one-in-sixty.

### 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(manifestutil): a CRD wait cut short by a cancelled context now reports one error naming the CRD it was waiting on, where before the name was present or missing depending on which of two exits the scheduler happened to pick
```


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

## Summary by CodeRabbit

* **Bug Fixes**
* Improved cancellation handling while waiting for custom resources to
become established.
* Cancelled operations now stop before issuing unnecessary requests and
identify the affected resource.
  * Timeout errors remain distinguishable from cancellation errors.

* **Tests**
* Added coverage for cancellation before, during, after, and between
polling attempts.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
2026-08-11 02:53:40 +03:00
Aleksei Sviridkin 9ace63b441 fix(e2e): stop the sandbox cluster CIDR overlapping the pod range
The sandbox Talos config and the platform install step both used
10.244.0.0/16, one as the Kubernetes cluster CIDR and the other as the
kube-ovn POD_CIDR, so two allocators drew from one range.

kube-ovn owns pod IPAM and allocates flat from POD_CIDR. Cilium is
chained behind it and allocates no pod addresses, but it still carves
its own per-node router IP and Ingress IP out of Node.spec.podCIDR,
which kube-controller-manager slices from the cluster CIDR. Sooner or
later kube-ovn hands a pod an address cilium already holds. When that
address is the Ingress IP the agent refuses the sandbox with "IP
ipv4:<addr> is already in use", and cilium never relents on it, so the
pod only proceeds if something reschedules it onto a different address.
In the failure this comes from, the same address was refused seven
times across six pod UIDs before the suite ran out of budget. When it
is the router IP nothing complains at all and only host-to-pod traffic
breaks.

Move the cluster CIDR to 100.65.0.0/16, which no other range in the
tree uses. That deliberately stops the sandbox matching the layout the
install documentation prescribes, so it is not a value to put back.
Pod addresses are unaffected: they still come from kube-ovn out of
10.244.0.0/16, and cilium does not require a chained endpoint's address
to sit inside its own allocation CIDR.

Excluding the addresses on the kube-ovn side would not work, because
cilium picks its offsets by random scan over the node's range.

The new guard checks the invariant rather than the literals, so either
value may move as long as they stay disjoint. It covers the service
and join ranges too, since landing on one of those instead is the next
way to get this wrong.

Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <3811295@gmail.com>
2026-08-11 02:31:27 +03:00
IvanHunters 8886ffa410 test(e2e): drop the parent-chart heredoc cases removed by the Phase 2 split
Phase 2 moved the worker talos-reconcile Job and its TalosConfigTemplate out of
the kubernetes chart into kubernetes-nodes, so the cases that rendered
packages/apps/kubernetes --show-only templates/talos/talos-reconcile-job.yaml
now fail with 'could not find template'. Keep the kubernetes-nodes cases, which
cover the same heredoc escaping (hostile registryMirrors + Talos image
coordinates) against the chart that actually renders the worker Job now.

Signed-off-by: IvanHunters <xorokhotnikov@gmail.com>
2026-08-10 23:24:11 +03:00
IvanHunters ea89cb188a test(e2e): match the guest-console-log assertion to the Phase 2 KubernetesNodes CR
Since the Phase 2 split the e2e worker pool is a separate KubernetesNodes CR
whose fields sit two spaces under spec, not six spaces under a nodeGroups.<name>
block, so the logSerialConsole assertion is re-anchored to the two-space key.

Signed-off-by: IvanHunters <xorokhotnikov@gmail.com>
2026-08-10 22:51:42 +03:00
IvanHunters b4a2f6f791 Merge remote-tracking branch 'origin/main' into pr-3315-lgtm
# Conflicts:
#	api/apps/v1alpha1/kubernetes/types.go
#	api/apps/v1alpha1/kubernetes/zz_generated.deepcopy.go
#	hack/e2e-chainsaw/_lib/run-kubernetes.sh
#	packages/apps/kubernetes-nodes/tests/render-parity.sh
#	packages/apps/kubernetes/README.md
#	packages/apps/kubernetes/templates/cluster.yaml
#	packages/apps/kubernetes/templates/talos/talos-reconcile-job.yaml
#	packages/apps/kubernetes/tests/talos_templates_test.yaml
#	packages/apps/kubernetes/values.schema.json
#	packages/apps/kubernetes/values.yaml
#	packages/system/kubernetes-nodes-rd/cozyrds/kubernetes-nodes.yaml
#	packages/system/kubernetes-rd/cozyrds/kubernetes.yaml
2026-08-10 22:11:21 +03:00
Andrey Kolkov d0990299ed feat(clickhouse): let the backup sidecar trust a private S3 CA, unblocking the backup e2e round-trip (#3385)
## What this PR does

Gives the ClickHouse `clickhouse-backup` sidecar a way to trust an S3
endpoint whose certificate is signed by a private CA, and uses that to
make the ClickHouse backup/restore e2e round-trip actually run in CI.
This actualizes #2600, whose feature code and example scripts have since
landed on `main` independently, and whose remaining contribution — the
e2e test — was still in the retired BATS format. #2600 can be closed
once this merges.

### 1. `backup.endpointCA` on the ClickHouse chart

The sidecar had **no TLS/CA surface at all**: bucket coordinates and
credentials arrive as `S3_*` env, and nothing let it verify an endpoint
signed by a private CA. Cozystack's own in-cluster SeaweedFS is exactly
that case — it serves `:8333` behind the self-signed "SeaweedFS CA" — so
a ClickHouse release could only ever back up to a publicly-trusted
endpoint. MariaDB (`storage.s3.tls.caSecretKeyRef`) and Postgres (barman
`endpointCA`) have had a CA field all along; this closes the gap for
ClickHouse.

`backup.endpointCA` takes `{name, key}`. When set, the chart mounts that
Secret into the sidecar and points `SSL_CERT_DIR` at it. Empty — the
default — renders no env, no mount and no volume, so existing releases
are byte-for-byte unchanged.

**Why `SSL_CERT_DIR` and not `AWS_CA_BUNDLE`.** Both are honoured by
`altinity/clickhouse-backup:2.7.4`; I measured them against a
self-signed endpoint and public AWS S3 in turn. Go reads `SSL_CERT_DIR`
*in addition to* the system bundle, so both endpoints verify.
`AWS_CA_BUNDLE` *replaces* the SDK's trust pool: the private endpoint
verifies, and public S3 then fails `x509: certificate signed by unknown
authority` — it would silently break a release that also talks to a
public endpoint. `S3_DISABLE_CERT_VERIFICATION` was rejected outright,
since it drops verification rather than trusting one more CA.

### 2. The e2e suite, and what it was really testing

Two Tests added to the existing `hack/e2e-chainsaw/clickhouse/` suite
(in name order, mirroring the `etcd`/`mariadb`/`postgres` shape — see
the layout rationale below for why they live here rather than in a new
directory):

- **`clickhouse-1-backup-contracts`** (CI, `tenant-test`, ~98s): a
backup-enabled ClickHouse asserted against the *running* cluster — the
`clickhouse-backup` sidecar on the `ch-backup-api` `:7171` port, and the
`<release>-backup-api-auth` Secret whose keys the sidecar's
`API_USERNAME`/`API_PASSWORD` actually resolve `valueFrom`. Each is a
silent break if a chart bump renames it. Needs no object store.
- **`clickhouse-2-backup-roundtrip`** (CI, `tenant-root`): `Bucket` →
`Altinity` strategy + `BackupClass` → source ClickHouse with a sentinel
row → `BackupJob` → in-place `RestoreJob` → to-copy `RestoreJob` into a
second instance, driving `examples/backups/clickhouse/run-all.sh` so
test and docs cannot drift.

The round-trip was originally gated behind `CLICKHOUSE_E2E_S3_ROUNDTRIP`
and **skipped in CI** — its `PASS (1.01s)` meant the gate fired, not
that a backup happened. `backup.endpointCA` removes the reason for the
gate, so the suite now adopts the same three preconditions
mariadb/postgres rely on: run in `tenant-root` (an isolated tenant's
Cilium egress allowlist blocks its Pods from reaching `tenant-root`'s
SeaweedFS), target the in-cluster endpoint instead of the `Bucket`'s
external ingress URL (an unroutable placeholder in CI), and trust its
certificate via the copied CA. It also pre-cleans with `cleanup.sh`,
because `tenant-root` is shared and a leftover `Succeeded` `BackupJob`
would satisfy the harness's wait without a backup ever running.

Both backup Tests live in the existing `hack/e2e-chainsaw/clickhouse/`
suite rather than a new suite directory, so no selector mapping is
added: `hack/select-e2e.sh` already derives `clickhouse` both from
`packages/apps/clickhouse/**` (the `*-application` rule) and from
`examples/backups/clickhouse/**` (the harness path), and both paths
therefore select the suite that actually runs the harness. That
alignment is the reason for the layout — a separate `clickhouse-backup/`
directory would have made an edit to the harness select a suite that
cannot exercise it. `hack/select-e2e.sh` and `hack/select-install.sh`
are otherwise untouched (the only change is a comment recording that
invariant), and the existing `backup example harness edit selects its
app suite` case in `hack/select-e2e_test.bats` covers the mapping.

### Verification

- `helm unittest` — 17 pass, including 5 new `endpointCA` cases: the
unset default renders nothing, the env/mount/volume wiring, a custom key
still projected as `ca.crt`, composition with `useSystemBucket`, and
`backup.enabled=false` renders nothing.
- The CA-trust behaviour was measured against the real image, not
assumed: without a CA knob the sidecar reports `x509: certificate signed
by unknown authority`; with `SSL_CERT_DIR` the handshake succeeds.
- `hack/select-e2e_test.bats` (16) and `hack/select-install_test.bats`
(17) pass; `select-install.sh --validate` reports graph OK.
- `make generate` is idempotent, so the pre-commit job's `git diff
--exit-code` stays clean.
- The contracts Test passed in CI on earlier runs of this branch. The
round-trip only became CI-live with this work, so it still needs a green
E2E run before merge — the branch has since been rebased onto current
`main` to get one.

### Screenshots

Not applicable — no UI changes.

### Downstream repositories

Walked the trigger map against the diff, file by file. The load-bearing
change is the new `backup.endpointCA` field in
`packages/apps/clickhouse/values.yaml` / `values.schema.json`;
everything else is `hack/e2e-chainsaw/`, `examples/backups/`, one docs
paragraph, and regenerated artifacts, none of which appear in the map.

- [ ] No downstream repository is affected by this change
- [x]
[cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack)
- follow-up:
https://github.com/cozystack/terraform-provider-cozystack/issues/18 —
"add, remove or rename a field in an app's `values.schema.json`" reaches
the provider's hand-maintained schema, model and expand/flatten pair, so
the new nested object is not surfaced until it is added there. Opened as
an issue rather than a PR because the natural moment is alongside that
repo's next `api/apps/v1alpha1` pin bump, which is a maintainer call.

Not ticked, with reasons rather than silence: **cozystack/website** —
the trigger is *adding, renaming or removing a package*, and this adds a
field to an existing one. `clickhouse` is already in the website
`Makefile`'s app list and its reference page is generated from this
package's `README.md`, so the regenerated `backup.endpointCA` rows are
carried by the existing release-tag automation with no manual PR.
**cozystack/external-apps-example** — vendors `hack/package.mk` and
`update-crd.sh`, neither of which this PR touches.
**cozystack/ansible-cozystack**, **ccp**, **talm**, **cozyhr**,
**cozy-proxy**, **cozystack-telemetry-server**, **examples** — no
installer values, `hack/` layout, namespace, variant, node-prerequisite,
annotation, label or metric in their trigger rows is touched here.

### Release note

```release-note
feat(clickhouse): add `backup.endpointCA` so the clickhouse-backup sidecar can verify an S3 endpoint whose certificate is signed by a private CA, such as Cozystack's in-cluster SeaweedFS. Leave it unset for publicly-trusted endpoints; existing releases are unaffected.
```


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

* **New Features**
* Added optional private S3 endpoint CA support for ClickHouse
backup/restore sidecars via `backup.endpointCA`, including configurable
Secret name and CA bundle key (default `ca.crt`).
* Sidecars mount the CA and extend trust while leaving public endpoints
unchanged.

* **Documentation**
* Updated ClickHouse chart docs, operational backup guidance, and
backup/restore examples to cover CA setup, overrides, and cleanup.

* **Tests**
* Added helm-unittest and end-to-end coverage validating CA
rendering/mounting behavior and ClickHouse backup/restore workflows.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
2026-08-10 21:17:47 +04:00
Aleksei Sviridkin 24157ec876 chore(codeowners): keep architects as owners in every non-governance rule (#3738)
## What this PR does

CODEOWNERS applies only the **last** matching rule, and matching is not
additive: a scoped rule fully replaces the `*` catch-all instead of
extending it. As a result `@kvaps` and `@lllamnyp` were not code owners
for most scoped paths (21 and 36 of the 76 owner-bearing rules,
respectively), and several paths (`/hack/`, `/.github/`, `/Makefile`,
`/.pre-commit-config.yaml`, `/docs/`) were owned by exactly two people —
with "Require review from Code Owners" enforced on `main`, a PR authored
by one of them could only be approved by the other.

This PR:

- appends `@kvaps` and `@lllamnyp` to every rule that lists owners,
keeping existing owners and their order; the governance block
deliberately stays with `@tym83 @kvaps`, and the ownerless rules for
generated files stay ownerless;
- spells out the non-additive last-match semantics in the file header;
- adds `hack/codeowners-invariant.bats` (auto-discovered by `make
bats-unit-tests`), so a future scoped rule that omits the catch-all
owners fails unit tests instead of silently carving its path out from
under them.

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

### Release note

```release-note
chore(codeowners): @kvaps and @lllamnyp are now code owners for every path except governance files, removing single-approver review bottlenecks; a unit test guards the invariant
```


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

## Summary by CodeRabbit

* **Chores**
* Updated code ownership rules across dashboards, packages, Kubernetes
components, backups, GPU and SeaweedFS components, Keycloak,
documentation, and CI files.
* Expanded area-specific review ownership while preserving existing
governance and API review assignments.

* **Tests**
* Added automated validation to ensure ownership rules include the
required default reviewers.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
2026-08-10 15:49:22 +03:00
Aleksei Sviridkin 5747747087 fix(manifestutil): report one message when the CRD wait is cancelled
WaitForCRDsEstablished reported a cancelled context two different ways.
The check at the top of the poll loop returned a message with no CRD
name; the select at the bottom named the CRD the poll had stopped on.
Which one ran was left to the scheduler: when the deadline and a tick
come ready in the same instant, a select picks between the ready cases
at random, so one cancellation reached the operator either way.

Build the message in one place and return it from both exits, so the two
are indistinguishable by construction. The name is carried across
iterations, since a cancellation noticed at the top of the loop has no
poll of its own to take one from, and starts as the first CRD of the set
for a wait cancelled before any poll observed anything.

Keeping the check at the top is what makes carrying the name worthwhile.
A client refuses Get on a dead context at the first name in the list
whichever CRD is actually lagging, so a loop that re-entered and polled
anyway would overwrite the name its last poll settled on. The check
closes that window and no more than that: a context dying part-way
through a poll still records the first Get to fail, because this branch
cannot tell a refused request from a CRD status.

Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <3811295@gmail.com>
2026-08-10 15:36:05 +03:00
IvanHunters cfaf2e086c fix(backupstrategy-controller): repair lookup-gated backup objects (#3524)
<!-- 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

Fixes #3518.

Objects in `packages/system/backupstrategy-controller` and
`packages/system/bucket` are gated on a Helm `lookup` of an object that
is still being created. When the lookup is empty the object is skipped —
and since helm-controller does not re-render a release whose chart and
values did not change (drift detection is off on operator-generated
HelmReleases), **the skip is permanent**, not a bootstrap window. On the
reporter's cluster the install-timing window was 4 seconds and the
objects were still missing months later:

- the 7 default backup `Strategy` CRs and the Velero
`BackupStorageLocation`, gated on `include
"backupstrategy-controller.bucketName"` → a `lookup` of `BucketClaim`;
- the `<bucket>-<user>-credentials` Secret
(`packages/system/bucket/templates/user-credentials.yaml`), gated on a
`lookup` of the COSI Secret.

Only `BackupClass cozy-default` was present, because it renders
unconditionally. Its own comment predicted this case while assuming
"Flux re-renders the chart", which does not happen — the same trap
`packages/core/platform/sources/cozystack-basics.yaml` already
documents: *"Drift detection is off on operator-generated HelmReleases,
so a capability gate in the templates would render the policy out at
first install and never add it back."*

Consequence: migration 50 (etcd adoption, v1.6.0) is fail-closed on the
snapshot target. It resolves the target from the projected credentials
Secret, which has no source without `<bucket>-<user>-credentials`, so
its Job reboots forever and the cluster cannot be upgraded at all.

### 1. `backupstrategy-controller` reconciles the objects into existence

New leader-elected `DefaultObjectsGate` runnable
(`internal/backupcontroller/default_objects_gate.go`), modelled on the
existing `SystemCredentialsProjector`. Every minute:

- resolves the bucket name from the **projector's source Secret**
(`parseSourceSecret`) — no new dependency, no `lookup`, and it covers
the external-S3 path too. While it is unresolvable the gate does
nothing: forcing then would only re-run the same empty lookup;
- reads `BackupClass cozy-default` as the manifest of what must exist
(its `strategyRef`s), plus the `cozy-default` BSL when
`velero.bslEnabled`;
- when any is missing, stamps `reconcile.fluxcd.io/forceAt` **and**
`requestedAt` on its own HelmRelease. Both are required: `requestedAt`
alone only requests a reconcile, which is a no-op for an unchanged
release; `forceAt` is what triggers a real Helm upgrade, and it is only
honoured together with `requestedAt`;
- throttled to one forced upgrade per 5 minutes, and a no-op in the
steady state. A `strategyRef` whose CRD is not installed is not counted
as missing, so a missing CRD cannot drive an endless force loop.

Two new metrics: `cozystack_backup_default_objects_missing{backupclass}`
— the alert that would have caught this, since it is non-zero regardless
of the HelmRelease's `Ready` condition — and
`cozystack_backup_default_objects_force_reconciles_total`.

RBAC delta is two lines: `patch` on `helmreleases` (`get` was already
there) and `get` on `backupstoragelocations`. Both accesses are point
Gets through the dynamic client, so no cluster-wide informer and no
`list`/`watch`.

Off switch: `backupStorage.reconcileDefaultObjects: false`.

**Why not a render-time `fail` in this chart:** it would deadlock.
`templates/bucket.yaml` in this same chart is the producer of the
`Bucket` whose `BucketClaim` the lookup reads, so a failed render never
applies it and the condition can never resolve.

**Why not template the objects in Go:** their bodies are values-driven
(endpoint, region, `forcePathStyle`, the Altinity strategy's whole
PodTemplateSpec). Helm stays their single author; the gate only makes
sure the render that produces them actually happens.

**Why not `dependsOn`:** an edge guarantees the `Bucket` CR is
*applied*, not that its claim is *reconciled*, so it does not close the
race.

### 2. The gate also repairs the credentials Secret, one release earlier

The `<bucket>-<user>-credentials` Secret is the same trap as the
Strategy CRs, one release up the chain:
`packages/system/bucket/templates/user-credentials.yaml` renders it
behind a `lookup` of the COSI Secret, and skips it just as permanently.
It is also the *first* domino — the projector reads it, so without it
there is no `cozy-backups-creds`, no strategy, no Velero, and no
migration 50.

An earlier revision of this PR made that render `fail` so the release
would retry and converge. [@IvanHunters pointed
out](https://github.com/cozystack/cozystack/pull/3524#issuecomment-5214269078)
that Helm cannot render a partial set, so the `fail` aborts the
**whole** `<bucket>-system` release — the other users' Secrets and the
bucket UI Deployment/Service/Ingress/HTTPRoute with them. One declared
user whose `BucketAccess` never provisions would take down every other
user of that bucket, and on an installed release park it in `Failed`,
blocking every later upgrade. That was a real blast-radius regression on
a universal path, and it is gone.

The repair now lives in the gate, where it is per-object:

- while the source Secret is absent, or carries no bucket name, the gate
stamps `forceAt` + `requestedAt` on the bucket's `bucket-<name>-system`
HelmRelease instead of doing nothing;
- **no self-deadlock:** the `BucketClaim` and the `BucketAccess` whose
COSI Secret that lookup reads are rendered unconditionally by the
**parent** release (`packages/apps/bucket/templates/bucketclaim.yaml`),
not by the one being forced;
- **no RBAC delta:** `patch` on `helmreleases` is already
cluster-scoped;
- the two releases are throttled **independently**. They are forced in
sequence on a bootstrap (credentials, then the objects the resolved
bucket unblocks), so one shared timestamp would delay the second by a
full `MinForceInterval` for no reason;
- coordinates come from the chart under `provisionBucket`, and are
omitted on external S3, where the Secret is admin-managed and no release
renders it.

The bucket chart keeps its partial render. Its comment now explains why
the skip is not self-healing and why failing is the wrong lever, and a
test renders the UI with *every* user unresolvable so a reintroduced
`fail` fails the suite.

**Scope this narrows, deliberately:** tenant buckets keep the
pre-existing silent skip. The gate owns only the platform bucket, which
is the cluster-breaking path. Generalising the repair to every bucket
needs a controller that owns tenant buckets and is a separate change.

Two more corrections from the same review:

- `cozystack_backup_default_objects_missing` was only written on the
happy path, so the state this PR exists to catch reported `0` and the
documented alert never fired. The credentials Secret now counts in
`missing`, so an absent or rotated-away Secret moves the gauge. On an
API error the gauge is deliberately left alone rather than flapping, and
a new `cozystack_backup_default_objects_check_errors_total` marks it
stale; the doc pairs the two instead of overstating the gauge.
`force_reconciles_total` gained `namespace`/`name`, since two releases
can now be forced.
- `forceHelmRelease` does a point `Get` and skips a release with
`spec.suspend: true`. helm-controller ignores both annotations while
suspended (`cozyhr suspend` sets exactly that), so the gate was
re-stamping every `MinForceInterval` for the whole suspension and
climbing the force counter — which the runbook attributes to a render
that is not producing the objects. The skip is logged and not counted.

### 3. Corrected comments and docs

`_helpers.tpl`, `backupclass-default.yaml`, `values.yaml` and
`docs/operations/backup-classes.md` all claimed the skip self-heals on
the HelmRelease interval, and the doc suggested a `flux reconcile` that
does not re-render. They now say it does not, and why. The two-release
manual recovery for already-affected clusters is documented, including
the easy-to-miss part: the credentials Secret is rendered by the
`bucket-<name>-system` release, not by `bucket-<name>`.

### Manual recovery for already-affected clusters

Only needed on a version without the gate. A plain `flux reconcile
helmrelease` does **nothing** — it does not re-render.

```bash
# 0. Confirm the bucket name is resolvable first — forcing before that
#    just re-runs the same empty lookup.
kubectl -n tenant-root get bucketclaim bucket-cozy-backups -o jsonpath='{.status.bucketName}'

# 1. The credentials Secret. Rendered by the -system release, NOT by
#    bucket-cozy-backups — easy to miss, and the projector (hence every
#    strategy, Velero, and migration 50) has no source without it.
ts=$(date -u +%Y-%m-%dT%H:%M:%SZ)
kubectl -n tenant-root annotate helmrelease bucket-cozy-backups-system \
  reconcile.fluxcd.io/forceAt="$ts" reconcile.fluxcd.io/requestedAt="$ts" --overwrite

# 2. The Strategy CRs and the Velero BSL.
ts=$(date -u +%Y-%m-%dT%H:%M:%SZ)
kubectl -n cozy-backup-controller annotate helmrelease backupstrategy-controller \
  reconcile.fluxcd.io/forceAt="$ts" reconcile.fluxcd.io/requestedAt="$ts" --overwrite
```

### Validation

- `go test ./internal/... -count=1` — pass, including 15 cases for the
gate: force on missing, no-op in the steady state, throttling, unmapped
kinds ignored, BSL skipped when disabled, patch failure surfaced without
advancing the throttle, absent HelmRelease tolerated, plus
force-on-absent-Secret, force-on-empty-bucket-name, external-S3
reports-but-does-not-force, independent throttles, suspend skip on each
of the two releases, gauge written on the unresolved path, and errors
counter on a failed check.
- `make helm-unit-tests` — pass, with 2 new suites: 3 cases for the
bucket chart (partial render preserved, and the UI still renders with
every user unresolvable — the guard against reintroducing the `fail`), 7
for the gate's env + RBAC wiring. Those include that `.Release.Name` and
`.Release.Namespace` match the HelmRelease the operator generates
(`releaseName` becomes the HR's `metadata.name`, and no
`spec.releaseName`/`targetNamespace` is set, so the Helm release name
equals the HR name), that the bucket release coordinates track a renamed
bucket together with `systemSecretName`, and that they are unwired on
`provisionBucket: false`.
- `make generate` — no diff.
- `make go-unit-tests rd-presets-check test-check-readiness
migrations-target-check` — pass.
- `make bats-unit-tests` — every suite passes except
`hack/migration-seaweedfs-db-adopt.bats`, which needs a Docker daemon
not available in this environment ("docker is required: these tests run
the migrations inside alpine"). Unrelated to this diff.

**Not verified — please read this before approving.** There was still no
live-cluster run. What that gap covers is smaller than it was: dropping
the chart `fail` removed the universal behaviour change on every
bucket's install path, so there is no longer a render-blind failure mode
whose convergence has to be observed. What remains is one mechanism —
`forceAt` + `requestedAt` on an unchanged release — applied to two
releases instead of one, argued from read code rather than an observed
install: `Strategy.Name = RetryOnFailure` on operator-generated
HelmReleases (`internal/operator/package_reconciler.go`), and
`bucketName = "bucket-" + claim.UID` in
`packages/system/objectstorage-controller/images/objectstorage/patches/92-bucketclaim-propagate-ready.diff`.
It is the same procedure as the manual recovery below, which has been
run by hand on the reporter's cluster. Someone with a bootstrap cluster
should still confirm that the gate fires within a minute or two of the
bucket becoming ready, and that forcing `bucket-<name>-system`
materialises the credentials Secret.

### Screenshots

Not applicable: no UI change. This PR touches a controller, two system
charts, and docs.

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

Walked file by file against the diff of this PR.

**`website` — affected, follow-up issue opened.**
`content/en/docs/next/operations/services/backup-classes.md` mirrors
this repo's `docs/operations/backup-classes.md` and carries the exact
stale claim this PR corrects, plus a `flux reconcile helmrelease`
recovery suggestion that has no effect.
`operations/configuration/platform-package.md` enumerates the forwarded
`backupStorage` keys, and `reconcileDefaultObjects` is admin-settable
through `spec.components.platform.values.backupStorage` (the whole block
is deep-merged), so that list becomes incomplete. Filed as an issue
rather than a PR because the wording depends on the shape this PR lands
in, and whether to touch the frozen `v1.5`/`v1.6` copies is a maintainer
call. No existing website issue or PR covered it.

**`terraform-provider-cozystack` — not affected.** Checked, not assumed:
`packages/apps/**` is untouched by this diff (`git show --stat HEAD --
packages/apps/` is empty), so no app `values.schema.json`, no
`values.yaml` default, no version enum, and no `ApplicationDefinition`
`kind`/`plural`/`release.prefix` changed. `reconcileDefaultObjects` is a
key on a **system** chart, which has no `values.schema.json` and is not
modelled by the provider. (An earlier revision added
`requireUserCredentials` to the `bucket` system chart; that is gone, and
no key was added to `apps/bucket`, so the app API is untouched.) The
`<bucket>-<user>-credentials` Secret the provider looks up by literal
name is **not renamed** — this PR only changes whether it gets created,
which if anything removes a case where the provider silently read null
from a Secret that never existed.

Everything else: no `hack/` layout or make-target change (`ccp`,
`external-apps-example`, `cozyhr`), no `ApplicationDefinition` CRD
change, no namespace rename, no node prerequisites in
`hack/e2e-prepare-cluster.bats` (`talm`, `ansible-cozystack`), no
`cozyhr.cozystack.io/values-files` annotation or chart-source-kind
change, no `service-proxy-name` / `wholeIP` / `allowICMP` rename
(`cozy-proxy`), no telemetry metric or label rename
(`cozystack-telemetry-server` — the two metrics here are new, not
renamed), no node or network requirement change (`examples`).

- [ ] No downstream repository is affected by this change
- [x] [cozystack/website](https://github.com/cozystack/website) -
follow-up: https://github.com/cozystack/website/issues/639
- [ ]
[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(backupstrategy-controller): the default backup Strategy CRs, the Velero BackupStorageLocation and the bucket user-credentials Secret were skipped permanently when their install-time Helm `lookup` came back empty, leaving clusters with a BackupClass but no strategies and blocking the v1.6.0 etcd migration. The controller now detects the missing objects and forces the Helm upgrade that creates them, including on the bucket release that renders the credentials Secret.
```


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

- **New Features**
- Added automatic detection and recovery for missing default backup
resources.
- Missing resources trigger a throttled Helm upgrade once bucket
configuration is available.
- Added configuration to enable or disable default-object
reconciliation.
- Added validation requiring user credential Secrets during bucket
installation by default, with an offline rendering option.

- **Bug Fixes**
- Improved recovery from incomplete backup and Velero storage
configuration.

- **Documentation**
- Expanded operational guidance, monitoring metrics, alerts, and manual
recovery steps.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
2026-08-10 13:10:08 +03:00
IvanHunters 980715250c fix(kube-ovn): reload kubeovn-webhook serving certificate on cert-manager renewal (#3557)
## What this PR does

The `kube-ovn-webhook` loaded its TLS serving certificate once at
process startup (`tls.LoadX509KeyPair` into a static
`tls.Config.Certificates`) and never re-read it. cert-manager renews the
backing Secret ahead of expiry, but a running pod kept serving the
certificate it loaded at boot. Once that certificate expired — roughly
one year after install, on any cluster whose webhook pods had not been
restarted after the renewal — the pod served an expired certificate, the
kube-apiserver's TLS call to the webhook failed verification, and
because the `MutatingWebhookConfiguration` uses `failurePolicy: Fail`,
every pod creation in tenant namespaces was rejected. In practice this
blocks `virt-launcher` pods, so no VMI can start.

This PR:

- Serves the certificate through a reloading `tls.Config.GetCertificate`
callback that re-reads the key pair when the mounted files change, and
keeps serving the last good certificate if a reload fails (e.g. a torn
read while the volume is being updated). cert-manager renewals are now
honoured without a pod restart.
- Widens the certificate `renewBefore` from `24h` to `720h`, so renewal
happens well ahead of expiry instead of one day before.
- Adds a regression test that fails against the previous static
implementation (the server keeps presenting the old certificate after
the files are replaced) and passes once the certificate is reloaded;
verified non-vacuous by mutation.

### Screenshots

Not applicable (no UI changes).

### Downstream repositories

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

Walked the trigger map file-by-file against the diff: the change is
confined to the `system/kubeovn-webhook` image and its cert-manager
`Certificate` `renewBefore`; it touches no package under
`apps/`/`extra/`, no `values.schema.json`, no app `values.yaml` default,
no `ApplicationDefinition`, no platform values, no CRD, and no `hack/`
tooling.

### Release note

```release-note
fix(kube-ovn): kubeovn-webhook now reloads its serving certificate when cert-manager renews it, so the webhook no longer serves an expired certificate that blocked all pod creation (including VMIs) roughly one year after install
```


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

* **New Features**
* Webhook TLS certificates now reload automatically when renewed,
without requiring a server restart.

* **Bug Fixes**
* Certificate reload failures now fail the TLS handshake instead of
serving an outdated certificate.
  * Added server timeouts to improve connection handling.

* **Configuration**
* Certificates are now renewed earlier to help prevent
expiration-related service disruptions.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
2026-08-10 12:49:36 +03:00
Aleksei Sviridkin efa7f5c8fb fix(velero): apply CRD updates on upgrade via CreateReplace (#3727)
## What this PR does

Velero CRDs stayed frozen at whatever version was first installed: Helm
never touches the chart's `crds/` directory on upgrade, the generated
HelmRelease left `spec.upgrade.crds` at the helm-controller default
(Skip), and the package disables the upstream upgrade-crds Job. When the
velero image moved to v1.18.1, which added the `Queued` and
`ReadyToStart` backup phases, the apiserver rejected every phase
transition against the stale CRDs (`status.phase: Unsupported value:
"Queued"`) and backups silently stopped while the HelmRelease stayed
green.

This opts the velero PackageSource into `upgradeCRDs: CreateReplace`,
the same policy mongodb-operator already uses, adds a platform test
pinning it, and corrects the stale Job-disable rationale in the package.
CreateReplace is safe here: velero only extends the phase enums between
versions.

Closes #3726

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

### Release note

```release-note
fix(velero): apply Velero CRD updates on upgrade; previously CRDs stayed at their first-install version, so after the image moved to v1.18.1 backups could silently stop with `status.phase: Unsupported value: "Queued"` while the HelmRelease stayed green
```


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

## Summary by CodeRabbit

* **New Features**
* Velero now automatically creates or replaces its Custom Resource
Definitions during installation and upgrades.
* **Bug Fixes**
* Improved Velero CRD lifecycle handling to ensure updated definitions
are applied reliably.
* **Tests**
* Added coverage verifying the CRD upgrade configuration and updated
validation guidance.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
2026-08-10 12:39:34 +03:00
Aleksei Sviridkin f4d812cfb9 fix(velero): apply CRD updates on upgrade via CreateReplace
velero evolves its CRD schema between versions; v1.18 adds the Queued
and ReadyToStart backup phases. Helm never upgrades CRDs shipped in
crds/, the generated HelmRelease left spec.upgrade.crds at the Skip
default, and the package disables the chart's upgrade-crds Job, so live
CRDs stayed frozen at first-install state: the server upgrades, the
apiserver rejects the new phases, and backups sit in New while the
HelmRelease stays green. Opt the velero PackageSource into
CreateReplace, consistent with mongodb-operator, add a platform test
pinning the policy, and correct the stale Job-disable rationale in the
package.

Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <3811295@gmail.com>
2026-08-10 12:23:16 +03:00
Aleksei Sviridkin f0bde76b9f docs(hack): fix a wrong claim about grep-error propagation in two contract tests
Both hack/promote-gate-contract.bats and hack/release-freeze-contract.bats
carried the same code_lines helper with the same wrong comment: it claimed
a real grep error fails the pin through the helper's own exit status.
code_lines is never the last stage of a pipeline in either file, so
ordinary (non-pipefail) pipe semantics discard whatever it returns on its
own, and hack/cozytest.sh's translator appends a return 0 to every line
that is exactly }, code_lines' closing brace included, making that exit
status even less trustworthy as a signal.

What catches a real grep error is the output, and only for one class of
pin. The error empties code_lines' stream, so a pin that requires
something to be present in that stream fails on it. A pin demanding an
absence stays blind: an exact count of zero is what an emptied stream
produces anyway, and what the pin was written to assert. Both files hold
pins of that shape.

The per-test [ -n "$block" ] guard is not the backstop either. In these
two files a block is a bare awk extraction, upstream of code_lines, so
that guard cannot see the filter fail; the comment now says so, because
reading it as the backstop gets the direction exactly wrong.

Signed-off-by: Aleksei Sviridkin <f@lex.la>
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <3811295@gmail.com>
2026-08-10 12:08:02 +03:00
Aleksei Sviridkin ccd8a289ef fix(e2e): budget each linstor wait separately in post-install-prep (#3715)
## What this PR does

Both waits in `hack/e2e-post-install-prep.sh` ran off one 15-minute
deadline anchored at the script's start, so the head of the chain ate
the window and the tail got whatever was left. Two runs on 2026-08-09
failed that way. The linstor HelmRelease went Ready 452s in, and the
Deployment needed a further ~520s because its DB migration lost the
apiserver and retried four times. Neither figure passes 900s alone.
Their sum does, and the install reported the Deployment as broken.

Each wait now gets its own budget, started when the previous link
finished. Same shape as the node wait and the MetalLB CRD wait further
down the same file. What it buys is that every link gets its whole
allowance regardless of what the links ahead of it spent.

The cost is on both sides and I measured it. Worst case goes from one
15m window to 900 + 900 + 300 + 300 = 40m, against the 180m the e2e job
carries. A Deployment that never converges is now reported at 1355s
instead of 900s, from the 452s above plus the loop's 5s poll. A
HelmRelease that never goes Ready is still reported at 900s, which I
checked by running the new suite against the pre-change script.

The timeout now dumps the pods and recent events of `cozy-linstor`. The
same "Deployment not Available" has come from a crash-looping migration
init container and from a pod waiting on a client TLS Secret
cert-manager had not issued yet, and telling those apart used to need
the run's bundle. Events come from `kubectl events`, which sorts by
last-seen and carries no `--sort-by` to fail on a key some item lacks.
Both reads keep their stderr and are bounded, with the client budget
smaller than the outer one so kubectl gets to say why before the kill
lands.

`hack/post-install-prep.bats` runs the script end to end against a stub
PATH with a virtual clock, so a 15-minute budget elapses in a few
hundred forks and the timings are exact. Four of its six tests fail
against the pre-change script. The other two guard the bound that stays.

### Known gaps in the new tests

Dropping the `2>&1` from either diagnostic read leaves all six tests
green, because the harness catches the script's own stderr into the same
file the assertion reads. The property holds in production anyway, since
the caller merges both streams into one log, but nothing catches the
removal. The test for "one failing read does not suppress the other"
needs the pods read to run first; swap the two reads and it passes
vacuously. Its comment says so at the point where someone would make
that change.

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

Walked the map against the diff. Two files, both under `hack/`. The
`ccp` trigger is about moving or renaming things under `hack/` or
changing what a make target does, and its anchors `hack/package.mk` and
`hack/common-envs.mk` are untouched. `talm` and `ansible-cozystack` key
off node prerequisites in `hack/e2e-prepare-cluster.bats`, a different
file.

### Related

Same family, found while working here and filed instead of folded in.
#3707 and #3708 for unbounded reads elsewhere in this script, #3712 for
the `--sort-by=.lastTimestamp` form in fourteen other places, #3713 for
the wait loop dropping kubectl's stderr for its whole budget, #3714 for
the MetalLB CRD wait failing without a message.

### Release note

```release-note
NONE
```


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

## Summary by CodeRabbit

* **Bug Fixes**
* Improved LINSTOR readiness checks by giving each prerequisite its own
15-minute wait period.
* Added clearer timeout diagnostics, including relevant pod and event
information.
* Ensured subsequent readiness checks are skipped when an earlier
HelmRelease check fails.

* **Tests**
* Added end-to-end coverage for timeout handling, sequential failures,
diagnostic errors, and readiness behavior.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
2026-08-10 11:44:15 +03:00
Aleksei Sviridkin 6ee56e3a60 fix(ci): decide E2E test coverage per changed path (#3441)
## What this PR does

`hack/select-e2e.sh` escalates to the full Chainsaw suite when a changed
package reaches no suite through the dependency graph, because otherwise
that change gets no E2E coverage at all. The escalation was read off the
merged selection, so it depended on the whole diff rather than on the
path that needed it. Any other changed path contributing a suite name
made the selection non-empty and the escalation vanished with nothing in
the output to say it had. Editing a platform component together with one
unrelated suite's Chainsaw test ran that single suite instead of
everything, which is an ordinary PR shape: change a component, adjust
the e2e near it.

Coverage is now decided per changed path. Each path resolves through its
owning sources to the suites that exist, and a path reaching none of
them forces the full run whatever the rest of the diff selected. The
unit is the path rather than the source, because
`system/postgres-operator` belongs to two PackageSources where one
reaches no suite, and deciding per source would run everything on every
change to it.

The other half is the mapping that left packages unreachable from their
own suites. `src_to_suites()` covered `*-application` names plus
`external-dns`, and the final filter dropped the rest, so
`cozystack.kuberture` and `cozystack.securitygroup-controller` were
walked to and then discarded. A CNI change ran neither of the two
controller suites most likely to regress from it, and a change to either
package escalated to the full run rather than selecting the suite that
covers it. Every reached source now maps through the table, which is the
inverse of `select-install.sh`'s `suite_to_source()`; a test walks every
suite through both, so the next source name fitting neither convention
fails there instead of quietly escalating forever.

## What was measured

Behaviour was diffed against the current selector over one `values.yaml`
per package under `packages/{apps,system,extra}`: 159 packages, 8
selections change, all of them the mapping repair. Five networking
packages and `prometheus-operator-crds` widen — 19 to 21 suites and 10
to 11, both gaining `kuberture` and `securitygroup`. `kuberture` and
`securitygroup-controller` narrow from the full suite to their own
single suite. Nothing narrows that should not.

That sweep is one path per diff, so it says nothing about the per-path
escalation, which only shows on a diff with more than one path. Pairing
each package with one unrelated Chainsaw edit gives that number. 82 of
the 159 packages escalate on their own and stop escalating once the pair
is added, which is the size of the hole: for those, adding an unrelated
suite edit to the same pull request replaced the full run with one
suite. 88 packages select something different after this change, the
extra six being the mapping repair widening a selection rather than
restoring an escalation.

Worth stating alongside it, because it decides whether the branch is a
corner or the common case: 83 of the 159 escalate on a single-path diff
today, and 81 do after this change — the two that leave are `kuberture`
and `securitygroup-controller`, which now select the suite that covers
them. So the escalation is the branch most packages take, and the
swallowed one was the branch most packages took. Counting that needs an
instrument that separates a graph walk which reached everything from an
escalation which printed everything, since the two agree once the
mapping is fixed: the count above comes from a scratch suite tree
carrying one extra suite that no source names, which only an escalation
can emit.

The suite selection is what decides which tests run, so a rule that
stops firing does not go red, it disappears. The tests pin the property
rather than a worked example: that a path reaching no suite escalates
whatever else the diff holds, that a covered path never drags the full
suite in with it, and that every suite round-trips between the two
mapping tables. Each was checked by mutation — removing the rule it
guards turns it red.

## Left for a follow-up

`resolve_suites` writes into the caller's scope because POSIX sh has no
`local`, and what keeps that safe is the single call site sitting inside
`$( )`, pinned by a test that counts occurrences of the name. Declaring
the function with `( … )` instead of `{ … }` would contain those
variables by construction and let both the test and the two paragraphs
explaining the constraint go away. A count-the-name grep is the weakest
kind of pin and it exists only because the function leaks.

The output contract at the top of the script lists the causes of a
full-suite selection but not the backstop, which after this change is a
mechanism of its own: an edit under `hack/e2e-chainsaw/<dir>/` where
that directory holds no `chainsaw-test.yaml`. The behaviour is pinned by
a test and explained where it lives; only the header block is short. In
the same area, the escalation line names the owning sources rather than
the changed path, and reports the first uncovered group rather than all
of them.

## Overlap

#3434 fixes the same escalation from a different angle: it decides the
escalation from the whole source-derived selection rather than per path.
That closes the "package with no coverage plus an unrelated suite edit"
shape, and leaves "package with no coverage plus another package that
does select suites", where the source-derived selection is non-empty and
the escalation is swallowed again. There is a test for that second shape
here.

### Screenshots

Not applicable, no UI changes.

### Downstream repositories

Walked the trigger map in `docs/agents/contributing.md` against the
diff. The changed files are `hack/select-e2e.sh`,
`hack/select-e2e_test.bats` and `docs/agents/e2e-testing.md`. The only
`hack/`-related trigger is cozystack/ccp, which gates on
`hack/package.mk`, `hack/common-envs.mk` and `hack/update-crd.sh` and on
what a make target does; nothing here is moved, renamed, or changed in
make-target behaviour. The ansible-cozystack and talm triggers are about
`hack/e2e-prepare-cluster.bats`, which is untouched.

- [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(ci): a changed package that no E2E suite covers now runs the full suite even when the same pull request also edits an unrelated suite, and changes to the kuberture and securitygroup packages select their own suite instead of escalating
```

Closes #3330

Closes #3665
2026-08-10 11:44:10 +03:00
Andrei Kvapil 361c729417 test(codeowners): guard the coverage invariant with a unit check
CODEOWNERS is last-match-wins, so a new scoped rule that omits the
default owners silently strips their ownership for that path. Add a
hack/*.bats unit test (auto-discovered by make bats-unit-tests) that
requires every rule listing owners to repeat the owners of the *
catch-all, exempting ownerless rules and the governance roster.

Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Andrei Kvapil <andrei.kvapil@aenix.io>
2026-08-10 10:30:30 +02:00
Andrei Kvapil bd7b76691e chore(codeowners): keep architects as owners in every non-governance rule
CODEOWNERS applies only the last matching pattern, so a scoped rule
replaces the * catch-all instead of extending it: @kvaps and @lllamnyp
were not code owners for most scoped paths, and several paths (/hack/,
/.github/, /Makefile, /.pre-commit-config.yaml, /docs/) were owned by
exactly two people, deadlocking PRs authored by either of them while
the other is away.

Append @kvaps and @lllamnyp to every rule that lists owners, except the
governance block which deliberately stays with @tym83 @kvaps, and spell
out the non-additive last-match semantics in the file header.

Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Andrei Kvapil <andrei.kvapil@aenix.io>
2026-08-10 10:26:04 +02:00
Aleksei Sviridkin 86674c7084 fix(e2e): budget each linstor wait separately in post-install-prep
The post-install prep script waits out a serial reconcile chain --
the linstor HelmRelease, then the linstor-controller Deployment --
against a single 15-minute deadline anchored at the script's start.
The head of the chain therefore eats the window and the tail inherits
whatever remains, so an install fails whenever the chain merely runs
long rather than never converging.

Measured on one such failure: the HelmRelease went Ready 452s in, and
the Deployment needed a further ~520s because the controller's DB
migration lost its apiserver connection and retried four times before
succeeding. Neither figure exceeds 900s on its own; only their sum
did, and the install reported the Deployment as though it were broken.

Give each wait its own budget, started when the link before it
completed -- the shape the LINSTOR node and MetalLB CRD waits further
down already have. What that buys is not speed but a guarantee: every
link gets its whole allowance regardless of what the links ahead of
it spent, so the tail can no longer be failed by the head being slow.

It is a trade, and the other half is worth stating. The script's worst
case becomes 900 + 900 + 300 + 300 = 40m against the 180m the job
carries, where before the two waits shared one 15m window. And a link
that is genuinely stuck is reported later rather than sooner: a
Deployment that never converges surfaces at 1355s where the shared
window surfaced it at 900s, on that run's timings. A HelmRelease that
never becomes Ready surfaces at the same moment as before, since its
budget still starts with the script.

A timeout now also lists the linstor pods and the namespace events.
The description names the object waited on, not the link that held it
up, and the same "Deployment not Available" has been reached both with
the controller pod crash-looping its migration init container and with
that pod blocked on a client TLS Secret cert-manager had not issued
yet. Telling those apart previously required the run's bundle.

The events come from `kubectl events` rather than `kubectl get
events`. It orders by when an event was last seen, so a condition
repeating for minutes -- what a stuck mount looks like -- stays inside
the window instead of being pushed out by newer one-off events, and it
carries no --sort-by, so it cannot fail the whole read the way a sort
key does when one item lacks it.

Both reads keep their stderr and carry a bound. A read that fails
silently is indistinguishable from a namespace with nothing to report,
which is the distinction being drawn; and the caller is blocked in
`wait`, so an unbounded read against a wedged apiserver would hold the
install open until the job's own timeout. The client budget is
strictly the smaller of the two so kubectl gets to name the reason
before the outer kill takes it away.

The wait helper is named for linstor because those diagnostics read
one namespace unconditionally: the wait is generic over its kubectl
arguments, what it dumps on the way out is not.

The new unit test runs the script end to end against a stub PATH
carrying a virtual clock, so a 15-minute budget elapses in a few
hundred forks and the timings are exact rather than wall-clock
approximate.

Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
2026-08-10 00:10:53 +03:00
Aleksei Sviridkin 587a7b9fbd fix(ci): decide E2E coverage per changed path
The full-suite escalation for a changed package that reaches no suite was
read off the merged selection, so it depended on the whole diff rather
than on the path that needed it. Any other changed path contributing a
suite name made the selection non-empty and the escalation vanished with
nothing in the output to record it: edit a platform component, adjust the
Chainsaw test beside it, and the component ran that single suite instead
of everything. Over one values.yaml per package under
packages/{apps,system,extra}, 82 of the 159 escalate on their own and
stop escalating once an unrelated suite edit joins the diff.

Coverage is now decided per changed path. Each path resolves through its
owning sources to the suites that exist, and a path reaching none of them
forces the full run whatever the rest of the diff selected. The unit is
the path rather than the source: system/postgres-operator belongs to two
PackageSources where one reaches no suite, so deciding per source would
escalate on that half and run everything for every change to it.

The escalation names its cause on stderr, as every other escalation in
this script already does. Most packages reach no suite of their own, so
this is the branch a full run usually takes; without a line saying which
sources bought it, "why did this run everything" has no answer in the log.

The walk moves into resolve_suites() to be called once per path. POSIX sh
has no `local`, so the function writes into the caller's scope and is
safe only inside the $( ) its single call site wraps it in; a test pins
that the call count stays one.

Two entries leave src_to_suites. `postgres-application` restated what
stripping the suffix produces and `external-dns` restated what carrying
the source name through produces, so both made the exception list read as
longer than the set of real exceptions.

Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
2026-08-09 22:24:37 +03:00
Aleksei Sviridkin 0980e755eb fix(ci): map E2E suites from sources not named after an app
src_to_suites() covered PackageSource names ending in -application plus
external-dns, and the filter after the graph walk dropped every other
source it reached. cozystack.kuberture and cozystack.securitygroup-
controller are neither, so both were walked to and then discarded: a CNI
change ran neither of the two controller suites most likely to regress
from it, and a change to either package reached no suite at all and
escalated to the full run instead of selecting the one that covers it.

Every reached source now maps through the table. That table is the
inverse of select-install.sh's suite_to_source(), and a test walks every
suite through both, so the next source name fitting neither convention
fails there rather than quietly making its own suite unreachable.

The cilium test measured the gap rather than hid it, asserting the
19 suites the selector could reach and naming the two it could not.
The subtraction is removed rather than inverted: kept as an exclusion
list it would stay green if the walk stopped reaching them for an
unrelated reason. What replaces it is a second assertion against a tree
holding one suite the graph reaches and one no source names, which an
escalation cannot satisfy.

Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
2026-08-09 21:28:00 +03:00
Aleksei Sviridkin 677d80a479 feat(kubernetes): route tenant worker image pulls through a registry mirror (#3575)
## What this PR does

Refs #3513. It addresses one variant of that flake and does not close
it; see "Scope against #3513" below.

### Root cause of the variant this addresses

The chainsaw `kubernetes-latest` / `kubernetes-previous` suites fail
with `node-join failed: fewer than 2 tenant nodes Ready within 18m` and
zero registered tenant nodes. The in-guest Talos capture on #3548 shows
where the time goes on one class of those runs: the worker loops trying
to pull `ghcr.io/siderolabs/kubelet` from public ghcr.io and fails with
`Head "https://ghcr.io/v2/siderolabs/kubelet/manifests/...": net/http:
TLS handshake timeout host=ghcr.io`. The Talos `kubelet` service never
starts (`kubelet was not registered`), so the node never registers,
cilium-operator stays `Pending` for lack of a node to schedule on, and
the budget expires.

On those runs the tenant apiserver is healthy and the worker VMs boot
with kube-ovn IPs, so the failing link is the worker's public image-pull
egress, and the worker machine config has no
`machine.registries.mirrors`, so there is no way to route around it.
Same class as the Talos OS image, which #3231 solved with the
`talos.imageFactoryURL` cache.

The thread also carries a later analysis putting worker-VM CPU
starvation from nested-virt oversubscription at the root, with
serial-console RCU stalls showing the guest never reaching Talos
userland, and reading the TLS-handshake variant as a symptom of the same
contention. Nothing here contradicts that. Removing a live dependency on
public egress from worker bring-up is worth doing on its own terms, and
if the contention account is the dominant one, this narrows what is left
to explain rather than competing with it.

### The change

**Chart (`packages/apps/kubernetes`,
`packages/apps/kubernetes-nodes`)**, a `talos.registryMirrors`
passthrough, a map of upstream registry host to `{ endpoints: [ ... ]
}`, rendered into `machine.registries.mirrors` of the worker
`TalosConfigTemplate` that the MachineDeployment clones from, so it
applies at first boot rather than after it. Empty by default: with the
default values the rendered chart is byte-identical to main, both
content-hash object names included, so no existing worker rolls. This is
the durable half; an air-gapped, rate-limited or flaky-egress
environment can point worker image pulls at its own mirror, which is the
`machine.registries.mirrors` knob the Talos rollover left unfilled.
Talos still falls back to the upstream registry unless a host also sets
`skipFallback`, so a mirror on its own is not air-gap enforcement, and
the field doc says so.

The value lands inside the reconcile Job's unquoted heredoc, so it is
escaped for backslash, dollar and backtick and renders as a literal.
`talos.installerRepository`, `talos.schematicID` and the Talos version
render into that same heredoc one block above it and now get the same
treatment, so the file is consistent about it rather than escaping only
the newest field. Escaping the rendered value covers every byte by
construction, which enumerating known-bad shapes does not. The chain is
a no-op for values carrying none of those characters, so the default
render, content-hash Job name included, is unchanged and no existing
worker is replaced.

**e2e harness**, an in-sandbox `registry:2` pull-through cache for
ghcr.io (`hack/e2e-ghcr-mirror.yaml`), applied at install time from its
own step, with `hack/e2e-chainsaw/_lib/ghcr-mirror.sh` setting
`spec.talos.registryMirrors` on the tenant CR when the mirror is up and
its egress allow is in place, and emitting nothing otherwise so workers
pull directly. A `CiliumClusterwideNetworkPolicy` opens one
tightly-scoped hole for worker VM (`kubevirt.io: virt-launcher`) Pods in
`tenant-test` to reach the mirror, mirroring the existing
`talos-image-cache` policy.

Two properties of the helper worth calling out, because both are easy to
get wrong and both are now pinned by tests. A failed API call is never
cached: the decision is reused by every later suite in the shared
sandbox, so caching one blip would disable the mirror for the whole run.
Readiness is decided once and acted on once. `rollout status` is the
cheap way to learn it, but it also exits non-zero on a broken watch, so
when it fails the Deployment's own `Available` condition is read rather
than the watch's exit code taken as the answer. Exactly one failure is
cached, a definite `Available=False`, and that is a trade rather than an
invariant: it avoids re-paying the wait in every later suite, and costs
a mirror that recovers mid-run.

On a node-join failure the suite dumps the mirror's state alongside the
Talos image cache it already dumped. Workers being pointed at the mirror
and then joining does not establish that the mirror served anything,
because Talos falls back to public ghcr.io on its own; the registry's
access log is what separates the two, so the dump counts kubelet-image
requests across the whole log rather than tailing it. The readiness
probe writes an access line every five seconds, so by the time a run has
spent its 18m budget a fixed tail window is hundreds of lines short of
the request worth finding.

### Testing

- `helm unittest packages/apps/kubernetes`, 205 tests, 22 suites; `helm
unittest packages/apps/kubernetes-nodes`, 14 tests, 4 suites. New cases:
the default omits `machine.registries.mirrors`, and a set value renders
it with the endpoint.
- `hack/ghcr-mirror_test.bats`, 23 tests. Beyond the manifest's
two-phase split, egress identities, pull-through config and the pure
YAML builder, this covers every outcome of
`resolve_ghcr_mirror_endpoint` against a stubbed `kubectl`: an API
NotFound caches, a transient query failure does not, a shell reporting a
missing binary is not mistaken for either, a failed egress allow does
not cache and its reason reaches the log, a rollout watch that failed
for a reason the API does not confirm does not cache while a definite
`Available=False` does, and a committed endpoint is cached and
short-circuits the next call. The stub honours `--tail`, so the
diagnostic's tests tell a filtered read from a truncated one instead of
trusting the flag, and it reports errors on stderr as the real binary
does. The suite also pins that the install step excludes the Cilium
policy from its pre-Cilium apply, not only what that exclusion produces.
- `hack/talos-reconcile-heredoc_test.bats`, 5 tests, which render both
charts and run the heredoc through a real shell with hostile values
rather than regexing the rendered string. A `matchRegex` cannot catch a
heredoc the shell refuses to emit; this runs it.
- `hack/run-kubernetes-talos-spec_test.bats`, 5 tests over
`talos_spec_block`, the point where the two optional fragments are
merged. The assertions splice the result under `spec:` and read it back
with yq, so a block indented into the wrong parent fails instead of
passing.
- `packages/apps/kubernetes-nodes/tests/render-parity.sh`
byte-identical; per-chart `make generate` in both charts with no drift;
both `cozyrds` `openAPISchema` blocks equal to their
`values.schema.json`.
- The pinned `mirror.gcr.io/library/registry:2.8.3@sha256:a3d8aaa6...`
is the digest Docker Hub serves for that tag, and `mirror.gcr.io` serves
the same OCI index with a linux/amd64 manifest. Run under exactly the
security context the Deployment sets, non-root 65532, read-only root
filesystem, all capabilities dropped, writable state only on the mounted
volumes, it starts, answers `/v2/` with 200, and serves
`ghcr.io/siderolabs/kubelet` manifests and blobs as an anonymous
pull-through.
- `shellcheck` and `git diff --check` clean on the new and edited shell.

### What a CI run still has to answer

Two runtime facts the harness half rests on, both recorded in the
manifest:

1. Talos honours a plain-http mirror endpoint via
`machine.registries.mirrors[ghcr.io].endpoints=[http://...]`. If a
scheme or TLS knob is additionally needed, `machine.registries.config`
has to be added too.
2. The containerd pull from inside the worker VM leaves with the
virt-launcher Pod's own Cilium identity, which is what the policy
selects. The label half is settled: `run-kubernetes.sh` already reads
tenant worker Pods by `kubevirt.io=virt-launcher`, but whether the
in-guest pull is subject to that Pod's egress policy at all is not.

The second one bounds how strong a claim the harness can make. The gate
is a `rollout status` plus an accepted egress-allow object, and neither
observes the tenant-side datapath, so the fallback paths cannot make CI
worse while the committed path is only as safe as that assumption. The
in-repo comments say this rather than claiming the mirror can only ever
help.

### Named improvements left out

A tenant-scoped reachability probe, like the byte-level 206 check
`talos-image-cache.sh` runs from a Pod carrying the consumer's own
label, is what turns assumption 2 from an assertion into a measurement,
and would also let the suite assert the mirror served the kubelet pull
instead of the worker quietly falling back. Left out because a probe
that does not exercise the tenant network path would pass while real
workers stayed blocked, and the one that does is a change of its own
size. The same gap has a cheaper edge: both the install step and the
resolver key on the Deployment alone, so an apply that made the
Deployment but not the Service still reads as success. That one costs
little in practice, since the guest gets a fast NXDOMAIN instead of a
dropped SYN, but the gate does check less than it commits to.

`registryMirrors` could be a typed `map[string]struct{ endpoints
[]string }`, the way `nodeGroups` already is in this chart, so malformed
input is rejected at admission instead of failing late inside Talos.

The helm-unittest escape cases assert the `$` path; the backtick path is
covered by the execution-level heredoc bats and the pure-backslash path
by the escape chain's construction, so the security contract is fully
exercised across the two layers. A backtick assertion at the
helm-unittest layer would express it at the fast layer too.

The escape chain is spelled out at five interpolation sites across the
two reconcile-Job templates. A named helper (an `include` template)
would make the escape a single reusable primitive and shrink the surface
where the heredoc invariant can be forgotten; the current per-site form
is correct and covered, so this is a readability refactor rather than a
fix.

Only `machine.registries.mirrors` is passed through.
`machine.registries.config` is not, so a mirror behind a private CA or
one needing credentials still has no knob, and the platform-wide
`registries` value does not reach tenant workers. The chart's
breaking-change note says so, and also says which knob covers which
artifact: the mirror routes registry pulls, while the worker OS disk
image arrives as a raw HTTP artifact under `talos.imageFactoryURL` and
no registry mirror can touch it.

The helper's failure model is worth revisiting as a whole rather than
per call site. Every call it makes to the cluster has three outcomes
(the world says no, the world says yes, the call could not be made) and
a shell exit code carries two, so the third is re-derived at each site
from a different signal: stderr text for a NotFound, a field value for
readiness, the returned message for a failed apply. Since the resulting
decision is cached for the whole sandbox, a misclassification is
permanent for the run rather than a blip, which makes each of those
sites load-bearing. A single prober answering `yes`/`no`/`unknown`, with
the caching rule stated once over that answer, would move the question
out of the branches; the current shape reaches the same behaviour but
re-establishes it five times. Left out here because it is a visible
rework of code this PR only extends, and it would need its own
verification of every site; tracked in #3682.

One nicety left in the same helper: the diagnostic reads the mirror log
twice, once with `--tail=-1` to count kubelet-image requests across the
whole log and once with `--tail=50 --prefix` for a bounded context tail.
The second read exists for `--prefix`, which attributes each line to its
pod; collapsing to one read drops that. The attribution is worthless
while the mirror is a single replica and load-bearing the moment it is
not, in the one artifact read after a failure, and the second read is
itself wall-clock bounded, so it is left in place.

### Merge-order dependency (resolved)

#3676 landed first and this branch is rebased onto it.
`ghcr_mirror_diagnose` now runs in the node-join failure block under the
same phase gate as its neighbours, and each of its reads is wall-clock
bounded by the same `COZY_DIAG_READ_TIMEOUT`/`COZY_DIAG_READ_GRACE` the
block validates, so no single read can hold the op open. It is placed
after the guest captures and before the image-cache re-probe: the
console evidence is irreplaceable and must not be starved of budget, and
the dump is still cheaper than the Pod-creating re-probe, so the spend
order puts it between the two. When the diagnostics phase runs out of
budget the dump is declined out loud instead of attempted. An ungated
dump would spend that time anyway and take it from the captures behind
it, while the mirror's state is partly recoverable from the reads that
already ran, so gating costs less than it saves. The known cost is
#3686: cheap reads ahead of the gate can drain the whole budget on a
viscous apiserver, and the dump is then declined in exactly the runs
where the mirror is a suspect. That is a defect of the budget, not of
this dump, and it is tracked there.

### Scope against #3513

#3513 asks for three things: capture the guest side, act on the
evidence, and do not raise the 18m budget again. The capture landed in
#3548 and this consumes its output; the budget is untouched. What this
acts on is the TLS-handshake variant only. The same thread documents
runs that issue no apid CSR at all inside the whole budget, time lost
before Talos reaches the kubelet image, which no registry mirror can
affect. So #3513 stays open after this merges, and a green suite
afterwards is not evidence the flake is closed. The manifest header says
so too, for whoever reads it next.

### Both tenant suites are covered

`kubernetes-previous` selects the previous Kubernetes *minor* out of
`packages/apps/kubernetes/files/versions.yaml`, `run_kubernetes_test
'keys | sort_by(.) | .[-2]'`, not a previously-released chart. Both
suites build their tenant CR from the same in-tree chart through the
same helper, so both get the mirror.

### Screenshots

Not applicable; no UI changes.

### Downstream repositories

Walking the trigger map in `docs/agents/contributing.md` against the
diff: **cozystack/terraform-provider-cozystack is affected**. The
provider is hand-written, and the map lists "add, remove or rename a
field in an app's `values.schema.json`" and "change a default in an
app's `values.yaml`" as triggers for its schema, model and
expand/flatten pair. This adds `talos.registryMirrors` with a `{}`
default to both the `kubernetes` and `kubernetes-nodes` schemas. No
other repository matches a trigger. The website's managed-app reference
pages regenerate from each package's `README.md` on a stable tag, and
this only changes an existing package's README.

I have not opened the provider follow-up and have not ticked the box,
because a ticked box with no link claims work that does not exist.
Leaving it to a maintainer to decide whether the provider models this
field and who files it.

- [ ] 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
feat(kubernetes): add `talos.registryMirrors` to route tenant worker node image pulls through a registry mirror for air-gapped or flaky-egress environments
```
2026-08-09 16:57:37 +03:00
Aleksei Sviridkin 3ae1e10c92 docs(kubernetes): correct the registryMirrors field and README claims
State in the README that in-guest registry pulls are routable again via
talos.registryMirrors, which knob covers which artifact (the mirror
routes registry pulls; the worker OS disk image arrives as a raw HTTP
artifact under talos.imageFactoryURL and no registry mirror touches
it), and the limits: skipFallback is needed for air-gap,
machine.registries.config is not passed through, and existing nodes
keep their boot-time config until replaced. Trim the field doc comment.
The regenerated schema, README tables and cozyrds openAPISchema follow
from those sources.

Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
2026-08-09 16:17:45 +03:00
Aleksei Sviridkin 9d4bcd8d66 fix(e2e): wire the ghcr.io mirror into the tenant install and node-join flow
Deploy the pull-through mirror from its own install step rather than
folding it into the Talos image-cache step, whose early return on a
missing values key would otherwise skip it silently. Build the tenant
CR spec.talos block from both the OS image cache and the registry
mirror, each included only when its in-sandbox backend is up. On a
node-join failure dump the mirror's state alongside the Talos image
cache, under the same phase gate as its neighbours so it cannot starve
the guest captures of budget. Record the mirror's legs in the step's
budget inventory.

talos_spec_block is covered by a bats suite over all four mirror-up and
mirror-down combinations, splicing the result under spec: and reading
it back with yq so a block indented into the wrong parent fails, and
the install step is pinned to exclude the Cilium policy from its
pre-Cilium apply.

Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
2026-08-09 16:17:44 +03:00
Aleksei Sviridkin 1c6ee7c047 fix(e2e): harden the ghcr.io mirror resolver and bound its diagnostics
resolve_ghcr_mirror_endpoint decides once, for the whole shared
sandbox, whether tenant workers are pointed at the in-sandbox mirror,
so a misclassified transient failure would disable it for the rest of
the run. Cache only stable answers: a genuine API NotFound and a
definite Available=False; never a transient query error, a missing
kubectl, or a failed egress-allow apply. Read readiness from the
Deployment's own Available condition when the rollout watch exits
non-zero, and commit the endpoint only once the egress allow is in
place, surfacing the apply failure reason rather than a generic blip.

The node-join diagnostic counts kubelet-image requests across the whole
access log rather than a fixed tail, and every read it makes is
wall-clock bounded by the caller's COZY_DIAG_READ_TIMEOUT so no single
read can hold the failure path open. The bats suite exercises every
resolver branch and the diagnostic's filtering, with a kubectl stub
that reports errors on stderr as the real binary does and a timeout
mock so the bounded reads stay reachable under test.

Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
2026-08-09 16:17:33 +03:00
Aleksei Sviridkin 3a1292d682 fix(kubernetes): escape tenant values in the worker reconcile heredoc
The worker TalosConfigTemplate is applied from an unquoted
`cat <<EOF | kubectl apply` heredoc, so every free-form value
interpolated into it is subject to shell parameter expansion and
command substitution at Job runtime. Escape backslash, dollar and
backtick on talosVersion, installerRepository, schematicID and the
registryMirrors block so each renders as a literal, and record the
invariant above the data block in both charts: a new free-form field
here must be escaped or render-time pattern-validated, or it reopens
command injection into a host-cluster Pod. Escaping covers every byte
by construction; the chain is a no-op for ordinary values, so the
default render and its content-hash Job name are unchanged.

Tests render both charts and run the heredoc through a real shell with
hostile values, which a rendered-string regex cannot do, and the
default-omits-mirrors assertion selects the Job by kind rather than by
document index so it cannot pass against the wrong document.

Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
2026-08-09 16:17:26 +03:00
Aleksei Sviridkin 02a3fda471 test(e2e): capture host-netns address attribution and a healthy-pod reference for the dataplane collector (#3617)
<!-- 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

Extends `hack/e2e-capture-dataplane.sh`, the diagnostic collector that
runs after an e2e test failure, with the instrumentation issue #3615
asks for to close its address-attribution gap.

Issue #3615 found that a NotReady pod's IP can turn out to be a local
address of the node's host netns instead of reachable through the ovn0
route, which makes kubelet's probes connect to the node's own loopback
and fail forever. The collector already captured `ip addr show ovn0` for
the affected pod's node, but nothing that could name which interface
actually holds a stolen address, since it only ever looked at ovn0.

This adds `ip -o addr show` (every interface) and `ip route show table
local` to the same host netns capture, so the interface actually holding
a local address can be identified on the next occurrence.

The collector also only ever inspected NotReady pods, so a run had no
same-window baseline for what a normal route and conntrack state look
like on the same node. This extracts the existing per-pod capture into a
reusable function and runs it a second time against one Ready, Running
pod on the same node as an in-run healthy-pod control, picked while
explicitly excluding hostNetwork pods (their podIP is the node's own
address, so one would show the exact same local-route fingerprint as the
bug for an unrelated reason). When no eligible Ready pod exists on the
node, that absence is recorded in the artifact instead of left silent.

This is diagnostic tooling only: it changes what gets captured after a
failure, not any test's pass/fail outcome.

### Known limitations

The baseline runs at a reduced scope: the host route and the kernel
conntrack for the healthy pod's address, and nothing else. What it drops
-- cilium CT, the OVN port binding, the OVS `ovn-installed` flag, and
the pod's own Ready conditions and events -- describes how a pod was
programmed, and a pod picked for being Ready and Running is programmed
correctly by definition. That scope is what holds the pod path's worst
case at 579s against the 600s the `cozytest.sh` trap allows; the
remaining 21 seconds are what this path leaves for the LoadBalancer
section that runs after it, not headroom the run has. The other caller's
envelope is 300s, and the worst case does not fit it -- nor did it at
the merge base, where the same walk bounds at 519s. That overrun is
tracked in #3671, and the term-by-term arithmetic is in the `MAX_PODS`
note in the script.

The 579s above assumes the lookup memo exists. When `mktemp` fails the
memo is empty, every per-node pod lookup runs live again, and the same
single-pod case bounds at 667s -- past the 600s envelope as well as the
300s one. At the merge base that memo-less case bounds at 585s, inside
600s, so this change is what moves it out. The degradation is visible
rather than silent: the caller's backstop truncates the capture and the
run reports `data-plane capture INCOMPLETE (exit 124)`.

A baseline can be present and still carry nothing. Every capture at that
scope goes through the node's `kube-ovn-cni` pod, so when that lookup
does not resolve, the reference section is its banner and the scope note
with nothing beneath them -- and, unlike `node-<node>.txt` next to it,
no line saying why. Two different standards of honesty inside one
artifact. The affected-pod path has the same shape today, so this
extends an existing gap rather than opening one, and it is left alone
here.

The per-node baseline is cached under `TMPDIR` for the run. A SIGKILL --
which is what a caller's wall-clock backstop delivers -- skips the
cleanup at the end of the script and leaves that directory behind. The
lookup memo this script already used has the same exposure.

Nodes are visited in the order their affected pods appear in the
cluster-wide list, which the API chooses. Once `MAX_PODS` cuts the walk,
which pods and therefore which nodes got captured is a function of that
ordering and not of anything a reader would pick.


### Screenshots

Not applicable, no UI change.

### 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
test(e2e): capture host netns address attribution and an in-run healthy-pod reference for the dataplane failure collector
```


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

* **New Features**
  * Added diagnostics for node addresses and local routing information.
* Added healthy, same-node reference captures for dataplane
troubleshooting, reused across affected pods.
* Expanded pod diagnostics to include status, events, connection
tracking, routes, network bindings, and interface state.

* **Tests**
* Added coverage for pod selection, retries, caching, exclusions,
incomplete records, unavailable responses, and missing baselines.
  * Recorded diagnostic failure reasons in artifacts.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
2026-08-09 15:39:36 +03:00
Aleksei Sviridkin 18be4ee272 test(e2e): capture host-netns address attribution and a healthy-pod reference
The collector characterises the host->local-pod route for an affected pod
through ovn0 alone, so when a pod IP turns out to be a local address of the
host netns instead, nothing records which interface actually holds it. The
host netns capture now also takes `ip -o addr show` across every interface
and `ip route show table local`, so the owner can be named on the next
occurrence rather than only ruled out on ovn0.

The collector also inspects NotReady pods only, so a run has no same-window
baseline for what a normal route and conntrack state look like on the same
node. The per-pod capture becomes a reusable function and runs a second time
against one Ready, Running, non-hostNetwork pod on that node, into the same
file, so the comparison is in front of whoever opens it. hostNetwork is
excluded because such a pod's podIP IS the node's own address and would show
the same local-route fingerprint as the fault for an entirely ordinary
reason.

That baseline depends on the node and nothing else -- the candidates are the
rows of the one snapshot the affected set was computed from, and a pod
reporting Ready there is by definition not one this run is investigating --
so it is captured once per node and folded into each affected pod's file
from a cache. Several affected pods on one node is what a wedged
host->local-pod datapath looks like rather than an edge case, and capturing
the same healthy pod once per affected pod would spend the caller's envelope
re-asking a question already answered -- the cost the node-global dedup and
the lookup memo beside it exist to avoid.

This adds no read of its own. The cluster-wide pod list gains one column
instead of being asked a second time, and that single read's status is what
lets the artifact separate a node with no eligible pod from a list that
never answered: one line for both would make an unchecked baseline read as
an established absence. The two host-netns additions are best-effort execs,
like the captures they sit beside. The new row filter carries the field
count its jsonpath emits, so a list cut mid-record cannot hand over a
fragment as the pod every reading of the affected one is compared against.

The unit suite pins these on what they are rather than on what they leave
behind. Both host-netns captures are asserted through the kubectl argv the
collector issues, container included, because the section headers are
separate echoes that outlive the exec being removed. The baseline is pinned
from both sides: several affected pods on one node share a single capture,
and an affected pod is served the healthy pod from its own node rather than
from whichever node the walk reached first. The pod list's column order is
asserted against the positions its readers index, because every runtime
test answers that list from a stub written by hand and none of them can
observe the jsonpath at all.

Assisted-By: Claude <noreply@anthropic.com>

Signed-off-by: Aleksei Sviridkin <f@lex.la>
2026-08-09 14:42:19 +03:00
IvanHunters 58910c0edc feat(kubernetes): route tenant worker image pulls through a registry mirror
Add a talos.registryMirrors passthrough to the kubernetes and kubernetes-nodes
charts that renders into the worker TalosConfigTemplate machine.registries.mirrors,
so an operator can point a registry such as ghcr.io at an in-cluster pull-through
mirror and a worker boot no longer depends on live public egress. Default {}
keeps the rendered worker reconcile Job byte-identical to before, content-hash
Job name included. Free-form mirror values are shell-escaped for the unquoted
apply heredoc, covered by an execution-level bats test.

Also add the e2e ghcr-mirror (registry:2 pull-through plus a Cilium egress
policy) and wire it into the kubernetes chainsaw suites, so CI exercises the
mirror path and takes the flaky public ghcr.io kubelet-image pull off live
egress.

Refs #3513 (addresses the egress variant of the tenant-worker node-join
failures; the dominant variant is CI-runner CPU starvation, tracked in the
issue).

Signed-off-by: IvanHunters <xorokhotnikov@gmail.com>
2026-08-09 14:10:16 +03:00
Aleksei Sviridkin dda339f351 fix(e2e): keep a failed image-mirror lookup from pinning the public factory (#3677)
## What this PR does

The tenant Kubernetes e2e suites decide once whether worker image pulls
go to the in-cluster Talos mirror or to the public Image Factory, and
that decision came from a plain `kubectl get deploy talos-image-cache`.
A plain `get` exits non-zero for a connection refused, an Unauthorized
and a real absence alike, so any blip during that one call read as "no
mirror", and the answer went straight into the decision file every later
tenant test reads. One unlucky moment sent every worker in the run to
the public factory over live egress, which is the dependency the mirror
exists to remove, and nothing in the log said so. The run went ahead
against the flaky thing and the failures read as environment noise.

The lookup now asks with `--ignore-not-found -o name`. A real absence
answers exit 0 with no output, which leaves a non-zero exit meaning only
that the question failed, and the helper re-asks a couple of times
before giving up. Only the empty answer turns the suite toward the
public factory. An unanswered lookup warns, falls back for its own
caller alone, and is deliberately not cached, so the next tenant test
asks again instead of inheriting a guess.

The warning has to be backed by something, and kubectl's stderr alone is
not enough. When the wall-clock bound kills an attempt, neither the
bound nor the SIGTERMed kubectl writes a word, and that stall is
precisely the failure the re-ask was written against, so a warning that
sent the reader to kubectl's message would be pointing at an empty
stretch of log exactly when the lookup mattered. The lookup therefore
names the last attempt's exit status before giving up, in the vocabulary
the diagnostic reads already use for 124 and 137, and kubectl's own text
stays on stderr for the cases where there is any.

Each attempt is bounded twice, and the two bounds are deliberately not
equal: `timeout -k 5 40` around `--request-timeout=30s`. kubectl
defaults the request timeout to 0, so a connection that establishes and
then stalls has no deadline at all, and the flag on its own is not a
wall-clock bound either, because kubectl retries discovery several times
before it gives up. Re-asking an unbounded call three times would have
multiplied a hang instead of riding out a blip.

The ten seconds between the two are load-bearing, and I only found out
why by measuring rather than reasoning. Driven against a stub apiserver
the three failure modes are not alike: a refused port answers in under a
second and names the refusal, one that completes TCP but stalls mid-TLS
hits kubectl's own handshake deadline and still produces text despite
being killed, and a blackholed address (SYN dropped, no RST, which is
what a wedged node or a netpol looks like) produces nothing at all until
some deadline expires. That last mode is the one the headroom buys back.
With both bounds at the same value the expiry that arrives first is
always the wall-clock kill, so the blackhole case ends as a bare exit
124; with the wrapper ten seconds above, the request deadline ends the
attempt and kubectl says what happened, and the wrapper stays as the
backstop for a client that overruns its own deadline. Controlled A/B,
same target and same inner bound, only the outer differs: 30 over 30
gives exit 124 and zero bytes, 40 over 30 gives exit 1 at the thirty
second mark and a message naming the cause. The cost is ten more seconds
per attempt on a path where the apiserver is already unreachable, inside
a 50m op.

Worth stating separately, because it is a different question with a
different answer: before this change the inner flag was not doing
anything at all. `--request-timeout=30s` sat under a `timeout` of the
same 30 seconds, and across all three measured modes it never once
fired. The refused case never reached it, the TLS stall was ended by
kubectl's own shorter handshake deadline, and the blackhole was ended by
the wall-clock kill landing at the same instant. So the flag was
decoration, and the comment above it claimed otherwise. That is a
separate defect from the lost diagnostics, and it is fixed by the same
separation rather than by adding anything.

Where `timeout` is not installed the read runs unwrapped instead of
through it. Its exit 127 would otherwise count as an attempt that did
not answer, three times over, so a missing local binary would pin the
whole suite to the public factory while every message blamed the
apiserver.

`talos_image_cache_diagnose` asks the same question on the node-join
failure path, and #3676 landed its own three-way gate there while this
branch was open. The two stay separate rather than sharing this helper.
That one runs under a phase budget where re-asking spends what the
tenant snapshot behind it needs, so it answers on a single attempt; this
one runs on the happy path, where a re-ask costs a suite nothing and
buys back the mirror. This branch leaves that gate as it was merged.

Unit tests put a throwaway `kubectl` and `timeout` on PATH and drive all
three outcomes, the re-ask, the caching rule, the `timeout`-absent path
and the killed-attempt path where nothing but the lookup itself can
explain the failure: `hack/cozytest.sh
hack/talos-image-cache_test.bats`. The stubs are files rather than shell
functions because an external `timeout` binary execs a real file and
would never see a shell override.

The re-ask is a retry loop, which `docs/agents/e2e-testing.md`
restricts. It falls under the exemption there rather than the
prohibition: what is retried is a network read with no product or test
logic under it, and the deterministic steps the rule protects are
untouched.

This relates to #3668 and closes the first door only. The decision is
cached once at the end of the function regardless of which inner check
produced the negative answer, and inside the "mirror is present" branch
the `rollout status` wait and the three reads in
`_talos_image_cache_reachable_from_tenant` still conflate "the mirror
does not work" with "the probe could not be run", with the same
suite-wide effect one layer down. The issue's own status comment says it
stays open after this change, so there is no closing keyword here.

Three weaknesses this PR does not remove, named because each is a real
thing a later reader will hit. The `absent` case in
`resolve_talos_image_factory_url` is reached by `else` fall-through
rather than by matching the token, so a fourth state added to the helper
later would land in the branch that turns the suite toward the public
factory rather than in the safe one; a test pins the `present` token
against exactly that drift, but the shape stays fall-through. The
lookup's own `30` and `5` are literals while the diagnostic reads next
door run off a validated, overridable pair, and that is deliberate
rather than an oversight: those knobs are named and documented for the
dump budget, so wiring the lookup to them would let a test that turns
the dumps down silently change which image factory a whole suite pulls
from. And `_TALOS_IMAGE_CACHE_QUERY_TRIES` is the one knob in the file
without that validation. A non-numeric value spins the loop forever
rather than failing: the comparison sits in an `if` condition, so `set
-e` is exempt, the branch is simply never taken, and the job burns to
its ceiling with no output. Nothing outside the tests sets it, which is
why it is here and not in the diff. Worth recording for whoever does fix
it: the obvious remedy, reusing this file's own
`_talos_image_cache_seconds`, does not work where the knob is assigned,
because the validator is defined a hundred lines further down and the
assignment runs at source time. The call resolves to nothing, the
substitution yields an empty string, and an empty bound spins the same
loop. A real fix has to move the definition up or validate inside the
function at call time.

### Screenshots

Not a UI change.

### Downstream repositories

Walked the trigger map against the diff. It touches
`hack/e2e-chainsaw/_lib/talos-image-cache.sh` and
`hack/talos-image-cache_test.bats` and nothing else: no file under
`hack/` is moved or renamed, no make target changes behaviour, and no
package, chart, CRD or values key is involved. The `cozystack/ccp` entry
is the closest one and it does not fire.

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

### Release note

```release-note
fix(e2e): a single failed apiserver lookup no longer sends a whole e2e run to the public Talos Image Factory
```
2026-08-09 14:06:12 +03:00
Aleksei Sviridkin 5400f165e3 fix(e2e): keep a failed mirror lookup from pinning the public factory
resolve_talos_image_factory_url asked whether the in-cluster Talos image
mirror exists with a plain `kubectl get deploy`, where a connection
refused, an Unauthorized and a real absence all arrive as the same
non-zero exit. It then cached that reading for the whole suite, so one
unlucky moment sent every tenant worker to the public Image Factory over
live egress -- the dependency the mirror exists to remove -- while the
run still looked ordinary.

Ask with --ignore-not-found -o name instead: a real absence answers exit
0 with no output, which leaves a non-zero exit meaning only that the
question failed, and re-ask a couple of times before giving up. Only the
empty answer turns the suite toward the public factory. An unanswered
lookup warns, falls back for its own caller alone, and is not written to
the decision file, so the next tenant test asks again rather than
inheriting a guess.

Each attempt is bounded twice, and the two bounds are deliberately not
equal. kubectl defaults --request-timeout to 0, so a connection that
establishes and then stalls has no deadline of its own; the flag alone is
not a wall-clock bound either, because kubectl retries discovery several
times before giving up. Measured against a stub apiserver, the three ways
this call fails behave differently: a refused port answers in under a
second and names the refusal, one that stalls mid-TLS gets kubectl's own
handshake deadline and still produces text, and a blackholed address
produces nothing until some deadline expires. With both bounds at the
same value the expiry that arrives first is always the wall-clock kill,
so --request-timeout can never fire and that last case stays silent. The
wrapper therefore sits ten seconds above it: the request deadline ends
the attempt and kubectl says why, and the wrapper remains the backstop
for a client that overruns its own deadline.

Where timeout is not installed the read runs unwrapped rather than
through it: its exit 127 would otherwise count as an attempt that did not
answer, three times over, and a missing local binary would pin the whole
suite to the public factory while every message blamed the apiserver.

kubectl's stderr is left alone, because it is the only thing separating
refused from unauthorized from stalled. It is still not guaranteed, since
a kill can land before the client has said anything, so the lookup also
names the last attempt's exit status before giving up, in the vocabulary
the diagnostic reads use for 124 and 137.

The diagnostic path asks the same question and keeps its own answer to
it. That one gives up after a single attempt, under a phase budget where
re-asking spends what the snapshot behind it needs; this one runs on the
happy path, where a re-ask costs a suite nothing and buys it the mirror.

Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
2026-08-09 13:31:55 +03:00
Aleksei Sviridkin 04494555c9 test(e2e): pin that an unanswered mirror lookup is not a mirror absence
The Talos image factory decision asks the apiserver once whether the
in-cluster mirror Deployment exists, reads any non-zero exit as an
absence, and caches that answer for the whole suite. A refused, throttled
or timed-out lookup therefore sends every tenant worker in the run to the
public Image Factory over live egress -- the dependency the mirror exists
to remove -- and the run reads as ordinary environment noise rather than
as a decision that was never actually made.

Route that question through one helper and pin what the helper owes its
caller: the three outcomes it must tell apart, the flags that make the
three-way answer possible at all, a deadline on every attempt, the
caching rule -- a real absence is resolved once and cached, an unanswered
lookup falls back for its own caller alone -- and that a missing local
timeout binary leaves the lookup asking the apiserver rather than
answering unknown three times over.

Two of those need saying carefully, because both concern what the caller
is told when the lookup fails. A killed attempt gets its own case, driven
by a wrapper that exits without running kubectl at all, so the lookup has
to account for why it gave up rather than relay someone else's words:
pinning the reason on a kubectl that failed loudly, as the sibling case
does, pins it only where it was never in doubt. And the wall-clock bound
is pinned as strictly greater than the request deadline beside it, which
is the property that lets kubectl reach its own deadline and say what
happened; level with it, the kill always lands first.

The helper is a placeholder in this commit and claims present
unconditionally, so the cases that need the distinction fail.

The stubs are files on PATH rather than shell functions because the
lookup runs under an external timeout binary, which execs a real file and
would never see a shell override. xtrace stays off for the rest of a test
that captures stderr: turned back on, a failing assertion wedges bats
instead of reporting it.

Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
2026-08-09 13:31:54 +03:00
Aleksei Sviridkin 048088bd29 fix(e2e): bound the tenant failure diagnostics per read and as a phase (#3676)
<!-- 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

The node-join failure diagnostics in
`hack/e2e-chainsaw/_lib/run-kubernetes.sh` issued about a dozen
`kubectl` reads with neither an outer wall-clock bound nor
`--request-timeout`, starting with a `describe nodes` through the tenant
kubeconfig. That read targets the component least likely to answer in a
node-join failure. An unbounded read there does not lose only itself: it
holds the Chainsaw op until the op is killed, so everything scheduled
after it is lost rather than truncated, the tenant crust-gather snapshot
above all.

Every read now carries the per-call bound the newer collectors on this
path already use. A read that fails or is cut off says so instead of
passing in silence, and says only what was observed, because a read that
did not finish is silence about the cluster rather than a finding about
it. The CDI importer walk gains a cap, and its listing is read on its
own so a listing that never answered cannot read as a namespace with no
importer in it.

Per-read bounds alone do not end the phase, and the collectors here can
together spend more than what is left of the op by the time they run. So
the phase is bounded as a phase: once its budget is spent, the
collectors that have not started are declined out loud. That is not a
worst case and is not written as one, but it stops the snapshot being
queued behind work begun past the deadline, which is how the snapshot
was being lost. The op ceiling is unchanged, since a larger ceiling
multiplied by a sum that grows with every collector added is the same
problem one collector later.

Because what runs last is what gets declined, the block is reordered so
the cheap reads come first. The CSR list and the signer log are the
discriminator for the failure mode a follow-up fix has to target, and
behind two minute-scale collectors they were exactly what a tight run
would drop.

Adjacent work on the same failure path, all of it in service of the same
requirement:

- the sibling scheduling-gate branch ends in the same exit and depends
on the same snapshot, so its two reads are bounded too;
- `talos_image_cache_diagnose` gets its reads bounded and its gate gets
`--ignore-not-found`, because `kubectl get` exits 1 for a refused
connection or Unauthorized as readily as for NotFound, so a call that
never reached the apiserver was being announced as a mirror that was
never deployed;
- the four numeric knobs are validated rather than only documented,
since a suffix or a leading zero on the budget reached the arithmetic in
the block's first statement and unwound the whole function before its
own headline, leaving a failing run with no diagnostics at all.

Comments carry invariants rather than arithmetic. A comment that states
a sum has to be kept true forever in every copy of it, and any edit to
the code or the order falsifies one of them. The numbers live in the
code as values and in the guard that pins them.

Relates to #3656.

### Known gaps, deliberately not closed here

Some collectors on this path carry no wall-clock bound at all. The
image-cache reachability re-probe makes seven unbounded
management-cluster calls, and the guest-Talos capture walks its VMI rows
uncapped with applies and waits bounded only by `--request-timeout`,
which bounds one HTTP request rather than a client retrying against a
wedged apiserver. The phase budget stops them being started late; it
does not stop one running long once admitted. They are pre-existing and
tracked in #3666. Bounding the re-probe in particular would change when
the same function gives up on the happy path, and so which image factory
tenant workers use, which is not a diagnostic's decision to make.

Named improvements left for later, none of them regressions introduced
here:

- the phase deadline is stamped at block entry rather than against what
is left of the op, so a long bringup still narrows the room the
derivation assumes;
- `bringup` and `largest` are literals in the guard, so a collector
heavier than today's will not move them, and the margin is small by
design;
- the integer-knob validator now exists in four places in this tree, and
a shared `_lib` file sourced ahead of both libraries would collapse two
of them;
- the per-request bound equals the wall-clock bound, so both deadlines
fire together and which of the two notes appears is a race;
- the ordered list in the chainsaw comments omits the ungated wedge
check, which runs first;
- section letters no longer sort after the reorder, and the
comment-to-code ratio in the touched block is high.

### Testing

`hack/run-kubernetes-node-join_test.bats` is new: 22 cases driving the
block with a `kubectl` stub that hangs, refuses and answers in part, and
a `timeout` stub that fails any read not wrapped in the bounding idiom.
It pins that no read escapes a bound, that a cut-off read is named
rather than reported as an empty cluster, that the walk cap reports both
counts, that a failed listing is not reported as no importers, that the
phase declines the rest out loud, that a malformed knob falls back and
says so, that the block returns zero so the caller keeps its `exit 1`,
and the source order that keeps the discriminating reads ahead of the
expensive collectors. Every one was checked by mutation against the
unfixed shape.


### 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): bound every read in the tenant Kubernetes node-join failure diagnostics and give the collection phase its own wall-clock budget, so a wedged apiserver can no longer hold the Chainsaw op until it is killed and take the tenant crust-gather snapshot with it
```


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

## Summary by CodeRabbit

- **Bug Fixes**
- Improved Kubernetes node-join and scheduling failure diagnostics with
bounded collection times, clearer timeout and failure reporting, and
more reliable preservation of critical snapshots.
- Added safer handling for unavailable diagnostic data, missing tools,
failed commands, and undeployed image caches.
- Added configurable and validated timeout controls for image-cache and
failure diagnostics.

- **Documentation**
- Clarified diagnostic budgets, collection ordering, and behavior when
available time is exhausted.

- **Tests**
- Added comprehensive coverage for diagnostic timeouts, ordering,
validation, failure handling, and bounded reads.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
2026-08-09 12:33:27 +03:00