Skip to content

fix(e2e): capture Talos logs for tenant worker join failures - #3548

Merged
IvanHunters merged 6 commits into
mainfrom
fix/e2e-tenant-bringup-diagnostics
Aug 7, 2026
Merged

fix(e2e): capture Talos logs for tenant worker join failures#3548
IvanHunters merged 6 commits into
mainfrom
fix/e2e-tenant-bringup-diagnostics

Conversation

@myasnikovdaniil

@myasnikovdaniil myasnikovdaniil commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

What this PR does

Addresses #3513.

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

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

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

Screenshots

Not applicable; no UI changes.

Testing

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

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

Downstream repositories

Release note

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

Summary by CodeRabbit

  • Bug Fixes

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

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

    • Ensured the required diagnostic image is preloaded on nodes.

Tenant Kubernetes worker join failures currently stop at host-side VMI
and CSR state because the worker-only topology does not materialize a
tenant talosconfig.

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

Assisted-By: GPT-5 <noreply@openai.com>
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
@coderabbitai

coderabbitai Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

Tenant worker-join failures now collect Talos dmesg and kubelet logs through temporary credentials and a hardened helper Pod. The workflow preserves partial failures, cleans up diagnostic resources, extends test timeouts, and pre-pulls the diagnostic image.

Changes

Tenant Talos diagnostics

Layer / File(s) Summary
Worker discovery and temporary access
hack/e2e-chainsaw/_lib/run-kubernetes.sh, hack/run-kubernetes-talos-diagnostics_test.bats
Worker VMI rows retain missing-IP entries. The workflow creates a short-lived reader Certificate and restricted temporary Talosconfig. Tests cover manifests, cleanup, readiness, credentials, and file permissions.
Hardened Pod and bounded capture
hack/e2e-chainsaw/_lib/run-kubernetes.sh, hack/run-kubernetes-talos-diagnostics_test.bats
A non-privileged Pod runs bounded Talos commands with restricted capabilities and filesystem access. Per-worker dmesg and kubelet output remains recorded after timeouts.
Join-failure orchestration and test execution
hack/e2e-chainsaw/_lib/run-kubernetes.sh, hack/e2e-chainsaw/kubernetes-latest/chainsaw-test.yaml, hack/e2e-chainsaw/kubernetes-previous/chainsaw-test.yaml, hack/e2e-install-cozystack.bats, hack/run-kubernetes-talos-diagnostics_test.bats
Node-join failures invoke diagnostics for reachable workers and preserve setup or capture errors. Cleanup removes diagnostic resources. Chainsaw timeouts increase to 50 minutes, and the diagnostic image is pre-pulled.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant NodeJoinTest
  participant KubernetesAPI
  participant TalosDiagnosticsPod
  participant WorkerTalos
  NodeJoinTest->>KubernetesAPI: discover worker VMIs and IPs
  NodeJoinTest->>KubernetesAPI: create reader Certificate and Secret
  NodeJoinTest->>TalosDiagnosticsPod: create hardened Pod with Talosconfig
  TalosDiagnosticsPod->>WorkerTalos: collect dmesg and kubelet logs
  TalosDiagnosticsPod-->>NodeJoinTest: return output and exit status
  NodeJoinTest->>KubernetesAPI: delete diagnostic Certificate, Pod, and Secret
Loading

Possibly related issues

Possibly related PRs

  • cozystack/cozystack#3315 — Both changes modify tenant diagnostic cleanup and teardown ordering in run-kubernetes.sh.

Suggested labels: area/kubernetes

Suggested reviewers: lexfrei, kvaps

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: capturing Talos logs for tenant worker join failures.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/e2e-tenant-bringup-diagnostics

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions github-actions Bot added area/testing Issues or PRs related to testing (e2e, bats, unit tests) kind/bug Categorizes issue or PR as related to a bug size/XL This PR changes 500-999 lines, ignoring generated files labels Aug 5, 2026
Use Bash and awk assertions in the Talos diagnostics regression tests so
they run in the standard E2E sandbox without optional search utilities.

Assisted-By: GPT-5 <noreply@openai.com>
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NOT LGTM. The reader Certificate is rejected at admission, so the capture never runs.

hack/e2e-chainsaw/_lib/run-kubernetes.sh:277-279 sets subject.organizations: [os:reader] and nothing else. cert-manager wants at least one of commonName / dnsNames / uris / emailAddresses / ipAddresses / otherNames, and it does not look at subject at all. From ValidateCertificateSpec in v1.20.2, which is what we ship (packages/system/cert-manager/charts/cert-manager/Chart.yaml:9):

if len(commonName) == 0 &&
	len(crt.DNSNames) == 0 &&
	len(crt.URIs) == 0 &&
	len(crt.EmailAddresses) == 0 &&
	len(crt.IPAddresses) == 0 &&
	len(crt.OtherNames) == 0 {
	el = append(el, field.Invalid(fldPath, "", "at least one of commonName (from the commonName field or from a literalSubject), dnsNames, emailSANs, ipAddresses, otherNames, or uriSANs must be set"))
}

