Unify some functions in pkg/agent/route

This PR unifies the functions in route_linux.go and route_windows.go:
  - AddClusterIPRoute
  - AddLoadBalancer
  - DeleteLoadBalancer

Removed
  - Interface DeleteClusterIPRoute and corresponding implementation

Signed-off-by: Hongliang Liu <lhongliang@vmware.com>

pkg/agent/controller/noderoute/node_route_controller.go

@@ -42,7 +42,6 @@ import (
 	"antrea.io/antrea/pkg/ovs/ovsconfig"
 	utilip "antrea.io/antrea/pkg/util/ip"
 	"antrea.io/antrea/pkg/util/k8s"
-	"antrea.io/antrea/pkg/util/runtime"
 )
 
 const (
@@ -203,27 +202,8 @@ func (c *Controller) removeStaleGatewayRoutes() error {
 		desiredPodCIDRs = append(desiredPodCIDRs, podCIDRs...)
 	}
 
-	// TODO: This is not the best place to keep the ClusterIP Service routes.
-	desiredClusterIPSvcIPs := map[string]bool{}
-	if c.proxyAll && runtime.IsWindowsPlatform() {
-		// The route for virtual IP -> antrea-gw0 should be always kept.
-		desiredClusterIPSvcIPs[config.VirtualServiceIPv4.String()] = true
-
-		svcs, err := c.svcLister.List(labels.Everything())
-		for _, svc := range svcs {
-			for _, ip := range svc.Spec.ClusterIPs {
-				desiredClusterIPSvcIPs[ip] = true
-			}
-		}
-		if err != nil {
-			return fmt.Errorf("error when listing ClusterIP Service IPs: %v", err)
-		}
-	}
-
 	// routeClient will remove orphaned routes whose destinations are not in desiredPodCIDRs.
-	// If proxyAll enabled, it will also remove routes that are for Windows ClusterIP Services
-	// which no longer exist.
-	if err := c.routeClient.Reconcile(desiredPodCIDRs, desiredClusterIPSvcIPs); err != nil {
+	if err := c.routeClient.Reconcile(desiredPodCIDRs); err != nil {
 		return err
 	}
 	return nil

pkg/agent/proxy/proxier.go

@@ -590,13 +590,6 @@ func (p *proxier) installServices() {
 							continue
 						}
 					}
-					// If previous Service which has ClusterIP should be removed, remove ClusterIP routes.
-					if svcInfo.ClusterIP() != nil {
-						if err := p.routeClient.DeleteClusterIPRoute(pSvcInfo.ClusterIP()); err != nil {
-							klog.ErrorS(err, "Failed to remove ClusterIP Service routes", "Service", svcPortName)
-							continue
-						}
-					}
 				}
 			}
 

pkg/agent/route/interfaces.go

@@ -29,7 +29,7 @@ type Interface interface {
 
 	// Reconcile should remove orphaned routes and related configuration based on the desired podCIDRs and Service IPs.
 	// If IPv6 is enabled in the cluster, Reconcile should also remove the orphaned IPv6 neighbors.
-	Reconcile(podCIDRs []string, svcIPs map[string]bool) error
+	Reconcile(podCIDRs []string) error
 
 	// AddRoutes should add routes to the provided podCIDR.
 	// It should override the routes if they already exist, without error.
@@ -61,10 +61,6 @@ type Interface interface {
 	// AddClusterIPRoute adds route on K8s node for Service ClusterIP.
 	AddClusterIPRoute(svcIP net.IP) error
 
-	// DeleteClusterIPRoute deletes route for a Service IP when AntreaProxy is configured to handle
-	// ClusterIP Service traffic from host network.
-	DeleteClusterIPRoute(svcIP net.IP) error
-
 	// AddLoadBalancer adds configurations when a LoadBalancer Service is created.
 	AddLoadBalancer(externalIPs []string) error
 

pkg/agent/route/route_linux.go

@@ -728,8 +728,8 @@ func (c *Client) initServiceIPRoutes() error {
 }
 
 // Reconcile removes orphaned podCIDRs from ipset and removes routes to orphaned podCIDRs
-// based on the desired podCIDRs. svcIPs are used for Windows only.
-func (c *Client) Reconcile(podCIDRs []string, svcIPs map[string]bool) error {
+// based on the desired podCIDRs.
+func (c *Client) Reconcile(podCIDRs []string) error {
 	desiredPodCIDRs := sets.NewString(podCIDRs...)
 	// Get the peer IPv6 gateways from pod CIDRs
 	desiredIPv6GWs := getIPv6Gateways(podCIDRs)
@@ -1004,10 +1004,6 @@ func (c *Client) DeleteRoutes(podCIDR *net.IPNet) error {
 	return nil
 }
 
-func (c *Client) DeleteClusterIPRoute(svcIP net.IP) error {
-	return nil
-}
-
 // Join all words with spaces, terminate with newline and write to buf.
 func writeLine(buf *bytes.Buffer, words ...string) {
 	// We avoid strings.Join for performance reasons.
@@ -1201,11 +1197,9 @@ func (c *Client) AddClusterIPRoute(svcIP net.IP) error {
 	linkIndex := c.nodeConfig.GatewayConfig.LinkIndex
 	scope := netlink.SCOPE_UNIVERSE
 	curClusterIPCIDR := c.clusterIPv4CIDR
-	mask := ipv4AddrLength
 	gw := config.VirtualServiceIPv4
 	if isIPv6 {
 		curClusterIPCIDR = c.clusterIPv6CIDR
-		mask = ipv6AddrLength
 		gw = config.VirtualServiceIPv6
 	}
 
@@ -1226,7 +1220,7 @@ func (c *Client) AddClusterIPRoute(svcIP net.IP) error {
 	} else {
 		// If the route doesn't exist, generate a new destination CIDR with the ClusterIP. Note that, this is the first
 		// ClusterIP since the route doesn't exist.
-		newClusterIPCIDR = &net.IPNet{IP: svcIP, Mask: net.CIDRMask(mask, mask)}
+		newClusterIPCIDR = util.NewIPNet(svcIP)
 	}
 
 	// Generate a route with the new destination CIDR and install it.
@@ -1246,7 +1240,8 @@ func (c *Client) AddClusterIPRoute(svcIP net.IP) error {
 	// Collect stale routes.
 	var staleRoutes []*netlink.Route
 	if curClusterIPCIDR != nil {
-		// If current destination CIDR is not nil, the route with current destination CIDR should be uninstalled.
+		// If current destination CIDR is not nil, the route with current destination CIDR should be uninstalled since
+		// a new route with a newly calculated destination CIDR has been installed.
 		route.Dst = curClusterIPCIDR
 		staleRoutes = []*netlink.Route{route}
 	} else {
@@ -1268,7 +1263,11 @@ func (c *Client) AddClusterIPRoute(svcIP net.IP) error {
 	// Remove stale routes.
 	for _, rt := range staleRoutes {
 		if err = netlink.RouteDel(rt); err != nil {
-			return fmt.Errorf("failed to uninstall stale ClusterIP route %s: %w", rt.String(), err)
+			if err.Error() == "no such process" {
+				klog.InfoS("Failed to delete stale ClusterIP route since the route doesn't exist", "route", route)
+			} else {
+				return fmt.Errorf("failed to delete routing entry for ClusterIP %s: %w", svcIP.String(), err)
+			}
 		}
 		klog.V(4).InfoS("Uninstalled stale ClusterIP route successfully", "stale route", rt)
 	}
@@ -1314,10 +1313,10 @@ func (c *Client) addLoadBalancerIngressIPRoute(svcIPStr string) error {
 
 	route := generateRoute(svcIP, mask, gw, linkIndex, netlink.SCOPE_UNIVERSE)
 	if err := netlink.RouteReplace(route); err != nil {
-		return fmt.Errorf("failed to install routing entry for LoadBalancer ingress IP %s: %w", svcIP.String(), err)
+		return fmt.Errorf("failed to install routing entry for LoadBalancer ingress IP %s: %w", svcIPStr, err)
 	}
 	klog.V(4).InfoS("Added LoadBalancer ingress IP route", "route", route)
-	c.serviceRoutes.Store(svcIP.String(), route)
+	c.serviceRoutes.Store(svcIPStr, route)
 
 	return nil
 }
@@ -1341,13 +1340,13 @@ func (c *Client) deleteLoadBalancerIngressIPRoute(svcIPStr string) error {
 	route := generateRoute(svcIP, mask, gw, linkIndex, netlink.SCOPE_UNIVERSE)
 	if err := netlink.RouteDel(route); err != nil {
 		if err.Error() == "no such process" {
-			klog.InfoS("Failed to delete LoadBalancer ingress IP route since the route has been deleted", "route", route)
+			klog.InfoS("Failed to delete LoadBalancer ingress IP route since the route doesn't exist", "route", route)
 		} else {
-			return fmt.Errorf("failed to delete routing entry for LoadBalancer ingress IP %s: %w", svcIP.String(), err)
+			return fmt.Errorf("failed to delete routing entry for LoadBalancer ingress IP %s: %w", svcIPStr, err)
 		}
 	}
 	klog.V(4).InfoS("Deleted LoadBalancer ingress IP route", "route", route)
-	c.serviceRoutes.Delete(svcIP.String())
+	c.serviceRoutes.Delete(svcIPStr)
 
 	return nil
 }