Support stepTemplate alongside script mode

MergeStepsWithStepTemplate now takes into account a step Script, instead
of zeroing it out.

There's still room for improvement here; Merge can happen after script
conversion, if script mode is able to accept template-provided args.

Author: imjasonh

pkg/apis/pipeline/v1alpha1/merge.go

@@ -84,7 +84,8 @@ func MergeStepsWithStepTemplate(template *v1.Container, steps []Step) ([]Step, e
 			merged.Args = []string{}
 		}
 
-		steps[i] = Step{Container: *merged}
+		// Pass through original step Script, for later conversion.
+		steps[i] = Step{Container: *merged, Script: s.Script}
 	}
 	return steps, nil
 }

pkg/pod/entrypoint_lookup_test.go

@@ -17,7 +17,6 @@ limitations under the License.
 package pod
 
 import (
-	"errors"
 	"fmt"
 	"testing"
 
@@ -92,7 +91,7 @@ func (f fakeCache) Get(imageName, _, _ string) ([]string, name.Digest, error) {
 
 	d, found := f[imageName]
 	if !found {
-		return nil, name.Digest{}, errors.New("not found")
+		return nil, name.Digest{}, fmt.Errorf("Image %q not found", imageName)
 	}
 	if d.seen {
 		return nil, name.Digest{}, fmt.Errorf("Image %q was already looked up!", imageName)

pkg/pod/pod_test.go

@@ -479,8 +479,12 @@ func TestMakePod(t *testing.T) {
 			Volumes: append(implicitVolumes, toolsVolume, downwardVolume),
 		},
 	}, {
-		desc: "step with script",
+		desc: "step with script and stepTemplate",
 		ts: v1alpha1.TaskSpec{
+			StepTemplate: &corev1.Container{
+				Env:  []corev1.EnvVar{{Name: "FOO", Value: "bar"}},
+				Args: []string{"template", "args"},
+			},
 			Steps: []v1alpha1.Step{{
 				Container: corev1.Container{
 					Name:  "one",
@@ -495,6 +499,12 @@ func TestMakePod(t *testing.T) {
 				},
 				Script: `#!/usr/bin/env python
 print("Hello from Python")`,
+			}, {
+				Container: corev1.Container{
+					Name:    "regular-step",
+					Image:   "image",
+					Command: []string{"regular", "command"},
+				},
 			}},
 		},
 		want: &corev1.PodSpec{
@@ -537,7 +547,7 @@ script-heredoc-randomly-generated-6nl7g
 					"/tekton/scripts/script-0-mz4c7",
 					"--",
 				},
-				Env:          implicitEnvVars,
+				Env:          append(implicitEnvVars, corev1.EnvVar{Name: "FOO", Value: "bar"}),
 				VolumeMounts: append([]corev1.VolumeMount{scriptsVolumeMount, toolsMount, downwardMount}, implicitVolumeMounts...),
 				WorkingDir:   workspaceDir,
 				Resources: corev1.ResourceRequirements{
@@ -560,7 +570,7 @@ script-heredoc-randomly-generated-6nl7g
 					"/tekton/scripts/script-1-78c5n",
 					"--",
 				},
-				Env:          implicitEnvVars,
+				Env:          append(implicitEnvVars, corev1.EnvVar{Name: "FOO", Value: "bar"}),
 				VolumeMounts: append([]corev1.VolumeMount{{Name: "i-have-a-volume-mount"}, scriptsVolumeMount, toolsMount}, implicitVolumeMounts...),
 				WorkingDir:   workspaceDir,
 				Resources: corev1.ResourceRequirements{
@@ -570,6 +580,32 @@ script-heredoc-randomly-generated-6nl7g
 						corev1.ResourceEphemeralStorage: resource.MustParse("0"),
 					},
 				},
+			}, {
+				Name:    "step-regular-step",
+				Image:   "image",
+				Command: []string{"/tekton/tools/entrypoint"},
+				Args: []string{
+					"-wait_file",
+					"/tekton/tools/1",
+					"-post_file",
+					"/tekton/tools/2",
+					"-entrypoint",
+					"regular",
+					"--",
+					"command",
+					"template",
+					"args",
+				},
+				Env:          append(implicitEnvVars, corev1.EnvVar{Name: "FOO", Value: "bar"}),
+				VolumeMounts: append([]corev1.VolumeMount{toolsMount}, implicitVolumeMounts...),
+				WorkingDir:   workspaceDir,
+				Resources: corev1.ResourceRequirements{
+					Requests: corev1.ResourceList{
+						corev1.ResourceCPU:              resource.MustParse("0"),
+						corev1.ResourceMemory:           resource.MustParse("0"),
+						corev1.ResourceEphemeralStorage: resource.MustParse("0"),
+					},
+				},
 			}},
 			Volumes: append(implicitVolumes, scriptsVolume, toolsVolume, downwardVolume),
 		},
@@ -604,10 +640,10 @@ script-heredoc-randomly-generated-6nl7g
 				},
 				Spec: c.trs,
 			}
-			entrypointCache, err := NewEntrypointCache(kubeclient)
-			if err != nil {
-				t.Fatalf("NewEntrypointCache: %v", err)
-			}
+
+			// No entrypoints should be looked up.
+			entrypointCache := fakeCache{}
+
 			got, err := MakePod(images, tr, c.ts, kubeclient, entrypointCache)
 			if err != nil {
 				t.Fatalf("MakePod: %v", err)

pkg/pod/script.go

@@ -90,8 +90,13 @@ cat > ${tmpfile} << '%s'
 %s
 `, tmpFile, heredoc, s.Script, heredoc)
 
-		// Set the command to execute the correct script in the mounted volume.
+		// Set the command to execute the correct script in the mounted
+		// volume.
+		// A previous merge with stepTemplate may have populated
+		// Command and Args, even though this is not normally valid, so
+		// we'll clear out the Args and overwrite Command.
 		steps[i].Command = []string{tmpFile}
+		steps[i].Args = nil // TODO(#1652): Don't overwrite this.
 		steps[i].VolumeMounts = append(steps[i].VolumeMounts, scriptsVolumeMount)
 		containers = append(containers, steps[i].Container)
 	}