migrate generic_scheduler.go and types.go to structured logs #98134

tanjing2020 · 2021-01-18T06:44:19Z

What type of PR is this?

/kind cleanup
What this PR does / why we need it:
Ref:
https://github.com/kubernetes/enhancements/tree/master/keps/sig-instrumentation/1602-structured-logging
https://github.com/kubernetes/community/blob/master/contributors/devel/sig-instrumentation/migration-to-structured-logging.md

Which issue(s) this PR fixes:

Verify log output (generic_scheduler.go):

before:
generic_scheduler.go:318] Skipping extender &{false pod0 false false} as it returned error not a binder and has ignorable flag set
generic_scheduler.go:376] Plugin NodeAffinity scores on default/test-pod => [{node0 5} {node1 3}]
generic_scheduler.go:414] test-pod_default -> tmp: fakeExtender, Score: (7)
generic_scheduler.go:432] Host tmp => Score 7

after
generic_scheduler.go:318] "Skipping extender as it returned error and has ignorable flag set"  err="not a binder" extender=&{isBinder:false interestedPodName:pod0 ignorable:false gotBind:false}
generic_scheduler.go:375] "Plugin scored node for pod" pod="default/test-pod" plugin="NodeAffinity" node="node0" score=5
generic_scheduler.go:375] "Plugin scored node for pod" pod="default/test-pod" plugin="NodeAffinity" node="node1" score=3
generic_scheduler.go:413] "Extender scored node for pod" pod="default/test-pod" host="tmp" extenders name="fakeExtender" score=7
generic_scheduler.go:431] "Calculated node's final score for pod" pod="default/test-pod" node="tmp" score=7

Verify log output (types.go):

before:
types.go:565] Cannot get pod key, err: Cannot get cache key for pod with empty UID
types.go:594] Cannot get pod key, err: Cannot get cache key for pod with empty UID

after
types.go:565] "Cannot get pod key" err="Cannot get cache key for pod with empty UID"
types.go:594] Cannot get pod key, err: Cannot get cache key for pod with empty UID

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

NONE

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

k8s-ci-robot · 2021-01-18T06:44:26Z

@tanjing2020: This issue is currently awaiting triage.

If a SIG or subproject determines this is a relevant issue, they will accept it by applying the triage/accepted label and provide further guidance.

The triage/accepted label can be added by org members by writing /triage accepted in a comment.

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.

k8s-ci-robot · 2021-01-18T06:44:27Z

Hi @tanjing2020. 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 /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

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.

pkg/scheduler/core/generic_scheduler.go

tanjing2020 · 2021-01-19T02:41:51Z

Please check again, thank you.
/recc @alculquicondor

JornShen · 2021-01-19T12:13:45Z

/ok-to-test

chendave

please revert all those file permission from 100755 -> 100644

chendave · 2021-01-20T09:00:38Z

and please fix bazel dependency issue as well.

tanjing2020 · 2021-01-21T07:05:51Z

/retest

alculquicondor · 2021-01-21T14:56:47Z

pkg/scheduler/core/generic_scheduler.go

