Merge pull request #28 from aws/feature/network-tags

mrog · web-flow · commit 623e4474aa6a · 2022-02-17T11:33:23.000-07:00
feature/tag-network
diff --git a/api/v1beta1/cloudstackcluster_webhook_test.go b/api/v1beta1/cloudstackcluster_webhook_test.go
@@ -31,6 +31,7 @@ var _ = Describe("CloudStackCluster webhooks", func() {
 		clusterKind        = "CloudStackCluster"
 		clusterName        = "test-cluster"
 		clusterNamespace   = "default"
+		clusterId          = "0"
 		identitySecretName = "IdentitySecret"
 		zone               = "Zone"
 		network            = "Network"
@@ -47,6 +48,7 @@ var _ = Describe("CloudStackCluster webhooks", func() {
 				ObjectMeta: metav1.ObjectMeta{
 					Name:      clusterName,
 					Namespace: clusterNamespace,
+					UID:       clusterId,
 				},
 				Spec: CloudStackClusterSpec{
 					IdentityRef: &CloudStackIdentityReference{
@@ -72,6 +74,7 @@ var _ = Describe("CloudStackCluster webhooks", func() {
 				ObjectMeta: metav1.ObjectMeta{
 					Name:      clusterName,
 					Namespace: clusterNamespace,
+					UID:       clusterId,
 				},
 				Spec: CloudStackClusterSpec{
 					IdentityRef: &CloudStackIdentityReference{
@@ -96,6 +99,7 @@ var _ = Describe("CloudStackCluster webhooks", func() {
 				ObjectMeta: metav1.ObjectMeta{
 					Name:      clusterName,
 					Namespace: clusterNamespace,
+					UID:       clusterId,
 				},
 				Spec: CloudStackClusterSpec{
 					IdentityRef: &CloudStackIdentityReference{
@@ -125,6 +129,7 @@ var _ = Describe("CloudStackCluster webhooks", func() {
 				ObjectMeta: metav1.ObjectMeta{
 					Name:      clusterName,
 					Namespace: clusterNamespace,
+					UID:       clusterId,
 				},
 				Spec: CloudStackClusterSpec{
 					IdentityRef: &CloudStackIdentityReference{
diff --git a/controllers/cloudstackcluster_controller.go b/controllers/cloudstackcluster_controller.go
@@ -145,8 +145,9 @@ func (r *CloudStackClusterReconciler) reconcileDelete(
 
 	log.V(1).Info("reconcileDelete CloudStackCluster...")
 
-	// TODO Decide what resources to remove w/Cluster if any.
-	// cloud.DestroyCluster(r.CS, csStackCluster)
+	if err := r.CS.DisposeClusterResources(csCluster); err != nil {
+		return ctrl.Result{}, err
+	}
 
 	controllerutil.RemoveFinalizer(csCluster, infrav1.ClusterFinalizer)
 	return ctrl.Result{}, nil
diff --git a/controllers/cloudstackcluster_controller_test.go b/controllers/cloudstackcluster_controller_test.go
@@ -40,6 +40,7 @@ func getCloudStackCluster() *infrav1.CloudStackCluster {
 		ObjectMeta: metav1.ObjectMeta{
 			GenerateName: "cs-cluster-test1-",
 			Namespace:    "default",
+			UID:          "0",
 		},
 		Spec: infrav1.CloudStackClusterSpec{
 			Zone:    "zone",
diff --git a/pkg/cloud/affinity_groups_test.go b/pkg/cloud/affinity_groups_test.go
@@ -18,7 +18,6 @@ package cloud_test
 
 import (
 	"errors"
-
 	"github.com/apache/cloudstack-go/v2/cloudstack"
 	infrav1 "github.com/aws/cluster-api-provider-cloudstack/api/v1beta1"
 	"github.com/aws/cluster-api-provider-cloudstack/pkg/cloud"
@@ -51,6 +50,7 @@ var _ = Describe("AffinityGroup Unit Tests", func() {
 			Type: cloud.AffinityGroupType}
 		cluster = &infrav1.CloudStackCluster{Spec: infrav1.CloudStackClusterSpec{
 			Zone: "Zone1", Network: "SharedGuestNet1"}}
+		cluster.ObjectMeta.SetUID("0")
 		machine = &infrav1.CloudStackMachine{Spec: infrav1.CloudStackMachineSpec{
 			Offering: "Medium Instance", Template: "Ubuntu20"}}
 		machine.ObjectMeta.SetName("rejoshed-affinity-group-test-vm")
diff --git a/pkg/cloud/client.go b/pkg/cloud/client.go
@@ -37,6 +37,7 @@ type Client interface {
 	ResolveLoadBalancerRuleDetails(*infrav1.CloudStackCluster) error
 	GetOrCreateLoadBalancerRule(*infrav1.CloudStackCluster) error
 	AffinityGroupIFace
+	TagIFace
 }
 
 type client struct {
diff --git a/pkg/cloud/cluster.go b/pkg/cloud/cluster.go
@@ -24,6 +24,7 @@ import (
 
 type ClusterIface interface {
 	GetOrCreateCluster(*infrav1.CloudStackCluster) error
+	DisposeClusterResources(cluster *infrav1.CloudStackCluster) error
 }
 
 func (c *client) resolveZone(csCluster *infrav1.CloudStackCluster) (retErr error) {
@@ -92,3 +93,10 @@ func (c *client) GetOrCreateCluster(csCluster *infrav1.CloudStackCluster) (retEr
 	csCluster.Status.Ready = true
 	return nil
 }
+
+func (c *client) DisposeClusterResources(csCluster *infrav1.CloudStackCluster) (retError error) {
+	if err := c.RemoveClusterTagFromNetwork(csCluster); err != nil {
+		return err
+	}
+	return c.DeleteNetworkIfNotInUse(csCluster)
+}
diff --git a/pkg/cloud/cluster_test.go b/pkg/cloud/cluster_test.go
@@ -17,6 +17,7 @@ package cloud_test
 
 import (
 	"fmt"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 
 	"github.com/apache/cloudstack-go/v2/cloudstack"
 	infrav1 "github.com/aws/cluster-api-provider-cloudstack/api/v1beta1"
@@ -58,7 +59,11 @@ var _ = Describe("Cluster", func() {
 		cluster := &infrav1.CloudStackCluster{
 			Spec: infrav1.CloudStackClusterSpec{
 				Zone:    zoneName,
-				Network: netName}}
+				Network: netName},
+			ObjectMeta: metav1.ObjectMeta{
+				UID: "0",
+			},
+		}
 
 		It("handles zone not found.", func() {
 			expectedErr := fmt.Errorf("Not found")
diff --git a/pkg/cloud/instance_test.go b/pkg/cloud/instance_test.go
@@ -18,6 +18,7 @@ package cloud_test
 
 import (
 	"fmt"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 
 	capiv1 "sigs.k8s.io/cluster-api/api/v1beta1"
 
@@ -64,8 +65,9 @@ var _ = Describe("Instance", func() {
 				Template:   "template-name"}}
 		csMachine.Name = "instance-name"
 		csCluster = &infrav1.CloudStackCluster{
-			Spec:   infrav1.CloudStackClusterSpec{},
-			Status: infrav1.CloudStackClusterStatus{ZoneID: "zone-id"}}
+			Spec:       infrav1.CloudStackClusterSpec{},
+			Status:     infrav1.CloudStackClusterStatus{ZoneID: "zone-id"},
+			ObjectMeta: metav1.ObjectMeta{UID: "0"}}
 		machine = &capiv1.Machine{}
 	})
 
diff --git a/pkg/cloud/network.go b/pkg/cloud/network.go
@@ -58,9 +58,13 @@ 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 nil
+		return addClusterTags(c, csCluster, false)
 	} else if !strings.Contains(retErr.Error(), "No match found") { // Some other error.
 		return retErr
 	} // Network not found.
@@ -86,6 +90,66 @@ 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
+}
+
+func (c *client) RemoveClusterTagFromNetwork(csCluster *infrav1.CloudStackCluster) (retError error) {
+	tags, err := c.GetNetworkTags(csCluster.Status.NetworkID)
+	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
+		}
+	}
+
+	return nil
+}
+
+func (c *client) DeleteNetworkIfNotInUse(csCluster *infrav1.CloudStackCluster) (retError error) {
+	tags, err := c.GetNetworkTags(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] != "" {
+		return c.DestroyNetwork(csCluster)
+	}
+
 	return nil
 }
 
diff --git a/pkg/cloud/network_test.go b/pkg/cloud/network_test.go
@@ -24,6 +24,7 @@ import (
 	. "github.com/onsi/ginkgo"
 	. "github.com/onsi/gomega"
 	"github.com/pkg/errors"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
 )
 
@@ -36,6 +37,7 @@ var _ = Describe("Network", func() {
 		fs         *cloudstack.MockFirewallServiceIface
 		as         *cloudstack.MockAddressServiceIface
 		lbs        *cloudstack.MockLoadBalancerServiceIface
+		rs         *cloudstack.MockResourcetagsServiceIface
 		csCluster  *infrav1.CloudStackCluster
 		client     cloud.Client
 	)
@@ -59,6 +61,7 @@ var _ = Describe("Network", func() {
 		fs = mockClient.Firewall.(*cloudstack.MockFirewallServiceIface)
 		as = mockClient.Address.(*cloudstack.MockAddressServiceIface)
 		lbs = mockClient.LoadBalancer.(*cloudstack.MockLoadBalancerServiceIface)
+		rs = mockClient.Resourcetags.(*cloudstack.MockResourcetagsServiceIface)
 		client = cloud.NewClientFromCSAPIClient(mockClient)
 
 		// Reset csCluster.
@@ -68,13 +71,26 @@ var _ = Describe("Network", func() {
 				Network:              fakeNetName,
 				ControlPlaneEndpoint: clusterv1.APIEndpoint{Port: int32(6443)},
 			},
+			ObjectMeta: metav1.ObjectMeta{
+				UID: "0",
+			},
 		}
 	})
 
 	AfterEach(func() {
 		mockCtrl.Finish()
 	})
 
+	// Sets expectations network tag creation.  To be used by tests that get/create networks.
+	expectNetworkTags := func(networkId string) {
+		listTagsParams := &cloudstack.ListTagsParams{}
+		createTagsParams := &cloudstack.CreateTagsParams{}
+		rs.EXPECT().NewListTagsParams().Return(listTagsParams)
+		rs.EXPECT().ListTags(listTagsParams).Return(&cloudstack.ListTagsResponse{}, nil)
+		rs.EXPECT().NewCreateTagsParams([]string{networkId}, "network", gomock.Any()).Return(createTagsParams)
+		rs.EXPECT().CreateTags(createTagsParams).Return(&cloudstack.CreateTagsResponse{}, nil)
+	}
+
 	Context("for an existing network", func() {
 		It("resolves network details in cluster status", func() {
 			ns.EXPECT().GetNetworkID(fakeNetName).Return(fakeNetId, 1, nil)
@@ -87,13 +103,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)
 
 			Ω(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)
 
 			csCluster.Spec.Network = fakeNetId
 			Ω(client.GetOrCreateNetwork(csCluster)).Should(Succeed())
@@ -108,6 +126,8 @@ 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)
+
 			Ω(client.GetOrCreateNetwork(csCluster)).Should(Succeed())
 		})
 	})
diff --git a/pkg/cloud/tags.go b/pkg/cloud/tags.go
@@ -0,0 +1,60 @@
+/*
+Copyright 2022.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package cloud
+
+type TagIFace interface {
+	AddNetworkTags(string, map[string]string) error
+	GetNetworkTags(string) (map[string]string, error)
+	DeleteNetworkTags(string, map[string]string) error
+}
+
+const (
+	clusterTagNamePrefix = "CAPC_cluster_"
+	createdByCapcTagName = "created_by_CAPC"
+	resourceTypeNetwork  = "network"
+)
+
+// 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)
+	_, err := c.cs.Resourcetags.CreateTags(p)
+	return err
+}
+
+// GetNetworkTags gets tags by network id.
+func (c *client) GetNetworkTags(networkId string) (map[string]string, error) {
+	p := c.cs.Resourcetags.NewListTagsParams()
+	p.SetResourceid(networkId)
+	p.SetResourcetype(resourceTypeNetwork)
+	if listTagResponse, err := c.cs.Resourcetags.ListTags(p); err != nil {
+		return nil, err
+	} else {
+		tags := make(map[string]string, listTagResponse.Count)
+		for _, t := range listTagResponse.Tags {
+			tags[t.Key] = t.Value
+		}
+		return tags, nil
+	}
+}
+
+// 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)
+	p.SetTags(tagsToDelete)
+	_, err := c.cs.Resourcetags.DeleteTags(p)
+	return err
+}
diff --git a/pkg/cloud/tags_test.go b/pkg/cloud/tags_test.go

Original file line number	Diff line number	Diff line change
`@@ -37,6 +37,7 @@ type Client interface {`
`37`	`37`	`ResolveLoadBalancerRuleDetails(*infrav1.CloudStackCluster) error`
`38`	`38`	`GetOrCreateLoadBalancerRule(*infrav1.CloudStackCluster) error`
`39`	`39`	`AffinityGroupIFace`
	`40`	`+ TagIFace`
`40`	`41`	`}`
`41`	`42`
`42`	`43`	`type client struct {`