From 666fcd79e16599d4ceafe3e4f01df0b5bc1e5705 Mon Sep 17 00:00:00 2001 From: Andrew Bayer Date: Tue, 16 Jul 2019 12:29:39 -0400 Subject: [PATCH] Add "volume" PipelineResource This will allow copying content either into or out of a `TaskRun`, either to an existing volume or a newly created volume. The immediate use case is for copying a pipeline's workspace to be made available as the input for another pipeline's workspace without needing to deal with uploading everything to a bucket. The volume, whether already existing or created, will not be deleted at the end of the `PipelineRun`, unlike the artifact storage PVC. The Volume resource is a sub-type of the general Storage resource. Since this type will require the creation of a PVC to function (to be configurable later), this commit adds a Setup interface that PipelineResources can implement if they need to do setup that involves instantiating objects in Kube. This could be a place to later add features like caching, and also is the sort of design we'd expect once PipelineResources are extensible (PipelineResources will be free to do whatever setup they need). fixes #1062 Co-authored-by: dlorenc Co-authored-by: Christie Wilson --- .../pipeline/v1alpha1/build_gcs_resource.go | 3 + .../v1alpha1/build_gcs_resource_test.go | 2 +- .../pipeline/v1alpha1/cloud_event_resource.go | 3 + .../pipeline/v1alpha1/cluster_resource.go | 3 + pkg/apis/pipeline/v1alpha1/gcs_resource.go | 3 + pkg/apis/pipeline/v1alpha1/git_resource.go | 3 + pkg/apis/pipeline/v1alpha1/image_resource.go | 3 + .../v1alpha1/pull_request_resource.go | 3 + pkg/apis/pipeline/v1alpha1/resource_types.go | 44 +++- .../pipeline/v1alpha1/storage_resource.go | 17 +- pkg/apis/pipeline/v1alpha1/taskrun_types.go | 7 + pkg/apis/pipeline/v1alpha1/volume_resource.go | 215 +++++++++++++++ .../pipeline/v1alpha1/volume_resource_test.go | 249 ++++++++++++++++++ .../v1alpha1/zz_generated.deepcopy.go | 16 ++ .../resources/conditionresolution.go | 9 +- .../taskrun/resources/input_resource_test.go | 175 ++++++++++++ .../taskrun/resources/input_resources.go | 1 + .../taskrun/resources/output_resource_test.go | 181 ++++++++++++- pkg/reconciler/taskrun/taskrun.go | 29 +- pkg/reconciler/taskrun/taskrun_test.go | 97 +++++-- 20 files changed, 1028 insertions(+), 35 deletions(-) create mode 100644 pkg/apis/pipeline/v1alpha1/volume_resource.go create mode 100644 pkg/apis/pipeline/v1alpha1/volume_resource_test.go diff --git a/pkg/apis/pipeline/v1alpha1/build_gcs_resource.go b/pkg/apis/pipeline/v1alpha1/build_gcs_resource.go index 07d4b86385e..1986631589a 100644 --- a/pkg/apis/pipeline/v1alpha1/build_gcs_resource.go +++ b/pkg/apis/pipeline/v1alpha1/build_gcs_resource.go @@ -72,6 +72,9 @@ type BuildGCSResource struct { ArtifactType GCSArtifactType } +// 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(r *PipelineResource) (*BuildGCSResource, error) { if r.Spec.Type != PipelineResourceTypeStorage { diff --git a/pkg/apis/pipeline/v1alpha1/build_gcs_resource_test.go b/pkg/apis/pipeline/v1alpha1/build_gcs_resource_test.go index 6208e79cae7..9f2716f327e 100644 --- a/pkg/apis/pipeline/v1alpha1/build_gcs_resource_test.go +++ b/pkg/apis/pipeline/v1alpha1/build_gcs_resource_test.go @@ -78,7 +78,7 @@ func Test_Invalid_BuildGCSResource(t *testing.T) { )), }} { t.Run(tc.name, func(t *testing.T) { - _, err := v1alpha1.NewStorageResource(tc.pipelineResource) + _, err := v1alpha1.NewBuildGCSResource(tc.pipelineResource) if err == nil { t.Error("Expected error creating BuildGCS resource") } diff --git a/pkg/apis/pipeline/v1alpha1/cloud_event_resource.go b/pkg/apis/pipeline/v1alpha1/cloud_event_resource.go index 71d53d593b2..8a98059a7ef 100644 --- a/pkg/apis/pipeline/v1alpha1/cloud_event_resource.go +++ b/pkg/apis/pipeline/v1alpha1/cloud_event_resource.go @@ -61,6 +61,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 4915b1ae20d..fa697161b88 100644 --- a/pkg/apis/pipeline/v1alpha1/cluster_resource.go +++ b/pkg/apis/pipeline/v1alpha1/cluster_resource.go @@ -57,6 +57,9 @@ type ClusterResource struct { Secrets []SecretParam `json:"secrets"` } +// 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(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 157b8ff0970..4fc8693dbd5 100644 --- a/pkg/apis/pipeline/v1alpha1/gcs_resource.go +++ b/pkg/apis/pipeline/v1alpha1/gcs_resource.go @@ -43,6 +43,9 @@ type GCSResource struct { Secrets []SecretParam `json:"secrets"` } +// 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(r *PipelineResource) (*GCSResource, error) { if r.Spec.Type != PipelineResourceTypeStorage { diff --git a/pkg/apis/pipeline/v1alpha1/git_resource.go b/pkg/apis/pipeline/v1alpha1/git_resource.go index 5885aa09ce7..378cb6fc0e8 100644 --- a/pkg/apis/pipeline/v1alpha1/git_resource.go +++ b/pkg/apis/pipeline/v1alpha1/git_resource.go @@ -46,6 +46,9 @@ type GitResource struct { Revision string `json:"revision"` } +// 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(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 f5066ab6b68..031b26e6cb8 100644 --- a/pkg/apis/pipeline/v1alpha1/image_resource.go +++ b/pkg/apis/pipeline/v1alpha1/image_resource.go @@ -55,6 +55,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/pull_request_resource.go b/pkg/apis/pipeline/v1alpha1/pull_request_resource.go index b202fd19d97..8fc01b969f7 100644 --- a/pkg/apis/pipeline/v1alpha1/pull_request_resource.go +++ b/pkg/apis/pipeline/v1alpha1/pull_request_resource.go @@ -49,6 +49,9 @@ type PullRequestResource struct { Secrets []SecretParam `json:"secrets"` } +// 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(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 238ce4482fc..7c1d2e437b1 100644 --- a/pkg/apis/pipeline/v1alpha1/resource_types.go +++ b/pkg/apis/pipeline/v1alpha1/resource_types.go @@ -20,6 +20,7 @@ import ( "golang.org/x/xerrors" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/kubernetes" "knative.dev/pkg/apis" ) @@ -53,13 +54,52 @@ 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 + // GetDownloadSteps returns the steps that should be added to the TaskRun execution + // before the user requested steps in order to initialize the PipelineResource on disk. GetDownloadSteps(sourcePath string) ([]Step, error) + // GetUploadSteps returns the steps that should be added to the TaskRun execution + // after the user requested steps in order to update the PipelineResource from the state + // on disk. GetUploadSteps(sourcePath string) ([]Step, error) + // GetUploadVolumeSpec returns the volume spec that should be added to the TaskRun + // execution if the PipelineResource is used as an output and the PipelineResource requires + // a volume. GetUploadVolumeSpec(spec *TaskSpec) ([]corev1.Volume, error) + // GetDownloadVolumeSpec returns the volume spec that should be added to the TaskRun + // execution if the PipelineResource is used as an input and the PipelineResource requires + // a volume. GetDownloadVolumeSpec(spec *TaskSpec) ([]corev1.Volume, 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 +} + +// 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 @@ -152,7 +192,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) (PipelineResourceInterface, error) { switch r.Spec.Type { case PipelineResourceTypeGit: diff --git a/pkg/apis/pipeline/v1alpha1/storage_resource.go b/pkg/apis/pipeline/v1alpha1/storage_resource.go index 58e46b91b33..972cae242e8 100644 --- a/pkg/apis/pipeline/v1alpha1/storage_resource.go +++ b/pkg/apis/pipeline/v1alpha1/storage_resource.go @@ -24,20 +24,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(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) @@ -50,6 +59,8 @@ func NewStorageResource(r *PipelineResource) (PipelineStorageResourceInterface, return NewGCSResource(r) case strings.EqualFold(param.Value, string(PipelineResourceTypeBuildGCS)): return NewBuildGCSResource(r) + case strings.EqualFold(param.Value, string(PipelineResourceTypeVolume)): + return NewVolumeResource(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 ae60cfd2049..82ca939114c 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..cee0ac31a12 --- /dev/null +++ b/pkg/apis/pipeline/v1alpha1/volume_resource.go @@ -0,0 +1,215 @@ +/* + 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/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" + + // VolumeMountDir is where the volume resource will be mounted + VolumeMountDir = "/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.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 and owner references, +// unless a PVC with the same name in the same namespace already exists. +func ApplyPVC(name, namespace 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, + }, + }, + }, + } + 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 from which to get artifacts which is required +// by a Build/Task for context (e.g. a archive from which to build an image). +type VolumeResource struct { + Name string + Namespace string + Type PipelineResourceType + SourceDir string + ParsedSize resource.Quantity +} + +// GetSetup returns a PipelineResourceSetupInterface that can create the backing PVC if needed. +func (s VolumeResource) GetSetup() PipelineResourceSetupInterface { return SetupPVC{} } + +// NewVolumeResource instantiates the VolumeResource by parsing its params. +func NewVolumeResource(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, + } + size := DefaultPvcSize + for _, param := range r.Spec.Params { + switch { + case strings.EqualFold(param.Name, "Size"): + size = param.Value + // TODO: does sourceDir make sense? or is it just used wrong? + case strings.EqualFold(param.Name, "SourceDir"): + s.SourceDir = 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 PipelineResourceTypeStorage +} + +// 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), + // TODO: sourceDir? size? + } +} + +// GetUploadSteps returns the steps that are needed to copy data from the sourcePath +// on disk onto the Volume so it can be persisted. +func (s VolumeResource) GetUploadSteps(sourcePath string) ([]Step, error) { + return []Step{{Container: corev1.Container{ + Name: names.SimpleNameGenerator.RestrictLengthWithRandomSuffix(fmt.Sprintf("upload-mkdir-%s", s.Name)), + Image: *BashNoopImage, + Command: []string{"/ko-app/bash"}, + Args: []string{ + "-args", strings.Join([]string{"mkdir", "-p", filepath.Join(VolumeMountDir, sourcePath)}, " "), + }, + VolumeMounts: []corev1.VolumeMount{s.getPvcMount()}, + }}, {Container: corev1.Container{ + Name: names.SimpleNameGenerator.RestrictLengthWithRandomSuffix(fmt.Sprintf("upload-copy-%s", s.Name)), + Image: *BashNoopImage, + Command: []string{"/ko-app/bash"}, + Args: []string{ + "-args", strings.Join([]string{"cp", "-r", fmt.Sprintf("%s/.", s.SourceDir), filepath.Join(VolumeMountDir, sourcePath)}, " "), + }, + VolumeMounts: []corev1.VolumeMount{s.getPvcMount()}, + }}}, nil +} + +// GetDownloadSteps 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) GetDownloadSteps(sourcePath string) ([]Step, error) { + return []Step{ + CreateDirStep(s.Name, sourcePath), + {Container: corev1.Container{ + Name: names.SimpleNameGenerator.RestrictLengthWithRandomSuffix(fmt.Sprintf("download-copy-%s", s.Name)), + Image: *BashNoopImage, + Command: []string{"/ko-app/bash"}, + Args: []string{ + "-args", strings.Join([]string{"cp", "-r", fmt.Sprintf("%s/.", filepath.Join(VolumeMountDir, s.SourceDir)), sourcePath}, " "), + }, + VolumeMounts: []corev1.VolumeMount{s.getPvcMount()}, + }}}, nil +} + +func (s VolumeResource) getPvcMount() corev1.VolumeMount { + return corev1.VolumeMount{ + Name: s.Name, + MountPath: VolumeMountDir, + } +} + +// GetDownloadVolumeSpec returns the spec of the PVC volume that will be used to hold data +// for the VolumeResource. Adding this spec to a pod will make the PVC resource available to the pod. +func (s VolumeResource) GetDownloadVolumeSpec(ts *TaskSpec) ([]corev1.Volume, error) { + return s.getVolumeSpec() +} + +// GetUploadVolumeSpec returns the spec of the PVC volume that will be used to hold data +// for the VolumeResource. Adding this spec to a pod will make the PVC resource available to the pod. +func (s VolumeResource) GetUploadVolumeSpec(ts *TaskSpec) ([]corev1.Volume, error) { + return s.getVolumeSpec() +} + +func (s VolumeResource) getVolumeSpec() ([]corev1.Volume, error) { + return []corev1.Volume{corev1.Volume{ + Name: s.Name, + VolumeSource: corev1.VolumeSource{ + PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ClaimName: s.Name}, + }, + }}, nil +} + +// 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..6fb157b898b --- /dev/null +++ b/pkg/apis/pipeline/v1alpha1/volume_resource_test.go @@ -0,0 +1,249 @@ +/* + 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.SourceDir != want.SourceDir { + t.Errorf("Expected both to have SourceDir %s but got %s", want.SourceDir, got.SourceDir) + } + if got.ParsedSize != want.ParsedSize { + t.Errorf("Expected both to have ParsedSize %v but got %v", want.ParsedSize, got.ParsedSize) + } +} + +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, + }, + }, { + desc: "volume resource with size", + 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"), + )), + pvcExists: true, + want: &v1alpha1.VolumeResource{ + Name: "test-volume-resource", + Type: v1alpha1.PipelineResourceTypeStorage, + ParsedSize: size10, + }, + }} { + t.Run(c.desc, func(t *testing.T) { + got, err := v1alpha1.NewVolumeResource(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) { + // Invalid size +} + +func TestApplyPVC_doesntExist(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) + } + 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, 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) + } +} + +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, size, ownerReferences, get, create) + if err != nil { + t.Fatalf("Didn't expect error since PVC already exists but got %v", err) + } +} + +func Test_VolumeResource_GetDownloadSteps(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 + wantErr bool + }{{ + name: "valid volume resource config", + volumeResource: &v1alpha1.VolumeResource{ + Name: "test-volume-resource", + Type: v1alpha1.PipelineResourceTypeVolume, + ParsedSize: size, + SourceDir: "/src-dir", + }, + 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"}, + }}, {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/src-dir/. /workspace"}, + VolumeMounts: []corev1.VolumeMount{{ + Name: "test-volume-resource", + MountPath: "/volumeresource", + }}, + }}}, + }} + for _, tc := range testcases { + t.Run(tc.name, func(t *testing.T) { + gotContainers, err := tc.volumeResource.GetDownloadSteps("/workspace") + if tc.wantErr && err == nil { + t.Fatalf("Expected error to be %t but got %v:", tc.wantErr, err) + } + if d := cmp.Diff(gotContainers, tc.wantSteps); d != "" { + t.Errorf("Error mismatch between download containers spec: %s", d) + } + }) + } +} + +func Test_VolumeResource_GetUploadSteps(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 + wantErr bool + }{{ + name: "valid volume resource config", + volumeResource: &v1alpha1.VolumeResource{ + Name: "test-volume-resource", + Type: v1alpha1.PipelineResourceTypeVolume, + ParsedSize: size, + SourceDir: "/src-dir", + }, + wantSteps: []v1alpha1.Step{{Container: corev1.Container{ + Name: "upload-mkdir-test-volume-resource-9l9zj", + Image: "override-with-bash-noop:latest", + Command: []string{"/ko-app/bash"}, + Args: []string{"-args", "mkdir -p /volumeresource/workspace"}, + VolumeMounts: []corev1.VolumeMount{{ + Name: "test-volume-resource", + MountPath: "/volumeresource", + }}, + }}, {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 /src-dir/. /volumeresource/workspace"}, + VolumeMounts: []corev1.VolumeMount{{ + Name: "test-volume-resource", + MountPath: "/volumeresource", + }}, + }}}, + }} + for _, tc := range testcases { + t.Run(tc.name, func(t *testing.T) { + gotContainers, err := tc.volumeResource.GetUploadSteps("/workspace") + if tc.wantErr && err == nil { + t.Fatalf("Expected error to be %t but got %v:", tc.wantErr, err) + } + if d := cmp.Diff(gotContainers, tc.wantSteps); d != "" { + t.Errorf("Error mismatch between download containers spec: %s", d) + } + }) + } +} diff --git a/pkg/apis/pipeline/v1alpha1/zz_generated.deepcopy.go b/pkg/apis/pipeline/v1alpha1/zz_generated.deepcopy.go index 08aafcd1c78..b3917d306f4 100644 --- a/pkg/apis/pipeline/v1alpha1/zz_generated.deepcopy.go +++ b/pkg/apis/pipeline/v1alpha1/zz_generated.deepcopy.go @@ -1858,3 +1858,19 @@ 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 + 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/resources/conditionresolution.go b/pkg/reconciler/pipelinerun/resources/conditionresolution.go index eeafee3a8ff..7d5a9dc1a5b 100644 --- a/pkg/reconciler/pipelinerun/resources/conditionresolution.go +++ b/pkg/reconciler/pipelinerun/resources/conditionresolution.go @@ -107,9 +107,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) if err != nil { @@ -119,7 +117,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 { @@ -130,8 +128,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) error { replacements := make(map[string]string) for _, cr := range conditionResources { @@ -150,7 +147,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/taskrun/resources/input_resource_test.go b/pkg/reconciler/taskrun/resources/input_resource_test.go index 2821c2f21ef..55a76a0a12b 100644 --- a/pkg/reconciler/taskrun/resources/input_resource_test.go +++ b/pkg/reconciler/taskrun/resources/input_resource_test.go @@ -66,6 +66,14 @@ var ( Type: "cluster", }}}, } + volumeInputs = &v1alpha1.Inputs{ + Resources: []v1alpha1.TaskResource{{ + ResourceDeclaration: v1alpha1.ResourceDeclaration{ + Name: "workspace", + Type: "storage", + TargetPath: "sub-dir", + }}}, + } ) func setUp(t *testing.T) { @@ -197,6 +205,23 @@ func setUp(t *testing.T) { 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 { @@ -234,6 +259,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{ @@ -699,6 +733,147 @@ 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{{ + 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/. /workspace/sub-dir"}, + VolumeMounts: []corev1.VolumeMount{{ + Name: "volume-valid", + MountPath: "/volumeresource", + }}, + }}}, + 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{{ + 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/. /workspace/sub-dir"}, + VolumeMounts: []corev1.VolumeMount{{ + Name: "volume-valid", + MountPath: "/volumeresource", + }}, + }}}, + 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{{ + 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-workspace-mz4c7", + Image: "override-with-bash-noop:latest", + Command: []string{"/ko-app/bash"}, + Args: []string{"-args", "mkdir -p /workspace/sub-dir"}, + }}, {Container: corev1.Container{ + Name: "source-copy-workspace-9l9zj", + Image: "override-with-bash-noop:latest", + Command: []string{"/ko-app/bash"}, + Args: []string{"-args", "cp -r prev-task-path/. /workspace/sub-dir"}, + VolumeMounts: []corev1.VolumeMount{{MountPath: "/pvc", Name: "pipelinerun-pvc"}}, + }}}, + Volumes: []corev1.Volume{{ + Name: "pipelinerun-pvc", + VolumeSource: corev1.VolumeSource{ + PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ClaimName: "pipelinerun-pvc"}, + }, + }}, + }, }} { t.Run(c.desc, func(t *testing.T) { setUp(t) diff --git a/pkg/reconciler/taskrun/resources/input_resources.go b/pkg/reconciler/taskrun/resources/input_resources.go index cb8373162c3..9cec32a3f25 100644 --- a/pkg/reconciler/taskrun/resources/input_resources.go +++ b/pkg/reconciler/taskrun/resources/input_resources.go @@ -114,6 +114,7 @@ func AddInputResource( resourceVolumes, err = resource.GetDownloadVolumeSpec(taskSpec) if err != nil { return nil, xerrors.Errorf("task %q invalid resource download spec: %q; error %w", taskName, boundResource.ResourceRef.Name, err) + } allResourceSteps = append(allResourceSteps, resourceSteps...) diff --git a/pkg/reconciler/taskrun/resources/output_resource_test.go b/pkg/reconciler/taskrun/resources/output_resource_test.go index cb4c6485af9..65c6da49c4e 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(t *testing.T) { 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) @@ -741,6 +759,165 @@ 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{{ + 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-mssqb", + Image: "override-with-bash-noop:latest", + Command: []string{"/ko-app/bash"}, + Args: []string{"-args", "mkdir -p /workspace"}, + }}, {Container: corev1.Container{ + Name: "upload-mkdir-source-volume-9l9zj", + Image: "override-with-bash-noop:latest", + VolumeMounts: []corev1.VolumeMount{{ + Name: "source-volume", MountPath: "/volumeresource", + }}, + Command: []string{"/ko-app/bash"}, + Args: []string{"-args", "mkdir -p /volumeresource/workspace"}, + }}, {Container: corev1.Container{ + Name: "upload-copy-source-volume-mz4c7", + Image: "override-with-bash-noop:latest", + VolumeMounts: []corev1.VolumeMount{{ + Name: "source-volume", MountPath: "/volumeresource", + }}, + Command: []string{"/ko-app/bash"}, + Args: []string{"-args", "cp -r /. /volumeresource/workspace"}, + }}}, + 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{{ + Name: "source-workspace", + ResourceRef: v1alpha1.PipelineResourceRef{ + Name: "source-volume", + }, + }}, + }, + Outputs: v1alpha1.TaskRunOutputs{ + Resources: []v1alpha1.TaskResourceBinding{{ + 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-6nl7g", + Image: "override-with-bash-noop:latest", + Command: []string{"/ko-app/bash"}, + Args: []string{"-args", "mkdir -p /workspace/output/source-workspace"}, + }}, {Container: corev1.Container{ + Name: "upload-mkdir-source-volume-9l9zj", + Image: "override-with-bash-noop:latest", + VolumeMounts: []corev1.VolumeMount{{ + Name: "source-volume", MountPath: "/volumeresource", + }}, + Command: []string{"/ko-app/bash"}, + Args: []string{"-args", "mkdir -p /volumeresource/workspace/output/source-workspace"}, + }}, {Container: corev1.Container{ + Name: "upload-copy-source-volume-mz4c7", + Image: "override-with-bash-noop:latest", + VolumeMounts: []corev1.VolumeMount{{ + Name: "source-volume", + MountPath: "/volumeresource", + }}, + Command: []string{"/ko-app/bash"}, + Args: []string{"-args", "cp -r /. /volumeresource/workspace/output/source-workspace"}, + }}, {Container: corev1.Container{ + Name: "source-mkdir-source-volume-mssqb", + 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"}}, + }}, {Container: corev1.Container{ + Name: "source-copy-source-volume-78c5n", + 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"}}, + }}}, + 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() @@ -752,8 +929,8 @@ 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() != "" { diff --git a/pkg/reconciler/taskrun/taskrun.go b/pkg/reconciler/taskrun/taskrun.go index d59170c31c7..e000f32c90f 100644 --- a/pkg/reconciler/taskrun/taskrun.go +++ b/pkg/reconciler/taskrun/taskrun.go @@ -417,17 +417,34 @@ 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) + inputResources, err := getPipelineResourceInstances(rtr.Inputs, tr.OwnerReferences, c.KubeClientSet) 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) + outputResources, err := getPipelineResourceInstances(rtr.Outputs, tr.OwnerReferences, c.KubeClientSet) 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 } + 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(tr, ts, c.resourceLister.PipelineResources(tr.Namespace).Get) @@ -548,8 +565,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) (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) (map[string]v1alpha1.PipelineResourceInterface, error) { p := make(map[string]v1alpha1.PipelineResourceInterface) for rName, r := range resources { i, err := v1alpha1.ResourceFromType(r) diff --git a/pkg/reconciler/taskrun/taskrun_test.go b/pkg/reconciler/taskrun/taskrun_test.go index 8e504fee003..f36230e46b1 100644 --- a/pkg/reconciler/taskrun/taskrun_test.go +++ b/pkg/reconciler/taskrun/taskrun_test.go @@ -96,7 +96,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")))) @@ -159,6 +159,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", @@ -193,6 +196,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{ @@ -308,7 +321,7 @@ func TestReconcile(t *testing.T) { ), ), tb.TaskRunOutputs( - tb.TaskRunOutputsResource(gitResource.Name, + tb.TaskRunOutputsResource(volumeResource.Name, tb.TaskResourceBindingPaths("output-folder"), ), ), @@ -419,7 +432,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 @@ -596,11 +609,11 @@ func TestReconcile(t *testing.T) { ReadOnly: false, }, }, - }, toolsVolume, downward, workspaceVolume, homeVolume), + }, volumeVolume, toolsVolume, downward, workspaceVolume, homeVolume), tb.PodRestartPolicy(corev1.RestartPolicyNever), - getCredentialsInitContainer("l22wn"), + getCredentialsInitContainer("mnq6l"), getPlaceToolsInitContainer(), - getMkdirResourceContainer("git-resource", "/workspace/output/git-resource", "vr6ds"), + getMkdirResourceContainer("volume-resource", "/workspace/output/volume-resource", "twkr2"), tb.PodContainer("step-create-dir-another-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", "--", @@ -677,9 +690,42 @@ func TestReconcile(t *testing.T) { tb.EphemeralStorage("0"), )), ), - tb.PodContainer("step-source-mkdir-git-resource-6nl7g", "override-with-bash-noop:latest", + tb.PodContainer("step-upload-mkdir-volume-resource-6nl7g", "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 /volumeresource/workspace/output/volume-resource"), + tb.WorkingDir(workspaceDir), + tb.EnvVar("HOME", "/builder/home"), + tb.VolumeMount("volume-resource", "/volumeresource"), + tb.VolumeMount("tools", "/builder/tools"), + tb.VolumeMount("workspace", workspaceDir), + tb.VolumeMount("home", "/builder/home"), + tb.Resources(tb.Requests( + tb.CPU("0"), + tb.Memory("0"), + tb.EphemeralStorage("0"), + )), + ), + tb.PodContainer("step-upload-copy-volume-resource-j2tds", "override-with-bash-noop:latest", + tb.Command(entrypointLocation), + tb.Args("-wait_file", "/builder/tools/6", "-post_file", "/builder/tools/7", "-entrypoint", "/ko-app/bash", "--", + // TODO: pretty sure this is a bug + "-args", "cp -r /. /volumeresource/workspace/output/volume-resource"), + tb.WorkingDir(workspaceDir), + tb.EnvVar("HOME", "/builder/home"), + tb.VolumeMount("volume-resource", "/volumeresource"), + tb.VolumeMount("tools", "/builder/tools"), + tb.VolumeMount("workspace", workspaceDir), + tb.VolumeMount("home", "/builder/home"), + tb.Resources(tb.Requests( + tb.CPU("0"), + tb.Memory("0"), + tb.EphemeralStorage("0"), + )), + ), + tb.PodContainer("step-source-mkdir-volume-resource-vr6ds", "override-with-bash-noop:latest", + tb.Command(entrypointLocation), + tb.Args("-wait_file", "/builder/tools/7", "-post_file", "/builder/tools/8", "-entrypoint", "/ko-app/bash", "--", "-args", "mkdir -p output-folder"), tb.WorkingDir(workspaceDir), tb.EnvVar("HOME", "/builder/home"), @@ -693,10 +739,10 @@ func TestReconcile(t *testing.T) { tb.EphemeralStorage("0"), )), ), - tb.PodContainer("step-source-copy-git-resource-j2tds", "override-with-bash-noop:latest", + tb.PodContainer("step-source-copy-volume-resource-l22wn", "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"), + tb.Args("-wait_file", "/builder/tools/8", "-post_file", "/builder/tools/9", "-entrypoint", "/ko-app/bash", "--", + "-args", "cp -r /workspace/output/volume-resource/. output-folder"), tb.WorkingDir(workspaceDir), tb.EnvVar("HOME", "/builder/home"), tb.VolumeMount("test-pvc", "/pvc"), @@ -1089,16 +1135,35 @@ func TestReconcile(t *testing.T) { t.Errorf("Pod metadata doesn't match, diff: %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 { + ensurePVCCreated(t, clients, volumeResource.Name, volumeResource.Namespace) } }) } } +func ensurePVCCreated(t *testing.T, clients test.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") + } +} + func TestReconcile_SetsStartTime(t *testing.T) { taskRun := tb.TaskRun("test-taskrun", "foo", tb.TaskRunSpec( tb.TaskRunTaskRef(simpleTask.Name), @@ -1177,10 +1242,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, taskMultipleSteps) + verifyTaskRunStatusStep(t, taskRun, taskMultipleSteps) } -func verify_TaskRunStatusStep(t *testing.T, taskRun *v1alpha1.TaskRun, task *v1alpha1.Task) { +func verifyTaskRunStatusStep(t *testing.T, taskRun *v1alpha1.TaskRun, task *v1alpha1.Task) { actualStepOrder := []string{} for _, state := range taskRun.Status.Steps { actualStepOrder = append(actualStepOrder, state.Name)