From 2f585f31957b8bde0302cd5c592a745e8cb33558 Mon Sep 17 00:00:00 2001 From: Ankitsinghsisodya Date: Sat, 4 Apr 2026 23:24:46 +0530 Subject: [PATCH 1/5] fix: clean PVC source workspace before each on-cluster build The named PVC reused across builds was accumulating stale source files, eventually causing 'No space left on device' errors during git-init. - Add CleanAndUploadToVolume which removes the source/ directory before re-extracting the tar stream, leaving the cache/ subpath intact - Switch the direct-upload path to use CleanAndUploadToVolume - Pass deleteExisting: "true" to the git-clone StepAction in both buildpack and s2i task templates so the git path also gets a clean slate Fixes #3516 --- pkg/k8s/persistent_volumes.go | 8 ++++++++ pkg/pipelines/tekton/pipelines_provider.go | 2 +- pkg/pipelines/tekton/task-buildpack.yaml.tmpl | 2 ++ pkg/pipelines/tekton/task-s2i.yaml.tmpl | 2 ++ 4 files changed, 13 insertions(+), 1 deletion(-) diff --git a/pkg/k8s/persistent_volumes.go b/pkg/k8s/persistent_volumes.go index c50ae15511..4b3131622d 100644 --- a/pkg/k8s/persistent_volumes.go +++ b/pkg/k8s/persistent_volumes.go @@ -79,6 +79,14 @@ func UploadToVolume(ctx context.Context, content io.Reader, claimName, namespace return runWithVolumeMounted(ctx, TarImage, []string{"sh", "-c", "umask 0000 && exec tar -xmf -"}, content, claimName, namespace) } +// CleanAndUploadToVolume removes the "source" directory from the volume root, +// then extracts the provided tar stream into the volume. The "cache" +// subdirectory is intentionally left intact so that build-layer caches +// accumulated by previous runs are preserved. +func CleanAndUploadToVolume(ctx context.Context, content io.Reader, claimName, namespace string) error { + return runWithVolumeMounted(ctx, TarImage, []string{"sh", "-c", "umask 0000 && rm -rf source && exec tar -xmf -"}, content, claimName, namespace) +} + // Runs a pod with given image, command and stdin // while having the volume mounted and working directory set to it. func runWithVolumeMounted(ctx context.Context, podImage string, podCommand []string, podInput io.Reader, claimName, namespace string) error { diff --git a/pkg/pipelines/tekton/pipelines_provider.go b/pkg/pipelines/tekton/pipelines_provider.go index 540f995c0d..93713e4bc1 100644 --- a/pkg/pipelines/tekton/pipelines_provider.go +++ b/pkg/pipelines/tekton/pipelines_provider.go @@ -160,7 +160,7 @@ func (pp *PipelinesProvider) Run(ctx context.Context, f fn.Function) (string, fn // Use direct upload to PVC if Git is not set up. content := sourcesAsTarStream(f) defer content.Close() - err = k8s.UploadToVolume(ctx, content, getPipelinePvcName(f), namespace) + err = k8s.CleanAndUploadToVolume(ctx, content, getPipelinePvcName(f), namespace) if err != nil { return "", f, fmt.Errorf("cannot upload sources to the PVC: %w", err) } diff --git a/pkg/pipelines/tekton/task-buildpack.yaml.tmpl b/pkg/pipelines/tekton/task-buildpack.yaml.tmpl index abda863372..56c9b3d5f6 100644 --- a/pkg/pipelines/tekton/task-buildpack.yaml.tmpl +++ b/pkg/pipelines/tekton/task-buildpack.yaml.tmpl @@ -87,6 +87,8 @@ spec: value: "$(params.GIT_REPOSITORY)" - name: "revision" value: "$(params.GIT_REVISION)" + - name: "deleteExisting" + value: "true" - name: func-scaffold image: '{{.ScaffoldImage}}' workingDir: $(workspaces.source.path) diff --git a/pkg/pipelines/tekton/task-s2i.yaml.tmpl b/pkg/pipelines/tekton/task-s2i.yaml.tmpl index b79c86d239..c9ca736d9d 100644 --- a/pkg/pipelines/tekton/task-s2i.yaml.tmpl +++ b/pkg/pipelines/tekton/task-s2i.yaml.tmpl @@ -75,6 +75,8 @@ spec: value: "$(params.GIT_REPOSITORY)" - name: "revision" value: "$(params.GIT_REVISION)" + - name: "deleteExisting" + value: "true" - name: func-scaffold image: '{{.ScaffoldImage}}' workingDir: $(workspaces.source.path) From eb7d0fd1bad01c81fa1a969a43f8751b8774562e Mon Sep 17 00:00:00 2001 From: Ankitsinghsisodya Date: Sun, 5 Apr 2026 04:14:30 +0530 Subject: [PATCH 2/5] fix: separate source and cache PVCs to prevent on-cluster build disk full MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Root cause: source/ and cache/ shared a single 256Mi PVC. The buildpack layer cache (cache/) grows across builds while source/ is already cleaned by git-clone's deleteExisting default. Once cache/ consumes all available space, git-clone fails with "No space left on device" during git-init. Changes: - Add getPipelineCachePvcName and a by-name DeletePersistentVolumeClaim - Standard pipeline: delete and recreate the source PVC before every build for a guaranteed clean workspace; create a separate persistent cache PVC so the buildpack layer cache survives across builds - PAC pipeline: use volumeClaimTemplate for the source workspace so Tekton provisions a fresh, ephemeral source PVC per run and cleans it up automatically; create a persistent cache PVC at repo-config time - Revert the redundant deleteExisting: "true" additions to both task templates — the pinned git-clone StepAction already defaults to true Fixes #3516 --- pkg/k8s/persistent_volumes.go | 14 ++++++++++ .../tekton/pipelines_pac_provider.go | 6 ++++ pkg/pipelines/tekton/pipelines_provider.go | 28 +++++++++++++++++-- pkg/pipelines/tekton/resources.go | 4 +++ pkg/pipelines/tekton/task-buildpack.yaml.tmpl | 2 -- pkg/pipelines/tekton/task-s2i.yaml.tmpl | 2 -- pkg/pipelines/tekton/templates.go | 15 ++++++++++ pkg/pipelines/tekton/templates_pack.go | 16 +++++------ pkg/pipelines/tekton/templates_s2i.go | 16 +++++------ 9 files changed, 81 insertions(+), 22 deletions(-) diff --git a/pkg/k8s/persistent_volumes.go b/pkg/k8s/persistent_volumes.go index 4b3131622d..5964f614fe 100644 --- a/pkg/k8s/persistent_volumes.go +++ b/pkg/k8s/persistent_volumes.go @@ -72,6 +72,20 @@ func DeletePersistentVolumeClaims(ctx context.Context, namespaceOverride string, return client.CoreV1().PersistentVolumeClaims(namespace).DeleteCollection(ctx, metav1.DeleteOptions{}, listOptions) } +// DeletePersistentVolumeClaim deletes a single PVC by name. It returns nil if +// the PVC does not exist. +func DeletePersistentVolumeClaim(ctx context.Context, name, namespaceOverride string) error { + client, namespace, err := NewClientAndResolvedNamespace(namespaceOverride) + if err != nil { + return err + } + err = client.CoreV1().PersistentVolumeClaims(namespace).Delete(ctx, name, metav1.DeleteOptions{}) + if k8serrors.IsNotFound(err) { + return nil + } + return err +} + var TarImage = "ghcr.io/knative/func-utils:v2" // UploadToVolume uploads files (passed in form of tar stream) into volume. diff --git a/pkg/pipelines/tekton/pipelines_pac_provider.go b/pkg/pipelines/tekton/pipelines_pac_provider.go index c67f6cbb0b..45c2ffd7e8 100644 --- a/pkg/pipelines/tekton/pipelines_pac_provider.go +++ b/pkg/pipelines/tekton/pipelines_pac_provider.go @@ -197,6 +197,12 @@ func (pp *PipelinesProvider) createClusterPACResources(ctx context.Context, f fn } fmt.Printf(" ✅ Persistent Volume is present on the cluster with name %q\n", getPipelinePvcName(f)) + err = createPipelineCachePersistentVolumeClaim(ctx, f, namespace, labels) + if err != nil { + return err + } + fmt.Printf(" ✅ Cache Persistent Volume is present on the cluster with name %q\n", getPipelineCachePvcName(f)) + err = ensurePACSecretExists(ctx, f, namespace, metadata, labels) if err != nil { return err diff --git a/pkg/pipelines/tekton/pipelines_provider.go b/pkg/pipelines/tekton/pipelines_provider.go index 93713e4bc1..2efbf809bf 100644 --- a/pkg/pipelines/tekton/pipelines_provider.go +++ b/pkg/pipelines/tekton/pipelines_provider.go @@ -151,8 +151,15 @@ func (pp *PipelinesProvider) Run(ctx context.Context, f fn.Function) (string, fn labels = pp.decorator.UpdateLabels(f, labels) } - err = createPipelinePersistentVolumeClaim(ctx, f, namespace, labels) - if err != nil { + // Delete any existing source PVC so each build starts with a clean workspace. + // The cache PVC is kept across builds so the buildpack layer cache is preserved. + if err = k8s.DeletePersistentVolumeClaim(ctx, getPipelinePvcName(f), namespace); err != nil { + return "", f, fmt.Errorf("cannot clean source PVC: %w", err) + } + if err = createPipelinePersistentVolumeClaim(ctx, f, namespace, labels); err != nil { + return "", f, err + } + if err = createPipelineCachePersistentVolumeClaim(ctx, f, namespace, labels); err != nil { return "", f, err } @@ -573,3 +580,20 @@ func createPipelinePersistentVolumeClaim(ctx context.Context, f fn.Function, nam } return nil } + +// createPipelineCachePersistentVolumeClaim creates the dedicated cache PVC that +// persists across builds so the buildpack layer cache is not lost. +func createPipelineCachePersistentVolumeClaim(ctx context.Context, f fn.Function, namespace string, labels map[string]string) error { + var err error + pvcs := DefaultPersistentVolumeClaimSize + if f.Build.PVCSize != "" { + if pvcs, err = resource.ParseQuantity(f.Build.PVCSize); err != nil { + return fmt.Errorf("cache PVC size value could not be parsed. %w", err) + } + } + err = createPersistentVolumeClaim(ctx, getPipelineCachePvcName(f), namespace, labels, f.Deploy.Annotations, corev1.ReadWriteOnce, pvcs, f.Build.RemoteStorageClass) + if err != nil && !k8serrors.IsAlreadyExists(err) { + return fmt.Errorf("problem creating cache persistent volume claim: %v", err) + } + return nil +} diff --git a/pkg/pipelines/tekton/resources.go b/pkg/pipelines/tekton/resources.go index be9c36b10a..61d714b5c8 100644 --- a/pkg/pipelines/tekton/resources.go +++ b/pkg/pipelines/tekton/resources.go @@ -78,3 +78,7 @@ func getPipelineSecretName(f fn.Function) string { func getPipelinePvcName(f fn.Function) string { return fmt.Sprintf("%s-pvc", getPipelineName(f)) } + +func getPipelineCachePvcName(f fn.Function) string { + return fmt.Sprintf("%s-cache-pvc", getPipelineName(f)) +} diff --git a/pkg/pipelines/tekton/task-buildpack.yaml.tmpl b/pkg/pipelines/tekton/task-buildpack.yaml.tmpl index 56c9b3d5f6..abda863372 100644 --- a/pkg/pipelines/tekton/task-buildpack.yaml.tmpl +++ b/pkg/pipelines/tekton/task-buildpack.yaml.tmpl @@ -87,8 +87,6 @@ spec: value: "$(params.GIT_REPOSITORY)" - name: "revision" value: "$(params.GIT_REVISION)" - - name: "deleteExisting" - value: "true" - name: func-scaffold image: '{{.ScaffoldImage}}' workingDir: $(workspaces.source.path) diff --git a/pkg/pipelines/tekton/task-s2i.yaml.tmpl b/pkg/pipelines/tekton/task-s2i.yaml.tmpl index c9ca736d9d..b79c86d239 100644 --- a/pkg/pipelines/tekton/task-s2i.yaml.tmpl +++ b/pkg/pipelines/tekton/task-s2i.yaml.tmpl @@ -75,8 +75,6 @@ spec: value: "$(params.GIT_REPOSITORY)" - name: "revision" value: "$(params.GIT_REVISION)" - - name: "deleteExisting" - value: "true" - name: func-scaffold image: '{{.ScaffoldImage}}' workingDir: $(workspaces.source.path) diff --git a/pkg/pipelines/tekton/templates.go b/pkg/pipelines/tekton/templates.go index c199f7ffac..5343c7f3fc 100644 --- a/pkg/pipelines/tekton/templates.go +++ b/pkg/pipelines/tekton/templates.go @@ -70,6 +70,8 @@ type templateData struct { PipelineName string PipelineRunName string PvcName string + CachePvcName string + PvcSize string SecretName string // The branch or tag we are targeting with Pipelines (ie: main, refs/tags/*) @@ -184,6 +186,8 @@ func createPipelineRunTemplatePAC(f fn.Function, labels map[string]string) error PipelineName: getPipelineName(f), PipelineRunName: fmt.Sprintf("%s-run", getPipelineName(f)), PvcName: getPipelinePvcName(f), + CachePvcName: getPipelineCachePvcName(f), + PvcSize: pipelinePvcSize(f), SecretName: getPipelineSecretName(f), PipelinesTargetBranch: pipelinesTargetBranch, @@ -383,6 +387,8 @@ func createAndApplyPipelineRunTemplate(f fn.Function, namespace string, labels m PipelineName: getPipelineName(f), PipelineRunName: getPipelineRunGenerateName(f), PvcName: getPipelinePvcName(f), + CachePvcName: getPipelineCachePvcName(f), + PvcSize: pipelinePvcSize(f), SecretName: getPipelineSecretName(f), S2iImageScriptsUrl: s2iImageScriptsUrl, @@ -469,3 +475,12 @@ func createAndApplyResource(projectRoot, fileName, fileTemplate, kind, resourceN return m.Apply() } + +// pipelinePvcSize returns the PVC size string for use in templates. +// It returns the user-configured value or the default (256Mi). +func pipelinePvcSize(f fn.Function) string { + if f.Build.PVCSize != "" { + return f.Build.PVCSize + } + return DefaultPersistentVolumeClaimSize.String() +} diff --git a/pkg/pipelines/tekton/templates_pack.go b/pkg/pipelines/tekton/templates_pack.go index 30ac671d11..0afc0b3a75 100644 --- a/pkg/pipelines/tekton/templates_pack.go +++ b/pkg/pipelines/tekton/templates_pack.go @@ -117,11 +117,9 @@ spec: - name: source-workspace persistentVolumeClaim: claimName: {{.PvcName}} - subPath: source - name: cache-workspace persistentVolumeClaim: - claimName: {{.PvcName}} - subPath: cache + claimName: {{.CachePvcName}} - name: dockerconfig-workspace secret: secretName: {{.SecretName}} @@ -175,13 +173,15 @@ spec: name: {{.PipelineName}} workspaces: - name: source-workspace - persistentVolumeClaim: - claimName: {{.PvcName}} - subPath: source + volumeClaimTemplate: + spec: + accessModes: ["ReadWriteOnce"] + resources: + requests: + storage: {{.PvcSize}} - name: cache-workspace persistentVolumeClaim: - claimName: {{.PvcName}} - subPath: cache + claimName: {{.CachePvcName}} - name: dockerconfig-workspace secret: secretName: {{.SecretName}} diff --git a/pkg/pipelines/tekton/templates_s2i.go b/pkg/pipelines/tekton/templates_s2i.go index 9441a9c5c6..7a0c774146 100644 --- a/pkg/pipelines/tekton/templates_s2i.go +++ b/pkg/pipelines/tekton/templates_s2i.go @@ -132,11 +132,9 @@ spec: - name: source-workspace persistentVolumeClaim: claimName: {{.PvcName}} - subPath: source - name: cache-workspace persistentVolumeClaim: - claimName: {{.PvcName}} - subPath: cache + claimName: {{.CachePvcName}} - name: dockerconfig-workspace secret: secretName: {{.SecretName}} @@ -197,13 +195,15 @@ spec: name: {{.PipelineName}} workspaces: - name: source-workspace - persistentVolumeClaim: - claimName: {{.PvcName}} - subPath: source + volumeClaimTemplate: + spec: + accessModes: ["ReadWriteOnce"] + resources: + requests: + storage: {{.PvcSize}} - name: cache-workspace persistentVolumeClaim: - claimName: {{.PvcName}} - subPath: cache + claimName: {{.CachePvcName}} - name: dockerconfig-workspace secret: secretName: {{.SecretName}} From 7c1e55cf02302857d8da3238c488809d2c8812f9 Mon Sep 17 00:00:00 2001 From: Ankitsinghsisodya Date: Sun, 5 Apr 2026 04:45:43 +0530 Subject: [PATCH 3/5] fix: correct three regressions introduced by PVC separation - Restore subPath: source on the standard PipelineRun source-workspace binding; sourcesAsTarStream archives under source/ so the workspace mount must point at that subPath for the task to find func.yaml at the expected path - Honour build.remoteStorageClass in PAC volumeClaimTemplate by adding a StorageClassName field to templateData and conditionally emitting storageClassName in both pack and s2i PAC PipelineRun templates - Remove the dead source PVC creation from createClusterPACResources; PAC PipelineRuns now bind source-workspace via volumeClaimTemplate so the named source PVC was provisioned but never mounted --- pkg/pipelines/tekton/pipelines_pac_provider.go | 6 ------ pkg/pipelines/tekton/templates.go | 14 ++++++++------ pkg/pipelines/tekton/templates_pack.go | 4 ++++ pkg/pipelines/tekton/templates_s2i.go | 4 ++++ 4 files changed, 16 insertions(+), 12 deletions(-) diff --git a/pkg/pipelines/tekton/pipelines_pac_provider.go b/pkg/pipelines/tekton/pipelines_pac_provider.go index 45c2ffd7e8..402f7d268b 100644 --- a/pkg/pipelines/tekton/pipelines_pac_provider.go +++ b/pkg/pipelines/tekton/pipelines_pac_provider.go @@ -191,12 +191,6 @@ func (pp *PipelinesProvider) createClusterPACResources(ctx context.Context, f fn metadata.RegistryPassword = creds.Password metadata.RegistryServer = registry - err = createPipelinePersistentVolumeClaim(ctx, f, namespace, labels) - if err != nil { - return err - } - fmt.Printf(" ✅ Persistent Volume is present on the cluster with name %q\n", getPipelinePvcName(f)) - err = createPipelineCachePersistentVolumeClaim(ctx, f, namespace, labels) if err != nil { return err diff --git a/pkg/pipelines/tekton/templates.go b/pkg/pipelines/tekton/templates.go index 5343c7f3fc..b19a199357 100644 --- a/pkg/pipelines/tekton/templates.go +++ b/pkg/pipelines/tekton/templates.go @@ -72,6 +72,7 @@ type templateData struct { PvcName string CachePvcName string PvcSize string + StorageClassName string SecretName string // The branch or tag we are targeting with Pipelines (ie: main, refs/tags/*) @@ -183,12 +184,13 @@ func createPipelineRunTemplatePAC(f fn.Function, labels map[string]string) error BuilderImage: getBuilderImage(f), BuildEnvs: buildEnvs, - PipelineName: getPipelineName(f), - PipelineRunName: fmt.Sprintf("%s-run", getPipelineName(f)), - PvcName: getPipelinePvcName(f), - CachePvcName: getPipelineCachePvcName(f), - PvcSize: pipelinePvcSize(f), - SecretName: getPipelineSecretName(f), + PipelineName: getPipelineName(f), + PipelineRunName: fmt.Sprintf("%s-run", getPipelineName(f)), + PvcName: getPipelinePvcName(f), + CachePvcName: getPipelineCachePvcName(f), + PvcSize: pipelinePvcSize(f), + StorageClassName: f.Build.RemoteStorageClass, + SecretName: getPipelineSecretName(f), PipelinesTargetBranch: pipelinesTargetBranch, diff --git a/pkg/pipelines/tekton/templates_pack.go b/pkg/pipelines/tekton/templates_pack.go index 0afc0b3a75..ef667229a4 100644 --- a/pkg/pipelines/tekton/templates_pack.go +++ b/pkg/pipelines/tekton/templates_pack.go @@ -117,6 +117,7 @@ spec: - name: source-workspace persistentVolumeClaim: claimName: {{.PvcName}} + subPath: source - name: cache-workspace persistentVolumeClaim: claimName: {{.CachePvcName}} @@ -176,6 +177,9 @@ spec: volumeClaimTemplate: spec: accessModes: ["ReadWriteOnce"] + {{- if .StorageClassName}} + storageClassName: {{.StorageClassName}} + {{- end}} resources: requests: storage: {{.PvcSize}} diff --git a/pkg/pipelines/tekton/templates_s2i.go b/pkg/pipelines/tekton/templates_s2i.go index 7a0c774146..af56a627f6 100644 --- a/pkg/pipelines/tekton/templates_s2i.go +++ b/pkg/pipelines/tekton/templates_s2i.go @@ -132,6 +132,7 @@ spec: - name: source-workspace persistentVolumeClaim: claimName: {{.PvcName}} + subPath: source - name: cache-workspace persistentVolumeClaim: claimName: {{.CachePvcName}} @@ -198,6 +199,9 @@ spec: volumeClaimTemplate: spec: accessModes: ["ReadWriteOnce"] + {{- if .StorageClassName}} + storageClassName: {{.StorageClassName}} + {{- end}} resources: requests: storage: {{.PvcSize}} From 14e08283f4606fcfe64cd365546700381bbc0b49 Mon Sep 17 00:00:00 2001 From: Ankitsinghsisodya Date: Sun, 5 Apr 2026 05:55:47 +0530 Subject: [PATCH 4/5] fix: wait for PVC termination and rotate cache PVC between builds MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two bugs: 1. Race condition: DeletePersistentVolumeClaim is asynchronous. The PVC enters Terminating state and the subsequent Create call was receiving AlreadyExists, treating it as success, and proceeding against a terminating object. Fix: add WaitForPVCDeletion which polls until the PVC is NotFound before returning, and call it after every delete. 2. Cache accumulation: the dedicated cache PVC was explicitly kept across builds and never trimmed. CNB does not automatically evict unused layers so the cache PVC would grow unboundedly over time — the same disk-full failure the fix was meant to prevent, just deferred. Fix: rotate both source and cache PVCs before every standard build. For PAC, switch cache workspace to volumeClaimTemplate so both workspaces are ephemeral per run with no long-lived PVCs on the cluster at all. --- pkg/k8s/persistent_volumes.go | 27 +++++++++++++++++++ .../tekton/pipelines_pac_provider.go | 6 ----- pkg/pipelines/tekton/pipelines_provider.go | 16 ++++++++--- pkg/pipelines/tekton/templates_pack.go | 11 ++++++-- pkg/pipelines/tekton/templates_s2i.go | 11 ++++++-- 5 files changed, 57 insertions(+), 14 deletions(-) diff --git a/pkg/k8s/persistent_volumes.go b/pkg/k8s/persistent_volumes.go index 5964f614fe..59d1b781fe 100644 --- a/pkg/k8s/persistent_volumes.go +++ b/pkg/k8s/persistent_volumes.go @@ -86,6 +86,33 @@ func DeletePersistentVolumeClaim(ctx context.Context, name, namespaceOverride st return err } +// WaitForPVCDeletion polls until the named PVC no longer exists or the context +// is cancelled. PVC deletion in Kubernetes is asynchronous — the object enters +// a Terminating phase before it disappears. Callers that need to recreate a +// PVC with the same name must wait for the old one to fully terminate first; +// otherwise the Create call receives AlreadyExists and silently reuses the +// terminating object. +func WaitForPVCDeletion(ctx context.Context, name, namespaceOverride string) error { + client, namespace, err := NewClientAndResolvedNamespace(namespaceOverride) + if err != nil { + return err + } + for { + _, err = client.CoreV1().PersistentVolumeClaims(namespace).Get(ctx, name, metav1.GetOptions{}) + if k8serrors.IsNotFound(err) { + return nil + } + if err != nil { + return err + } + select { + case <-ctx.Done(): + return fmt.Errorf("timed out waiting for PVC %q to terminate: %w", name, ctx.Err()) + case <-time.After(2 * time.Second): + } + } +} + var TarImage = "ghcr.io/knative/func-utils:v2" // UploadToVolume uploads files (passed in form of tar stream) into volume. diff --git a/pkg/pipelines/tekton/pipelines_pac_provider.go b/pkg/pipelines/tekton/pipelines_pac_provider.go index 402f7d268b..5d7dc502ea 100644 --- a/pkg/pipelines/tekton/pipelines_pac_provider.go +++ b/pkg/pipelines/tekton/pipelines_pac_provider.go @@ -191,12 +191,6 @@ func (pp *PipelinesProvider) createClusterPACResources(ctx context.Context, f fn metadata.RegistryPassword = creds.Password metadata.RegistryServer = registry - err = createPipelineCachePersistentVolumeClaim(ctx, f, namespace, labels) - if err != nil { - return err - } - fmt.Printf(" ✅ Cache Persistent Volume is present on the cluster with name %q\n", getPipelineCachePvcName(f)) - err = ensurePACSecretExists(ctx, f, namespace, metadata, labels) if err != nil { return err diff --git a/pkg/pipelines/tekton/pipelines_provider.go b/pkg/pipelines/tekton/pipelines_provider.go index 2efbf809bf..b4725cf977 100644 --- a/pkg/pipelines/tekton/pipelines_provider.go +++ b/pkg/pipelines/tekton/pipelines_provider.go @@ -151,10 +151,18 @@ func (pp *PipelinesProvider) Run(ctx context.Context, f fn.Function) (string, fn labels = pp.decorator.UpdateLabels(f, labels) } - // Delete any existing source PVC so each build starts with a clean workspace. - // The cache PVC is kept across builds so the buildpack layer cache is preserved. - if err = k8s.DeletePersistentVolumeClaim(ctx, getPipelinePvcName(f), namespace); err != nil { - return "", f, fmt.Errorf("cannot clean source PVC: %w", err) + // Rotate both PVCs before every build so neither source nor cache can + // accumulate data across runs. Deletion is asynchronous in Kubernetes — + // we wait for each PVC to fully terminate before recreating it, otherwise + // the subsequent Create receives AlreadyExists (PVC still terminating) and + // silently reuses the old, unusable object. + for _, pvcName := range []string{getPipelinePvcName(f), getPipelineCachePvcName(f)} { + if err = k8s.DeletePersistentVolumeClaim(ctx, pvcName, namespace); err != nil { + return "", f, fmt.Errorf("cannot delete PVC %q: %w", pvcName, err) + } + if err = k8s.WaitForPVCDeletion(ctx, pvcName, namespace); err != nil { + return "", f, fmt.Errorf("PVC %q did not finish terminating: %w", pvcName, err) + } } if err = createPipelinePersistentVolumeClaim(ctx, f, namespace, labels); err != nil { return "", f, err diff --git a/pkg/pipelines/tekton/templates_pack.go b/pkg/pipelines/tekton/templates_pack.go index ef667229a4..950297755e 100644 --- a/pkg/pipelines/tekton/templates_pack.go +++ b/pkg/pipelines/tekton/templates_pack.go @@ -184,8 +184,15 @@ spec: requests: storage: {{.PvcSize}} - name: cache-workspace - persistentVolumeClaim: - claimName: {{.CachePvcName}} + volumeClaimTemplate: + spec: + accessModes: ["ReadWriteOnce"] + {{- if .StorageClassName}} + storageClassName: {{.StorageClassName}} + {{- end}} + resources: + requests: + storage: {{.PvcSize}} - name: dockerconfig-workspace secret: secretName: {{.SecretName}} diff --git a/pkg/pipelines/tekton/templates_s2i.go b/pkg/pipelines/tekton/templates_s2i.go index af56a627f6..c0a6ed90db 100644 --- a/pkg/pipelines/tekton/templates_s2i.go +++ b/pkg/pipelines/tekton/templates_s2i.go @@ -206,8 +206,15 @@ spec: requests: storage: {{.PvcSize}} - name: cache-workspace - persistentVolumeClaim: - claimName: {{.CachePvcName}} + volumeClaimTemplate: + spec: + accessModes: ["ReadWriteOnce"] + {{- if .StorageClassName}} + storageClassName: {{.StorageClassName}} + {{- end}} + resources: + requests: + storage: {{.PvcSize}} - name: dockerconfig-workspace secret: secretName: {{.SecretName}} From 86cd2c5ffd33c56eb230ad570617fd4158b733b3 Mon Sep 17 00:00:00 2001 From: Ankitsinghsisodya Date: Sun, 5 Apr 2026 14:45:17 +0530 Subject: [PATCH 5/5] fix: use volumeClaimTemplate for cache and harden AlreadyExists on recreate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two bugs: 1. coschedule constraint: both standard pipeline templates bound source and cache to distinct named PVCs. Under Tekton's stable coschedule:workspaces mode the affinity assistant creates one pod per PVC and tries to pin the same TaskRun to both nodes — an impossible constraint. Fix: switch cache to volumeClaimTemplate in both standard templates (matching PAC). Now each run has exactly one named PVC (source, required for pre-upload) and one Tekton-managed ephemeral PVC (cache), eliminating the conflicting affinity. Remove the now-dead createPipelineCachePersistentVolumeClaim, getPipelineCachePvcName, and CachePvcName templateData field. 2. AlreadyExists race: after delete+WaitForPVCDeletion a concurrent deploy can create the PVC in the window before our Create call. The suppressed IsAlreadyExists check would silently reuse that PVC, giving both deploys a false clean-workspace guarantee. Fix: remove the IsAlreadyExists suppression — return any Create error and let the caller retry. Update the corresponding test to assert the new wantErr:true behaviour. --- pkg/pipelines/tekton/pipelines_provider.go | 46 +++++-------------- .../tekton/pipelines_provider_test.go | 6 ++- pkg/pipelines/tekton/resources.go | 4 -- pkg/pipelines/tekton/templates.go | 24 +++++----- pkg/pipelines/tekton/templates_pack.go | 11 ++++- pkg/pipelines/tekton/templates_s2i.go | 11 ++++- 6 files changed, 45 insertions(+), 57 deletions(-) diff --git a/pkg/pipelines/tekton/pipelines_provider.go b/pkg/pipelines/tekton/pipelines_provider.go index b4725cf977..ff46c976e3 100644 --- a/pkg/pipelines/tekton/pipelines_provider.go +++ b/pkg/pipelines/tekton/pipelines_provider.go @@ -151,25 +151,21 @@ func (pp *PipelinesProvider) Run(ctx context.Context, f fn.Function) (string, fn labels = pp.decorator.UpdateLabels(f, labels) } - // Rotate both PVCs before every build so neither source nor cache can - // accumulate data across runs. Deletion is asynchronous in Kubernetes — - // we wait for each PVC to fully terminate before recreating it, otherwise - // the subsequent Create receives AlreadyExists (PVC still terminating) and - // silently reuses the old, unusable object. - for _, pvcName := range []string{getPipelinePvcName(f), getPipelineCachePvcName(f)} { - if err = k8s.DeletePersistentVolumeClaim(ctx, pvcName, namespace); err != nil { - return "", f, fmt.Errorf("cannot delete PVC %q: %w", pvcName, err) - } - if err = k8s.WaitForPVCDeletion(ctx, pvcName, namespace); err != nil { - return "", f, fmt.Errorf("PVC %q did not finish terminating: %w", pvcName, err) - } + // Rotate the source PVC before every build so the workspace is clean. + // Cache uses volumeClaimTemplate in the PipelineRun so Tekton provisions + // and destroys an ephemeral cache PVC per run — no rotation needed here. + // Deletion is asynchronous: wait for the object to reach NotFound before + // recreating so that Create does not receive AlreadyExists on a still- + // terminating PVC and silently reuse it. + if err = k8s.DeletePersistentVolumeClaim(ctx, getPipelinePvcName(f), namespace); err != nil { + return "", f, fmt.Errorf("cannot delete source PVC: %w", err) + } + if err = k8s.WaitForPVCDeletion(ctx, getPipelinePvcName(f), namespace); err != nil { + return "", f, fmt.Errorf("source PVC did not finish terminating: %w", err) } if err = createPipelinePersistentVolumeClaim(ctx, f, namespace, labels); err != nil { return "", f, err } - if err = createPipelineCachePersistentVolumeClaim(ctx, f, namespace, labels); err != nil { - return "", f, err - } if f.Build.Git.URL == "" { // Use direct upload to PVC if Git is not set up. @@ -582,26 +578,8 @@ func createPipelinePersistentVolumeClaim(ctx context.Context, f fn.Function, nam return fmt.Errorf("PVC size value could not be parsed. %w", err) } } - err = createPersistentVolumeClaim(ctx, getPipelinePvcName(f), namespace, labels, f.Deploy.Annotations, corev1.ReadWriteOnce, pvcs, f.Build.RemoteStorageClass) - if err != nil && !k8serrors.IsAlreadyExists(err) { + if err = createPersistentVolumeClaim(ctx, getPipelinePvcName(f), namespace, labels, f.Deploy.Annotations, corev1.ReadWriteOnce, pvcs, f.Build.RemoteStorageClass); err != nil { return fmt.Errorf("problem creating persistent volume claim: %v", err) } return nil } - -// createPipelineCachePersistentVolumeClaim creates the dedicated cache PVC that -// persists across builds so the buildpack layer cache is not lost. -func createPipelineCachePersistentVolumeClaim(ctx context.Context, f fn.Function, namespace string, labels map[string]string) error { - var err error - pvcs := DefaultPersistentVolumeClaimSize - if f.Build.PVCSize != "" { - if pvcs, err = resource.ParseQuantity(f.Build.PVCSize); err != nil { - return fmt.Errorf("cache PVC size value could not be parsed. %w", err) - } - } - err = createPersistentVolumeClaim(ctx, getPipelineCachePvcName(f), namespace, labels, f.Deploy.Annotations, corev1.ReadWriteOnce, pvcs, f.Build.RemoteStorageClass) - if err != nil && !k8serrors.IsAlreadyExists(err) { - return fmt.Errorf("problem creating cache persistent volume claim: %v", err) - } - return nil -} diff --git a/pkg/pipelines/tekton/pipelines_provider_test.go b/pkg/pipelines/tekton/pipelines_provider_test.go index 95fc81374f..3411b2dd66 100644 --- a/pkg/pipelines/tekton/pipelines_provider_test.go +++ b/pkg/pipelines/tekton/pipelines_provider_test.go @@ -104,7 +104,7 @@ func Test_createPipelinePersistentVolumeClaim(t *testing.T) { wantErr: true, }, { - name: "returns nil if pvc already exists", + name: "returns error if pvc already exists", args: args{ ctx: t.Context(), f: fn.Function{}, @@ -115,7 +115,9 @@ func Test_createPipelinePersistentVolumeClaim(t *testing.T) { mock: func(ctx context.Context, name, namespaceOverride string, labels map[string]string, annotations map[string]string, accessMode corev1.PersistentVolumeAccessMode, resourceRequest resource.Quantity, storageClass string) (err error) { return &apiErrors.StatusError{ErrStatus: metav1.Status{Reason: metav1.StatusReasonAlreadyExists}} }, - wantErr: false, + // After delete+WaitForPVCDeletion, AlreadyExists on Create means a + // concurrent deploy snuck in — must not be silently swallowed. + wantErr: true, }, { name: "returns err if namespace not defined and default returns an err", diff --git a/pkg/pipelines/tekton/resources.go b/pkg/pipelines/tekton/resources.go index 61d714b5c8..be9c36b10a 100644 --- a/pkg/pipelines/tekton/resources.go +++ b/pkg/pipelines/tekton/resources.go @@ -78,7 +78,3 @@ func getPipelineSecretName(f fn.Function) string { func getPipelinePvcName(f fn.Function) string { return fmt.Sprintf("%s-pvc", getPipelineName(f)) } - -func getPipelineCachePvcName(f fn.Function) string { - return fmt.Sprintf("%s-cache-pvc", getPipelineName(f)) -} diff --git a/pkg/pipelines/tekton/templates.go b/pkg/pipelines/tekton/templates.go index b19a199357..103281a83b 100644 --- a/pkg/pipelines/tekton/templates.go +++ b/pkg/pipelines/tekton/templates.go @@ -67,13 +67,12 @@ type templateData struct { BuilderImage string BuildEnvs []string - PipelineName string - PipelineRunName string - PvcName string - CachePvcName string - PvcSize string + PipelineName string + PipelineRunName string + PvcName string + PvcSize string StorageClassName string - SecretName string + SecretName string // The branch or tag we are targeting with Pipelines (ie: main, refs/tags/*) PipelinesTargetBranch string @@ -187,7 +186,6 @@ func createPipelineRunTemplatePAC(f fn.Function, labels map[string]string) error PipelineName: getPipelineName(f), PipelineRunName: fmt.Sprintf("%s-run", getPipelineName(f)), PvcName: getPipelinePvcName(f), - CachePvcName: getPipelineCachePvcName(f), PvcSize: pipelinePvcSize(f), StorageClassName: f.Build.RemoteStorageClass, SecretName: getPipelineSecretName(f), @@ -386,12 +384,12 @@ func createAndApplyPipelineRunTemplate(f fn.Function, namespace string, labels m BuilderImage: getBuilderImage(f), BuildEnvs: buildEnvs, - PipelineName: getPipelineName(f), - PipelineRunName: getPipelineRunGenerateName(f), - PvcName: getPipelinePvcName(f), - CachePvcName: getPipelineCachePvcName(f), - PvcSize: pipelinePvcSize(f), - SecretName: getPipelineSecretName(f), + PipelineName: getPipelineName(f), + PipelineRunName: getPipelineRunGenerateName(f), + PvcName: getPipelinePvcName(f), + PvcSize: pipelinePvcSize(f), + StorageClassName: f.Build.RemoteStorageClass, + SecretName: getPipelineSecretName(f), S2iImageScriptsUrl: s2iImageScriptsUrl, TlsVerify: tlsVerify, diff --git a/pkg/pipelines/tekton/templates_pack.go b/pkg/pipelines/tekton/templates_pack.go index 950297755e..3f021a1fb9 100644 --- a/pkg/pipelines/tekton/templates_pack.go +++ b/pkg/pipelines/tekton/templates_pack.go @@ -119,8 +119,15 @@ spec: claimName: {{.PvcName}} subPath: source - name: cache-workspace - persistentVolumeClaim: - claimName: {{.CachePvcName}} + volumeClaimTemplate: + spec: + accessModes: ["ReadWriteOnce"] + {{- if .StorageClassName}} + storageClassName: {{.StorageClassName}} + {{- end}} + resources: + requests: + storage: {{.PvcSize}} - name: dockerconfig-workspace secret: secretName: {{.SecretName}} diff --git a/pkg/pipelines/tekton/templates_s2i.go b/pkg/pipelines/tekton/templates_s2i.go index c0a6ed90db..b7c607c9af 100644 --- a/pkg/pipelines/tekton/templates_s2i.go +++ b/pkg/pipelines/tekton/templates_s2i.go @@ -134,8 +134,15 @@ spec: claimName: {{.PvcName}} subPath: source - name: cache-workspace - persistentVolumeClaim: - claimName: {{.CachePvcName}} + volumeClaimTemplate: + spec: + accessModes: ["ReadWriteOnce"] + {{- if .StorageClassName}} + storageClassName: {{.StorageClassName}} + {{- end}} + resources: + requests: + storage: {{.PvcSize}} - name: dockerconfig-workspace secret: secretName: {{.SecretName}}