Why two different log packages?

[jaypipes]

I'm wondering why the manager's main.go uses the sigs.k8s.io/controller-runtime/pkg/log/zap library:

kubebuilder/pkg/scaffold/v2/main.go

Line 139 in 19458ba

"sigs.k8s.io/controller-runtime/pkg/log/zap"

the webhook generator uses the sigs.k8s.io/controller-runtime/pkg/log package as well:

kubebuilder/pkg/scaffold/v2/webhook/webhook.go

Line 94 in 19458ba

logf "sigs.k8s.io/controller-runtime/pkg/runtime/log"

but the controller uses the github.com/go-logr/logr library:

kubebuilder/pkg/scaffold/v2/controller.go

Line 74 in 19458ba

"github.com/go-logr/logr"

Would it be beneficial to standardize on a single package?

/triage support

[DirectXMan12]

I'm wondering why the manager's main.go uses the sigs.k8s.io/controller-runtime/pkg/log/zap library:

pkg/log/zap is a helper for setting up logr with Zap. Logr is a generic logging interface. Zap is one concrete implementation.

the webhook generator uses the sigs.k8s.io/controller-runtime/pkg/log package as well

pkg/log is a helper for cases where you haven't plumbed through a copy of logr.Logger and want a handle to a semi-global logger

That being said, it's actually using pkg/runtime/log, which is a deprecated path. Not sure how that snuck into v2. cc @mengqiy

but the controller uses the github.com/go-logr/logr library:

Right. Most things should try to just pass down a copy of logr.Logger when they can. When they can't, they can fall back to pkg/log if they need to. Since the webhook code doesn't have its own struct to store the logger in, it has to use the global logger for the moment.

[DirectXMan12]

/good-first-issue

on the replacement of pkg/runtime/log for pkg/log in just the v2 scaffolding.

[k8s-ci-robot]

@DirectXMan12:
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:

/good-first-issue

on the replacement of pkg/runtime/log for pkg/log in just the v2 scaffolding.

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.

[jaypipes]

Thanks very much for the background @DirectXMan12! :) I'll try to hop on this contribution early next week.

[mengqiy]

@jaypipes That's a bug got snuck in.
Thanks for catching and trying to fix it! Feel free to assign me when you have the PR.

[dionysius]

Confirm that as well with staticcheck (golanci-lint) we should move from pkg/runtime/log to pkg/log

api/v1/sampleresource_webhook.go:6:2: SA1019: Package sigs.k8s.io/controller-runtime/pkg/runtime/log is deprecated: use pkg/log  (staticcheck)
        logf "sigs.k8s.io/controller-runtime/pkg/runtime/log"

[mengqiy]

It seems no PR has been opened to fix the bug.
@camilamacedo86 Do you have bandwidth to fix this bug? It should be a straightforward one.

[camilamacedo86]

/assign @camilamacedo86