diff --git a/cmd/kube-proxy/app/BUILD b/cmd/kube-proxy/app/BUILD index 30b66837aa2..86a69857cf1 100644 --- a/cmd/kube-proxy/app/BUILD +++ b/cmd/kube-proxy/app/BUILD @@ -171,43 +171,33 @@ go_test( ] + select({ "@io_bazel_rules_go//go/platform:android": [ "//pkg/proxy/ipvs:go_default_library", - "//pkg/util/iptables:go_default_library", ], "@io_bazel_rules_go//go/platform:darwin": [ "//pkg/proxy/ipvs:go_default_library", - "//pkg/util/iptables:go_default_library", ], "@io_bazel_rules_go//go/platform:dragonfly": [ "//pkg/proxy/ipvs:go_default_library", - "//pkg/util/iptables:go_default_library", ], "@io_bazel_rules_go//go/platform:freebsd": [ "//pkg/proxy/ipvs:go_default_library", - "//pkg/util/iptables:go_default_library", ], "@io_bazel_rules_go//go/platform:linux": [ "//pkg/proxy/ipvs:go_default_library", - "//pkg/util/iptables:go_default_library", ], "@io_bazel_rules_go//go/platform:nacl": [ "//pkg/proxy/ipvs:go_default_library", - "//pkg/util/iptables:go_default_library", ], "@io_bazel_rules_go//go/platform:netbsd": [ "//pkg/proxy/ipvs:go_default_library", - "//pkg/util/iptables:go_default_library", ], "@io_bazel_rules_go//go/platform:openbsd": [ "//pkg/proxy/ipvs:go_default_library", - "//pkg/util/iptables:go_default_library", ], "@io_bazel_rules_go//go/platform:plan9": [ "//pkg/proxy/ipvs:go_default_library", - "//pkg/util/iptables:go_default_library", ], "@io_bazel_rules_go//go/platform:solaris": [ "//pkg/proxy/ipvs:go_default_library", - "//pkg/util/iptables:go_default_library", ], "//conditions:default": [], }), diff --git a/cmd/kube-proxy/app/server_others.go b/cmd/kube-proxy/app/server_others.go index 1000fda27a3..a43767a8136 100644 --- a/cmd/kube-proxy/app/server_others.go +++ b/cmd/kube-proxy/app/server_others.go @@ -133,7 +133,7 @@ func newProxyServer( var proxier proxy.ProxyProvider - proxyMode := getProxyMode(string(config.Mode), iptInterface, kernelHandler, ipsetInterface, iptables.LinuxKernelCompatTester{}) + proxyMode := getProxyMode(string(config.Mode), kernelHandler, ipsetInterface, iptables.LinuxKernelCompatTester{}) nodeIP := net.ParseIP(config.BindAddress) if nodeIP.IsUnspecified() { nodeIP = utilnode.GetNodeIP(client, hostname) @@ -238,20 +238,20 @@ func newProxyServer( }, nil } -func getProxyMode(proxyMode string, iptver iptables.Versioner, khandle ipvs.KernelHandler, ipsetver ipvs.IPSetVersioner, kcompat iptables.KernelCompatTester) string { +func getProxyMode(proxyMode string, khandle ipvs.KernelHandler, ipsetver ipvs.IPSetVersioner, kcompat iptables.KernelCompatTester) string { switch proxyMode { case proxyModeUserspace: return proxyModeUserspace case proxyModeIPTables: - return tryIPTablesProxy(iptver, kcompat) + return tryIPTablesProxy(kcompat) case proxyModeIPVS: - return tryIPVSProxy(iptver, khandle, ipsetver, kcompat) + return tryIPVSProxy(khandle, ipsetver, kcompat) } klog.Warningf("Flag proxy-mode=%q unknown, assuming iptables proxy", proxyMode) - return tryIPTablesProxy(iptver, kcompat) + return tryIPTablesProxy(kcompat) } -func tryIPVSProxy(iptver iptables.Versioner, khandle ipvs.KernelHandler, ipsetver ipvs.IPSetVersioner, kcompat iptables.KernelCompatTester) string { +func tryIPVSProxy(khandle ipvs.KernelHandler, ipsetver ipvs.IPSetVersioner, kcompat iptables.KernelCompatTester) string { // guaranteed false on error, error only necessary for debugging // IPVS Proxier relies on ip_vs_* kernel modules and ipset useIPVSProxy, err := ipvs.CanUseIPVSProxier(khandle, ipsetver) @@ -265,12 +265,12 @@ func tryIPVSProxy(iptver iptables.Versioner, khandle ipvs.KernelHandler, ipsetve // Try to fallback to iptables before falling back to userspace klog.V(1).Infof("Can't use ipvs proxier, trying iptables proxier") - return tryIPTablesProxy(iptver, kcompat) + return tryIPTablesProxy(kcompat) } -func tryIPTablesProxy(iptver iptables.Versioner, kcompat iptables.KernelCompatTester) string { +func tryIPTablesProxy(kcompat iptables.KernelCompatTester) string { // guaranteed false on error, error only necessary for debugging - useIPTablesProxy, err := iptables.CanUseIPTablesProxier(iptver, kcompat) + useIPTablesProxy, err := iptables.CanUseIPTablesProxier(kcompat) if err != nil { utilruntime.HandleError(fmt.Errorf("can't determine whether to use iptables proxy, using userspace proxier: %v", err)) return proxyModeUserspace diff --git a/cmd/kube-proxy/app/server_others_test.go b/cmd/kube-proxy/app/server_others_test.go index a7b7f216bbf..eb6b086142a 100644 --- a/cmd/kube-proxy/app/server_others_test.go +++ b/cmd/kube-proxy/app/server_others_test.go @@ -23,68 +23,75 @@ import ( "testing" "k8s.io/kubernetes/pkg/proxy/ipvs" - "k8s.io/kubernetes/pkg/util/iptables" ) +type fakeIPSetVersioner struct { + version string // what to return + err error // what to return +} + +func (fake *fakeIPSetVersioner) GetVersion() (string, error) { + return fake.version, fake.err +} + +type fakeKernelCompatTester struct { + ok bool +} + +func (fake *fakeKernelCompatTester) IsCompatible() error { + if !fake.ok { + return fmt.Errorf("error") + } + return nil +} + +// fakeKernelHandler implements KernelHandler. +type fakeKernelHandler struct { + modules []string + kernelVersion string +} + +func (fake *fakeKernelHandler) GetModules() ([]string, error) { + return fake.modules, nil +} + +func (fake *fakeKernelHandler) GetKernelVersion() (string, error) { + return fake.kernelVersion, nil +} + func Test_getProxyMode(t *testing.T) { var cases = []struct { - flag string - iptablesVersion string - ipsetVersion string - kmods []string - kernelVersion string - kernelCompat bool - iptablesError error - ipsetError error - expected string + flag string + ipsetVersion string + kmods []string + kernelVersion string + kernelCompat bool + ipsetError error + expected string }{ { // flag says userspace flag: "userspace", expected: proxyModeUserspace, }, - { // flag says iptables, error detecting version - flag: "iptables", - iptablesError: fmt.Errorf("flag says iptables, error detecting version"), - expected: proxyModeUserspace, + { // flag says iptables, kernel not compatible + flag: "iptables", + kernelCompat: false, + expected: proxyModeUserspace, }, - { // flag says iptables, version too low - flag: "iptables", - iptablesVersion: "0.0.0", - expected: proxyModeUserspace, + { // flag says iptables, kernel is compatible + flag: "iptables", + kernelCompat: true, + expected: proxyModeIPTables, }, - { // flag says iptables, version ok, kernel not compatible - flag: "iptables", - iptablesVersion: iptables.MinCheckVersion, - kernelCompat: false, - expected: proxyModeUserspace, + { // detect, kernel not compatible + flag: "", + kernelCompat: false, + expected: proxyModeUserspace, }, - { // flag says iptables, version ok, kernel is compatible - flag: "iptables", - iptablesVersion: iptables.MinCheckVersion, - kernelCompat: true, - expected: proxyModeIPTables, - }, - { // detect, error - flag: "", - iptablesError: fmt.Errorf("oops"), - expected: proxyModeUserspace, - }, - { // detect, version too low - flag: "", - iptablesVersion: "0.0.0", - expected: proxyModeUserspace, - }, - { // detect, version ok, kernel not compatible - flag: "", - iptablesVersion: iptables.MinCheckVersion, - kernelCompat: false, - expected: proxyModeUserspace, - }, - { // detect, version ok, kernel is compatible - flag: "", - iptablesVersion: iptables.MinCheckVersion, - kernelCompat: true, - expected: proxyModeIPTables, + { // detect, kernel is compatible + flag: "", + kernelCompat: true, + expected: proxyModeIPTables, }, { // flag says ipvs, ipset version ok, kernel modules installed for linux kernel before 4.19 flag: "ipvs", @@ -101,69 +108,38 @@ func Test_getProxyMode(t *testing.T) { expected: proxyModeIPVS, }, { // flag says ipvs, ipset version too low, fallback on iptables mode - flag: "ipvs", - kmods: []string{"ip_vs", "ip_vs_rr", "ip_vs_wrr", "ip_vs_sh", "nf_conntrack"}, - kernelVersion: "4.19", - ipsetVersion: "0.0", - iptablesVersion: iptables.MinCheckVersion, - kernelCompat: true, - expected: proxyModeIPTables, + flag: "ipvs", + kmods: []string{"ip_vs", "ip_vs_rr", "ip_vs_wrr", "ip_vs_sh", "nf_conntrack"}, + kernelVersion: "4.19", + ipsetVersion: "0.0", + kernelCompat: true, + expected: proxyModeIPTables, }, { // flag says ipvs, bad ipset version, fallback on iptables mode - flag: "ipvs", - kmods: []string{"ip_vs", "ip_vs_rr", "ip_vs_wrr", "ip_vs_sh", "nf_conntrack"}, - kernelVersion: "4.19", - ipsetVersion: "a.b.c", - iptablesVersion: iptables.MinCheckVersion, - kernelCompat: true, - expected: proxyModeIPTables, + flag: "ipvs", + kmods: []string{"ip_vs", "ip_vs_rr", "ip_vs_wrr", "ip_vs_sh", "nf_conntrack"}, + kernelVersion: "4.19", + ipsetVersion: "a.b.c", + kernelCompat: true, + expected: proxyModeIPTables, }, { // flag says ipvs, required kernel modules are not installed, fallback on iptables mode - flag: "ipvs", - kmods: []string{"foo", "bar", "baz"}, - kernelVersion: "4.19", - ipsetVersion: ipvs.MinIPSetCheckVersion, - iptablesVersion: iptables.MinCheckVersion, - kernelCompat: true, - expected: proxyModeIPTables, - }, - { // flag says ipvs, required kernel modules are not installed, iptables version too old, fallback on userspace mode - flag: "ipvs", - kmods: []string{"foo", "bar", "baz"}, - kernelVersion: "4.19", - ipsetVersion: ipvs.MinIPSetCheckVersion, - iptablesVersion: "0.0.0", - kernelCompat: true, - expected: proxyModeUserspace, - }, - { // flag says ipvs, required kernel modules are not installed, iptables version too old, fallback on userspace mode - flag: "ipvs", - kmods: []string{"foo", "bar", "baz"}, - kernelVersion: "4.19", - ipsetVersion: ipvs.MinIPSetCheckVersion, - iptablesVersion: "0.0.0", - kernelCompat: true, - expected: proxyModeUserspace, - }, - { // flag says ipvs, ipset version too low, iptables version too old, kernel not compatible, fallback on userspace mode - flag: "ipvs", - kmods: []string{"ip_vs", "ip_vs_rr", "ip_vs_wrr", "ip_vs_sh", "nf_conntrack"}, - kernelVersion: "4.19", - ipsetVersion: "0.0", - iptablesVersion: iptables.MinCheckVersion, - kernelCompat: false, - expected: proxyModeUserspace, + flag: "ipvs", + kmods: []string{"foo", "bar", "baz"}, + kernelVersion: "4.19", + ipsetVersion: ipvs.MinIPSetCheckVersion, + kernelCompat: true, + expected: proxyModeIPTables, }, } for i, c := range cases { - versioner := &fakeIPTablesVersioner{c.iptablesVersion, c.iptablesError} kcompater := &fakeKernelCompatTester{c.kernelCompat} ipsetver := &fakeIPSetVersioner{c.ipsetVersion, c.ipsetError} khandler := &fakeKernelHandler{ modules: c.kmods, kernelVersion: c.kernelVersion, } - r := getProxyMode(c.flag, versioner, khandler, ipsetver, kcompater) + r := getProxyMode(c.flag, khandler, ipsetver, kcompater) if r != c.expected { t.Errorf("Case[%d] Expected %q, got %q", i, c.expected, r) } diff --git a/cmd/kube-proxy/app/server_test.go b/cmd/kube-proxy/app/server_test.go index bc38b6005a9..0fc014e9308 100644 --- a/cmd/kube-proxy/app/server_test.go +++ b/cmd/kube-proxy/app/server_test.go @@ -38,53 +38,6 @@ import ( utilpointer "k8s.io/utils/pointer" ) -type fakeIPTablesVersioner struct { - version string // what to return - err error // what to return -} - -func (fake *fakeIPTablesVersioner) GetVersion() (string, error) { - return fake.version, fake.err -} - -func (fake *fakeIPTablesVersioner) IsCompatible() error { - return fake.err -} - -type fakeIPSetVersioner struct { - version string // what to return - err error // what to return -} - -func (fake *fakeIPSetVersioner) GetVersion() (string, error) { - return fake.version, fake.err -} - -type fakeKernelCompatTester struct { - ok bool -} - -func (fake *fakeKernelCompatTester) IsCompatible() error { - if !fake.ok { - return fmt.Errorf("error") - } - return nil -} - -// fakeKernelHandler implements KernelHandler. -type fakeKernelHandler struct { - modules []string - kernelVersion string -} - -func (fake *fakeKernelHandler) GetModules() ([]string, error) { - return fake.modules, nil -} - -func (fake *fakeKernelHandler) GetKernelVersion() (string, error) { - return fake.kernelVersion, nil -} - // This test verifies that NewProxyServer does not crash when CleanupAndExit is true. func TestProxyServerWithCleanupAndExit(t *testing.T) { // Each bind address below is a separate test case diff --git a/pkg/kubelet/dockershim/network/hostport/fake_iptables.go b/pkg/kubelet/dockershim/network/hostport/fake_iptables.go index ab28a1d8747..83ec5ba7994 100644 --- a/pkg/kubelet/dockershim/network/hostport/fake_iptables.go +++ b/pkg/kubelet/dockershim/network/hostport/fake_iptables.go @@ -52,10 +52,6 @@ func NewFakeIPTables() *fakeIPTables { } } -func (f *fakeIPTables) GetVersion() (string, error) { - return "1.4.21", nil -} - func (f *fakeIPTables) getTable(tableName utiliptables.Table) (*fakeTable, error) { table, ok := f.tables[string(tableName)] if !ok { diff --git a/pkg/proxy/iptables/BUILD b/pkg/proxy/iptables/BUILD index 3c0ef8f1334..f7e0d409c6b 100644 --- a/pkg/proxy/iptables/BUILD +++ b/pkg/proxy/iptables/BUILD @@ -21,7 +21,6 @@ go_library( "//pkg/util/sysctl:go_default_library", "//staging/src/k8s.io/api/core/v1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/types:go_default_library", - "//staging/src/k8s.io/apimachinery/pkg/util/version:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/util/wait:go_default_library", "//staging/src/k8s.io/client-go/tools/record:go_default_library", "//vendor/k8s.io/klog:go_default_library", diff --git a/pkg/proxy/iptables/proxier.go b/pkg/proxy/iptables/proxier.go index 34e4d3e8f83..1ce47a5e409 100644 --- a/pkg/proxy/iptables/proxier.go +++ b/pkg/proxy/iptables/proxier.go @@ -36,7 +36,6 @@ import ( v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/types" - utilversion "k8s.io/apimachinery/pkg/util/version" "k8s.io/apimachinery/pkg/util/wait" "k8s.io/client-go/tools/record" "k8s.io/kubernetes/pkg/proxy" @@ -52,15 +51,6 @@ import ( ) const ( - // iptablesMinVersion is the minimum version of iptables for which we will use the Proxier - // from this package instead of the userspace Proxier. While most of the - // features we need were available earlier, the '-C' flag was added more - // recently. We use that indirectly in Ensure* functions, and if we don't - // have it, we have to be extra careful about the exact args we feed in being - // the same as the args we read back (iptables itself normalizes some args). - // This is the "new" Proxier, so we require "new" versions of tools. - iptablesMinVersion = utiliptables.MinCheckVersion - // the services chain kubeServicesChain utiliptables.Chain = "KUBE-SERVICES" @@ -83,12 +73,6 @@ const ( kubeForwardChain utiliptables.Chain = "KUBE-FORWARD" ) -// Versioner can query the current iptables version. -type Versioner interface { - // returns "X.Y.Z" - GetVersion() (string, error) -} - // KernelCompatTester tests whether the required kernel capabilities are // present to run the iptables proxier. type KernelCompatTester interface { @@ -96,28 +80,8 @@ type KernelCompatTester interface { } // CanUseIPTablesProxier returns true if we should use the iptables Proxier -// instead of the "classic" userspace Proxier. This is determined by checking -// the iptables version and for the existence of kernel features. It may return -// an error if it fails to get the iptables version without error, in which -// case it will also return false. -func CanUseIPTablesProxier(iptver Versioner, kcompat KernelCompatTester) (bool, error) { - minVersion, err := utilversion.ParseGeneric(iptablesMinVersion) - if err != nil { - return false, err - } - versionString, err := iptver.GetVersion() - if err != nil { - return false, err - } - version, err := utilversion.ParseGeneric(versionString) - if err != nil { - return false, err - } - if version.LessThan(minVersion) { - return false, nil - } - - // Check that the kernel supports what we need. +// instead of the "classic" userspace Proxier. +func CanUseIPTablesProxier(kcompat KernelCompatTester) (bool, error) { if err := kcompat.IsCompatible(); err != nil { return false, err } @@ -131,7 +95,6 @@ type LinuxKernelCompatTester struct{} // that it exists. If this Proxier is chosen, we'll initialize it as we // need. func (lkct LinuxKernelCompatTester) IsCompatible() error { - _, err := utilsysctl.New().GetSysctl(sysctlRouteLocalnet) return err } diff --git a/pkg/util/iptables/BUILD b/pkg/util/iptables/BUILD index 6e62709b6da..a0d32ac7f78 100644 --- a/pkg/util/iptables/BUILD +++ b/pkg/util/iptables/BUILD @@ -45,6 +45,7 @@ go_test( "@io_bazel_rules_go//go/platform:linux": [ "//pkg/util/dbus:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/util/sets:go_default_library", + "//staging/src/k8s.io/apimachinery/pkg/util/version:go_default_library", "//vendor/k8s.io/utils/exec:go_default_library", "//vendor/k8s.io/utils/exec/testing:go_default_library", ], diff --git a/pkg/util/iptables/iptables.go b/pkg/util/iptables/iptables.go index 761e55c94a1..2f1faed3e07 100644 --- a/pkg/util/iptables/iptables.go +++ b/pkg/util/iptables/iptables.go @@ -43,8 +43,6 @@ const ( // An injectable interface for running iptables commands. Implementations must be goroutine-safe. type Interface interface { - // GetVersion returns the "X.Y.Z" version string for iptables. - GetVersion() (string, error) // EnsureChain checks if the specified chain exists and, if not, creates it. If the chain existed, return true. EnsureChain(table Table, chain Chain) (bool, error) // FlushChain clears the specified chain. If the chain did not exist, return error. @@ -121,11 +119,13 @@ const NoFlushTables FlushFlag = false // Versions of iptables less than this do not support the -C / --check flag // (test whether a rule exists). -const MinCheckVersion = "1.4.11" +var MinCheckVersion = utilversion.MustParseGeneric("1.4.11") // Minimum iptables versions supporting the -w and -w flags -const WaitMinVersion = "1.4.20" -const WaitSecondsMinVersion = "1.4.22" +var WaitMinVersion = utilversion.MustParseGeneric("1.4.20") +var WaitSecondsMinVersion = utilversion.MustParseGeneric("1.4.22") +var WaitRestoreMinVersion = utilversion.MustParseGeneric("1.6.2") + const WaitString = "-w" const WaitSecondsValue = "5" @@ -150,10 +150,10 @@ type runner struct { // newInternal returns a new Interface which will exec iptables, and allows the // caller to change the iptables-restore lockfile path func newInternal(exec utilexec.Interface, dbus utildbus.Interface, protocol Protocol, lockfilePath string) Interface { - vstring, err := getIPTablesVersionString(exec, protocol) + version, err := getIPTablesVersion(exec, protocol) if err != nil { klog.Warningf("Error checking iptables version, assuming version at least %s: %v", MinCheckVersion, err) - vstring = MinCheckVersion + version = MinCheckVersion } if lockfilePath == "" { @@ -164,10 +164,10 @@ func newInternal(exec utilexec.Interface, dbus utildbus.Interface, protocol Prot exec: exec, dbus: dbus, protocol: protocol, - hasCheck: getIPTablesHasCheckCommand(vstring), + hasCheck: version.AtLeast(MinCheckVersion), hasListener: false, - waitFlag: getIPTablesWaitFlag(vstring), - restoreWaitFlag: getIPTablesRestoreWaitFlag(exec, protocol), + waitFlag: getIPTablesWaitFlag(version), + restoreWaitFlag: getIPTablesRestoreWaitFlag(version), lockfilePath: lockfilePath, } return runner @@ -214,11 +214,6 @@ func (runner *runner) connectToFirewallD() { go runner.dbusSignalHandler(bus) } -// GetVersion returns the version string. -func (runner *runner) GetVersion() (string, error) { - return getIPTablesVersionString(runner.exec, runner.protocol) -} - // EnsureChain is part of Interface. func (runner *runner) EnsureChain(table Table, chain Chain) (bool, error) { fullArgs := makeFullArgs(table, chain) @@ -539,106 +534,46 @@ func makeFullArgs(table Table, chain Chain, args ...string) []string { return append([]string{string(chain), "-t", string(table)}, args...) } -// Checks if iptables has the "-C" flag -func getIPTablesHasCheckCommand(vstring string) bool { - minVersion, err := utilversion.ParseGeneric(MinCheckVersion) - if err != nil { - klog.Errorf("MinCheckVersion (%s) is not a valid version string: %v", MinCheckVersion, err) - return true - } - version, err := utilversion.ParseGeneric(vstring) - if err != nil { - klog.Errorf("vstring (%s) is not a valid version string: %v", vstring, err) - return true - } - return version.AtLeast(minVersion) -} - -// Checks if iptables version has a "wait" flag -func getIPTablesWaitFlag(vstring string) []string { - version, err := utilversion.ParseGeneric(vstring) - if err != nil { - klog.Errorf("vstring (%s) is not a valid version string: %v", vstring, err) - return nil - } - - minVersion, err := utilversion.ParseGeneric(WaitMinVersion) - if err != nil { - klog.Errorf("WaitMinVersion (%s) is not a valid version string: %v", WaitMinVersion, err) - return nil - } - if version.LessThan(minVersion) { - return nil - } - - minVersion, err = utilversion.ParseGeneric(WaitSecondsMinVersion) - if err != nil { - klog.Errorf("WaitSecondsMinVersion (%s) is not a valid version string: %v", WaitSecondsMinVersion, err) - return nil - } - if version.LessThan(minVersion) { - return []string{WaitString} - } - return []string{WaitString, WaitSecondsValue} - -} - -// getIPTablesVersionString runs "iptables --version" to get the version string -// in the form "X.X.X" -func getIPTablesVersionString(exec utilexec.Interface, protocol Protocol) (string, error) { +// getIPTablesVersion runs "iptables --version" and parses the returned version +func getIPTablesVersion(exec utilexec.Interface, protocol Protocol) (*utilversion.Version, error) { // this doesn't access mutable state so we don't need to use the interface / runner iptablesCmd := iptablesCommand(protocol) bytes, err := exec.Command(iptablesCmd, "--version").CombinedOutput() if err != nil { - return "", err + return nil, err } versionMatcher := regexp.MustCompile("v([0-9]+(\\.[0-9]+)+)") match := versionMatcher.FindStringSubmatch(string(bytes)) if match == nil { - return "", fmt.Errorf("no iptables version found in string: %s", bytes) + return nil, fmt.Errorf("no iptables version found in string: %s", bytes) + } + version, err := utilversion.ParseGeneric(match[1]) + if err != nil { + return nil, fmt.Errorf("iptables version %q is not a valid version string: %v", match[1], err) + } + + return version, nil +} + +// Checks if iptables version has a "wait" flag +func getIPTablesWaitFlag(version *utilversion.Version) []string { + switch { + case version.AtLeast(WaitSecondsMinVersion): + return []string{WaitString, WaitSecondsValue} + case version.AtLeast(WaitMinVersion): + return []string{WaitString} + default: + return nil } - return match[1], nil } // Checks if iptables-restore has a "wait" flag -// --wait support landed in v1.6.1+ right before --version support, so -// any version of iptables-restore that supports --version will also -// support --wait -func getIPTablesRestoreWaitFlag(exec utilexec.Interface, protocol Protocol) []string { - vstring, err := getIPTablesRestoreVersionString(exec, protocol) - if err != nil || vstring == "" { - klog.V(3).Infof("couldn't get iptables-restore version; assuming it doesn't support --wait") +func getIPTablesRestoreWaitFlag(version *utilversion.Version) []string { + if version.AtLeast(WaitRestoreMinVersion) { + return []string{WaitString, WaitSecondsValue} + } else { return nil } - if _, err := utilversion.ParseGeneric(vstring); err != nil { - klog.V(3).Infof("couldn't parse iptables-restore version; assuming it doesn't support --wait") - return nil - } - - return []string{WaitString, WaitSecondsValue} -} - -// getIPTablesRestoreVersionString runs "iptables-restore --version" to get the version string -// in the form "X.X.X" -func getIPTablesRestoreVersionString(exec utilexec.Interface, protocol Protocol) (string, error) { - // this doesn't access mutable state so we don't need to use the interface / runner - - // iptables-restore hasn't always had --version, and worse complains - // about unrecognized commands but doesn't exit when it gets them. - // Work around that by setting stdin to nothing so it exits immediately. - iptablesRestoreCmd := iptablesRestoreCommand(protocol) - cmd := exec.Command(iptablesRestoreCmd, "--version") - cmd.SetStdin(bytes.NewReader([]byte{})) - bytes, err := cmd.CombinedOutput() - if err != nil { - return "", err - } - versionMatcher := regexp.MustCompile("v([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[1], nil } // goroutine to listen for D-Bus signals diff --git a/pkg/util/iptables/iptables_test.go b/pkg/util/iptables/iptables_test.go index dfc58095381..f263c1d2eef 100644 --- a/pkg/util/iptables/iptables_test.go +++ b/pkg/util/iptables/iptables_test.go @@ -29,6 +29,7 @@ import ( "time" "k8s.io/apimachinery/pkg/util/sets" + utilversion "k8s.io/apimachinery/pkg/util/version" "k8s.io/kubernetes/pkg/util/dbus" "k8s.io/utils/exec" fakeexec "k8s.io/utils/exec/testing" @@ -46,24 +47,17 @@ func protocolStr(protocol Protocol) string { func testIPTablesVersionCmds(t *testing.T, protocol Protocol) { version := " v1.9.22" iptablesCmd := iptablesCommand(protocol) - iptablesRestoreCmd := iptablesRestoreCommand(protocol) protoStr := protocolStr(protocol) fcmd := fakeexec.FakeCmd{ CombinedOutputScript: []fakeexec.FakeCombinedOutputAction{ // iptables version response (for runner instantiation) func() ([]byte, error) { return []byte(iptablesCmd + version), nil }, - // iptables-restore version response (for runner instantiation) - func() ([]byte, error) { return []byte(iptablesRestoreCmd + version), nil }, - // iptables version response (for call to runner.GetVersion()) - func() ([]byte, error) { return []byte(iptablesCmd + version), 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, dbus.NewFake(nil, nil), protocol) @@ -73,21 +67,6 @@ func testIPTablesVersionCmds(t *testing.T, protocol Protocol) { if !sets.NewString(fcmd.CombinedOutputLog[0]...).HasAll(iptablesCmd, "--version") { t.Errorf("%s runner instantiate: Expected cmd '%s --version', Got '%s'", protoStr, iptablesCmd, fcmd.CombinedOutputLog[0]) } - - // Check that proper iptables restore version command was used during runner instantiation - if !sets.NewString(fcmd.CombinedOutputLog[1]...).HasAll(iptablesRestoreCmd, "--version") { - t.Errorf("%s runner instantiate: Expected cmd '%s --version', Got '%s'", protoStr, iptablesRestoreCmd, fcmd.CombinedOutputLog[1]) - } - - _, err := runner.GetVersion() - if err != nil { - t.Errorf("%s GetVersion: Expected success, got %v", protoStr, err) - } - - // Check that proper iptables version command was used for runner.GetVersion - if !sets.NewString(fcmd.CombinedOutputLog[2]...).HasAll(iptablesCmd, "--version") { - t.Errorf("%s GetVersion: Expected cmd '%s --version', Got '%s'", protoStr, iptablesCmd, fcmd.CombinedOutputLog[2]) - } } func TestIPTablesVersionCmdsIPv4(t *testing.T) { @@ -105,8 +84,6 @@ func testEnsureChain(t *testing.T, protocol Protocol) { CombinedOutputScript: []fakeexec.FakeCombinedOutputAction{ // iptables version check func() ([]byte, error) { return []byte("iptables v1.9.22"), nil }, - // iptables-restore version check - func() ([]byte, error) { return []byte("iptables-restore v1.9.22"), nil }, // Success. func() ([]byte, error) { return []byte{}, nil }, // Exists. @@ -121,7 +98,6 @@ func testEnsureChain(t *testing.T, protocol Protocol) { 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...) }, - func(cmd string, args ...string) exec.Cmd { return fakeexec.InitFakeCmd(&fcmd, cmd, args...) }, }, } runner := New(&fexec, dbus.NewFake(nil, nil), protocol) @@ -134,11 +110,11 @@ func testEnsureChain(t *testing.T, protocol Protocol) { if exists { t.Errorf("%s new chain: Expected exists = false", protoStr) } - if fcmd.CombinedOutputCalls != 3 { - t.Errorf("%s new chain: Expected 3 CombinedOutput() calls, got %d", protoStr, fcmd.CombinedOutputCalls) + if fcmd.CombinedOutputCalls != 2 { + t.Errorf("%s new chain: Expected 2 CombinedOutput() calls, got %d", protoStr, fcmd.CombinedOutputCalls) } cmd := iptablesCommand(protocol) - if !sets.NewString(fcmd.CombinedOutputLog[2]...).HasAll(cmd, "-t", "nat", "-N", "FOOBAR") { + if !sets.NewString(fcmd.CombinedOutputLog[1]...).HasAll(cmd, "-t", "nat", "-N", "FOOBAR") { t.Errorf("%s new chain: Expected cmd containing '%s -t nat -N FOOBAR', got %s", protoStr, cmd, fcmd.CombinedOutputLog[2]) } // Exists. @@ -169,8 +145,6 @@ func TestFlushChain(t *testing.T) { CombinedOutputScript: []fakeexec.FakeCombinedOutputAction{ // iptables version check func() ([]byte, error) { return []byte("iptables v1.9.22"), nil }, - // iptables-restore version check - func() ([]byte, error) { return []byte("iptables-restore v1.9.22"), nil }, // Success. func() ([]byte, error) { return []byte{}, nil }, // Failure. @@ -182,7 +156,6 @@ func TestFlushChain(t *testing.T) { 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...) }, - func(cmd string, args ...string) exec.Cmd { return fakeexec.InitFakeCmd(&fcmd, cmd, args...) }, }, } runner := New(&fexec, dbus.NewFake(nil, nil), ProtocolIpv4) @@ -192,10 +165,10 @@ func TestFlushChain(t *testing.T) { 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 fcmd.CombinedOutputCalls != 2 { + t.Errorf("expected 2 CombinedOutput() calls, got %d", fcmd.CombinedOutputCalls) } - if !sets.NewString(fcmd.CombinedOutputLog[2]...).HasAll("iptables", "-t", "nat", "-F", "FOOBAR") { + if !sets.NewString(fcmd.CombinedOutputLog[1]...).HasAll("iptables", "-t", "nat", "-F", "FOOBAR") { t.Errorf("wrong CombinedOutput() log, got %s", fcmd.CombinedOutputLog[2]) } // Failure. @@ -210,8 +183,6 @@ func TestDeleteChain(t *testing.T) { CombinedOutputScript: []fakeexec.FakeCombinedOutputAction{ // iptables version check func() ([]byte, error) { return []byte("iptables v1.9.22"), nil }, - // iptables-restore version check - func() ([]byte, error) { return []byte("iptables-restore v1.9.22"), nil }, // Success. func() ([]byte, error) { return []byte{}, nil }, // Failure. @@ -223,7 +194,6 @@ func TestDeleteChain(t *testing.T) { 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...) }, - func(cmd string, args ...string) exec.Cmd { return fakeexec.InitFakeCmd(&fcmd, cmd, args...) }, }, } runner := New(&fexec, dbus.NewFake(nil, nil), ProtocolIpv4) @@ -233,10 +203,10 @@ func TestDeleteChain(t *testing.T) { 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 fcmd.CombinedOutputCalls != 2 { + t.Errorf("expected 2 CombinedOutput() calls, got %d", fcmd.CombinedOutputCalls) } - if !sets.NewString(fcmd.CombinedOutputLog[2]...).HasAll("iptables", "-t", "nat", "-X", "FOOBAR") { + if !sets.NewString(fcmd.CombinedOutputLog[1]...).HasAll("iptables", "-t", "nat", "-X", "FOOBAR") { t.Errorf("wrong CombinedOutput() log, got %s", fcmd.CombinedOutputLog[2]) } // Failure. @@ -251,8 +221,6 @@ func TestEnsureRuleAlreadyExists(t *testing.T) { CombinedOutputScript: []fakeexec.FakeCombinedOutputAction{ // iptables version check func() ([]byte, error) { return []byte("iptables v1.9.22"), nil }, - // iptables-restore version check - func() ([]byte, error) { return []byte("iptables-restore v1.9.22"), nil }, // Success. func() ([]byte, error) { return []byte{}, nil }, }, @@ -261,8 +229,6 @@ func TestEnsureRuleAlreadyExists(t *testing.T) { CommandScript: []fakeexec.FakeCommandAction{ // iptables version check func(cmd string, args ...string) exec.Cmd { return fakeexec.InitFakeCmd(&fcmd, cmd, args...) }, - // iptables-restore version check - func(cmd string, args ...string) exec.Cmd { return fakeexec.InitFakeCmd(&fcmd, cmd, args...) }, // The second Command() call is checking the rule. Success of that exec means "done". func(cmd string, args ...string) exec.Cmd { return fakeexec.InitFakeCmd(&fcmd, cmd, args...) }, }, @@ -276,10 +242,10 @@ func TestEnsureRuleAlreadyExists(t *testing.T) { if !exists { t.Errorf("expected exists = true") } - if fcmd.CombinedOutputCalls != 3 { - t.Errorf("expected 3 CombinedOutput() calls, got %d", fcmd.CombinedOutputCalls) + if fcmd.CombinedOutputCalls != 2 { + t.Errorf("expected 2 CombinedOutput() calls, got %d", fcmd.CombinedOutputCalls) } - if !sets.NewString(fcmd.CombinedOutputLog[2]...).HasAll("iptables", "-t", "nat", "-C", "OUTPUT", "abc", "123") { + if !sets.NewString(fcmd.CombinedOutputLog[1]...).HasAll("iptables", "-t", "nat", "-C", "OUTPUT", "abc", "123") { t.Errorf("wrong CombinedOutput() log, got %s", fcmd.CombinedOutputLog[2]) } } @@ -289,8 +255,6 @@ func TestEnsureRuleNew(t *testing.T) { CombinedOutputScript: []fakeexec.FakeCombinedOutputAction{ // iptables version check func() ([]byte, error) { return []byte("iptables v1.9.22"), nil }, - // iptables-restore version check - func() ([]byte, error) { return []byte("iptables-restore v1.9.22"), nil }, // Status 1 on the first call. func() ([]byte, error) { return nil, &fakeexec.FakeExitError{Status: 1} }, // Success on the second call. @@ -301,8 +265,6 @@ func TestEnsureRuleNew(t *testing.T) { CommandScript: []fakeexec.FakeCommandAction{ // iptables version check func(cmd string, args ...string) exec.Cmd { return fakeexec.InitFakeCmd(&fcmd, cmd, args...) }, - // iptables-restore version check - func(cmd string, args ...string) exec.Cmd { return fakeexec.InitFakeCmd(&fcmd, cmd, args...) }, // The second Command() call is checking the rule. Failure of that means create it. 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...) }, @@ -317,10 +279,10 @@ func TestEnsureRuleNew(t *testing.T) { if exists { t.Errorf("expected exists = false") } - if fcmd.CombinedOutputCalls != 4 { - t.Errorf("expected 4 CombinedOutput() calls, got %d", fcmd.CombinedOutputCalls) + if fcmd.CombinedOutputCalls != 3 { + t.Errorf("expected 3 CombinedOutput() calls, got %d", fcmd.CombinedOutputCalls) } - if !sets.NewString(fcmd.CombinedOutputLog[3]...).HasAll("iptables", "-t", "nat", "-A", "OUTPUT", "abc", "123") { + if !sets.NewString(fcmd.CombinedOutputLog[2]...).HasAll("iptables", "-t", "nat", "-A", "OUTPUT", "abc", "123") { t.Errorf("wrong CombinedOutput() log, got %s", fcmd.CombinedOutputLog[3]) } } @@ -330,8 +292,6 @@ func TestEnsureRuleErrorChecking(t *testing.T) { CombinedOutputScript: []fakeexec.FakeCombinedOutputAction{ // iptables version check func() ([]byte, error) { return []byte("iptables v1.9.22"), nil }, - // iptables-restore version check - func() ([]byte, error) { return []byte("iptables-restore v1.9.22"), nil }, // Status 2 on the first call. func() ([]byte, error) { return nil, &fakeexec.FakeExitError{Status: 2} }, }, @@ -340,10 +300,39 @@ func TestEnsureRuleErrorChecking(t *testing.T) { CommandScript: []fakeexec.FakeCommandAction{ // iptables version check func(cmd string, args ...string) exec.Cmd { return fakeexec.InitFakeCmd(&fcmd, cmd, args...) }, - // iptables-restore version check + // The second Command() call is checking the rule. Failure of that means create it. + func(cmd string, args ...string) exec.Cmd { return fakeexec.InitFakeCmd(&fcmd, cmd, args...) }, + }, + } + runner := New(&fexec, dbus.NewFake(nil, nil), ProtocolIpv4) + defer runner.Destroy() + _, err := runner.EnsureRule(Append, TableNAT, ChainOutput, "abc", "123") + if err == nil { + t.Errorf("expected failure") + } + if fcmd.CombinedOutputCalls != 2 { + t.Errorf("expected 2 CombinedOutput() calls, got %d", fcmd.CombinedOutputCalls) + } +} + +func TestEnsureRuleErrorCreating(t *testing.T) { + fcmd := fakeexec.FakeCmd{ + CombinedOutputScript: []fakeexec.FakeCombinedOutputAction{ + // iptables version check + func() ([]byte, error) { return []byte("iptables v1.9.22"), nil }, + // Status 1 on the first call. + func() ([]byte, error) { return nil, &fakeexec.FakeExitError{Status: 1} }, + // Status 1 on the second call. + func() ([]byte, error) { return nil, &fakeexec.FakeExitError{Status: 1} }, + }, + } + fexec := fakeexec.FakeExec{ + CommandScript: []fakeexec.FakeCommandAction{ + // iptables version check func(cmd string, args ...string) exec.Cmd { return fakeexec.InitFakeCmd(&fcmd, cmd, args...) }, // The second Command() call is checking the rule. Failure of that means create it. 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, dbus.NewFake(nil, nil), ProtocolIpv4) @@ -357,48 +346,11 @@ func TestEnsureRuleErrorChecking(t *testing.T) { } } -func TestEnsureRuleErrorCreating(t *testing.T) { - fcmd := fakeexec.FakeCmd{ - CombinedOutputScript: []fakeexec.FakeCombinedOutputAction{ - // iptables version check - func() ([]byte, error) { return []byte("iptables v1.9.22"), nil }, - // iptables-restore version check - func() ([]byte, error) { return []byte("iptables-restore v1.9.22"), nil }, - // Status 1 on the first call. - func() ([]byte, error) { return nil, &fakeexec.FakeExitError{Status: 1} }, - // Status 1 on the second call. - func() ([]byte, error) { return nil, &fakeexec.FakeExitError{Status: 1} }, - }, - } - fexec := fakeexec.FakeExec{ - CommandScript: []fakeexec.FakeCommandAction{ - // iptables version check - func(cmd string, args ...string) exec.Cmd { return fakeexec.InitFakeCmd(&fcmd, cmd, args...) }, - // iptables-restore version check - func(cmd string, args ...string) exec.Cmd { return fakeexec.InitFakeCmd(&fcmd, cmd, args...) }, - // The second Command() call is checking the rule. Failure of that means create it. - 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, dbus.NewFake(nil, nil), ProtocolIpv4) - defer runner.Destroy() - _, err := runner.EnsureRule(Append, TableNAT, ChainOutput, "abc", "123") - if err == nil { - t.Errorf("expected failure") - } - if fcmd.CombinedOutputCalls != 4 { - t.Errorf("expected 4 CombinedOutput() calls, got %d", fcmd.CombinedOutputCalls) - } -} - func TestDeleteRuleDoesNotExist(t *testing.T) { fcmd := fakeexec.FakeCmd{ CombinedOutputScript: []fakeexec.FakeCombinedOutputAction{ // iptables version check func() ([]byte, error) { return []byte("iptables v1.9.22"), nil }, - // iptables-restore version check - func() ([]byte, error) { return []byte("iptables-restore v1.9.22"), nil }, // Status 1 on the first call. func() ([]byte, error) { return nil, &fakeexec.FakeExitError{Status: 1} }, }, @@ -407,8 +359,6 @@ func TestDeleteRuleDoesNotExist(t *testing.T) { CommandScript: []fakeexec.FakeCommandAction{ // iptables version check func(cmd string, args ...string) exec.Cmd { return fakeexec.InitFakeCmd(&fcmd, cmd, args...) }, - // iptables-restore version check - func(cmd string, args ...string) exec.Cmd { return fakeexec.InitFakeCmd(&fcmd, cmd, args...) }, // The second Command() call is checking the rule. Failure of that exec means "does not exist". func(cmd string, args ...string) exec.Cmd { return fakeexec.InitFakeCmd(&fcmd, cmd, args...) }, }, @@ -419,10 +369,10 @@ func TestDeleteRuleDoesNotExist(t *testing.T) { 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 fcmd.CombinedOutputCalls != 2 { + t.Errorf("expected 2 CombinedOutput() calls, got %d", fcmd.CombinedOutputCalls) } - if !sets.NewString(fcmd.CombinedOutputLog[2]...).HasAll("iptables", "-t", "nat", "-C", "OUTPUT", "abc", "123") { + if !sets.NewString(fcmd.CombinedOutputLog[1]...).HasAll("iptables", "-t", "nat", "-C", "OUTPUT", "abc", "123") { t.Errorf("wrong CombinedOutput() log, got %s", fcmd.CombinedOutputLog[2]) } } @@ -432,8 +382,6 @@ func TestDeleteRuleExists(t *testing.T) { CombinedOutputScript: []fakeexec.FakeCombinedOutputAction{ // iptables version check func() ([]byte, error) { return []byte("iptables v1.9.22"), nil }, - // iptables-restore version check - func() ([]byte, error) { return []byte("iptables-restore v1.9.22"), nil }, // Success on the first call. func() ([]byte, error) { return []byte{}, nil }, // Success on the second call. @@ -444,8 +392,6 @@ func TestDeleteRuleExists(t *testing.T) { CommandScript: []fakeexec.FakeCommandAction{ // iptables version check func(cmd string, args ...string) exec.Cmd { return fakeexec.InitFakeCmd(&fcmd, cmd, args...) }, - // iptables-restore version check - func(cmd string, args ...string) exec.Cmd { return fakeexec.InitFakeCmd(&fcmd, cmd, args...) }, // The second Command() call is checking the rule. Success of that means delete it. 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...) }, @@ -457,10 +403,10 @@ func TestDeleteRuleExists(t *testing.T) { if err != nil { t.Errorf("expected success, got %v", err) } - if fcmd.CombinedOutputCalls != 4 { - t.Errorf("expected 4 CombinedOutput() calls, got %d", fcmd.CombinedOutputCalls) + if fcmd.CombinedOutputCalls != 3 { + t.Errorf("expected 3 CombinedOutput() calls, got %d", fcmd.CombinedOutputCalls) } - if !sets.NewString(fcmd.CombinedOutputLog[3]...).HasAll("iptables", "-t", "nat", "-D", "OUTPUT", "abc", "123") { + if !sets.NewString(fcmd.CombinedOutputLog[2]...).HasAll("iptables", "-t", "nat", "-D", "OUTPUT", "abc", "123") { t.Errorf("wrong CombinedOutput() log, got %s", fcmd.CombinedOutputLog[3]) } } @@ -470,8 +416,6 @@ func TestDeleteRuleErrorChecking(t *testing.T) { CombinedOutputScript: []fakeexec.FakeCombinedOutputAction{ // iptables version check func() ([]byte, error) { return []byte("iptables v1.9.22"), nil }, - // iptables-restore version check - func() ([]byte, error) { return []byte("iptables-restore v1.9.22"), nil }, // Status 2 on the first call. func() ([]byte, error) { return nil, &fakeexec.FakeExitError{Status: 2} }, }, @@ -480,8 +424,6 @@ func TestDeleteRuleErrorChecking(t *testing.T) { CommandScript: []fakeexec.FakeCommandAction{ // iptables version check func(cmd string, args ...string) exec.Cmd { return fakeexec.InitFakeCmd(&fcmd, cmd, args...) }, - // iptables-restore version check - func(cmd string, args ...string) exec.Cmd { return fakeexec.InitFakeCmd(&fcmd, cmd, args...) }, // The second Command() call is checking the rule. Failure of that means create it. func(cmd string, args ...string) exec.Cmd { return fakeexec.InitFakeCmd(&fcmd, cmd, args...) }, }, @@ -492,57 +434,52 @@ func TestDeleteRuleErrorChecking(t *testing.T) { if err == nil { t.Errorf("expected failure") } + if fcmd.CombinedOutputCalls != 2 { + t.Errorf("expected 2 CombinedOutput() calls, got %d", fcmd.CombinedOutputCalls) + } +} + +func TestDeleteRuleErrorDeleting(t *testing.T) { + fcmd := fakeexec.FakeCmd{ + CombinedOutputScript: []fakeexec.FakeCombinedOutputAction{ + // iptables version check + func() ([]byte, error) { return []byte("iptables v1.9.22"), nil }, + // Success on the first call. + func() ([]byte, error) { return []byte{}, nil }, + // Status 1 on the second call. + func() ([]byte, error) { return nil, &fakeexec.FakeExitError{Status: 1} }, + }, + } + fexec := fakeexec.FakeExec{ + CommandScript: []fakeexec.FakeCommandAction{ + // iptables version check + func(cmd string, args ...string) exec.Cmd { return fakeexec.InitFakeCmd(&fcmd, cmd, args...) }, + // The second Command() call is checking the rule. Success of that means delete it. + 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, dbus.NewFake(nil, nil), ProtocolIpv4) + defer runner.Destroy() + err := runner.DeleteRule(TableNAT, ChainOutput, "abc", "123") + if err == nil { + t.Errorf("expected failure") + } if fcmd.CombinedOutputCalls != 3 { t.Errorf("expected 3 CombinedOutput() calls, got %d", fcmd.CombinedOutputCalls) } } -func TestDeleteRuleErrorDeleting(t *testing.T) { - fcmd := fakeexec.FakeCmd{ - CombinedOutputScript: []fakeexec.FakeCombinedOutputAction{ - // iptables version check - func() ([]byte, error) { return []byte("iptables v1.9.22"), nil }, - // iptables-restore version check - func() ([]byte, error) { return []byte("iptables-restore v1.9.22"), nil }, - // Success on the first call. - func() ([]byte, error) { return []byte{}, nil }, - // Status 1 on the second call. - func() ([]byte, error) { return nil, &fakeexec.FakeExitError{Status: 1} }, - }, - } - fexec := fakeexec.FakeExec{ - CommandScript: []fakeexec.FakeCommandAction{ - // iptables version check - func(cmd string, args ...string) exec.Cmd { return fakeexec.InitFakeCmd(&fcmd, cmd, args...) }, - // iptables-restore version check - func(cmd string, args ...string) exec.Cmd { return fakeexec.InitFakeCmd(&fcmd, cmd, args...) }, - // The second Command() call is checking the rule. Success of that means delete it. - 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, dbus.NewFake(nil, nil), ProtocolIpv4) - defer runner.Destroy() - err := runner.DeleteRule(TableNAT, ChainOutput, "abc", "123") - if err == nil { - t.Errorf("expected failure") - } - if fcmd.CombinedOutputCalls != 4 { - t.Errorf("expected 4 CombinedOutput() calls, got %d", fcmd.CombinedOutputCalls) - } -} - func TestGetIPTablesHasCheckCommand(t *testing.T) { testCases := []struct { Version string - Err bool Expected bool }{ - {"iptables v1.4.7", false, false}, - {"iptables v1.4.11", false, true}, - {"iptables v1.4.19.1", false, true}, - {"iptables v2.0.0", false, true}, - {"total junk", true, false}, + {"iptables v1.4.7", false}, + {"iptables v1.4.11", true}, + {"iptables v1.4.19.1", true}, + {"iptables v2.0.0", true}, + {"total junk", true}, } for _, testCase := range testCases { @@ -556,15 +493,10 @@ func TestGetIPTablesHasCheckCommand(t *testing.T) { func(cmd string, args ...string) exec.Cmd { return fakeexec.InitFakeCmd(&fcmd, cmd, args...) }, }, } - version, err := getIPTablesVersionString(&fexec, ProtocolIpv4) - if (err != nil) != testCase.Err { - t.Errorf("Expected error: %v, Got error: %v", testCase.Err, err) - } - if err == nil { - check := getIPTablesHasCheckCommand(version) - if testCase.Expected != check { - t.Errorf("Expected result: %v, Got result: %v", testCase.Expected, check) - } + ipt := New(&fexec, nil, ProtocolIpv4) + runner := ipt.(*runner) + if testCase.Expected != runner.hasCheck { + t.Errorf("Expected result: %v, Got result: %v", testCase.Expected, runner.hasCheck) } } } @@ -695,7 +627,7 @@ func TestIPTablesWaitFlag(t *testing.T) { } for _, testCase := range testCases { - result := getIPTablesWaitFlag(testCase.Version) + result := getIPTablesWaitFlag(utilversion.MustParseGeneric(testCase.Version)) if !reflect.DeepEqual(result, testCase.Result) { t.Errorf("For %s expected %v got %v", testCase.Version, testCase.Result, result) } @@ -707,8 +639,6 @@ func TestWaitFlagUnavailable(t *testing.T) { CombinedOutputScript: []fakeexec.FakeCombinedOutputAction{ // iptables version check func() ([]byte, error) { return []byte("iptables v1.4.19"), nil }, - // iptables-restore version check - func() ([]byte, error) { return []byte("iptables-restore v1.9.22"), nil }, // Success. func() ([]byte, error) { return []byte{}, nil }, }, @@ -717,8 +647,6 @@ func TestWaitFlagUnavailable(t *testing.T) { CommandScript: []fakeexec.FakeCommandAction{ // iptables version check func(cmd string, args ...string) exec.Cmd { return fakeexec.InitFakeCmd(&fcmd, cmd, args...) }, - // iptables-restore version check - 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...) }, }, } @@ -728,10 +656,10 @@ func TestWaitFlagUnavailable(t *testing.T) { 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 fcmd.CombinedOutputCalls != 2 { + t.Errorf("expected 2 CombinedOutput() calls, got %d", fcmd.CombinedOutputCalls) } - if sets.NewString(fcmd.CombinedOutputLog[2]...).Has(WaitString) { + if sets.NewString(fcmd.CombinedOutputLog[1]...).Has(WaitString) { t.Errorf("wrong CombinedOutput() log, got %s", fcmd.CombinedOutputLog[2]) } } @@ -741,8 +669,6 @@ func TestWaitFlagOld(t *testing.T) { CombinedOutputScript: []fakeexec.FakeCombinedOutputAction{ // iptables version check func() ([]byte, error) { return []byte("iptables v1.4.20"), nil }, - // iptables-restore version check - func() ([]byte, error) { return []byte("iptables-restore v1.9.22"), nil }, // Success. func() ([]byte, error) { return []byte{}, nil }, }, @@ -751,7 +677,6 @@ func TestWaitFlagOld(t *testing.T) { 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, dbus.NewFake(nil, nil), ProtocolIpv4) @@ -760,14 +685,14 @@ func TestWaitFlagOld(t *testing.T) { 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 fcmd.CombinedOutputCalls != 2 { + t.Errorf("expected 2 CombinedOutput() calls, got %d", fcmd.CombinedOutputCalls) } - if !sets.NewString(fcmd.CombinedOutputLog[2]...).HasAll("iptables", WaitString) { - t.Errorf("wrong CombinedOutput() log, got %s", fcmd.CombinedOutputLog[2]) + if !sets.NewString(fcmd.CombinedOutputLog[1]...).HasAll("iptables", WaitString) { + t.Errorf("wrong CombinedOutput() log, got %s", fcmd.CombinedOutputLog[1]) } - if sets.NewString(fcmd.CombinedOutputLog[2]...).Has(WaitSecondsValue) { - t.Errorf("wrong CombinedOutput() log, got %s", fcmd.CombinedOutputLog[2]) + if sets.NewString(fcmd.CombinedOutputLog[1]...).Has(WaitSecondsValue) { + t.Errorf("wrong CombinedOutput() log, got %s", fcmd.CombinedOutputLog[1]) } } @@ -776,8 +701,6 @@ func TestWaitFlagNew(t *testing.T) { CombinedOutputScript: []fakeexec.FakeCombinedOutputAction{ // iptables version check func() ([]byte, error) { return []byte("iptables v1.4.22"), nil }, - // iptables-restore version check - func() ([]byte, error) { return []byte("iptables-restore v1.9.22"), nil }, // Success. func() ([]byte, error) { return []byte{}, nil }, }, @@ -786,7 +709,6 @@ func TestWaitFlagNew(t *testing.T) { 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, dbus.NewFake(nil, nil), ProtocolIpv4) @@ -795,11 +717,11 @@ func TestWaitFlagNew(t *testing.T) { 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 fcmd.CombinedOutputCalls != 2 { + t.Errorf("expected 2 CombinedOutput() calls, got %d", fcmd.CombinedOutputCalls) } - if !sets.NewString(fcmd.CombinedOutputLog[2]...).HasAll("iptables", WaitString, WaitSecondsValue) { - t.Errorf("wrong CombinedOutput() log, got %s", fcmd.CombinedOutputLog[2]) + if !sets.NewString(fcmd.CombinedOutputLog[1]...).HasAll("iptables", WaitString, WaitSecondsValue) { + t.Errorf("wrong CombinedOutput() log, got %s", fcmd.CombinedOutputLog[1]) } } @@ -815,8 +737,6 @@ func TestReload(t *testing.T) { CombinedOutputScript: []fakeexec.FakeCombinedOutputAction{ // iptables version check func() ([]byte, error) { return []byte("iptables v1.4.22"), nil }, - // iptables-restore version check - func() ([]byte, error) { return []byte("iptables-restore v1.9.22"), nil }, // first reload // EnsureChain @@ -844,7 +764,6 @@ func TestReload(t *testing.T) { 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...) }, - func(cmd string, args ...string) exec.Cmd { return fakeexec.InitFakeCmd(&fcmd, cmd, args...) }, }, } @@ -877,16 +796,16 @@ func TestReload(t *testing.T) { <-reloaded <-reloaded - if fcmd.CombinedOutputCalls != 5 { - t.Errorf("expected 5 CombinedOutput() calls total, got %d", fcmd.CombinedOutputCalls) + if fcmd.CombinedOutputCalls != 4 { + t.Errorf("expected 4 CombinedOutput() calls total, got %d", fcmd.CombinedOutputCalls) } - if !sets.NewString(fcmd.CombinedOutputLog[2]...).HasAll("iptables", "-t", "nat", "-N", "FOOBAR") { + if !sets.NewString(fcmd.CombinedOutputLog[1]...).HasAll("iptables", "-t", "nat", "-N", "FOOBAR") { t.Errorf("wrong CombinedOutput() log, got %s", fcmd.CombinedOutputLog[2]) } - if !sets.NewString(fcmd.CombinedOutputLog[3]...).HasAll("iptables", "-t", "nat", "-C", "OUTPUT", "abc", "123") { + if !sets.NewString(fcmd.CombinedOutputLog[2]...).HasAll("iptables", "-t", "nat", "-C", "OUTPUT", "abc", "123") { t.Errorf("wrong CombinedOutput() log, got %s", fcmd.CombinedOutputLog[3]) } - if !sets.NewString(fcmd.CombinedOutputLog[4]...).HasAll("iptables", "-t", "nat", "-A", "OUTPUT", "abc", "123") { + if !sets.NewString(fcmd.CombinedOutputLog[3]...).HasAll("iptables", "-t", "nat", "-A", "OUTPUT", "abc", "123") { t.Errorf("wrong CombinedOutput() log, got %s", fcmd.CombinedOutputLog[4]) } @@ -895,7 +814,7 @@ func TestReload(t *testing.T) { dbusConn.EmitSignal("org.freedesktop.DBus", "/org/freedesktop/DBus", "org.freedesktop.DBus", "NameOwnerChanged", "io.k8s.Something", "", ":1.1") <-reloaded - if fcmd.CombinedOutputCalls != 5 { + if fcmd.CombinedOutputCalls != 4 { t.Errorf("Incorrect signal caused a reload") } @@ -903,16 +822,16 @@ func TestReload(t *testing.T) { <-reloaded <-reloaded - if fcmd.CombinedOutputCalls != 8 { - t.Errorf("expected 8 CombinedOutput() calls total, got %d", fcmd.CombinedOutputCalls) + if fcmd.CombinedOutputCalls != 7 { + t.Errorf("expected 7 CombinedOutput() calls total, got %d", fcmd.CombinedOutputCalls) } - if !sets.NewString(fcmd.CombinedOutputLog[5]...).HasAll("iptables", "-t", "nat", "-N", "FOOBAR") { + if !sets.NewString(fcmd.CombinedOutputLog[4]...).HasAll("iptables", "-t", "nat", "-N", "FOOBAR") { t.Errorf("wrong CombinedOutput() log, got %s", fcmd.CombinedOutputLog[5]) } - if !sets.NewString(fcmd.CombinedOutputLog[6]...).HasAll("iptables", "-t", "nat", "-C", "OUTPUT", "abc", "123") { + if !sets.NewString(fcmd.CombinedOutputLog[5]...).HasAll("iptables", "-t", "nat", "-C", "OUTPUT", "abc", "123") { t.Errorf("wrong CombinedOutput() log, got %s", fcmd.CombinedOutputLog[6]) } - if !sets.NewString(fcmd.CombinedOutputLog[7]...).HasAll("iptables", "-t", "nat", "-A", "OUTPUT", "abc", "123") { + if !sets.NewString(fcmd.CombinedOutputLog[6]...).HasAll("iptables", "-t", "nat", "-A", "OUTPUT", "abc", "123") { t.Errorf("wrong CombinedOutput() log, got %s", fcmd.CombinedOutputLog[7]) } } @@ -921,7 +840,6 @@ func testSaveInto(t *testing.T, protocol Protocol) { version := " v1.9.22" iptablesCmd := iptablesCommand(protocol) iptablesSaveCmd := iptablesSaveCommand(protocol) - iptablesRestoreCmd := iptablesRestoreCommand(protocol) protoStr := protocolStr(protocol) output := fmt.Sprintf(`# Generated by %s on Thu Jan 19 11:38:09 2017 @@ -938,8 +856,6 @@ COMMIT CombinedOutputScript: []fakeexec.FakeCombinedOutputAction{ // iptables version check func() ([]byte, error) { return []byte(iptablesCmd + version), nil }, - // iptables-restore version check - func() ([]byte, error) { return []byte(iptablesRestoreCmd + version), nil }, }, RunScript: []fakeexec.FakeRunAction{ func() ([]byte, []byte, error) { return []byte(output), []byte(stderrOutput), nil }, @@ -951,7 +867,6 @@ COMMIT 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...) }, - func(cmd string, args ...string) exec.Cmd { return fakeexec.InitFakeCmd(&fcmd, cmd, args...) }, }, } runner := New(&fexec, dbus.NewFake(nil, nil), protocol) @@ -968,8 +883,8 @@ COMMIT t.Errorf("%s: Expected output '%s', got '%v'", protoStr, output, string(buffer.Bytes())) } - if fcmd.CombinedOutputCalls != 2 { - t.Errorf("%s: Expected 2 CombinedOutput() calls, got %d", protoStr, fcmd.CombinedOutputCalls) + if fcmd.CombinedOutputCalls != 1 { + t.Errorf("%s: Expected 1 CombinedOutput() calls, got %d", protoStr, fcmd.CombinedOutputCalls) } if fcmd.RunCalls != 1 { t.Errorf("%s: Expected 1 Run() call, got %d", protoStr, fcmd.RunCalls) @@ -1007,8 +922,6 @@ func testRestore(t *testing.T, protocol Protocol) { CombinedOutputScript: []fakeexec.FakeCombinedOutputAction{ // iptables version check func() ([]byte, error) { return []byte(iptablesCmd + version), nil }, - // iptables-restore version check - func() ([]byte, error) { return []byte(iptablesRestoreCmd + version), nil }, func() ([]byte, error) { return []byte{}, nil }, func() ([]byte, error) { return []byte{}, nil }, func() ([]byte, error) { return []byte{}, nil }, @@ -1024,7 +937,6 @@ func testRestore(t *testing.T, protocol Protocol) { 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...) }, - func(cmd string, args ...string) exec.Cmd { return fakeexec.InitFakeCmd(&fcmd, cmd, args...) }, }, } runner := New(&fexec, dbus.NewFake(nil, nil), protocol) @@ -1036,9 +948,9 @@ func testRestore(t *testing.T, protocol Protocol) { t.Errorf("%s flush,restore: Expected success, got %v", protoStr, err) } - commandSet := sets.NewString(fcmd.CombinedOutputLog[2]...) + commandSet := sets.NewString(fcmd.CombinedOutputLog[1]...) if !commandSet.HasAll(iptablesRestoreCmd, "-T", string(TableNAT), "--counters") || commandSet.HasAny("--noflush") { - t.Errorf("%s flush, restore: Expected cmd containing '%s -T %s --counters', got '%s'", protoStr, iptablesRestoreCmd, string(TableNAT), fcmd.CombinedOutputLog[2]) + t.Errorf("%s flush, restore: Expected cmd containing '%s -T %s --counters', got '%s'", protoStr, iptablesRestoreCmd, string(TableNAT), fcmd.CombinedOutputLog[1]) } // FlushTables, NoRestoreCounters @@ -1047,9 +959,9 @@ func testRestore(t *testing.T, protocol Protocol) { t.Errorf("%s flush, no restore: Expected success, got %v", protoStr, err) } - commandSet = sets.NewString(fcmd.CombinedOutputLog[3]...) + commandSet = sets.NewString(fcmd.CombinedOutputLog[2]...) if !commandSet.HasAll(iptablesRestoreCmd, "-T", string(TableNAT)) || commandSet.HasAny("--noflush", "--counters") { - t.Errorf("%s flush, no restore: Expected cmd containing '--noflush' or '--counters', got '%s'", protoStr, fcmd.CombinedOutputLog[3]) + t.Errorf("%s flush, no restore: Expected cmd containing '--noflush' or '--counters', got '%s'", protoStr, fcmd.CombinedOutputLog[2]) } // NoFlushTables, RestoreCounters @@ -1058,9 +970,9 @@ func testRestore(t *testing.T, protocol Protocol) { t.Errorf("%s no flush, restore: Expected success, got %v", protoStr, err) } - commandSet = sets.NewString(fcmd.CombinedOutputLog[4]...) + commandSet = sets.NewString(fcmd.CombinedOutputLog[3]...) if !commandSet.HasAll(iptablesRestoreCmd, "-T", string(TableNAT), "--noflush", "--counters") { - t.Errorf("%s no flush, restore: Expected cmd containing '--noflush' and '--counters', got '%s'", protoStr, fcmd.CombinedOutputLog[4]) + t.Errorf("%s no flush, restore: Expected cmd containing '--noflush' and '--counters', got '%s'", protoStr, fcmd.CombinedOutputLog[3]) } // NoFlushTables, NoRestoreCounters @@ -1069,13 +981,13 @@ func testRestore(t *testing.T, protocol Protocol) { t.Errorf("%s no flush, no restore: Expected success, got %v", protoStr, err) } - commandSet = sets.NewString(fcmd.CombinedOutputLog[5]...) + commandSet = sets.NewString(fcmd.CombinedOutputLog[4]...) if !commandSet.HasAll(iptablesRestoreCmd, "-T", string(TableNAT), "--noflush") || commandSet.HasAny("--counters") { - t.Errorf("%s no flush, no restore: Expected cmd containing '%s -T %s --noflush', got '%s'", protoStr, iptablesRestoreCmd, string(TableNAT), fcmd.CombinedOutputLog[5]) + t.Errorf("%s no flush, no restore: Expected cmd containing '%s -T %s --noflush', got '%s'", protoStr, iptablesRestoreCmd, string(TableNAT), fcmd.CombinedOutputLog[4]) } - if fcmd.CombinedOutputCalls != 6 { - t.Errorf("%s: Expected 6 total CombinedOutput() calls, got %d", protoStr, fcmd.CombinedOutputCalls) + if fcmd.CombinedOutputCalls != 5 { + t.Errorf("%s: Expected 5 total CombinedOutput() calls, got %d", protoStr, fcmd.CombinedOutputCalls) } // Failure. @@ -1099,8 +1011,6 @@ func TestRestoreAll(t *testing.T) { CombinedOutputScript: []fakeexec.FakeCombinedOutputAction{ // iptables version check func() ([]byte, error) { return []byte("iptables v1.9.22"), nil }, - // iptables-restore version check - func() ([]byte, error) { return []byte("iptables-restore v1.9.22"), nil }, func() ([]byte, error) { return []byte{}, nil }, func() ([]byte, error) { return nil, &fakeexec.FakeExitError{Status: 1} }, }, @@ -1110,7 +1020,6 @@ func TestRestoreAll(t *testing.T) { 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...) }, - func(cmd string, args ...string) exec.Cmd { return fakeexec.InitFakeCmd(&fcmd, cmd, args...) }, }, } runner := newInternal(&fexec, dbus.NewFake(nil, nil), ProtocolIpv4, TestLockfilePath) @@ -1122,13 +1031,13 @@ func TestRestoreAll(t *testing.T) { t.Fatalf("expected success, got %v", err) } - commandSet := sets.NewString(fcmd.CombinedOutputLog[2]...) + commandSet := sets.NewString(fcmd.CombinedOutputLog[1]...) if !commandSet.HasAll("iptables-restore", "--counters", "--noflush") { t.Errorf("wrong CombinedOutput() log, got %s", fcmd.CombinedOutputLog[2]) } - if fcmd.CombinedOutputCalls != 3 { - t.Errorf("expected 3 CombinedOutput() calls, got %d", fcmd.CombinedOutputCalls) + if fcmd.CombinedOutputCalls != 2 { + t.Errorf("expected 2 CombinedOutput() calls, got %d", fcmd.CombinedOutputCalls) } // Failure. @@ -1144,8 +1053,6 @@ func TestRestoreAllWait(t *testing.T) { CombinedOutputScript: []fakeexec.FakeCombinedOutputAction{ // iptables version check func() ([]byte, error) { return []byte("iptables v1.9.22"), nil }, - // iptables-restore version check - func() ([]byte, error) { return []byte("iptables-restore v1.9.22"), nil }, func() ([]byte, error) { return []byte{}, nil }, func() ([]byte, error) { return nil, &fakeexec.FakeExitError{Status: 1} }, }, @@ -1155,7 +1062,6 @@ func TestRestoreAllWait(t *testing.T) { 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...) }, - func(cmd string, args ...string) exec.Cmd { return fakeexec.InitFakeCmd(&fcmd, cmd, args...) }, }, } runner := newInternal(&fexec, dbus.NewFake(nil, nil), ProtocolIpv4, TestLockfilePath) @@ -1167,13 +1073,13 @@ func TestRestoreAllWait(t *testing.T) { t.Fatalf("expected success, got %v", err) } - commandSet := sets.NewString(fcmd.CombinedOutputLog[2]...) + commandSet := sets.NewString(fcmd.CombinedOutputLog[1]...) if !commandSet.HasAll("iptables-restore", WaitString, WaitSecondsValue, "--counters", "--noflush") { - t.Errorf("wrong CombinedOutput() log, got %s", fcmd.CombinedOutputLog[2]) + t.Errorf("wrong CombinedOutput() log, got %s", fcmd.CombinedOutputLog[1]) } - if fcmd.CombinedOutputCalls != 3 { - t.Errorf("expected 3 CombinedOutput() calls, got %d", fcmd.CombinedOutputCalls) + if fcmd.CombinedOutputCalls != 2 { + t.Errorf("expected 2 CombinedOutput() calls, got %d", fcmd.CombinedOutputCalls) } // Failure. @@ -1184,14 +1090,12 @@ func TestRestoreAllWait(t *testing.T) { } // TestRestoreAllWaitOldIptablesRestore tests that the "wait" flag is not passed -// to a in-compatible iptables-restore +// to an old iptables-restore func TestRestoreAllWaitOldIptablesRestore(t *testing.T) { fcmd := fakeexec.FakeCmd{ CombinedOutputScript: []fakeexec.FakeCombinedOutputAction{ // iptables version check - func() ([]byte, error) { return []byte("iptables v1.9.22"), nil }, - // iptables-restore version check - func() ([]byte, error) { return []byte("unrecognized option: --version"), nil }, + func() ([]byte, error) { return []byte("iptables v1.4.22"), nil }, func() ([]byte, error) { return []byte{}, nil }, func() ([]byte, error) { return nil, &fakeexec.FakeExitError{Status: 1} }, }, @@ -1201,7 +1105,6 @@ func TestRestoreAllWaitOldIptablesRestore(t *testing.T) { 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...) }, - func(cmd string, args ...string) exec.Cmd { return fakeexec.InitFakeCmd(&fcmd, cmd, args...) }, }, } runner := newInternal(&fexec, dbus.NewFake(nil, nil), ProtocolIpv4, TestLockfilePath) @@ -1213,16 +1116,16 @@ func TestRestoreAllWaitOldIptablesRestore(t *testing.T) { t.Fatalf("expected success, got %v", err) } - commandSet := sets.NewString(fcmd.CombinedOutputLog[2]...) + commandSet := sets.NewString(fcmd.CombinedOutputLog[1]...) if !commandSet.HasAll("iptables-restore", "--counters", "--noflush") { t.Errorf("wrong CombinedOutput() log, got %s", fcmd.CombinedOutputLog[2]) } if commandSet.HasAll(WaitString, WaitSecondsValue) { - t.Errorf("wrong CombinedOutput() log (unexpected %s option), got %s", WaitString, fcmd.CombinedOutputLog[2]) + t.Errorf("wrong CombinedOutput() log (unexpected %s option), got %s", WaitString, fcmd.CombinedOutputLog[1]) } - if fcmd.CombinedOutputCalls != 3 { - t.Errorf("expected 3 CombinedOutput() calls, got %d", fcmd.CombinedOutputCalls) + if fcmd.CombinedOutputCalls != 2 { + t.Errorf("expected 2 CombinedOutput() calls, got %d", fcmd.CombinedOutputCalls) } // Failure. @@ -1239,15 +1142,12 @@ func TestRestoreAllGrabNewLock(t *testing.T) { fcmd := fakeexec.FakeCmd{ CombinedOutputScript: []fakeexec.FakeCombinedOutputAction{ // iptables version check - func() ([]byte, error) { return []byte("iptables v1.9.22"), nil }, - // iptables-restore version check - func() ([]byte, error) { return []byte("unrecognized option: --version"), nil }, + func() ([]byte, error) { return []byte("iptables v1.4.22"), 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...) }, }, } @@ -1282,15 +1182,12 @@ func TestRestoreAllGrabOldLock(t *testing.T) { fcmd := fakeexec.FakeCmd{ CombinedOutputScript: []fakeexec.FakeCombinedOutputAction{ // iptables version check - func() ([]byte, error) { return []byte("iptables v1.9.22"), nil }, - // iptables-restore version check - func() ([]byte, error) { return []byte("unrecognized option: --version"), nil }, + func() ([]byte, error) { return []byte("iptables v1.4.22"), 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...) }, }, } diff --git a/pkg/util/iptables/testing/fake.go b/pkg/util/iptables/testing/fake.go index 66adc1a275e..6470eb86289 100644 --- a/pkg/util/iptables/testing/fake.go +++ b/pkg/util/iptables/testing/fake.go @@ -48,10 +48,6 @@ func NewFake() *FakeIPTables { return &FakeIPTables{} } -func (*FakeIPTables) GetVersion() (string, error) { - return "0.0.0", nil -} - func (*FakeIPTables) EnsureChain(table iptables.Table, chain iptables.Chain) (bool, error) { return true, nil }