Structured Logging migration:modify Scheduler part logs. #99273

yangjunmyfm192085 · 2021-02-20T17:59:10Z

Signed-off-by: JunYang yang.jun22@zte.com.cn

What type of PR is this?

/kind cleanup

What this PR does / why we need it:

Structured Logging migration:modify Scheduler part logs.
xref:
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:

Fixes #

Special notes for your reviewer:

part of kubernetes/enhancements#1602
As it is related to log, change the log here to structured log.

Does this PR introduce a user-facing change?

Migrate `pkg/scheduler` to structured logging

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

k8s-ci-robot · 2021-02-20T17:59:17Z

Hi @yangjunmyfm192085. 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/framework/runtime/framework.go

gavinfish · 2021-02-21T10:11:16Z

/ok-to-test

yangjunmyfm192085 · 2021-02-21T11:48:04Z

/test pull-kubernetes-e2e-kind

chendave

I have seen many PR to address this simple issue, which are scattered here and there, is that better to make this fixed in one PR instead?

yangjunmyfm192085 · 2021-02-22T02:39:32Z

I have seen many PR to address this simple issue, which are scattered here and there, is that better to make this fixed in one PR instead?

Good question.
I think that all modifications are done in one pr may not be realistic because the amount of modification is too much.
Any better suggestions?
Modify by package?

chendave · 2021-02-22T02:33:29Z

pkg/scheduler/algorithmprovider/registry.go

