Skip reconciling objects being deleted when no finalization needed

issue #3687

controllers/controllers/workloads/build/controller.go

@@ -66,6 +66,10 @@ func (r *Reconciler) SetupWithManager(mgr ctrl.Manager) *builder.Builder {
 func (r *Reconciler) ReconcileResource(ctx context.Context, cfBuild *korifiv1alpha1.CFBuild) (ctrl.Result, error) {
 	log := logr.FromContextOrDiscard(ctx)
 
+	if !cfBuild.GetDeletionTimestamp().IsZero() {
+		return ctrl.Result{}, nil
+	}
+
 	cfBuild.Status.ObservedGeneration = cfBuild.Generation
 	log.V(1).Info("set observed generation", "generation", cfBuild.Status.ObservedGeneration)
 

controllers/controllers/workloads/build/controller_test.go

@@ -126,6 +126,26 @@ var _ = Describe("CFBuildReconciler", func() {
 		}).Should(Succeed())
 	})
 
+	When("the build is being deleted", func() {
+		BeforeEach(func() {
+			cfBuild.Finalizers = []string{"do-not-delete-yet"}
+		})
+
+		JustBeforeEach(func() {
+			Expect(k8sManager.GetClient().Delete(ctx, cfBuild)).To(Succeed())
+			Expect(k8s.Patch(ctx, adminClient, cfBuild, func() {
+				cfBuild.Spec.StagingDiskMB = 345
+			})).To(Succeed())
+		})
+
+		It("does not reconcile the build", func() {
+			Consistently(func(g Gomega) {
+				g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(cfBuild), cfBuild)).To(Succeed())
+				g.Expect(cfBuild.Status.ObservedGeneration).NotTo(Equal(cfBuild.Generation))
+			}).Should(Succeed())
+		})
+	})
+
 	Describe("package type and build type mismatch", func() {
 		When("the package type is bits and build type is docker", func() {
 			BeforeEach(func() {

controllers/controllers/workloads/build/suite_test.go

@@ -30,6 +30,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/envtest"
 	logf "sigs.k8s.io/controller-runtime/pkg/log"
 	"sigs.k8s.io/controller-runtime/pkg/log/zap"
+	"sigs.k8s.io/controller-runtime/pkg/manager"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 )
 
@@ -40,6 +41,7 @@ var (
 	testEnv         *envtest.Environment
 	adminClient     client.Client
 	testNamespace   string
+	k8sManager      manager.Manager
 
 	reconciledBuildsSync sync.Map
 	buildCleanupsSync    sync.Map
@@ -72,7 +74,7 @@ var _ = BeforeSuite(func() {
 
 	Expect(korifiv1alpha1.AddToScheme(scheme.Scheme)).To(Succeed())
 
-	k8sManager := helpers.NewK8sManager(testEnv, filepath.Join("helm", "korifi", "controllers", "role.yaml"))
+	k8sManager = helpers.NewK8sManager(testEnv, filepath.Join("helm", "korifi", "controllers", "role.yaml"))
 	Expect(shared.SetupIndexWithManager(k8sManager)).To(Succeed())
 
 	adminClient, stopClientCache = helpers.NewCachedClient(testEnv.Config)

controllers/controllers/workloads/processes/controller.go

@@ -126,6 +126,10 @@ func (r *Reconciler) enqueueCFProcessRequestsForRoute(ctx context.Context, o cli
 func (r *Reconciler) ReconcileResource(ctx context.Context, cfProcess *korifiv1alpha1.CFProcess) (ctrl.Result, error) {
 	log := logr.FromContextOrDiscard(ctx)
 
+	if !cfProcess.GetDeletionTimestamp().IsZero() {
+		return ctrl.Result{}, nil
+	}
+
 	cfProcess.Status.ObservedGeneration = cfProcess.Generation
 	log.V(1).Info("set observed generation", "generation", cfProcess.Status.ObservedGeneration)
 

controllers/controllers/workloads/processes/controller_test.go

@@ -181,6 +181,26 @@ var _ = Describe("CFProcessReconciler Integration Tests", func() {
 		}).Should(Succeed())
 	})
 
+	When("the process is being deleted", func() {
+		BeforeEach(func() {
+			cfProcess.Finalizers = []string{"do-not-delete-yet"}
+		})
+
+		JustBeforeEach(func() {
+			Expect(k8sManager.GetClient().Delete(ctx, cfProcess)).To(Succeed())
+			Expect(k8s.Patch(ctx, adminClient, cfProcess, func() {
+				cfProcess.Spec.DiskQuotaMB = 345
+			})).To(Succeed())
+		})
+
+		It("does not reconcile the process", func() {
+			Consistently(func(g Gomega) {
+				g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(cfProcess), cfProcess)).To(Succeed())
+				g.Expect(cfProcess.Status.ObservedGeneration).NotTo(Equal(cfProcess.Generation))
+			}).Should(Succeed())
+		})
+	})
+
 	When("the CFApp desired state is STARTED", func() {
 		BeforeEach(func() {
 			Expect(k8s.PatchResource(ctx, adminClient, cfApp, func() {

controllers/controllers/workloads/processes/suite_test.go

@@ -25,6 +25,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/envtest"
 	logf "sigs.k8s.io/controller-runtime/pkg/log"
 	"sigs.k8s.io/controller-runtime/pkg/log/zap"
+	"sigs.k8s.io/controller-runtime/pkg/manager"
 )
 
 var (
@@ -34,6 +35,7 @@ var (
 	testEnv         *envtest.Environment
 	adminClient     client.Client
 	testNamespace   string
+	k8sManager      manager.Manager
 )
 
 func TestWorkloadsControllers(t *testing.T) {
@@ -62,7 +64,7 @@ var _ = BeforeSuite(func() {
 	Expect(korifiv1alpha1.AddToScheme(scheme.Scheme)).To(Succeed())
 	Expect(corev1.AddToScheme(scheme.Scheme)).To(Succeed())
 
-	k8sManager := helpers.NewK8sManager(testEnv, filepath.Join("helm", "korifi", "controllers", "role.yaml"))
+	k8sManager = helpers.NewK8sManager(testEnv, filepath.Join("helm", "korifi", "controllers", "role.yaml"))
 	Expect(shared.SetupIndexWithManager(k8sManager)).To(Succeed())
 
 	adminClient, stopClientCache = helpers.NewCachedClient(testEnv.Config)

controllers/controllers/workloads/tasks/controller.go

@@ -92,6 +92,10 @@ func (r *Reconciler) SetupWithManager(mgr ctrl.Manager) *builder.Builder {
 func (r *Reconciler) ReconcileResource(ctx context.Context, cfTask *korifiv1alpha1.CFTask) (ctrl.Result, error) {
 	log := logr.FromContextOrDiscard(ctx)
 
+	if !cfTask.GetDeletionTimestamp().IsZero() {
+		return ctrl.Result{}, nil
+	}
+
 	cfTask.Status.ObservedGeneration = cfTask.Generation
 	log.V(1).Info("set observed generation", "generation", cfTask.Status.ObservedGeneration)
 

controllers/controllers/workloads/tasks/controller_test.go

@@ -288,6 +288,33 @@ var _ = Describe("CFTaskReconciler Integration Tests", func() {
 			Expect(eventMessageArgs).To(Equal([]interface{}{cfTask.Name}), "Unexpected event message args in event record")
 		})
 
+		When("the task is being deleted", func() {
+			BeforeEach(func() {
+				Expect(k8s.PatchResource(ctx, adminClient, cfTask, func() {
+					cfTask.Finalizers = []string{"do-not-delete-yet"}
+				})).To(Succeed())
+			})
+
+			JustBeforeEach(func() {
+				Expect(k8sManager.GetClient().Delete(ctx, cfTask)).To(Succeed())
+				Expect(k8s.Patch(ctx, adminClient, cfTask, func() {
+					cfTask.Spec.Canceled = true
+				})).To(Succeed())
+			})
+
+			It("does not reconcile the task", func() {
+				Eventually(func(g Gomega) {
+					g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(cfTask), cfTask)).To(Succeed())
+					g.Expect(cfTask.Status.ObservedGeneration).NotTo(Equal(cfTask.Generation))
+				}).Should(Succeed())
+
+				Consistently(func(g Gomega) {
+					g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(cfTask), cfTask)).To(Succeed())
+					g.Expect(cfTask.Status.ObservedGeneration).NotTo(Equal(cfTask.Generation))
+				}).Should(Succeed())
+			})
+		})
+
 		When("the task workload status condition changes", func() {
 			JustBeforeEach(func() {
 				Eventually(func(g Gomega) {

controllers/controllers/workloads/tasks/suite_test.go

@@ -25,6 +25,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/envtest"
 	logf "sigs.k8s.io/controller-runtime/pkg/log"
 	"sigs.k8s.io/controller-runtime/pkg/log/zap"
+	"sigs.k8s.io/controller-runtime/pkg/manager"
 )
 
 var (
@@ -35,6 +36,7 @@ var (
 	adminClient     client.Client
 	testNamespace   string
 	eventRecorder   *controllerfake.EventRecorder
+	k8sManager      manager.Manager
 )
 
 func TestWorkloadsControllers(t *testing.T) {
@@ -63,7 +65,7 @@ var _ = BeforeSuite(func() {
 	Expect(korifiv1alpha1.AddToScheme(scheme.Scheme)).To(Succeed())
 	Expect(corev1.AddToScheme(scheme.Scheme)).To(Succeed())
 
-	k8sManager := helpers.NewK8sManager(testEnv, filepath.Join("helm", "korifi", "controllers", "role.yaml"))
+	k8sManager = helpers.NewK8sManager(testEnv, filepath.Join("helm", "korifi", "controllers", "role.yaml"))
 	Expect(shared.SetupIndexWithManager(k8sManager)).To(Succeed())
 
 	adminClient, stopClientCache = helpers.NewCachedClient(testEnv.Config)

job-task-runner/controllers/taskworkload_controller.go

@@ -91,6 +91,10 @@ func (r *TaskWorkloadReconciler) SetupWithManager(mgr ctrl.Manager) *builder.Bui
 func (r *TaskWorkloadReconciler) ReconcileResource(ctx context.Context, taskWorkload *korifiv1alpha1.TaskWorkload) (ctrl.Result, error) {
 	log := logr.FromContextOrDiscard(ctx)
 
+	if !taskWorkload.GetDeletionTimestamp().IsZero() {
+		return ctrl.Result{}, nil
+	}
+
 	taskWorkload.Status.ObservedGeneration = taskWorkload.Generation
 	log.V(1).Info("set observed generation", "generation", taskWorkload.Status.ObservedGeneration)
 

job-task-runner/controllers/taskworkload_controller_test.go

@@ -215,6 +215,25 @@ var _ = Describe("TaskworkloadController", func() {
 		Expect(meta.IsStatusConditionTrue(patchedTaskWorkload.Status.Conditions, "foo")).To(BeTrue())
 	})
 
+	When("the taskworkload is being deleted", func() {
+		BeforeEach(func() {
+			taskWorkload.DeletionTimestamp = &metav1.Time{Time: time.Now()}
+		})
+
+		It("returns an empty result and does not return error", func() {
+			Expect(reconcileResult).To(Equal(ctrl.Result{}))
+			Expect(reconcileErr).NotTo(HaveOccurred())
+		})
+
+		It("does not reconcile the task", func() {
+			Expect(fakeStatusWriter.PatchCallCount()).To(Equal(1))
+			_, object, _, _ := fakeStatusWriter.PatchArgsForCall(0)
+			patchedTaskWorkload, ok := object.(*korifiv1alpha1.TaskWorkload)
+			Expect(ok).To(BeTrue())
+			Expect(patchedTaskWorkload.Status.ObservedGeneration).To(BeZero())
+		})
+	})
+
 	When("getting the status conditions fails", func() {
 		BeforeEach(func() {
 			statusGetter.GetStatusConditionsReturns(nil, errors.New("get-conditions-error"))

kpack-image-builder/controllers/builderinfo_controller.go

@@ -107,6 +107,10 @@ func (r *BuilderInfoReconciler) filterBuilderInfos(object client.Object) bool {
 func (r *BuilderInfoReconciler) ReconcileResource(ctx context.Context, info *korifiv1alpha1.BuilderInfo) (ctrl.Result, error) {
 	log := logr.FromContextOrDiscard(ctx)
 
+	if !info.GetDeletionTimestamp().IsZero() {
+		return ctrl.Result{}, nil
+	}
+
 	info.Status.ObservedGeneration = info.Generation
 	log.V(1).Info("set observed generation", "generation", info.Status.ObservedGeneration)
 

kpack-image-builder/controllers/builderinfo_controller_test.go

@@ -3,9 +3,11 @@ package controllers_test
 import (
 	"context"
 	"fmt"
+	"time"
 
 	"code.cloudfoundry.org/korifi/controllers/api/v1alpha1"
 	"code.cloudfoundry.org/korifi/kpack-image-builder/controllers"
+	"code.cloudfoundry.org/korifi/tools/k8s"
 
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
@@ -41,7 +43,7 @@ var _ = Describe("BuilderInfoReconciler", Serial, func() {
 
 	AfterEach(func() {
 		if info != nil {
-			Expect(adminClient.Delete(context.Background(), info)).To(Succeed())
+			Expect(client.IgnoreNotFound(adminClient.Delete(context.Background(), info))).To(Succeed())
 		}
 		if clusterBuilder != nil {
 			Expect(adminClient.Delete(context.Background(), clusterBuilder)).To(Succeed())
@@ -143,6 +145,36 @@ var _ = Describe("BuilderInfoReconciler", Serial, func() {
 				}).Should(Succeed())
 			})
 
+			When("the builder info is being deleted", func() {
+				JustBeforeEach(func() {
+					Expect(k8s.PatchResource(ctx, adminClient, info, func() {
+						info.Finalizers = []string{"do-not-delete-yet"}
+					})).To(Succeed())
+
+					Expect(k8sManager.GetClient().Delete(ctx, info)).To(Succeed())
+					Expect(k8s.Patch(ctx, adminClient, info, func() {
+						info.Status.Buildpacks = []v1alpha1.BuilderInfoStatusBuildpack{{
+							Name:              "foobar",
+							CreationTimestamp: metav1.NewTime(time.Now()),
+							UpdatedTimestamp:  metav1.NewTime(time.Now()),
+						}}
+					})).To(Succeed())
+				})
+
+				It("does not reconcile the process", func() {
+					Consistently(func(g Gomega) {
+						g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(info), info)).To(Succeed())
+						g.Expect(info.Status.ObservedGeneration).NotTo(Equal(info.Generation))
+					}).Should(Succeed())
+
+					// Gross workaround to make `AfterEach` that ensures the info is gone happy
+					// see https://github.com/cloudfoundry/korifi/issues/3743
+					Expect(k8s.PatchResource(ctx, adminClient, info, func() {
+						info.Finalizers = []string{}
+					})).To(Succeed())
+				})
+			})
+
 			When("the cluster builder status is not ready", func() {
 				JustBeforeEach(func() {
 					ok := false

kpack-image-builder/controllers/suite_test.go

@@ -44,6 +44,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/envtest"
 	logf "sigs.k8s.io/controller-runtime/pkg/log"
 	"sigs.k8s.io/controller-runtime/pkg/log/zap"
+	"sigs.k8s.io/controller-runtime/pkg/manager"
 	//+kubebuilder:scaffold:imports
 )
 
@@ -65,6 +66,7 @@ var (
 	buildWorkloadReconciler *k8s.PatchingReconciler[korifiv1alpha1.BuildWorkload, *korifiv1alpha1.BuildWorkload]
 	rootNamespace           *v1.Namespace
 	imageRepoCreator        *fake.RepositoryCreator
+	k8sManager              manager.Manager
 )
 
 func TestAPIs(t *testing.T) {
@@ -101,7 +103,7 @@ var _ = BeforeSuite(func() {
 	Expect(korifiv1alpha1.AddToScheme(scheme.Scheme)).To(Succeed())
 	Expect(buildv1alpha2.AddToScheme(scheme.Scheme)).To(Succeed())
 
-	k8sManager := helpers.NewK8sManager(testEnv, filepath.Join("helm", "korifi", "kpack-image-builder", "role.yaml"))
+	k8sManager = helpers.NewK8sManager(testEnv, filepath.Join("helm", "korifi", "kpack-image-builder", "role.yaml"))
 	adminClient, stopClientCache = helpers.NewCachedClient(testEnv.Config)
 
 	finalizer.NewKpackImageBuilderFinalizerWebhook().SetupWebhookWithManager(k8sManager)

statefulset-runner/controllers/runnerinfo_controller.go

@@ -71,6 +71,10 @@ func filterRunnerInfos(object client.Object) bool {
 func (r *RunnerInfoReconciler) ReconcileResource(ctx context.Context, runnerInfo *korifiv1alpha1.RunnerInfo) (ctrl.Result, error) {
 	log := logr.FromContextOrDiscard(ctx)
 
+	if !runnerInfo.GetDeletionTimestamp().IsZero() {
+		return ctrl.Result{}, nil
+	}
+
 	runnerInfo.Status.ObservedGeneration = runnerInfo.Generation
 	log.V(1).Info("set observed generation", "generation", runnerInfo.Status.ObservedGeneration)