From 18bd7ff49a406da84be989b3a017357d96d6da52 Mon Sep 17 00:00:00 2001 From: Andrew Bayer Date: Tue, 16 Jul 2019 12:29:39 -0400 Subject: [PATCH] =?UTF-8?q?Add=20"volume"=20PipelineResource=20?= =?UTF-8?q?=F0=9F=94=8A?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This will allow copying content either into or out of a `TaskRun`, either to an existing volume or a newly created volume. The immediate use case is for copying a pipeline's workspace to be made available as the input for another pipeline's workspace without needing to deal with uploading everything to a bucket. The volume, whether already existing or created, will not be deleted at the end of the `PipelineRun`, unlike the artifact storage PVC. The Volume resource is a sub-type of the general Storage resource. Since this type will require the creation of a PVC to function (to be configurable later), this commit adds a Setup interface that PipelineResources can implement if they need to do setup that involves instantiating objects in Kube. This could be a place to later add features like caching, and also is the sort of design we'd expect once PipelineResources are extensible (PipelineResources will be free to do whatever setup they need). The behavior of this volume resource is: 1. For inputs, copy data _from_ the PVC to the workspace path 2. For outputs, copy data _to_ the PVC from the workspace path If a user does want to control where the data is copied from, they can: 1. Add a step that copies from the location they want to copy from on disk to /workspace/whatever 2. Use the "targetPath" argument in the PipelineResource to control the location the data is copied to (still relative to targetPath https://github.com/tektoncd/pipeline/blob/master/docs/resources.md#controlling-where-resources-are-mounted) 3. Use `path` https://github.com/tektoncd/pipeline/blob/master/docs/resources.md#overriding-where-resources-are-copied-from (tbd if we want to keep this feature post PVC) The underlying PVC will need to be created by the Task reonciler, if only a TaskRun is being used, or by the PipelineRun reconciler if a Pipeline is being used. The PipelineRun reconciler cannot delegate this to the TaskRun reconciler b/c when two different reconcilers create PVCs and Tekton is running on a regional GKE cluster, they can get created in different zones, resulting in a pod that tries to use both being unschedulable. In order to actually schedule a pod using two volume resources, we had to: - Use a storage class that can be scheduled in a GKE regional cluster https://cloud.google.com/kubernetes-engine/docs/how-to/persistent-volumes/regional-pd - Either use the same storage class for the PVC attached automatically for input/output linking or don't use the PVC (chose the latter!) This commit removes automatic PVC copying for input output linking of the VolumeResource b/c since it itself is a PVC, there is no need to copy between an intermediate PVCs. This makes it simpler to make a Task using the VolumeResource schedulable, removes redundant copying, and removes a side effect where if a VolumeResources output was linked to an input, the Task with the input would see _only_ the changes made by the output and none of the other contents of the PVC. Also removing the docs on the `paths` param (i.e. "overriding where resources are copied from") because it was implemented such that it would only work in the output -> input linking PVC case and can't actually be used by users and it will be removed in #1284. fixes #1062 Co-authored-by: Dan Lorenc Co-authored-by: Christie Wilson --- docs/install.md | 10 +- docs/resources.md | 139 +++---- .../volume-output-pipelinerun.yaml | 183 +++++++++ pkg/apis/pipeline/v1alpha1/artifact_pvc.go | 17 +- .../pipeline/v1alpha1/artifact_pvc_test.go | 24 +- .../pipeline/v1alpha1/build_gcs_resource.go | 7 +- .../pipeline/v1alpha1/cloud_event_resource.go | 3 + .../pipeline/v1alpha1/cluster_resource.go | 3 + pkg/apis/pipeline/v1alpha1/gcs_resource.go | 5 +- pkg/apis/pipeline/v1alpha1/git_resource.go | 3 + pkg/apis/pipeline/v1alpha1/image_resource.go | 3 + .../v1alpha1/pipelineresource_validation.go | 26 +- .../pipelineresource_validation_test.go | 37 +- .../v1alpha1/pull_request_resource.go | 3 + pkg/apis/pipeline/v1alpha1/resource_types.go | 81 +++- .../pipeline/v1alpha1/resource_types_test.go | 147 +++++++ .../pipeline/v1alpha1/storage_resource.go | 17 +- pkg/apis/pipeline/v1alpha1/taskrun_types.go | 7 + pkg/apis/pipeline/v1alpha1/volume_resource.go | 229 +++++++++++ .../pipeline/v1alpha1/volume_resource_test.go | 380 ++++++++++++++++++ .../v1alpha1/zz_generated.deepcopy.go | 49 +++ pkg/reconciler/pipelinerun/pipelinerun.go | 19 + .../pipelinerun/pipelinerun_test.go | 31 +- .../resources/conditionresolution.go | 9 +- .../resources/input_output_steps.go | 4 +- .../taskrun/resources/input_resource_test.go | 191 ++++++++- .../taskrun/resources/input_resources.go | 11 +- .../taskrun/resources/output_resource.go | 14 +- .../taskrun/resources/output_resource_test.go | 253 +++++++++++- pkg/reconciler/taskrun/resources/pod.go | 2 +- pkg/reconciler/taskrun/taskrun.go | 30 +- pkg/reconciler/taskrun/taskrun_test.go | 55 ++- test/pvc.go | 42 ++ 33 files changed, 1842 insertions(+), 192 deletions(-) create mode 100644 examples/pipelineruns/volume-output-pipelinerun.yaml create mode 100644 pkg/apis/pipeline/v1alpha1/resource_types_test.go create mode 100644 pkg/apis/pipeline/v1alpha1/volume_resource.go create mode 100644 pkg/apis/pipeline/v1alpha1/volume_resource_test.go create mode 100644 test/pvc.go diff --git a/docs/install.md b/docs/install.md index 801e1c1281d..6b8e2be5ace 100644 --- a/docs/install.md +++ b/docs/install.md @@ -124,16 +124,16 @@ or a [GCS storage bucket](https://cloud.google.com/storage/) The PVC option can be configured using a ConfigMap with the name `config-artifact-pvc` and the following attributes: -- size: the size of the volume (5Gi by default) -- storageClassName: the [storage class](https://kubernetes.io/docs/concepts/storage/storage-classes/) of the volume (default storage class by default). The possible values depend on the cluster configuration and the underlying infrastructure provider. +- `size`: the size of the volume (5Gi by default) +- `storageClassName`: the [storage class](https://kubernetes.io/docs/concepts/storage/storage-classes/) of the volume (default storage class by default). The possible values depend on the cluster configuration and the underlying infrastructure provider. The GCS storage bucket can be configured using a ConfigMap with the name `config-artifact-bucket` with the following attributes: -- location: the address of the bucket (for example gs://mybucket) -- bucket.service.account.secret.name: the name of the secret that will contain +- `location`: the address of the bucket (for example gs://mybucket) +- `bucket.service.account.secret.name`: the name of the secret that will contain the credentials for the service account with access to the bucket -- bucket.service.account.secret.key: the key in the secret with the required +- `bucket.service.account.secret.key`: the key in the secret with the required service account json. - The bucket is recommended to be configured with a retention policy after which files will be deleted. diff --git a/docs/resources.md b/docs/resources.md index 9b3ba94aa73..87c3574a7bf 100644 --- a/docs/resources.md +++ b/docs/resources.md @@ -17,6 +17,15 @@ For example: - [Syntax](#syntax) - [Resource types](#resource-types) + - [Git Resource](#git-resource) + - [Pull Request Resource](#pull-request-resource) + - [Image Resource](#image-resource) + - [Cluster Resource](#cluster-resource) + - [Storage Resource](#storage-resource) + - [GCS Storage Resource](#gcs-storage-resource) + - [BuildGCS Storage Resource](#buildgcs-storage-resource) + - [Volume Resource](#volume-resource) + - [Cloud Event Resource](#cloud-event-resource) - [Using Resources](#using-resources) ## Syntax @@ -119,94 +128,8 @@ spec: value: /workspace/go ``` -### Overriding where resources are copied from - -When specifying input and output `PipelineResources`, you can optionally specify -`paths` for each resource. `paths` will be used by `TaskRun` as the resource's -new source paths i.e., copy the resource from specified list of paths. `TaskRun` -expects the folder and contents to be already present in specified paths. -`paths` feature could be used to provide extra files or altered version of -existing resource before execution of steps. - -Output resource includes name and reference to pipeline resource and optionally -`paths`. `paths` will be used by `TaskRun` as the resource's new destination -paths i.e., copy the resource entirely to specified paths. `TaskRun` will be -responsible for creating required directories and copying contents over. `paths` -feature could be used to inspect the results of taskrun after execution of -steps. - -`paths` feature for input and output resource is heavily used to pass same -version of resources across tasks in context of pipelinerun. - -In the following example, task and taskrun are defined with input resource, -output resource and step which builds war artifact. After execution of -taskrun(`volume-taskrun`), `custom` volume will have entire resource -`java-git-resource` (including the war artifact) copied to the destination path -`/custom/workspace/`. - -```yaml -apiVersion: tekton.dev/v1alpha1 -kind: Task -metadata: - name: volume-task - namespace: default -spec: - inputs: - resources: - - name: workspace - type: git - outputs: - resources: - - name: workspace - steps: - - name: build-war - image: objectuser/run-java-jar #https://hub.docker.com/r/objectuser/run-java-jar/ - command: jar - args: ["-cvf", "projectname.war", "*"] - volumeMounts: - - name: custom-volume - mountPath: /custom -``` - -```yaml -apiVersion: tekton.dev/v1alpha1 -kind: TaskRun -metadata: - name: volume-taskrun - namespace: default -spec: - taskRef: - name: volume-task - inputs: - resources: - - name: workspace - resourceRef: - name: java-git-resource - outputs: - resources: - - name: workspace - paths: - - /custom/workspace/ - resourceRef: - name: java-git-resource - volumes: - - name: custom-volume - emptyDir: {} -``` - ## Resource Types -The following `PipelineResources` are currently supported: - -- [Git Resource](#git-resource) -- [Pull Request Resource](#pull-request-resource) -- [Image Resource](#image-resource) -- [Cluster Resource](#cluster-resource) -- [Storage Resource](#storage-resource) - - [GCS Storage Resource](#gcs-storage-resource) - - [BuildGCS Storage Resource](#buildgcs-storage-resource) -- [Cloud Event Resource](#cloud-event-resource) - ### Git Resource Git resource represents a [git](https://git-scm.com/) repository, that contains @@ -770,6 +693,50 @@ the container image [gcr.io/cloud-builders//gcs-fetcher](https://github.com/GoogleCloudPlatform/cloud-builders/tree/master/gcs-fetcher) does not support configuring secrets. +#### Volume Resource + +The Volume `PipelineResource` will create and manage an underlying +[Persistent Volume](https://kubernetes.io/docs/concepts/storage/persistent-volumes/) (PVC). + +To create a Volume resource: + +```yaml +apiVersion: tekton.dev/v1alpha1 +kind: PipelineResource +metadata: + name: volume-resource-1 +spec: + type: storage + params: + - name: type + value: volume + - name: size + value: 5Gi + - name: subPath + value: some/path/on/the/pvc + - name: storageClassName + value: regional-disk +``` + +Supported `params` are: + +* `size` - **Required** The size to make the underlying PVC, expressed as a + [Quantity](https://godoc.org/k8s.io/apimachinery/pkg/api/resource#Quantity) +* `subPath` - By default, data will be placed at the root of the PVC. This allows data to + instead be placed in a subfolder on the PVC +* `storageClassName` - The [storage class](https://kubernetes.io/docs/concepts/storage/storage-classes/) + that the PVC should use. For example, this is how you can use multiple Volume PipelineResources + [with GKE regional clusters](#using-with-gke-regional-clusters). + +##### Using with GKE Regional Clusters + +When using GKE regional clusters, when PVCs are created they will be assigned to zones +round robin. This means if two Volume PipelineResources are used by one Task, you must specify a +[`regional-pd`](https://cloud.google.com/kubernetes-engine/docs/how-to/persistent-volumes/regional-pd) storage class, otherwise the PVCs could be created in different zones, and it will +be impossible to schedule a Task's pod that can use both. + +[See the volume PipelineResource example.](../examples/pipelineruns/volume-output-pipelinerun.yaml) + ### Cloud Event Resource The Cloud Event Resource represents a [cloud event](https://github.com/cloudevents/spec) diff --git a/examples/pipelineruns/volume-output-pipelinerun.yaml b/examples/pipelineruns/volume-output-pipelinerun.yaml new file mode 100644 index 00000000000..8dd36183a31 --- /dev/null +++ b/examples/pipelineruns/volume-output-pipelinerun.yaml @@ -0,0 +1,183 @@ +# This example will be using multiple PVCs and will be run against a regional GKE. +# This means we have to make sure that the PVCs aren't created in different zones, +# and the only way to do this is to create regional PVCs. +apiVersion: storage.k8s.io/v1 +kind: StorageClass +metadata: + name: regional-disk +provisioner: kubernetes.io/gce-pd +parameters: + type: pd-ssd + replication-type: regional-pd +--- +apiVersion: tekton.dev/v1alpha1 +kind: PipelineResource +metadata: + name: volume-resource-1 +spec: + type: storage + params: + - name: type + value: volume + - name: storageClassName + value: regional-disk +--- +apiVersion: tekton.dev/v1alpha1 +kind: PipelineResource +metadata: + name: volume-resource-2 +spec: + type: storage + params: + - name: type + value: volume + - name: path + value: special-folder + - name: storageClassName + value: regional-disk +--- +# Task writes data to a predefined path +apiVersion: tekton.dev/v1alpha1 +kind: Task +metadata: + name: create-files +spec: + outputs: + # This Task uses two volume outputs to ensure that multiple volume + # outputs can be used + resources: + - name: volume1 + type: storage + - name: volume2 + type: storage + steps: + - name: write-new-stuff-1 + image: ubuntu + command: ['bash'] + args: ['-c', 'echo stuff1 > $(outputs.resources.volume1.path)/stuff1'] + - name: write-new-stuff-2 + image: ubuntu + command: ['bash'] + args: ['-c', 'echo stuff2 > $(outputs.resources.volume2.path)/stuff2'] +--- +# Reads a file from a predefined path and writes as well +apiVersion: tekton.dev/v1alpha1 +kind: Task +metadata: + name: files-exist-and-add-new +spec: + inputs: + resources: + - name: volume1 + type: storage + targetPath: newpath + - name: volume2 + type: storage + outputs: + resources: + - name: volume1 + type: storage + steps: + - name: read1 + image: ubuntu + command: ["/bin/bash"] + args: + - '-c' + - '[[ stuff1 == $(cat $(inputs.resources.volume1.path)/stuff1) ]]' + - name: read2 + image: ubuntu + command: ["/bin/bash"] + args: + - '-c' + - '[[ stuff2 == $(cat $(inputs.resources.volume2.path)/stuff2) ]]' + - name: write-new-stuff-3 + image: ubuntu + command: ['bash'] + args: ['-c', 'echo stuff3 > $(outputs.resources.volume1.path)/stuff3'] +--- +# Reads a file from a predefined path and writes as well +apiVersion: tekton.dev/v1alpha1 +kind: Task +metadata: + name: files-exist +spec: + inputs: + resources: + - name: volume1 + type: storage + steps: + - name: read1 + image: ubuntu + command: ["/bin/bash"] + args: + - '-c' + - '[[ stuff1 == $(cat $(inputs.resources.volume1.path)/stuff1) ]]' + - name: read3 + image: ubuntu + command: ["/bin/bash"] + args: + - '-c' + - '[[ stuff3 == $(cat $(inputs.resources.volume1.path)/stuff3) ]]' +--- +# First task writees files to two volumes. The next task ensures these files exist +# then writes a third file to the first volume. The last Task ensures both expected +# files exist on this volume. +apiVersion: tekton.dev/v1alpha1 +kind: Pipeline +metadata: + name: volume-output-pipeline +spec: + resources: + - name: volume1 + type: storage + - name: volume2 + type: storage + tasks: + - name: first-create-files + taskRef: + name: create-files + resources: + outputs: + - name: volume1 + resource: volume1 + - name: volume2 + resource: volume2 + - name: then-check-and-write + taskRef: + name: files-exist-and-add-new + resources: + inputs: + - name: volume1 + resource: volume1 + from: [first-create-files] + - name: volume2 + resource: volume2 + from: [first-create-files] + outputs: + - name: volume1 + # This Task uses the same volume as an input and an output to ensure this works + resource: volume1 + - name: then-check + taskRef: + name: files-exist + resources: + inputs: + - name: volume1 + resource: volume1 + from: [then-check-and-write] +--- +apiVersion: tekton.dev/v1alpha1 +kind: PipelineRun +metadata: + name: volume-output-pipeline-run +spec: + pipelineRef: + name: volume-output-pipeline + serviceAccount: 'default' + resources: + - name: volume1 + resourceRef: + name: volume-resource-1 + - name: volume2 + resourceRef: + name: volume-resource-2 diff --git a/pkg/apis/pipeline/v1alpha1/artifact_pvc.go b/pkg/apis/pipeline/v1alpha1/artifact_pvc.go index 2a4ec48b80f..8875497ec3c 100644 --- a/pkg/apis/pipeline/v1alpha1/artifact_pvc.go +++ b/pkg/apis/pipeline/v1alpha1/artifact_pvc.go @@ -57,7 +57,7 @@ func (p *ArtifactPVC) GetCopyFromStorageToSteps(name, sourcePath, destinationPat }}} } -// GetCopyToStorageFromSteps returns a container used to upload artifacts for temporary storage +// GetCopyToStorageFromSteps returns a container used to upload artifacts for temporary storageCreateDirStep func (p *ArtifactPVC) GetCopyToStorageFromSteps(name, sourcePath, destinationPath string) []Step { return []Step{{Container: corev1.Container{ Name: names.SimpleNameGenerator.RestrictLengthWithRandomSuffix(fmt.Sprintf("source-mkdir-%s", name)), @@ -86,13 +86,16 @@ func GetPvcMount(name string) corev1.VolumeMount { } } -// CreateDirStep returns a container step to create a dir -func CreateDirStep(bashNoopImage string, name, destinationPath string) Step { +// CreateDirStep returns a container step to create a dir at destinationPath. The name +// of the step will include name. Optionally will mount included volumeMounts if the +// dir is to be created on the volume. +func CreateDirStep(bashNoopImage string, name, destinationPath string, volumeMounts []corev1.VolumeMount) Step { return Step{Container: corev1.Container{ - Name: names.SimpleNameGenerator.RestrictLengthWithRandomSuffix(fmt.Sprintf("create-dir-%s", strings.ToLower(name))), - Image: bashNoopImage, - Command: []string{"/ko-app/bash"}, - Args: []string{"-args", strings.Join([]string{"mkdir", "-p", destinationPath}, " ")}, + Name: names.SimpleNameGenerator.RestrictLengthWithRandomSuffix(fmt.Sprintf("create-dir-%s", strings.ToLower(name))), + Image: bashNoopImage, + Command: []string{"/ko-app/bash"}, + Args: []string{"-args", strings.Join([]string{"mkdir", "-p", destinationPath}, " ")}, + VolumeMounts: volumeMounts, }} } diff --git a/pkg/apis/pipeline/v1alpha1/artifact_pvc_test.go b/pkg/apis/pipeline/v1alpha1/artifact_pvc_test.go index 6250e9e7a5f..8c768ee1828 100644 --- a/pkg/apis/pipeline/v1alpha1/artifact_pvc_test.go +++ b/pkg/apis/pipeline/v1alpha1/artifact_pvc_test.go @@ -88,7 +88,7 @@ func TestPVCGetPvcMount(t *testing.T) { } } -func TestPVCGetMakeStep(t *testing.T) { +func TestCreateDirStepWithoutVolume(t *testing.T) { names.TestingSeed() want := v1alpha1.Step{Container: corev1.Container{ @@ -97,9 +97,27 @@ func TestPVCGetMakeStep(t *testing.T) { Command: []string{"/ko-app/bash"}, Args: []string{"-args", "mkdir -p /workspace/destination"}, }} - got := v1alpha1.CreateDirStep("override-with-bash-noop:latest", "workspace", "/workspace/destination") + got := v1alpha1.CreateDirStep("override-with-bash-noop:latest", "workspace", "/workspace/destination", nil) if d := cmp.Diff(got, want); d != "" { - t.Errorf("Diff:\n%s", d) + t.Errorf("Did not get expected step for creating directory (-want, +got): %s", d) + } +} + +func TestCreateDirStepWithVolume(t *testing.T) { + names.TestingSeed() + + want := v1alpha1.Step{Container: corev1.Container{ + Name: "create-dir-workspace-9l9zj", + Image: "override-with-bash-noop:latest", + Command: []string{"/ko-app/bash"}, + Args: []string{"-args", "mkdir -p /special-workspace/destination"}, + VolumeMounts: []corev1.VolumeMount{{Name: "foo", MountPath: "/special-workspace"}}, + }} + got := v1alpha1.CreateDirStep("override-with-bash-noop:latest", "workspace", "/special-workspace/destination", []corev1.VolumeMount{{ + Name: "foo", MountPath: "/special-workspace", + }}) + if d := cmp.Diff(want, got); d != "" { + t.Errorf("Did not get expected step for creating directory (-want, +got): %s", d) } } diff --git a/pkg/apis/pipeline/v1alpha1/build_gcs_resource.go b/pkg/apis/pipeline/v1alpha1/build_gcs_resource.go index 721a9bacff1..5d7ce8a0b26 100644 --- a/pkg/apis/pipeline/v1alpha1/build_gcs_resource.go +++ b/pkg/apis/pipeline/v1alpha1/build_gcs_resource.go @@ -70,7 +70,10 @@ type BuildGCSResource struct { BuildGCSFetcherImage string `json:"-"` } -// creates a new BuildGCS resource to pass to a Task +// GetSetup returns a PipelineResourceSetupInterface that does nothing because no setup is needed. +func (s BuildGCSResource) GetSetup() PipelineResourceSetupInterface { return &NoSetup{} } + +// NewBuildGCSResource creates a new BuildGCS resource to pass to a Task func NewBuildGCSResource(images pipeline.Images, r *PipelineResource) (*BuildGCSResource, error) { if r.Spec.Type != PipelineResourceTypeStorage { return nil, xerrors.Errorf("BuildGCSResource: Cannot create a BuildGCS resource from a %s Pipeline Resource", r.Spec.Type) @@ -135,7 +138,7 @@ func (s *BuildGCSResource) GetInputTaskModifier(ts *TaskSpec, sourcePath string) } steps := []Step{ - CreateDirStep(s.BashNoopImage, s.Name, sourcePath), + CreateDirStep(s.BashNoopImage, s.Name, sourcePath, nil), {Container: corev1.Container{ Name: names.SimpleNameGenerator.RestrictLengthWithRandomSuffix(fmt.Sprintf("storage-fetch-%s", s.Name)), Command: []string{"/ko-app/gcs-fetcher"}, diff --git a/pkg/apis/pipeline/v1alpha1/cloud_event_resource.go b/pkg/apis/pipeline/v1alpha1/cloud_event_resource.go index e83fb0de827..498aa9ca95d 100644 --- a/pkg/apis/pipeline/v1alpha1/cloud_event_resource.go +++ b/pkg/apis/pipeline/v1alpha1/cloud_event_resource.go @@ -58,6 +58,9 @@ func NewCloudEventResource(r *PipelineResource) (*CloudEventResource, error) { }, nil } +// GetSetup returns a PipelineResourceSetupInterface that can create the backing PVC if needed. +func (s CloudEventResource) GetSetup() PipelineResourceSetupInterface { return SetupPVC{} } + // GetName returns the name of the resource func (s CloudEventResource) GetName() string { return s.Name diff --git a/pkg/apis/pipeline/v1alpha1/cluster_resource.go b/pkg/apis/pipeline/v1alpha1/cluster_resource.go index 8034d3e8964..aa79dde0153 100644 --- a/pkg/apis/pipeline/v1alpha1/cluster_resource.go +++ b/pkg/apis/pipeline/v1alpha1/cluster_resource.go @@ -54,6 +54,9 @@ type ClusterResource struct { KubeconfigWriterImage string `json:"-"` } +// GetSetup returns a PipelineResourceSetupInterface that does nothing because no setup is needed. +func (s ClusterResource) GetSetup() PipelineResourceSetupInterface { return &NoSetup{} } + // NewClusterResource create a new k8s cluster resource to pass to a pipeline task func NewClusterResource(kubeconfigWriterImage string, r *PipelineResource) (*ClusterResource, error) { if r.Spec.Type != PipelineResourceTypeCluster { diff --git a/pkg/apis/pipeline/v1alpha1/gcs_resource.go b/pkg/apis/pipeline/v1alpha1/gcs_resource.go index 514b5e1c381..1ce97058ea6 100644 --- a/pkg/apis/pipeline/v1alpha1/gcs_resource.go +++ b/pkg/apis/pipeline/v1alpha1/gcs_resource.go @@ -45,6 +45,9 @@ type GCSResource struct { GsutilImage string `json:"-"` } +// GetSetup returns a PipelineResourceSetupInterface that does nothing because no setup is needed. +func (s GCSResource) GetSetup() PipelineResourceSetupInterface { return &NoSetup{} } + // NewGCSResource creates a new GCS resource to pass to a Task func NewGCSResource(images pipeline.Images, r *PipelineResource) (*GCSResource, error) { if r.Spec.Type != PipelineResourceTypeStorage { @@ -143,7 +146,7 @@ func (s *GCSResource) GetInputTaskModifier(ts *TaskSpec, path string) (TaskModif envVars, secretVolumeMount := getSecretEnvVarsAndVolumeMounts(s.Name, gcsSecretVolumeMountPath, s.Secrets) steps := []Step{ - CreateDirStep(s.BashNoopImage, s.Name, path), + CreateDirStep(s.BashNoopImage, s.Name, path, nil), {Container: corev1.Container{ Name: names.SimpleNameGenerator.RestrictLengthWithRandomSuffix(fmt.Sprintf("fetch-%s", s.Name)), Image: s.GsutilImage, diff --git a/pkg/apis/pipeline/v1alpha1/git_resource.go b/pkg/apis/pipeline/v1alpha1/git_resource.go index 8f19e8e8418..feb6ab189a5 100644 --- a/pkg/apis/pipeline/v1alpha1/git_resource.go +++ b/pkg/apis/pipeline/v1alpha1/git_resource.go @@ -44,6 +44,9 @@ type GitResource struct { GitImage string `json:"-"` } +// GetSetup returns a PipelineResourceSetupInterface that does nothing because no setup is needed. +func (s GitResource) GetSetup() PipelineResourceSetupInterface { return &NoSetup{} } + // NewGitResource creates a new git resource to pass to a Task func NewGitResource(gitImage string, r *PipelineResource) (*GitResource, error) { if r.Spec.Type != PipelineResourceTypeGit { diff --git a/pkg/apis/pipeline/v1alpha1/image_resource.go b/pkg/apis/pipeline/v1alpha1/image_resource.go index be960934817..1289cab8b59 100644 --- a/pkg/apis/pipeline/v1alpha1/image_resource.go +++ b/pkg/apis/pipeline/v1alpha1/image_resource.go @@ -54,6 +54,9 @@ type ImageResource struct { OutputImageDir string } +// GetSetup returns a PipelineResourceSetupInterface that does nothing because no setup is needed. +func (s ImageResource) GetSetup() PipelineResourceSetupInterface { return &NoSetup{} } + // GetName returns the name of the resource func (s ImageResource) GetName() string { return s.Name diff --git a/pkg/apis/pipeline/v1alpha1/pipelineresource_validation.go b/pkg/apis/pipeline/v1alpha1/pipelineresource_validation.go index 14af9eb3d45..c837d5f5e83 100644 --- a/pkg/apis/pipeline/v1alpha1/pipelineresource_validation.go +++ b/pkg/apis/pipeline/v1alpha1/pipelineresource_validation.go @@ -22,6 +22,7 @@ import ( "strings" "k8s.io/apimachinery/pkg/api/equality" + "k8s.io/apimachinery/pkg/api/resource" "knative.dev/pkg/apis" ) @@ -77,24 +78,39 @@ func (rs *PipelineResourceSpec) Validate(ctx context.Context) *apis.FieldError { } } if rs.Type == PipelineResourceTypeStorage { - foundTypeParam := false var location string + t := "" for _, param := range rs.Params { switch { case strings.EqualFold(param.Name, "type"): if !AllowedStorageType(param.Value) { return apis.ErrInvalidValue(param.Value, "spec.params.type") } - foundTypeParam = true + t = param.Value case strings.EqualFold(param.Name, "Location"): location = param.Value } } - if !foundTypeParam { + if t == "" { return apis.ErrMissingField("spec.params.type") } - if location == "" { + if t == string(PipelineResourceTypeVolume) { + size := "" + for _, param := range rs.Params { + switch { + case strings.EqualFold(param.Name, "size"): + size = param.Value + } + } + if size == "" { + return apis.ErrMissingField("spec.params.size") + } + _, err := resource.ParseQuantity(size) + if err != nil { + return apis.ErrInvalidValue("invalid-size", "spec.params.size") + } + } else if location == "" { return apis.ErrMissingField("spec.params.location") } } @@ -114,6 +130,8 @@ func AllowedStorageType(gotType string) bool { return true case string(PipelineResourceTypeBuildGCS): return true + case string(PipelineResourceTypeVolume): + return true } return false } diff --git a/pkg/apis/pipeline/v1alpha1/pipelineresource_validation_test.go b/pkg/apis/pipeline/v1alpha1/pipelineresource_validation_test.go index 69a3d179acf..a5ef67f836b 100644 --- a/pkg/apis/pipeline/v1alpha1/pipelineresource_validation_test.go +++ b/pkg/apis/pipeline/v1alpha1/pipelineresource_validation_test.go @@ -116,12 +116,27 @@ func TestResourceValidation_Invalid(t *testing.T) { }, }, want: apis.ErrInvalidValue("spec.type", "not-supported"), + }, { + name: "volume resource without size", + res: tb.PipelineResource("volume-resource", "foo", tb.PipelineResourceSpec( + v1alpha1.PipelineResourceTypeStorage, + tb.PipelineResourceSpecParam("type", "volume"), + )), + want: apis.ErrMissingField("spec.params.size"), + }, { + name: "volume resource with invalid size", + res: tb.PipelineResource("volume-resource", "foo", tb.PipelineResourceSpec( + v1alpha1.PipelineResourceTypeStorage, + tb.PipelineResourceSpecParam("type", "volume"), + tb.PipelineResourceSpecParam("size", "bigger than a breadbox"), + )), + want: apis.ErrInvalidValue("invalid-size", "spec.params.size"), }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { err := tt.res.Validate(context.Background()) - if d := cmp.Diff(err.Error(), tt.want.Error()); d != "" { + if d := cmp.Diff(tt.want.Error(), err.Error()); d != "" { t.Errorf("PipelineResource.Validate/%s (-want, +got) = %v", tt.name, d) } }) @@ -134,7 +149,7 @@ func TestClusterResourceValidation_Valid(t *testing.T) { res *v1alpha1.PipelineResource }{ { - name: "success validate", + name: "success validate cluster", res: tb.PipelineResource("test-cluster-resource", "foo", tb.PipelineResourceSpec( v1alpha1.PipelineResourceTypeCluster, tb.PipelineResourceSpecParam("name", "test_cluster_resource"), @@ -145,7 +160,7 @@ func TestClusterResourceValidation_Valid(t *testing.T) { )), }, { - name: "specify insecure without cadata", + name: "specify insecure cluster without cadata", res: tb.PipelineResource("test-cluster-resource", "foo", tb.PipelineResourceSpec( v1alpha1.PipelineResourceTypeCluster, tb.PipelineResourceSpecParam("name", "test_cluster_resource"), @@ -154,6 +169,22 @@ func TestClusterResourceValidation_Valid(t *testing.T) { tb.PipelineResourceSpecParam("token", "my-token"), tb.PipelineResourceSpecParam("insecure", "true"), )), + }, { + name: "valid minimal volume resource", + res: tb.PipelineResource("volume-resource", "foo", tb.PipelineResourceSpec( + v1alpha1.PipelineResourceTypeStorage, + tb.PipelineResourceSpecParam("type", "volume"), + tb.PipelineResourceSpecParam("size", "5Gi"), + )), + }, { + name: "valid fully specified volume resource", + res: tb.PipelineResource("volume-resource", "foo", tb.PipelineResourceSpec( + v1alpha1.PipelineResourceTypeStorage, + tb.PipelineResourceSpecParam("type", "volume"), + tb.PipelineResourceSpecParam("size", "5Gi"), + tb.PipelineResourceSpecParam("subPath", "some/path"), + tb.PipelineResourceSpecParam("storageClassName", "magicstorage"), + )), }, } for _, tt := range tests { diff --git a/pkg/apis/pipeline/v1alpha1/pull_request_resource.go b/pkg/apis/pipeline/v1alpha1/pull_request_resource.go index e7afd9c95c7..2f3fa51fb48 100644 --- a/pkg/apis/pipeline/v1alpha1/pull_request_resource.go +++ b/pkg/apis/pipeline/v1alpha1/pull_request_resource.go @@ -44,6 +44,9 @@ type PullRequestResource struct { PRImage string `json:"-"` } +// GetSetup returns a PipelineResourceSetupInterface that does nothing because no setup is needed. +func (s PullRequestResource) GetSetup() PipelineResourceSetupInterface { return &NoSetup{} } + // NewPullRequestResource create a new git resource to pass to a Task func NewPullRequestResource(prImage string, r *PipelineResource) (*PullRequestResource, error) { if r.Spec.Type != PipelineResourceTypePullRequest { diff --git a/pkg/apis/pipeline/v1alpha1/resource_types.go b/pkg/apis/pipeline/v1alpha1/resource_types.go index bb3bc737f48..e6a5b1cfad9 100644 --- a/pkg/apis/pipeline/v1alpha1/resource_types.go +++ b/pkg/apis/pipeline/v1alpha1/resource_types.go @@ -17,10 +17,12 @@ limitations under the License. package v1alpha1 import ( + "github.com/google/go-cmp/cmp" "github.com/tektoncd/pipeline/pkg/apis/pipeline" "golang.org/x/xerrors" v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/kubernetes" "knative.dev/pkg/apis" ) @@ -54,11 +56,21 @@ var AllResourceTypes = []PipelineResourceType{PipelineResourceTypeGit, PipelineR // PipelineResourceInterface interface to be implemented by different PipelineResource types type PipelineResourceInterface interface { + // GetName returns the name of this PipelineResource instnace GetName() string + // GetType returns the type of this PipelineResource (often a super type) GetType() PipelineResourceType + // Replacements returns all the attributes that this PipelineResource has that + // can be used for variable replacement. Replacements() map[string]string + GetOutputTaskModifier(ts *TaskSpec, path string) (TaskModifier, error) GetInputTaskModifier(ts *TaskSpec, path string) (TaskModifier, error) + + // GetSetup returns the instance of PipelineResourceSetupInterface that the PipelineResource + // needs in order to perform operations before it can be realized. This function should be + // idempotent. NoSetup can be used by PipelineResources that do not require setup. + GetSetup() PipelineResourceSetupInterface } // TaskModifier is an interface to be implemented by different PipelineResources @@ -80,7 +92,7 @@ func (tm *InternalTaskModifier) GetStepsToPrepend() []Step { return tm.StepsToPrepend } -// GetStepsToPrepend returns a set of Steps to append to the Task. +// GetStepsToAppend returns a set of Steps to append to the Task. func (tm *InternalTaskModifier) GetStepsToAppend() []Step { return tm.StepsToAppend } @@ -90,16 +102,75 @@ func (tm *InternalTaskModifier) GetVolumes() []v1.Volume { return tm.Volumes } +func checkStepNotAlreadyAdded(s Step, steps []Step) error { + for _, step := range steps { + if s.Name == step.Name { + return xerrors.Errorf("Step %s cannot be added again", step.Name) + } + } + return nil +} + // ApplyTaskModifier applies a modifier to the task by appending and prepending steps and volumes. -func ApplyTaskModifier(ts *TaskSpec, tm TaskModifier) { +// If steps with the same name exist in ts an error will be returned. If identical Volumes have +// been added, they will not be added again. If Volumes with the same name but different contents +// have been added, an error will be returned. +func ApplyTaskModifier(ts *TaskSpec, tm TaskModifier) error { steps := tm.GetStepsToPrepend() + for _, step := range steps { + if err := checkStepNotAlreadyAdded(step, ts.Steps); err != nil { + return err + } + } ts.Steps = append(steps, ts.Steps...) steps = tm.GetStepsToAppend() + for _, step := range steps { + if err := checkStepNotAlreadyAdded(step, ts.Steps); err != nil { + return err + } + } ts.Steps = append(ts.Steps, steps...) volumes := tm.GetVolumes() - ts.Volumes = append(ts.Volumes, volumes...) + for _, volume := range volumes { + var alreadyAdded bool + for _, v := range ts.Volumes { + if volume.Name == v.Name { + // If a Volume with the same name but different contents has already been added, we can't add both + if d := cmp.Diff(volume, v); d != "" { + return xerrors.Errorf("Tried to add volume %s already added but with different contents", volume.Name) + } + // If an identical Volume has already been added, don't add it again + alreadyAdded = true + } + } + if !alreadyAdded { + ts.Volumes = append(ts.Volumes, volume) + } + } + + return nil +} + +// PipelineResourceSetupInterface is an interface that can be implemented by objects that know +// how to perform setup required by PipelineResources before they can be realized. PipelineResources +// can return the instance of the appropriate PipelineResourceSetupInterface. +type PipelineResourceSetupInterface interface { + // Setup is called to setup any state that is required by a PipelineResource before + // executing. It is provided with a kubernetes clientset c so that it can make changes + // in the outside world if required, the owner references o that it should + // add to any new kubernetes objects it instantiates, and the PipelineResourceInterface r. + Setup(r PipelineResourceInterface, o []metav1.OwnerReference, c kubernetes.Interface) error +} + +// NoSetup is a PipelineResourceSetupInterface that doesn't do anything. It can be used by +// PipelineResources that do not require any setup. +type NoSetup struct{} + +// Setup for a NoSetup object does nothing, indicating that no setup is required. +func (n *NoSetup) Setup(r PipelineResourceInterface, o []metav1.OwnerReference, c kubernetes.Interface) error { + return nil } // SecretParam indicates which secret can be used to populate a field of the resource @@ -196,7 +267,9 @@ type ResourceDeclaration struct { TargetPath string `json:"targetPath,omitempty"` } -// ResourceFromType returns a PipelineResourceInterface from a PipelineResource's type. +// ResourceFromType returns an instance of the correct PipelineResource object type which can be +// used to add input and ouput containers as well as volumes to a TaskRun's pod in order to realize +// a PipelineResource in a pod. func ResourceFromType(r *PipelineResource, images pipeline.Images) (PipelineResourceInterface, error) { switch r.Spec.Type { case PipelineResourceTypeGit: diff --git a/pkg/apis/pipeline/v1alpha1/resource_types_test.go b/pkg/apis/pipeline/v1alpha1/resource_types_test.go new file mode 100644 index 00000000000..6d4ec5249ae --- /dev/null +++ b/pkg/apis/pipeline/v1alpha1/resource_types_test.go @@ -0,0 +1,147 @@ +/* +Copyright 2019 The Tekton Authors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package v1alpha1_test + +import ( + "testing" + + "github.com/google/go-cmp/cmp" + "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1" + corev1 "k8s.io/api/core/v1" +) + +var ( + prependStep = corev1.Container{ + Name: "prepend-step", + Image: "dummy", + Command: []string{"doit"}, + Args: []string{"stuff", "things"}, + } + appendStep = corev1.Container{ + Name: "append-step", + Image: "dummy", + Command: []string{"doit"}, + Args: []string{"other stuff", "other things"}, + } + volume = corev1.Volume{ + Name: "magic-volume", + VolumeSource: corev1.VolumeSource{ + PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ClaimName: "some-claim"}, + }, + } +) + +type TestTaskModifier struct{} + +func (tm *TestTaskModifier) GetStepsToPrepend() []v1alpha1.Step { + return []v1alpha1.Step{{ + Container: prependStep, + }} +} + +func (tm *TestTaskModifier) GetStepsToAppend() []v1alpha1.Step { + return []v1alpha1.Step{{ + Container: appendStep, + }} +} + +func (tm *TestTaskModifier) GetVolumes() []corev1.Volume { + return []corev1.Volume{volume} +} + +func TestApplyTaskModifier(t *testing.T) { + testcases := []struct { + name string + ts v1alpha1.TaskSpec + }{{ + name: "success", + ts: v1alpha1.TaskSpec{}, + }, { + name: "identical volume already added", + ts: v1alpha1.TaskSpec{ + // Trying to add the same Volume that has already been added shouldn't be an error + // and it should not be added twice + Volumes: []corev1.Volume{volume}, + }, + }} + for _, tc := range testcases { + t.Run(tc.name, func(t *testing.T) { + if err := v1alpha1.ApplyTaskModifier(&tc.ts, &TestTaskModifier{}); err != nil { + t.Fatalf("Did not expect error modifying TaskSpec but got %v", err) + } + + expectedTaskSpec := v1alpha1.TaskSpec{ + Steps: []v1alpha1.Step{{ + Container: prependStep, + }, { + Container: appendStep, + }}, + Volumes: []corev1.Volume{ + volume, + }, + } + + if d := cmp.Diff(expectedTaskSpec, tc.ts); d != "" { + t.Errorf("TaskSpec was not modified as expected (-want, +got): %s", d) + } + }) + } +} + +func TestApplyTaskModifier_AlreadyAdded(t *testing.T) { + testcases := []struct { + name string + ts v1alpha1.TaskSpec + }{{ + name: "prepend already added", + ts: v1alpha1.TaskSpec{ + Steps: []v1alpha1.Step{{Container: prependStep}}, + }, + }, { + name: "append already added", + ts: v1alpha1.TaskSpec{ + Steps: []v1alpha1.Step{{Container: appendStep}}, + }, + }, { + name: "both steps already added", + ts: v1alpha1.TaskSpec{ + Steps: []v1alpha1.Step{{Container: prependStep}, {Container: appendStep}}, + }, + }, { + name: "both steps already added reverse order", + ts: v1alpha1.TaskSpec{ + Steps: []v1alpha1.Step{{Container: appendStep}, {Container: prependStep}}, + }, + }, { + name: "volume with same name but diff content already added", + ts: v1alpha1.TaskSpec{ + Volumes: []corev1.Volume{{ + Name: "magic-volume", + VolumeSource: corev1.VolumeSource{ + EmptyDir: &corev1.EmptyDirVolumeSource{}, + }, + }}, + }, + }} + for _, tc := range testcases { + t.Run(tc.name, func(t *testing.T) { + if err := v1alpha1.ApplyTaskModifier(&tc.ts, &TestTaskModifier{}); err == nil { + t.Errorf("Expected error when applying values already added but got none") + } + }) + } +} diff --git a/pkg/apis/pipeline/v1alpha1/storage_resource.go b/pkg/apis/pipeline/v1alpha1/storage_resource.go index 19e0d43012a..84bd5919290 100644 --- a/pkg/apis/pipeline/v1alpha1/storage_resource.go +++ b/pkg/apis/pipeline/v1alpha1/storage_resource.go @@ -25,20 +25,29 @@ import ( corev1 "k8s.io/api/core/v1" ) +// PipelineResourceStorageType is used as an enum for subtypes of the storage resource. type PipelineResourceStorageType string const ( - // PipelineResourceTypeGCS indicates that resource source is a GCS blob/directory. - PipelineResourceTypeGCS PipelineResourceType = "gcs" + // PipelineResourceTypeGCS is the subtype for the GCSResources, which is backed by a GCS blob/directory. + PipelineResourceTypeGCS PipelineResourceType = "gcs" + // PipelineResourceTypeBuildGCS is the subtype for the BuildGCSResources, which is simialr to the GCSResource but + // with additional funcitonality that was added to be compatible with knative build. PipelineResourceTypeBuildGCS PipelineResourceType = "build-gcs" + // PipelineResourceTypeVolume is the subtype for the VolumeResource, which is backed by a PVC. + PipelineResourceTypeVolume PipelineResourceType = "volume" ) -// PipelineResourceInterface interface to be implemented by different PipelineResource types +// PipelineStorageResourceInterface is the interface for subtypes of the storage type. +// It adds a function to the PipelineResourceInterface for retrieving secrets that are usually +// needed for storage PipelineResources. type PipelineStorageResourceInterface interface { PipelineResourceInterface GetSecretParams() []SecretParam } +// NewStorageResource returns an instance of the requested storage subtype, which can be used +// to add input and output steps and volumes to an executing pod. func NewStorageResource(images pipeline.Images, r *PipelineResource) (PipelineStorageResourceInterface, error) { if r.Spec.Type != PipelineResourceTypeStorage { return nil, xerrors.Errorf("StoreResource: Cannot create a storage resource from a %s Pipeline Resource", r.Spec.Type) @@ -51,6 +60,8 @@ func NewStorageResource(images pipeline.Images, r *PipelineResource) (PipelineSt return NewGCSResource(images, r) case strings.EqualFold(param.Value, string(PipelineResourceTypeBuildGCS)): return NewBuildGCSResource(images, r) + case strings.EqualFold(param.Value, string(PipelineResourceTypeVolume)): + return NewVolumeResource(images, r) default: return nil, xerrors.Errorf("%s is an invalid or unimplemented PipelineStorageResource", param.Value) } diff --git a/pkg/apis/pipeline/v1alpha1/taskrun_types.go b/pkg/apis/pipeline/v1alpha1/taskrun_types.go index 74f46da130d..d2358f77fd6 100644 --- a/pkg/apis/pipeline/v1alpha1/taskrun_types.go +++ b/pkg/apis/pipeline/v1alpha1/taskrun_types.go @@ -258,6 +258,13 @@ func (tr *TaskRun) GetPipelineRunPVCName() string { return "" } +// GetOwnerReference gets the task run as owner reference for any related objects +func (tr *TaskRun) GetOwnerReference() []metav1.OwnerReference { + return []metav1.OwnerReference{ + *metav1.NewControllerRef(tr, groupVersionKind), + } +} + // HasPipelineRunOwnerReference returns true of TaskRun has // owner reference of type PipelineRun func (tr *TaskRun) HasPipelineRunOwnerReference() bool { diff --git a/pkg/apis/pipeline/v1alpha1/volume_resource.go b/pkg/apis/pipeline/v1alpha1/volume_resource.go new file mode 100644 index 00000000000..e9f90d1a352 --- /dev/null +++ b/pkg/apis/pipeline/v1alpha1/volume_resource.go @@ -0,0 +1,229 @@ +/* + Copyright 2019 The Tekton Authors + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + http://www.apache.org/licenses/LICENSE-2.0 + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +package v1alpha1 + +import ( + "fmt" + "path/filepath" + "strings" + + "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/api/resource" + "k8s.io/client-go/kubernetes" + + "github.com/tektoncd/pipeline/pkg/apis/pipeline" + "github.com/tektoncd/pipeline/pkg/names" + "golang.org/x/xerrors" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +const ( + // DefaultPvcSize is the default size of the PVC to create + DefaultPvcSize = "5Gi" + + // VolumeMountDirBase will be joined with the name of the Resource itself to produce the location + // where the volume is mounted + VolumeMountDirBase = "/volumeresource" +) + +// SetupPVC is a PipelineResourceSetupInterface that can idempotently create the PVC +// that is expected by the VolumeResource. +type SetupPVC struct{} + +// Setup creates an instance of the PVC required by VolumeResource, unless it already exists. +// The PVC will have the same name as the PipelineResource. +func (n SetupPVC) Setup(r PipelineResourceInterface, o []metav1.OwnerReference, c kubernetes.Interface) error { + v, ok := r.(*VolumeResource) + if !ok { + return xerrors.Errorf("Setup expected to be called with instance of VolumeResource but was called with %v", r) + } + return ApplyPVC(v.Name, v.Namespace, &v.StorageClassName, v.ParsedSize, o, c.CoreV1().PersistentVolumeClaims(v.Namespace).Get, c.CoreV1().PersistentVolumeClaims(v.Namespace).Create) +} + +// CreatePVC is a function that creates a PVC from the specified spec. +type CreatePVC func(*corev1.PersistentVolumeClaim) (*corev1.PersistentVolumeClaim, error) + +// GetPVC retrieves the requested PVC and returns an error if it can't be found. +type GetPVC func(name string, options metav1.GetOptions) (*corev1.PersistentVolumeClaim, error) + +// ApplyPVC will create a PVC with the requested name, namespace, size, storageClassName and owner references, +// unless a PVC with the same name in the same namespace already exists. +func ApplyPVC(name, namespace string, storageClassName *string, size resource.Quantity, o []metav1.OwnerReference, get GetPVC, create CreatePVC) error { + if _, err := get(name, metav1.GetOptions{}); err != nil { + if !errors.IsNotFound(err) { + return xerrors.Errorf("failed to retrieve Persistent Volume Claim %q for VolumeResource: %w", name, err) + } + pvcSpec := &corev1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: namespace, + Name: name, + OwnerReferences: o, + }, + Spec: corev1.PersistentVolumeClaimSpec{ + AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadWriteOnce}, + Resources: corev1.ResourceRequirements{ + Requests: map[corev1.ResourceName]resource.Quantity{ + corev1.ResourceStorage: size, + }, + }, + StorageClassName: storageClassName, + }, + } + if _, err = create(pvcSpec); err != nil { + return xerrors.Errorf("failed to create Persistent Volume Claim %q for VolumeResource: %w", name, err) + } + } + return nil +} + +// VolumeResource is a volume which can be used to share data between Tasks. +type VolumeResource struct { + // Name of the underlying PipelineResource that this was instantiated for. + Name string + // Namespace of the underlying PipelineResource that this was instantiated for. + Namespace string + // Type of this PipelineResource. Exists because string comparison is easier than introspecting. + Type PipelineResourceType + // SubPath is the path to directory on the underlying PVC to either copy from or to. SubPath is always relative + // to the mount path of the PVC. + SubPath string + // StorageClassName is the name of the storage class to use when creating the PVC. + StorageClassName string + // ParsedSize is the size that was reuquested by the user for the PVC. + ParsedSize resource.Quantity + + BashNoopImage string `json:"-"` +} + +// GetSetup returns a PipelineResourceSetupInterface ti think we can probably give more visitbhat can create the backing PVC if needed. +func (s VolumeResource) GetSetup() PipelineResourceSetupInterface { return SetupPVC{} } + +// NewVolumeResource instantiates the VolumeResource by parsing its params. +func NewVolumeResource(images pipeline.Images, r *PipelineResource) (*VolumeResource, error) { + if r.Spec.Type != PipelineResourceTypeStorage { + return nil, xerrors.Errorf("VolumeResource: Cannot create a volume resource from a %s Pipeline Resource", r.Spec.Type) + } + s := &VolumeResource{ + Name: r.Name, + Namespace: r.Namespace, + Type: r.Spec.Type, + BashNoopImage: images.BashNoopImage, + } + size := DefaultPvcSize + for _, param := range r.Spec.Params { + switch { + case strings.EqualFold(param.Name, "Size"): + size = param.Value + case strings.EqualFold(param.Name, "SubPath"): + s.SubPath = param.Value + case strings.EqualFold(param.Name, "StorageClassName"): + s.StorageClassName = param.Value + } + } + var err error + s.ParsedSize, err = resource.ParseQuantity(size) + if err != nil { + return nil, xerrors.Errorf("failed to parse size for VolumeResource %q: %w", r.Name, err) + } + return s, nil +} + +// GetName returns the name of the resource +func (s VolumeResource) GetName() string { + return s.Name +} + +// GetType returns the type of the resource, in this case "volume" +func (s VolumeResource) GetType() PipelineResourceType { + return PipelineResourceTypeVolume +} + +// Replacements is used for template replacement on an VolumeResource inside of a Taskrun. +func (s VolumeResource) Replacements() map[string]string { + return map[string]string{ + "name": s.Name, + "type": string(s.Type), + "subPath": s.SubPath, + "size": s.ParsedSize.String(), + "storageClassName": s.StorageClassName, + } +} + +// GetOutputTaskModifier returns the TaskModifier that adds steps to copy data from the sourcePath +// on disk onto the Volume so it can be persisted. +func (s VolumeResource) GetOutputTaskModifier(_ *TaskSpec, sourcePath string) (TaskModifier, error) { + // Cant' use filepath.Join because Join does a "clean" which removes the trailing "." + pathToCopy := fmt.Sprintf("%s/.", sourcePath) + steps := []Step{ + CreateDirStep(s.BashNoopImage, s.Name, filepath.Join(s.getVolumeMountDirPath(), s.SubPath), []corev1.VolumeMount{s.getPvcMount()}), + {Container: corev1.Container{ + Name: names.SimpleNameGenerator.RestrictLengthWithRandomSuffix(fmt.Sprintf("upload-copy-%s", s.Name)), + Image: s.BashNoopImage, + Command: []string{"/ko-app/bash"}, + Args: []string{ + "-args", strings.Join([]string{"cp", "-r", pathToCopy, filepath.Join(s.getVolumeMountDirPath(), s.SubPath)}, " "), + }, + VolumeMounts: []corev1.VolumeMount{s.getPvcMount()}, + }}} + return &InternalTaskModifier{ + StepsToAppend: steps, + Volumes: s.getVolumeSpec(), + }, nil +} + +// GetInputTaskModifier returns the steps that are needed to copy data from the volume to the +// sourcePath on disk so that the steps in the Task will have access to it. +func (s VolumeResource) GetInputTaskModifier(_ *TaskSpec, sourcePath string) (TaskModifier, error) { + // Cant' use filepath.Join because Join does a "clean" which removes the trailing "." + pathToCopy := fmt.Sprintf("%s/.", filepath.Join(s.getVolumeMountDirPath(), s.SubPath)) + steps := []Step{ + CreateDirStep(s.BashNoopImage, s.Name, sourcePath, nil), + {Container: corev1.Container{ + Name: names.SimpleNameGenerator.RestrictLengthWithRandomSuffix(fmt.Sprintf("download-copy-%s", s.Name)), + Image: s.BashNoopImage, + Command: []string{"/ko-app/bash"}, + Args: []string{ + "-args", strings.Join([]string{"cp", "-r", pathToCopy, sourcePath}, " "), + }, + VolumeMounts: []corev1.VolumeMount{s.getPvcMount()}, + }}} + return &InternalTaskModifier{ + StepsToPrepend: steps, + Volumes: s.getVolumeSpec(), + }, nil +} + +func (s VolumeResource) getPvcMount() corev1.VolumeMount { + return corev1.VolumeMount{ + Name: s.Name, + MountPath: s.getVolumeMountDirPath(), + } +} + +func (s VolumeResource) getVolumeMountDirPath() string { + return fmt.Sprintf("%s-%s", VolumeMountDirBase, s.Name) +} + +func (s VolumeResource) getVolumeSpec() []corev1.Volume { + return []corev1.Volume{{ + Name: s.Name, + VolumeSource: corev1.VolumeSource{ + PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ClaimName: s.Name}, + }, + }} +} + +// GetSecretParams returns nothing because the VolumeResource does not use secrets. +func (s VolumeResource) GetSecretParams() []SecretParam { return nil } diff --git a/pkg/apis/pipeline/v1alpha1/volume_resource_test.go b/pkg/apis/pipeline/v1alpha1/volume_resource_test.go new file mode 100644 index 00000000000..5882ba76ab7 --- /dev/null +++ b/pkg/apis/pipeline/v1alpha1/volume_resource_test.go @@ -0,0 +1,380 @@ +/* + Copyright 2019 The Tekton Authors. + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + http://www.apache.org/licenses/LICENSE-2.0 + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +package v1alpha1_test + +import ( + "testing" + + "github.com/google/go-cmp/cmp" + "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1" + tb "github.com/tektoncd/pipeline/test/builder" + "github.com/tektoncd/pipeline/test/names" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/api/resource" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +func compareVolumeResource(t *testing.T, got, want *v1alpha1.VolumeResource) { + t.Helper() + if got.Name != want.Name { + t.Errorf("Expected both to have name %s but got %s", want.Name, got.Name) + } + if got.Type != want.Type { + t.Errorf("Expected both to have type %s but got %s", want.Type, got.Type) + } + if got.SubPath != want.SubPath { + t.Errorf("Expected both to have SubPath %s but got %s", want.SubPath, got.SubPath) + } + if got.ParsedSize != want.ParsedSize { + t.Errorf("Expected both to have ParsedSize %v but got %v", want.ParsedSize, got.ParsedSize) + } + if got.BashNoopImage != want.BashNoopImage { + t.Errorf("Expected both to have BashNoopImage %v but got %v", want.BashNoopImage, got.BashNoopImage) + } + if got.StorageClassName != want.StorageClassName { + t.Errorf("Expected both to have StorageClassName %v but got %v", want.StorageClassName, got.StorageClassName) + } +} + +func TestNewVolumeResource(t *testing.T) { + size5, err := resource.ParseQuantity("5Gi") + if err != nil { + t.Fatalf("Failed to parse size: %v", err) + } + size10, err := resource.ParseQuantity("10Gi") + if err != nil { + t.Fatalf("Failed to parse size: %v", err) + } + for _, c := range []struct { + desc string + resource *v1alpha1.PipelineResource + want *v1alpha1.VolumeResource + pvcExists bool + }{{ + desc: "basic volume resource", + resource: tb.PipelineResource("test-volume-resource", "default", tb.PipelineResourceSpec( + v1alpha1.PipelineResourceTypeStorage, + tb.PipelineResourceSpecParam("name", "test-volume-resource"), + tb.PipelineResourceSpecParam("type", "volume"), + )), + pvcExists: true, + want: &v1alpha1.VolumeResource{ + Name: "test-volume-resource", + Type: v1alpha1.PipelineResourceTypeStorage, + ParsedSize: size5, + BashNoopImage: "override-with-bash-noop:latest", + }, + }, { + desc: "volume resource with size, storage class, and subpath", + resource: tb.PipelineResource("test-volume-resource", "default", tb.PipelineResourceSpec( + v1alpha1.PipelineResourceTypeStorage, + tb.PipelineResourceSpecParam("name", "test-volume-resource"), + tb.PipelineResourceSpecParam("size", "10Gi"), + tb.PipelineResourceSpecParam("type", "volume"), + tb.PipelineResourceSpecParam("subpath", "greatfolder"), + tb.PipelineResourceSpecParam("storageClassName", "myStorageClass"), + )), + pvcExists: true, + want: &v1alpha1.VolumeResource{ + Name: "test-volume-resource", + Type: v1alpha1.PipelineResourceTypeStorage, + ParsedSize: size10, + SubPath: "greatfolder", + StorageClassName: "myStorageClass", + BashNoopImage: "override-with-bash-noop:latest", + }, + }} { + t.Run(c.desc, func(t *testing.T) { + got, err := v1alpha1.NewVolumeResource(images, c.resource) + if err != nil { + t.Errorf("Didn't expect error creating volume resource but got %v", err) + } + compareVolumeResource(t, got, c.want) + }) + } +} + +func TestNewVolumeResource_Invalid(t *testing.T) { + resource := tb.PipelineResource("test-volume-resource", "default", tb.PipelineResourceSpec( + v1alpha1.PipelineResourceTypeStorage, + tb.PipelineResourceSpecParam("name", "test-volume-resource"), + tb.PipelineResourceSpecParam("size", "about the size of a small dog"), + tb.PipelineResourceSpecParam("type", "volume"), + )) + _, err := v1alpha1.NewVolumeResource(images, resource) + if err == nil { + t.Errorf("Epxected error when creating resource with invalid size but got none") + } +} + +func TestReplacements(t *testing.T) { + size5, err := resource.ParseQuantity("5Gi") + if err != nil { + t.Fatalf("Failed to parse size: %v", err) + } + volume := &v1alpha1.VolumeResource{ + Name: "test-volume-resource", + Type: v1alpha1.PipelineResourceTypeStorage, + ParsedSize: size5, + SubPath: "greatfolder", + StorageClassName: "greatstorageclass", + } + replacements := volume.Replacements() + expectedReplacements := map[string]string{ + "name": "test-volume-resource", + "type": "storage", + "size": "5Gi", + "subPath": "greatfolder", + "storageClassName": "greatstorageclass", + } + if d := cmp.Diff(expectedReplacements, replacements); d != "" { + t.Errorf("Did not get expected replacements (-want, +got): %v", d) + } +} + +func TestApplyPVC_doesntExist(t *testing.T) { + ownerReferences := []metav1.OwnerReference{{Name: "SomeTaskRun"}} + name, namespace, storageClassName := "mypvc", "foospace", "mystorageclass" + size, err := resource.ParseQuantity("7Gi") + if err != nil { + t.Fatalf("Unexpected error parsing size argument: %v", err) + } + var pvcToCreate *corev1.PersistentVolumeClaim + create := func(pvc *corev1.PersistentVolumeClaim) (*corev1.PersistentVolumeClaim, error) { + pvcToCreate = pvc + return pvc, nil + } + get := func(name string, options metav1.GetOptions) (*corev1.PersistentVolumeClaim, error) { + return nil, errors.NewNotFound(corev1.Resource("persistentvolumeclaim"), name) + } + err = v1alpha1.ApplyPVC(name, namespace, &storageClassName, size, ownerReferences, get, create) + if err != nil { + t.Fatalf("Didn't expect error when creating PVC that didn't exist but got %v", err) + } + if pvcToCreate == nil { + t.Fatalf("Expected create to be called with PVC to create but it wasn't") + } + if len(pvcToCreate.OwnerReferences) != 1 || pvcToCreate.OwnerReferences[0].Name != "SomeTaskRun" { + t.Errorf("Expected PVC to be created with passed in owner references but they were %v", pvcToCreate.OwnerReferences) + } + if pvcToCreate.Name != name || pvcToCreate.Namespace != namespace { + t.Errorf("Expected PVC to be called %s/%s but was called %s/%s", namespace, name, pvcToCreate.Namespace, pvcToCreate.Name) + } + if pvcToCreate.Spec.StorageClassName != nil { + if *pvcToCreate.Spec.StorageClassName != storageClassName { + t.Errorf("Expected PVC to be created with storage class %s but was %s", storageClassName, *pvcToCreate.Spec.StorageClassName) + } + } else { + t.Errorf("Expected PVC storage class to be set but as nil") + } +} + +func TestApplyPVC_exists(t *testing.T) { + ownerReferences := []metav1.OwnerReference{{Name: "SomeTaskRun"}} + name, namespace := "mypvc", "foospace" + size, err := resource.ParseQuantity("7Gi") + if err != nil { + t.Fatalf("Unexpected error parsing size argument: %v", err) + } + create := func(pvc *corev1.PersistentVolumeClaim) (*corev1.PersistentVolumeClaim, error) { + return nil, errors.NewAlreadyExists(corev1.Resource("persistentvolumeclaim"), "Didn't expect create to be called") + } + existingPVC := &corev1.PersistentVolumeClaim{} + get := func(name string, options metav1.GetOptions) (*corev1.PersistentVolumeClaim, error) { + return existingPVC, nil + } + err = v1alpha1.ApplyPVC(name, namespace, nil, size, ownerReferences, get, create) + if err != nil { + t.Fatalf("Didn't expect error since PVC already exists but got %v", err) + } +} + +func Test_VolumeResource_GetInputTaskModifier(t *testing.T) { + size, err := resource.ParseQuantity(v1alpha1.DefaultPvcSize) + if err != nil { + t.Fatalf("Failed to parse size: %v", err) + } + names.TestingSeed() + testcases := []struct { + name string + volumeResource *v1alpha1.VolumeResource + wantSteps []v1alpha1.Step + }{{ + name: "with path", + volumeResource: &v1alpha1.VolumeResource{ + Name: "test-volume-resource", + Type: v1alpha1.PipelineResourceTypeVolume, + ParsedSize: size, + SubPath: "/src-dir", + BashNoopImage: "override-with-bash-noop:latest", + }, + wantSteps: []v1alpha1.Step{{Container: corev1.Container{ + Name: "create-dir-test-volume-resource-9l9zj", + Image: "override-with-bash-noop:latest", + Command: []string{"/ko-app/bash"}, + Args: []string{"-args", "mkdir -p /workspace/foobar"}, + }}, {Container: corev1.Container{ + Name: "download-copy-test-volume-resource-mz4c7", + Image: "override-with-bash-noop:latest", + Command: []string{"/ko-app/bash"}, + Args: []string{"-args", "cp -r /volumeresource-test-volume-resource/src-dir/. /workspace/foobar"}, + VolumeMounts: []corev1.VolumeMount{{ + Name: "test-volume-resource", + MountPath: "/volumeresource-test-volume-resource", + }}, + }}}, + }, { + name: "without path", + volumeResource: &v1alpha1.VolumeResource{ + Name: "test-volume-resource", + Type: v1alpha1.PipelineResourceTypeVolume, + ParsedSize: size, + BashNoopImage: "override-with-bash-noop:latest", + }, + wantSteps: []v1alpha1.Step{{Container: corev1.Container{ + Name: "create-dir-test-volume-resource-mssqb", + Image: "override-with-bash-noop:latest", + Command: []string{"/ko-app/bash"}, + Args: []string{"-args", "mkdir -p /workspace/foobar"}, + }}, {Container: corev1.Container{ + Name: "download-copy-test-volume-resource-78c5n", + Image: "override-with-bash-noop:latest", + Command: []string{"/ko-app/bash"}, + Args: []string{"-args", "cp -r /volumeresource-test-volume-resource/. /workspace/foobar"}, + VolumeMounts: []corev1.VolumeMount{{ + Name: "test-volume-resource", + MountPath: "/volumeresource-test-volume-resource", + }}, + }}}, + }} + for _, tc := range testcases { + t.Run(tc.name, func(t *testing.T) { + expectedVolumes := []corev1.Volume{{ + Name: tc.volumeResource.Name, + VolumeSource: corev1.VolumeSource{ + PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ClaimName: tc.volumeResource.Name}, + }, + }} + + tm, err := tc.volumeResource.GetInputTaskModifier(nil, "/workspace/foobar") + if err != nil { + t.Fatalf("Did not expect error when getting steps but got %v", err) + } + + if d := cmp.Diff(tc.wantSteps, tm.GetStepsToPrepend()); d != "" { + t.Errorf("Mismatch between steps to prepend (-want, +got): %s", d) + } + if len(tm.GetStepsToAppend()) != 0 { + t.Errorf("Did not expect to append steps but got %v", tm.GetStepsToPrepend()) + } + if d := cmp.Diff(expectedVolumes, tm.GetVolumes()); d != "" { + t.Errorf("Mismatch between volumes (-want, +got): %s", d) + } + }) + } +} + +func Test_VolumeResource_GetOutputTaskModifier(t *testing.T) { + size, err := resource.ParseQuantity(v1alpha1.DefaultPvcSize) + if err != nil { + t.Fatalf("Failed to parse size: %v", err) + } + names.TestingSeed() + testcases := []struct { + name string + volumeResource *v1alpha1.VolumeResource + wantSteps []v1alpha1.Step + }{{ + name: "with path", + volumeResource: &v1alpha1.VolumeResource{ + Name: "test-volume-resource", + Type: v1alpha1.PipelineResourceTypeVolume, + ParsedSize: size, + SubPath: "/src-dir", + BashNoopImage: "override-with-bash-noop:latest", + }, + wantSteps: []v1alpha1.Step{{Container: corev1.Container{ + Name: "create-dir-test-volume-resource-9l9zj", + Image: "override-with-bash-noop:latest", + Command: []string{"/ko-app/bash"}, + Args: []string{"-args", "mkdir -p /volumeresource-test-volume-resource/src-dir"}, + VolumeMounts: []corev1.VolumeMount{{ + Name: "test-volume-resource", + MountPath: "/volumeresource-test-volume-resource", + }}, + }}, {Container: corev1.Container{ + Name: "upload-copy-test-volume-resource-mz4c7", + Image: "override-with-bash-noop:latest", + Command: []string{"/ko-app/bash"}, + Args: []string{"-args", "cp -r /workspace/output/foobar/. /volumeresource-test-volume-resource/src-dir"}, + VolumeMounts: []corev1.VolumeMount{{ + Name: "test-volume-resource", + MountPath: "/volumeresource-test-volume-resource", + }}, + }}}, + }, { + name: "without path", + volumeResource: &v1alpha1.VolumeResource{ + Name: "test-volume-resource", + Type: v1alpha1.PipelineResourceTypeVolume, + ParsedSize: size, + BashNoopImage: "override-with-bash-noop:latest", + }, + wantSteps: []v1alpha1.Step{{Container: corev1.Container{ + Name: "create-dir-test-volume-resource-mssqb", + Image: "override-with-bash-noop:latest", + Command: []string{"/ko-app/bash"}, + Args: []string{"-args", "mkdir -p /volumeresource-test-volume-resource"}, + VolumeMounts: []corev1.VolumeMount{{ + Name: "test-volume-resource", + MountPath: "/volumeresource-test-volume-resource", + }}, + }}, {Container: corev1.Container{ + Name: "upload-copy-test-volume-resource-78c5n", + Image: "override-with-bash-noop:latest", + Command: []string{"/ko-app/bash"}, + Args: []string{"-args", "cp -r /workspace/output/foobar/. /volumeresource-test-volume-resource"}, + VolumeMounts: []corev1.VolumeMount{{ + Name: "test-volume-resource", + MountPath: "/volumeresource-test-volume-resource", + }}, + }}}, + }} + for _, tc := range testcases { + t.Run(tc.name, func(t *testing.T) { + expectedVolumes := []corev1.Volume{{ + Name: tc.volumeResource.Name, + VolumeSource: corev1.VolumeSource{ + PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ClaimName: tc.volumeResource.Name}, + }, + }} + + tm, err := tc.volumeResource.GetOutputTaskModifier(nil, "/workspace/output/foobar") + if err != nil { + t.Fatalf("Did not expect error when getting steps but got %v", err) + } + + if len(tm.GetStepsToPrepend()) != 0 { + t.Errorf("Did not expect to prepend steps but got %v", tm.GetStepsToPrepend()) + } + if d := cmp.Diff(tc.wantSteps, tm.GetStepsToAppend()); d != "" { + t.Errorf("Mismatch between steps to append (-want, +got): %s", d) + } + if d := cmp.Diff(expectedVolumes, tm.GetVolumes()); d != "" { + t.Errorf("Mismatch between volumes (-want, +got): %s", d) + } + }) + } +} diff --git a/pkg/apis/pipeline/v1alpha1/zz_generated.deepcopy.go b/pkg/apis/pipeline/v1alpha1/zz_generated.deepcopy.go index c8e8360badf..63bac1fd53e 100644 --- a/pkg/apis/pipeline/v1alpha1/zz_generated.deepcopy.go +++ b/pkg/apis/pipeline/v1alpha1/zz_generated.deepcopy.go @@ -544,6 +544,22 @@ func (in *InternalTaskModifier) DeepCopy() *InternalTaskModifier { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *NoSetup) DeepCopyInto(out *NoSetup) { + *out = *in + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new NoSetup. +func (in *NoSetup) DeepCopy() *NoSetup { + if in == nil { + return nil + } + out := new(NoSetup) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *Node) DeepCopyInto(out *Node) { *out = *in @@ -1503,6 +1519,22 @@ func (in *SecretParam) DeepCopy() *SecretParam { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *SetupPVC) DeepCopyInto(out *SetupPVC) { + *out = *in + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new SetupPVC. +func (in *SetupPVC) DeepCopy() *SetupPVC { + if in == nil { + return nil + } + out := new(SetupPVC) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *Step) DeepCopyInto(out *Step) { *out = *in @@ -1928,3 +1960,20 @@ func (in *TestResult) DeepCopy() *TestResult { in.DeepCopyInto(out) return out } + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *VolumeResource) DeepCopyInto(out *VolumeResource) { + *out = *in + out.ParsedSize = in.ParsedSize.DeepCopy() + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new VolumeResource. +func (in *VolumeResource) DeepCopy() *VolumeResource { + if in == nil { + return nil + } + out := new(VolumeResource) + in.DeepCopyInto(out) + return out +} diff --git a/pkg/reconciler/pipelinerun/pipelinerun.go b/pkg/reconciler/pipelinerun/pipelinerun.go index 57979f9c877..612a15d9a46 100644 --- a/pkg/reconciler/pipelinerun/pipelinerun.go +++ b/pkg/reconciler/pipelinerun/pipelinerun.go @@ -277,6 +277,25 @@ func (c *Reconciler) reconcile(ctx context.Context, pr *v1alpha1.PipelineRun) er return nil } + // Some PipelineResources will require state to be setup before they can be used. + // If this PipelineResource is being used as part of a PipelineRun, any state that it needs + // created should be created in the same reconcile loop. For example, creating two different + // PVCs in two separate reconcile loops with GKE then trying to create a TaskRun that uses + // both can create an unschedulable pod if the PVCs end up created in different zones. + for name, r := range providedResources { + rr, err := v1alpha1.ResourceFromType(r, c.Images) + if err != nil { + c.Logger.Errorf("Unexpected error getting PipelineResource instance for %s: %v", name, err) + return nil + } + s := rr.GetSetup() + err = s.Setup(rr, pr.GetOwnerReference(), c.KubeClientSet) + if err != nil { + c.Logger.Errorf("Failed to setup PipelineResource %s: %v", name, err) + return nil + } + } + // Ensure that the parameters from the PipelineRun are overriding Pipeline parameters with the same type. // Weird substitution issues can occur if this is not validated (ApplyParameters() does not verify type). err = resources.ValidateParamTypesMatching(pipelineSpec, pr) diff --git a/pkg/reconciler/pipelinerun/pipelinerun_test.go b/pkg/reconciler/pipelinerun/pipelinerun_test.go index b8abb1d9033..66f4288c7b3 100644 --- a/pkg/reconciler/pipelinerun/pipelinerun_test.go +++ b/pkg/reconciler/pipelinerun/pipelinerun_test.go @@ -98,6 +98,7 @@ func TestReconcile(t *testing.T) { }}, }, )), + tb.PipelineRunResourceBinding("volume-resource", tb.PipelineResourceBindingRef("some-volume")), tb.PipelineRunParam("bar", "somethingmorefun"), ), ), @@ -109,6 +110,7 @@ func TestReconcile(t *testing.T) { tb.Pipeline("test-pipeline", "foo", tb.PipelineSpec( tb.PipelineDeclaredResource("git-repo", "git"), + tb.PipelineDeclaredResource("volume-resource", "storage"), tb.PipelineDeclaredResource("best-image", "image"), tb.PipelineParamSpec("pipeline-param", v1alpha1.ParamTypeString, tb.ParamSpecDefault("somethingdifferent")), tb.PipelineParamSpec("rev-param", v1alpha1.ParamTypeString, tb.ParamSpecDefault("revision")), @@ -118,24 +120,24 @@ func TestReconcile(t *testing.T) { tb.RunAfter("unit-test-2"), tb.PipelineTaskInputResource("workspace", "git-repo"), tb.PipelineTaskOutputResource("image-to-use", "best-image"), - tb.PipelineTaskOutputResource("workspace", "git-repo"), + tb.PipelineTaskOutputResource("workspace", "volume-resource"), ), // unit-test-1 can run right away because it has no dependencies tb.PipelineTask("unit-test-1", "unit-test-task", funParam, moreFunParam, templatedParam, tb.PipelineTaskInputResource("workspace", "git-repo"), tb.PipelineTaskOutputResource("image-to-use", "best-image"), - tb.PipelineTaskOutputResource("workspace", "git-repo"), + tb.PipelineTaskOutputResource("workspace", "volume-resource"), ), // unit-test-2 uses `from` to indicate it should run after `unit-test-1` tb.PipelineTask("unit-test-2", "unit-test-followup-task", - tb.PipelineTaskInputResource("workspace", "git-repo", tb.From("unit-test-1")), + tb.PipelineTaskInputResource("workspace", "volume-resource", tb.From("unit-test-1")), ), // unit-test-cluster-task can run right away because it has no dependencies tb.PipelineTask("unit-test-cluster-task", "unit-test-cluster-task", tb.PipelineTaskRefKind(v1alpha1.ClusterTaskKind), funParam, moreFunParam, templatedParam, - tb.PipelineTaskInputResource("workspace", "git-repo"), + tb.PipelineTaskInputResource("workspace", "volume-resource"), tb.PipelineTaskOutputResource("image-to-use", "best-image"), tb.PipelineTaskOutputResource("workspace", "git-repo"), ), @@ -150,17 +152,17 @@ func TestReconcile(t *testing.T) { ), tb.TaskOutputs( tb.OutputsResource("image-to-use", v1alpha1.PipelineResourceTypeImage), - tb.OutputsResource("workspace", v1alpha1.PipelineResourceTypeGit), + tb.OutputsResource("workspace", v1alpha1.PipelineResourceTypeStorage), ), )), tb.Task("unit-test-followup-task", "foo", tb.TaskSpec( - tb.TaskInputs(tb.InputsResource("workspace", v1alpha1.PipelineResourceTypeGit)), + tb.TaskInputs(tb.InputsResource("workspace", v1alpha1.PipelineResourceTypeStorage)), )), } clusterTasks := []*v1alpha1.ClusterTask{ tb.ClusterTask("unit-test-cluster-task", tb.ClusterTaskSpec( tb.TaskInputs( - tb.InputsResource("workspace", v1alpha1.PipelineResourceTypeGit), + tb.InputsResource("workspace", v1alpha1.PipelineResourceTypeStorage), tb.InputsParamSpec("foo", v1alpha1.ParamTypeString), tb.InputsParamSpec("bar", v1alpha1.ParamTypeString), tb.InputsParamSpec("templatedparam", v1alpha1.ParamTypeString), ), tb.TaskOutputs( @@ -177,12 +179,18 @@ func TestReconcile(t *testing.T) { v1alpha1.PipelineResourceTypeGit, tb.PipelineResourceSpecParam("url", "https://github.com/kristoff/reindeer"), )), + tb.PipelineResource("some-volume", "foo", tb.PipelineResourceSpec( + v1alpha1.PipelineResourceTypeStorage, + tb.PipelineResourceSpecParam("type", "volume"), + )), } // When PipelineResources are created in the cluster, Kubernetes will add a SelfLink. We // are using this to differentiate between Resources that we are referencing by Spec or by Ref // after we have resolved them. - rs[0].SelfLink = "some/link" + for _, r := range rs { + r.SelfLink = "some/link" + } d := test.Data{ PipelineRuns: prs, @@ -244,7 +252,7 @@ func TestReconcile(t *testing.T) { ), tb.TaskResourceBindingPaths("/pvc/unit-test-1/image-to-use"), ), - tb.TaskRunOutputsResource("workspace", tb.TaskResourceBindingRef("some-repo"), + tb.TaskRunOutputsResource("workspace", tb.TaskResourceBindingRef("some-volume"), tb.TaskResourceBindingPaths("/pvc/unit-test-1/workspace"), ), ), @@ -278,6 +286,11 @@ func TestReconcile(t *testing.T) { if _, exists := reconciledRun.Status.TaskRuns["test-pipeline-run-success-unit-test-cluster-task-78c5n"]; !exists { t.Errorf("Expected PipelineRun status to include TaskRun status but was %v", reconciledRun.Status.TaskRuns) } + + // Two PVCs should be created: one for input output linking (to be removed in #1284) + // and another for the VolumeResource (must be created by PipelineRun so there is not a scheudling conflict) + test.EnsurePVCCreated(t, clients, expectedTaskRun.GetPipelineRunPVCName(), "foo") + test.EnsurePVCCreated(t, clients, "some-volume", "foo") } func TestReconcile_InvalidPipelineRuns(t *testing.T) { diff --git a/pkg/reconciler/pipelinerun/resources/conditionresolution.go b/pkg/reconciler/pipelinerun/resources/conditionresolution.go index 19e977d76b4..297588b1791 100644 --- a/pkg/reconciler/pipelinerun/resources/conditionresolution.go +++ b/pkg/reconciler/pipelinerun/resources/conditionresolution.go @@ -110,9 +110,7 @@ func (rcc *ResolvedConditionCheck) ConditionToTaskSpec() (*v1alpha1.TaskSpec, er }) } - // convert param strings of type $(params.x) to $(inputs.params.x) convertParamTemplates(&t.Steps[0], rcc.Condition.Spec.Params) - // convert resource strings of type $(resources.name.key) to $(inputs.resources.name.key) err := ApplyResourceSubstitution(&t.Steps[0], rcc.ResolvedResources, rcc.Condition.Spec.Resources, rcc.images) if err != nil { @@ -122,7 +120,7 @@ func (rcc *ResolvedConditionCheck) ConditionToTaskSpec() (*v1alpha1.TaskSpec, er return t, nil } -// Replaces all instances of $(params.x) in the container to $(inputs.params.x) for each param name +// Replaces all instances of $(params.x) in the container to $(inputs.params.x) for each param name. func convertParamTemplates(step *v1alpha1.Step, params []v1alpha1.ParamSpec) { replacements := make(map[string]string) for _, p := range params { @@ -133,8 +131,7 @@ func convertParamTemplates(step *v1alpha1.Step, params []v1alpha1.ParamSpec) { v1alpha1.ApplyStepReplacements(step, replacements, map[string][]string{}) } -// ApplyResources applies the substitution from values in resources which are referenced -// in spec as subitems of the replacementStr. +// ApplyResourceSubstitution applies resource attribute variable substitution. func ApplyResourceSubstitution(step *v1alpha1.Step, resolvedResources map[string]*v1alpha1.PipelineResource, conditionResources []v1alpha1.ResourceDeclaration, images pipeline.Images) error { replacements := make(map[string]string) for _, cr := range conditionResources { @@ -153,7 +150,7 @@ func ApplyResourceSubstitution(step *v1alpha1.Step, resolvedResources map[string return nil } -// NewConditionCheck status creates a ConditionCheckStatus from a ConditionCheck +// NewConditionCheckStatus creates a ConditionCheckStatus from a ConditionCheck func (rcc *ResolvedConditionCheck) NewConditionCheckStatus() *v1alpha1.ConditionCheckStatus { var checkStep corev1.ContainerState trs := rcc.ConditionCheck.Status diff --git a/pkg/reconciler/pipelinerun/resources/input_output_steps.go b/pkg/reconciler/pipelinerun/resources/input_output_steps.go index 4c7a78856db..97b179af503 100644 --- a/pkg/reconciler/pipelinerun/resources/input_output_steps.go +++ b/pkg/reconciler/pipelinerun/resources/input_output_steps.go @@ -22,7 +22,7 @@ import ( "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1" ) -// GetOutputSteps will add the correct `path` to the input resources for pt +// GetOutputSteps will add the correct `path` to the output resources for pt func GetOutputSteps(outputs map[string]*v1alpha1.PipelineResource, taskName, storageBasePath string) []v1alpha1.TaskResourceBinding { var taskOutputResources []v1alpha1.TaskResourceBinding @@ -78,6 +78,8 @@ func GetInputSteps(inputs map[string]*v1alpha1.PipelineResource, pt *v1alpha1.Pi } } + // Determine if the value is meant to come `from` a previous Task - if so, add the path to the pvc + // that contains the data as the `path` the resulting TaskRun should get the data from var stepSourceNames []string if pt.Resources != nil { for _, pipelineTaskInput := range pt.Resources.Inputs { diff --git a/pkg/reconciler/taskrun/resources/input_resource_test.go b/pkg/reconciler/taskrun/resources/input_resource_test.go index 75274c903db..8f89eee2b34 100644 --- a/pkg/reconciler/taskrun/resources/input_resource_test.go +++ b/pkg/reconciler/taskrun/resources/input_resource_test.go @@ -79,6 +79,14 @@ var ( Type: "cluster", }}}, } + volumeInputs = &v1alpha1.Inputs{ + Resources: []v1alpha1.TaskResource{{ + ResourceDeclaration: v1alpha1.ResourceDeclaration{ + Name: "workspace", + Type: "storage", + TargetPath: "sub-dir", + }}}, + } ) func setUp() { @@ -210,6 +218,23 @@ func setUp() { Value: "non-existent", }}, }, + }, { + ObjectMeta: metav1.ObjectMeta{ + Name: "volume-valid", + Namespace: "marshmallow", + }, + Spec: v1alpha1.PipelineResourceSpec{ + Type: "storage", + Params: []v1alpha1.ResourceParam{ + { + Name: "Size", + Value: "10Gi", + }, + { + Name: "Type", + Value: "volume", + }}, + }, }} inputResourceInterfaces = make(map[string]v1alpha1.PipelineResourceInterface) for _, r := range rs { @@ -247,6 +272,15 @@ func TestAddResourceToTask(t *testing.T) { Inputs: gcsInputs, }, } + taskWithVolume := &v1alpha1.Task{ + ObjectMeta: metav1.ObjectMeta{ + Name: "task-with-volume", + Namespace: "marshmallow", + }, + Spec: v1alpha1.TaskSpec{ + Inputs: volumeInputs, + }, + } taskRun := &v1alpha1.TaskRun{ ObjectMeta: metav1.ObjectMeta{ @@ -738,6 +772,159 @@ func TestAddResourceToTask(t *testing.T) { }}, }}}, }, + }, { + desc: "volume resource as input with paths", + task: taskWithVolume, + taskRun: &v1alpha1.TaskRun{ + ObjectMeta: metav1.ObjectMeta{ + Name: "get-from-volume", + Namespace: "marshmallow", + }, + Spec: v1alpha1.TaskRunSpec{ + Inputs: v1alpha1.TaskRunInputs{ + Resources: []v1alpha1.TaskResourceBinding{{ + PipelineResourceBinding: v1alpha1.PipelineResourceBinding{ + ResourceRef: v1alpha1.PipelineResourceRef{ + Name: "volume-valid", + }, + Name: "workspace", + }, + Paths: []string{"workspace"}, + }}, + }, + }, + }, + wantErr: false, + want: &v1alpha1.TaskSpec{ + Inputs: volumeInputs, + Steps: []v1alpha1.Step{{Container: corev1.Container{ + Name: "create-dir-volume-valid-9l9zj", + Image: "override-with-bash-noop:latest", + Command: []string{"/ko-app/bash"}, + Args: []string{"-args", "mkdir -p /workspace/sub-dir"}, + }}, {Container: corev1.Container{ + Name: "download-copy-volume-valid-mz4c7", + Image: "override-with-bash-noop:latest", + Command: []string{"/ko-app/bash"}, + Args: []string{"-args", "cp -r /volumeresource-volume-valid/. /workspace/sub-dir"}, + VolumeMounts: []corev1.VolumeMount{{ + Name: "volume-valid", + MountPath: "/volumeresource-volume-valid", + }}, + }}}, + Volumes: []corev1.Volume{{ + Name: "volume-valid", + VolumeSource: corev1.VolumeSource{ + PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ + ClaimName: "volume-valid", + ReadOnly: false, + }, + }, + }}, + }, + }, { + desc: "volume resource as input without paths", + task: taskWithVolume, + taskRun: &v1alpha1.TaskRun{ + ObjectMeta: metav1.ObjectMeta{ + Name: "get-from-volume", + Namespace: "marshmallow", + }, + Spec: v1alpha1.TaskRunSpec{ + Inputs: v1alpha1.TaskRunInputs{ + Resources: []v1alpha1.TaskResourceBinding{{ + PipelineResourceBinding: v1alpha1.PipelineResourceBinding{ + ResourceRef: v1alpha1.PipelineResourceRef{ + Name: "volume-valid", + }, + Name: "workspace", + }, + }}, + }, + }, + }, + wantErr: false, + want: &v1alpha1.TaskSpec{ + Inputs: volumeInputs, + Steps: []v1alpha1.Step{{Container: corev1.Container{ + Name: "create-dir-volume-valid-9l9zj", + Image: "override-with-bash-noop:latest", + Command: []string{"/ko-app/bash"}, + Args: []string{"-args", "mkdir -p /workspace/sub-dir"}, + }}, {Container: corev1.Container{ + Name: "download-copy-volume-valid-mz4c7", + Image: "override-with-bash-noop:latest", + Command: []string{"/ko-app/bash"}, + Args: []string{"-args", "cp -r /volumeresource-volume-valid/. /workspace/sub-dir"}, + VolumeMounts: []corev1.VolumeMount{{ + Name: "volume-valid", + MountPath: "/volumeresource-volume-valid", + }}, + }}}, + Volumes: []corev1.Volume{{ + Name: "volume-valid", + VolumeSource: corev1.VolumeSource{ + PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ + ClaimName: "volume-valid", + ReadOnly: false, + }, + }, + }}, + }, + }, { + desc: "volume resource as input from previous task", + task: taskWithVolume, + taskRun: &v1alpha1.TaskRun{ + ObjectMeta: metav1.ObjectMeta{ + Name: "get-from-volume", + Namespace: "marshmallow", + OwnerReferences: []metav1.OwnerReference{{ + Kind: "PipelineRun", + Name: "pipelinerun", + }}, + }, + Spec: v1alpha1.TaskRunSpec{ + Inputs: v1alpha1.TaskRunInputs{ + Resources: []v1alpha1.TaskResourceBinding{{ + PipelineResourceBinding: v1alpha1.PipelineResourceBinding{ + ResourceRef: v1alpha1.PipelineResourceRef{ + Name: "volume-valid", + }, + Name: "workspace", + }, + Paths: []string{"prev-task-path"}, + }}, + }, + }, + }, + wantErr: false, + want: &v1alpha1.TaskSpec{ + Inputs: volumeInputs, + Steps: []v1alpha1.Step{{Container: corev1.Container{ + Name: "create-dir-volume-valid-9l9zj", + Image: "override-with-bash-noop:latest", + Command: []string{"/ko-app/bash"}, + Args: []string{"-args", "mkdir -p /workspace/sub-dir"}, + }}, {Container: corev1.Container{ + Name: "download-copy-volume-valid-mz4c7", + Image: "override-with-bash-noop:latest", + Command: []string{"/ko-app/bash"}, + Args: []string{"-args", "cp -r /volumeresource-volume-valid/. /workspace/sub-dir"}, + VolumeMounts: []corev1.VolumeMount{{ + Name: "volume-valid", + MountPath: "/volumeresource-volume-valid", + }}, + }}}, + Volumes: []corev1.Volume{{ + Name: "volume-valid", + VolumeSource: corev1.VolumeSource{ + PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ + ClaimName: "volume-valid", + ReadOnly: false, + }, + }, + }}, + }, }} { t.Run(c.desc, func(t *testing.T) { setUp() @@ -748,8 +935,8 @@ func TestAddResourceToTask(t *testing.T) { t.Errorf("Test: %q; AddInputResource() error = %v, WantErr %v", c.desc, err, c.wantErr) } if got != nil { - if d := cmp.Diff(got, c.want); d != "" { - t.Errorf("Diff:\n%s", d) + if d := cmp.Diff(c.want, got); d != "" { + t.Errorf("Didn't get expected Task spec (-want +got): %s", d) } } }) diff --git a/pkg/reconciler/taskrun/resources/input_resources.go b/pkg/reconciler/taskrun/resources/input_resources.go index 38932c53d98..755ed26a4e5 100644 --- a/pkg/reconciler/taskrun/resources/input_resources.go +++ b/pkg/reconciler/taskrun/resources/input_resources.go @@ -84,8 +84,9 @@ func AddInputResource( var copyStepsFromPrevTasks []v1alpha1.Step dPath := destinationPath(input.Name, input.TargetPath) // if taskrun is fetching resource from previous task then execute copy step instead of fetching new copy - // to the desired destination directory, as long as the resource exports output to be copied - if allowedOutputResources[resource.GetType()] && taskRun.HasPipelineRunOwnerReference() { + // to the desired destination directory, as long as the resource exports output to be copied. + // The VolumeResource is exempted from this because it will already copy to and from an underlying PVC. + if allowedOutputResources[resource.GetType()] && resource.GetType() != v1alpha1.PipelineResourceTypeVolume && taskRun.HasPipelineRunOwnerReference() { for _, path := range boundResource.Paths { cpSteps := as.GetCopyFromStorageToSteps(boundResource.Name, path, dPath) if as.GetType() == v1alpha1.ArtifactStoragePVCType { @@ -93,7 +94,7 @@ func AddInputResource( for _, s := range cpSteps { s.VolumeMounts = []corev1.VolumeMount{v1alpha1.GetPvcMount(pvcName)} copyStepsFromPrevTasks = append(copyStepsFromPrevTasks, - v1alpha1.CreateDirStep(images.BashNoopImage, boundResource.Name, dPath), + v1alpha1.CreateDirStep(images.BashNoopImage, boundResource.Name, dPath, nil), s) } } else { @@ -112,7 +113,9 @@ func AddInputResource( if err != nil { return nil, err } - v1alpha1.ApplyTaskModifier(taskSpec, modifier) + if err := v1alpha1.ApplyTaskModifier(taskSpec, modifier); err != nil { + return nil, xerrors.Errorf("Unabled to apply Resource %s: %w", boundResource.Name, err) + } } } diff --git a/pkg/reconciler/taskrun/resources/output_resource.go b/pkg/reconciler/taskrun/resources/output_resource.go index 7dcaa9dcecc..d4ff5fe4eef 100644 --- a/pkg/reconciler/taskrun/resources/output_resource.go +++ b/pkg/reconciler/taskrun/resources/output_resource.go @@ -91,10 +91,12 @@ func AddOutputResources( } // Add containers to mkdir each output directory. This should run before the build steps themselves. - mkdirSteps := []v1alpha1.Step{v1alpha1.CreateDirStep(images.BashNoopImage, boundResource.Name, sourcePath)} + mkdirSteps := []v1alpha1.Step{v1alpha1.CreateDirStep(images.BashNoopImage, boundResource.Name, sourcePath, nil)} taskSpec.Steps = append(mkdirSteps, taskSpec.Steps...) - if allowedOutputResources[resource.GetType()] && taskRun.HasPipelineRunOwnerReference() { + // If the output has a Path specified, this implies that data should be copied to that path. This is used for `from` linking only. + // The VolumeResource is exempted from this because it will already copy to and from an underlying PVC. + if allowedOutputResources[resource.GetType()] && resource.GetType() != v1alpha1.PipelineResourceTypeVolume && taskRun.HasPipelineRunOwnerReference() { var newSteps []v1alpha1.Step for _, dPath := range boundResource.Paths { newSteps = append(newSteps, as.GetCopyToStorageFromSteps(resource.GetName(), sourcePath, dPath)...) @@ -108,9 +110,13 @@ func AddOutputResources( if err != nil { return nil, err } - v1alpha1.ApplyTaskModifier(taskSpec, modifier) + if err := v1alpha1.ApplyTaskModifier(taskSpec, modifier); err != nil { + return nil, xerrors.Errorf("Unabled to apply Resource %s: %w", boundResource.Name, err) + } - if as.GetType() == v1alpha1.ArtifactStoragePVCType { + // Attach the PVC that will be used for `from` copying. + // The VolumeResource is exempted from this because it will already copy to and from an underlying PVC. + if resource.GetType() != v1alpha1.PipelineResourceTypeVolume && as.GetType() == v1alpha1.ArtifactStoragePVCType { if pvcName == "" { return taskSpec, nil } diff --git a/pkg/reconciler/taskrun/resources/output_resource_test.go b/pkg/reconciler/taskrun/resources/output_resource_test.go index b81e7ce902b..9a5d36378ae 100644 --- a/pkg/reconciler/taskrun/resources/output_resource_test.go +++ b/pkg/reconciler/taskrun/resources/output_resource_test.go @@ -89,6 +89,24 @@ func outputResourceSetup() { Spec: v1alpha1.PipelineResourceSpec{ Type: "image", }, + }, { + ObjectMeta: metav1.ObjectMeta{ + Name: "source-volume", + Namespace: "marshmallow", + }, + Spec: v1alpha1.PipelineResourceSpec{ + Type: "storage", + Params: []v1alpha1.ResourceParam{ + { + Name: "Size", + Value: "10Gi", + }, + { + Name: "Type", + Value: "volume", + }, + }, + }, }} outputResources = make(map[string]v1alpha1.PipelineResourceInterface) @@ -188,6 +206,14 @@ func TestValidOutputResources(t *testing.T) { MountPath: "/pvc", }}, }}}, + wantVolumes: []corev1.Volume{{ + Name: "pipelinerun-pvc", + VolumeSource: corev1.VolumeSource{ + PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ + ClaimName: "pipelinerun-pvc", + }, + }, + }}, }, { name: "git resource in output only", desc: "git resource declared as output with pipelinerun owner reference", @@ -253,6 +279,14 @@ func TestValidOutputResources(t *testing.T) { MountPath: "/pvc", }}, }}}, + wantVolumes: []corev1.Volume{{ + Name: "pipelinerun-pvc", + VolumeSource: corev1.VolumeSource{ + PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ + ClaimName: "pipelinerun-pvc", + }, + }, + }}, }, { name: "image resource in output with pipelinerun with owner", desc: "image resource declared as output with pipelinerun owner reference should not generate any steps", @@ -300,7 +334,14 @@ func TestValidOutputResources(t *testing.T) { Command: []string{"/ko-app/bash"}, Args: []string{"-args", "mkdir -p /workspace/output/source-workspace"}, }}}, - wantVolumes: nil, + wantVolumes: []corev1.Volume{{ + Name: "pipelinerun-pvc", + VolumeSource: corev1.VolumeSource{ + PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ + ClaimName: "pipelinerun-pvc", + }, + }, + }}, }, { name: "git resource in output", desc: "git resource declared in output without pipelinerun owner reference", @@ -352,7 +393,7 @@ func TestValidOutputResources(t *testing.T) { Namespace: "marshmallow", OwnerReferences: []metav1.OwnerReference{{ Kind: "PipelineRun", - Name: "pipelinerun-parent", + Name: "pipelinerun", }}, }, Spec: v1alpha1.TaskRunSpec{ @@ -414,14 +455,14 @@ func TestValidOutputResources(t *testing.T) { Image: "override-with-bash-noop:latest", Command: []string{"/ko-app/bash"}, Args: []string{"-args", "mkdir -p pipeline-task-path"}, - VolumeMounts: []corev1.VolumeMount{{Name: "pipelinerun-parent-pvc", MountPath: "/pvc"}}, + VolumeMounts: []corev1.VolumeMount{{Name: "pipelinerun-pvc", MountPath: "/pvc"}}, }}, {Container: corev1.Container{ Name: "source-copy-source-gcs-mssqb", Image: "override-with-bash-noop:latest", Command: []string{"/ko-app/bash"}, Args: []string{"-args", "cp -r /workspace/output/source-workspace/. pipeline-task-path"}, - VolumeMounts: []corev1.VolumeMount{{Name: "pipelinerun-parent-pvc", MountPath: "/pvc"}}, + VolumeMounts: []corev1.VolumeMount{{Name: "pipelinerun-pvc", MountPath: "/pvc"}}, }}, {Container: corev1.Container{ Name: "upload-source-gcs-78c5n", @@ -443,6 +484,13 @@ func TestValidOutputResources(t *testing.T) { VolumeSource: corev1.VolumeSource{ Secret: &corev1.SecretVolumeSource{SecretName: "sname"}, }, + }, { + Name: "pipelinerun-pvc", + VolumeSource: corev1.VolumeSource{ + PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ + ClaimName: "pipelinerun-pvc", + }, + }, }}, }, { name: "storage resource as output", @@ -524,6 +572,13 @@ func TestValidOutputResources(t *testing.T) { VolumeSource: corev1.VolumeSource{ Secret: &corev1.SecretVolumeSource{SecretName: "sname"}, }, + }, { + Name: "pipelinerun-pvc", + VolumeSource: corev1.VolumeSource{ + PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ + ClaimName: "pipelinerun-pvc", + }, + }, }}, }, { name: "storage resource as output with no owner", @@ -696,6 +751,14 @@ func TestValidOutputResources(t *testing.T) { Command: []string{"/ko-app/bash"}, Args: []string{"-args", "mkdir -p /workspace/output/source-workspace"}, }}}, + wantVolumes: []corev1.Volume{{ + Name: "pipelinerun-pvc", + VolumeSource: corev1.VolumeSource{ + PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ + ClaimName: "pipelinerun-pvc", + }, + }, + }}, }, { name: "Resource with TargetPath as output", desc: "Resource with TargetPath defined only in output", @@ -743,6 +806,14 @@ func TestValidOutputResources(t *testing.T) { Command: []string{"/ko-app/bash"}, Args: []string{"-args", "mkdir -p /workspace"}, }}}, + wantVolumes: []corev1.Volume{{ + Name: "pipelinerun-pvc", + VolumeSource: corev1.VolumeSource{ + PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ + ClaimName: "pipelinerun-pvc", + }, + }, + }}, }, { desc: "image output resource with no steps", taskRun: &v1alpha1.TaskRun{ @@ -784,6 +855,159 @@ func TestValidOutputResources(t *testing.T) { Command: []string{"/ko-app/bash"}, Args: []string{"-args", "mkdir -p /workspace/output/source-workspace"}, }}}, + }, { + name: "volume resource as output with no owner", + desc: "volume resource defined only in output without pipelinerun reference", + taskRun: &v1alpha1.TaskRun{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-taskrun-run-only-output-step", + Namespace: "marshmallow", + }, + Spec: v1alpha1.TaskRunSpec{ + Outputs: v1alpha1.TaskRunOutputs{ + Resources: []v1alpha1.TaskResourceBinding{{ + PipelineResourceBinding: v1alpha1.PipelineResourceBinding{ + Name: "source-workspace", + ResourceRef: v1alpha1.PipelineResourceRef{ + Name: "source-volume", + }, + }, + }}, + }, + }, + }, + task: &v1alpha1.Task{ + ObjectMeta: metav1.ObjectMeta{ + Name: "task1", + Namespace: "marshmallow", + }, + Spec: v1alpha1.TaskSpec{ + Outputs: &v1alpha1.Outputs{ + Resources: []v1alpha1.TaskResource{{ + ResourceDeclaration: v1alpha1.ResourceDeclaration{ + Name: "source-workspace", + Type: "storage", + TargetPath: "/workspace", + }}}, + }, + }, + }, + wantSteps: []v1alpha1.Step{{Container: corev1.Container{ + Name: "create-dir-source-workspace-9l9zj", + Image: "override-with-bash-noop:latest", + Command: []string{"/ko-app/bash"}, + Args: []string{"-args", "mkdir -p /workspace"}, + }}, {Container: corev1.Container{ + Name: "create-dir-source-volume-mz4c7", + Image: "override-with-bash-noop:latest", + VolumeMounts: []corev1.VolumeMount{{ + Name: "source-volume", MountPath: "/volumeresource-source-volume", + }}, + Command: []string{"/ko-app/bash"}, + Args: []string{"-args", "mkdir -p /volumeresource-source-volume"}, + }}, {Container: corev1.Container{ + Name: "upload-copy-source-volume-mssqb", + Image: "override-with-bash-noop:latest", + VolumeMounts: []corev1.VolumeMount{{ + Name: "source-volume", MountPath: "/volumeresource-source-volume", + }}, + Command: []string{"/ko-app/bash"}, + Args: []string{"-args", "cp -r /workspace/. /volumeresource-source-volume"}, + }}}, + wantVolumes: []corev1.Volume{{ + Name: "source-volume", + VolumeSource: corev1.VolumeSource{ + PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ClaimName: "source-volume", ReadOnly: false}, + }, + }}, + }, { + name: "volume resource as both input and output", + desc: "volume resource defined in both input and output", + taskRun: &v1alpha1.TaskRun{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-taskrun-run-output-steps", + Namespace: "marshmallow", + OwnerReferences: []metav1.OwnerReference{{ + Kind: "PipelineRun", + Name: "pipelinerun-parent", + }}, + }, + Spec: v1alpha1.TaskRunSpec{ + Inputs: v1alpha1.TaskRunInputs{ + Resources: []v1alpha1.TaskResourceBinding{{ + PipelineResourceBinding: v1alpha1.PipelineResourceBinding{ + Name: "source-workspace", + ResourceRef: v1alpha1.PipelineResourceRef{ + Name: "source-volume", + }, + }, + }}, + }, + Outputs: v1alpha1.TaskRunOutputs{ + Resources: []v1alpha1.TaskResourceBinding{{ + PipelineResourceBinding: v1alpha1.PipelineResourceBinding{ + Name: "source-workspace", + ResourceRef: v1alpha1.PipelineResourceRef{ + Name: "source-volume", + }, + }, + Paths: []string{"pipeline-task-path"}, + }}, + }, + }, + }, + task: &v1alpha1.Task{ + ObjectMeta: metav1.ObjectMeta{ + Name: "task1", + Namespace: "marshmallow", + }, + Spec: v1alpha1.TaskSpec{ + Inputs: &v1alpha1.Inputs{ + Resources: []v1alpha1.TaskResource{{ + ResourceDeclaration: v1alpha1.ResourceDeclaration{ + Name: "source-workspace", + Type: "volume", + TargetPath: "faraway-disk", + }}}, + }, + Outputs: &v1alpha1.Outputs{ + Resources: []v1alpha1.TaskResource{{ + ResourceDeclaration: v1alpha1.ResourceDeclaration{ + Name: "source-workspace", + Type: "volume", + }}}, + }, + }, + }, + wantSteps: []v1alpha1.Step{{Container: corev1.Container{ + Name: "create-dir-source-workspace-9l9zj", + Image: "override-with-bash-noop:latest", + Command: []string{"/ko-app/bash"}, + Args: []string{"-args", "mkdir -p /workspace/output/source-workspace"}, + }}, {Container: corev1.Container{ + Name: "create-dir-source-volume-mz4c7", + Image: "override-with-bash-noop:latest", + VolumeMounts: []corev1.VolumeMount{{ + Name: "source-volume", MountPath: "/volumeresource-source-volume", + }}, + Command: []string{"/ko-app/bash"}, + Args: []string{"-args", "mkdir -p /volumeresource-source-volume"}, + }}, {Container: corev1.Container{ + Name: "upload-copy-source-volume-mssqb", + Image: "override-with-bash-noop:latest", + VolumeMounts: []corev1.VolumeMount{{ + Name: "source-volume", + MountPath: "/volumeresource-source-volume", + }}, + Command: []string{"/ko-app/bash"}, + Args: []string{"-args", "cp -r /workspace/output/source-workspace/. /volumeresource-source-volume"}, + }}}, + wantVolumes: []corev1.Volume{{ + Name: "source-volume", + VolumeSource: corev1.VolumeSource{ + PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ClaimName: "source-volume", ReadOnly: false}, + }, + }}, }} { t.Run(c.name, func(t *testing.T) { names.TestingSeed() @@ -795,25 +1019,12 @@ func TestValidOutputResources(t *testing.T) { } if got != nil { - if d := cmp.Diff(got.Steps, c.wantSteps); d != "" { - t.Fatalf("post build steps mismatch: %s", d) + if d := cmp.Diff(c.wantSteps, got.Steps); d != "" { + t.Fatalf("post build steps mismatch (-want, +got): %v", d) } - if c.taskRun.GetPipelineRunPVCName() != "" { - c.wantVolumes = append( - c.wantVolumes, - corev1.Volume{ - Name: c.taskRun.GetPipelineRunPVCName(), - VolumeSource: corev1.VolumeSource{ - PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ - ClaimName: c.taskRun.GetPipelineRunPVCName(), - }, - }, - }, - ) - } - if d := cmp.Diff(got.Volumes, c.wantVolumes); d != "" { - t.Fatalf("post build steps volumes mismatch: %s", d) + if d := cmp.Diff(c.wantVolumes, got.Volumes); d != "" { + t.Fatalf("post build steps volumes mismatch (-want, +got) : %s", d) } } }) diff --git a/pkg/reconciler/taskrun/resources/pod.go b/pkg/reconciler/taskrun/resources/pod.go index 91437b36751..fbc8c34f9d0 100644 --- a/pkg/reconciler/taskrun/resources/pod.go +++ b/pkg/reconciler/taskrun/resources/pod.go @@ -213,7 +213,7 @@ func initOutputResourcesDefaultDir(bashNoopImage string, taskRun *v1alpha1.TaskR for _, o := range taskSpec.Outputs.Resources { if o.Name == r.Name { if strings.HasPrefix(o.OutputImageDir, v1alpha1.TaskOutputImageDefaultDir) { - s := v1alpha1.CreateDirStep(bashNoopImage, "default-image-output", fmt.Sprintf("%s/%s", v1alpha1.TaskOutputImageDefaultDir, r.Name)) + s := v1alpha1.CreateDirStep(bashNoopImage, "default-image-output", fmt.Sprintf("%s/%s", v1alpha1.TaskOutputImageDefaultDir, r.Name), nil) s.VolumeMounts = append(s.VolumeMounts, implicitVolumeMounts...) makeDirSteps = append(makeDirSteps, s) } diff --git a/pkg/reconciler/taskrun/taskrun.go b/pkg/reconciler/taskrun/taskrun.go index 2d83d041364..0e75b986640 100644 --- a/pkg/reconciler/taskrun/taskrun.go +++ b/pkg/reconciler/taskrun/taskrun.go @@ -417,17 +417,35 @@ func (c *Reconciler) updateReady(pod *corev1.Pod) error { // TODO(dibyom): Refactor resource setup/substitution logic to its own function in the resources package func (c *Reconciler) createPod(tr *v1alpha1.TaskRun, rtr *resources.ResolvedTaskResources) (*corev1.Pod, error) { ts := rtr.TaskSpec.DeepCopy() - inputResources, err := resourceImplBinding(rtr.Inputs, c.Images) + inputResources, err := getPipelineResourceInstances(rtr.Inputs, tr.OwnerReferences, c.KubeClientSet, c.Images) if err != nil { - c.Logger.Errorf("Failed to initialize input resources: %v", err) + c.Logger.Errorf("Failed to instantiate input resources: %v", err) return nil, err } - outputResources, err := resourceImplBinding(rtr.Outputs, c.Images) + outputResources, err := getPipelineResourceInstances(rtr.Outputs, tr.OwnerReferences, c.KubeClientSet, c.Images) if err != nil { - c.Logger.Errorf("Failed to initialize output resources: %v", err) + c.Logger.Errorf("Failed to Instantiate output resources: %v", err) return nil, err } + // Some PipelineResources will require state to be setup before they can be used + for name, r := range inputResources { + s := r.GetSetup() + err := s.Setup(r, tr.GetOwnerReference(), c.KubeClientSet) + if err != nil { + c.Logger.Errorf("Failed to setup input PipelineResource %s: %v", name, err) + return nil, err + } + } + for name, r := range outputResources { + s := r.GetSetup() + err := s.Setup(r, tr.GetOwnerReference(), c.KubeClientSet) + if err != nil { + c.Logger.Errorf("Failed to setup output PipelineResource %s: %v", name, err) + return nil, err + } + } + // Get actual resource err = resources.AddOutputImageDigestExporter(c.Images.ImageDigestExporterImage, tr, ts, c.resourceLister.PipelineResources(tr.Namespace).Get) @@ -548,8 +566,8 @@ func isExceededResourceQuotaError(err error) bool { return err != nil && errors.IsForbidden(err) && strings.Contains(err.Error(), "exceeded quota") } -// resourceImplBinding maps pipeline resource names to the actual resource type implementations -func resourceImplBinding(resources map[string]*v1alpha1.PipelineResource, images pipeline.Images) (map[string]v1alpha1.PipelineResourceInterface, error) { +// getPipelineResourceInstances maps pipeline resource names to the actual resource type implementations +func getPipelineResourceInstances(resources map[string]*v1alpha1.PipelineResource, o []metav1.OwnerReference, c kubernetes.Interface, images pipeline.Images) (map[string]v1alpha1.PipelineResourceInterface, error) { p := make(map[string]v1alpha1.PipelineResourceInterface) for rName, r := range resources { i, err := v1alpha1.ResourceFromType(r, images) diff --git a/pkg/reconciler/taskrun/taskrun_test.go b/pkg/reconciler/taskrun/taskrun_test.go index bafdb7bf414..b7625dbf8ac 100644 --- a/pkg/reconciler/taskrun/taskrun_test.go +++ b/pkg/reconciler/taskrun/taskrun_test.go @@ -109,7 +109,7 @@ var ( tb.InputsResource(gitResource.Name, v1alpha1.PipelineResourceTypeGit), tb.InputsResource(anotherGitResource.Name, v1alpha1.PipelineResourceTypeGit), ), - tb.TaskOutputs(tb.OutputsResource(gitResource.Name, v1alpha1.PipelineResourceTypeGit)), + tb.TaskOutputs(tb.OutputsResource(volumeResource.Name, v1alpha1.PipelineResourceTypeStorage)), )) saTask = tb.Task("test-with-sa", "foo", tb.TaskSpec(tb.Step("sa-step", "foo", tb.StepCommand("/mycmd")))) @@ -171,6 +171,9 @@ var ( anotherCloudEventResource = tb.PipelineResource("another-cloud-event-resource", "foo", tb.PipelineResourceSpec( v1alpha1.PipelineResourceTypeCloudEvent, tb.PipelineResourceSpecParam("TargetURI", cloudEventTarget2), )) + volumeResource = tb.PipelineResource("volume-resource", "foo", tb.PipelineResourceSpec( + v1alpha1.PipelineResourceTypeStorage, tb.PipelineResourceSpecParam("type", "volume"), + )) toolsVolume = corev1.Volume{ Name: "tools", @@ -205,6 +208,16 @@ var ( }, }, } + // volumeVolume is the volume for the VolumeResource + volumeVolume = corev1.Volume{ + Name: "volume-resource", + VolumeSource: corev1.VolumeSource{ + PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ + ClaimName: "volume-resource", + ReadOnly: false, + }, + }, + } getCredentialsInitContainer = func(suffix string, ops ...tb.ContainerOp) tb.PodSpecOp { actualOps := []tb.ContainerOp{ @@ -480,8 +493,8 @@ func TestReconcile(t *testing.T) { ), ), tb.TaskRunOutputs( - tb.TaskRunOutputsResource(gitResource.Name, - tb.TaskResourceBindingRef(gitResource.Name), + tb.TaskRunOutputsResource(volumeResource.Name, + tb.TaskResourceBindingRef(volumeResource.Name), tb.TaskResourceBindingPaths("output-folder"), ), ), @@ -592,7 +605,7 @@ func TestReconcile(t *testing.T) { TaskRuns: taskruns, Tasks: []*v1alpha1.Task{simpleTask, saTask, templatedTask, outputTask, taskEnvTask}, ClusterTasks: []*v1alpha1.ClusterTask{clustertask}, - PipelineResources: []*v1alpha1.PipelineResource{gitResource, anotherGitResource, imageResource}, + PipelineResources: []*v1alpha1.PipelineResource{gitResource, anotherGitResource, imageResource, volumeResource}, } for _, tc := range []struct { name string @@ -806,11 +819,11 @@ func TestReconcile(t *testing.T) { ReadOnly: false, }, }, - }, toolsVolume, downward, workspaceVolume, homeVolume), + }, volumeVolume, toolsVolume, downward, workspaceVolume, homeVolume), tb.PodRestartPolicy(corev1.RestartPolicyNever), getCredentialsInitContainer("l22wn"), getPlaceToolsInitContainer(), - getMkdirResourceContainer("git-resource", "/workspace/output/git-resource", "6nl7g"), + getMkdirResourceContainer("volume-resource", "/workspace/output/volume-resource", "6nl7g"), tb.PodContainer("step-create-dir-git-resource-78c5n", "override-with-bash-noop:latest", tb.Command(entrypointLocation), tb.Args("-wait_file", "/builder/tools/0", "-post_file", "/builder/tools/1", "-entrypoint", "/ko-app/bash", "--", @@ -887,13 +900,13 @@ func TestReconcile(t *testing.T) { tb.EphemeralStorage("0"), )), ), - tb.PodContainer("step-source-mkdir-git-resource-j2tds", "override-with-bash-noop:latest", + tb.PodContainer("step-create-dir-volume-resource-j2tds", "override-with-bash-noop:latest", tb.Command(entrypointLocation), tb.Args("-wait_file", "/builder/tools/5", "-post_file", "/builder/tools/6", "-entrypoint", "/ko-app/bash", "--", - "-args", "mkdir -p output-folder"), + "-args", "mkdir -p /volumeresource-volume-resource"), tb.WorkingDir(workspaceDir), tb.EnvVar("HOME", "/builder/home"), - tb.VolumeMount("test-pvc", "/pvc"), + tb.VolumeMount("volume-resource", "/volumeresource-volume-resource"), tb.VolumeMount("tools", "/builder/tools"), tb.VolumeMount("workspace", workspaceDir), tb.VolumeMount("home", "/builder/home"), @@ -903,13 +916,13 @@ func TestReconcile(t *testing.T) { tb.EphemeralStorage("0"), )), ), - tb.PodContainer("step-source-copy-git-resource-vr6ds", "override-with-bash-noop:latest", + tb.PodContainer("step-upload-copy-volume-resource-vr6ds", "override-with-bash-noop:latest", tb.Command(entrypointLocation), tb.Args("-wait_file", "/builder/tools/6", "-post_file", "/builder/tools/7", "-entrypoint", "/ko-app/bash", "--", - "-args", "cp -r /workspace/output/git-resource/. output-folder"), + "-args", "cp -r /workspace/output/volume-resource/. /volumeresource-volume-resource"), tb.WorkingDir(workspaceDir), tb.EnvVar("HOME", "/builder/home"), - tb.VolumeMount("test-pvc", "/pvc"), + tb.VolumeMount("volume-resource", "/volumeresource-volume-resource"), tb.VolumeMount("tools", "/builder/tools"), tb.VolumeMount("workspace", workspaceDir), tb.VolumeMount("home", "/builder/home"), @@ -1303,15 +1316,17 @@ func TestReconcile(t *testing.T) { t.Fatalf("Failed to fetch build pod: %v", err) } - if d := cmp.Diff(pod.ObjectMeta, tc.wantPod.ObjectMeta, ignoreRandomPodNameSuffix); d != "" { - t.Errorf("Pod metadata doesn't match, diff: %s", d) + if d := cmp.Diff(tc.wantPod.ObjectMeta, pod.ObjectMeta, ignoreRandomPodNameSuffix); d != "" { + t.Errorf("Pod metadata doesn't match (-want, +got): %s", d) } - if d := cmp.Diff(pod.Spec, tc.wantPod.Spec, resourceQuantityCmp); d != "" { - t.Errorf("Pod spec doesn't match, diff: %s", d) + if d := cmp.Diff(tc.wantPod.Spec, pod.Spec, resourceQuantityCmp); d != "" { + t.Errorf("Pod spec doesn't match (-want, +got): %s", d) } - if len(clients.Kube.Actions()) == 0 { - t.Fatalf("Expected actions to be logged in the kubeclient, got none") + + // If the TaskRun used a volume resource, make sure the backing PVC was created + if name == taskRunInputOutput.Name { + test.EnsurePVCCreated(t, clients, volumeResource.Name, volumeResource.Namespace) } }) } @@ -1395,10 +1410,10 @@ func TestReconcile_SortTaskRunStatusSteps(t *testing.T) { if err := testAssets.Controller.Reconciler.Reconcile(context.Background(), getRunName(taskRun)); err != nil { t.Errorf("expected no error reconciling valid TaskRun but got %v", err) } - verify_TaskRunStatusStep(t, taskRun) + verifyTaskRunStatusStep(t, taskRun) } -func verify_TaskRunStatusStep(t *testing.T, taskRun *v1alpha1.TaskRun) { +func verifyTaskRunStatusStep(t *testing.T, taskRun *v1alpha1.TaskRun) { actualStepOrder := []string{} for _, state := range taskRun.Status.Steps { actualStepOrder = append(actualStepOrder, state.Name) diff --git a/test/pvc.go b/test/pvc.go new file mode 100644 index 00000000000..7ce6f84b7db --- /dev/null +++ b/test/pvc.go @@ -0,0 +1,42 @@ +/* +Copyright 2019 The Tekton Authors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package test + +import ( + "testing" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +// EnsurePVCCreated will check the kube client in clients and make sure that a PVC called name +// was created in namespace. +func EnsurePVCCreated(t *testing.T, clients Clients, name, namespace string) { + t.Helper() + _, err := clients.Kube.CoreV1().PersistentVolumeClaims(namespace).Get(name, metav1.GetOptions{}) + if err != nil { + t.Errorf("Expected PVC %s to be created for VolumeResource but did not exist", name) + } + pvcCreated := false + for _, a := range clients.Kube.Actions() { + if a.GetVerb() == "create" && a.GetResource().Resource == "persistentvolumeclaims" { + pvcCreated = true + } + } + if !pvcCreated { + t.Errorf("Expected to see volume resource PVC created but didn't") + } +}