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") + } +}