From dce7b2f131993f1b6386608915b61a4722408826 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 ++ .../v1alpha1/cluster_resource_test.go | 49 ++ pkg/apis/pipeline/v1alpha1/gcs_resource.go | 15 +- .../pipeline/v1alpha1/gcs_resource_test.go | 50 +- 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 | 427 ++++++------------ .../taskrun/resources/input_resources.go | 159 +++---- .../taskrun/resources/output_resource.go | 124 ++--- .../taskrun/resources/output_resource_test.go | 77 ++-- .../v1alpha1/taskrun/resources/pod.go | 64 --- .../v1alpha1/taskrun/resources/pod_test.go | 279 ------------ .../v1alpha1/taskrun/taskrun_test.go | 41 +- 16 files changed, 512 insertions(+), 899 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/cluster_resource_test.go b/pkg/apis/pipeline/v1alpha1/cluster_resource_test.go index a2b03da1f36..e0db2e0d0b4 100644 --- a/pkg/apis/pipeline/v1alpha1/cluster_resource_test.go +++ b/pkg/apis/pipeline/v1alpha1/cluster_resource_test.go @@ -17,6 +17,7 @@ import ( "testing" "github.com/google/go-cmp/cmp" + corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -163,3 +164,51 @@ func TestNewClusterResource(t *testing.T) { }) } } + +func Test_ClusterResource_GetDownloadContainerSpec(t *testing.T) { + testcases := []struct { + name string + clusterResource *ClusterResource + wantContainers []corev1.Container + wantErr bool + }{{ + name: "valid cluster resource config", + clusterResource: &ClusterResource{ + Name: "test-cluster-resource", + Type: PipelineResourceTypeCluster, + URL: "http://10.10.10.10", + Secrets: []SecretParam{{ + FieldName: "cadata", + SecretKey: "cadatakey", + SecretName: "secret1", + }}, + }, + wantContainers: []corev1.Container{{ + Name: "kubeconfig", + Image: "override-with-kubeconfig-writer:latest", + Args: []string{"-clusterConfig", `{"name":"test-cluster-resource","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{{ + Name: "CADATA", + ValueFrom: &corev1.EnvVarSource{ + SecretKeyRef: &corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: "secret1", + }, + Key: "cadatakey", + }, + }, + }}, + }}, + }} + for _, tc := range testcases { + t.Run(tc.name, func(t *testing.T) { + gotContainers, err := tc.clusterResource.GetDownloadContainerSpec() + if tc.wantErr && err == nil { + t.Fatalf("Expected error to be %t but got %v:", tc.wantErr, err) + } + if d := cmp.Diff(gotContainers, tc.wantContainers); d != "" { + t.Errorf("Error mismatch between download containers spec: %s", d) + } + }) + } +} diff --git a/pkg/apis/pipeline/v1alpha1/gcs_resource.go b/pkg/apis/pipeline/v1alpha1/gcs_resource.go index 6c405462308..3abafaed1dd 100644 --- a/pkg/apis/pipeline/v1alpha1/gcs_resource.go +++ b/pkg/apis/pipeline/v1alpha1/gcs_resource.go @@ -139,11 +139,12 @@ func (s *GCSResource) GetDownloadContainerSpec() ([]corev1.Container, error) { } envVars, secretVolumeMount := getSecretEnvVarsAndVolumeMounts(s.Name, gcsSecretVolumeMountPath, s.Secrets) - return []corev1.Container{{ - Name: fmt.Sprintf("storage-fetch-%s", s.Name), - Image: *gsutilImage, - Args: args, - Env: envVars, - VolumeMounts: secretVolumeMount, - }}, nil + return []corev1.Container{ + CreateDirContainer(s.Name, s.DestinationDir), { + Name: fmt.Sprintf("storage-fetch-%s", s.Name), + Image: *gsutilImage, + Args: args, + Env: envVars, + VolumeMounts: secretVolumeMount, + }}, nil } diff --git a/pkg/apis/pipeline/v1alpha1/gcs_resource_test.go b/pkg/apis/pipeline/v1alpha1/gcs_resource_test.go index 10a28f2fb94..ef7f010a0ef 100644 --- a/pkg/apis/pipeline/v1alpha1/gcs_resource_test.go +++ b/pkg/apis/pipeline/v1alpha1/gcs_resource_test.go @@ -220,19 +220,20 @@ func Test_GetDownloadContainerSpec(t *testing.T) { SecretKey: "key.json", }}, }, - wantContainers: []corev1.Container{{ - Name: "storage-fetch-gcs-valid", - Image: "override-with-gsutil-image:latest", - Args: []string{"-args", "cp -r gs://some-bucket/** /workspace"}, - Env: []corev1.EnvVar{{ - Name: "GOOGLE_APPLICATION_CREDENTIALS", - Value: "/var/secret/secretName/key.json", - }}, - VolumeMounts: []corev1.VolumeMount{{ - Name: "volume-gcs-valid-secretName", - MountPath: "/var/secret/secretName", + wantContainers: []corev1.Container{ + CreateDirContainer("gcs-valid", "/workspace"), { + Name: "storage-fetch-gcs-valid", + Image: "override-with-gsutil-image:latest", + Args: []string{"-args", "cp -r gs://some-bucket/** /workspace"}, + Env: []corev1.EnvVar{{ + Name: "GOOGLE_APPLICATION_CREDENTIALS", + Value: "/var/secret/secretName/key.json", + }}, + VolumeMounts: []corev1.VolumeMount{{ + Name: "volume-gcs-valid-secretName", + MountPath: "/var/secret/secretName", + }}, }}, - }}, }, { name: "duplicate secret mount paths", gcsResource: &GCSResource{ @@ -249,19 +250,20 @@ func Test_GetDownloadContainerSpec(t *testing.T) { FieldName: "GOOGLE_APPLICATION_CREDENTIALS", }}, }, - wantContainers: []corev1.Container{{ - Name: "storage-fetch-gcs-valid", - Image: "override-with-gsutil-image:latest", - Args: []string{"-args", "cp gs://some-bucket /workspace"}, - Env: []corev1.EnvVar{{ - Name: "GOOGLE_APPLICATION_CREDENTIALS", - Value: "/var/secret/secretName/key.json", - }}, - VolumeMounts: []corev1.VolumeMount{{ - Name: "volume-gcs-valid-secretName", - MountPath: "/var/secret/secretName", + wantContainers: []corev1.Container{ + CreateDirContainer("gcs-valid", "/workspace"), { + Name: "storage-fetch-gcs-valid", + Image: "override-with-gsutil-image:latest", + Args: []string{"-args", "cp gs://some-bucket /workspace"}, + Env: []corev1.EnvVar{{ + Name: "GOOGLE_APPLICATION_CREDENTIALS", + Value: "/var/secret/secretName/key.json", + }}, + VolumeMounts: []corev1.VolumeMount{{ + Name: "volume-gcs-valid-secretName", + MountPath: "/var/secret/secretName", + }}, }}, - }}, }, { name: "invalid no destination directory set", gcsResource: &GCSResource{ 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 20b587205f1..a3ab6b3ba01 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 132a885a0da..1452cb80913 100644 --- a/pkg/reconciler/v1alpha1/taskrun/resources/input_resource_test.go +++ b/pkg/reconciler/v1alpha1/taskrun/resources/input_resource_test.go @@ -185,7 +185,6 @@ func build() *buildv1alpha1.Build { } } func TestAddResourceToBuild(t *testing.T) { - boolTrue := true task := &v1alpha1.Task{ ObjectMeta: metav1.ObjectMeta{ Name: "build-from-repo", @@ -259,37 +258,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", @@ -315,32 +297,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", @@ -371,41 +334,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{ @@ -428,33 +371,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{ @@ -477,31 +403,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", @@ -529,38 +437,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{ @@ -580,36 +476,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", @@ -637,35 +513,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", @@ -688,7 +552,6 @@ func TestAddResourceToBuild(t *testing.T) { }, build: build(), wantErr: true, - want: nil, }, { desc: "invalid gcs resource type name", task: task, @@ -710,7 +573,6 @@ func TestAddResourceToBuild(t *testing.T) { }, build: build(), wantErr: true, - want: nil, }, { desc: "invalid resource name", task: &v1alpha1.Task{ @@ -730,7 +592,6 @@ func TestAddResourceToBuild(t *testing.T) { taskRun: taskRun, build: build(), wantErr: true, - want: nil, }, { desc: "cluster resource with plain text", task: &v1alpha1.Task{ @@ -768,30 +629,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", @@ -830,41 +675,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) { @@ -874,8 +703,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) + } } }) } @@ -1004,15 +835,13 @@ func Test_StorageInputResource(t *testing.T) { }, Spec: buildv1alpha1.BuildSpec{ Steps: []corev1.Container{{ - Name: "create-dir-gcs-input-resource", - Image: "override-with-bash-noop:latest", - Args: []string{"-args", "mkdir -p /workspace/gcs-input-resource"}, - VolumeMounts: []corev1.VolumeMount{{Name: "workspace", MountPath: "/workspace"}}, + Name: "create-dir-gcs-input-resource", + Image: "override-with-bash-noop:latest", + Args: []string{"-args", "mkdir -p /workspace/gcs-input-resource"}, }, { - Name: "storage-fetch-gcs-input-resource", - Image: "override-with-gsutil-image:latest", - Args: []string{"-args", "cp gs://fake-bucket/rules.zip /workspace/gcs-input-resource"}, - VolumeMounts: []corev1.VolumeMount{{Name: "workspace", MountPath: "/workspace"}}, + Name: "storage-fetch-gcs-input-resource", + Image: "override-with-gsutil-image:latest", + Args: []string{"-args", "cp gs://fake-bucket/rules.zip /workspace/gcs-input-resource"}, }}, }, }, @@ -1085,17 +914,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 e39bad79ba1..75cab941898 100644 --- a/pkg/reconciler/v1alpha1/taskrun/resources/input_resources.go +++ b/pkg/reconciler/v1alpha1/taskrun/resources/input_resources.go @@ -17,10 +17,8 @@ limitations under the License. package resources import ( - "flag" "fmt" "path/filepath" - "strings" "github.com/knative/build-pipeline/pkg/apis/pipeline" "github.com/knative/build-pipeline/pkg/apis/pipeline/v1alpha1" @@ -33,10 +31,6 @@ import ( "k8s.io/client-go/kubernetes" ) -var ( - kubeconfigWriterImage = flag.String("kubeconfig-writer-image", "override-with-kubeconfig-writer:latest", "The container image containing our kubeconfig writer binary.") -) - func getBoundResource(resourceName string, boundResources []v1alpha1.TaskResourceBinding) (*v1alpha1.TaskResourceBinding, error) { for _, br := range boundResources { if br.Name == resourceName { @@ -87,74 +81,64 @@ func AddInputResource( if err != nil { return nil, fmt.Errorf("task %q failed to Get Pipeline Resource: %v: error: %s", taskName, boundResource, err.Error()) } - + var ( + resourceContainers []corev1.Container + resourceVolumes []corev1.Volume + copyStepsFromPrevTasks []corev1.Container + dPath = destinationPath(input.Name, input.TargetPath) + ) // if taskrun is fetching resource from previous task then execute copy step instead of fetching new copy // to the desired destination directory, as long as the resource exports output to be copied - var copyStepsFromPrevTasks []corev1.Container - if allowedOutputResources[resource.Spec.Type] { + if allowedOutputResources[resource.Spec.Type] && taskRun.HasPipelineRunOwnerReference() { 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) - } - cpContainers := as.GetCopyFromContainerSpec(fmt.Sprintf("%s-%d", boundResource.Name, i), path, dPath) if as.GetType() == v1alpha1.ArtifactStoragePVCType { + mountPVC = true for _, ct := range cpContainers { ct.VolumeMounts = []corev1.VolumeMount{getPvcMount(pvcName)} - copyStepsFromPrevTasks = append(copyStepsFromPrevTasks, ct) + createAndCopyContainers := []corev1.Container{v1alpha1.CreateDirContainer(boundResource.Name, dPath), ct} + copyStepsFromPrevTasks = append(copyStepsFromPrevTasks, createAndCopyContainers...) } } else { + // bucket copyStepsFromPrevTasks = append(copyStepsFromPrevTasks, cpContainers...) } } } - - // 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...) build.Spec.Volumes = append(build.Spec.Volumes, as.GetSecretsVolumes()...) } 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...) } } @@ -164,81 +148,39 @@ 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 := v1alpha1.CreateDirContainer(storageResource.GetName(), destDirectory) - gcsDownloadContainers, err := storageResource.GetDownloadContainerSpec() +func addStorageFetchStep(build *buildv1alpha1.Build, storageResource v1alpha1.PipelineStorageResourceInterface, destPath string) ([]corev1.Container, []corev1.Volume, error) { + storageResource.SetDestinationDirectory(destPath) + gcsContainers, err := storageResource.GetDownloadContainerSpec() if err != nil { - return err + return nil, nil, err } + + 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) + 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, - }, + 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 getResource(r *v1alpha1.TaskResourceBinding, getter GetResource) (*v1alpha1.PipelineResource, error) { @@ -260,3 +202,10 @@ func getResource(r *v1alpha1.TaskResourceBinding, getter GetResource) (*v1alpha1 } return nil, fmt.Errorf("Neither ResourseRef not ResourceSpec is defined") } + +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 cc05c223ec8..81f7ff9c592 100644 --- a/pkg/reconciler/v1alpha1/taskrun/resources/output_resource.go +++ b/pkg/reconciler/v1alpha1/taskrun/resources/output_resource.go @@ -77,11 +77,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) } } @@ -95,56 +91,72 @@ func AddOutputResources( if err != nil { return fmt.Errorf("Failed to get output pipeline Resource for task %q resource %v; error: %s", taskName, boundResource, err.Error()) } - + 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) + } } - 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) + 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()) + } } } - // 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. - if allowedOutputResources[resource.Spec.Type] && taskRun.HasPipelineRunOwnerReference() { + if taskRun.HasPipelineRunOwnerReference() { var newSteps []corev1.Container for _, dPath := range boundResource.Paths { containers := as.GetCopyToContainerSpec(resource.GetName(), sourcePath, dPath) newSteps = append(newSteps, containers...) } - b.Spec.Steps = append(b.Spec.Steps, newSteps...) - b.Spec.Volumes = append(b.Spec.Volumes, as.GetSecretsVolumes()...) + resourceContainers = append(resourceContainers, newSteps...) + resourceVolumes = append(resourceVolumes, as.GetSecretsVolumes()...) } - } - if as.GetType() == v1alpha1.ArtifactStoragePVCType { - if pvcName == "" { - return nil - } + b.Spec.Steps = append(b.Spec.Steps, resourceContainers...) + b.Spec.Volumes = append(b.Spec.Volumes, resourceVolumes...) - // attach pvc volume only if it is not already attached - for _, buildVol := range b.Spec.Volumes { - if buildVol.Name == pvcName { + if as.GetType() == v1alpha1.ArtifactStoragePVCType { + if pvcName == "" { return nil } + + // attach pvc volume only if it is not already attached + for _, buildVol := range b.Spec.Volumes { + if buildVol.Name == pvcName { + return nil + } + } + b.Spec.Volumes = append(b.Spec.Volumes, GetPVCVolume(pvcName)) } - b.Spec.Volumes = append(b.Spec.Volumes, GetPVCVolume(pvcName)) } return nil } @@ -152,43 +164,39 @@ 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 + return gcsContainers, storageVol, nil } diff --git a/pkg/reconciler/v1alpha1/taskrun/resources/output_resource_test.go b/pkg/reconciler/v1alpha1/taskrun/resources/output_resource_test.go index 68f8a60befe..7001a89d4bd 100644 --- a/pkg/reconciler/v1alpha1/taskrun/resources/output_resource_test.go +++ b/pkg/reconciler/v1alpha1/taskrun/resources/output_resource_test.go @@ -279,7 +279,7 @@ func Test_Valid_OutputResources(t *testing.T) { Namespace: "marshmallow", OwnerReferences: []metav1.OwnerReference{{ Kind: "PipelineRun", - Name: "pipelinerun", + Name: "pipelinerun-parent", }}, }, Spec: v1alpha1.TaskRunSpec{ @@ -329,9 +329,6 @@ 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{{ @@ -341,12 +338,12 @@ func Test_Valid_OutputResources(t *testing.T) { Name: "source-mkdir-source-gcs", Image: "override-with-bash-noop:latest", Args: []string{"-args", "mkdir -p pipeline-task-path"}, - VolumeMounts: []corev1.VolumeMount{{Name: "pipelinerun-pvc", MountPath: "/pvc"}}, + VolumeMounts: []corev1.VolumeMount{{Name: "pipelinerun-parent-pvc", MountPath: "/pvc"}}, }, { Name: "source-copy-source-gcs", Image: "override-with-bash-noop:latest", Args: []string{"-args", "cp -r /workspace/faraway-disk/. pipeline-task-path"}, - VolumeMounts: []corev1.VolumeMount{{Name: "pipelinerun-pvc", MountPath: "/pvc"}}, + VolumeMounts: []corev1.VolumeMount{{Name: "pipelinerun-parent-pvc", MountPath: "/pvc"}}, }}, build: build(), wantVolumes: []corev1.Volume{{ @@ -398,8 +395,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", @@ -462,8 +457,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", @@ -515,8 +508,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", @@ -597,6 +588,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() @@ -1034,33 +1059,7 @@ func Test_InValid_OutputResources(t *testing.T) { fakekubeclient := fakek8s.NewSimpleClientset() err := AddOutputResources(fakekubeclient, build(), c.task.Name, &c.task.Spec, c.taskRun, outputpipelineResourceLister, logger) if (err != nil) != c.wantErr { - t.Fatalf("Test AddOutputResourceSteps %v ; error: %s", c.desc, err) - } - }) - } -} - -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 c7bfe2eedbf..4d001163436 100644 --- a/pkg/reconciler/v1alpha1/taskrun/taskrun_test.go +++ b/pkg/reconciler/v1alpha1/taskrun/taskrun_test.go @@ -54,7 +54,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), @@ -262,7 +266,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"}`), @@ -278,10 +289,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"}), @@ -305,7 +322,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"}`), @@ -323,10 +347,17 @@ func TestReconcile(t *testing.T) { tb.BuildVolume(getToolsVolume(taskRunWithClusterTask.Name)), ), }, { - name: "taskrun-with-resource-spec", + name: "taskrun-with-resource-spec-task-spec", taskRun: taskRunWithResourceSpecAndTaskSpec, wantBuildSpec: tb.BuildSpec( - tb.BuildSource("workspace", tb.BuildSourceGit("github.com/build-pipeline.git", "rel-can")), + tb.BuildStep("git-source-workspace", "override-with-git:latest", + tb.Args("-url", "github.com/build-pipeline.git", "-revision", "rel-can", "-path", "/workspace/workspace"), + tb.VolumeMount(corev1.VolumeMount{ + Name: "workspace", + MountPath: workspaceDir, + }), + tb.VolumeMount(implicitBuilderVolumeMounts), + ), entrypointCopyStep, tb.BuildStep("mystep", "ubuntu", tb.Command(entrypointLocation), tb.EnvVar("ENTRYPOINT_OPTIONS", `{"args":["mycmd"],"process_log":"/tools/process-log.txt","marker_file":"/tools/marker-file.txt"}`),