fix reconcilation when pods was deleted

When pods are deleted then podRunnerPairs should be reloaded, so it's
better to requeue reconcilation immediately.

When new pods are scale up then it takes some time usually a few seconds
to sync between pods and runner API and so there is no need to wait
whole period (default 1m) to sync.

Signed-off-by: Lukas Hybner <[email protected]>

Author: Lukas Hybner

controllers/githubactionrunner_controller.go

@@ -116,13 +116,18 @@ func (r *GithubActionRunnerReconciler) handleScaling(ctx context.Context, instan
 
 	// safety guard - always look for finalizers in order to unregister runners for pods about to delete
 	// pods could have been deleted by user directly and not through operator
-	if err = r.handleFinalization(ctx, instance, podRunnerPairs); err != nil {
+	// if some pods are deleted then it's better to requeue immediately, because we have to reload podRunnerPairs.
+	numDeleted, err := r.handleFinalization(ctx, instance, podRunnerPairs)
+	if err != nil {
 		return r.manageOutcome(ctx, instance, err)
 	}
+	if numDeleted > 0 {
+		return r.ManageSuccess(ctx, instance)
+	}
 
 	if !podRunnerPairs.inSync() {
-		logger.Info("Pods and runner API not in sync, returning early")
-		return r.manageOutcome(ctx, instance, nil)
+		logger.Info("Pods and runner API not in sync, returning early", "Pods", podRunnerPairs.numPods(), "Runners", podRunnerPairs.numRunners())
+		return r.ManageSuccessWithRequeue(ctx, instance, 3*time.Second)
 	}
 
 	if shouldScaleUp(podRunnerPairs, instance) {
@@ -362,29 +367,31 @@ func (r *GithubActionRunnerReconciler) unregisterRunner(ctx context.Context, cr
 }
 
 // handleFinalization will remove runner from github based on presence of finalizer
-func (r *GithubActionRunnerReconciler) handleFinalization(ctx context.Context, cr *garov1alpha1.GithubActionRunner, list podRunnerPairList) error {
+func (r *GithubActionRunnerReconciler) handleFinalization(ctx context.Context, cr *garov1alpha1.GithubActionRunner, list podRunnerPairList) (int, error) {
+	numDeleted := 0
 	for _, item := range list.getPodsBeingDeletedOrEvictedOrCompleted() {
+		numDeleted++
 		// TODO - cause of failure should be checked more closely, if it does not exist we can ignore it. If it is a comms error we should stick around
 		if err := r.unregisterRunner(ctx, cr, item); err != nil {
-			return err
+			return numDeleted, err
 		}
 		if isEvicted(&item.pod) && env.GetBoolDefault(deleteEvictedPodsEnvVarName, true) {
 			logr.FromContextOrDiscard(ctx).Info("Deleting evicted pod", "podname", item.pod.Name)
 			err := r.DeleteResourceIfExists(ctx, &item.pod)
 			if err != nil {
-				return err
+				return numDeleted, err
 			}
 		}
 		if isCompleted(&item.pod) {
 			logr.FromContextOrDiscard(ctx).Info("Deleting succeeded pod", "podname", item.pod.Name)
 			err := r.DeleteResourceIfExists(ctx, &item.pod)
 			if err != nil {
-				return err
+				return numDeleted, err
 			}
 		}
 	}
 
-	return nil
+	return numDeleted, nil
 }
 
 // tokenForRef returns the token referenced from the GithubActionRunner Spec.TokenRef