fix: cleanup outdated routes

openshift · Jun 18, 2021 · 22d66e7 · 22d66e7
1 parent 0eb55a3
commit 22d66e7
Show file tree

Hide file tree

Showing 2 changed files with 113 additions and 2 deletions.
diff --git a/staging/src/k8s.io/legacy-cloud-providers/azure/azure_routes.go b/staging/src/k8s.io/legacy-cloud-providers/azure/azure_routes.go
@@ -149,12 +149,17 @@ func (d *delayedRouteUpdater) updateRoutes() {
 	}
 
 	// reconcile routes.
-	dirty := false
+	dirty, onlyUpdateTags := false, true
 	routes := []network.Route{}
 	if routeTable.Routes != nil {
 		routes = *routeTable.Routes
 	}
-	onlyUpdateTags := true
+
+	routes, dirty = d.cleanupOutdatedRoutes(routes)
+	if dirty {
+		onlyUpdateTags = false
+	}
+
 	for _, rt := range d.routesToUpdate {
 		if rt.operation == routeTableOperationUpdateTags {
 			routeTable.Tags = rt.routeTableTags
@@ -204,6 +209,34 @@ func (d *delayedRouteUpdater) updateRoutes() {
 	}
 }
 
+// cleanupOutdatedRoutes deletes all non-dualstack routes when dualstack is enabled,
+// and deletes all dualstack routes when dualstack is not enabled.
+func (d *delayedRouteUpdater) cleanupOutdatedRoutes(existingRoutes []network.Route) (routes []network.Route, changed bool) {
+	for i := len(existingRoutes) - 1; i >= 0; i-- {
+		existingRouteName := to.String(existingRoutes[i].Name)
+		split := strings.Split(existingRouteName, routeNameSeparator)
+
+		// filter out unmanaged routes
+		deleteRoute := false
+		if d.az.nodeNames.Has(split[0]) {
+			if d.az.ipv6DualStackEnabled && len(split) == 1 {
+				klog.V(2).Infof("cleanupOutdatedRoutes: deleting outdated non-dualstack route %s", existingRouteName)
+				deleteRoute = true
+			} else if !d.az.ipv6DualStackEnabled && len(split) == 2 {
+				klog.V(2).Infof("cleanupOutdatedRoutes: deleting outdated dualstack route %s", existingRouteName)
+				deleteRoute = true
+			}
+
+			if deleteRoute {
+				existingRoutes = append(existingRoutes[:i], existingRoutes[i+1:]...)
+				changed = true
+			}
+		}
+	}
+
+	return existingRoutes, changed
+}
+
 // addRouteOperation adds the routeOperation to delayedRouteUpdater and returns a delayedRouteOperation.
 func (d *delayedRouteUpdater) addRouteOperation(operation routeOperation, route network.Route) (*delayedRouteOperation, error) {
 	d.lock.Lock()

diff --git a/staging/src/k8s.io/legacy-cloud-providers/azure/azure_routes_test.go b/staging/src/k8s.io/legacy-cloud-providers/azure/azure_routes_test.go
@@ -662,3 +662,81 @@ func TestListRoutes(t *testing.T) {
 		assert.Equal(t, test.expectedErrMsg, err, test.name)
 	}
 }
+
+func TestCleanupOutdatedRoutes(t *testing.T) {
+	ctrl := gomock.NewController(t)
+	defer ctrl.Finish()
+
+	for _, testCase := range []struct {
+		description                          string
+		existingRoutes, expectedRoutes       []network.Route
+		existingNodeNames                    sets.String
+		expectedChanged, enableIPV6DualStack bool
+	}{
+		{
+			description: "cleanupOutdatedRoutes should delete outdated non-dualstack routes when dualstack is enabled",
+			existingRoutes: []network.Route{
+				{Name: to.StringPtr("aks-node1-vmss000000____xxx")},
+				{Name: to.StringPtr("aks-node1-vmss000000")},
+			},
+			expectedRoutes: []network.Route{
+				{Name: to.StringPtr("aks-node1-vmss000000____xxx")},
+			},
+			existingNodeNames:   sets.NewString("aks-node1-vmss000000"),
+			enableIPV6DualStack: true,
+			expectedChanged:     true,
+		},
+		{
+			description: "cleanupOutdatedRoutes should delete outdated dualstack routes when dualstack is disabled",
+			existingRoutes: []network.Route{
+				{Name: to.StringPtr("aks-node1-vmss000000____xxx")},
+				{Name: to.StringPtr("aks-node1-vmss000000")},
+			},
+			expectedRoutes: []network.Route{
+				{Name: to.StringPtr("aks-node1-vmss000000")},
+			},
+			existingNodeNames: sets.NewString("aks-node1-vmss000000"),
+			expectedChanged:   true,
+		},
+		{
+			description: "cleanupOutdatedRoutes should not delete unmanaged routes when dualstack is enabled",
+			existingRoutes: []network.Route{
+				{Name: to.StringPtr("aks-node1-vmss000000____xxx")},
+				{Name: to.StringPtr("aks-node1-vmss000000")},
+			},
+			expectedRoutes: []network.Route{
+				{Name: to.StringPtr("aks-node1-vmss000000____xxx")},
+				{Name: to.StringPtr("aks-node1-vmss000000")},
+			},
+			existingNodeNames:   sets.NewString("aks-node1-vmss000001"),
+			enableIPV6DualStack: true,
+		},
+		{
+			description: "cleanupOutdatedRoutes should not delete unmanaged routes when dualstack is disabled",
+			existingRoutes: []network.Route{
+				{Name: to.StringPtr("aks-node1-vmss000000____xxx")},
+				{Name: to.StringPtr("aks-node1-vmss000000")},
+			},
+			expectedRoutes: []network.Route{
+				{Name: to.StringPtr("aks-node1-vmss000000____xxx")},
+				{Name: to.StringPtr("aks-node1-vmss000000")},
+			},
+			existingNodeNames: sets.NewString("aks-node1-vmss000001"),
+		},
+	} {
+		t.Run(testCase.description, func(t *testing.T) {
+			cloud := &Cloud{
+				ipv6DualStackEnabled: testCase.enableIPV6DualStack,
+				nodeNames:            testCase.existingNodeNames,
+			}
+
+			d := &delayedRouteUpdater{
+				az: cloud,
+			}
+
+			routes, changed := d.cleanupOutdatedRoutes(testCase.existingRoutes)
+			assert.Equal(t, testCase.expectedChanged, changed)
+			assert.Equal(t, testCase.expectedRoutes, routes)
+		})
+	}
+}