fix(aks): cordon undrainable nodes instead of failing pool upgrades

[Lytol]

Description

The AKS step declares upgradeSettings with only maxSurge, and upgradeSettings is not in the step's ignoreChanges list (lib/steps/aks.go:167-187). Pulumi therefore owns the whole object and resets drainTimeoutInMinutes and undrainableNodeBehavior to null on every apply.

With those fields null, an agent pool becomes permanently wedged if it hosts a pod that cannot be evicted. Workbench components get a PodDisruptionBudget with maxUnavailable: 0 per component, which makes disruptionsAllowed structurally zero — the API server then rejects every eviction for the selected pods (HTTP 429, "Cannot evict pod as it would violate the pod's disruption budget"). A node running a Workbench session is consequently undrainable indefinitely.

When an automatic node-image upgrade hits such a node it fails the whole operation after the 30 minute default drain timeout, leaving the pool in provisioningState: Failed. AKS then retries on a daily cadence: it surges a new node, cordons the old one — so newly launched pods land on the surge node — times out draining, and deletes the surge instance at the VMSS layer, bypassing the Eviction API, terminating whatever was running on it. The pool stays Failed indefinitely with its nodes pinned to a stale image, and long-running Workbench background jobs are killed off by the repeating surge/teardown cycle.

Code Flow

undrainableNodeBehavior: Cordon makes AKS cordon and skip an undrainable node rather than failing the operation, which stops both the wedge and the repeating surge/teardown loop. drainTimeoutInMinutes: 10 aligns with the cluster autoscaler's default maxGracefulTerminationSec of 600s, so both subsystems allow the same graceful-shutdown budget instead of the 30 minute default stalling every upgrade.

Applied at both declaration sites. The per-pool AgentPool resources are the load-bearing case. The system node pool profile on the ManagedCluster is also set, but "agentPoolProfiles" is unconditionally added to ignoreChanges (:189-193) — ResolveUserNodePools errors when user_node_pools is empty, so that branch always fires — meaning the system-pool values apply only at cluster creation. Existing clusters' system pools are unaffected; this is recorded in a code comment so the next reader isn't misled. maxSurge unchanged at 10%.

Field names and types verified against pulumi-azure-native-sdk/containerservice/v3@v3.16.0 (pulumiTypes.go:2099): UndrainableNodeBehavior is pulumi.StringPtrInput, DrainTimeoutInMinutes is pulumi.IntPtrInput; REST API version 2025-09-01 supports both. Uses the typed constant containerservice.UndrainableNodeBehaviorCordon, consistent with OSSKUUbuntu / ScaleDownModeDelete elsewhere in the file. upgradeSettings updates in place and does not force pool replacement.

Hardcoded, not configurable

Consistent with maxSurge, ScaleDownMode, OsSKU and the other upgrade-path settings here, all hardcoded. The legacy null behaviour has no scenario in which it is preferable, so a per-workload knob would only create a way to stay broken. These settings also have to be correct for automatic node-image upgrades, which run with nobody invoking ptd ensure — an opt-in would protect only workloads someone remembered to configure. Adding drain_timeout_minutes to AzureUserNodePoolConfig later is a small additive change if a real need appears.

Caveat: cordoned nodes are released by session lifetime, not by the upgrade

Cordon leaves the undrainable node running and cordoned rather than replacing it. It is released only once its blocking pods clear — for Workbench, the session's own kill/suspend timeout.

• These settings govern unattended AKS-initiated upgrades (NodeOSUpgradeChannel: NodeImage, UpgradeChannel: Patch, :243-246), not just operator-run ptd ensure.
• AKS does not uncordon skipped nodes. A cordoned node still counts toward the pool's MaxCount for the cluster autoscaler, so accumulation reduces schedulable capacity and can stop scale-out with no alert and nothing visible in a Pulumi preview.
• A cordoned node still bills, and since the autoscaler counts it as unavailable it scales up a replacement, so capacity overlaps.
• It keeps the old node image, so CVE patches are deferred on it. Previously this failed loudly (pool Failed); now it is quiet. Because AKS reports the operation Succeeded and bumps the pool-level orchestratorVersion, skipped nodes are invisible to subsequent previews and can drift toward the kubelet version-skew boundary while the pool looks healthy. Monitoring on cordoned/quarantined node count is a sensible follow-up.
• Accumulation is bounded in practice by session lifetime and pool MaxCount, but a workload with no session kill-timeout could hold a node cordoned indefinitely.

Still strictly better than the status quo, which doesn't merely strand a node but wedges the pool and runs a repeating loop destroying surge instances out from under running pods.

10 minutes is the conservative choice against the 30 minute default; 5 is the more aggressive option if upgrades stall on blocked nodes.

Category of change

• Bug fix (non-breaking change which fixes an issue)

Checklist

• I have reviewed my own diff

[claude]

Claude finished @Lytol's task in 2m 7s —— View job

---

Review

• Read review guidelines and changed files
• Analyze the diff
• Post review feedback

---

The fix is minimal and correct. Two observations worth noting:

User pools (load-bearing case): The AgentPool resources at :373-377 are managed as separate Pulumi resources and are not covered by ignoreChanges, so DrainTimeoutInMinutes and UndrainableNodeBehavior will take effect on the next ptd ensure for existing clusters. This is the right path for the Workbench pod disruption scenario.

System pool comment accuracy: The comment at :165-167 correctly states that the system pool values only apply at cluster creation because agentPoolProfiles is always in ignoreChanges. However, the reason it's always in ignoreChanges is indirect: ResolveUserNodePools() errors when user_node_pools is empty (:728-731), so execution never reaches the if len(userNodePools) > 0 guard at :195 with a false condition. The comment is accurate, but a reader following the code path has to connect those two dots. Not a blocker — the existing if len(userNodePools) > 0 condition is pre-existing — just worth knowing if the guard ever changes.

pulumi.String(containerservice.UndrainableNodeBehaviorCordon) vs typed input: This matches the pattern used throughout the file for OSSKUUbuntu, ScaleDownModeDelete, etc. Consistent.

No issues with the approach, the field names, or the 10-minute drain timeout rationale. The caveats section of the PR description (cordoned nodes still count toward MaxCount, still bill, patch deferral) is thorough — consider filing a follow-up ticket for the cordoned-node monitoring suggestion if one doesn't exist.