fix(windows): retry kubelet start check after NodeResetScriptTask - #9344
fix(windows): retry kubelet start check after NodeResetScriptTask#9344Xu Xue (xuexu6666) wants to merge 2 commits into
Conversation
The kubelet-running assertion added in #8970 checked the service status exactly once immediately after the k8s-restart-job reset task reported success. windowsnodereset.ps1 issues Start-Service kubelet, but on slower first boots -- notably sysprepped/cached (golden) images provisioned in the Windows VHDCaching e2e scenario -- kubelet can still be in StartPending at that instant, so the single-shot check failed CSE with WINDOWS_CSE_ERROR_START_NODE_RESET_SCRIPT_TASK intermittently. Poll for kubelet to reach Running with bounded retries (6 x ~5s) and re-nudge Start-Service on each attempt, mirroring the Start-Containerd pattern in containerdfunc.ps1. A genuinely failed kubelet still exits with the same code/message after the timeout, so #8970's intent is preserved. Adds a Pester case covering recovery after a retry.
Windows Unit Test Results 3 files 13 suites 52s ⏱️ Results for commit ec7171f. ♻️ This comment has been updated with latest results. |
There was a problem hiding this comment.
Pull request overview
Adds bounded kubelet startup retries after the Windows node-reset task to reduce cached-image provisioning flakes.
Changes:
- Retries and re-nudges kubelet startup before failing CSE.
- Adds a Pester recovery-path test.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
parts/windows/windowscsehelper.ps1 |
Adds kubelet startup polling and retry logic. |
parts/windows/windowscsehelper.tests.ps1 |
Adds mocks and retry recovery coverage. |
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| $script:kubeletCallCount = 0 | ||
| Mock Get-Service -MockWith { | ||
| $script:kubeletCallCount++ | ||
| if ($script:kubeletCallCount -eq 1) { | ||
| return [pscustomobject]@{ Status = "StartPending" } | ||
| } | ||
| return [pscustomobject]@{ Status = "Running" } | ||
| } |
There was a problem hiding this comment.
Good catch — fixed in ec7171f. The service now stays StartPending through the presence check and the loop's first check, with a no-op WaitForStatus, so the recovery branch runs Start-Service exactly once and then exits Running.
…etry Address Copilot review: the mock returned Running on the loop's first Get-Service call, so Start-NodeResetScriptTask broke out before Start-Service and the Times 1 assertion could not hold. Hold the service in StartPending through the presence check and first loop check, and give it a no-op WaitForStatus so the recovery branch (Start-Service + WaitForStatus) runs and the loop exits Running.
| } | ||
|
|
||
| if ($retryCount -lt $maxRetryCount) { | ||
| Start-Sleep -Seconds 5 |
There was a problem hiding this comment.
This is large number. We don't need to protect any downstream service. You probably can retry every few ms without negative consequences
But it may frequently delay VM provision time by extra 5 seconds
| if ($retryCount -lt $maxRetryCount) { | ||
| Start-Sleep -Seconds 5 | ||
| } | ||
| } while ($retryCount -lt $maxRetryCount) |
There was a problem hiding this comment.
those type of fixes always make me nervous.
why is this a new problem ? as if something regressed ? kubelet is having a harder time to start ?
or stuck in a retry loop until eventually working ?
What
Start-NodeResetScriptTask(inparts/windows/windowscsehelper.ps1) asserted thekubeletservice wasRunningexactly once, immediately after thek8s-restart-jobreset task reported success.windowsnodereset.ps1issuesStart-Service kubelet, but on slower first boots — notably sysprepped/cached (golden) images provisioned in the Windows VHDCaching e2e scenario — kubelet can still be inStartPendingat that instant, so CSE failed intermittently withWINDOWS_CSE_ERROR_START_NODE_RESET_SCRIPT_TASK("kubelet service is not running after NodeResetScriptTask completed").Fix
Poll for kubelet to reach
Runningwith bounded retries (6 × ~5s) and re-nudgeStart-Serviceon each attempt, mirroring the existingStart-Containerdpattern instaging/cse/windows/containerdfunc.ps1. A genuinely failed kubelet still exits with the same code/message after the timeout, so the intent of #8970 (validate the reset result) is preserved — this only absorbs start-latency.Evidence
Agentbaker Windows E2Eis flaky on this exact scenario on unchangedmain(green some days, red others). The failing case is alwaysTest_Windows2022_VHDCaching_LegacyTLSBootstrap/VHDCreation/VMProvision: the base node comes up and validates, but the node provisioned from the sysprepped/captured image fails the reset kubelet check. Within a run, the faster base node passes while the slower cached node fails — consistent with a start-latency race, not a deterministic break.Testing
Adds a Pester case in
windowscsehelper.tests.ps1covering kubelet reachingRunningafter a retry; existing cases (including "fails when kubelet is not running") still hold. Windows E2E triggered on this PR.