Support structure logging for livenessprobe container #168

Kartik494 · 2022-11-02T04:18:41Z

What type of PR is this?

/kind bug

What this PR does / why we need it:
This PR add support for structure logging on livenessprobe container

Which issue(s) this PR fixes:

Fixes #165

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

Added support for structure logging

Kartik494 · 2022-11-02T05:07:17Z

@mauriciopoppe Could you please take a look and suggest if this changes seems ok to you.

Kartik494 · 2022-11-02T05:12:39Z

Added commit to use Suffix S InfoS.

Kartik494 · 2022-11-02T05:15:52Z

/test pull-kubernetes-csi-livenessprobe-unit

Kartik494 · 2022-11-03T05:03:14Z

/test pull-kubernetes-csi-livenessprobe-unit

mauriciopoppe · 2022-11-07T18:35:06Z

cmd/livenessprobe/main.go

+	if *logFormatJSON {
+		jsonFactory := json.Factory{}
+		logger, _ := jsonFactory.Create(logsapi.LoggingConfiguration{Format: "json"})
+		klog.InfoS("logging livenessprobe",logger)


I think this is just logging logger but not telling klog to use it for logging

@mauriciopoppe ok i will re check this, if it's possible could you provide me pointer on this.

I wrote some pointers in kubernetes-csi/node-driver-registrar#228 (comment)

sure i will check that thanks

mauriciopoppe · 2022-11-07T18:36:24Z

cmd/livenessprobe/main.go

 	metricsAddress = flag.String("metrics-address", "", "(deprecated) The TCP network address where the prometheus metrics endpoint will listen (example: `:8080`). The default is empty string, which means metrics endpoint is disabled. If set, `--http-endpoint` cannot be set, and the address cannot resolve to localhost + the port from `--health-port`.")
 	httpEndpoint   = flag.String("http-endpoint", "", "The TCP network address where the HTTP server for diagnostics, including CSI driver health check and metrics. The default is empty string, which means the server is disabled. If set, `--health-port` and `--metrics-address` cannot be explicitly set.")
 	metricsPath    = flag.String("metrics-path", "/metrics", "The HTTP path where prometheus metrics will be exposed. Default is `/metrics`.")
+	logFormatJSON  = flag.Bool("log-format-json", false, "set log formatter to json")


we should follow the same flag as the kubelet which is logging-format https://github.com/kubernetes/component-base/blob/v0.25.3/logs/api/v1/options.go#L185

Sure i will modify this to logging-format

Kartik494 · 2022-11-29T05:00:34Z

/retest

Kartik494 · 2022-12-19T06:48:01Z

Resolve the merge conflict.

ggriffiths

Looks like this needs a rebase, feel free to re-request review when this is ready.

Kartik494 · 2023-01-19T03:13:53Z

@ggriffiths Sure will do this .

Kartik494 · 2023-01-19T05:44:51Z

Resolved the merge conflict

Kartik494 · 2023-01-19T05:45:47Z

/retest

linux-foundation-easycla · 2023-05-28T10:43:27Z

✅ login: Kartik494 / name: Kartik Sharma (f7a9dea)
❌ The commit (dffae97, cc85cbe, 817e991, 4ccdb51). This user is missing the User's ID, preventing the EasyCLA check. Consult GitHub Help to resolve.For further assistance with EasyCLA, please submit a support request ticket.

k8s-ci-robot · 2023-05-28T10:43:31Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: Kartik494
Once this PR has been reviewed and has the lgtm label, please assign msau42 for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

OWNERS

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 · 2023-05-28T10:44:18Z

@Kartik494: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
pull-kubernetes-csi-livenessprobe-1-23-on-kubernetes-1-23	`cc85cbe`	link	true	`/test pull-kubernetes-csi-livenessprobe-1-23-on-kubernetes-1-23`
pull-kubernetes-csi-livenessprobe-1-26-on-kubernetes-1-26	`f7a9dea`	link	false	`/test pull-kubernetes-csi-livenessprobe-1-26-on-kubernetes-1-26`
pull-kubernetes-csi-livenessprobe-1-25-on-kubernetes-1-25	`f7a9dea`	link	true	`/test pull-kubernetes-csi-livenessprobe-1-25-on-kubernetes-1-25`
pull-kubernetes-csi-livenessprobe-unit	`f7a9dea`	link	true	`/test pull-kubernetes-csi-livenessprobe-unit`
pull-kubernetes-csi-livenessprobe-1-24-on-kubernetes-1-24	`f7a9dea`	link	true	`/test pull-kubernetes-csi-livenessprobe-1-24-on-kubernetes-1-24`

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Details

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. I understand the commands that are listed here.

Kartik494 · 2023-05-28T10:51:05Z

/easycla

carlory · 2023-06-06T06:57:02Z

✅ login: Kartik494 / name: Kartik Sharma (f7a9dea)

❌ The commit (dffae97, cc85cbe, 817e991, 4ccdb51). This user is missing the User's ID, preventing the EasyCLA check. Consult GitHub Help to resolve.For further assistance with EasyCLA, please submit a support request ticket.

@Kartik494 you can squash those commits into one to fix it.

git rebase -i HEAD~5

for example:

k8s-ci-robot · 2023-06-20T06:36:17Z

PR needs rebase.

Details

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.

jsafrane · 2023-09-15T10:56:27Z

/close
in favor of #202

k8s-ci-robot · 2023-09-15T10:56:31Z

@jsafrane: Closed this PR.

Details

In response to this:

/close
in favor of #202

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 added release-note Denotes a PR that will be considered when it comes time to generate release notes. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Nov 2, 2022

k8s-ci-robot requested review from ggriffiths and xing-yang November 2, 2022 04:18

k8s-ci-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Nov 2, 2022

Kartik494 force-pushed the structurelogging branch from 31599dc to 900dd34 Compare November 2, 2022 04:20

Support structure logging for livenessprobe container

dffae97

Kartik494 force-pushed the structurelogging branch from 900dd34 to dffae97 Compare November 2, 2022 05:11

mauriciopoppe reviewed Nov 7, 2022

View reviewed changes

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

Merge branch 'master' into structurelogging

cc85cbe

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

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

Merge branch 'master' into structurelogging

817e991

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

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

ggriffiths suggested changes Jan 19, 2023

View reviewed changes

Merge branch 'master' into structurelogging

4ccdb51

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

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

Merge branch 'master' into structurelogging

f7a9dea

k8s-ci-robot added cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. and removed cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels May 28, 2023

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

bells17 mentioned this pull request Aug 12, 2023

Support structured logging #165

Closed

k8s-ci-robot closed this Sep 15, 2023

Support structure logging for livenessprobe container #168

Support structure logging for livenessprobe container #168

Uh oh!

Conversation

Kartik494 commented Nov 2, 2022

Uh oh!

Kartik494 commented Nov 2, 2022

Uh oh!

Kartik494 commented Nov 2, 2022

Uh oh!

Kartik494 commented Nov 2, 2022

Uh oh!

Kartik494 commented Nov 3, 2022

Uh oh!

mauriciopoppe Nov 7, 2022

Choose a reason for hiding this comment

Uh oh!

Kartik494 Nov 9, 2022

Choose a reason for hiding this comment

Uh oh!

mauriciopoppe Nov 9, 2022

Choose a reason for hiding this comment

Uh oh!

Kartik494 Nov 10, 2022

Choose a reason for hiding this comment

Uh oh!

mauriciopoppe Nov 7, 2022

Choose a reason for hiding this comment

Uh oh!

Kartik494 Nov 9, 2022

Choose a reason for hiding this comment

Uh oh!

Kartik494 commented Nov 29, 2022

Uh oh!

Kartik494 commented Dec 19, 2022

Uh oh!

ggriffiths left a comment

Choose a reason for hiding this comment

Uh oh!

Kartik494 commented Jan 19, 2023

Uh oh!

Kartik494 commented Jan 19, 2023

Uh oh!

Kartik494 commented Jan 19, 2023

Uh oh!

linux-foundation-easycla bot commented May 28, 2023

Uh oh!

k8s-ci-robot commented May 28, 2023

Uh oh!

k8s-ci-robot commented May 28, 2023

Uh oh!

Kartik494 commented May 28, 2023

Uh oh!

carlory commented Jun 6, 2023

Uh oh!

k8s-ci-robot commented Jun 20, 2023

Uh oh!

jsafrane commented Sep 15, 2023

Uh oh!

k8s-ci-robot commented Sep 15, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants