Merge pull request kubernetes#85771 from aojea/iptables_lock

Be more agressive acquiring the iptables lock
rasata · Dec 17, 2019 · d0e9018 · d0e9018
2 parents 623b697 + 51814ae
commit d0e9018
Show file tree

Hide file tree

Showing 3 changed files with 49 additions and 7 deletions.
diff --git a/pkg/util/iptables/iptables.go b/pkg/util/iptables/iptables.go
@@ -163,6 +163,9 @@ var RandomFullyMinVersion = utilversion.MustParseGeneric("1.6.2")
 // WaitMinVersion a minimum iptables versions supporting the -w and -w<seconds> flags
 var WaitMinVersion = utilversion.MustParseGeneric("1.4.20")
 
+// WaitIntervalMinVersion a minimum iptables versions supporting the wait interval useconds
+var WaitIntervalMinVersion = utilversion.MustParseGeneric("1.6.1")
+
 // WaitSecondsMinVersion a minimum iptables versions supporting the wait seconds
 var WaitSecondsMinVersion = utilversion.MustParseGeneric("1.4.22")
 
@@ -175,6 +178,12 @@ const WaitString = "-w"
 // WaitSecondsValue a constant for specifying the default wait seconds
 const WaitSecondsValue = "5"
 
+// WaitIntervalString a constant for specifying the wait interval flag
+const WaitIntervalString = "-W"
+
+// WaitIntervalUsecondsValue a constant for specifying the default wait interval useconds
+const WaitIntervalUsecondsValue = "100000"
+
 // LockfilePath16x is the iptables lock file acquired by any process that's making any change in the iptable rule
 const LockfilePath16x = "/run/xtables.lock"
 
@@ -638,6 +647,8 @@ func getIPTablesVersion(exec utilexec.Interface, protocol Protocol) (*utilversio
 // Checks if iptables version has a "wait" flag
 func getIPTablesWaitFlag(version *utilversion.Version) []string {
 	switch {
+	case version.AtLeast(WaitIntervalMinVersion):
+		return []string{WaitString, WaitSecondsValue, WaitIntervalString, WaitIntervalUsecondsValue}
 	case version.AtLeast(WaitSecondsMinVersion):
 		return []string{WaitString, WaitSecondsValue}
 	case version.AtLeast(WaitMinVersion):
@@ -650,7 +661,7 @@ func getIPTablesWaitFlag(version *utilversion.Version) []string {
 // Checks if iptables-restore has a "wait" flag
 func getIPTablesRestoreWaitFlag(version *utilversion.Version, exec utilexec.Interface, protocol Protocol) []string {
 	if version.AtLeast(WaitRestoreMinVersion) {
-		return []string{WaitString, WaitSecondsValue}
+		return []string{WaitString, WaitSecondsValue, WaitIntervalString, WaitIntervalUsecondsValue}
 	}
 
 	// Older versions may have backported features; if iptables-restore supports

diff --git a/pkg/util/iptables/iptables_test.go b/pkg/util/iptables/iptables_test.go
@@ -620,7 +620,7 @@ func TestIPTablesWaitFlag(t *testing.T) {
 		{"1.4.21", []string{WaitString}},
 		{"1.4.22", []string{WaitString, WaitSecondsValue}},
 		{"1.5.0", []string{WaitString, WaitSecondsValue}},
-		{"2.0.0", []string{WaitString, WaitSecondsValue}},
+		{"2.0.0", []string{WaitString, WaitSecondsValue, WaitIntervalString, WaitIntervalUsecondsValue}},
 	}
 
 	for _, testCase := range testCases {
@@ -729,6 +729,37 @@ func TestWaitFlagNew(t *testing.T) {
 	}
 }
 
+func TestWaitIntervalFlagNew(t *testing.T) {
+	fcmd := fakeexec.FakeCmd{
+		CombinedOutputScript: []fakeexec.FakeCombinedOutputAction{
+			// iptables version check
+			func() ([]byte, error) { return []byte("iptables v1.6.1"), nil },
+			// iptables-restore version check
+			func() ([]byte, error) { return []byte{}, nil },
+			// Success.
+			func() ([]byte, error) { return []byte{}, nil },
+		},
+	}
+	fexec := fakeexec.FakeExec{
+		CommandScript: []fakeexec.FakeCommandAction{
+			func(cmd string, args ...string) exec.Cmd { return fakeexec.InitFakeCmd(&fcmd, cmd, args...) },
+			func(cmd string, args ...string) exec.Cmd { return fakeexec.InitFakeCmd(&fcmd, cmd, args...) },
+			func(cmd string, args ...string) exec.Cmd { return fakeexec.InitFakeCmd(&fcmd, cmd, args...) },
+		},
+	}
+	runner := New(&fexec, ProtocolIpv4)
+	err := runner.DeleteChain(TableNAT, Chain("FOOBAR"))
+	if err != nil {
+		t.Errorf("expected success, got %v", err)
+	}
+	if fcmd.CombinedOutputCalls != 3 {
+		t.Errorf("expected 3 CombinedOutput() calls, got %d", fcmd.CombinedOutputCalls)
+	}
+	if !sets.NewString(fcmd.CombinedOutputLog[2]...).HasAll("iptables", WaitString, WaitSecondsValue, WaitIntervalString, WaitIntervalUsecondsValue) {
+		t.Errorf("wrong CombinedOutput() log, got %s", fcmd.CombinedOutputLog[2])
+	}
+}
+
 func testSaveInto(t *testing.T, protocol Protocol) {
 	version := " v1.9.22"
 	iptablesCmd := iptablesCommand(protocol)
@@ -963,7 +994,7 @@ func TestRestoreAllWait(t *testing.T) {
 	}
 
 	commandSet := sets.NewString(fcmd.CombinedOutputLog[1]...)
-	if !commandSet.HasAll("iptables-restore", WaitString, WaitSecondsValue, "--counters", "--noflush") {
+	if !commandSet.HasAll("iptables-restore", WaitString, WaitSecondsValue, WaitIntervalString, WaitIntervalUsecondsValue, "--counters", "--noflush") {
 		t.Errorf("wrong CombinedOutput() log, got %s", fcmd.CombinedOutputLog[1])
 	}
 

diff --git a/pkg/util/iptables/monitor_test.go b/pkg/util/iptables/monitor_test.go
@@ -100,12 +100,12 @@ func (mfc *monitorFakeCmd) CombinedOutput() ([]byte, error) {
 		return []byte("iptables v1.6.2"), nil
 	}
 
-	if len(mfc.args) != 6 || mfc.args[0] != WaitString || mfc.args[1] != WaitSecondsValue || mfc.args[4] != "-t" {
+	if len(mfc.args) != 8 || mfc.args[0] != WaitString || mfc.args[1] != WaitSecondsValue || mfc.args[2] != WaitIntervalString || mfc.args[3] != WaitIntervalUsecondsValue || mfc.args[6] != "-t" {
 		panic(fmt.Sprintf("bad args %#v", mfc.args))
 	}
-	op := operation(mfc.args[2])
-	chainName := mfc.args[3]
-	tableName := mfc.args[5]
+	op := operation(mfc.args[4])
+	chainName := mfc.args[5]
+	tableName := mfc.args[7]
 
 	mfc.mfe.Lock()
 	defer mfc.mfe.Unlock()