@@ -373,7 +371,7 @@ func (g *genericScheduler) prioritizeNodes(

 	if klog.V(10).Enabled() {
 		for plugin, nodeScoreList := range scoresMap {
-			klog.Infof("Plugin %s scores on %v/%v => %v", plugin, pod.Namespace, pod.Name, nodeScoreList)
+			klog.InfoS("Plugin scored nodes for pod", "pod", klog.KObj(pod), "plugin", plugin, "nodeScoreList", nodeScoreList)


Extend this to a for loop, and have one line for each node, just like we do for extenders.

Okey, I fixed it like this:
for _, nodeScore := range nodeScoreList {
klog.InfoS("Plugin scored node for pod", "pod", klog.KObj(pod), "plugin", plugin, "node", nodeScore.Name, "score", nodeScore.Score)
}

alculquicondor · 2021-01-21T14:58:53Z

pkg/scheduler/core/generic_scheduler.go

@@ -429,7 +427,7 @@ func (g *genericScheduler) prioritizeNodes(

 	if klog.V(10).Enabled() {
 		for i := range result {
-			klog.Infof("Host %s => Score %d", result[i].Name, result[i].Score)
+			klog.InfoS("Combined (added) all scores together for that node", "node", result[i].Name, "score", result[i].Score)


"Calculated final score for node"

Thanks, done.

tanjing2020 · 2021-01-22T02:06:58Z

/retest

kikisdeliveryservice · 2021-01-22T02:11:04Z

Linking to open issue: kubernetes/enhancements#1602

alculquicondor

/approve

alculquicondor · 2021-01-22T14:12:02Z

pkg/scheduler/core/generic_scheduler.go

@@ -429,7 +429,7 @@ func (g *genericScheduler) prioritizeNodes(

 	if klog.V(10).Enabled() {
 		for i := range result {
-			klog.Infof("Host %s => Score %d", result[i].Name, result[i].Score)
+			klog.InfoS("Calculated final score for node", "node", result[i].Name, "score", result[i].Score)


Please add the pod. Sorry I missed this before.

k8s-ci-robot · 2021-01-22T14:12:46Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: alculquicondor, tanjing2020

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:

~~pkg/scheduler/OWNERS~~ [alculquicondor]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

tanjing2020 · 2021-01-26T01:37:45Z

/recc @alculquicondor @chendave

tanjing2020 · 2021-01-26T02:21:33Z

/retest

chendave · 2021-01-26T02:45:02Z

@alculquicondor 's comments has been addressed.

/lgtm

tanjing2020 · 2021-01-26T03:11:54Z

/test pull-kubernetes-e2e-gce-ubuntu-containerd

fejta-bot · 2021-01-26T07:04:39Z

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

fejta-bot · 2021-01-26T10:34:40Z

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

fejta-bot · 2021-01-26T14:04:40Z

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

alculquicondor · 2021-01-26T15:00:45Z

pkg/scheduler/framework/types.go

@@ -562,7 +562,7 @@ func removeFromSlice(s []*PodInfo, k string) []*PodInfo {
 	for i := range s {
 		k2, err := GetPodKey(s[i].Pod)
 		if err != nil {
-			klog.Errorf("Cannot get pod key, err: %v", err)
+			klog.ErrorS(err, "Cannot get pod key")


Add pod here and below

chendave · 2021-01-27T01:42:47Z

@tanjing2020 please submit a following up PR to address @alculquicondor 's comments, thanks!

k8s-ci-robot added the needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. label Jan 18, 2021

k8s-ci-robot added needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. needs-priority Indicates a PR lacks a `priority/foo` label and requires one. labels Jan 18, 2021

k8s-ci-robot requested review from alculquicondor and damemi January 18, 2021 06:45

k8s-ci-robot added sig/scheduling Categorizes an issue or PR as relevant to SIG Scheduling. and removed do-not-merge/needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Jan 18, 2021

tanjing2020 force-pushed the change-log branch 2 times, most recently from e5bea41 to ec8115c Compare January 18, 2021 08:50

alculquicondor reviewed Jan 18, 2021

View reviewed changes

tanjing2020 force-pushed the change-log branch 5 times, most recently from 5b752e3 to 6636100 Compare January 19, 2021 02:29

k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Jan 19, 2021

chendave reviewed Jan 20, 2021

View reviewed changes

tanjing2020 force-pushed the change-log branch 2 times, most recently from c70d54a to ce29376 Compare January 21, 2021 03:04

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 21, 2021

k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. labels Jan 21, 2021

alculquicondor reviewed Jan 21, 2021

View reviewed changes

tanjing2020 force-pushed the change-log branch from b76f0fa to 2417a02 Compare January 22, 2021 01:34

k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 22, 2021

alculquicondor reviewed Jan 22, 2021

View reviewed changes

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 22, 2021

migrate generic_scheduler.go and types.go to structured logs

d59d107

tanjing2020 force-pushed the change-log branch from 2417a02 to d59d107 Compare January 26, 2021 01:31

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 26, 2021

alculquicondor reviewed Jan 26, 2021

View reviewed changes

k8s-ci-robot merged commit 35871e4 into kubernetes:master Jan 26, 2021

k8s-ci-robot added this to the v1.21 milestone Jan 26, 2021

tanjing2020 mentioned this pull request Jan 27, 2021

Add pod information to the error logs #98453

Merged

tanjing2020 mentioned this pull request Mar 3, 2021

REQUEST: New membership for tanjing2020 kubernetes/org#2536

Closed

6 tasks

k8s-ci-robot added release-note-none Denotes a PR that doesn't merit a release note. and removed release-note Denotes a PR that will be considered when it comes time to generate release notes. labels Mar 29, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

migrate generic_scheduler.go and types.go to structured logs #98134

migrate generic_scheduler.go and types.go to structured logs #98134

tanjing2020 commented Jan 18, 2021 •

edited by liggitt

k8s-ci-robot commented Jan 18, 2021

k8s-ci-robot commented Jan 18, 2021

tanjing2020 commented Jan 19, 2021 •

edited

JornShen commented Jan 19, 2021

chendave left a comment

chendave commented Jan 20, 2021

tanjing2020 commented Jan 21, 2021

alculquicondor Jan 21, 2021

tanjing2020 Jan 22, 2021 •

edited

alculquicondor Jan 21, 2021

tanjing2020 Jan 22, 2021

tanjing2020 commented Jan 22, 2021

kikisdeliveryservice commented Jan 22, 2021

alculquicondor left a comment

alculquicondor Jan 22, 2021

tanjing2020 Jan 26, 2021

k8s-ci-robot commented Jan 22, 2021

tanjing2020 commented Jan 26, 2021

tanjing2020 commented Jan 26, 2021

chendave commented Jan 26, 2021

tanjing2020 commented Jan 26, 2021

fejta-bot commented Jan 26, 2021

fejta-bot commented Jan 26, 2021

fejta-bot commented Jan 26, 2021

alculquicondor Jan 26, 2021

chendave commented Jan 27, 2021

migrate generic_scheduler.go and types.go to structured logs #98134

migrate generic_scheduler.go and types.go to structured logs #98134

Conversation

tanjing2020 commented Jan 18, 2021 • edited by liggitt

k8s-ci-robot commented Jan 18, 2021

k8s-ci-robot commented Jan 18, 2021

tanjing2020 commented Jan 19, 2021 • edited

JornShen commented Jan 19, 2021

chendave left a comment

Choose a reason for hiding this comment

chendave commented Jan 20, 2021

tanjing2020 commented Jan 21, 2021

alculquicondor Jan 21, 2021

Choose a reason for hiding this comment

tanjing2020 Jan 22, 2021 • edited

Choose a reason for hiding this comment

alculquicondor Jan 21, 2021

Choose a reason for hiding this comment

tanjing2020 Jan 22, 2021

Choose a reason for hiding this comment

tanjing2020 commented Jan 22, 2021

kikisdeliveryservice commented Jan 22, 2021

alculquicondor left a comment

Choose a reason for hiding this comment

alculquicondor Jan 22, 2021

Choose a reason for hiding this comment

tanjing2020 Jan 26, 2021

Choose a reason for hiding this comment

k8s-ci-robot commented Jan 22, 2021

tanjing2020 commented Jan 26, 2021

tanjing2020 commented Jan 26, 2021

chendave commented Jan 26, 2021

tanjing2020 commented Jan 26, 2021

fejta-bot commented Jan 26, 2021

fejta-bot commented Jan 26, 2021

fejta-bot commented Jan 26, 2021

alculquicondor Jan 26, 2021

Choose a reason for hiding this comment

chendave commented Jan 27, 2021

tanjing2020 commented Jan 18, 2021 •

edited by liggitt

tanjing2020 commented Jan 19, 2021 •

edited

tanjing2020 Jan 22, 2021 •

edited