New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
gateway-api: set controller-runtime logger #27961
gateway-api: set controller-runtime logger #27961
Conversation
This commit adds the dependency github.com/bombsimon/logrusr which is the logrus implementation of go-logr. It's used to pass a Cilium logrus logger instance to the controller-runtime (which uses go-logr). Signed-off-by: Marco Hofstetter <marco.hofstetter@isovalent.com>
Currently, the controller-runtime emits an error in the log, if the global logger instance (go-logr) isn't set after 30 seconds. (https://github.com/kubernetes-sigs/controller-runtime/blob/v0.15.1/pkg/log/log.go#L57-L62) ``` [controller-runtime] log.SetLogger(...) was never called, logs will not be displayed: goroutine 663 [running]: runtime/debug.Stack() /usr/local/go/src/runtime/debug/stack.go:24 +0x6b sigs.k8s.io/controller-runtime/pkg/log.eventuallyFulfillRoot() /go/pkg/mod/sigs.k8s.io/controller-runtime@v0.15.1/pkg/log/log.go:59 +0x9b sigs.k8s.io/controller-runtime/pkg/log.(*delegatingLogSink).WithValues(0xc00093fa00, {0xc00092e780, 0x2, 0x2}) /go/pkg/mod/sigs.k8s.io/controller-runtime@v0.15.1/pkg/log/deleg.go:168 +0x4b github.com/go-logr/logr.Logger.WithValues({{0x43f19e0, 0xc00093fa00}, 0x0}, {0xc00092e780, 0x2, 0x2}) /go/pkg/mod/github.com/go-logr/logr@v1.2.4/logr.go:323 +0x70 sigs.k8s.io/controller-runtime/pkg/builder.(*Builder).doController.func1(0xc00092e760) /go/pkg/mod/sigs.k8s.io/controller-runtime@v0.15.1/pkg/builder/controller.go:398 +0x227 sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).reconcileHandler(0xc00077c460, {0x43ec290, 0xc00003f360}, {0x3d2c480, 0xc00092e740}) /go/pkg/mod/sigs.k8s.io/controller-runtime@v0.15.1/pkg/internal/controller/controller.go:305 +0x276 sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).processNextWorkItem(0xc00077c460, {0x43ec290, 0xc00003f360}) /go/pkg/mod/sigs.k8s.io/controller-runtime@v0.15.1/pkg/internal/controller/controller.go:265 +0x3c5 sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).Start.func2.2() /go/pkg/mod/sigs.k8s.io/controller-runtime@v0.15.1/pkg/internal/controller/controller.go:226 +0xca created by sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).Start.func2 in goroutine 532 /go/pkg/mod/sigs.k8s.io/controller-runtime@v0.15.1/pkg/internal/controller/controller.go:222 +0x985 ``` Therefore, this commit explicitly sets the logger instances that should be used by the controller-runtime: * global logger * gateway-api controller logger Signed-off-by: Marco Hofstetter <marco.hofstetter@isovalent.com>
/test |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM for me
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The sigs.k8s.io/controller-runtime
module requires a go-logr compatible logger via it's API, which the logrusr
dependency implements and allows us to convert our logrus logger to a compatible one. Vendor changes lgtm in light of that.
Currently, the controller-runtime emits an error in the Cilium operator log, if the global logger instance (go-logr) isn't set after 30 seconds.
(https://github.com/kubernetes-sigs/controller-runtime/blob/v0.15.1/pkg/log/log.go#L57-L62)
In addition, the log messages of the controller-runtime aren't logged at all (
SetLogger(logr.New(NullLogSink{}))
)Therefore, this commit explicitly sets the logger instances that should be used by the controller-runtime: