update pod state of scheduler cache when UpdatePod #64692
Conversation
k8s-ci-robot
added
size/XS
k8s-ci-robot
added
sig/scheduling
|
/assign @k82cn |
| @@ -317,6 +317,7 @@ func (cache *schedulerCache) UpdatePod(oldPod, newPod *v1.Pod) error { | |||
| if err := cache.updatePod(oldPod, newPod); err != nil { | |||
| return err | |||
| } | |||
| currState.pod = newPod | |||
There was a problem hiding this comment.
Would you share the case that this PR handled? And as we're changed to use pod's UID as key, that may make cache in-consistent.
There was a problem hiding this comment.
oh, sorry for confusion: the UID did not change when updating pod. so the fix seems ok. Let me check it today :)
There was a problem hiding this comment.
@k82cn I think this PR just makes sure the cache always keep consistent. We won't have any problem now if we don't update the podStates map, but if pod resources could be updated(like in-place pod resource update), this will bring problem.
There was a problem hiding this comment.
Yes, resource update is one case. Another case is GetPod, which may return 'old' pod; can you help to add a test for it? For assumed pod, GetPod will return the correct value; but for assigned pods, it may return old pod (although no one uses this feature right now).
There was a problem hiding this comment.
sure I will add a test case asap.
k8s-ci-robot
added
size/M
|
@k82cn test added, ptal. |
|
/lgtm |
k8s-ci-robot
added
the
lgtm
|
please update release note :) |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: adohe, k82cn 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:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
the
approved
|
/approve |
k8s-ci-robot
added
release-note
|
add release note done. |
|
/retest Review the full test history for this PR. Silence the bot with an |
|
/test pull-kubernetes-node-e2e |
|
/test pull-kubernetes-bazel-build |
|
/retest Review the full test history for this PR. Silence the bot with an |
1 similar comment
|
/retest Review the full test history for this PR. Silence the bot with an |
|
Automatic merge from submit-queue (batch tested with PRs 64882, 64692, 64389, 60626, 64840). If you want to cherry-pick this change to another branch, please follow the instructions here. |
update pod state map in scheduler cache when call UpdatePod. @k82cn @bsalamat