Expose execution error in logs

Signed-off-by: Adam Malcontenti-Wilson <[email protected]>
keikoproj · Dec 29, 2020 · 96d9a28 · 96d9a28
1 parent 59a5204
commit 96d9a28
Show file tree

Hide file tree

Showing 2 changed files with 28 additions and 18 deletions.
diff --git a/controllers/rollingupgrade_controller.go b/controllers/rollingupgrade_controller.go
@@ -561,8 +561,20 @@ func (r *RollingUpgradeReconciler) runRestack(ctx *context.Context, ruObj *upgra
 	return processedInstances, nil
 }
 
-func (r *RollingUpgradeReconciler) finishExecution(finalStatus string, nodesProcessed int, ctx *context.Context, ruObj *upgrademgrv1alpha1.RollingUpgrade) {
-	r.info(ruObj, "Marked object as", "finalStatus", finalStatus)
+func (r *RollingUpgradeReconciler) finishExecution(err error, nodesProcessed int, ctx *context.Context, ruObj *upgrademgrv1alpha1.RollingUpgrade) {
+	var level string
+	var finalStatus string
+
+	if err == nil {
+		finalStatus = upgrademgrv1alpha1.StatusComplete
+		level = EventLevelNormal
+		r.info(ruObj, "Marked object as", "finalStatus", finalStatus)
+	} else {
+		finalStatus = upgrademgrv1alpha1.StatusError
+		level = EventLevelWarning
+		r.error(ruObj, err, "Marked object as", "finalStatus", finalStatus)
+	}
+
 	endTime := time.Now()
 	ruObj.Status.EndTime = endTime.Format(time.RFC3339)
 	ruObj.Status.CurrentStatus = finalStatus
@@ -581,12 +593,7 @@ func (r *RollingUpgradeReconciler) finishExecution(finalStatus string, nodesProc
 		ruObj.Status.TotalProcessingTime = endTime.Sub(startTime).String()
 	}
 	// end event
-	var level string
-	if finalStatus == upgrademgrv1alpha1.StatusComplete {
-		level = EventLevelNormal
-	} else {
-		level = EventLevelWarning
-	}
+
 	r.createK8sV1Event(ruObj, EventReasonRUFinished, level, map[string]string{
 		"status":   finalStatus,
 		"asgName":  ruObj.Spec.AsgName,
@@ -638,26 +645,26 @@ func (r *RollingUpgradeReconciler) Process(ctx *context.Context,
 	r.CacheConfig.FlushCache("autoscaling")
 	err := r.populateAsg(ruObj)
 	if err != nil {
-		r.finishExecution(upgrademgrv1alpha1.StatusError, 0, ctx, ruObj)
+		r.finishExecution(err, 0, ctx, ruObj)
 		return
 	}
 
 	//TODO(shri): Ensure that no node is Unschedulable at this time.
 	err = r.populateNodeList(ruObj, r.generatedClient.CoreV1().Nodes())
 	if err != nil {
-		r.finishExecution(upgrademgrv1alpha1.StatusError, 0, ctx, ruObj)
+		r.finishExecution(err, 0, ctx, ruObj)
 		return
 	}
 
 	if err := r.populateLaunchTemplates(ruObj); err != nil {
-		r.finishExecution(upgrademgrv1alpha1.StatusError, 0, ctx, ruObj)
+		r.finishExecution(err, 0, ctx, ruObj)
 		return
 	}
 
 	asg, err := r.GetAutoScalingGroup(ruObj.NamespacedName())
 	if err != nil {
 		r.error(ruObj, err, "Unable to load ASG for rolling upgrade")
-		r.finishExecution(upgrademgrv1alpha1.StatusError, 0, ctx, ruObj)
+		r.finishExecution(err, 0, ctx, ruObj)
 		return
 	}
 
@@ -675,19 +682,20 @@ func (r *RollingUpgradeReconciler) Process(ctx *context.Context,
 	nodesProcessed, err := r.runRestack(ctx, ruObj)
 	if err != nil {
 		r.error(ruObj, err, "Failed to runRestack")
-		r.finishExecution(upgrademgrv1alpha1.StatusError, nodesProcessed, ctx, ruObj)
+		r.finishExecution(err, nodesProcessed, ctx, ruObj)
 		return
 	}
 
 	//Validation step: check if all the nodes have the latest launchconfig.
 	r.info(ruObj, "Validating the launch definition of nodes and ASG")
 	if err := r.validateNodesLaunchDefinition(ruObj); err != nil {
 		r.error(ruObj, err, "Launch definition validation failed")
-		r.finishExecution(upgrademgrv1alpha1.StatusError, nodesProcessed, ctx, ruObj)
+		r.finishExecution(err, nodesProcessed, ctx, ruObj)
 		return
 	}
 
-	r.finishExecution(upgrademgrv1alpha1.StatusComplete, nodesProcessed, ctx, ruObj)
+	// no error -> report success
+	r.finishExecution(nil, nodesProcessed, ctx, ruObj)
 }
 
 //Check if ec2Instances and the ASG have same launch config.

diff --git a/controllers/rollingupgrade_controller_test.go b/controllers/rollingupgrade_controller_test.go
@@ -65,8 +65,9 @@ func TestErrorStatusMarkJanitor(t *testing.T) {
 	}
 
 	ctx := context.TODO()
+	err = fmt.Errorf("execution error")
 	rcRollingUpgrade.inProcessASGs.Store(someAsg, "processing")
-	rcRollingUpgrade.finishExecution(upgrademgrv1alpha1.StatusError, 3, &ctx, instance)
+	rcRollingUpgrade.finishExecution(err, 3, &ctx, instance)
 	g.Expect(instance.ObjectMeta.Annotations[JanitorAnnotation]).To(gomega.Equal(ClearErrorFrequency))
 	_, exists := rcRollingUpgrade.inProcessASGs.Load(someAsg)
 	g.Expect(exists).To(gomega.BeFalse())
@@ -935,7 +936,7 @@ func TestFinishExecutionCompleted(t *testing.T) {
 	ctx := context.TODO()
 	mockNodesProcessed := 3
 
-	rcRollingUpgrade.finishExecution(upgrademgrv1alpha1.StatusComplete, mockNodesProcessed, &ctx, ruObj)
+	rcRollingUpgrade.finishExecution(nil, mockNodesProcessed, &ctx, ruObj)
 
 	g.Expect(ruObj.Status.CurrentStatus).To(gomega.Equal(upgrademgrv1alpha1.StatusComplete))
 	g.Expect(ruObj.Status.NodesProcessed).To(gomega.Equal(mockNodesProcessed))
@@ -970,7 +971,8 @@ func TestFinishExecutionError(t *testing.T) {
 	ctx := context.TODO()
 	mockNodesProcessed := 3
 
-	rcRollingUpgrade.finishExecution(upgrademgrv1alpha1.StatusError, mockNodesProcessed, &ctx, ruObj)
+	err = fmt.Errorf("execution error")
+	rcRollingUpgrade.finishExecution(err, mockNodesProcessed, &ctx, ruObj)
 
 	g.Expect(ruObj.Status.CurrentStatus).To(gomega.Equal(upgrademgrv1alpha1.StatusError))
 	g.Expect(ruObj.Status.NodesProcessed).To(gomega.Equal(mockNodesProcessed))