Avoid retry on permanent errors in pipelinerun

docs/pipelineruns.md

@@ -1261,6 +1261,7 @@ False    | Failed             |           Yes           |
 False    | \[Error message\]  |           Yes           |                 The `PipelineRun` failed with a permanent error (usually validation).
 False    | Cancelled          |           Yes           |                                         The `PipelineRun` was cancelled successfully.
 False    | PipelineRunTimeout |           Yes           |                                                          The `PipelineRun` timed out.
+False    | CreateRunFailed    |           Yes           |                                        The `PipelineRun` create run resources failed.
 
 When a `PipelineRun` changes status, [events](events.md#pipelineruns) are triggered accordingly.
 

pkg/reconciler/pipelinerun/pipelinerun.go

@@ -126,6 +126,8 @@ const (
 	// ReasonResourceVerificationFailed indicates that the pipeline fails the trusted resource verification,
 	// it could be the content has changed, signature is invalid or public key is invalid
 	ReasonResourceVerificationFailed = "ResourceVerificationFailed"
+	// ReasonCreateRunFailed indicates that the pipeline fails to create the taskrun or other run resources
+	ReasonCreateRunFailed = "CreateRunFailed"
 )
 
 // constants used as kind descriptors for various types of runs; these constants
@@ -755,30 +757,41 @@ func (c *Reconciler) runNextSchedulableTask(ctx context.Context, pr *v1beta1.Pip
 			continue
 		}
 
+		defer func() {
+			// If it is a permanent error, set pipelinerun to a failure state directly to avoid unnecessary retries.
+			if err != nil && controller.IsPermanentError(err) {
+				pr.Status.MarkFailed(ReasonCreateRunFailed, err.Error())
+			}
+		}()
+
 		switch {
 		case rpt.IsCustomTask() && rpt.IsMatrixed():
 			rpt.RunObjects, err = c.createRunObjects(ctx, rpt, pr)
 			if err != nil {
 				recorder.Eventf(pr, corev1.EventTypeWarning, "RunsCreationFailed", "Failed to create Runs %q: %v", rpt.RunObjectNames, err)
-				return fmt.Errorf("error creating Runs called %s for PipelineTask %s from PipelineRun %s: %w", rpt.RunObjectNames, rpt.PipelineTask.Name, pr.Name, err)
+				err = fmt.Errorf("error creating Runs called %s for PipelineTask %s from PipelineRun %s: %w", rpt.RunObjectNames, rpt.PipelineTask.Name, pr.Name, err)
+				return err
 			}
 		case rpt.IsCustomTask():
 			rpt.RunObject, err = c.createRunObject(ctx, rpt.RunObjectName, nil, rpt, pr)
 			if err != nil {
 				recorder.Eventf(pr, corev1.EventTypeWarning, "RunCreationFailed", "Failed to create Run %q: %v", rpt.RunObjectName, err)
-				return fmt.Errorf("error creating Run called %s for PipelineTask %s from PipelineRun %s: %w", rpt.RunObjectName, rpt.PipelineTask.Name, pr.Name, err)
+				err = fmt.Errorf("error creating Run called %s for PipelineTask %s from PipelineRun %s: %w", rpt.RunObjectName, rpt.PipelineTask.Name, pr.Name, err)
+				return err
 			}
 		case rpt.IsMatrixed():
 			rpt.TaskRuns, err = c.createTaskRuns(ctx, rpt, pr)
 			if err != nil {
 				recorder.Eventf(pr, corev1.EventTypeWarning, "TaskRunsCreationFailed", "Failed to create TaskRuns %q: %v", rpt.TaskRunNames, err)
-				return fmt.Errorf("error creating TaskRuns called %s for PipelineTask %s from PipelineRun %s: %w", rpt.TaskRunNames, rpt.PipelineTask.Name, pr.Name, err)
+				err = fmt.Errorf("error creating TaskRuns called %s for PipelineTask %s from PipelineRun %s: %w", rpt.TaskRunNames, rpt.PipelineTask.Name, pr.Name, err)
+				return err
 			}
 		default:
 			rpt.TaskRun, err = c.createTaskRun(ctx, rpt.TaskRunName, nil, rpt, pr)
 			if err != nil {
 				recorder.Eventf(pr, corev1.EventTypeWarning, "TaskRunCreationFailed", "Failed to create TaskRun %q: %v", rpt.TaskRunName, err)
-				return fmt.Errorf("error creating TaskRun called %s for PipelineTask %s from PipelineRun %s: %w", rpt.TaskRunName, rpt.PipelineTask.Name, pr.Name, err)
+				err = fmt.Errorf("error creating TaskRun called %s for PipelineTask %s from PipelineRun %s: %w", rpt.TaskRunName, rpt.PipelineTask.Name, pr.Name, err)
+				return err
 			}
 		}
 	}
