Honor stderrthreshold when logtostderr is enabled

[pierluigilenoci]

What this PR does

Fixes the -stderrthreshold flag so it is honored even when -logtostderr=true (the klog v2 default).

Problem

klog v2 defaults -logtostderr to true. When this flag is active, the -stderrthreshold flag is silently ignored — all log messages (INFO, WARNING, ERROR, FATAL) are unconditionally written to stderr. There is no way for users to filter which severity levels reach stderr.

This has been an open issue since 2020: kubernetes/klog#212.

Fix

PR kubernetes/klog#432 introduced the fix in klog v2.140.0 via a new flag legacy_stderr_threshold_behavior. This PR:

1. Bumps klog from v2.130.1 to v2.140.0
2. Opts into the fixed behavior by setting legacy_stderr_threshold_behavior=false and stderrthreshold=INFO after klog.InitFlags()

Setting stderrthreshold=INFO preserves the current default behavior (all logs go to stderr). Users can now override it on the command line (e.g., -stderrthreshold=WARNING) and it will actually work.

Changes

• pkg/cmd/velero/velero.go: Add flag.CommandLine.Set() calls after klog.InitFlags(flag.CommandLine)
• go.mod / go.sum: Bump k8s.io/klog/v2 v2.130.1 → v2.140.0

Ref: kubernetes/klog#212, kubernetes/klog#432

[pierluigilenoci]

Hi @Lyndon-Li, @skriss — gentle ping. Would you be able to review this when you get a chance? Thank you!

[kaovilai]

Please create an issue if this was a bug/regression. And please run make new-changelog

Please include some examples of new behaviors vs old.

[codecov]

Codecov Report

❌ Patch coverage is 0% with 5 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
pkg/cmd/velero/velero.go	0.00%	5 Missing ⚠️

📢 Thoughts on this report? Let us know!

[pierluigilenoci]

@kaovilai Thanks for the feedback! I've created issue #9658 for this.

Old vs New Behavior

Before this PR (klog < v2.140.0 or legacy mode)

$ velero server --stderrthreshold=WARNING
# stderr output:
I0328 10:00:00.000000   12345 server.go:100] Starting Velero server   ← INFO appears on stderr (unexpected)
W0328 10:00:01.000000   12345 server.go:200] Deprecation warning       ← WARNING on stderr (expected)
E0328 10:00:02.000000   12345 server.go:300] Connection failed          ← ERROR on stderr (expected)

The --stderrthreshold=WARNING flag is silently ignored — all levels including INFO are written to stderr because --logtostderr=true bypasses the threshold check entirely.

After this PR

$ velero server --stderrthreshold=WARNING
# stderr output:
W0328 10:00:01.000000   12345 server.go:200] Deprecation warning       ← WARNING on stderr (expected)
E0328 10:00:02.000000   12345 server.go:300] Connection failed          ← ERROR on stderr (expected)

The --stderrthreshold flag now works correctly. Only messages at or above the threshold level appear on stderr.

Default behavior (unchanged)

With the default --stderrthreshold=INFO, the output is identical to before — all log levels appear on stderr. This PR only changes behavior for users who explicitly set a higher threshold.

See: kubernetes/klog#432 for the upstream fix details.

[kaovilai]

Note

Responses generated with Claude

[kaovilai]

nit: Consider adding a comment clarifying that this sets the default, but users can still override it via --stderrthreshold on the command line since flag parsing happens later at Execute() time.

Note

Responses generated with Claude

[pierluigilenoci]

Good point! I'll add a clarifying comment. The flag.Set call here sets the programmatic default so that --stderrthreshold=ERROR is the baseline, but since this runs before cmd.Execute() invokes pflag parsing, any explicit --stderrthreshold= on the command line will override it as expected.

[pierluigilenoci]

Thanks @kaovilai! I'll create an issue and add the changelog entry.

Behavior comparison:

Scenario	Before (broken)	After (fixed)
Default (--logtostderr=true, --stderrthreshold=ERROR)	All log levels go to stderr (threshold ignored)	All log levels go to stderr (same — no behavior change)
User sets --stderrthreshold=WARNING	Still all levels go to stderr (flag silently ignored)	Only WARNING and ERROR go to stderr (flag honored)
User sets --stderrthreshold=ERROR	Still all levels go to stderr (flag silently ignored)	Only ERROR goes to stderr (flag honored)

The key point: default behavior does not change. The fix only matters when users explicitly set --stderrthreshold to filter log output — which was previously impossible despite the flag existing.

I'll push the changelog entry shortly and create the upstream issue.

[pierluigilenoci]

@kaovilai Done! Here's what I've added:

• Issue: klog --stderrthreshold flag silently ignored when --logtostderr is enabled #9658 tracks this as a bug fix
• Changelog: changelogs/unreleased/9654-pierluigilenoci is included in this PR
• Behavior comparison: See my earlier comment for old vs new behavior examples

The codecov/patch failure is expected — the two new flag.Set() lines in velero.go aren't covered by unit tests, but they're simple flag initialization that's already tested via integration in klog itself.

Is there anything else you'd like me to address?

[blackpiglet]

@pierluigilenoci
Will the --legacy_stderr_threshold_behavior parameter be automatically included in the Velero CLI as a result of this PR?

[pierluigilenoci]

@blackpiglet Good question. Yes, --legacy_stderr_threshold_behavior is already exposed as a Velero CLI flag — both before and after this PR. It is not new to this change.

Here is why: Velero calls klog.InitFlags(flag.CommandLine) in NewCommand() (see pkg/cmd/velero/velero.go), which registers all klog flags into Go's standard flag.CommandLine. The command then adds them to the Cobra command via c.PersistentFlags().AddGoFlagSet(flag.CommandLine). Since klog registers legacy_stderr_threshold_behavior as a boolean flag in its init() function (see klog.go), it becomes available on the Velero CLI as --legacy_stderr_threshold_behavior.

What this PR changes is the programmatic default: it calls flag.Set("legacy_stderr_threshold_behavior", "false") before flag parsing, so the fixed behavior is active by default without users needing to pass the flag explicitly. Users can still override it with --legacy_stderr_threshold_behavior=true on the command line if they need the old behavior.

In summary:

• The flag was already present in the Velero CLI (inherited from klog via InitFlags).
• This PR flips its effective default from true to false so that --stderrthreshold is honored when --logtostderr=true.
• No new flags are introduced by this PR; the behavior change comes from setting a different default for an existing klog flag.

[pierluigilenoci]

@blackpiglet Could you please approve the pending CI workflow runs when you get a chance? All checks (PR CI Check, PR Linter Check, PR Changelog Check, PR Codespell Check, E2E test) are currently in action_required state, waiting for maintainer approval to run on this fork PR. Thank you!

[pierluigilenoci]

Thank you @blackpiglet for the review and approval! All CI checks should be green now — could this be merged when convenient? Happy to address anything else if needed.

[blackpiglet]

@pierluigilenoci
Merging needs two approvals.
Let's wait for other maintainers' input.

[blackpiglet]

Sorry, clicked the close button accidentally.

[pierluigilenoci]

Hi — friendly ping. Is this PR still on the radar for a second review? Happy to rebase or make changes if needed. Thanks!

[blackpiglet]

Hi — friendly ping. Is this PR still on the radar for a second review? Happy to rebase or make changes if needed. Thanks!

Yes, approved.
Still need one more vote from the maintainers.