Initial work for tagging and cleaning up IP addresses

Author: mrog

config/webhook/manifests.yaml

@@ -1,3 +1,4 @@
+
 ---
 apiVersion: admissionregistration.k8s.io/v1
 kind: MutatingWebhookConfiguration
@@ -45,6 +46,7 @@ webhooks:
     resources:
     - cloudstackmachines
   sideEffects: None
+
 ---
 apiVersion: admissionregistration.k8s.io/v1
 kind: ValidatingWebhookConfiguration

pkg/cloud/affinity_groups.go

@@ -37,7 +37,7 @@ type AffinityGroupIFace interface {
 	GetOrCreateAffinityGroup(*infrav1.CloudStackCluster, *AffinityGroup) error
 	DeleteAffinityGroup(*AffinityGroup) error
 	AssociateAffinityGroup(*infrav1.CloudStackMachine, AffinityGroup) error
-	DissassociateAffinityGroup(*infrav1.CloudStackMachine, AffinityGroup) error
+	DisassociateAffinityGroup(*infrav1.CloudStackMachine, AffinityGroup) error
 }
 
 func (c *client) FetchAffinityGroup(group *AffinityGroup) (reterr error) {
@@ -171,7 +171,7 @@ func (c *client) AssociateAffinityGroup(csMachine *infrav1.CloudStackMachine, gr
 	}
 }
 
-func (c *client) DissassociateAffinityGroup(csMachine *infrav1.CloudStackMachine, group AffinityGroup) (retErr error) {
+func (c *client) DisassociateAffinityGroup(csMachine *infrav1.CloudStackMachine, group AffinityGroup) (retErr error) {
 	if groups, err := c.GetCurrentAffinityGroups(csMachine); err != nil {
 		return err
 	} else {

pkg/cloud/cluster.go

@@ -95,8 +95,18 @@ func (c *client) GetOrCreateCluster(csCluster *infrav1.CloudStackCluster) (retEr
 }
 
 func (c *client) DisposeClusterResources(csCluster *infrav1.CloudStackCluster) (retError error) {
-	if err := c.RemoveClusterTagFromNetwork(csCluster); err != nil {
+	if csCluster.Status.PublicIPID != "" {
+		if err := c.DeleteClusterTag(ResourceTypeIpAddress, csCluster.Status.PublicIPID, csCluster); err != nil {
+			return err
+		}
+		if err := c.DisassociatePublicIpAddressIfNotInUse(csCluster); err != nil {
+			return err
+		}
+	}
+
+	if err := c.DeleteClusterTag(ResourceTypeNetwork, csCluster.Status.NetworkID, csCluster); err != nil {
 		return err
 	}
+
 	return c.DeleteNetworkIfNotInUse(csCluster)
 }

pkg/cloud/network.go

@@ -58,13 +58,9 @@ func (c *client) ResolveNetwork(csCluster *infrav1.CloudStackCluster) (retErr er
 	return nil
 }
 
-func generateNetworkTagName(csCluster *infrav1.CloudStackCluster) string {
-	return clusterTagNamePrefix + string(csCluster.UID)
-}
-
 func (c *client) GetOrCreateNetwork(csCluster *infrav1.CloudStackCluster) (retErr error) {
 	if retErr = c.ResolveNetwork(csCluster); retErr == nil { // Found network.
-		return addClusterTags(c, csCluster, false)
+		return c.AddClusterTag(ResourceTypeNetwork, csCluster.Status.NetworkID, csCluster, false)
 	} else if !strings.Contains(retErr.Error(), "No match found") { // Some other error.
 		return retErr
 	} // Network not found.
@@ -90,63 +86,29 @@ func (c *client) GetOrCreateNetwork(csCluster *infrav1.CloudStackCluster) (retEr
 	csCluster.Status.NetworkID = resp.Id
 	csCluster.Status.NetworkType = resp.Type
 
-	return addClusterTags(c, csCluster, true)
-}
-
-func addClusterTags(c *client, csCluster *infrav1.CloudStackCluster, addCreatedByTag bool) error {
-	clusterTagName := generateNetworkTagName(csCluster)
-	newTags := map[string]string{}
-
-	existingTags, err := c.GetNetworkTags(csCluster.Status.NetworkID)
-	if err != nil {
-		return err
-	}
-
-	if existingTags[clusterTagName] == "" {
-		newTags[clusterTagName] = "1"
-	}
-
-	if addCreatedByTag && existingTags[createdByCapcTagName] == "" {
-		newTags[createdByCapcTagName] = "1"
-	}
-
-	if len(newTags) > 0 {
-		return c.AddNetworkTags(csCluster.Status.NetworkID, newTags)
-	}
-
-	return nil
+	return c.AddClusterTag(ResourceTypeNetwork, csCluster.Status.NetworkID, csCluster, true)
 }
 
-func (c *client) RemoveClusterTagFromNetwork(csCluster *infrav1.CloudStackCluster) (retError error) {
-	tags, err := c.GetNetworkTags(csCluster.Status.NetworkID)
+func (c *client) DisassociatePublicIpAddressIfNotInUse(csCluster *infrav1.CloudStackCluster) (retError error) {
+	okayToDelete, err := c.DoClusterTagsAllowDisposal(ResourceTypeIpAddress, csCluster.Status.PublicIPID)
 	if err != nil {
 		return err
 	}
 
-	clusterTagName := generateNetworkTagName(csCluster)
-	if tagValue := tags[clusterTagName]; tagValue != "" {
-		if err = c.DeleteNetworkTags(csCluster.Status.NetworkID, map[string]string{clusterTagName: tagValue}); err != nil {
-			return err
-		}
+	if okayToDelete {
+		return c.DisassociatePublicIpAddress(csCluster)
 	}
 
 	return nil
 }
 
 func (c *client) DeleteNetworkIfNotInUse(csCluster *infrav1.CloudStackCluster) (retError error) {
-	tags, err := c.GetNetworkTags(csCluster.Status.NetworkID)
+	okayToDelete, err := c.DoClusterTagsAllowDisposal(ResourceTypeNetwork, csCluster.Status.NetworkID)
 	if err != nil {
 		return err
 	}
 
-	var clusterTagCount int
-	for tagName := range tags {
-		if strings.HasPrefix(tagName, clusterTagNamePrefix) {
-			clusterTagCount++
-		}
-	}
-
-	if clusterTagCount == 0 && tags[createdByCapcTagName] != "" {
+	if okayToDelete {
 		return c.DestroyNetwork(csCluster)
 	}
 
@@ -171,12 +133,12 @@ func (c *client) ResolvePublicIPDetails(csCluster *infrav1.CloudStackCluster) (*
 		// Ignore already allocated here since the IP was specified.
 		return publicAddresses.PublicIpAddresses[0], nil
 	} else if publicAddresses.Count > 0 { // Endpoint not specified.
-		for _, v := range publicAddresses.PublicIpAddresses { // Pick first availabe address.
+		for _, v := range publicAddresses.PublicIpAddresses { // Pick first available address.
 			if v.Allocated == "" { // Found un-allocated Public IP.
 				return v, nil
 			}
 		}
-		return nil, errors.New("all Public IP Adresse(s) found were already allocated")
+		return nil, errors.New("all Public IP Address(es) found were already allocated")
 	}
 	return nil, errors.Errorf(`no public addresses found in network: "%s"`, csCluster.Spec.Network)
 }
@@ -190,10 +152,11 @@ func (c *client) AssociatePublicIpAddress(csCluster *infrav1.CloudStackCluster)
 
 	csCluster.Spec.ControlPlaneEndpoint.Host = publicAddress.Ipaddress
 	csCluster.Status.PublicIPID = publicAddress.Id
+	allocatedByCapc := publicAddress.Allocated == ""
 
 	if publicAddress.Allocated != "" && publicAddress.Associatednetworkid == csCluster.Status.NetworkID {
 		// Address already allocated to network. Allocated is a timestamp -- not a boolean.
-		return nil
+		return c.AddClusterTag(ResourceTypeIpAddress, publicAddress.Id, csCluster, allocatedByCapc)
 	} // Address not yet allocated. Allocate now.
 
 	// Public IP found, but not yet allocated to network.
@@ -205,7 +168,13 @@ func (c *client) AssociatePublicIpAddress(csCluster *infrav1.CloudStackCluster)
 	if _, err := c.cs.Address.AssociateIpAddress(p); err != nil {
 		return err
 	}
-	return nil
+	return c.AddClusterTag(ResourceTypeIpAddress, publicAddress.Id, csCluster, allocatedByCapc)
+}
+
+func (c *client) DisassociatePublicIpAddress(csCluster *infrav1.CloudStackCluster) (retErr error) {
+	p := c.cs.Address.NewDisassociateIpAddressParams(csCluster.Status.PublicIPID)
+	_, retErr = c.cs.Address.DisassociateIpAddress(p)
+	return retErr
 }
 
 func (c *client) OpenFirewallRules(csCluster *infrav1.CloudStackCluster) (retErr error) {

pkg/cloud/tags.go

@@ -1,5 +1,5 @@
 /*
-Copyright 2022.
+Copyright 2022 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.
@@ -16,30 +16,93 @@ limitations under the License.
 
 package cloud
 
+import (
+	infrav1 "github.com/aws/cluster-api-provider-cloudstack/api/v1beta1"
+	"strings"
+)
+
 type TagIFace interface {
-	AddNetworkTags(string, map[string]string) error
-	GetNetworkTags(string) (map[string]string, error)
-	DeleteNetworkTags(string, map[string]string) error
+	AddClusterTag(resourceType ResourceType, resourceId string, csCluster *infrav1.CloudStackCluster, addCreatedByCapcTag bool) error
+	DeleteClusterTag(resourceType ResourceType, resourceId string, csCluster *infrav1.CloudStackCluster) error
+	DoClusterTagsAllowDisposal(resourceType ResourceType, resourceId string) (bool, error)
+	AddTags(resourceType ResourceType, resourceId string, tags map[string]string) error
+	GetTags(resourceType ResourceType, resourceId string) (map[string]string, error)
+	DeleteTags(resourceType ResourceType, resourceId string, tagsToDelete map[string]string) error
 }
 
+type ResourceType string
+
 const (
-	clusterTagNamePrefix = "CAPC_cluster_"
-	createdByCapcTagName = "created_by_CAPC"
-	resourceTypeNetwork  = "network"
+	clusterTagNamePrefix               = "CAPC_cluster_"
+	createdByCapcTagName               = "created_by_CAPC"
+	ResourceTypeNetwork   ResourceType = "network"
+	ResourceTypeIpAddress ResourceType = "ipaddress"
 )
 
-// TagNetwork adds tags to a network by network id.
-func (c *client) AddNetworkTags(networkId string, tags map[string]string) error {
-	p := c.cs.Resourcetags.NewCreateTagsParams([]string{networkId}, resourceTypeNetwork, tags)
+func (c *client) AddClusterTag(resourceType ResourceType, resourceId string, csCluster *infrav1.CloudStackCluster, addCreatedByCapcTag bool) error {
+	clusterTagName := generateClusterTagName(csCluster)
+	newTags := map[string]string{}
+
+	existingTags, err := c.GetTags(resourceType, resourceId)
+	if err != nil {
+		return err
+	}
+
+	if existingTags[clusterTagName] == "" {
+		newTags[clusterTagName] = "1"
+	}
+
+	if addCreatedByCapcTag && existingTags[createdByCapcTagName] == "" {
+		newTags[createdByCapcTagName] = "1"
+	}
+
+	if len(newTags) > 0 {
+		return c.AddTags(resourceType, resourceId, newTags)
+	}
+
+	return nil
+}
+
+func (c *client) DeleteClusterTag(resourceType ResourceType, resourceId string, csCluster *infrav1.CloudStackCluster) error {
+	tags, err := c.GetTags(resourceType, csCluster.Status.NetworkID)
+	if err != nil {
+		return err
+	}
+
+	clusterTagName := generateClusterTagName(csCluster)
+	if tagValue := tags[clusterTagName]; tagValue != "" {
+		return c.DeleteTags(resourceType, csCluster.Status.NetworkID, map[string]string{clusterTagName: tagValue})
+	}
+
+	return nil
+}
+
+func (c *client) DoClusterTagsAllowDisposal(resourceType ResourceType, resourceId string) (bool, error) {
+	tags, err := c.GetTags(resourceType, resourceId)
+	if err != nil {
+		return false, err
+	}
+
+	var clusterTagCount int
+	for tagName := range tags {
+		if strings.HasPrefix(tagName, clusterTagNamePrefix) {
+			clusterTagCount++
+		}
+	}
+
+	return clusterTagCount == 0 && tags[createdByCapcTagName] != "", nil
+}
+
+func (c *client) AddTags(resourceType ResourceType, resourceId string, tags map[string]string) error {
+	p := c.cs.Resourcetags.NewCreateTagsParams([]string{resourceId}, string(resourceType), tags)
 	_, err := c.cs.Resourcetags.CreateTags(p)
 	return err
 }
 
-// GetNetworkTags gets tags by network id.
-func (c *client) GetNetworkTags(networkId string) (map[string]string, error) {
+func (c *client) GetTags(resourceType ResourceType, resourceId string) (map[string]string, error) {
 	p := c.cs.Resourcetags.NewListTagsParams()
-	p.SetResourceid(networkId)
-	p.SetResourcetype(resourceTypeNetwork)
+	p.SetResourceid(resourceId)
+	p.SetResourcetype(string(resourceType))
 	if listTagResponse, err := c.cs.Resourcetags.ListTags(p); err != nil {
 		return nil, err
 	} else {
@@ -51,10 +114,13 @@ func (c *client) GetNetworkTags(networkId string) (map[string]string, error) {
 	}
 }
 
-// DeleteNetworkTags deletes matching tags from a network
-func (c *client) DeleteNetworkTags(networkId string, tagsToDelete map[string]string) error {
-	p := c.cs.Resourcetags.NewDeleteTagsParams([]string{networkId}, resourceTypeNetwork)
+func (c *client) DeleteTags(resourceType ResourceType, resourceId string, tagsToDelete map[string]string) error {
+	p := c.cs.Resourcetags.NewDeleteTagsParams([]string{resourceId}, string(resourceType))
 	p.SetTags(tagsToDelete)
 	_, err := c.cs.Resourcetags.DeleteTags(p)
 	return err
 }
+
+func generateClusterTagName(csCluster *infrav1.CloudStackCluster) string {
+	return clusterTagNamePrefix + string(csCluster.UID)
+}

pkg/cloud/tags_test.go

@@ -1,5 +1,5 @@
 /*
-Copyright 2022.
+Copyright 2022 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.
@@ -68,19 +68,19 @@ var _ = Describe("Tag Unit Tests", func() {
 
 		It("Tags a network with an arbitrary tag.", func() {
 			// Delete the tag if it already exists from a prior test run, otherwise the test will fail.
-			_ = client.DeleteNetworkTags(networkId, testTags)
-			Ω(client.AddNetworkTags(networkId, testTags)).Should(Succeed())
+			_ = client.DeleteTags(cloud.ResourceTypeNetwork, networkId, testTags)
+			Ω(client.AddTags(cloud.ResourceTypeNetwork, networkId, testTags)).Should(Succeed())
 		})
 
 		It("Fetches said tag.", func() {
-			tags, err := client.GetNetworkTags(networkId)
+			tags, err := client.GetTags(cloud.ResourceTypeNetwork, networkId)
 			Ω(err).Should(BeNil())
 			Ω(tags[tagKey]).Should(Equal(tagValue))
 		})
 
 		It("Deletes said tag.", func() {
-			Ω(client.DeleteNetworkTags(networkId, testTags)).Should(Succeed())
-			remainingTags, err := client.GetNetworkTags(networkId)
+			Ω(client.DeleteTags(cloud.ResourceTypeNetwork, networkId, testTags)).Should(Succeed())
+			remainingTags, err := client.GetTags(cloud.ResourceTypeNetwork, networkId)
 			Ω(err).Should(BeNil())
 			Ω(remainingTags[tagKey]).Should(Equal(""))
 		})