Migrate ttl_controller to contextual logging #113916

songxiao-wang87 · 2022-11-15T10:31:01Z

What type of PR is this?
/kind feature

What this PR does / why we need it:
Update the ttl controller to use contextual logging.

Which issue(s) this PR fixes:
Part of kubernetes/enhancements#3077

Special notes for your reviewer:
Does this PR introduce a user-facing change?


Migrated the “TTL after finished” controller (within `kube-controller-manager`) to use [contextual logging](https://k8s.io/docs/concepts/cluster-administration/system-logs/#contextual-logging).

k8s-ci-robot · 2022-11-15T10:31:11Z

Hi @songxiao-wang87. 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.

songxiao-wang87 · 2022-11-15T10:37:54Z

@yangjunmyfm192085

yangjunmyfm192085 · 2022-11-15T10:43:43Z

pkg/controller/ttl/ttl_controller.go

 	if err != nil {
-		klog.Errorf("Couldn't get key for object %+v", node)
+		logger.Error(err, "Couldn't get key for object", "object", node)


Suggested change

logger.Error(err, "Couldn't get key for object", "object", node)

logger.Error(nil, "Couldn't get key for object", "object", klog.KObj(node)

yangjunmyfm192085 · 2022-11-15T10:44:21Z

pkg/controller/ttl/ttl_controller.go

 	if err != nil {
-		klog.Errorf("Couldn't get key for object %+v", node)
+		logger.Error(err, "Couldn't get key for object", "object", node)
+		klog.FlushAndExit(klog.ExitFlushTimeout, 1)


Suggested change

klog.FlushAndExit(klog.ExitFlushTimeout, 1)

yangjunmyfm192085 · 2022-11-15T10:46:14Z

pkg/controller/ttl/ttl_controller.go

-		klog.Warningf("Cannot convert the value %q with annotation key %q for the node %q",
-			annotationValue, annotationKey, node.Name)
+		logger.Info("Cannot convert the value with annotation key for the node", "annotationValue",
+			annotationValue, "annotationKey", annotationKey, "nodeName", node.Name)


Suggested change

annotationValue, "annotationKey", annotationKey, "nodeName", node.Name)

annotationValue, "annotationKey", annotationKey, "node", klog.KObj(node))

yangjunmyfm192085 · 2022-11-15T10:53:47Z

/sig instrumentation
/wg structured-logging

yangjunmyfm192085 · 2022-11-15T10:54:13Z

/cc @pohly @shivanshu1333

yangjunmyfm192085

/ok-to-test

yangjunmyfm192085 · 2022-11-15T11:07:32Z

Also need to modify the file
cmd/kube-controller-manager/app/core.go

pohly · 2022-12-08T08:48:25Z

/assign @soltysh

For approval.

songxiao-wang87 · 2022-12-12T09:05:58Z

/assign @soltysh

sftim · 2022-12-29T19:55:39Z

Suggestion for the release note:

Migrated the “TTL after finished” controller (within `kube-controller-manager`) to use [contextual logging](https://k8s.io/docs/concepts/cluster-administration/system-logs/#contextual-logging).

AIUI, end users could observe this change.

Signed-off-by: songxiao-wang87 <wang.xiaosong23@zte.com.cn>

yangjunmyfm192085

/lgtm

k8s-ci-robot · 2023-01-28T03:21:35Z

LGTM label has been added.

Git tree hash: 42663067a0eab1fc3b943e1f10a1ec41977ad1f8

songxiao-wang87 · 2023-01-28T04:15:18Z

/test pull-kubernetes-unit

pohly · 2023-02-15T11:31:13Z

@songxiao-wang87: can you add the changelog entry to your PR description?

pohly · 2023-02-22T10:13:49Z

/assign @soltysh

For approval.

soltysh

/approve

soltysh · 2023-03-07T07:53:28Z

/priority important-longterm

k8s-ci-robot · 2023-03-07T07:53:41Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: alculquicondor, pohly, soltysh, songxiao-wang87, 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:

~~cmd/kube-controller-manager/OWNERS~~ [soltysh]
~~pkg/controller/OWNERS~~ [soltysh]
~~test/OWNERS~~ [pohly,soltysh]

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 the needs-priority Indicates a PR lacks a `priority/foo` label and requires one. label Nov 15, 2022

k8s-ci-robot requested review from alculquicondor and soltysh November 15, 2022 10:32

k8s-ci-robot added sig/apps Categorizes an issue or PR as relevant to SIG Apps. and removed do-not-merge/needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Nov 15, 2022

yangjunmyfm192085 reviewed Nov 15, 2022

View reviewed changes

k8s-ci-robot added sig/instrumentation Categorizes an issue or PR as relevant to SIG Instrumentation. wg/structured-logging Categorizes an issue or PR as relevant to WG Structured Logging. labels Nov 15, 2022

k8s-ci-robot requested review from pohly and shiva1333 November 15, 2022 10:54

songxiao-wang87 force-pushed the runwxs-test1 branch from 39c52fc to ea170c2 Compare November 15, 2022 10:59

yangjunmyfm192085 approved these changes Nov 15, 2022

View reviewed changes

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 Nov 15, 2022

songxiao-wang87 force-pushed the runwxs-test1 branch from ea170c2 to e9fff82 Compare November 15, 2022 11:19

k8s-ci-robot added area/test sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. sig/testing Categorizes an issue or PR as relevant to SIG Testing. labels Nov 15, 2022

k8s-ci-robot assigned soltysh Dec 8, 2022

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

pohly mentioned this pull request Jan 9, 2023

contextual logging kubernetes/enhancements#3077

Open

12 tasks

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

Making a run test.

8c7f3c2

Signed-off-by: songxiao-wang87 <wang.xiaosong23@zte.com.cn>

songxiao-wang87 force-pushed the runwxs-test1 branch from ac068c4 to 8c7f3c2 Compare January 28, 2023 03:17

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

k8s-ci-robot requested review from alculquicondor, pohly and yangjunmyfm192085 January 28, 2023 03:17

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

yangjunmyfm192085 reviewed Jan 28, 2023

View reviewed changes

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

k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Feb 15, 2023

soltysh approved these changes Mar 7, 2023

View reviewed changes

k8s-ci-robot added priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. and removed needs-priority Indicates a PR lacks a `priority/foo` label and requires one. labels Mar 7, 2023

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 7, 2023

k8s-ci-robot merged commit 471b392 into kubernetes:master Mar 7, 2023

k8s-ci-robot added this to the v1.27 milestone Mar 7, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Migrate ttl_controller to contextual logging #113916

Migrate ttl_controller to contextual logging #113916

songxiao-wang87 commented Nov 15, 2022 •

edited

k8s-ci-robot commented Nov 15, 2022

songxiao-wang87 commented Nov 15, 2022

yangjunmyfm192085 Nov 15, 2022

yangjunmyfm192085 Nov 15, 2022

yangjunmyfm192085 Nov 15, 2022

yangjunmyfm192085 commented Nov 15, 2022

yangjunmyfm192085 commented Nov 15, 2022

yangjunmyfm192085 left a comment

yangjunmyfm192085 commented Nov 15, 2022 •

edited

pohly commented Dec 8, 2022

songxiao-wang87 commented Dec 12, 2022

sftim commented Dec 29, 2022

yangjunmyfm192085 left a comment

k8s-ci-robot commented Jan 28, 2023

songxiao-wang87 commented Jan 28, 2023

pohly commented Feb 15, 2023

pohly commented Feb 22, 2023

soltysh left a comment

soltysh commented Mar 7, 2023

k8s-ci-robot commented Mar 7, 2023

	logger.Error(err, "Couldn't get key for object", "object", node)
	logger.Error(nil, "Couldn't get key for object", "object", klog.KObj(node)

	annotationValue, "annotationKey", annotationKey, "nodeName", node.Name)
	annotationValue, "annotationKey", annotationKey, "node", klog.KObj(node))

Migrate ttl_controller to contextual logging #113916

Migrate ttl_controller to contextual logging #113916

Conversation

songxiao-wang87 commented Nov 15, 2022 • edited

k8s-ci-robot commented Nov 15, 2022

songxiao-wang87 commented Nov 15, 2022

yangjunmyfm192085 Nov 15, 2022

Choose a reason for hiding this comment

yangjunmyfm192085 Nov 15, 2022

Choose a reason for hiding this comment

yangjunmyfm192085 Nov 15, 2022

Choose a reason for hiding this comment

yangjunmyfm192085 commented Nov 15, 2022

yangjunmyfm192085 commented Nov 15, 2022

yangjunmyfm192085 left a comment

Choose a reason for hiding this comment

yangjunmyfm192085 commented Nov 15, 2022 • edited

pohly commented Dec 8, 2022

songxiao-wang87 commented Dec 12, 2022

sftim commented Dec 29, 2022

yangjunmyfm192085 left a comment

Choose a reason for hiding this comment

k8s-ci-robot commented Jan 28, 2023

songxiao-wang87 commented Jan 28, 2023

pohly commented Feb 15, 2023

pohly commented Feb 22, 2023

soltysh left a comment

Choose a reason for hiding this comment

soltysh commented Mar 7, 2023

k8s-ci-robot commented Mar 7, 2023

songxiao-wang87 commented Nov 15, 2022 •

edited

yangjunmyfm192085 commented Nov 15, 2022 •

edited