From debeb3f4ce43e2e13a381e718c707825deee95ab Mon Sep 17 00:00:00 2001 From: Shash Reddy Date: Sat, 26 Jan 2019 22:34:15 -0500 Subject: [PATCH] Refactor resource interface Try to unify different types of pipeline resource when processing input / output resources. As we add more resources the contract should be more simple. Removed git from build source and add the functionality similar to other resources. Resource defines list of container definitions to download and list of container definitions to upload as well. --- cmd/git-init/main.go | 20 +- .../pipeline/v1alpha1/cluster_resource.go | 42 ++ pkg/apis/pipeline/v1alpha1/gcs_resource.go | 1 + pkg/apis/pipeline/v1alpha1/git_resource.go | 44 +- pkg/apis/pipeline/v1alpha1/image_resource.go | 11 + pkg/apis/pipeline/v1alpha1/resource_types.go | 4 + .../pipeline/v1alpha1/storage_resource.go | 5 - .../taskrun/resources/input_resource_test.go | 413 +++++------------- .../taskrun/resources/input_resources.go | 161 +++---- .../taskrun/resources/output_resource.go | 148 +++---- .../taskrun/resources/output_resource_test.go | 79 ++-- .../v1alpha1/taskrun/resources/pod.go | 64 --- .../v1alpha1/taskrun/resources/pod_test.go | 279 ------------ .../v1alpha1/taskrun/taskrun_test.go | 32 +- 14 files changed, 430 insertions(+), 873 deletions(-) diff --git a/cmd/git-init/main.go b/cmd/git-init/main.go index e7173db0ca5..31c13dc7046 100644 --- a/cmd/git-init/main.go +++ b/cmd/git-init/main.go @@ -20,7 +20,6 @@ import ( "flag" "os" "os/exec" - "path/filepath" "github.com/knative/pkg/logging" "go.uber.org/zap" @@ -67,20 +66,19 @@ func main() { if err != nil { logger.Fatalf("Unexpected error creating symlink: %v", err) } - - dir, err := os.Getwd() - if err != nil { - logger.Errorf("Failed to get current dir", err) + if *revision == "" { + *revision = "master" } - if *path != "" { runOrFail(logger, "git", "init", *path) - path := filepath.Join(dir, *path) - if err := os.Chdir(path); err != nil { + if _, err := os.Stat(*path); os.IsNotExist(err) { + if err := os.Mkdir(*path, os.ModePerm); err != nil { + logger.Debugf("Creating directory at path %s", *path) + } + } + if err := os.Chdir(*path); err != nil { logger.Fatalf("Failed to change directory with path %s; err %v", path, err) } - // update dir variable with new path - dir = path } else { run(logger, "git", "init") } @@ -89,5 +87,5 @@ func main() { runOrFail(logger, "git", "fetch", "--depth=1", "--recurse-submodules=yes", "origin", *revision) runOrFail(logger, "git", "reset", "--hard", "FETCH_HEAD") - logger.Infof("Successfully cloned %q @ %q in path %q", *url, *revision, dir) + logger.Infof("Successfully cloned %q @ %q in path %q", *url, *revision, *path) } diff --git a/pkg/apis/pipeline/v1alpha1/cluster_resource.go b/pkg/apis/pipeline/v1alpha1/cluster_resource.go index 07209e6eeb3..92e37274aaf 100644 --- a/pkg/apis/pipeline/v1alpha1/cluster_resource.go +++ b/pkg/apis/pipeline/v1alpha1/cluster_resource.go @@ -19,9 +19,16 @@ package v1alpha1 import ( b64 "encoding/base64" "encoding/json" + "flag" "fmt" "strconv" "strings" + + corev1 "k8s.io/api/core/v1" +) + +var ( + kubeconfigWriterImage = flag.String("kubeconfig-writer-image", "override-with-kubeconfig-writer:latest", "The container image containing our kubeconfig writer binary.") ) // ClusterResource represents a cluster configuration (kubeconfig) @@ -134,3 +141,38 @@ func (s ClusterResource) String() string { json, _ := json.Marshal(s) return string(json) } + +func (s *ClusterResource) GetUploadContainerSpec() ([]corev1.Container, error) { + return nil, nil +} + +func (s *ClusterResource) SetDestinationDirectory(path string) { +} +func (s *ClusterResource) GetDownloadContainerSpec() ([]corev1.Container, error) { + var envVars []corev1.EnvVar + for _, sec := range s.Secrets { + ev := corev1.EnvVar{ + Name: strings.ToUpper(sec.FieldName), + ValueFrom: &corev1.EnvVarSource{ + SecretKeyRef: &corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: sec.SecretName, + }, + Key: sec.SecretKey, + }, + }, + } + envVars = append(envVars, ev) + } + + clusterContainer := corev1.Container{ + Name: "kubeconfig", + Image: *kubeconfigWriterImage, + Args: []string{ + "-clusterConfig", s.String(), + }, + Env: envVars, + } + + return []corev1.Container{clusterContainer}, nil +} diff --git a/pkg/apis/pipeline/v1alpha1/gcs_resource.go b/pkg/apis/pipeline/v1alpha1/gcs_resource.go index 98f5c3e11d4..937ead3d265 100644 --- a/pkg/apis/pipeline/v1alpha1/gcs_resource.go +++ b/pkg/apis/pipeline/v1alpha1/gcs_resource.go @@ -139,6 +139,7 @@ func (s *GCSResource) GetDownloadContainerSpec() ([]corev1.Container, error) { } envVars, secretVolumeMount := getSecretEnvVarsAndVolumeMounts(s.Name, s.Secrets) + return []corev1.Container{{ Name: fmt.Sprintf("storage-fetch-%s", s.Name), Image: *gsutilImage, diff --git a/pkg/apis/pipeline/v1alpha1/git_resource.go b/pkg/apis/pipeline/v1alpha1/git_resource.go index 868a7a11dc0..0cc00eb13c3 100644 --- a/pkg/apis/pipeline/v1alpha1/git_resource.go +++ b/pkg/apis/pipeline/v1alpha1/git_resource.go @@ -17,8 +17,20 @@ limitations under the License. package v1alpha1 import ( + "flag" "fmt" "strings" + + corev1 "k8s.io/api/core/v1" +) + +const workspaceDir = "/workspace" + +var ( + gitSource = "git-source" + // The container with Git that we use to implement the Git source step. + gitImage = flag.String("git-image", "override-with-git:latest", + "The container image containing our Git binary.") ) // GitResource is an endpoint from which to get data which is required @@ -30,7 +42,8 @@ type GitResource struct { // Git revision (branch, tag, commit SHA or ref) to clone. See // https://git-scm.com/docs/gitrevisions#_specifying_revisions for more // information. - Revision string `json:"revision"` + Revision string `json:"revision"` + TargetPath string } // NewGitResource create a new git resource to pass to Knative Build @@ -84,3 +97,32 @@ func (s *GitResource) Replacements() map[string]string { "revision": s.Revision, } } + +func (s *GitResource) GetDownloadContainerSpec() ([]corev1.Container, error) { + args := []string{"-url", s.URL, + "-revision", s.Revision, + } + var dPath string + if s.TargetPath != "" { + dPath = s.TargetPath + } else { + dPath = s.Name + } + + args = append(args, []string{"-path", dPath}...) + + return []corev1.Container{{ + Name: gitSource + "-" + s.Name, + Image: *gitImage, + Args: args, + WorkingDir: workspaceDir, + }}, nil +} + +func (s *GitResource) SetDestinationDirectory(path string) { + s.TargetPath = path +} + +func (s *GitResource) GetUploadContainerSpec() ([]corev1.Container, error) { + return nil, nil +} diff --git a/pkg/apis/pipeline/v1alpha1/image_resource.go b/pkg/apis/pipeline/v1alpha1/image_resource.go index 06c9135b5b4..b26023e1472 100644 --- a/pkg/apis/pipeline/v1alpha1/image_resource.go +++ b/pkg/apis/pipeline/v1alpha1/image_resource.go @@ -19,6 +19,8 @@ package v1alpha1 import ( "fmt" "strings" + + corev1 "k8s.io/api/core/v1" ) // NewImageResource creates a new ImageResource from a PipelineResource. @@ -73,3 +75,12 @@ func (s *ImageResource) Replacements() map[string]string { "digest": s.Digest, } } + +func (s *ImageResource) GetUploadContainerSpec() ([]corev1.Container, error) { + return nil, nil +} +func (s *ImageResource) GetDownloadContainerSpec() ([]corev1.Container, error) { + return nil, nil +} +func (s *ImageResource) SetDestinationDirectory(path string) { +} diff --git a/pkg/apis/pipeline/v1alpha1/resource_types.go b/pkg/apis/pipeline/v1alpha1/resource_types.go index 483b3e6f9db..4afaf4c2f45 100644 --- a/pkg/apis/pipeline/v1alpha1/resource_types.go +++ b/pkg/apis/pipeline/v1alpha1/resource_types.go @@ -21,6 +21,7 @@ import ( "github.com/knative/pkg/apis" "github.com/knative/pkg/webhook" + corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -52,6 +53,9 @@ type PipelineResourceInterface interface { GetType() PipelineResourceType GetParams() []Param Replacements() map[string]string + GetDownloadContainerSpec() ([]corev1.Container, error) + GetUploadContainerSpec() ([]corev1.Container, error) + SetDestinationDirectory(string) } // SecretParam indicates which secret can be used to populate a field of the resource diff --git a/pkg/apis/pipeline/v1alpha1/storage_resource.go b/pkg/apis/pipeline/v1alpha1/storage_resource.go index 34306bba4ad..4f7d4f6018e 100644 --- a/pkg/apis/pipeline/v1alpha1/storage_resource.go +++ b/pkg/apis/pipeline/v1alpha1/storage_resource.go @@ -19,8 +19,6 @@ package v1alpha1 import ( "fmt" "strings" - - corev1 "k8s.io/api/core/v1" ) type PipelineResourceStorageType string @@ -34,9 +32,6 @@ const ( type PipelineStorageResourceInterface interface { PipelineResourceInterface GetSecretParams() []SecretParam - GetDownloadContainerSpec() ([]corev1.Container, error) - GetUploadContainerSpec() ([]corev1.Container, error) - SetDestinationDirectory(string) } func NewStorageResource(r *PipelineResource) (PipelineStorageResourceInterface, error) { diff --git a/pkg/reconciler/v1alpha1/taskrun/resources/input_resource_test.go b/pkg/reconciler/v1alpha1/taskrun/resources/input_resource_test.go index 9fda08ec863..3205c189b7e 100644 --- a/pkg/reconciler/v1alpha1/taskrun/resources/input_resource_test.go +++ b/pkg/reconciler/v1alpha1/taskrun/resources/input_resource_test.go @@ -184,7 +184,6 @@ func build() *buildv1alpha1.Build { } } func TestAddResourceToBuild(t *testing.T) { - boolTrue := true task := &v1alpha1.Task{ ObjectMeta: metav1.ObjectMeta{ Name: "build-from-repo", @@ -258,37 +257,20 @@ func TestAddResourceToBuild(t *testing.T) { taskRun *v1alpha1.TaskRun build *buildv1alpha1.Build wantErr bool - want *buildv1alpha1.Build + want buildv1alpha1.BuildSpec }{{ desc: "simple with default revision", task: task, taskRun: taskRun, build: build(), wantErr: false, - want: &buildv1alpha1.Build{ - TypeMeta: metav1.TypeMeta{ - Kind: "Build", - APIVersion: "build.knative.dev/v1alpha1"}, - ObjectMeta: metav1.ObjectMeta{ - Name: "build-from-repo", - Namespace: "marshmallow", - OwnerReferences: []metav1.OwnerReference{{ - APIVersion: "pipeline.knative.dev/v1alpha1", - Kind: "TaskRun", - Name: "build-from-repo-run", - Controller: &boolTrue, - BlockOwnerDeletion: &boolTrue, - }}, - }, - Spec: buildv1alpha1.BuildSpec{ - Sources: []buildv1alpha1.SourceSpec{{ - Git: &buildv1alpha1.GitSourceSpec{ - Url: "https://github.com/grafeas/kritis", - Revision: "master", - }, - Name: "gitspace", - }}, - }, + want: buildv1alpha1.BuildSpec{ + Steps: []corev1.Container{{ + Name: "git-source-the-git", + Image: "override-with-git:latest", + Args: []string{"-url", "https://github.com/grafeas/kritis", "-revision", "master", "-path", "/workspace/gitspace"}, + WorkingDir: "/workspace", + }}, }, }, { desc: "simple with branch", @@ -314,32 +296,13 @@ func TestAddResourceToBuild(t *testing.T) { }, build: build(), wantErr: false, - want: &buildv1alpha1.Build{ - TypeMeta: metav1.TypeMeta{ - Kind: "Build", - APIVersion: "build.knative.dev/v1alpha1"}, - ObjectMeta: metav1.ObjectMeta{ - Name: "build-from-repo", - Namespace: "marshmallow", - OwnerReferences: []metav1.OwnerReference{ - { - APIVersion: "pipeline.knative.dev/v1alpha1", - Kind: "TaskRun", - Name: "build-from-repo-run", - Controller: &boolTrue, - BlockOwnerDeletion: &boolTrue, - }, - }, - }, - Spec: buildv1alpha1.BuildSpec{ - Sources: []buildv1alpha1.SourceSpec{{ - Name: "gitspace", - Git: &buildv1alpha1.GitSourceSpec{ - Url: "https://github.com/grafeas/kritis", - Revision: "branch", - }, - }}, - }, + want: buildv1alpha1.BuildSpec{ + Steps: []corev1.Container{{ + Name: "git-source-the-git-with-branch", + Image: "override-with-git:latest", + Args: []string{"-url", "https://github.com/grafeas/kritis", "-revision", "branch", "-path", "/workspace/gitspace"}, + WorkingDir: "/workspace", + }}, }, }, { desc: "same git input resource for task with diff resource name", @@ -370,41 +333,21 @@ func TestAddResourceToBuild(t *testing.T) { }, build: build(), wantErr: false, - want: &buildv1alpha1.Build{ - TypeMeta: metav1.TypeMeta{ - Kind: "Build", - APIVersion: "build.knative.dev/v1alpha1"}, - ObjectMeta: metav1.ObjectMeta{ - Name: "build-from-repo", - Namespace: "marshmallow", - OwnerReferences: []metav1.OwnerReference{ - { - APIVersion: "pipeline.knative.dev/v1alpha1", - Kind: "TaskRun", - Name: "build-from-repo-run", - Controller: &boolTrue, - BlockOwnerDeletion: &boolTrue, - }, - }, - }, - Spec: buildv1alpha1.BuildSpec{ - Sources: []buildv1alpha1.SourceSpec{{ - Name: "gitspace", - Git: &buildv1alpha1.GitSourceSpec{ - Url: "https://github.com/grafeas/kritis", - Revision: "branch", - }, - }, { - Name: "git-duplicate-space", - Git: &buildv1alpha1.GitSourceSpec{ - Url: "https://github.com/grafeas/kritis", - Revision: "branch", - }, - }}, - }, + want: buildv1alpha1.BuildSpec{ + Steps: []corev1.Container{{ + Name: "git-source-the-git-with-branch", + Image: "override-with-git:latest", + Args: []string{"-url", "https://github.com/grafeas/kritis", "-revision", "branch", "-path", "/workspace/git-duplicate-space"}, + WorkingDir: "/workspace", + }, { + Name: "git-source-the-git-with-branch", + Image: "override-with-git:latest", + Args: []string{"-url", "https://github.com/grafeas/kritis", "-revision", "branch", "-path", "/workspace/gitspace"}, + WorkingDir: "/workspace", + }}, }, }, { - desc: "set revision to default value", + desc: "set revision to default value 1", task: task, taskRun: &v1alpha1.TaskRun{ ObjectMeta: metav1.ObjectMeta{ @@ -427,33 +370,16 @@ func TestAddResourceToBuild(t *testing.T) { }, build: build(), wantErr: false, - want: &buildv1alpha1.Build{ - TypeMeta: metav1.TypeMeta{ - Kind: "Build", - APIVersion: "build.knative.dev/v1alpha1"}, - ObjectMeta: metav1.ObjectMeta{ - Name: "build-from-repo", - Namespace: "marshmallow", - OwnerReferences: []metav1.OwnerReference{{ - APIVersion: "pipeline.knative.dev/v1alpha1", - Kind: "TaskRun", - Name: "build-from-repo-run", - Controller: &boolTrue, - BlockOwnerDeletion: &boolTrue, - }}, - }, - Spec: buildv1alpha1.BuildSpec{ - Sources: []buildv1alpha1.SourceSpec{{ - Git: &buildv1alpha1.GitSourceSpec{ - Url: "https://github.com/grafeas/kritis", - Revision: "master", - }, - Name: "gitspace", - }}, - }, + want: buildv1alpha1.BuildSpec{ + Steps: []corev1.Container{{ + Name: "git-source-the-git", + Image: "override-with-git:latest", + Args: []string{"-url", "https://github.com/grafeas/kritis", "-revision", "master", "-path", "/workspace/gitspace"}, + WorkingDir: "/workspace", + }}, }, }, { - desc: "set revision to default value", + desc: "set revision to provdided branch", task: task, taskRun: &v1alpha1.TaskRun{ ObjectMeta: metav1.ObjectMeta{ @@ -476,31 +402,13 @@ func TestAddResourceToBuild(t *testing.T) { }, build: build(), wantErr: false, - want: &buildv1alpha1.Build{ - TypeMeta: metav1.TypeMeta{ - Kind: "Build", - APIVersion: "build.knative.dev/v1alpha1"}, - ObjectMeta: metav1.ObjectMeta{ - Name: "build-from-repo", - Namespace: "marshmallow", - OwnerReferences: []metav1.OwnerReference{ - { - APIVersion: "pipeline.knative.dev/v1alpha1", - Kind: "TaskRun", - Name: "build-from-repo-run", - Controller: &boolTrue, - BlockOwnerDeletion: &boolTrue, - }, - }}, - Spec: buildv1alpha1.BuildSpec{ - Sources: []buildv1alpha1.SourceSpec{{ - Git: &buildv1alpha1.GitSourceSpec{ - Url: "https://github.com/grafeas/kritis", - Revision: "branch", - }, - Name: "gitspace", - }}, - }, + want: buildv1alpha1.BuildSpec{ + Steps: []corev1.Container{{ + Name: "git-source-the-git-with-branch", + Image: "override-with-git:latest", + Args: []string{"-url", "https://github.com/grafeas/kritis", "-revision", "branch", "-path", "/workspace/gitspace"}, + WorkingDir: "/workspace", + }}, }, }, { desc: "git resource as input from previous task", @@ -528,38 +436,26 @@ func TestAddResourceToBuild(t *testing.T) { }, build: build(), wantErr: false, - want: &buildv1alpha1.Build{ - TypeMeta: metav1.TypeMeta{ - Kind: "Build", - APIVersion: "build.knative.dev/v1alpha1"}, - ObjectMeta: metav1.ObjectMeta{ - Name: "build-from-repo", - Namespace: "marshmallow", - OwnerReferences: []metav1.OwnerReference{{ - APIVersion: "pipeline.knative.dev/v1alpha1", - Kind: "TaskRun", - Name: "build-from-repo-run", - Controller: &boolTrue, - BlockOwnerDeletion: &boolTrue, - }}, - }, - Spec: buildv1alpha1.BuildSpec{ - Steps: []corev1.Container{{ - Name: "source-copy-gitspace-0", - Image: "override-with-bash-noop:latest", - Args: []string{"-args", "cp -r prev-task-path/. /workspace/gitspace"}, - VolumeMounts: []corev1.VolumeMount{{MountPath: "/pvc", Name: "pipelinerun-pvc"}}, - }}, - Volumes: []corev1.Volume{{ - Name: "pipelinerun-pvc", - VolumeSource: corev1.VolumeSource{ - PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ClaimName: "pipelinerun-pvc"}, - }, - }}, - }, + want: buildv1alpha1.BuildSpec{ + Steps: []corev1.Container{{ + Name: "create-dir-gitspace", + Image: "override-with-bash-noop:latest", + Args: []string{"-args", "mkdir -p /workspace/gitspace"}, + }, { + Name: "source-copy-gitspace-0", + Image: "override-with-bash-noop:latest", + Args: []string{"-args", "cp -r prev-task-path/. /workspace/gitspace"}, + VolumeMounts: []corev1.VolumeMount{{MountPath: "/pvc", Name: "pipelinerun-pvc"}}, + }}, + Volumes: []corev1.Volume{{ + Name: "pipelinerun-pvc", + VolumeSource: corev1.VolumeSource{ + PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ClaimName: "pipelinerun-pvc"}, + }, + }}, }, }, { - desc: "storage resource as input", + desc: "storage resource as input with target path", task: taskWithTargetPath, taskRun: &v1alpha1.TaskRun{ ObjectMeta: metav1.ObjectMeta{ @@ -579,36 +475,16 @@ func TestAddResourceToBuild(t *testing.T) { }, build: build(), wantErr: false, - want: &buildv1alpha1.Build{ - TypeMeta: metav1.TypeMeta{ - Kind: "Build", - APIVersion: "build.knative.dev/v1alpha1"}, - ObjectMeta: metav1.ObjectMeta{ - Name: "build-from-repo", - Namespace: "marshmallow", - OwnerReferences: []metav1.OwnerReference{{ - APIVersion: "pipeline.knative.dev/v1alpha1", - Kind: "TaskRun", - Name: "build-from-repo-run", - Controller: &boolTrue, - BlockOwnerDeletion: &boolTrue, - }}, - }, - Spec: buildv1alpha1.BuildSpec{ - Steps: []corev1.Container{{ - Name: "create-dir-storage1", - Image: "override-with-bash-noop:latest", - Args: []string{"-args", "mkdir -p /workspace/gcs-dir"}, - VolumeMounts: []corev1.VolumeMount{{ - Name: "workspace", MountPath: "/workspace", - }}, - }, { - Name: "storage-fetch-storage1", - Image: "override-with-gsutil-image:latest", - Args: []string{"-args", "cp gs://fake-bucket/rules.zip /workspace/gcs-dir"}, - VolumeMounts: []corev1.VolumeMount{{Name: "workspace", MountPath: "/workspace"}}, - }}, - }, + want: buildv1alpha1.BuildSpec{ + Steps: []corev1.Container{{ + Name: "create-dir-storage1", + Image: "override-with-bash-noop:latest", + Args: []string{"-args", "mkdir -p /workspace/gcs-dir"}, + }, { + Name: "storage-fetch-storage1", + Image: "override-with-gsutil-image:latest", + Args: []string{"-args", "cp gs://fake-bucket/rules.zip /workspace/gcs-dir"}, + }}, }, }, { desc: "storage resource as input from previous task", @@ -636,35 +512,23 @@ func TestAddResourceToBuild(t *testing.T) { }, build: build(), wantErr: false, - want: &buildv1alpha1.Build{ - TypeMeta: metav1.TypeMeta{ - Kind: "Build", - APIVersion: "build.knative.dev/v1alpha1"}, - ObjectMeta: metav1.ObjectMeta{ - Name: "build-from-repo", - Namespace: "marshmallow", - OwnerReferences: []metav1.OwnerReference{{ - APIVersion: "pipeline.knative.dev/v1alpha1", - Kind: "TaskRun", - Name: "build-from-repo-run", - Controller: &boolTrue, - BlockOwnerDeletion: &boolTrue, - }}, - }, - Spec: buildv1alpha1.BuildSpec{ - Steps: []corev1.Container{{ - Name: "source-copy-workspace-0", - Image: "override-with-bash-noop:latest", - Args: []string{"-args", "cp -r prev-task-path/. /workspace/gcs-dir"}, - VolumeMounts: []corev1.VolumeMount{{MountPath: "/pvc", Name: "pipelinerun-pvc"}}, - }}, - Volumes: []corev1.Volume{{ - Name: "pipelinerun-pvc", - VolumeSource: corev1.VolumeSource{ - PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ClaimName: "pipelinerun-pvc"}, - }, - }}, - }, + want: buildv1alpha1.BuildSpec{ + Steps: []corev1.Container{{ + Name: "create-dir-workspace", + Image: "override-with-bash-noop:latest", + Args: []string{"-args", "mkdir -p /workspace/gcs-dir"}, + }, { + Name: "source-copy-workspace-0", + Image: "override-with-bash-noop:latest", + Args: []string{"-args", "cp -r prev-task-path/. /workspace/gcs-dir"}, + VolumeMounts: []corev1.VolumeMount{{MountPath: "/pvc", Name: "pipelinerun-pvc"}}, + }}, + Volumes: []corev1.Volume{{ + Name: "pipelinerun-pvc", + VolumeSource: corev1.VolumeSource{ + PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ClaimName: "pipelinerun-pvc"}, + }, + }}, }, }, { desc: "invalid gcs resource type name", @@ -687,7 +551,6 @@ func TestAddResourceToBuild(t *testing.T) { }, build: build(), wantErr: true, - want: nil, }, { desc: "invalid gcs resource type name", task: task, @@ -709,7 +572,6 @@ func TestAddResourceToBuild(t *testing.T) { }, build: build(), wantErr: true, - want: nil, }, { desc: "invalid resource name", task: &v1alpha1.Task{ @@ -729,7 +591,6 @@ func TestAddResourceToBuild(t *testing.T) { taskRun: taskRun, build: build(), wantErr: true, - want: nil, }, { desc: "cluster resource with plain text", task: &v1alpha1.Task{ @@ -767,30 +628,14 @@ func TestAddResourceToBuild(t *testing.T) { }, build: build(), wantErr: false, - want: &buildv1alpha1.Build{ - TypeMeta: metav1.TypeMeta{ - Kind: "Build", - APIVersion: "build.knative.dev/v1alpha1"}, - ObjectMeta: metav1.ObjectMeta{ - Name: "build-from-repo", - Namespace: "marshmallow", - OwnerReferences: []metav1.OwnerReference{{ - APIVersion: "pipeline.knative.dev/v1alpha1", - Kind: "TaskRun", - Name: "build-from-repo-run", - Controller: &boolTrue, - BlockOwnerDeletion: &boolTrue, - }}, - }, - Spec: buildv1alpha1.BuildSpec{ - Steps: []corev1.Container{{ - Name: "kubeconfig", - Image: "override-with-kubeconfig-writer:latest", - Args: []string{ - "-clusterConfig", `{"name":"cluster3","type":"cluster","url":"http://10.10.10.10","revision":"","username":"","password":"","token":"","Insecure":false,"cadata":"bXktY2EtY2VydAo=","secrets":null}`, - }, - }}, - }, + want: buildv1alpha1.BuildSpec{ + Steps: []corev1.Container{{ + Name: "kubeconfig", + Image: "override-with-kubeconfig-writer:latest", + Args: []string{ + "-clusterConfig", `{"name":"cluster3","type":"cluster","url":"http://10.10.10.10","revision":"","username":"","password":"","token":"","Insecure":false,"cadata":"bXktY2EtY2VydAo=","secrets":null}`, + }, + }}, }, }, { desc: "cluster resource with secrets", @@ -829,41 +674,25 @@ func TestAddResourceToBuild(t *testing.T) { }, build: build(), wantErr: false, - want: &buildv1alpha1.Build{ - TypeMeta: metav1.TypeMeta{ - Kind: "Build", - APIVersion: "build.knative.dev/v1alpha1"}, - ObjectMeta: metav1.ObjectMeta{ - Name: "build-from-repo", - Namespace: "marshmallow", - OwnerReferences: []metav1.OwnerReference{{ - APIVersion: "pipeline.knative.dev/v1alpha1", - Kind: "TaskRun", - Name: "build-from-repo-run", - Controller: &boolTrue, - BlockOwnerDeletion: &boolTrue, - }}, - }, - Spec: buildv1alpha1.BuildSpec{ - Steps: []corev1.Container{{ - Name: "kubeconfig", - Image: "override-with-kubeconfig-writer:latest", - Args: []string{ - "-clusterConfig", `{"name":"cluster2","type":"cluster","url":"http://10.10.10.10","revision":"","username":"","password":"","token":"","Insecure":false,"cadata":null,"secrets":[{"fieldName":"cadata","secretKey":"cadatakey","secretName":"secret1"}]}`, - }, - Env: []corev1.EnvVar{{ - ValueFrom: &corev1.EnvVarSource{ - SecretKeyRef: &corev1.SecretKeySelector{ - LocalObjectReference: corev1.LocalObjectReference{ - Name: "secret1", - }, - Key: "cadatakey", + want: buildv1alpha1.BuildSpec{ + Steps: []corev1.Container{{ + Name: "kubeconfig", + Image: "override-with-kubeconfig-writer:latest", + Args: []string{ + "-clusterConfig", `{"name":"cluster2","type":"cluster","url":"http://10.10.10.10","revision":"","username":"","password":"","token":"","Insecure":false,"cadata":null,"secrets":[{"fieldName":"cadata","secretKey":"cadatakey","secretName":"secret1"}]}`, + }, + Env: []corev1.EnvVar{{ + ValueFrom: &corev1.EnvVarSource{ + SecretKeyRef: &corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: "secret1", }, + Key: "cadatakey", }, - Name: "CADATA", - }}, + }, + Name: "CADATA", }}, - }, + }}, }, }} { t.Run(c.desc, func(t *testing.T) { @@ -872,8 +701,10 @@ func TestAddResourceToBuild(t *testing.T) { if (err != nil) != c.wantErr { t.Errorf("Test: %q; AddInputResource() error = %v, WantErr %v", c.desc, err, c.wantErr) } - if d := cmp.Diff(got, c.want); d != "" { - t.Errorf("Diff:\n%s", d) + if got != nil { + if d := cmp.Diff(got.Spec, c.want); d != "" { + t.Errorf("Diff:\n%s", d) + } } }) } @@ -957,17 +788,13 @@ func Test_StorageInputResource(t *testing.T) { Steps: []corev1.Container{{ Name: "create-dir-storage-gcs-keys", Image: "override-with-bash-noop:latest", - Args: []string{"-args", "mkdir -p /workspace/storage-gcs-keys"}, - VolumeMounts: []corev1.VolumeMount{{ - Name: "workspace", MountPath: "/workspace", - }}, + Args: []string{"-args", "mkdir -p /workspace/gcs-input-resource"}, }, { Name: "storage-fetch-storage-gcs-keys", Image: "override-with-gsutil-image:latest", - Args: []string{"-args", "cp -r gs://fake-bucket/rules.zip/** /workspace/storage-gcs-keys"}, - VolumeMounts: []corev1.VolumeMount{{ - Name: "volume-storage-gcs-keys-secret-name", MountPath: "/var/secret/secret-name"}, { - Name: "workspace", MountPath: "/workspace"}, + Args: []string{"-args", "cp -r gs://fake-bucket/rules.zip/** /workspace/gcs-input-resource"}, + VolumeMounts: []corev1.VolumeMount{ + {Name: "volume-storage-gcs-keys-secret-name", MountPath: "/var/secret/secret-name"}, }, Env: []corev1.EnvVar{ {Name: "GOOGLE_APPLICATION_CREDENTIALS", Value: "/var/secret/secret-name/key.json"}, diff --git a/pkg/reconciler/v1alpha1/taskrun/resources/input_resources.go b/pkg/reconciler/v1alpha1/taskrun/resources/input_resources.go index 556337bd84f..5fc2b3ce65c 100644 --- a/pkg/reconciler/v1alpha1/taskrun/resources/input_resources.go +++ b/pkg/reconciler/v1alpha1/taskrun/resources/input_resources.go @@ -30,8 +30,7 @@ import ( ) var ( - kubeconfigWriterImage = flag.String("kubeconfig-writer-image", "override-with-kubeconfig-writer:latest", "The container image containing our kubeconfig writer binary.") - bashNoopImage = flag.String("bash-noop-image", "override-with-bash-noop:latest", "The container image containing bash shell") + bashNoopImage = flag.String("bash-noop-image", "override-with-bash-noop:latest", "The container image containing bash shell") ) func getBoundResource(resourceName string, boundResources []v1alpha1.TaskResourceBinding) (*v1alpha1.TaskResourceBinding, error) { @@ -77,62 +76,52 @@ func AddInputResource( // if taskrun is fetching resource from previous task then execute copy step instead of fetching new copy // to the desired destination directory - var copyStepsFromPrevTasks []corev1.Container + var ( + copyStepsFromPrevTasks []corev1.Container + resourceContainers []corev1.Container + resourceVolumes []corev1.Volume + dPath = destinationPath(input.Name, input.TargetPath) + ) for i, path := range boundResource.Paths { - var dPath string - if input.TargetPath == "" { - dPath = filepath.Join(workspaceDir, input.Name) - } else { - dPath = filepath.Join(workspaceDir, input.TargetPath) - } + createContainer := createDirContainer(boundResource.Name, dPath, nil) cpContainer := copyContainer(fmt.Sprintf("%s-%d", boundResource.Name, i), path, dPath) cpContainer.VolumeMounts = []corev1.VolumeMount{getPvcMount(pvcName)} - copyStepsFromPrevTasks = append(copyStepsFromPrevTasks, cpContainer) + copyStepsFromPrevTasks = append(copyStepsFromPrevTasks, []corev1.Container{createContainer, cpContainer}...) mountPVC = true } - // source is copied from previous task so skip fetching cluster , storage types + // source is copied from previous task so skip fetching download container definition if len(copyStepsFromPrevTasks) > 0 { build.Spec.Steps = append(copyStepsFromPrevTasks, build.Spec.Steps...) } else { switch resource.Spec.Type { - case v1alpha1.PipelineResourceTypeGit: + case v1alpha1.PipelineResourceTypeStorage: { - gitResource, err := v1alpha1.NewGitResource(resource) + storageResource, err := v1alpha1.NewStorageResource(resource) if err != nil { - return nil, fmt.Errorf("task %q invalid git Pipeline Resource: %q; error %s", taskName, boundResource.ResourceRef.Name, err.Error()) - } - gitSourceSpec := &buildv1alpha1.GitSourceSpec{ - Url: gitResource.URL, - Revision: gitResource.Revision, + return nil, fmt.Errorf("task %q invalid gcs Pipeline Resource: %q: %s", taskName, boundResource.ResourceRef.Name, err.Error()) } - build.Spec.Sources = append(build.Spec.Sources, buildv1alpha1.SourceSpec{ - Git: gitSourceSpec, - TargetPath: input.TargetPath, - Name: input.Name, - }) - } - case v1alpha1.PipelineResourceTypeCluster: - { - clusterResource, err := v1alpha1.NewClusterResource(resource) + resourceContainers, resourceVolumes, err = addStorageFetchStep(build, storageResource, dPath) if err != nil { - return nil, fmt.Errorf("task %q invalid cluster Pipeline Resource: %q: error %s", taskName, boundResource.ResourceRef.Name, err.Error()) + return nil, fmt.Errorf("task %q invalid gcs Pipeline Resource download steps: %q: %s", taskName, boundResource.ResourceRef.Name, err.Error()) } - addClusterBuildStep(build, clusterResource) } - case v1alpha1.PipelineResourceTypeStorage: + default: { - storageResource, err := v1alpha1.NewStorageResource(resource) + resSpec, err := v1alpha1.ResourceFromType(resource) if err != nil { - return nil, fmt.Errorf("task %q invalid gcs Pipeline Resource: %q: %s", taskName, boundResource.ResourceRef.Name, err.Error()) + return nil, err } - fetchErr := addStorageFetchStep(build, storageResource, input.TargetPath) + resSpec.SetDestinationDirectory(dPath) + resourceContainers, err = resSpec.GetDownloadContainerSpec() if err != nil { - return nil, fmt.Errorf("task %q invalid gcs Pipeline Resource download steps: %q: %s", taskName, boundResource.ResourceRef.Name, fetchErr.Error()) + return nil, fmt.Errorf("task %q invalid resource download spec: %q; error %s", taskName, boundResource.ResourceRef.Name, err.Error()) } } } + build.Spec.Steps = append(resourceContainers, build.Spec.Steps...) + build.Spec.Volumes = append(build.Spec.Volumes, resourceVolumes...) } } @@ -142,88 +131,49 @@ func AddInputResource( return build, nil } -func addClusterBuildStep(build *buildv1alpha1.Build, clusterResource *v1alpha1.ClusterResource) { - var envVars []corev1.EnvVar - for _, sec := range clusterResource.Secrets { - ev := corev1.EnvVar{ - Name: strings.ToUpper(sec.FieldName), - ValueFrom: &corev1.EnvVarSource{ - SecretKeyRef: &corev1.SecretKeySelector{ - LocalObjectReference: corev1.LocalObjectReference{ - Name: sec.SecretName, - }, - Key: sec.SecretKey, - }, - }, - } - envVars = append(envVars, ev) - } - - clusterContainer := corev1.Container{ - Name: "kubeconfig", - Image: *kubeconfigWriterImage, - Args: []string{ - "-clusterConfig", clusterResource.String(), - }, - Env: envVars, - } - - buildSteps := append([]corev1.Container{clusterContainer}, build.Spec.Steps...) - build.Spec.Steps = buildSteps -} - -func addStorageFetchStep(build *buildv1alpha1.Build, storageResource v1alpha1.PipelineStorageResourceInterface, destPath string) error { - var destDirectory = filepath.Join(workspaceDir, storageResource.GetName()) - if destPath != "" { - destDirectory = filepath.Join(workspaceDir, destPath) - } - - storageResource.SetDestinationDirectory(destDirectory) - gcsCreateDirContainer := createDirContainer(storageResource.GetName(), destDirectory) +func addStorageFetchStep(build *buildv1alpha1.Build, storageResource v1alpha1.PipelineStorageResourceInterface, destPath string) ([]corev1.Container, []corev1.Volume, error) { + storageResource.SetDestinationDirectory(destPath) gcsDownloadContainers, err := storageResource.GetDownloadContainerSpec() if err != nil { - return err + return nil, nil, err } + gcsContainers := append([]corev1.Container{createDirContainer(storageResource.GetName(), destPath, nil)}, + gcsDownloadContainers...) + + var buildVol, storageVol []corev1.Volume mountedSecrets := map[string]string{} for _, volume := range build.Spec.Volumes { mountedSecrets[volume.Name] = "" + buildVol = append(buildVol, volume) } - var buildSteps []corev1.Container - gcsContainers := append([]corev1.Container{gcsCreateDirContainer}, gcsDownloadContainers...) - for _, gcsContainer := range gcsContainers { - - gcsContainer.VolumeMounts = append(gcsContainer.VolumeMounts, corev1.VolumeMount{ - Name: "workspace", - MountPath: workspaceDir, - }) - 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, - }, + + 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 { - build.Spec.Volumes = append(build.Spec.Volumes, gcsSecretVolume) - mountedSecrets[volName] = "" - } + if _, ok := mountedSecrets[volName]; !ok { + buildVol = append(buildVol, gcsSecretVolume) + storageVol = append(storageVol, gcsSecretVolume) + mountedSecrets[volName] = "" } - buildSteps = append(buildSteps, gcsContainer) } - build.Spec.Steps = append(buildSteps, build.Spec.Steps...) - return nil + return gcsContainers, storageVol, nil } -func createDirContainer(name, destinationPath string) corev1.Container { +func createDirContainer(name, destinationPath string, volMount []corev1.VolumeMount) corev1.Container { return corev1.Container{ - Name: fmt.Sprintf("create-dir-%s", name), - Image: *bashNoopImage, - Args: []string{"-args", strings.Join([]string{"mkdir", "-p", destinationPath}, " ")}, + Name: fmt.Sprintf("create-dir-%s", name), + Image: *bashNoopImage, + Args: []string{"-args", strings.Join([]string{"mkdir", "-p", destinationPath}, " ")}, + VolumeMounts: volMount, } } @@ -234,3 +184,10 @@ func copyContainer(name, sourcePath, destinationPath string) corev1.Container { Args: []string{"-args", strings.Join([]string{"cp", "-r", fmt.Sprintf("%s/.", sourcePath), destinationPath}, " ")}, } } + +func destinationPath(name, path string) string { + if path == "" { + return filepath.Join(workspaceDir, name) + } + return filepath.Join(workspaceDir, path) +} diff --git a/pkg/reconciler/v1alpha1/taskrun/resources/output_resource.go b/pkg/reconciler/v1alpha1/taskrun/resources/output_resource.go index fcfb3ba6f4f..96180cd4958 100644 --- a/pkg/reconciler/v1alpha1/taskrun/resources/output_resource.go +++ b/pkg/reconciler/v1alpha1/taskrun/resources/output_resource.go @@ -19,7 +19,6 @@ package resources import ( "fmt" "path/filepath" - "strings" "github.com/knative/build-pipeline/pkg/apis/pipeline/v1alpha1" listers "github.com/knative/build-pipeline/pkg/client/listers/pipeline/v1alpha1" @@ -71,11 +70,7 @@ func AddOutputResources( if taskSpec.Inputs != nil { for _, input := range taskSpec.Inputs.Resources { - var targetPath = filepath.Join(workspaceDir, input.Name) - if input.TargetPath != "" { - targetPath = filepath.Join(workspaceDir, input.TargetPath) - } - inputResourceMap[input.Name] = targetPath + inputResourceMap[input.Name] = destinationPath(input.Name, input.TargetPath) } } @@ -89,55 +84,54 @@ func AddOutputResources( if err != nil { return fmt.Errorf("Failed to get output pipeline Resource for task %q: %q", boundResource.ResourceRef.Name, taskName) } - + var ( + resourceContainers []corev1.Container + resourceVolumes []corev1.Volume + ) // if resource is declared in input then copy outputs to pvc // To build copy step it needs source path(which is targetpath of input resourcemap) from task input source sourcePath := inputResourceMap[boundResource.Name] if sourcePath == "" { sourcePath = filepath.Join(outputDir, boundResource.Name) } + switch resource.Spec.Type { case v1alpha1.PipelineResourceTypeStorage: - storageResource, err := v1alpha1.NewStorageResource(resource) - if err != nil { - return fmt.Errorf("task %q invalid storage Pipeline Resource: %q", - taskName, - boundResource.ResourceRef.Name, - ) + { + storageResource, err := v1alpha1.NewStorageResource(resource) + if err != nil { + return fmt.Errorf("task %q invalid storage Pipeline Resource: %q", + taskName, + boundResource.ResourceRef.Name, + ) + } + resourceContainers, resourceVolumes, err = addStoreUploadStep(b, storageResource, sourcePath) + if err != nil { + return fmt.Errorf("task %q invalid Pipeline Resource: %q; invalid upload steps err: %v", + taskName, boundResource.ResourceRef.Name, err) + } + resourceContainers = append(resourceContainers, + copyToPVCContainers(boundResource.Paths, resource.GetName(), pipelineRunpvcName, sourcePath)...) } - err = addStoreUploadStep(b, storageResource, sourcePath) - if err != nil { - return fmt.Errorf("task %q invalid Pipeline Resource: %q; invalid upload steps err: %v", - taskName, boundResource.ResourceRef.Name, err) + case v1alpha1.PipelineResourceTypeGit: + { + resourceContainers = append(resourceContainers, + copyToPVCContainers(boundResource.Paths, resource.GetName(), pipelineRunpvcName, sourcePath)...) } - } - - // Workaround for issue #401. Unless all resource types are implemented as - // outputs, or until we have metadata on the resource that declares whether - // the output should be copied to the PVC, only copy git and storage output - // resources. - // copy to pvc if pvc is present - if allowedOutputResources[resource.Spec.Type] && pipelineRunpvcName != "" { - var newSteps []corev1.Container - for _, dPath := range boundResource.Paths { - newSteps = append(newSteps, []corev1.Container{{ - Name: fmt.Sprintf("source-mkdir-%s", resource.GetName()), - Image: *bashNoopImage, - Args: []string{ - "-args", strings.Join([]string{"mkdir", "-p", dPath}, " "), - }, - VolumeMounts: []corev1.VolumeMount{getPvcMount(pipelineRunpvcName)}, - }, { - Name: fmt.Sprintf("source-copy-%s", resource.GetName()), - Image: *bashNoopImage, - Args: []string{ - "-args", strings.Join([]string{"cp", "-r", fmt.Sprintf("%s/.", sourcePath), dPath}, " "), - }, - VolumeMounts: []corev1.VolumeMount{getPvcMount(pipelineRunpvcName)}, - }}...) + default: + { + resSpec, err := v1alpha1.ResourceFromType(resource) + if err != nil { + return err + } + resourceContainers, err = resSpec.GetUploadContainerSpec() + if err != nil { + return fmt.Errorf("task %q invalid download spec: %q; error %s", taskName, boundResource.ResourceRef.Name, err.Error()) + } } - b.Spec.Steps = append(b.Spec.Steps, newSteps...) } + b.Spec.Steps = append(b.Spec.Steps, resourceContainers...) + b.Spec.Volumes = append(b.Spec.Volumes, resourceVolumes...) } if pipelineRunpvcName == "" { @@ -157,50 +151,56 @@ func AddOutputResources( func addStoreUploadStep(build *buildv1alpha1.Build, storageResource v1alpha1.PipelineStorageResourceInterface, sourcePath string, -) error { +) ([]corev1.Container, []corev1.Volume, error) { + storageResource.SetDestinationDirectory(sourcePath) gcsContainers, err := storageResource.GetUploadContainerSpec() if err != nil { - return err + return nil, nil, err } + var totalBuildVol, storageVol []corev1.Volume mountedSecrets := map[string]string{} for _, volume := range build.Spec.Volumes { mountedSecrets[volume.Name] = "" + totalBuildVol = append(totalBuildVol, volume) } - var buildSteps []corev1.Container - for _, gcsContainer := range gcsContainers { - gcsContainer.VolumeMounts = append(gcsContainer.VolumeMounts, corev1.VolumeMount{ - Name: "workspace", - MountPath: workspaceDir, - }) - // Map holds list of secrets that are mounted as volumes - 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, - }, + + // Map holds list of secrets that are mounted as volumes + 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 { - build.Spec.Volumes = append(build.Spec.Volumes, gcsSecretVolume) - mountedSecrets[volName] = "" - } + if _, ok := mountedSecrets[volName]; !ok { + totalBuildVol = append(totalBuildVol, gcsSecretVolume) + storageVol = append(storageVol, gcsSecretVolume) + mountedSecrets[volName] = "" } - buildSteps = append(buildSteps, gcsContainer) } - build.Spec.Steps = append(build.Spec.Steps, buildSteps...) - return nil -} -// allowedOutputResource checks if an output resource type produces -// an output that should be copied to the PVC -func allowedOutputResource(resourceType v1alpha1.PipelineResourceType) bool { + return gcsContainers, storageVol, nil +} - return allowedOutputResources[resourceType] +// copy to pvc if pvc is present +func copyToPVCContainers(paths []string, name, pvcName, sourcePath string) []corev1.Container { + var newSteps []corev1.Container + + pvcVolumeMout := []corev1.VolumeMount{getPvcMount(pvcName)} + if pvcName != "" { + for _, dPath := range paths { + createCont := createDirContainer(name, dPath, pvcVolumeMout) + copyCont := copyContainer(name, sourcePath, dPath) + copyCont.VolumeMounts = pvcVolumeMout + newSteps = append(newSteps, []corev1.Container{createCont, copyCont}...) + } + } + return newSteps } diff --git a/pkg/reconciler/v1alpha1/taskrun/resources/output_resource_test.go b/pkg/reconciler/v1alpha1/taskrun/resources/output_resource_test.go index ec4ba10757c..a7108a43563 100644 --- a/pkg/reconciler/v1alpha1/taskrun/resources/output_resource_test.go +++ b/pkg/reconciler/v1alpha1/taskrun/resources/output_resource_test.go @@ -162,7 +162,7 @@ func Test_Valid_OutputResources(t *testing.T) { }, }, wantSteps: []corev1.Container{{ - Name: "source-mkdir-source-git", + Name: "create-dir-source-git", Image: "override-with-bash-noop:latest", Args: []string{"-args", "mkdir -p pipeline-task-name"}, VolumeMounts: []corev1.VolumeMount{{ @@ -218,7 +218,7 @@ func Test_Valid_OutputResources(t *testing.T) { }, }, wantSteps: []corev1.Container{{ - Name: "source-mkdir-source-git", + Name: "create-dir-source-git", Image: "override-with-bash-noop:latest", Args: []string{"-args", "mkdir -p pipeline-task-name"}, VolumeMounts: []corev1.VolumeMount{{ @@ -328,16 +328,13 @@ func Test_Valid_OutputResources(t *testing.T) { VolumeMounts: []corev1.VolumeMount{{ Name: "volume-source-gcs-sname", MountPath: "/var/secret/sname", - }, { - Name: "workspace", - MountPath: "/workspace", }}, Args: []string{"-args", "cp -r /workspace/faraway-disk/* gs://some-bucket"}, Env: []corev1.EnvVar{{ Name: "GOOGLE_APPLICATION_CREDENTIALS", Value: "/var/secret/sname/key.json", }}, }, { - Name: "source-mkdir-source-gcs", + Name: "create-dir-source-gcs", Image: "override-with-bash-noop:latest", Args: []string{"-args", "mkdir -p pipeline-task-path"}, VolumeMounts: []corev1.VolumeMount{{Name: "pipelinerun-pvc", MountPath: "/pvc"}}, @@ -397,15 +394,13 @@ func Test_Valid_OutputResources(t *testing.T) { Image: "override-with-gsutil-image:latest", VolumeMounts: []corev1.VolumeMount{{ Name: "volume-source-gcs-sname", MountPath: "/var/secret/sname", - }, { - Name: "workspace", MountPath: "/workspace", }}, Env: []corev1.EnvVar{{ Name: "GOOGLE_APPLICATION_CREDENTIALS", Value: "/var/secret/sname/key.json", }}, Args: []string{"-args", "cp -r /workspace/output/source-workspace/* gs://some-bucket"}, }, { - Name: "source-mkdir-source-gcs", + Name: "create-dir-source-gcs", Image: "override-with-bash-noop:latest", Args: []string{"-args", "mkdir -p pipeline-task-path"}, VolumeMounts: []corev1.VolumeMount{{Name: "pipelinerun-pvc", MountPath: "/pvc"}}, @@ -461,8 +456,6 @@ func Test_Valid_OutputResources(t *testing.T) { Image: "override-with-gsutil-image:latest", VolumeMounts: []corev1.VolumeMount{{ Name: "volume-source-gcs-sname", MountPath: "/var/secret/sname", - }, { - Name: "workspace", MountPath: "/workspace", }}, Env: []corev1.EnvVar{{ Name: "GOOGLE_APPLICATION_CREDENTIALS", Value: "/var/secret/sname/key.json", @@ -514,8 +507,6 @@ func Test_Valid_OutputResources(t *testing.T) { Image: "override-with-gsutil-image:latest", VolumeMounts: []corev1.VolumeMount{{ Name: "volume-source-gcs-sname", MountPath: "/var/secret/sname", - }, { - Name: "workspace", MountPath: "/workspace", }}, Env: []corev1.EnvVar{{ Name: "GOOGLE_APPLICATION_CREDENTIALS", Value: "/var/secret/sname/key.json", @@ -596,6 +587,40 @@ func Test_Valid_OutputResources(t *testing.T) { }, wantSteps: nil, build: build(), + }, { + desc: "image output resource with no steps", + taskRun: &v1alpha1.TaskRun{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-taskrun-run-output-steps", + Namespace: "marshmallow", + }, + Spec: v1alpha1.TaskRunSpec{ + Outputs: v1alpha1.TaskRunOutputs{ + Resources: []v1alpha1.TaskResourceBinding{{ + Name: "source-workspace", + ResourceRef: v1alpha1.PipelineResourceRef{ + Name: "source-image", + }, + }}, + }, + }, + }, + task: &v1alpha1.Task{ + ObjectMeta: metav1.ObjectMeta{ + Name: "task1", + Namespace: "marshmallow", + }, + Spec: v1alpha1.TaskSpec{ + Outputs: &v1alpha1.Outputs{ + Resources: []v1alpha1.TaskResource{{ + Name: "source-workspace", + Type: "image", + }}, + }, + }, + }, + wantSteps: nil, + build: build(), }} { t.Run(c.name, func(t *testing.T) { outputResourcesetUp() @@ -758,33 +783,7 @@ func Test_InValid_OutputResources(t *testing.T) { outputResourcesetUp() err := AddOutputResources(build(), c.task.Name, &c.task.Spec, c.taskRun, outputpipelineResourceLister, logger) if (err != nil) != c.wantErr { - t.Fatalf("Test AddOutputResourceSteps error %v ", c.desc) - } - }) - } -} - -func Test_AllowedOutputResource(t *testing.T) { - for _, c := range []struct { - desc string - resourceType v1alpha1.PipelineResourceType - expectedAllowed bool - }{{ - desc: "storage type is allowed", - resourceType: v1alpha1.PipelineResourceTypeStorage, - expectedAllowed: true, - }, { - desc: "git type is allowed", - resourceType: v1alpha1.PipelineResourceTypeGit, - expectedAllowed: true, - }, { - desc: "anything else is not allowed", - resourceType: "fooType", - expectedAllowed: false, - }} { - t.Run(c.desc, func(t *testing.T) { - if c.expectedAllowed != allowedOutputResources[c.resourceType] { - t.Fatalf("Test AllowedOutputResource %s expected %t but got %t", c.desc, c.expectedAllowed, allowedOutputResources[c.resourceType]) + t.Fatalf("Test AddOutputResourceSteps %v : error%v", c.desc, err) } }) } diff --git a/pkg/reconciler/v1alpha1/taskrun/resources/pod.go b/pkg/reconciler/v1alpha1/taskrun/resources/pod.go index 5ec208cab51..782ccd5c844 100644 --- a/pkg/reconciler/v1alpha1/taskrun/resources/pod.go +++ b/pkg/reconciler/v1alpha1/taskrun/resources/pod.go @@ -93,9 +93,6 @@ var ( // The container used to initialize credentials before the build runs. credsImage = flag.String("creds-image", "override-with-creds:latest", "The container image for preparing our Build's credentials.") - // The container with Git that we use to implement the Git source step. - gitImage = flag.String("git-image", "override-with-git:latest", - "The container image containing our Git binary.") // The container that just prints build successful. nopImage = flag.String("nop-image", "override-with-nop:latest", "The container image run at the end of the build to log build success") @@ -103,45 +100,6 @@ var ( "The container image containing our GCS fetcher binary.") ) -// TODO(mattmoor): Should we move this somewhere common, because of the flag? -func gitToContainer(source v1alpha1.SourceSpec, index int) (*corev1.Container, error) { - git := source.Git - if git.Url == "" { - return nil, apis.ErrMissingField("b.spec.source.git.url") - } - if git.Revision == "" { - return nil, apis.ErrMissingField("b.spec.source.git.revision") - } - - args := []string{"-url", git.Url, - "-revision", git.Revision, - } - - if source.TargetPath != "" { - args = append(args, []string{"-path", source.TargetPath}...) - } else { - args = append(args, []string{"-path", source.Name}...) - } - - containerName := initContainerPrefix + gitSource + "-" - - // update container name to suffix source name - if source.Name != "" { - containerName = containerName + source.Name - } else { - containerName = containerName + strconv.Itoa(index) - } - - return &corev1.Container{ - Name: containerName, - Image: *gitImage, - Args: args, - VolumeMounts: implicitVolumeMounts, - WorkingDir: workspaceDir, - Env: implicitEnvVars, - }, nil -} - func gcsToContainer(source v1alpha1.SourceSpec, index int) (*corev1.Container, error) { gcs := source.GCS if gcs.Location == "" { @@ -276,32 +234,16 @@ func MakePod(build *v1alpha1.Build, kubeclient kubernetes.Interface) (*corev1.Po for _, source := range build.Spec.Sources { sources = append(sources, source) } - workspaceSubPath := "" for i, source := range sources { switch { - case source.Git != nil: - git, err := gitToContainer(source, i) - if err != nil { - return nil, err - } - initContainers = append(initContainers, *git) case source.GCS != nil: gcs, err := gcsToContainer(source, i) if err != nil { return nil, err } initContainers = append(initContainers, *gcs) - case source.Custom != nil: - cust, err := customToContainer(source.Custom, source.Name) - if err != nil { - return nil, err - } - // Prepend the custom container to the steps, to be augmented later with env, volume mounts, etc. - build.Spec.Steps = append([]corev1.Container{*cust}, build.Spec.Steps...) } - // webhook validation checks that only one source has subPath defined - workspaceSubPath = source.SubPath } for i, step := range build.Spec.Steps { @@ -316,12 +258,6 @@ func MakePod(build *v1alpha1.Build, kubeclient kubernetes.Interface) (*corev1.Po } for _, imp := range implicitVolumeMounts { if !requestedVolumeMounts[filepath.Clean(imp.MountPath)] { - // If the build's source specifies a subpath, - // use that in the implicit workspace volume - // mount. - if workspaceSubPath != "" && imp.Name == "workspace" { - imp.SubPath = workspaceSubPath - } step.VolumeMounts = append(step.VolumeMounts, imp) } } diff --git a/pkg/reconciler/v1alpha1/taskrun/resources/pod_test.go b/pkg/reconciler/v1alpha1/taskrun/resources/pod_test.go index cadff3ca5ae..eedc2dd0a41 100644 --- a/pkg/reconciler/v1alpha1/taskrun/resources/pod_test.go +++ b/pkg/reconciler/v1alpha1/taskrun/resources/pod_test.go @@ -107,246 +107,6 @@ func TestMakePod(t *testing.T) { Containers: []corev1.Container{nopContainer}, Volumes: implicitVolumes, }, - }, { - desc: "source", - b: v1alpha1.BuildSpec{ - Source: &v1alpha1.SourceSpec{ - Name: "myrepo", - Git: &v1alpha1.GitSourceSpec{ - Url: "github.com/my/repo", - Revision: "master", - }, - }, - Steps: []corev1.Container{{ - Name: "name", - Image: "image", - }}, - }, - want: &corev1.PodSpec{ - RestartPolicy: corev1.RestartPolicyNever, - InitContainers: []corev1.Container{{ - Name: initContainerPrefix + credsInit, - Image: *credsImage, - Args: []string{}, - Env: implicitEnvVars, - VolumeMounts: implicitVolumeMounts, - WorkingDir: workspaceDir, - }, { - Name: initContainerPrefix + gitSource + "-myrepo", - Image: *gitImage, - Args: []string{"-url", "github.com/my/repo", "-revision", "master", "-path", "myrepo"}, - Env: implicitEnvVars, - VolumeMounts: implicitVolumeMounts, - WorkingDir: workspaceDir, - }, { - Name: "build-step-name", - Image: "image", - Env: implicitEnvVars, - VolumeMounts: implicitVolumeMounts, - WorkingDir: workspaceDir, - }}, - Containers: []corev1.Container{nopContainer}, - Volumes: implicitVolumes, - }, - }, { - desc: "sources", - b: v1alpha1.BuildSpec{ - Sources: []v1alpha1.SourceSpec{{ - Git: &v1alpha1.GitSourceSpec{ - Url: "github.com/my/repo", - Revision: "master", - }, - Name: "repo1", - }, { - Git: &v1alpha1.GitSourceSpec{ - Url: "github.com/my/repo", - Revision: "master", - }, - Name: "repo2", - }}, - Steps: []corev1.Container{{ - Name: "name", - Image: "image", - }}, - }, - want: &corev1.PodSpec{ - RestartPolicy: corev1.RestartPolicyNever, - InitContainers: []corev1.Container{{ - Name: initContainerPrefix + credsInit, - Image: *credsImage, - Args: []string{}, - Env: implicitEnvVars, - VolumeMounts: implicitVolumeMounts, - WorkingDir: workspaceDir, - }, { - Name: initContainerPrefix + gitSource + "-" + "repo1", - Image: *gitImage, - Args: []string{"-url", "github.com/my/repo", - "-revision", "master", - "-path", "repo1", - }, - Env: implicitEnvVars, - VolumeMounts: implicitVolumeMounts, - WorkingDir: workspaceDir, - }, { - Name: initContainerPrefix + gitSource + "-" + "repo2", - Image: *gitImage, - Args: []string{"-url", "github.com/my/repo", - "-revision", "master", - "-path", "repo2", - }, - Env: implicitEnvVars, - VolumeMounts: implicitVolumeMounts, - WorkingDir: workspaceDir, - }, { - Name: "build-step-name", - Image: "image", - Env: implicitEnvVars, - VolumeMounts: implicitVolumeMounts, - WorkingDir: workspaceDir, - }}, - Containers: []corev1.Container{nopContainer}, - Volumes: implicitVolumes, - }, - }, { - desc: "git-source-with-subpath", - b: v1alpha1.BuildSpec{ - Source: &v1alpha1.SourceSpec{ - Name: "myrepo", - Git: &v1alpha1.GitSourceSpec{ - Url: "github.com/my/repo", - Revision: "master", - }, - SubPath: subPath, - }, - Steps: []corev1.Container{{ - Name: "name", - Image: "image", - }}, - }, - want: &corev1.PodSpec{ - RestartPolicy: corev1.RestartPolicyNever, - InitContainers: []corev1.Container{{ - Name: initContainerPrefix + credsInit, - Image: *credsImage, - Args: []string{}, - Env: implicitEnvVars, - VolumeMounts: implicitVolumeMounts, // without subpath - WorkingDir: workspaceDir, - }, { - Name: initContainerPrefix + gitSource + "-myrepo", - Image: *gitImage, - Args: []string{"-url", "github.com/my/repo", "-revision", "master", "-path", "myrepo"}, - Env: implicitEnvVars, - VolumeMounts: implicitVolumeMounts, // without subpath - WorkingDir: workspaceDir, - }, { - Name: "build-step-name", - Image: "image", - Env: implicitEnvVars, - VolumeMounts: implicitVolumeMountsWithSubPath, - WorkingDir: workspaceDir, - }}, - Containers: []corev1.Container{nopContainer}, - Volumes: implicitVolumes, - }, - }, { - desc: "git-sources-with-subpath", - b: v1alpha1.BuildSpec{ - Sources: []v1alpha1.SourceSpec{{ - Name: "myrepo", - Git: &v1alpha1.GitSourceSpec{ - Url: "github.com/my/repo", - Revision: "master", - }, - SubPath: subPath, - }, { - Name: "ownrepo", - Git: &v1alpha1.GitSourceSpec{ - Url: "github.com/own/repo", - Revision: "master", - }, - SubPath: subPath, - }}, - Steps: []corev1.Container{{ - Name: "name", - Image: "image", - }}, - }, - want: &corev1.PodSpec{ - RestartPolicy: corev1.RestartPolicyNever, - InitContainers: []corev1.Container{{ - Name: initContainerPrefix + credsInit, - Image: *credsImage, - Args: []string{}, - Env: implicitEnvVars, - VolumeMounts: implicitVolumeMounts, // without subpath - WorkingDir: workspaceDir, - }, { - Name: initContainerPrefix + gitSource + "-myrepo", - Image: *gitImage, - Args: []string{"-url", "github.com/my/repo", "-revision", "master", "-path", "myrepo"}, - Env: implicitEnvVars, - VolumeMounts: implicitVolumeMounts, // without subpath - WorkingDir: workspaceDir, - }, { - Name: initContainerPrefix + gitSource + "-" + "ownrepo", - Image: *gitImage, - Args: []string{"-url", "github.com/own/repo", "-revision", "master", "-path", "ownrepo"}, - Env: implicitEnvVars, - VolumeMounts: implicitVolumeMounts, // without subpath - WorkingDir: workspaceDir, - }, { - Name: "build-step-name", - Image: "image", - Env: implicitEnvVars, - VolumeMounts: implicitVolumeMountsWithSubPath, - WorkingDir: workspaceDir, - }}, - Containers: []corev1.Container{nopContainer}, - Volumes: implicitVolumes, - }, - }, { - desc: "gcs-source-with-subpath", - b: v1alpha1.BuildSpec{ - Source: &v1alpha1.SourceSpec{ - GCS: &v1alpha1.GCSSourceSpec{ - Type: v1alpha1.GCSManifest, - Location: "gs://foo/bar", - }, - SubPath: subPath, - }, - Steps: []corev1.Container{{ - Name: "name", - Image: "image", - }}, - }, - want: &corev1.PodSpec{ - RestartPolicy: corev1.RestartPolicyNever, - InitContainers: []corev1.Container{{ - Name: initContainerPrefix + credsInit, - Image: *credsImage, - Args: []string{}, - Env: implicitEnvVars, - VolumeMounts: implicitVolumeMounts, // without subpath - WorkingDir: workspaceDir, - }, { - Name: initContainerPrefix + gcsSource + "-0", - Image: *gcsFetcherImage, - Args: []string{"--type", "Manifest", "--location", "gs://foo/bar", "--dest_dir", "/workspace"}, - Env: implicitEnvVars, - VolumeMounts: implicitVolumeMounts, // without subpath - WorkingDir: workspaceDir, - }, { - Name: "build-step-name", - Image: "image", - Env: implicitEnvVars, - VolumeMounts: implicitVolumeMountsWithSubPath, - WorkingDir: workspaceDir, - }}, - Containers: []corev1.Container{nopContainer}, - Volumes: implicitVolumes, - }, }, { desc: "gcs-source-with-targetPath", b: v1alpha1.BuildSpec{ @@ -379,45 +139,6 @@ func TestMakePod(t *testing.T) { Containers: []corev1.Container{nopContainer}, Volumes: implicitVolumes, }, - }, { - desc: "custom-source-with-subpath", - b: v1alpha1.BuildSpec{ - Source: &v1alpha1.SourceSpec{ - Custom: &corev1.Container{ - Image: "image", - }, - SubPath: subPath, - }, - Steps: []corev1.Container{{ - Name: "name", - Image: "image", - }}, - }, - want: &corev1.PodSpec{ - RestartPolicy: corev1.RestartPolicyNever, - InitContainers: []corev1.Container{{ - Name: initContainerPrefix + credsInit, - Image: *credsImage, - Args: []string{}, - Env: implicitEnvVars, - VolumeMounts: implicitVolumeMounts, // without subpath - WorkingDir: workspaceDir, - }, { - Name: initContainerPrefix + customSource, - Image: "image", - Env: implicitEnvVars, - VolumeMounts: implicitVolumeMountsWithSubPath, // *with* subpath - WorkingDir: workspaceDir, - }, { - Name: "build-step-name", - Image: "image", - Env: implicitEnvVars, - VolumeMounts: implicitVolumeMountsWithSubPath, - WorkingDir: workspaceDir, - }}, - Containers: []corev1.Container{nopContainer}, - Volumes: implicitVolumes, - }, }, { desc: "with-service-account", b: v1alpha1.BuildSpec{ diff --git a/pkg/reconciler/v1alpha1/taskrun/taskrun_test.go b/pkg/reconciler/v1alpha1/taskrun/taskrun_test.go index 2ea8096bb62..b588031ee78 100644 --- a/pkg/reconciler/v1alpha1/taskrun/taskrun_test.go +++ b/pkg/reconciler/v1alpha1/taskrun/taskrun_test.go @@ -55,7 +55,11 @@ var ( Name: toolsMountName, MountPath: "/tools", } - + workspaceDir = "/workspace" + implicitBuilderVolumeMounts = corev1.VolumeMount{ + Name: "home", + MountPath: "/builder/home", + } entrypointCopyStep = tb.BuildStep("place-tools", config.DefaultEntrypointImage, tb.Command("/bin/cp"), tb.Args("/entrypoint", entrypointLocation), @@ -264,7 +268,14 @@ func TestReconcile(t *testing.T) { name: "params", taskRun: taskRunTemplating, wantBuildSpec: tb.BuildSpec( - tb.BuildSource("workspace", tb.BuildSourceGit("https://foo.git", "master")), + tb.BuildStep("git-source-git-resource", "override-with-git:latest", + tb.Args("-url", "https://foo.git", "-revision", "master", "-path", "/workspace/workspace"), + tb.VolumeMount(corev1.VolumeMount{ + Name: "workspace", + MountPath: workspaceDir, + }), + tb.VolumeMount(implicitBuilderVolumeMounts), + ), entrypointCopyStep, tb.BuildStep("mycontainer", "myimage", tb.Command(entrypointLocation), tb.EnvVar("ENTRYPOINT_OPTIONS", `{"args":["/mycmd","--my-arg=foo","--my-arg-with-default=bar","--my-arg-with-default2=thedefault","--my-additional-arg=gcr.io/kristoff/sven"],"process_log":"/tools/process-log.txt","marker_file":"/tools/marker-file.txt"}`), @@ -280,10 +291,16 @@ func TestReconcile(t *testing.T) { name: "wrap-steps", taskRun: taskRunInputOutput, wantBuildSpec: tb.BuildSpec( + tb.BuildStep("create-dir-another-git-resource", "override-with-bash-noop:latest", + tb.Args("-args", "mkdir -p /workspace/another-git-resource"), + ), tb.BuildStep("source-copy-another-git-resource-0", "override-with-bash-noop:latest", tb.Args("-args", "cp -r source-folder/. /workspace/another-git-resource"), tb.VolumeMount(corev1.VolumeMount{Name: "test-pvc", MountPath: "/pvc"}), ), + tb.BuildStep("create-dir-git-resource", "override-with-bash-noop:latest", + tb.Args("-args", "mkdir -p /workspace/git-resource"), + ), tb.BuildStep("source-copy-git-resource-0", "override-with-bash-noop:latest", tb.Args("-args", "cp -r source-folder/. /workspace/git-resource"), tb.VolumeMount(corev1.VolumeMount{Name: "test-pvc", MountPath: "/pvc"}), @@ -292,7 +309,7 @@ func TestReconcile(t *testing.T) { tb.BuildStep("simple-step", "foo", tb.Command(entrypointLocation), entrypointOptionEnvVar, tb.VolumeMount(toolsMount), ), - tb.BuildStep("source-mkdir-git-resource", "override-with-bash-noop:latest", + tb.BuildStep("create-dir-git-resource", "override-with-bash-noop:latest", tb.Args("-args", "mkdir -p output-folder"), tb.VolumeMount(corev1.VolumeMount{Name: "test-pvc", MountPath: "/pvc"}), ), @@ -307,7 +324,14 @@ func TestReconcile(t *testing.T) { name: "taskrun-with-taskspec", taskRun: taskRunWithTaskSpec, wantBuildSpec: tb.BuildSpec( - tb.BuildSource("workspace", tb.BuildSourceGit("https://foo.git", "master")), + tb.BuildStep("git-source-git-resource", "override-with-git:latest", + tb.Args("-url", "https://foo.git", "-revision", "master", "-path", "/workspace/workspace"), + tb.VolumeMount(corev1.VolumeMount{ + Name: "workspace", + MountPath: workspaceDir, + }), + tb.VolumeMount(implicitBuilderVolumeMounts), + ), entrypointCopyStep, tb.BuildStep("mycontainer", "myimage", tb.Command(entrypointLocation), tb.EnvVar("ENTRYPOINT_OPTIONS", `{"args":["/mycmd","--my-arg=foo"],"process_log":"/tools/process-log.txt","marker_file":"/tools/marker-file.txt"}`),