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 a2b51b7b3f7..072e860b0f9 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 }