Fix that Service routes may get lost when starting on Windows

Fix antrea-io#4467

Signed-off-by: Hongliang Liu <[email protected]>

pkg/agent/controller/noderoute/node_route_controller.go

@@ -43,7 +43,6 @@ import (
 	"antrea.io/antrea/pkg/ovs/ovsctl"
 	utilip "antrea.io/antrea/pkg/util/ip"
 	"antrea.io/antrea/pkg/util/k8s"
-	"antrea.io/antrea/pkg/util/runtime"
 )
 
 const (
@@ -74,7 +73,6 @@ type Controller struct {
 	nodeInformer     coreinformers.NodeInformer
 	nodeLister       corelisters.NodeLister
 	nodeListerSynced cache.InformerSynced
-	svcLister        corelisters.ServiceLister
 	queue            workqueue.RateLimitingInterface
 	// installedNodes records routes and flows installation states of Nodes.
 	// The key is the host name of the Node, the value is the nodeRouteInfo of the Node.
@@ -105,7 +103,6 @@ func NewNodeRouteController(
 	ipsecCertificateManager ipseccertificate.Manager,
 ) *Controller {
 	nodeInformer := informerFactory.Core().V1().Nodes()
-	svcLister := informerFactory.Core().V1().Services()
 	controller := &Controller{
 		kubeClient:              kubeClient,
 		ovsBridgeClient:         ovsBridgeClient,
@@ -118,7 +115,6 @@ func NewNodeRouteController(
 		nodeInformer:            nodeInformer,
 		nodeLister:              nodeInformer.Lister(),
 		nodeListerSynced:        nodeInformer.Informer().HasSynced,
-		svcLister:               svcLister.Lister(),
 		queue:                   workqueue.NewNamedRateLimitingQueue(workqueue.NewItemExponentialFailureRateLimiter(minRetryDelay, maxRetryDelay), "noderoute"),
 		installedNodes:          cache.NewIndexer(nodeRouteInfoKeyFunc, cache.Indexers{nodeRouteInfoPodCIDRIndexName: nodeRouteInfoPodCIDRIndexFunc}),
 		wireGuardClient:         wireguardClient,
@@ -207,27 +203,10 @@ 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/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.

pkg/agent/route/route_linux.go

@@ -843,7 +843,7 @@ 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 {
+func (c *Client) Reconcile(podCIDRs []string) error {
 	desiredPodCIDRs := sets.NewString(podCIDRs...)
 	// Get the peer IPv6 gateways from pod CIDRs
 	desiredIPv6GWs := getIPv6Gateways(podCIDRs)

pkg/agent/route/route_linux_test.go

@@ -622,7 +622,7 @@ func TestReconcile(t *testing.T) {
 		{IP: net.ParseIP("2001:ab03:cd04:55ee:100b::1")}, // non-existing podCIDR, should be deleted.
 	}, nil)
 	mockNetlink.EXPECT().NeighDel(&netlink.Neigh{IP: net.ParseIP("2001:ab03:cd04:55ee:100b::1")})
-	assert.NoError(t, c.Reconcile(podCIDRs, nil))
+	assert.NoError(t, c.Reconcile(podCIDRs))
 }
 
 func TestAddRoutes(t *testing.T) {

pkg/agent/route/route_windows.go

@@ -122,7 +122,7 @@ func (c *Client) initServiceIPRoutes() error {
 
 // Reconcile removes the orphaned routes and related configuration based on the desired podCIDRs and Service IPs. Only
 // the route entries on the host gateway interface are stored in the cache.
-func (c *Client) Reconcile(podCIDRs []string, svcIPs map[string]bool) error {
+func (c *Client) Reconcile(podCIDRs []string) error {
 	desiredPodCIDRs := sets.NewString(podCIDRs...)
 	routes, err := c.listRoutes()
 	if err != nil {
@@ -133,8 +133,8 @@ func (c *Client) Reconcile(podCIDRs []string, svcIPs map[string]bool) error {
 			c.hostRoutes.Store(dst, rt)
 			continue
 		}
-		if _, ok := svcIPs[dst]; ok {
-			c.hostRoutes.Store(dst, rt)
+		// Don't delete the routes which are added by AntreaProxy.
+		if c.isServiceRoute(rt) {
 			continue
 		}
 		err := util.RemoveNetRoute(rt)
@@ -260,8 +260,8 @@ func (c *Client) addVirtualServiceIPRoute(isIPv6 bool) error {
 
 // TODO: Follow the code style in Linux that maintains one Service CIDR.
 func (c *Client) addServiceRoute(svcIP net.IP) error {
-	obj, found := c.hostRoutes.Load(svcIP.String())
 	svcIPNet := util.NewIPNet(svcIP)
+	obj, found := c.hostRoutes.Load(svcIPNet.String())
 
 	// Route: Service IP -> VirtualServiceIPv4 (169.254.0.253)
 	route := &util.Route{
@@ -288,7 +288,7 @@ func (c *Client) addServiceRoute(svcIP net.IP) error {
 		return err
 	}
 
-	c.hostRoutes.Store(route.DestinationSubnet.String(), route)
+	c.hostRoutes.Store(svcIPNet.String(), route)
 	klog.V(2).InfoS("Added Service route", "ServiceIP", route.DestinationSubnet, "GatewayIP", route.GatewayAddress)
 	return nil
 }
@@ -305,7 +305,7 @@ func (c *Client) deleteServiceRoute(svcIP net.IP) error {
 	if err := util.RemoveNetRoute(rt); err != nil {
 		return err
 	}
-	c.hostRoutes.Delete(svcIP.String())
+	c.hostRoutes.Delete(svcIPNet.String())
 	klog.V(2).InfoS("Deleted Service route from host gateway", "DestinationIP", svcIP)
 	return nil
 }
@@ -332,6 +332,15 @@ func (c *Client) UnMigrateRoutesFromGw(route *net.IPNet, linkName string) error
 func (c *Client) Run(stopCh <-chan struct{}) {
 }
 
+func (c *Client) isServiceRoute(route *util.Route) bool {
+	// If the destination IP or gateway IP is the virtual Service IP, then it is the Service route added by AntreaProxy.
+	if route.DestinationSubnet != nil && route.DestinationSubnet.IP.Equal(config.VirtualServiceIPv4) ||
+		route.GatewayAddress != nil && route.GatewayAddress.Equal(config.VirtualServiceIPv4) {
+		return true
+	}
+	return false
+}
+
 func (c *Client) listRoutes() (map[string]*util.Route, error) {
 	routes, err := util.GetNetRoutesAll()
 	if err != nil {

pkg/agent/route/route_windows_test.go

@@ -89,14 +89,20 @@ func TestRouteOperation(t *testing.T) {
 	route3, err := util.GetNetRoutes(gwLink, svcIPNet1)
 	require.Nil(t, err)
 	assert.Equal(t, 1, len(route3))
+	obj, found := client.hostRoutes.Load(svcIPNet1.String())
+	assert.True(t, found)
+	assert.EqualValues(t, route3[0], *obj.(*util.Route))
 
 	err = client.AddClusterIPRoute(svcIP2)
 	require.Nil(t, err)
 	route4, err := util.GetNetRoutes(gwLink, svcIPNet2)
 	require.Nil(t, err)
 	assert.Equal(t, 1, len(route4))
+	obj, found = client.hostRoutes.Load(svcIPNet2.String())
+	assert.True(t, found)
+	assert.EqualValues(t, route4[0], *obj.(*util.Route))
 
-	err = client.Reconcile([]string{dest2}, map[string]bool{svcIPNet1.String(): true})
+	err = client.Reconcile([]string{dest2})
 	require.Nil(t, err)
 
 	routes5, err := util.GetNetRoutes(gwLink, destCIDR1)
@@ -105,7 +111,7 @@ func TestRouteOperation(t *testing.T) {
 
 	routes6, err := util.GetNetRoutes(gwLink, svcIPNet2)
 	require.Nil(t, err)
-	assert.Equal(t, 0, len(routes6))
+	assert.Equal(t, 1, len(routes6))
 
 	err = client.DeleteRoutes(destCIDR2)
 	require.Nil(t, err)
@@ -118,4 +124,6 @@ func TestRouteOperation(t *testing.T) {
 	routes8, err := util.GetNetRoutes(gwLink, svcIPNet1)
 	require.Nil(t, err)
 	assert.Equal(t, 0, len(routes8))
+	_, found = client.hostRoutes.Load(svcIPNet1.String())
+	assert.False(t, found)
 }

pkg/agent/util/net.go

@@ -383,7 +383,7 @@ func GetAllNodeAddresses(excludeDevices []string) ([]net.IP, []net.IP, error) {
 // NewIPNet generates an IPNet from an ip address using a netmask of 32 or 128.
 func NewIPNet(ip net.IP) *net.IPNet {
 	if ip.To4() != nil {
-		return &net.IPNet{IP: ip, Mask: net.CIDRMask(32, 32)}
+		return &net.IPNet{IP: ip.To4(), Mask: net.CIDRMask(32, 32)}
 	}
 	return &net.IPNet{IP: ip, Mask: net.CIDRMask(128, 128)}
 }

test/integration/agent/route_test.go

@@ -517,7 +517,6 @@ func TestReconcile(t *testing.T) {
 		addedRoutes      []peer
 		desiredPeerCIDRs []string
 		desiredNodeIPs   []string
-		desiredServices  map[string]bool
 		// expectations
 		expRoutes map[string]netlink.Link
 	}{
@@ -530,7 +529,6 @@ func TestReconcile(t *testing.T) {
 			},
 			desiredPeerCIDRs: []string{"10.10.20.0/24"},
 			desiredNodeIPs:   []string{remotePeerIP.String()},
-			desiredServices:  map[string]bool{"200.200.10.10": true},
 			expRoutes:        map[string]netlink.Link{"10.10.20.0/24": gwLink, "10.10.30.0/24": nil},
 		},
 		{
@@ -542,7 +540,6 @@ func TestReconcile(t *testing.T) {
 			},
 			desiredPeerCIDRs: []string{"10.10.20.0/24"},
 			desiredNodeIPs:   []string{localPeerIP.String()},
-			desiredServices:  map[string]bool{"200.200.10.10": true},
 			expRoutes:        map[string]netlink.Link{"10.10.20.0/24": nodeLink, "10.10.30.0/24": nil},
 		},
 		{
@@ -556,7 +553,6 @@ func TestReconcile(t *testing.T) {
 			},
 			desiredPeerCIDRs: []string{"10.10.20.0/24", "10.10.40.0/24"},
 			desiredNodeIPs:   []string{localPeerIP.String(), remotePeerIP.String()},
-			desiredServices:  map[string]bool{"200.200.10.10": true},
 			expRoutes:        map[string]netlink.Link{"10.10.20.0/24": nodeLink, "10.10.30.0/24": nil, "10.10.40.0/24": gwLink, "10.10.50.0/24": nil},
 		},
 	}
@@ -574,7 +570,7 @@ func TestReconcile(t *testing.T) {
 			assert.NoError(t, routeClient.AddRoutes(peerNet, tc.nodeName, route.peerIP, peerGwIP), "adding routes failed")
 		}
 
-		assert.NoError(t, routeClient.Reconcile(tc.desiredPeerCIDRs, tc.desiredServices), "reconcile failed")
+		assert.NoError(t, routeClient.Reconcile(tc.desiredPeerCIDRs), "reconcile failed")
 
 		for dst, uplink := range tc.expRoutes {
 			expNum := 0