Add ownerRef check for vSphereClusterIdentity

Signed-off-by: killianmuldoon <[email protected]>

Makefile

@@ -315,6 +315,7 @@ generate-e2e-templates-main: $(KUSTOMIZE) ## Generate test templates for the mai
 	"$(KUSTOMIZE)" --load-restrictor LoadRestrictionsNone build $(E2E_TEMPLATE_DIR)/main/pci > $(E2E_TEMPLATE_DIR)/main/cluster-template-pci.yaml
 	# for DHCP overrides
 	"$(KUSTOMIZE)" --load-restrictor LoadRestrictionsNone build $(E2E_TEMPLATE_DIR)/main/dhcp-overrides > $(E2E_TEMPLATE_DIR)/main/cluster-template-dhcp-overrides.yaml
+	"$(KUSTOMIZE)" --load-restrictor LoadRestrictionsNone build $(E2E_TEMPLATE_DIR)/main/ownerreferences > $(E2E_TEMPLATE_DIR)/main/cluster-template-ownerreferences.yaml
 
 
 ## --------------------------------------

controllers/vsphereclusteridentity_controller.go

@@ -136,30 +136,30 @@ func (r clusterIdentityReconciler) Reconcile(ctx context.Context, req reconcile.
 		return reconcile.Result{}, errors.Errorf("secret: %s not found in namespace: %s", secretKey.Name, secretKey.Namespace)
 	}
 
-	if !clusterutilv1.IsOwnedByObject(secret, identity) {
-		ownerReferences := secret.GetOwnerReferences()
-		if pkgidentity.IsOwnedByIdentityOrCluster(ownerReferences) {
-			conditions.MarkFalse(identity, infrav1.CredentialsAvailableCondidtion, infrav1.SecretAlreadyInUseReason, clusterv1.ConditionSeverityError, "secret being used by another Cluster/VSphereIdentity")
-			identity.Status.Ready = false
-			return reconcile.Result{}, errors.New("secret being used by another Cluster/VSphereIdentity")
-		}
-
-		ownerReferences = append(ownerReferences, metav1.OwnerReference{
-			APIVersion: infrav1.GroupVersion.String(),
-			Kind:       identity.Kind,
-			Name:       identity.Name,
-			UID:        identity.UID,
-		})
-		secret.SetOwnerReferences(ownerReferences)
+	// If this secret is owned by a different VSphereClusterIdentity or a VSphereCluster, mark the identity as not ready and return an error.
+	if !clusterutilv1.IsOwnedByObject(secret, identity) && pkgidentity.IsOwnedByIdentityOrCluster(secret.GetOwnerReferences()) {
+		conditions.MarkFalse(identity, infrav1.CredentialsAvailableCondidtion, infrav1.SecretAlreadyInUseReason, clusterv1.ConditionSeverityError, "secret being used by another Cluster/VSphereIdentity")
+		identity.Status.Ready = false
+		return reconcile.Result{}, errors.New("secret being used by another Cluster/VSphereIdentity")
+	}
 
-		if !ctrlutil.ContainsFinalizer(secret, infrav1.SecretIdentitySetFinalizer) {
-			ctrlutil.AddFinalizer(secret, infrav1.SecretIdentitySetFinalizer)
-		}
-		err = r.Client.Update(ctx, secret)
-		if err != nil {
-			conditions.MarkFalse(identity, infrav1.CredentialsAvailableCondidtion, infrav1.SecretOwnerReferenceFailedReason, clusterv1.ConditionSeverityWarning, err.Error())
-			return reconcile.Result{}, err
-		}
+	// Ensure the VSphereClusterIdentity is set as the owner of the secret, and that the reference has an up to date APIVersion.
+	secret.SetOwnerReferences(
+		clusterutilv1.EnsureOwnerRef(secret.GetOwnerReferences(),
+			metav1.OwnerReference{
+				APIVersion: infrav1.GroupVersion.String(),
+				Kind:       identity.Kind,
+				Name:       identity.Name,
+				UID:        identity.UID,
+			}))
+
+	if !ctrlutil.ContainsFinalizer(secret, infrav1.SecretIdentitySetFinalizer) {
+		ctrlutil.AddFinalizer(secret, infrav1.SecretIdentitySetFinalizer)
+	}
+	err = r.Client.Update(ctx, secret)
+	if err != nil {
+		conditions.MarkFalse(identity, infrav1.CredentialsAvailableCondidtion, infrav1.SecretOwnerReferenceFailedReason, clusterv1.ConditionSeverityWarning, err.Error())
+		return reconcile.Result{}, err
 	}
 
 	conditions.MarkTrue(identity, infrav1.CredentialsAvailableCondidtion)

controllers/vspheredeploymentzone_controller.go

@@ -182,26 +182,8 @@ func (r vsphereDeploymentZoneReconciler) reconcileNormal(deploymentZoneCtx *capv
 		deploymentZoneCtx.VSphereDeploymentZone.Status.Ready = pointer.Bool(false)
 		return errors.Wrapf(err, "failed to reconcile failure domain")
 	}
-	conditions.MarkTrue(deploymentZoneCtx.VSphereDeploymentZone, infrav1.VSphereFailureDomainValidatedCondition)
-
-	// Ensure the VSphereDeploymentZone is marked as an owner of the VSphereFailureDomain.
-	if !clusterutilv1.HasOwnerRef(deploymentZoneCtx.VSphereFailureDomain.GetOwnerReferences(), metav1.OwnerReference{
-		APIVersion: infrav1.GroupVersion.String(),
-		Kind:       "VSphereDeploymentZone",
-		Name:       deploymentZoneCtx.VSphereDeploymentZone.Name,
-	}) {
-		if err := updateOwnerReferences(deploymentZoneCtx, deploymentZoneCtx.VSphereFailureDomain, r.Client, func() []metav1.OwnerReference {
-			return append(deploymentZoneCtx.VSphereFailureDomain.OwnerReferences, metav1.OwnerReference{
-				APIVersion: infrav1.GroupVersion.String(),
-				Kind:       deploymentZoneCtx.VSphereDeploymentZone.Kind,
-				Name:       deploymentZoneCtx.VSphereDeploymentZone.Name,
-				UID:        deploymentZoneCtx.VSphereDeploymentZone.UID,
-			})
-		}); err != nil {
-			return err
-		}
-	}
 
+	// Mark the deployment zone as ready.
 	deploymentZoneCtx.VSphereDeploymentZone.Status.Ready = pointer.Bool(true)
 	return nil
 }

