proxy: Take into account exclude CIDRs while deleting legacy real servers

Author: namreg

pkg/proxy/ipvs/proxier.go

@@ -1671,15 +1671,17 @@ func (proxier *Proxier) syncEndpoint(svcPortName proxy.ServicePortName, onlyNode
 func (proxier *Proxier) cleanLegacyService(activeServices map[string]bool, currentServices map[string]*utilipvs.VirtualServer, legacyBindAddrs map[string]bool) {
 	for cs := range currentServices {
 		svc := currentServices[cs]
+		if proxier.isIPInExcludeCIDRs(svc.Address) {
+			continue
+		}
 		if _, ok := activeServices[cs]; !ok {
-			// This service was not processed in the latest sync loop so before deleting it,
-			okayToDelete := true
 			rsList, _ := proxier.ipvs.GetRealServers(svc)
 
 			// If we still have real servers graceful termination is not done
 			if len(rsList) > 0 {
-				okayToDelete = false
+				continue
 			}
+
 			// Applying graceful termination to all real servers
 			for _, rs := range rsList {
 				uniqueRS := GetUniqueRSName(svc, rs)
@@ -1692,35 +1694,36 @@ func (proxier *Proxier) cleanLegacyService(activeServices map[string]bool, curre
 					klog.Errorf("Failed to delete destination: %v, error: %v", uniqueRS, err)
 				}
 			}
-			// make sure it does not fall within an excluded CIDR range.
-			for _, excludedCIDR := range proxier.excludeCIDRs {
-				// Any validation of this CIDR already should have occurred.
-				_, n, _ := net.ParseCIDR(excludedCIDR)
-				if n.Contains(svc.Address) {
-					okayToDelete = false
-					break
-				}
+			klog.V(4).Infof("Delete service %s", svc.String())
+			if err := proxier.ipvs.DeleteVirtualServer(svc); err != nil {
+				klog.Errorf("Failed to delete service %s, error: %v", svc.String(), err)
 			}
-			if okayToDelete {
-				klog.V(4).Infof("Delete service %s", svc.String())
-				if err := proxier.ipvs.DeleteVirtualServer(svc); err != nil {
-					klog.Errorf("Failed to delete service %s, error: %v", svc.String(), err)
-				}
-				addr := svc.Address.String()
-				if _, ok := legacyBindAddrs[addr]; ok {
-					klog.V(4).Infof("Unbinding address %s", addr)
-					if err := proxier.netlinkHandle.UnbindAddress(addr, DefaultDummyDevice); err != nil {
-						klog.Errorf("Failed to unbind service addr %s from dummy interface %s: %v", addr, DefaultDummyDevice, err)
-					} else {
-						// In case we delete a multi-port service, avoid trying to unbind multiple times
-						delete(legacyBindAddrs, addr)
-					}
+			addr := svc.Address.String()
+			if _, ok := legacyBindAddrs[addr]; ok {
+				klog.V(4).Infof("Unbinding address %s", addr)
+				if err := proxier.netlinkHandle.UnbindAddress(addr, DefaultDummyDevice); err != nil {
+					klog.Errorf("Failed to unbind service addr %s from dummy interface %s: %v", addr, DefaultDummyDevice, err)
+				} else {
+					// In case we delete a multi-port service, avoid trying to unbind multiple times
+					delete(legacyBindAddrs, addr)
 				}
 			}
 		}
 	}
 }
 
+func (proxier *Proxier) isIPInExcludeCIDRs(ip net.IP) bool {
+	// make sure it does not fall within an excluded CIDR range.
+	for _, excludedCIDR := range proxier.excludeCIDRs {
+		// Any validation of this CIDR already should have occurred.
+		_, n, _ := net.ParseCIDR(excludedCIDR)
+		if n.Contains(ip) {
+			return true
+		}
+	}
+	return false
+}
+
 func (proxier *Proxier) getLegacyBindAddr(activeBindAddrs map[string]bool, currentBindAddrs []string) map[string]bool {
 	legacyAddrs := make(map[string]bool)
 	isIpv6 := utilnet.IsIPv6(proxier.nodeIP)

pkg/proxy/ipvs/proxier_test.go

@@ -2925,6 +2925,61 @@ func TestCleanLegacyService(t *testing.T) {
 
 }
 
+func TestCleanLegacyRealServersExcludeCIDRs(t *testing.T) {
+	ipt := iptablestest.NewFake()
+	ipvs := ipvstest.NewFake()
+	ipset := ipsettest.NewFake(testIPSetVersion)
+	gtm := NewGracefulTerminationManager(ipvs)
+	fp := NewFakeProxier(ipt, ipvs, ipset, nil, []string{"4.4.4.4/32"})
+	fp.gracefuldeleteManager = gtm
+
+	vs := &utilipvs.VirtualServer{
+		Address:   net.ParseIP("4.4.4.4"),
+		Protocol:  string(v1.ProtocolUDP),
+		Port:      56,
+		Scheduler: "rr",
+		Flags:     utilipvs.FlagHashed,
+	}
+
+	fp.ipvs.AddVirtualServer(vs)
+
+	rss := []*utilipvs.RealServer{
+		{
+			Address:      net.ParseIP("10.10.10.10"),
+			Port:         56,
+			ActiveConn:   0,
+			InactiveConn: 0,
+		},
+		{
+			Address:      net.ParseIP("11.11.11.11"),
+			Port:         56,
+			ActiveConn:   0,
+			InactiveConn: 0,
+		},
+	}
+	for _, rs := range rss {
+		fp.ipvs.AddRealServer(vs, rs)
+	}
+
+	fp.netlinkHandle.EnsureDummyDevice(DefaultDummyDevice)
+
+	fp.netlinkHandle.EnsureAddressBind("4.4.4.4", DefaultDummyDevice)
+
+	fp.cleanLegacyService(
+		map[string]bool{},
+		map[string]*utilipvs.VirtualServer{"ipvs0": vs},
+		map[string]bool{"4.4.4.4": true},
+	)
+
+	fp.gracefuldeleteManager.tryDeleteRs()
+
+	remainingRealServers, _ := fp.ipvs.GetRealServers(vs)
+
+	if len(remainingRealServers) != 2 {
+		t.Errorf("Expected number of remaining IPVS real servers after cleanup should be %v. Got %v", 2, len(remainingRealServers))
+	}
+}
+
 func TestCleanLegacyService6(t *testing.T) {
 	ipt := iptablestest.NewFake()
 	ipvs := ipvstest.NewFake()