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 commit also makes a few minor changes to the way our bash output gets logged.
I discovered this was missing during debugging, and made it consistent with the gsutil
wrapper.

This is a followup to tektoncd#1119 and should be submitted
once the next release is cut.

cmd/bash/main.go

@@ -42,8 +42,8 @@ package main
 
 import (
 	"flag"
-	"log"
 	"os/exec"
+	"strings"
 
 	"github.com/tektoncd/pipeline/pkg/logging"
 )
@@ -59,8 +59,7 @@ func main() {
 	cmd := exec.Command("sh", "-c", *args)
 	stdoutStderr, err := cmd.CombinedOutput()
 	if err != nil {
-		logger.Errorf("Error executing command %q with arguments %s", *args, stdoutStderr)
-		log.Fatal(err)
+		logger.Fatalf("Error executing command %q ; error %s; cmd_output %s", strings.Join(cmd.Args, " "), err.Error(), stdoutStderr)
 	}
-	logger.Infof("Successfully executed command %q", *args)
+	logger.Infof("Successfully executed command %q; output %s", strings.Join(cmd.Args, " "), stdoutStderr)
 }

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() {

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",

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"),

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)

test/pipelinerun_test.go

@@ -121,7 +121,7 @@ func TestPipelineRun(t *testing.T) {
 			t.Parallel()
 			c, namespace := setup(t)
 
-			knativetest.CleanupOnInterrupt(func() { tearDown(t, c, namespace) }, t.Logf)
+			// knativetest.CleanupOnInterrupt(func() { tearDown(t, c, namespace) }, t.Logf)
 			defer tearDown(t, c, namespace)
 
 			t.Logf("Setting up test resources for %q test in namespace %s", td.name, namespace)
@@ -216,36 +216,48 @@ func getFanInFanOutTasks(namespace string) []*v1alpha1.Task {
 			)),
 			tb.TaskOutputs(outWorkspaceResource),
 			tb.Step("write-data-task-0-step-0", "ubuntu", tb.Command("/bin/bash"),
-				tb.Args("-c", "echo stuff > /workspace/brandnewspace/stuff"),
+				tb.Args("-c", "mkdir -p /workspace/output/workspace && echo stuff > /workspace/output/workspace/stuff"),
 			),
 			tb.Step("write-data-task-0-step-1", "ubuntu", tb.Command("/bin/bash"),
-				tb.Args("-c", "echo other > /workspace/brandnewspace/other"),
+				tb.Args("-c", "echo other > /workspace/output/workspace/other"),
+			),
+			tb.Step("find", "ubuntu", tb.Command("/bin/bash"),
+				tb.Args("-c", "find /workspace"),
 			),
 		)),
 		tb.Task("check-create-files-exists", namespace, tb.TaskSpec(
 			tb.TaskInputs(inWorkspaceResource),
 			tb.TaskOutputs(outWorkspaceResource),
+			tb.Step("find", "ubuntu", tb.Command("/bin/bash"),
+				tb.Args("-c", "find /workspace"),
+			),
 			tb.Step("read-from-task-0", "ubuntu", tb.Command("/bin/bash"),
-				tb.Args("-c", "[[ stuff == $(cat /workspace//workspace/stuff) ]]"),
+				tb.Args("-c", "[[ stuff == $(cat /workspace/workspace/stuff) ]]"),
 			),
 			tb.Step("write-data-task-1", "ubuntu", tb.Command("/bin/bash"),
-				tb.Args("-c", "echo something > /workspace/workspace/something"),
+				tb.Args("-c", "mkdir -p /workspace/output/workspace && cp -r /workspace/workspace/. /workspace/output/workspace && echo something > /workspace/output/workspace/something"),
 			),
 		)),
 		tb.Task("check-create-files-exists-2", namespace, tb.TaskSpec(
 			tb.TaskInputs(inWorkspaceResource),
 			tb.TaskOutputs(outWorkspaceResource),
+			tb.Step("find", "ubuntu", tb.Command("/bin/bash"),
+				tb.Args("-c", "find /workspace"),
+			),
 			tb.Step("read-from-task-0", "ubuntu", tb.Command("/bin/bash"),
 				tb.Args("-c", "[[ other == $(cat /workspace/workspace/other) ]]"),
 			),
 			tb.Step("write-data-task-1", "ubuntu", tb.Command("/bin/bash"),
-				tb.Args("-c", "echo else > /workspace/workspace/else"),
+				tb.Args("-c", "mkdir -p /workspace/output/workspace && cp -r /workspace/workspace/. /workspace/output/workspace && echo else > /workspace/output/workspace/else"),
 			),
 		)),
 		tb.Task("read-files", namespace, tb.TaskSpec(
 			tb.TaskInputs(tb.InputsResource("workspace", v1alpha1.PipelineResourceTypeGit,
 				tb.ResourceTargetPath("readingspace"),
 			)),
+			tb.Step("find", "ubuntu", tb.Command("/bin/bash"),
+				tb.Args("-c", "find /workspace"),
+			),
 			tb.Step("read-from-task-0", "ubuntu", tb.Command("/bin/bash"),
 				tb.Args("-c", "[[ stuff == $(cat /workspace/readingspace/stuff) ]]"),
 			),