diff --git a/pkg/cloudprovider/cloud.go b/pkg/cloudprovider/cloud.go index 247cb15ce78..2fb837b7192 100644 --- a/pkg/cloudprovider/cloud.go +++ b/pkg/cloudprovider/cloud.go @@ -150,6 +150,9 @@ type Route struct { // DestinationCIDR is the CIDR format IP range that this routing rule // applies to. DestinationCIDR string + // Blackhole is set to true if this is a blackhole route + // The node controller will delete the route if it is in the managed range. + Blackhole bool } // Routes is an abstract, pluggable interface for advanced routing rules. diff --git a/pkg/cloudprovider/providers/aws/aws_routes.go b/pkg/cloudprovider/providers/aws/aws_routes.go index fffcdf42c12..f24220d78fa 100644 --- a/pkg/cloudprovider/providers/aws/aws_routes.go +++ b/pkg/cloudprovider/providers/aws/aws_routes.go @@ -90,21 +90,34 @@ func (c *Cloud) ListRoutes(clusterName string) ([]*cloudprovider.Route, error) { } for _, r := range table.Routes { - instanceID := orEmpty(r.InstanceId) - destinationCIDR := orEmpty(r.DestinationCidrBlock) - - if instanceID == "" || destinationCIDR == "" { + destinationCIDR := aws.StringValue(r.DestinationCidrBlock) + if destinationCIDR == "" { continue } - instance, found := instances[instanceID] - if !found { - glog.Warningf("unable to find instance ID %s in the list of instances being routed to", instanceID) + route := &cloudprovider.Route{ + Name: clusterName + "-" + destinationCIDR, + DestinationCIDR: destinationCIDR, + } + + // Capture blackhole routes + if aws.StringValue(r.State) == ec2.RouteStateBlackhole { + route.Blackhole = true + routes = append(routes, route) continue } - nodeName := mapInstanceToNodeName(instance) - routeName := clusterName + "-" + destinationCIDR - routes = append(routes, &cloudprovider.Route{Name: routeName, TargetNode: nodeName, DestinationCIDR: destinationCIDR}) + + // Capture instance routes + instanceID := aws.StringValue(r.InstanceId) + if instanceID != "" { + instance, found := instances[instanceID] + if found { + route.TargetNode = mapInstanceToNodeName(instance) + routes = append(routes, route) + } else { + glog.Warningf("unable to find instance ID %s in the list of instances being routed to", instanceID) + } + } } return routes, nil diff --git a/pkg/controller/route/routecontroller.go b/pkg/controller/route/routecontroller.go index 827deaa6e2a..d46137d1b88 100644 --- a/pkg/controller/route/routecontroller.go +++ b/pkg/controller/route/routecontroller.go @@ -118,7 +118,9 @@ func (rc *RouteController) reconcile(nodes []*v1.Node, routes []*cloudprovider.R // routeMap maps routeTargetNode->route routeMap := make(map[types.NodeName]*cloudprovider.Route) for _, route := range routes { - routeMap[route.TargetNode] = route + if route.TargetNode != "" { + routeMap[route.TargetNode] = route + } } wg := sync.WaitGroup{} @@ -171,8 +173,8 @@ func (rc *RouteController) reconcile(nodes []*v1.Node, routes []*cloudprovider.R } for _, route := range routes { if rc.isResponsibleForRoute(route) { - // Check if this route applies to a node we know about & has correct CIDR. - if nodeCIDRs[route.TargetNode] != route.DestinationCIDR { + // Check if this route is a blackhole, or applies to a node we know about & has an incorrect CIDR. + if route.Blackhole || (nodeCIDRs[route.TargetNode] != route.DestinationCIDR) { wg.Add(1) // Delete the route. go func(route *cloudprovider.Route, startTime time.Time) { diff --git a/pkg/controller/route/routecontroller_test.go b/pkg/controller/route/routecontroller_test.go index 5f41c0eac73..b861a6a06b3 100644 --- a/pkg/controller/route/routecontroller_test.go +++ b/pkg/controller/route/routecontroller_test.go @@ -97,12 +97,12 @@ func TestReconcile(t *testing.T) { &node2, }, initialRoutes: []*cloudprovider.Route{ - {cluster + "-01", "node-1", "10.120.0.0/24"}, - {cluster + "-02", "node-2", "10.120.1.0/24"}, + {cluster + "-01", "node-1", "10.120.0.0/24", false}, + {cluster + "-02", "node-2", "10.120.1.0/24", false}, }, expectedRoutes: []*cloudprovider.Route{ - {cluster + "-01", "node-1", "10.120.0.0/24"}, - {cluster + "-02", "node-2", "10.120.1.0/24"}, + {cluster + "-01", "node-1", "10.120.0.0/24", false}, + {cluster + "-02", "node-2", "10.120.1.0/24", false}, }, expectedNetworkUnavailable: []bool{true, true}, clientset: fake.NewSimpleClientset(&v1.NodeList{Items: []v1.Node{node1, node2}}), @@ -114,11 +114,11 @@ func TestReconcile(t *testing.T) { &node2, }, initialRoutes: []*cloudprovider.Route{ - {cluster + "-01", "node-1", "10.120.0.0/24"}, + {cluster + "-01", "node-1", "10.120.0.0/24", false}, }, expectedRoutes: []*cloudprovider.Route{ - {cluster + "-01", "node-1", "10.120.0.0/24"}, - {cluster + "-02", "node-2", "10.120.1.0/24"}, + {cluster + "-01", "node-1", "10.120.0.0/24", false}, + {cluster + "-02", "node-2", "10.120.1.0/24", false}, }, expectedNetworkUnavailable: []bool{true, true}, clientset: fake.NewSimpleClientset(&v1.NodeList{Items: []v1.Node{node1, node2}}), @@ -131,8 +131,8 @@ func TestReconcile(t *testing.T) { }, initialRoutes: []*cloudprovider.Route{}, expectedRoutes: []*cloudprovider.Route{ - {cluster + "-01", "node-1", "10.120.0.0/24"}, - {cluster + "-02", "node-2", "10.120.1.0/24"}, + {cluster + "-01", "node-1", "10.120.0.0/24", false}, + {cluster + "-02", "node-2", "10.120.1.0/24", false}, }, expectedNetworkUnavailable: []bool{true, true}, clientset: fake.NewSimpleClientset(&v1.NodeList{Items: []v1.Node{node1, node2}}), @@ -144,14 +144,14 @@ func TestReconcile(t *testing.T) { &node2, }, initialRoutes: []*cloudprovider.Route{ - {cluster + "-01", "node-1", "10.120.0.0/24"}, - {cluster + "-02", "node-2", "10.120.1.0/24"}, - {cluster + "-03", "node-3", "10.120.2.0/24"}, - {cluster + "-04", "node-4", "10.120.3.0/24"}, + {cluster + "-01", "node-1", "10.120.0.0/24", false}, + {cluster + "-02", "node-2", "10.120.1.0/24", false}, + {cluster + "-03", "node-3", "10.120.2.0/24", false}, + {cluster + "-04", "node-4", "10.120.3.0/24", false}, }, expectedRoutes: []*cloudprovider.Route{ - {cluster + "-01", "node-1", "10.120.0.0/24"}, - {cluster + "-02", "node-2", "10.120.1.0/24"}, + {cluster + "-01", "node-1", "10.120.0.0/24", false}, + {cluster + "-02", "node-2", "10.120.1.0/24", false}, }, expectedNetworkUnavailable: []bool{true, true}, clientset: fake.NewSimpleClientset(&v1.NodeList{Items: []v1.Node{node1, node2}}), @@ -163,12 +163,12 @@ func TestReconcile(t *testing.T) { &node2, }, initialRoutes: []*cloudprovider.Route{ - {cluster + "-01", "node-1", "10.120.0.0/24"}, - {cluster + "-03", "node-3", "10.120.2.0/24"}, + {cluster + "-01", "node-1", "10.120.0.0/24", false}, + {cluster + "-03", "node-3", "10.120.2.0/24", false}, }, expectedRoutes: []*cloudprovider.Route{ - {cluster + "-01", "node-1", "10.120.0.0/24"}, - {cluster + "-02", "node-2", "10.120.1.0/24"}, + {cluster + "-01", "node-1", "10.120.0.0/24", false}, + {cluster + "-02", "node-2", "10.120.1.0/24", false}, }, expectedNetworkUnavailable: []bool{true, true}, clientset: fake.NewSimpleClientset(&v1.NodeList{Items: []v1.Node{node1, node2}}), @@ -181,11 +181,48 @@ func TestReconcile(t *testing.T) { }, initialRoutes: []*cloudprovider.Route{}, expectedRoutes: []*cloudprovider.Route{ - {cluster + "-01", "node-1", "10.120.0.0/24"}, + {cluster + "-01", "node-1", "10.120.0.0/24", false}, }, expectedNetworkUnavailable: []bool{true, false}, clientset: fake.NewSimpleClientset(&v1.NodeList{Items: []v1.Node{node1, nodeNoCidr}}), }, + // 2 nodes, an extra blackhole route in our range + { + nodes: []*v1.Node{ + &node1, + &node2, + }, + initialRoutes: []*cloudprovider.Route{ + {cluster + "-01", "node-1", "10.120.0.0/24", false}, + {cluster + "-02", "node-2", "10.120.1.0/24", false}, + {cluster + "-03", "", "10.120.2.0/24", true}, + }, + expectedRoutes: []*cloudprovider.Route{ + {cluster + "-01", "node-1", "10.120.0.0/24", false}, + {cluster + "-02", "node-2", "10.120.1.0/24", 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 + { + nodes: []*v1.Node{ + &node1, + &node2, + }, + initialRoutes: []*cloudprovider.Route{ + {cluster + "-01", "node-1", "10.120.0.0/24", false}, + {cluster + "-02", "node-2", "10.120.1.0/24", false}, + {cluster + "-03", "", "10.1.2.0/24", true}, + }, + expectedRoutes: []*cloudprovider.Route{ + {cluster + "-01", "node-1", "10.120.0.0/24", false}, + {cluster + "-02", "node-2", "10.120.1.0/24", false}, + {cluster + "-03", "", "10.1.2.0/24", true}, + }, + expectedNetworkUnavailable: []bool{true, true}, + clientset: fake.NewSimpleClientset(&v1.NodeList{Items: []v1.Node{node1, node2}}), + }, } for i, testCase := range testCases { cloud := &fakecloud.FakeCloud{RouteMap: make(map[string]*fakecloud.FakeRoute)}