Skip to content

feat(deviceplugin): accept the product resource naming strategy - #666

Draft
karlkfi wants to merge 1 commit into
ROCm:mainfrom
karlkfi:karlkfi/accept-product-naming-strategy
Draft

karlkfi wants to merge 1 commit into
ROCm:mainfrom
karlkfi:karlkfi/accept-product-naming-strategy

Conversation

@karlkfi

@karlkfi karlkfi commented Oct 14, 2026

Copy link
Copy Markdown

Depends on ROCm/k8s-device-plugin#246. Draft until that merges and a plugin image carrying the strategy ships.

Follow-up to #663, which was the other half: that one makes the driver-upgrade eviction predicate recognise the new resource names, this one makes the operator accept the flag that produces them. #663 stands alone and is correct against the plugin as shipped today — it also fixes nps2/nps8, which miss right now — so it is not blocked on anything.

Problem

The operator keeps its own copy of the accepted resource_naming_strategy values. A DeviceConfig setting product fails ValidateDevicePluginSpec before any pod ever advertises the new name, so the strategy is unreachable through DeviceConfig no matter what the device plugin supports.

Changes

  • internal/utils.go — ProductStrategy constant, alongside SingleStrategy and MixedStrategy.
  • internal/validator/specValidators.go:160 — added to the allow-list.
  • api/v1alpha1/deviceconfig_types.go:477 — the doc comment that kubectl explain and the OLM bundle report.
  • Four generated artifacts carrying that comment: config/crd/bases/, config/manifests/bases/, both bundle/manifests/, and helm-charts-k8s/crds/.
  • Two hand-maintained values.yaml comments that enumerate the same values: hack/k8s-patch/metadata-patch/ (source) and helm-charts-k8s/ (its copy).
  • internal/plugin/plugin.go:112-119 — the PF/VF passthrough force-default used bare "resource_naming_strategy" and "mixed" literals rather than the utils constants, so a grep-driven update of those constants missed it. No behaviour change; it still defaults to mixed.

On regenerating

The CRD artifacts were regenerated with controller-gen crd directly rather than make manifests. That target also runs update-registry and update-version, which sed image tags and project versions across the tree — unrelated churn that would bury a one-line doc change. The remaining artifacts were edited to match; each is a single line.

Testing

go build ./..., go vet ./internal/... and go test ./internal/... pass.

internal/validator/ had no test file. Added specValidators_test.go covering the three strategies and two negatives. Reverting the allow-list entry fails it on exactly the product case and nothing else:

ValidateDevicePluginSpec accepts every naming strategy the device plugin implements [It] product
  [FAILED] Expected success, but got an error:
      Invalid flag value: resource_naming_strategy=product. Supported values: [single mixed]

Not exercised against a live cluster or an OLM install.

Sequencing

Merging this before a plugin image carrying product is published would let a DeviceConfig request a value the plugin exits at startup on. The operator's default is the floating rocm/k8s-device-plugin:latest (internal/plugin/plugin.go:55), so users on the default pick the support up with the image; users pinning an older tag would get a crash-looping device plugin container until they move. That is the reason this is split out of #663 rather than folded into it.

The device plugin gains a third -resource_naming_strategy value, product,
in ROCm/k8s-device-plugin#246, which names a whole GPU for the product it
is rather than for its partition style: amd.com/gpu-mi300x against
amd.com/spx_nps1. A cluster whose nodes hold more than one GPU product
cannot express per-product quota or scheduling without it.

The operator keeps its own copy of the accepted values, so a DeviceConfig
setting resource_naming_strategy: product fails ValidateDevicePluginSpec
before any pod ever advertises the new name. Add the value to that list,
and to the CRD doc comment that advertises it to kubectl explain and the
OLM bundle.

The doc comment generates into four artifacts, regenerated here with
controller-gen rather than `make manifests`, which also seds image tags
and project versions across the tree and would bury the change. The two
values.yaml comments are hand-maintained and updated to match.

While here, internal/plugin/plugin.go force-defaults the flag for PF and
VF passthrough using bare string literals rather than the utils
constants, so a grep-driven update of those constants misses it. Switched
to the constants. No behaviour change: it still defaults to mixed.

Adds specValidators_test.go, which had no coverage. The strategy table
fails on exactly the product entry when the allow-list is reverted.

Depends on ROCm/k8s-device-plugin#246. Merging this before a plugin image
carrying the strategy ships would let a DeviceConfig request a value the
plugin exits at startup on, so this should land after that PR and after
an image is published.

Signed-off-by: Karl Isenberg <kisenber@amd.com>

This branch has not been deployed

No deployments
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.

1 participant