Skip to content

fix(windows): wait for kubelet healthz after node reset - #9356

Closed
r2k1 wants to merge 1 commit into
mainfrom
r2k1-windows-kubelet-readiness
Closed

fix(windows): wait for kubelet healthz after node reset#9356
r2k1 wants to merge 1 commit into
mainfrom
r2k1-windows-kubelet-readiness

Conversation

@r2k1

@r2k1 r2k1 commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

What this PR does / why we need it:

Windows CSE cannot use NSSM service state to decide whether kubelet is healthy. NSSM reports Running after it launches the process and Paused during restart delay. A healthy recovery after the containerd named-pipe race can fail a one-time service check, while a kubelet that is about to crash can pass it.

RP node readiness eventually replaces the node, but only after extra time and API calls.

This PR:

  • waits for k8s-restart-job to succeed, then polls kubelet's local http://127.0.0.1:10248/healthz
  • completes immediately on the first HTTP 200 (no added delay on the healthy path)
  • retries up to 12 times with a 2s delay and 2s request timeout to cover the expected NSSM restart
  • fails CSE with WINDOWS_CSE_ERROR_START_NODE_RESET_SCRIPT_TASK if kubelet never becomes healthy
  • makes Start-Service kubelet terminating and logs direct SCM failures
  • restores the HNS remediator task in finally, including when kubelet startup fails

The health check runs from current CustomData and works with old VHDs. The stricter Start-Service handling is VHD-baked and applies to updated VHDs.

/healthz proves local kubelet health. It does not prove node registration or Kubernetes Ready. RP still owns that check.

Validated that AKS does not override kubelet --healthz-port / --healthz-bind-address for Windows:

  • RP defaults_kubelet.go leaves the kubelet defaults (127.0.0.1:10248)
  • CustomKubeletConfig does not expose healthz settings
  • kubelet still serves /healthz by default through current supported versions

Related: #9354, #8970, #9344

Which issue(s) this PR fixes:

N/A

Testing:

  • PowerShell parsing passed for windowscsehelper.ps1, windowscsehelper.tests.ps1, and windowsnodereset.ps1
  • Added Pester coverage for immediate /healthz success, retry through transient failures, exhausted retries, and skipping the health check when the reset task itself fails

NSSM Running/Paused does not mean kubelet initialized. Poll local
/healthz after k8s-restart-job so CSE tolerates the containerd pipe
race and fails before RP node readiness when kubelet never comes up.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@github-actions

github-actions Bot commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Windows Unit Test Results

  3 files   13 suites   54s ⏱️
405 tests 405 ✅ 0 💤 0 ❌
408 runs  408 ✅ 0 💤 0 ❌

Results for commit 07b5ffc.

Copilot AI 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.

Pull request overview

Adds reliable Windows kubelet health validation after node reset.

Changes:

  • Polls kubelet /healthz with bounded retries.
  • Makes kubelet startup failures terminating while restoring HNS remediation.
  • Adds Pester coverage for success, retry, exhaustion, and task failure.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
staging/cse/windows/provisioningscripts/windowsnodereset.ps1 Improves kubelet startup handling and cleanup.
parts/windows/windowscsehelper.ps1 Replaces service-state validation with health polling.
parts/windows/windowscsehelper.tests.ps1 Tests the new health-check behavior.

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@r2k1 r2k1 closed this Sep 2, 2026
@r2k1
r2k1 deleted the r2k1-windows-kubelet-readiness branch September 2, 2026 01:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants