From 1a283ccbb64d4e9e46da0db79c76950d281f103e Mon Sep 17 00:00:00 2001 From: Qingqing Zheng Date: Mon, 6 Jan 2020 13:35:19 -0800 Subject: [PATCH] use az.List() to check route existence --- .../azure/azure_fakes.go | 5 +++++ .../azure/azure_routes.go | 17 ++++++++++++++- .../azure/azure_routes_test.go | 21 +++++++++++++++++++ 3 files changed, 42 insertions(+), 1 deletion(-) diff --git a/staging/src/k8s.io/legacy-cloud-providers/azure/azure_fakes.go b/staging/src/k8s.io/legacy-cloud-providers/azure/azure_fakes.go index 71f82cebc49..94d269692db 100644 --- a/staging/src/k8s.io/legacy-cloud-providers/azure/azure_fakes.go +++ b/staging/src/k8s.io/legacy-cloud-providers/azure/azure_fakes.go @@ -664,6 +664,7 @@ func (fVMSSC *fakeVirtualMachineScaleSetsClient) UpdateInstances(ctx context.Con type fakeRoutesClient struct { mutex *sync.Mutex FakeStore map[string]map[string]network.Route + Calls []string } func newFakeRoutesClient() *fakeRoutesClient { @@ -677,6 +678,8 @@ func (fRC *fakeRoutesClient) CreateOrUpdate(ctx context.Context, resourceGroupNa fRC.mutex.Lock() defer fRC.mutex.Unlock() + fRC.Calls = append(fRC.Calls, "CreateOrUpdate") + if _, ok := fRC.FakeStore[routeTableName]; !ok { fRC.FakeStore[routeTableName] = make(map[string]network.Route) } @@ -689,6 +692,8 @@ func (fRC *fakeRoutesClient) Delete(ctx context.Context, resourceGroupName strin fRC.mutex.Lock() defer fRC.mutex.Unlock() + fRC.Calls = append(fRC.Calls, "Delete") + if routes, ok := fRC.FakeStore[routeTableName]; ok { if _, ok := routes[routeName]; ok { delete(routes, routeName) 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 index 27d807ed47d..25506665a4c 100644 --- a/staging/src/k8s.io/legacy-cloud-providers/azure/azure_routes.go +++ b/staging/src/k8s.io/legacy-cloud-providers/azure/azure_routes.go @@ -138,7 +138,7 @@ func (az *Cloud) CreateRoute(ctx context.Context, clusterName string, nameHint s } if unmanaged { if az.ipv6DualStackEnabled { - //TODO (khenidak) add support for unmanaged nodes when the feature reaches beta + //TODO (khenidak) add support for unmanaged nodes when the feature reaches beta return fmt.Errorf("unmanaged nodes are not supported in dual stack mode") } klog.V(2).Infof("CreateRoute: omitting unmanaged node %q", kubeRoute.TargetNode) @@ -184,6 +184,21 @@ func (az *Cloud) CreateRoute(ctx context.Context, clusterName string, nameHint s }, } + actualRoutes, err := az.ListRoutes(ctx, clusterName) + if err != nil { + klog.V(3).Infof("CreateRoute: creating route: failed(ListRoutes) clusterName= %q instance=%q with error=%v", clusterName, kubeRoute.TargetNode, err) + return err + } + + for _, actualRoute := range actualRoutes { + if strings.EqualFold(actualRoute.Name, kubeRoute.Name) && + strings.EqualFold(string(actualRoute.TargetNode), string(kubeRoute.TargetNode)) && + strings.EqualFold(actualRoute.DestinationCIDR, kubeRoute.DestinationCIDR) { + klog.V(2).Infof("CreateRoute: route is already existed and matched, no need to re-create or update") + return nil + } + } + klog.V(3).Infof("CreateRoute: creating route: instance=%q cidr=%q", kubeRoute.TargetNode, kubeRoute.DestinationCIDR) err = az.CreateOrUpdateRoute(route) if err != nil { 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 index 45d5bd79ced..b8f16905c7f 100644 --- 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 @@ -137,6 +137,9 @@ func TestCreateRoute(t *testing.T) { if len(fakeTable.Calls) != 1 || fakeTable.Calls[0] != "Get" { t.Errorf("unexpected calls create if not exists, exists: %v", fakeTable.Calls) } + if len(fakeRoutes.Calls) != 1 || fakeRoutes.Calls[0] != "CreateOrUpdate" { + t.Errorf("unexpected route calls create if not exists, exists: %v", fakeRoutes.Calls) + } routeName := mapNodeNameToRouteName(false, route.TargetNode, string(route.DestinationCIDR)) routeInfo, found := fakeRoutes.FakeStore[cloud.RouteTableName][routeName] @@ -154,6 +157,24 @@ func TestCreateRoute(t *testing.T) { t.Errorf("Expected IP address: %s, saw %s", nodeIP, *routeInfo.NextHopIPAddress) } + // test create again without real creation, clean fakeRoute calls + fakeRoutes.Calls = []string{} + routeInfo.Name = &routeName + route.Name = routeName + expectedTable.RouteTablePropertiesFormat = &network.RouteTablePropertiesFormat{ + Routes: &[]network.Route{routeInfo}, + } + cloud.rtCache.Set(cloud.RouteTableName, &expectedTable) + + err = cloud.CreateRoute(context.TODO(), "cluster", "unused", &route) + if err != nil { + t.Errorf("unexpected error creating route: %v", err) + t.FailNow() + } + if len(fakeRoutes.Calls) != 0 { + t.Errorf("unexpected route calls create if not exists, exists: %v", fakeRoutes.Calls) + } + // test create route for unmanaged nodes. nodeName := "node1" nodeCIDR := "4.3.2.1/24"