Remove raw klog calls in controllers

[chuckha]

/kind feature

Describe the solution you'd like
A reconciler should own its own logger and not depend on global functions to log. This will allow us to print log lines more consistently at the expensive of one extra parameter somewhere.

The logging should be pluggable. If I decide to write my own main.go and import the machine and controller managers I should be able to use whatever logger I bring so long as it satisfies the recommended logger interface.

We can use github.com/go-logr/logr for the logging interface since that seems to be the standard for cluster-api across the board. This issue should keep the logs as exact as possible (log levels, context, etc).

I would recommend this post in controller-runtime for anyone interested in logging and why logr may not be the best choice. https://github.com/kubernetes-sigs/controller-runtime/blob/master/TMP-LOGGING.md

Out of scope: Any calls to klog outside of the controllers can be saved for another issue.

Note: Getting klogr to work is tricky. You will have to manually parse the -v flag and set the value on the logger. It is not automatic. Please test your changes by running the manager in a cluster and viewing logging output.

/good-first-issue
/priority important-longterm

My recommended workflow, entirely optional, can completely ignore this part

I would do this in two commits and one PR

1. Add a logging interface to the reconcilers but don't use them yet. In main.go we should use klogr to satisfy the interface created/used in the reconciler. (likely go-logr/logr).

2. After the infrastructure is set up, replace all of the klog calls with calls to the reconciler's logger. Ensure the controllers do not import k8s.io/klog.

[k8s-ci-robot]

@chuckha:
This request has been marked as suitable for new contributors.

Please ensure the request meets the requirements listed here.

If this request no longer meets these requirements, the label can be removed
by commenting with the /remove-good-first-issue command.

In response to this:

/kind feature

Describe the solution you'd like
A reconciler should own its own logger and not depend on global functions to log. This will allow us to print log lines more consistently at the expensive of one extra parameter somewhere.

The logging should be pluggable. If I decide to write my own main.go and import the machine and controller managers I should be able to use whatever logger I bring so long as it satisfies the recommended logger interface.

We can use github.com/go-logr/logr for the logging interface since that seems to be the standard for cluster-api across the board. This issue should keep the logs as exact as possible (log levels, context, etc).

I would recommend this post in controller-runtime for anyone interested in logging and why logr may not be the best choice. https://github.com/kubernetes-sigs/controller-runtime/blob/master/TMP-LOGGING.md

Out of scope: Any calls to klog outside of the controllers can be saved for another issue.

Note: Getting klogr to work is tricky. You will have to manually parse the -v flag and set the value on the logger. It is not automatic. Please test your changes by running the manager in a cluster and viewing logging output.

/good-first-issue
/priority important-longterm

My recommended workflow, entirely optional, can completely ignore this part

I would do this in two commits and one PR

1. Add a logging interface to the reconcilers but don't use them yet. In main.go we should use klogr to satisfy the interface created/used in the reconciler. (likely go-logr/logr).

2. After the infrastructure is set up, replace all of the klog calls with calls to the reconciler's logger. Ensure the controllers do not import k8s.io/klog.

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.

[vincepri]

Awesome @chuckha! This is a really needed improvement. I was hoping we could tackle some parts of this PR with #1075 at some point in the future.

+1 starting to have the reconcilers own their own loggers.

[protess]

Greetings @chuckha @vincepri , I am new to cluster-api group and as this issue is labeled 'good first issue' I'd love to contribute if this hasn't been assigned to anyone yet.
Thanks!

[chuckha]

@protess that would be great! I can't assign the issue to you, but i'll assign it to me as a proxy for you. I'm happy to help offer guidance if you need any, but otherwise, tag me in the PR!

/assign
/lifecycle active

[chuckha]

@protess Any updates on this? Do you need any assistance from me?

[protess]

@chuckha My apologies, I am a slow starter and I am still reading thru https://cluster-api.sigs.k8s.io/ along with source codes. If not I will prolly ask too many questions in slack channel. If you don't mind allow me to read it thru few more days and I will ask for assistance.

[chuckha]

not a problem! don't worry about making noise in the channel or feel free to ping me directly on slack!

[ncdc]

@protess just checking back in - are you still interested in working on this and will you have some time?

[ncdc]

@protess checking in again - do you have any updates? Thanks!

[tahsinrahman]

/assign

[tahsinrahman]

/lifecycle active