Working but needs cleanup.

Joshua Reed · Joshua Reed · commit d932f81b656a · 2022-08-10T12:22:29.000-07:00
diff --git a/api/v1beta1/zz_generated.conversion.go b/api/v1beta1/zz_generated.conversion.go
diff --git a/api/v1beta2/cloudstackfailuredomain_types.go b/api/v1beta2/cloudstackfailuredomain_types.go
@@ -17,10 +17,21 @@ limitations under the License.
 package v1beta2
 
 import (
+	"crypto/md5"
+	"fmt"
+
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
+// FailureDomainHashedMetaName returns an MD5 name generated from the FailureDomain and Cluster name.
+// FailureDomains must have a unique name even when potentially sharing a namespace with other clusters.
+// In the future we may remove the ability to run multiple clusters in a single namespace, but today
+// this is a consequence of being upstream of EKS-A which does run multiple clusters in a single namepace.
+func FailureDomainHashedMetaName(fdName, clusterName string) string {
+	return fmt.Sprintf("%x", md5.Sum([]byte(fdName+clusterName)))
+}
+
 const (
 	FailureDomainFinalizer = "cloudstackfailuredomain.infrastructure.cluster.x-k8s.io"
 	FailureDomainLabelName = "cloudstackfailuredomain.infrastructure.cluster.x-k8s.io/name"
diff --git a/api/v1beta2/cloudstackisolatednetwork_types.go b/api/v1beta2/cloudstackisolatednetwork_types.go
@@ -41,7 +41,7 @@ type CloudStackIsolatedNetworkSpec struct {
 
 	//+k8s:conversion-gen=false
 	// FailureDomain -- the FailureDomain the network is placed in.
-	FailureDomain CloudStackFailureDomainSpec `json:"failureDomain"`
+	FailureDomainName string `json:"failureDomainName"`
 }
 
 // CloudStackIsolatedNetworkStatus defines the observed state of CloudStackIsolatedNetwork
diff --git a/api/v1beta2/zz_generated.deepcopy.go b/api/v1beta2/zz_generated.deepcopy.go
diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_cloudstackisolatednetworks.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_cloudstackisolatednetworks.yaml
@@ -118,66 +118,10 @@ spec:
                 - host
                 - port
                 type: object
-              failureDomain:
+              failureDomainName:
                 description: FailureDomain -- the FailureDomain the network is placed
                   in.
-                properties:
-                  account:
-                    description: CloudStack account.
-                    type: string
-                  acsendpoint:
-                    description: Apache CloudStack Endpoint secret reference.
-                    properties:
-                      name:
-                        description: Name is unique within a namespace to reference
-                          a secret resource.
-                        type: string
-                      namespace:
-                        description: Namespace defines the space within which the
-                          secret name must be unique.
-                        type: string
-                    type: object
-                  domain:
-                    description: CloudStack domain.
-                    type: string
-                  name:
-                    description: The failure domain unique name.
-                    type: string
-                  zone:
-                    description: The ACS Zone for this failure domain.
-                    properties:
-                      id:
-                        description: ID.
-                        type: string
-                      name:
-                        description: Name.
-                        type: string
-                      network:
-                        description: The network within the Zone to use.
-                        properties:
-                          id:
-                            description: Cloudstack Network ID the cluster is built
-                              in.
-                            type: string
-                          name:
-                            description: Cloudstack Network Name the cluster is built
-                              in.
-                            type: string
-                          type:
-                            description: Cloudstack Network Type the cluster is built
-                              in.
-                            type: string
-                        required:
-                        - name
-                        type: object
-                    required:
-                    - network
-                    type: object
-                required:
-                - acsendpoint
-                - name
-                - zone
-                type: object
+                type: string
               id:
                 description: ID.
                 type: string
@@ -186,7 +130,7 @@ spec:
                 type: string
             required:
             - controlPlaneEndpoint
-            - failureDomain
+            - failureDomainName
             type: object
           status:
             description: CloudStackIsolatedNetworkStatus defines the observed state
diff --git a/controllers/cloudstackaffinitygroup_controller.go b/controllers/cloudstackaffinitygroup_controller.go
@@ -20,7 +20,6 @@ import (
 	"context"
 
 	ctrl "sigs.k8s.io/controller-runtime"
-	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
 
 	infrav1 "sigs.k8s.io/cluster-api-provider-cloudstack/api/v1beta2"
@@ -60,9 +59,7 @@ func (reconciler *CloudStackAffinityGroupReconciler) Reconcile(ctx context.Conte
 	r := NewCSAGReconciliationRunner()
 	r.UsingBaseReconciler(reconciler.ReconcilerBase).ForRequest(req).WithRequestCtx(ctx)
 	r.WithAdditionalCommonStages(
-		r.GetObjectByName("placeholder", r.FailureDomain,
-			func() string { return r.ReconciliationSubject.Spec.FailureDomainName }),
-		r.CheckPresent(map[string]client.Object{"CloudStackFailureDomain": r.FailureDomain}),
+		r.GetFailureDomainByName(func() string { return r.ReconciliationSubject.Spec.FailureDomainName }, r.FailureDomain),
 		r.AsFailureDomainUser(&r.FailureDomain.Spec))
 	return r.RunBaseReconciliationStages()
 }
diff --git a/controllers/cloudstackcluster_controller.go b/controllers/cloudstackcluster_controller.go
@@ -30,7 +30,6 @@ import (
 
 	"github.com/pkg/errors"
 	infrav1 "sigs.k8s.io/cluster-api-provider-cloudstack/api/v1beta2"
-	"sigs.k8s.io/cluster-api-provider-cloudstack/controllers/utils"
 	csCtrlrUtils "sigs.k8s.io/cluster-api-provider-cloudstack/controllers/utils"
 	clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
 	"sigs.k8s.io/cluster-api/util"
@@ -106,20 +105,20 @@ func (r *CloudStackClusterReconciliationRunner) SetReady() (ctrl.Result, error)
 // VerifyFailureDomainCRDs verifies the FailureDomains found match against those requested.
 func (r *CloudStackClusterReconciliationRunner) VerifyFailureDomainCRDs() (ctrl.Result, error) {
 	// Check that all required failure domains are present and ready.
-	for _, requiredFd := range r.ReconciliationSubject.Spec.FailureDomains {
+	fmt.Println(r.ReconciliationSubject.Spec.FailureDomains)
+	for _, requiredFdSpec := range r.ReconciliationSubject.Spec.FailureDomains {
 		found := false
 		for _, fd := range r.FailureDomains.Items {
-			requiredFDName := utils.WithClusterSuffix(requiredFd.Name, r.CAPICluster.Name)
-			if requiredFDName == fd.Name {
+			if requiredFdSpec.Name == fd.Spec.Name {
 				found = true
 				if !fd.Status.Ready {
-					return r.RequeueWithMessage(fmt.Sprintf("Required FailureDomain %s not ready, requeueing.", fd.Name))
+					return r.RequeueWithMessage(fmt.Sprintf("Required FailureDomain %s not ready, requeueing.", fd.Spec.Name))
 				}
 				break
 			}
 		}
 		if !found {
-			return r.RequeueWithMessage(fmt.Sprintf("Required FailureDomain %s not found, requeueing.", requiredFd.Name))
+			return r.RequeueWithMessage(fmt.Sprintf("Required FailureDomain %s not found, requeueing.", requiredFdSpec.Name))
 		}
 	}
 	return ctrl.Result{}, nil
@@ -129,8 +128,9 @@ func (r *CloudStackClusterReconciliationRunner) VerifyFailureDomainCRDs() (ctrl.
 func (r *CloudStackClusterReconciliationRunner) SetFailureDomainsStatusMap() (ctrl.Result, error) {
 	r.ReconciliationSubject.Status.FailureDomains = clusterv1.FailureDomains{}
 	for _, fdSpec := range r.ReconciliationSubject.Spec.FailureDomains {
-		fdSpec.Name = utils.WithClusterSuffix(fdSpec.Name, r.CAPICluster.Name)
-		r.ReconciliationSubject.Status.FailureDomains[fdSpec.Name] = clusterv1.FailureDomainSpec{ControlPlane: true}
+		metaHashName := infrav1.FailureDomainHashedMetaName(fdSpec.Name, r.CAPICluster.Name)
+		r.ReconciliationSubject.Status.FailureDomains[fdSpec.Name] = clusterv1.FailureDomainSpec{
+			ControlPlane: true, Attributes: map[string]string{"MetaHashName": metaHashName}}
 	}
 	return ctrl.Result{}, nil
 }
diff --git a/controllers/cloudstackfailuredomain_controller.go b/controllers/cloudstackfailuredomain_controller.go
@@ -84,17 +84,14 @@ func (r *CloudStackFailureDomainReconciliationRunner) Reconcile() (retRes ctrl.R
 		!csCtrlrUtils.ContainsNoMatchSubstring(err) {
 		return ctrl.Result{}, errors.Wrap(err, "resolving Cloudstack network information")
 	}
-	// Patch Zone ID back before potentially creating an isolated network.
-	if err := r.Patcher.Patch(r.RequestCtx, r.ReconciliationSubject); err != nil {
-		return ctrl.Result{}, err
-	}
 
 	// Check if the passed network was an isolated network or the network was missing. In either case, create a
 	// CloudStackIsolatedNetwork to manage the many intricacies and wait until CloudStackIsolatedNetwork is ready.
 	if r.ReconciliationSubject.Spec.Zone.Network.ID == "" ||
 		r.ReconciliationSubject.Spec.Zone.Network.Type == infrav1.NetworkTypeIsolated {
 		netName := r.ReconciliationSubject.Spec.Zone.Network.Name
-		if res, err := r.GenerateIsolatedNetwork(netName)(); r.ShouldReturn(res, err) {
+		if res, err := r.GenerateIsolatedNetwork(
+			netName, func() string { return r.ReconciliationSubject.Spec.Name })(); r.ShouldReturn(res, err) {
 			return res, err
 		} else if res, err := r.GetObjectByName(r.IsoNetMetaName(netName), r.IsoNet)(); r.ShouldReturn(res, err) {
 			return res, err
diff --git a/controllers/cloudstackisolatednetwork_controller.go b/controllers/cloudstackisolatednetwork_controller.go
@@ -22,7 +22,6 @@ import (
 
 	"sigs.k8s.io/cluster-api/util/patch"
 	ctrl "sigs.k8s.io/controller-runtime"
-	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
 
 	"github.com/pkg/errors"
@@ -61,8 +60,7 @@ func (reconciler *CloudStackIsoNetReconciler) Reconcile(ctx context.Context, req
 	r := NewCSIsoNetReconciliationRunner()
 	r.UsingBaseReconciler(reconciler.ReconcilerBase).ForRequest(req).WithRequestCtx(ctx)
 	r.WithAdditionalCommonStages(
-		r.GetParent(r.ReconciliationSubject, r.FailureDomain),
-		r.CheckPresent(map[string]client.Object{"CloudStackFailureDomain": r.FailureDomain}),
+		r.GetFailureDomainByName(func() string { return r.ReconciliationSubject.Spec.FailureDomainName }, r.FailureDomain),
 		r.AsFailureDomainUser(&r.FailureDomain.Spec),
 	)
 	return r.RunBaseReconciliationStages()
diff --git a/controllers/cloudstackmachine_controller.go b/controllers/cloudstackmachine_controller.go
@@ -21,7 +21,6 @@ import (
 	"fmt"
 	"math/rand"
 	"reflect"
-	"strings"
 
 	"github.com/pkg/errors"
 	corev1 "k8s.io/api/core/v1"
@@ -87,9 +86,7 @@ func (reconciler *CloudStackMachineReconciler) Reconcile(ctx context.Context, re
 		r.GetParent(r.ReconciliationSubject, r.CAPIMachine),
 		r.RequeueIfCloudStackClusterNotReady,
 		r.SetFailureDomainOnCSMachine,
-		r.GetObjectByName("placeholder", r.FailureDomain,
-			func() string { return r.ReconciliationSubject.Spec.FailureDomainName }),
-		r.CheckPresent(map[string]client.Object{"CloudStackFailureDomain": r.FailureDomain}),
+		r.GetFailureDomainByName(func() string { return r.ReconciliationSubject.Spec.FailureDomainName }, r.FailureDomain),
 		r.AsFailureDomainUser(&r.FailureDomain.Spec))
 	return r.RunBaseReconciliationStages()
 }
@@ -147,9 +144,6 @@ func (r *CloudStackMachineReconciliationRunner) SetFailureDomainOnCSMachine() (r
 			randNum := (rand.Int() % len(r.CSCluster.Spec.FailureDomains)) // #nosec G404 -- weak crypt rand doesn't matter here.
 			name = r.CSCluster.Spec.FailureDomains[randNum].Name
 		}
-		if !strings.HasSuffix(name, "-"+r.CAPICluster.Name) { // Add cluster name suffix if missing.
-			name = name + "-" + r.CAPICluster.Name
-		}
 		r.ReconciliationSubject.Spec.FailureDomainName = name
 		r.ReconciliationSubject.Labels[infrav1.FailureDomainLabelName] = r.ReconciliationSubject.Spec.FailureDomainName
 	}
diff --git a/controllers/cloudstackmachinestatechecker_controller.go b/controllers/cloudstackmachinestatechecker_controller.go
@@ -74,9 +74,7 @@ func (r *CloudStackMachineStateCheckerReconciliationRunner) Reconcile() (ctrl.Re
 		r.GetParent(r.ReconciliationSubject, r.CSMachine),
 		r.GetParent(r.CSMachine, r.CAPIMachine),
 		r.CheckPresent(map[string]client.Object{"CloudStackMachine": r.CSMachine, "Machine": r.CAPIMachine}),
-		r.GetObjectByName("placeholder", r.FailureDomain,
-			func() string { return r.CSMachine.Spec.FailureDomainName }),
-		r.CheckPresent(map[string]client.Object{"CloudStackFailureDomain": r.FailureDomain}),
+		r.GetFailureDomainByName(func() string { return r.CSMachine.Spec.FailureDomainName }, r.FailureDomain),
 		r.AsFailureDomainUser(&r.FailureDomain.Spec),
 		func() (ctrl.Result, error) {
 			if err := r.CSClient.ResolveVMInstanceDetails(r.CSMachine); err != nil {
diff --git a/controllers/utils/failuredomains.go b/controllers/utils/failuredomains.go
@@ -33,8 +33,9 @@ import (
 
 // CreateFailureDomain creates a specified CloudStackFailureDomain CRD owned by the ReconcilationSubject.
 func (r *ReconciliationRunner) CreateFailureDomain(fdSpec infrav1.CloudStackFailureDomainSpec) error {
+	metaHashName := infrav1.FailureDomainHashedMetaName(fdSpec.Name, r.CAPICluster.Name)
 	csFD := &infrav1.CloudStackFailureDomain{
-		ObjectMeta: r.NewChildObjectMeta(fdSpec.Name),
+		ObjectMeta: r.NewChildObjectMeta(metaHashName),
 		Spec:       fdSpec,
 	}
 	return errors.Wrap(r.K8sClient.Create(r.RequestCtx, csFD), "creating CloudStackFailureDomain")
@@ -44,9 +45,6 @@ func (r *ReconciliationRunner) CreateFailureDomain(fdSpec infrav1.CloudStackFail
 func (r *ReconciliationRunner) CreateFailureDomains(fdSpecs []infrav1.CloudStackFailureDomainSpec) CloudStackReconcilerMethod {
 	return func() (ctrl.Result, error) {
 		for _, fdSpec := range fdSpecs {
-			if !strings.HasSuffix(fdSpec.Name, "-"+r.CAPICluster.Name) { // Add cluster name suffix if missing.
-				fdSpec.Name = fdSpec.Name + "-" + r.CAPICluster.Name
-			}
 			if err := r.CreateFailureDomain(fdSpec); err != nil {
 				if !strings.Contains(strings.ToLower(err.Error()), "already exists") {
 					return reconcile.Result{}, errors.Wrap(err, "creating CloudStackFailureDomains")
@@ -73,22 +71,30 @@ func (r *ReconciliationRunner) GetFailureDomains(fds *infrav1.CloudStackFailureD
 	}
 }
 
+// GetFailureDomains gets CloudStackFailureDomains owned by a CloudStackCluster.
+func (r *ReconciliationRunner) GetFailureDomainByName(nameFunc func() string, fd *infrav1.CloudStackFailureDomain) CloudStackReconcilerMethod {
+	return func() (ctrl.Result, error) {
+		metaHashName := infrav1.FailureDomainHashedMetaName(nameFunc(), r.CAPICluster.Name)
+		if err := r.K8sClient.Get(r.RequestCtx, client.ObjectKey{Namespace: r.Request.Namespace, Name: metaHashName}, fd); err != nil {
+			return ctrl.Result{}, errors.Wrapf(err, "failed to get failure domain with name %s", nameFunc())
+		}
+		return ctrl.Result{}, nil
+	}
+}
+
 // RemoveExtraneousFailureDomains deletes failure domains no longer listed under the CloudStackCluster's spec.
 func (r *ReconciliationRunner) RemoveExtraneousFailureDomains(fds *infrav1.CloudStackFailureDomainList) CloudStackReconcilerMethod {
 	return func() (ctrl.Result, error) {
 		// Toss together a precense map.
 		fdPresenceByName := map[string]bool{}
 		for _, fdSpec := range r.CSCluster.Spec.FailureDomains {
 			name := fdSpec.Name
-			if !strings.HasSuffix(name, "-"+r.CAPICluster.Name) { // Add cluster name suffix if missing.
-				name = name + "-" + r.CAPICluster.Name
-			}
 			fdPresenceByName[name] = true
 		}
 
 		// Send a deletion request for each FailureDomain not speced for.
 		for _, fd := range fds.Items {
-			if _, present := fdPresenceByName[fd.Name]; !present {
+			if _, present := fdPresenceByName[fd.Spec.Name]; !present {
 				toDelete := fd
 				r.Log.Info(fmt.Sprintf("Deleting extraneous failure domain: %s.", fd.Name))
 				if err := r.K8sClient.Delete(r.RequestCtx, &toDelete); err != nil {
diff --git a/controllers/utils/isolated_network.go b/controllers/utils/isolated_network.go
@@ -29,13 +29,14 @@ func (r *ReconciliationRunner) IsoNetMetaName(name string) string {
 }
 
 // GenerateIsolatedNetwork of the passed name that's owned by the ReconciliationSubject.
-func (r *ReconciliationRunner) GenerateIsolatedNetwork(name string) CloudStackReconcilerMethod {
+func (r *ReconciliationRunner) GenerateIsolatedNetwork(name string, fdNameFunc func() string) CloudStackReconcilerMethod {
 	return func() (ctrl.Result, error) {
 		lowerName := strings.ToLower(name)
 		metaName := fmt.Sprintf("%s-%s", r.CSCluster.Name, lowerName)
 		csIsoNet := &infrav1.CloudStackIsolatedNetwork{}
 		csIsoNet.ObjectMeta = r.NewChildObjectMeta(metaName)
 		csIsoNet.Spec.Name = lowerName
+		csIsoNet.Spec.FailureDomainName = fdNameFunc()
 		csIsoNet.Spec.ControlPlaneEndpoint.Host = r.CSCluster.Spec.ControlPlaneEndpoint.Host
 		csIsoNet.Spec.ControlPlaneEndpoint.Port = r.CSCluster.Spec.ControlPlaneEndpoint.Port
 

Original file line number	Diff line number	Diff line change
`@@ -41,7 +41,7 @@ type CloudStackIsolatedNetworkSpec struct {`
`41`	`41`
`42`	`42`	`//+k8s:conversion-gen=false`
`43`	`43`	`// FailureDomain -- the FailureDomain the network is placed in.`
`44`		- FailureDomain CloudStackFailureDomainSpec `json:"failureDomain"`
	`44`	+ FailureDomainName string `json:"failureDomainName"`
`45`	`45`	`}`
`46`	`46`
`47`	`47`	`// CloudStackIsolatedNetworkStatus defines the observed state of CloudStackIsolatedNetwork`