Merge place-tools and step-init together

This is part of an effort to reduce the number of init container to
the minimum. Both place-tools and step-init are using the same image
and can be easily and safely merged together.

This has few benefits, but the main one is that it reduces the number
of container to run, and thus doesn't reduce the max size of a Result.

Signed-off-by: Andrew Bayer <andrew.bayer@gmail.com>
Signed-off-by: Vincent Demeester <vdemeest@redhat.com>

cmd/entrypoint/subcommands/init.go

@@ -0,0 +1,13 @@
+package subcommands
+
+// InitCommand is the name of main initialization command
+const InitCommand = "init"
+
+// init copies the entrypoint to the right place and sets up /tekton/steps directory for the pod.
+// This expects  the list of steps (in order matching the Task spec).
+func entrypointInit(src, dst string, steps []string) error {
+	if err := cp(src, dst); err != nil {
+		return err
+	}
+	return stepInit(steps)
+}

cmd/entrypoint/subcommands/init_test.go

@@ -0,0 +1,82 @@
+package subcommands
+
+import (
+	"io/ioutil"
+	"os"
+	"path/filepath"
+	"testing"
+)
+
+func TestEntrypointInit(t *testing.T) {
+	tmp := t.TempDir()
+	src := filepath.Join(tmp, "foo.txt")
+	dst := filepath.Join(tmp, "bar.txt")
+	if err := ioutil.WriteFile(src, []byte("hello world"), 0700); err != nil {
+		t.Fatalf("error writing source file: %v", err)
+	}
+
+	// Override tektonRoot for testing.
+	tektonRoot = tmp
+
+	// Create step directory so that symlinks can be successfully created.
+	// This is typically done by volume mounts, so it needs to be done manually
+	// in tests.
+	stepDir := filepath.Join(tmp, "steps")
+	if err := os.Mkdir(stepDir, os.ModePerm); err != nil {
+		t.Fatalf("error creating step directory: %v", err)
+	}
+
+	steps := []string{"a", "b"}
+	if err := entrypointInit(src, dst, steps); err != nil {
+		t.Fatalf("stepInit: %v", err)
+	}
+
+	info, err := os.Lstat(dst)
+	if err != nil {
+		t.Fatalf("error statting destination file: %v", err)
+	}
+
+	// os.OpenFile is subject to umasks, so the created permissions of the
+	// created dst file might be more restrictive than dstPermissions.
+	// excludePerm represents the value of permissions we do not want in the
+	// resulting file - e.g. if dstPermissions is 0311, excludePerm should be
+	// 0466.
+	// This is done instead of trying to look up the system umask, since this
+	// relies on syscalls that we are not sure will be portable across
+	// environments.
+	excludePerm := os.ModePerm ^ dstPermissions
+	if p := info.Mode().Perm(); p&excludePerm != 0 {
+		t.Errorf("expected permissions <= %#o for destination file but found %#o", dstPermissions, p)
+	}
+
+	// Map of symlinks to expected /tekton/run folders.
+	// Expected format:
+	// Key: /tekton/steps/<key>
+	// Value: /tekton/run/<value>/status
+	wantLinks := map[string]string{
+		"a": "0",
+		"0": "0",
+		"b": "1",
+		"1": "1",
+	}
+
+	direntry, err := os.ReadDir(stepDir)
+	if err != nil {
+		t.Fatalf("os.ReadDir: %v", err)
+	}
+	for _, de := range direntry {
+		t.Run(de.Name(), func(t *testing.T) {
+			l, err := os.Readlink(filepath.Join(stepDir, de.Name()))
+			if err != nil {
+				t.Fatal(err)
+			}
+			want, ok := wantLinks[de.Name()]
+			if !ok {
+				t.Fatalf("unexpected symlink: %s", de.Name())
+			}
+			if wantDir := filepath.Join(tmp, "run", want, "status"); l != wantDir {
+				t.Errorf("want %s, got %s", wantDir, l)
+			}
+		})
+	}
+}

cmd/entrypoint/subcommands/subcommands.go

@@ -50,6 +50,18 @@ func Process(args []string) error {
 		return nil
 	}
 	switch args[0] {
+	case InitCommand:
+		// If invoked in "init mode" (`entrypoint init <src> <dst> [<step-name>]`),
+		// it will copy the src path to the dst path (like CopyCommand), and initialize
+		// the /tekton/steps folder (like StepInitCommand)
+		if len(args) >= 3 {
+			src, dst := args[1], args[2]
+			steps := args[3:]
+			if err := entrypointInit(src, dst, steps); err != nil {
+				return SubcommandError{subcommand: InitCommand, message: err.Error()}
+			}
+			return SubcommandSuccessful{message: "Entrypoint initialization"}
+		}
 	case CopyCommand:
 		// If invoked in "cp mode" (`entrypoint cp <src> <dst>`), simply copy
 		// the src path to the dst path. This is used to place the entrypoint

pkg/pod/entrypoint.go

@@ -26,6 +26,7 @@ import (
 	"strconv"
 	"strings"
 
+	"github.com/tektoncd/pipeline/pkg/apis/pipeline"
 	"github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1"
 	"gomodules.xyz/jsonpatch/v2"
 	corev1 "k8s.io/api/core/v1"
@@ -71,6 +72,10 @@ var (
 		Name:         binVolumeName,
 		VolumeSource: corev1.VolumeSource{EmptyDir: &corev1.EmptyDirVolumeSource{}},
 	}
+	internalStepsMount = corev1.VolumeMount{
+		Name:      "tekton-internal-steps",
+		MountPath: pipeline.StepsDir,
+	}
 
 	// TODO(#1605): Signal sidecar readiness by injecting entrypoint,
 	// remove dependency on Downward API.

pkg/pod/pod.go

@@ -23,8 +23,6 @@ import (
 	"path/filepath"
 	"strconv"
 
-	"knative.dev/pkg/kmeta"
-
 	"github.com/tektoncd/pipeline/pkg/apis/config"
 	"github.com/tektoncd/pipeline/pkg/apis/pipeline"
 	"github.com/tektoncd/pipeline/pkg/apis/pipeline/pod"
@@ -35,6 +33,7 @@ import (
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/client-go/kubernetes"
 	"knative.dev/pkg/changeset"
+	"knative.dev/pkg/kmeta"
 )
 
 const (
@@ -154,20 +153,25 @@ func (b *Builder) Build(ctx context.Context, taskRun *v1beta1.TaskRun, taskSpec
 	} else {
 		scriptsInit, stepContainers, sidecarContainers = convertScripts(b.Images.ShellImage, "", steps, sidecars, nil)
 	}
+
 	if scriptsInit != nil {
 		initContainers = append(initContainers, *scriptsInit)
 		volumes = append(volumes, scriptsVolume)
 	}
-
 	if alphaAPIEnabled && taskRun.Spec.Debug != nil {
 		volumes = append(volumes, debugScriptsVolume, debugInfoVolume)
 	}
-
 	// Initialize any workingDirs under /workspace.
 	if workingDirInit := workingDirInit(b.Images.WorkingDirInitImage, stepContainers); workingDirInit != nil {
 		initContainers = append(initContainers, *workingDirInit)
 	}
 
+	// place the entrypoint first in case other init containers rely on its
+	// features (e.g. decode-script).
+	initContainers = append([]corev1.Container{
+		prepareInitContainer(b.Images.EntrypointImage, steps),
+	}, initContainers...)
+
 	// By default, use an empty pod template and take the one defined in the task run spec if any
 	podTemplate := pod.Template{}
 
@@ -181,22 +185,6 @@ func (b *Builder) Build(ctx context.Context, taskRun *v1beta1.TaskRun, taskSpec
 		return nil, err
 	}
 
-	// Rewrite steps with entrypoint binary. Append the entrypoint init
-	// container to place the entrypoint binary. Also add timeout flags
-	// to entrypoint binary.
-	entrypointInit := corev1.Container{
-		Name:  "place-tools",
-		Image: b.Images.EntrypointImage,
-		// Rewrite default WorkingDir from "/home/nonroot" to "/"
-		// as suggested at https://github.com/GoogleContainerTools/distroless/issues/718
-		// to avoid permission errors with nonroot users not equal to `65532`
-		WorkingDir: "/",
-		// Invoke the entrypoint binary in "cp mode" to copy itself
-		// into the correct location for later steps.
-		Command:      []string{"/ko-app/entrypoint", "cp", "/ko-app/entrypoint", entrypointBinary},
-		VolumeMounts: []corev1.VolumeMount{binMount},
-	}
-
 	if alphaAPIEnabled {
 		stepContainers, err = orderContainers(credEntrypointArgs, stepContainers, &taskSpec, taskRun.Spec.Debug)
 	} else {
@@ -205,12 +193,6 @@ func (b *Builder) Build(ctx context.Context, taskRun *v1beta1.TaskRun, taskSpec
 	if err != nil {
 		return nil, err
 	}
-	// place the entrypoint first in case other init containers rely on its
-	// features (e.g. decode-script).
-	initContainers = append([]corev1.Container{
-		entrypointInit,
-		tektonDirInit(b.Images.EntrypointImage, steps),
-	}, initContainers...)
 	volumes = append(volumes, binVolume, downwardVolume)
 
 	// Add implicit env vars.
@@ -421,21 +403,29 @@ func runVolume(i int) corev1.Volume {
 	}
 }
 
-func tektonDirInit(image string, steps []v1beta1.Step) corev1.Container {
-	cmd := make([]string, 0, len(steps)+2)
-	cmd = append(cmd, "/ko-app/entrypoint", "step-init")
+// prepareInitContainers generate a few init containers based of a set of command (in images) and volumes to run
+// This should effectively merge multiple command and volumes together.
+func prepareInitContainer(image string, steps []v1beta1.Step) corev1.Container {
+	// Invoke the entrypoint binary in "cp mode" to copy itself
+	// into the correct location for later steps and initialize steps folder
+	command := []string{"/ko-app/entrypoint", "init", "/ko-app/entrypoint", entrypointBinary}
 	for i, s := range steps {
-		cmd = append(cmd, StepName(s.Name, i))
+		command = append(command, StepName(s.Name, i))
 	}
+	volumeMounts := []corev1.VolumeMount{binMount, internalStepsMount}
 
-	return corev1.Container{
-		Name:       "step-init",
-		Image:      image,
-		WorkingDir: "/",
-		Command:    cmd,
-		VolumeMounts: []corev1.VolumeMount{{
-			Name:      "tekton-internal-steps",
-			MountPath: pipeline.StepsDir,
-		}},
+	// Rewrite steps with entrypoint binary. Append the entrypoint init
+	// container to place the entrypoint binary. Also add timeout flags
+	// to entrypoint binary.
+	prepareInitContainer := corev1.Container{
+		Name:  "prepare",
+		Image: image,
+		// Rewrite default WorkingDir from "/home/nonroot" to "/"
+		// as suggested at https://github.com/GoogleContainerTools/distroless/issues/718
+		// to avoid permission errors with nonroot users not equal to `65532`
+		WorkingDir:   "/",
+		Command:      command,
+		VolumeMounts: volumeMounts,
 	}
+	return prepareInitContainer
 }