From 81eb706df133d9b1195f8c7e13bcd9ab92de2e65 Mon Sep 17 00:00:00 2001 From: Chmouel Boudjnah Date: Thu, 17 Sep 2026 14:49:39 +0200 Subject: [PATCH] fix(SRVKP-14519): report snippet on new taskrun failures Tekton now describes a failed task with more precise reasons than the old generic "Failed", a step that exits with an error is reported as "StepFailed" and there are new reasons for out of memory kills, sidecar failures, evicted pods and failed init containers. Pipelines-as-Code only recognised the old name, so every step failure was quietly ignored: no failure snippet at the bottom of the comment or check run, and no inline error annotations on the changed files in GitHub. The failure reasons are now listed explicitly, split between the ones where the container logs can be fetched and the ones that fail before anything runs and only have a message to show, which also covers the validation, resolution and parameter errors that were only partially handled before. Failures that the pipeline author chose to ignore are still left out. An unknown reason is now logged as a warning instead of being dropped silently, so the next addition on the Tekton side is easy to spot. Signed-off-by: Chmouel Boudjnah --- pkg/kubeinteraction/status/task_status.go | 40 ++++- .../status/task_status_test.go | 138 ++++++++++++++++-- test/gitea_error_snippets_test.go | 7 +- .../TestGiteaErrorSnippetCustomLines.golden | 2 +- 4 files changed, 172 insertions(+), 15 deletions(-) diff --git a/pkg/kubeinteraction/status/task_status.go b/pkg/kubeinteraction/status/task_status.go index 8be7d2e626..a7c6783a63 100644 --- a/pkg/kubeinteraction/status/task_status.go +++ b/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 } diff --git a/pkg/kubeinteraction/status/task_status_test.go b/pkg/kubeinteraction/status/task_status_test.go index c79f200394..b3e327999a 100644 --- a/pkg/kubeinteraction/status/task_status_test.go +++ b/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 := ¶ms.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) { diff --git a/test/gitea_error_snippets_test.go b/test/gitea_error_snippets_test.go index 33c45b114d..f67afe6f3e 100644 --- a/test/gitea_error_snippets_test.go +++ b/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)

Failure snippet:

.*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 "FAILURE_REASON"`) golden.Assert(t, body, strings.ReplaceAll(fmt.Sprintf("%s.golden", t.Name()), "/", "-")) } +var taskStatusReasonRe = regexp.MustCompile(`has the status "[^"]*"`) + func TestGiteaErrorSnippetWithSecret(t *testing.T) { var err error ctx := context.Background() diff --git a/test/testdata/TestGiteaErrorSnippetCustomLines.golden b/test/testdata/TestGiteaErrorSnippetCustomLines.golden index b718ff57f8..2583b83bed 100644 --- a/test/testdata/TestGiteaErrorSnippetCustomLines.golden +++ b/test/testdata/TestGiteaErrorSnippetCustomLines.golden @@ -1,5 +1,5 @@

Failure snippet:

-task task has the status "Failed": +task task has the status "FAILURE_REASON": 
3
 4
 5