From 532413e7dd68e972aea12b2d1a1ee709c44e1c78 Mon Sep 17 00:00:00 2001 From: Quan Zhang Date: Thu, 30 Jun 2022 09:54:02 -0400 Subject: [PATCH] Publish Pipeline Results from successful TaskRuns in Failed PipelineRuns Publish Pipeline Results from successful TaskRuns in Failed PipelineRuns: #3749 Prior to this change a failed PipelineRun does not emit any PipelineResult even when some PipelineTasks referenced by the PipelineResult succeeded. This commit changes to publish Pipeline Results only from successful PipelineTaskRuns in the failed PipelineRun Such Results are valid because they are generated by successful TaskRuns. They can provide more context and successful artifacts to users of the partially successful PipelinRun. --- pkg/reconciler/pipelinerun/pipelinerun.go | 2 +- .../pipelinerun/pipelinerun_test.go | 155 ++++++++++++++++++ 2 files changed, 156 insertions(+), 1 deletion(-) diff --git a/pkg/reconciler/pipelinerun/pipelinerun.go b/pkg/reconciler/pipelinerun/pipelinerun.go index 450a6fdffcb..0196f32f9ca 100644 --- a/pkg/reconciler/pipelinerun/pipelinerun.go +++ b/pkg/reconciler/pipelinerun/pipelinerun.go @@ -602,7 +602,7 @@ func (c *Reconciler) reconcile(ctx context.Context, pr *v1beta1.PipelineRun, get } pr.Status.SkippedTasks = pipelineRunFacts.GetSkippedTasks() - if after.Status == corev1.ConditionTrue { + if after.Status == corev1.ConditionTrue || after.Status == corev1.ConditionFalse { pr.Status.PipelineResults = resources.ApplyTaskResultsToPipelineResults(pipelineSpec.Results, pipelineRunFacts.State.GetTaskRunsResults(), pipelineRunFacts.State.GetRunsResults()) } diff --git a/pkg/reconciler/pipelinerun/pipelinerun_test.go b/pkg/reconciler/pipelinerun/pipelinerun_test.go index a90a7e45d5a..e507bd9f30a 100644 --- a/pkg/reconciler/pipelinerun/pipelinerun_test.go +++ b/pkg/reconciler/pipelinerun/pipelinerun_test.go @@ -4868,6 +4868,161 @@ spec: } } +func TestReconcileWithPipelineResults_OnFailedPipelineRun(t *testing.T) { + testCases := []struct { + name string + embeddedVal string + }{ + { + name: "default embedded status", + embeddedVal: config.DefaultEmbeddedStatus, + }, + { + name: "full embedded status", + embeddedVal: config.FullEmbeddedStatus, + }, + { + name: "both embedded status", + embeddedVal: config.BothEmbeddedStatus, + }, + { + name: "minimal embedded status", + embeddedVal: config.MinimalEmbeddedStatus, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + runTestReconcileWithPipelineResultsOnFailedPipelineRun(t, tc.embeddedVal) + }) + } +} + +func runTestReconcileWithPipelineResultsOnFailedPipelineRun(t *testing.T, embeddedStatus string) { + names.TestingSeed() + ps := []*v1beta1.Pipeline{parse.MustParsePipeline(t, ` +metadata: + name: test-pipeline + namespace: foo +spec: + results: + - description: pipeline result + name: result + value: $(tasks.a-task.results.aResult) + tasks: + - name: a-task + taskRef: + name: a-task + - name: b-task + params: + - name: bParam + value: $(tasks.a-task.results.aResult) + taskRef: + name: b-task +`)} + trs := []*v1beta1.TaskRun{mustParseTaskRunWithObjectMeta(t, + taskRunObjectMeta("test-pipeline-run-different-service-accs-a-task", "foo", + "test-pipeline-run-different-service-accs", "test-pipeline", "a-task", true), + ` +spec: + resources: {} + serviceAccountName: test-sa + taskRef: + name: hello-world + timeout: 1h0m0s +status: + conditions: + - status: "True" + type: Succeeded + taskResults: + - name: aResult + value: aResultValue +`), mustParseTaskRunWithObjectMeta(t, + taskRunObjectMeta("test-pipeline-run-different-service-accs-b-task", "foo", + "test-pipeline-run-different-service-accs", "test-pipeline", "b-task", true), + ` +spec: + resources: {} + serviceAccountName: test-sa + taskRef: + name: hello-world + timeout: 1h0m0s +status: + conditions: + - status: "False" + type: Succeeded +`)} + prs := []*v1beta1.PipelineRun{parse.MustParsePipelineRun(t, ` +metadata: + name: test-pipeline-run-different-service-accs + namespace: foo +spec: + pipelineRef: + name: test-pipeline + serviceAccountName: test-sa-0 + timeouts: + pipeline: "0" +status: + conditions: + - message: Message + reason: Running + status: "Unknown" + type: Succeeded + startTime: "2021-12-31T00:00:00Z" +`)} + ts := []*v1beta1.Task{ + {ObjectMeta: baseObjectMeta("a-task", "foo")}, + parse.MustParseTask(t, ` +metadata: + name: b-task + namespace: foo +spec: + params: + - name: bParam + type: string +`), + } + wantPrs := []*v1beta1.PipelineRun{parse.MustParsePipelineRun(t, ` +metadata: + name: test-pipeline-run-different-service-accs + namespace: foo +spec: + pipelineRef: + name: test-pipeline + serviceAccountName: test-sa-0 + timeouts: + pipeline: "0" +status: + conditions: + - message: "Tasks Completed: 2 (Failed: 1, Cancelled 0), Skipped: 0" + reason: Failed + status: "False" + type: Succeeded + pipelineResults: + - name: result + value: aResultValue +`)} + + d := test.Data{ + PipelineRuns: prs, + Pipelines: ps, + Tasks: ts, + TaskRuns: trs, + ConfigMaps: []*corev1.ConfigMap{withEmbeddedStatus(newFeatureFlagsConfigMap(), embeddedStatus)}, + } + prt := newPipelineRunTest(d, t) + defer prt.Cancel() + + reconciledRun, _ := prt.reconcileRun("foo", "test-pipeline-run-different-service-accs", []string{}, false) + + if d := cmp.Diff(reconciledRun.Status.Conditions, wantPrs[0].Status.Conditions, ignoreResourceVersion, ignoreLastTransitionTime); d != "" { + t.Errorf("expected to see pipeline run marked as failed. Diff %s", diff.PrintWantGot(d)) + } + if d := cmp.Diff(reconciledRun.Status.PipelineResults, wantPrs[0].Status.PipelineResults, ignoreResourceVersion, ignoreLastTransitionTime); d != "" { + t.Errorf("expected to see pipeline run results created. Diff %s", diff.PrintWantGot(d)) + } +} + func Test_storePipelineSpec(t *testing.T) { labels := map[string]string{"lbl": "value"} annotations := map[string]string{"io.annotation": "value"}