controllers/vspheredeploymentzone_controller_domain.go

@@ -18,8 +18,10 @@ package controllers
 
 import (
 	"github.com/pkg/errors"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	kerrors "k8s.io/apimachinery/pkg/util/errors"
 	clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
+	clusterutilv1 "sigs.k8s.io/cluster-api/util"
 	"sigs.k8s.io/cluster-api/util/conditions"
 	ctrl "sigs.k8s.io/controller-runtime"
 
@@ -58,6 +60,24 @@ func (r vsphereDeploymentZoneReconciler) reconcileFailureDomain(deploymentZoneCt
 		logger.Error(err, "topology is not configured correctly")
 		return errors.Wrap(err, "topology is not configured correctly")
 	}
+
+	// Ensure the VSphereDeploymentZone is marked as an owner of the VSphereFailureDomain.
+	if err := updateOwnerReferences(deploymentZoneCtx, deploymentZoneCtx.VSphereFailureDomain, r.Client,
+		func() []metav1.OwnerReference {
+			return clusterutilv1.EnsureOwnerRef(
+				deploymentZoneCtx.VSphereFailureDomain.OwnerReferences,
+				metav1.OwnerReference{
+					APIVersion: infrav1.GroupVersion.String(),
+					Kind:       deploymentZoneCtx.VSphereDeploymentZone.Kind,
+					Name:       deploymentZoneCtx.VSphereDeploymentZone.Name,
+					UID:        deploymentZoneCtx.VSphereDeploymentZone.UID,
+				})
+		}); err != nil {
+		return err
+	}
+
+	// Mark the VSphereDeploymentZone as having a valid VSphereFailureDomain.
+	conditions.MarkTrue(deploymentZoneCtx.VSphereDeploymentZone, infrav1.VSphereFailureDomainValidatedCondition)
 	return nil
 }
 

controllers/vspheremachine_controller.go

