Skip to content

fix: honor stderrthreshold when logtostderr is enabled#192

Open
pierluigilenoci wants to merge 1 commit intowarm-metal:mainfrom
pierluigilenoci:fix/klog-stderrthreshold-behavior
Open

fix: honor stderrthreshold when logtostderr is enabled#192
pierluigilenoci wants to merge 1 commit intowarm-metal:mainfrom
pierluigilenoci:fix/klog-stderrthreshold-behavior

Conversation

@pierluigilenoci
Copy link
Copy Markdown

Summary

Since kubernetes/klog#432, klog changed its legacy behavior: when logtostderr=true (the default in this driver), WARNING and ERROR messages are no longer duplicated to stderr unless you explicitly opt in.

This PR:

  • Adds flag.Set("stderrthreshold", "INFO") right after the existing flag.Set("logtostderr", "true") call, so that all log severities continue to appear on stderr as expected.
  • Bumps k8s.io/klog/v2 from v2.130.1 to v2.140.0.

Why

Without this fix, users relying on stderr output (e.g. for log aggregation pipelines, kubectl logs, or sidecar log collectors) may silently lose WARNING and ERROR messages on stderr.

Test plan

  • go build ./cmd/plugin/ succeeds
  • Deploy the CSI driver with the updated binary and verify that klog messages at all severities (INFO, WARNING, ERROR) appear on stderr

References

@pierluigilenoci pierluigilenoci requested a review from a team as a code owner March 28, 2026 20:29
@pierluigilenoci
Copy link
Copy Markdown
Author

cc @kitt1987 @mugdha-adhav — would appreciate your review on this klog fix. Thanks!

@mugdha-adhav
Copy link
Copy Markdown
Collaborator

Thanks for the PR @pierluigilenoci! I looked into the linked klog#432 and had a question.

klog v2.140.0 introduces legacy_stderr_threshold_behavior which defaults to true, preserving the old behavior where stderrthreshold is ignored when logtostderr=true. That means the flag.Set("stderrthreshold", "INFO") added here would be a no-op without also setting legacy_stderr_threshold_behavior=false.

I noticed your other downstream PRs (secrets-store-csi-driver, metrics-server, etc.) all include flag.Set("legacy_stderr_threshold_behavior", "false") — was that omitted here by mistake?

Do we need to add it for this fix to take effect?

Since kubernetes/klog#432, the legacy behavior of copying WARNING and
above to stderr when logtostderr=true is deprecated. Explicitly set
stderrthreshold=INFO so that all log levels continue to appear on
stderr as expected.

Also bumps k8s.io/klog/v2 from v2.130.1 to v2.140.0.

See: kubernetes/klog#432

Signed-off-by: Pierluigi Lenoci <pierluigi.lenoci@gmail.com>
@pierluigilenoci pierluigilenoci force-pushed the fix/klog-stderrthreshold-behavior branch from 82a4619 to f30b786 Compare March 30, 2026 15:42
@pierluigilenoci
Copy link
Copy Markdown
Author

Good catch @mugdha-adhav! I've added legacy_stderr_threshold_behavior=false — it was indeed an oversight. Since this repo calls klog.InitFlags(nil), the flag is registered and available. Also rebased to resolve conflicts.

@pierluigilenoci
Copy link
Copy Markdown
Author

Friendly follow-up — @mugdha-adhav thank you for the great feedback earlier! I addressed your question about legacy_stderr_threshold_behavior 11 days ago. Would you be able to take another look? @kitt1987 would also appreciate your review if you have a moment. CI is green. Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants