Remove permanent error and improve skip logic

Signed-off-by: divyansh42 <[email protected]>

Author: divyansh42

pkg/reconciler/pipelinerun/pipelinerun.go

@@ -849,20 +849,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

pkg/reconciler/pipelinerun/pipelinerun_test.go

@@ -1290,8 +1290,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:
@@ -1307,15 +1307,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

pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go

@@ -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

pkg/reconciler/pipelinerun/resources/pipelinerunstate.go

@@ -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
 			}
 		}