Merge pull request #117 from razo7/log-err-on-failure

Improve Logs from Updating Conditions

controllers/fenceagentsremediation_controller.go

@@ -124,15 +124,15 @@ func (r *FenceAgentsRemediationReconciler) Reconcile(ctx context.Context, req ct
 	}
 	if !valid {
 		r.Log.Error(err, "Didn't find a node matching the CR's name", "CR's Name", req.Name)
-		err := utils.UpdateConditions(utils.RemediationFinishedNodeNotFound, far, r.Log)
+		utils.UpdateConditions(utils.RemediationFinishedNodeNotFound, far, r.Log)
 		return emptyResult, err
 	}
 
 	// Check NHC timeout annotation
 	if isTimedOutByNHC(far) {
 		r.Log.Info("FAR remediation was stopped by Node Healthcheck Operator")
 		r.Executor.Remove(far.GetUID())
-		err := utils.UpdateConditions(utils.RemediationInterruptedByNHC, far, r.Log)
+		utils.UpdateConditions(utils.RemediationInterruptedByNHC, far, r.Log)
 		return emptyResult, err
 	}
 
@@ -144,9 +144,7 @@ func (r *FenceAgentsRemediationReconciler) Reconcile(ctx context.Context, req ct
 		}
 		r.Log.Info("Finalizer was added", "CR Name", req.Name)
 
-		if err := utils.UpdateConditions(utils.RemediationStarted, far, r.Log); err != nil {
-			return emptyResult, err
-		}
+		utils.UpdateConditions(utils.RemediationStarted, far, r.Log)
 		return requeueImmediately, nil
 	} else if controllerutil.ContainsFinalizer(far, v1alpha1.FARFinalizer) && !far.ObjectMeta.DeletionTimestamp.IsZero() {
 		// Delete CR only when a finalizer and DeletionTimestamp are set
@@ -212,9 +210,7 @@ func (r *FenceAgentsRemediationReconciler) Reconcile(ctx context.Context, req ct
 			r.Log.Error(err, "Manual workload deletion has failed", "CR's Name", req.Name)
 			return emptyResult, err
 		}
-		if err := utils.UpdateConditions(utils.RemediationFinishedSuccessfully, far, r.Log); err != nil {
-			return emptyResult, err
-		}
+		utils.UpdateConditions(utils.RemediationFinishedSuccessfully, far, r.Log)
 
 		r.Executor.Remove(far.GetUID())
 		r.Log.Info("FenceAgentsRemediation CR has completed to remediate the node", "Node Name", req.Name)

pkg/cli/cliexecuter.go

@@ -246,10 +246,6 @@ func (e *Executer) updateStatus(ctx context.Context, far *v1alpha1.FenceAgentsRe
 		reason = utils.FenceAgentFailed
 	}
 
-	err = utils.UpdateConditions(reason, far, e.log)
-	if err != nil {
-		e.log.Error(err, "failed to update conditions", "FAR uid", far.UID)
-		return err
-	}
+	utils.UpdateConditions(reason, far, e.log)
 	return e.Status().Update(ctx, far)
 }

pkg/utils/conditions.go

@@ -47,12 +47,14 @@ const (
 
 // updateConditions updates the status conditions of a FenceAgentsRemediation object based on the provided ConditionsChangeReason.
 // return an error if an unknown ConditionsChangeReason is provided
-func UpdateConditions(reason ConditionsChangeReason, far *v1alpha1.FenceAgentsRemediation, log logr.Logger) error {
+func UpdateConditions(reason ConditionsChangeReason, far *v1alpha1.FenceAgentsRemediation, log logr.Logger) {
 
 	var (
 		processingConditionStatus, fenceAgentActionSucceededConditionStatus, succeededConditionStatus metav1.ConditionStatus
 		conditionMessage                                                                              string
 	)
+	conditionUpdateMessage := "Couldn't update FAR Status Conditions"
+	unknownError := fmt.Errorf("unknown ConditionsChangeReason")
 	currentConditions := &far.Status.Conditions
 	conditionHasBeenChanged := false
 
@@ -77,9 +79,9 @@ func UpdateConditions(reason ConditionsChangeReason, far *v1alpha1.FenceAgentsRe
 		case FenceAgentTimedOut:
 			conditionMessage = FenceAgentTimedOutConditionMessage
 		default:
-			err := fmt.Errorf("unknown ConditionsChangeReason:%s", reason)
-			log.Error(err, "couldn't update FAR Status Conditions")
-			return err
+			// couldn't be reached...
+			log.Error(unknownError, conditionUpdateMessage, "CR name", far.Name, "Reason", reason)
+			return
 		}
 	case RemediationStarted:
 		processingConditionStatus = metav1.ConditionTrue
@@ -94,9 +96,8 @@ func UpdateConditions(reason ConditionsChangeReason, far *v1alpha1.FenceAgentsRe
 		succeededConditionStatus = metav1.ConditionTrue
 		conditionMessage = RemediationFinishedSuccessfullyConditionMessage
 	default:
-		err := fmt.Errorf("unknown ConditionsChangeReason:%s", reason)
-		log.Error(err, "couldn't update FAR Status Conditions")
-		return err
+		log.Error(unknownError, conditionUpdateMessage, "CR name", far.Name, "Reason", reason)
+		return
 	}
 
 	// if the requested Status.Conditions.Processing is different then the current one, then update Status.Conditions.Processing value
@@ -136,7 +137,7 @@ func UpdateConditions(reason ConditionsChangeReason, far *v1alpha1.FenceAgentsRe
 		now := metav1.Now()
 		far.Status.LastUpdateTime = &now
 	}
-	log.Info("Updating Status Condition", "processingConditionStatus", processingConditionStatus, "fenceAgentActionSucceededConditionStatus", fenceAgentActionSucceededConditionStatus, "succeededConditionStatus", succeededConditionStatus, "reason", string(reason), "LastUpdateTime", far.Status.LastUpdateTime)
+	log.Info("Updating Status Condition", "processingConditionStatus", processingConditionStatus, "fenceAgentActionSucceededConditionStatus", fenceAgentActionSucceededConditionStatus, "succeededConditionStatus", succeededConditionStatus, "reason", string(reason), "LastUpdateTime", far.Status.LastUpdateTime.Time)
 
-	return nil
+	return
 }