From 5e8d2ea551db20eb4a1cb714c0cd3e1dfb617117 Mon Sep 17 00:00:00 2001 From: Shash Reddy Date: Fri, 18 Jan 2019 12:13:36 -0500 Subject: [PATCH] Update resource to be namepaced under path "workspace/task_resource_name" - In PR #393 resource was placed under path "/workspace/resource_name", there is a use case where a resource can be used by a task multiple times as source with duplicate names. - update storage resource(gcs) to have new namespace path as well - Add more unit test to have more coverage for this use case. - Enable helm deploy task in this PR (#393 PR disabled it as there is cyclic dependency) --- .../taskrun/resources/input_resource_test.go | 92 +++++++++++++++++-- .../taskrun/resources/input_resources.go | 4 +- .../v1alpha1/taskrun/resources/pod.go | 4 +- .../v1alpha1/taskrun/resources/pod_test.go | 2 +- .../v1alpha1/taskrun/taskrun_test.go | 4 +- test/helm_task_test.go | 17 ++-- test/kaniko_task_test.go | 8 +- 7 files changed, 104 insertions(+), 27 deletions(-) diff --git a/pkg/reconciler/v1alpha1/taskrun/resources/input_resource_test.go b/pkg/reconciler/v1alpha1/taskrun/resources/input_resource_test.go index fe1b59274ec..cb63216c87c 100644 --- a/pkg/reconciler/v1alpha1/taskrun/resources/input_resource_test.go +++ b/pkg/reconciler/v1alpha1/taskrun/resources/input_resource_test.go @@ -199,7 +199,23 @@ func TestAddResourceToBuild(t *testing.T) { }, }, } - + taskWithMultipleGitSources := &v1alpha1.Task{ + ObjectMeta: metav1.ObjectMeta{ + Name: "build-from-repo", + Namespace: "marshmallow", + }, + Spec: v1alpha1.TaskSpec{ + Inputs: &v1alpha1.Inputs{ + Resources: []v1alpha1.TaskResource{{ + Name: "gitspace", + Type: "git", + }, { + Name: "git-duplicate-space", + Type: "git", + }}, + }, + }, + } taskWithTargetPath := &v1alpha1.Task{ ObjectMeta: metav1.ObjectMeta{ Name: "task-with-targetpath", @@ -270,7 +286,7 @@ func TestAddResourceToBuild(t *testing.T) { Url: "https://github.com/grafeas/kritis", Revision: "master", }, - Name: "the-git", + Name: "gitspace", }}, }, }, @@ -317,7 +333,69 @@ func TestAddResourceToBuild(t *testing.T) { }, Spec: buildv1alpha1.BuildSpec{ Sources: []buildv1alpha1.SourceSpec{{ - Name: "the-git-with-branch", + Name: "gitspace", + Git: &buildv1alpha1.GitSourceSpec{ + Url: "https://github.com/grafeas/kritis", + Revision: "branch", + }, + }}, + }, + }, + }, { + desc: "same git input resource for task with diff resource name", + task: taskWithMultipleGitSources, + taskRun: &v1alpha1.TaskRun{ + ObjectMeta: metav1.ObjectMeta{ + Name: "build-from-repo-run", + Namespace: "marshmallow", + }, + Spec: v1alpha1.TaskRunSpec{ + TaskRef: &v1alpha1.TaskRef{ + Name: "simpleTask", + }, + Inputs: v1alpha1.TaskRunInputs{ + Resources: []v1alpha1.TaskResourceBinding{{ + ResourceRef: v1alpha1.PipelineResourceRef{ + Name: "the-git-with-branch", + }, + Name: "gitspace", + }, { + ResourceRef: v1alpha1.PipelineResourceRef{ + Name: "the-git-with-branch", + }, + Name: "git-duplicate-space", + }}, + }, + }, + }, + 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", @@ -370,7 +448,7 @@ func TestAddResourceToBuild(t *testing.T) { Url: "https://github.com/grafeas/kritis", Revision: "master", }, - Name: "the-git", + Name: "gitspace", }}, }, }, @@ -420,7 +498,7 @@ func TestAddResourceToBuild(t *testing.T) { Url: "https://github.com/grafeas/kritis", Revision: "branch", }, - Name: "the-git-with-branch", + Name: "gitspace", }}, }, }, @@ -879,14 +957,14 @@ 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"}, + Args: []string{"-args", "mkdir -p /workspace/storage-gcs-keys"}, VolumeMounts: []corev1.VolumeMount{{ Name: "workspace", MountPath: "/workspace", }}, }, { Name: "storage-fetch-storage-gcs-keys", Image: "override-with-gsutil-image:latest", - Args: []string{"-args", "cp -r gs://fake-bucket/rules.zip/** /workspace"}, + 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: "volume-storage-gcs-keys-secret-name2", MountPath: "/var/secret/secret-name2"}, { diff --git a/pkg/reconciler/v1alpha1/taskrun/resources/input_resources.go b/pkg/reconciler/v1alpha1/taskrun/resources/input_resources.go index 2dc058950f7..556337bd84f 100644 --- a/pkg/reconciler/v1alpha1/taskrun/resources/input_resources.go +++ b/pkg/reconciler/v1alpha1/taskrun/resources/input_resources.go @@ -110,7 +110,7 @@ func AddInputResource( build.Spec.Sources = append(build.Spec.Sources, buildv1alpha1.SourceSpec{ Git: gitSourceSpec, TargetPath: input.TargetPath, - Name: gitResource.Name, + Name: input.Name, }) } case v1alpha1.PipelineResourceTypeCluster: @@ -173,7 +173,7 @@ func addClusterBuildStep(build *buildv1alpha1.Build, clusterResource *v1alpha1.C } func addStorageFetchStep(build *buildv1alpha1.Build, storageResource v1alpha1.PipelineStorageResourceInterface, destPath string) error { - var destDirectory = workspaceDir + var destDirectory = filepath.Join(workspaceDir, storageResource.GetName()) if destPath != "" { destDirectory = filepath.Join(workspaceDir, destPath) } diff --git a/pkg/reconciler/v1alpha1/taskrun/resources/pod.go b/pkg/reconciler/v1alpha1/taskrun/resources/pod.go index b88a795df6c..5ec208cab51 100644 --- a/pkg/reconciler/v1alpha1/taskrun/resources/pod.go +++ b/pkg/reconciler/v1alpha1/taskrun/resources/pod.go @@ -118,7 +118,7 @@ func gitToContainer(source v1alpha1.SourceSpec, index int) (*corev1.Container, e } if source.TargetPath != "" { - args = append(args, []string{"-path", filepath.Join(source.Name, source.TargetPath)}...) + args = append(args, []string{"-path", source.TargetPath}...) } else { args = append(args, []string{"-path", source.Name}...) } @@ -150,7 +150,7 @@ func gcsToContainer(source v1alpha1.SourceSpec, index int) (*corev1.Container, e args := []string{"--type", string(gcs.Type), "--location", gcs.Location} // dest_dir is the destination directory for GCS files to be copies" if source.TargetPath != "" { - args = append(args, "--dest_dir", filepath.Join(workspaceDir, source.Name, source.TargetPath)) + args = append(args, "--dest_dir", filepath.Join(workspaceDir, source.TargetPath)) } else { args = append(args, "--dest_dir", filepath.Join(workspaceDir, source.Name)) } diff --git a/pkg/reconciler/v1alpha1/taskrun/resources/pod_test.go b/pkg/reconciler/v1alpha1/taskrun/resources/pod_test.go index 395e9bb95dc..cadff3ca5ae 100644 --- a/pkg/reconciler/v1alpha1/taskrun/resources/pod_test.go +++ b/pkg/reconciler/v1alpha1/taskrun/resources/pod_test.go @@ -371,7 +371,7 @@ func TestMakePod(t *testing.T) { }, { Name: initContainerPrefix + gcsSource + "-gcs-foo-bar", Image: *gcsFetcherImage, - Args: []string{"--type", "Manifest", "--location", "gs://foo/bar", "--dest_dir", "/workspace/gcs-foo-bar/path/foo"}, + Args: []string{"--type", "Manifest", "--location", "gs://foo/bar", "--dest_dir", "/workspace/path/foo"}, Env: implicitEnvVars, VolumeMounts: implicitVolumeMounts, // without subpath WorkingDir: workspaceDir, diff --git a/pkg/reconciler/v1alpha1/taskrun/taskrun_test.go b/pkg/reconciler/v1alpha1/taskrun/taskrun_test.go index b1696bdd71d..f54e985844d 100644 --- a/pkg/reconciler/v1alpha1/taskrun/taskrun_test.go +++ b/pkg/reconciler/v1alpha1/taskrun/taskrun_test.go @@ -264,7 +264,7 @@ func TestReconcile(t *testing.T) { name: "params", taskRun: taskRunTemplating, wantBuildSpec: tb.BuildSpec( - tb.BuildSource("git-resource", tb.BuildSourceGit("https://foo.git", "master")), + tb.BuildSource("workspace", tb.BuildSourceGit("https://foo.git", "master")), 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"}`), @@ -307,7 +307,7 @@ func TestReconcile(t *testing.T) { name: "taskrun-with-taskspec", taskRun: taskRunWithTaskSpec, wantBuildSpec: tb.BuildSpec( - tb.BuildSource("git-resource", tb.BuildSourceGit("https://foo.git", "master")), + tb.BuildSource("workspace", tb.BuildSourceGit("https://foo.git", "master")), 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"}`), diff --git a/test/helm_task_test.go b/test/helm_task_test.go index abca6993234..198c82039a7 100644 --- a/test/helm_task_test.go +++ b/test/helm_task_test.go @@ -52,7 +52,6 @@ var ( // TestHelmDeployPipelineRun is an integration test that will verify a pipeline build an image // and then using helm to deploy it func TestHelmDeployPipelineRun(t *testing.T) { - t.Skip() logger := logging.GetContextLogger(t.Name()) c, namespace := setup(t, logger) setupClusterBindingForHelm(c, t, namespace, logger) @@ -159,9 +158,10 @@ func getCreateImageTask(namespace string, t *testing.T) *v1alpha1.Task { t.Logf("Image to be pusblished: %s", imageName) return tb.Task(createImageTaskName, namespace, tb.TaskSpec( - tb.TaskInputs(tb.InputsResource("workspace", v1alpha1.PipelineResourceTypeGit)), + tb.TaskInputs(tb.InputsResource("gitsource", v1alpha1.PipelineResourceTypeGit)), tb.Step("kaniko", "gcr.io/kaniko-project/executor", tb.Args( - "--dockerfile=/workspace/test/gohelloworld/Dockerfile", + "--dockerfile=/workspace/gitsource/test/gohelloworld/Dockerfile", + "--context=/workspace/gitsource/", fmt.Sprintf("--destination=%s", imageName), )), )) @@ -170,7 +170,7 @@ func getCreateImageTask(namespace string, t *testing.T) *v1alpha1.Task { func getHelmDeployTask(namespace string) *v1alpha1.Task { return tb.Task(helmDeployTaskName, namespace, tb.TaskSpec( tb.TaskInputs( - tb.InputsResource("workspace", v1alpha1.PipelineResourceTypeGit), + tb.InputsResource("gitsource", v1alpha1.PipelineResourceTypeGit), tb.InputsParam("pathToHelmCharts", tb.ParamDescription("Path to the helm charts")), tb.InputsParam("image"), tb.InputsParam("chartname"), ), @@ -190,9 +190,8 @@ func getHelmDeployPipeline(namespace string) *v1alpha1.Pipeline { return tb.Pipeline(helmDeployPipelineName, namespace, tb.PipelineSpec( tb.PipelineTask("push-image", createImageTaskName), tb.PipelineTask("helm-deploy", helmDeployTaskName, - tb.PipelineTaskResourceDependency("workspace"), // tb.ProvidedBy(createImageTaskName), //TODO: https://github.com/knative/build-pipeline/issues/148 - - tb.PipelineTaskParam("pathToHelmCharts", "/workspace/test/gohelloworld/gohelloworld-chart"), + tb.PipelineTaskResourceDependency("gitsource"), // tb.ProvidedBy(createImageTaskName), //TODO: https://github.com/knative/build-pipeline/issues/148 + tb.PipelineTaskParam("pathToHelmCharts", "/workspace/gitsource/test/gohelloworld/gohelloworld-chart"), tb.PipelineTaskParam("chartname", "gohelloworld"), tb.PipelineTaskParam("image", imageName), ), @@ -203,10 +202,10 @@ func getHelmDeployPipelineRun(namespace string) *v1alpha1.PipelineRun { return tb.PipelineRun(helmDeployPipelineRunName, namespace, tb.PipelineRunSpec( helmDeployPipelineName, tb.PipelineRunTaskResource("helm-deploy", - tb.PipelineTaskResourceInputs("workspace", tb.ResourceBindingRef(sourceResourceName)), + tb.PipelineTaskResourceInputs("gitsource", tb.ResourceBindingRef(sourceResourceName)), ), tb.PipelineRunTaskResource("push-image", - tb.PipelineTaskResourceInputs("workspace", tb.ResourceBindingRef(sourceResourceName)), + tb.PipelineTaskResourceInputs("gitsource", tb.ResourceBindingRef(sourceResourceName)), ), )) } diff --git a/test/kaniko_task_test.go b/test/kaniko_task_test.go index de742a80686..763ef295b6d 100644 --- a/test/kaniko_task_test.go +++ b/test/kaniko_task_test.go @@ -88,14 +88,14 @@ func createSecret(c *knativetest.KubeClient, namespace string) (bool, error) { func getTask(repo, namespace string, withSecretConfig bool) *v1alpha1.Task { taskSpecOps := []tb.TaskSpecOp{ - tb.TaskInputs(tb.InputsResource("workspace", v1alpha1.PipelineResourceTypeGit)), + tb.TaskInputs(tb.InputsResource("gitsource", v1alpha1.PipelineResourceTypeGit)), tb.TaskTimeout(2 * time.Minute), } stepOps := []tb.ContainerOp{ tb.Args( - "--dockerfile=/workspace/go-example-git/Dockerfile", + "--dockerfile=/workspace/gitsource/Dockerfile", fmt.Sprintf("--destination=%s", repo), - "--context=/workspace/go-example-git", + "--context=/workspace/gitsource", ), } if withSecretConfig { @@ -118,7 +118,7 @@ func getTask(repo, namespace string, withSecretConfig bool) *v1alpha1.Task { func getTaskRun(namespace string) *v1alpha1.TaskRun { return tb.TaskRun(kanikoTaskRunName, namespace, tb.TaskRunSpec( tb.TaskRunTaskRef(kanikoTaskName), - tb.TaskRunInputs(tb.TaskRunInputsResource("workspace", tb.ResourceBindingRef(kanikoResourceName))), + tb.TaskRunInputs(tb.TaskRunInputsResource("gitsource", tb.ResourceBindingRef(kanikoResourceName))), )) }