In DevicePluginHandlerImpl.Allocate(), skips untracked extended resou… #53547
Conversation
k8s-ci-robot
added
do-not-merge/release-note-label-needed
…rces. Otherwise, we would fail a Pod allocation request that has an extended resource not managed by any device plugin.
jiayingz
force-pushed
the
deviceplugin-fix
branch
from
2030c4b to
ee1ffa6
Compare
|
/assign @yguo0905 |
|
/lgtm |
k8s-ci-robot
added
the
lgtm
|
/release-note-none |
k8s-ci-robot
added
release-note-none
|
/assign @dchen1107 |
|
Can I suggest to add a regression test which expose the issue without this pr and fixed by with it? |
|
Thanks a lot for the suggestion, @dchen1107 . I will try to add an e2e or e2e_node test to expose this issue. In terms of timing, we do hope this fix can be picked into 1.8.1. I will check with the release manager to see when 1.8.1 will be cut. |
|
A unit test might suffice for this PR. Given the timeline I'm OK with approving this as long as you can add unit tests soon(ish). |
k8s-ci-robot
added
release-note
|
Thanks a lot for the review! Just for this bug, I think we may extend the existing TestPodContainerDeviceAllocation() to expose it. I am working on that right now. We do want to have an integration test to exercise the whole non device-plugin extended resource scheduling. I found there is actually a test/e2e/scheduling/opaque_resource.go e2e test but unfortunately it was disabled due to flakiness. I will see whether I can extend the existing gpu_device_plugin e2e_node test with some additional extended resources. |
vishh
added
the
approved
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jiayingz, vishh, yguo0905 Associated issue: 53548 The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these OWNERS Files:
You can indicate your approval by writing |
|
FYI, the unit test PR #53607. |
|
Automatic merge from submit-queue (batch tested with PRs 52662, 53547, 53588, 53573, 53599). If you want to cherry-pick this change to another branch, please follow the instructions here. |
jpbetz
added
the
cherry-pick-approved
…47-upstream-release-1.8 Automatic merge from submit-queue. Automated cherry pick of #53547 Cherry pick of #53547 on release-1.8. #53547: In DevicePluginHandlerImpl.Allocate(), skips untracked ```release-note Ignore extended resources that are not registered with kubelet during container resource allocation. ```
|
Commit found in the "release-1.8" branch appears to be this PR. Removing the "cherrypick-candidate" label. If this is an error find help to get your PR picked. |
…rces.
Otherwise, we would fail a Pod allocation request that has an extended
resource not managed by any device plugin.
What this PR does / why we need it:
Which issue this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)format, will close that issue when PR gets merged): fixes ##53548
Special notes for your reviewer:
Release note: