Merge pull request #129826 from danwinship/iptables-constructors

fix up iptables construction, kubelet iptables startup messages

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)
+		ipts := utiliptables.NewDualStack()
-		ipv4Supported = ipt[0].Present()
+		ipv4Supported = ipts[v1.IPv4Protocol] != nil
-		ipv6Supported = ipt[1].Present()
+		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()
 

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.NewDualStack()
-	iptClients := []utiliptables.Interface{
+	if len(iptClients) == 0 {
-		utiliptables.New(exec, utiliptables.ProtocolIPv4),
+		klog.InfoS("No iptables support on this system; not creating the KUBE-IPTABLES-HINT chain")
-		utiliptables.New(exec, utiliptables.ProtocolIPv6),
+		return
 	}
 
-	for i := range iptClients {
+	for family := range iptClients {
-		iptClient := iptClients[i]
+		iptClient := iptClients[family]
 		if kl.syncIPTablesRules(iptClient) {
 			klog.InfoS("Initialized iptables rules.", "protocol", iptClient.Protocol())
 			go iptClient.Monitor(

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)

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,

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 {
+func New(protocol Protocol) Interface {
-	return newInternal(exec, protocol, "", "")
+	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.

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) {
+type testCommand struct {
-	version := " v1.4.22"
+	command string
-	iptablesCmd := iptablesCommand(protocol)
+	action  fakeexec.FakeAction
-	iptablesRestoreCmd := iptablesRestoreCommand(protocol)
-
-	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{
+		CommandScript: make([]fakeexec.FakeCommandAction, len(commands)),
-			func(cmd string, args ...string) exec.Cmd { return fakeexec.InitFakeCmd(&fcmd, cmd, args...) },
+		ExactOrder:    true,
-			func(cmd string, args ...string) exec.Cmd { return fakeexec.InitFakeCmd(&fcmd, cmd, args...) },
+	}
+	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
+	for _, tc := range testCases {
-	if !sets.New(fcmd.CombinedOutputLog[0]...).HasAll(iptablesCmd, "--version") {
+		t.Run(tc.name, func(t *testing.T) {
-		t.Errorf("%s runner instantiate: Expected cmd '%s --version', Got '%s'", protocol, iptablesCmd, fcmd.CombinedOutputLog[0])
+			fexec := fakeExecForCommands(tc.commands)
+			runner := newInternal(fexec, ProtocolIPv4, "", "").(*runner)
+
+			if runner.hasCheck != tc.expected.hasCheck {
+				t.Errorf("Expected hasCheck=%v, got %v", tc.expected.hasCheck, runner.hasCheck)
 			}
-
+			if runner.hasRandomFully != tc.expected.hasRandomFully {
-	// Check that proper iptables restore version command was used during runner instantiation
+				t.Errorf("Expected hasRandomFully=%v, got %v", tc.expected.hasRandomFully, runner.hasRandomFully)
-	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 !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) {
+func TestNewDualStack(t *testing.T) {
-	testIPTablesVersionCmds(t, ProtocolIPv4)
+	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) {
+	for _, tc := range testCases {
-	testIPTablesVersionCmds(t, ProtocolIPv6)
+		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)

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{})