Repository navigation
test(e2e): wait 10m for tenant worker MachineDeployment to match the bootstrap budget - #3139
Aleksei Sviridkin (lexfrei) wants to merge 1 commit into
Conversation
…bootstrap budget The md0 MachineDeployment scale-to-2 wait in run-kubernetes.sh gave up after 5m, but the product can legitimately take up to 10m to get there. The MachineDeployment bootstrap.configRef gates on a TalosConfigTemplate that is only produced once the talos-reconcile hook Job observes its PKI lookup inputs (apiserver Service ClusterIP, Talos CA, K8s CA, management CoreDNS). That Job's own wait loop budgets 10m (120 x 5s) for those inputs, so on a cold-start CI sandbox the test could time out while the product was still within its real budget, surfacing as md0 stuck ScalingUp "blocked because TalosConfigTemplate does not exist". Raise the test wait from 5m to 10m to match the bootstrap budget and the later readyReplicas wait, which is already 10m. Both kubernetes-latest and kubernetes-previous source this one script, so the single change covers both variants. Assisted-By: Claude <[email protected]> Signed-off-by: Aleksei Sviridkin <[email protected]>
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 updates the end-to-end test infrastructure to resolve intermittent failures in the tenant worker-VM deployment. By extending the wait timeout to match the product's internal bootstrap budget, the test now correctly accounts for the time required for the talos-reconcile hook to process necessary PKI inputs, preventing premature timeouts in CI environments. 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
|
📝 WalkthroughWalkthroughIn ChangesE2E Kubernetes Test Timeout
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 increases the timeout for waiting on the Kubernetes MachineDeployment in the end-to-end test script hack/e2e-apps/run-kubernetes.sh from 5 minutes to 10 minutes. This adjustment aligns the test wait time with the 10-minute bootstrap budget of the talos-reconcile hook Job, preventing premature test failures. There are no review comments, and I have no 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
🤖 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 `@hack/e2e-apps/run-kubernetes.sh`:
- Around line 168-172: Update the timeout rationale comment in the Kubernetes
e2e script so it matches the new 10m wait value instead of referencing the stale
old 1m budget. Keep the explanation aligned with the surrounding bootstrap flow
in run-kubernetes.sh, especially the talos-reconcile hook Job and the later
readyReplicas wait, so the comment accurately describes why the longer budget is
needed.
🪄 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: c6ffba0d-20e5-4e76-8701-fb02a08fa70a
📒 Files selected for processing (1)
hack/e2e-apps/run-kubernetes.sh
| # MachineSet scale-up past the old 1m budget. The talos-reconcile hook Job | ||
| # awaits those PKI inputs (its own wait loop budgets 10m, 120 x 5s) before it | ||
| # produces the TalosConfigTemplate, so a 5m test wait can give up while the | ||
| # product is still legitimately within budget. Use 10m to match that bootstrap | ||
| # budget (and the later readyReplicas wait below). |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Fix the stale timeout value in the comment.
The PR changes this wait from 5m to 10m, but Line 168 still says “old 1m budget.” That makes the rationale misleading for the next person tuning this wait.
🤖 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/e2e-apps/run-kubernetes.sh` around lines 168 - 172, Update the timeout
rationale comment in the Kubernetes e2e script so it matches the new 10m wait
value instead of referencing the stale old 1m budget. Keep the explanation
aligned with the surrounding bootstrap flow in run-kubernetes.sh, especially the
talos-reconcile hook Job and the later readyReplicas wait, so the comment
accurately describes why the longer budget is needed.
|
Superseded by #3145, which fixes the root cause instead of widening the timeout. The md0 wait was not mis-calibrated on duration: the TalosConfigTemplate is created by a post-install hook that never runs when the install stalls in main-wait deadlocked on the tenant cilium HelmRelease (cilium needs worker nodes, which need the template, which needs the hook, which only runs after main-wait). #3145 moves that Job to the main install phase so the template is created during apply and the deadlock is broken — validated: md0 reaches replicas=2 in ~4-6s instead of timing out. |
#3145) ## What this PR does The tenant worker `MachineDeployment` `md0` intermittently never reaches `status.replicas=2`. CAPI blocks MachineSet creation until the `TalosConfigTemplate` that the deployment's `bootstrap.configRef` points to exists, and on failing installs that template is never created. ### Root cause — an ordering deadlock The `TalosConfigTemplate` is created only by the `talos-reconcile` **post-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 the `TalosConfigTemplate`, 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 and `md0` never scales. The release sets `helm-install-disable-wait` precisely 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-latest` and `kubernetes-previous`, 640 polls) captured: `TalosConfigTemplate` absent in 640/640 polls; both post-install hook Jobs (`talos-reconcile` and `bootstrap-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 at `Running 'install' action 20m` then looping into `upgrade`, with the cilium sub-release timing out on `cilium-operator` (which has no worker node to schedule on) — downstream of the missing template. ### The fix Render the `talos-reconcile` Job and its RBAC / ServiceAccount / egress `CiliumNetworkPolicy` as 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 tenant `cilium-operator`). It produces the `TalosConfigTemplate`, `md0` scales, 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 not `helm-install-disable-wait` takes 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 `KubevirtMachineTemplate` idiom in `cluster.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 applied `TalosConfigTemplate` keeps its stable `<release>-<group>` name (referenced by `bootstrap.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 single `kubectl apply` of the template with a `KamajiControlPlane` ownerReference for GC, and the `extraHostEntries` are 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/hook` on 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 ```release-note fix(kubernetes): create the tenant worker TalosConfigTemplate from a main-phase Job instead of a post-install hook, so worker MachineDeployments reliably scale during install ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved Kubernetes addon install behavior by running reconcile work during the main chart phase, reducing readiness deadlocks and upgrade issues. * Added more reliable reconciliation handling with stable, change-detected job recreation and updated network access for the reconcile workload. * Refined chart and test guidance to match the new install flow and Helm behavior. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
What this PR does
The e2e tenant-Kubernetes test waited only 5m for the tenant worker MachineDeployment (
md0) to reachstatus.replicas=2, but the product's real bootstrap budget for that step is 10m. The MachineDeployment'sbootstrap.configRefgates on aTalosConfigTemplatethat the post-install talos-reconcile hook Job produces only after it observes its PKI lookup inputs (apiserver Service ClusterIP, Talos CA, Kubernetes CA, management CoreDNS). That Job's own wait loop budgets 10m for those inputs (for i in $(seq 1 120); do … sleep 5, i.e. 120 × 5s = 600s). On a cold-start CI sandbox the test could therefore expire at 5m while the product was still legitimately within budget, surfacing asmd0stuckScalingUp: "blocked because TalosConfigTemplate does not exist"— a false failure, not a product regression (node metrics show no resource pressure).This raises the test wait from 5m to 10m so it matches the bootstrap budget and the later
readyReplicas=2wait in the same script, which is already 10m.kubernetes-latestandkubernetes-previousboth source this single script, so one change covers both variants. This unblocks the recurring tenant worker-VM e2e flake that has been failing the kubernetes e2e on multiple in-flight PRs.Test-infrastructure only — no product code changes. The wait already complies with the e2e conventions (no new retries, no traps; the longer-than-5m budget is justified in-line).
Screenshots
N/A — test-infrastructure change, no UI impact.
Release note
Summary by CodeRabbit