Allow implicit extended resource name to be used no matter explicit extendedResourceName field is set or not in device class #133363
Conversation
|
/assign @johnbelamaric @macsko |
| @@ -430,7 +430,7 @@ func hasDeviceClassMappedExtendedResource(reqs v1.ResourceList, deviceClassMappi | |||
| // We only care about the resources requested by the pod we are trying to schedule. | |||
| continue | |||
| } | |||
| if v1helper.IsExtendedResourceName(rName) { | |||
| if v1helper.IsExtendedResourceName(rName) || strings.HasPrefix(string(rName), resourceapi.ResourceDeviceClassPrefix) { | |||
There was a problem hiding this comment.
What with the withDeviceClass func in NodeResourcesFit plugin?
There was a problem hiding this comment.
right, we also need to add that. added. PTAL
There was a problem hiding this comment.
You can consider exporting v1helper.IsExtendedResourceName(rName) || strings.HasPrefix(string(rName), resourceapi.ResourceDeviceClassPrefix) to a separate function, e.g. in pkg/scheduler/util/utils.go
|
/triage accepted |
|
/retest |
1 similar comment
|
/retest |
|
/approve Please add a release note ( |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: macsko, yliaog The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
@klueska could you take a look at this PR? |
|
/retest |
1 similar comment
|
/retest |
|
/retest |
1 similar comment
|
/retest |
|
/assign @SergeyKanzhelev |
test/e2e/dra/dra.go
Outdated
| @@ -1910,6 +1910,22 @@ var _ = framework.SIGDescribe("node")(framework.WithLabel("DRA"), func() { | |||
| b := drautils.NewBuilder(f, driver) | |||
| b.UseExtendedResourceName = true | |||
|
|
|||
| ginkgo.It("must run a pod with implicit extended resource with one container one resource", func(ctx context.Context) { | |||
There was a problem hiding this comment.
let's add a test that will confirm that a pod with both definitions works as described in #133366
There was a problem hiding this comment.
sure, added, the test has one two resources, one implicit, one explicit, both are using the same device class.
no matter the explicit extendedResourceName field in device class is set or not.
|
/retest |
6 similar comments
|
/retest |
|
/retest |
|
/retest |
|
/retest |
|
/retest |
|
/retest |
|
@yliaog: The following tests failed, say
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR. 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. I understand the commands that are listed here. |
|
The test failure is due to the release, as seen from the error below. it is not related to the test, or code change. @SergeyKanzhelev PTAL |
The upgrade/downgrade test will resolve when 1.34 is released. Right now in master it sees 1.35 and tries then to look for 1.34 to download, but it's not available yet. |
What type of PR is this?
/kind bug
#133366
What this PR does / why we need it:
Which issue(s) this PR is related to:
Special notes for your reviewer:
Does this PR introduce a user-facing change?
Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.: