Add validation for unsupported DRA features #7226

harche · 2025-10-10T16:54:51Z

What type of PR is this?

/kind bug

What this PR does / why we need it:

This PR adds validation to reject DRA (Dynamic Resource Allocation) v1 API features that are enabled
by default in Kubernetes 1.34 but explicitly excluded from Kueue's DRA support scope per KEP-2941.

Without validation, Kueue silently ignores unsupported DRA features, leading to:

Silent admission failures - FirstAvailable workloads are admitted with 0 DRA resources, causing
runtime failures
Misleading errors - 5 out of 6 unsupported features produce confusing "DeviceClass not mapped"
errors instead of clear feature-not-supported messages
Poor UX - Users waste time debugging DeviceClass mappings when the actual issue is using
unsupported features

Solution is to add early validation in pkg/dra/claims.go to explicitly reject all 6 enabled-by-default DRA features
that Kueue doesn't support:

GA Features:

AllocationMode 'All'
CEL Selectors
Device Constraints
Device Config

Beta Features (enabled by default in K8s 1.34):

FirstAvailable (DRAPrioritizedList)
AdminAccess (DRAAdminAccess)

Each feature now returns a clear, actionable error message like "FirstAvailable device selection is
not supported" instead of misleading DeviceClass mapping errors.

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?

Kueue now properly validates and rejects unsupported DRA (Dynamic Resource Allocation) features with clear error messages instead of silently failing or producing misleading "DeviceClass not mapped" errors. Unsupported features include: AllocationMode 'All', CEL Selectors, Device Constraints, Device Config, FirstAvailable device selection, and AdminAccess.

netlify · 2025-10-10T16:55:17Z

✅ Deploy Preview for kubernetes-sigs-kueue ready!

Name	Link
🔨 Latest commit	`057dd22`
🔍 Latest deploy log	https://app.netlify.com/projects/kubernetes-sigs-kueue/deploys/68fa302c29e15b0008adf648
😎 Deploy Preview	https://deploy-preview-7226--kubernetes-sigs-kueue.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

k8s-ci-robot · 2025-10-10T16:55:20Z

Hi @harche. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

mimowo · 2025-10-10T16:58:27Z

/ok-to-test
@alaypatel07 would you like to give it a pass?

kannon92 · 2025-10-10T17:00:45Z

/ok-to-test

/cc @alaypatel07

alaypatel07 · 2025-10-10T17:11:56Z

Yes I'll take a look.

alaypatel07 · 2025-10-10T17:42:48Z

pkg/dra/claims.go

 		var dcName string
 		var q int64
 		if req.Exactly != nil {
+			// Check for CEL selectors


can you please convert the entire if condition into a switch case handling all the unsupported features first before considering the valid supported case?

Addressed, thanks.

@harche why does this have to be nested ifs and switch cases? Can we work out a logic with one single switch case?

For example:

switch { case req.FirstAvailable != nil: case req.Exactly != nil && len(req.Exactly.Selectors) > 0: // error case req.Exactly != nil && req.Exactly.AdminAccess != nil && *req.Exactly.AdminAccess: // error case req.Exactly != nil && req.Exactly.AllocationMode == DeviceAllocationModeAll: // error }

@alaypatel07 I didn't want to repeat req.Exactly != nil, would something like following do?

switch { case req.FirstAvailable != nil: return nil, fmt.Errorf("%w: FirstAvailable device selection is not supported", ErrUnsupportedDRAFeature) case req.Exactly == nil: continue case len(req.Exactly.Selectors) > 0: return nil, fmt.Errorf("%w: CEL selectors are not supported", ErrUnsupportedDRAFeature) case req.Exactly.AdminAccess != nil && *req.Exactly.AdminAccess: return nil, fmt.Errorf("%w: AdminAccess is not supported", ErrUnsupportedDRAFeature) case req.Exactly.AllocationMode == resourcev1.DeviceAllocationModeAll: return nil, fmt.Errorf("%w: AllocationMode 'All' is not supported", ErrUnsupportedDRAFeature) case req.Exactly.AllocationMode == resourcev1.DeviceAllocationModeExactCount: dcName = req.Exactly.DeviceClassName q = req.Exactly.Count case req.Exactly.AllocationMode == "": dcName = req.Exactly.DeviceClassName default: return nil, fmt.Errorf("%w: unsupported allocation mode: %s", ErrUnsupportedDRAFeature, req.Exactly.AllocationMode) }

Updated, let me know if that works, but I am open to changing it to repeated req.Exactly != nil.

alaypatel07 · 2025-10-10T17:45:55Z

pkg/dra/claims.go

-	ErrDeviceClassNotMapped = errors.New("DeviceClass is not mapped in DRA configuration")
-	ErrResourceClaimInUse   = errors.New("ResourceClaim is in use")
-	ErrClaimSpecNotFound    = errors.New("failed to get claim spec")
+	ErrDeviceClassNotMapped          = errors.New("DeviceClass is not mapped in DRA configuration")


Nit: can we use a common error "UnsupportedDRAfeature" and then wrap a human readable error message to be returned from the function later.

I think we only need to have the following concrete error types:

ErrResourceClaimInUse and other

ErrClaimSpecNotFound

ErrUnsupportedDRAfeature

Addressed, thanks.

alaypatel07 · 2025-10-10T17:47:38Z

pkg/dra/claims_test.go

 			},
 			want: map[kueuev1beta1.PodSetReference]corev1.ResourceList{"main": {"res-1": resource.MustParse("2")}},
 		},
