diff --git a/controllers/controllers/workloads/build/controller.go b/controllers/controllers/workloads/build/controller.go index e14939d01..f5db407cf 100644 --- a/controllers/controllers/workloads/build/controller.go +++ b/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) diff --git a/controllers/controllers/workloads/build/controller_test.go b/controllers/controllers/workloads/build/controller_test.go index bacffde3b..3e5840476 100644 --- a/controllers/controllers/workloads/build/controller_test.go +++ b/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() { diff --git a/controllers/controllers/workloads/build/suite_test.go b/controllers/controllers/workloads/build/suite_test.go index 184de92eb..1043bab98 100644 --- a/controllers/controllers/workloads/build/suite_test.go +++ b/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) diff --git a/controllers/controllers/workloads/processes/controller.go b/controllers/controllers/workloads/processes/controller.go index 78574534e..94f4b21ca 100644 --- a/controllers/controllers/workloads/processes/controller.go +++ b/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) diff --git a/controllers/controllers/workloads/processes/controller_test.go b/controllers/controllers/workloads/processes/controller_test.go index 7c508369c..233790169 100644 --- a/controllers/controllers/workloads/processes/controller_test.go +++ b/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() { diff --git a/controllers/controllers/workloads/processes/suite_test.go b/controllers/controllers/workloads/processes/suite_test.go index d238ce5bf..f63b43f94 100644 --- a/controllers/controllers/workloads/processes/suite_test.go +++ b/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) diff --git a/controllers/controllers/workloads/tasks/controller.go b/controllers/controllers/workloads/tasks/controller.go index 0de63cf0e..29aa7d0a8 100644 --- a/controllers/controllers/workloads/tasks/controller.go +++ b/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) diff --git a/controllers/controllers/workloads/tasks/controller_test.go b/controllers/controllers/workloads/tasks/controller_test.go index 3fd235083..e7cea8fb8 100644 --- a/controllers/controllers/workloads/tasks/controller_test.go +++ b/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) { diff --git a/controllers/controllers/workloads/tasks/suite_test.go b/controllers/controllers/workloads/tasks/suite_test.go index 5988ebe40..542bbaa25 100644 --- a/controllers/controllers/workloads/tasks/suite_test.go +++ b/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) diff --git a/job-task-runner/controllers/taskworkload_controller.go b/job-task-runner/controllers/taskworkload_controller.go index f2d1f9e01..db4989c9d 100644 --- a/job-task-runner/controllers/taskworkload_controller.go +++ b/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) diff --git a/job-task-runner/controllers/taskworkload_controller_test.go b/job-task-runner/controllers/taskworkload_controller_test.go index 5d983f133..956f0aee5 100644 --- a/job-task-runner/controllers/taskworkload_controller_test.go +++ b/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")) diff --git a/kpack-image-builder/controllers/builderinfo_controller.go b/kpack-image-builder/controllers/builderinfo_controller.go index ee890deb8..4c7ba07c2 100644 --- a/kpack-image-builder/controllers/builderinfo_controller.go +++ b/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) diff --git a/kpack-image-builder/controllers/builderinfo_controller_test.go b/kpack-image-builder/controllers/builderinfo_controller_test.go index 7f6cbb474..9bccd1dd8 100644 --- a/kpack-image-builder/controllers/builderinfo_controller_test.go +++ b/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 diff --git a/kpack-image-builder/controllers/suite_test.go b/kpack-image-builder/controllers/suite_test.go index 7c04cb8ce..fe6fbdf18 100644 --- a/kpack-image-builder/controllers/suite_test.go +++ b/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) diff --git a/statefulset-runner/controllers/runnerinfo_controller.go b/statefulset-runner/controllers/runnerinfo_controller.go index d37460604..e327c390c 100644 --- a/statefulset-runner/controllers/runnerinfo_controller.go +++ b/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) diff --git a/statefulset-runner/controllers/runnerinfo_controller_test.go b/statefulset-runner/controllers/runnerinfo_controller_test.go index c64f64e06..480cb75e4 100644 --- a/statefulset-runner/controllers/runnerinfo_controller_test.go +++ b/statefulset-runner/controllers/runnerinfo_controller_test.go @@ -2,6 +2,7 @@ package controllers_test import ( "context" + "time" korifiv1alpha1 "code.cloudfoundry.org/korifi/controllers/api/v1alpha1" "code.cloudfoundry.org/korifi/statefulset-runner/controllers" @@ -19,35 +20,32 @@ import ( var _ = Describe("RunnerInfo Reconcile", func() { var ( - reconciler *k8s.PatchingReconciler[korifiv1alpha1.RunnerInfo, *korifiv1alpha1.RunnerInfo] - reconcileResult ctrl.Result - reconcileErr error - req ctrl.Request - getRunnerInfoError error - runnerInfo *korifiv1alpha1.RunnerInfo - runnerName string + reconciler *k8s.PatchingReconciler[korifiv1alpha1.RunnerInfo, *korifiv1alpha1.RunnerInfo] + reconcileResult ctrl.Result + reconcileErr error + req ctrl.Request + runnerInfo *korifiv1alpha1.RunnerInfo ) - JustBeforeEach(func() { + BeforeEach(func() { Expect(korifiv1alpha1.AddToScheme(scheme.Scheme)).To(Succeed()) runnerInfo = &korifiv1alpha1.RunnerInfo{ ObjectMeta: v1.ObjectMeta{ - Name: runnerName, - Namespace: uuid.NewString(), + Name: "statefulset-runner", + Namespace: uuid.NewString(), + Generation: 1, }, Spec: korifiv1alpha1.RunnerInfoSpec{ - RunnerName: runnerName, + RunnerName: "statefulset-runner", }, } - getRunnerInfoError = nil - fakeClient.GetStub = func(_ context.Context, _ types.NamespacedName, obj client.Object, _ ...client.GetOption) error { switch obj := obj.(type) { case *korifiv1alpha1.RunnerInfo: runnerInfo.DeepCopyInto(obj) - return getRunnerInfoError + return nil default: panic("TestClient Get provided an unexpected object type") } @@ -58,28 +56,45 @@ var _ = Describe("RunnerInfo Reconcile", func() { scheme.Scheme, ctrl.Log.WithName("controllers").WithName("TestRunnerInfo"), ) + }) + + JustBeforeEach(func() { reconcileResult, reconcileErr = reconciler.Reconcile(context.Background(), req) }) - When("the RunnerInfo is being reconciled", func() { - It("reconciles without error", func() { - Expect(reconcileResult).To(Equal(ctrl.Result{})) - Expect(reconcileErr).NotTo(HaveOccurred()) - }) + It("reconciles without error", func() { + Expect(reconcileResult).To(Equal(ctrl.Result{})) + Expect(reconcileErr).NotTo(HaveOccurred()) + _, object, _, _ := fakeStatusWriter.PatchArgsForCall(0) + patchedRunnerInfo, ok := object.(*korifiv1alpha1.RunnerInfo) + Expect(ok).To(BeTrue()) + Expect(patchedRunnerInfo.Status.ObservedGeneration).To(Equal(patchedRunnerInfo.Generation)) + }) + + It("applies the Status.Capabilities.RollingDeploy field", func() { + _, object, _, _ := fakeStatusWriter.PatchArgsForCall(0) + patchedRunnerInfo, ok := object.(*korifiv1alpha1.RunnerInfo) + Expect(ok).To(BeTrue()) + Expect(patchedRunnerInfo.Status.ObservedGeneration).To(Equal(patchedRunnerInfo.Generation)) + Expect(patchedRunnerInfo.Status.Capabilities.RollingDeploy).To(BeTrue()) }) - // Filtering is done via predicate. This directly invokes the reconcile function, so the negative case cannot be tested here. - When("the RunnerName matches the AppWorkloadReconcilerName", func() { + When("the RunnerInfo is being deleted", func() { BeforeEach(func() { - runnerName = "statefulset-runner" + runnerInfo.DeletionTimestamp = &v1.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("applies the Status.Capabilities.RollingDeploy field", func() { + It("does not reconcile the info", func() { + Expect(fakeStatusWriter.PatchCallCount()).To(Equal(1)) _, object, _, _ := fakeStatusWriter.PatchArgsForCall(0) patchedRunnerInfo, ok := object.(*korifiv1alpha1.RunnerInfo) Expect(ok).To(BeTrue()) - Expect(patchedRunnerInfo.Status.ObservedGeneration).To(Equal(patchedRunnerInfo.Generation)) - Expect(patchedRunnerInfo.Status.Capabilities.RollingDeploy).To(BeTrue()) + Expect(patchedRunnerInfo.Status.ObservedGeneration).To(BeZero()) }) }) })