PCP-5371: fixed network tags to always have one capc cluster tag at a time (#7)

Author: kpiyush17

.gitignore

@@ -49,3 +49,5 @@ cloud-config.yaml
 *~
 examples/*
 hack/tools/share/*
+
+vendor/

pkg/cloud/isolated_network_test.go

@@ -105,10 +105,19 @@ var _ = ginkgo.Describe("Network", func() {
 					Return(&csapi.CreateEgressFirewallRuleResponse{}, nil))
 
 			// Will add cluster tag once to Network and once to PublicIP.
+			// Each AddClusterTag calls GetTags twice (IsCapcManaged + removeOldClusterTags)
 			createdByResponse := &csapi.ListTagsResponse{Tags: []*csapi.Tag{{Key: cloud.CreatedByCAPCTagName, Value: "1"}}}
 			gomock.InOrder(
+				// Network: IsCapcManaged GetTags
 				rs.EXPECT().NewListTagsParams().Return(&csapi.ListTagsParams{}),
 				rs.EXPECT().ListTags(gomock.Any()).Return(createdByResponse, nil),
+				// Network: removeOldClusterTags GetTags
+				rs.EXPECT().NewListTagsParams().Return(&csapi.ListTagsParams{}),
+				rs.EXPECT().ListTags(gomock.Any()).Return(createdByResponse, nil),
+				// PublicIP: IsCapcManaged GetTags
+				rs.EXPECT().NewListTagsParams().Return(&csapi.ListTagsParams{}),
+				rs.EXPECT().ListTags(gomock.Any()).Return(createdByResponse, nil),
+				// PublicIP: removeOldClusterTags GetTags
 				rs.EXPECT().NewListTagsParams().Return(&csapi.ListTagsParams{}),
 				rs.EXPECT().ListTags(gomock.Any()).Return(createdByResponse, nil))
 
@@ -252,9 +261,14 @@ var _ = ginkgo.Describe("Network", func() {
 			aip := &csapi.AssociateIpAddressParams{}
 			as.EXPECT().NewAssociateIpAddressParams().Return(aip)
 			as.EXPECT().AssociateIpAddress(aip).Return(&csapi.AssociateIpAddressResponse{}, nil)
-			// Will add cluster tag once to Network and once to PublicIP.
+			// Will add cluster tag once to PublicIP
+			// Each AddClusterTag calls GetTags twice (IsCapcManaged + removeOldClusterTags)
 			createdByResponse := &csapi.ListTagsResponse{Tags: []*csapi.Tag{{Key: cloud.CreatedByCAPCTagName, Value: "1"}}}
 			gomock.InOrder(
+				// PublicIP: IsCapcManaged GetTags
+				rs.EXPECT().NewListTagsParams().Return(&csapi.ListTagsParams{}),
+				rs.EXPECT().ListTags(gomock.Any()).Return(createdByResponse, nil),
+				// PublicIP: removeOldClusterTags GetTags
 				rs.EXPECT().NewListTagsParams().Return(&csapi.ListTagsParams{}),
 				rs.EXPECT().ListTags(gomock.Any()).Return(createdByResponse, nil))
 

pkg/cloud/tags.go

@@ -65,11 +65,18 @@ func (c *client) IsCapcManaged(resourceType ResourceType, resourceID string) (bo
 }
 
 // AddClusterTag adds cluster tag to a resource. This tag indicates the resource is used by a given the cluster.
+// If old cluster tags exist (from previous cluster UIDs), they are removed first to prevent dual-tagging.
 func (c *client) AddClusterTag(rType ResourceType, rID string, csCluster *infrav1.CloudStackCluster) error {
 	if managedByCAPC, err := c.IsCapcManaged(rType, rID); err != nil {
 		return err
 	} else if managedByCAPC {
 		ClusterTagName := generateClusterTagName(csCluster)
+
+		// Remove old cluster tags before adding new one to prevent dual-tagging during pivot
+		if err := c.removeOldClusterTags(rType, rID, ClusterTagName); err != nil {
+			return errors.Wrapf(err, "removing old cluster tags from %s %s", rType, rID)
+		}
+
 		return c.AddTags(rType, rID, map[string]string{ClusterTagName: "1"})
 	}
 	return nil
@@ -163,6 +170,32 @@ func (c *client) DeleteTags(resourceType ResourceType, resourceID string, tagsTo
 	return nil
 }
 
+// removeOldClusterTags removes any cluster tags that don't match the current cluster UID.
+// This prevents dual-tagging when a resource is moved during pivot and gets a new cluster UID.
+func (c *client) removeOldClusterTags(rType ResourceType, rID string, currentClusterTagName string) error {
+	tags, err := c.GetTags(rType, rID)
+	if err != nil {
+		return errors.Wrapf(err, "getting tags for %s %s", rType, rID)
+	}
+
+	oldClusterTags := make(map[string]string)
+	for tagName, tagValue := range tags {
+		// Find cluster tags that are NOT the current cluster's tag
+		if strings.HasPrefix(tagName, ClusterTagNamePrefix) && tagName != currentClusterTagName {
+			oldClusterTags[tagName] = tagValue
+		}
+	}
+
+	// Delete old cluster tags if any exist
+	if len(oldClusterTags) > 0 {
+		if err := c.DeleteTags(rType, rID, oldClusterTags); err != nil {
+			return errors.Wrapf(err, "deleting old cluster tags %v from %s %s", oldClusterTags, rType, rID)
+		}
+	}
+
+	return nil
+}
+
 func generateClusterTagName(csCluster *infrav1.CloudStackCluster) string {
 	return ClusterTagNamePrefix + string(csCluster.UID)
 }

pkg/cloud/tags_test.go

@@ -156,6 +156,10 @@ var _ = ginkgo.Describe("Tag Unit Tests", func() {
 			createdByCAPCResponse := &csapi.ListTagsResponse{Tags: []*csapi.Tag{{Key: cloud.CreatedByCAPCTagName, Value: "1"}}}
 			rtlp := &csapi.ListTagsParams{}
 			ctp := &csapi.CreateTagsParams{}
+			// First GetTags call in IsCapcManaged
+			rs.EXPECT().NewListTagsParams().Return(rtlp)
+			rs.EXPECT().ListTags(rtlp).Return(createdByCAPCResponse, nil)
+			// Second GetTags call in removeOldClusterTags
 			rs.EXPECT().NewListTagsParams().Return(rtlp)
 			rs.EXPECT().ListTags(rtlp).Return(createdByCAPCResponse, nil)
 			rs.EXPECT().NewCreateTagsParams(gomock.Any(), gomock.Any(), gomock.Any()).Return(ctp)