Log abridged set of rules at v2 in kube-proxy on error#48624
Log abridged set of rules at v2 in kube-proxy on error#48624k8s-github-robot merged 1 commit intokubernetes:masterfrom
Conversation
01aee1d to
2fa2c95
Compare
pkg/proxy/iptables/proxier.go
Outdated
There was a problem hiding this comment.
can we do one or the other?
if glog.V(4) {
// full
else
// abridged
pkg/proxy/iptables/proxier.go
Outdated
There was a problem hiding this comment.
Printing the rules is only useful if the error has to do with a specific rule (as opposed to "could not get xtables lock" or something like that). But the code to generate the rules only has so many different cases in it and none of them are super obscure, so any bug in that code would get hit by developers pretty quickly rather than escaping into a release.
So, printing the rules is probably only ever going to be useful on non-production systems. So just change it from V(2) to V(4).
There was a problem hiding this comment.
Actually, we already log them at V(5) a few lines earlier whether there's an error or not. So we could just drop the logging here.
|
We might want to parse the error string looking for a line number and print
that line + context?
…On Fri, Jul 7, 2017 at 12:49 PM, Dan Winship ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In pkg/proxy/iptables/proxier.go
<#48624 (comment)>
:
> @@ -1572,7 +1572,17 @@ func (proxier *Proxier) syncProxyRules() {
err = proxier.iptables.RestoreAll(proxier.iptablesData.Bytes(), utiliptables.NoFlushTables, utiliptables.RestoreCounters)
if err != nil {
glog.Errorf("Failed to execute iptables-restore: %v", err)
- glog.V(2).Infof("Rules:\n%s", proxier.iptablesData.Bytes())
Actually, we already log them at V(5) a few lines earlier whether there's
an error or not. So we could just drop the logging here.
—
You are receiving this because you were assigned.
Reply to this email directly, view it on GitHub
<#48624 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AFVgVMq7Kfuw49UVGFQm5dksxAhPLX32ks5sLovPgaJpZM4ORRyb>
.
|
2fa2c95 to
3bddef7
Compare
|
updated per @thockin request, i think this is incremental improvement. |
|
/lgtm |
|
@k8s-bot test this Tests are more than 96 hours old. Re-running tests. |
|
/approve no-issue |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: derekwaynecarr, thockin Associated issue requirement bypassed by: thockin The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these OWNERS Files:
You can indicate your approval by writing |
|
/retest Review the full test history for this PR. |
|
Automatic merge from submit-queue |
What this PR does / why we need it:
this is a follow-on to #48085
Special notes for your reviewer:
we hit this in operations where we typically run in v2, and would like to log abridged set of output rather than full output.
Release note: