Merge pull request #134124 from danwinship/iptables-setup

drop utiliptables.NewDualStack, which is too confusing

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