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 <[email protected]>
cloudfoundry · Oct 8, 2024 · 72c7b58 · 72c7b58
1 parent 2dfdb38
commit 72c7b58
Showing 1 changed file with 16 additions and 18 deletions.
diff --git 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 {