From 75a4fd5dbbcca647514ad9f452329c201a386b6c Mon Sep 17 00:00:00 2001 From: Dan Lorenc Date: Wed, 24 Jul 2019 10:19:57 -0500 Subject: [PATCH] Change the behavior of outputs that are also used as inputs. This change makes the handling of Resources within a Task consistent, regardless of whether the same Resource is used as both an input and an output. Previously these were special cased, which made it hard to write Tasks consistently. This is a followup to https://github.com/tektoncd/pipeline/pull/1119 and should be submitted once the next release is cut. --- .../taskrun/resources/output_resource.go | 25 ++++--------------- .../taskrun/resources/output_resource_test.go | 8 +++--- .../v1alpha1/taskrun/taskrun_test.go | 2 +- test/dag_test.go | 2 ++ 4 files changed, 12 insertions(+), 25 deletions(-) diff --git a/pkg/reconciler/v1alpha1/taskrun/resources/output_resource.go b/pkg/reconciler/v1alpha1/taskrun/resources/output_resource.go index c5f71385860..4815198552f 100644 --- a/pkg/reconciler/v1alpha1/taskrun/resources/output_resource.go +++ b/pkg/reconciler/v1alpha1/taskrun/resources/output_resource.go @@ -72,15 +72,6 @@ func AddOutputResources( return nil, err } - // track resources that are present in input of task cuz these resources will be copied onto PVC - inputResourceMap := map[string]string{} - - if taskSpec.Inputs != nil { - for _, input := range taskSpec.Inputs.Resources { - inputResourceMap[input.Name] = destinationPath(input.Name, input.TargetPath) - } - } - for _, output := range taskSpec.Outputs.Resources { boundResource, err := getBoundResource(output.Name, taskRun.Spec.Outputs.Resources) if err != nil { @@ -95,18 +86,12 @@ func AddOutputResources( 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 != "" { - logger.Warn(`This task uses the same resource as an input and output. The behavior of this will change in a future release. - See https://github.com/tektoncd/pipeline/issues/1118 for more information.`) + + var sourcePath string + if output.TargetPath == "" { + sourcePath = filepath.Join(outputDir, boundResource.Name) } else { - if output.TargetPath == "" { - sourcePath = filepath.Join(outputDir, boundResource.Name) - } else { - sourcePath = output.TargetPath - } + sourcePath = output.TargetPath } resource.SetDestinationDirectory(sourcePath) switch resource.GetType() { diff --git a/pkg/reconciler/v1alpha1/taskrun/resources/output_resource_test.go b/pkg/reconciler/v1alpha1/taskrun/resources/output_resource_test.go index ef628d361ce..3ab07d870ab 100644 --- a/pkg/reconciler/v1alpha1/taskrun/resources/output_resource_test.go +++ b/pkg/reconciler/v1alpha1/taskrun/resources/output_resource_test.go @@ -171,7 +171,7 @@ func TestValidOutputResources(t *testing.T) { Name: "source-copy-source-git-mz4c7", Image: "override-with-bash-noop:latest", Command: []string{"/ko-app/bash"}, - Args: []string{"-args", "cp -r /workspace/source-workspace/. pipeline-task-name"}, + Args: []string{"-args", "cp -r /workspace/output/source-workspace/. pipeline-task-name"}, VolumeMounts: []corev1.VolumeMount{{ Name: "pipelinerun-pvc", MountPath: "/pvc", @@ -368,7 +368,7 @@ func TestValidOutputResources(t *testing.T) { MountPath: "/var/secret/sname", }}, Command: []string{"/ko-app/gsutil"}, - Args: []string{"-args", "rsync -d -r /workspace/faraway-disk gs://some-bucket"}, + Args: []string{"-args", "rsync -d -r /workspace/output/source-workspace gs://some-bucket"}, Env: []corev1.EnvVar{{ Name: "GOOGLE_APPLICATION_CREDENTIALS", Value: "/var/secret/sname/key.json", }}, @@ -382,7 +382,7 @@ func TestValidOutputResources(t *testing.T) { Name: "source-copy-source-gcs-mssqb", Image: "override-with-bash-noop:latest", Command: []string{"/ko-app/bash"}, - Args: []string{"-args", "cp -r /workspace/faraway-disk/. pipeline-task-path"}, + Args: []string{"-args", "cp -r /workspace/output/source-workspace/. pipeline-task-path"}, VolumeMounts: []corev1.VolumeMount{{Name: "pipelinerun-parent-pvc", MountPath: "/pvc"}}, }}, wantVolumes: []corev1.Volume{{ @@ -771,7 +771,7 @@ func TestValidOutputResourcesWithBucketStorage(t *testing.T) { Name: "artifact-copy-to-source-git-9l9zj", Image: "override-with-gsutil-image:latest", Command: []string{"/ko-app/gsutil"}, - Args: []string{"-args", "cp -P -r /workspace/source-workspace gs://fake-bucket/pipeline-task-name"}, + Args: []string{"-args", "cp -P -r /workspace/output/source-workspace gs://fake-bucket/pipeline-task-name"}, }}, }, { name: "git resource in output only with bucket storage", diff --git a/pkg/reconciler/v1alpha1/taskrun/taskrun_test.go b/pkg/reconciler/v1alpha1/taskrun/taskrun_test.go index eecf2003316..d98609d80c4 100644 --- a/pkg/reconciler/v1alpha1/taskrun/taskrun_test.go +++ b/pkg/reconciler/v1alpha1/taskrun/taskrun_test.go @@ -672,7 +672,7 @@ func TestReconcile(t *testing.T) { tb.PodContainer("step-source-copy-git-resource-j2tds", "override-with-bash-noop:latest", tb.Command(entrypointLocation), tb.Args("-wait_file", "/builder/tools/5", "-post_file", "/builder/tools/6", "-entrypoint", "/ko-app/bash", "--", - "-args", "cp -r /workspace/git-resource/. output-folder"), + "-args", "cp -r /workspace/output/git-resource/. output-folder"), tb.WorkingDir(workspaceDir), tb.EnvVar("HOME", "/builder/home"), tb.VolumeMount("test-pvc", "/pvc"), diff --git a/test/dag_test.go b/test/dag_test.go index 1b3f86b05da..534ab071a1e 100644 --- a/test/dag_test.go +++ b/test/dag_test.go @@ -53,6 +53,8 @@ func TestDAGPipelineRun(t *testing.T) { ), tb.TaskOutputs(tb.OutputsResource("repo", v1alpha1.PipelineResourceTypeGit)), tb.Step("echo-text", "busybox", tb.Command("echo"), tb.Args("${inputs.params.text}")), + tb.Step("mkdir", "busybox", tb.Command("mkdir"), tb.Args("-p", "${outputs.resources.repo.path}")), + tb.Step("cp", "busybox", tb.Command("cp"), tb.Args("-r", "${inputs.resources.repo.path}", "${outputs.resources.repo.path}")), )) if _, err := c.TaskClient.Create(echoTask); err != nil { t.Fatalf("Failed to create echo Task: %s", err)