@@ -1010,6 +1023,7 @@ func propagateWorkspaces(rpt *resources.ResolvedPipelineTask) (*resources.Resolv
 }
 
 func getTaskrunWorkspaces(ctx context.Context, pr *v1beta1.PipelineRun, rpt *resources.ResolvedPipelineTask) ([]v1beta1.WorkspaceBinding, string, error) {
+	var err error
 	var workspaces []v1beta1.WorkspaceBinding
 	var pipelinePVCWorkspaceName string
 	pipelineRunWorkspaces := make(map[string]v1beta1.WorkspaceBinding)
@@ -1020,10 +1034,10 @@ func getTaskrunWorkspaces(ctx context.Context, pr *v1beta1.PipelineRun, rpt *res
 	if config.FromContextOrDefaults(ctx).FeatureFlags.EnableAPIFields != config.StableAPIFields {
 		// Propagate required workspaces from pipelineRun to the pipelineTasks
 		if rpt.PipelineTask.TaskSpec != nil {
-			var err error
 			rpt, err = propagateWorkspaces(rpt)
 			if err != nil {
-				return nil, "", err
+				// This error cannot be recovered without modifying the TaskSpec
+				return nil, "", controller.NewPermanentError(err)
 			}
 		}
 	}
@@ -1052,7 +1066,9 @@ func getTaskrunWorkspaces(ctx context.Context, pr *v1beta1.PipelineRun, rpt *res
 				}
 			}
 			if !workspaceIsOptional {
-				return nil, "", fmt.Errorf("expected workspace %q to be provided by pipelinerun for pipeline task %q", pipelineWorkspace, rpt.PipelineTask.Name)
+				err = fmt.Errorf("expected workspace %q to be provided by pipelinerun for pipeline task %q", pipelineWorkspace, rpt.PipelineTask.Name)
+				// This error cannot be recovered without modifying the PipelineRun
+				return nil, "", controller.NewPermanentError(err)
 			}
 		}
 	}

pkg/reconciler/pipelinerun/pipelinerun_test.go

@@ -7263,6 +7263,8 @@ spec:
 				t.Errorf("Pipeline.getTaskrunWorkspaces() did not return error for invalid workspace")
 			} else if d := cmp.Diff(tt.expectedError, err.Error(), cmpopts.IgnoreUnexported(apis.FieldError{})); d != "" {
 				t.Errorf("Pipeline.getTaskrunWorkspaces() errors diff %s", diff.PrintWantGot(d))
+			} else if !controller.IsPermanentError(err) {
+				t.Errorf("Pipeline.getTaskrunWorkspaces() not returned a permanent error for invalid workspace")
 			}
 		})
 	}
@@ -10079,3 +10081,68 @@ spec:
 		})
 	}
 }
+
+func TestReconcileForPipelineRunCreateRunFailed(t *testing.T) {
+	// TestReconcileForPipelineRunCreateRunFailed runs "Reconcile" on a PipelineRun that has permanent error.
+	// It verifies that reconcile is successful, no TaskRun is created, the PipelineTask is marked as CreateRunFailed, and the
+	// pipeline status updated and events generated.
+	ps := []*v1beta1.Pipeline{}
+	prs := []*v1beta1.PipelineRun{parse.MustParseV1beta1PipelineRun(t, `
+metadata:
+  name: test-pipeline-run-with-create-run-failed
+  namespace: foo
+spec:
+  pipelineSpec:
+    tasks:
+    - name: hello-world
+      taskRef:
+        name: unit-test-task
+      workspaces:
+      - name: source
+`)}
+
+	ts := []*v1beta1.Task{
+		parse.MustParseV1beta1Task(t, `
+metadata:
+  name: unit-test-task
+  namespace: foo
+spec:
+  workspaces:
+  - name: source
+  steps:
+  - image: alpine:latest
+    name: test
+    script: |
+      echo "hello world"
+    workspaces:
+    - name: source
+`)}
+
+	trs := []*v1beta1.TaskRun{}
+
+	d := test.Data{
+		PipelineRuns: prs,
+		Pipelines:    ps,
+		Tasks:        ts,
+		TaskRuns:     trs,
+	}
+	prt := newPipelineRunTest(t, d)
+	defer prt.Cancel()
+
+	wantEvents := []string{
+		"Normal Started",
+		"Warning TaskRunCreationFailed",
+		"error creating TaskRun called test-pipeline-run-with-create-run-failed-hello-world for PipelineTask hello-world from PipelineRun test-pipeline-run-with-create-run-failed: expected workspace \"source\" to be provided by pipelinerun for pipeline task \"hello-world\"",
+		"error creating TaskRun called test-pipeline-run-with-create-run-failed-hello-world for PipelineTask hello-world from PipelineRun test-pipeline-run-with-create-run-failed: expected workspace \"source\" to be provided by pipelinerun for pipeline task \"hello-world\"",
+	}
+	reconciledRun, _ := prt.reconcileRun("foo", "test-pipeline-run-with-create-run-failed", wantEvents, true)
+
+	if reconciledRun.Status.CompletionTime == nil {
+		t.Errorf("Expected a CompletionTime on invalid PipelineRun but was nil")
+	}
+
+	// The PipelineRun should be create run failed.
+	if reconciledRun.Status.GetCondition(apis.ConditionSucceeded).Reason != "CreateRunFailed" {
+		t.Errorf("Expected PipelineRun to be create run failed, but condition reason is %s", reconciledRun.Status.GetCondition(apis.ConditionSucceeded))
+	}
+}