Skip to content

Commit

Permalink
Add check before creating task
Browse files Browse the repository at this point in the history
Tasks with when expressions using variables that evaluated to false were
mistakenly were mistakenly executed because we missed an additional check
to validate that the task should be skipped after variable replacement.

Fixes tektoncd#3188
  • Loading branch information
jerop committed Sep 10, 2020
1 parent f97f6b5 commit 7621819
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,16 @@ spec:
- name: echo
image: ubuntu
script: 'echo file exists'
- name: echo-file-missing
when:
- input: "$(tasks.check-file.results.status)"
operator: in
values: ["missing"]
taskSpec:
steps:
- name: echo
image: ubuntu
script: 'echo file missing'
- name: task-should-be-skipped
when:
- input: "$(params.path)"
Expand Down
2 changes: 1 addition & 1 deletion pkg/reconciler/pipelinerun/pipelinerun.go
Original file line number Diff line number Diff line change
Expand Up @@ -522,7 +522,7 @@ func (c *Reconciler) runNextSchedulableTask(ctx context.Context, pr *v1beta1.Pip
resources.ApplyTaskResults(nextRprts, resolvedResultRefs)

for _, rprt := range nextRprts {
if rprt == nil {
if rprt == nil || rprt.Skip(pipelineState, d) {
continue
}
if rprt.ResolvedConditionChecks == nil || rprt.ResolvedConditionChecks.IsSuccess() {
Expand Down
22 changes: 22 additions & 0 deletions pkg/reconciler/pipelinerun/pipelinerun_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2189,6 +2189,28 @@ func TestReconcileWithWhenExpressionsWithTaskResults(t *testing.T) {
if d := cmp.Diff(actualSkippedTasks, expectedSkippedTasks); d != "" {
t.Errorf("expected to find Skipped Tasks %v. Diff %s", expectedSkippedTasks, diff.PrintWantGot(d))
}

skippedCTask, err := clients.Pipeline.TektonV1beta1().TaskRuns("foo").List(metav1.ListOptions{
LabelSelector: "tekton.dev/pipelineTask=c-task,tekton.dev/pipelineRun=test-pipeline-run-different-service-accs",
Limit: 1,
})
if err != nil {
t.Fatalf("Failure to list TaskRun's %s", err)
}
if len(skippedCTask.Items) != 0 {
t.Fatalf("Expected 0 TaskRuns got %d", len(skippedCTask.Items))
}

skippedDTask, err := clients.Pipeline.TektonV1beta1().TaskRuns("foo").List(metav1.ListOptions{
LabelSelector: "tekton.dev/pipelineTask=d-task,tekton.dev/pipelineRun=test-pipeline-run-different-service-accs",
Limit: 1,
})
if err != nil {
t.Fatalf("Failure to list TaskRun's %s", err)
}
if len(skippedDTask.Items) != 0 {
t.Fatalf("Expected 0 TaskRuns got %d", len(skippedCTask.Items))
}
}

// TestReconcileWithAffinityAssistantStatefulSet tests that given a pipelineRun with workspaces,
Expand Down

0 comments on commit 7621819

Please sign in to comment.