fix: honor --stderrthreshold flag when --logtostderr is enabled#6786
Conversation
|
Hi @pierluigilenoci. Thanks for your PR. I'm waiting for a pingcap member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. 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. |
|
Welcome @pierluigilenoci! It looks like this is your first PR to pingcap/tidb-operator 🎉 |
|
Hi @cofyc, @csuzhangxc — this small fix adds proper error handling for the |
|
|
|
@pierluigilenoci Hi, thanks for your contribution. Could you resolve the conflicts? Notice the base should be |
When logtostderr is enabled in the backup manager, klog's legacy behavior ignores the stderrthreshold flag. This opts into the fixed behavior introduced in klog v2.130.0 by setting legacy_stderr_threshold_behavior=false. Also adds proper error handling for the existing bare pflag.Set() call. See: kubernetes/klog#432 Signed-off-by: Pierluigi Lenoci <pierluigi.lenoci@gmail.com>
pierluigilenoci
force-pushed
the
fix/honor-stderrthreshold
branch
from
e7325a3 to
9673285
Compare
|
@liubog2008 Thanks for the review! I've rebased onto |
|
Hi @liubog2008, thanks for pointing that out! I've already rebased the branch onto |
|
/ok-to-test |
|
@pierluigilenoci Fix CI issues. Thanks |
|
/retest |
Update k8s.io/klog/v2 from v2.110.1 to v2.140.0 and github.com/go-logr/logr from v1.3.0 to v1.4.1 in pkg/apis and pkg/client submodules to match the main module versions. This fixes the check-modules CI verification failure. Signed-off-by: Pierluigi Lenoci <pierluigi.lenoci@gmail.com>
|
Pushed a fix to align klog/v2 and go-logr/logr versions in the pkg/apis and pkg/client submodules to match the main module. The verify check should now pass. |
|
/retest |
|
/test pull-e2e-kind-across-kubernetes |
|
/lgtm |
[LGTM Timeline notifier]Timeline:
|
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: liubog2008 The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/cherry-pick release-1.6 |
|
@liubog2008: once the present PR merges, I will cherry-pick it on top of release-1.6 in the new PR and assign it to you. DetailsIn response to this:
Instructions 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 ti-community-infra/tichi repository. |
|
@liubog2008: new pull request created to branch DetailsIn response to this:
Instructions 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 ti-community-infra/tichi repository. |
3 participants
What this PR does
The backup manager calls
pflag.Set("logtostderr", "true")which enables klog's logtostderr mode. However, due to klog's legacy behavior, the--stderrthresholdflag is silently ignored whenlogtostderris true.This PR:
pflag.Set("logtostderr", "true")calllegacy_stderr_threshold_behavior=false— disables the legacy overridestderrthreshold=INFO— preserves the current default behaviorAfter this change, the
--stderrthresholdflag works as expected in the backup manager.Why
See upstream fix: kubernetes/klog#432