Add unit test for deleting untagged route table

Makefile

@@ -192,6 +192,7 @@ copydeps:
 
 gofmt:
 	gofmt -w -s channels/
+	gofmt -w -s cloudmock/
 	gofmt -w -s cmd/
 	gofmt -w -s examples/
 	gofmt -w -s util/

cloudmock/aws/mockec2/api.go

@@ -0,0 +1,12 @@
+package mockec2
+
+import (
+	"github.com/aws/aws-sdk-go/service/ec2"
+	"github.com/aws/aws-sdk-go/service/ec2/ec2iface"
+)
+
+type MockEC2 struct {
+	RouteTables []*ec2.RouteTable
+}
+
+var _ ec2iface.EC2API = &MockEC2{}

cloudmock/aws/mockec2/routetable.go

@@ -0,0 +1,30 @@
+package mockec2
+
+import (
+	"github.com/aws/aws-sdk-go/aws/request"
+	"github.com/aws/aws-sdk-go/service/ec2"
+	"github.com/golang/glog"
+)
+
+func (m *MockEC2) DescribeRouteTablesRequest(*ec2.DescribeRouteTablesInput) (*request.Request, *ec2.DescribeRouteTablesOutput) {
+	panic("Not implemented")
+	return nil, nil
+}
+
+func (m *MockEC2) DescribeRouteTables(request *ec2.DescribeRouteTablesInput) (*ec2.DescribeRouteTablesOutput, error) {
+	if request.Filters != nil {
+		glog.Fatalf("filters not implemented: %v", request.Filters)
+	}
+	if request.DryRun != nil {
+		glog.Fatalf("DryRun not implemented")
+	}
+	if request.RouteTableIds != nil {
+		glog.Fatalf("RouteTableIds not implemented")
+	}
+
+	response := &ec2.DescribeRouteTablesOutput{}
+	for _, rt := range m.RouteTables {
+		response.RouteTables = append(response.RouteTables, rt)
+	}
+	return response, nil
+}

upup/pkg/fi/cloudup/awsup/aws_cloud.go

