Improve logging

[meyskens]

What this PR does / why we need it:
This PR implements 3 things:

1. Use klog/v2 following changes in Kubernetes
2. Define a log level everywhere to improve log level filtering
3. Move all logging logic to use our package to later on add extra features such as JSON logs

Side effects that happened:

1. I had to update our Kubernetes dependencies, this also caused CRDs to be changed due to improvements in controller-gen Go was messing with me... it compiles somehow now /shrug

Which issue this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close that issue when PR gets merged): fixes #

Fixes #3051
Fixes #2887

Special notes for your reviewer:

Let's have a discussion about which logs should be on which level, I had a quick run through but sure will have missed some incorrect levels.

Release note:

Use klog v2 and improve the use of log levels

[wallrj]

Thanks @meyskens ,

I agree that it'll be nice to have all the logs use consistent levels.

I haven't read all the changes, but it seems strange / unfamiliar to me to have to specify the level when logging errors and warnings.
In other projects (and in Kubernetes) those methods imply a log level, don't they? i.e. klog.Error(... would always log at level 0, right?

Also unfamiliar to me is the use of a named level, rather than a simple .V(2). I find the number more readable, and works well if the levels are all documented, as in that community logging document you linked to.

Could be that the logging style in this PR is based on precedents from another project, in which case maybe add a note explaining that and why this style has been chosen.

[meyskens]

don't they? i.e. klog.Error(... would always log at level 0, right?

Yes, I was about to change those back those were a mistake of mine

Also unfamiliar to me is the use of a named level,

the log levels did exist in our codebase before, personally I find these more readable as they are explicit to their purpose, also it allows to shift log levels better imo. Happy to discuss this!

[meyskens]

/kind cleanup

[meyskens]

/test pull-cert-manager-deps

[james-w]

Looks good to me with one comment

[wallrj]

A partial review 56 / 119 files viewed.

• I couldn't understand why the generated pkg/client code had changed?

Here are the logsI get from the cert-manager deployment:

$ kubectl -n cert-manager logs deploy/cert-manager
I0806 15:02:38.552323       1 start.go:74] cert-manager "msg"="starting controller"  "git-commit"="448f9534fc90d235aa915d57f253354c97c3c58a" "version"="1596725812"
W0806 15:02:38.552402       1 client_config.go:608] Neither --kubeconfig nor --master was specified.  Using the inClusterConfig.  This might not work.
I0806 15:02:38.553954       1 controller.go:161] cert-manager/controller/build-context "msg"="configured acme dns01 nameservers" "nameservers"=["10.0.0.16:53"] 
I0806 15:02:38.555113       1 controller.go:124] cert-manager/controller "msg"="starting leader election"  
I0806 15:02:38.555380       1 metrics.go:162] cert-manager/controller/build-context/metrics "msg"="listening for connections on" "address"={"IP":"::","Port":9402,"Zone":""} 
I0806 15:02:38.556248       1 leaderelection.go:243] attempting to acquire leader lease  kube-system/cert-manager-controller...
I0806 15:02:38.570941       1 leaderelection.go:253] successfully acquired lease kube-system/cert-manager-controller
I0806 15:02:38.573044       1 reflector.go:207] Starting reflector *v1.Secret (5m0s) from external/io_k8s_client_go/tools/cache/reflector.go:156
I0806 15:02:38.673309       1 reflector.go:207] Starting reflector *v1beta1.Ingress (30s) from external/io_k8s_client_go/tools/cache/reflector.go:156
I0806 15:02:38.673393       1 reflector.go:207] Starting reflector *v1alpha2.Certificate (30s) from external/io_k8s_client_go/tools/cache/reflector.go:156
I0806 15:02:38.673426       1 reflector.go:207] Starting reflector *v1.Secret (30s) from external/io_k8s_client_go/tools/cache/reflector.go:156
I0806 15:02:38.673428       1 reflector.go:207] Starting reflector *v1alpha2.ClusterIssuer (30s) from external/io_k8s_client_go/tools/cache/reflector.go:156
I0806 15:02:38.673688       1 reflector.go:207] Starting reflector *v1alpha2.Order (30s) from external/io_k8s_client_go/tools/cache/reflector.go:156
I0806 15:02:38.673702       1 reflector.go:207] Starting reflector *v1.Pod (30s) from external/io_k8s_client_go/tools/cache/reflector.go:156
I0806 15:02:38.673325       1 reflector.go:207] Starting reflector *v1.Service (30s) from external/io_k8s_client_go/tools/cache/reflector.go:156
I0806 15:02:38.673309       1 reflector.go:207] Starting reflector *v1alpha2.Challenge (30s) from external/io_k8s_client_go/tools/cache/reflector.go:156
I0806 15:02:38.673968       1 reflector.go:207] Starting reflector *v1alpha2.Issuer (30s) from external/io_k8s_client_go/tools/cache/reflector.go:156
I0806 15:02:38.674011       1 reflector.go:207] Starting reflector *v1alpha2.CertificateRequest (30s) from external/io_k8s_client_go/tools/cache/reflector.go:156

[wallrj]

A few more comments.

[meyskens]

@wallrj first of all great reviews keep them coming! I am first going to fix the test before implementing the feedback.
So why the generated code changed? I had to update all deps to Kubernetes 1.19 so we use the same version of klog. That release has a bunch of fixes in the code generation par as the CRD generation.

[meyskens]

/hold

Waiting on #3166 to merge to rebase this on

[meyskens]

Sometimes go modules can mess with you.... on cherry picking 1.19 out i found out it somehow works on 1.18 after all...
So removed that part...

/unhold

[meyskens]

/test pull-cert-manager-bazel

[wallrj]

I've read through the files changed since last review and all looks ok to me.

[wallrj]

/lgtm

[jetstack-bot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: meyskens, wallrj

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 [meyskens]

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