From d43878f970fc39a4ffc3208512a30635771aa859 Mon Sep 17 00:00:00 2001 From: Dan Winship Date: Tue, 20 Dec 2022 22:28:11 -0500 Subject: [PATCH] Put all iptables nodeport address handling in one place For some reason we were calculating the available nodeport IPs at the top of syncProxyRules even though we didn't use them until the end. (Well, the previous code avoided generating KUBE-NODEPORTS chain rules if there were no node IPs available, but that case is considered an error anyway, so there's no need to optimize it.) (Also fix a stale `err` reference exposed by this move.) --- pkg/proxy/iptables/proxier.go | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/pkg/proxy/iptables/proxier.go b/pkg/proxy/iptables/proxier.go index 715fa5ec14b..59e0ba700f0 100644 --- a/pkg/proxy/iptables/proxier.go +++ b/pkg/proxy/iptables/proxier.go @@ -988,20 +988,6 @@ func (proxier *Proxier) syncProxyRules() { } proxier.largeClusterMode = (totalEndpoints > largeClusterEndpointsThreshold) - nodeAddresses, err := utilproxy.GetNodeAddresses(proxier.nodePortAddresses, proxier.networkInterfacer) - if err != nil { - klog.ErrorS(err, "Failed to get node ip address matching nodeport cidrs, services with nodeport may not work as intended", "CIDRs", proxier.nodePortAddresses) - } - // nodeAddresses may contain dual-stack zero-CIDRs if proxier.nodePortAddresses is empty. - // Ensure nodeAddresses only contains the addresses for this proxier's IP family. - for addr := range nodeAddresses { - if utilproxy.IsZeroCIDR(addr) && isIPv6 == netutils.IsIPv6CIDRString(addr) { - // if any of the addresses is zero cidr of this IP family, non-zero IPs can be excluded. - nodeAddresses = sets.NewString(addr) - break - } - } - // These two variables are used to publish the sync_proxy_rules_no_endpoints_total // metric. serviceNoLocalEndpointsTotalInternal := 0 @@ -1218,7 +1204,7 @@ func (proxier *Proxier) syncProxyRules() { } // Capture nodeports. - if svcInfo.NodePort() != 0 && len(nodeAddresses) != 0 { + if svcInfo.NodePort() != 0 { if hasEndpoints { // Jump to the external destination chain. For better or for // worse, nodeports are not subect to loadBalancerSourceRanges, @@ -1414,7 +1400,7 @@ func (proxier *Proxier) syncProxyRules() { for _, ep := range allLocallyReachableEndpoints { epInfo, ok := ep.(*endpointsInfo) if !ok { - klog.ErrorS(err, "Failed to cast endpointsInfo", "endpointsInfo", ep) + klog.ErrorS(nil, "Failed to cast endpointsInfo", "endpointsInfo", ep) continue } @@ -1474,6 +1460,20 @@ func (proxier *Proxier) syncProxyRules() { // Finally, tail-call to the nodePorts chain. This needs to be after all // other service portal rules. + nodeAddresses, err := utilproxy.GetNodeAddresses(proxier.nodePortAddresses, proxier.networkInterfacer) + if err != nil { + klog.ErrorS(err, "Failed to get node ip address matching nodeport cidrs, services with nodeport may not work as intended", "CIDRs", proxier.nodePortAddresses) + } + // nodeAddresses may contain dual-stack zero-CIDRs if proxier.nodePortAddresses is empty. + // Ensure nodeAddresses only contains the addresses for this proxier's IP family. + for addr := range nodeAddresses { + if utilproxy.IsZeroCIDR(addr) && isIPv6 == netutils.IsIPv6CIDRString(addr) { + // if any of the addresses is zero cidr of this IP family, non-zero IPs can be excluded. + nodeAddresses = sets.NewString(addr) + break + } + } + for address := range nodeAddresses { if utilproxy.IsZeroCIDR(address) { destinations := []string{"-m", "addrtype", "--dst-type", "LOCAL"}