Resolve review comments 1

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

pkg/reconciler/pipelinerun/pipelinerun.go

@@ -833,33 +833,25 @@ func (c *Reconciler) runNextSchedulableTask(ctx context.Context, pr *v1.Pipeline
 	recorder := controller.GetEventRecorder(ctx)
 
 	// nextRpts holds a list of pipeline tasks which should be executed next
-	// tmpNextRpts holds the nextRpts temporarily,
-	// tmpNextRpts is later filtered to check for the missing result reference
-	// if the pipelineTask is valid then it is added to the nextRpts
-	tmpNextRpts, err := pipelineRunFacts.DAGExecutionQueue()
+	nextRpts, err := pipelineRunFacts.DAGExecutionQueue()
 	if err != nil {
 		logger.Errorf("Error getting potential next tasks for valid pipelinerun %s: %v", pr.Name, err)
 		return controller.NewPermanentError(err)
 	}
 
-	var nextRpts resources.PipelineRunState
-	for _, nextRpt := range tmpNextRpts {
-		// Check for Missing Result References and
-		// store the faulty task in missingRefTask
-		missingRefTask, err := resources.CheckMissingResultReferences(pipelineRunFacts.State, nextRpt)
+	for _, rpt := range nextRpts {
+		// Check for Missing Result References
+		// if error found, present rpt will be
+		// added to the validationFailedTask list
+		err := resources.CheckMissingResultReferences(pipelineRunFacts.State, rpt)
 		if err != nil {
 			logger.Infof("Failed to resolve task result reference for %q with error %v", pr.Name, err)
-			pr.Status.MarkFailed(v1.PipelineRunReasonInvalidTaskResultReference.String(), err.Error())
-			// check if pipeline contains finally tasks
-			// return the permanent error only if there is no finally task
-			fTaskNames := pipelineRunFacts.GetFinalTaskNames()
-			pipelineRunFacts.ValidationFailedTask = append(pipelineRunFacts.ValidationFailedTask, missingRefTask)
-			if len(fTaskNames) == 0 {
-				return controller.NewPermanentError(err)
-			}
-		} else {
-			// if task is valid then add it to nextRpts for the further execution
-			nextRpts = append(nextRpts, nextRpt)
+			// 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
+			// be added to the nextRpts
+			nextRpts = nil
+			pipelineRunFacts.ValidationFailedTask = append(pipelineRunFacts.ValidationFailedTask, rpt)
 		}
 	}
 	// GetFinalTasks only returns final tasks when a DAG is complete

pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go

@@ -835,33 +835,33 @@ func isCustomRunCancelledByPipelineRunTimeout(cr *v1beta1.CustomRun) bool {
 // CheckMissingResultReferences returns an error if it is missing any result references.
 // Missing result references can occur if task fails to produce a result but has
 // OnError: continue (ie TestMissingResultWhenStepErrorIsIgnored)
-func CheckMissingResultReferences(pipelineRunState PipelineRunState, target *ResolvedPipelineTask) (*ResolvedPipelineTask, error) {
+func CheckMissingResultReferences(pipelineRunState PipelineRunState, target *ResolvedPipelineTask) error {
 	for _, resultRef := range v1.PipelineTaskResultRefs(target.PipelineTask) {
 		referencedPipelineTask, ok := pipelineRunState.ToMap()[resultRef.PipelineTask]
 		if !ok {
-			return target, fmt.Errorf("Result reference error: Could not find ref \"%s\" in internal pipelineRunState", resultRef.PipelineTask)
+			return fmt.Errorf("Result reference error: Could not find ref \"%s\" in internal pipelineRunState", resultRef.PipelineTask)
 		}
 		if referencedPipelineTask.IsCustomTask() {
 			if len(referencedPipelineTask.CustomRuns) == 0 {
-				return target, fmt.Errorf("Result reference error: Internal result ref \"%s\" has zero-length CustomRuns", resultRef.PipelineTask)
+				return fmt.Errorf("Result reference error: Internal result ref \"%s\" has zero-length CustomRuns", resultRef.PipelineTask)
 			}
 			customRun := referencedPipelineTask.CustomRuns[0]
 			_, err := findRunResultForParam(customRun, resultRef)
 			if err != nil {
-				return target, err
+				return err
 			}
 		} else {
 			if len(referencedPipelineTask.TaskRuns) == 0 {
-				return target, fmt.Errorf("Result reference error: Internal result ref \"%s\" has zero-length TaskRuns", resultRef.PipelineTask)
+				return fmt.Errorf("Result reference error: Internal result ref \"%s\" has zero-length TaskRuns", resultRef.PipelineTask)
 			}
 			taskRun := referencedPipelineTask.TaskRuns[0]
 			_, err := findTaskResultForParam(taskRun, resultRef)
 			if err != nil {
-				return target, err
+				return err
 			}
 		}
 	}
-	return target, nil
+	return nil
 }
 
 // createResultsCacheMatrixedTaskRuns creates a cache of results that have been fanned out from a

pkg/reconciler/pipelinerun/resources/resultrefresolution_test.go

@@ -817,7 +817,7 @@ func TestCheckMissingResultReferences(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 			var err error
 			for _, target := range tt.targets {
-				_, tmpErr := CheckMissingResultReferences(tt.pipelineRunState, target)
+				tmpErr := CheckMissingResultReferences(tt.pipelineRunState, target)
 				if tmpErr != nil {
 					err = tmpErr
 				}