Skip to content

Commit

Permalink
Remove permanent error and improve skip logic
Browse files Browse the repository at this point in the history
Signed-off-by: divyansh42 <[email protected]>
  • Loading branch information
divyansh42 authored and tekton-robot committed Nov 5, 2024
1 parent ee9c44c commit 7a358c3
Show file tree
Hide file tree
Showing 4 changed files with 9 additions and 18 deletions.
13 changes: 2 additions & 11 deletions pkg/reconciler/pipelinerun/pipelinerun.go
Original file line number Diff line number Diff line change
Expand Up @@ -848,20 +848,11 @@ func (c *Reconciler) runNextSchedulableTask(ctx context.Context, pr *v1.Pipeline
logger.Infof("Failed to resolve task result reference for %q with error %v", pr.Name, err)
// If there is an error encountered, no new task
// will be scheduled, hence nextRpts should be empty
// if finally tasks are found, then those tasks will
// If finally tasks are found, then those tasks will
// be added to the nextRpts
nextRpts = nil
logger.Infof("Adding the task %q to the validation failed list", rpt.ResolvedTask)
pipelineRunFacts.ValidationFailedTask = append(pipelineRunFacts.ValidationFailedTask, rpt)
fTaskNames := pipelineRunFacts.GetFinalTaskNames()
if len(fTaskNames) == 0 {
// If finally is not present, we should mark pipelinerun as
// failed so that no further execution happens. Also,
// this will set the completion time of the pipelineRun.
// NewPermanentError should also be returned so that
// reconcilation stops here
pr.Status.MarkFailed(v1.PipelineRunReasonInvalidTaskResultReference.String(), err.Error())
return controller.NewPermanentError(err)
}
}
}
// GetFinalTasks only returns final tasks when a DAG is complete
Expand Down
10 changes: 5 additions & 5 deletions pkg/reconciler/pipelinerun/pipelinerun_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1284,8 +1284,8 @@ status:
image: busybox
script: 'exit 0'
conditions:
- message: "Invalid task result reference: Could not find result with name result2 for task task1"
reason: InvalidTaskResultReference
- message: "Tasks Completed: 2 (Failed: 1, Cancelled 0), Skipped: 0"
reason: Failed
status: "False"
type: Succeeded
childReferences:
Expand All @@ -1301,15 +1301,15 @@ status:
prt := newPipelineRunTest(t, d)
defer prt.Cancel()

reconciledRun, clients := prt.reconcileRun("foo", "test-pipeline-missing-results", []string{}, true)
reconciledRun, clients := prt.reconcileRun("foo", "test-pipeline-missing-results", []string{}, false)
if reconciledRun.Status.CompletionTime == nil {
t.Errorf("Expected a CompletionTime on invalid PipelineRun but was nil")
}

// The PipelineRun should be marked as failed due to InvalidTaskResultReference.
// The PipelineRun should be marked as failed
if d := cmp.Diff(expectedPipelineRun, reconciledRun, ignoreResourceVersion, ignoreLastTransitionTime, ignoreTypeMeta,
ignoreStartTime, ignoreCompletionTime, ignoreProvenance); d != "" {
t.Errorf("Expected to see PipelineRun run marked as failed with the reason: InvalidTaskResultReference. Diff %s", diff.PrintWantGot(d))
t.Errorf("Expected to see PipelineRun run marked as failed. Diff %s", diff.PrintWantGot(d))
}

// Check that the expected TaskRun was created
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -349,7 +349,7 @@ func (t *ResolvedPipelineTask) skip(facts *PipelineRunFacts) TaskSkipStatus {
var skippingReason v1.SkippingReason

switch {
case facts.isFinalTask(t.PipelineTask.Name) || t.isScheduled():
case facts.isFinalTask(t.PipelineTask.Name) || t.isScheduled() || t.isValidationFailed(facts.ValidationFailedTask):
skippingReason = v1.None
case facts.IsStopping():
skippingReason = v1.StoppingSkip
Expand Down
2 changes: 1 addition & 1 deletion pkg/reconciler/pipelinerun/resources/pipelinerunstate.go
Original file line number Diff line number Diff line change
Expand Up @@ -364,7 +364,7 @@ func (state PipelineRunState) getNextTasks(candidateTasks sets.String) []*Resolv
func (facts *PipelineRunFacts) IsStopping() bool {
for _, t := range facts.State {
if facts.isDAGTask(t.PipelineTask.Name) {
if t.isFailure() && t.PipelineTask.OnError != v1.PipelineTaskContinue {
if (t.isFailure() || t.isValidationFailed(facts.ValidationFailedTask)) && t.PipelineTask.OnError != v1.PipelineTaskContinue {
return true
}
}
Expand Down

0 comments on commit 7a358c3

Please sign in to comment.