diff --git a/cmd/kube-proxy/app/server_linux.go b/cmd/kube-proxy/app/server_linux.go index 19e5170d339..a4769d3817b 100644 --- a/cmd/kube-proxy/app/server_linux.go +++ b/cmd/kube-proxy/app/server_linux.go @@ -50,7 +50,6 @@ import ( "k8s.io/kubernetes/pkg/proxy/nftables" proxyutil "k8s.io/kubernetes/pkg/proxy/util" utiliptables "k8s.io/kubernetes/pkg/util/iptables" - "k8s.io/utils/exec" ) // timeoutForNodePodCIDR is the time to wait for allocators to assign a PodCIDR to the @@ -105,37 +104,15 @@ func isIPTablesBased(mode proxyconfigapi.ProxyMode) bool { return mode == proxyconfigapi.ProxyModeIPTables || mode == proxyconfigapi.ProxyModeIPVS } -// getIPTables returns an array of [IPv4, IPv6] utiliptables.Interfaces. If primaryFamily -// is not v1.IPFamilyUnknown then it will also separately return the interface for just -// that family. -func getIPTables(primaryFamily v1.IPFamily) ([2]utiliptables.Interface, utiliptables.Interface) { - execer := exec.New() - - // Create iptables handlers for both families. Always ordered as IPv4, IPv6 - ipt := [2]utiliptables.Interface{ - utiliptables.New(execer, utiliptables.ProtocolIPv4), - utiliptables.New(execer, utiliptables.ProtocolIPv6), - } - - var iptInterface utiliptables.Interface - if primaryFamily == v1.IPv4Protocol { - iptInterface = ipt[0] - } else if primaryFamily == v1.IPv6Protocol { - iptInterface = ipt[1] - } - - return ipt, iptInterface -} - // platformCheckSupported is called immediately before creating the Proxier, to check // what IP families are supported (and whether the configuration is usable at all). func (s *ProxyServer) platformCheckSupported(ctx context.Context) (ipv4Supported, ipv6Supported, dualStackSupported bool, err error) { logger := klog.FromContext(ctx) if isIPTablesBased(s.Config.Mode) { - ipt, _ := getIPTables(v1.IPFamilyUnknown) - ipv4Supported = ipt[0].Present() - ipv6Supported = ipt[1].Present() + ipts := utiliptables.NewDualStack() + ipv4Supported = ipts[v1.IPv4Protocol] != nil + ipv6Supported = ipts[v1.IPv6Protocol] != nil if !ipv4Supported && !ipv6Supported { err = fmt.Errorf("iptables is not available on this host") @@ -166,14 +143,13 @@ func (s *ProxyServer) createProxier(ctx context.Context, config *proxyconfigapi. if config.Mode == proxyconfigapi.ProxyModeIPTables { logger.Info("Using iptables Proxier") + ipts := utiliptables.NewDualStack() if dualStack { - ipt, _ := getIPTables(s.PrimaryIPFamily) - // TODO this has side effects that should only happen when Run() is invoked. proxier, err = iptables.NewDualStackProxier( ctx, - ipt, + ipts, utilsysctl.New(), config.SyncPeriod.Duration, config.MinSyncPeriod.Duration, @@ -190,13 +166,12 @@ func (s *ProxyServer) createProxier(ctx context.Context, config *proxyconfigapi. ) } else { // Create a single-stack proxier if and only if the node does not support dual-stack (i.e, no iptables support). - _, iptInterface := getIPTables(s.PrimaryIPFamily) // TODO this has side effects that should only happen when Run() is invoked. proxier, err = iptables.NewProxier( ctx, s.PrimaryIPFamily, - iptInterface, + ipts[s.PrimaryIPFamily], utilsysctl.New(), config.SyncPeriod.Duration, config.MinSyncPeriod.Duration, @@ -222,13 +197,13 @@ func (s *ProxyServer) createProxier(ctx context.Context, config *proxyconfigapi. if err := ipvs.CanUseIPVSProxier(ctx, ipvsInterface, ipsetInterface, config.IPVS.Scheduler); err != nil { return nil, fmt.Errorf("can't use the IPVS proxier: %v", err) } + ipts := utiliptables.NewDualStack() logger.Info("Using ipvs Proxier") if dualStack { - ipt, _ := getIPTables(s.PrimaryIPFamily) proxier, err = ipvs.NewDualStackProxier( ctx, - ipt, + ipts, ipvsInterface, ipsetInterface, utilsysctl.New(), @@ -251,11 +226,10 @@ func (s *ProxyServer) createProxier(ctx context.Context, config *proxyconfigapi. initOnly, ) } else { - _, iptInterface := getIPTables(s.PrimaryIPFamily) proxier, err = ipvs.NewProxier( ctx, s.PrimaryIPFamily, - iptInterface, + ipts[s.PrimaryIPFamily], ipvsInterface, ipsetInterface, utilsysctl.New(), @@ -509,7 +483,7 @@ func platformCleanup(ctx context.Context, mode proxyconfigapi.ProxyMode, cleanup // Clean up iptables and ipvs rules if switching to nftables, or if cleanupAndExit if !isIPTablesBased(mode) || cleanupAndExit { - ipts, _ := getIPTables(v1.IPFamilyUnknown) + ipts := utiliptables.NewDualStack() ipsetInterface := utilipset.New() ipvsInterface := utilipvs.New() diff --git a/pkg/kubelet/kubelet_network_linux.go b/pkg/kubelet/kubelet_network_linux.go index fce57689061..32ed9eb2429 100644 --- a/pkg/kubelet/kubelet_network_linux.go +++ b/pkg/kubelet/kubelet_network_linux.go @@ -25,7 +25,6 @@ import ( "k8s.io/apimachinery/pkg/util/wait" "k8s.io/klog/v2" utiliptables "k8s.io/kubernetes/pkg/util/iptables" - utilexec "k8s.io/utils/exec" ) const ( @@ -38,14 +37,14 @@ const ( ) func (kl *Kubelet) initNetworkUtil() { - exec := utilexec.New() - iptClients := []utiliptables.Interface{ - utiliptables.New(exec, utiliptables.ProtocolIPv4), - utiliptables.New(exec, utiliptables.ProtocolIPv6), + iptClients := utiliptables.NewDualStack() + if len(iptClients) == 0 { + klog.InfoS("No iptables support on this system; not creating the KUBE-IPTABLES-HINT chain") + return } - for i := range iptClients { - iptClient := iptClients[i] + for family := range iptClients { + iptClient := iptClients[family] if kl.syncIPTablesRules(iptClient) { klog.InfoS("Initialized iptables rules.", "protocol", iptClient.Protocol()) go iptClient.Monitor( diff --git a/pkg/proxy/iptables/proxier.go b/pkg/proxy/iptables/proxier.go index 80c00cd53a5..85a6adb2e5a 100644 --- a/pkg/proxy/iptables/proxier.go +++ b/pkg/proxy/iptables/proxier.go @@ -94,7 +94,7 @@ const sysctlNFConntrackTCPBeLiberal = "net/netfilter/nf_conntrack_tcp_be_liberal // NewDualStackProxier creates a MetaProxier instance, with IPv4 and IPv6 proxies. func NewDualStackProxier( ctx context.Context, - ipt [2]utiliptables.Interface, + ipts map[v1.IPFamily]utiliptables.Interface, sysctl utilsysctl.Interface, syncPeriod time.Duration, minSyncPeriod time.Duration, @@ -110,7 +110,7 @@ func NewDualStackProxier( initOnly bool, ) (proxy.Provider, error) { // Create an ipv4 instance of the single-stack proxier - ipv4Proxier, err := NewProxier(ctx, v1.IPv4Protocol, ipt[0], sysctl, + ipv4Proxier, err := NewProxier(ctx, v1.IPv4Protocol, ipts[v1.IPv4Protocol], sysctl, syncPeriod, minSyncPeriod, masqueradeAll, localhostNodePorts, masqueradeBit, localDetectors[v1.IPv4Protocol], hostname, nodeIPs[v1.IPv4Protocol], recorder, healthzServer, nodePortAddresses, initOnly) @@ -118,7 +118,7 @@ func NewDualStackProxier( return nil, fmt.Errorf("unable to create ipv4 proxier: %v", err) } - ipv6Proxier, err := NewProxier(ctx, v1.IPv6Protocol, ipt[1], sysctl, + ipv6Proxier, err := NewProxier(ctx, v1.IPv6Protocol, ipts[v1.IPv6Protocol], sysctl, syncPeriod, minSyncPeriod, masqueradeAll, false, masqueradeBit, localDetectors[v1.IPv6Protocol], hostname, nodeIPs[v1.IPv6Protocol], recorder, healthzServer, nodePortAddresses, initOnly) diff --git a/pkg/proxy/ipvs/proxier.go b/pkg/proxy/ipvs/proxier.go index c83a3cad5ee..9d70be2ee7e 100644 --- a/pkg/proxy/ipvs/proxier.go +++ b/pkg/proxy/ipvs/proxier.go @@ -110,7 +110,7 @@ const ( // NewDualStackProxier returns a new Proxier for dual-stack operation func NewDualStackProxier( ctx context.Context, - ipt [2]utiliptables.Interface, + ipts map[v1.IPFamily]utiliptables.Interface, ipvs utilipvs.Interface, ipset utilipset.Interface, sysctl utilsysctl.Interface, @@ -133,7 +133,7 @@ func NewDualStackProxier( initOnly bool, ) (proxy.Provider, error) { // Create an ipv4 instance of the single-stack proxier - ipv4Proxier, err := NewProxier(ctx, v1.IPv4Protocol, ipt[0], ipvs, ipset, sysctl, + ipv4Proxier, err := NewProxier(ctx, v1.IPv4Protocol, ipts[v1.IPv4Protocol], ipvs, ipset, sysctl, syncPeriod, minSyncPeriod, filterCIDRs(false, excludeCIDRs), strictARP, tcpTimeout, tcpFinTimeout, udpTimeout, masqueradeAll, masqueradeBit, localDetectors[v1.IPv4Protocol], hostname, nodeIPs[v1.IPv4Protocol], recorder, @@ -142,7 +142,7 @@ func NewDualStackProxier( return nil, fmt.Errorf("unable to create ipv4 proxier: %v", err) } - ipv6Proxier, err := NewProxier(ctx, v1.IPv6Protocol, ipt[1], ipvs, ipset, sysctl, + ipv6Proxier, err := NewProxier(ctx, v1.IPv6Protocol, ipts[v1.IPv6Protocol], ipvs, ipset, sysctl, syncPeriod, minSyncPeriod, filterCIDRs(true, excludeCIDRs), strictARP, tcpTimeout, tcpFinTimeout, udpTimeout, masqueradeAll, masqueradeBit, localDetectors[v1.IPv6Protocol], hostname, nodeIPs[v1.IPv6Protocol], recorder, diff --git a/pkg/util/iptables/iptables.go b/pkg/util/iptables/iptables.go index c6c64e45f73..d38ab0f9d0e 100644 --- a/pkg/util/iptables/iptables.go +++ b/pkg/util/iptables/iptables.go @@ -30,6 +30,7 @@ import ( "sync" "time" + "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/util/sets" utilversion "k8s.io/apimachinery/pkg/util/version" utilwait "k8s.io/apimachinery/pkg/util/wait" @@ -219,12 +220,6 @@ 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, protocol Protocol, lockfilePath14x, lockfilePath16x string) Interface { - version, err := getIPTablesVersion(exec, protocol) - if err != nil { - klog.InfoS("Error checking iptables version, assuming version at least", "version", MinCheckVersion, "err", err) - version = MinCheckVersion - } - if lockfilePath16x == "" { lockfilePath16x = LockfilePath16x } @@ -235,19 +230,50 @@ func newInternal(exec utilexec.Interface, protocol Protocol, lockfilePath14x, lo runner := &runner{ exec: exec, protocol: protocol, - hasCheck: version.AtLeast(MinCheckVersion), - hasRandomFully: version.AtLeast(RandomFullyMinVersion), - waitFlag: getIPTablesWaitFlag(version), - restoreWaitFlag: getIPTablesRestoreWaitFlag(version, exec, protocol), lockfilePath14x: lockfilePath14x, lockfilePath16x: lockfilePath16x, } + + version, err := getIPTablesVersion(exec, protocol) + if err != nil { + // The only likely error is "no such file or directory", in which case any + // further commands will fail the same way, so we don't need to do + // anything special here. + return runner + } + + runner.hasCheck = version.AtLeast(MinCheckVersion) + runner.hasRandomFully = version.AtLeast(RandomFullyMinVersion) + runner.waitFlag = getIPTablesWaitFlag(version) + runner.restoreWaitFlag = getIPTablesRestoreWaitFlag(version, exec, protocol) return runner } // New returns a new Interface which will exec iptables. -func New(exec utilexec.Interface, protocol Protocol) Interface { - return newInternal(exec, protocol, "", "") +func New(protocol Protocol) Interface { + return newInternal(utilexec.New(), protocol, "", "") +} + +func newDualStackInternal(exec utilexec.Interface) map[v1.IPFamily]Interface { + interfaces := map[v1.IPFamily]Interface{} + + iptv4 := newInternal(exec, ProtocolIPv4, "", "") + if iptv4.Present() { + interfaces[v1.IPv4Protocol] = iptv4 + } + iptv6 := newInternal(exec, ProtocolIPv6, "", "") + if iptv6.Present() { + interfaces[v1.IPv6Protocol] = iptv6 + } + + return interfaces +} + +// NewDualStack returns a map containing an IPv4 Interface (if IPv4 iptables is supported) +// and an IPv6 Interface (if IPv6 iptables is supported). If either family is not +// supported, no Interface will be returned for that family. +func NewDualStack() map[v1.IPFamily]Interface { + return newDualStackInternal(utilexec.New()) } // EnsureChain is part of Interface. diff --git a/pkg/util/iptables/iptables_test.go b/pkg/util/iptables/iptables_test.go index 652b7786431..26ab3a48e41 100644 --- a/pkg/util/iptables/iptables_test.go +++ b/pkg/util/iptables/iptables_test.go @@ -29,6 +29,7 @@ import ( "testing" "time" + v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/util/sets" utilversion "k8s.io/apimachinery/pkg/util/version" "k8s.io/apimachinery/pkg/util/wait" @@ -42,46 +43,307 @@ func getLockPaths() (string, string) { return lock14x, lock16x } -func testIPTablesVersionCmds(t *testing.T, protocol Protocol) { - version := " v1.4.22" - iptablesCmd := iptablesCommand(protocol) - iptablesRestoreCmd := iptablesRestoreCommand(protocol) +type testCommand struct { + command string + action fakeexec.FakeAction +} - fcmd := fakeexec.FakeCmd{ - CombinedOutputScript: []fakeexec.FakeAction{ - // iptables version response (for runner instantiation) - func() ([]byte, []byte, error) { return []byte(iptablesCmd + version), nil, nil }, - // iptables-restore version response (for runner instantiation) - func() ([]byte, []byte, error) { return []byte(iptablesRestoreCmd + version), nil, nil }, - }, - } +// Creates a FakeExec that expects exactly commands to be run (and will fail otherwise). +func fakeExecForCommands(commands []testCommand) *fakeexec.FakeExec { 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...) }, + CommandScript: make([]fakeexec.FakeCommandAction, len(commands)), + ExactOrder: true, + } + for i := range commands { + fcmd := fakeexec.FakeCmd{ + CombinedOutputScript: []fakeexec.FakeAction{commands[i].action}, + } + argv := strings.Fields(commands[i].command) + fexec.CommandScript[i] = func(cmd string, args ...string) exec.Cmd { return fakeexec.InitFakeCmd(&fcmd, argv[0], argv[1:]...) } + } + return fexec +} + +func TestFakeExecForCommands(t *testing.T) { + var panicresult interface{} + defer func() { + panicresult = recover() + }() + + fake1 := fakeExecForCommands([]testCommand{{ + command: "foo bar baz", + action: func() ([]byte, []byte, error) { return []byte("output"), nil, nil }, + }}) + cmd := fake1.Command("foo", "bar", "baz") + out, err := cmd.CombinedOutput() + if string(out) != "output" { + t.Errorf("fake1: wrong output: expected %q, got %q", "output", out) + } + if err != nil { + t.Errorf("fake1: expected no error, got %v", err) + } + if panicresult != nil { + t.Errorf("fake1: expected no panic, got %q", panicresult) + } + + fake2 := fakeExecForCommands([]testCommand{{ + command: "foo bar baz", + action: func() ([]byte, []byte, error) { return []byte("output"), nil, nil }, + }}) + _ = fake2.Command("foo", "baz") + if panicresult == nil { + t.Errorf("fake2: expected panic from FakeExec, got none") + } +} + +func TestNew(t *testing.T) { + testCases := []struct { + name string + commands []testCommand + expected *runner + }{ + { + name: "ancient", + commands: []testCommand{ + { + command: "iptables --version", + action: func() ([]byte, []byte, error) { return []byte("iptables v1.4.0"), nil, nil }, + }, + { + // iptables-restore version check: ignores --version and just no-ops + command: "iptables-restore --version", + action: func() ([]byte, []byte, error) { return nil, nil, nil }, + }, + }, + expected: &runner{ + hasCheck: false, + hasRandomFully: false, + waitFlag: nil, + restoreWaitFlag: nil, + }, + }, + { + name: "RHEL/CentOS 7", + commands: []testCommand{ + { + command: "iptables --version", + action: func() ([]byte, []byte, error) { return []byte("iptables v1.4.21"), nil, nil }, + }, + { + command: "iptables-restore --version", + action: func() ([]byte, []byte, error) { return []byte("iptables-restore v1.4.21"), nil, nil }, + }, + }, + expected: &runner{ + hasCheck: true, + hasRandomFully: false, + waitFlag: []string{"-w"}, + restoreWaitFlag: []string{"-w"}, + }, + }, + { + name: "1.6", + commands: []testCommand{ + { + command: "iptables --version", + action: func() ([]byte, []byte, error) { return []byte("iptables v1.6.2"), nil, nil }, + }, + }, + expected: &runner{ + hasCheck: true, + hasRandomFully: true, + waitFlag: []string{"-w", "5", "-W", "100000"}, + restoreWaitFlag: []string{"-w", "5", "-W", "100000"}, + }, + }, + { + name: "1.8", + commands: []testCommand{ + { + command: "iptables --version", + action: func() ([]byte, []byte, error) { return []byte("iptables v1.8.11"), nil, nil }, + }, + }, + expected: &runner{ + hasCheck: true, + hasRandomFully: true, + waitFlag: []string{"-w", "5", "-W", "100000"}, + restoreWaitFlag: []string{"-w", "5", "-W", "100000"}, + }, + }, + { + name: "no iptables", + commands: []testCommand{ + { + command: "iptables --version", + action: func() ([]byte, []byte, error) { return nil, nil, fmt.Errorf("no such file or directory") }, + }, + }, + expected: &runner{ + hasCheck: false, + hasRandomFully: false, + waitFlag: nil, + restoreWaitFlag: nil, + }, }, } - _ = New(fexec, protocol) - // Check that proper iptables version command was used during runner instantiation - if !sets.New(fcmd.CombinedOutputLog[0]...).HasAll(iptablesCmd, "--version") { - t.Errorf("%s runner instantiate: Expected cmd '%s --version', Got '%s'", protocol, iptablesCmd, fcmd.CombinedOutputLog[0]) - } + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + fexec := fakeExecForCommands(tc.commands) + runner := newInternal(fexec, ProtocolIPv4, "", "").(*runner) - // Check that proper iptables restore version command was used during runner instantiation - if !sets.New(fcmd.CombinedOutputLog[1]...).HasAll(iptablesRestoreCmd, "--version") { - t.Errorf("%s runner instantiate: Expected cmd '%s --version', Got '%s'", protocol, iptablesRestoreCmd, fcmd.CombinedOutputLog[1]) + if runner.hasCheck != tc.expected.hasCheck { + t.Errorf("Expected hasCheck=%v, got %v", tc.expected.hasCheck, runner.hasCheck) + } + if runner.hasRandomFully != tc.expected.hasRandomFully { + t.Errorf("Expected hasRandomFully=%v, got %v", tc.expected.hasRandomFully, runner.hasRandomFully) + } + if !reflect.DeepEqual(runner.waitFlag, tc.expected.waitFlag) { + t.Errorf("Expected waitFlag=%v, got %v", tc.expected.waitFlag, runner.waitFlag) + } + if !reflect.DeepEqual(runner.restoreWaitFlag, tc.expected.restoreWaitFlag) { + t.Errorf("Expected restoreWaitFlag=%v, got %v", tc.expected.restoreWaitFlag, runner.restoreWaitFlag) + } + }) } } -func TestIPTablesVersionCmdsIPv4(t *testing.T) { - testIPTablesVersionCmds(t, ProtocolIPv4) -} +func TestNewDualStack(t *testing.T) { + testCases := []struct { + name string + commands []testCommand + ipv4 bool + ipv6 bool + }{ + { + name: "both available", + commands: []testCommand{ + { + // ipv4 creation + command: "iptables --version", + action: func() ([]byte, []byte, error) { return []byte("iptables v1.8.0"), nil, nil }, + }, + { + // ipv4 Present() + command: "iptables -w 5 -W 100000 -S POSTROUTING -t nat", + action: func() ([]byte, []byte, error) { return nil, nil, nil }, + }, + { + // ipv6 creation + command: "ip6tables --version", + action: func() ([]byte, []byte, error) { return []byte("iptables v1.8.0"), nil, nil }, + }, + { + // ipv6 Present() + command: "ip6tables -w 5 -W 100000 -S POSTROUTING -t nat", + action: func() ([]byte, []byte, error) { return nil, nil, nil }, + }, + }, + ipv4: true, + ipv6: true, + }, + { + name: "ipv4 available, ipv6 not installed", + commands: []testCommand{ + { + // ipv4 creation + command: "iptables --version", + action: func() ([]byte, []byte, error) { return []byte("iptables v1.8.0"), nil, nil }, + }, + { + // ipv4 Present() + command: "iptables -w 5 -W 100000 -S POSTROUTING -t nat", + action: func() ([]byte, []byte, error) { return nil, nil, nil }, + }, + { + // ipv6 creation + command: "ip6tables --version", + action: func() ([]byte, []byte, error) { return nil, nil, fmt.Errorf("no such file or directory") }, + }, + { + // ipv6 Present() + command: "ip6tables -S POSTROUTING -t nat", + action: func() ([]byte, []byte, error) { return nil, nil, fmt.Errorf("no such file or directory") }, + }, + }, + ipv4: true, + ipv6: false, + }, + { + name: "ipv4 available, ipv6 disabled", + commands: []testCommand{ + { + // ipv4 creation + command: "iptables --version", + action: func() ([]byte, []byte, error) { return []byte("iptables v1.8.0"), nil, nil }, + }, + { + // ipv4 Present() + command: "iptables -w 5 -W 100000 -S POSTROUTING -t nat", + action: func() ([]byte, []byte, error) { return nil, nil, nil }, + }, + { + // ipv6 creation + command: "ip6tables --version", + action: func() ([]byte, []byte, error) { return []byte("iptables v1.8.0"), nil, nil }, + }, + { + // ipv6 Present() + command: "ip6tables -w 5 -W 100000 -S POSTROUTING -t nat", + action: func() ([]byte, []byte, error) { return nil, nil, fmt.Errorf("ipv6 is broken") }, + }, + }, + ipv4: true, + ipv6: false, + }, + { + name: "no iptables support", + commands: []testCommand{ + { + // ipv4 creation + command: "iptables --version", + action: func() ([]byte, []byte, error) { return nil, nil, fmt.Errorf("no such file or directory") }, + }, + { + // ipv4 Present() + command: "iptables -S POSTROUTING -t nat", + action: func() ([]byte, []byte, error) { return nil, nil, fmt.Errorf("no such file or directory") }, + }, + { + // ipv6 creation + command: "ip6tables --version", + action: func() ([]byte, []byte, error) { return nil, nil, fmt.Errorf("no such file or directory") }, + }, + { + // ipv6 Present() + command: "ip6tables -S POSTROUTING -t nat", + action: func() ([]byte, []byte, error) { return nil, nil, fmt.Errorf("no such file or directory") }, + }, + }, + ipv4: false, + ipv6: false, + }, + } -func TestIPTablesVersionCmdsIPv6(t *testing.T) { - testIPTablesVersionCmds(t, ProtocolIPv6) -} + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + fexec := fakeExecForCommands(tc.commands) + runners := newDualStackInternal(fexec) + if tc.ipv4 && runners[v1.IPv4Protocol] == nil { + t.Errorf("Expected ipv4 runner, got nil") + } else if !tc.ipv4 && runners[v1.IPv4Protocol] != nil { + t.Errorf("Expected no ipv4 runner, got one") + } + if tc.ipv6 && runners[v1.IPv6Protocol] == nil { + t.Errorf("Expected ipv6 runner, got nil") + } else if !tc.ipv6 && runners[v1.IPv6Protocol] != nil { + t.Errorf("Expected no ipv6 runner, got one") + } + }) + } +} func testEnsureChain(t *testing.T, protocol Protocol) { fcmd := fakeexec.FakeCmd{ CombinedOutputScript: []fakeexec.FakeAction{ @@ -103,7 +365,7 @@ func testEnsureChain(t *testing.T, protocol Protocol) { func(cmd string, args ...string) exec.Cmd { return fakeexec.InitFakeCmd(&fcmd, cmd, args...) }, }, } - runner := New(fexec, protocol) + runner := newInternal(fexec, protocol, "", "") // Success. exists, err := runner.EnsureChain(TableNAT, Chain("FOOBAR")) if err != nil { @@ -160,7 +422,7 @@ func TestFlushChain(t *testing.T) { func(cmd string, args ...string) exec.Cmd { return fakeexec.InitFakeCmd(&fcmd, cmd, args...) }, }, } - runner := New(fexec, ProtocolIPv4) + runner := newInternal(fexec, ProtocolIPv4, "", "") // Success. err := runner.FlushChain(TableNAT, Chain("FOOBAR")) if err != nil { @@ -197,7 +459,7 @@ func TestDeleteChain(t *testing.T) { func(cmd string, args ...string) exec.Cmd { return fakeexec.InitFakeCmd(&fcmd, cmd, args...) }, }, } - runner := New(fexec, ProtocolIPv4) + runner := newInternal(fexec, ProtocolIPv4, "", "") // Success. err := runner.DeleteChain(TableNAT, Chain("FOOBAR")) if err != nil { @@ -233,7 +495,7 @@ func TestEnsureRuleAlreadyExists(t *testing.T) { func(cmd string, args ...string) exec.Cmd { return fakeexec.InitFakeCmd(&fcmd, cmd, args...) }, }, } - runner := New(fexec, ProtocolIPv4) + runner := newInternal(fexec, ProtocolIPv4, "", "") exists, err := runner.EnsureRule(Append, TableNAT, ChainOutput, "abc", "123") if err != nil { t.Errorf("expected success, got %v", err) @@ -269,7 +531,7 @@ func TestEnsureRuleNew(t *testing.T) { func(cmd string, args ...string) exec.Cmd { return fakeexec.InitFakeCmd(&fcmd, cmd, args...) }, }, } - runner := New(fexec, ProtocolIPv4) + runner := newInternal(fexec, ProtocolIPv4, "", "") exists, err := runner.EnsureRule(Append, TableNAT, ChainOutput, "abc", "123") if err != nil { t.Errorf("expected success, got %v", err) @@ -302,7 +564,7 @@ func TestEnsureRuleErrorChecking(t *testing.T) { func(cmd string, args ...string) exec.Cmd { return fakeexec.InitFakeCmd(&fcmd, cmd, args...) }, }, } - runner := New(fexec, ProtocolIPv4) + runner := newInternal(fexec, ProtocolIPv4, "", "") _, err := runner.EnsureRule(Append, TableNAT, ChainOutput, "abc", "123") if err == nil { t.Errorf("expected failure") @@ -332,7 +594,7 @@ func TestEnsureRuleErrorCreating(t *testing.T) { func(cmd string, args ...string) exec.Cmd { return fakeexec.InitFakeCmd(&fcmd, cmd, args...) }, }, } - runner := New(fexec, ProtocolIPv4) + runner := newInternal(fexec, ProtocolIPv4, "", "") _, err := runner.EnsureRule(Append, TableNAT, ChainOutput, "abc", "123") if err == nil { t.Errorf("expected failure") @@ -359,7 +621,7 @@ func TestDeleteRuleDoesNotExist(t *testing.T) { func(cmd string, args ...string) exec.Cmd { return fakeexec.InitFakeCmd(&fcmd, cmd, args...) }, }, } - runner := New(fexec, ProtocolIPv4) + runner := newInternal(fexec, ProtocolIPv4, "", "") err := runner.DeleteRule(TableNAT, ChainOutput, "abc", "123") if err != nil { t.Errorf("expected success, got %v", err) @@ -392,7 +654,7 @@ func TestDeleteRuleExists(t *testing.T) { func(cmd string, args ...string) exec.Cmd { return fakeexec.InitFakeCmd(&fcmd, cmd, args...) }, }, } - runner := New(fexec, ProtocolIPv4) + runner := newInternal(fexec, ProtocolIPv4, "", "") err := runner.DeleteRule(TableNAT, ChainOutput, "abc", "123") if err != nil { t.Errorf("expected success, got %v", err) @@ -422,7 +684,7 @@ func TestDeleteRuleErrorChecking(t *testing.T) { func(cmd string, args ...string) exec.Cmd { return fakeexec.InitFakeCmd(&fcmd, cmd, args...) }, }, } - runner := New(fexec, ProtocolIPv4) + runner := newInternal(fexec, ProtocolIPv4, "", "") err := runner.DeleteRule(TableNAT, ChainOutput, "abc", "123") if err == nil { t.Errorf("expected failure") @@ -452,7 +714,7 @@ func TestDeleteRuleErrorDeleting(t *testing.T) { func(cmd string, args ...string) exec.Cmd { return fakeexec.InitFakeCmd(&fcmd, cmd, args...) }, }, } - runner := New(fexec, ProtocolIPv4) + runner := newInternal(fexec, ProtocolIPv4, "", "") err := runner.DeleteRule(TableNAT, ChainOutput, "abc", "123") if err == nil { t.Errorf("expected failure") @@ -471,7 +733,7 @@ func TestGetIPTablesHasCheckCommand(t *testing.T) { {"iptables v1.4.11", true}, {"iptables v1.4.19.1", true}, {"iptables v2.0.0", true}, - {"total junk", true}, + {"total junk", false}, } for _, testCase := range testCases { @@ -487,7 +749,7 @@ func TestGetIPTablesHasCheckCommand(t *testing.T) { func(cmd string, args ...string) exec.Cmd { return fakeexec.InitFakeCmd(&fcmd, cmd, args...) }, }, } - ipt := New(fexec, ProtocolIPv4) + ipt := newInternal(fexec, ProtocolIPv4, "", "") runner := ipt.(*runner) if testCase.Expected != runner.hasCheck { t.Errorf("Expected result: %v, Got result: %v", testCase.Expected, runner.hasCheck) @@ -648,7 +910,7 @@ func TestWaitFlagUnavailable(t *testing.T) { func(cmd string, args ...string) exec.Cmd { return fakeexec.InitFakeCmd(&fcmd, cmd, args...) }, }, } - runner := New(fexec, ProtocolIPv4) + runner := newInternal(fexec, ProtocolIPv4, "", "") err := runner.DeleteChain(TableNAT, Chain("FOOBAR")) if err != nil { t.Errorf("expected success, got %v", err) @@ -679,7 +941,7 @@ func TestWaitFlagOld(t *testing.T) { func(cmd string, args ...string) exec.Cmd { return fakeexec.InitFakeCmd(&fcmd, cmd, args...) }, }, } - runner := New(fexec, ProtocolIPv4) + runner := newInternal(fexec, ProtocolIPv4, "", "") err := runner.DeleteChain(TableNAT, Chain("FOOBAR")) if err != nil { t.Errorf("expected success, got %v", err) @@ -713,7 +975,7 @@ func TestWaitFlagNew(t *testing.T) { func(cmd string, args ...string) exec.Cmd { return fakeexec.InitFakeCmd(&fcmd, cmd, args...) }, }, } - runner := New(fexec, ProtocolIPv4) + runner := newInternal(fexec, ProtocolIPv4, "", "") err := runner.DeleteChain(TableNAT, Chain("FOOBAR")) if err != nil { t.Errorf("expected success, got %v", err) @@ -744,7 +1006,7 @@ func TestWaitIntervalFlagNew(t *testing.T) { func(cmd string, args ...string) exec.Cmd { return fakeexec.InitFakeCmd(&fcmd, cmd, args...) }, }, } - runner := New(fexec, ProtocolIPv4) + runner := newInternal(fexec, ProtocolIPv4, "", "") err := runner.DeleteChain(TableNAT, Chain("FOOBAR")) if err != nil { t.Errorf("expected success, got %v", err) @@ -789,7 +1051,7 @@ COMMIT func(cmd string, args ...string) exec.Cmd { return fakeexec.InitFakeCmd(&fcmd, cmd, args...) }, }, } - runner := New(fexec, protocol) + runner := newInternal(fexec, protocol, "", "") buffer := bytes.NewBuffer(nil) // Success. @@ -857,7 +1119,7 @@ func testRestore(t *testing.T, protocol Protocol) { func(cmd string, args ...string) exec.Cmd { return fakeexec.InitFakeCmd(&fcmd, cmd, args...) }, }, } - runner := New(fexec, protocol) + runner := newInternal(fexec, protocol, "", "") // both flags true err := runner.Restore(TableNAT, []byte{}, FlushTables, RestoreCounters) diff --git a/pkg/util/iptables/monitor_test.go b/pkg/util/iptables/monitor_test.go index de522c59ef6..223fbaa04e0 100644 --- a/pkg/util/iptables/monitor_test.go +++ b/pkg/util/iptables/monitor_test.go @@ -191,7 +191,7 @@ func (mfc *monitorFakeCmd) Stop() { func TestIPTablesMonitor(t *testing.T) { mfe := newMonitorFakeExec() - ipt := New(mfe, ProtocolIPv4) + ipt := newInternal(mfe, ProtocolIPv4, "", "") var reloads uint32 stopCh := make(chan struct{})