AWS: Remove blackhole routes in our managed range

Blackhole routes otherwise acccumulate unboundedly.  We also are careful
to ensure that we do so only within the managed range, which requires
enlisting the help of the routecontroller.

Fix #47524
This commit is contained in:
Justin Santa Barbara 2017-06-14 22:34:15 -04:00
parent ad43147e77
commit 11f8886f12
4 changed files with 88 additions and 33 deletions

View File

@ -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.

View File

@ -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

View File

@ -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) {

View File

@ -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)}