✨ feat(logging): make logging work by default (only v3+)

[georgettica]

Description

This PR makes the kubebuilder init initialize the default logging in a way that:

• by Default the log level is debug
• any flag passed to the operator will change that log level accordingly (see https://sdk.operatorframework.io/docs/building-operators/golang/references/logging/)

Motivation

to reduce friction and allow an operator developer not have to meddle with the main.go if not required, this will be related to trying inside the Reconcile function to call r.Log.V(2).Info("message") and trying to enable it on demand

Related issues

• OSDK->KB: Logging Flags #885 related to changing logging levels

[k8s-ci-robot]

Thanks for your pull request. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please follow instructions at https://git.k8s.io/community/CLA.md#the-contributor-license-agreement to sign the CLA.

It may take a couple minutes for the CLA signature to be fully registered; after that, please reply here with a new comment and we'll verify. Thanks.

---

• If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
• If you signed the CLA as a corporation, please sign in with your organization's credentials at https://identity.linuxfoundation.org/projects/cncf to be authorized.
• If you have done the above and are still having issues with the CLA being reported as unsigned, please log a ticket with the Linux Foundation Helpdesk: https://support.linuxfoundation.org/
• Should you encounter any issues with the Linux Foundation Helpdesk, send a message to the backup e-mail support address at: login-issues@jira.linuxfoundation.org

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.

[k8s-ci-robot]

Welcome @georgettica!

It looks like this is your first PR to kubernetes-sigs/kubebuilder 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes-sigs/kubebuilder has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

[k8s-ci-robot]

Hi @georgettica. Thanks for your PR.

I'm waiting for a kubernetes-sigs 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.

[camilamacedo86]

Hi @georgettica,

Thank you for your contribution. 🥇

some nits:

• Could you please squash the commits for we are able to move forward with?
• also, would not this PR close the issue OSDK->KB: Logging Flags #885? if yes, could you please add it in the first comment? Wdyt @joelanford ?
• Note that the first comment should have a description and the motivation for the change as well.

[camilamacedo86]

/ok-to-test

[camilamacedo86]

/test pull-kubebuilder-e2e-k8s-1-16-2

[georgettica]

do you think this could count as an help wanted issue?

[georgettica]

@camilamacedo86 rewrote the description and squashed commits

[georgettica]

as the test doesn't have an issue with my code and only a timeout will retest it
/test pull-kubebuilder-e2e-k8s-1-17-0

[camilamacedo86]

/test pull-kubebuilder-e2e-k8s-1-17-0

[camilamacedo86]

It shows ok for me 👍
/approve

@joelanford WDYT?

[georgettica]

@camilamacedo86 I think there is something off in my local deployment of this..

I seem to not be able to configure the log level from the deployment

can you TAL on that whenever you have a cycle?

the operator in question: https://github.com/openshift/route-monitor-operator

for now
/hold

[camilamacedo86]

Hi @georgettica,

shows that your project has a diff/old version of controller-runtime.
however, your comment makes me think that would be nice to add a check to ensure this change in kubebuilder/test/e2e/v3/. wDYT could you please add it for we are able to move forward with?

[georgettica]

@camilamacedo86 I want to add this change but it is a bit above my level of understanding for now.. can I merge this and pass the autotests to someone else?

[camilamacedo86]

It shows ok for me

@prafull01 @gabbifish @hasbro17 could you please give a hand to review this one?

[prafull01]

Verified this change locally. It shows OK for me 👍

[georgettica]

@prafull01 can you help with guiding me on this?

however, your comment makes me think that would be nice to add a check to ensure this change in kubebuilder/test/e2e/v3/. WDYT could you please add it for we are able to move forward with?

I know the change is to be made here https://github.com/kubernetes-sigs/kubebuilder/blob/master/test/e2e/v3/e2e_suite.go, but aside of that I wasn't able to figure out how to add my validation

[prafull01]

@prafull01 can you help with guiding me on this?

however, your comment makes me think that would be nice to add a check to ensure this change in kubebuilder/test/e2e/v3/. WDYT could you please add it for we are able to move forward with?

I know the change is to be made here https://github.com/kubernetes-sigs/kubebuilder/blob/master/test/e2e/v3/e2e_suite.go, but aside of that I wasn't able to figure out how to add my validation

@georgettica This zap logging changes are recently added to the controller-runtime and is not present in the older versions. Hence, e2e test should have a check which verifies that the log controller-manager is generating is following the log level, stacktrace level etc. The output of logs before this change is something like this:

2020-10-22T10:19:04.236+0530	INFO	controller-runtime.metrics	metrics server is starting to listen	{"addr": ":8080"}
2020-10-22T10:19:04.236+0530	INFO	controller-runtime.builder	Registering a mutating webhook	{"GVK": "crew.testproject.org/v1, Kind=Captain", "path": "/mutate-crew-testproject-org-v1-captain"}

The output of controller-manager logs after this change:

{"level":"info","ts":1603342108.069236,"logger":"controller-runtime.metrics","msg":"metrics server is starting to listen","addr":":8080"}
{"level":"info","ts":1603342108.069433,"logger":"controller-runtime.builder","msg":"Registering a mutating webhook","GVK":"crew.testproject.org/v1, Kind=Captain","path":"/mutate-crew-testproject-org-v1-captain"}

So, verification of this change that the logs are generated in proper format can be added to e2e tests, just after the following tests in e2e.

By("validate the controller-manager pod running as expected")

@camilamacedo86 Please correct me if there is mistake in my understanding.

[camilamacedo86]

Really thank you @prafull01. It is perfect.

@georgettica,

Ideally, when we do a change we need to ensure its tests as well. However, since @prafull01 help here and manually checks it in #1721 (comment) and it is very small, if you still strangle to add the tests as he described I think that it is fine we can move forward and to do it a follow-up. But we need to raise an issue at least for that do not get lost. Let's see what others think as well. So, it is for me:

/approve

[camilamacedo86]

The follow up to the tests was tacked in : #1739

/approve
/lgtm

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: camilamacedo86, georgettica

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:

• OWNERS [camilamacedo86]

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

[georgettica]

Thanks @camilamacedo86 ! I will follow up on it there and see if I can help.push it forward

[estroz]

There's an upstream bug that results in Development: true not being applied, which is why you're seeing a different output format. See kubernetes-sigs/controller-runtime#1228 (fixed in v0.7.0).