Skip to content

feat: add CEL selector in dra.go#899

Open
rich7420 wants to merge 2 commits intoNVIDIA:mainfrom
rich7420:dra-e2e
Open

feat: add CEL selector in dra.go#899
rich7420 wants to merge 2 commits intoNVIDIA:mainfrom
rich7420:dra-e2e

Conversation

@rich7420
Copy link
Contributor

Description

The original test code used slice.Spec.Driver != deviceClass to filter devices, which is incorrect. DeviceClass names are not the same as drivers, and we should use CEL selectors to determine if a device belongs to a DeviceClass.

Related Issues

Fixes #

Checklist

Note: Ensure your PR title follows the Conventional Commits format (e.g., feat(scheduler): add new feature)

  • Self-reviewed
  • Added/updated tests (if needed)
  • Updated documentation (if needed)

Breaking Changes

Additional Notes

@rich7420 rich7420 marked this pull request as ready for review January 19, 2026 16:09
@rich7420
Copy link
Contributor Author

cc @itsomri , @enoodle , @davidLif

@enoodle
Copy link
Collaborator

enoodle commented Feb 1, 2026

@rich7420 Currently the DRA e2e tests are all not functioning so we don't have a way to test it. Did you get to test them with this change?

@rich7420
Copy link
Contributor Author

rich7420 commented Feb 2, 2026

I didn’t get to run the DRA e2e tests with this change ,they’re not working in CI right now. The code mirrors the scheduler’s CEL logic in dynamicresources (same DeviceClass + CEL selectors and feature flags), and it passes make validate. Happy to add unit tests for the new helper if you want that before merge.

@enoodle
Copy link
Collaborator

enoodle commented Feb 2, 2026

@rich7420 As we are still trying to stabilize this and enable those tests I prefer not to add this before #933 is merged and it can be tested.

@rich7420
Copy link
Contributor Author

rich7420 commented Feb 2, 2026

@enoodle got it! thanks for the reminder


func ListDevicesByNode(clientset kubernetes.Interface, deviceClass string) map[string]int {
// ListDevicesByNode counts devices per node that match the given DeviceClass selectors (CEL).
// If the DeviceClass doesn't exist, the test is skipped. If it has no selectors, all devices match.
Copy link
Collaborator

Choose a reason for hiding this comment

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

The fallback should be to the current behavior, or no device at all.

}
}

// CleanupResourceClaims deletes all ResourceClaims with the engine-e2e label in the given namespace.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Lets avoid obvious comments on code that didn't change.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

got it!

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.

2 participants