Merge pull request #54763 from rajatchopra/iptables_wait

Automatic merge from submit-queue (batch tested with PRs 54533, 54777, 54763, 54806, 54703). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

make iptables wait flag generic and tune it to 5 seconds

Excerpt from [bug](https://bugzilla.redhat.com/show_bug.cgi?id=1506396) opened by @eparis 

> iptables-restore has a 2s wait timeout. Data collected today shows that even with a much faster kernel we can reasonably expect iptables-restore to take upwards of 2.4 seconds. (with unpatched/released RHEL kernel this can easily take 7-8 second)

> longest runs I saw over about 30 minutes were:
> 2.267244
> 2.284707
> 2.291535
> 2.376457

> If we get 2 iptables restores going at the same time, with a 2s timeout it is very likely the second will fail.

> I'd like to suggest a 5s timeout. It should still bound the number of thread we may be waiting on and increases the reliability that a common situation will be automatically resolved without failing up the stack.
pull/6/head
Kubernetes Submit Queue 2017-10-30 17:38:19 -07:00 committed by GitHub
commit 59cee2c73c
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 21 additions and 19 deletions

View File

@ -118,9 +118,11 @@ const NoFlushTables FlushFlag = false
// (test whether a rule exists). // (test whether a rule exists).
const MinCheckVersion = "1.4.11" const MinCheckVersion = "1.4.11"
// Minimum iptables versions supporting the -w and -w2 flags // Minimum iptables versions supporting the -w and -w<seconds> flags
const MinWaitVersion = "1.4.20" const WaitMinVersion = "1.4.20"
const MinWait2Version = "1.4.22" const WaitSecondsMinVersion = "1.4.22"
const WaitString = "-w"
const WaitSecondsString = "-w5"
const LockfilePath16x = "/run/xtables.lock" const LockfilePath16x = "/run/xtables.lock"
@ -537,24 +539,24 @@ func getIPTablesWaitFlag(vstring string) []string {
return nil return nil
} }
minVersion, err := utilversion.ParseGeneric(MinWaitVersion) minVersion, err := utilversion.ParseGeneric(WaitMinVersion)
if err != nil { if err != nil {
glog.Errorf("MinWaitVersion (%s) is not a valid version string: %v", MinWaitVersion, err) glog.Errorf("WaitMinVersion (%s) is not a valid version string: %v", WaitMinVersion, err)
return nil return nil
} }
if version.LessThan(minVersion) { if version.LessThan(minVersion) {
return nil return nil
} }
minVersion, err = utilversion.ParseGeneric(MinWait2Version) minVersion, err = utilversion.ParseGeneric(WaitSecondsMinVersion)
if err != nil { if err != nil {
glog.Errorf("MinWait2Version (%s) is not a valid version string: %v", MinWait2Version, err) glog.Errorf("WaitSecondsMinVersion (%s) is not a valid version string: %v", WaitSecondsMinVersion, err)
return nil return nil
} }
if version.LessThan(minVersion) { if version.LessThan(minVersion) {
return []string{"-w"} return []string{WaitString}
} else { } else {
return []string{"-w2"} return []string{WaitSecondsString}
} }
} }

View File

@ -686,11 +686,11 @@ func TestIPTablesWaitFlag(t *testing.T) {
{"0.55.55", ""}, {"0.55.55", ""},
{"1.0.55", ""}, {"1.0.55", ""},
{"1.4.19", ""}, {"1.4.19", ""},
{"1.4.20", "-w"}, {"1.4.20", WaitString},
{"1.4.21", "-w"}, {"1.4.21", WaitString},
{"1.4.22", "-w2"}, {"1.4.22", WaitSecondsString},
{"1.5.0", "-w2"}, {"1.5.0", WaitSecondsString},
{"2.0.0", "-w2"}, {"2.0.0", WaitSecondsString},
} }
for _, testCase := range testCases { for _, testCase := range testCases {
@ -730,7 +730,7 @@ func TestWaitFlagUnavailable(t *testing.T) {
if fcmd.CombinedOutputCalls != 3 { if fcmd.CombinedOutputCalls != 3 {
t.Errorf("expected 3 CombinedOutput() calls, got %d", fcmd.CombinedOutputCalls) t.Errorf("expected 3 CombinedOutput() calls, got %d", fcmd.CombinedOutputCalls)
} }
if sets.NewString(fcmd.CombinedOutputLog[2]...).HasAny("-w", "-w2") { if sets.NewString(fcmd.CombinedOutputLog[2]...).HasAny(WaitString, WaitSecondsString) {
t.Errorf("wrong CombinedOutput() log, got %s", fcmd.CombinedOutputLog[2]) t.Errorf("wrong CombinedOutput() log, got %s", fcmd.CombinedOutputLog[2])
} }
} }
@ -762,10 +762,10 @@ func TestWaitFlagOld(t *testing.T) {
if fcmd.CombinedOutputCalls != 3 { if fcmd.CombinedOutputCalls != 3 {
t.Errorf("expected 3 CombinedOutput() calls, got %d", fcmd.CombinedOutputCalls) t.Errorf("expected 3 CombinedOutput() calls, got %d", fcmd.CombinedOutputCalls)
} }
if !sets.NewString(fcmd.CombinedOutputLog[2]...).HasAll("iptables", "-w") { if !sets.NewString(fcmd.CombinedOutputLog[2]...).HasAll("iptables", WaitString) {
t.Errorf("wrong CombinedOutput() log, got %s", fcmd.CombinedOutputLog[2]) t.Errorf("wrong CombinedOutput() log, got %s", fcmd.CombinedOutputLog[2])
} }
if sets.NewString(fcmd.CombinedOutputLog[2]...).HasAny("-w2") { if sets.NewString(fcmd.CombinedOutputLog[2]...).HasAny(WaitSecondsString) {
t.Errorf("wrong CombinedOutput() log, got %s", fcmd.CombinedOutputLog[2]) t.Errorf("wrong CombinedOutput() log, got %s", fcmd.CombinedOutputLog[2])
} }
} }
@ -797,10 +797,10 @@ func TestWaitFlagNew(t *testing.T) {
if fcmd.CombinedOutputCalls != 3 { if fcmd.CombinedOutputCalls != 3 {
t.Errorf("expected 3 CombinedOutput() calls, got %d", fcmd.CombinedOutputCalls) t.Errorf("expected 3 CombinedOutput() calls, got %d", fcmd.CombinedOutputCalls)
} }
if !sets.NewString(fcmd.CombinedOutputLog[2]...).HasAll("iptables", "-w2") { if !sets.NewString(fcmd.CombinedOutputLog[2]...).HasAll("iptables", WaitSecondsString) {
t.Errorf("wrong CombinedOutput() log, got %s", fcmd.CombinedOutputLog[2]) t.Errorf("wrong CombinedOutput() log, got %s", fcmd.CombinedOutputLog[2])
} }
if sets.NewString(fcmd.CombinedOutputLog[2]...).HasAny("-w") { if sets.NewString(fcmd.CombinedOutputLog[2]...).HasAny(WaitString) {
t.Errorf("wrong CombinedOutput() log, got %s", fcmd.CombinedOutputLog[2]) t.Errorf("wrong CombinedOutput() log, got %s", fcmd.CombinedOutputLog[2])
} }
} }