Repository navigation
Fix GPU pod detection for native sidecars during driver upgrades - #3028
Conversation
📝 WalkthroughWalkthrough
Priority: ⬇️ Low Merge Risk: ⚪ Minimal · up to A native-sidecar MIG-limit test would protect this behavior from regression. The supplied evidence shows no current production failure, so this test improvement does not block merging.
Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
cmd/gpu-operator/main_test.go (1)
211-240: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd a native-sidecar MIG-limit case.
The changed tests cover a native sidecar with a MIG request, but not one with a MIG limit. A regression that removes the native-sidecar
Limitscheck would pass the new test. The ordinary-container MIG-limit test does not cover that branch.Suggested fix
{ name: "running pod with MIG request only in native sidecar -> GPU pod", pod: podWithInitContainer(corev1.PodRunning, nativeSidecar(nil, corev1.ResourceList{"nvidia.com/mig-1g.5gb": {}})), wantGPUPod: true, }, + { + name: "running pod with MIG limit only in native sidecar -> GPU pod", + pod: podWithInitContainer(corev1.PodRunning, nativeSidecar(corev1.ResourceList{"nvidia.com/mig-1g.5gb": {}}, nil)), + wantGPUPod: true, + },
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository: NVIDIA/gpu-operator/.coderabbit.yaml
- Review profile: QUIET
- Plan: Enterprise
- Run ID:
36b8feb3-700b-4a14-8bc8-0a12613c0a7b
📒 Files selected for processing (2)
cmd/gpu-operator/main.gocmd/gpu-operator/main_test.go
Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 11 remain after this review.
|
@shin4141 thanks for your contribution! can you rebase and make sure commit is signed using a GPG key. |
f4d11f6 to
6eded4d
Compare
|
/ok to test 6eded4d |
|
@shin4141 could you rebase this PR? |
Signed-off-by: Nagata Shinichi <128954611+shin4141@users.noreply.github.com>
6eded4d to
0cd4c46
Compare
|
/ok-to-test 0cd4c46 |
Description
Addresses #3026. During a driver upgrade,
gpuPodSpecFilterchecks GPU resources inpod.Spec.Containersbut misses native sidecars inpod.Spec.InitContainers(restartPolicy: Always). A pod whose main container has no GPU request and whose native sidecar requests a GPU can therefore be skipped by pod deletion.Check GPU and MIG limits and requests for native sidecars using the existing resource predicate. The ordinary container check, phase gate, DRA fallback, and handling of regular init containers remain unchanged.
Known limitation: A Pending pod with a currently running regular (non-sidecar) GPU init container is still not selected. This is an existing behavior outside this change. Scanning all init containers unconditionally would also select Running pods whose GPU init has already completed; a status-aware fix needs separate decisions for missing or stale status and retries. Please advise whether that case should be tracked separately.
Checklist
make lint) — not run locally;golangci-lintis unavailable in this environmentmake validate-generated-assets) — not run; no generated assets changedmake validate-modules) — not run; no module or vendor files changedTesting
GOTOOLCHAIN=local GOFLAGS=-mod=vendor go test -count=1 ./cmd/gpu-operator ./controllerspassed: 20 operator subtests and 387 controller subtests (407 total). Run with Go 1.26.3 on Darwin arm64 using vendored dependencies.-racefilter tests passed (20 subtests) on the same production logic before the final comment-only edit. The final two-file diff passesgofmtandgit diff --check.