From 8c98dee1edbb900a88952ce295788dd93402f885 Mon Sep 17 00:00:00 2001 From: Dan Winship Date: Sat, 11 Jan 2025 07:44:39 -0500 Subject: [PATCH] Add utiliptables.NewDualStack Basically all callers want dual-stack-if-possible, so simplify that. Also, tweak the startup-time checking in kubelet to treat "no iptables support" as interesting but not an error. --- cmd/kube-proxy/app/server_linux.go | 44 ++------- pkg/kubelet/kubelet_network_linux.go | 11 ++- pkg/proxy/iptables/proxier.go | 6 +- pkg/proxy/ipvs/proxier.go | 6 +- pkg/util/iptables/iptables.go | 23 +++++ pkg/util/iptables/iptables_test.go | 136 +++++++++++++++++++++++++++ 6 files changed, 181 insertions(+), 45 deletions(-) diff --git a/cmd/kube-proxy/app/server_linux.go b/cmd/kube-proxy/app/server_linux.go index 9cf782c1a3f..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,35 +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) { - // Create iptables handlers for both families. Always ordered as IPv4, IPv6 - ipt := [2]utiliptables.Interface{ - utiliptables.New(utiliptables.ProtocolIPv4), - utiliptables.New(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") @@ -164,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, @@ -188,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, @@ -220,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(), @@ -249,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(), @@ -507,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 b814e9b829d..32ed9eb2429 100644 --- a/pkg/kubelet/kubelet_network_linux.go +++ b/pkg/kubelet/kubelet_network_linux.go @@ -37,13 +37,14 @@ const ( ) func (kl *Kubelet) initNetworkUtil() { - iptClients := []utiliptables.Interface{ - utiliptables.New(utiliptables.ProtocolIPv4), - utiliptables.New(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 a41f0ea7bb8..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" @@ -253,6 +254,28 @@ 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. func (runner *runner) EnsureChain(table Table, chain Chain) (bool, error) { fullArgs := makeFullArgs(table, chain) diff --git a/pkg/util/iptables/iptables_test.go b/pkg/util/iptables/iptables_test.go index 21fa1d0e067..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" @@ -208,6 +209,141 @@ func TestNew(t *testing.T) { } } +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, + }, + } + + 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{