Early return if finalizer does not exist to make it more readable (#2262)

controllers/actions.github.com/autoscalinglistener_controller.go

@@ -73,30 +73,31 @@ func (r *AutoscalingListenerReconciler) Reconcile(ctx context.Context, req ctrl.
 	}
 
 	if !autoscalingListener.ObjectMeta.DeletionTimestamp.IsZero() {
-		if controllerutil.ContainsFinalizer(autoscalingListener, autoscalingListenerFinalizerName) {
+		if !controllerutil.ContainsFinalizer(autoscalingListener, autoscalingListenerFinalizerName) {
-			log.Info("Deleting resources")
+			return ctrl.Result{}, nil
-			done, err := r.cleanupResources(ctx, autoscalingListener, log)
-			if err != nil {
-				log.Error(err, "Failed to cleanup resources after deletion")
-				return ctrl.Result{}, err
-			}
-			if !done {
-				log.Info("Waiting for resources to be deleted before removing finalizer")
-				return ctrl.Result{}, nil
-			}
-
-			log.Info("Removing finalizer")
-			err = patch(ctx, r.Client, autoscalingListener, func(obj *v1alpha1.AutoscalingListener) {
-				controllerutil.RemoveFinalizer(obj, autoscalingListenerFinalizerName)
-			})
-			if err != nil && !kerrors.IsNotFound(err) {
-				log.Error(err, "Failed to remove finalizer")
-				return ctrl.Result{}, err
-			}
-
-			log.Info("Successfully removed finalizer after cleanup")
 		}
-		return ctrl.Result{}, nil
+
+		log.Info("Deleting resources")
+		done, err := r.cleanupResources(ctx, autoscalingListener, log)
+		if err != nil {
+			log.Error(err, "Failed to cleanup resources after deletion")
+			return ctrl.Result{}, err
+		}
+		if !done {
+			log.Info("Waiting for resources to be deleted before removing finalizer")
+			return ctrl.Result{}, nil
+		}
+
+		log.Info("Removing finalizer")
+		err = patch(ctx, r.Client, autoscalingListener, func(obj *v1alpha1.AutoscalingListener) {
+			controllerutil.RemoveFinalizer(obj, autoscalingListenerFinalizerName)
+		})
+		if err != nil && !kerrors.IsNotFound(err) {
+			log.Error(err, "Failed to remove finalizer")
+			return ctrl.Result{}, err
+		}
+
+		log.Info("Successfully removed finalizer after cleanup")
 	}
 
 	if !controllerutil.ContainsFinalizer(autoscalingListener, autoscalingListenerFinalizerName) {

controllers/actions.github.com/autoscalingrunnerset_controller.go

@@ -86,48 +86,49 @@ func (r *AutoscalingRunnerSetReconciler) Reconcile(ctx context.Context, req ctrl
 	}
 
 	if !autoscalingRunnerSet.ObjectMeta.DeletionTimestamp.IsZero() {
-		if controllerutil.ContainsFinalizer(autoscalingRunnerSet, autoscalingRunnerSetFinalizerName) {
+		if !controllerutil.ContainsFinalizer(autoscalingRunnerSet, autoscalingRunnerSetFinalizerName) {
-			log.Info("Deleting resources")
+			return ctrl.Result{}, nil
-			done, err := r.cleanupListener(ctx, autoscalingRunnerSet, log)
-			if err != nil {
-				log.Error(err, "Failed to clean up listener")
-				return ctrl.Result{}, err
-			}
-			if !done {
-				// we are going to get notified anyway to proceed with rest of the
-				// cleanup. No need to re-queue
-				log.Info("Waiting for listener to be deleted")
-				return ctrl.Result{}, nil
-			}
-
-			done, err = r.cleanupEphemeralRunnerSets(ctx, autoscalingRunnerSet, log)
-			if err != nil {
-				log.Error(err, "Failed to clean up ephemeral runner sets")
-				return ctrl.Result{}, err
-			}
-			if !done {
-				log.Info("Waiting for ephemeral runner sets to be deleted")
-				return ctrl.Result{}, nil
-			}
-
-			err = r.deleteRunnerScaleSet(ctx, autoscalingRunnerSet, log)
-			if err != nil {
-				log.Error(err, "Failed to delete runner scale set")
-				return ctrl.Result{}, err
-			}
-
-			log.Info("Removing finalizer")
-			err = patch(ctx, r.Client, autoscalingRunnerSet, func(obj *v1alpha1.AutoscalingRunnerSet) {
-				controllerutil.RemoveFinalizer(obj, autoscalingRunnerSetFinalizerName)
-			})
-			if err != nil && !kerrors.IsNotFound(err) {
-				log.Error(err, "Failed to update autoscaling runner set without finalizer")
-				return ctrl.Result{}, err
-			}
-
-			log.Info("Successfully removed finalizer after cleanup")
 		}
-		return ctrl.Result{}, nil
+
+		log.Info("Deleting resources")
+		done, err := r.cleanupListener(ctx, autoscalingRunnerSet, log)
+		if err != nil {
+			log.Error(err, "Failed to clean up listener")
+			return ctrl.Result{}, err
+		}
+		if !done {
+			// we are going to get notified anyway to proceed with rest of the
+			// cleanup. No need to re-queue
+			log.Info("Waiting for listener to be deleted")
+			return ctrl.Result{}, nil
+		}
+
+		done, err = r.cleanupEphemeralRunnerSets(ctx, autoscalingRunnerSet, log)
+		if err != nil {
+			log.Error(err, "Failed to clean up ephemeral runner sets")
+			return ctrl.Result{}, err
+		}
+		if !done {
+			log.Info("Waiting for ephemeral runner sets to be deleted")
+			return ctrl.Result{}, nil
+		}
+
+		err = r.deleteRunnerScaleSet(ctx, autoscalingRunnerSet, log)
+		if err != nil {
+			log.Error(err, "Failed to delete runner scale set")
+			return ctrl.Result{}, err
+		}
+
+		log.Info("Removing finalizer")
+		err = patch(ctx, r.Client, autoscalingRunnerSet, func(obj *v1alpha1.AutoscalingRunnerSet) {
+			controllerutil.RemoveFinalizer(obj, autoscalingRunnerSetFinalizerName)
+		})
+		if err != nil && !kerrors.IsNotFound(err) {
+			log.Error(err, "Failed to update autoscaling runner set without finalizer")
+			return ctrl.Result{}, err
+		}
+
+		log.Info("Successfully removed finalizer after cleanup")
 	}
 
 	if !controllerutil.ContainsFinalizer(autoscalingRunnerSet, autoscalingRunnerSetFinalizerName) {

controllers/actions.github.com/ephemeralrunner_controller.go

@@ -76,40 +76,41 @@ func (r *EphemeralRunnerReconciler) Reconcile(ctx context.Context, req ctrl.Requ
 	}
 
 	if !ephemeralRunner.ObjectMeta.DeletionTimestamp.IsZero() {
-		if controllerutil.ContainsFinalizer(ephemeralRunner, ephemeralRunnerFinalizerName) {
+		if !controllerutil.ContainsFinalizer(ephemeralRunner, ephemeralRunnerFinalizerName) {
-			log.Info("Finalizing ephemeral runner")
-			done, err := r.cleanupResources(ctx, ephemeralRunner, log)
-			if err != nil {
-				log.Error(err, "Failed to clean up ephemeral runner owned resources")
-				return ctrl.Result{}, err
-			}
-			if !done {
-				log.Info("Waiting for ephemeral runner owned resources to be deleted")
-				return ctrl.Result{}, nil
-			}
-
-			done, err = r.cleanupContainerHooksResources(ctx, ephemeralRunner, log)
-			if err != nil {
-				log.Error(err, "Failed to clean up container hooks resources")
-				return ctrl.Result{}, err
-			}
-			if !done {
-				log.Info("Waiting for container hooks resources to be deleted")
-				return ctrl.Result{RequeueAfter: 5 * time.Second}, nil
-			}
-
-			log.Info("Removing finalizer")
-			err = patch(ctx, r.Client, ephemeralRunner, func(obj *v1alpha1.EphemeralRunner) {
-				controllerutil.RemoveFinalizer(obj, ephemeralRunnerFinalizerName)
-			})
-			if err != nil && !kerrors.IsNotFound(err) {
-				log.Error(err, "Failed to update ephemeral runner without the finalizer")
-				return ctrl.Result{}, err
-			}
-
-			log.Info("Successfully removed finalizer after cleanup")
 			return ctrl.Result{}, nil
 		}
+
+		log.Info("Finalizing ephemeral runner")
+		done, err := r.cleanupResources(ctx, ephemeralRunner, log)
+		if err != nil {
+			log.Error(err, "Failed to clean up ephemeral runner owned resources")
+			return ctrl.Result{}, err
+		}
+		if !done {
+			log.Info("Waiting for ephemeral runner owned resources to be deleted")
+			return ctrl.Result{}, nil
+		}
+
+		done, err = r.cleanupContainerHooksResources(ctx, ephemeralRunner, log)
+		if err != nil {
+			log.Error(err, "Failed to clean up container hooks resources")
+			return ctrl.Result{}, err
+		}
+		if !done {
+			log.Info("Waiting for container hooks resources to be deleted")
+			return ctrl.Result{RequeueAfter: 5 * time.Second}, nil
+		}
+
+		log.Info("Removing finalizer")
+		err = patch(ctx, r.Client, ephemeralRunner, func(obj *v1alpha1.EphemeralRunner) {
+			controllerutil.RemoveFinalizer(obj, ephemeralRunnerFinalizerName)
+		})
+		if err != nil && !kerrors.IsNotFound(err) {
+			log.Error(err, "Failed to update ephemeral runner without the finalizer")
+			return ctrl.Result{}, err
+		}
+
+		log.Info("Successfully removed finalizer after cleanup")
 		return ctrl.Result{}, nil
 	}
 

controllers/actions.github.com/ephemeralrunnerset_controller.go

@@ -81,29 +81,30 @@ func (r *EphemeralRunnerSetReconciler) Reconcile(ctx context.Context, req ctrl.R
 
 	// Requested deletion does not need reconciled.
 	if !ephemeralRunnerSet.ObjectMeta.DeletionTimestamp.IsZero() {
-		if controllerutil.ContainsFinalizer(ephemeralRunnerSet, ephemeralRunnerSetFinalizerName) {
+		if !controllerutil.ContainsFinalizer(ephemeralRunnerSet, ephemeralRunnerSetFinalizerName) {
-			log.Info("Deleting resources")
-			done, err := r.cleanUpEphemeralRunners(ctx, ephemeralRunnerSet, log)
-			if err != nil {
-				log.Error(err, "Failed to clean up EphemeralRunners")
-				return ctrl.Result{}, err
-			}
-			if !done {
-				log.Info("Waiting for resources to be deleted")
-				return ctrl.Result{}, nil
-			}
-
-			log.Info("Removing finalizer")
-			if err := patch(ctx, r.Client, ephemeralRunnerSet, func(obj *v1alpha1.EphemeralRunnerSet) {
-				controllerutil.RemoveFinalizer(obj, ephemeralRunnerSetFinalizerName)
-			}); err != nil && !kerrors.IsNotFound(err) {
-				log.Error(err, "Failed to update ephemeral runner set with removed finalizer")
-				return ctrl.Result{}, err
-			}
-
-			log.Info("Successfully removed finalizer after cleanup")
 			return ctrl.Result{}, nil
 		}
+
+		log.Info("Deleting resources")
+		done, err := r.cleanUpEphemeralRunners(ctx, ephemeralRunnerSet, log)
+		if err != nil {
+			log.Error(err, "Failed to clean up EphemeralRunners")
+			return ctrl.Result{}, err
+		}
+		if !done {
+			log.Info("Waiting for resources to be deleted")
+			return ctrl.Result{}, nil
+		}
+
+		log.Info("Removing finalizer")
+		if err := patch(ctx, r.Client, ephemeralRunnerSet, func(obj *v1alpha1.EphemeralRunnerSet) {
+			controllerutil.RemoveFinalizer(obj, ephemeralRunnerSetFinalizerName)
+		}); err != nil && !kerrors.IsNotFound(err) {
+			log.Error(err, "Failed to update ephemeral runner set with removed finalizer")
+			return ctrl.Result{}, err
+		}
+
+		log.Info("Successfully removed finalizer after cleanup")
 		return ctrl.Result{}, nil
 	}