feat: support to produce results from a failed task

docs/pipelines.md

@@ -1257,8 +1257,8 @@ spec:
 `finally` tasks after all non-`finally` tasks are done.
 
 The controller resolves task results before executing the `finally` task `discover-git-commit`. If the task
-`clone-app-repo` failed or skipped with [when expression](#guard-task-execution-using-when-expressions) resulting in
-uninitialized task result `commit`, the `finally` Task `discover-git-commit` will be included in the list of
+`clone-app-repo` failed before initializing `commit` or skipped with [when expression](#guard-task-execution-using-when-expressions)
+resulting in uninitialized task result `commit`, the `finally` Task `discover-git-commit` will be included in the list of
 `skippedTasks` and continues executing rest of the `finally` tasks. The pipeline exits with `completion` instead of
 `success` if a `finally` task is added to the list of `skippedTasks`.
 

pkg/pod/status.go

@@ -183,7 +183,7 @@ func setTaskRunStatusBasedOnStepStatus(ctx context.Context, logger *zap.SugaredL
 
 		// populate task run CRD with results from sidecar logs
 		taskResults, _ := filterResults(sidecarLogResults, specResults)
-		if tr.IsSuccessful() {
+		if tr.IsDone() {
 			trs.TaskRunResults = append(trs.TaskRunResults, taskResults...)
 		}
 	}
@@ -209,7 +209,7 @@ func setTaskRunStatusBasedOnStepStatus(ctx context.Context, logger *zap.SugaredL
 				}
 
 				taskResults, filteredResults := filterResults(results, specResults)
-				if tr.IsSuccessful() {
+				if tr.IsDone() {
 					trs.TaskRunResults = append(trs.TaskRunResults, taskResults...)
 				}
 				msg, err = createMessageFromResults(filteredResults)

pkg/pod/status_test.go

@@ -795,6 +795,40 @@ func TestMakeTaskRunStatus(t *testing.T) {
 				CompletionTime: &metav1.Time{Time: time.Now()},
 			},
 		},
+	}, {
+		desc: "the failed task show task results",
+		podStatus: corev1.PodStatus{
+			Phase: corev1.PodFailed,
+			ContainerStatuses: []corev1.ContainerStatus{{
+				Name: "step-task-result",
+				State: corev1.ContainerState{
+					Terminated: &corev1.ContainerStateTerminated{
+						Message: `[{"key":"resultName","value":"resultValue", "type":1}]`,
+					},
+				},
+			}},
+		},
+		want: v1beta1.TaskRunStatus{
+			Status: statusFailure(v1beta1.TaskRunReasonFailed.String(), "build failed for unspecified reasons."),
+			TaskRunStatusFields: v1beta1.TaskRunStatusFields{
+				Steps: []v1beta1.StepState{{
+					ContainerState: corev1.ContainerState{
+						Terminated: &corev1.ContainerStateTerminated{
+							Message: `[{"key":"resultName","value":"resultValue","type":1}]`,
+						},
+					},
+					Name:          "task-result",
+					ContainerName: "step-task-result",
+				}},
+				Sidecars:       []v1beta1.SidecarState{},
+				CompletionTime: &metav1.Time{Time: time.Now()},
+				TaskRunResults: []v1beta1.TaskRunResult{{
+					Name:  "resultName",
+					Type:  v1beta1.ResultsTypeString,
+					Value: *v1beta1.NewStructuredValues("resultValue"),
+				}},
+			},
+		},
 	}, {
 		desc: "taskrun status set to failed if task fails",
 		podStatus: corev1.PodStatus{

pkg/reconciler/pipelinerun/pipelinerun_test.go

@@ -6228,13 +6228,22 @@ spec:
       operator: in
       values:
       - aResultValue
+  - name: final-task-7
+    params:
+    - name: finalParam
+      value: $(tasks.dag-task-3.results.aResult)
+    taskRef:
+      name: final-task
   tasks:
   - name: dag-task-1
     taskRef:
       name: dag-task
   - name: dag-task-2
     taskRef:
       name: dag-task
+  - name: dag-task-3
+    taskRef:
+      name: dag-task
 `)}
 
 	prs := []*v1beta1.PipelineRun{parse.MustParseV1beta1PipelineRun(t, `
@@ -6295,6 +6304,23 @@ status:
   - lastTransitionTime: null
     status: "False"
     type: Succeeded
+`),
+		mustParseTaskRunWithObjectMeta(t,
+			taskRunObjectMeta("test-pipeline-run-final-task-results-dag-task-3-xxyyy", "foo",
+				"test-pipeline-run-final-task-results", "test-pipeline", "dag-task-3", false),
+			`
+spec:
+  serviceAccountName: test-sa
+  taskRef:
+    name: hello-world
+status:
+  conditions:
+  - lastTransitionTime: null
+    status: "False"
+    type: Succeeded
+  taskResults:
+  - name: aResult
+    value: aResultValue
 `),
 	}
 

pkg/reconciler/pipelinerun/resources/resultrefresolution.go

@@ -122,8 +122,8 @@ func resolveResultRef(pipelineState PipelineRunState, resultRef *v1beta1.ResultR
 	if referencedPipelineTask == nil {
 		return nil, resultRef.PipelineTask, fmt.Errorf("could not find task %q referenced by result", resultRef.PipelineTask)
 	}
-	if !referencedPipelineTask.isSuccessful() {
-		return nil, resultRef.PipelineTask, fmt.Errorf("task %q referenced by result was not successful", referencedPipelineTask.PipelineTask.Name)
+	if !referencedPipelineTask.isSuccessful() && !referencedPipelineTask.isFailure() {
+		return nil, resultRef.PipelineTask, fmt.Errorf("task %q referenced by result was not finished", referencedPipelineTask.PipelineTask.Name)
 	}
 
 	var runName, runValue, taskRunName string

pkg/reconciler/pipelinerun/resources/resultrefresolution_test.go

@@ -36,6 +36,10 @@ var (
 		Type:   apis.ConditionSucceeded,
 		Status: corev1.ConditionTrue,
 	}
+	failedCondition = apis.Condition{
+		Type:   apis.ConditionSucceeded,
+		Status: corev1.ConditionFalse,
+	}
 )
 
 var pipelineRunState = PipelineRunState{{
@@ -180,6 +184,35 @@ var pipelineRunState = PipelineRunState{{
 			Value: *v1beta1.NewStructuredValues("$(tasks.dTask.results.dResult[3])"),
 		}},
 	},
+}, {
+	TaskRunName: "eTaskRun",
+	TaskRun: &v1beta1.TaskRun{
+		ObjectMeta: metav1.ObjectMeta{Name: "eTaskRun"},
+		Status: v1beta1.TaskRunStatus{
+			Status: duckv1.Status{
+				Conditions: duckv1.Conditions{failedCondition},
+			},
+			TaskRunStatusFields: v1beta1.TaskRunStatusFields{
+				TaskRunResults: []v1beta1.TaskRunResult{{
+					Name:  "eResult",
+					Value: *v1beta1.NewStructuredValues("eResultValue"),
+				}},
+			},
+		},
+	},
+	PipelineTask: &v1beta1.PipelineTask{
+		Name:    "eTask",
+		TaskRef: &v1beta1.TaskRef{Name: "eTask"},
+	},
+}, {
+	PipelineTask: &v1beta1.PipelineTask{
+		Name:    "fTask",
+		TaskRef: &v1beta1.TaskRef{Name: "fTask"},
+		Params: v1beta1.Params{{
+			Name:  "fParam",
+			Value: *v1beta1.NewStructuredValues("$(tasks.eTask.results.eResult)"),
+		}},
+	},
 }}
 
 func TestResolveResultRefs(t *testing.T) {
@@ -285,6 +318,20 @@ func TestResolveResultRefs(t *testing.T) {
 			FromRun: "aRun",
 		}},
 		wantErr: false,
+	}, {
+		name:             "Test successful result references resolution - params - failed taskrun",
+		pipelineRunState: pipelineRunState,
+		targets: PipelineRunState{
+			pipelineRunState[10],
+		},
+		want: ResolvedResultRefs{{
+			Value: *v1beta1.NewStructuredValues("eResultValue"),
+			ResultReference: v1beta1.ResultRef{
+				PipelineTask: "eTask",
+				Result:       "eResult",
+			},
+			FromTaskRun: "eTaskRun",
+		}},
 	}} {
 		t.Run(tt.name, func(t *testing.T) {
 			got, pt, err := ResolveResultRefs(tt.pipelineRunState, tt.targets)

test/task_results_from_failed_tasks_test.go

@@ -0,0 +1,120 @@
+//go:build e2e
+// +build e2e
+
+/*
+Copyright 2023 The Tekton Authors
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package test
+
+import (
+	"context"
+	"fmt"
+	"testing"
+
+	"github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1"
+	"github.com/tektoncd/pipeline/test/parse"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	knativetest "knative.dev/pkg/test"
+	"knative.dev/pkg/test/helpers"
+)
+
+func TestTaskResultsFromFailedTasks(t *testing.T) {
+	ctx := context.Background()
+	ctx, cancel := context.WithCancel(ctx)
+	defer cancel()
+	c, namespace := setup(ctx, t)
+	knativetest.CleanupOnInterrupt(func() { tearDown(ctx, t, c, namespace) }, t.Logf)
+	defer tearDown(ctx, t, c, namespace)
+
+	pipelineRun := parse.MustParseV1beta1PipelineRun(t, fmt.Sprintf(`
+metadata:
+  name: %s
+spec:
+  pipelineSpec:
+    tasks:
+    - name: task1
+      taskSpec:
+        results:
+        - name: result1
+        - name: result2
+        steps:
+        - name: failing-step
+          image: busybox
+          script: 'echo -n 123 | tee $(results.result1.path); exit 1; echo -n 456 | tee $(results.result2.path)'
+    finally:
+    - name: finaltask1
+      params:
+      - name: param1
+        value: $(tasks.task1.results.result1)
+      taskSpec:
+        params:
+        - name: param1
+        steps:
+        - image: busybox
+          script: 'exit 0'
+    - name: finaltask2
+      params:
+      - name: param1
+        value: $(tasks.task1.results.result2)
+      taskSpec:
+        params:
+        - name: param1
+        steps:
+        - image: busybox
+          script: exit 0`, helpers.ObjectNameForTest(t)))
+
+	if _, err := c.V1beta1PipelineRunClient.Create(ctx, pipelineRun, metav1.CreateOptions{}); err != nil {
+		t.Fatalf("Failed to create PipelineRun `%s`: %s", pipelineRun.Name, err)
+	}
+
+	t.Logf("Waiting for PipelineRun in namespace %s to fail", namespace)
+	if err := WaitForPipelineRunState(ctx, c, pipelineRun.Name, timeout, FailedWithReason(v1beta1.PipelineRunReasonFailed.String(), pipelineRun.Name), "InvalidTaskResultReference", v1beta1Version); err != nil {
+		t.Errorf("Error waiting for PipelineRun to fail: %s", err)
+	}
+
+	taskrunList, err := c.V1beta1TaskRunClient.List(ctx, metav1.ListOptions{LabelSelector: "tekton.dev/pipelineRun=" + pipelineRun.Name})
+	if err != nil {
+		t.Fatalf("Error listing TaskRuns for PipelineRun %s: %s", pipelineRun.Name, err)
+	}
+
+	if len(taskrunList.Items) != 2 {
+		t.Fatalf("The pipelineRun \"%s\" should have exactly 2 taskRuns, one for the task \"task1\""+
+			"and one more for the final task \"finaltask1\" instead it has \"%d\" taskRuns", pipelineRun.Name, len(taskrunList.Items))
+	}
+
+	for _, taskrunItem := range taskrunList.Items {
+		switch n := taskrunItem.Labels["tekton.dev/pipelineTask"]; n {
+		case "task1":
+			if !isFailed(t, "", taskrunItem.Status.Conditions) {
+				t.Fatalf("task1 should have been a failure")
+			}
+			if len(taskrunItem.Status.TaskRunResults) != 1 {
+				t.Fatalf("task1 should have produced a result even with the failing step")
+			}
+			for _, r := range taskrunItem.Status.TaskRunResults {
+				if r.Name == "result1" && r.Value.StringVal != "123" {
+					t.Fatalf("task1 should have initialized a result \"result1\" to \"123\"")
+				}
+			}
+		case "finaltask1":
+			if !isSuccessful(t, "", taskrunItem.Status.Conditions) {
+				t.Fatalf("finaltask1 should have been successful")
+			}
+		default:
+			t.Fatalf("TaskRuns were not found for both final and dag tasks")
+		}
+	}
+}