Merge pull request #1584 from grafana/fix/alert-rule-group-error-prop…

…agation fix: use correct error propagation logic in alert rule group handler
grafana · Jun 19, 2024 · fb7c800 · fb7c800
2 parents bf7bf2c + 4b5e175
commit fb7c800
Show file tree

Hide file tree

Showing 2 changed files with 13 additions and 9 deletions.
diff --git a/controllers/controller_shared.go b/controllers/controller_shared.go
@@ -122,6 +122,10 @@ func setNoMatchingInstance(conditions *[]metav1.Condition, generation int64, rea
 	})
 }
 
+func removeNoMatchingInstance(conditions *[]metav1.Condition) {
+	meta.RemoveStatusCondition(conditions, conditionNoMatchingInstance)
+}
+
 func setNoMatchingFolder(conditions *[]metav1.Condition, generation int64, reason, message string) {
 	meta.SetStatusCondition(conditions, metav1.Condition{
 		Type:               conditionNoMatchingFolder,
@@ -135,8 +139,8 @@ func setNoMatchingFolder(conditions *[]metav1.Condition, generation int64, reaso
 	})
 }
 
-func removeNoMatchingInstance(conditions *[]metav1.Condition) {
-	meta.RemoveStatusCondition(conditions, conditionNoMatchingInstance)
+func removeNoMatchingFolder(conditions *[]metav1.Condition) {
+	meta.RemoveStatusCondition(conditions, conditionNoMatchingFolder)
 }
 
 func ignoreStatusUpdates() predicate.Predicate {

diff --git a/controllers/grafanaalertrulegroup_controller.go b/controllers/grafanaalertrulegroup_controller.go
@@ -76,20 +76,20 @@ func (r *GrafanaAlertRuleGroupReconciler) Reconcile(ctx context.Context, req ctr
 			return ctrl.Result{}, nil
 		}
 		controllerLog.Error(err, "error getting grafana alertrulegroup cr")
-		return ctrl.Result{RequeueAfter: RequeueDelay}, err
+		return ctrl.Result{}, err
 	}
 
 	if group.GetDeletionTimestamp() != nil {
 		if controllerutil.ContainsFinalizer(group, grafanaFinalizer) {
 			// still need to clean up
 			err := r.finalize(ctx, group)
 			if err != nil {
-				return ctrl.Result{RequeueAfter: RequeueDelay}, fmt.Errorf("cleaning up alert rule group: %w", err)
+				return ctrl.Result{}, fmt.Errorf("cleaning up alert rule group: %w", err)
 			}
 			controllerutil.RemoveFinalizer(group, grafanaFinalizer)
 			if err := r.Update(ctx, group); err != nil {
 				r.Log.Error(err, "failed to remove finalizer")
-				return ctrl.Result{RequeueAfter: RequeueDelay}, err
+				return ctrl.Result{}, err
 			}
 		}
 		return ctrl.Result{}, nil
@@ -114,20 +114,19 @@ func (r *GrafanaAlertRuleGroupReconciler) Reconcile(ctx context.Context, req ctr
 		setNoMatchingInstance(&group.Status.Conditions, group.Generation, "ErrFetchingInstances", fmt.Sprintf("error occurred during fetching of instances: %s", err.Error()))
 		meta.RemoveStatusCondition(&group.Status.Conditions, conditionAlertGroupSynchronized)
 		r.Log.Error(err, "could not find matching instances")
-		return ctrl.Result{RequeueAfter: RequeueDelay}, err
+		return ctrl.Result{}, err
 	}
 
 	if len(instances) == 0 {
 		meta.RemoveStatusCondition(&group.Status.Conditions, conditionAlertGroupSynchronized)
 		setNoMatchingInstance(&group.Status.Conditions, group.Generation, "EmptyAPIReply", "Instances could not be fetched, reconciliation will be retried")
-		return ctrl.Result{}, nil
+		return ctrl.Result{}, fmt.Errorf("no instances found")
 	}
 
 	removeNoMatchingInstance(&group.Status.Conditions)
 	folderUID := r.GetFolderUID(ctx, group)
 	if folderUID == "" {
-		// error is already set in conditions
-		return ctrl.Result{}, nil
+		return ctrl.Result{}, fmt.Errorf("folder uid not found")
 	}
 
 	applyErrors := make(map[string]string)
@@ -375,5 +374,6 @@ func (r *GrafanaAlertRuleGroupReconciler) GetFolderUID(ctx context.Context, grou
 		setNoMatchingFolder(&group.Status.Conditions, group.Generation, "ErrFetchingFolder", fmt.Sprintf("Failed to fetch folder: %s", err.Error()))
 		return ""
 	}
+	removeNoMatchingFolder(&group.Status.Conditions)
 	return string(folder.UID)
 }