Do not fail PipelineRun if pvc creation error is because of exceeded quotas

In case of the PVC creation (from volumeclaimtemplate) is due to a
quota error (quota exceeded), do not fail with a permanent error, and
instead mark the PipelineRun as pending. Once there is some quota
available back, it will be able to start.

Signed-off-by: Vincent Demeester <[email protected]>

Author: vdemeester

pkg/reconciler/pipelinerun/affinity_assistant.go

@@ -50,7 +50,8 @@ const (
 )
 
 var (
-	ErrPvcCreationFailed               = errors.New("PVC creation error")
+	ErrPvcCreationFailed               = volumeclaim.ErrPvcCreationFailed
+	ErrPvcCreationFailedRetryable      = volumeclaim.ErrPvcCreationFailedRetryable
 	ErrAffinityAssistantCreationFailed = errors.New("Affinity Assistant creation error")
 )
 
@@ -95,7 +96,7 @@ func (c *Reconciler) createOrUpdateAffinityAssistantsAndPVCs(ctx context.Context
 			// To support PVC auto deletion at pipelinerun deletion time, the OwnerReference of the PVCs should be set to the owning pipelinerun instead of the StatefulSets,
 			// so we create PVCs from PipelineRuns' VolumeClaimTemplate and pass the PVCs to the Affinity Assistant StatefulSet for volume scheduling.
 			if err := c.pvcHandler.CreatePVCFromVolumeClaimTemplate(ctx, workspace, *kmeta.NewControllerRef(pr), pr.Namespace); err != nil {
-				return fmt.Errorf("%w: %v", ErrPvcCreationFailed, err) //nolint:errorlint
+				return err
 			}
 			aaName := GetAffinityAssistantName(workspace.Name, pr.Name)
 			if err := c.createOrUpdateAffinityAssistant(ctx, aaName, pr, nil, []string{claimTemplate.Name}, unschedulableNodes); err != nil {
@@ -114,7 +115,7 @@ func (c *Reconciler) createOrUpdateAffinityAssistantsAndPVCs(ctx context.Context
 	case aa.AffinityAssistantDisabled:
 		for _, workspace := range claimTemplateToWorkspace {
 			if err := c.pvcHandler.CreatePVCFromVolumeClaimTemplate(ctx, workspace, *kmeta.NewControllerRef(pr), pr.Namespace); err != nil {
-				return fmt.Errorf("%w: %v", ErrPvcCreationFailed, err) //nolint:errorlint
+				return err
 			}
 		}
 	}

pkg/reconciler/pipelinerun/affinity_assistant_test.go

@@ -577,54 +577,56 @@ func TestCreateOrUpdateAffinityAssistantsAndPVCs_Failure(t *testing.T) {
 		name:        "pvc creation failed - per workspace",
 		failureType: "pvc",
 		aaBehavior:  aa.AffinityAssistantPerWorkspace,
-		expectedErr: fmt.Errorf("%w: failed to create PVC pvc-b9eea16dce: error creating persistentvolumeclaims", ErrPvcCreationFailed),
+		expectedErr: fmt.Errorf("%w for pvc-b9eea16dce: error creating persistentvolumeclaims", ErrPvcCreationFailed),
 	}, {
 		name:        "pvc creation failed - disabled",
 		failureType: "pvc",
 		aaBehavior:  aa.AffinityAssistantDisabled,
-		expectedErr: fmt.Errorf("%w: failed to create PVC pvc-b9eea16dce: error creating persistentvolumeclaims", ErrPvcCreationFailed),
+		expectedErr: fmt.Errorf("%w for pvc-b9eea16dce: error creating persistentvolumeclaims", ErrPvcCreationFailed),
 	}}
 
 	for _, tc := range testCases {
-		ctx := t.Context()
-		kubeClientSet := fakek8s.NewSimpleClientset()
-		c := Reconciler{
-			KubeClientSet: kubeClientSet,
-			pvcHandler:    volumeclaim.NewPVCHandler(kubeClientSet, zap.NewExample().Sugar()),
-		}
+		t.Run(tc.name, func(t *testing.T) {
+			ctx := t.Context()
+			kubeClientSet := fakek8s.NewSimpleClientset()
+			c := Reconciler{
+				KubeClientSet: kubeClientSet,
+				pvcHandler:    volumeclaim.NewPVCHandler(kubeClientSet, zap.NewExample().Sugar()),
+			}
 
-		switch tc.failureType {
-		case "pvc":
-			c.KubeClientSet.CoreV1().(*fake.FakeCoreV1).PrependReactor("create", "persistentvolumeclaims",
-				func(action testing2.Action) (handled bool, ret runtime.Object, err error) {
-					return true, &corev1.PersistentVolumeClaim{}, errors.New("error creating persistentvolumeclaims")
-				})
-		case "statefulset":
-			c.KubeClientSet.CoreV1().(*fake.FakeCoreV1).PrependReactor("create", "statefulsets",
-				func(action testing2.Action) (handled bool, ret runtime.Object, err error) {
-					return true, &appsv1.StatefulSet{}, errors.New("error creating statefulsets")
-				})
-		}
+			switch tc.failureType {
+			case "pvc":
+				c.KubeClientSet.CoreV1().(*fake.FakeCoreV1).PrependReactor("create", "persistentvolumeclaims",
+					func(action testing2.Action) (handled bool, ret runtime.Object, err error) {
+						return true, &corev1.PersistentVolumeClaim{}, errors.New("error creating persistentvolumeclaims")
+					})
+			case "statefulset":
+				c.KubeClientSet.CoreV1().(*fake.FakeCoreV1).PrependReactor("create", "statefulsets",
+					func(action testing2.Action) (handled bool, ret runtime.Object, err error) {
+						return true, &appsv1.StatefulSet{}, errors.New("error creating statefulsets")
+					})
+			}
 
-		err := c.createOrUpdateAffinityAssistantsAndPVCs(ctx, testPRWithVolumeClaimTemplate, tc.aaBehavior)
+			err := c.createOrUpdateAffinityAssistantsAndPVCs(ctx, testPRWithVolumeClaimTemplate, tc.aaBehavior)
 
-		if err == nil {
-			t.Errorf("expect error from createOrUpdateAffinityAssistantsAndPVCs but got nil")
-		}
+			if err == nil {
+				t.Errorf("expect error from createOrUpdateAffinityAssistantsAndPVCs but got nil")
+			}
 
-		switch tc.failureType {
-		case "pvc":
-			if !errors.Is(err, ErrPvcCreationFailed) {
-				t.Errorf("expected err type mismatching, expecting %v but got: %v", ErrPvcCreationFailed, err)
+			switch tc.failureType {
+			case "pvc":
+				if !errors.Is(err, ErrPvcCreationFailed) {
+					t.Errorf("expected err type mismatching, expecting %v but got: %v", ErrPvcCreationFailed, err)
+				}
+			case "statefulset":
+				if !errors.Is(err, ErrAffinityAssistantCreationFailed) {
+					t.Errorf("expected err type mismatching, expecting %v but got: %v", ErrAffinityAssistantCreationFailed, err)
+				}
 			}
-		case "statefulset":
-			if !errors.Is(err, ErrAffinityAssistantCreationFailed) {
-				t.Errorf("expected err type mismatching, expecting %v but got: %v", ErrAffinityAssistantCreationFailed, err)
+			if d := cmp.Diff(tc.expectedErr.Error(), err.Error()); d != "" {
+				t.Errorf("expected err mismatching: %v", diff.PrintWantGot(d))
 			}
-		}
-		if d := cmp.Diff(tc.expectedErr.Error(), err.Error()); d != "" {
-			t.Errorf("expected err mismatching: %v", diff.PrintWantGot(d))
-		}
+		})
 	}
 }
 

pkg/reconciler/pipelinerun/pipelinerun.go

@@ -758,6 +758,10 @@ func (c *Reconciler) reconcile(ctx context.Context, pr *v1.PipelineRun, getPipel
 				pr.Status.MarkFailed(volumeclaim.ReasonCouldntCreateWorkspacePVC,
 					"Failed to create PVC for PipelineRun %s/%s correctly: %s",
 					pr.Namespace, pr.Name, err)
+			case errors.Is(err, ErrPvcCreationFailedRetryable):
+				logger.Errorf("Failed to create PVC for PipelineRun %s: %v", pr.Name, err)
+				pr.Status.MarkRunning(ReasonPending, "Waiting for PVC creation to succeed: %v", err)
+				return err // not a permanent error, will requeue
 			case errors.Is(err, ErrAffinityAssistantCreationFailed):
 				logger.Errorf("Failed to create affinity assistant StatefulSet for PipelineRun %s: %v", pr.Name, err)
 				pr.Status.MarkFailed(ReasonCouldntCreateOrUpdateAffinityAssistantStatefulSet,

pkg/reconciler/volumeclaim/pvchandler.go

@@ -21,7 +21,9 @@ import (
 	"crypto/sha256"
 	"encoding/hex"
 	"encoding/json"
+	"errors"
 	"fmt"
+	"strings"
 
 	v1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1"
 	"go.uber.org/zap"
@@ -39,6 +41,11 @@ const (
 	ReasonCouldntCreateWorkspacePVC = "CouldntCreateWorkspacePVC"
 )
 
+var (
+	ErrPvcCreationFailed          = errors.New("PVC creation error")
+	ErrPvcCreationFailedRetryable = errors.New("PVC creation error, retryable")
+)
+
 // PvcHandler is used to create PVCs for workspaces
 type PvcHandler interface {
 	CreatePVCFromVolumeClaimTemplate(ctx context.Context, wb v1.WorkspaceBinding, ownerReference metav1.OwnerReference, namespace string) error
@@ -73,14 +80,20 @@ func (c *defaultPVCHandler) CreatePVCFromVolumeClaimTemplate(ctx context.Context
 			if apierrors.IsAlreadyExists(err) {
 				c.logger.Infof("Tried to create PersistentVolumeClaim %s in namespace %s, but it already exists",
 					claim.Name, claim.Namespace)
+			} else if apierrors.IsForbidden(err) {
+				if strings.Contains(err.Error(), "exceeded quota") {
+					// This is a retry-able error
+					return fmt.Errorf("%w: %v", ErrPvcCreationFailedRetryable, err.Error())
+				}
+				return fmt.Errorf("%w for %s: %w", ErrPvcCreationFailed, claim.Name, err)
 			} else {
-				return fmt.Errorf("failed to create PVC %s: %w", claim.Name, err)
+				return fmt.Errorf("%w for %s: %w", ErrPvcCreationFailed, claim.Name, err)
 			}
 		} else {
 			c.logger.Infof("Created PersistentVolumeClaim %s in namespace %s", claim.Name, claim.Namespace)
 		}
 	case err != nil:
-		return fmt.Errorf("failed to retrieve PVC %s: %w", claim.Name, err)
+		return fmt.Errorf("%w: failed to retrieve PVC %s: %w", ErrPvcCreationFailed, claim.Name, err)
 	}
 
 	return nil