So kubectl apply fails, cozy_prepare_tenant_talosconfig returns 1 at :313, and every node-join failure prints failed to create short-lived tenant Talos reader config with zero dmesg and zero kubelet output. Adding a commonName fixes it. Talos takes the role from the Organization, so the value does not matter.

The unit suite can't catch this. kubectl is a mock that returns 0 for apply (hack/run-kubernetes-talos-diagnostics_test.bats:18-21), so the tests check the rendered YAML and never that an API server accepts it. No CI leg reaches this code either, it needs a real node-join failure.

Second one. timeout: 40m on the script op is unchanged (hack/e2e-chainsaw/kubernetes-latest/chainsaw-test.yaml:22, hack/e2e-chainsaw/kubernetes-previous/chainsaw-test.yaml:16), but the new block adds about 4m45s of bounded budget for two workers: 10s + 10s stale deletes, 30s issuance, 10s pod delete, 60s pod readiness, 60s + 20s streaming, 40s per VMI. That lands after the 18m node-join wait, before (a2) and (c), and before the EXIT-trap tenant crust-gather at :235 which can take another 390s. A kill there is SIGKILL on the process group, so the tenant snapshot goes with it. Raise the 40m or show the measured headroom.

Related, same budget: cozy_capture_tenant_talos:474 mints the Certificate and starts the Pod before checking whether any VMI has an IP. In the mode where no worker booted, which is what the empty-IP branch at :516 is for, that burns ~3m30s to produce only capture-error.log.

Nits:

  • ubuntu:24.04 (:374) is not prepulled and is not used anywhere else in the tree, so the 60s readiness wait has to cover a cold Docker Hub pull on an already degraded cluster. talos_image_cache_diagnose on the same failure path reuses alpine/k8s, which is on every node already (hack/e2e-talos-image-cache.yaml:81). If the talosctl build is statically linked that image works here too, otherwise prepull the ubuntu digest.
  • stat -c '%a' (hack/run-kubernetes-talos-diagnostics_test.bats:123) is GNU-only and the only one in the tree. Linux CI is fine, on a BSD stat the suite goes red for a reason that has nothing to do with the code under test.
  • #3444 is open and touches the same file and .chainsaw.yaml. Worth sequencing the two.

