diff --git a/pkg/k8s/persistent_volumes.go b/pkg/k8s/persistent_volumes.go index c50ae15511..59d1b781fe 100644 --- a/pkg/k8s/persistent_volumes.go +++ b/pkg/k8s/persistent_volumes.go @@ -72,6 +72,47 @@ 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 +} + +// 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. @@ -79,6 +120,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_pac_provider.go b/pkg/pipelines/tekton/pipelines_pac_provider.go index c67f6cbb0b..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 = 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 = 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 540f995c0d..ff46c976e3 100644 --- a/pkg/pipelines/tekton/pipelines_provider.go +++ b/pkg/pipelines/tekton/pipelines_provider.go @@ -151,8 +151,19 @@ 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 { + // 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 } @@ -160,7 +171,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) } @@ -567,8 +578,7 @@ 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 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/templates.go b/pkg/pipelines/tekton/templates.go index c199f7ffac..103281a83b 100644 --- a/pkg/pipelines/tekton/templates.go +++ b/pkg/pipelines/tekton/templates.go @@ -67,10 +67,12 @@ type templateData struct { BuilderImage string BuildEnvs []string - PipelineName string - PipelineRunName string - PvcName string - SecretName string + PipelineName string + PipelineRunName string + PvcName string + PvcSize string + StorageClassName string + SecretName string // The branch or tag we are targeting with Pipelines (ie: main, refs/tags/*) PipelinesTargetBranch string @@ -181,10 +183,12 @@ 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), - SecretName: getPipelineSecretName(f), + PipelineName: getPipelineName(f), + PipelineRunName: fmt.Sprintf("%s-run", getPipelineName(f)), + PvcName: getPipelinePvcName(f), + PvcSize: pipelinePvcSize(f), + StorageClassName: f.Build.RemoteStorageClass, + SecretName: getPipelineSecretName(f), PipelinesTargetBranch: pipelinesTargetBranch, @@ -380,10 +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), - 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, @@ -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..3f021a1fb9 100644 --- a/pkg/pipelines/tekton/templates_pack.go +++ b/pkg/pipelines/tekton/templates_pack.go @@ -119,9 +119,15 @@ spec: claimName: {{.PvcName}} subPath: source - name: cache-workspace - persistentVolumeClaim: - claimName: {{.PvcName}} - subPath: cache + volumeClaimTemplate: + spec: + accessModes: ["ReadWriteOnce"] + {{- if .StorageClassName}} + storageClassName: {{.StorageClassName}} + {{- end}} + resources: + requests: + storage: {{.PvcSize}} - name: dockerconfig-workspace secret: secretName: {{.SecretName}} @@ -175,13 +181,25 @@ spec: name: {{.PipelineName}} workspaces: - name: source-workspace - persistentVolumeClaim: - claimName: {{.PvcName}} - subPath: source + volumeClaimTemplate: + spec: + accessModes: ["ReadWriteOnce"] + {{- if .StorageClassName}} + storageClassName: {{.StorageClassName}} + {{- end}} + resources: + requests: + storage: {{.PvcSize}} - name: cache-workspace - persistentVolumeClaim: - claimName: {{.PvcName}} - subPath: cache + 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 9441a9c5c6..b7c607c9af 100644 --- a/pkg/pipelines/tekton/templates_s2i.go +++ b/pkg/pipelines/tekton/templates_s2i.go @@ -134,9 +134,15 @@ spec: claimName: {{.PvcName}} subPath: source - name: cache-workspace - persistentVolumeClaim: - claimName: {{.PvcName}} - subPath: cache + volumeClaimTemplate: + spec: + accessModes: ["ReadWriteOnce"] + {{- if .StorageClassName}} + storageClassName: {{.StorageClassName}} + {{- end}} + resources: + requests: + storage: {{.PvcSize}} - name: dockerconfig-workspace secret: secretName: {{.SecretName}} @@ -197,13 +203,25 @@ spec: name: {{.PipelineName}} workspaces: - name: source-workspace - persistentVolumeClaim: - claimName: {{.PvcName}} - subPath: source + volumeClaimTemplate: + spec: + accessModes: ["ReadWriteOnce"] + {{- if .StorageClassName}} + storageClassName: {{.StorageClassName}} + {{- end}} + resources: + requests: + storage: {{.PvcSize}} - name: cache-workspace - persistentVolumeClaim: - claimName: {{.PvcName}} - subPath: cache + volumeClaimTemplate: + spec: + accessModes: ["ReadWriteOnce"] + {{- if .StorageClassName}} + storageClassName: {{.StorageClassName}} + {{- end}} + resources: + requests: + storage: {{.PvcSize}} - name: dockerconfig-workspace secret: secretName: {{.SecretName}}