-
Notifications
You must be signed in to change notification settings - Fork 822
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
silence the 'log.SetLogger(...) was never called; logs will not be displayed' error #4885
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## master #4885 +/- ##
==========================================
- Coverage 53.07% 53.06% -0.01%
==========================================
Files 251 251
Lines 20389 20389
==========================================
- Hits 10821 10820 -1
Misses 8855 8855
- Partials 713 714 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
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.
Thanks a lot!
/lgtm
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.
Does this problem exist other components?
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.
Thank you for your excellent analysis.
maybe we also need to update the webhook of example?
https://github.com/karmada-io/karmada/blob/master/examples/customresourceinterpreter/webhook/main.go
Good advice. I'm lining it up. |
After troubleshooting, components karmada/pkg/util/gclient/gclient.go Lines 64 to 69 in 0e3c382
log.SetLogger(...) was never called; logs will not be displayed error.Component karmada-controller-manager only calls the above function if len(opts.ClusterAPIKubeconfig) !=0 , and that's why we don't usually find this error in component karmada-controller-manager !
|
…splayed' error Signed-off-by: zhzhuang-zju <m17799853869@163.com>
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.
/approve
@whitewindmills @liangyuanpeng
Do you want to take another look?
Does this problem exist other components?
maybe we also need to update the webhook of example?
Excellent comments! By the way. Thanks.
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: RainbowMango 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:
Approvers can indicate their approval by writing |
/lgtm |
What type of PR is this?
/kind cleanup
What this PR does / why we need it:
Refer to #4868, when
karmada-webhook
is started, the errorlog.SetLogger(...) was never called; logs will not be displayed
will be displayed, which is very confusing.Here's the answer to why this log appears and how to fix it.
Viewing the function call chain, refer to:
karmada/vendor/sigs.k8s.io/controller-runtime/pkg/webhook/admission/webhook.go
Lines 178 to 184 in 95bf37f
If the log of webhook is nil, it will end up calling
karmada/vendor/sigs.k8s.io/controller-runtime/pkg/log/log.go
Lines 54 to 72 in 95bf37f
And the
SetLogger
is not called within the first 30 seconds of a binaries lifetime, finally printed outlog.SetLogger(...) was never called; logs will not be displayed
error.Through the above analysis, there are two ways to solve this problem:
Initializing the webhook's log
Looking at the source code, there are two ways to initialize, in addition to the one above, there is one as follows:
karmada/vendor/sigs.k8s.io/controller-runtime/pkg/webhook/admission/webhook.go
Lines 216 to 233 in 95bf37f
Since this initialization method does not apply to
webhook.Server
, this method does not workcall
SetLogger
to get any actual logging.That's what I ended up using, by calling
SetLogger
first to silence the 'log.SetLogger(...) was never called; logs will not be displayed' error.BTW, it was a breaking change in the release notes
0.15.0
ofcontroller-runtime
. Previously, when thepkg/log
package was imported, there was aninit
function that spawned a goroutine, which set a null logger after 30 seconds. Therefore, versions prior to0.15.0
will not have this problem.Which issue(s) this PR fixes:
Fixes #4868
Special notes for your reviewer:
Does this PR introduce a user-facing change?: