Skip to content

Commit

Permalink
Refactor input resource volume handling to remove a type switch state…
Browse files Browse the repository at this point in the history
…ment.

This change refactors the way input resources attach volumes to their steps.
Previously, there was a special-cased switch statement for Storage type resources.
This made the logic for resources hard to follow, and spread across the codebase. This
change refactors that by moving it up into the general Resource interface.

This is another follow-on to #1135
  • Loading branch information
dlorenc authored and tekton-robot committed Jul 31, 2019
1 parent 9d7e37e commit 520e46a
Show file tree
Hide file tree
Showing 8 changed files with 32 additions and 52 deletions.
4 changes: 4 additions & 0 deletions pkg/apis/pipeline/v1alpha1/build_gcs_resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -177,3 +177,7 @@ func getArtifactType(val string) (GCSArtifactType, error) {
func (s *BuildGCSResource) GetUploadVolumeSpec(spec *TaskSpec) ([]corev1.Volume, error) {
return getStorageUploadVolumeSpec(s, spec)
}

func (s *BuildGCSResource) GetDownloadVolumeSpec(spec *TaskSpec) ([]corev1.Volume, error) {
return getStorageUploadVolumeSpec(s, spec)
}
4 changes: 4 additions & 0 deletions pkg/apis/pipeline/v1alpha1/cluster_resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -177,3 +177,7 @@ func (s *ClusterResource) GetDownloadContainerSpec() ([]corev1.Container, error)
func (s *ClusterResource) GetUploadVolumeSpec(spec *TaskSpec) ([]corev1.Volume, error) {
return nil, nil
}

func (s *ClusterResource) GetDownloadVolumeSpec(spec *TaskSpec) ([]corev1.Volume, error) {
return nil, nil
}
4 changes: 4 additions & 0 deletions pkg/apis/pipeline/v1alpha1/gcs_resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -154,3 +154,7 @@ func (s *GCSResource) GetDownloadContainerSpec() ([]corev1.Container, error) {
func (s *GCSResource) GetUploadVolumeSpec(spec *TaskSpec) ([]corev1.Volume, error) {
return getStorageUploadVolumeSpec(s, spec)
}

func (s *GCSResource) GetDownloadVolumeSpec(spec *TaskSpec) ([]corev1.Volume, error) {
return getStorageUploadVolumeSpec(s, spec)
}
4 changes: 4 additions & 0 deletions pkg/apis/pipeline/v1alpha1/git_resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -124,3 +124,7 @@ func (s *GitResource) GetUploadContainerSpec() ([]corev1.Container, error) {
func (s *GitResource) GetUploadVolumeSpec(spec *TaskSpec) ([]corev1.Volume, error) {
return nil, nil
}

func (s *GitResource) GetDownloadVolumeSpec(spec *TaskSpec) ([]corev1.Volume, error) {
return nil, nil
}
4 changes: 4 additions & 0 deletions pkg/apis/pipeline/v1alpha1/image_resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -105,3 +105,7 @@ func (s ImageResource) String() string {
func (s *ImageResource) GetUploadVolumeSpec(spec *TaskSpec) ([]corev1.Volume, error) {
return nil, nil
}

func (s *ImageResource) GetDownloadVolumeSpec(spec *TaskSpec) ([]corev1.Volume, error) {
return nil, nil
}
4 changes: 4 additions & 0 deletions pkg/apis/pipeline/v1alpha1/pull_request_resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -142,3 +142,7 @@ func (s *PullRequestResource) SetDestinationDirectory(dir string) {
func (s *PullRequestResource) GetUploadVolumeSpec(spec *TaskSpec) ([]corev1.Volume, error) {
return nil, nil
}

func (s *PullRequestResource) GetDownloadVolumeSpec(spec *TaskSpec) ([]corev1.Volume, error) {
return nil, nil
}
1 change: 1 addition & 0 deletions pkg/apis/pipeline/v1alpha1/resource_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ type PipelineResourceInterface interface {
GetDownloadContainerSpec() ([]corev1.Container, error)
GetUploadContainerSpec() ([]corev1.Container, error)
GetUploadVolumeSpec(spec *TaskSpec) ([]corev1.Volume, error)
GetDownloadVolumeSpec(spec *TaskSpec) ([]corev1.Volume, error)
SetDestinationDirectory(string)
}

Expand Down
59 changes: 7 additions & 52 deletions pkg/reconciler/v1alpha1/taskrun/resources/input_resources.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ limitations under the License.
package resources

import (
"fmt"
"path/filepath"

"github.com/tektoncd/pipeline/pkg/apis/pipeline"
Expand Down Expand Up @@ -113,25 +112,13 @@ func AddInputResource(
taskSpec.Steps = append(copyStepsFromPrevTasks, taskSpec.Steps...)
taskSpec.Volumes = append(taskSpec.Volumes, as.GetSecretsVolumes()...)
} else {
switch resource.GetType() {
case v1alpha1.PipelineResourceTypeStorage:
{
storageResource, ok := resource.(v1alpha1.PipelineStorageResourceInterface)
if !ok {
return nil, xerrors.Errorf("task %q invalid gcs Pipeline Resource: %q", taskName, boundResource.ResourceRef.Name)
}
resourceContainers, resourceVolumes, err = addStorageFetchStep(taskSpec, storageResource)
if err != nil {
return nil, xerrors.Errorf("task %q invalid gcs Pipeline Resource download steps: %q: %w", taskName, boundResource.ResourceRef.Name, err)
}
}
default:
{
resourceContainers, err = resource.GetDownloadContainerSpec()
if err != nil {
return nil, xerrors.Errorf("task %q invalid resource download spec: %q; error %w", taskName, boundResource.ResourceRef.Name, err)
}
}
resourceContainers, err = resource.GetDownloadContainerSpec()
if err != nil {
return nil, xerrors.Errorf("task %q invalid resource download spec: %q; error %w", taskName, boundResource.ResourceRef.Name, err)
}
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)
}

allResourceContainers = append(allResourceContainers, resourceContainers...)
Expand All @@ -146,38 +133,6 @@ func AddInputResource(
return taskSpec, nil
}

func addStorageFetchStep(taskSpec *v1alpha1.TaskSpec, storageResource v1alpha1.PipelineStorageResourceInterface) ([]corev1.Container, []corev1.Volume, error) {
gcsContainers, err := storageResource.GetDownloadContainerSpec()
if err != nil {
return nil, nil, err
}

var storageVol []corev1.Volume
mountedSecrets := map[string]string{}
for _, volume := range taskSpec.Volumes {
mountedSecrets[volume.Name] = ""
}

for _, secretParam := range storageResource.GetSecretParams() {
volName := fmt.Sprintf("volume-%s-%s", storageResource.GetName(), secretParam.SecretName)

gcsSecretVolume := corev1.Volume{
Name: volName,
VolumeSource: corev1.VolumeSource{
Secret: &corev1.SecretVolumeSource{
SecretName: secretParam.SecretName,
},
},
}

if _, ok := mountedSecrets[volName]; !ok {
storageVol = append(storageVol, gcsSecretVolume)
mountedSecrets[volName] = ""
}
}
return gcsContainers, storageVol, nil
}

func getResource(r *v1alpha1.TaskResourceBinding, getter GetResource) (*v1alpha1.PipelineResource, error) {
// Check both resource ref or resource Spec are not present. Taskrun webhook should catch this in validation error.
if r.ResourceRef.Name != "" && r.ResourceSpec != nil {
Expand Down

0 comments on commit 520e46a

Please sign in to comment.