diff --git a/.gitignore b/.gitignore index 48558efa..6bc0e614 100644 --- a/.gitignore +++ b/.gitignore @@ -49,3 +49,5 @@ cloud-config.yaml *~ examples/* hack/tools/share/* + +vendor/ \ No newline at end of file diff --git a/pkg/cloud/isolated_network_test.go b/pkg/cloud/isolated_network_test.go index 8db967d7..d79f6b33 100644 --- a/pkg/cloud/isolated_network_test.go +++ b/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)) diff --git a/pkg/cloud/tags.go b/pkg/cloud/tags.go index 28dc7907..3297bd8b 100644 --- a/pkg/cloud/tags.go +++ b/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) } diff --git a/pkg/cloud/tags_test.go b/pkg/cloud/tags_test.go index 4c92343a..8f8771cb 100644 --- a/pkg/cloud/tags_test.go +++ b/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)