From be2c9139f56c6695341d8727edf4e088ce19229d Mon Sep 17 00:00:00 2001 From: Zhecheng Li Date: Fri, 11 Feb 2022 16:36:02 +0800 Subject: [PATCH] Route controller should update routes with NodeIP changed When a node reboots or kubelet restarts, it is possible that its IP is changed. In this case, node route should be updated with the correct IP. In this PR, it checks if the IP in an existing route is the same as the actual one. If not, it marks it as "update" so the old route will be deleted and a new one will be created. There's a new field EnableNodeAddresses, which is a feature gate for specific cloud providers to enable after they update their cloud provider code for CreateRoute(). Signed-off-by: Zhecheng Li --- staging/src/k8s.io/cloud-provider/cloud.go | 5 + .../controllers/route/route_controller.go | 244 ++++++++---- .../route/route_controller_test.go | 356 ++++++++++-------- 3 files changed, 386 insertions(+), 219 deletions(-) diff --git a/staging/src/k8s.io/cloud-provider/cloud.go b/staging/src/k8s.io/cloud-provider/cloud.go index 44c62ccc03a..7e7bf9dfab8 100644 --- a/staging/src/k8s.io/cloud-provider/cloud.go +++ b/staging/src/k8s.io/cloud-provider/cloud.go @@ -218,6 +218,11 @@ type Route struct { Name string // TargetNode is the NodeName of the target instance. TargetNode types.NodeName + // EnableNodeAddresses is a feature gate for TargetNodeAddresses. If false, ignore TargetNodeAddresses. + // Without this, if users haven't updated their cloud-provider, reconcile() will delete and create same route every time. + EnableNodeAddresses bool + // TargetNodeAddresses are the Node IPs of the target Node. + TargetNodeAddresses []v1.NodeAddress // DestinationCIDR is the CIDR format IP range that this routing rule // applies to. DestinationCIDR string diff --git a/staging/src/k8s.io/cloud-provider/controllers/route/route_controller.go b/staging/src/k8s.io/cloud-provider/controllers/route/route_controller.go index 65f18fc41b0..ffe87e96e1b 100644 --- a/staging/src/k8s.io/cloud-provider/controllers/route/route_controller.go +++ b/staging/src/k8s.io/cloud-provider/controllers/route/route_controller.go @@ -20,6 +20,7 @@ import ( "context" "fmt" "net" + "reflect" "sync" "time" @@ -46,9 +47,9 @@ import ( ) const ( - // Maximal number of concurrent CreateRoute API calls. + // Maximal number of concurrent route operation API calls. // TODO: This should be per-provider. - maxConcurrentRouteCreations int = 200 + maxConcurrentRouteOperations int = 200 ) var updateNetworkConditionBackoff = wait.Backoff{ @@ -135,22 +136,56 @@ func (rc *RouteController) reconcileNodeRoutes(ctx context.Context) error { return rc.reconcile(ctx, nodes, routeList) } +type routeAction string + +var ( + keep routeAction = "keep" + add routeAction = "add" + remove routeAction = "remove" + update routeAction = "update" +) + +type routeNode struct { + name types.NodeName + addrs []v1.NodeAddress + routes []*cloudprovider.Route + cidrWithActions *map[string]routeAction +} + func (rc *RouteController) reconcile(ctx context.Context, nodes []*v1.Node, routes []*cloudprovider.Route) error { var l sync.Mutex - // for each node a map of podCIDRs and their created status - nodeRoutesStatuses := make(map[types.NodeName]map[string]bool) - // routeMap maps routeTargetNode->route - routeMap := make(map[types.NodeName][]*cloudprovider.Route) + // routeMap includes info about a target Node and its addresses, routes and a map between Pod CIDRs and actions. + // If action is add/remove, the route will be added/removed. + // If action is keep, the route will not be touched. + // If action is update, the route will be deleted and then added. + routeMap := make(map[types.NodeName]routeNode) + + // Put current routes into routeMap. for _, route := range routes { - if route.TargetNode != "" { - routeMap[route.TargetNode] = append(routeMap[route.TargetNode], route) + if route.TargetNode == "" { + continue } + rn, ok := routeMap[route.TargetNode] + if !ok { + rn = routeNode{ + name: route.TargetNode, + addrs: []v1.NodeAddress{}, + routes: []*cloudprovider.Route{}, + cidrWithActions: &map[string]routeAction{}, + } + } else if rn.routes == nil { + rn.routes = []*cloudprovider.Route{} + } + rn.routes = append(rn.routes, route) + routeMap[route.TargetNode] = rn } wg := sync.WaitGroup{} - rateLimiter := make(chan struct{}, maxConcurrentRouteCreations) + rateLimiter := make(chan struct{}, maxConcurrentRouteOperations) // searches existing routes by node for a matching route + // Check Nodes and their Pod CIDRs. Then put expected route actions into nodePodCIDRActionMap. + // Add addresses of Nodes into routeMap. for _, node := range nodes { // Skip if the node hasn't been assigned a CIDR yet. if len(node.Spec.PodCIDRs) == 0 { @@ -158,26 +193,101 @@ func (rc *RouteController) reconcile(ctx context.Context, nodes []*v1.Node, rout } nodeName := types.NodeName(node.Name) l.Lock() - nodeRoutesStatuses[nodeName] = make(map[string]bool) + rn, ok := routeMap[nodeName] + if !ok { + rn = routeNode{ + name: nodeName, + addrs: []v1.NodeAddress{}, + routes: []*cloudprovider.Route{}, + cidrWithActions: &map[string]routeAction{}, + } + } + rn.addrs = node.Status.Addresses + routeMap[nodeName] = rn l.Unlock() // for every node, for every cidr for _, podCIDR := range node.Spec.PodCIDRs { - // we add it to our nodeCIDRs map here because add and delete go routines run at the same time + // we add it to our nodeCIDRs map here because if we don't consider Node addresses change, + // add and delete go routines run simultaneously. l.Lock() - nodeRoutesStatuses[nodeName][podCIDR] = false + action := getRouteAction(rn.routes, podCIDR, nodeName, node.Status.Addresses) + (*routeMap[nodeName].cidrWithActions)[podCIDR] = action l.Unlock() - // ignore if already created - if hasRoute(routeMap, nodeName, podCIDR) { - l.Lock() - nodeRoutesStatuses[nodeName][podCIDR] = true // a route for this podCIDR is already created - l.Unlock() + klog.Infof("action for Node %q with CIDR %q: %q", nodeName, podCIDR, action) + } + } + + // searches our bag of node -> cidrs for a match + // If the action doesn't exist, action is remove or update, then the route should be deleted. + shouldDeleteRoute := func(nodeName types.NodeName, cidr string) bool { + l.Lock() + defer l.Unlock() + + cidrWithActions := routeMap[nodeName].cidrWithActions + if cidrWithActions == nil { + return true + } + action, exist := (*cidrWithActions)[cidr] + if !exist || action == remove || action == update { + klog.Infof("route should be deleted, spec: exist: %v, action: %q, Node %q, CIDR %q", exist, action, nodeName, cidr) + return true + } + return false + } + + // remove routes that are not in use or need to be updated. + for _, route := range routes { + if !rc.isResponsibleForRoute(route) { + continue + } + // Check if this route is a blackhole, or applies to a node we know about & CIDR status is created. + if route.Blackhole || shouldDeleteRoute(route.TargetNode, route.DestinationCIDR) { + wg.Add(1) + // Delete the route. + go func(route *cloudprovider.Route, startTime time.Time) { + defer wg.Done() + // respect the rate limiter + rateLimiter <- struct{}{} + klog.Infof("Deleting route %s %s", route.Name, route.DestinationCIDR) + if err := rc.routes.DeleteRoute(ctx, rc.clusterName, route); err != nil { + klog.Errorf("Could not delete route %s %s after %v: %v", route.Name, route.DestinationCIDR, time.Since(startTime), err) + } else { + klog.Infof("Deleted route %s %s after %v", route.Name, route.DestinationCIDR, time.Since(startTime)) + } + <-rateLimiter + }(route, time.Now()) + } + } + // https://github.com/kubernetes/kubernetes/issues/98359 + // When routesUpdated is true, Route addition and deletion cannot run simultaneously because if action is update, + // the same route may be added and deleted. + if len(routes) != 0 && routes[0].EnableNodeAddresses { + wg.Wait() + } + + // Now create new routes or update existing ones. + for _, node := range nodes { + // Skip if the node hasn't been assigned a CIDR yet. + if len(node.Spec.PodCIDRs) == 0 { + continue + } + nodeName := types.NodeName(node.Name) + + // for every node, for every cidr + for _, podCIDR := range node.Spec.PodCIDRs { + l.Lock() + action := (*routeMap[nodeName].cidrWithActions)[podCIDR] + l.Unlock() + if action == keep || action == remove { continue } // if we are here, then a route needs to be created for this node route := &cloudprovider.Route{ - TargetNode: nodeName, - DestinationCIDR: podCIDR, + TargetNode: nodeName, + TargetNodeAddresses: node.Status.Addresses, + DestinationCIDR: podCIDR, } + klog.Infof("route spec to be created: %v", route) // cloud providers that: // - depend on nameHint // - trying to support dual stack @@ -188,7 +298,7 @@ func (rc *RouteController) reconcile(ctx context.Context, nodes []*v1.Node, rout defer wg.Done() err := clientretry.RetryOnConflict(updateNetworkConditionBackoff, func() error { startTime := time.Now() - // Ensure that we don't have more than maxConcurrentRouteCreations + // Ensure that we don't have more than maxConcurrentRouteOperations // CreateRoute calls in flight. rateLimiter <- struct{}{} klog.Infof("Creating route for node %s %s with hint %s, throttled %v", nodeName, route.DestinationCIDR, nameHint, time.Since(startTime)) @@ -209,7 +319,8 @@ func (rc *RouteController) reconcile(ctx context.Context, nodes []*v1.Node, rout } } l.Lock() - nodeRoutesStatuses[nodeName][route.DestinationCIDR] = true + // Mark the route action as done (keep) + (*routeMap[nodeName].cidrWithActions)[route.DestinationCIDR] = keep l.Unlock() klog.Infof("Created route for node %s %s with hint %s after %v", nodeName, route.DestinationCIDR, nameHint, time.Since(startTime)) return nil @@ -220,64 +331,32 @@ func (rc *RouteController) reconcile(ctx context.Context, nodes []*v1.Node, rout }(nodeName, nameHint, route) } } - - // searches our bag of node->cidrs for a match - nodeHasCidr := func(nodeName types.NodeName, cidr string) bool { - l.Lock() - defer l.Unlock() - - nodeRoutes := nodeRoutesStatuses[nodeName] - if nodeRoutes == nil { - return false - } - _, exist := nodeRoutes[cidr] - return exist - } - // delete routes that are not in use - for _, route := range routes { - if rc.isResponsibleForRoute(route) { - // Check if this route is a blackhole, or applies to a node we know about & has an incorrect CIDR. - if route.Blackhole || !nodeHasCidr(route.TargetNode, route.DestinationCIDR) { - wg.Add(1) - // Delete the route. - go func(route *cloudprovider.Route, startTime time.Time) { - defer wg.Done() - // respect the rate limiter - rateLimiter <- struct{}{} - klog.Infof("Deleting route %s %s", route.Name, route.DestinationCIDR) - if err := rc.routes.DeleteRoute(ctx, rc.clusterName, route); err != nil { - klog.Errorf("Could not delete route %s %s after %v: %v", route.Name, route.DestinationCIDR, time.Since(startTime), err) - } else { - klog.Infof("Deleted route %s %s after %v", route.Name, route.DestinationCIDR, time.Since(startTime)) - } - <-rateLimiter - }(route, time.Now()) - } - } - } wg.Wait() - // after all routes have been created (or not), we start updating + // after all route actions have been done (or not), we start updating // all nodes' statuses with the outcome for _, node := range nodes { - wg.Add(1) - nodeRoutes := nodeRoutesStatuses[types.NodeName(node.Name)] - allRoutesCreated := true + actions := routeMap[types.NodeName(node.Name)].cidrWithActions + if actions == nil { + continue + } - if len(nodeRoutes) == 0 { + wg.Add(1) + if len(*actions) == 0 { go func(n *v1.Node) { defer wg.Done() klog.Infof("node %v has no routes assigned to it. NodeNetworkUnavailable will be set to true", n.Name) if err := rc.updateNetworkingCondition(n, false); err != nil { - klog.Errorf("failed to update networking condition when no nodeRoutes: %v", err) + klog.Errorf("failed to update networking condition when no actions: %v", err) } }(node) continue } - // check if all routes were created. if so, then it should be ready - for _, created := range nodeRoutes { - if !created { + // check if all route actions were done. if so, then it should be ready + allRoutesCreated := true + for _, action := range *actions { + if action == add || action == update { allRoutesCreated = false break } @@ -365,14 +444,35 @@ func (rc *RouteController) isResponsibleForRoute(route *cloudprovider.Route) boo return false } -// checks if a node owns a route with a specific cidr -func hasRoute(rm map[types.NodeName][]*cloudprovider.Route, nodeName types.NodeName, cidr string) bool { - if routes, ok := rm[nodeName]; ok { - for _, route := range routes { - if route.DestinationCIDR == cidr { - return true +// getRouteAction returns an action according to if there's a route matches a specific cidr and target Node addresses. +func getRouteAction(routes []*cloudprovider.Route, cidr string, nodeName types.NodeName, realNodeAddrs []v1.NodeAddress) routeAction { + for _, route := range routes { + if route.DestinationCIDR == cidr { + if !route.EnableNodeAddresses || equalNodeAddrs(realNodeAddrs, route.TargetNodeAddresses) { + return keep } + klog.Infof("Node addresses have changed from %v to %v", route.TargetNodeAddresses, realNodeAddrs) + return update } } - return false + return add +} + +func equalNodeAddrs(addrs0 []v1.NodeAddress, addrs1 []v1.NodeAddress) bool { + if len(addrs0) != len(addrs1) { + return false + } + for _, ip0 := range addrs0 { + found := false + for _, ip1 := range addrs1 { + if reflect.DeepEqual(ip0, ip1) { + found = true + break + } + } + if !found { + return false + } + } + return true } diff --git a/staging/src/k8s.io/cloud-provider/controllers/route/route_controller_test.go b/staging/src/k8s.io/cloud-provider/controllers/route/route_controller_test.go index e9297a2782c..03bf423a6e5 100644 --- a/staging/src/k8s.io/cloud-provider/controllers/route/route_controller_test.go +++ b/staging/src/k8s.io/cloud-provider/controllers/route/route_controller_test.go @@ -32,6 +32,8 @@ import ( fakecloud "k8s.io/cloud-provider/fake" nodeutil "k8s.io/component-helpers/node/util" netutils "k8s.io/utils/net" + + "github.com/stretchr/testify/assert" ) func alwaysReady() bool { return true } @@ -82,14 +84,16 @@ func TestIsResponsibleForRoute(t *testing.T) { func TestReconcile(t *testing.T) { cluster := "my-k8s" - node1 := v1.Node{ObjectMeta: metav1.ObjectMeta{Name: "node-1", UID: "01"}, Spec: v1.NodeSpec{PodCIDR: "10.120.0.0/24", PodCIDRs: []string{"10.120.0.0/24"}}} - node2 := v1.Node{ObjectMeta: metav1.ObjectMeta{Name: "node-2", UID: "02"}, Spec: v1.NodeSpec{PodCIDR: "10.120.1.0/24", PodCIDRs: []string{"10.120.1.0/24"}}} - nodeNoCidr := v1.Node{ObjectMeta: metav1.ObjectMeta{Name: "node-2", UID: "02"}, Spec: v1.NodeSpec{PodCIDR: ""}} + node1 := v1.Node{ObjectMeta: metav1.ObjectMeta{Name: "node-1", UID: "01"}, Spec: v1.NodeSpec{PodCIDR: "10.120.0.0/24", PodCIDRs: []string{"10.120.0.0/24"}}, Status: v1.NodeStatus{Addresses: []v1.NodeAddress{{Type: v1.NodeInternalIP, Address: "10.0.1.1"}}}} + // node1NoAddr := v1.Node{ObjectMeta: metav1.ObjectMeta{Name: "node-1", UID: "01"}, Spec: v1.NodeSpec{PodCIDR: "10.120.0.0/24", PodCIDRs: []string{"10.120.0.0/24"}}} + node2 := v1.Node{ObjectMeta: metav1.ObjectMeta{Name: "node-2", UID: "02"}, Spec: v1.NodeSpec{PodCIDR: "10.120.1.0/24", PodCIDRs: []string{"10.120.1.0/24"}}, Status: v1.NodeStatus{Addresses: []v1.NodeAddress{{Type: v1.NodeInternalIP, Address: "10.0.2.1"}}}} + nodeNoCidr := v1.Node{ObjectMeta: metav1.ObjectMeta{Name: "node-2", UID: "02"}, Spec: v1.NodeSpec{PodCIDR: ""}, Status: v1.NodeStatus{Addresses: []v1.NodeAddress{{Type: v1.NodeInternalIP, Address: "10.0.5.1"}}}} - node3 := v1.Node{ObjectMeta: metav1.ObjectMeta{Name: "node-3", UID: "03"}, Spec: v1.NodeSpec{PodCIDR: "10.120.0.0/24", PodCIDRs: []string{"10.120.0.0/24", "a00:100::/24"}}} - node4 := v1.Node{ObjectMeta: metav1.ObjectMeta{Name: "node-4", UID: "04"}, Spec: v1.NodeSpec{PodCIDR: "10.120.1.0/24", PodCIDRs: []string{"10.120.1.0/24", "a00:200::/24"}}} + node3 := v1.Node{ObjectMeta: metav1.ObjectMeta{Name: "node-3", UID: "03"}, Spec: v1.NodeSpec{PodCIDR: "10.120.0.0/24", PodCIDRs: []string{"10.120.0.0/24", "a00:100::/24"}}, Status: v1.NodeStatus{Addresses: []v1.NodeAddress{{Type: v1.NodeInternalIP, Address: "10.0.3.1"}}}} + node4 := v1.Node{ObjectMeta: metav1.ObjectMeta{Name: "node-4", UID: "04"}, Spec: v1.NodeSpec{PodCIDR: "10.120.1.0/24", PodCIDRs: []string{"10.120.1.0/24", "a00:200::/24"}}, Status: v1.NodeStatus{Addresses: []v1.NodeAddress{{Type: v1.NodeInternalIP, Address: "10.0.4.1"}}}} testCases := []struct { + description string nodes []*v1.Node initialRoutes []*cloudprovider.Route expectedRoutes []*cloudprovider.Route @@ -97,291 +101,348 @@ func TestReconcile(t *testing.T) { clientset *fake.Clientset dualStack bool }{ - // multicidr - // 2 nodes, no routes yet { - dualStack: true, + description: "routes have no TargetNodeAddresses at the beginning", + dualStack: true, + nodes: []*v1.Node{ + &node3, + &node4, + }, + initialRoutes: []*cloudprovider.Route{ + {Name: cluster + "-01", TargetNode: "node-3", DestinationCIDR: "10.120.0.0/24", Blackhole: false, EnableNodeAddresses: true}, + {Name: cluster + "-02", TargetNode: "node-4", DestinationCIDR: "10.120.1.0/24", Blackhole: false, EnableNodeAddresses: true}, + + {Name: cluster + "-03", TargetNode: "node-3", DestinationCIDR: "a00:100::/24", Blackhole: false, EnableNodeAddresses: true}, + {Name: cluster + "-04", TargetNode: "node-4", DestinationCIDR: "a00:200::/24", Blackhole: false, EnableNodeAddresses: true}, + }, + expectedRoutes: []*cloudprovider.Route{ + {Name: cluster + "-01", TargetNode: "node-3", TargetNodeAddresses: []v1.NodeAddress{{Type: v1.NodeInternalIP, Address: "10.0.3.1"}}, DestinationCIDR: "10.120.0.0/24", Blackhole: false, EnableNodeAddresses: true}, + {Name: cluster + "-02", TargetNode: "node-4", TargetNodeAddresses: []v1.NodeAddress{{Type: v1.NodeInternalIP, Address: "10.0.4.1"}}, DestinationCIDR: "10.120.1.0/24", Blackhole: false, EnableNodeAddresses: true}, + + {Name: cluster + "-03", TargetNode: "node-3", TargetNodeAddresses: []v1.NodeAddress{{Type: v1.NodeInternalIP, Address: "10.0.3.1"}}, DestinationCIDR: "a00:100::/24", Blackhole: false, EnableNodeAddresses: true}, + {Name: cluster + "-04", TargetNode: "node-4", TargetNodeAddresses: []v1.NodeAddress{{Type: v1.NodeInternalIP, Address: "10.0.4.1"}}, DestinationCIDR: "a00:200::/24", Blackhole: false, EnableNodeAddresses: true}, + }, + expectedNetworkUnavailable: []bool{true, true}, + clientset: fake.NewSimpleClientset(&v1.NodeList{Items: []v1.Node{node1, node2}}), + }, + { + description: "routes' TargetNodeAddresses changed", + dualStack: true, + nodes: []*v1.Node{ + &node3, + &node4, + }, + initialRoutes: []*cloudprovider.Route{ + {Name: cluster + "-01", TargetNode: "node-3", TargetNodeAddresses: []v1.NodeAddress{{Type: v1.NodeInternalIP, Address: "10.0.13.1"}}, DestinationCIDR: "10.120.0.0/24", Blackhole: false, EnableNodeAddresses: true}, + {Name: cluster + "-02", TargetNode: "node-4", TargetNodeAddresses: []v1.NodeAddress{{Type: v1.NodeInternalIP, Address: "10.0.14.1"}}, DestinationCIDR: "10.120.1.0/24", Blackhole: false, EnableNodeAddresses: true}, + + {Name: cluster + "-03", TargetNode: "node-3", TargetNodeAddresses: []v1.NodeAddress{{Type: v1.NodeInternalIP, Address: "10.0.13.1"}}, DestinationCIDR: "a00:100::/24", Blackhole: false, EnableNodeAddresses: true}, + {Name: cluster + "-04", TargetNode: "node-4", TargetNodeAddresses: []v1.NodeAddress{{Type: v1.NodeInternalIP, Address: "10.0.14.1"}}, DestinationCIDR: "a00:200::/24", Blackhole: false, EnableNodeAddresses: true}, + }, + expectedRoutes: []*cloudprovider.Route{ + {Name: cluster + "-01", TargetNode: "node-3", TargetNodeAddresses: []v1.NodeAddress{{Type: v1.NodeInternalIP, Address: "10.0.3.1"}}, DestinationCIDR: "10.120.0.0/24", Blackhole: false, EnableNodeAddresses: true}, + {Name: cluster + "-02", TargetNode: "node-4", TargetNodeAddresses: []v1.NodeAddress{{Type: v1.NodeInternalIP, Address: "10.0.4.1"}}, DestinationCIDR: "10.120.1.0/24", Blackhole: false, EnableNodeAddresses: true}, + + {Name: cluster + "-03", TargetNode: "node-3", TargetNodeAddresses: []v1.NodeAddress{{Type: v1.NodeInternalIP, Address: "10.0.3.1"}}, DestinationCIDR: "a00:100::/24", Blackhole: false, EnableNodeAddresses: true}, + {Name: cluster + "-04", TargetNode: "node-4", TargetNodeAddresses: []v1.NodeAddress{{Type: v1.NodeInternalIP, Address: "10.0.4.1"}}, DestinationCIDR: "a00:200::/24", Blackhole: false, EnableNodeAddresses: true}, + }, + expectedNetworkUnavailable: []bool{true, true}, + clientset: fake.NewSimpleClientset(&v1.NodeList{Items: []v1.Node{node1, node2}}), + }, + { + description: "multicidr 2 nodes and no routes", + dualStack: true, nodes: []*v1.Node{ &node3, &node4, }, initialRoutes: []*cloudprovider.Route{}, expectedRoutes: []*cloudprovider.Route{ - {Name: cluster + "-01", TargetNode: "node-3", DestinationCIDR: "10.120.0.0/24", Blackhole: false}, - {Name: cluster + "-02", TargetNode: "node-4", DestinationCIDR: "10.120.1.0/24", Blackhole: false}, + {Name: cluster + "-01", TargetNode: "node-3", TargetNodeAddresses: []v1.NodeAddress{{Type: v1.NodeInternalIP, Address: "10.0.3.1"}}, DestinationCIDR: "10.120.0.0/24", Blackhole: false}, + {Name: cluster + "-02", TargetNode: "node-4", TargetNodeAddresses: []v1.NodeAddress{{Type: v1.NodeInternalIP, Address: "10.0.4.1"}}, DestinationCIDR: "10.120.1.0/24", Blackhole: false}, - {Name: cluster + "-03", TargetNode: "node-3", DestinationCIDR: "a00:100::/24", Blackhole: false}, - {Name: cluster + "-04", TargetNode: "node-4", DestinationCIDR: "a00:200::/24", Blackhole: false}, + {Name: cluster + "-03", TargetNode: "node-3", TargetNodeAddresses: []v1.NodeAddress{{Type: v1.NodeInternalIP, Address: "10.0.3.1"}}, DestinationCIDR: "a00:100::/24", Blackhole: false}, + {Name: cluster + "-04", TargetNode: "node-4", TargetNodeAddresses: []v1.NodeAddress{{Type: v1.NodeInternalIP, Address: "10.0.4.1"}}, DestinationCIDR: "a00:200::/24", Blackhole: false}, }, expectedNetworkUnavailable: []bool{true, true}, clientset: fake.NewSimpleClientset(&v1.NodeList{Items: []v1.Node{node1, node2}}), }, - // 2 nodes, all routes already created { - dualStack: true, + description: "multicidr 2 nodes and all routes created", + dualStack: true, nodes: []*v1.Node{ &node3, &node4, }, initialRoutes: []*cloudprovider.Route{ - {Name: cluster + "-01", TargetNode: "node-3", DestinationCIDR: "10.120.0.0/24", Blackhole: false}, - {Name: cluster + "-02", TargetNode: "node-4", DestinationCIDR: "10.120.1.0/24", Blackhole: false}, + {Name: cluster + "-01", TargetNode: "node-3", TargetNodeAddresses: []v1.NodeAddress{{Type: v1.NodeInternalIP, Address: "10.0.3.1"}}, DestinationCIDR: "10.120.0.0/24", Blackhole: false}, + {Name: cluster + "-02", TargetNode: "node-4", TargetNodeAddresses: []v1.NodeAddress{{Type: v1.NodeInternalIP, Address: "10.0.4.1"}}, DestinationCIDR: "10.120.1.0/24", Blackhole: false}, - {Name: cluster + "-03", TargetNode: "node-3", DestinationCIDR: "a00:100::/24", Blackhole: false}, - {Name: cluster + "-04", TargetNode: "node-4", DestinationCIDR: "a00:200::/24", Blackhole: false}, + {Name: cluster + "-03", TargetNode: "node-3", TargetNodeAddresses: []v1.NodeAddress{{Type: v1.NodeInternalIP, Address: "10.0.3.1"}}, DestinationCIDR: "a00:100::/24", Blackhole: false}, + {Name: cluster + "-04", TargetNode: "node-4", TargetNodeAddresses: []v1.NodeAddress{{Type: v1.NodeInternalIP, Address: "10.0.4.1"}}, DestinationCIDR: "a00:200::/24", Blackhole: false}, }, expectedRoutes: []*cloudprovider.Route{ - {Name: cluster + "-01", TargetNode: "node-3", DestinationCIDR: "10.120.0.0/24", Blackhole: false}, - {Name: cluster + "-02", TargetNode: "node-4", DestinationCIDR: "10.120.1.0/24", Blackhole: false}, + {Name: cluster + "-01", TargetNode: "node-3", TargetNodeAddresses: []v1.NodeAddress{{Type: v1.NodeInternalIP, Address: "10.0.3.1"}}, DestinationCIDR: "10.120.0.0/24", Blackhole: false}, + {Name: cluster + "-02", TargetNode: "node-4", TargetNodeAddresses: []v1.NodeAddress{{Type: v1.NodeInternalIP, Address: "10.0.4.1"}}, DestinationCIDR: "10.120.1.0/24", Blackhole: false}, - {Name: cluster + "-03", TargetNode: "node-3", DestinationCIDR: "a00:100::/24", Blackhole: false}, - {Name: cluster + "-04", TargetNode: "node-4", DestinationCIDR: "a00:200::/24", Blackhole: false}, + {Name: cluster + "-03", TargetNode: "node-3", TargetNodeAddresses: []v1.NodeAddress{{Type: v1.NodeInternalIP, Address: "10.0.3.1"}}, DestinationCIDR: "a00:100::/24", Blackhole: false}, + {Name: cluster + "-04", TargetNode: "node-4", TargetNodeAddresses: []v1.NodeAddress{{Type: v1.NodeInternalIP, Address: "10.0.4.1"}}, DestinationCIDR: "a00:200::/24", Blackhole: false}, }, expectedNetworkUnavailable: []bool{true, true}, clientset: fake.NewSimpleClientset(&v1.NodeList{Items: []v1.Node{node1, node2}}), }, - // 2 nodes, few wrong routes { - dualStack: true, + description: "multicidr 2 nodes and few wrong routes", + dualStack: true, nodes: []*v1.Node{ &node3, &node4, }, initialRoutes: []*cloudprovider.Route{ - {Name: cluster + "-01", TargetNode: "node-3", DestinationCIDR: "10.120.1.0/24", Blackhole: false}, - {Name: cluster + "-02", TargetNode: "node-4", DestinationCIDR: "10.120.0.0/24", Blackhole: false}, + {Name: cluster + "-01", TargetNode: "node-3", TargetNodeAddresses: []v1.NodeAddress{{Type: v1.NodeInternalIP, Address: "10.0.3.1"}}, DestinationCIDR: "10.120.1.0/24", Blackhole: false}, + {Name: cluster + "-02", TargetNode: "node-4", TargetNodeAddresses: []v1.NodeAddress{{Type: v1.NodeInternalIP, Address: "10.0.4.1"}}, DestinationCIDR: "10.120.0.0/24", Blackhole: false}, - {Name: cluster + "-03", TargetNode: "node-3", DestinationCIDR: "a00:200::/24", Blackhole: false}, - {Name: cluster + "-04", TargetNode: "node-4", DestinationCIDR: "a00:100::/24", Blackhole: false}, + {Name: cluster + "-03", TargetNode: "node-3", TargetNodeAddresses: []v1.NodeAddress{{Type: v1.NodeInternalIP, Address: "10.0.3.1"}}, DestinationCIDR: "a00:200::/24", Blackhole: false}, + {Name: cluster + "-04", TargetNode: "node-4", TargetNodeAddresses: []v1.NodeAddress{{Type: v1.NodeInternalIP, Address: "10.0.4.1"}}, DestinationCIDR: "a00:100::/24", Blackhole: false}, }, expectedRoutes: []*cloudprovider.Route{ - {Name: cluster + "-01", TargetNode: "node-3", DestinationCIDR: "10.120.0.0/24", Blackhole: false}, - {Name: cluster + "-02", TargetNode: "node-4", DestinationCIDR: "10.120.1.0/24", Blackhole: false}, + {Name: cluster + "-01", TargetNode: "node-3", TargetNodeAddresses: []v1.NodeAddress{{Type: v1.NodeInternalIP, Address: "10.0.3.1"}}, DestinationCIDR: "10.120.0.0/24", Blackhole: false}, + {Name: cluster + "-02", TargetNode: "node-4", TargetNodeAddresses: []v1.NodeAddress{{Type: v1.NodeInternalIP, Address: "10.0.4.1"}}, DestinationCIDR: "10.120.1.0/24", Blackhole: false}, - {Name: cluster + "-03", TargetNode: "node-3", DestinationCIDR: "a00:100::/24", Blackhole: false}, - {Name: cluster + "-04", TargetNode: "node-4", DestinationCIDR: "a00:200::/24", Blackhole: false}, + {Name: cluster + "-03", TargetNode: "node-3", TargetNodeAddresses: []v1.NodeAddress{{Type: v1.NodeInternalIP, Address: "10.0.3.1"}}, DestinationCIDR: "a00:100::/24", Blackhole: false}, + {Name: cluster + "-04", TargetNode: "node-4", TargetNodeAddresses: []v1.NodeAddress{{Type: v1.NodeInternalIP, Address: "10.0.4.1"}}, DestinationCIDR: "a00:200::/24", Blackhole: false}, }, expectedNetworkUnavailable: []bool{true, true}, clientset: fake.NewSimpleClientset(&v1.NodeList{Items: []v1.Node{node1, node2}}), }, - // 2 nodes, some routes already created { - dualStack: true, + description: "multicidr 2 nodes and some routes created", + dualStack: true, nodes: []*v1.Node{ &node3, &node4, }, initialRoutes: []*cloudprovider.Route{ - {Name: cluster + "-01", TargetNode: "node-3", DestinationCIDR: "10.120.0.0/24", Blackhole: false}, - {Name: cluster + "-04", TargetNode: "node-4", DestinationCIDR: "a00:200::/24", Blackhole: false}, + {Name: cluster + "-01", TargetNode: "node-3", TargetNodeAddresses: []v1.NodeAddress{{Type: v1.NodeInternalIP, Address: "10.0.3.1"}}, DestinationCIDR: "10.120.0.0/24", Blackhole: false}, + {Name: cluster + "-04", TargetNode: "node-4", TargetNodeAddresses: []v1.NodeAddress{{Type: v1.NodeInternalIP, Address: "10.0.4.1"}}, DestinationCIDR: "a00:200::/24", Blackhole: false}, }, expectedRoutes: []*cloudprovider.Route{ - {Name: cluster + "-01", TargetNode: "node-3", DestinationCIDR: "10.120.0.0/24", Blackhole: false}, - {Name: cluster + "-02", TargetNode: "node-4", DestinationCIDR: "10.120.1.0/24", Blackhole: false}, + {Name: cluster + "-01", TargetNode: "node-3", TargetNodeAddresses: []v1.NodeAddress{{Type: v1.NodeInternalIP, Address: "10.0.3.1"}}, DestinationCIDR: "10.120.0.0/24", Blackhole: false}, + {Name: cluster + "-02", TargetNode: "node-4", TargetNodeAddresses: []v1.NodeAddress{{Type: v1.NodeInternalIP, Address: "10.0.4.1"}}, DestinationCIDR: "10.120.1.0/24", Blackhole: false}, - {Name: cluster + "-03", TargetNode: "node-3", DestinationCIDR: "a00:100::/24", Blackhole: false}, - {Name: cluster + "-04", TargetNode: "node-4", DestinationCIDR: "a00:200::/24", Blackhole: false}, + {Name: cluster + "-03", TargetNode: "node-3", TargetNodeAddresses: []v1.NodeAddress{{Type: v1.NodeInternalIP, Address: "10.0.3.1"}}, DestinationCIDR: "a00:100::/24", Blackhole: false}, + {Name: cluster + "-04", TargetNode: "node-4", TargetNodeAddresses: []v1.NodeAddress{{Type: v1.NodeInternalIP, Address: "10.0.4.1"}}, DestinationCIDR: "a00:200::/24", Blackhole: false}, }, expectedNetworkUnavailable: []bool{true, true}, clientset: fake.NewSimpleClientset(&v1.NodeList{Items: []v1.Node{node1, node2}}), }, - // 2 nodes, too many routes { - dualStack: true, + description: "multicidr 2 nodes and too many routes", + dualStack: true, nodes: []*v1.Node{ &node3, &node4, }, initialRoutes: []*cloudprovider.Route{ - {Name: cluster + "-01", TargetNode: "node-3", DestinationCIDR: "10.120.0.0/24", Blackhole: false}, - {Name: cluster + "-02", TargetNode: "node-4", DestinationCIDR: "10.120.1.0/24", Blackhole: false}, - {Name: cluster + "-001", TargetNode: "node-x", DestinationCIDR: "10.120.2.0/24", Blackhole: false}, + {Name: cluster + "-01", TargetNode: "node-3", TargetNodeAddresses: []v1.NodeAddress{{Type: v1.NodeInternalIP, Address: "10.0.3.1"}}, DestinationCIDR: "10.120.0.0/24", Blackhole: false}, + {Name: cluster + "-02", TargetNode: "node-4", TargetNodeAddresses: []v1.NodeAddress{{Type: v1.NodeInternalIP, Address: "10.0.4.1"}}, DestinationCIDR: "10.120.1.0/24", Blackhole: false}, + {Name: cluster + "-001", TargetNode: "node-x", TargetNodeAddresses: []v1.NodeAddress{{Type: v1.NodeInternalIP, Address: "10.0.5.1"}}, DestinationCIDR: "10.120.2.0/24", Blackhole: false}, - {Name: cluster + "-03", TargetNode: "node-3", DestinationCIDR: "a00:100::/24", Blackhole: false}, - {Name: cluster + "-04", TargetNode: "node-4", DestinationCIDR: "a00:200::/24", Blackhole: false}, - {Name: cluster + "-0002", TargetNode: "node-y", DestinationCIDR: "a00:300::/24", Blackhole: false}, + {Name: cluster + "-03", TargetNode: "node-3", TargetNodeAddresses: []v1.NodeAddress{{Type: v1.NodeInternalIP, Address: "10.0.3.1"}}, DestinationCIDR: "a00:100::/24", Blackhole: false}, + {Name: cluster + "-04", TargetNode: "node-4", TargetNodeAddresses: []v1.NodeAddress{{Type: v1.NodeInternalIP, Address: "10.0.4.1"}}, DestinationCIDR: "a00:200::/24", Blackhole: false}, + {Name: cluster + "-0002", TargetNode: "node-y", TargetNodeAddresses: []v1.NodeAddress{{Type: v1.NodeInternalIP, Address: "10.0.6.1"}}, DestinationCIDR: "a00:300::/24", Blackhole: false}, }, expectedRoutes: []*cloudprovider.Route{ - {Name: cluster + "-01", TargetNode: "node-3", DestinationCIDR: "10.120.0.0/24", Blackhole: false}, - {Name: cluster + "-02", TargetNode: "node-4", DestinationCIDR: "10.120.1.0/24", Blackhole: false}, + {Name: cluster + "-01", TargetNode: "node-3", TargetNodeAddresses: []v1.NodeAddress{{Type: v1.NodeInternalIP, Address: "10.0.3.1"}}, DestinationCIDR: "10.120.0.0/24", Blackhole: false}, + {Name: cluster + "-02", TargetNode: "node-4", TargetNodeAddresses: []v1.NodeAddress{{Type: v1.NodeInternalIP, Address: "10.0.4.1"}}, DestinationCIDR: "10.120.1.0/24", Blackhole: false}, - {Name: cluster + "-03", TargetNode: "node-3", DestinationCIDR: "a00:100::/24", Blackhole: false}, - {Name: cluster + "-04", TargetNode: "node-4", DestinationCIDR: "a00:200::/24", Blackhole: false}, + {Name: cluster + "-03", TargetNode: "node-3", TargetNodeAddresses: []v1.NodeAddress{{Type: v1.NodeInternalIP, Address: "10.0.3.1"}}, DestinationCIDR: "a00:100::/24", Blackhole: false}, + {Name: cluster + "-04", TargetNode: "node-4", TargetNodeAddresses: []v1.NodeAddress{{Type: v1.NodeInternalIP, Address: "10.0.4.1"}}, DestinationCIDR: "a00:200::/24", Blackhole: false}, }, expectedNetworkUnavailable: []bool{true, true}, clientset: fake.NewSimpleClientset(&v1.NodeList{Items: []v1.Node{node1, node2}}), }, - - // single cidr - // 2 nodes, routes already there { + description: "single cidr 2 nodes and routes created", nodes: []*v1.Node{ &node1, &node2, }, initialRoutes: []*cloudprovider.Route{ - {Name: cluster + "-01", TargetNode: "node-1", DestinationCIDR: "10.120.0.0/24", Blackhole: false}, - {Name: cluster + "-02", TargetNode: "node-2", DestinationCIDR: "10.120.1.0/24", Blackhole: false}, + {Name: cluster + "-01", TargetNode: "node-1", TargetNodeAddresses: []v1.NodeAddress{{Type: v1.NodeInternalIP, Address: "10.0.1.1"}}, DestinationCIDR: "10.120.0.0/24", Blackhole: false}, + {Name: cluster + "-02", TargetNode: "node-2", TargetNodeAddresses: []v1.NodeAddress{{Type: v1.NodeInternalIP, Address: "10.0.2.1"}}, DestinationCIDR: "10.120.1.0/24", Blackhole: false}, }, expectedRoutes: []*cloudprovider.Route{ - {Name: cluster + "-01", TargetNode: "node-1", DestinationCIDR: "10.120.0.0/24", Blackhole: false}, - {Name: cluster + "-02", TargetNode: "node-2", DestinationCIDR: "10.120.1.0/24", Blackhole: false}, + {Name: cluster + "-01", TargetNode: "node-1", TargetNodeAddresses: []v1.NodeAddress{{Type: v1.NodeInternalIP, Address: "10.0.1.1"}}, DestinationCIDR: "10.120.0.0/24", Blackhole: false}, + {Name: cluster + "-02", TargetNode: "node-2", TargetNodeAddresses: []v1.NodeAddress{{Type: v1.NodeInternalIP, Address: "10.0.2.1"}}, DestinationCIDR: "10.120.1.0/24", Blackhole: false}, }, expectedNetworkUnavailable: []bool{true, true}, clientset: fake.NewSimpleClientset(&v1.NodeList{Items: []v1.Node{node1, node2}}), }, - // 2 nodes, one route already there { + description: "single cidr node ips changed so routes should be updated", nodes: []*v1.Node{ &node1, &node2, }, initialRoutes: []*cloudprovider.Route{ - {Name: cluster + "-01", TargetNode: "node-1", DestinationCIDR: "10.120.0.0/24", Blackhole: false}, + {Name: cluster + "-01", TargetNode: "node-1", TargetNodeAddresses: []v1.NodeAddress{{Type: v1.NodeInternalIP, Address: "10.0.1.2"}}, DestinationCIDR: "10.120.0.0/24", Blackhole: false, EnableNodeAddresses: true}, + {Name: cluster + "-02", TargetNode: "node-2", TargetNodeAddresses: []v1.NodeAddress{{Type: v1.NodeInternalIP, Address: "10.0.2.2"}}, DestinationCIDR: "10.120.1.0/24", Blackhole: false, EnableNodeAddresses: true}, }, expectedRoutes: []*cloudprovider.Route{ - {Name: cluster + "-01", TargetNode: "node-1", DestinationCIDR: "10.120.0.0/24", Blackhole: false}, - {Name: cluster + "-02", TargetNode: "node-2", DestinationCIDR: "10.120.1.0/24", Blackhole: false}, + {Name: cluster + "-01", TargetNode: "node-1", TargetNodeAddresses: []v1.NodeAddress{{Type: v1.NodeInternalIP, Address: "10.0.1.1"}}, DestinationCIDR: "10.120.0.0/24", Blackhole: false, EnableNodeAddresses: true}, + {Name: cluster + "-02", TargetNode: "node-2", TargetNodeAddresses: []v1.NodeAddress{{Type: v1.NodeInternalIP, Address: "10.0.2.1"}}, DestinationCIDR: "10.120.1.0/24", Blackhole: false, EnableNodeAddresses: true}, }, expectedNetworkUnavailable: []bool{true, true}, clientset: fake.NewSimpleClientset(&v1.NodeList{Items: []v1.Node{node1, node2}}), }, - // 2 nodes, no routes yet { + description: "single cidr 2 nodes and one route created", + nodes: []*v1.Node{ + &node1, + &node2, + }, + initialRoutes: []*cloudprovider.Route{ + {Name: cluster + "-01", TargetNode: "node-1", TargetNodeAddresses: []v1.NodeAddress{{Type: v1.NodeInternalIP, Address: "10.0.1.1"}}, DestinationCIDR: "10.120.0.0/24", Blackhole: false}, + }, + expectedRoutes: []*cloudprovider.Route{ + {Name: cluster + "-01", TargetNode: "node-1", TargetNodeAddresses: []v1.NodeAddress{{Type: v1.NodeInternalIP, Address: "10.0.1.1"}}, DestinationCIDR: "10.120.0.0/24", Blackhole: false}, + {Name: cluster + "-02", TargetNode: "node-2", TargetNodeAddresses: []v1.NodeAddress{{Type: v1.NodeInternalIP, Address: "10.0.2.1"}}, DestinationCIDR: "10.120.1.0/24", Blackhole: false}, + }, + expectedNetworkUnavailable: []bool{true, true}, + clientset: fake.NewSimpleClientset(&v1.NodeList{Items: []v1.Node{node1, node2}}), + }, + { + description: "single cidr 2 nodes and no routes", nodes: []*v1.Node{ &node1, &node2, }, initialRoutes: []*cloudprovider.Route{}, expectedRoutes: []*cloudprovider.Route{ - {Name: cluster + "-01", TargetNode: "node-1", DestinationCIDR: "10.120.0.0/24", Blackhole: false}, - {Name: cluster + "-02", TargetNode: "node-2", DestinationCIDR: "10.120.1.0/24", Blackhole: false}, + {Name: cluster + "-01", TargetNode: "node-1", TargetNodeAddresses: []v1.NodeAddress{{Type: v1.NodeInternalIP, Address: "10.0.1.1"}}, DestinationCIDR: "10.120.0.0/24", Blackhole: false}, + {Name: cluster + "-02", TargetNode: "node-2", TargetNodeAddresses: []v1.NodeAddress{{Type: v1.NodeInternalIP, Address: "10.0.2.1"}}, DestinationCIDR: "10.120.1.0/24", Blackhole: false}, }, expectedNetworkUnavailable: []bool{true, true}, clientset: fake.NewSimpleClientset(&v1.NodeList{Items: []v1.Node{node1, node2}}), }, - // 2 nodes, a few too many routes { + description: "single cidr 2 nodes and too many routes", nodes: []*v1.Node{ &node1, &node2, }, initialRoutes: []*cloudprovider.Route{ - {Name: cluster + "-01", TargetNode: "node-1", DestinationCIDR: "10.120.0.0/24", Blackhole: false}, - {Name: cluster + "-02", TargetNode: "node-2", DestinationCIDR: "10.120.1.0/24", Blackhole: false}, - {Name: cluster + "-03", TargetNode: "node-3", DestinationCIDR: "10.120.2.0/24", Blackhole: false}, - {Name: cluster + "-04", TargetNode: "node-4", DestinationCIDR: "10.120.3.0/24", Blackhole: false}, + {Name: cluster + "-01", TargetNode: "node-1", TargetNodeAddresses: []v1.NodeAddress{{Type: v1.NodeInternalIP, Address: "10.0.1.1"}}, DestinationCIDR: "10.120.0.0/24", Blackhole: false}, + {Name: cluster + "-02", TargetNode: "node-2", TargetNodeAddresses: []v1.NodeAddress{{Type: v1.NodeInternalIP, Address: "10.0.2.1"}}, DestinationCIDR: "10.120.1.0/24", Blackhole: false}, + {Name: cluster + "-03", TargetNode: "node-3", TargetNodeAddresses: []v1.NodeAddress{{Type: v1.NodeInternalIP, Address: "10.0.3.1"}}, DestinationCIDR: "10.120.2.0/24", Blackhole: false}, + {Name: cluster + "-04", TargetNode: "node-4", TargetNodeAddresses: []v1.NodeAddress{{Type: v1.NodeInternalIP, Address: "10.0.4.1"}}, DestinationCIDR: "10.120.3.0/24", Blackhole: false}, }, expectedRoutes: []*cloudprovider.Route{ - {Name: cluster + "-01", TargetNode: "node-1", DestinationCIDR: "10.120.0.0/24", Blackhole: false}, - {Name: cluster + "-02", TargetNode: "node-2", DestinationCIDR: "10.120.1.0/24", Blackhole: false}, + {Name: cluster + "-01", TargetNode: "node-1", TargetNodeAddresses: []v1.NodeAddress{{Type: v1.NodeInternalIP, Address: "10.0.1.1"}}, DestinationCIDR: "10.120.0.0/24", Blackhole: false}, + {Name: cluster + "-02", TargetNode: "node-2", TargetNodeAddresses: []v1.NodeAddress{{Type: v1.NodeInternalIP, Address: "10.0.2.1"}}, DestinationCIDR: "10.120.1.0/24", Blackhole: false}, }, expectedNetworkUnavailable: []bool{true, true}, clientset: fake.NewSimpleClientset(&v1.NodeList{Items: []v1.Node{node1, node2}}), }, - // 2 nodes, 2 routes, but only 1 is right { + description: "single cidr 2 nodes and 2 routes with 1 incorrect", nodes: []*v1.Node{ &node1, &node2, }, initialRoutes: []*cloudprovider.Route{ - {Name: cluster + "-01", TargetNode: "node-1", DestinationCIDR: "10.120.0.0/24", Blackhole: false}, - {Name: cluster + "-03", TargetNode: "node-3", DestinationCIDR: "10.120.2.0/24", Blackhole: false}, + {Name: cluster + "-01", TargetNode: "node-1", TargetNodeAddresses: []v1.NodeAddress{{Type: v1.NodeInternalIP, Address: "10.0.1.1"}}, DestinationCIDR: "10.120.0.0/24", Blackhole: false}, + {Name: cluster + "-03", TargetNode: "node-3", TargetNodeAddresses: []v1.NodeAddress{{Type: v1.NodeInternalIP, Address: "10.0.2.1"}}, DestinationCIDR: "10.120.2.0/24", Blackhole: false}, }, expectedRoutes: []*cloudprovider.Route{ - {Name: cluster + "-01", TargetNode: "node-1", DestinationCIDR: "10.120.0.0/24", Blackhole: false}, - {Name: cluster + "-02", TargetNode: "node-2", DestinationCIDR: "10.120.1.0/24", Blackhole: false}, + {Name: cluster + "-01", TargetNode: "node-1", TargetNodeAddresses: []v1.NodeAddress{{Type: v1.NodeInternalIP, Address: "10.0.1.1"}}, DestinationCIDR: "10.120.0.0/24", Blackhole: false}, + {Name: cluster + "-02", TargetNode: "node-2", TargetNodeAddresses: []v1.NodeAddress{{Type: v1.NodeInternalIP, Address: "10.0.2.1"}}, DestinationCIDR: "10.120.1.0/24", Blackhole: false}, }, expectedNetworkUnavailable: []bool{true, true}, clientset: fake.NewSimpleClientset(&v1.NodeList{Items: []v1.Node{node1, node2}}), }, - // 2 nodes, one node without CIDR assigned. { + description: "single cidr 2 nodes and one node without cidr assigned", nodes: []*v1.Node{ &node1, &nodeNoCidr, }, initialRoutes: []*cloudprovider.Route{}, expectedRoutes: []*cloudprovider.Route{ - {Name: cluster + "-01", TargetNode: "node-1", DestinationCIDR: "10.120.0.0/24", Blackhole: false}, + {Name: cluster + "-01", TargetNode: "node-1", TargetNodeAddresses: []v1.NodeAddress{{Type: v1.NodeInternalIP, Address: "10.0.1.1"}}, DestinationCIDR: "10.120.0.0/24", Blackhole: false}, }, expectedNetworkUnavailable: []bool{true, false}, clientset: fake.NewSimpleClientset(&v1.NodeList{Items: []v1.Node{node1, nodeNoCidr}}), }, - // 2 nodes, an extra blackhole route in our range { + description: "single cidr 2 nodes and an extra blackhole route in our range", nodes: []*v1.Node{ &node1, &node2, }, initialRoutes: []*cloudprovider.Route{ - {Name: cluster + "-01", TargetNode: "node-1", DestinationCIDR: "10.120.0.0/24", Blackhole: false}, - {Name: cluster + "-02", TargetNode: "node-2", DestinationCIDR: "10.120.1.0/24", Blackhole: false}, - {Name: cluster + "-03", TargetNode: "", DestinationCIDR: "10.120.2.0/24", Blackhole: true}, + {Name: cluster + "-01", TargetNode: "node-1", TargetNodeAddresses: []v1.NodeAddress{{Type: v1.NodeInternalIP, Address: "10.0.1.1"}}, DestinationCIDR: "10.120.0.0/24", Blackhole: false}, + {Name: cluster + "-02", TargetNode: "node-2", TargetNodeAddresses: []v1.NodeAddress{{Type: v1.NodeInternalIP, Address: "10.0.2.1"}}, DestinationCIDR: "10.120.1.0/24", Blackhole: false}, + {Name: cluster + "-03", TargetNode: "", TargetNodeAddresses: []v1.NodeAddress{{Type: v1.NodeInternalIP, Address: "10.0.100.1"}}, DestinationCIDR: "10.120.2.0/24", Blackhole: true}, }, expectedRoutes: []*cloudprovider.Route{ - {Name: cluster + "-01", TargetNode: "node-1", DestinationCIDR: "10.120.0.0/24", Blackhole: false}, - {Name: cluster + "-02", TargetNode: "node-2", DestinationCIDR: "10.120.1.0/24", Blackhole: false}, + {Name: cluster + "-01", TargetNode: "node-1", TargetNodeAddresses: []v1.NodeAddress{{Type: v1.NodeInternalIP, Address: "10.0.1.1"}}, DestinationCIDR: "10.120.0.0/24", Blackhole: false}, + {Name: cluster + "-02", TargetNode: "node-2", TargetNodeAddresses: []v1.NodeAddress{{Type: v1.NodeInternalIP, Address: "10.0.2.1"}}, DestinationCIDR: "10.120.1.0/24", Blackhole: false}, }, expectedNetworkUnavailable: []bool{true, true}, clientset: fake.NewSimpleClientset(&v1.NodeList{Items: []v1.Node{node1, node2}}), }, - // 2 nodes, an extra blackhole route not in our range { + description: "single cidr 2 nodes and an extra blackhole route not in our range", nodes: []*v1.Node{ &node1, &node2, }, initialRoutes: []*cloudprovider.Route{ - {Name: cluster + "-01", TargetNode: "node-1", DestinationCIDR: "10.120.0.0/24", Blackhole: false}, - {Name: cluster + "-02", TargetNode: "node-2", DestinationCIDR: "10.120.1.0/24", Blackhole: false}, - {Name: cluster + "-03", TargetNode: "", DestinationCIDR: "10.1.2.0/24", Blackhole: true}, + {Name: cluster + "-01", TargetNode: "node-1", TargetNodeAddresses: []v1.NodeAddress{{Type: v1.NodeInternalIP, Address: "10.0.1.1"}}, DestinationCIDR: "10.120.0.0/24", Blackhole: false}, + {Name: cluster + "-02", TargetNode: "node-2", TargetNodeAddresses: []v1.NodeAddress{{Type: v1.NodeInternalIP, Address: "10.0.2.1"}}, DestinationCIDR: "10.120.1.0/24", Blackhole: false}, + {Name: cluster + "-03", TargetNode: "", TargetNodeAddresses: []v1.NodeAddress{{Type: v1.NodeInternalIP, Address: "10.0.100.1"}}, DestinationCIDR: "10.1.2.0/24", Blackhole: true}, }, expectedRoutes: []*cloudprovider.Route{ - {Name: cluster + "-01", TargetNode: "node-1", DestinationCIDR: "10.120.0.0/24", Blackhole: false}, - {Name: cluster + "-02", TargetNode: "node-2", DestinationCIDR: "10.120.1.0/24", Blackhole: false}, - {Name: cluster + "-03", TargetNode: "", DestinationCIDR: "10.1.2.0/24", Blackhole: true}, + {Name: cluster + "-01", TargetNode: "node-1", TargetNodeAddresses: []v1.NodeAddress{{Type: v1.NodeInternalIP, Address: "10.0.1.1"}}, DestinationCIDR: "10.120.0.0/24", Blackhole: false}, + {Name: cluster + "-02", TargetNode: "node-2", TargetNodeAddresses: []v1.NodeAddress{{Type: v1.NodeInternalIP, Address: "10.0.2.1"}}, DestinationCIDR: "10.120.1.0/24", Blackhole: false}, + {Name: cluster + "-03", TargetNode: "", TargetNodeAddresses: []v1.NodeAddress{{Type: v1.NodeInternalIP, Address: "10.0.100.1"}}, DestinationCIDR: "10.1.2.0/24", Blackhole: true}, }, expectedNetworkUnavailable: []bool{true, true}, clientset: fake.NewSimpleClientset(&v1.NodeList{Items: []v1.Node{node1, node2}}), }, } - for i, testCase := range testCases { - ctx, cancel := context.WithCancel(context.Background()) - defer cancel() - cloud := &fakecloud.Cloud{RouteMap: make(map[string]*fakecloud.Route)} - for _, route := range testCase.initialRoutes { - fakeRoute := &fakecloud.Route{} - fakeRoute.ClusterName = cluster - fakeRoute.Route = *route - cloud.RouteMap[route.Name] = fakeRoute - } - routes, ok := cloud.Routes() - if !ok { - t.Error("Error in test: fakecloud doesn't support Routes()") - } - cidrs := make([]*net.IPNet, 0) - _, cidr, _ := netutils.ParseCIDRSloppy("10.120.0.0/16") - cidrs = append(cidrs, cidr) - if testCase.dualStack { - _, cidrv6, _ := netutils.ParseCIDRSloppy("ace:cab:deca::/8") - cidrs = append(cidrs, cidrv6) - } + for _, testCase := range testCases { + t.Run(testCase.description, func(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + cloud := &fakecloud.Cloud{RouteMap: make(map[string]*fakecloud.Route)} + for _, route := range testCase.initialRoutes { + fakeRoute := &fakecloud.Route{} + fakeRoute.ClusterName = cluster + fakeRoute.Route = *route + cloud.RouteMap[route.Name] = fakeRoute + } + routes, ok := cloud.Routes() + assert.True(t, ok, "fakecloud failed to run Routes()") + cidrs := make([]*net.IPNet, 0) + _, cidr, _ := netutils.ParseCIDRSloppy("10.120.0.0/16") + cidrs = append(cidrs, cidr) + if testCase.dualStack { + _, cidrv6, _ := netutils.ParseCIDRSloppy("ace:cab:deca::/8") + cidrs = append(cidrs, cidrv6) + } - informerFactory := informers.NewSharedInformerFactory(testCase.clientset, 0) - rc := New(routes, testCase.clientset, informerFactory.Core().V1().Nodes(), cluster, cidrs) - rc.nodeListerSynced = alwaysReady - if err := rc.reconcile(ctx, testCase.nodes, testCase.initialRoutes); err != nil { - t.Errorf("%d. Error from rc.reconcile(): %v", i, err) - } - for _, action := range testCase.clientset.Actions() { - if action.GetVerb() == "update" && action.GetResource().Resource == "nodes" { - node := action.(core.UpdateAction).GetObject().(*v1.Node) - _, condition := nodeutil.GetNodeCondition(&node.Status, v1.NodeNetworkUnavailable) - if condition == nil { - t.Errorf("%d. Missing NodeNetworkUnavailable condition for Node %v", i, node.Name) - } else { + informerFactory := informers.NewSharedInformerFactory(testCase.clientset, 0) + rc := New(routes, testCase.clientset, informerFactory.Core().V1().Nodes(), cluster, cidrs) + rc.nodeListerSynced = alwaysReady + assert.NoError(t, rc.reconcile(ctx, testCase.nodes, testCase.initialRoutes), "failed to reconcile") + for _, action := range testCase.clientset.Actions() { + if action.GetVerb() == "update" && action.GetResource().Resource == "nodes" { + node := action.(core.UpdateAction).GetObject().(*v1.Node) + _, condition := nodeutil.GetNodeCondition(&node.Status, v1.NodeNetworkUnavailable) + assert.NotEmpty(t, condition, "Missing NodeNetworkUnavailable condition for Node %q", node.Name) check := func(index int) bool { return (condition.Status == v1.ConditionFalse) == testCase.expectedNetworkUnavailable[index] } @@ -395,30 +456,30 @@ func TestReconcile(t *testing.T) { // Something's wrong continue } - if !check(index) { - t.Errorf("%d. Invalid NodeNetworkUnavailable condition for Node %v, expected %v, got %v", - i, node.Name, testCase.expectedNetworkUnavailable[index], (condition.Status == v1.ConditionFalse)) - } + assert.True(t, check(index), "Invalid NodeNetworkUnavailable condition for Node %q, expected %v, got %v", + node.Name, testCase.expectedNetworkUnavailable[index], (condition.Status == v1.ConditionFalse)) } } - } - var finalRoutes []*cloudprovider.Route - var err error - timeoutChan := time.After(200 * time.Millisecond) - tick := time.NewTicker(10 * time.Millisecond) - defer tick.Stop() - poll: - for { - select { - case <-tick.C: - if finalRoutes, err = routes.ListRoutes(ctx, cluster); err == nil && routeListEqual(finalRoutes, testCase.expectedRoutes) { + var finalRoutes []*cloudprovider.Route + var err error + timeoutChan := time.After(200 * time.Millisecond) + tick := time.NewTicker(10 * time.Millisecond) + defer tick.Stop() + poll: + for { + select { + case <-tick.C: + if finalRoutes, err = routes.ListRoutes(ctx, cluster); err == nil && routeListEqual(finalRoutes, testCase.expectedRoutes) { + break poll + } + case <-timeoutChan: + t.Errorf("rc.reconcile() err is %v,\nfound routes:\n%v\nexpected routes:\n%v\n", + err, flatten(finalRoutes), flatten(testCase.expectedRoutes)) break poll } - case <-timeoutChan: - t.Errorf("%d. rc.reconcile() = %v,\nfound routes:\n%v\nexpected routes:\n%v\n", i, err, flatten(finalRoutes), flatten(testCase.expectedRoutes)) - break poll } - } + }) + } } @@ -432,7 +493,8 @@ func routeListEqual(list1, list2 []*cloudprovider.Route) bool { for _, route1 := range list1 { for _, route2 := range list2 { - if route1.DestinationCIDR == route2.DestinationCIDR && route1.TargetNode == route2.TargetNode { + if route1.DestinationCIDR == route2.DestinationCIDR && route1.TargetNode == route2.TargetNode && + equalNodeAddrs(route1.TargetNodeAddresses, route2.TargetNodeAddresses) { seen[string(route1.TargetNode)+route1.DestinationCIDR] = true break }