Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Split ResolveResultRefs in two #2548

Merged
merged 1 commit into from
May 5, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions pkg/reconciler/pipelinerun/pipelinerun.go
Original file line number Diff line number Diff line change
Expand Up @@ -318,7 +318,7 @@ func (c *Reconciler) updatePipelineResults(ctx context.Context, pr *v1alpha1.Pip
})
}
}
resolvedResultRefs, _ := resources.ResolveResultRefs(pipelineState, nil, pipelineSpec.Results)
resolvedResultRefs := resources.ResolvePipelineResultRefs(pipelineState, pipelineSpec.Results)
pr.Status.PipelineResults = getPipelineRunResults(pipelineSpec, resolvedResultRefs)
}
}
Expand Down Expand Up @@ -557,7 +557,7 @@ func (c *Reconciler) reconcile(ctx context.Context, pr *v1alpha1.PipelineRun) er
}

nextRprts := pipelineState.GetNextTasks(candidateTasks)
resolvedResultRefs, err := resources.ResolveResultRefs(pipelineState, nextRprts, nil)
resolvedResultRefs, err := resources.ResolveResultRefs(pipelineState, nextRprts)
if err != nil {
c.Logger.Infof("Failed to resolve all task params for %q with error %v", pr.Name, err)
pr.Status.SetCondition(&apis.Condition{
Expand Down
11 changes: 9 additions & 2 deletions pkg/reconciler/pipelinerun/resources/resultrefresolution.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ type ResolvedResultRef struct {
}

// ResolveResultRefs resolves any ResultReference that are found in the target ResolvedPipelineRunTask
func ResolveResultRefs(pipelineRunState PipelineRunState, targets PipelineRunState, pipelineResults []v1beta1.PipelineResult) (ResolvedResultRefs, error) {
func ResolveResultRefs(pipelineRunState PipelineRunState, targets PipelineRunState) (ResolvedResultRefs, error) {
var allResolvedResultRefs ResolvedResultRefs
for _, target := range targets {
resolvedResultRefs, err := convertParamsToResultRefs(pipelineRunState, target)
Expand All @@ -46,13 +46,20 @@ func ResolveResultRefs(pipelineRunState PipelineRunState, targets PipelineRunSta
}
allResolvedResultRefs = append(allResolvedResultRefs, resolvedResultRefs...)
}
return removeDup(allResolvedResultRefs), nil
}

// ResolvePipelineResultRefs takes a list of PipelineResults and resolves any references they
// include to Task results in the given PipelineRunState
func ResolvePipelineResultRefs(pipelineRunState PipelineRunState, pipelineResults []v1beta1.PipelineResult) ResolvedResultRefs {
var allResolvedResultRefs ResolvedResultRefs
for _, result := range pipelineResults {
resolvedResultRefs := convertPipelineResultToResultRefs(pipelineRunState, result)
if resolvedResultRefs != nil {
allResolvedResultRefs = append(allResolvedResultRefs, resolvedResultRefs...)
}
}
return removeDup(allResolvedResultRefs), nil
return removeDup(allResolvedResultRefs)
}

// extractResultRefs resolves any ResultReference that are found in param or pipeline result
Expand Down
119 changes: 118 additions & 1 deletion pkg/reconciler/pipelinerun/resources/resultrefresolution_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ import (
"github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1"
"github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1"
tb "github.com/tektoncd/pipeline/test/builder"
corev1 "k8s.io/api/core/v1"
"knative.dev/pkg/apis"
)

func TestTaskParamResolver_ResolveResultRefs(t *testing.T) {
Expand Down Expand Up @@ -366,7 +368,7 @@ func TestResolveResultRefs(t *testing.T) {
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got, err := ResolveResultRefs(tt.args.pipelineRunState, tt.args.targets, nil)
got, err := ResolveResultRefs(tt.args.pipelineRunState, tt.args.targets)
sort.SliceStable(got, func(i, j int) bool {
return strings.Compare(got[i].FromTaskRun, got[j].FromTaskRun) < 0
})
Expand All @@ -380,3 +382,118 @@ func TestResolveResultRefs(t *testing.T) {
})
}
}

func TestResolvePipelineResultRefs(t *testing.T) {
type args struct {
pipelineRunState PipelineRunState
pipelineResults []v1beta1.PipelineResult
}
pipelineRunState := PipelineRunState{
{
TaskRunName: "aTaskRun",
TaskRun: tb.TaskRun("aTaskRun", tb.TaskRunStatus(
tb.TaskRunResult("aResult", "aResultValue"),
)),
PipelineTask: &v1alpha1.PipelineTask{
Name: "aTask",
TaskRef: &v1alpha1.TaskRef{Name: "aTask"},
},
}, {
TaskRunName: "bTaskRun",
TaskRun: tb.TaskRun("bTaskRun", tb.TaskRunStatus(
tb.StatusCondition(apis.Condition{
Type: apis.ConditionSucceeded,
Status: corev1.ConditionFalse})),
),
PipelineTask: &v1alpha1.PipelineTask{
Name: "bTask",
TaskRef: &v1alpha1.TaskRef{Name: "bTask"},
},
}, {
PipelineTask: &v1alpha1.PipelineTask{
Name: "cTask",
TaskRef: &v1alpha1.TaskRef{Name: "cTask"},
},
},
}

tests := []struct {
name string
args args
want ResolvedResultRefs
}{
{
name: "Test pipeline result from a successful task",
args: args{
pipelineRunState: pipelineRunState,
pipelineResults: []v1beta1.PipelineResult{
{
Name: "from-a",
Value: "$(tasks.aTask.results.aResult)",
Description: "a result from a",
},
},
},
want: ResolvedResultRefs{
{
Value: v1beta1.ArrayOrString{
Type: v1beta1.ParamTypeString,
StringVal: "aResultValue",
},
ResultReference: v1beta1.ResultRef{
PipelineTask: "aTask",
Result: "aResult",
},
FromTaskRun: "aTaskRun",
},
},
},
{
name: "Test results from a task that did not run and one and failed",
args: args{
pipelineRunState: pipelineRunState,
pipelineResults: []v1beta1.PipelineResult{
{
Name: "from-a",
Value: "$(tasks.aTask.results.aResult)",
Description: "a result from a",
},
{
Name: "from-b",
Value: "$(tasks.bTask.results.bResult)",
Description: "a result from b",
},
{
Name: "from-c",
Value: "$(tasks.cTask.results.cResult)",
Description: "a result from c",
},
},
},
want: ResolvedResultRefs{
{
Value: v1beta1.ArrayOrString{
Type: v1beta1.ParamTypeString,
StringVal: "aResultValue",
},
ResultReference: v1beta1.ResultRef{
PipelineTask: "aTask",
Result: "aResult",
},
FromTaskRun: "aTaskRun",
},
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got := ResolvePipelineResultRefs(tt.args.pipelineRunState, tt.args.pipelineResults)
sort.SliceStable(got, func(i, j int) bool {
return strings.Compare(got[i].FromTaskRun, got[j].FromTaskRun) < 0
})
if d := cmp.Diff(tt.want, got); d != "" {
t.Fatalf("ResolveResultRef -want, +got: %v", d)
}
})
}
}
2 changes: 1 addition & 1 deletion pkg/reconciler/pipelinerun/resources/validate_params.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import (
"github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1"
)

// Validate that parameters in PipelineRun override corresponding parameters in Pipeline of the same type.
// ValidateParamTypesMatching validate that parameters in PipelineRun override corresponding parameters in Pipeline of the same type.
func ValidateParamTypesMatching(p *v1alpha1.PipelineSpec, pr *v1alpha1.PipelineRun) error {
// Build a map of parameter names/types declared in p.
paramTypes := make(map[string]v1alpha1.ParamType)
Expand Down