CONSOLE-1523: Change log level values to use klog types #250

zherman0 · 2019-06-28T17:58:26Z

This changes the capnslog values to accepted klog log level values. I have just converted them as best I can but we might want to re-evaluate what each maps to.

CONSOLE-1523

zherman0 · 2019-06-28T18:11:13Z

/assign @jhadvig

zherman0 · 2019-06-28T18:12:04Z

/hold
hold until Console openshift/console#1862 merges

zherman0 · 2019-06-28T20:54:40Z

/retest

pkg/console/subresource/deployment/deployment.go

benjaminapetersen · 2019-11-12T15:11:30Z

@zherman0 this one has sat for a bit, finally getting the open PRs list down enough to take a look. Is there a matching console PR? I assume both need to change at more or less the same time. Or does the console have some logic to ensure we don't get in a bad state due to flag mismatch?

benjaminapetersen · 2019-11-12T15:11:50Z

Button click misfire :)

zherman0 · 2019-11-12T16:22:16Z

@benjaminapetersen @spadgett - There is a a corresponding console PR: openshift/console#1862
Currently, the changes I made required importing api machinery (I think) which makes it a XXL change and Sam was not sure he wanted all that extra weight, so we put it in hold.
There might be a way to re-write the klog stuff on the console side to not use the specific library but I am not sure we want to do that. Any thoughts if we should close these for now?

benjaminapetersen · 2019-11-14T22:54:58Z

@zherman0 so the klog go.mod file is small:

module k8s.io/klog/v2

go 1.12

require github.com/go-logr/logr v0.1.0

Curious, do you need the kubernetes component base?

I'm guessing thats why the PR blew up.

benjaminapetersen · 2019-12-02T17:23:13Z

So as a control, if I run console-operator as ManagementState: Unmanaged, so that I can pass either TRACE or -v=4 (either alternative to the log level flag) I will see a CrashLoopBackoff:

The e2e tests pass here likely because we don't actually check that pods are all happy.

pkg/console/subresource/deployment/deployment.go

benjaminapetersen

Hasn't quite got the correct flags yet.

zherman0 · 2019-12-05T16:24:41Z

For log level values, I am using this doc: https://github.com/kubernetes/community/blob/master/contributors/devel/sig-instrumentation/logging.md

benjaminapetersen

Need the correct flag here.

benjaminapetersen · 2019-12-06T17:16:37Z

