fix(SRVKP-14519): report snippet on new taskrun failures

pkg/kubeinteraction/status/task_status.go

@@ -20,6 +20,36 @@ var reasonMessageReplacementRegexp = regexp.MustCompile(`\(image: .*`)
 
 const maxErrorSnippetCharacterLimit = 65535 // This is the maximum size allowed by Github check run logs and may apply to all other providers
 
+// reasonsWithoutPodLogs are the failure reasons where no container ever ran the
+// user code, the snippet is then taken from the task condition message.
+var reasonsWithoutPodLogs = map[string]struct{}{
+	tektonv1.TaskRunReasonFailedValidation.String():           {},
+	tektonv1.TaskRunReasonTaskFailedValidation.String():       {},
+	tektonv1.TaskRunReasonFailedResolution.String():           {},
+	tektonv1.TaskRunReasonInvalidParamValue.String():          {},
+	tektonv1.TaskRunReasonResourceVerificationFailed.String(): {},
+	tektonv1.TaskRunReasonCancelled.String():                  {},
+	tektonv1.TaskRunReasonTimedOut.String():                   {},
+	tektonv1.TaskRunReasonImagePullFailed.String():            {},
+	tektonv1.TaskRunReasonCreateContainerConfigError.String(): {},
+	tektonv1.TaskRunReasonPodCreationFailed.String():          {},
+	tektonv1.TaskRunReasonInitContainerFailed.String():        {},
+	tektonv1.TaskRunReasonInitContainerOOM.String():           {},
+}
+
+// reasonsWithPodLogs are the failure reasons where the pod has run and we can
+// get a log snippet out of the failed containers.
+var reasonsWithPodLogs = map[string]struct{}{
+	tektonv1.TaskRunReasonFailed.String():                       {},
+	tektonv1.TaskRunReasonStepFailed.String():                   {},
+	tektonv1.TaskRunReasonStepOOM.String():                      {},
+	tektonv1.TaskRunReasonSidecarFailed.String():                {},
+	tektonv1.TaskRunReasonSidecarOOM.String():                   {},
+	tektonv1.TaskRunReasonStopSidecarFailed.String():            {},
+	tektonv1.TaskRunReasonPodEvicted.String():                   {},
+	tektonv1.TaskRunReasonResultLargerThanAllowedLimit.String(): {},
+}
+
 func waitingMessage(steps []tektonv1.StepState) string {
 	for _, step := range steps {
 		if step.Waiting == nil || step.Waiting.Message == "" {
@@ -115,10 +145,16 @@ func CollectFailedTasksLogSnippet(ctx context.Context, cs *params.Run, kinteract
 			ti.LogSnippet = ti.Message
 		}
 		// don't check for pod logs into those
-		if ti.Reason == "TaskRunValidationFailed" || ti.Reason == tektonv1.TaskRunReasonCancelled.String() || ti.Reason == tektonv1.TaskRunReasonTimedOut.String() || ti.Reason == tektonv1.TaskRunReasonImagePullFailed.String() || ti.Reason == tektonv1.TaskRunReasonCreateContainerConfigError.String() || ti.Reason == tektonv1.TaskRunReasonPodCreationFailed.String() {
+		if _, ok := reasonsWithoutPodLogs[ti.Reason]; ok {
 			failureReasons[task.PipelineTaskName] = ti
 			continue
-		} else if ti.Reason != tektonv1.PipelineRunReasonFailed.String() {
+		}
+		if _, ok := reasonsWithPodLogs[ti.Reason]; !ok {
+			// a failure we don't know about, tekton may have added a new
+			// reason, log it so we don't silently drop the snippet.
+			if task.Status.Conditions[0].IsFalse() && ti.Reason != tektonv1.TaskRunReasonFailureIgnored.String() {
+				cs.Clients.Log.Warnf("unknown taskrun failure reason %q on task %q, skipping log snippet", ti.Reason, task.PipelineTaskName)
+			}
 			continue
 		}
 

pkg/kubeinteraction/status/task_status_test.go

@@ -36,22 +36,98 @@ func TestCollectFailedTasksLogSnippet(t *testing.T) {
 	tests := []struct {
 		name, displayName string
 		message, status   string
+		conditionStatus   corev1.ConditionStatus
 		wantFailure       int
 		podOutput         string
+		wantSnippet       string
+		wantWarning       string
 	}{
 		{
-			name:        "no failures",
-			status:      "Success",
-			message:     "never gonna make you fail",
-			wantFailure: 0,
+			name:            "no failures",
+			status:          "Success",
+			conditionStatus: corev1.ConditionTrue,
+			message:         "never gonna make you fail",
+			wantFailure:     0,
 		},
 		{
-			name:        "failure pod output",
-			status:      "Failed",
-			message:     "i am gonna to make you fail",
-			podOutput:   "hahah i am the devil of the pod",
-			wantFailure: 1,
-			displayName: "A task",
+			name:            "failure pod output",
+			status:          "Failed",
+			conditionStatus: corev1.ConditionFalse,
+			message:         "i am gonna to make you fail",
+			podOutput:       "hahah i am the devil of the pod",
+			wantFailure:     1,
+			displayName:     "A task",
+		},
+		{
+			name:            "step failed",
+			status:          tektonv1.TaskRunReasonStepFailed.String(),
+			conditionStatus: corev1.ConditionFalse,
+			message:         `"step-lint" exited with code 2: Error`,
+			podOutput:       "the step went wrong",
+			wantFailure:     1,
+		},
+		{
+			name:            "step out of memory",
+			status:          tektonv1.TaskRunReasonStepOOM.String(),
+			conditionStatus: corev1.ConditionFalse,
+			message:         `"step-build" exited because of OOMKilled`,
+			podOutput:       "out of memory",
+			wantFailure:     1,
+		},
+		{
+			name:            "sidecar failed",
+			status:          tektonv1.TaskRunReasonSidecarFailed.String(),
+			conditionStatus: corev1.ConditionFalse,
+			message:         "sidecar crashed",
+			podOutput:       "sidecar logs",
+			wantFailure:     1,
+		},
+		{
+			name:            "sidecar could not be stopped",
+			status:          tektonv1.TaskRunReasonStopSidecarFailed.String(),
+			conditionStatus: corev1.ConditionFalse,
+			message:         "sidecar could not be stopped",
+			podOutput:       "stop sidecar logs",
+			wantFailure:     1,
+		},
+		{
+			name:            "result larger than the allowed limit",
+			status:          tektonv1.TaskRunReasonResultLargerThanAllowedLimit.String(),
+			conditionStatus: corev1.ConditionFalse,
+			message:         "result is way too large",
+			podOutput:       "task result logs",
+			wantFailure:     1,
+		},
+		{
+			name:            "pod evicted",
+			status:          tektonv1.TaskRunReasonPodEvicted.String(),
+			conditionStatus: corev1.ConditionFalse,
+			message:         "pod was evicted",
+			podOutput:       "evicted logs",
+			wantFailure:     1,
+		},
+		{
+			name:            "init container failed falls back to the message",
+			status:          tektonv1.TaskRunReasonInitContainerFailed.String(),
+			conditionStatus: corev1.ConditionFalse,
+			message:         "init container prepare failed",
+			wantFailure:     1,
+			wantSnippet:     "init container prepare failed",
+		},
+		{
+			name:            "ignored failure is skipped",
+			status:          tektonv1.TaskRunReasonFailureIgnored.String(),
+			conditionStatus: corev1.ConditionFalse,
+			message:         "we don't care about this one",
+			wantFailure:     0,
+		},
+		{
+			name:            "unknown failure reason is skipped and reported",
+			status:          "ANewTektonFailureReason",
+			conditionStatus: corev1.ConditionFalse,
+			message:         "something new happened",
+			wantFailure:     0,
+			wantWarning:     "unknown taskrun failure reason",
 		},
 	}
 	for _, tt := range tests {
@@ -93,7 +169,7 @@ func TestCollectFailedTasksLogSnippet(t *testing.T) {
 						map[string]string{}, taskStatus, knativeduckv1.Conditions{
 							{
 								Type:    knativeapi.ConditionSucceeded,
-								Status:  corev1.ConditionTrue,
+								Status:  tt.conditionStatus,
 								Reason:  tt.status,
 								Message: tt.message,
 							},
@@ -103,8 +179,10 @@ func TestCollectFailedTasksLogSnippet(t *testing.T) {
 			}
 			ctx, _ := rtesting.SetupFakeContext(t)
 			stdata, _ := testclient.SeedTestData(t, ctx, tdata)
+			observer, logCatcher := zapobserver.New(zap.WarnLevel)
 			cs := &params.Run{Clients: paramclients.Clients{
 				Tekton: stdata.Pipeline,
+				Log:    zap.New(observer).Sugar(),
 			}}
 			intf := &kubernetestint.KinterfaceTest{}
 			if tt.podOutput != "" {
@@ -117,9 +195,17 @@ func TestCollectFailedTasksLogSnippet(t *testing.T) {
 			if tt.podOutput != "" {
 				assert.Equal(t, tt.podOutput, got["task1"].LogSnippet)
 			}
+			if tt.wantSnippet != "" {
+				assert.Equal(t, tt.wantSnippet, got["task1"].LogSnippet)
+			}
 			if tt.displayName != "" {
 				assert.Equal(t, tt.displayName, got["task1"].DisplayName)
 			}
+			if tt.wantWarning != "" {
+				assert.Assert(t, logCatcher.FilterMessageSnippet(tt.wantWarning).Len() > 0, "expected a warning matching %q", tt.wantWarning)
+			} else {
+				assert.Equal(t, 0, logCatcher.Len(), "no warning was expected")
+			}
 		})
 	}
 }
@@ -322,12 +408,42 @@ func TestCollectFailedTasksLogSnippetWaitingReasons(t *testing.T) {
 			// TaskRunValidationFailed/PodCreationFailed happen before any
 			// step/pod is created, so waitingMessage() has nothing to
 			// inspect and we must fall back to the condition message.
+			// The reasons are spelled out on purpose here so the mapping of
+			// the tekton constants to their string value is pinned down.
 			name:        "no steps falls back to condition message",
 			reason:      "TaskRunValidationFailed",
 			condMessage: "task validation failed: unknown field foo",
 			steps:       nil,
 			wantSnippet: "task validation failed: unknown field foo",
 		},
+		{
+			name:        "task validation failure falls back to condition message",
+			reason:      "TaskValidationFailed",
+			condMessage: "task validation failed: missing step name",
+			steps:       nil,
+			wantSnippet: "task validation failed: missing step name",
+		},
+		{
+			name:        "resolution failure falls back to condition message",
+			reason:      "TaskRunResolutionFailed",
+			condMessage: "error getting task: cannot resolve task from git",
+			steps:       nil,
+			wantSnippet: "error getting task: cannot resolve task from git",
+		},
+		{
+			name:        "invalid param value falls back to condition message",
+			reason:      "InvalidParamValue",
+			condMessage: "param foo is not allowed",
+			steps:       nil,
+			wantSnippet: "param foo is not allowed",
+		},
+		{
+			name:        "resource verification failure falls back to condition message",
+			reason:      "ResourceVerificationFailed",
+			condMessage: "resource verification failed",
+			steps:       nil,
+			wantSnippet: "resource verification failed",
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {

test/gitea_error_snippets_test.go

@@ -35,7 +35,7 @@ func TestGiteaErrorSnippet(t *testing.T) {
 	_, f := tgitea.TestPR(t, topts)
 	defer f()
 
-	topts.Regexp = regexp.MustCompile(`Hey man i just wanna to say i am not such a failure, i am useful in my failure`)
+	topts.Regexp = regexp.MustCompile(`(?s)<h4>Failure snippet:</h4>.*Hey man i just wanna to say i am not such a failure, i am useful in my failure`)
 	tgitea.WaitForPullRequestCommentMatch(t, topts)
 }
 
@@ -80,9 +80,14 @@ func TestGiteaErrorSnippetCustomLines(t *testing.T) {
 	if idx := strings.Index(body, marker); idx != -1 {
 		body = body[idx:]
 	}
+	// The taskrun failure reason depends on the tekton version (Failed,
+	// StepFailed, ...), normalize it so the golden file stays stable.
+	body = taskStatusReasonRe.ReplaceAllString(body, `has the status <b>"FAILURE_REASON"</b>`)
 	golden.Assert(t, body, strings.ReplaceAll(fmt.Sprintf("%s.golden", t.Name()), "/", "-"))
 }
 
+var taskStatusReasonRe = regexp.MustCompile(`has the status <b>"[^"]*"</b>`)
+
 func TestGiteaErrorSnippetWithSecret(t *testing.T) {
 	var err error
 	ctx := context.Background()

test/testdata/TestGiteaErrorSnippetCustomLines.golden

@@ -1,5 +1,5 @@
 <h4>Failure snippet:</h4>
-task <b>task</b> has the status <b>"Failed"</b>:
+task <b>task</b> has the status <b>"FAILURE_REASON"</b>:
 <pre>3
 4
 5