-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Controller Runtime Outputs klog
in non-standard format
#2656
Comments
/kind support |
This issue explains why this is happening. I believe if you are not setting the log.SetLogger() you will not have those logs displayed. In the release below, it explains why this was done, with this PR being the commit where it landed. https://github.com/kubernetes-sigs/controller-runtime/releases/tag/v0.15.0 Are you trying not to set this or are you setting this in main with log.SetLogger(logr) and the leader-election logging in not being structured. I'm trying to understand the issue you are having. |
It's the second one. We are calling |
Sounds like something that should be fixed |
I'm a tad skeptical that we can "just fix" this in controller-runtime itself. Calls to klog are global and I don't think controller-runtime can assume that it is the sole one that is controlling the logger in the binary. It seems like there may be no way for a library to just enable this. Maybe the best solve here is for kubebuilder to bootstrap a |
Interesting. We get a different behavior in Cluster API. We are using component-base/logs to setup our logger: https://github.com/kubernetes-sigs/cluster-api/blob/edc8348877aa08f15de24564f0f1dd121b408f5b/main.go#L239-L245 IIRC, under the hood this uses zap for JSON logging and klog directly (?) for normal text logging. Resulting logs look like this: https://storage.googleapis.com/kubernetes-jenkins/logs/periodic-cluster-api-e2e-main/1755221027673083904/artifacts/clusters/bootstrap/logs/capi-system/capi-controller-manager/capi-controller-manager-55c7446fcb-dzrcx/manager.log
Just for context for other readers. These are the "offending" calls: https://github.com/kubernetes/client-go/blob/master/tools/leaderelection/leaderelection.go#L245-L264 klog.Infof("attempting to acquire leader lease %v...", desc) I assume component-base/logs sets up this stuff in a way that klog "forwards" to zap when JSON logging is configured. I see two possible options:
|
The Kubernetes project currently lacks enough contributors to adequately respond to all issues. This bot triages un-triaged issues according to the following rules:
You can:
Please send feedback to sig-contributor-experience at kubernetes/community. /lifecycle stale |
The Kubernetes project currently lacks enough active contributors to adequately respond to all issues. This bot triages un-triaged issues according to the following rules:
You can:
Please send feedback to sig-contributor-experience at kubernetes/community. /lifecycle rotten |
/remove-lifecycle rotten |
A temporary workaround would be importing
It is kind of annoying that that a line of unstructured log just shows up while everything else are nicely formatted. |
I think the best way to address this issue is to engage with upstream folks maintaining this package. Controller-runtime won't fork the package just to get rid of the log. |
The Kubernetes project currently lacks enough contributors to adequately respond to all issues. This bot triages un-triaged issues according to the following rules:
You can:
Please send feedback to sig-contributor-experience at kubernetes/community. /lifecycle stale |
The Kubernetes project currently lacks enough active contributors to adequately respond to all issues. This bot triages un-triaged issues according to the following rules:
You can:
Please send feedback to sig-contributor-experience at kubernetes/community. /lifecycle rotten |
/remove-lifecycle rotten |
We do end up doing this (https://github.com/kubernetes-sigs/karpenter/blob/main/pkg/operator/operator.go#L122) -- it would just be nice if we didn't have to :) Agreed that working with changing the actual leader election client is probably the way though. |
It seems the upstream leader election package, which controller runtime consumes will not output structured logging unless a global logger via klog is explicitly set. kubernetes-sigs/controller-runtime#2656 Signed-off-by: Jacob Weinstock <[email protected]>
We are leveraging controller-runtime default logging to write our reconcilers; however, oddly, the leader election logs do not get output in the same format as our other logs. Our other logs use structured logging through zap, but without a call to
klog.SetLogger()
we don't get the same formatting for leader election logs. I would have anticipated that controller runtime could do something in the startup log configuration (similar to therest.SetDefaultWarningHandler
call that is made)Considering how common leader election is, am I just missing some easy configuration knob in controller-runtime or do we just need to add a feature to support this out-of-the-box?
The text was updated successfully, but these errors were encountered: