Skip to content

Commit afc4db7

Browse files
cugykwpritidesai
authored andcommitted
feat: support to extract failed task results
If the task fails or succeeds, as long as the task result is initialized, it can be successfully parsed and can be referenced by the final task. This commit enables the failed task to produce the task results, and the final task can reference it. Please see detailed description in issue #5749 Co-authored-by: yang kewei <2524563635@qq.com> Signed-off-by: pritidesai <pdesai@us.ibm.com>
1 parent fb38679 commit afc4db7

7 files changed

+232
-5
lines changed

docs/pipelines.md

+2-2
Original file line numberDiff line numberDiff line change
@@ -1317,8 +1317,8 @@ spec:
13171317
`finally` tasks after all non-`finally` tasks are done.
13181318

13191319
The controller resolves task results before executing the `finally` task `discover-git-commit`. If the task
1320-
`clone-app-repo` failed or skipped with [when expression](#guard-task-execution-using-when-expressions) resulting in
1321-
uninitialized task result `commit`, the `finally` Task `discover-git-commit` will be included in the list of
1320+
`clone-app-repo` failed before initializing `commit` or skipped with [when expression](#guard-task-execution-using-when-expressions)
1321+
resulting in uninitialized task result `commit`, the `finally` Task `discover-git-commit` will be included in the list of
13221322
`skippedTasks` and continues executing rest of the `finally` tasks. The pipeline exits with `completion` instead of
13231323
`success` if a `finally` task is added to the list of `skippedTasks`.
13241324

pkg/pod/status.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -208,7 +208,7 @@ func setTaskRunStatusBasedOnStepStatus(ctx context.Context, logger *zap.SugaredL
208208
}
209209

210210
taskResults, filteredResults := filterResults(results, specResults)
211-
if tr.IsSuccessful() {
211+
if tr.IsDone() {
212212
trs.TaskRunResults = append(trs.TaskRunResults, taskResults...)
213213
}
214214
msg, err = createMessageFromResults(filteredResults)

pkg/pod/status_test.go

+34
Original file line numberDiff line numberDiff line change
@@ -795,6 +795,40 @@ func TestMakeTaskRunStatus(t *testing.T) {
795795
CompletionTime: &metav1.Time{Time: time.Now()},
796796
},
797797
},
798+
}, {
799+
desc: "the failed task show task results",
800+
podStatus: corev1.PodStatus{
801+
Phase: corev1.PodFailed,
802+
ContainerStatuses: []corev1.ContainerStatus{{
803+
Name: "step-task-result",
804+
State: corev1.ContainerState{
805+
Terminated: &corev1.ContainerStateTerminated{
806+
Message: `[{"key":"resultName","value":"resultValue", "type":1}]`,
807+
},
808+
},
809+
}},
810+
},
811+
want: v1beta1.TaskRunStatus{
812+
Status: statusFailure(v1beta1.TaskRunReasonFailed.String(), "build failed for unspecified reasons."),
813+
TaskRunStatusFields: v1beta1.TaskRunStatusFields{
814+
Steps: []v1beta1.StepState{{
815+
ContainerState: corev1.ContainerState{
816+
Terminated: &corev1.ContainerStateTerminated{
817+
Message: `[{"key":"resultName","value":"resultValue","type":1}]`,
818+
},
819+
},
820+
Name: "task-result",
821+
ContainerName: "step-task-result",
822+
}},
823+
Sidecars: []v1beta1.SidecarState{},
824+
CompletionTime: &metav1.Time{Time: time.Now()},
825+
TaskRunResults: []v1beta1.TaskRunResult{{
826+
Name: "resultName",
827+
Type: v1beta1.ResultsTypeString,
828+
Value: *v1beta1.NewStructuredValues("resultValue"),
829+
}},
830+
},
831+
},
798832
}, {
799833
desc: "taskrun status set to failed if task fails",
800834
podStatus: corev1.PodStatus{

pkg/reconciler/pipelinerun/pipelinerun_test.go

+26
Original file line numberDiff line numberDiff line change
@@ -5991,13 +5991,22 @@ spec:
59915991
operator: in
59925992
values:
59935993
- aResultValue
5994+
- name: final-task-7
5995+
params:
5996+
- name: finalParam
5997+
value: $(tasks.dag-task-3.results.aResult)
5998+
taskRef:
5999+
name: final-task
59946000
tasks:
59956001
- name: dag-task-1
59966002
taskRef:
59976003
name: dag-task
59986004
- name: dag-task-2
59996005
taskRef:
60006006
name: dag-task
6007+
- name: dag-task-3
6008+
taskRef:
6009+
name: dag-task
60016010
`)}
60026011

60036012
prs := []*v1beta1.PipelineRun{parse.MustParseV1beta1PipelineRun(t, `
@@ -6058,6 +6067,23 @@ status:
60586067
- lastTransitionTime: null
60596068
status: "False"
60606069
type: Succeeded
6070+
`),
6071+
mustParseTaskRunWithObjectMeta(t,
6072+
taskRunObjectMeta("test-pipeline-run-final-task-results-dag-task-3-xxyyy", "foo",
6073+
"test-pipeline-run-final-task-results", "test-pipeline", "dag-task-3", false),
6074+
`
6075+
spec:
6076+
serviceAccountName: test-sa
6077+
taskRef:
6078+
name: hello-world
6079+
status:
6080+
conditions:
6081+
- lastTransitionTime: null
6082+
status: "False"
6083+
type: Succeeded
6084+
taskResults:
6085+
- name: aResult
6086+
value: aResultValue
60616087
`),
60626088
}
60636089

pkg/reconciler/pipelinerun/resources/resultrefresolution.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -123,8 +123,8 @@ func resolveResultRef(pipelineState PipelineRunState, resultRef *v1beta1.ResultR
123123
if referencedPipelineTask == nil {
124124
return nil, resultRef.PipelineTask, fmt.Errorf("could not find task %q referenced by result", resultRef.PipelineTask)
125125
}
126-
if !referencedPipelineTask.isSuccessful() {
127-
return nil, resultRef.PipelineTask, fmt.Errorf("task %q referenced by result was not successful", referencedPipelineTask.PipelineTask.Name)
126+
if !referencedPipelineTask.isSuccessful() && !referencedPipelineTask.isFailure() {
127+
return nil, resultRef.PipelineTask, fmt.Errorf("task %q referenced by result was not finished", referencedPipelineTask.PipelineTask.Name)
128128
}
129129

130130
var runName, runValue, taskRunName string

pkg/reconciler/pipelinerun/resources/resultrefresolution_test.go

+47
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,10 @@ var (
3636
Type: apis.ConditionSucceeded,
3737
Status: corev1.ConditionTrue,
3838
}
39+
failedCondition = apis.Condition{
40+
Type: apis.ConditionSucceeded,
41+
Status: corev1.ConditionFalse,
42+
}
3943
)
4044

4145
var pipelineRunState = PipelineRunState{{
@@ -180,6 +184,35 @@ var pipelineRunState = PipelineRunState{{
180184
Value: *v1beta1.NewStructuredValues("$(tasks.dTask.results.dResult[3])"),
181185
}},
182186
},
187+
}, {
188+
TaskRunName: "eTaskRun",
189+
TaskRun: &v1beta1.TaskRun{
190+
ObjectMeta: metav1.ObjectMeta{Name: "eTaskRun"},
191+
Status: v1beta1.TaskRunStatus{
192+
Status: duckv1.Status{
193+
Conditions: duckv1.Conditions{failedCondition},
194+
},
195+
TaskRunStatusFields: v1beta1.TaskRunStatusFields{
196+
TaskRunResults: []v1beta1.TaskRunResult{{
197+
Name: "eResult",
198+
Value: *v1beta1.NewStructuredValues("eResultValue"),
199+
}},
200+
},
201+
},
202+
},
203+
PipelineTask: &v1beta1.PipelineTask{
204+
Name: "eTask",
205+
TaskRef: &v1beta1.TaskRef{Name: "eTask"},
206+
},
207+
}, {
208+
PipelineTask: &v1beta1.PipelineTask{
209+
Name: "fTask",
210+
TaskRef: &v1beta1.TaskRef{Name: "fTask"},
211+
Params: v1beta1.Params{{
212+
Name: "fParam",
213+
Value: *v1beta1.NewStructuredValues("$(tasks.eTask.results.eResult)"),
214+
}},
215+
},
183216
}}
184217

185218
func TestResolveResultRefs(t *testing.T) {
@@ -285,6 +318,20 @@ func TestResolveResultRefs(t *testing.T) {
285318
FromRun: "aRun",
286319
}},
287320
wantErr: false,
321+
}, {
322+
name: "Test successful result references resolution - params - failed taskrun",
323+
pipelineRunState: pipelineRunState,
324+
targets: PipelineRunState{
325+
pipelineRunState[10],
326+
},
327+
want: ResolvedResultRefs{{
328+
Value: *v1beta1.NewStructuredValues("eResultValue"),
329+
ResultReference: v1beta1.ResultRef{
330+
PipelineTask: "eTask",
331+
Result: "eResult",
332+
},
333+
FromTaskRun: "eTaskRun",
334+
}},
288335
}} {
289336
t.Run(tt.name, func(t *testing.T) {
290337
got, pt, err := ResolveResultRefs(tt.pipelineRunState, tt.targets)
+120
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,120 @@
1+
//go:build e2e
2+
// +build e2e
3+
4+
/*
5+
Copyright 2023 The Tekton Authors
6+
7+
Licensed under the Apache License, Version 2.0 (the "License");
8+
you may not use this file except in compliance with the License.
9+
You may obtain a copy of the License at
10+
11+
http://www.apache.org/licenses/LICENSE-2.0
12+
13+
Unless required by applicable law or agreed to in writing, software
14+
distributed under the License is distributed on an "AS IS" BASIS,
15+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
16+
See the License for the specific language governing permissions and
17+
limitations under the License.
18+
*/
19+
20+
package test
21+
22+
import (
23+
"context"
24+
"fmt"
25+
"testing"
26+
27+
"github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1"
28+
"github.com/tektoncd/pipeline/test/parse"
29+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
30+
knativetest "knative.dev/pkg/test"
31+
"knative.dev/pkg/test/helpers"
32+
)
33+
34+
func TestTaskResultsFromFailedTasks(t *testing.T) {
35+
ctx := context.Background()
36+
ctx, cancel := context.WithCancel(ctx)
37+
defer cancel()
38+
c, namespace := setup(ctx, t)
39+
knativetest.CleanupOnInterrupt(func() { tearDown(ctx, t, c, namespace) }, t.Logf)
40+
defer tearDown(ctx, t, c, namespace)
41+
42+
pipelineRun := parse.MustParseV1beta1PipelineRun(t, fmt.Sprintf(`
43+
metadata:
44+
name: %s
45+
spec:
46+
pipelineSpec:
47+
tasks:
48+
- name: task1
49+
taskSpec:
50+
results:
51+
- name: result1
52+
- name: result2
53+
steps:
54+
- name: failing-step
55+
image: busybox
56+
script: 'echo -n 123 | tee $(results.result1.path); exit 1; echo -n 456 | tee $(results.result2.path)'
57+
finally:
58+
- name: finaltask1
59+
params:
60+
- name: param1
61+
value: $(tasks.task1.results.result1)
62+
taskSpec:
63+
params:
64+
- name: param1
65+
steps:
66+
- image: busybox
67+
script: 'exit 0'
68+
- name: finaltask2
69+
params:
70+
- name: param1
71+
value: $(tasks.task1.results.result2)
72+
taskSpec:
73+
params:
74+
- name: param1
75+
steps:
76+
- image: busybox
77+
script: exit 0`, helpers.ObjectNameForTest(t)))
78+
79+
if _, err := c.V1beta1PipelineRunClient.Create(ctx, pipelineRun, metav1.CreateOptions{}); err != nil {
80+
t.Fatalf("Failed to create PipelineRun `%s`: %s", pipelineRun.Name, err)
81+
}
82+
83+
t.Logf("Waiting for PipelineRun in namespace %s to fail", namespace)
84+
if err := WaitForPipelineRunState(ctx, c, pipelineRun.Name, timeout, FailedWithReason(v1beta1.PipelineRunReasonFailed.String(), pipelineRun.Name), "InvalidTaskResultReference", v1beta1Version); err != nil {
85+
t.Errorf("Error waiting for PipelineRun to fail: %s", err)
86+
}
87+
88+
taskrunList, err := c.V1beta1TaskRunClient.List(ctx, metav1.ListOptions{LabelSelector: "tekton.dev/pipelineRun=" + pipelineRun.Name})
89+
if err != nil {
90+
t.Fatalf("Error listing TaskRuns for PipelineRun %s: %s", pipelineRun.Name, err)
91+
}
92+
93+
if len(taskrunList.Items) != 2 {
94+
t.Fatalf("The pipelineRun \"%s\" should have exactly 2 taskRuns, one for the task \"task1\""+
95+
"and one more for the final task \"finaltask1\" instead it has \"%d\" taskRuns", pipelineRun.Name, len(taskrunList.Items))
96+
}
97+
98+
for _, taskrunItem := range taskrunList.Items {
99+
switch n := taskrunItem.Labels["tekton.dev/pipelineTask"]; n {
100+
case "task1":
101+
if !isFailed(t, "", taskrunItem.Status.Conditions) {
102+
t.Fatalf("task1 should have been a failure")
103+
}
104+
if len(taskrunItem.Status.TaskRunResults) != 1 {
105+
t.Fatalf("task1 should have produced a result even with the failing step")
106+
}
107+
for _, r := range taskrunItem.Status.TaskRunResults {
108+
if r.Name == "result1" && r.Value.StringVal != "123" {
109+
t.Fatalf("task1 should have initialized a result \"result1\" to \"123\"")
110+
}
111+
}
112+
case "finaltask1":
113+
if !isSuccessful(t, "", taskrunItem.Status.Conditions) {
114+
t.Fatalf("finaltask1 should have been successful")
115+
}
116+
default:
117+
t.Fatalf("TaskRuns were not found for both final and dag tasks")
118+
}
119+
}
120+
}

0 commit comments

Comments
 (0)