+		{


we should also update the integration tests for these errors at https://github.com/kubernetes-sigs/kueue/tree/main/test/integration/singlecluster/controller/dra

Added integration tests.

harche · 2025-10-10T19:10:44Z

/test pull-kueue-test-e2e-multikueue-main

harche · 2025-10-10T19:13:08Z

@alaypatel07 I have updated the PR. Can you take another look? Thanks.

alaypatel07 · 2025-10-10T20:42:25Z

test/integration/singlecluster/controller/dra/dra_test.go

+			wl := utiltesting.MakeWorkload("test-wl-all-mode", ns.Name).
+				Queue("test-lq").
+				Obj()
+			wl.Spec.PodSets[0].Template.Spec.ResourceClaims = []corev1.PodResourceClaim{


can we use this opportunity to start using, https://github.com/kubernetes-sigs/kueue/blob/main/pkg/util/testing/wrappers.go#L623 and https://github.com/kubernetes-sigs/kueue/blob/main/pkg/util/testing/wrappers.go#L637?

I think this change is needed across all the test cases.

Updated, thanks.

alaypatel07 · 2025-10-10T20:44:33Z

test/integration/singlecluster/controller/dra/dra_test.go

+
+		ginkgo.It("Should reject workload with AllocationMode 'All'", func() {
+			ginkgo.By("Creating a ResourceClaimTemplate with AllocationMode All")
+			rct := &resourcev1.ResourceClaimTemplate{


I wonder if we can create a ResourceClaimWrapper to produce these templates/claims in utiltesting:

kueue/pkg/util/testing/wrappers.go

Line 422 in 5a6b10a

type PodSetWrapper struct{ kueue.PodSet }

it will be helpful in unit testing as well as integration tests

Sounds like a great idea, but should we tackle this outside this PR? I can create a separate issue to look into it. I am afraid it might escalate the scope of this PR (which is just to harden the validation around unsupported DRA fields/features)

Added the wrapper, https://github.com/kubernetes-sigs/kueue/pull/7226/files#diff-8c600775ee9d35183196effef13d4df7be756e077046325d17b6214b080cba2bR1823

+1. IMHO, since this PR is adding all the permutations of creating resource slices with different features, this is the right moment to add the wrapper. I dont really think it expands the scope. Thanks for tackling that.

alaypatel07 · 2025-10-22T16:42:53Z

@harche from what I can see, AllocationMode field itself can be "" i.e. empty because it is not a pointer but it should always be defaulted to AllocationModeExact https://github.com/kubernetes/kubernetes/blob/e1b996a4d3358b783a17e5f54a70e573e394a697/pkg/apis/resource/v1beta1/defaults.go#L43

Are you seeing that this defaulting is not working?

harche · 2025-10-22T16:52:57Z

@harche from what I can see, AllocationMode field itself can be "" i.e. empty because it is not a pointer but it should always be defaulted to AllocationModeExact https://github.com/kubernetes/kubernetes/blob/e1b996a4d3358b783a17e5f54a70e573e394a697/pkg/apis/resource/v1beta1/defaults.go#L43

Are you seeing that this defaulting is not working?

yeah, if I remove the req.Exactly.AllocationMode == "" check in the switch, the test that specifically sets AllocationMode to "" fails, but it passes if I bring back that check.

% go test ./pkg/dra/...
--- FAIL: Test_GetResourceRequests (0.05s)
    --- FAIL: Test_GetResourceRequests/Empty_allocation_mode_defaults_to_exact_count (0.00s)
        claims_test.go:392: unexpected error: unsupported DRA feature in ResourceClaimTemplate claim-tmpl-default in workload wl podset main: unsupported DRA feature: unsupported allocation mode: 
FAIL
FAIL	sigs.k8s.io/kueue/pkg/dra	0.402s
FAIL

but passes if I add req.Exactly.AllocationMode == ""

go test ./pkg/dra/...
ok  	sigs.k8s.io/kueue/pkg/dra	(cached)

alaypatel07 · 2025-10-22T16:56:25Z

@harche from what I can see, AllocationMode field itself can be "" i.e. empty because it is not a pointer but it should always be defaulted to AllocationModeExact https://github.com/kubernetes/kubernetes/blob/e1b996a4d3358b783a17e5f54a70e573e394a697/pkg/apis/resource/v1beta1/defaults.go#L43
Are you seeing that this defaulting is not working?

yeah, if I remove the req.Exactly.AllocationMode == "" check in the switch, the test that specifically sets AllocationMode to "" fails, but it passes if I bring back that check.
% go test ./pkg/dra/...
--- FAIL: Test_GetResourceRequests (0.05s)
    --- FAIL: Test_GetResourceRequests/Empty_allocation_mode_defaults_to_exact_count (0.00s)
        claims_test.go:392: unexpected error: unsupported DRA feature in ResourceClaimTemplate claim-tmpl-default in workload wl podset main: unsupported DRA feature: unsupported allocation mode: 
FAIL
FAIL	sigs.k8s.io/kueue/pkg/dra	0.402s
FAIL
but passes if I add req.Exactly.AllocationMode == ""
go test ./pkg/dra/...
ok  	sigs.k8s.io/kueue/pkg/dra	(cached)

That's because this is a unit test and the resource claim is not going through the API server which has defaulting logic. This test will not fail in the e2e since it goes through API server there. The correct thing IMO is to remove this unit test and let it just be an e2e test

harche · 2025-10-22T17:00:23Z

@harche from what I can see, AllocationMode field itself can be "" i.e. empty because it is not a pointer but it should always be defaulted to AllocationModeExact https://github.com/kubernetes/kubernetes/blob/e1b996a4d3358b783a17e5f54a70e573e394a697/pkg/apis/resource/v1beta1/defaults.go#L43
Are you seeing that this defaulting is not working?

yeah, if I remove the req.Exactly.AllocationMode == "" check in the switch, the test that specifically sets AllocationMode to "" fails, but it passes if I bring back that check.
% go test ./pkg/dra/...
--- FAIL: Test_GetResourceRequests (0.05s)
    --- FAIL: Test_GetResourceRequests/Empty_allocation_mode_defaults_to_exact_count (0.00s)
        claims_test.go:392: unexpected error: unsupported DRA feature in ResourceClaimTemplate claim-tmpl-default in workload wl podset main: unsupported DRA feature: unsupported allocation mode: 
FAIL
FAIL	sigs.k8s.io/kueue/pkg/dra	0.402s
FAIL
but passes if I add req.Exactly.AllocationMode == ""
go test ./pkg/dra/...
ok  	sigs.k8s.io/kueue/pkg/dra	(cached)
That's because this is a unit test and the resource claim is not going through the API server which has defaulting logic. This test will not fail in the e2e since it goes through API server there. The correct thing IMO is to remove this unit test and let it just be an e2e test

@alaypatel07 good point, removed it. Thanks.

harche · 2025-10-22T18:25:42Z

/test pull-kueue-test-e2e-tas-main

harche · 2025-10-22T18:27:04Z

That's because this is a unit test and the resource claim is not going through the API server which has defaulting logic. This test will not fail in the e2e since it goes through API server there. The correct thing IMO is to remove this unit test and let it just be an e2e test

@alaypatel07 good point, removed it. Thanks.

@alaypatel07 replaced with an integration test, https://github.com/kubernetes-sigs/kueue/pull/7226/files#diff-5dc66fc561764c1765de2f7abf5aca47f48c675141ab4c6c5fd6ce7a3a40b1a6R727

Signed-off-by: Harshal Patil <[email protected]>

harche · 2025-10-23T22:37:58Z

@mimowo @alaypatel07 I have addressed all the review comments. Thanks.

mimowo

/lgtm
/approve
Looks better than before, thank you!

k8s-ci-robot · 2025-10-24T09:01:17Z

LGTM label has been added.

Git tree hash: c89963c8bc4ae64e075c076acfad0c422a188c56

k8s-ci-robot · 2025-10-24T09:01:17Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: harche, mimowo

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [mimowo]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

mimowo · 2025-10-24T09:02:51Z

@harche let me know if:

you could follow up on DRA cleanup: use structured assert on errors using cmp.Diff #7343
you want to cherrypick to 0.14, this can be done but requires manual conflict resolution. Since the integration is an alpha feature it is not necessary

harche · 2025-10-24T11:09:25Z

@harche let me know if:

you could follow up on DRA cleanup: use structured assert on errors using cmp.Diff #7343

you want to cherrypick to 0.14, this can be done but requires manual conflict resolution. Since the integration is an alpha feature it is not necessary

@mimowo thanks for your review.

I have assigned DRA cleanup: use structured assert on errors using cmp.Diff #7343 to me.
I am not looking for backporting it to 0.14, since DRA support is in Kueue is alpha anyway.

Signed-off-by: Harshal Patil <[email protected]>

k8s-ci-robot added kind/bug Categorizes issue or PR as related to a bug. do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Oct 10, 2025

k8s-ci-robot requested review from PBundyra and mimowo October 10, 2025 16:55

k8s-ci-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Oct 10, 2025

k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Oct 10, 2025

k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Oct 10, 2025

k8s-ci-robot requested a review from alaypatel07 October 10, 2025 17:00

harche force-pushed the dra_validation branch from c524414 to 4318946 Compare October 10, 2025 17:20

alaypatel07 reviewed Oct 10, 2025

View reviewed changes

harche force-pushed the dra_validation branch from 4318946 to 006e7b3 Compare October 10, 2025 18:52

k8s-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Oct 10, 2025

harche requested a review from alaypatel07 October 10, 2025 18:54

alaypatel07 reviewed Oct 10, 2025

View reviewed changes

harche force-pushed the dra_validation branch 3 times, most recently from 03ca256 to f7b275d Compare October 11, 2025 00:12

harche force-pushed the dra_validation branch from e46910b to 8ad5160 Compare October 22, 2025 17:00

k8s-ci-robot removed the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Oct 22, 2025

k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Oct 22, 2025

mimowo mentioned this pull request Oct 22, 2025

DRA cleanup: use structured assert on errors using cmp.Diff #7343

Open

harche force-pushed the dra_validation branch from 8ad5160 to 081eea8 Compare October 22, 2025 17:54

k8s-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Oct 22, 2025

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 23, 2025

Add validation for unsupported DRA features

057dd22

Signed-off-by: Harshal Patil <[email protected]>

harche force-pushed the dra_validation branch from 081eea8 to 057dd22 Compare October 23, 2025 13:39

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 23, 2025

mimowo reviewed Oct 24, 2025

View reviewed changes

k8s-ci-robot assigned mimowo Oct 24, 2025

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 24, 2025

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 24, 2025

k8s-ci-robot merged commit 32f93ef into kubernetes-sigs:main Oct 24, 2025
23 checks passed

k8s-ci-robot added this to the v0.15 milestone Oct 24, 2025

harche deleted the dra_validation branch October 24, 2025 11:08

Singularity23x0 pushed a commit to Singularity23x0/kueue that referenced this pull request Nov 3, 2025

Add validation for unsupported DRA features (kubernetes-sigs#7226)

a416c2e

Signed-off-by: Harshal Patil <[email protected]>

Add validation for unsupported DRA features #7226

Add validation for unsupported DRA features #7226

Uh oh!

Conversation

harche commented Oct 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What type of PR is this?

What this PR does / why we need it:

Which issue(s) this PR fixes:

Special notes for your reviewer:

Does this PR introduce a user-facing change?

Uh oh!

netlify bot commented Oct 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for kubernetes-sigs-kueue ready!

Uh oh!

k8s-ci-robot commented Oct 10, 2025

Uh oh!

mimowo commented Oct 10, 2025

Uh oh!

kannon92 commented Oct 10, 2025

Uh oh!

alaypatel07 commented Oct 10, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

harche commented Oct 10, 2025

Uh oh!

harche commented Oct 10, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alaypatel07 commented Oct 22, 2025

Uh oh!

harche commented Oct 22, 2025

Uh oh!

alaypatel07 commented Oct 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

harche commented Oct 22, 2025

Uh oh!

harche commented Oct 22, 2025

Uh oh!

harche commented Oct 22, 2025

Uh oh!

harche commented Oct 23, 2025

Uh oh!

mimowo left a comment

Choose a reason for hiding this comment

Uh oh!

k8s-ci-robot commented Oct 24, 2025

Uh oh!

k8s-ci-robot commented Oct 24, 2025

Uh oh!

harche commented Oct 10, 2025 •

edited

Loading

netlify bot commented Oct 10, 2025 •

edited

Loading

alaypatel07 commented Oct 22, 2025 •

edited

Loading