From fb84c4f0f0e531ebc748fd510634a979b0877d25 Mon Sep 17 00:00:00 2001 From: Dan Winship Date: Thu, 22 Dec 2022 14:57:14 -0500 Subject: [PATCH 1/3] Fix kube-proxy dual-stack-iptables-binary-presence check Kube-proxy was checking that iptables supports both IPv4 and IPv6 and falling back to single-stack if not. But it always fell back to the primary IP family, regardless of which family iptables supported... Fix it so that if the primary IP family isn't supported then it bails out entirely. --- cmd/kube-proxy/app/server_others.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/cmd/kube-proxy/app/server_others.go b/cmd/kube-proxy/app/server_others.go index 8a7b316ab6a..2a83d7191da 100644 --- a/cmd/kube-proxy/app/server_others.go +++ b/cmd/kube-proxy/app/server_others.go @@ -165,11 +165,11 @@ func newProxyServer( ipt[1] = iptInterface } - for _, perFamilyIpt := range ipt { - if !perFamilyIpt.Present() { - klog.InfoS("kube-proxy running in single-stack mode, this ipFamily is not supported", "ipFamily", perFamilyIpt.Protocol()) - dualStack = false - } + if !ipt[0].Present() { + return nil, fmt.Errorf("iptables is not supported for primary IP family %q", primaryProtocol) + } else if !ipt[1].Present() { + klog.InfoS("kube-proxy running in single-stack mode: secondary ipFamily is not supported", "ipFamily", ipt[1].Protocol()) + dualStack = false } if proxyMode == proxyconfigapi.ProxyModeIPTables { From e7ed7220eb5e0b33cde480fbbe5fcb14c22894e6 Mon Sep 17 00:00:00 2001 From: Dan Winship Date: Thu, 22 Dec 2022 14:38:38 -0500 Subject: [PATCH 2/3] Explicitly pass IP family to proxier Rather than re-determining it from the iptables object in both proxies. --- cmd/kube-proxy/app/server_others.go | 4 ++++ pkg/kubemark/hollow_proxy.go | 5 +++++ pkg/proxy/iptables/proxier.go | 12 ++++-------- pkg/proxy/ipvs/proxier.go | 12 ++++-------- 4 files changed, 17 insertions(+), 16 deletions(-) diff --git a/cmd/kube-proxy/app/server_others.go b/cmd/kube-proxy/app/server_others.go index 2a83d7191da..d55f9b2cae3 100644 --- a/cmd/kube-proxy/app/server_others.go +++ b/cmd/kube-proxy/app/server_others.go @@ -145,8 +145,10 @@ func newProxyServer( klog.V(2).InfoS("DetectLocalMode", "LocalMode", string(detectLocalMode)) + primaryFamily := v1.IPv4Protocol primaryProtocol := utiliptables.ProtocolIPv4 if netutils.IsIPv6(nodeIP) { + primaryFamily = v1.IPv6Protocol primaryProtocol = utiliptables.ProtocolIPv6 } execer := exec.New() @@ -216,6 +218,7 @@ func newProxyServer( // TODO this has side effects that should only happen when Run() is invoked. proxier, err = iptables.NewProxier( + primaryFamily, iptInterface, utilsysctl.New(), execer, @@ -290,6 +293,7 @@ func newProxyServer( } proxier, err = ipvs.NewProxier( + primaryFamily, iptInterface, ipvsInterface, ipsetInterface, diff --git a/pkg/kubemark/hollow_proxy.go b/pkg/kubemark/hollow_proxy.go index bbd4bd04d5d..87780614002 100644 --- a/pkg/kubemark/hollow_proxy.go +++ b/pkg/kubemark/hollow_proxy.go @@ -85,9 +85,14 @@ func NewHollowProxyOrDie( klog.InfoS("can't determine this node's IP, assuming 127.0.0.1") nodeIP = netutils.ParseIPSloppy("127.0.0.1") } + family := v1.IPv4Protocol + if iptInterface.IsIPv6() { + family = v1.IPv6Protocol + } // Real proxier with fake iptables, sysctl, etc underneath it. //var err error proxier, err = iptables.NewProxier( + family, iptInterface, sysctl, execer, diff --git a/pkg/proxy/iptables/proxier.go b/pkg/proxy/iptables/proxier.go index 87cad7c8669..85d958bec1b 100644 --- a/pkg/proxy/iptables/proxier.go +++ b/pkg/proxy/iptables/proxier.go @@ -221,7 +221,8 @@ var _ proxy.Provider = &Proxier{} // An error will be returned if iptables fails to update or acquire the initial lock. // Once a proxier is created, it will keep iptables up to date in the background and // will not terminate if a particular iptables call fails. -func NewProxier(ipt utiliptables.Interface, +func NewProxier(ipFamily v1.IPFamily, + ipt utiliptables.Interface, sysctl utilsysctl.Interface, exec utilexec.Interface, syncPeriod time.Duration, @@ -259,11 +260,6 @@ func NewProxier(ipt utiliptables.Interface, serviceHealthServer := healthcheck.NewServiceHealthServer(hostname, recorder, nodePortAddresses) - ipFamily := v1.IPv4Protocol - if ipt.IsIPv6() { - ipFamily = v1.IPv6Protocol - } - ipFamilyMap := utilproxy.MapCIDRsByIPFamily(nodePortAddresses) nodePortAddresses = ipFamilyMap[ipFamily] // Log the IPs not matching the ipFamily @@ -337,14 +333,14 @@ func NewDualStackProxier( ) (proxy.Provider, error) { // Create an ipv4 instance of the single-stack proxier ipFamilyMap := utilproxy.MapCIDRsByIPFamily(nodePortAddresses) - ipv4Proxier, err := NewProxier(ipt[0], sysctl, + ipv4Proxier, err := NewProxier(v1.IPv4Protocol, ipt[0], sysctl, exec, syncPeriod, minSyncPeriod, masqueradeAll, localhostNodePorts, masqueradeBit, localDetectors[0], hostname, nodeIP[0], recorder, healthzServer, ipFamilyMap[v1.IPv4Protocol]) if err != nil { return nil, fmt.Errorf("unable to create ipv4 proxier: %v", err) } - ipv6Proxier, err := NewProxier(ipt[1], sysctl, + ipv6Proxier, err := NewProxier(v1.IPv6Protocol, ipt[1], sysctl, exec, syncPeriod, minSyncPeriod, masqueradeAll, false, masqueradeBit, localDetectors[1], hostname, nodeIP[1], recorder, healthzServer, ipFamilyMap[v1.IPv6Protocol]) if err != nil { diff --git a/pkg/proxy/ipvs/proxier.go b/pkg/proxy/ipvs/proxier.go index fbe0e7876d4..5f1bcfb1ef8 100644 --- a/pkg/proxy/ipvs/proxier.go +++ b/pkg/proxy/ipvs/proxier.go @@ -354,7 +354,8 @@ var _ proxy.Provider = &Proxier{} // An error will be returned if it fails to update or acquire the initial lock. // Once a proxier is created, it will keep iptables and ipvs rules up to date in the background and // will not terminate if a particular iptables or ipvs call fails. -func NewProxier(ipt utiliptables.Interface, +func NewProxier(ipFamily v1.IPFamily, + ipt utiliptables.Interface, ipvs utilipvs.Interface, ipset utilipset.Interface, sysctl utilsysctl.Interface, @@ -449,11 +450,6 @@ func NewProxier(ipt utiliptables.Interface, masqueradeValue := 1 << uint(masqueradeBit) masqueradeMark := fmt.Sprintf("%#08x", masqueradeValue) - ipFamily := v1.IPv4Protocol - if ipt.IsIPv6() { - ipFamily = v1.IPv6Protocol - } - klog.V(2).InfoS("Record nodeIP and family", "nodeIP", nodeIP, "family", ipFamily) if len(scheduler) == 0 { @@ -551,7 +547,7 @@ func NewDualStackProxier( ipFamilyMap := utilproxy.MapCIDRsByIPFamily(nodePortAddresses) // Create an ipv4 instance of the single-stack proxier - ipv4Proxier, err := NewProxier(ipt[0], ipvs, safeIpset, sysctl, + ipv4Proxier, err := NewProxier(v1.IPv4Protocol, ipt[0], ipvs, safeIpset, sysctl, exec, syncPeriod, minSyncPeriod, filterCIDRs(false, excludeCIDRs), strictARP, tcpTimeout, tcpFinTimeout, udpTimeout, masqueradeAll, masqueradeBit, localDetectors[0], hostname, nodeIP[0], @@ -560,7 +556,7 @@ func NewDualStackProxier( return nil, fmt.Errorf("unable to create ipv4 proxier: %v", err) } - ipv6Proxier, err := NewProxier(ipt[1], ipvs, safeIpset, sysctl, + ipv6Proxier, err := NewProxier(v1.IPv6Protocol, ipt[1], ipvs, safeIpset, sysctl, exec, syncPeriod, minSyncPeriod, filterCIDRs(true, excludeCIDRs), strictARP, tcpTimeout, tcpFinTimeout, udpTimeout, masqueradeAll, masqueradeBit, localDetectors[1], hostname, nodeIP[1], From 169604d906f202253ace2192d3740f78cef1c61c Mon Sep 17 00:00:00 2001 From: Dan Winship Date: Thu, 22 Dec 2022 14:59:57 -0500 Subject: [PATCH 3/3] Validate single-stack --nodeport-addresses sooner In the dual-stack case, iptables.NewDualStackProxier and ipvs.NewDualStackProxier filtered the nodeport addresses values by IP family before creating the single-stack proxiers. But in the single-stack case, the kube-proxy startup code just passed the value to the single-stack proxiers without validation, so they had to re-check it themselves. Fix that. --- cmd/kube-proxy/app/server_others.go | 20 ++++++++++++++++---- pkg/proxy/iptables/proxier.go | 7 ------- pkg/proxy/ipvs/proxier.go | 7 ------- pkg/proxy/ipvs/proxier_test.go | 6 +++--- 4 files changed, 19 insertions(+), 21 deletions(-) diff --git a/cmd/kube-proxy/app/server_others.go b/cmd/kube-proxy/app/server_others.go index d55f9b2cae3..7a179d31540 100644 --- a/cmd/kube-proxy/app/server_others.go +++ b/cmd/kube-proxy/app/server_others.go @@ -54,6 +54,7 @@ import ( "k8s.io/kubernetes/pkg/proxy/iptables" "k8s.io/kubernetes/pkg/proxy/ipvs" proxymetrics "k8s.io/kubernetes/pkg/proxy/metrics" + proxyutil "k8s.io/kubernetes/pkg/proxy/util" proxyutiliptables "k8s.io/kubernetes/pkg/proxy/util/iptables" utilipset "k8s.io/kubernetes/pkg/util/ipset" utiliptables "k8s.io/kubernetes/pkg/util/iptables" @@ -167,11 +168,22 @@ func newProxyServer( ipt[1] = iptInterface } + nodePortAddresses := config.NodePortAddresses + if !ipt[0].Present() { return nil, fmt.Errorf("iptables is not supported for primary IP family %q", primaryProtocol) } else if !ipt[1].Present() { klog.InfoS("kube-proxy running in single-stack mode: secondary ipFamily is not supported", "ipFamily", ipt[1].Protocol()) dualStack = false + + // Validate NodePortAddresses is single-stack + npaByFamily := proxyutil.MapCIDRsByIPFamily(config.NodePortAddresses) + secondaryFamily := proxyutil.OtherIPFamily(primaryFamily) + badAddrs := npaByFamily[secondaryFamily] + if len(badAddrs) > 0 { + klog.InfoS("Ignoring --nodeport-addresses of the wrong family", "ipFamily", secondaryFamily, "addresses", badAddrs) + nodePortAddresses = npaByFamily[primaryFamily] + } } if proxyMode == proxyconfigapi.ProxyModeIPTables { @@ -206,7 +218,7 @@ func newProxyServer( nodeIPTuple(config.BindAddress), recorder, healthzServer, - config.NodePortAddresses, + nodePortAddresses, ) } else { // Create a single-stack proxier if and only if the node does not support dual-stack (i.e, no iptables support). @@ -232,7 +244,7 @@ func newProxyServer( nodeIP, recorder, healthzServer, - config.NodePortAddresses, + nodePortAddresses, ) } @@ -282,7 +294,7 @@ func newProxyServer( recorder, healthzServer, config.IPVS.Scheduler, - config.NodePortAddresses, + nodePortAddresses, kernelHandler, ) } else { @@ -314,7 +326,7 @@ func newProxyServer( recorder, healthzServer, config.IPVS.Scheduler, - config.NodePortAddresses, + nodePortAddresses, kernelHandler, ) } diff --git a/pkg/proxy/iptables/proxier.go b/pkg/proxy/iptables/proxier.go index 85d958bec1b..fbde2f8db8b 100644 --- a/pkg/proxy/iptables/proxier.go +++ b/pkg/proxy/iptables/proxier.go @@ -260,13 +260,6 @@ func NewProxier(ipFamily v1.IPFamily, serviceHealthServer := healthcheck.NewServiceHealthServer(hostname, recorder, nodePortAddresses) - ipFamilyMap := utilproxy.MapCIDRsByIPFamily(nodePortAddresses) - nodePortAddresses = ipFamilyMap[ipFamily] - // Log the IPs not matching the ipFamily - if ips, ok := ipFamilyMap[utilproxy.OtherIPFamily(ipFamily)]; ok && len(ips) > 0 { - klog.InfoS("Found node IPs of the wrong family", "ipFamily", ipFamily, "IPs", strings.Join(ips, ",")) - } - proxier := &Proxier{ svcPortMap: make(proxy.ServicePortMap), serviceChanges: proxy.NewServiceChangeTracker(newServiceInfo, ipFamily, recorder, nil), diff --git a/pkg/proxy/ipvs/proxier.go b/pkg/proxy/ipvs/proxier.go index 5f1bcfb1ef8..8527b410e33 100644 --- a/pkg/proxy/ipvs/proxier.go +++ b/pkg/proxy/ipvs/proxier.go @@ -459,13 +459,6 @@ func NewProxier(ipFamily v1.IPFamily, serviceHealthServer := healthcheck.NewServiceHealthServer(hostname, recorder, nodePortAddresses) - ipFamilyMap := utilproxy.MapCIDRsByIPFamily(nodePortAddresses) - nodePortAddresses = ipFamilyMap[ipFamily] - // Log the IPs not matching the ipFamily - if ips, ok := ipFamilyMap[utilproxy.OtherIPFamily(ipFamily)]; ok && len(ips) > 0 { - klog.InfoS("Found node IPs of the wrong family", "ipFamily", ipFamily, "IPs", ips) - } - // excludeCIDRs has been validated before, here we just parse it to IPNet list parsedExcludeCIDRs, _ := netutils.ParseCIDRs(excludeCIDRs) diff --git a/pkg/proxy/ipvs/proxier_test.go b/pkg/proxy/ipvs/proxier_test.go index 203f0ffb0c3..0e1e8b5383e 100644 --- a/pkg/proxy/ipvs/proxier_test.go +++ b/pkg/proxy/ipvs/proxier_test.go @@ -2117,11 +2117,11 @@ func TestOnlyLocalNodePorts(t *testing.T) { addrs1 := []net.Addr{&net.IPNet{IP: netutils.ParseIPSloppy("2001:db8::"), Mask: net.CIDRMask(64, 128)}} fp.networkInterfacer.(*proxyutiltest.FakeNetwork).AddInterfaceAddr(&itf, addrs) fp.networkInterfacer.(*proxyutiltest.FakeNetwork).AddInterfaceAddr(&itf1, addrs1) - fp.nodePortAddresses = []string{"100.101.102.0/24", "2001:db8::0/64"} + fp.nodePortAddresses = []string{"100.101.102.0/24"} fp.syncProxyRules() - // Expect 2 (matching ipvs IPFamily field) services and 1 destination + // Expect 2 services and 1 destination epVS := &netlinktest.ExpectedVirtualServer{ VSNum: 2, IP: nodeIP.String(), Port: uint16(svcNodePort), Protocol: string(v1.ProtocolTCP), RS: []netlinktest.ExpectedRealServer{{ @@ -2205,7 +2205,7 @@ func TestHealthCheckNodePort(t *testing.T) { addrs1 := []net.Addr{&net.IPNet{IP: netutils.ParseIPSloppy("2001:db8::"), Mask: net.CIDRMask(64, 128)}} fp.networkInterfacer.(*proxyutiltest.FakeNetwork).AddInterfaceAddr(&itf, addrs) fp.networkInterfacer.(*proxyutiltest.FakeNetwork).AddInterfaceAddr(&itf1, addrs1) - fp.nodePortAddresses = []string{"100.101.102.0/24", "2001:db8::0/64"} + fp.nodePortAddresses = []string{"100.101.102.0/24"} fp.syncProxyRules()