add log includes pod preemption details #117214
Conversation
|
Please note that we're already in Test Freeze for the Fast forwards are scheduled to happen every 6 hours, whereas the most recent run was: Tue Apr 11 14:07:05 UTC 2023. |
|
This issue is currently awaiting triage. If a SIG or subproject determines this is a relevant issue, they will accept it by applying the The 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/test-infra repository. |
|
Hi @HirazawaUi. Thanks for your PR. I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the 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/test-infra repository. |
|
@alculquicondor If this PR meets your requirements? |
| @@ -378,6 +378,7 @@ func (ev *Evaluator) prepareCandidate(ctx context.Context, c Candidate, pod *v1. | |||
| return | |||
| } | |||
| } | |||
| klog.V(2).Infof("victim %v/%v preempted by %v/%v on node %v", victim.Namespace, victim.Name, pod.Namespace, pod.Name, c.Name()) | |||
There was a problem hiding this comment.
There was a problem hiding this comment.
Please follow the guide https://github.com/kubernetes/community/blob/master/contributors/devel/sig-instrumentation/migration-to-structured-logging.md
I have read the guide and made the corrections
e4bb6bb
to
77987d7
Compare
| @@ -378,6 +378,7 @@ func (ev *Evaluator) prepareCandidate(ctx context.Context, c Candidate, pod *v1. | |||
| return | |||
| } | |||
| } | |||
| klog.V(2).InfoS("Preemptor pod preempts the victim pod", "preemptor", klog.KObj(pod), "victim", klog.KObj(victim), "node", c.Name()) | |||
There was a problem hiding this comment.
@Huang-Wei do you think we should have a different message for preemptions of waiting pods?
Also, use past tense: Preemptor Pod preempted victim Pod
There was a problem hiding this comment.
We should. A waitingPod is actually not physically deleted, but unreserved from internal cache. So let's add the message below waitingPod.Reject():
klog.V(2).InfoS("Preemptor pod rejected a waiting pod", "preemptor", klog.KObj(pod), "waitingPod", klog.KObj(victim), "node", c.Name())
@HirazawaUi please also move the message to the end of else section; otherwise it's printed for waitingPod case as well.
There was a problem hiding this comment.
@Huang-Wei do you think we should have a different message for preemptions of waiting pods?
Also, use past tense:
Preemptor Pod preempted victim Pod
thanks for remind
There was a problem hiding this comment.
We should. A waitingPod is actually not physically deleted, but unreserved from internal cache. So let's add the message below
waitingPod.Reject():klog.V(2).InfoS("Preemptor pod rejected a waiting pod", "preemptor", klog.KObj(pod), "waitingPod", klog.KObj(victim), "node", c.Name())@HirazawaUi please also move the message to the end of
elsesection; otherwise it's printed for waitingPod case as well.
Thanks for remind, has made corrections
|
/ok-to-test |
77987d7
to
ed01248
Compare
|
LGTM label has been added. Git tree hash: f84a758f405562d66055dd4a9775e5a272f91669
|
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: alculquicondor, HirazawaUi 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 |
|
/retest |
|
The Kubernetes project has merge-blocking tests that are currently too flaky to consistently pass. This bot retests PRs for certain kubernetes repos according to the following rules:
You can:
/retest |
|
@HirazawaUi can you cherry_pick to all supported versions? |
Of course, I'll do cherry-pick later |
|
You also need a cherry-pick for |
All right |
…117214-upstream-release-1.25 Automated cherry pick of #117214: add log includes pod preemption details
…117214-upstream-release-1.24 Automated cherry pick of #117214: add log includes pod preemption details
…117214-upstream-release-1.27 Automated cherry pick of #117214: add log includes pod preemption details
…117214-upstream-release-1.26 Automated cherry pick of #117214: add log includes pod preemption details
What type of PR is this?
/kind cleanup
What this PR does / why we need it:
add log includes pod preemption details
Which issue(s) this PR fixes:
Fixes #117210
Special notes for your reviewer:
Does this PR introduce a user-facing change?
Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.: