Use iptables --wait flag if available

Use iptables --wait (if available) to avoid race conditions with
util.iptables failing if it tries to modify the tables at the same
time as another process.

pkg/util/iptables/iptables.go

@@ -110,12 +110,17 @@ const NoFlushTables FlushFlag = false
 // (test whether a rule exists).
 const MinCheckVersion = "1.4.11"
 
+// Minimum iptables versions supporting the -w and -w2 flags
+const MinWaitVersion = "1.4.20"
+const MinWait2Version = "1.4.22"
+
 // runner implements Interface in terms of exec("iptables").
 type runner struct {
 	mu       sync.Mutex
 	exec     utilexec.Interface
 	protocol Protocol
 	hasCheck bool
+	waitFlag []string
 }
 
 // New returns a new Interface which will exec iptables.
@@ -129,6 +134,7 @@ func New(exec utilexec.Interface, protocol Protocol) Interface {
 		exec:     exec,
 		protocol: protocol,
 		hasCheck: getIptablesHasCheckCommand(vstring),
+		waitFlag: getIptablesWaitFlag(vstring),
 	}
 }
 
@@ -309,7 +315,8 @@ func (runner *runner) iptablesCommand() string {
 func (runner *runner) run(op operation, args []string) ([]byte, error) {
 	iptablesCmd := runner.iptablesCommand()
 
-	fullArgs := append([]string{string(op)}, args...)
+	fullArgs := append(runner.waitFlag, string(op))
+	fullArgs = append(fullArgs, args...)
 	glog.V(4).Infof("running iptables %s %v", string(op), args)
 	return runner.exec.Command(iptablesCmd, fullArgs...).CombinedOutput()
 	// Don't log err here - callers might not think it is an error.
@@ -421,6 +428,35 @@ func getIptablesHasCheckCommand(vstring string) bool {
 	return true
 }
 
+// Checks if iptables version has a "wait" flag
+func getIptablesWaitFlag(vstring string) []string {
+	version, err := semver.NewVersion(vstring)
+	if err != nil {
+		glog.Errorf("vstring (%s) is not a valid version string: %v", vstring, err)
+		return nil
+	}
+
+	minVersion, err := semver.NewVersion(MinWaitVersion)
+	if err != nil {
+		glog.Errorf("MinWaitVersion (%s) is not a valid version string: %v", MinWaitVersion, err)
+		return nil
+	}
+	if version.LessThan(*minVersion) {
+		return nil
+	}
+
+	minVersion, err = semver.NewVersion(MinWait2Version)
+	if err != nil {
+		glog.Errorf("MinWait2Version (%s) is not a valid version string: %v", MinWait2Version, err)
+		return nil
+	}
+	if version.LessThan(*minVersion) {
+		return []string{"-w"}
+	} else {
+		return []string{"-w2"}
+	}
+}
+
 // GetIptablesVersionString runs "iptables --version" to get the version string
 // in the form "X.X.X"
 func GetIptablesVersionString(exec utilexec.Interface) (string, error) {

pkg/util/iptables/iptables_test.go

@@ -17,6 +17,7 @@ limitations under the License.
 package iptables
 
 import (
+	"strings"
 	"testing"
 
 	"k8s.io/kubernetes/pkg/util"
@@ -525,3 +526,116 @@ COMMIT
 		t.Errorf("wrong CombinedOutput() log, got %s", fcmd.CombinedOutputLog[0])
 	}
 }
+
+func TestIptablesWaitFlag(t *testing.T) {
+	testCases := []struct {
+		Version string
+		Result  string
+	}{
+		{"0.55.55", ""},
+		{"1.0.55", ""},
+		{"1.4.19", ""},
+		{"1.4.20", "-w"},
+		{"1.4.21", "-w"},
+		{"1.4.22", "-w2"},
+		{"1.5.0", "-w2"},
+		{"2.0.0", "-w2"},
+	}
+
+	for _, testCase := range testCases {
+		result := getIptablesWaitFlag(testCase.Version)
+		if strings.Join(result, "") != testCase.Result {
+			t.Errorf("For %s expected %v got %v", testCase.Version, testCase.Result, result)
+		}
+	}
+}
+
+func TestWaitFlagUnavailable(t *testing.T) {
+	fcmd := exec.FakeCmd{
+		CombinedOutputScript: []exec.FakeCombinedOutputAction{
+			// iptables version check
+			func() ([]byte, error) { return []byte("iptables v1.4.19"), nil },
+			// Success.
+			func() ([]byte, error) { return []byte{}, nil },
+		},
+	}
+	fexec := exec.FakeExec{
+		CommandScript: []exec.FakeCommandAction{
+			func(cmd string, args ...string) exec.Cmd { return exec.InitFakeCmd(&fcmd, cmd, args...) },
+			func(cmd string, args ...string) exec.Cmd { return exec.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 != 2 {
+		t.Errorf("expected 2 CombinedOutput() calls, got %d", fcmd.CombinedOutputCalls)
+	}
+	if util.NewStringSet(fcmd.CombinedOutputLog[1]...).HasAny("-w", "-w2") {
+		t.Errorf("wrong CombinedOutput() log, got %s", fcmd.CombinedOutputLog[1])
+	}
+}
+
+func TestWaitFlagOld(t *testing.T) {
+	fcmd := exec.FakeCmd{
+		CombinedOutputScript: []exec.FakeCombinedOutputAction{
+			// iptables version check
+			func() ([]byte, error) { return []byte("iptables v1.4.20"), nil },
+			// Success.
+			func() ([]byte, error) { return []byte{}, nil },
+		},
+	}
+	fexec := exec.FakeExec{
+		CommandScript: []exec.FakeCommandAction{
+			func(cmd string, args ...string) exec.Cmd { return exec.InitFakeCmd(&fcmd, cmd, args...) },
+			func(cmd string, args ...string) exec.Cmd { return exec.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 != 2 {
+		t.Errorf("expected 2 CombinedOutput() calls, got %d", fcmd.CombinedOutputCalls)
+	}
+	if !util.NewStringSet(fcmd.CombinedOutputLog[1]...).HasAll("iptables", "-w") {
+		t.Errorf("wrong CombinedOutput() log, got %s", fcmd.CombinedOutputLog[1])
+	}
+	if util.NewStringSet(fcmd.CombinedOutputLog[1]...).HasAny("-w2") {
+		t.Errorf("wrong CombinedOutput() log, got %s", fcmd.CombinedOutputLog[1])
+	}
+}
+
+func TestWaitFlagNew(t *testing.T) {
+	fcmd := exec.FakeCmd{
+		CombinedOutputScript: []exec.FakeCombinedOutputAction{
+			// iptables version check
+			func() ([]byte, error) { return []byte("iptables v1.4.22"), nil },
+			// Success.
+			func() ([]byte, error) { return []byte{}, nil },
+		},
+	}
+	fexec := exec.FakeExec{
+		CommandScript: []exec.FakeCommandAction{
+			func(cmd string, args ...string) exec.Cmd { return exec.InitFakeCmd(&fcmd, cmd, args...) },
+			func(cmd string, args ...string) exec.Cmd { return exec.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 != 2 {
+		t.Errorf("expected 2 CombinedOutput() calls, got %d", fcmd.CombinedOutputCalls)
+	}
+	if !util.NewStringSet(fcmd.CombinedOutputLog[1]...).HasAll("iptables", "-w2") {
+		t.Errorf("wrong CombinedOutput() log, got %s", fcmd.CombinedOutputLog[1])
+	}
+	if util.NewStringSet(fcmd.CombinedOutputLog[1]...).HasAny("-w") {
+		t.Errorf("wrong CombinedOutput() log, got %s", fcmd.CombinedOutputLog[1])
+	}
+}