Merge pull request #3887 from georgebuckerfield/fix-routetable-deletion

Automatic merge from submit-queue. Respect the shared tag when deleting route tables Fixes #3828. Modifies the `buildTrackerForRouteTable` function (used by `ListRouteTables`) to set the `Shared` field of each returned route table resource, based on the presence of the `kubernetes.io/cluster/<clustername>: shared` tag. This prevents route tables with this tag from being deleted. WIP while I add some more tests.
kubernetes · Nov 22, 2017 · cabe972 · cabe972
2 parents 3f2e537 + eab351c
commit cabe972
Show file tree

Hide file tree

Showing 3 changed files with 61 additions and 5 deletions.
diff --git a/pkg/resources/aws.go b/pkg/resources/aws.go
@@ -220,7 +220,7 @@ func addUntaggedRouteTables(cloud awsup.AWSCloud, clusterName string, resources
 			continue
 		}
 
-		t := buildTrackerForRouteTable(rt)
+		t := buildTrackerForRouteTable(rt, clusterName)
 		if resources[t.Type+":"+t.ID] == nil {
 			resources[t.Type+":"+t.ID] = t
 		}
@@ -973,19 +973,20 @@ func ListRouteTables(cloud fi.Cloud, clusterName string) ([]*Resource, error) {
 	var resourceTrackers []*Resource
 
 	for _, rt := range routeTables {
-		resourceTracker := buildTrackerForRouteTable(rt)
+		resourceTracker := buildTrackerForRouteTable(rt, clusterName)
 		resourceTrackers = append(resourceTrackers, resourceTracker)
 	}
 
 	return resourceTrackers, nil
 }
 
-func buildTrackerForRouteTable(rt *ec2.RouteTable) *Resource {
+func buildTrackerForRouteTable(rt *ec2.RouteTable, clusterName string) *Resource {
 	resourceTracker := &Resource{
 		Name:    FindName(rt.Tags),
 		ID:      aws.StringValue(rt.RouteTableId),
 		Type:    ec2.ResourceTypeRouteTable,
 		Deleter: DeleteRouteTable,
+		Shared:  HasSharedTag(ec2.ResourceTypeRouteTable+":"+*rt.RouteTableId, rt.Tags, clusterName),
 	}
 
 	var blocks []string

diff --git a/pkg/resources/aws_test.go b/pkg/resources/aws_test.go
@@ -88,3 +88,55 @@ func TestAddUntaggedRouteTables(t *testing.T) {
 		t.Fatalf("expected=%q, actual=%q", expected, keys)
 	}
 }
+
+func TestListRouteTables(t *testing.T) {
+	cloud := awsup.BuildMockAWSCloud("us-east-1", "abc")
+	//resources := make(map[string]*Resource)
+	clusterName := "me.example.com"
+	ownershipTagKey := "kubernetes.io/cluster/" + clusterName
+
+	c := &mockec2.MockEC2{}
+	cloud.MockEC2 = c
+
+	c.RouteTables = append(c.RouteTables, &ec2.RouteTable{
+		VpcId:        aws.String("vpc-1234"),
+		RouteTableId: aws.String("rt-shared"),
+		Tags: []*ec2.Tag{
+			{
+				Key:   aws.String("KubernetesCluster"),
+				Value: aws.String(clusterName),
+			},
+			{
+				Key:   aws.String(ownershipTagKey),
+				Value: aws.String("shared"),
+			},
+		},
+	})
+	c.RouteTables = append(c.RouteTables, &ec2.RouteTable{
+		VpcId:        aws.String("vpc-1234"),
+		RouteTableId: aws.String("rt-owned"),
+		Tags: []*ec2.Tag{
+			{
+				Key:   aws.String("KubernetesCluster"),
+				Value: aws.String(clusterName),
+			},
+			{
+				Key:   aws.String(ownershipTagKey),
+				Value: aws.String("owned"),
+			},
+		},
+	})
+
+	resources, err := ListRouteTables(cloud, clusterName)
+	if err != nil {
+		t.Fatalf("error listing route tables: %v", err)
+	}
+	for _, rt := range resources {
+		if rt.ID == "rt-shared" && !rt.Shared {
+			t.Fatalf("expected Shared: true, got: %v", rt.Shared)
+		}
+		if rt.ID == "rt-owned" && rt.Shared {
+			t.Fatalf("expected Shared: false, got: %v", rt.Shared)
+		}
+	}
+}
diff --git a/upup/pkg/fi/cloudup/awsup/mock_aws_cloud.go b/upup/pkg/fi/cloudup/awsup/mock_aws_cloud.go
@@ -133,8 +133,11 @@ func (c *MockAWSCloud) BuildTags(name *string) map[string]string {
 }
 
 func (c *MockAWSCloud) Tags() map[string]string {
-	glog.Fatalf("MockAWSCloud Tags not implemented")
-	return nil
+	tags := make(map[string]string)
+	for k, v := range c.tags {
+		tags[k] = v
+	}
+	return tags
 }
 
 func (c *MockAWSCloud) CreateTags(resourceId string, tags map[string]string) error {