diff --git a/pkg/proxy/iptables/proxier.go b/pkg/proxy/iptables/proxier.go index 2b0b6690a1f..6fb74068cce 100644 --- a/pkg/proxy/iptables/proxier.go +++ b/pkg/proxy/iptables/proxier.go @@ -74,13 +74,12 @@ func ShouldUseIptablesProxier() (bool, error) { if err != nil { return false, err } - // returns "vX.X.X", err + // returns "X.X.X", err versionString, err := utiliptables.GetIptablesVersionString(exec) if err != nil { return false, err } - // make a semver of the part after the v in "vX.X.X" - version, err := semver.NewVersion(versionString[1:]) + version, err := semver.NewVersion(versionString) if err != nil { return false, err } diff --git a/pkg/util/iptables/iptables.go b/pkg/util/iptables/iptables.go index 5e2ee94aee4..5de379a9fe5 100644 --- a/pkg/util/iptables/iptables.go +++ b/pkg/util/iptables/iptables.go @@ -110,16 +110,32 @@ 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. func New(exec utilexec.Interface, protocol Protocol) Interface { - return &runner{exec: exec, protocol: protocol} + vstring, err := GetIptablesVersionString(exec) + if err != nil { + glog.Warningf("Error checking iptables version, assuming version at least %s: %v", MinCheckVersion, err) + vstring = MinCheckVersion + } + return &runner{ + exec: exec, + protocol: protocol, + hasCheck: getIptablesHasCheckCommand(vstring), + waitFlag: getIptablesWaitFlag(vstring), + } } // EnsureChain is part of Interface. @@ -299,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. @@ -308,12 +325,7 @@ func (runner *runner) run(op operation, args []string) ([]byte, error) { // Returns (bool, nil) if it was able to check the existence of the rule, or // (, error) if the process of checking failed. func (runner *runner) checkRule(table Table, chain Chain, args ...string) (bool, error) { - checkPresent, err := getIptablesHasCheckCommand(runner.exec) - if err != nil { - glog.Warningf("Error checking iptables version, assuming version at least 1.4.11: %v", err) - checkPresent = true - } - if checkPresent { + if runner.hasCheck { return runner.checkRuleUsingCheck(makeFullArgs(table, chain, args...)) } else { return runner.checkRuleWithoutCheck(table, chain, args...) @@ -399,40 +411,64 @@ func makeFullArgs(table Table, chain Chain, args ...string) []string { } // Checks if iptables has the "-C" flag -func getIptablesHasCheckCommand(exec utilexec.Interface) (bool, error) { +func getIptablesHasCheckCommand(vstring string) bool { minVersion, err := semver.NewVersion(MinCheckVersion) if err != nil { - return false, err + glog.Errorf("MinCheckVersion (%s) is not a valid version string: %v", MinCheckVersion, err) + return true } - // Returns "vX.Y.Z". - vstring, err := GetIptablesVersionString(exec) + version, err := semver.NewVersion(vstring) if err != nil { - return false, err - } - // Make a semver of the part after the v in "vX.X.X". - version, err := semver.NewVersion(vstring[1:]) - if err != nil { - return false, err + glog.Errorf("vstring (%s) is not a valid version string: %v", vstring, err) + return true } if version.LessThan(*minVersion) { - return false, nil + return false } - return true, nil + return true } -// GetIptablesVersionString runs "iptables --version" to get the version string, -// then matches for vX.X.X e.g. if "iptables --version" outputs: "iptables v1.3.66" -// then it would would return "v1.3.66", nil +// 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) { // this doesn't access mutable state so we don't need to use the interface / runner bytes, err := exec.Command(cmdIptables, "--version").CombinedOutput() if err != nil { return "", err } - versionMatcher := regexp.MustCompile("v[0-9]+\\.[0-9]+\\.[0-9]+") + versionMatcher := regexp.MustCompile("v([0-9]+\\.[0-9]+\\.[0-9]+)") match := versionMatcher.FindStringSubmatch(string(bytes)) if match == nil { return "", fmt.Errorf("no iptables version found in string: %s", bytes) } - return match[0], nil + return match[1], nil } diff --git a/pkg/util/iptables/iptables_test.go b/pkg/util/iptables/iptables_test.go index b8a0e45a787..b515df2614d 100644 --- a/pkg/util/iptables/iptables_test.go +++ b/pkg/util/iptables/iptables_test.go @@ -17,6 +17,7 @@ limitations under the License. package iptables import ( + "strings" "testing" "k8s.io/kubernetes/pkg/util" @@ -36,6 +37,8 @@ func getIptablesCommand(protocol Protocol) string { func testEnsureChain(t *testing.T, protocol Protocol) { fcmd := exec.FakeCmd{ CombinedOutputScript: []exec.FakeCombinedOutputAction{ + // iptables version check + func() ([]byte, error) { return []byte("iptables v1.9.22"), nil }, // Success. func() ([]byte, error) { return []byte{}, nil }, // Exists. @@ -49,6 +52,7 @@ func testEnsureChain(t *testing.T, protocol Protocol) { 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...) }, 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, protocol) @@ -60,12 +64,12 @@ func testEnsureChain(t *testing.T, protocol Protocol) { if exists { t.Errorf("expected exists = false") } - if fcmd.CombinedOutputCalls != 1 { - t.Errorf("expected 1 CombinedOutput() call, got %d", fcmd.CombinedOutputCalls) + if fcmd.CombinedOutputCalls != 2 { + t.Errorf("expected 2 CombinedOutput() calls, got %d", fcmd.CombinedOutputCalls) } cmd := getIptablesCommand(protocol) - if !util.NewStringSet(fcmd.CombinedOutputLog[0]...).HasAll(cmd, "-t", "nat", "-N", "FOOBAR") { - t.Errorf("wrong CombinedOutput() log, got %s", fcmd.CombinedOutputLog[0]) + if !util.NewStringSet(fcmd.CombinedOutputLog[1]...).HasAll(cmd, "-t", "nat", "-N", "FOOBAR") { + t.Errorf("wrong CombinedOutput() log, got %s", fcmd.CombinedOutputLog[1]) } // Exists. exists, err = runner.EnsureChain(TableNAT, Chain("FOOBAR")) @@ -93,6 +97,8 @@ func TestEnsureChainIpv6(t *testing.T) { func TestFlushChain(t *testing.T) { fcmd := exec.FakeCmd{ CombinedOutputScript: []exec.FakeCombinedOutputAction{ + // iptables version check + func() ([]byte, error) { return []byte("iptables v1.9.22"), nil }, // Success. func() ([]byte, error) { return []byte{}, nil }, // Failure. @@ -103,6 +109,7 @@ func TestFlushChain(t *testing.T) { 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...) }, + func(cmd string, args ...string) exec.Cmd { return exec.InitFakeCmd(&fcmd, cmd, args...) }, }, } runner := New(&fexec, ProtocolIpv4) @@ -111,11 +118,11 @@ func TestFlushChain(t *testing.T) { if err != nil { t.Errorf("expected success, got %v", err) } - if fcmd.CombinedOutputCalls != 1 { - t.Errorf("expected 1 CombinedOutput() call, got %d", fcmd.CombinedOutputCalls) + if fcmd.CombinedOutputCalls != 2 { + t.Errorf("expected 2 CombinedOutput() calls, got %d", fcmd.CombinedOutputCalls) } - if !util.NewStringSet(fcmd.CombinedOutputLog[0]...).HasAll("iptables", "-t", "nat", "-F", "FOOBAR") { - t.Errorf("wrong CombinedOutput() log, got %s", fcmd.CombinedOutputLog[0]) + if !util.NewStringSet(fcmd.CombinedOutputLog[1]...).HasAll("iptables", "-t", "nat", "-F", "FOOBAR") { + t.Errorf("wrong CombinedOutput() log, got %s", fcmd.CombinedOutputLog[1]) } // Failure. err = runner.FlushChain(TableNAT, Chain("FOOBAR")) @@ -127,6 +134,8 @@ func TestFlushChain(t *testing.T) { func TestDeleteChain(t *testing.T) { fcmd := exec.FakeCmd{ CombinedOutputScript: []exec.FakeCombinedOutputAction{ + // iptables version check + func() ([]byte, error) { return []byte("iptables v1.9.22"), nil }, // Success. func() ([]byte, error) { return []byte{}, nil }, // Failure. @@ -137,6 +146,7 @@ func TestDeleteChain(t *testing.T) { 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...) }, + func(cmd string, args ...string) exec.Cmd { return exec.InitFakeCmd(&fcmd, cmd, args...) }, }, } runner := New(&fexec, ProtocolIpv4) @@ -145,11 +155,11 @@ func TestDeleteChain(t *testing.T) { if err != nil { t.Errorf("expected success, got %v", err) } - if fcmd.CombinedOutputCalls != 1 { - t.Errorf("expected 1 CombinedOutput() call, got %d", fcmd.CombinedOutputCalls) + if fcmd.CombinedOutputCalls != 2 { + t.Errorf("expected 2 CombinedOutput() calls, got %d", fcmd.CombinedOutputCalls) } - if !util.NewStringSet(fcmd.CombinedOutputLog[0]...).HasAll("iptables", "-t", "nat", "-X", "FOOBAR") { - t.Errorf("wrong CombinedOutput() log, got %s", fcmd.CombinedOutputLog[0]) + if !util.NewStringSet(fcmd.CombinedOutputLog[1]...).HasAll("iptables", "-t", "nat", "-X", "FOOBAR") { + t.Errorf("wrong CombinedOutput() log, got %s", fcmd.CombinedOutputLog[1]) } // Failure. err = runner.DeleteChain(TableNAT, Chain("FOOBAR")) @@ -184,10 +194,10 @@ func TestEnsureRuleAlreadyExists(t *testing.T) { t.Errorf("expected exists = true") } if fcmd.CombinedOutputCalls != 2 { - t.Errorf("expected 2 CombinedOutput() call, got %d", fcmd.CombinedOutputCalls) + t.Errorf("expected 2 CombinedOutput() calls, got %d", fcmd.CombinedOutputCalls) } if !util.NewStringSet(fcmd.CombinedOutputLog[1]...).HasAll("iptables", "-t", "nat", "-C", "OUTPUT", "abc", "123") { - t.Errorf("wrong CombinedOutput() log, got %s", fcmd.CombinedOutputLog[0]) + t.Errorf("wrong CombinedOutput() log, got %s", fcmd.CombinedOutputLog[1]) } } @@ -223,7 +233,7 @@ func TestEnsureRuleNew(t *testing.T) { t.Errorf("expected 3 CombinedOutput() calls, got %d", fcmd.CombinedOutputCalls) } if !util.NewStringSet(fcmd.CombinedOutputLog[2]...).HasAll("iptables", "-t", "nat", "-A", "OUTPUT", "abc", "123") { - t.Errorf("wrong CombinedOutput() log, got %s", fcmd.CombinedOutputLog[1]) + t.Errorf("wrong CombinedOutput() log, got %s", fcmd.CombinedOutputLog[2]) } } @@ -250,7 +260,7 @@ func TestEnsureRuleErrorChecking(t *testing.T) { t.Errorf("expected failure") } if fcmd.CombinedOutputCalls != 2 { - t.Errorf("expected 2 CombinedOutput() call, got %d", fcmd.CombinedOutputCalls) + t.Errorf("expected 2 CombinedOutput() calls, got %d", fcmd.CombinedOutputCalls) } } @@ -307,10 +317,10 @@ func TestDeleteRuleAlreadyExists(t *testing.T) { t.Errorf("expected success, got %v", err) } if fcmd.CombinedOutputCalls != 2 { - t.Errorf("expected 2 CombinedOutput() call, got %d", fcmd.CombinedOutputCalls) + t.Errorf("expected 2 CombinedOutput() calls, got %d", fcmd.CombinedOutputCalls) } if !util.NewStringSet(fcmd.CombinedOutputLog[1]...).HasAll("iptables", "-t", "nat", "-C", "OUTPUT", "abc", "123") { - t.Errorf("wrong CombinedOutput() log, got %s", fcmd.CombinedOutputLog[0]) + t.Errorf("wrong CombinedOutput() log, got %s", fcmd.CombinedOutputLog[1]) } } @@ -343,7 +353,7 @@ func TestDeleteRuleNew(t *testing.T) { t.Errorf("expected 3 CombinedOutput() calls, got %d", fcmd.CombinedOutputCalls) } if !util.NewStringSet(fcmd.CombinedOutputLog[2]...).HasAll("iptables", "-t", "nat", "-D", "OUTPUT", "abc", "123") { - t.Errorf("wrong CombinedOutput() log, got %s", fcmd.CombinedOutputLog[1]) + t.Errorf("wrong CombinedOutput() log, got %s", fcmd.CombinedOutputLog[2]) } } @@ -370,7 +380,7 @@ func TestDeleteRuleErrorChecking(t *testing.T) { t.Errorf("expected failure") } if fcmd.CombinedOutputCalls != 2 { - t.Errorf("expected 2 CombinedOutput() call, got %d", fcmd.CombinedOutputCalls) + t.Errorf("expected 2 CombinedOutput() calls, got %d", fcmd.CombinedOutputCalls) } } @@ -428,12 +438,15 @@ func TestGetIptablesHasCheckCommand(t *testing.T) { func(cmd string, args ...string) exec.Cmd { return exec.InitFakeCmd(&fcmd, cmd, args...) }, }, } - check, err := getIptablesHasCheckCommand(&fexec) + version, err := GetIptablesVersionString(&fexec) if (err != nil) != testCase.Err { t.Errorf("Expected error: %v, Got error: %v", testCase.Err, err) } - if err == nil && testCase.Expected != check { - t.Errorf("Expected result: %v, Got result: %v", testCase.Expected, check) + if err == nil { + check := getIptablesHasCheckCommand(version) + if testCase.Expected != check { + t.Errorf("Expected result: %v, Got result: %v", testCase.Expected, check) + } } } } @@ -513,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]) + } +}