From 72c7b58520b7e583c6e8806dc547bc95accca762 Mon Sep 17 00:00:00 2001 From: Georgi Sabev Date: Tue, 8 Oct 2024 16:20:49 +0000 Subject: [PATCH] Do not requeue when finalizing managed service instance A couple of flakes on the CI showed that a managed service might be deprovisioned with the broker twice: ``` https://ci.korifi.cf-app.com/teams/main/pipelines/main/jobs/run-tests-periodic/builds/19402 https://ci.korifi.cf-app.com/teams/main/pipelines/main/jobs/run-tests-periodic/builds/19396 https://ci.korifi.cf-app.com/teams/main/pipelines/main/jobs/run-tests-periodic/builds/19358 ``` Analysis showed that when the controller [requeues the reconcile event on finalization](https://github.com/cloudfoundry/korifi/blob/2dfdb3848bc8690e3e5be14a535ae52c46152d46/controllers/controllers/services/instances/managed/controller.go#L265), the very same object (same generation, same resource version) could be reconciled twice thus request deprovision the instance from the broker twice. In order to address this, this change removes the finalizer when deprovisioning with the broker succeeds and does not requeue the reconcile event. Co-authored-by: Danail Branekov --- .../services/instances/managed/controller.go | 34 +++++++++---------- 1 file changed, 16 insertions(+), 18 deletions(-) diff --git a/controllers/controllers/services/instances/managed/controller.go b/controllers/controllers/services/instances/managed/controller.go index 64f1b5821..36d3a2b4e 100644 --- a/controllers/controllers/services/instances/managed/controller.go +++ b/controllers/controllers/services/instances/managed/controller.go @@ -242,31 +242,29 @@ func (r *Reconciler) finalizeCFServiceInstance( return ctrl.Result{}, nil } - if isDeprovisionRequested(serviceInstance) { - if controllerutil.RemoveFinalizer(serviceInstance, korifiv1alpha1.CFManagedServiceInstanceFinalizerName) { - log.V(1).Info("finalizer removed") + if !isDeprovisionRequested(serviceInstance) { + err := r.deprovisionServiceInstance(ctx, serviceInstance) + if err != nil { + return ctrl.Result{}, err } - return ctrl.Result{}, nil + meta.SetStatusCondition(&serviceInstance.Status.Conditions, metav1.Condition{ + Type: korifiv1alpha1.DeprovisionRequestedCondition, + Status: metav1.ConditionTrue, + ObservedGeneration: serviceInstance.Generation, + LastTransitionTime: metav1.NewTime(time.Now()), + Reason: "DeprovisionRequested", + }) } - err := r.deprovisionServiceinstance(ctx, serviceInstance) - if err != nil { - return ctrl.Result{}, err - } + controllerutil.RemoveFinalizer(serviceInstance, korifiv1alpha1.CFManagedServiceInstanceFinalizerName) + log.V(1).Info("finalizer removed") - meta.SetStatusCondition(&serviceInstance.Status.Conditions, metav1.Condition{ - Type: korifiv1alpha1.DeprovisionRequestedCondition, - Status: metav1.ConditionTrue, - ObservedGeneration: serviceInstance.Generation, - LastTransitionTime: metav1.NewTime(time.Now()), - Reason: "DeprovisionRequested", - }) - return ctrl.Result{Requeue: true}, nil + return ctrl.Result{}, nil } -func (r *Reconciler) deprovisionServiceinstance(ctx context.Context, serviceInstance *korifiv1alpha1.CFServiceInstance) error { - log := logr.FromContextOrDiscard(ctx).WithName("finalizeCFServiceInstance") +func (r *Reconciler) deprovisionServiceInstance(ctx context.Context, serviceInstance *korifiv1alpha1.CFServiceInstance) error { + log := logr.FromContextOrDiscard(ctx).WithName("deprovisionServiceInstance") servicePlan, err := r.getServicePlan(ctx, serviceInstance.Spec.PlanGUID) if err != nil {