From 155644a4ec63bc953b3084d2d033a6b63c86671e Mon Sep 17 00:00:00 2001 From: Yan Sun Date: Fri, 10 Jul 2026 15:08:52 -0700 Subject: [PATCH 1/2] fix(kmm): gate ContainerStatusUnknown pod deletion on node readiness (#1595) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(kmm): gate ContainerStatusUnknown pod deletion on node readiness When a KMM build pod hits ContainerStatusUnknown due to a NodeNotReady event, deleting it immediately causes KMM to spawn a replacement build pod before the kubelet's volume registry has re-synced. The replacement then fails with FailedMount on the Dockerfile ConfigMap (the ConfigMap exists in etcd but the kubelet's in-memory volume plugin registry is stale from the network partition window). Gate the deletion on the node's current Ready condition. If the node is still NotReady, skip deletion — the pod will transition to Failed once the node recovers, firing another Update event at which point deletion proceeds and KMM's replacement starts cleanly. Observed in CI job 32149765 (test_driver_upgrade_cycle[30.30], asrock-126-b3-1b, ubuntu-22.04, kernel 6.8.0-100-generic). Plan: docs-internal/knowledge/plans/2026-07-10-kmm-build-pod-node-readiness-gate.md Co-Authored-By: Claude * fix(kmm): gate ContainerStatusUnknown pod deletion on node readiness Co-Authored-By: Claude * fix(kmm): address copilot review comments - Scope node-readiness check to KMM builder pods only (pod-type=builder) - Add node name to node GET error log - Use structured logging for delete failure Co-Authored-By: Claude * fix(kmm): correct build pod-type label value from "builder" to "build" The build-pod gate and the pre-existing hasExpectedPodLabel watch predicate both matched kmm.node.kubernetes.io/pod-type == "builder". The deployed KMM (ROCm fork, release-v1.5.0) labels build pods pod-type=build (PodTypeBuild in internal/utils/podhelper.go); "builder" never matched, so both the node-readiness gate and the original auto-remove-unknown-build-pod feature (PR #133) were dead code against ROCm KMM. Correct both sites to "build" via shared local constants. KMM's constant lives in an internal, non-importable package, so the value is duplicated. Addresses review feedback from spraveenio on PR #1595. Co-Authored-By: Claude * fix(kmm): extend ContainerStatusUnknown node-readiness gate to sign pods Sign pods share KMM's recreate-if-missing lifecycle (internal/sign/pod/manager.go Sync respawns a missing sign pod exactly as the build manager does) and are real in this deployment via the ImageSign secure-boot spec. Extend the gate and the watch predicate to admit pod-type in {build, sign} via a shared isKMMBuildOrSignPod helper. Worker (workerMgr) pods are intentionally excluded: they mount HostPath volumes, not the Dockerfile ConfigMap, so the FailedMount race does not apply, and they have a dedicated lifecycle path respawned by our own workermgr reconcile. Co-Authored-By: Claude --------- Co-authored-by: Claude (cherry picked from commit f25b3614fe194aa5498af3f9bd7a7b16a91ec1c1) --- ...07-10-kmm-build-pod-node-readiness-gate.md | 92 +++++++++++++++++++ internal/controllers/watchers/pod.go | 58 ++++++++++-- 2 files changed, 140 insertions(+), 10 deletions(-) create mode 100644 docs-internal/knowledge/plans/2026-07-10-kmm-build-pod-node-readiness-gate.md diff --git a/docs-internal/knowledge/plans/2026-07-10-kmm-build-pod-node-readiness-gate.md b/docs-internal/knowledge/plans/2026-07-10-kmm-build-pod-node-readiness-gate.md new file mode 100644 index 000000000..a691ca19f --- /dev/null +++ b/docs-internal/knowledge/plans/2026-07-10-kmm-build-pod-node-readiness-gate.md @@ -0,0 +1,92 @@ +# Plan: Gate KMM Build Pod Deletion on Node Readiness + +## Context + +When upgrading the amdgpu driver via KMM (Kernel Module Management), Kaniko build pods +occasionally hit `ContainerStatusUnknown` due to a `NodeNotReady` event (e.g., network +instability). The pod watcher at `internal/controllers/watchers/pod.go` was deleting these +pods unconditionally and immediately. + +This eager deletion triggers KMM to spawn a replacement build pod before the kubelet's +volume registry has re-synced from the `NodeNotReady` window. The replacement pod then +fails immediately with: + +``` +FailedMount: MountVolume.SetUp failed for volume "dockerfile": +object "kube-amd-gpu"/"ubuntu-22.04-devcfg-clusterwide-gpu-kube-amd-gpu" not registered +``` + +The Dockerfile ConfigMap is not lost — it persists in etcd, owned by the DeviceConfig. +The error is a kubelet VolumeManager informer-sync issue: the kubelet's in-memory volume +registry has not yet re-populated after the node recovered. + +Observed in CI job 32149765 on node asrock-126-b3-1b (MI300X, ubuntu-22.04, kernel +6.8.0-100-generic) during `test_driver_upgrade_cycle[30.30]`. The build pod ran for ~20 +min, was killed, and the replacement hit FailedMount immediately. The node remained in +`Upgrade-Started` for the full 20-min polling window, causing a test timeout and cascading +failures on subsequent tests. + +## Approach + +Gate the `ContainerStatusUnknown` pod deletion on the node's current Ready condition: + +- Before deleting, fetch the node by `pod.Spec.NodeName`. +- If the node is not Ready, skip deletion and log at Info level. The pod watcher will + fire again on the next pod status update (when the kubelet re-syncs and transitions + the pod to Failed), at which point the node is Ready and deletion proceeds cleanly. +- If the node is Ready, the `ContainerStatusUnknown` is a genuine stuck state (not a + transient network partition artifact) — delete as before. + +This is a structural fix: it eliminates the race rather than racing to enqueue a reconcile. + +**Label-value correction (found in review, PR #1595):** The build-pod gate and the +pre-existing `hasExpectedPodLabel` predicate both matched +`kmm.node.kubernetes.io/pod-type == "builder"`. The deployed KMM (ROCm fork, +`release-v1.5.0`) actually labels build pods `pod-type=build` (`PodTypeBuild = "build"` +in KMM `internal/utils/podhelper.go`; sign pods are `"sign"`). `"builder"` never +matched, so the auto-remove-unknown-build-pod feature (originally PR #133) and its +watch predicate were dead code against ROCm KMM. Corrected both sites via shared +local constants (`kmmPodTypeLabelKey`, `kmmPodTypeBuildValue`, `kmmPodTypeSignValue`); +KMM's constant is in an internal, non-importable package so the values are duplicated. +This activates the feature, and the node-readiness gate above makes that activation safe. + +**Sign pods included:** Sign pods are real in this deployment (secure-boot signing via +the `ImageSign` spec on DeviceConfig) and share KMM's identical recreate-if-missing +lifecycle — `internal/sign/pod/manager.go` `Sync` recreates a missing sign pod exactly +as the build manager does. So the same delete-to-retrigger + node-readiness gate applies. +Both the gate and the watch predicate now admit `pod-type in {build, sign}` via the +shared `isKMMBuildOrSignPod` helper. Worker (workerMgr) pods are intentionally excluded: +they mount `HostPath` volumes (not the Dockerfile ConfigMap), so the FailedMount race +does not apply, and they already have a dedicated lifecycle path (`handleWorkerMgrPodEvt`) +respawned by our own workermgr reconcile rather than KMM's `Sync`. + +**Alternatives considered:** +- Enqueue a DeviceConfig reconcile after deletion to re-assert the ConfigMap: rejected + because the race between the enqueue and KMM's pod creation cannot be closed reliably. +- Add `Owns(&v1.ConfigMap{})` watch: useful defense-in-depth for accidental ConfigMap + deletion but does not address the kubelet informer-sync issue; deferred. + +## Scope + +- **In scope:** `internal/controllers/watchers/pod.go` — node-readiness check before + `ContainerStatusUnknown` deletion of KMM build and sign pods, plus the pod-type + label-value correction (build/sign) in the gate and watch predicate. +- **Out of scope:** worker (workerMgr) pods (different volume/lifecycle), test timeout + adjustment (separate PR), ConfigMap watch defense-in-depth. + +## Validation + +- `go build ./internal/controllers/watchers/...` passes. +- Unit test: add a test for `PodEventHandler.Update` covering ContainerStatusUnknown + + NodeNotReady asserting `Delete` is NOT called (follow-up). +- Integration: re-run `test_driver_upgrade_cycle[30.30]` on asrock-126-b3-1b to confirm + the build pod is no longer killed and replaced mid-build during a NodeNotReady event. + +## Risks / Rollback + +- **Risk:** If a node enters a permanently-NotReady state, the ContainerStatusUnknown pod + is never deleted by this watcher. However, Kubernetes' pod eviction controller + (`pod-eviction-timeout`, default 5 min) will evict it independently, and KMM's own + reconcile loop will handle the replacement cleanly after eviction. +- **Rollback:** Revert `internal/controllers/watchers/pod.go` to restore the original + unconditional deletion behavior. diff --git a/internal/controllers/watchers/pod.go b/internal/controllers/watchers/pod.go index d4685bc3e..52a9d9954 100644 --- a/internal/controllers/watchers/pod.go +++ b/internal/controllers/watchers/pod.go @@ -51,6 +51,24 @@ import ( "sigs.k8s.io/controller-runtime/pkg/reconcile" ) +// KMM labels its helper pods with kmm.node.kubernetes.io/pod-type. Build pods +// carry the value "build" and sign pods "sign" in the deployed ROCm KMM fork; +// the package that defines these is internal to KMM and not importable, so the +// values are duplicated here. +const ( + kmmPodTypeLabelKey = "kmm.node.kubernetes.io/pod-type" + kmmPodTypeBuildValue = "build" + kmmPodTypeSignValue = "sign" +) + +// isKMMBuildOrSignPod reports whether the pod is a KMM build or sign helper pod. +// Both share the same recreate-if-missing lifecycle (KMM's Sync respawns them), +// so both can be safely deleted to retrigger KMM. +func isKMMBuildOrSignPod(labels map[string]string) bool { + podType := labels[kmmPodTypeLabelKey] + return podType == kmmPodTypeBuildValue || podType == kmmPodTypeSignValue +} + //go:generate mockgen -source=pod.go -package=watchers -destination=mock_pod.go PodEventHandlerAPI type PodEventHandlerAPI interface { Create(ctx context.Context, evt event.CreateEvent, q workqueue.TypedRateLimitingInterface[reconcile.Request]) @@ -116,14 +134,35 @@ func (h *PodEventHandler) Update( } logger := log.FromContext(ctx) - // if any builder pod container status went to ContainerStatusUnknown, delete the pod - // otherwise the build pod won't be automatically retriggered - for _, containerStatus := range pod.Status.ContainerStatuses { - if containerStatus.State.Waiting != nil && containerStatus.State.Waiting.Reason == "ContainerStatusUnknown" { - if err := h.client.Delete(ctx, pod); err != nil { - logger.Error(err, fmt.Sprintf("failed to delete ContainerStatusUnknown pod %+v", pod.GetName())) + // Delete ContainerStatusUnknown build/sign pods to retrigger KMM, but only when the node + // is Ready — deleting while NotReady causes KMM to spawn a replacement before the kubelet + // volume registry re-syncs, producing a FailedMount on the Dockerfile ConfigMap. + if isKMMBuildOrSignPod(pod.Labels) { + for _, containerStatus := range pod.Status.ContainerStatuses { + if containerStatus.State.Waiting != nil && containerStatus.State.Waiting.Reason == "ContainerStatusUnknown" { + node := &v1.Node{} + if err := h.client.Get(ctx, types.NamespacedName{Name: pod.Spec.NodeName}, node); err != nil { + logger.Error(err, "failed to get node for ContainerStatusUnknown pod, skipping deletion", + "pod", pod.GetName(), "node", pod.Spec.NodeName) + break + } + nodeReady := false + for _, cond := range node.Status.Conditions { + if cond.Type == v1.NodeReady && cond.Status == v1.ConditionTrue { + nodeReady = true + break + } + } + if !nodeReady { + logger.Info("skipping ContainerStatusUnknown pod deletion: node is not Ready, will retry when node recovers", + "pod", pod.GetName(), "node", pod.Spec.NodeName) + break + } + if err := h.client.Delete(ctx, pod); err != nil { + logger.Error(err, "failed to delete ContainerStatusUnknown pod", "pod", pod.GetName()) + } + break } - break } } @@ -189,8 +228,7 @@ func hasExpectedPodLabel(obj metav1.Object) bool { if labels == nil { return false } - value := labels["kmm.node.kubernetes.io/pod-type"] - isKMMBuilder := value == "builder" + isKMMBuildOrSign := isKMMBuildOrSignPod(labels) _, isWorkerMgrPod := labels[utils.WorkerActionLabelKey] - return isKMMBuilder || isWorkerMgrPod + return isKMMBuildOrSign || isWorkerMgrPod } From 2a735895ed2db841f1f684e8dc163620dc406084 Mon Sep 17 00:00:00 2001 From: Praveen Kumar Shanmugam <58961022+spraveenio@users.noreply.github.com> Date: Fri, 10 Jul 2026 16:08:45 -0700 Subject: [PATCH 2/2] Delete docs-internal/knowledge/plans/2026-07-10-kmm-build-pod-node-readiness-gate.md --- ...07-10-kmm-build-pod-node-readiness-gate.md | 92 ------------------- 1 file changed, 92 deletions(-) delete mode 100644 docs-internal/knowledge/plans/2026-07-10-kmm-build-pod-node-readiness-gate.md diff --git a/docs-internal/knowledge/plans/2026-07-10-kmm-build-pod-node-readiness-gate.md b/docs-internal/knowledge/plans/2026-07-10-kmm-build-pod-node-readiness-gate.md deleted file mode 100644 index a691ca19f..000000000 --- a/docs-internal/knowledge/plans/2026-07-10-kmm-build-pod-node-readiness-gate.md +++ /dev/null @@ -1,92 +0,0 @@ -# Plan: Gate KMM Build Pod Deletion on Node Readiness - -## Context - -When upgrading the amdgpu driver via KMM (Kernel Module Management), Kaniko build pods -occasionally hit `ContainerStatusUnknown` due to a `NodeNotReady` event (e.g., network -instability). The pod watcher at `internal/controllers/watchers/pod.go` was deleting these -pods unconditionally and immediately. - -This eager deletion triggers KMM to spawn a replacement build pod before the kubelet's -volume registry has re-synced from the `NodeNotReady` window. The replacement pod then -fails immediately with: - -``` -FailedMount: MountVolume.SetUp failed for volume "dockerfile": -object "kube-amd-gpu"/"ubuntu-22.04-devcfg-clusterwide-gpu-kube-amd-gpu" not registered -``` - -The Dockerfile ConfigMap is not lost — it persists in etcd, owned by the DeviceConfig. -The error is a kubelet VolumeManager informer-sync issue: the kubelet's in-memory volume -registry has not yet re-populated after the node recovered. - -Observed in CI job 32149765 on node asrock-126-b3-1b (MI300X, ubuntu-22.04, kernel -6.8.0-100-generic) during `test_driver_upgrade_cycle[30.30]`. The build pod ran for ~20 -min, was killed, and the replacement hit FailedMount immediately. The node remained in -`Upgrade-Started` for the full 20-min polling window, causing a test timeout and cascading -failures on subsequent tests. - -## Approach - -Gate the `ContainerStatusUnknown` pod deletion on the node's current Ready condition: - -- Before deleting, fetch the node by `pod.Spec.NodeName`. -- If the node is not Ready, skip deletion and log at Info level. The pod watcher will - fire again on the next pod status update (when the kubelet re-syncs and transitions - the pod to Failed), at which point the node is Ready and deletion proceeds cleanly. -- If the node is Ready, the `ContainerStatusUnknown` is a genuine stuck state (not a - transient network partition artifact) — delete as before. - -This is a structural fix: it eliminates the race rather than racing to enqueue a reconcile. - -**Label-value correction (found in review, PR #1595):** The build-pod gate and the -pre-existing `hasExpectedPodLabel` predicate both matched -`kmm.node.kubernetes.io/pod-type == "builder"`. The deployed KMM (ROCm fork, -`release-v1.5.0`) actually labels build pods `pod-type=build` (`PodTypeBuild = "build"` -in KMM `internal/utils/podhelper.go`; sign pods are `"sign"`). `"builder"` never -matched, so the auto-remove-unknown-build-pod feature (originally PR #133) and its -watch predicate were dead code against ROCm KMM. Corrected both sites via shared -local constants (`kmmPodTypeLabelKey`, `kmmPodTypeBuildValue`, `kmmPodTypeSignValue`); -KMM's constant is in an internal, non-importable package so the values are duplicated. -This activates the feature, and the node-readiness gate above makes that activation safe. - -**Sign pods included:** Sign pods are real in this deployment (secure-boot signing via -the `ImageSign` spec on DeviceConfig) and share KMM's identical recreate-if-missing -lifecycle — `internal/sign/pod/manager.go` `Sync` recreates a missing sign pod exactly -as the build manager does. So the same delete-to-retrigger + node-readiness gate applies. -Both the gate and the watch predicate now admit `pod-type in {build, sign}` via the -shared `isKMMBuildOrSignPod` helper. Worker (workerMgr) pods are intentionally excluded: -they mount `HostPath` volumes (not the Dockerfile ConfigMap), so the FailedMount race -does not apply, and they already have a dedicated lifecycle path (`handleWorkerMgrPodEvt`) -respawned by our own workermgr reconcile rather than KMM's `Sync`. - -**Alternatives considered:** -- Enqueue a DeviceConfig reconcile after deletion to re-assert the ConfigMap: rejected - because the race between the enqueue and KMM's pod creation cannot be closed reliably. -- Add `Owns(&v1.ConfigMap{})` watch: useful defense-in-depth for accidental ConfigMap - deletion but does not address the kubelet informer-sync issue; deferred. - -## Scope - -- **In scope:** `internal/controllers/watchers/pod.go` — node-readiness check before - `ContainerStatusUnknown` deletion of KMM build and sign pods, plus the pod-type - label-value correction (build/sign) in the gate and watch predicate. -- **Out of scope:** worker (workerMgr) pods (different volume/lifecycle), test timeout - adjustment (separate PR), ConfigMap watch defense-in-depth. - -## Validation - -- `go build ./internal/controllers/watchers/...` passes. -- Unit test: add a test for `PodEventHandler.Update` covering ContainerStatusUnknown + - NodeNotReady asserting `Delete` is NOT called (follow-up). -- Integration: re-run `test_driver_upgrade_cycle[30.30]` on asrock-126-b3-1b to confirm - the build pod is no longer killed and replaced mid-build during a NodeNotReady event. - -## Risks / Rollback - -- **Risk:** If a node enters a permanently-NotReady state, the ContainerStatusUnknown pod - is never deleted by this watcher. However, Kubernetes' pod eviction controller - (`pod-eviction-timeout`, default 5 min) will evict it independently, and KMM's own - reconcile loop will handle the replacement cleanly after eviction. -- **Rollback:** Revert `internal/controllers/watchers/pod.go` to restore the original - unconditional deletion behavior.