Skip to content

Commit

Permalink
Fix that Service routes may get lost when starting on Windows
Browse files Browse the repository at this point in the history
Fix antrea-io#4467

Signed-off-by: Hongliang Liu <lhongliang@vmware.com>
  • Loading branch information
hongliangl committed Dec 13, 2022
1 parent b503a46 commit 186d6f5
Show file tree
Hide file tree
Showing 7 changed files with 28 additions and 17 deletions.
19 changes: 13 additions & 6 deletions pkg/agent/controller/noderoute/node_route_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"github.com/containernetworking/plugins/pkg/ip"
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/labels"
"k8s.io/apimachinery/pkg/util/sets"
"k8s.io/apimachinery/pkg/util/wait"
"k8s.io/client-go/informers"
coreinformers "k8s.io/client-go/informers/core/v1"
Expand Down Expand Up @@ -204,15 +205,21 @@ func (c *Controller) removeStaleGatewayRoutes() error {
}

// TODO: This is not the best place to keep the ClusterIP Service routes.
desiredClusterIPSvcIPs := map[string]bool{}
desiredClusterIPSvcIPNets := sets.NewString()
if c.proxyAll && runtime.IsWindowsPlatform() {
// The route for virtual IP -> antrea-gw0 should be always kept.
desiredClusterIPSvcIPs[config.VirtualServiceIPv4.String()] = true
vIPNet := util.NewIPNet(config.VirtualServiceIPv4).String()
desiredClusterIPSvcIPNets.Insert(vIPNet)

svcs, err := c.svcLister.List(labels.Everything())
for _, svc := range svcs {
for _, ip := range svc.Spec.ClusterIPs {
desiredClusterIPSvcIPs[ip] = true
for _, clusterIP := range svc.Spec.ClusterIPs {
clusterIPNet := util.NewIPNet(net.ParseIP(clusterIP)).String()
desiredClusterIPSvcIPNets.Insert(clusterIPNet)
}
for _, ingressIP := range svc.Status.LoadBalancer.Ingress {
ingressIPNet := util.NewIPNet(net.ParseIP(ingressIP.IP)).String()
desiredClusterIPSvcIPNets.Insert(ingressIPNet)
}
}
if err != nil {
Expand All @@ -222,8 +229,8 @@ func (c *Controller) removeStaleGatewayRoutes() error {

// 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 {
// which no longer exists.
if err := c.routeClient.Reconcile(desiredPodCIDRs, desiredClusterIPSvcIPNets); err != nil {
return err
}
return nil
Expand Down
4 changes: 3 additions & 1 deletion pkg/agent/route/interfaces.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ package route
import (
"net"

"k8s.io/apimachinery/pkg/util/sets"

"antrea.io/antrea/pkg/agent/config"
binding "antrea.io/antrea/pkg/ovs/openflow"
)
Expand All @@ -29,7 +31,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, svcIPNets sets.String) error

// AddRoutes should add routes to the provided podCIDR.
// It should override the routes if they already exist, without error.
Expand Down
2 changes: 1 addition & 1 deletion pkg/agent/route/route_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -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, svcIPNets sets.String) error {
desiredPodCIDRs := sets.NewString(podCIDRs...)
// Get the peer IPv6 gateways from pod CIDRs
desiredIPv6GWs := getIPv6Gateways(podCIDRs)
Expand Down
4 changes: 2 additions & 2 deletions pkg/agent/route/route_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -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, svcIPNets sets.String) error {
desiredPodCIDRs := sets.NewString(podCIDRs...)
routes, err := c.listRoutes()
if err != nil {
Expand All @@ -133,7 +133,7 @@ func (c *Client) Reconcile(podCIDRs []string, svcIPs map[string]bool) error {
c.hostRoutes.Store(dst, rt)
continue
}
if _, ok := svcIPs[dst]; ok {
if _, ok := svcIPNets[dst]; ok {
c.hostRoutes.Store(dst, rt)
continue
}
Expand Down
3 changes: 2 additions & 1 deletion pkg/agent/route/route_windows_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"k8s.io/apimachinery/pkg/util/sets"
"k8s.io/klog/v2"

"antrea.io/antrea/pkg/agent/config"
Expand Down Expand Up @@ -96,7 +97,7 @@ func TestRouteOperation(t *testing.T) {
require.Nil(t, err)
assert.Equal(t, 1, len(route4))

err = client.Reconcile([]string{dest2}, map[string]bool{svcIPNet1.String(): true})
err = client.Reconcile([]string{dest2}, sets.NewString(svcIPNet1.String()))
require.Nil(t, err)

routes5, err := util.GetNetRoutes(gwLink, destCIDR1)
Expand Down
5 changes: 3 additions & 2 deletions pkg/agent/route/testing/mock_route.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 4 additions & 4 deletions test/integration/agent/route_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -517,7 +517,7 @@ func TestReconcile(t *testing.T) {
addedRoutes []peer
desiredPeerCIDRs []string
desiredNodeIPs []string
desiredServices map[string]bool
desiredServices sets.String
// expectations
expRoutes map[string]netlink.Link
}{
Expand All @@ -530,7 +530,7 @@ 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},
desiredServices: sets.NewString("200.200.10.10/32"),
expRoutes: map[string]netlink.Link{"10.10.20.0/24": gwLink, "10.10.30.0/24": nil},
},
{
Expand All @@ -542,7 +542,7 @@ 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},
desiredServices: sets.NewString("200.200.10.10/32"),
expRoutes: map[string]netlink.Link{"10.10.20.0/24": nodeLink, "10.10.30.0/24": nil},
},
{
Expand All @@ -556,7 +556,7 @@ 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},
desiredServices: sets.NewString("200.200.10.10/32"),
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},
},
}
Expand Down

0 comments on commit 186d6f5

Please sign in to comment.