Drop utiliptables.NewDualStack()

The semantics (sometimes it returns an error that is really just a
warning) are too confusing, and it turns out that we really only need
it in one place (platformCheckSupported()); after that we've already
figured out what IP families are supported, so we could just use
utiliptables.NewBestEffort() instead, knowing we want exactly what it
returns.

So we can just expand the semantics of the old NewDualStack() inline
in the one place we care, without hiding any of it behind a
too-complicated return value.

cmd/kube-proxy/app/server_linux.go

@@ -90,17 +90,21 @@ func (s *ProxyServer) platformCheckSupported(ctx context.Context) (ipv4Supported
 
 	if isIPTablesBased(s.Config.Mode) {
 		// Check for the iptables and ip6tables binaries.
-		ipts, errDS := utiliptables.NewDualStack()
+		errv4 := utiliptables.New(utiliptables.ProtocolIPv4).Present()
+		errv6 := utiliptables.New(utiliptables.ProtocolIPv6).Present()
 
-		ipv4Supported = ipts[v1.IPv4Protocol] != nil
-		ipv6Supported = ipts[v1.IPv6Protocol] != nil
+		ipv4Supported = errv4 == nil
+		ipv6Supported = errv6 == nil
 
 		if !ipv4Supported && !ipv6Supported {
-			err = fmt.Errorf("iptables is not available on this host : %w", errDS)
+			// errv4 and errv6 are almost certainly the same underlying error
+			// ("iptables isn't installed" or "kernel modules not available")
+			// so it doesn't make sense to try to combine them.
+			err = fmt.Errorf("iptables is not available on this host : %w", errv4)
 		} else if !ipv4Supported {
-			logger.Info("No iptables support for family", "ipFamily", v1.IPv4Protocol, "error", errDS)
+			logger.Info("No iptables support for family", "ipFamily", v1.IPv4Protocol, "error", errv4)
 		} else if !ipv6Supported {
-			logger.Info("No iptables support for family", "ipFamily", v1.IPv6Protocol, "error", errDS)
+			logger.Info("No iptables support for family", "ipFamily", v1.IPv6Protocol, "error", errv6)
 		}
 	} else {
 		// The nft CLI always supports both families.
@@ -130,7 +134,7 @@ func (s *ProxyServer) createProxier(ctx context.Context, config *proxyconfigapi.
 
 	if config.Mode == proxyconfigapi.ProxyModeIPTables {
 		logger.Info("Using iptables Proxier")
-		ipts, _ := utiliptables.NewDualStack()
+		ipts := utiliptables.NewBestEffort()
 
 		if dualStack {
 			// TODO this has side effects that should only happen when Run() is invoked.
@@ -184,7 +188,7 @@ 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()
+		ipts := utiliptables.NewBestEffort()
 
 		logger.Info("Using ipvs Proxier")
 		if dualStack {

pkg/proxy/iptables/proxier.go

@@ -404,7 +404,7 @@ var iptablesCleanupOnlyChains = []iptablesJumpChain{}
 // CleanupLeftovers removes all iptables rules and chains created by the Proxier
 // It returns true if an error was encountered. Errors are logged.
 func CleanupLeftovers(ctx context.Context) (encounteredError bool) {
-	ipts, _ := utiliptables.NewDualStack()
+	ipts := utiliptables.NewBestEffort()
 	for _, ipt := range ipts {
 		encounteredError = cleanupLeftoversForFamily(ctx, ipt) || encounteredError
 	}

pkg/proxy/ipvs/proxier.go

@@ -706,7 +706,7 @@ func CleanupLeftovers(ctx context.Context) (encounteredError bool) {
 		return false
 	}
 
-	ipts, _ := utiliptables.NewDualStack()
+	ipts := utiliptables.NewBestEffort()
 	ipsetInterface := utilipset.New()
 	ipvsInterface := utilipvs.New()
 

pkg/util/iptables/iptables.go

@@ -247,37 +247,17 @@ func New(protocol Protocol) Interface {
 	return newInternal(utilexec.New(), protocol, "", "")
 }
 
-func newDualStackInternal(exec utilexec.Interface) (map[v1.IPFamily]Interface, error) {
-	var err error
+func newDualStackInternal(exec utilexec.Interface) map[v1.IPFamily]Interface {
 	interfaces := map[v1.IPFamily]Interface{}
-
 	iptv4 := newInternal(exec, ProtocolIPv4, "", "")
-	if presentErr := iptv4.Present(); presentErr != nil {
-		err = presentErr
-	} else {
+	if presentErr := iptv4.Present(); presentErr == nil {
 		interfaces[v1.IPv4Protocol] = iptv4
 	}
 	iptv6 := newInternal(exec, ProtocolIPv6, "", "")
-	if presentErr := iptv6.Present(); presentErr != nil {
-		// If we get an error for both IPv4 and IPv6 Present() calls, it's virtually guaranteed that
-		// they're going to be the same error. We ignore the error for IPv6 if IPv4 has already failed.
-		if err == nil {
-			err = presentErr
-		}
-	} else {
+	if presentErr := iptv6.Present(); presentErr == nil {
 		interfaces[v1.IPv6Protocol] = iptv6
 	}
-
-	return interfaces, err
-}
-
-// NewDualStack returns a map containing an IPv4 Interface (if IPv4 iptables is supported)
-// and an IPv6 Interface (if IPv6 iptables is supported). If only one family is supported,
-// it will return a map with one Interface *and* an error (indicating the problem with the
-// other family). If neither family is supported, it will return an empty map and an
-// error.
-func NewDualStack() (map[v1.IPFamily]Interface, error) {
-	return newDualStackInternal(utilexec.New())
+	return interfaces
 }
 
 // NewBestEffort returns a map containing an IPv4 Interface (if IPv4 iptables is
@@ -286,8 +266,7 @@ func NewDualStack() (map[v1.IPFamily]Interface, error) {
 // simple for callers that just want "best-effort" iptables support, where neither partial
 // nor complete lack of iptables support is considered an error.
 func NewBestEffort() map[v1.IPFamily]Interface {
-	ipts, _ := newDualStackInternal(utilexec.New())
-	return ipts
+	return newDualStackInternal(utilexec.New())
 }
 
 // EnsureChain is part of Interface.

pkg/util/iptables/iptables_test.go

@@ -329,7 +329,7 @@ func TestNewDualStack(t *testing.T) {
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
 			fexec := fakeExecForCommands(tc.commands)
-			runners, err := newDualStackInternal(fexec)
+			runners := newDualStackInternal(fexec)
 
 			if tc.ipv4 && runners[v1.IPv4Protocol] == nil {
 				t.Errorf("Expected ipv4 runner, got nil")
@@ -341,12 +341,6 @@ func TestNewDualStack(t *testing.T) {
 			} else if !tc.ipv6 && runners[v1.IPv6Protocol] != nil {
 				t.Errorf("Expected no ipv6 runner, got one")
 			}
-
-			if len(runners) == 2 && err != nil {
-				t.Errorf("Got 2 runners but also an error (%v)", err)
-			} else if len(runners) != 2 && err == nil {
-				t.Errorf("Got %d runners but no error", len(runners))
-			}
 		})
 	}
 }