pkg/console/subresource/deployment/deployment.go

 	switch logLevel {
 	case operatorv1.Normal:
-		flag = "--log-level=*=NOTICE"
+		flag = "--log-level=2"


These are still

--log-level=2

But we need to update to

-v=2

Wrong flag.

I am leaving this like this for now since console can handle the old values. We will get console portion merged first and then update this. I will update the code but I want to make sure this does not get merged.

This PR won't merge until the -v flag is updated, so you should be safe. Definitely free to do whatever locally, but I would prob push the correct code up for the open PR.

benjaminapetersen · 2019-12-13T19:50:11Z

pkg/console/subresource/deployment/deployment.go

 	flag := ""
-	// Since the console-operator logging has different logging levels then the capnslog,
-	// that we use for console server(bridge) we need to map them to each other
+	// console server(bridge) expects flag log-level


This comment can go away, it will no longer expect log-level after these merge.

zherman0 · 2019-12-16T16:51:36Z

/retest

jhadvig · 2020-05-11T14:49:41Z

/retest

jhadvig · 2020-05-12T14:38:03Z

/test e2e-aws-operator

openshift-ci-robot · 2020-09-09T17:03:13Z

@zherman0: The following tests failed, say /retest to rerun all failed tests:

Test name	Commit	Details	Rerun command
ci/prow/e2e-aws-upgrade	`8646869`	link	`/test e2e-aws-upgrade`
ci/prow/e2e-aws-operator	`613da74`	link	`/test e2e-aws-operator`
ci/prow/e2e-upgrade	`613da74`	link	`/test e2e-upgrade`

Full PR test history. Your PR dashboard.

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.

jhadvig

Aligning to the values that we are using in the Console's Switch to klog PR

jhadvig · 2020-10-15T08:57:53Z

pkg/console/subresource/deployment/deployment.go

+	case operatorv1.Trace:
+		flag = "-v=4"
+	case operatorv1.TraceAll:
+		flag = "-v=5"


Suggested change

flag = "-v=5"

flag = "-v=10"

jhadvig · 2020-10-15T08:58:07Z

pkg/console/subresource/deployment/deployment.go

-		flag = "--log-level=*=DEBUG"
-	case operatorv1.Trace, operatorv1.TraceAll:
-		flag = "--log-level=*=TRACE"
+		flag = "-v=3"


Suggested change

flag = "-v=3"

flag = "-v=4"

jhadvig · 2020-10-15T08:58:23Z

pkg/console/subresource/deployment/deployment.go

-		flag = "--log-level=*=TRACE"
+		flag = "-v=3"
+	case operatorv1.Trace:
+		flag = "-v=4"


Suggested change

flag = "-v=4"

flag = "-v=6"

jhadvig · 2020-10-20T14:00:14Z

/retest

spadgett · 2020-10-20T14:37:20Z

/assign @yapei @ahardin-rh @sferich888
for QE / docs / CEE approval

This is ready to test. @jhadvig @zherman0 I assume this should be tested with openshift/console#1862 together using cluster-bot?

spadgett

/approve

jhadvig · 2020-10-20T15:58:18Z

@spadgett was successfully able to launch a cluster using cluster with combination of both PRs.
launch openshift/console-operator#250,openshift/console#1862

Checked if the --v=2 flag was present in the console deployment

Also check the console pod if it's running:

yapei · 2020-10-21T02:24:22Z

/label qe-approved

openshift-ci-robot · 2020-10-21T02:24:23Z

@yapei: The label(s) qe-approved cannot be applied, because the repository doesn't have them

Details

In response to this:

/label qe-approved

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.

yapei · 2020-10-21T02:27:27Z

Launched cluster launch openshift/console-operator#250,openshift/console#1862 and tested following the steps in test case. The changes look good to me

yapei · 2020-10-21T02:28:30Z

/lgtm

sferich888 · 2020-10-22T12:36:30Z

@ahardin-rh do we have a corresponding docs PR to link this to? That shows users how to manipulate this?
I don't seen any docs related work trackers linked from https://issues.redhat.com/browse/CONSOLE-1523 and I want to make sure we don't miss documenting a that this change (possibly in release notes) and how it can be manipulated.

openshift-merge-robot · 2020-10-23T05:17:00Z

@zherman0: The following test failed, say /retest to rerun all failed tests:

Test name	Commit	Details	Rerun command
ci/prow/e2e-upgrade	`cbfb761`	link	`/test e2e-upgrade`

Full PR test history. Your PR dashboard.

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.

jhadvig · 2020-10-27T08:41:57Z

/hold cancel

jhadvig · 2020-10-27T08:42:02Z

/retest

openshift-bot · 2020-10-27T08:46:58Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-ci-robot · 2020-10-27T10:24:30Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jhadvig, spadgett, yapei, zherman0

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

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [jhadvig,spadgett]

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

jhadvig · 2020-10-27T10:25:00Z

@ahardin-rh could you please respond to #250 (comment) . Thanks ! :)

openshift-ci-robot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Jun 28, 2019

openshift-ci-robot requested review from benjaminapetersen and spadgett June 28, 2019 17:58

zherman0 mentioned this pull request Jun 28, 2019

CONSOLE-1523: Switch logging to klog openshift/console#1862

Merged

openshift-ci-robot assigned jhadvig Jun 28, 2019

openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jun 28, 2019

jhadvig requested changes Jul 1, 2019

View reviewed changes

pkg/console/subresource/deployment/deployment.go Show resolved Hide resolved

openshift-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Jul 1, 2019

benjaminapetersen closed this Nov 12, 2019

benjaminapetersen reopened this Nov 12, 2019

benjaminapetersen mentioned this pull request Dec 2, 2019

[WIP] Add ConsolePodsMustSettle() helper to test/e2e #357

Closed

zherman0 force-pushed the loglevel branch from 8646869 to 9af86e0 Compare December 4, 2019 15:13

benjaminapetersen reviewed Dec 5, 2019

View reviewed changes

pkg/console/subresource/deployment/deployment.go Outdated Show resolved Hide resolved

benjaminapetersen suggested changes Dec 5, 2019

View reviewed changes

zherman0 force-pushed the loglevel branch from 29a20d7 to 75e9564 Compare December 5, 2019 17:39

benjaminapetersen suggested changes Dec 6, 2019

View reviewed changes

zherman0 force-pushed the loglevel branch from 75e9564 to 2aa8308 Compare December 13, 2019 00:44

benjaminapetersen reviewed Dec 13, 2019

View reviewed changes

zherman0 force-pushed the loglevel branch from 2aa8308 to 613da74 Compare December 16, 2019 15:20

jhadvig requested changes Oct 15, 2020

View reviewed changes

Change log level values to use klog types

cbfb761

jhadvig force-pushed the loglevel branch from 613da74 to cbfb761 Compare October 20, 2020 11:36

spadgett changed the title ~~Change log level values to use klog types~~ CONSOLE-1523: Change log level values to use klog types Oct 20, 2020

openshift-ci-robot assigned ahardin-rh, sferich888 and yapei Oct 20, 2020

spadgett approved these changes Oct 20, 2020

View reviewed changes

openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 20, 2020

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Oct 21, 2020

openshift-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 27, 2020

jhadvig approved these changes Oct 27, 2020

View reviewed changes

openshift-merge-robot merged commit 54699a7 into openshift:master Oct 27, 2020

CONSOLE-1523: Change log level values to use klog types #250

CONSOLE-1523: Change log level values to use klog types #250

Uh oh!

Conversation

zherman0 commented Jun 28, 2019 • edited by openshift-ci-robot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

zherman0 commented Jun 28, 2019

Uh oh!

zherman0 commented Jun 28, 2019

Uh oh!

zherman0 commented Jun 28, 2019

Uh oh!

Uh oh!

benjaminapetersen commented Nov 12, 2019

Uh oh!

benjaminapetersen commented Nov 12, 2019

Uh oh!

zherman0 commented Nov 12, 2019

Uh oh!

benjaminapetersen commented Nov 14, 2019

Uh oh!

benjaminapetersen commented Dec 2, 2019

Uh oh!

Uh oh!

benjaminapetersen left a comment

Choose a reason for hiding this comment

Uh oh!

zherman0 commented Dec 5, 2019

Uh oh!

benjaminapetersen left a comment

Choose a reason for hiding this comment

Uh oh!

benjaminapetersen Dec 6, 2019

Choose a reason for hiding this comment

Uh oh!

zherman0 Dec 6, 2019

Choose a reason for hiding this comment

Uh oh!

benjaminapetersen Dec 9, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

benjaminapetersen Dec 13, 2019

Choose a reason for hiding this comment

Uh oh!

zherman0 commented Dec 16, 2019

Uh oh!

jhadvig commented May 11, 2020

Uh oh!

jhadvig commented May 12, 2020

Uh oh!

openshift-ci-robot commented Sep 9, 2020

Uh oh!

jhadvig left a comment

Choose a reason for hiding this comment

Uh oh!

jhadvig Oct 15, 2020

Choose a reason for hiding this comment

Uh oh!

jhadvig Oct 15, 2020

Choose a reason for hiding this comment

Uh oh!

jhadvig Oct 15, 2020

Choose a reason for hiding this comment

Uh oh!

jhadvig commented Oct 20, 2020

Uh oh!

spadgett commented Oct 20, 2020

Uh oh!

spadgett left a comment

Choose a reason for hiding this comment

Uh oh!

jhadvig commented Oct 20, 2020

Uh oh!

yapei commented Oct 21, 2020

Uh oh!

openshift-ci-robot commented Oct 21, 2020

Uh oh!

yapei commented Oct 21, 2020

zherman0 commented Jun 28, 2019 •

edited by openshift-ci-robot

Loading

benjaminapetersen Dec 9, 2019 •

edited

Loading

openshift-merge-robot commented Oct 23, 2020 •

edited

Loading