Skip to content

Commit

Permalink
de-dup order and resource dependencies
Browse files Browse the repository at this point in the history 
In case of a pipeline with a huge list of redundant dependencies, the list is
growing exponentially. This way of calculating the list of dependencies is
causing extra delay in the validation cycle. This delay sometimes hit the
webhook timeout during validation. This is one of the changes being proposed to
make the validation cycle efficient and avoid unneccesary delay.
  • Loading branch information
@pritidesai @tekton-robot
pritidesai authored and tekton-robot committed Sep 14, 2022
1 parent 3171a67 commit 6a789d4
Show file tree
Hide file tree
Showing 5 changed files with 341 additions and 51 deletions.
27 changes: 10 additions & 17 deletions pkg/apis/pipeline/v1/pipeline_types.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -468,29 +468,22 @@ func (pt PipelineTask) Validate(ctx context.Context) (errs *apis.FieldError) {
return
}

// Deps returns all other PipelineTask dependencies of this PipelineTask, based on ordering
// Deps returns all other PipelineTask dependencies of this PipelineTask, based on resource usage or ordering
func (pt PipelineTask) Deps() []string {
deps := []string{}
// hold the list of dependencies in a set to avoid duplicates
deps := sets.NewString()

deps = append(deps, pt.orderingDeps()...)

uniqueDeps := sets.NewString()
for _, w := range deps {
if uniqueDeps.Has(w) {
continue
}
uniqueDeps.Insert(w)
// add any new dependents from result references - resource dependency
for _, ref := range PipelineTaskResultRefs(&pt) {
deps.Insert(ref.PipelineTask)
}

return uniqueDeps.List()
}

func (pt PipelineTask) orderingDeps() []string {
orderingDeps := []string{}
// add any new dependents from runAfter - order dependency
for _, runAfter := range pt.RunAfter {
orderingDeps = append(orderingDeps, runAfter)
deps.Insert(runAfter)
}
return orderingDeps

return deps.List()
}

// PipelineTaskList is a list of PipelineTasks
Expand Down
199 changes: 199 additions & 0 deletions pkg/apis/pipeline/v1/pipeline_types_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -366,6 +366,205 @@ func TestPipelineTaskList_Deps(t *testing.T) {
expectedDeps: map[string][]string{
"task-2": {"task-1"},
},
}, {
name: "valid pipeline with Task Results deps",
tasks: []PipelineTask{{
Name: "task-1",
}, {
Name: "task-2",
Params: []Param{{
Value: ParamValue{
Type: "string",
StringVal: "$(tasks.task-1.results.result)",
}},
}},
},
expectedDeps: map[string][]string{
"task-2": {"task-1"},
},
}, {
name: "valid pipeline with Task Results in Matrix deps",
tasks: []PipelineTask{{
Name: "task-1",
}, {
Name: "task-2",
Matrix: []Param{{
Value: ParamValue{
Type: ParamTypeArray,
ArrayVal: []string{
"$(tasks.task-1.results.result)",
},
}},
}},
},
expectedDeps: map[string][]string{
"task-2": {"task-1"},
},
}, {
name: "valid pipeline with When Expressions deps",
tasks: []PipelineTask{{
Name: "task-1",
}, {
Name: "task-2",
When: WhenExpressions{{
Input: "$(tasks.task-1.results.result)",
Operator: "in",
Values: []string{"foo"},
}},
}},
expectedDeps: map[string][]string{
"task-2": {"task-1"},
},
}, {
name: "valid pipeline with ordering deps and resource deps",
tasks: []PipelineTask{{
Name: "task-1",
}, {
Name: "task-2", RunAfter: []string{"task-1"},
}, {
Name: "task-3",
RunAfter: []string{"task-1"},
}, {
Name: "task-4",
RunAfter: []string{"task-1"},
Params: []Param{{
Value: ParamValue{
Type: "string",
StringVal: "$(tasks.task-3.results.result)",
}},
},
}, {
Name: "task-5",
RunAfter: []string{"task-1"},
When: WhenExpressions{{
Input: "$(tasks.task-4.results.result)",
Operator: "in",
Values: []string{"foo"},
}},
}, {
Name: "task-6",
RunAfter: []string{"task-1"},
Matrix: []Param{{
Value: ParamValue{
Type: ParamTypeArray,
ArrayVal: []string{
"$(tasks.task-2.results.result)",
"$(tasks.task-5.results.result)",
},
}},
},
}},
expectedDeps: map[string][]string{
"task-2": {"task-1"},
"task-3": {"task-1"},
"task-4": {"task-1", "task-3"},
"task-5": {"task-1", "task-4"},
"task-6": {"task-1", "task-2", "task-5"},
},
}, {
name: "valid pipeline with ordering deps and resource deps - verify unique dependencies",
tasks: []PipelineTask{{
Name: "task-1",
}, {
Name: "task-2", RunAfter: []string{"task-1"},
}, {
Name: "task-3",
RunAfter: []string{"task-1"},
}, {
Name: "task-4",
RunAfter: []string{"task-1", "task-3"},
Params: []Param{{
Value: ParamValue{
Type: "string",
StringVal: "$(tasks.task-2.results.result)",
}}, {
Value: ParamValue{
Type: "string",
StringVal: "$(tasks.task-3.results.result)",
}},
},
}, {
Name: "task-5",
RunAfter: []string{"task-1", "task-2", "task-3", "task-4"},
Params: []Param{{
Value: ParamValue{
Type: "string",
StringVal: "$(tasks.task-4.results.result)",
}},
},
When: WhenExpressions{{
Input: "$(tasks.task-3.results.result)",
Operator: "in",
Values: []string{"foo"},
}, {
Input: "$(tasks.task-4.results.result)",
Operator: "in",
Values: []string{"foo"},
}},
}, {
Name: "task-6",
RunAfter: []string{"task-1", "task-2", "task-3", "task-4", "task-5"},
Params: []Param{{
Value: ParamValue{
Type: "string",
StringVal: "$(tasks.task-4.results.result)",
}},
},
When: WhenExpressions{{
Input: "$(tasks.task-3.results.result)",
Operator: "in",
Values: []string{"foo"},
}, {
Input: "$(tasks.task-4.results.result)",
Operator: "in",
Values: []string{"foo"},
}},
Matrix: []Param{{
Value: ParamValue{
Type: ParamTypeArray,
ArrayVal: []string{
"$(tasks.task-2.results.result)",
"$(tasks.task-5.results.result)",
},
}},
},
}, {
Name: "task-7",
When: WhenExpressions{{
Input: "$(tasks.task-3.results.result1)",
Operator: "in",
Values: []string{"foo"},
}, {
Input: "$(tasks.task-3.results.result2)",
Operator: "in",
Values: []string{"foo"},
}},
}, {
Name: "task-8",
Params: []Param{{
Value: ParamValue{
Type: "string",
StringVal: "$(tasks.task-4.results.result1)",
}}, {
Value: ParamValue{
Type: "string",
StringVal: "$(tasks.task-4.results.result2)",
}},
},
}, {
Name: "task-9",
RunAfter: []string{"task-1", "task-1", "task-1", "task-1"},
}},
expectedDeps: map[string][]string{
"task-2": {"task-1"},
"task-3": {"task-1"},
"task-4": {"task-1", "task-2", "task-3"},
"task-5": {"task-1", "task-2", "task-3", "task-4"},
"task-6": {"task-1", "task-2", "task-3", "task-4", "task-5"},
"task-7": {"task-3"},
"task-8": {"task-4"},
"task-9": {"task-1"},
},
}}
for _, tc := range pipelines {
t.Run(tc.name, func(t *testing.T) {
Expand Down
39 changes: 12 additions & 27 deletions pkg/apis/pipeline/v1beta1/pipeline_types.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -512,44 +512,29 @@ func (pt PipelineTask) Validate(ctx context.Context) (errs *apis.FieldError) {

// Deps returns all other PipelineTask dependencies of this PipelineTask, based on resource usage or ordering
func (pt PipelineTask) Deps() []string {
deps := []string{}
// hold the list of dependencies in a set to avoid duplicates
deps := sets.NewString()

deps = append(deps, pt.resourceDeps()...)
deps = append(deps, pt.orderingDeps()...)

uniqueDeps := sets.NewString()
for _, w := range deps {
if uniqueDeps.Has(w) {
continue
}
uniqueDeps.Insert(w)
}

return uniqueDeps.List()
}

func (pt PipelineTask) resourceDeps() []string {
resourceDeps := []string{}
// add any new dependents from a resource/workspace
if pt.Resources != nil {
for _, rd := range pt.Resources.Inputs {
resourceDeps = append(resourceDeps, rd.From...)
for _, f := range rd.From {
deps.Insert(f)
}
}
}

// Add any dependents from result references.
// add any new dependents from result references - resource dependency
for _, ref := range PipelineTaskResultRefs(&pt) {
resourceDeps = append(resourceDeps, ref.PipelineTask)
deps.Insert(ref.PipelineTask)
}

return resourceDeps
}

func (pt PipelineTask) orderingDeps() []string {
orderingDeps := []string{}
// add any new dependents from runAfter - order dependency
for _, runAfter := range pt.RunAfter {
orderingDeps = append(orderingDeps, runAfter)
deps.Insert(runAfter)
}
return orderingDeps

return deps.List()
}

// PipelineTaskList is a list of PipelineTasks
Expand Down
47 changes: 42 additions & 5 deletions pkg/apis/pipeline/v1beta1/pipeline_types_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -643,13 +643,50 @@ func TestPipelineTaskList_Deps(t *testing.T) {
},
}},
},
}, {
Name: "task-7",
Resources: &PipelineTaskResources{
Inputs: []PipelineTaskInputResource{{
From: []string{"task-1", "task-1"},
}},
},
}, {
Name: "task-8",
WhenExpressions: WhenExpressions{{
Input: "$(tasks.task-3.results.result1)",
Operator: "in",
Values: []string{"foo"},
}, {
Input: "$(tasks.task-3.results.result2)",
Operator: "in",
Values: []string{"foo"},
}},
}, {
Name: "task-9",
Params: []Param{{
Value: ParamValue{
Type: "string",
StringVal: "$(tasks.task-4.results.result1)",
}}, {
Value: ParamValue{
Type: "string",
StringVal: "$(tasks.task-4.results.result2)",
}},
},
}, {
Name: "task-10",
RunAfter: []string{"task-1", "task-1", "task-1", "task-1"},
}},
expectedDeps: map[string][]string{
"task-2": {"task-1"},
"task-3": {"task-1", "task-2"},
"task-4": {"task-1", "task-2", "task-3"},
"task-5": {"task-1", "task-2", "task-3", "task-4"},
"task-6": {"task-1", "task-2", "task-3", "task-4", "task-5"},
"task-2": {"task-1"},
"task-3": {"task-1", "task-2"},
"task-4": {"task-1", "task-2", "task-3"},
"task-5": {"task-1", "task-2", "task-3", "task-4"},
"task-6": {"task-1", "task-2", "task-3", "task-4", "task-5"},
"task-7": {"task-1"},
"task-8": {"task-3"},
"task-9": {"task-4"},
"task-10": {"task-1"},
},
}}
for _, tc := range pipelines {
Expand Down
Loading

0 comments on commit 6a789d4

Please sign in to comment.