Add ownerReference resilience test

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

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

go.mod

@@ -2,7 +2,11 @@ module sigs.k8s.io/cluster-api-provider-vsphere
 
 go 1.20
 
-replace sigs.k8s.io/cluster-api => sigs.k8s.io/cluster-api v1.5.0
+replace (
+	sigs.k8s.io/cluster-api => sigs.k8s.io/cluster-api v1.5.1-0.20230825144253-1c45db7b5e28
+	sigs.k8s.io/cluster-api/test => sigs.k8s.io/cluster-api/test v1.5.1-0.20230825144253-1c45db7b5e28
+
+)
 
 require (
 	github.com/blang/semver/v4 v4.0.0
@@ -34,7 +38,7 @@ require (
 	k8s.io/component-base v0.27.4
 	k8s.io/klog/v2 v2.90.1
 	k8s.io/utils v0.0.0-20230209194617-a36077c30491
-	sigs.k8s.io/cluster-api v0.0.0-00010101000000-000000000000
+	sigs.k8s.io/cluster-api v1.5.1-0.20230825144253-1c45db7b5e28
 	sigs.k8s.io/cluster-api/test v1.5.0
 	sigs.k8s.io/controller-runtime v0.15.1
 	sigs.k8s.io/kind v0.20.0
@@ -77,7 +81,7 @@ require (
 	github.com/blang/semver v3.5.1+incompatible
 	github.com/cespare/xxhash/v2 v2.2.0 // indirect
 	github.com/coredns/caddy v1.1.1 // indirect
-	github.com/coredns/corefile-migration v1.0.20 // indirect
+	github.com/coredns/corefile-migration v1.0.21 // indirect
 	github.com/davecgh/go-spew v1.1.1 // indirect
 	github.com/docker/distribution v2.8.2+incompatible // indirect
 	github.com/docker/docker v24.0.5+incompatible // indirect

go.sum

@@ -122,8 +122,8 @@ github.com/cockroachdb/datadriven v0.0.0-20190809214429-80d97fb3cbaa/go.mod h1:z
 github.com/coredns/caddy v1.1.0/go.mod h1:A6ntJQlAWuQfFlsd9hvigKbo2WS0VUs2l1e2F+BawD4=
 github.com/coredns/caddy v1.1.1 h1:2eYKZT7i6yxIfGP3qLJoJ7HAsDJqYB+X68g4NYjSrE0=
 github.com/coredns/caddy v1.1.1/go.mod h1:A6ntJQlAWuQfFlsd9hvigKbo2WS0VUs2l1e2F+BawD4=
-github.com/coredns/corefile-migration v1.0.20 h1:MdOkT6F3ehju/n9tgxlGct8XAajOX2vN+wG7To4BWSI=
-github.com/coredns/corefile-migration v1.0.20/go.mod h1:XnhgULOEouimnzgn0t4WPuFDN2/PJQcTxdWKC5eXNGE=
+github.com/coredns/corefile-migration v1.0.21 h1:W/DCETrHDiFo0Wj03EyMkaQ9fwsmSgqTCQDHpceaSsE=
+github.com/coredns/corefile-migration v1.0.21/go.mod h1:XnhgULOEouimnzgn0t4WPuFDN2/PJQcTxdWKC5eXNGE=
 github.com/coreos/bbolt v1.3.2/go.mod h1:iRUV2dpdMOn7Bo10OQBFzIJO9kkE559Wcmn+qkEiiKk=
 github.com/coreos/etcd v3.3.13+incompatible/go.mod h1:uF7uidLiAD3TWHmW31ZFd/JWoc32PjwdhPthX9715RE=
 github.com/coreos/go-oidc v2.1.0+incompatible/go.mod h1:CgnwVTmzoESiwO9qyAFEMiHoZ1nMCKZlZ9V6mm3/LKc=
@@ -1162,10 +1162,10 @@ rsc.io/binaryregexp v0.2.0/go.mod h1:qTv7/COck+e2FymRvadv62gMdZztPaShugOCi3I+8D8
 rsc.io/quote/v3 v3.1.0/go.mod h1:yEA65RcK8LyAZtP9Kv3t0HmxON59tX3rD+tICJqUlj0=
 rsc.io/sampler v1.3.0/go.mod h1:T1hPZKmBbMNahiBKFy5HrXp6adAjACjK9JXDnKaTXpA=
 sigs.k8s.io/apiserver-network-proxy/konnectivity-client v0.0.15/go.mod h1:LEScyzhFmoF5pso/YSeBstl57mOzx9xlU9n85RGrDQg=
-sigs.k8s.io/cluster-api v1.5.0 h1:pwXvzScbAwnrB7EWHTApzW+VQfrj2OSrWAQDC9+bcbU=
-sigs.k8s.io/cluster-api v1.5.0/go.mod h1:ZSEP01t8oT6104gB4ljsOwwp5uJcI8SWy8IFp2HUvrc=
-sigs.k8s.io/cluster-api/test v1.5.0 h1:ePfrh7S+eaVWy+D0ca4AjzhBqrhXxRE9NTr5te4hCa0=
-sigs.k8s.io/cluster-api/test v1.5.0/go.mod h1:Ii5Mh9oVq7QJEtiUkGg9mM2qojjWxvsqmL8TMlwZViM=
+sigs.k8s.io/cluster-api v1.5.1-0.20230825144253-1c45db7b5e28 h1:1OBQLku6/vUAWvgobfi2yFKNVOo764SIgyLE4Fn+/XM=
+sigs.k8s.io/cluster-api v1.5.1-0.20230825144253-1c45db7b5e28/go.mod h1:EGJUNpFWi7dF426tO8MG/jE+w7T0UO5KyMnOwQ5riUY=
+sigs.k8s.io/cluster-api/test v1.5.1-0.20230825144253-1c45db7b5e28 h1:7k/q+uMC6mcHHKCesNUagCdI/H4JWp7xtmSdfnOt7Xw=
+sigs.k8s.io/cluster-api/test v1.5.1-0.20230825144253-1c45db7b5e28/go.mod h1:mFlsY1y0lApBgQyXbmVprdzCK+9MQNp1C38K+aZdn5A=
 sigs.k8s.io/controller-runtime v0.9.0/go.mod h1:TgkfvrhhEw3PlI0BRL/5xM+89y3/yc0ZDfdbTl84si8=
 sigs.k8s.io/controller-runtime v0.15.1 h1:9UvgKD4ZJGcj24vefUFgZFP3xej/3igL9BsOUTb/+4c=
 sigs.k8s.io/controller-runtime v0.15.1/go.mod h1:7ngYvp1MLT+9GeZ+6lH3LOlcHkp/+tzA/fmHa4iq9kk=

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,9 @@ 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"
 )
 
 var _ = Describe("Cluster Creation using Cluster API quick-start test [PR-Blocking]", func() {
@@ -29,6 +31,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,
+					framework.ExpOwnerReferenceAssertions,
+					VSphereKubernetesReferenceAssertions,
+					VSphereReferenceAssertions,
+				)
+
+				// This check ensures that owner references are always updated to the most recent apiVersion.
+				framework.ValidateOwnerReferencesOnUpdate(ctx, proxy, namespace, clusterName,
+					framework.CoreOwnerReferenceAssertion,
+					framework.KubeadmBootstrapOwnerReferenceAssertions,
+					framework.KubeadmControlPlaneOwnerReferenceAssertions,
+					framework.ExpOwnerReferenceAssertions,
+					VSphereKubernetesReferenceAssertions,
+					VSphereReferenceAssertions,
+				)
+			},
+		}
+	})
+})
+
+var _ = Describe("ClusterClass Creation using Cluster API quick-starto 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,
+					framework.ExpOwnerReferenceAssertions,
+					VSphereKubernetesReferenceAssertions,
+					VSphereReferenceAssertions,
+				)
+				// This check ensures that owner references are always updated to the most recent apiVersion.
+				framework.ValidateOwnerReferencesOnUpdate(ctx, proxy, namespace, clusterName,
+					framework.CoreOwnerReferenceAssertion,
+					framework.KubeadmBootstrapOwnerReferenceAssertions,
+					framework.KubeadmControlPlaneOwnerReferenceAssertions,
+					framework.ExpOwnerReferenceAssertions,
+					VSphereKubernetesReferenceAssertions,
+					VSphereReferenceAssertions,
+				)
+			},
 		}
 	})
 })