@@ -263,7 +263,7 @@ func (r *machineReconciler) Reconcile(_ context.Context, req ctrl.Request) (_ ct
 }
 
 func (r *machineReconciler) reconcileDelete(machineCtx capvcontext.MachineContext) (reconcile.Result, error) {
-	machineCtx.GetLogger().Info("Handling deleted SphereMachine")
+	machineCtx.GetLogger().Info("Handling deleted VSphereMachine")
 	conditions.MarkFalse(machineCtx.GetVSphereMachine(), infrav1.VMProvisionedCondition, clusterv1.DeletingReason, clusterv1.ConditionSeverityInfo, "")
 
 	if err := r.VMService.ReconcileDelete(machineCtx); err != nil {

test/e2e/config/vsphere-ci.yaml

@@ -92,6 +92,7 @@ providers:
           - sourcePath: "../../../test/e2e/data/infrastructure-vsphere/main/cluster-template-storage-policy.yaml"
           - sourcePath: "../../../test/e2e/data/infrastructure-vsphere/main/cluster-template-topology.yaml"
           - sourcePath: "../../../test/e2e/data/infrastructure-vsphere/main/cluster-template-dhcp-overrides.yaml"
+          - sourcePath: "../../../test/e2e/data/infrastructure-vsphere/main/cluster-template-ownerreferences.yaml"
           - sourcePath: "../../../test/e2e/data/infrastructure-vsphere/main/clusterclass-quick-start.yaml"
           - sourcePath: "../../../test/e2e/data/infrastructure-vsphere/main/cluster-template-ignition.yaml"
           - sourcePath: "../data/shared/main/v1beta1_provider/metadata.yaml"
@@ -106,6 +107,7 @@ variables:
   WORKER_MACHINE_COUNT: 1
   IP_FAMILY: "IPv4"
   CLUSTER_CLASS_NAME: "quick-start"
+  VSPHERE_COMPUTE_CLUSTER: "cluster0"
   VSPHERE_DATACENTER:  "SDDC-Datacenter"
   VSPHERE_FOLDER: "clusterapi"
   VSPHERE_RESOURCE_POOL: "clusterapi"

test/e2e/config/vsphere-dev.yaml

@@ -97,6 +97,7 @@ providers:
           - sourcePath: "../../../test/e2e/data/infrastructure-vsphere/main/cluster-template-dhcp-overrides.yaml"
           - sourcePath: "../../../test/e2e/data/infrastructure-vsphere/main/clusterclass-quick-start.yaml"
           - sourcePath: "../../../test/e2e/data/infrastructure-vsphere/main/cluster-template-ignition.yaml"
+          - sourcePath: "../../../test/e2e/data/infrastructure-vsphere/main/cluster-template-ownerreferences.yaml"
           - sourcePath: "../data/shared/main/v1beta1_provider/metadata.yaml"
 
 variables:
@@ -113,6 +114,7 @@ variables:
   VSPHERE_SERVER: "vcenter.vmware.com"
   VSPHERE_TLS_THUMBPRINT: "AA:BB:CC:DD:11:22:33:44:EE:FF"
   VSPHERE_DATACENTER: "SDDC-Datacenter"
+  VSPHERE_COMPUTE_CLUSTER: "cluster0"
   VSPHERE_FOLDER: "FolderName"
   VSPHERE_RESOURCE_POOL: "ResourcePool"
   VSPHERE_DATASTORE: "WorkloadDatastore"

test/e2e/data/infrastructure-vsphere/main/ownerreferences/cluster-identity.yaml

@@ -0,0 +1,11 @@
+---
+apiVersion: infrastructure.cluster.x-k8s.io/v1beta1
+kind: VSphereClusterIdentity
+metadata:
+  name: ownerreferences
+spec:
+  secretName: ownerreferences
+  allowedNamespaces:
+    selector:
+      matchLabels:
+        kubernetes.io/metadata.name: '${NAMESPACE}'

test/e2e/data/infrastructure-vsphere/main/ownerreferences/drop-existing-identity-secret.yaml

@@ -0,0 +1,7 @@
+# This secret is not needed. This cluster uses a ClusterIdentity instead
+$patch: delete
+apiVersion: v1
+kind: Secret
+metadata:
+  name: ${CLUSTER_NAME}
+  namespace: ${NAMESPACE}

test/e2e/data/infrastructure-vsphere/main/ownerreferences/failure-domains.yaml

@@ -0,0 +1,29 @@
+---
+apiVersion: infrastructure.cluster.x-k8s.io/v1beta1
+kind: VSphereFailureDomain
+metadata:
+  name: "ownerreferences"
+spec:
+  region:
+    name: '${VSPHERE_DATACENTER}'
+    type: Datacenter
+    tagCategory: k8s-region
+  zone:
+    name: '${VSPHERE_COMPUTE_CLUSTER}'
+    type: ComputeCluster
+    tagCategory: k8s-zone
+  topology:
+    datacenter: '${VSPHERE_DATACENTER}'
+    # datastore is optional and should\can be set when only one compute cluster is set
+    # or we should use storage policy
+    computeCluster: '${VSPHERE_COMPUTE_CLUSTER}'
+---
+apiVersion: infrastructure.cluster.x-k8s.io/v1beta1
+kind: VSphereDeploymentZone
+metadata:
+  name: "ownerreferences"
+spec:
+  server: '${VSPHERE_SERVER}'
+  failureDomain: "ownerreferences"
+  placementConstraint:
+    resourcePool: '${VSPHERE_RESOURCE_POOL}'

test/e2e/data/infrastructure-vsphere/main/ownerreferences/kustomization.yaml

@@ -0,0 +1,12 @@
+apiVersion: kustomize.config.k8s.io/v1beta1
+kind: Kustomization
+resources:
+  - ../base
+  - cluster-identity.yaml
+  - failure-domains.yaml
+patchesStrategicMerge:
+  - ../commons/cluster-resource-set-label.yaml
+  - ../commons/cluster-network-CIDR.yaml
+  - ../commons/cluster-resource-set-csi-insecure.yaml
+  - vsphereclusteridentity.yaml
+  - drop-existing-identity-secret.yaml

test/e2e/data/infrastructure-vsphere/main/ownerreferences/vsphereclusteridentity.yaml

@@ -0,0 +1,11 @@
+---
+apiVersion: infrastructure.cluster.x-k8s.io/v1beta1
+kind: VSphereCluster
+metadata:
+  name: "${CLUSTER_NAME}"
+  namespace: "${NAMESPACE}"
+spec:
+  identityRef:
+    kind: VSphereClusterIdentity
+    name: ownerreferences
+