Add ownerReference resilience test

Signed-off-by: killianmuldoon <kmuldoon@vmware.com>

controllers/vspherecluster_reconciler.go

@@ -284,38 +284,46 @@ func (r clusterReconciler) reconcileNormal(ctx *context.ClusterContext) (reconci
 
 func (r clusterReconciler) reconcileIdentitySecret(ctx *context.ClusterContext) error {
 	vsphereCluster := ctx.VSphereCluster
-	if identity.IsSecretIdentity(vsphereCluster) {
-		secret := &corev1.Secret{}
-		secretKey := client.ObjectKey{
-			Namespace: vsphereCluster.Namespace,
-			Name:      vsphereCluster.Spec.IdentityRef.Name,
-		}
-		err := ctx.Client.Get(ctx, secretKey, secret)
-		if err != nil {
-			return err
-		}
+	if !identity.IsSecretIdentity(vsphereCluster) {
+		return nil
+	}
+	secret := &corev1.Secret{}
+	secretKey := client.ObjectKey{
+		Namespace: vsphereCluster.Namespace,
+		Name:      vsphereCluster.Spec.IdentityRef.Name,
+	}
+	err := ctx.Client.Get(ctx, secretKey, secret)
+	if err != nil {
+		return err
+	}
 
-		// check if cluster is already an owner
-		if !clusterutilv1.IsOwnedByObject(secret, vsphereCluster) {
-			ownerReferences := secret.GetOwnerReferences()
-			if identity.IsOwnedByIdentityOrCluster(ownerReferences) {
-				return fmt.Errorf("another cluster has set the OwnerRef for secret: %s/%s", secret.Namespace, secret.Name)
-			}
-			ownerReferences = append(ownerReferences, metav1.OwnerReference{
-				APIVersion: infrav1.GroupVersion.String(),
-				Kind:       vsphereCluster.Kind,
-				Name:       vsphereCluster.Name,
-				UID:        vsphereCluster.UID,
-			})
-			secret.SetOwnerReferences(ownerReferences)
-		}
-		if !ctrlutil.ContainsFinalizer(secret, infrav1.SecretIdentitySetFinalizer) {
-			ctrlutil.AddFinalizer(secret, infrav1.SecretIdentitySetFinalizer)
-		}
-		err = r.Client.Update(ctx, secret)
-		if err != nil {
-			return err
-		}
+	// If a different VSphereCluster is an owner return an error.
+	if !clusterutilv1.IsOwnedByObject(secret, vsphereCluster) && identity.IsOwnedByIdentityOrCluster(secret.GetOwnerReferences()) {
+		return fmt.Errorf("another cluster has set the OwnerRef for secret: %s/%s", secret.Namespace, secret.Name)
+	}
+
+	helper, err := patch.NewHelper(secret, ctx.Client)
+	if err != nil {
+		return err
+	}
+
+	// Ensure the VSphereCluster is an owner and that the APIVersion is up to date.
+	secret.SetOwnerReferences(clusterutilv1.EnsureOwnerRef(secret.GetOwnerReferences(),
+		metav1.OwnerReference{
+			APIVersion: infrav1.GroupVersion.String(),
+			Kind:       vsphereCluster.Kind,
+			Name:       vsphereCluster.Name,
+			UID:        vsphereCluster.UID,
+		},
+	))
+
+	// Ensure the finalizer is added.
+	if !ctrlutil.ContainsFinalizer(secret, infrav1.SecretIdentitySetFinalizer) {
+		ctrlutil.AddFinalizer(secret, infrav1.SecretIdentitySetFinalizer)
+	}
+	err = helper.Patch(ctx, secret)
+	if err != nil {
+		return err
 	}
 
 	return nil

pkg/identity/identity.go

@@ -122,11 +122,11 @@ func validateInputs(c client.Client, cluster *infrav1.VSphereCluster) error {
 	return nil
 }
 
+// IsSecretIdentity returns true if the VsphereCluster identity is a Secret.
 func IsSecretIdentity(cluster *infrav1.VSphereCluster) bool {
 	if cluster == nil || cluster.Spec.IdentityRef == nil {
 		return false
 	}
-
 	return cluster.Spec.IdentityRef.Kind == infrav1.SecretKind
 }
 

test/e2e/README.md

@@ -14,36 +14,36 @@ In order to run the e2e tests the following requirements must be met:
 * The testing must occur on a host that can access the VMs deployed to vSphere via the network
 * Ginkgo ([download](https://onsi.github.io/ginkgo/#getting-ginkgo))
 * Docker ([download](https://www.docker.com/get-started))
-* Kind v0.7.0+ ([download](https://kind.sigs.k8s.io))
+* Kind v0.20.0+ ([download](https://kind.sigs.k8s.io))
 
 ### Environment variables
 
 The first step to running the e2e tests is setting up the required environment variables:
 
-| Environment variable          | Description                                                                         | Example                                                                          |
-| ----------------------------- | ----------------------------------------------------------------------------------- | -------------------------------------------------------------------------------- |
-| `VSPHERE_SERVER`              | The IP address or FQDN of a vCenter 6.7u3 server                                    | `my.vcenter.com`                                                                 |
-| `VSPHERE_USERNAME`            | The username used to access the vSphere server                                      | `my-username`                                                                    |
-| `VSPHERE_PASSWORD`            | The password used to access the vSphere server                                      | `my-password`                                                                    |
-| `VSPHERE_DATACENTER`          | The unique name or inventory path of the datacenter in which VMs will be created    | `my-datacenter` or `/my-datacenter`                                              |
-| `VSPHERE_FOLDER`              | The unique name or inventory path of the folder in which VMs will be created        | `my-folder` or `/my-datacenter/vm/my-folder`                                     |
-| `VSPHERE_RESOURCE_POOL`       | The unique name or inventory path of the resource pool in which VMs will be created | `my-resource-pool` or `/my-datacenter/host/Cluster-1/Resources/my-resource-pool` |
-| `VSPHERE_DATASTORE`           | The unique name or inventory path of the datastore in which VMs will be created     | `my-datastore` or `/my-datacenter/datstore/my-datastore`                         |
-| `VSPHERE_NETWORK`             | The unique name or inventory path of the network to which VMs will be connected     | `my-network` or `/my-datacenter/network/my-network`                              |
-| `VSPHERE_SSH_PRIVATE_KEY`     | The file path of the private key used to ssh into the CAPV VMs                      | `/home/foo/bar-ssh.key`                                                          |
-| `VSPHERE_SSH_AUTHORIZED_KEY`  | The public key that is added to the CAPV VMs                                        | `ssh-rsa ABCDEF...XYZ=`                                                          |
-| `VSPHERE_TLS_THUMBPRINT`      | The TLS thumbprint of the vSphere server's certificate which should be trusted      | `2A:3F:BC:CA:C0:96:35:D4:B7:A2:AA:3C:C1:33:D9:D7:BE:EC:31:55`                    |
-| `CONTROL_PLANE_ENDPOINT_IP`   | The IP that kube-vip should use as a control plane endpoint                         | `10.10.123.100`                                                                  |
-| `VSPHERE_STORAGE_POLICY`      | The name of an existing vSphere storage policy to be assigned to created VMs        | `my-test-sp`                                                                     |
+| Environment variable         | Description                                                                         | Example                                                                          |
+|------------------------------|-------------------------------------------------------------------------------------|----------------------------------------------------------------------------------|
+| `VSPHERE_SERVER`             | The IP address or FQDN of a vCenter 6.7u3 server                                    | `my.vcenter.com`                                                                 |
+| `VSPHERE_USERNAME`           | The username used to access the vSphere server                                      | `my-username`                                                                    |
+| `VSPHERE_PASSWORD`           | The password used to access the vSphere server                                      | `my-password`                                                                    |
+| `VSPHERE_DATACENTER`         | The unique name or inventory path of the datacenter in which VMs will be created    | `my-datacenter` or `/my-datacenter`                                              |
+| `VSPHERE_FOLDER`             | The unique name or inventory path of the folder in which VMs will be created        | `my-folder` or `/my-datacenter/vm/my-folder`                                     |
+| `VSPHERE_RESOURCE_POOL`      | The unique name or inventory path of the resource pool in which VMs will be created | `my-resource-pool` or `/my-datacenter/host/Cluster-1/Resources/my-resource-pool` |
+| `VSPHERE_DATASTORE`          | The unique name or inventory path of the datastore in which VMs will be created     | `my-datastore` or `/my-datacenter/datstore/my-datastore`                         |
+| `VSPHERE_NETWORK`            | The unique name or inventory path of the network to which VMs will be connected     | `my-network` or `/my-datacenter/network/my-network`                              |
+| `VSPHERE_SSH_PRIVATE_KEY`    | The file path of the private key used to ssh into the CAPV VMs                      | `/home/foo/bar-ssh.key`                                                          |
+| `VSPHERE_SSH_AUTHORIZED_KEY` | The public key that is added to the CAPV VMs                                        | `ssh-rsa ABCDEF...XYZ=`                                                          |
+| `VSPHERE_TLS_THUMBPRINT`     | The TLS thumbprint of the vSphere server's certificate which should be trusted      | `2A:3F:BC:CA:C0:96:35:D4:B7:A2:AA:3C:C1:33:D9:D7:BE:EC:31:55`                    |
+| `CONTROL_PLANE_ENDPOINT_IP`  | The IP that kube-vip should use as a control plane endpoint                         | `10.10.123.100`                                                                  |
+| `VSPHERE_STORAGE_POLICY`     | The name of an existing vSphere storage policy to be assigned to created VMs        | `my-test-sp`                                                                     |
 
 ### Flags
 
 | Flag                    | Description                                                                                              | Default Value |
-|-------------------------|----------------------------------------------------------------------------------------------------------|-----------|
-| `SKIP_RESOURCE_CLEANUP` | This flags skips cleanup of the resources created during the tests as well as the kind/bootstrap cluster | `false`   |
-| `USE_EXISTING_CLUSTER`  | This flag enables the usage of an existing K8S cluster as the management cluster to run tests against.   | `false`   |
-| `GINKGO_TEST_TIMEOUT`   | This sets the timeout for the E2E test suite.                                                            | `2h`      |
-| `GINKGO_FOCUS`          | This populates the `-focus` flag of the `ginkgo` run command.                                            | `""`      |
+|-------------------------|----------------------------------------------------------------------------------------------------------|---------------|
+| `SKIP_RESOURCE_CLEANUP` | This flags skips cleanup of the resources created during the tests as well as the kind/bootstrap cluster | `false`       |
+| `USE_EXISTING_CLUSTER`  | This flag enables the usage of an existing K8S cluster as the management cluster to run tests against.   | `false`       |
+| `GINKGO_TEST_TIMEOUT`   | This sets the timeout for the E2E test suite.                                                            | `2h`          |
+| `GINKGO_FOCUS`          | This populates the `-focus` flag of the `ginkgo` run command.                                            | `""`          |
 
 ### Running the e2e tests
 

test/e2e/capv_quick_start_test.go

@@ -18,7 +18,11 @@ package e2e
 
 import (
 	. "github.com/onsi/ginkgo/v2"
+	"k8s.io/utils/pointer"
 	capi_e2e "sigs.k8s.io/cluster-api/test/e2e"
+	"sigs.k8s.io/cluster-api/test/framework"
+
+	"sigs.k8s.io/cluster-api-provider-vsphere/test/helpers"
 )
 
 var _ = Describe("Cluster Creation using Cluster API quick-start test [PR-Blocking]", func() {
@@ -29,6 +33,60 @@ var _ = Describe("Cluster Creation using Cluster API quick-start test [PR-Blocki
 			BootstrapClusterProxy: bootstrapClusterProxy,
 			ArtifactFolder:        artifactFolder,
 			SkipCleanup:           skipCleanup,
+			PostMachinesProvisioned: func(proxy framework.ClusterProxy, namespace, clusterName string) {
+				// This check ensures that owner references are resilient - i.e. correctly re-reconciled - when removed.
+				framework.ValidateOwnerReferencesResilience(ctx, proxy, namespace, clusterName,
+					framework.CoreOwnerReferenceAssertion,
+					framework.KubeadmBootstrapOwnerReferenceAssertions,
+					framework.KubeadmControlPlaneOwnerReferenceAssertions,
+					helpers.VSphereKubernetesReferenceAssertions,
+					helpers.VSphereExpOwnerReferenceAssertions,
+					helpers.VSphereReferenceAssertions,
+				)
+
+				// This check ensures that owner references are always updated to the most recent apiVersion.
+				helpers.ValidateOwnerReferencesOnUpdate(ctx, proxy, namespace, clusterName,
+					framework.CoreOwnerReferenceAssertion,
+					framework.KubeadmBootstrapOwnerReferenceAssertions,
+					framework.KubeadmControlPlaneOwnerReferenceAssertions,
+					helpers.VSphereKubernetesReferenceAssertions,
+					helpers.VSphereExpOwnerReferenceAssertions,
+					helpers.VSphereReferenceAssertions,
+				)
+			},
+		}
+	})
+})
+
+var _ = Describe("ClusterClass Creation using Cluster API quick-start test [PR-Blocking] [ClusterClass]", func() {
+	capi_e2e.QuickStartSpec(ctx, func() capi_e2e.QuickStartSpecInput {
+		return capi_e2e.QuickStartSpecInput{
+			E2EConfig:             e2eConfig,
+			ClusterctlConfigPath:  clusterctlConfigPath,
+			BootstrapClusterProxy: bootstrapClusterProxy,
+			ArtifactFolder:        artifactFolder,
+			SkipCleanup:           skipCleanup,
+			Flavor:                pointer.String("topology"),
+			PostMachinesProvisioned: func(proxy framework.ClusterProxy, namespace, clusterName string) {
+				// This check ensures that owner references are resilient - i.e. correctly re-reconciled - when removed.
+				framework.ValidateOwnerReferencesResilience(ctx, proxy, namespace, clusterName,
+					framework.CoreOwnerReferenceAssertion,
+					framework.KubeadmBootstrapOwnerReferenceAssertions,
+					framework.KubeadmControlPlaneOwnerReferenceAssertions,
+					helpers.VSphereKubernetesReferenceAssertions,
+					helpers.VSphereExpOwnerReferenceAssertions,
+					helpers.VSphereReferenceAssertions,
+				)
+				// This check ensures that owner references are always updated to the most recent apiVersion.
+				helpers.ValidateOwnerReferencesOnUpdate(ctx, proxy, namespace, clusterName,
+					framework.CoreOwnerReferenceAssertion,
+					framework.KubeadmBootstrapOwnerReferenceAssertions,
+					framework.KubeadmControlPlaneOwnerReferenceAssertions,
+					helpers.VSphereKubernetesReferenceAssertions,
+					helpers.VSphereExpOwnerReferenceAssertions,
+					helpers.VSphereReferenceAssertions,
+				)
+			},
 		}
 	})
 })

test/e2e/govmomi_test.go

@@ -55,6 +55,7 @@ func initVSphereSession() {
 	By("parsing vSphere server URL")
 	serverURL, err := soap.ParseURL(vsphereServer)
 	Expect(err).ShouldNot(HaveOccurred())
+	Expect(serverURL).ToNot(BeNil())
 
 	var vimClient *vim25.Client