fix(kubernetes): run the Talos reconcile Job in the main install phase - #3145
Conversation
The talos-reconcile Job is the only thing that creates the TalosConfigTemplate the worker MachineDeployment's bootstrap.configRef points to; CAPI blocks MachineSet creation until it exists, so tenant workers cannot scale until the Job has run. The Job was a post-install/post-upgrade Helm hook, which runs only after Helm has applied the release's main resources and, when Helm waits, after they become Ready. But the same release emits in-tenant addon HelmReleases — notably cilium — whose readiness needs worker nodes, and those nodes need the TalosConfigTemplate this Job produces. So when the main-resource wait is in effect the install deadlocks: it waits for cilium to be Ready, cilium waits for workers, the workers wait for the template, and the template waits for this hook — which never runs because the install never finishes the wait that precedes the hook phase. The install times out and the MachineDeployment never scales. The release sets helm-install-disable-wait to skip that wait, but its effect is unreliable, which makes the failure bimodal: the template lands in ~46s when the wait is skipped, and the install is stranded when it is not. Render the Job and its RBAC/ServiceAccount/egress policy as ordinary main-phase resources. The Job is then created during the apply itself, not after the wait, and runs on the management cluster where nodes exist (unlike the tenant cilium-operator), so it produces the template, the MachineDeployment scales, workers join, cilium goes Ready, and the wait (if any) closes within the install budget — independent of whether helm-install-disable-wait takes effect. The Job is immutable, so it carries a content-hash name suffix over its rendered spec (mirrors the KubevirtMachineTemplate idiom in cluster.yaml): an identical render keeps the name and Helm no-ops it, while any spec change yields a new name and a fresh Job, so no spec change ever lands on the same name and triggers an immutable-field patch error. The applied TalosConfigTemplate keeps its stable <release>-<group> name, so an unchanged render re-applies a byte-identical template — a CAPI no-op that never rolls existing workers. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la>
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request resolves a structural deadlock in the Kubernetes tenant installation process. Previously, the Highlights
New Features🧠 You can now enable Memory (public preview) to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on Gemini (@gemini-code-assist) comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
📝 WalkthroughWalkthroughThe Changestalos-reconcile hook → main-phase Job
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Code Review
This pull request refactors the talos-reconcile Job in the kubernetes application from a post-install/post-upgrade Helm hook to a main-phase Job. This change resolves a deadlock during installation where the main-resource wait blocks the hook from running while in-tenant addon HelmReleases (like Cilium) wait for worker nodes that require the TalosConfigTemplate produced by the Job. To prevent upgrade conflicts on this immutable resource, the Job is now rendered with a content-hash name suffix. The unit tests, comments, and documentation across the codebase have been updated to reflect this architectural shift. There are no review comments, and I have no additional feedback to provide.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/apps/kubernetes/templates/talos/talos-reconcile-job.yaml (1)
110-116: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winAdd
kamajicontrolplanes/finalizersupdate permissionThe Job sets
blockOwnerDeletion: trueon theKamajiControlPlaneownerReference, so the Role also needsupdateonkamajicontrolplanes/finalizers(scoped to{{ .Release.Name }}) or theTalosConfigTemplateapply can be forbidden.🤖 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 `@packages/apps/kubernetes/templates/talos/talos-reconcile-job.yaml` around lines 110 - 116, The Role for the Talos reconcile Job is missing the finalizer update permission needed for the KamajiControlPlane ownerReference flow. Update the RBAC rule in talos-reconcile-job.yaml for kamajicontrolplanes to include update on the kamajicontrolplanes/finalizers subresource, scoped to {{ .Release.Name }}, alongside the existing get and patch permissions so the TalosConfigTemplate apply is not forbidden.
🤖 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.
Inline comments:
In `@packages/apps/kubernetes/templates/talos/talos-reconcile-job.yaml`:
- Around line 218-225: Add a client-side request timeout to every kubectl
invocation in the talos reconcile Job so a hung apiserver cannot bypass the
retry window. Update the kubectl calls in the wait loop and the later
patch/get/apply steps to pass a consistent --request-timeout value, using the
existing Job template logic where those commands are constructed. Make sure the
timeout is applied uniformly in the talos-reconcile-job.yaml flow so all kubectl
operations respect the same bounded failure behavior.
---
Outside diff comments:
In `@packages/apps/kubernetes/templates/talos/talos-reconcile-job.yaml`:
- Around line 110-116: The Role for the Talos reconcile Job is missing the
finalizer update permission needed for the KamajiControlPlane ownerReference
flow. Update the RBAC rule in talos-reconcile-job.yaml for kamajicontrolplanes
to include update on the kamajicontrolplanes/finalizers subresource, scoped to
{{ .Release.Name }}, alongside the existing get and patch permissions so the
TalosConfigTemplate apply is not forbidden.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 90426baa-a632-43df-bc6f-0f3e26eb793a
📒 Files selected for processing (10)
packages/apps/kubernetes/templates/cluster.yamlpackages/apps/kubernetes/templates/helmreleases/cilium.yamlpackages/apps/kubernetes/templates/talos/talos-reconcile-job.yamlpackages/apps/kubernetes/tests/cluster_test.yamlpackages/apps/kubernetes/tests/gpu_node_labels_test.yamlpackages/apps/kubernetes/tests/kubelet_reservation_test.yamlpackages/apps/kubernetes/tests/talos_templates_test.yamlpackages/system/kubernetes-rd/cozyrds/kubernetes.yamlpkg/config/config.gopkg/registry/apps/application/rest.go
| {{- /* Each pod attempt runs the ~10-minute (120 × 5s) input-wait loop, so this | ||
| backoffLimit sets the self-heal window to roughly 5 hours. The Job does | ||
| not block the install — when disable-wait is in effect Helm does not wait | ||
| on it, and when Helm does wait the Job completes in tens of seconds — so a | ||
| long retry budget costs nothing on the install path. It only governs how | ||
| long a Job whose inputs are unusually slow keeps retrying on its own | ||
| before giving up. Bounded (not infinite) so a permanently-broken input | ||
| eventually stops spawning pods. */}} |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Static check: every kubectl call in the reconcile script should use a bounded request timeout.
rg -n -C2 'kubectl|request-timeout' packages/apps/kubernetes/templates/talos/talos-reconcile-job.yamlRepository: cozystack/cozystack
Length of output: 2939
🏁 Script executed:
#!/bin/bash
sed -n '250,340p' packages/apps/kubernetes/templates/talos/talos-reconcile-job.yamlRepository: cozystack/cozystack
Length of output: 5020
🌐 Web query:
kubectl --request-timeout default no timeout client-side request deadline docs
💡 Result:
The official documentation for kubectl specifies that the --request-timeout flag defaults to a value of "0" [1][2][3]. In this context, a value of zero explicitly means that there is no client-side timeout, and kubectl will not automatically give up on a request [1][2]. When the --request-timeout is set to 0, kubectl will wait indefinitely for a response from the API server [1][4]. If you wish to impose a deadline, you must explicitly provide a duration with a time unit, such as 1s, 2m, or 3h [1][3]. While some external guides or community resources may suggest that a default 30-second or 60-second timeout exists [5][6], these claims conflict with the official Kubernetes documentation which defines the default behavior as 0 (no timeout) [1][2][3]. It is important to distinguish between the client-side --request-timeout (which governs the kubectl client's patience) and server-side timeouts (governed by the kube-apiserver configuration), as misalignment between these two can lead to client-side errors like context deadline exceeded while the server continues processing the request [5].
Citations:
- 1: https://kubernetes.io/docs/reference/kubectl/kubectl/
- 2: https://kubernetes.io/docs/reference/kubectl/generated/kubectl/
- 3: https://kubernetes.io/docs/reference/kubectl/generated/kubectl_options/
- 4: https://manpages.debian.org/unstable/kubernetes-client/kubectl.1.en.html
- 5: https://markaicode.com/errors/kubernetes-timeout-error-fix/
- 6: Set a default request timeout for discovery client kubernetes/kubernetes#62733
Add a client-side timeout to every kubectl call in this Job.
kubectl defaults to no request timeout, so a hung apiserver connection can block the pod forever and bypass the 120×5s retry window. Apply --request-timeout consistently to the wait loop and the later patch, get, and apply calls.
🤖 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 `@packages/apps/kubernetes/templates/talos/talos-reconcile-job.yaml` around
lines 218 - 225, Add a client-side request timeout to every kubectl invocation
in the talos reconcile Job so a hung apiserver cannot bypass the retry window.
Update the kubectl calls in the wait loop and the later patch/get/apply steps to
pass a consistent --request-timeout value, using the existing Job template logic
where those commands are constructed. Make sure the timeout is applied uniformly
in the talos-reconcile-job.yaml flow so all kubectl operations respect the same
bounded failure behavior.
myasnikovdaniil
left a comment
There was a problem hiding this comment.
The fix is correct and the implementation is clean. The root-cause analysis is rigorous, the content-hash idiom mirrors the existing KubevirtMachineTemplate pattern, and the test coverage (128 tests, including hash-rotation and truncation cases) is strong. Approving with a few documentation-only suggestions below.
One design note not tied to a specific line: the ServiceAccount, Role, RoleBinding, ClusterRole, ClusterRoleBinding, and CiliumNetworkPolicy are now permanent main-phase resources (no hook-delete-policy). In the old version they were cleaned up after each successful run. This is intentional — the Job may be recreated after TTL expiry and needs its RBAC in place — but it is a behaviour change on upgrade (Helm adopts these from hook-managed to main-phase-managed via 3-way merge). A one-line note on the ServiceAccount would help future readers understand why the RBAC is permanent.
| RBAC, ServiceAccount and egress CiliumNetworkPolicy are main-phase too; | ||
| the wait loop tolerates its dependencies (and its own network policy) | ||
| not yet being in place, so Helm's kind-ordered apply needs no explicit | ||
| sequencing. The bootstrap-token Job stays a post-install hook. |
There was a problem hiding this comment.
Nit / documentation: The comment says bootstrap-token "stays a post-install hook" but doesn't flag the ordering change. In the old version with DisableWait=true, hook-weight ordering (talos-reconcile weight 5 → bootstrap-token weight 10) guaranteed bootstrap-token only started after talos-reconcile completed. Now they run concurrently (talos-reconcile is main-phase; bootstrap-token fires as soon as Helm finishes applying resources, without waiting for this Job to finish). This is safe because bootstrap-token's backoffLimit: 10 handles a still-booting tenant apiserver, but a one-line note would prevent future readers from being surprised when they see bootstrap-token fail its first attempt on a fresh install.
| spec: | ||
| backoffLimit: 5 | ||
| {{- /* Each pod attempt runs the ~10-minute (120 × 5s) input-wait loop, so this | ||
| backoffLimit sets the self-heal window to roughly 5 hours. The Job does |
There was a problem hiding this comment.
Nit: The comment says "roughly 5 hours" — this is the worst-case when every attempt exhausts the 10-minute wait loop. In the normal path the first attempt succeeds in tens of seconds. Worth clarifying: worst-case 5 hours if inputs are permanently absent; in practice the first attempt succeeds in tens of seconds once Kamaji emits the Service and cert-manager issues the CAs.
| TTL a Failed Job would keep the same content-hash name and be a no-op on | ||
| every later upgrade — Helm never patches the immutable Job, so it would | ||
| stay Failed until deleted by hand. No churn from the TTL: the repo runs no | ||
| Flux drift detection, so an interval reconcile never re-applies a cleared |
There was a problem hiding this comment.
Nit: The comment correctly states that Flux drift detection is off by default, so a cleared Job won't be recreated on interval reconciles. Worth adding a brief note that if drift detection is explicitly enabled (.spec.drift.detect: true on the HelmRelease), the cleared Job will be recreated every ~10 minutes. The kubectl apply is idempotent (CAPI no-op if the TCT is unchanged), so there is no correctness risk — just unexpected pod activity.
| # Skip the readiness wait so the hook fires and the chicken-and-egg | ||
| # resolves. | ||
| # created without the TalosConfigTemplate the Job produces. Skip the | ||
| # readiness wait so the release settles and the chicken-and-egg resolves; |
There was a problem hiding this comment.
Nit / documentation: The comment says "the chicken-and-egg resolves" — this was accurate when DisableWait was required for correctness (old hook version). With this fix, the deadlock is broken independently of DisableWait. The annotation is now a performance optimisation (avoids blocking the HelmRelease for the 5-8 minutes it takes workers to boot and addons to become Ready), not a correctness requirement. A reader might ask "can I remove this now that the deadlock is fixed?" Suggest clarifying:
# Skip the readiness wait so the release does not block for 5-8 minutes on
# worker-boot + addon-readiness. The talos-reconcile main-phase Job breaks
# the install deadlock independently; DisableWait is a performance opt.
| asserts: | ||
| - equal: | ||
| path: metadata.name | ||
| value: test-k8s-talos-reconcile-md0-6ddd7d |
There was a problem hiding this comment.
Nit: Hardcoded hash 6ddd7d (and ceb45e on line 197) will silently go stale if anyone edits the Job spec template without reading the full test-suite description. The test comment at the top of this block explains this well, but a short inline note — e.g. # BREAKING: update this hash when the Job spec changes — directly above each value: line would make the requirement visible to someone who edits the template and only runs the test to see what failed.
What this PR does
The tenant worker
MachineDeploymentmd0intermittently never reachesstatus.replicas=2. CAPI blocks MachineSet creation until theTalosConfigTemplatethat the deployment'sbootstrap.configRefpoints to exists, and on failing installs that template is never created.Root cause — an ordering deadlock
The
TalosConfigTemplateis created only by thetalos-reconcilepost-install Helm hook (the chart can no longer render it at template time — three of its inputs are produced asynchronously after apply). A post-install hook runs only after Helm has applied the release's main resources and, when Helm waits, after they become Ready.But the same release also emits in-tenant addon HelmReleases — notably cilium — whose readiness needs tenant worker nodes, and those nodes can only be created once this hook has produced the
TalosConfigTemplate. So when the main-resource wait is in effect, the install deadlocks: it waits for cilium to be Ready, cilium waits for worker nodes, the nodes wait for theTalosConfigTemplate, and the template waits for the post-install hook — which never runs, because the install never finishes the wait that precedes the hook phase. The install times out andmd0never scales.The release sets
helm-install-disable-waitprecisely to skip that main-resource wait so the hooks run, but its effect is unreliable — which is what makes the failure bimodal: when the wait is skipped the hook runs and the template lands in ~46s; when it is not, the deadlock strands the install.A timestamped poller over a failing run (both
kubernetes-latestandkubernetes-previous, 640 polls) captured:TalosConfigTemplateabsent in 640/640 polls; both post-install hook Jobs (talos-reconcileandbootstrap-token) never created — the install never reached the post-install hook phase; all main resources applied (MachineDeployment, KubevirtMachineTemplate, MachineHealthCheck, WorkloadMonitor, child HelmReleases, Talos PKI/secrets) and the four hook inputs ready early and stable; the parent HelmRelease stuck atRunning 'install' action 20mthen looping intoupgrade, with the cilium sub-release timing out oncilium-operator(which has no worker node to schedule on) — downstream of the missing template.The fix
Render the
talos-reconcileJob and its RBAC / ServiceAccount / egressCiliumNetworkPolicyas ordinary main-phase resources instead of post-install hooks. The Job is then created during the apply itself — not after the wait — and runs on the management cluster, which has nodes (unlike the tenantcilium-operator). It produces theTalosConfigTemplate,md0scales, workers join, cilium goes Ready, and the main-resource wait (if any) closes within the install budget: the Job completes in tens of seconds, and node boot plus cilium readiness is a few minutes — comfortably inside the 20m install timeout. This works whether or nothelm-install-disable-waittakes effect, so it does not depend on that annotation's reliability.Because a main-phase Job is immutable, it carries a content-hash name suffix over its whole rendered spec (mirroring the
KubevirtMachineTemplateidiom incluster.yaml): an identical render keeps the same name and Helm no-ops it; any spec change yields a new name and a fresh Job, so no change ever lands on the same name and trips an immutable-field patch error. The appliedTalosConfigTemplatekeeps its stable<release>-<group>name (referenced bybootstrap.configRef), so an unchanged render re-applies a byte-identical template — a CAPI no-op that never rolls existing workers. The Job name is truncated to stay within the 63-char DNS-label limit while always preserving the hash. The runtime input-waits, the singlekubectl applyof the template with aKamajiControlPlaneownerReference for GC, and theextraHostEntriesare all unchanged.helm-install-disable-wait's unreliable effectiveness is a real latent issue (it explains the created-vs-never bimodality), but this fix breaks the deadlock independent of it, so that is a separate follow-up rather than a blocker here.Supersedes #3139 (a 10-minute wait-budget bump that treated the symptom; the deadlock is structural and a larger budget would not have helped).
Screenshots
Not applicable — no UI changes.
Testing
helm-unittest extended to pin: no
helm.sh/hookon any of the seven talos-reconcile documents, the content-hash name,ttlSecondsAfterFinished, the preserved template apply + ownerReference + extraHostEntries, the hash rotating on a spec change, and the long-name truncation staying within 63 chars. Full chart suite green (128 tests). The applied template is byte-identical to the previous hook output (verified by diff), so existing tenants are upgrade-safe.Release note
Summary by CodeRabbit