From 39234d618083006fe49911d4933f790403d4b9cf Mon Sep 17 00:00:00 2001 From: "Khaled (Kal) Henidak" Date: Thu, 3 Mar 2022 17:51:33 +0000 Subject: [PATCH] iptables: remove port opener --- pkg/proxy/iptables/proxier.go | 99 +----------------------------- pkg/proxy/iptables/proxier_test.go | 23 ------- 2 files changed, 1 insertion(+), 121 deletions(-) diff --git a/pkg/proxy/iptables/proxier.go b/pkg/proxy/iptables/proxier.go index e71a5e8730719..a6bc9c7666b91 100644 --- a/pkg/proxy/iptables/proxier.go +++ b/pkg/proxy/iptables/proxier.go @@ -189,7 +189,6 @@ type Proxier struct { mu sync.Mutex // protects the following fields serviceMap proxy.ServiceMap endpointsMap proxy.EndpointsMap - portsMap map[utilnet.LocalPort]utilnet.Closeable nodeLabels map[string]string // endpointsSynced, endpointSlicesSynced, and servicesSynced are set to true // when corresponding objects are synced after startup. This is used to avoid @@ -209,7 +208,6 @@ type Proxier struct { localDetector proxyutiliptables.LocalTrafficDetector hostname string nodeIP net.IP - portMapper utilnet.PortOpener recorder record.EventRecorder serviceHealthServer healthcheck.ServiceHealthServer @@ -298,7 +296,6 @@ func NewProxier(ipt utiliptables.Interface, } proxier := &Proxier{ - portsMap: make(map[utilnet.LocalPort]utilnet.Closeable), serviceMap: make(proxy.ServiceMap), serviceChanges: proxy.NewServiceChangeTracker(newServiceInfo, ipFamily, recorder, nil), endpointsMap: make(proxy.EndpointsMap), @@ -311,7 +308,6 @@ func NewProxier(ipt utiliptables.Interface, localDetector: localDetector, hostname: hostname, nodeIP: nodeIP, - portMapper: &utilnet.ListenPortOpener, recorder: recorder, serviceHealthServer: serviceHealthServer, healthzServer: healthzServer, @@ -977,9 +973,6 @@ func (proxier *Proxier) syncProxyRules() { // Accumulate NAT chains to keep. activeNATChains := map[utiliptables.Chain]bool{} // use a map as a set - // Accumulate the set of local ports that we will be holding open once this update is complete - replacementPortsMap := map[utilnet.LocalPort]utilnet.Closeable{} - // We are creating those slices ones here to avoid memory reallocations // in every loop. Note that reuse the memory, instead of doing: // slice = @@ -1000,7 +993,6 @@ func (proxier *Proxier) syncProxyRules() { proxier.endpointChainsNumber += len(proxier.endpointsMap[svcName]) } - localAddrSet := utilproxy.GetLocalAddrSet() 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) @@ -1014,10 +1006,6 @@ func (proxier *Proxier) syncProxyRules() { continue } isIPv6 := utilnet.IsIPv6(svcInfo.ClusterIP()) - localPortIPFamily := utilnet.IPv4 - if isIPv6 { - localPortIPFamily = utilnet.IPv6 - } protocol := strings.ToLower(string(svcInfo.Protocol())) svcNameString := svcInfo.serviceNameString @@ -1096,40 +1084,6 @@ func (proxier *Proxier) syncProxyRules() { // Capture externalIPs. for _, externalIP := range svcInfo.ExternalIPStrings() { - // If the "external" IP happens to be an IP that is local to this - // machine, hold the local port open so no other process can open it - // (because the socket might open but it would never work). - if (svcInfo.Protocol() != v1.ProtocolSCTP) && localAddrSet.Has(net.ParseIP(externalIP)) { - lp := utilnet.LocalPort{ - Description: "externalIP for " + svcNameString, - IP: externalIP, - IPFamily: localPortIPFamily, - Port: svcInfo.Port(), - Protocol: utilnet.Protocol(svcInfo.Protocol()), - } - if proxier.portsMap[lp] != nil { - klog.V(4).InfoS("Port was open before and is still needed", "port", lp.String()) - replacementPortsMap[lp] = proxier.portsMap[lp] - } else { - socket, err := proxier.portMapper.OpenLocalPort(&lp) - if err != nil { - msg := fmt.Sprintf("can't open %s, skipping this externalIP: %v", lp.String(), err) - - proxier.recorder.Eventf( - &v1.ObjectReference{ - Kind: "Node", - Name: proxier.hostname, - UID: types.UID(proxier.hostname), - Namespace: "", - }, v1.EventTypeWarning, err.Error(), msg) - klog.ErrorS(err, "can't open port, skipping externalIP", "port", lp.String()) - continue - } - klog.V(2).InfoS("Opened local port", "port", lp.String()) - replacementPortsMap[lp] = socket - } - } - if hasEndpoints { args = append(args[:0], "-A", string(kubeServicesChain), @@ -1252,47 +1206,7 @@ func (proxier *Proxier) syncProxyRules() { // Capture nodeports. If we had more than 2 rules it might be // worthwhile to make a new per-service chain for nodeport rules, but // with just 2 rules it ends up being a waste and a cognitive burden. - if svcInfo.NodePort() != 0 { - // Hold the local port open so no other process can open it - // (because the socket might open but it would never work). - if len(nodeAddresses) == 0 { - continue - } - - lps := make([]utilnet.LocalPort, 0) - for address := range nodeAddresses { - lp := utilnet.LocalPort{ - Description: "nodePort for " + svcNameString, - IP: address, - IPFamily: localPortIPFamily, - Port: svcInfo.NodePort(), - Protocol: utilnet.Protocol(svcInfo.Protocol()), - } - if utilproxy.IsZeroCIDR(address) { - // Empty IP address means all - lp.IP = "" - lps = append(lps, lp) - // If we encounter a zero CIDR, then there is no point in processing the rest of the addresses. - break - } - lps = append(lps, lp) - } - - // For ports on node IPs, open the actual port and hold it. - for _, lp := range lps { - if proxier.portsMap[lp] != nil { - klog.V(4).InfoS("Port was open before and is still needed", "port", lp.String()) - replacementPortsMap[lp] = proxier.portsMap[lp] - } else if svcInfo.Protocol() != v1.ProtocolSCTP { - socket, err := proxier.portMapper.OpenLocalPort(&lp) - if err != nil { - klog.ErrorS(err, "can't open port, skipping this nodePort", "port", lp.String()) - continue - } - klog.V(2).InfoS("Opened local port", "port", lp.String()) - replacementPortsMap[lp] = socket - } - } + if svcInfo.NodePort() != 0 && len(nodeAddresses) != 0 { if hasEndpoints { args = append(args[:0], @@ -1614,9 +1528,6 @@ func (proxier *Proxier) syncProxyRules() { if err != nil { klog.ErrorS(err, "Failed to execute iptables-restore") metrics.IptablesRestoreFailuresTotal.Inc() - // Revert new local ports. - klog.V(2).InfoS("Closing local ports after iptables-restore failure") - utilproxy.RevertPorts(replacementPortsMap, proxier.portsMap) return } success = true @@ -1629,14 +1540,6 @@ func (proxier *Proxier) syncProxyRules() { } } - // Close old local ports and save new ones. - for k, v := range proxier.portsMap { - if replacementPortsMap[k] == nil { - v.Close() - } - } - proxier.portsMap = replacementPortsMap - if proxier.healthzServer != nil { proxier.healthzServer.Updated() } diff --git a/pkg/proxy/iptables/proxier_test.go b/pkg/proxy/iptables/proxier_test.go index e9315364bd47b..e253a80bb0e4c 100644 --- a/pkg/proxy/iptables/proxier_test.go +++ b/pkg/proxy/iptables/proxier_test.go @@ -51,7 +51,6 @@ import ( iptablestest "k8s.io/kubernetes/pkg/util/iptables/testing" "k8s.io/utils/exec" fakeexec "k8s.io/utils/exec/testing" - utilnet "k8s.io/utils/net" utilpointer "k8s.io/utils/pointer" ) @@ -461,26 +460,6 @@ func TestDeleteEndpointConnectionsIPv6(t *testing.T) { } } -// fakeCloseable implements utilproxy.Closeable -type fakeCloseable struct{} - -// Close fakes out the close() used by syncProxyRules to release a local port. -func (f *fakeCloseable) Close() error { - return nil -} - -// fakePortOpener implements portOpener. -type fakePortOpener struct { - openPorts []*utilnet.LocalPort -} - -// OpenLocalPort fakes out the listen() and bind() used by syncProxyRules -// to lock a local port. -func (f *fakePortOpener) OpenLocalPort(lp *utilnet.LocalPort) (utilnet.Closeable, error) { - f.openPorts = append(f.openPorts, lp) - return &fakeCloseable{}, nil -} - const testHostname = "test-hostname" func NewFakeProxier(ipt utiliptables.Interface, endpointSlicesEnabled bool) *Proxier { @@ -501,8 +480,6 @@ func NewFakeProxier(ipt utiliptables.Interface, endpointSlicesEnabled bool) *Pro masqueradeMark: "0x4000", localDetector: detectLocal, hostname: testHostname, - portsMap: make(map[utilnet.LocalPort]utilnet.Closeable), - portMapper: &fakePortOpener{[]*utilnet.LocalPort{}}, serviceHealthServer: healthcheck.NewFakeServiceHealthServer(), precomputedProbabilities: make([]string, 0, 1001), iptablesData: bytes.NewBuffer(nil),