Skip to content

[CP 1595] fix(kmm): gate ContainerStatusUnknown pod deletion on node readiness#599

Merged
spraveenio merged 2 commits into
ROCm:mainfrom
ci-penbot-01:CP.O2O.pensando.gpu-operator.1595.rocm.gpu-operator.main
Jul 11, 2026
Merged

[CP 1595] fix(kmm): gate ContainerStatusUnknown pod deletion on node readiness#599
spraveenio merged 2 commits into
ROCm:mainfrom
ci-penbot-01:CP.O2O.pensando.gpu-operator.1595.rocm.gpu-operator.main

Conversation

@ci-penbot-01

@ci-penbot-01 ci-penbot-01 commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

cp of pensando/gpu-operator#1595


Source PR Description (pensando/gpu-operator#1595):

Summary

  • Gate KMM build/sign pod deletion on the node Ready condition to prevent a FailedMount race
  • When the node is NotReady, skip deletion — the pod transitions to Failed once the node recovers, then KMM spawns its replacement cleanly
  • Correct the kmm.node.kubernetes.io/pod-type label value the watcher matches on (see below)

Root Cause

Eager deletion of ContainerStatusUnknown build pods while the node is still recovering from NotReady causes KMM to immediately spawn a replacement build pod before the kubelet's volume registry re-syncs. The replacement fails with FailedMount: object not registered on the Dockerfile ConfigMap.

Observed: CI job 32149765, test_driver_upgrade_cycle[30.30], node asrock-126-b3-1b (MI300X, ubuntu-22.04, kernel 6.8.0-100-generic).

Label-value correction (found in review)

The gate and the pre-existing hasExpectedPodLabel watch predicate both matched pod-type == "builder". The deployed KMM (ROCm fork, release-v1.5.0) actually labels build pods pod-type=build and sign pods pod-type=sign (PodTypeBuild/PodTypeSign in KMM internal/utils/podhelper.go). "builder" never matched, so the auto-remove-unknown-pod feature (originally PR #133) and its watch predicate were dead code against ROCm KMM. Both sites are corrected via shared local constants (KMM's constant lives in a non-importable internal package).

Sign pods included

Sign pods are real in this deployment (secure-boot signing via the ImageSign spec) and share KMM's identical recreate-if-missing lifecycle (internal/sign/pod/manager.go Sync). The gate and the watch predicate now 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 race does not apply, and they have a dedicated lifecycle path respawned by our own workermgr reconcile.

Plan

Plan: docs-internal/knowledge/plans/2026-07-10-kmm-build-pod-node-readiness-gate.md

Test plan

  • go build ./internal/controllers/... and go vet ./internal/controllers/watchers/... pass
  • Re-run test_driver_upgrade_cycle[30.30] on asrock-host to confirm no FailedMount on build pod replacement

Jira: GPUOP-980

🤖 Generated with Claude Code

Cherrypick triggered by: ACP-Automation

yansun1996 and others added 2 commits July 10, 2026 22:09
…(#1595)

* 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 <noreply@anthropic.com>

* fix(kmm): gate ContainerStatusUnknown pod deletion on node readiness

Co-Authored-By: Claude <noreply@anthropic.com>

* 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 <noreply@anthropic.com>

* 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 ROCm#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 <noreply@anthropic.com>

* 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 <noreply@anthropic.com>

---------

Co-authored-by: Claude <noreply@anthropic.com>
(cherry picked from commit f25b3614fe194aa5498af3f9bd7a7b16a91ec1c1)

@spraveenio spraveenio left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@spraveenio spraveenio merged commit 975193a into ROCm:main Jul 11, 2026
4 checks passed
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.

3 participants