@@ -23,6 +23,7 @@ import (
 	"github.com/aws/aws-sdk-go/aws/session"
 	"github.com/aws/aws-sdk-go/service/autoscaling"
 	"github.com/aws/aws-sdk-go/service/ec2"
+	"github.com/aws/aws-sdk-go/service/ec2/ec2iface"
 	"github.com/aws/aws-sdk-go/service/elb"
 	"github.com/aws/aws-sdk-go/service/iam"
 	"github.com/aws/aws-sdk-go/service/route53"
@@ -53,7 +54,7 @@ type AWSCloud interface {
 
 	Region() string
 
-	EC2() *ec2.EC2
+	EC2() ec2iface.EC2API
 	IAM() *iam.IAM
 	ELB() *elb.ELB
 	Autoscaling() *autoscaling.AutoScaling
@@ -662,7 +663,7 @@ func (c *awsCloudImplementation) FindDNSHostedZone(clusterDNSName string) (strin
 	return "", fmt.Errorf("Found multiple hosted zones matching cluster %q; please specify the ID of the zone to use", clusterDNSName)
 }
 
-func (c *awsCloudImplementation) EC2() *ec2.EC2 {
+func (c *awsCloudImplementation) EC2() ec2iface.EC2API {
 	return c.ec2
 }
 

upup/pkg/fi/cloudup/awsup/mock_aws_cloud.go

@@ -21,6 +21,7 @@ import (
 	"github.com/aws/aws-sdk-go/aws"
 	"github.com/aws/aws-sdk-go/service/autoscaling"
 	"github.com/aws/aws-sdk-go/service/ec2"
+	"github.com/aws/aws-sdk-go/service/ec2/ec2iface"
 	"github.com/aws/aws-sdk-go/service/elb"
 	"github.com/aws/aws-sdk-go/service/iam"
 	"github.com/aws/aws-sdk-go/service/route53"
@@ -40,8 +41,15 @@ type MockAWSCloud struct {
 var _ fi.Cloud = (*MockAWSCloud)(nil)
 
 func InstallMockAWSCloud(region string, zoneLetters string) {
-	i := &MockAWSCloud{region: region}
+	i := BuildMockAWSCloud(region, zoneLetters)
 	awsCloudInstances[region] = i
+	allRegions = []*ec2.Region{
+		{RegionName: aws.String(region)},
+	}
+}
+
+func BuildMockAWSCloud(region string, zoneLetters string) *MockAWSCloud {
+	i := &MockAWSCloud{region: region}
 	for _, c := range zoneLetters {
 		azName := fmt.Sprintf("%s%c", region, c)
 		az := &ec2.AvailabilityZone{
@@ -51,13 +59,11 @@ func InstallMockAWSCloud(region string, zoneLetters string) {
 		}
 		i.zones = append(i.zones, az)
 	}
-
-	allRegions = []*ec2.Region{
-		{RegionName: aws.String(region)},
-	}
+	return i
 }
 
 type MockCloud struct {
+	MockEC2 ec2iface.EC2API
 }
 
 func (c *MockCloud) ProviderID() fi.CloudProviderID {
@@ -138,9 +144,11 @@ func (c *MockAWSCloud) WithTags(tags map[string]string) AWSCloud {
 	return m
 }
 
-func (c *MockAWSCloud) EC2() *ec2.EC2 {
-	glog.Fatalf("MockAWSCloud EC2 not implemented")
-	return nil
+func (c *MockAWSCloud) EC2() ec2iface.EC2API {
+	if c.MockEC2 == nil {
+		glog.Fatalf("MockAWSCloud MockEC2 not set")
+	}
+	return c.MockEC2
 }
 
 func (c *MockAWSCloud) IAM() *iam.IAM {

upup/pkg/kutil/delete_cluster.go

@@ -164,43 +164,8 @@ func (c *DeleteCluster) ListResources() (map[string]*ResourceTracker, error) {
 		}
 	}
 
-	{
-		// We sometimes have trouble tagging the route table (eventual consistency, e.g. #597)
-		// If we are deleting the VPC, we should delete the route table
-		// (no real reason not to; easy to recreate; no real state etc)
-		routeTables, err := DescribeRouteTablesIgnoreTags(cloud)
-		if err != nil {
-			return nil, err
-		}
-
-		for _, rt := range routeTables {
-			rtID := aws.StringValue(rt.RouteTableId)
-			vpcID := aws.StringValue(rt.VpcId)
-			if vpcID == "" || rtID == "" {
-				continue
-			}
-
-			if resources["vpc:"+vpcID] == nil {
-				// Not deleting this VPC; ignore
-				continue
-			}
-
-			isMain := true
-			for _, a := range rt.Associations {
-				if aws.BoolValue(a.Main) == false {
-					isMain = false
-				}
-			}
-			if isMain {
-				glog.V(4).Infof("ignoring main routetable %q", rtID)
-				continue
-			}
-
-			t := buildTrackerForRouteTable(rt)
-			if resources[t.Type+":"+t.ID] == nil {
-				resources[t.Type+":"+t.ID] = t
-			}
-		}
+	if err := addUntaggedRouteTables(cloud, resources); err != nil {
+		return nil, err
 	}
 
 	for k, t := range resources {
@@ -211,6 +176,47 @@ func (c *DeleteCluster) ListResources() (map[string]*ResourceTracker, error) {
 	return resources, nil
 }
 
+func addUntaggedRouteTables(cloud awsup.AWSCloud, resources map[string]*ResourceTracker) error {
+	// We sometimes have trouble tagging the route table (eventual consistency, e.g. #597)
+	// If we are deleting the VPC, we should delete the route table
+	// (no real reason not to; easy to recreate; no real state etc)
+	routeTables, err := DescribeRouteTablesIgnoreTags(cloud)
+	if err != nil {
+		return err
+	}
+
+	for _, rt := range routeTables {
+		rtID := aws.StringValue(rt.RouteTableId)
+		vpcID := aws.StringValue(rt.VpcId)
+		if vpcID == "" || rtID == "" {
+			continue
+		}
+
+		if resources["vpc:"+vpcID] == nil {
+			// Not deleting this VPC; ignore
+			continue
+		}
+
+		isMain := false
+		for _, a := range rt.Associations {
+			if aws.BoolValue(a.Main) == true {
+				isMain = true
+			}
+		}
+		if isMain {
+			glog.V(4).Infof("ignoring main routetable %q", rtID)
+			continue
+		}
+
+		t := buildTrackerForRouteTable(rt)
+		if resources[t.Type+":"+t.ID] == nil {
+			resources[t.Type+":"+t.ID] = t
+		}
+	}
+
+	return nil
+}
+
 func (c *DeleteCluster) DeleteResources(resources map[string]*ResourceTracker) error {
 	depMap := make(map[string][]string)
 

upup/pkg/kutil/deletecluster_test.go

@@ -0,0 +1,75 @@
+/*
+Copyright 2016 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package kutil
+
+import (
+	"github.com/aws/aws-sdk-go/aws"
+	"github.com/aws/aws-sdk-go/service/ec2"
+	"k8s.io/kops/cloudmock/aws/mockec2"
+	"k8s.io/kops/upup/pkg/fi/cloudup/awsup"
+	"reflect"
+	"sort"
+	"testing"
+)
+
+func TestAddUntaggedRouteTables(t *testing.T) {
+	cloud := awsup.BuildMockAWSCloud("us-east-1", "abc")
+	resources := make(map[string]*ResourceTracker)
+
+	c := &mockec2.MockEC2{}
+	cloud.MockEC2 = c
+
+	// Matches by vpc id
+	c.RouteTables = append(c.RouteTables, &ec2.RouteTable{
+		VpcId:        aws.String("vpc-1234"),
+		RouteTableId: aws.String("rt-1234"),
+	})
+
+	// Skips main route tables
+	c.RouteTables = append(c.RouteTables, &ec2.RouteTable{
+		VpcId:        aws.String("vpc-1234"),
+		RouteTableId: aws.String("rt-1234main"),
+		Associations: []*ec2.RouteTableAssociation{
+			{
+				Main: aws.Bool(true),
+			},
+		},
+	})
+
+	// Ignores non-matching vpcs
+	c.RouteTables = append(c.RouteTables, &ec2.RouteTable{
+		VpcId:        aws.String("vpc-5555"),
+		RouteTableId: aws.String("rt-5555"),
+	})
+
+	resources["vpc:vpc-1234"] = &ResourceTracker{}
+
+	err := addUntaggedRouteTables(cloud, resources)
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+
+	var keys []string
+	for k := range resources {
+		keys = append(keys, k)
+	}
+	sort.Strings(keys)
+	expected := []string{"route-table:rt-1234", "vpc:vpc-1234"}
+	if !reflect.DeepEqual(expected, keys) {
+		t.Fatalf("expected=%q, actual=%q", expected, keys)
+	}
+}