Merge pull request #39 from aws/only_tag_own_networks

Don't tag pre-existing networks or IP addresses

Author: mrog

pkg/cloud/network_test.go

@@ -82,13 +82,15 @@ var _ = Describe("Network", func() {
 	})
 
 	// Sets expectations network tag creation.  To be used by tests that get/create networks.
-	expectNetworkTags := func(networkID string) {
+	expectNetworkTags := func(networkID string, expectTagCreation bool) {
 		listTagsParams := &cloudstack.ListTagsParams{}
 		createTagsParams := &cloudstack.CreateTagsParams{}
 		rs.EXPECT().NewListTagsParams().Return(listTagsParams)
 		rs.EXPECT().ListTags(listTagsParams).Return(&cloudstack.ListTagsResponse{}, nil)
-		rs.EXPECT().NewCreateTagsParams([]string{networkID}, string(cloud.ResourceTypeNetwork), gomock.Any()).Return(createTagsParams)
-		rs.EXPECT().CreateTags(createTagsParams).Return(&cloudstack.CreateTagsResponse{}, nil)
+		if expectTagCreation {
+			rs.EXPECT().NewCreateTagsParams([]string{networkID}, string(cloud.ResourceTypeNetwork), gomock.Any()).Return(createTagsParams)
+			rs.EXPECT().CreateTags(createTagsParams).Return(&cloudstack.CreateTagsResponse{}, nil)
+		}
 	}
 
 	Context("for an existing network", func() {
@@ -103,15 +105,15 @@ var _ = Describe("Network", func() {
 		It("does not call to create a new network via GetOrCreateNetwork", func() {
 			ns.EXPECT().GetNetworkID(fakeNetName).Return(fakeNetID, 1, nil)
 			ns.EXPECT().GetNetworkByID(fakeNetID).Return(&cloudstack.Network{Type: isolatedNetworkType}, 1, nil)
-			expectNetworkTags(fakeNetID)
+			expectNetworkTags(fakeNetID, false)
 
 			Ω(client.GetOrCreateNetwork(csCluster)).Should(Succeed())
 		})
 
 		It("resolves network details with network ID instead of network name", func() {
 			ns.EXPECT().GetNetworkID(gomock.Any()).Return("", -1, errors.New("no match found for blah"))
 			ns.EXPECT().GetNetworkByID(fakeNetID).Return(&cloudstack.Network{Type: isolatedNetworkType}, 1, nil)
-			expectNetworkTags(fakeNetID)
+			expectNetworkTags(fakeNetID, false)
 
 			csCluster.Spec.Network = fakeNetID
 			Ω(client.GetOrCreateNetwork(csCluster)).Should(Succeed())
@@ -126,7 +128,7 @@ var _ = Describe("Network", func() {
 			ns.EXPECT().NewCreateNetworkParams(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).
 				Return(&cloudstack.CreateNetworkParams{})
 			ns.EXPECT().CreateNetwork(gomock.Any()).Return(&cloudstack.CreateNetworkResponse{Id: netID}, nil)
-			expectNetworkTags(netID)
+			expectNetworkTags(netID, true)
 
 			Ω(client.GetOrCreateNetwork(csCluster)).Should(Succeed())
 		})

pkg/cloud/tags.go

@@ -51,6 +51,11 @@ func (c *client) AddClusterTag(resourceType ResourceType, resourceID string, csC
 		return err
 	}
 
+	// Don't tag resources unless they are now being created by CAPC, or were previously created by CAPC.
+	if !addCreatedByCAPCTag && existingTags[createdByCAPCTagName] == "" {
+		return nil
+	}
+
 	if existingTags[clusterTagName] == "" {
 		newTags[clusterTagName] = "1"
 	}

pkg/cloud/tags_test.go

@@ -76,9 +76,11 @@ var _ = Describe("Tag Unit Tests", func() {
 			if err != nil {
 				Fail("Failed to get existing tags. Error: " + err.Error())
 			}
-			err = client.DeleteTags(cloud.ResourceTypeNetwork, networkID, existingTags)
-			if err != nil {
-				Fail("Failed to delete existing tags. Error: " + err.Error())
+			if len(existingTags) > 0 {
+				err = client.DeleteTags(cloud.ResourceTypeNetwork, networkID, existingTags)
+				if err != nil {
+					Fail("Failed to delete existing tags. Error: " + err.Error())
+				}
 			}
 		})
 
@@ -116,14 +118,14 @@ var _ = Describe("Tag Unit Tests", func() {
 			Ω(client.AddClusterTag(cloud.ResourceTypeNetwork, networkID, csCluster, true)).Should(Succeed())
 		})
 
-		It("adds the tags for a cluster (resource NOT created by CAPC)", func() {
+		It("doesn't add the tags for a cluster (resource NOT created by CAPC)", func() {
 			Ω(client.AddClusterTag(cloud.ResourceTypeNetwork, networkID, csCluster, false)).Should(Succeed())
 
 			// Verify tags
 			tags, err := client.GetTags(cloud.ResourceTypeNetwork, networkID)
 			Ω(err).Should(BeNil())
 			Ω(tags[createdByCAPCTag]).Should(Equal(""))
-			Ω(tags[clusterTag]).Should(Equal("1"))
+			Ω(tags[clusterTag]).Should(Equal(""))
 		})
 
 		It("deletes a cluster tag", func() {