The rest holds up and I checked it instead of assuming. os:reader is exactly enough, Talos maps Dmesg and Logs to role.Reader, and no admin credential gets materialised anywhere. <release>-talos-ca is the machine CA the workers carry as machine.ca.crt. Workers are on pod-bridge (packages/apps/kubernetes/templates/cluster.yaml:174), so the VMI IP is routable and server-side TLS stays verified, which an LB or port-forward address would have broken. The Pod passes restricted PSA. Both crust-gather calls use --exclude-kind Secret and the workdir sits outside the report tree. The () function body gets its own trap table, so _tenant_snapshot_on_fail survives. The original failure, the 18m budget and the success path are untouched, and the test-impact claim holds: hack/select-e2e.sh:86 escalates _lib/* to the full suite.

Ran locally: 8/8 new tests green, drain suite still 7/7, bash -n clean, shellcheck --severity=warning only the pre-existing SC2034 at :531.

Fix admission, capture setup, timeout headroom, and portability.

Assisted-By: GPT-5 <noreply@openai.com>
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NOT LGTM. Both blockers are closed, but the new test helper is a bashism and the unit job is already red on it.

hack/run-kubernetes-talos-diagnostics_test.bats:84 reads the file with case "$(<"${file}")". hack/cozytest.sh is #!/bin/sh and Makefile:170 runs it directly, so on Ubuntu the interpreter is dash, where $(<file) expands to an empty string instead of the file contents. Every assert_file_contains fails after that.

dash -c 'x=$(</tmp/f); echo "[$x]"'   ->  []
bash -c 'x=$(</tmp/f); echo "[$x]"'   ->  [hello-needle]

The suite under /bin/dash ./hack/cozytest.sh stops at "reader Certificate is short-lived and never embeds credential data" with expected ... to contain: name: kubernetes-test-latest-version-e2e-talos-reader. The unit job on 60e44b2 fails at that same test with that same message and make: *** [Makefile:165: bats-unit-tests] Error 1. $(cat "${file}") is the whole fix, line 84 is the only occurrence in the tree. Worth turning around quickly: a red unit job also skips the e2e gate, so the diagnostics path stays unexercised until this is green.

Everything else is closed, and I checked each one rather than looking for the edit.

commonName is in, subject.organizations: [os:reader] is untouched, and the CN comes out 47 and 49 characters for the two suites, nowhere near the 64 limit. ValidateCertificateSpec passes now.

Budget is 50m in both suites, and the arithmetic in the comment is tighter than what I posted: 5m10s counts the -k 5 kill grace on every bounded call, which my 4m45s missed. The yq test pins 50m, so the number is guarded by a check now instead of a comment. One thing to watch, and it is not an objection to this change: two kubernetes suites at 50m move the worst case from 80m to 100m inside the 180-minute job cap that the .chainsaw.yaml preamble warns about. Raising the containing timeout is exactly what that preamble prescribes, so the fix is right, the cap is just closer than it was.

The image moved to alpine/k8s on the same digest the cache manifest already pins, it went into the prepull list, and a test asserts the pod digest also appears in hack/e2e-install-cozystack.bats, so the two cannot drift apart silently. The static-linking premise holds: upstream Talos builds with CGO_ENABLED ?= 0, and even the race build passes -extldflags '-static'.

stat -c is gone and LC_ALL=C ls -ld with a case on -rw------- reads the same on both stats. Dropping the rg dependency was a good call on its own, it just needs line 84 fixed to land.

Skipping Certificate issuance and the helper Pod when no VMI reports an IP is in, with a test that asserts no apply and no exec happen on that path.

Ran locally: 10/10 green under bash, both through the repo runner and plain bats, and the same suite under dash fails as described above.

assert_file_contains used $(<file), a bash-only construct. cozytest.sh is
run under /bin/sh (dash on the Ubuntu CI runner via Makefile), where
$(<file) expands to an empty string instead of the file contents, so every
assertion after it failed and reddened the unit-tests job, which in turn
skips the e2e gate and leaves the diagnostics path unexercised. Read the
file with $(cat ...), which is POSIX and behaves identically under bash and
dash.

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

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

Signed-off-by: IvanHunters <xorokhotnikov@gmail.com>
The describe/get-events/get-certificaterequests calls that run when the
reader Certificate or the diagnostics Pod misses its readiness wait were
unbounded, yet those branches fire precisely when the management apiserver is
wedged. A stalled forensic call there can burn minutes of the 50m step budget
before the pre-existing (a2)/(c) diagnostics and the EXIT-trap tenant
snapshot run. Add --request-timeout=30s to them, matching the happy-path
capture calls.

Signed-off-by: IvanHunters <xorokhotnikov@gmail.com>
@IvanHunters
IvanHunters marked this pull request as ready for review August 5, 2026 12:26

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
hack/run-kubernetes-talos-diagnostics_test.bats (1)

247-251: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Document why the test creates $tmp/talosctl and changes directory.

cozy_prepare_tenant_talos_diagnostics_pod runs talosctl_bin=$(command -v talosctl). In this file talosctl is a shell function, so command -v prints the bare name talosctl instead of an absolute path. The later <"${talosctl_bin}" redirect therefore resolves against the current directory, which is why lines 247-249 create a same-named regular file and cd into $tmp. This coupling is invisible to a future reader and breaks silently if the mock changes.

Add a short comment recording the mechanism.

♻️ Proposed comment
+  # `talosctl` is mocked as a shell function, so `command -v talosctl` inside
+  # cozy_prepare_tenant_talos_diagnostics_pod yields the bare name. Its
+  # `<"${talosctl_bin}"` redirect then resolves relative to the cwd, so provide
+  # a readable stand-in file and run from $tmp.
   printf '%s\n' '#!/bin/sh' >"$tmp/talosctl"
   chmod 755 "$tmp/talosctl"
   cd "$tmp"
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@hack/run-kubernetes-talos-diagnostics_test.bats` around lines 247 - 251, Add
a short explanatory comment immediately before the temporary talosctl creation
and directory change, referencing cozy_prepare_tenant_talos_diagnostics_pod,
command -v resolving the shell-function mock to the bare name talosctl, and the
redirect requiring that same-named file in the current directory.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@hack/run-kubernetes-talos-diagnostics_test.bats`:
- Around line 247-251: Add a short explanatory comment immediately before the
temporary talosctl creation and directory change, referencing
cozy_prepare_tenant_talos_diagnostics_pod, command -v resolving the
shell-function mock to the bare name talosctl, and the redirect requiring that
same-named file in the current directory.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: e576d0e9-8b07-4ca2-a3fb-ad9a509f7737

📥 Commits

Reviewing files that changed from the base of the PR and between ee26a50 and 0612a53.

📒 Files selected for processing (5)
  • hack/e2e-chainsaw/_lib/run-kubernetes.sh
  • hack/e2e-chainsaw/kubernetes-latest/chainsaw-test.yaml
  • hack/e2e-chainsaw/kubernetes-previous/chainsaw-test.yaml
  • hack/e2e-install-cozystack.bats
  • hack/run-kubernetes-talos-diagnostics_test.bats

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

$(< is gone from the tree, and the suite now passes under dash, which is the shell it failed on before: 10/10 there and under bash. The unit job on cfbfd77 is green.

Bounding the previously open-ended calls with --request-timeout=30s is a good addition on its own. It does leave the worst case in the timeout: 50m comment stale. Counted the same way that comment counts, the bounded setup is now around 9m20s rather than 5m10s, mostly from kubectl_wait_retry (three 30s attempts plus two 5s sleeps) and six calls that can each take 30s. 50m still covers 18m + 9m20s + 6m30s with room left for the bring-up waits, so nothing needs raising, but the number in the comment should be corrected: the yq test pins 50m, not the arithmetic behind it, so a future edit will be sized against a figure that is off by nearly half.

One thing for whoever touches this next. The kubectl_wait_retry call added here sits under if !, which suppresses errexit for the whole function body, so the retry allow-list is reachable and the helper does what it says. The other calls to that helper in this file are plain commands, where _out=$(kubectl wait ...) kills the shell on the assignment line under set -e and the retry never runs. Not this PR's problem, and I am not asking for it here, but the two forms behave differently and only one of them retries.

IvanHunters added a commit that referenced this pull request Aug 6, 2026
Tenant worker Talos nodes pull ghcr.io/siderolabs/kubelet directly; the CI
runner's public ghcr.io egress is flaky and the pull times out (TLS handshake
timeout), so the kubelet service never starts and no tenant node joins within
the node-join budget (#3513, in-guest evidence from #3548).

Run an in-sandbox registry:2 pull-through cache for ghcr.io and point tenant
workers at it via the chart's talos.registryMirrors knob, mirroring the
talos-image-cache pattern that already decouples the flaky Talos OS-image
fetch. A CiliumClusterwideNetworkPolicy punches a tightly-scoped egress hole
for the worker VM (kubevirt.io: virt-launcher) pods to reach the mirror; the
run-kubernetes helper emits registryMirrors only when the mirror is up, so
workers fall back to direct pulls and the mirror can only help.

NOT locally validatable (no live cluster): see the draft PR body for the CI
assumptions to confirm (registry image availability, Talos http-mirror
handling, worker egress identity) and the kubernetes-previous caveat.

Refs #3513

Signed-off-by: IvanHunters <xorokhotnikov@gmail.com>
Aleksei Sviridkin (lexfrei) added a commit that referenced this pull request Aug 6, 2026
…#3579)

## What this PR does

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

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

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

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

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

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

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

Observed in #3577.

### Screenshots

Not a UI change.

### Downstream repositories

- [x] No downstream repository is affected by this change
- [ ] [cozystack/website](https://github.com/cozystack/website) -
follow-up:
- [ ]
[cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack)
- follow-up:
- [ ]
[cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack)
- follow-up:
- [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up:
- [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up:
- [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) -
follow-up:
- [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) -
follow-up:
- [ ]
[cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server)
- follow-up:
- [ ]
[cozystack/external-apps-example](https://github.com/cozystack/external-apps-example)
- follow-up:
- [ ] [cozystack/examples](https://github.com/cozystack/examples) -
follow-up:

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

### Release note

```release-note
NONE
```


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

## Summary by CodeRabbit

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

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

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
@IvanHunters
IvanHunters merged commit 318be93 into main Aug 7, 2026
22 of 24 checks passed
@IvanHunters
IvanHunters deleted the fix/e2e-tenant-bringup-diagnostics branch August 7, 2026 07:40
IvanHunters added a commit that referenced this pull request Aug 7, 2026
Tenant worker Talos nodes pull ghcr.io/siderolabs/kubelet directly; the CI
runner's public ghcr.io egress is flaky and the pull times out (TLS handshake
timeout), so the kubelet service never starts and no tenant node joins within
the node-join budget (#3513, in-guest evidence from #3548).

Run an in-sandbox registry:2 pull-through cache for ghcr.io and point tenant
workers at it via the chart's talos.registryMirrors knob, mirroring the
talos-image-cache pattern that already decouples the flaky Talos OS-image
fetch. A CiliumClusterwideNetworkPolicy punches a tightly-scoped egress hole
for the worker VM (kubevirt.io: virt-launcher) pods to reach the mirror; the
run-kubernetes helper emits registryMirrors only when the mirror is up, so
workers fall back to direct pulls and the mirror can only help.

NOT locally validatable (no live cluster): see the draft PR body for the CI
assumptions to confirm (registry image availability, Talos http-mirror
handling, worker egress identity) and the kubernetes-previous caveat.

Refs #3513

Signed-off-by: IvanHunters <xorokhotnikov@gmail.com>
IvanHunters added a commit that referenced this pull request Aug 7, 2026
Tenant worker Talos nodes pull ghcr.io/siderolabs/kubelet directly; the CI
runner's public ghcr.io egress is flaky and the pull times out (TLS handshake
timeout), so the kubelet service never starts and no tenant node joins within
the node-join budget (#3513, in-guest evidence from #3548).

Run an in-sandbox registry:2 pull-through cache for ghcr.io and point tenant
workers at it via the chart's talos.registryMirrors knob, mirroring the
talos-image-cache pattern that already decouples the flaky Talos OS-image
fetch. A CiliumClusterwideNetworkPolicy punches a tightly-scoped egress hole
for the worker VM (kubevirt.io: virt-launcher) pods to reach the mirror; the
run-kubernetes helper emits registryMirrors only when the mirror is up, so
workers fall back to direct pulls and the mirror can only help.

NOT locally validatable (no live cluster): see the draft PR body for the CI
assumptions to confirm (registry image availability, Talos http-mirror
handling, worker egress identity) and the kubernetes-previous caveat.

Refs #3513

Signed-off-by: IvanHunters <xorokhotnikov@gmail.com>
IvanHunters added a commit that referenced this pull request Aug 7, 2026
…ostics_test.bats

#3548 added hack/run-kubernetes-talos-diagnostics_test.bats carrying 8 EXIT-trap
cleanups but did not register it in the frozen set of the cozyreport.bats
EXIT-trap guard, so the guard's found set no longer matches frozen and the
'Unit & controller tests' job fails on main and on every branch rebased onto it.
#3584 fixed the same class for multus-install-cni-plugins.bats but not this file.
Add the count, exactly as that guard's own comment prescribes for a new file
that arrives carrying traps. Value 8 confirmed by the guard's own fold_source +
grep and by the failing CI run.

Signed-off-by: IvanHunters <xorokhotnikov@gmail.com>
Aleksei Sviridkin (lexfrei) added a commit that referenced this pull request Aug 7, 2026
The ban on EXIT-trap cleanup in hack/*.bats was enforced by two lists
inside hack/cozyreport.bats: one naming the files known to be clean, and
one freezing an exact trap count for each file that was not. Both had to
be edited from whatever change moved them, and that is where the guard
failed, and it never worked even once. The inventory landed in #3567 at
15:25:20; #3195 had landed multus-install-cni-plugins.bats carrying
twelve traps forty-three seconds earlier, so replaying the old guard
against the tree at the commit that introduced it already gives
found != frozen. Main stayed red for about twenty-two and three quarter
hours. The first repair, #3584, landed already red the next morning
because #3548 had brought in run-kubernetes-talos-diagnostics_test.bats
with eight traps two minutes ahead of it, so that fix bought no green
time at all; green came only with the second repair. Neither pair of
PRs shared a line, and each was green against its own base.

A change adding a trap to its own file had to edit a string in a suite
it otherwise never touches, so two
changes sharing no line still invalidated each other: each stayed green
against its own base, git merged both cleanly, and the guard went red
only once the second landed.
A file arriving with traps hit the same wall from the other side -- the
inventory did notice it, since the string it compared was built by
scanning the directory, but absorbing it meant an edit in a file nothing
in the author's diff pointed at.

Replace both lists with a declaration each file makes about itself: one
"# EXIT-TRAP DEBT: N" comment, exact rather than a ceiling. A file
carrying none must install no EXIT trap, which covers the converted
files, the files that never had one, and every file added later.
Growing or shedding a trap now fails in the file the change already
edits, so two changes that disagree about a count collide textually
instead of silently.

That collision is a steady-state property, and this change's own arrival
is the exception. A branch forked before the declaration existed has no
line to disagree with: it converts traps in its own file, merges clean,
and the count goes wrong only once both sides are on main -- verified
against a sibling branch that takes select-e2e_test.bats from fifteen
traps to zero. What the move buys even then is that the red names a file
that branch already edited, the repair is one line inside it, and
rebasing before merge catches it on the branch's own CI. None of those
three held against the inventory.

The declaration is read from the leading comment block, not from
anywhere in the file. A .bats file is shell that writes shell, so the
same line turns up inside a heredoc, a fixture writer or an
expected-output string, where it is data belonging to one test. Reading
it there as a statement about the whole file would let an unrelated
fixture excuse a real trap, and would do it silently, since nothing in
that test's own diff looks like a declaration. A comment block is the
region with no interior: stopping instead at the first @test would still
read a line out of a helper's heredoc.

Not every counted handler is debt. A trap inside an explicit subshell
does not replace the bats binary's own, so a test failing inside
`( ... )` still prints its `not ok`. hack/e2e-test-openapi.bats kills a
backgrounded kubectl proxy that way, and moving the kill to the end of
the body would leak a process holding a fixed port. Its declaration
records that rather than scheduling a conversion, and because the
ratchet is exact, removing the trap fails too -- the count protects the
construct. What the count cannot do is tell the two apart: substituting
a test-level trap for the subshell one keeps the total at 1 and stays
green, which the header states rather than leaves to be found.

Counting bounds the keyword and the signal the same way, at any
character that cannot be part of an identifier, and matches the signal
in either case. Whitespace on the right missed `trap ... EXIT; cd
"$tmp"`; whitespace on the left missed `tmp=$(mktemp -d);trap ... EXIT`
and `(trap ... EXIT; true)`; upper case missed `trap ... exit`, which
bash and dash both install. All are real handlers that scored zero, and
the left boundary matters most, since the inventory being replaced had
none and caught the semicolon form. A bare word boundary is not enough
either way: `bootstrap ` ends in `trap `, and it must keep scoring
nothing. A quoted signal counts for the same reason as the rest.

Two handlers sharing one line are reported rather than counted, since
the count is a count of lines
and the second would otherwise arrive without moving the total;
splitting the line properly needs a shell parser, a semicolon inside a
handler's own action not being a separator, and guessing wrong
undercounts -- the one direction a ratchet cannot afford.

The scan recurses, so hack/e2e-apps/*.bats is covered rather than
sitting one directory below the guard that claims the tree. It reads
.bats and nothing else, so a handler arriving through a sourced .sh
stays outside it -- hack/e2e-chainsaw/_lib/run-kubernetes.sh installs
two, each benign for its own reason rather than by design: one sits in
a function declared with `(` and so runs in a subshell, the other in a
brace function no @test calls. That boundary is stated in the header
rather than papered over.

The guard moves to hack/bats-no-exit-trap.bats: its subject is every
unit suite under hack/, not the report collector it grew up in. Its
fixture helpers assemble the trap keyword and the signal from separate
arguments, because the guard scans its own source and a fixture written
as a literal would be counted as a real trap in it.

Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
Aleksei Sviridkin (lexfrei) added a commit that referenced this pull request Aug 8, 2026
…#3622)

<!-- 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 ban on EXIT-trap cleanup in `hack/*.bats` was enforced by two lists
inside `hack/cozyreport.bats`: one naming the files known to be clean,
one freezing an exact trap count for each file that was not. Both had to
be edited from whatever change moved them, and that is where the guard
kept failing.

It is worth being exact about how badly, because the history is sharper
than "it went stale a few times". **The inventory was never correct on
main for a single commit.** It landed in #3567 at 15:25:20. #3195 had
landed `multus-install-cni-plugins.bats` carrying twelve traps at
15:24:37, forty-three seconds earlier. Replaying the old guard's own
logic against the tree at the very commit that introduced it already
gives `found != frozen`. Main then stayed red for roughly twenty-two and
three quarter hours. The first repair, #3584, landed *already red* the
next morning at 10:42:43, because #3548 had brought in
`run-kubernetes-talos-diagnostics_test.bats` with eight traps at
10:40:03, under three minutes ahead of it. That fix was correct and
bought zero green time. Green arrived only with the second repair,
#3602.

Neither pair of PRs shared a line, and each was green against its own
base. That is the whole mechanism, and it is why a third one-line repair
is not the answer.

The mechanism is structural rather than careless. A change that adds a
trap to its own file had to edit a string in a suite it otherwise never
touches, so two changes sharing no line still invalidated each other:
each stayed green against its own base, git merged both cleanly, and the
guard went red only once the second one landed. A file *arriving* with
traps was worse still, because nothing in its author's diff pointed at
that string at all. That is exactly how
`run-kubernetes-talos-diagnostics_test.bats` got in, twice.

That contention is not in the past tense. Two open PRs are editing that
one line right now, and they disagree about what it should say: #3575
adds `run-kubernetes-talos-diagnostics_test.bats=8` to it, repeating a
repair that has already landed, and #3441 removes
`select-e2e_test.bats=15` from it, because it converts that file.
Neither PR is about EXIT traps. Both have to touch that string anyway,
and whichever lands second is wrong until someone edits it again.

So this PR is not fixing a red main. It removes the thing that keeps
making main red, which is why it is worth more than the one-line fix
that is now the established habit.

One honest caveat about its own landing. The textual-conflict property
is steady-state: a branch forked *before* the declaration exists has no
line to disagree with, so it converts traps in its own file, merges
clean, and the count only goes wrong once both sides are on main. I
checked this against #3441, which takes `select-e2e_test.bats` from
fifteen traps to zero. Merged after this, that file would declare
fifteen and hold none. What the move buys even in that case is that the
red names a file the branch already edited, the repair is one line
inside it, and rebasing before merge catches it on the branch's own CI.
None of those three held against the central inventory. Whoever merges
this should expect one such adjustment on the conversion branches still
in flight.

So this replaces both lists with a declaration each file makes about
itself: one `# EXIT-TRAP DEBT: N` comment in its leading comment block.
A file carrying no declaration must install no EXIT trap. Growing or
shedding a trap now fails in the file the change already edits, so two
changes that disagree about a count get a real textual conflict instead
of silently invalidating each other, and a change that leaves the traps
alone edits nothing.

**The include list is redundant, not lost.** It named the files proven
clean, so that a trap reappearing in one of them would fail. Under the
new rule those files carry no declaration, and a file with no
declaration must hold zero traps, so a trap reappearing in any of them
fails on its own, with no list to be on. Coverage widens rather than
narrows: the two lists named twenty files between them, and the rule
covers all fifty bats files under `hack/`, subdirectories included, plus
the ones added tomorrow.

Rebasing this branch onto current main is the property working. Main has
since gained `hack/kubernetes-pre-delete-hook.bats` and
`hack/tenant-pre-delete-hook.bats`, and `hack/cozyreport.bats` grew by
some eight hundred lines. Neither new file installs an EXIT trap, so
neither needed a declaration and neither needed an edit here; the rebase
took no conflict at all. Under the inventory, each arriving file was a
coin toss on whether somebody had remembered the string.

To be precise about what the inventory could and could not do, since it
is easy to overstate: it did *notice* a new file carrying traps. The
string it compared was built by scanning the directory, so an arriving
file appended a token and failed the comparison, which is exactly how
main went red. What it could not do is let that file arrive without an
edit in a foreign suite. Being seen and being absorbable are different
properties, and only the second one decides whether two changes can land
independently.

The declaration is pinned in both directions. Declaring N while holding
N+1 fails, obviously; declaring N while holding N−1 fails too. Without
that second half the number becomes a ceiling and rots upward: somebody
converts half a file, the declaration stays, and the guard quietly
licenses traps that were removed long ago.

It is read only from the leading comment block, under the shebang and
above the first line of code. A `.bats` file is shell that writes shell,
so the same line turns up inside a heredoc, a fixture writer or an
expected-output string, where it is data belonging to one test;
honouring it there would let an unrelated fixture excuse a real trap,
silently, with nothing in that test's own diff looking like a
declaration. A comment block is the region with no interior; stopping
instead at the first `@test` would still read a line out of a helper's
heredoc.

Not every counted handler is debt. A trap inside an explicit subshell
does not replace the one the `bats` binary installs, so a test failing
inside `( … )` still prints its `not ok`, checked against a test-level
trap in the same file, where the TAP line vanishes.
`hack/e2e-test-openapi.bats` kills a backgrounded `kubectl proxy`
exactly that way, and "convert it like the others" would leak a process
holding a fixed port and wedge the next run. Its declaration now records
the carve-out instead of scheduling a conversion, and because the
ratchet is exact in both directions, *removing* that trap fails too, so
the count protects the construct rather than marking it for deletion.
`docs/agents/e2e-testing.md` previously scoped this exception to
Chainsaw `script` steps only; it now names the BATS subshell case as
well.

The counting bounds the keyword and the signal the same way, at any
character that cannot be part of an identifier, and matches the signal
in either case. Whitespace on the right missed `trap … EXIT; cd "$tmp"`;
whitespace on the left missed `tmp=$(mktemp -d);trap … EXIT` and `(trap
… EXIT; true)`; upper case missed `trap … exit`, which bash and dash
both install. All of those are real handlers that scored zero. The left
boundary is the one worth dwelling on, because the inventory being
replaced had none at all and *did* catch the semicolon form. Getting it
wrong here would have narrowed coverage while the commit claimed to
widen it. A plain word boundary is not enough either: `bootstrap ` ends
in `trap `, and it has to keep scoring nothing, or the documented answer
to a red guard (add a debt line) would buy a file a permanent licence
for one real trap to silence a line that has none.

Two handlers sharing one line are reported rather than counted, because
the count is a count of lines and the second would otherwise arrive
free. Splitting such a line properly needs a shell parser, since a
semicolon inside a handler's own quoted action is not a separator, and
guessing wrong undercounts, the one direction a ratchet cannot afford.

**What this does not fix.** The declaration is still a loophole: a new
file can write `# EXIT-TRAP DEBT: 8` instead of cleaning up, and nothing
here makes that impossible. What changes is that the admission is local
and visible. It sits at the top of the file it excuses, in front of
whoever reviews that file, instead of being a number in a neighbouring
suite nobody in that review is reading. Today's loophole is the same
size and invisible.

Three more limits, all stated in the guard's own header rather than left
to be discovered. The scan is lexical, so a signal computed at runtime
and a quoted action spanning physical lines without a backslash are both
invisible. An exact count catches addition and removal but never
substitution: swap the openapi file's subshell trap for a test-level one
and the total stays 1. And the scan reads `.bats` only, so a handler
arriving through a sourced `.sh` is outside it.
`hack/e2e-chainsaw/_lib/run-kubernetes.sh` installs two right now, and
each is benign for its own reason rather than by design: the one in
`cozy_capture_tenant_talos` because that function is declared with `(`
and so runs in a subshell, the one in `run_kubernetes_test` because no
`@test` calls it despite being declared with `{`. Three `hack/*.bats`
source that library, and two tests in the converted file call
`cozy_capture_tenant_talos`, so flipping a single `(` to `{` reinstates
a test-level handler in both of them with the guard green. Widening the
scan to `.sh` would mean counting handlers that are correct in a script
and wrong only in a test body, so the honest answer is that this is
where the instrument stops.

Three further boundaries, recorded here so they land as known edges
rather than as surprises. The old include list also failed when a file
named on it disappeared from the tree; the new rule can only judge a
file that is present, so a deleted converted file goes unnoticed. That
is a genuinely smaller check, though its absence shows up in the diff
that deletes the file. The guard's own failure messages are code lines,
so they are scanned by the pattern they belong to: they pass today only
because no bare `EXIT` or `0` happens to follow the keyword in any of
them, and a rewording that introduced one would make the file demand a
debt of itself. It fails loudly rather than quietly, and the fixture
writers and test titles already split the keyword from the signal for
this reason, but the messages do not. Finally,
`docs/agents/e2e-testing.md` bans test-level `EXIT` *and* `RETURN`
traps, while every mechanical guard this repo has had, the one being
deleted included, matches only `EXIT` and `0`. `hack/` holds no RETURN
trap today, so nothing regresses here, but half of that documented rule
has never had an executor.

The guard moves out of `hack/cozyreport.bats` into
`hack/bats-no-exit-trap.bats`, because its subject is every unit suite
under `hack/` and not the report collector it grew up in. Living inside
`cozyreport.bats` is precisely why unrelated PRs kept converging on one
line. `hack/md-no-hardwrap.bats` is the neighbouring precedent.

Separately, the eight EXIT traps in
`hack/run-kubernetes-talos-diagnostics_test.bats` are converted to
end-of-body cleanup rather than given a declaration. That file was
written six weeks after the ban was documented, so "it predates the
rule" is not available to it, and handing it a debt is literally the
move that failed the last two times. The remaining files keep their
declarations, since a mass conversion would collide with the branches
that own them.

One note on where the prose sits. The guard's own header carries only
what a reader has to keep true: why the ban exists, the two carve-outs,
where the declaration lives and why there, that the count is exact in
both directions, why the marker is anchored at column zero, and where
the lexical scan stops. Everything above about *how* the inventory
failed (the timestamps, the PR numbers, the arithmetic) is deliberately
not in the file. It is true today and will rot on its own; a comment
nobody is obliged to maintain is how a file grows a paragraph that is
quietly false.

### 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
test(tests): each hack/*.bats file now declares its own remaining EXIT-trap debt in a `# EXIT-TRAP DEBT: N` header comment, checked by hack/bats-no-exit-trap.bats, replacing the central inventory in hack/cozyreport.bats
```


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

## Summary by CodeRabbit

- **Testing**
- Added automated auditing for `EXIT` traps across Bats end-to-end
tests, including validation of tracking declarations and edge cases.
- Improved diagnostics tests by replacing trap-based temporary-directory
cleanup with explicit cleanup steps.
  - Added tracking annotations for remaining trap-related cleanup work.

- **Documentation**
- Clarified when traps are permitted inside self-contained subshells and
how remaining cleanup debt is reported.
  - Updated review guidance for consistent end-to-end test maintenance.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
Aleksei Sviridkin (lexfrei) added a commit that referenced this pull request Aug 9, 2026
…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
```
Aleksei Sviridkin (lexfrei) added a commit that referenced this pull request Aug 14, 2026
…3768)

## What this PR does

The most expensive e2e flake, #3513, is a tenant
worker missing the 18m node-Ready budget with zero Nodes registered. The
in-guest capture in #3548 diagnosed one variant: the
worker's kubelet image pull crawls to ghcr.io at 40-165 KB/s over the
runner egress and does not finish inside the budget, Talos holds the
kubelet service until its image arrives, so the node never registers and
the tenant cilium HelmRelease then fails for want of a schedulable node
rather than for any fault of its own.

The sandbox already runs an in-cluster pull-through mirror for ghcr.io,
but a pull-through cache fetches a blob only when a client first asks
for it, so the first tenant worker pays the cold fetch inside its
node-Ready budget. Measured with the same fetch script against a local
instance of the same registry image: 32m47s cold, 1.1s warm, for the
same tags the failing runs stalled on.

This PR fills the cache from a Job started by the same install step that
deploys the mirror, tens of minutes before the kubernetes suites run. It
warms the two kubelet tags the suites actually select, resolved from the
chart's version map in the same order the suites resolve it, and one
platform per tag (the one the workers run, ~61 MiB each) - warming what
nobody pulls would be a true addition to the same throttled egress the
install itself is using.

The warm-up is best-effort by construction: a Job that fails, times out
or never starts does not fail the install, and the suites behave exactly
as they did before it existed. The Job itself exits non-zero on any
missed tag, so its status stays honest.

The node-join failure diagnostics now answer the questions a red run
used to leave open: whether a worker pulled through the mirror at all
(counting GET and HEAD access lines per request, excluding the warm-up's
own requests, which name themselves via user-agent), how long the mirror
took to answer (server-side `http.response.duration` is the only
after-the-fact signal separating a warm cache from a cold one, since the
proxy stores blobs in the background and the Job reports success either
way), and whether the warm-up itself got anywhere. The diagnostic
section grows from five to seven bounded reads, moving its worst case
from roughly 100s to roughly 150s inside the existing phase budget gate;
unbudgeted diagnostic residuals stay tracked in
#3666.

This addresses one variant of #3513 and does not
close it: the zero-CSR class remains untouched, and a green suite after
this change is not evidence the flake is gone. The 18m node-Ready budget
is not raised.

Covered by 65 unit tests in `hack/ghcr-mirror_test.bats`: tag selection
and ordering against the suites' own expressions, rejection of malformed
and injected tag values, Job shape and restricted security context, the
readiness wait as a wall clock, every failure path of the new
diagnostics including an over-1-MiB log fixture that pins the duration
filter against real log sizes, and the exclusion of the warm-up's own
traffic from both the request count and the duration report.

### Screenshots

No UI changes.

### Downstream repositories

The diff is confined to e2e infrastructure under `hack/`: the mirror
manifest, the warm-up library and its tests, the install step, and
diagnostic labels. Walked the trigger map file by file - no chart
values, no APIs, no documentation content, nothing any downstream
repository consumes.

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


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

* **New Features**
  * Added best-effort GHCR mirror warm-up during installation.
* Preloads selected Kubernetes image manifests and blobs to improve
cache readiness.
  * Warm-up failures safely fall back to direct image pulls.

* **Diagnostics**
* Improved request classification, timing, repository identification,
and reporting of warm-up status and logs.

* **Tests**
* Expanded coverage for warm-up behavior, validation, retries, security
settings, caching, logging, installation, and diagnostics.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/testing Issues or PRs related to testing (e2e, bats, unit tests) kind/bug Categorizes issue or PR as related to a bug size/XL This PR changes 500-999 lines, ignoring generated files

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants