Add log level by tuannvm · Pull Request #1527 · cert-manager/cert-manager

tuannvm · 2019-04-05T04:16:04Z

What this PR does / why we need it:

To allow specifying log level

Which issue this PR fixes:

There's no way to configure log level at the moment

Special notes for your reviewer:

Release note:

Allow specifying log level

jetstack-bot · 2019-04-05T04:16:21Z

Hi @tuannvm. Thanks for your PR.

I'm waiting for a jetstack or cert-manager 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.

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.

Signed-off-by: Tuan Nguyen <me@tuannvm.com>

munnerz · 2019-04-05T10:24:47Z

deploy/charts/cert-manager/values.yaml

  #   maxSurge: 0
  #   maxUnavailable: 1

+logLevel: 2


Can we make this variable a global, and use the same setting across the cainjector, controller and webhook deployments? 😄

webhook is currently hard coded to --v=12 and floods my log collector with ~500k entries per day.

Yep, definitely not ideal! If you can update this PR to make the variable a global, we should be able to hit two birds with one stone!

marc-barry · 2019-04-20T05:19:31Z

I think this pull request will address #1526.

munnerz · 2019-04-24T11:18:38Z

@tuannvm would you mind addressing the comment above, and making this variable a part of the global stanza and setting the --v flag on the webhook and cainjector component?

munnerz · 2019-04-24T11:19:04Z

You'll also need to bump the Helm chart versions and run bazel run //hack:update-deploy-gen to make the CI tests pass

/ok-to-test

tuannvm · 2019-04-25T04:14:59Z

@munnerz sure let me address this later today, thanks.

Signed-off-by: Tuan Nguyen <me@tuannvm.com>

tuannvm · 2019-04-30T09:03:43Z

/retest

Signed-off-by: Tuan Nguyen <me@tuannvm.com>

tuannvm · 2019-04-30T09:56:58Z

/test verify

tuannvm · 2019-04-30T10:09:43Z

@munnerz any idea how to fix the error above?

Signed-off-by: Tuan Nguyen <me@tuannvm.com>

munnerz · 2019-04-30T10:17:20Z

Same as before, you need to run bazel run //hack:update-deploy-gen and commit the results:

diff '--exclude=__main__' -Naupr /bazel-scratch/.cache/bazel/_bazel_root/e50ce42c8209d553b7e7491e69e67e89/sandbox/processwrapper-sandbox/4691/execroot/__main__/bazel-out/k8-fastbuild/bin/hack/verify-deploy-gen.runfiles/__main__/hack/..//deploy/manifests/cert-manager.yaml /tmp/tmp.NZ9gxKqHro/src/github.com/jetstack/cert-manager/deploy/manifests/cert-manager.yaml
--- /bazel-scratch/.cache/bazel/_bazel_root/e50ce42c8209d553b7e7491e69e67e89/sandbox/processwrapper-sandbox/4691/execroot/__main__/bazel-out/k8-fastbuild/bin/hack/verify-deploy-gen.runfiles/__main__/hack/..//deploy/manifests/cert-manager.yaml	2019-04-30 09:57:47.382035316 +0000
+++ /tmp/tmp.NZ9gxKqHro/src/github.com/jetstack/cert-manager/deploy/manifests/cert-manager.yaml	2019-04-30 10:01:17.849961350 +0000
@@ -1384,7 +1384,7 @@ spec:
 
 ---
 # Source: cert-manager/charts/webhook/templates/deployment.yaml
-piVersion: apps/v1beta1
+apiVersion: apps/v1beta1
 kind: Deployment
 metadata:
   name: cert-manager-webhook
/bazel-scratch/.cache/bazel/_bazel_root/e50ce42c8209d553b7e7491e69e67e89/sandbox/processwrapper-sandbox/4691/execroot/__main__/bazel-out/k8-fastbuild/bin/hack/verify-deploy-gen.runfiles/__main__/hack/../ is out of date. Please run 'bazel run //hack:update-deploy-gen'

tuannvm · 2019-04-30T10:21:08Z

/retest

tuannvm · 2019-04-30T10:33:03Z

@munnerz guess is okay already, can you help take another look? thanks

munnerz · 2019-04-30T14:41:53Z

Thanks for working through the issues here 😄

/lgtm
/approve

jetstack-bot · 2019-04-30T14:42:00Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: munnerz, tuannvm

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~~ [munnerz]

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

jetstack-bot added dco-signoff: no Indicates that at least one commit in this pull request is missing the DCO sign-off message. 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 Apr 5, 2019

jetstack-bot requested a review from kragniz April 5, 2019 04:16

jetstack-bot added area/deploy Indicates a PR modifies deployment configuration needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Apr 5, 2019

Add log level

affc45f

Signed-off-by: Tuan Nguyen <me@tuannvm.com>

tuannvm force-pushed the master branch 2 times, most recently from cba47fa to affc45f Compare April 5, 2019 04:26

jetstack-bot added dco-signoff: yes Indicates that all commits in the pull request have the valid DCO sign-off message. and removed dco-signoff: no Indicates that at least one commit in this pull request is missing the DCO sign-off message. labels Apr 5, 2019

munnerz reviewed Apr 5, 2019

View reviewed changes

marc-barry mentioned this pull request Apr 20, 2019

Excessive logging in cert-manager-webhook pod? #1526

Closed

jetstack-bot added ok-to-test and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Apr 24, 2019

tuannvm added 2 commits April 30, 2019 16:09

Address comments

545d040

Signed-off-by: Tuan Nguyen <me@tuannvm.com>

Add log level for cainjector

ac258bf

Signed-off-by: Tuan Nguyen <me@tuannvm.com>

tuannvm force-pushed the master branch from c52a481 to ac258bf Compare April 30, 2019 08:09

jetstack-bot removed the dco-signoff: no Indicates that at least one commit in this pull request is missing the DCO sign-off message. label Apr 30, 2019

jetstack-bot added the dco-signoff: yes Indicates that all commits in the pull request have the valid DCO sign-off message. label Apr 30, 2019

tuannvm added 3 commits April 30, 2019 17:16

Change order

f1de69a

Signed-off-by: Tuan Nguyen <me@tuannvm.com>

Update chart version

138208d

Signed-off-by: Tuan Nguyen <me@tuannvm.com>

Update chart version

723aadc

Signed-off-by: Tuan Nguyen <me@tuannvm.com>

jetstack-bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Apr 30, 2019

Bump chart version

ed027fc

Signed-off-by: Tuan Nguyen <me@tuannvm.com>

jetstack-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 30, 2019

Merge branch 'master' of github.com:jetstack/cert-manager

e17ff92

Signed-off-by: Tuan Nguyen <me@tuannvm.com>

jetstack-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 30, 2019

Fix typo

ce9cb8e

Signed-off-by: Tuan Nguyen <me@tuannvm.com>

munnerz added the tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges. label Apr 30, 2019

jetstack-bot assigned munnerz Apr 30, 2019

jetstack-bot added the lgtm Indicates that a PR is ready to be merged. label Apr 30, 2019

jetstack-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 30, 2019

jetstack-bot merged commit 4b4742d into cert-manager:master Apr 30, 2019

JoshVanL mentioned this pull request Oct 16, 2019

High rate of error log messages in webhook #1479

Closed

This was referenced Jan 3, 2024

Fix helm loglevel parsing #5860

Merged

global.logLevel is not documented in the Helm README file #6588

Closed

Conversation

tuannvm commented Apr 5, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jetstack-bot commented Apr 5, 2019

Uh oh!

munnerz Apr 5, 2019

Choose a reason for hiding this comment

Uh oh!

pabens Apr 15, 2019

Choose a reason for hiding this comment

Uh oh!

munnerz Apr 17, 2019

Choose a reason for hiding this comment

Uh oh!

marc-barry commented Apr 20, 2019

Uh oh!

munnerz commented Apr 24, 2019

Uh oh!

munnerz commented Apr 24, 2019

Uh oh!

tuannvm commented Apr 25, 2019

Uh oh!

tuannvm commented Apr 30, 2019

Uh oh!

tuannvm commented Apr 30, 2019

Uh oh!

tuannvm commented Apr 30, 2019

Uh oh!

munnerz commented Apr 30, 2019

Uh oh!

tuannvm commented Apr 30, 2019

Uh oh!

tuannvm commented Apr 30, 2019

Uh oh!

munnerz commented Apr 30, 2019

Uh oh!

jetstack-bot commented Apr 30, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

tuannvm commented Apr 5, 2019 •

edited

Loading