-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Set klog's logtostderr flag #2529
Conversation
Thank you for your contribution. I've just checked the OWNERS files to find a suitable reviewer. This search was successful and I've asked fturib (via If you have questions or suggestions for this bot, please file an issue against the miekg/dreck repository. The bot understands the commands that are listed here. |
Codecov Report
@@ Coverage Diff @@
## master #2529 +/- ##
==========================================
+ Coverage 55.37% 55.38% +0.01%
==========================================
Files 204 204
Lines 10346 10349 +3
==========================================
+ Hits 5729 5732 +3
- Misses 4196 4197 +1
+ Partials 421 420 -1
Continue to review full report at Codecov.
|
plugin/kubernetes/setup.go
Outdated
@@ -33,12 +36,15 @@ import ( | |||
var log = clog.NewWithPlugin("kubernetes") | |||
|
|||
func init() { | |||
// Kubernetes plugin uses the kubernetes library, which uses glog (ugh), we must set this *flag*, | |||
// Kubernetes plugin uses the kubernetes library, which now uses klog (ugh), we must set and parse this *flag*, |
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.
I though klog was better than glog? Remove the 'ugh' at least (glog is almost evil on how it interacts with its environment)
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.
Yeah, it is a significant step forward from glog and looks to be making progress on continuing to clean up a lot of the minor pain points, have cleared up the comment.
Think it's worth still keeping there given the current behaviour, at least as used by the k8s libraries, is still to log to disk.
* Parse as well as setlogtostderr flag * Enforce setting of logtostderr for klog * Clearup comment on klog
* Parse as well as setlogtostderr flag * Enforce setting of logtostderr for klog * Clearup comment on klog
1. Why is this pull request needed and what does it do?
The update of client-go to v10.0.0 has moved K8s logging from glog to klog.
This change removes the previous non-obvious flag setting behaviour used for glog and then ensures that
klog
, the replacement forglog
has the relevant flag set. See klog code for the reason why this has to happen: https://github.com/coredns/coredns/blob/92836cc6f9aaf47a349993271ddae273d8e7d25d/vendor/k8s.io/klog/klog.goNote that simply updating to the version of klog updating this flag to default to
true
rather thanfalse
was tested and still caused crashes.Tested locally:
No restarts of pods observed.
2. Which issues (if any) are related?
#2464 , supersedes the sticking plaster attempted by #2525
3. Which documentation changes (if any) need to be made?
Noted as a bug fix for a regression in v1.3.1 only.