Use the last 8 chars of zoneID when generating a failure domain name

Create unit test for v1beta1 conversion

Author: wongni

api/v1beta1/conversion.go

@@ -22,6 +22,7 @@ import (
 	corev1 "k8s.io/api/core/v1"
 	conv "k8s.io/apimachinery/pkg/conversion"
 	"sigs.k8s.io/cluster-api-provider-cloudstack/api/v1beta2"
+	"sigs.k8s.io/cluster-api-provider-cloudstack/controllers"
 	"sigs.k8s.io/cluster-api-provider-cloudstack/pkg/cloud"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 )
@@ -31,7 +32,7 @@ const DefaultEndpointCredential = "global"
 //nolint:golint,revive,stylecheck
 func Convert_v1beta1_CloudStackCluster_To_v1beta2_CloudStackCluster(in *CloudStackCluster, out *v1beta2.CloudStackCluster, s conv.Scope) error {
 	out.ObjectMeta = in.ObjectMeta
-	failureDomains, err := getFailureDomains(in)
+	failureDomains, err := GetFailureDomains(in)
 	if err != nil {
 		return err
 	}
@@ -80,8 +81,8 @@ func getZones(csCluster *v1beta2.CloudStackCluster) []Zone {
 	return zones
 }
 
-// getFailureDomains maps v1beta1 zones to v1beta2 failure domains.
-func getFailureDomains(csCluster *CloudStackCluster) ([]v1beta2.CloudStackFailureDomainSpec, error) {
+// GetFailureDomains maps v1beta1 zones to v1beta2 failure domains.
+func GetFailureDomains(csCluster *CloudStackCluster) ([]v1beta2.CloudStackFailureDomainSpec, error) {
 	var failureDomains []v1beta2.CloudStackFailureDomainSpec
 	namespace := csCluster.Namespace
 	for _, zone := range csCluster.Spec.Zones {
@@ -103,7 +104,7 @@ func getFailureDomains(csCluster *CloudStackCluster) ([]v1beta2.CloudStackFailur
 			Domain:  csCluster.Spec.Domain,
 			Account: csCluster.Spec.Account,
 			ACSEndpoint: corev1.SecretReference{
-				Namespace: csCluster.Namespace,
+				Namespace: namespace,
 				Name:      DefaultEndpointCredential,
 			},
 		})
@@ -119,7 +120,7 @@ func getFailureDomains(csCluster *CloudStackCluster) ([]v1beta2.CloudStackFailur
 // When upgrading cluster using clusterctl directly, zoneID is fetched directly from kubernetes cluster in cloudstackzones.
 func GetDefaultFailureDomainName(namespace string, clusterName string, zoneID string, zoneName string) (string, error) {
 	if len(zoneID) > 0 {
-		return zoneID + "-" + clusterName, nil
+		return WithZoneID(zoneID, clusterName), nil
 	}
 
 	secret, err := GetK8sSecret(DefaultEndpointCredential, namespace)
@@ -130,14 +131,18 @@ func GetDefaultFailureDomainName(namespace string, clusterName string, zoneID st
 	// try fetch zoneID using zoneName through cloudstack client
 	zoneID, err = fetchZoneIDUsingCloudStack(secret, zoneName)
 	if err == nil {
-		return zoneID + "-" + clusterName, nil
+		return WithZoneID(zoneID, clusterName), nil
 	}
 
 	zoneID, err = fetchZoneIDUsingK8s(namespace, zoneName)
 	if err != nil {
 		return "", nil
 	}
-	return zoneID + "-" + clusterName, nil
+	return WithZoneID(zoneID, clusterName), nil
+}
+
+func WithZoneID(zoneID, clusterName string) string {
+	return controllers.WithClusterSuffix(zoneID[len(zoneID)-8:], clusterName)
 }
 
 func fetchZoneIDUsingK8s(namespace string, zoneName string) (string, error) {

api/v1beta1/conversion_test.go

@@ -0,0 +1,62 @@
+/*
+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.
+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 v1beta1_test
+
+import (
+	. "github.com/onsi/ginkgo/v2"
+	. "github.com/onsi/gomega"
+	infrav1 "sigs.k8s.io/cluster-api-provider-cloudstack/api/v1beta1"
+	capiv1 "sigs.k8s.io/cluster-api/api/v1beta1"
+)
+
+var _ = Describe("Conversion", func() {
+	BeforeEach(func() { // Reset test vars to initial state.
+	})
+
+	Context("GetFailureDomains function", func() {
+		It("Return the correct value when the last state update time is known", func() {
+			csCluster := &infrav1.CloudStackCluster{
+				Spec: infrav1.CloudStackClusterSpec{
+					Zones: []infrav1.Zone{
+						{
+							Name: "zone-name1",
+							Network: infrav1.Network{
+								Name: "network1",
+							},
+						},
+						{
+							ID: "76472a84-d23f-4e97-b154-ee1b975ed936",
+							Network: infrav1.Network{
+								Name: "network1",
+							},
+						},
+					},
+					ControlPlaneEndpoint: capiv1.APIEndpoint{
+						Host: "endpoint1",
+						Port: 443,
+					},
+					Account: "account1",
+					Domain:  "domain1",
+				},
+				Status: infrav1.CloudStackClusterStatus{},
+			}
+			failureDomains, err := infrav1.GetFailureDomains(csCluster)
+			Ω(err).ShouldNot(HaveOccurred())
+			Ω(failureDomains).Should(HaveLen(2))
+		})
+	})
+})

controllers/cloudstackcluster_controller.go

@@ -109,7 +109,7 @@ func (r *CloudStackClusterReconciliationRunner) VerifyFailureDomainCRDs() (ctrl.
 	for _, requiredFd := range r.ReconciliationSubject.Spec.FailureDomains {
 		found := false
 		for _, fd := range r.FailureDomains.Items {
-			requiredFDName := withClusterSuffix(requiredFd.Name, r.CAPICluster.Name)
+			requiredFDName := WithClusterSuffix(requiredFd.Name, r.CAPICluster.Name)
 			if requiredFDName == fd.Name {
 				found = true
 				if !fd.Status.Ready {
@@ -125,8 +125,8 @@ func (r *CloudStackClusterReconciliationRunner) VerifyFailureDomainCRDs() (ctrl.
 	return ctrl.Result{}, nil
 }
 
-// withClusterSuffix appends a hyphen and the cluster name to a name if not already present.
-func withClusterSuffix(name string, clusterName string) string {
+// WithClusterSuffix appends a hyphen and the cluster name to a name if not already present.
+func WithClusterSuffix(name string, clusterName string) string {
 	newName := name
 	if !strings.HasSuffix(name, "-"+clusterName) { // Add cluster name suffix if missing.
 		newName = name + "-" + clusterName
@@ -138,7 +138,7 @@ func withClusterSuffix(name string, clusterName string) string {
 func (r *CloudStackClusterReconciliationRunner) SetFailureDomainsStatusMap() (ctrl.Result, error) {
 	r.ReconciliationSubject.Status.FailureDomains = clusterv1.FailureDomains{}
 	for _, fdSpec := range r.ReconciliationSubject.Spec.FailureDomains {
-		fdSpec.Name = withClusterSuffix(fdSpec.Name, r.CAPICluster.Name)
+		fdSpec.Name = WithClusterSuffix(fdSpec.Name, r.CAPICluster.Name)
 		r.ReconciliationSubject.Status.FailureDomains[fdSpec.Name] = clusterv1.FailureDomainSpec{ControlPlane: true}
 	}
 	return ctrl.Result{}, nil