fix: align DRA driver DaemonSet/DeviceClass with AutoPartition (k8s-gpu-dra-driver#88) - #648
Open
yansun1996 wants to merge 3 commits into
Open
yansun1996 wants to merge 3 commits into
yansun1996 wants to merge 3 commits into
Conversation
The AMD GPU DRA driver added AutoPartition: it advertises every valid compute+memory partition configuration as a synthetic device and dynamically repartitions the GPU via amd-smi when a matching ResourceClaim is prepared, hardware-validated on 8xMI300X. The operator vendors its own copies of the DRA driver DaemonSet spec (internal/plugin/plugin.go) and default DeviceClass (helm-charts-k8s/templates/dra-driver-deviceclass.yaml) rather than consuming the upstream chart directly, and those copies had fallen out of sync: - DeviceClass: split gpu.amd.com into gpu.amd.com (unconstrained) and gpu.amd.com-spx (extendedResourceName, constrained to computePartition == spx) when draDriver.deviceClass.autoPartition is set, so a classic amd.com/gpu request can't resolve to a leftover partition fraction. Default behavior unchanged aside from gaining extendedResourceName support. Ported into both the generated chart and hack/k8s-patch/template-patch/dra-driver-deviceclass.yaml (the true source make helm-k8s copies over it from). - SetDRADriverAsDesired: set KMM_DRIVER_ENABLED=true when utils.ShouldUseKMM(devConfig), mirroring the existing KMM node-selector wiring. The driver reads this via os.Getenv directly, so cmdLineArguments (which only becomes CLI args) could never set it. - SetDRADriverAsDesired: mount /lib/modules (read-only) unconditionally -- both of the driver's amdgpu reload paths shell out to modprobe, which needs it regardless of KMM. Also includes the regenerated helm-docs README table rows for the two new values, and picks up unrelated pre-existing generated-artifact drift exposed by re-running the generators (k8s.io/api Toleration doc-comment wording, an import alias normalization, mockgen method ordering, and stale timestamps in Chart.lock/the bundle CSV). Co-Authored-By: Claude <noreply@anthropic.com>
make all re-run on this repo's own Makefile (DOCKER_REGISTRY defaults to docker.io/rocm here, vs. registry.test.pensando.io upstream). The prior commit's bundle CSV containerImage field had leaked the pensando-internal registry (copied verbatim from the upstream repo's already-regenerated tree); this run corrects it to docker.io/rocm/amd-gpu-operator:dev via this repo's own generators, plus its accompanying createdAt bump. Also commits the other artifacts a full generate/manifests/helm-k8s pass produces on top of the prior commit: mockgen/controller-gen tool-version output (method ordering, copyright header handling), gofmt struct-literal realignment in upgrademgr.go, and the Chart.lock timestamp bump from the redone helm dependency update. Co-Authored-By: Claude <noreply@anthropic.com>
yansun1996
force-pushed
the
align-autopartition-pr88
branch
from
September 22, 2026 11:12
5cb801f to
45e40b4
Compare
…en regen make copyrights (a required check) failed on the previous commit: the pinned mockgen@v0.3.0/controller-gen@v0.17.0 versions strip the header on regeneration instead of preserving it. Re-run make copyrights && make fmt and commit the restored headers so the check passes. Co-Authored-By: Claude <noreply@anthropic.com>
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
ROCm/k8s-gpu-dra-driver#88 added the AutoPartition feature to the DRA driver (hardware-validated on 8xMI300X, merged as
f93028f). The operator vendors its own copies of the DRA driver DaemonSet spec (internal/plugin/plugin.go) and defaultDeviceClass(helm-charts-k8s/templates/dra-driver-deviceclass.yaml) rather than consuming the upstream chart directly, and those copies predate AutoPartition. This PR brings them into alignment.gpu.amd.cominto an unconstrained class + a newgpu.amd.com-spx(carryingextendedResourceName, constrained tocomputePartition == "spx") whendraDriver.deviceClass.autoPartitionis set — otherwise a classicamd.com/gpurequest can resolve to a leftover partition fraction from a released claim (same bug PR Utils Container Openshift Security Context Constraint access #88 fixed upstream, reproduced independently here since the operator's copy never got that fix). Default rendering only gainsextendedResourceNamesupport (previously absent), nothing else changes.SetDRADriverAsDesired: setsKMM_DRIVER_ENABLED=trueon the DRA driver container whenutils.ShouldUseKMM(devConfig), mirroring the existing KMM node-selector wiring. The driver reads this viaos.Getenvdirectly, so the existingcmdLineArgumentspassthrough (CLI args only) could never set it.SetDRADriverAsDesired: mounts/lib/modules(read-only) unconditionally — both of the driver's amdgpu reload paths shell out tomodprobe, which needs it regardless of KMM.Enabling
AutoPartitionitself needs no operator code change —--feature-gates/FEATURE_GATESis a real CLI flag with an env alias, socmdLineArguments: {feature-gates: "AutoPartition=true"}already reaches it through the existing generic passthrough.Same change as pensando/gpu-operator#1678, raised separately (and labeled
No-DoubleCommitthere) since that repo has diverged from this one.Test plan
go build ./...cleango vet ./internal/... ./api/...cleango test ./internal/plugin/...— added coverage for/lib/modulesmount (always present) andKMM_DRIVER_ENABLED(set iffShouldUseKMM, including the pf-passthrough exclusion)helm templateondra-driver-deviceclass.yamlverified byte-for-byte withautoPartition: false(aside from the newextendedResourceNamefield) and verified the two-class split renders correctly withautoPartition: trueDeviceConfigreconciliation path (this closes the gaps found while doing that live validation manually in feat: auto-partition (dynamic GPU repartitioning) — hardware-validated k8s-gpu-dra-driver#88; a follow-up hardware pass through the operator itself is recommended before general release)🤖 Generated with Claude Code