@@ -164,7 +164,7 @@ func applyFeatureGates(config *schedulerapi.Plugins) {
 	if !utilfeature.DefaultFeatureGate.Enabled(features.DefaultPodTopologySpread) {
 		// When feature is enabled, the default spreading is done by
 		// PodTopologySpread plugin, which is enabled by default.
-		klog.Infof("Registering SelectorSpread plugin")
+		klog.InfoS("Registering SelectorSpread plugin")


Any difference here?

Yeah,new Structured Logging migration uses InfoS.

but for this case, I cannot see any benefit from the change, Infof is still there.

Yes,for this case,may not see the difference.But when there are variables, the format is different.
According to the following rules, Infof will be replaced by InfoS.
https://github.com/kubernetes/community/blob/master/contributors/devel/sig-instrumentation/migration-to-structured-logging.md
https://github.com/kubernetes/enhancements/tree/master/keps/sig-instrumentation/1602-structured-logging

pkg/scheduler/scheduler.go

adtac

couple of nits

pkg/scheduler/framework/runtime/framework.go

adtac · 2021-03-01T03:11:15Z

pkg/scheduler/util/utils.go

@@ -56,7 +56,7 @@ func GetPodStartTime(pod *v1.Pod) *metav1.Time {
 func GetEarliestPodStartTime(victims *extenderv1.Victims) *metav1.Time {
 	if len(victims.Pods) == 0 {
 		// should not reach here.
-		klog.Errorf("victims.Pods is empty. Should not reach here.")
+		klog.ErrorS(nil, "Victims.Pods is empty. Should not reach here.")


does this need to be changed? it's referring to a variable name here

Any better suggestion?
According to structured Logging migration,Errorf needs to be converted to ErrorS.

nvm, I'm fine with this

yangjunmyfm192085 · 2021-05-28T06:09:04Z

/test pull-kubernetes-node-e2e-containerd
/wg structured-logging
/triage accepted

yangjunmyfm192085 · 2021-06-17T07:24:27Z

/test pull-kubernetes-unit

yangjunmyfm192085 · 2021-06-17T07:58:33Z

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

yangjunmyfm192085

/retest

pkg/scheduler/factory.go

pkg/scheduler/scheduler.go

yangjunmyfm192085 · 2021-08-27T05:04:26Z

@ahg-g, I have applied your suggestion. Could you take a look again? Thanks.

yangjunmyfm192085 · 2021-08-27T05:41:15Z

/retest

yangjunmyfm192085 · 2021-08-27T09:54:59Z

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

pkg/scheduler/util/utils.go

Signed-off-by: JunYang <yang.jun22@zte.com.cn>

yangjunmyfm192085

It has been updated@ahg-g

ahg-g · 2021-08-30T12:55:55Z

/lgtm
/approve

k8s-ci-robot · 2021-08-30T12:56:28Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ahg-g, yangjunmyfm192085

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~~ [ahg-g]

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

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

k8s-ci-robot requested review from chendave and k82cn February 20, 2021 17:59

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 Feb 20, 2021

yangjunmyfm192085 force-pushed the run-test20 branch from f8955cc to bcaac26 Compare February 20, 2021 22:39

gavinfish reviewed Feb 21, 2021

View reviewed changes

pkg/scheduler/framework/runtime/framework.go Outdated Show resolved Hide resolved

pkg/scheduler/framework/runtime/framework.go Outdated Show resolved Hide resolved

pkg/scheduler/framework/runtime/framework.go Outdated Show resolved Hide resolved

yangjunmyfm192085 force-pushed the run-test20 branch from bcaac26 to b740d7c Compare February 21, 2021 10:08

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 Feb 21, 2021

chendave reviewed Feb 22, 2021

View reviewed changes

adtac reviewed Mar 1, 2021

View reviewed changes

yangjunmyfm192085 force-pushed the run-test20 branch from 5a7155a to e54d9c6 Compare March 1, 2021 05:48

shubheksha added this to Waiting on Author in Structured Logging Migration for Kubelet, 1.21 Mar 5, 2021

ehashman removed this from Waiting on Author in Structured Logging Migration for Kubelet, 1.21 Mar 5, 2021

pacoxu mentioned this pull request Apr 15, 2021

migrate pkg/scheduler/framework/plugins/nodevolumelimits to structured logs #98651

Closed

shubheksha added this to Needs Review in WG Structured Logging - Migration Review Board May 10, 2021

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 5, 2021

yangjunmyfm192085 force-pushed the run-test20 branch 2 times, most recently from 138bf6f to dacf884 Compare July 13, 2021 12:30

k8s-ci-robot removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Jul 13, 2021

yangjunmyfm192085 commented Jul 13, 2021

View reviewed changes

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 10, 2021

yangjunmyfm192085 force-pushed the run-test20 branch from dacf884 to c5b404b Compare August 13, 2021 00:28

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 13, 2021

ahg-g reviewed Aug 26, 2021

View reviewed changes

pkg/scheduler/factory.go Outdated Show resolved Hide resolved

pkg/scheduler/scheduler.go Show resolved Hide resolved

yangjunmyfm192085 force-pushed the run-test20 branch from c5b404b to 7a40cf4 Compare August 27, 2021 04:53

k8s-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Aug 27, 2021

yangjunmyfm192085 force-pushed the run-test20 branch from 7a40cf4 to 176efcc Compare August 27, 2021 05:01

ahg-g reviewed Aug 27, 2021

View reviewed changes

pkg/scheduler/util/utils.go Outdated Show resolved Hide resolved

Structured Logging migration:modify Scheduler part logs.

9304782

Signed-off-by: JunYang <yang.jun22@zte.com.cn>

yangjunmyfm192085 force-pushed the run-test20 branch from 176efcc to 9304782 Compare August 29, 2021 12:18

yangjunmyfm192085 commented Aug 29, 2021

View reviewed changes

k8s-ci-robot assigned ahg-g Aug 30, 2021

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

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

k8s-ci-robot merged commit 7282c20 into kubernetes:master Aug 30, 2021

k8s-ci-robot added this to the v1.23 milestone Aug 30, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Structured Logging migration:modify Scheduler part logs. #99273

Structured Logging migration:modify Scheduler part logs. #99273

yangjunmyfm192085 commented Feb 20, 2021 •

edited

k8s-ci-robot commented Feb 20, 2021

gavinfish commented Feb 21, 2021

yangjunmyfm192085 commented Feb 21, 2021

chendave left a comment

yangjunmyfm192085 commented Feb 22, 2021

chendave Feb 22, 2021

yangjunmyfm192085 Feb 22, 2021

chendave Feb 22, 2021

yangjunmyfm192085 Feb 22, 2021

adtac left a comment

adtac Mar 1, 2021

yangjunmyfm192085 Mar 1, 2021

adtac Mar 10, 2021

yangjunmyfm192085 commented May 28, 2021 •

edited

yangjunmyfm192085 commented Jun 17, 2021

yangjunmyfm192085 commented Jun 17, 2021

yangjunmyfm192085 left a comment

yangjunmyfm192085 commented Aug 27, 2021 •

edited

yangjunmyfm192085 commented Aug 27, 2021

yangjunmyfm192085 commented Aug 27, 2021

yangjunmyfm192085 left a comment •

edited

ahg-g commented Aug 30, 2021

k8s-ci-robot commented Aug 30, 2021

Structured Logging migration:modify Scheduler part logs. #99273

Structured Logging migration:modify Scheduler part logs. #99273

Conversation

yangjunmyfm192085 commented Feb 20, 2021 • edited

What type of PR is this?

What this PR does / why we need it:

Which issue(s) this PR fixes:

Special notes for your reviewer:

Does this PR introduce a user-facing change?

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

k8s-ci-robot commented Feb 20, 2021

gavinfish commented Feb 21, 2021

yangjunmyfm192085 commented Feb 21, 2021

chendave left a comment

Choose a reason for hiding this comment

yangjunmyfm192085 commented Feb 22, 2021

chendave Feb 22, 2021

Choose a reason for hiding this comment

yangjunmyfm192085 Feb 22, 2021

Choose a reason for hiding this comment

chendave Feb 22, 2021

Choose a reason for hiding this comment

yangjunmyfm192085 Feb 22, 2021

Choose a reason for hiding this comment

adtac left a comment

Choose a reason for hiding this comment

adtac Mar 1, 2021

Choose a reason for hiding this comment

yangjunmyfm192085 Mar 1, 2021

Choose a reason for hiding this comment

adtac Mar 10, 2021

Choose a reason for hiding this comment

yangjunmyfm192085 commented May 28, 2021 • edited

yangjunmyfm192085 commented Jun 17, 2021

yangjunmyfm192085 commented Jun 17, 2021

yangjunmyfm192085 left a comment

Choose a reason for hiding this comment

yangjunmyfm192085 commented Aug 27, 2021 • edited

yangjunmyfm192085 commented Aug 27, 2021

yangjunmyfm192085 commented Aug 27, 2021

yangjunmyfm192085 left a comment • edited

Choose a reason for hiding this comment

ahg-g commented Aug 30, 2021

k8s-ci-robot commented Aug 30, 2021

yangjunmyfm192085 commented Feb 20, 2021 •

edited

yangjunmyfm192085 commented May 28, 2021 •

edited

yangjunmyfm192085 commented Aug 27, 2021 •

edited

yangjunmyfm192085 left a comment •

edited