Merge pull request #147 from rejoshed/feature/hash-failure-domain-naming

Feature/hash failure domain naming

.gitignore

@@ -29,6 +29,9 @@ cluster-api
 # Ignore output of Makefile sed operations created when generating manifests.
 config/default/manager_image_patch_edited.yaml
 
+# Ignore output of e2e kustomization of templates.
+test/e2e/data/infrastructure-cloudstack/v1beta*/*yaml
+
 # Test binary, build with `go test -c`
 *.test
 

Makefile

@@ -188,6 +188,7 @@ clean: ## Clean.
 	rm -rf $(RELEASE_DIR)
 	rm -rf bin
 	rm -rf cluster-api
+	rm -rf test/e2e/data/infrastructure-cloudstack/*/*yaml
 
 ##@ Testing
 
@@ -223,8 +224,7 @@ cluster-api/tilt-settings.json: hack/tilt-settings.json cluster-api
 	cp ./hack/tilt-settings.json cluster-api
 
 ##@ End-to-End Testing
-
-CLUSTER_TEMPLATES_INPUT_FILES=$(shell find test/e2e/data/infrastructure-cloudstack/v1beta2/*/cluster-template*/* test/e2e/data/infrastructure-cloudstack/*/bases/* -type f)
+CLUSTER_TEMPLATES_INPUT_FILES=$(shell find test/e2e/data/infrastructure-cloudstack/v1beta*/*/cluster-template* test/e2e/data/infrastructure-cloudstack/*/bases/* -type f)
 CLUSTER_TEMPLATES_OUTPUT_FILES=$(shell find test/e2e/data/infrastructure-cloudstack -type d -name "cluster-template*" -exec echo {}.yaml \;)
 .PHONY: e2e-cluster-templates
 e2e-cluster-templates: $(CLUSTER_TEMPLATES_OUTPUT_FILES) ## Generate cluster template files for e2e testing.

api/v1beta2/cloudstackfailuredomain_types.go

@@ -17,10 +17,21 @@ limitations under the License.
 package v1beta2
 
 import (
+	"crypto/md5" // #nosec G501 -- weak cryptographic primitive doesn't matter here. Not security related.
+	"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))) // #nosec G401 -- weak cryptographic primitive doesn't matter here. Not security related.
+}
+
 const (
 	FailureDomainFinalizer = "cloudstackfailuredomain.infrastructure.cluster.x-k8s.io"
 	FailureDomainLabelName = "cloudstackfailuredomain.infrastructure.cluster.x-k8s.io/name"
@@ -70,8 +81,9 @@ type CloudStackFailureDomainSpec struct {
 	// CloudStack domain.
 	// +optional
 	Domain string `json:"domain,omitempty"`
+
 	// Apache CloudStack Endpoint secret reference.
-	ACSEndpoint corev1.SecretReference `json:"acsendpoint"`
+	ACSEndpoint corev1.SecretReference `json:"acsEndpoint"`
 }
 
 // CloudStackFailureDomainStatus defines the observed state of CloudStackFailureDomain

api/v1beta2/cloudstackisolatednetwork_types.go

@@ -40,8 +40,8 @@ type CloudStackIsolatedNetworkSpec struct {
 	ControlPlaneEndpoint clusterv1.APIEndpoint `json:"controlPlaneEndpoint"`
 
 	//+k8s:conversion-gen=false
-	// FailureDomain -- the FailureDomain the network is placed in.
-	FailureDomain CloudStackFailureDomainSpec `json:"failureDomain"`
+	// FailureDomainName -- the FailureDomain the network is placed in.
+	FailureDomainName string `json:"failureDomainName"`
 }
 
 // CloudStackIsolatedNetworkStatus defines the observed state of CloudStackIsolatedNetwork

api/v1beta2/cloudstackmachine_types.go

@@ -63,7 +63,7 @@ type CloudStackMachineSpec struct {
 
 	// Optional affinitygroupids for deployVirtualMachine
 	// +optional
-	AffinityGroupIDs []string `json:"affinitygroupids,omitempty"`
+	AffinityGroupIDs []string `json:"affinityGroupIDs,omitempty"`
 
 	// Mutually exclusive parameter with AffinityGroupIDs.
 	// Defaults to `no`. Can be `pro` or `anti`. Will create an affinity group per machine set.
@@ -73,7 +73,7 @@ type CloudStackMachineSpec struct {
 	// Mutually exclusive parameter with AffinityGroupIDs.
 	// Is a reference to a CloudStack affinity group CRD.
 	// +optional
-	AffinityGroupRef *corev1.ObjectReference `json:"cloudstackaffinityref,omitempty"`
+	AffinityGroupRef *corev1.ObjectReference `json:"cloudstackAffinityRef,omitempty"`
 
 	// The CS specific unique identifier. Of the form: fmt.Sprintf("cloudstack:///%s", CS Machine ID)
 	// +optional

config/crd/bases/infrastructure.cluster.x-k8s.io_cloudstackclusters.yaml

@@ -223,7 +223,7 @@ spec:
                     account:
                       description: CloudStack account.
                       type: string
-                    acsendpoint:
+                    acsEndpoint:
                       description: Apache CloudStack Endpoint secret reference.
                       properties:
                         name:
@@ -272,7 +272,7 @@ spec:
                       - network
                       type: object
                   required:
-                  - acsendpoint
+                  - acsEndpoint
                   - name
                   - zone
                   type: object

config/crd/bases/infrastructure.cluster.x-k8s.io_cloudstackfailuredomains.yaml

@@ -41,7 +41,7 @@ spec:
               account:
                 description: CloudStack account.
                 type: string
-              acsendpoint:
+              acsEndpoint:
                 description: Apache CloudStack Endpoint secret reference.
                 properties:
                   name:
@@ -89,7 +89,7 @@ spec:
                 - network
                 type: object
             required:
-            - acsendpoint
+            - acsEndpoint
             - name
             - zone
             type: object

config/crd/bases/infrastructure.cluster.x-k8s.io_cloudstackisolatednetworks.yaml

@@ -118,66 +118,10 @@ spec:
                 - host
                 - port
                 type: object
-              failureDomain:
-                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
+              failureDomainName:
+                description: FailureDomainName -- the FailureDomain the network is
+                  placed in.
+                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

config/crd/bases/infrastructure.cluster.x-k8s.io_cloudstackmachines.yaml

@@ -301,12 +301,12 @@ spec:
                   to `no`. Can be `pro` or `anti`. Will create an affinity group per
                   machine set.
                 type: string
-              affinitygroupids:
+              affinityGroupIDs:
                 description: Optional affinitygroupids for deployVirtualMachine
                 items:
                   type: string
                 type: array
-              cloudstackaffinityref:
+              cloudstackAffinityRef:
                 description: Mutually exclusive parameter with AffinityGroupIDs. Is
                   a reference to a CloudStack affinity group CRD.
                 properties:

config/crd/bases/infrastructure.cluster.x-k8s.io_cloudstackmachinetemplates.yaml

@@ -251,12 +251,12 @@ spec:
                           Defaults to `no`. Can be `pro` or `anti`. Will create an
                           affinity group per machine set.
                         type: string
-                      affinitygroupids:
+                      affinityGroupIDs:
                         description: Optional affinitygroupids for deployVirtualMachine
                         items:
                           type: string
                         type: array
-                      cloudstackaffinityref:
+                      cloudstackAffinityRef:
                         description: Mutually exclusive parameter with AffinityGroupIDs.
                           Is a reference to a CloudStack affinity group CRD.
                         properties:

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()
 }

controllers/cloudstackaffinitygroup_controller_test.go

@@ -34,8 +34,7 @@ var _ = Describe("CloudStackAffinityGroupReconciler", func() {
 
 	It("Should patch back the affinity group as ready after calling GetOrCreateAffinityGroup.", func() {
 		// Modify failure domain name the same way the cluster controller would.
-		dummies.CSFailureDomain1.Name = dummies.CSFailureDomain1.Name + "-" + dummies.CSCluster.Name
-		dummies.CSAffinityGroup.Spec.FailureDomainName = dummies.CSFailureDomain1.Name
+		dummies.CSAffinityGroup.Spec.FailureDomainName = dummies.CSFailureDomain1.Spec.Name
 
 		Ω(k8sClient.Create(ctx, dummies.CSFailureDomain1))
 		Ω(k8sClient.Create(ctx, dummies.CSAffinityGroup)).Should(Succeed())

controllers/cloudstackcluster_controller.go

@@ -105,20 +105,19 @@ 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 {
+	for _, requiredFdSpec := range r.ReconciliationSubject.Spec.FailureDomains {
 		found := false
 		for _, fd := range r.FailureDomains.Items {
-			requiredFDName := csCtrlrUtils.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
@@ -128,8 +127,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 = csCtrlrUtils.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
 }