Adjust tests to new fakeClient deletionTimestamp handling

Signed-off-by: Stefan Büringer [email protected]

Author: sbueringer

controlplane/kubeadm/internal/controllers/controller_test.go

@@ -637,7 +637,7 @@ func TestKubeadmControlPlaneReconciler_adoption(t *testing.T) {
 	})
 
 	t.Run("Deleted KubeadmControlPlanes don't adopt machines", func(t *testing.T) {
-		// Usually we won't get into the inner reconcile with a deleted control plane, but it's possible when deleting with "oprhanDependents":
+		// Usually we won't get into the inner reconcile with a deleted control plane, but it's possible when deleting with "orphanDependents":
 		// 1. The deletion timestamp is set in the API server, but our cache has not yet updated
 		// 2. The garbage collector removes our ownership reference from a Machine, triggering a re-reconcile (or we get unlucky with the periodic reconciliation)
 		// 3. We get into the inner reconcile function and re-adopt the Machine
@@ -652,6 +652,9 @@ func TestKubeadmControlPlaneReconciler_adoption(t *testing.T) {
 
 		now := metav1.Now()
 		kcp.DeletionTimestamp = &now
+		// We also have to set a finalizer as fake client doesn't accept objects
+		// with a deletionTimestamp without a finalizer.
+		kcp.Finalizers = []string{"block-deletion"}
 
 		fmc := &fakeManagementCluster{
 			Machines: collections.Machines{},

exp/internal/controllers/machinepool_controller_phases_test.go

@@ -460,6 +460,7 @@ func TestReconcileMachinePoolPhases(t *testing.T) {
 
 		// Set Deletion Timestamp.
 		machinepool.SetDeletionTimestamp(&deletionTimestamp)
+		machinepool.Finalizers = []string{expv1.MachinePoolFinalizer}
 
 		r := &MachinePoolReconciler{
 			Client: fake.NewClientBuilder().WithObjects(defaultCluster, defaultKubeconfigSecret, machinepool, bootstrapConfig, infraConfig, builder.TestBootstrapConfigCRD, builder.TestInfrastructureMachineTemplateCRD).Build(),

internal/controllers/cluster/cluster_controller_phases_test.go

@@ -346,6 +346,7 @@ func TestClusterReconciler_reconcilePhase(t *testing.T) {
 				ObjectMeta: metav1.ObjectMeta{
 					Name:              "test-cluster",
 					DeletionTimestamp: &metav1.Time{Time: time.Now().UTC()},
+					Finalizers:        []string{clusterv1.ClusterFinalizer},
 				},
 				Status: clusterv1.ClusterStatus{
 					InfrastructureReady: true,

internal/controllers/machine/machine_controller_test.go

@@ -1501,6 +1501,7 @@ func TestIsDeleteNodeAllowed(t *testing.T) {
 					Name:              "test-cluster",
 					Namespace:         metav1.NamespaceDefault,
 					DeletionTimestamp: &deletionts,
+					Finalizers:        []string{clusterv1.ClusterFinalizer},
 				},
 			},
 			machine:       &clusterv1.Machine{},
@@ -1642,6 +1643,7 @@ func TestIsDeleteNodeAllowed(t *testing.T) {
 	mcpBeingDeleted.SetName("test-cluster-2")
 	mcpBeingDeleted.SetNamespace("test-cluster")
 	mcpBeingDeleted.SetDeletionTimestamp(&metav1.Time{Time: time.Now()})
+	mcpBeingDeleted.SetFinalizers([]string{"block-deletion"})
 
 	empBeingDeleted := &unstructured.Unstructured{
 		Object: map[string]interface{}{
@@ -1655,6 +1657,7 @@ func TestIsDeleteNodeAllowed(t *testing.T) {
 	empBeingDeleted.SetName("test-cluster-3")
 	empBeingDeleted.SetNamespace("test-cluster")
 	empBeingDeleted.SetDeletionTimestamp(&metav1.Time{Time: time.Now()})
+	empBeingDeleted.SetFinalizers([]string{"block-deletion"})
 
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {

internal/controllers/machineset/machineset_controller_test.go

@@ -505,6 +505,7 @@ func TestMachineSetReconcile(t *testing.T) {
 				Name:              "machineset1",
 				Namespace:         metav1.NamespaceDefault,
 				DeletionTimestamp: &dt,
+				Finalizers:        []string{"block-deletion"},
 			},
 			Spec: clusterv1.MachineSetSpec{
 				ClusterName: testClusterName,

internal/controllers/topology/machinedeployment/machinedeployment_controller_test.go

@@ -46,18 +46,14 @@ func TestMachineDeploymentTopologyFinalizer(t *testing.T) {
 	md := mdBuilder.Build()
 	mdWithFinalizer := mdBuilder.Build()
 	mdWithFinalizer.Finalizers = []string{clusterv1.MachineDeploymentTopologyFinalizer}
-	mdWithDeletionTimestamp := mdBuilder.Build()
-	deletionTimestamp := metav1.Now()
-	mdWithDeletionTimestamp.DeletionTimestamp = &deletionTimestamp
-
-	mdWithDeletionTimestampAndFinalizer := mdWithDeletionTimestamp.DeepCopy()
-	mdWithDeletionTimestampAndFinalizer.Finalizers = []string{clusterv1.MachineDeploymentTopologyFinalizer}
 
 	testCases := []struct {
 		name            string
 		md              *clusterv1.MachineDeployment
 		expectFinalizer bool
 	}{
+		// Note: We are not testing the case of a MD with deletionTimestamp and no finalizer.
+		// This case is impossible to reproduce in fake client without deleting the object.
 		{
 			name:            "should add ClusterTopology finalizer to a MachineDeployment with no finalizer",
 			md:              md,
@@ -68,11 +64,6 @@ func TestMachineDeploymentTopologyFinalizer(t *testing.T) {
 			md:              mdWithFinalizer,
 			expectFinalizer: true,
 		},
-		{
-			name:            "should not add ClusterTopology finalizer on MachineDeployment with Deletion Timestamp and no finalizer ",
-			md:              mdWithDeletionTimestamp,
-			expectFinalizer: false,
-		},
 	}
 
 	for _, tc := range testCases {
@@ -117,10 +108,14 @@ func TestMachineDeploymentReconciler_ReconcileDelete(t *testing.T) {
 		Build()
 	mhc := builder.MachineHealthCheck(metav1.NamespaceDefault, "md").Build()
 	md.SetDeletionTimestamp(&deletionTimeStamp)
+	md.SetFinalizers([]string{clusterv1.MachineDeploymentTopologyFinalizer})
 
 	t.Run("Should delete templates of a MachineDeployment", func(t *testing.T) {
 		g := NewWithT(t)
 
+		// Copying the MD so changes made by reconcileDelete do not affect other tests.
+		md := md.DeepCopy()
+
 		fakeClient := fake.NewClientBuilder().
 			WithScheme(fakeScheme).
 			WithObjects(md, mdBT, mdIMT).
@@ -133,10 +128,7 @@ func TestMachineDeploymentReconciler_ReconcileDelete(t *testing.T) {
 		_, err := r.reconcileDelete(ctx, md)
 		g.Expect(err).ToNot(HaveOccurred())
 
-		afterMD := &clusterv1.MachineDeployment{}
-		g.Expect(fakeClient.Get(ctx, client.ObjectKeyFromObject(md), afterMD)).To(Succeed())
-
-		g.Expect(controllerutil.ContainsFinalizer(afterMD, clusterv1.MachineDeploymentTopologyFinalizer)).To(BeFalse())
+		g.Expect(controllerutil.ContainsFinalizer(md, clusterv1.MachineDeploymentTopologyFinalizer)).To(BeFalse())
 		g.Expect(templateExists(fakeClient, mdBT)).To(BeFalse())
 		g.Expect(templateExists(fakeClient, mdIMT)).To(BeFalse())
 	})
@@ -149,6 +141,7 @@ func TestMachineDeploymentReconciler_ReconcileDelete(t *testing.T) {
 			WithInfrastructureTemplate(mdWithoutBootstrapTemplateIMT).
 			Build()
 		mdWithoutBootstrapTemplate.SetDeletionTimestamp(&deletionTimeStamp)
+		mdWithoutBootstrapTemplate.SetFinalizers([]string{clusterv1.MachineDeploymentTopologyFinalizer})
 
 		fakeClient := fake.NewClientBuilder().
 			WithScheme(fakeScheme).
@@ -162,16 +155,16 @@ func TestMachineDeploymentReconciler_ReconcileDelete(t *testing.T) {
 		_, err := r.reconcileDelete(ctx, mdWithoutBootstrapTemplate)
 		g.Expect(err).ToNot(HaveOccurred())
 
-		afterMD := &clusterv1.MachineDeployment{}
-		g.Expect(fakeClient.Get(ctx, client.ObjectKeyFromObject(mdWithoutBootstrapTemplate), afterMD)).To(Succeed())
-
-		g.Expect(controllerutil.ContainsFinalizer(afterMD, clusterv1.MachineDeploymentTopologyFinalizer)).To(BeFalse())
+		g.Expect(controllerutil.ContainsFinalizer(mdWithoutBootstrapTemplate, clusterv1.MachineDeploymentTopologyFinalizer)).To(BeFalse())
 		g.Expect(templateExists(fakeClient, mdWithoutBootstrapTemplateIMT)).To(BeFalse())
 	})
 
 	t.Run("Should not delete templates of a MachineDeployment when they are still in use in a MachineSet", func(t *testing.T) {
 		g := NewWithT(t)
 
+		// Copying the MD so changes made by reconcileDelete do not affect other tests.
+		md := md.DeepCopy()
+
 		ms := builder.MachineSet(md.Namespace, "md").
 			WithBootstrapTemplate(mdBT).
 			WithInfrastructureTemplate(mdIMT).
@@ -192,16 +185,16 @@ func TestMachineDeploymentReconciler_ReconcileDelete(t *testing.T) {
 		_, err := r.reconcileDelete(ctx, md)
 		g.Expect(err).ToNot(HaveOccurred())
 
-		afterMD := &clusterv1.MachineDeployment{}
-		g.Expect(fakeClient.Get(ctx, client.ObjectKeyFromObject(md), afterMD)).To(Succeed())
-
-		g.Expect(controllerutil.ContainsFinalizer(afterMD, clusterv1.MachineDeploymentTopologyFinalizer)).To(BeFalse())
+		g.Expect(controllerutil.ContainsFinalizer(md, clusterv1.MachineDeploymentTopologyFinalizer)).To(BeFalse())
 		g.Expect(templateExists(fakeClient, mdBT)).To(BeTrue())
 		g.Expect(templateExists(fakeClient, mdIMT)).To(BeTrue())
 	})
 	t.Run("Should delete a MachineHealthCheck when its linked MachineDeployment is deleted", func(t *testing.T) {
 		g := NewWithT(t)
 
+		// Copying the MD so changes made by reconcileDelete do not affect other tests.
+		md := md.DeepCopy()
+
 		fakeClient := fake.NewClientBuilder().
 			WithScheme(fakeScheme).
 			WithObjects(md, mhc).
@@ -214,6 +207,8 @@ func TestMachineDeploymentReconciler_ReconcileDelete(t *testing.T) {
 		_, err := r.reconcileDelete(ctx, md)
 		g.Expect(err).ToNot(HaveOccurred())
 
+		g.Expect(controllerutil.ContainsFinalizer(md, clusterv1.MachineDeploymentTopologyFinalizer)).To(BeFalse())
+
 		gotMHC := clusterv1.MachineHealthCheck{}
 		err = fakeClient.Get(ctx, client.ObjectKeyFromObject(mhc), &gotMHC)
 		g.Expect(apierrors.IsNotFound(err)).To(BeTrue())

internal/controllers/topology/machineset/machineset_controller_test.go

@@ -60,18 +60,14 @@ func TestMachineSetTopologyFinalizer(t *testing.T) {
 	ms := msBuilder.Build()
 	msWithFinalizer := msBuilder.Build()
 	msWithFinalizer.Finalizers = []string{clusterv1.MachineSetTopologyFinalizer}
-	msWithDeletionTimestamp := msBuilder.Build()
-	deletionTimestamp := metav1.Now()
-	msWithDeletionTimestamp.DeletionTimestamp = &deletionTimestamp
-
-	msWithDeletionTimestampAndFinalizer := msWithDeletionTimestamp.DeepCopy()
-	msWithDeletionTimestampAndFinalizer.Finalizers = []string{clusterv1.MachineSetTopologyFinalizer}
 
 	testCases := []struct {
 		name            string
 		ms              *clusterv1.MachineSet
 		expectFinalizer bool
 	}{
+		// Note: We are not testing the case of a MS with deletionTimestamp and no finalizer.
+		// This case is impossible to reproduce in fake client without deleting the object.
 		{
 			name:            "should add ClusterTopology finalizer to a MachineSet with no finalizer",
 			ms:              ms,
@@ -82,11 +78,6 @@ func TestMachineSetTopologyFinalizer(t *testing.T) {
 			ms:              msWithFinalizer,
 			expectFinalizer: true,
 		},
-		{
-			name:            "should not add ClusterTopology finalizer on MachineSet with Deletion Timestamp and no finalizer ",
-			ms:              msWithDeletionTimestamp,
-			expectFinalizer: false,
-		},
 	}
 
 	for _, tc := range testCases {
@@ -135,6 +126,7 @@ func TestMachineSetReconciler_ReconcileDelete(t *testing.T) {
 		}).
 		Build()
 	ms.SetDeletionTimestamp(&deletionTimeStamp)
+	ms.SetFinalizers([]string{clusterv1.MachineSetTopologyFinalizer})
 	ms.SetOwnerReferences([]metav1.OwnerReference{
 		{
 			Kind:       "MachineDeployment",
@@ -146,6 +138,9 @@ func TestMachineSetReconciler_ReconcileDelete(t *testing.T) {
 	t.Run("Should delete templates of a MachineSet", func(t *testing.T) {
 		g := NewWithT(t)
 
+		// Copying the MS so changes made by reconcileDelete do not affect other tests.
+		ms := ms.DeepCopy()
+
 		fakeClient := fake.NewClientBuilder().
 			WithScheme(fakeScheme).
 			WithObjects(ms, msBT, msIMT).
@@ -158,10 +153,7 @@ func TestMachineSetReconciler_ReconcileDelete(t *testing.T) {
 		_, err := r.reconcileDelete(ctx, ms)
 		g.Expect(err).ToNot(HaveOccurred())
 
-		afterMS := &clusterv1.MachineSet{}
-		g.Expect(fakeClient.Get(ctx, client.ObjectKeyFromObject(ms), afterMS)).To(Succeed())
-
-		g.Expect(controllerutil.ContainsFinalizer(afterMS, clusterv1.MachineSetTopologyFinalizer)).To(BeFalse())
+		g.Expect(controllerutil.ContainsFinalizer(ms, clusterv1.MachineSetTopologyFinalizer)).To(BeFalse())
 		g.Expect(templateExists(fakeClient, msBT)).To(BeFalse())
 		g.Expect(templateExists(fakeClient, msIMT)).To(BeFalse())
 	})
@@ -177,6 +169,7 @@ func TestMachineSetReconciler_ReconcileDelete(t *testing.T) {
 			}).
 			Build()
 		msWithoutBootstrapTemplate.SetDeletionTimestamp(&deletionTimeStamp)
+		msWithoutBootstrapTemplate.SetFinalizers([]string{clusterv1.MachineSetTopologyFinalizer})
 		msWithoutBootstrapTemplate.SetOwnerReferences([]metav1.OwnerReference{
 			{
 				Kind:       "MachineDeployment",
@@ -197,16 +190,16 @@ func TestMachineSetReconciler_ReconcileDelete(t *testing.T) {
 		_, err := r.reconcileDelete(ctx, msWithoutBootstrapTemplate)
 		g.Expect(err).ToNot(HaveOccurred())
 
-		afterMS := &clusterv1.MachineSet{}
-		g.Expect(fakeClient.Get(ctx, client.ObjectKeyFromObject(msWithoutBootstrapTemplate), afterMS)).To(Succeed())
-
-		g.Expect(controllerutil.ContainsFinalizer(afterMS, clusterv1.MachineSetTopologyFinalizer)).To(BeFalse())
+		g.Expect(controllerutil.ContainsFinalizer(msWithoutBootstrapTemplate, clusterv1.MachineSetTopologyFinalizer)).To(BeFalse())
 		g.Expect(templateExists(fakeClient, msWithoutBootstrapTemplateIMT)).To(BeFalse())
 	})
 
 	t.Run("Should not delete templates of a MachineSet when they are still in use in a MachineDeployment", func(t *testing.T) {
 		g := NewWithT(t)
 
+		// Copying the MS so changes made by reconcileDelete do not affect other tests.
+		ms := ms.DeepCopy()
+
 		md := builder.MachineDeployment(metav1.NamespaceDefault, "md").
 			WithBootstrapTemplate(msBT).
 			WithInfrastructureTemplate(msIMT).
@@ -224,10 +217,7 @@ func TestMachineSetReconciler_ReconcileDelete(t *testing.T) {
 		_, err := r.reconcileDelete(ctx, ms)
 		g.Expect(err).ToNot(HaveOccurred())
 
-		afterMS := &clusterv1.MachineSet{}
-		g.Expect(fakeClient.Get(ctx, client.ObjectKeyFromObject(ms), afterMS)).To(Succeed())
-
-		g.Expect(controllerutil.ContainsFinalizer(afterMS, clusterv1.MachineSetTopologyFinalizer)).To(BeFalse())
+		g.Expect(controllerutil.ContainsFinalizer(ms, clusterv1.MachineSetTopologyFinalizer)).To(BeFalse())
 		g.Expect(templateExists(fakeClient, msBT)).To(BeTrue())
 		g.Expect(templateExists(fakeClient, msIMT)).To(BeTrue())
 	})
@@ -240,6 +230,7 @@ func TestMachineSetReconciler_ReconcileDelete(t *testing.T) {
 			WithInfrastructureTemplate(msIMT).
 			Build()
 		md.SetDeletionTimestamp(&deletionTimeStamp)
+		md.SetFinalizers([]string{clusterv1.MachineDeploymentTopologyFinalizer})
 
 		// anotherMS is another MachineSet of the same MachineDeployment using the same templates.
 		// Because anotherMS is not in deleting, reconcileDelete should not delete the templates.
@@ -270,10 +261,7 @@ func TestMachineSetReconciler_ReconcileDelete(t *testing.T) {
 		_, err := r.reconcileDelete(ctx, ms)
 		g.Expect(err).ToNot(HaveOccurred())
 
-		afterMS := &clusterv1.MachineSet{}
-		g.Expect(fakeClient.Get(ctx, client.ObjectKeyFromObject(ms), afterMS)).To(Succeed())
-
-		g.Expect(controllerutil.ContainsFinalizer(afterMS, clusterv1.MachineSetTopologyFinalizer)).To(BeFalse())
+		g.Expect(controllerutil.ContainsFinalizer(ms, clusterv1.MachineSetTopologyFinalizer)).To(BeFalse())
 		g.Expect(templateExists(fakeClient, msBT)).To(BeTrue())
 		g.Expect(templateExists(fakeClient, msIMT)).To(BeTrue())
 	})