feat(operator): emit K8s events with detailed messages for failed eva…

…luations (#477)

Signed-off-by: Florian Bacher <florian.bacher@dynatrace.com>

operator/api/v1alpha1/common/phases.go

@@ -79,6 +79,8 @@ var (
 	PhaseAppPreEvaluation       = KeptnPhaseType{LongName: "App Pre-Deployment Evaluations", ShortName: "AppPreDeployEvaluations"}
 	PhaseAppPostEvaluation      = KeptnPhaseType{LongName: "App Post-Deployment Evaluations", ShortName: "AppPostDeployEvaluations"}
 	PhaseAppDeployment          = KeptnPhaseType{LongName: "App Deployment", ShortName: "AppDeploy"}
+	PhaseReconcileEvaluation    = KeptnPhaseType{LongName: "Reconcile Evaluation", ShortName: "ReconcileEvaluation"}
+	PhaseCreateEvaluation       = KeptnPhaseType{LongName: "Create Evaluation", ShortName: "Create Evaluation"}
 	PhaseCompleted              = KeptnPhaseType{LongName: "Completed", ShortName: "Completed"}
 	PhaseCancelled              = KeptnPhaseType{LongName: "Cancelled", ShortName: "Cancelled"}
 )

operator/controllers/common/evaluationhandler.go

@@ -41,11 +41,6 @@ func (r EvaluationHandler) ReconcileEvaluations(ctx context.Context, phaseCtx co
 		return nil, apicommon.StatusSummary{}, err
 	}
 
-	phase := apicommon.KeptnPhaseType{
-		ShortName: "ReconcileEvaluations",
-		LongName:  "Reconcile Evaluations",
-	}
-
 	var evaluations []string
 	var statuses []klcv1alpha1.EvaluationStatus
 
@@ -75,7 +70,7 @@ func (r EvaluationHandler) ReconcileEvaluations(ctx context.Context, phaseCtx co
 		evaluationExists := false
 
 		if oldstatus != evaluationStatus.Status {
-			RecordEvent(r.Recorder, phase, "Normal", reconcileObject, "EvaluationStatusChanged", fmt.Sprintf("evaluation status changed from %s to %s", oldstatus, evaluationStatus.Status), piWrapper.GetVersion())
+			RecordEvent(r.Recorder, apicommon.PhaseReconcileEvaluation, "Normal", reconcileObject, "EvaluationStatusChanged", fmt.Sprintf("evaluation status changed from %s to %s", oldstatus, evaluationStatus.Status), piWrapper.GetVersion())
 		}
 
 		// Check if evaluation has already succeeded or failed
@@ -119,9 +114,10 @@ func (r EvaluationHandler) ReconcileEvaluations(ctx context.Context, phaseCtx co
 				if evaluationStatus.Status.IsSucceeded() {
 					spanEvaluationTrace.AddEvent(evaluation.Name + " has finished")
 					spanEvaluationTrace.SetStatus(codes.Ok, "Finished")
+					RecordEvent(r.Recorder, apicommon.PhaseReconcileEvaluation, "Normal", evaluation, "Succeeded", "evaluation succeeded", piWrapper.GetVersion())
 				} else {
 					spanEvaluationTrace.AddEvent(evaluation.Name + " has failed")
-					r.setEvaluationFailureEvents(evaluation, spanEvaluationTrace)
+					r.emitEvaluationFailureEvents(evaluation, spanEvaluationTrace, piWrapper)
 					spanEvaluationTrace.SetStatus(codes.Error, "Failed")
 				}
 				spanEvaluationTrace.End()
@@ -139,7 +135,7 @@ func (r EvaluationHandler) ReconcileEvaluations(ctx context.Context, phaseCtx co
 		summary = apicommon.UpdateStatusSummary(ns.Status, summary)
 	}
 	if apicommon.GetOverallState(summary) != apicommon.StateSucceeded {
-		RecordEvent(r.Recorder, phase, "Warning", reconcileObject, "NotFinished", "has not finished", piWrapper.GetVersion())
+		RecordEvent(r.Recorder, apicommon.PhaseReconcileEvaluation, "Warning", reconcileObject, "NotFinished", "has not finished", piWrapper.GetVersion())
 	}
 	return newStatus, summary, nil
 }
@@ -150,10 +146,7 @@ func (r EvaluationHandler) CreateKeptnEvaluation(ctx context.Context, namespace
 		return "", err
 	}
 
-	phase := apicommon.KeptnPhaseType{
-		ShortName: "KeptnEvaluationCreate",
-		LongName:  "Keptn Evaluation Create",
-	}
+	phase := apicommon.PhaseCreateEvaluation
 
 	newEvaluation := piWrapper.GenerateEvaluation(evaluationCreateAttributes.EvaluationDefinition, evaluationCreateAttributes.CheckType)
 	err = controllerutil.SetControllerReference(reconcileObject, &newEvaluation, r.Scheme)
@@ -171,10 +164,14 @@ func (r EvaluationHandler) CreateKeptnEvaluation(ctx context.Context, namespace
 	return newEvaluation.Name, nil
 }
 
-func (r EvaluationHandler) setEvaluationFailureEvents(evaluation *klcv1alpha1.KeptnEvaluation, spanTrace trace.Span) {
+func (r EvaluationHandler) emitEvaluationFailureEvents(evaluation *klcv1alpha1.KeptnEvaluation, spanTrace trace.Span, piWrapper *interfaces.PhaseItemWrapper) {
+	k8sEventMessage := "evaluation failed"
 	for k, v := range evaluation.Status.EvaluationStatus {
 		if v.Status == apicommon.StateFailed {
-			spanTrace.AddEvent(fmt.Sprintf("evaluation of '%s' failed with value: '%s' and reason: '%s'", k, v.Value, v.Message), trace.WithTimestamp(time.Now().UTC()))
+			msg := fmt.Sprintf("evaluation of '%s' failed with value: '%s' and reason: '%s'", k, v.Value, v.Message)
+			spanTrace.AddEvent(msg, trace.WithTimestamp(time.Now().UTC()))
+			k8sEventMessage = fmt.Sprintf("%s\n%s", k8sEventMessage, msg)
 		}
 	}
+	RecordEvent(r.Recorder, apicommon.PhaseReconcileEvaluation, "Warning", evaluation, "Failed", k8sEventMessage, piWrapper.GetVersion())
 }

operator/controllers/common/evaluationhandler_test.go

@@ -2,6 +2,7 @@ package common
 
 import (
 	"context"
+	"fmt"
 	"strings"
 	"testing"
 
@@ -30,6 +31,7 @@ func TestEvaluationHandler(t *testing.T) {
 		wantErr         error
 		getSpanCalls    int
 		unbindSpanCalls int
+		events          []string
 	}{
 		{
 			name:            "cannot unwrap object",
@@ -150,6 +152,13 @@ func TestEvaluationHandler(t *testing.T) {
 				},
 				Status: v1alpha1.KeptnEvaluationStatus{
 					OverallStatus: apicommon.StateFailed,
+					EvaluationStatus: map[string]v1alpha1.EvaluationStatusItem{
+						"my-target": {
+							Value:   "1",
+							Status:  apicommon.StateFailed,
+							Message: "failed",
+						},
+					},
 				},
 			},
 			createAttr: EvaluationCreateAttributes{
@@ -168,6 +177,9 @@ func TestEvaluationHandler(t *testing.T) {
 			wantErr:         nil,
 			getSpanCalls:    1,
 			unbindSpanCalls: 1,
+			events: []string{
+				"evaluation of 'my-target' failed with value: '1' and reason: 'failed'",
+			},
 		},
 		{
 			name: "succeeded evaluation",
@@ -216,6 +228,9 @@ func TestEvaluationHandler(t *testing.T) {
 			wantErr:         nil,
 			getSpanCalls:    1,
 			unbindSpanCalls: 1,
+			events: []string{
+				"ReconcileEvaluationSucceeded",
+			},
 		},
 	}
 
@@ -231,10 +246,11 @@ func TestEvaluationHandler(t *testing.T) {
 					return nil
 				},
 			}
+			fakeRecorder := record.NewFakeRecorder(100)
 			handler := EvaluationHandler{
 				SpanHandler: &spanHandlerMock,
 				Log:         ctrl.Log.WithName("controller"),
-				Recorder:    record.NewFakeRecorder(100),
+				Recorder:    fakeRecorder,
 				Client:      fake.NewClientBuilder().WithObjects(&tt.evalObj).Build(),
 				Tracer:      trace.NewNoopTracerProvider().Tracer("tracer"),
 				Scheme:      scheme.Scheme,
@@ -253,6 +269,16 @@ func TestEvaluationHandler(t *testing.T) {
 			require.Equal(t, tt.wantErr, err)
 			require.Equal(t, tt.getSpanCalls, len(spanHandlerMock.GetSpanCalls()))
 			require.Equal(t, tt.unbindSpanCalls, len(spanHandlerMock.UnbindSpanCalls()))
+
+			if tt.events != nil {
+				for _, e := range tt.events {
+					event := <-fakeRecorder.Events
+					require.Equal(t, strings.Contains(event, tt.object.GetName()), true, "wrong appversion")
+					require.Equal(t, strings.Contains(event, tt.object.GetNamespace()), true, "wrong namespace")
+					require.Equal(t, strings.Contains(event, e), true, fmt.Sprintf("no %s found in %s", e, event))
+				}
+
+			}
 		})
 	}
 }

operator/controllers/keptnevaluation/controller.go

@@ -177,8 +177,6 @@ func (r *KeptnEvaluationReconciler) Reconcile(ctx context.Context, req ctrl.Requ
 }
 
 func (r *KeptnEvaluationReconciler) updateFinishedEvaluationMetrics(ctx context.Context, evaluation *klcv1alpha1.KeptnEvaluation, span trace.Span) error {
-	r.recordEvent("Normal", evaluation, string(evaluation.Status.OverallStatus), "the evaluation has "+string(evaluation.Status.OverallStatus))
-
 	evaluation.SetEndTime()
 
 	err := r.Client.Status().Update(ctx, evaluation)

operator/test/component/workloadinstancecontroller_test.go

@@ -2,6 +2,7 @@ package component
 
 import (
 	"context"
+	"strings"
 	"time"
 
 	klcv1alpha1 "github.com/keptn/lifecycle-toolkit/operator/api/v1alpha1"
@@ -17,6 +18,7 @@ import (
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
+	"sigs.k8s.io/controller-runtime/pkg/client"
 )
 
 func getPodTemplateSpec() corev1.PodTemplateSpec {
@@ -387,6 +389,24 @@ var _ = Describe("KeptnWorkloadInstanceController", Ordered, func() {
 					g.Expect(wi.Status.PostDeploymentEvaluationStatus).To(BeEquivalentTo(apicommon.StateCancelled))
 					g.Expect(wi.Status.Status).To(BeEquivalentTo(apicommon.StateFailed))
 				}, "30s").Should(Succeed())
+
+				By("Ensuring that a K8s Event containing the reason for the failed evaluation has been sent")
+
+				Eventually(func(g Gomega) {
+					eventList := &corev1.EventList{}
+					err := k8sClient.List(ctx, eventList, client.InNamespace(namespace))
+					g.Expect(err).To(BeNil())
+
+					foundEvent := &corev1.Event{}
+
+					for _, e := range eventList.Items {
+						if strings.Contains(e.Name, wi.GetName()) && e.Reason == "AppPreDeployEvaluationsFailed" {
+							foundEvent = &e
+							break
+						}
+					}
+					g.Expect(foundEvent).NotTo(BeNil())
+				}, "30s").Should(Succeed())
 			})
 			AfterEach(func() {
 				// Remember to clean up the cluster after each test