fix: honor -stderrthreshold flag in klog v2#3086
fix: honor -stderrthreshold flag in klog v2#3086pierluigilenoci wants to merge 2 commits intokubesphere:mainfrom
Conversation
klog v2 defaults -logtostderr=true, which silently ignores the -stderrthreshold flag (kubernetes/klog#212). klog v2.140.0 introduced the legacy_stderr_threshold_behavior flag to fix this (kubernetes/klog#432). After every klog.InitFlags() call, set logtostderr=false and legacy_stderr_threshold_behavior=true so that -stderrthreshold works as expected. Bump k8s.io/klog/v2 from v2.130.1 to v2.140.0. Signed-off-by: Pierluigi Lenoci <pierluigilenoci@gmail.com>
|
Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it. DetailsInstructions 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-sigs/prow repository. |
kubesphere-prow
Bot
added
do-not-merge/work-in-progress
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: pierluigilenoci The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
Welcome @pierluigilenoci! It looks like this is your first PR to kubesphere/kubekey 🎉 |
kubesphere-prow
Bot
added
the
size/S
Set legacy_stderr_threshold_behavior=false (not true) to enable the fix, and set stderrthreshold=INFO (not logtostderr=false) to preserve default behavior while allowing users to override on the command line. Signed-off-by: Pierluigi Lenoci <pierluigi.lenoci@gmail.com> Signed-off-by: Pierluigi Lenoci <pierluigilenoci@gmail.com>
|
This PR has multiple commits, and the default merge method is: squash. DetailsInstructions 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-sigs/prow repository. |
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the k8s.io/klog/v2 dependency to version 2.140.0 and configures the logtostderr and legacy_stderr_threshold_behavior flags in cmd/controller-manager/app/options/common.go and cmd/kk/app/options/other.go to resolve an issue where -stderrthreshold was ignored. The review feedback recommends adding error handling for the local.Set calls to prevent silent configuration failures during application startup.
| _ = local.Set("legacy_stderr_threshold_behavior", "false") | ||
| _ = local.Set("stderrthreshold", "INFO") |
There was a problem hiding this comment.
The errors returned by local.Set are being ignored. While it's unlikely to fail in this case, it's a good practice to handle errors during application initialization. A failure to set these flags would lead to incorrect logging behavior that could be difficult to debug. Consider checking the error and logging a fatal error if setting the flag fails, to ensure the application exits if it cannot be configured correctly.
if err := local.Set("logtostderr", "false"); err != nil {
klog.Fatalf("failed to set klog logtostderr flag: %v", err)
}
if err := local.Set("legacy_stderr_threshold_behavior", "true"); err != nil {
klog.Fatalf("failed to set klog legacy_stderr_threshold_behavior flag: %v", err)
}| _ = local.Set("legacy_stderr_threshold_behavior", "false") | ||
| _ = local.Set("stderrthreshold", "INFO") |
There was a problem hiding this comment.
The errors returned by local.Set are being ignored. While it's unlikely to fail in this case, it's a good practice to handle errors during application initialization. A failure to set these flags would lead to incorrect logging behavior that could be difficult to debug. Consider checking the error and logging a fatal error if setting the flag fails, to ensure the application exits if it cannot be configured correctly.
if err := local.Set("logtostderr", "false"); err != nil {
klog.Fatalf("failed to set klog logtostderr flag: %v", err)
}
if err := local.Set("legacy_stderr_threshold_behavior", "true"); err != nil {
klog.Fatalf("failed to set klog legacy_stderr_threshold_behavior flag: %v", err)
}
1 participant
What this PR does
klog v2 defaults
-logtostderr=true, which silently ignores the-stderrthresholdflag (kubernetes/klog#212).Users who set
-stderrthresholdexpect it to control which severitylevel is written to stderr, but the flag has no effect because
-logtostderrtakes precedence.klog v2.140.0 introduced the
legacy_stderr_threshold_behaviorflag to restore the intended behaviour
(kubernetes/klog#432).
Changes
cmd/kk/app/options/other.goandcmd/controller-manager/app/options/common.go: after everyklog.InitFlags()call, setlogtostderr=falseandlegacy_stderr_threshold_behavior=trueso that-stderrthresholdworks as expected.
go.mod: bumpk8s.io/klog/v2from v2.130.1 to v2.140.0.Why
Without this fix, any
-stderrthresholdvalue passed tokkorcontroller-manageris silently ignored, making it impossible tosuppress low-severity log output on stderr.
How to test
Both binaries compile and the new flag values are set programmatically
(no user-facing CLI change).