From cb23f4571f673f0c24e0fdb255c97c29b43c0072 Mon Sep 17 00:00:00 2001 From: Andrew Bayer Date: Wed, 13 Apr 2022 09:28:43 -0400 Subject: [PATCH] Switch ApplyTaskResultsToPipelineResults to not use status maps This comes out of discussions on #4739 - with the new minimal embedded status changes which will be introduced in that PR, we can see that we're currently using the output of `pipelineRunFacts.State.GetTaskRunsStatus(pr)` and `pipelineRunFacts.State.GetRunsStatus(pr)` for two separate purposes: * To set `pr.Status.TaskRuns` and `pr.Status.Runs` with the full embedded status * To pass to `resources.ApplyTaskResultsToPipelineResults` for populating results It's understandable why `ApplyTaskResultsToPipelineResults` is using the maps from `pr.Status.[TaskRuns|Runs]`, since those maps do contain everything needed for propagating results up from the tasks to the pipeline run, but if you look at the current implementation, you can see that it's shuffling the maps around into a different form that's more suited for what it's doing than the original form. So this PR reworks `ApplyTaskResultsToPipelineResults` to instead take maps in the form the current implementation uses internally, with new functions on `PipelineRunState` to get these new maps without needing to use the `pr.Status.[TaskRuns|Runs]` form as an intermediary. This makes the pre-minimal-embedded-status implementation cleaner, and is particularly helpful in that regard once we do have minimal embedded status in place. Signed-off-by: Andrew Bayer --- pkg/reconciler/pipelinerun/pipelinerun.go | 3 +- pkg/reconciler/pipelinerun/resources/apply.go | 61 +-- .../pipelinerun/resources/apply_test.go | 365 ++++-------------- .../pipelinerun/resources/pipelinerunstate.go | 36 +- .../resources/pipelinerunstate_test.go | 196 ++++++++++ 5 files changed, 320 insertions(+), 341 deletions(-) diff --git a/pkg/reconciler/pipelinerun/pipelinerun.go b/pkg/reconciler/pipelinerun/pipelinerun.go index de38b28f27a..4e140caaae0 100644 --- a/pkg/reconciler/pipelinerun/pipelinerun.go +++ b/pkg/reconciler/pipelinerun/pipelinerun.go @@ -577,7 +577,8 @@ func (c *Reconciler) reconcile(ctx context.Context, pr *v1beta1.PipelineRun, get pr.Status.Runs = pipelineRunFacts.State.GetRunsStatus(pr) pr.Status.SkippedTasks = pipelineRunFacts.GetSkippedTasks() if after.Status == corev1.ConditionTrue { - pr.Status.PipelineResults = resources.ApplyTaskResultsToPipelineResults(pipelineSpec.Results, pr.Status.TaskRuns, pr.Status.Runs) + pr.Status.PipelineResults = resources.ApplyTaskResultsToPipelineResults(pipelineSpec.Results, + pipelineRunFacts.State.GetTaskRunsResults(), pipelineRunFacts.State.GetRunsResults()) } logger.Infof("PipelineRun %s status is being set to %s", pr.Name, after) diff --git a/pkg/reconciler/pipelinerun/resources/apply.go b/pkg/reconciler/pipelinerun/resources/apply.go index 3de4016a1e8..4187b915a84 100644 --- a/pkg/reconciler/pipelinerun/resources/apply.go +++ b/pkg/reconciler/pipelinerun/resources/apply.go @@ -21,10 +21,10 @@ import ( "strconv" "strings" + "github.com/tektoncd/pipeline/pkg/apis/run/v1alpha1" + "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1" "github.com/tektoncd/pipeline/pkg/substitution" - corev1 "k8s.io/api/core/v1" - "knative.dev/pkg/apis" ) // ApplyParameters applies the params from a PipelineRun.Params to a PipelineSpec. @@ -183,17 +183,8 @@ func replaceParamValues(params []v1beta1.Param, stringReplacements map[string]st // results are invalid. func ApplyTaskResultsToPipelineResults( results []v1beta1.PipelineResult, - taskRunStatuses map[string]*v1beta1.PipelineRunTaskRunStatus, - runStatuses map[string]*v1beta1.PipelineRunRunStatus) []v1beta1.PipelineRunResult { - - taskStatuses := map[string]*v1beta1.PipelineRunTaskRunStatus{} - for _, trStatus := range taskRunStatuses { - taskStatuses[trStatus.PipelineTaskName] = trStatus - } - customTaskStatuses := map[string]*v1beta1.PipelineRunRunStatus{} - for _, runStatus := range runStatuses { - customTaskStatuses[runStatus.PipelineTaskName] = runStatus - } + taskRunResults map[string][]v1beta1.TaskRunResult, + customTaskResults map[string][]v1alpha1.RunResult) []v1beta1.PipelineRunResult { var runResults []v1beta1.PipelineRunResult stringReplacements := map[string]string{} @@ -207,9 +198,9 @@ func ApplyTaskResultsToPipelineResults( variableParts := strings.Split(variable, ".") if len(variableParts) == 4 && variableParts[0] == "tasks" && variableParts[2] == "results" { taskName, resultName := variableParts[1], variableParts[3] - if resultValue := taskResultValue(taskName, resultName, taskStatuses); resultValue != nil { + if resultValue := taskResultValue(taskName, resultName, taskRunResults); resultValue != nil { stringReplacements[variable] = *resultValue - } else if resultValue := runResultValue(taskName, resultName, customTaskStatuses); resultValue != nil { + } else if resultValue := runResultValue(taskName, resultName, customTaskResults); resultValue != nil { stringReplacements[variable] = *resultValue } else { validPipelineResult = false @@ -234,21 +225,11 @@ func ApplyTaskResultsToPipelineResults( return runResults } -// taskResultValue checks if a TaskRun result exists for a given pipeline task and result name. -// A nil pointer is returned if the variable is invalid for any reason. -func taskResultValue(taskName string, resultName string, taskStatuses map[string]*v1beta1.PipelineRunTaskRunStatus) *string { - - status, taskExists := taskStatuses[taskName] - if !taskExists || status.Status == nil { - return nil - } - - cond := status.Status.GetCondition(apis.ConditionSucceeded) - if cond == nil || cond.Status != corev1.ConditionTrue { - return nil - } - - for _, trResult := range status.Status.TaskRunResults { +// taskResultValue returns the result value for a given pipeline task name and result name in a map of TaskRunResults for +// pipeline task names. It returns nil if either the pipeline task name isn't present in the map, or if there is no +// result with the result name in the pipeline task name's slice of results. +func taskResultValue(taskName string, resultName string, taskResults map[string][]v1beta1.TaskRunResult) *string { + for _, trResult := range taskResults[taskName] { if trResult.Name == resultName { return &trResult.Value } @@ -256,21 +237,11 @@ func taskResultValue(taskName string, resultName string, taskStatuses map[string return nil } -// runResultValue checks if a Run result exists for a given pipeline task and result name. -// A nil pointer is returned if the variable is invalid for any reason. -func runResultValue(taskName string, resultName string, runStatuses map[string]*v1beta1.PipelineRunRunStatus) *string { - - status, runExists := runStatuses[taskName] - if !runExists || status.Status == nil { - return nil - } - - cond := status.Status.GetCondition(apis.ConditionSucceeded) - if cond == nil || cond.Status != corev1.ConditionTrue { - return nil - } - - for _, runResult := range status.Status.Results { +// runResultValue returns the result value for a given pipeline task name and result name in a map of RunResults for +// pipeline task names. It returns nil if either the pipeline task name isn't present in the map, or if there is no +// result with the result name in the pipeline task name's slice of results. +func runResultValue(taskName string, resultName string, runResults map[string][]v1alpha1.RunResult) *string { + for _, runResult := range runResults[taskName] { if runResult.Name == resultName { return &runResult.Value } diff --git a/pkg/reconciler/pipelinerun/resources/apply_test.go b/pkg/reconciler/pipelinerun/resources/apply_test.go index c45e211301e..8dd15896080 100644 --- a/pkg/reconciler/pipelinerun/resources/apply_test.go +++ b/pkg/reconciler/pipelinerun/resources/apply_test.go @@ -24,14 +24,9 @@ import ( "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1" "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1" resourcev1alpha1 "github.com/tektoncd/pipeline/pkg/apis/resource/v1alpha1" - runv1alpha1 "github.com/tektoncd/pipeline/pkg/apis/run/v1alpha1" "github.com/tektoncd/pipeline/test/diff" - corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/selection" - "knative.dev/pkg/apis" - duckv1 "knative.dev/pkg/apis/duck/v1" - duckv1beta1 "knative.dev/pkg/apis/duck/v1beta1" ) func TestApplyParameters(t *testing.T) { @@ -787,30 +782,17 @@ func TestApplyTaskResultsToPipelineResults(t *testing.T) { for _, tc := range []struct { description string results []v1beta1.PipelineResult - statuses map[string]*v1beta1.PipelineRunTaskRunStatus - runStatuses map[string]*v1beta1.PipelineRunRunStatus + taskResults map[string][]v1beta1.TaskRunResult + runResults map[string][]v1alpha1.RunResult expected []v1beta1.PipelineRunResult }{{ description: "no-pipeline-results-no-returned-results", results: []v1beta1.PipelineResult{}, - statuses: map[string]*v1beta1.PipelineRunTaskRunStatus{ - "task1": { - PipelineTaskName: "pt1", - Status: &v1beta1.TaskRunStatus{ - Status: duckv1beta1.Status{ - Conditions: []apis.Condition{{ - Type: apis.ConditionSucceeded, - Status: corev1.ConditionTrue, - }}, - }, - TaskRunStatusFields: v1beta1.TaskRunStatusFields{ - TaskRunResults: []v1beta1.TaskRunResult{{ - Name: "foo", - Value: "bar", - }}, - }, - }, - }, + taskResults: map[string][]v1beta1.TaskRunResult{ + "pt1": {{ + Name: "foo", + Value: "bar", + }}, }, expected: nil, }, { @@ -819,24 +801,11 @@ func TestApplyTaskResultsToPipelineResults(t *testing.T) { Name: "foo", Value: "$(tasks.pt1_results.foo)", }}, - statuses: map[string]*v1beta1.PipelineRunTaskRunStatus{ - "task1": { - PipelineTaskName: "pt1", - Status: &v1beta1.TaskRunStatus{ - Status: duckv1beta1.Status{ - Conditions: []apis.Condition{{ - Type: apis.ConditionSucceeded, - Status: corev1.ConditionTrue, - }}, - }, - TaskRunStatusFields: v1beta1.TaskRunStatusFields{ - TaskRunResults: []v1beta1.TaskRunResult{{ - Name: "foo", - Value: "bar", - }}, - }, - }, - }, + taskResults: map[string][]v1beta1.TaskRunResult{ + "pt1": {{ + Name: "foo", + Value: "bar", + }}, }, expected: nil, }, { @@ -845,21 +814,8 @@ func TestApplyTaskResultsToPipelineResults(t *testing.T) { Name: "foo", Value: "$(tasks.pt1.results.foo)", }}, - statuses: map[string]*v1beta1.PipelineRunTaskRunStatus{ - "task1": { - PipelineTaskName: "pt1", - Status: &v1beta1.TaskRunStatus{ - Status: duckv1beta1.Status{ - Conditions: []apis.Condition{{ - Type: apis.ConditionSucceeded, - Status: corev1.ConditionTrue, - }}, - }, - TaskRunStatusFields: v1beta1.TaskRunStatusFields{ - TaskRunResults: []v1beta1.TaskRunResult{}, - }, - }, - }, + taskResults: map[string][]v1beta1.TaskRunResult{ + "pt1": {}, }, expected: nil, }, { @@ -868,24 +824,11 @@ func TestApplyTaskResultsToPipelineResults(t *testing.T) { Name: "foo", Value: "$(tasks.pt1.results.foo)", }}, - statuses: map[string]*v1beta1.PipelineRunTaskRunStatus{ - "task1": { - PipelineTaskName: "definitely-not-pt1", - Status: &v1beta1.TaskRunStatus{ - Status: duckv1beta1.Status{ - Conditions: []apis.Condition{{ - Type: apis.ConditionSucceeded, - Status: corev1.ConditionTrue, - }}, - }, - TaskRunStatusFields: v1beta1.TaskRunStatusFields{ - TaskRunResults: []v1beta1.TaskRunResult{{ - Name: "foo", - Value: "bar", - }}, - }, - }, - }, + taskResults: map[string][]v1beta1.TaskRunResult{ + "definitely-not-pt1": {{ + Name: "foo", + Value: "bar", + }}, }, expected: nil, }, { @@ -894,24 +837,11 @@ func TestApplyTaskResultsToPipelineResults(t *testing.T) { Name: "foo", Value: "$(tasks.pt1.results.foo)", }}, - statuses: map[string]*v1beta1.PipelineRunTaskRunStatus{ - "task1": { - PipelineTaskName: "pt1", - Status: &v1beta1.TaskRunStatus{ - Status: duckv1beta1.Status{ - Conditions: []apis.Condition{{ - Type: apis.ConditionSucceeded, - Status: corev1.ConditionTrue, - }}, - }, - TaskRunStatusFields: v1beta1.TaskRunStatusFields{ - TaskRunResults: []v1beta1.TaskRunResult{{ - Name: "definitely-not-foo", - Value: "bar", - }}, - }, - }, - }, + taskResults: map[string][]v1beta1.TaskRunResult{ + "pt1": {{ + Name: "definitely-not-foo", + Value: "bar", + }}, }, expected: nil, }, { @@ -920,26 +850,8 @@ func TestApplyTaskResultsToPipelineResults(t *testing.T) { Name: "foo", Value: "$(tasks.pt1.results.foo)", }}, - statuses: map[string]*v1beta1.PipelineRunTaskRunStatus{ - "task1": { - PipelineTaskName: "pt1", - Status: &v1beta1.TaskRunStatus{ - Status: duckv1beta1.Status{ - Conditions: []apis.Condition{{ - Type: apis.ConditionSucceeded, - Status: corev1.ConditionFalse, - }}, - }, - TaskRunStatusFields: v1beta1.TaskRunStatusFields{ - TaskRunResults: []v1beta1.TaskRunResult{{ - Name: "foo", - Value: "bar", - }}, - }, - }, - }, - }, - expected: nil, + taskResults: map[string][]v1beta1.TaskRunResult{}, + expected: nil, }, { description: "mixed-success-tasks-some-returned-results", results: []v1beta1.PipelineResult{{ @@ -949,41 +861,11 @@ func TestApplyTaskResultsToPipelineResults(t *testing.T) { Name: "bar", Value: "$(tasks.pt2.results.bar)", }}, - statuses: map[string]*v1beta1.PipelineRunTaskRunStatus{ - "task1": { - PipelineTaskName: "pt1", - Status: &v1beta1.TaskRunStatus{ - Status: duckv1beta1.Status{ - Conditions: []apis.Condition{{ - Type: apis.ConditionSucceeded, - Status: corev1.ConditionFalse, - }}, - }, - TaskRunStatusFields: v1beta1.TaskRunStatusFields{ - TaskRunResults: []v1beta1.TaskRunResult{{ - Name: "foo", - Value: "do", - }}, - }, - }, - }, - "task2": { - PipelineTaskName: "pt2", - Status: &v1beta1.TaskRunStatus{ - Status: duckv1beta1.Status{ - Conditions: []apis.Condition{{ - Type: apis.ConditionSucceeded, - Status: corev1.ConditionTrue, - }}, - }, - TaskRunStatusFields: v1beta1.TaskRunStatusFields{ - TaskRunResults: []v1beta1.TaskRunResult{{ - Name: "bar", - Value: "rae", - }}, - }, - }, - }, + taskResults: map[string][]v1beta1.TaskRunResult{ + "pt2": {{ + Name: "bar", + Value: "rae", + }}, }, expected: []v1beta1.PipelineRunResult{{ Name: "bar", @@ -998,44 +880,20 @@ func TestApplyTaskResultsToPipelineResults(t *testing.T) { Name: "pipeline-result-2", Value: "$(tasks.pt1.results.foo), $(tasks.pt2.results.baz), $(tasks.pt1.results.bar), $(tasks.pt2.results.baz), $(tasks.pt1.results.foo)", }}, - statuses: map[string]*v1beta1.PipelineRunTaskRunStatus{ - "task1": { - PipelineTaskName: "pt1", - Status: &v1beta1.TaskRunStatus{ - Status: duckv1beta1.Status{ - Conditions: []apis.Condition{{ - Type: apis.ConditionSucceeded, - Status: corev1.ConditionTrue, - }}, - }, - TaskRunStatusFields: v1beta1.TaskRunStatusFields{ - TaskRunResults: []v1beta1.TaskRunResult{{ - Name: "foo", - Value: "do", - }, { - Name: "bar", - Value: "mi", - }}, - }, - }, - }, - "task2": { - PipelineTaskName: "pt2", - Status: &v1beta1.TaskRunStatus{ - Status: duckv1beta1.Status{ - Conditions: []apis.Condition{{ - Type: apis.ConditionSucceeded, - Status: corev1.ConditionTrue, - }}, - }, - TaskRunStatusFields: v1beta1.TaskRunStatusFields{ - TaskRunResults: []v1beta1.TaskRunResult{{ - Name: "baz", - Value: "rae", - }}, - }, + taskResults: map[string][]v1beta1.TaskRunResult{ + "pt1": { + { + Name: "foo", + Value: "do", + }, { + Name: "bar", + Value: "mi", }, }, + "pt2": {{ + Name: "baz", + Value: "rae", + }}, }, expected: []v1beta1.PipelineRunResult{{ Name: "pipeline-result-1", @@ -1050,47 +908,19 @@ func TestApplyTaskResultsToPipelineResults(t *testing.T) { Name: "foo", Value: "$(tasks.customtask.results.foo)", }}, - runStatuses: map[string]*v1beta1.PipelineRunRunStatus{ - "task1": { - PipelineTaskName: "customtask", - Status: &runv1alpha1.RunStatus{ - Status: duckv1.Status{ - Conditions: []apis.Condition{{ - Type: apis.ConditionSucceeded, - Status: corev1.ConditionTrue, - }}, - }, - RunStatusFields: runv1alpha1.RunStatusFields{ - Results: []runv1alpha1.RunResult{}, - }, - }, - }, - }, - expected: nil, + runResults: map[string][]v1alpha1.RunResult{}, + expected: nil, }, { description: "wrong-customtask-name-no-returned-result", results: []v1beta1.PipelineResult{{ Name: "foo", Value: "$(tasks.customtask.results.foo)", }}, - runStatuses: map[string]*v1beta1.PipelineRunRunStatus{ - "task1": { - PipelineTaskName: "differentcustomtask", - Status: &runv1alpha1.RunStatus{ - Status: duckv1.Status{ - Conditions: []apis.Condition{{ - Type: apis.ConditionSucceeded, - Status: corev1.ConditionTrue, - }}, - }, - RunStatusFields: runv1alpha1.RunStatusFields{ - Results: []runv1alpha1.RunResult{{ - Name: "foo", - Value: "bar", - }}, - }, - }, - }, + runResults: map[string][]v1alpha1.RunResult{ + "differentcustomtask": {{ + Name: "foo", + Value: "bar", + }}, }, expected: nil, }, { @@ -1099,24 +929,11 @@ func TestApplyTaskResultsToPipelineResults(t *testing.T) { Name: "foo", Value: "$(tasks.customtask.results.foo)", }}, - runStatuses: map[string]*v1beta1.PipelineRunRunStatus{ - "task1": { - PipelineTaskName: "customtask", - Status: &runv1alpha1.RunStatus{ - Status: duckv1.Status{ - Conditions: []apis.Condition{{ - Type: apis.ConditionSucceeded, - Status: corev1.ConditionTrue, - }}, - }, - RunStatusFields: runv1alpha1.RunStatusFields{ - Results: []runv1alpha1.RunResult{{ - Name: "notfoo", - Value: "bar", - }}, - }, - }, - }, + runResults: map[string][]v1alpha1.RunResult{ + "customtask": {{ + Name: "notfoo", + Value: "bar", + }}, }, expected: nil, }, { @@ -1125,24 +942,8 @@ func TestApplyTaskResultsToPipelineResults(t *testing.T) { Name: "foo", Value: "$(tasks.customtask.results.foo)", }}, - runStatuses: map[string]*v1beta1.PipelineRunRunStatus{ - "task1": { - PipelineTaskName: "customtask", - Status: &runv1alpha1.RunStatus{ - Status: duckv1.Status{ - Conditions: []apis.Condition{{ - Type: apis.ConditionSucceeded, - Status: corev1.ConditionFalse, - }}, - }, - RunStatusFields: runv1alpha1.RunStatusFields{ - Results: []runv1alpha1.RunResult{{ - Name: "foo", - Value: "bar", - }}, - }, - }, - }, + runResults: map[string][]v1alpha1.RunResult{ + "customtask": {}, }, expected: nil, }, { @@ -1154,46 +955,22 @@ func TestApplyTaskResultsToPipelineResults(t *testing.T) { Name: "pipeline-result-2", Value: "$(tasks.customtask.results.foo), $(tasks.normaltask.results.baz), $(tasks.customtask.results.bar), $(tasks.normaltask.results.baz), $(tasks.customtask.results.foo)", }}, - runStatuses: map[string]*v1beta1.PipelineRunRunStatus{ - "task1": { - PipelineTaskName: "customtask", - Status: &runv1alpha1.RunStatus{ - Status: duckv1.Status{ - Conditions: []apis.Condition{{ - Type: apis.ConditionSucceeded, - Status: corev1.ConditionTrue, - }}, - }, - RunStatusFields: runv1alpha1.RunStatusFields{ - Results: []runv1alpha1.RunResult{{ - Name: "foo", - Value: "do", - }, { - Name: "bar", - Value: "mi", - }}, - }, + runResults: map[string][]v1alpha1.RunResult{ + "customtask": { + { + Name: "foo", + Value: "do", + }, { + Name: "bar", + Value: "mi", }, }, }, - statuses: map[string]*v1beta1.PipelineRunTaskRunStatus{ - "task2": { - PipelineTaskName: "normaltask", - Status: &v1beta1.TaskRunStatus{ - Status: duckv1beta1.Status{ - Conditions: []apis.Condition{{ - Type: apis.ConditionSucceeded, - Status: corev1.ConditionTrue, - }}, - }, - TaskRunStatusFields: v1beta1.TaskRunStatusFields{ - TaskRunResults: []v1beta1.TaskRunResult{{ - Name: "baz", - Value: "rae", - }}, - }, - }, - }, + taskResults: map[string][]v1beta1.TaskRunResult{ + "normaltask": {{ + Name: "baz", + Value: "rae", + }}, }, expected: []v1beta1.PipelineRunResult{{ Name: "pipeline-result-1", @@ -1204,7 +981,7 @@ func TestApplyTaskResultsToPipelineResults(t *testing.T) { }}, }} { t.Run(tc.description, func(t *testing.T) { - received := ApplyTaskResultsToPipelineResults(tc.results, tc.statuses, tc.runStatuses) + received := ApplyTaskResultsToPipelineResults(tc.results, tc.taskResults, tc.runResults) if d := cmp.Diff(tc.expected, received); d != "" { t.Errorf(diff.PrintWantGot(d)) } diff --git a/pkg/reconciler/pipelinerun/resources/pipelinerunstate.go b/pkg/reconciler/pipelinerun/resources/pipelinerunstate.go index d1dc65eecee..ff40b917886 100644 --- a/pkg/reconciler/pipelinerun/resources/pipelinerunstate.go +++ b/pkg/reconciler/pipelinerun/resources/pipelinerunstate.go @@ -187,6 +187,23 @@ func (state PipelineRunState) GetTaskRunsStatus(pr *v1beta1.PipelineRun) map[str return status } +// GetTaskRunsResults returns a map of all successfully completed TaskRuns in the state, with the pipeline task name as +// the key and the results from the corresponding TaskRun as the value. It only includes tasks which have completed successfully. +func (state PipelineRunState) GetTaskRunsResults() map[string][]v1beta1.TaskRunResult { + results := make(map[string][]v1beta1.TaskRunResult) + for _, rprt := range state { + if rprt.IsCustomTask() { + continue + } + if !rprt.IsSuccessful() { + continue + } + results[rprt.PipelineTask.Name] = rprt.TaskRun.Status.TaskRunResults + } + + return results +} + // GetRunsStatus returns a map of run name and the run. // Ignore a nil run in pipelineRunState, otherwise, capture run object from PipelineRun Status. // Update run status based on the pipelineRunState before returning it in the map. @@ -216,12 +233,29 @@ func (state PipelineRunState) GetRunsStatus(pr *v1beta1.PipelineRun) map[string] prrs.Status = &rprt.Run.Status } - // TODO(#3133): Include any condition check statuses here too. + // TODO(#3133): Include any condition check taskResults here too. status[rprt.RunName] = prrs } return status } +// GetRunsResults returns a map of all successfully completed Runs in the state, with the pipeline task name as the key +// and the results from the corresponding TaskRun as the value. It only includes runs which have completed successfully. +func (state PipelineRunState) GetRunsResults() map[string][]v1alpha1.RunResult { + results := make(map[string][]v1alpha1.RunResult) + for _, rprt := range state { + if !rprt.IsCustomTask() { + continue + } + if !rprt.IsSuccessful() { + continue + } + results[rprt.PipelineTask.Name] = rprt.Run.Status.Results + } + + return results +} + // getNextTasks returns a list of tasks which should be executed next i.e. // a list of tasks from candidateTasks which aren't yet indicated in state to be running and // a list of cancelled/failed tasks from candidateTasks which haven't exhausted their retries diff --git a/pkg/reconciler/pipelinerun/resources/pipelinerunstate_test.go b/pkg/reconciler/pipelinerun/resources/pipelinerunstate_test.go index ae29af9594d..69b915d3d61 100644 --- a/pkg/reconciler/pipelinerun/resources/pipelinerunstate_test.go +++ b/pkg/reconciler/pipelinerun/resources/pipelinerunstate_test.go @@ -2309,6 +2309,202 @@ spec: } } +func TestPipelineRunState_GetResultsFuncs(t *testing.T) { + state := PipelineRunState{{ + TaskRunName: "successful-task-with-results", + PipelineTask: &v1beta1.PipelineTask{ + Name: "successful-task-with-results-1", + }, + TaskRun: &v1beta1.TaskRun{ + Status: v1beta1.TaskRunStatus{ + Status: duckv1beta1.Status{Conditions: []apis.Condition{{ + Type: apis.ConditionSucceeded, + Status: corev1.ConditionTrue, + }}}, + TaskRunStatusFields: v1beta1.TaskRunStatusFields{ + TaskRunResults: []v1beta1.TaskRunResult{{ + Name: "foo", + Value: "oof", + }, { + Name: "bar", + Value: "rab", + }}, + }, + }, + }, + }, { + TaskRunName: "successful-task-without-results", + PipelineTask: &v1beta1.PipelineTask{ + Name: "successful-task-without-results-1", + }, + TaskRun: &v1beta1.TaskRun{ + Status: v1beta1.TaskRunStatus{ + Status: duckv1beta1.Status{Conditions: []apis.Condition{{ + Type: apis.ConditionSucceeded, + Status: corev1.ConditionTrue, + }}}, + TaskRunStatusFields: v1beta1.TaskRunStatusFields{}, + }, + }, + }, { + TaskRunName: "failed-task", + PipelineTask: &v1beta1.PipelineTask{ + Name: "failed-task-1", + }, + TaskRun: &v1beta1.TaskRun{ + Status: v1beta1.TaskRunStatus{ + Status: duckv1beta1.Status{Conditions: []apis.Condition{{ + Type: apis.ConditionSucceeded, + Status: corev1.ConditionFalse, + }}}, + TaskRunStatusFields: v1beta1.TaskRunStatusFields{ + TaskRunResults: []v1beta1.TaskRunResult{{ + Name: "fail-foo", + Value: "fail-oof", + }}, + }, + }, + }, + }, { + TaskRunName: "incomplete-task", + PipelineTask: &v1beta1.PipelineTask{ + Name: "incomplete-task-1", + }, + TaskRun: &v1beta1.TaskRun{ + Status: v1beta1.TaskRunStatus{ + Status: duckv1beta1.Status{Conditions: []apis.Condition{{ + Type: apis.ConditionSucceeded, + Status: corev1.ConditionUnknown, + }}}, + TaskRunStatusFields: v1beta1.TaskRunStatusFields{ + TaskRunResults: []v1beta1.TaskRunResult{{ + Name: "unknown-foo", + Value: "unknown-oof", + }}, + }, + }, + }, + }, { + TaskRunName: "nil-taskrun", + PipelineTask: &v1beta1.PipelineTask{ + Name: "nil-taskrun-1", + }, + }, { + RunName: "successful-run-with-results", + CustomTask: true, + PipelineTask: &v1beta1.PipelineTask{ + Name: "successful-run-with-results-1", + }, + Run: &v1alpha1.Run{ + Status: v1alpha1.RunStatus{ + Status: duckv1.Status{Conditions: []apis.Condition{{ + Type: apis.ConditionSucceeded, + Status: corev1.ConditionTrue, + }}}, + RunStatusFields: v1alpha1.RunStatusFields{ + Results: []v1alpha1.RunResult{{ + Name: "foo", + Value: "oof", + }, { + Name: "bar", + Value: "rab", + }}, + }, + }, + }, + }, { + RunName: "successful-run-without-results", + CustomTask: true, + PipelineTask: &v1beta1.PipelineTask{ + Name: "successful-run-without-results-1", + }, + Run: &v1alpha1.Run{ + Status: v1alpha1.RunStatus{ + Status: duckv1.Status{Conditions: []apis.Condition{{ + Type: apis.ConditionSucceeded, + Status: corev1.ConditionTrue, + }}}, + RunStatusFields: v1alpha1.RunStatusFields{}, + }, + }, + }, { + RunName: "failed-run", + PipelineTask: &v1beta1.PipelineTask{ + Name: "failed-run-1", + }, + Run: &v1alpha1.Run{ + Status: v1alpha1.RunStatus{ + Status: duckv1.Status{Conditions: []apis.Condition{{ + Type: apis.ConditionSucceeded, + Status: corev1.ConditionFalse, + }}}, + RunStatusFields: v1alpha1.RunStatusFields{ + Results: []v1alpha1.RunResult{{ + Name: "fail-foo", + Value: "fail-oof", + }}, + }, + }, + }, + }, { + RunName: "incomplete-run", + PipelineTask: &v1beta1.PipelineTask{ + Name: "incomplete-run-1", + }, + Run: &v1alpha1.Run{ + Status: v1alpha1.RunStatus{ + Status: duckv1.Status{Conditions: []apis.Condition{{ + Type: apis.ConditionSucceeded, + Status: corev1.ConditionUnknown, + }}}, + RunStatusFields: v1alpha1.RunStatusFields{ + Results: []v1alpha1.RunResult{{ + Name: "unknown-foo", + Value: "unknown-oof", + }}, + }, + }, + }, + }, { + RunName: "nil-run", + CustomTask: true, + PipelineTask: &v1beta1.PipelineTask{ + Name: "nil-run-1", + }, + }} + + expectedTaskResults := map[string][]v1beta1.TaskRunResult{ + "successful-task-with-results-1": {{ + Name: "foo", + Value: "oof", + }, { + Name: "bar", + Value: "rab", + }}, + "successful-task-without-results-1": nil, + } + expectedRunResults := map[string][]v1alpha1.RunResult{ + "successful-run-with-results-1": {{ + Name: "foo", + Value: "oof", + }, { + Name: "bar", + Value: "rab", + }}, + "successful-run-without-results-1": nil, + } + + actualTaskResults := state.GetTaskRunsResults() + if d := cmp.Diff(expectedTaskResults, actualTaskResults); d != "" { + t.Errorf("Didn't get expected TaskRun results map: %s", diff.PrintWantGot(d)) + } + + actualRunResults := state.GetRunsResults() + if d := cmp.Diff(expectedRunResults, actualRunResults); d != "" { + t.Errorf("Didn't get expected Run results map: %s", diff.PrintWantGot(d)) + } +} + // conditionCheckFromTaskRun takes a pointer to a TaskRun and wraps it into a ConditionCheck func conditionCheckFromTaskRun(tr *v1beta1.TaskRun) *v1beta1.ConditionCheck { cc := v1beta1.ConditionCheck(*tr)