From f5a7fdf90c1584bef2e3d08efd1d5ffe690216f2 Mon Sep 17 00:00:00 2001 From: Vincent Demeester Date: Wed, 4 May 2022 17:38:09 +0200 Subject: [PATCH] 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 Signed-off-by: Vincent Demeester --- cmd/entrypoint/subcommands/init.go | 13 ++ cmd/entrypoint/subcommands/init_test.go | 82 ++++++++++++ cmd/entrypoint/subcommands/subcommands.go | 12 ++ .../v1beta1/pipelineruns/4808-regression.yaml | 95 ++++++++++++++ pkg/pod/entrypoint.go | 5 + pkg/pod/pod.go | 66 ++++------ pkg/pod/pod_test.go | 120 +++++++++++------- pkg/reconciler/taskrun/taskrun_test.go | 44 +++---- 8 files changed, 327 insertions(+), 110 deletions(-) create mode 100644 cmd/entrypoint/subcommands/init.go create mode 100644 cmd/entrypoint/subcommands/init_test.go create mode 100644 examples/v1beta1/pipelineruns/4808-regression.yaml diff --git a/cmd/entrypoint/subcommands/init.go b/cmd/entrypoint/subcommands/init.go new file mode 100644 index 00000000000..444036a581e --- /dev/null +++ b/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) +} diff --git a/cmd/entrypoint/subcommands/init_test.go b/cmd/entrypoint/subcommands/init_test.go new file mode 100644 index 00000000000..03d399f171a --- /dev/null +++ b/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/ + // Value: /tekton/run//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) + } + }) + } +} diff --git a/cmd/entrypoint/subcommands/subcommands.go b/cmd/entrypoint/subcommands/subcommands.go index 2cae28b2011..82ce03c4b68 100644 --- a/cmd/entrypoint/subcommands/subcommands.go +++ b/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 []`), + // 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 `), simply copy // the src path to the dst path. This is used to place the entrypoint diff --git a/examples/v1beta1/pipelineruns/4808-regression.yaml b/examples/v1beta1/pipelineruns/4808-regression.yaml new file mode 100644 index 00000000000..df4502a8a88 --- /dev/null +++ b/examples/v1beta1/pipelineruns/4808-regression.yaml @@ -0,0 +1,95 @@ +--- +apiVersion: tekton.dev/v1beta1 +kind: Task +metadata: + name: print-result +spec: + description: >- + Prints a result from another task + params: + - name: TO_PRINT + type: string + steps: + - name: print-result + image: bash:latest + env: + - name: PARAM_TO_PRINT + value: $(params.TO_PRINT) + script: | + #!/usr/bin/env bash + set -e + echo $PARAM_TO_PRINT +--- +apiVersion: tekton.dev/v1beta1 +kind: Task +metadata: + name: generate-result +spec: + description: >- + Creates strings of length based on parameters and puts them into results fields + params: + - name: STRING_LENGTH + description: Length of the string to create + - name: STRING_CHAR + description: Char to use when creating string + type: string + default: '.' + results: + - name: RESULT_STRING + description: A result string + steps: + - name: gen-result + image: bash:latest + env: + - name: PARAM_STRING_LENGTH + value: $(params.STRING_LENGTH) + - name: PARAM_STRING_CHAR + value: $(params.STRING_CHAR) + script: | + #!/usr/bin/env bash + set -e + len=$PARAM_STRING_LENGTH + ch=$PARAM_STRING_CHAR + printf '%*s' "$len" | tr ' ' "$ch" >> $(results.RESULT_STRING.path) +--- +apiVersion: tekton.dev/v1beta1 +kind: Pipeline +metadata: + name: result-test +spec: + description: >- + Generate a result of a certain length in a task and print the result in another task + params: + - name: RESULT_STRING_LENGTH + description: Length of string to generate for generate-result task + - name: RESULT_STRING_CHAR + description: Char to repeat in result string + default: '.' + tasks: + - name: generate-result + params: + - name: STRING_LENGTH + value: $(params.RESULT_STRING_LENGTH) + - name: STRING_CHAR + value: $(params.RESULT_STRING_CHAR) + taskRef: + kind: Task + name: generate-result + - name: print-result + params: + - name: TO_PRINT + value: $(tasks.generate-result.results.RESULT_STRING) + taskRef: + kind: Task + name: print-result +--- +apiVersion: tekton.dev/v1beta1 +kind: PipelineRun +metadata: + name: result-test-run +spec: + pipelineRef: + name: result-test + params: + - name: RESULT_STRING_LENGTH + value: "3000" diff --git a/pkg/pod/entrypoint.go b/pkg/pod/entrypoint.go index 2dc83c139ce..d1aaa30a06d 100644 --- a/pkg/pod/entrypoint.go +++ b/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. diff --git a/pkg/pod/pod.go b/pkg/pod/pod.go index c1ed7f12b02..b038942933a 100644 --- a/pkg/pod/pod.go +++ b/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 ( @@ -147,6 +146,10 @@ func (b *Builder) Build(ctx context.Context, taskRun *v1beta1.TaskRun, taskSpec return nil, err } + initContainers = []corev1.Container{ + entrypointInitContainer(b.Images.EntrypointImage, steps), + } + // Convert any steps with Script to command+args. // If any are found, append an init container to initialize scripts. if alphaAPIEnabled { @@ -154,15 +157,14 @@ 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) @@ -181,22 +183,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 +191,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 +401,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 generates 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 entrypointInitContainer(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 } diff --git a/pkg/pod/pod_test.go b/pkg/pod/pod_test.go index e573cebe606..bc7aee52359 100644 --- a/pkg/pod/pod_test.go +++ b/pkg/pod/pod_test.go @@ -85,13 +85,6 @@ func TestPodBuild(t *testing.T) { Name: "tekton-internal-secret-volume-multi-creds-9l9zj", VolumeSource: corev1.VolumeSource{Secret: &corev1.SecretVolumeSource{SecretName: "multi-creds"}}, } - placeToolsInit := corev1.Container{ - Name: "place-tools", - Image: images.EntrypointImage, - WorkingDir: "/", - Command: []string{"/ko-app/entrypoint", "cp", "/ko-app/entrypoint", "/tekton/bin/entrypoint"}, - VolumeMounts: []corev1.VolumeMount{binMount}, - } runtimeClassName := "gvisor" automountServiceAccountToken := false dnsPolicy := corev1.DNSNone @@ -121,7 +114,7 @@ func TestPodBuild(t *testing.T) { }, want: &corev1.PodSpec{ RestartPolicy: corev1.RestartPolicyNever, - InitContainers: []corev1.Container{placeToolsInit, tektonDirInit(images.EntrypointImage, []v1beta1.Step{{Name: "name"}})}, + InitContainers: []corev1.Container{entrypointInitContainer(images.EntrypointImage, []v1beta1.Step{{Name: "name"}})}, Containers: []corev1.Container{{ Name: "step-name", Image: "image", @@ -168,7 +161,7 @@ func TestPodBuild(t *testing.T) { }, want: &corev1.PodSpec{ RestartPolicy: corev1.RestartPolicyNever, - InitContainers: []corev1.Container{placeToolsInit, tektonDirInit(images.EntrypointImage, []v1beta1.Step{{Name: "name"}})}, + InitContainers: []corev1.Container{entrypointInitContainer(images.EntrypointImage, []v1beta1.Step{{Name: "name"}})}, Containers: []corev1.Container{{ Name: "step-name", Image: "image", @@ -213,7 +206,7 @@ func TestPodBuild(t *testing.T) { }, want: &corev1.PodSpec{ RestartPolicy: corev1.RestartPolicyNever, - InitContainers: []corev1.Container{placeToolsInit, tektonDirInit(images.EntrypointImage, []v1beta1.Step{{Name: "name"}})}, + InitContainers: []corev1.Container{entrypointInitContainer(images.EntrypointImage, []v1beta1.Step{{Name: "name"}})}, Containers: []corev1.Container{{ Name: "step-name", Image: "image", @@ -262,7 +255,7 @@ func TestPodBuild(t *testing.T) { want: &corev1.PodSpec{ ServiceAccountName: "service-account", RestartPolicy: corev1.RestartPolicyNever, - InitContainers: []corev1.Container{placeToolsInit, tektonDirInit(images.EntrypointImage, []v1beta1.Step{{Name: "name"}})}, + InitContainers: []corev1.Container{entrypointInitContainer(images.EntrypointImage, []v1beta1.Step{{Name: "name"}})}, Containers: []corev1.Container{{ Name: "step-name", Image: "image", @@ -329,7 +322,7 @@ func TestPodBuild(t *testing.T) { }, want: &corev1.PodSpec{ RestartPolicy: corev1.RestartPolicyNever, - InitContainers: []corev1.Container{placeToolsInit, tektonDirInit(images.EntrypointImage, []v1beta1.Step{{Name: "name"}})}, + InitContainers: []corev1.Container{entrypointInitContainer(images.EntrypointImage, []v1beta1.Step{{Name: "name"}})}, Containers: []corev1.Container{{ Name: "step-name", Image: "image", @@ -386,7 +379,7 @@ func TestPodBuild(t *testing.T) { }, want: &corev1.PodSpec{ RestartPolicy: corev1.RestartPolicyNever, - InitContainers: []corev1.Container{placeToolsInit, tektonDirInit(images.EntrypointImage, []v1beta1.Step{{Name: "a-very-very-long-character-step-name-to-trigger-max-len----and-invalid-characters"}})}, + InitContainers: []corev1.Container{entrypointInitContainer(images.EntrypointImage, []v1beta1.Step{{Name: "a-very-very-long-character-step-name-to-trigger-max-len----and-invalid-characters"}})}, Containers: []corev1.Container{{ Name: "step-a-very-very-long-character-step-name-to-trigger-max-len", // step name trimmed. Image: "image", @@ -428,7 +421,7 @@ func TestPodBuild(t *testing.T) { }, want: &corev1.PodSpec{ RestartPolicy: corev1.RestartPolicyNever, - InitContainers: []corev1.Container{placeToolsInit, tektonDirInit(images.EntrypointImage, []v1beta1.Step{{Name: "ends-with-invalid-%%__$$"}})}, + InitContainers: []corev1.Container{entrypointInitContainer(images.EntrypointImage, []v1beta1.Step{{Name: "ends-with-invalid-%%__$$"}})}, Containers: []corev1.Container{{ Name: "step-ends-with-invalid", // invalid suffix removed. Image: "image", @@ -472,8 +465,7 @@ func TestPodBuild(t *testing.T) { want: &corev1.PodSpec{ RestartPolicy: corev1.RestartPolicyNever, InitContainers: []corev1.Container{ - placeToolsInit, - tektonDirInit(images.EntrypointImage, []v1beta1.Step{{Name: "name"}}), + entrypointInitContainer(images.EntrypointImage, []v1beta1.Step{{Name: "name"}}), { Name: "working-dir-initializer", Image: images.WorkingDirInitImage, @@ -530,7 +522,7 @@ func TestPodBuild(t *testing.T) { wantAnnotations: map[string]string{}, want: &corev1.PodSpec{ RestartPolicy: corev1.RestartPolicyNever, - InitContainers: []corev1.Container{placeToolsInit, tektonDirInit(images.EntrypointImage, []v1beta1.Step{{Name: "primary-name"}})}, + InitContainers: []corev1.Container{entrypointInitContainer(images.EntrypointImage, []v1beta1.Step{{Name: "primary-name"}})}, Containers: []corev1.Container{{ Name: "step-primary-name", Image: "primary-image", @@ -585,8 +577,7 @@ func TestPodBuild(t *testing.T) { want: &corev1.PodSpec{ RestartPolicy: corev1.RestartPolicyNever, InitContainers: []corev1.Container{ - placeToolsInit, - tektonDirInit(images.EntrypointImage, []v1beta1.Step{{Name: "primary-name"}}), + entrypointInitContainer(images.EntrypointImage, []v1beta1.Step{{Name: "primary-name"}}), { Name: "place-scripts", Image: "busybox", @@ -655,7 +646,7 @@ _EOF_ wantAnnotations: map[string]string{}, // no ready annotations on pod create since sidecars are present want: &corev1.PodSpec{ RestartPolicy: corev1.RestartPolicyNever, - InitContainers: []corev1.Container{placeToolsInit, tektonDirInit(images.EntrypointImage, []v1beta1.Step{{Name: "primary-name"}})}, + InitContainers: []corev1.Container{entrypointInitContainer(images.EntrypointImage, []v1beta1.Step{{Name: "primary-name"}})}, Containers: []corev1.Container{{ Name: "step-primary-name", Image: "primary-image", @@ -714,7 +705,7 @@ _EOF_ }, want: &corev1.PodSpec{ RestartPolicy: corev1.RestartPolicyNever, - InitContainers: []corev1.Container{placeToolsInit, tektonDirInit(images.EntrypointImage, []v1beta1.Step{ + InitContainers: []corev1.Container{entrypointInitContainer(images.EntrypointImage, []v1beta1.Step{ {Name: "unnamed-0"}, {Name: "unnamed-1"}, })}, @@ -813,7 +804,7 @@ _EOF_ }, want: &corev1.PodSpec{ RestartPolicy: corev1.RestartPolicyNever, - InitContainers: []corev1.Container{placeToolsInit, tektonDirInit(images.EntrypointImage, []v1beta1.Step{ + InitContainers: []corev1.Container{entrypointInitContainer(images.EntrypointImage, []v1beta1.Step{ {Name: "step1"}, })}, Containers: []corev1.Container{{ @@ -882,7 +873,7 @@ _EOF_ }, want: &corev1.PodSpec{ RestartPolicy: corev1.RestartPolicyNever, - InitContainers: []corev1.Container{placeToolsInit, tektonDirInit(images.EntrypointImage, []v1beta1.Step{ + InitContainers: []corev1.Container{entrypointInitContainer(images.EntrypointImage, []v1beta1.Step{ {Name: "step1"}, })}, Containers: []corev1.Container{{ @@ -954,7 +945,7 @@ _EOF_ wantAnnotations: map[string]string{}, want: &corev1.PodSpec{ RestartPolicy: corev1.RestartPolicyNever, - InitContainers: []corev1.Container{placeToolsInit, tektonDirInit(images.EntrypointImage, []v1beta1.Step{{Name: "primary-name"}})}, + InitContainers: []corev1.Container{entrypointInitContainer(images.EntrypointImage, []v1beta1.Step{{Name: "primary-name"}})}, Containers: []corev1.Container{{ Name: "step-primary-name", Image: "primary-image", @@ -1020,8 +1011,7 @@ print("Hello from Python")`, want: &corev1.PodSpec{ RestartPolicy: corev1.RestartPolicyNever, InitContainers: []corev1.Container{ - placeToolsInit, - tektonDirInit(images.EntrypointImage, []v1beta1.Step{ + entrypointInitContainer(images.EntrypointImage, []v1beta1.Step{ {Name: "one"}, {Name: "two"}, {Name: "regular-step"}, @@ -1147,8 +1137,7 @@ _EOF_ }, want: &corev1.PodSpec{ RestartPolicy: corev1.RestartPolicyNever, - InitContainers: []corev1.Container{placeToolsInit, - tektonDirInit(images.EntrypointImage, []v1beta1.Step{{Name: "one"}}), + InitContainers: []corev1.Container{entrypointInitContainer(images.EntrypointImage, []v1beta1.Step{{Name: "one"}}), { Name: "place-scripts", Image: images.ShellImage, @@ -1211,7 +1200,7 @@ _EOF_ }, want: &corev1.PodSpec{ RestartPolicy: corev1.RestartPolicyNever, - InitContainers: []corev1.Container{placeToolsInit, tektonDirInit(images.EntrypointImage, []v1beta1.Step{{Name: "schedule-me"}})}, + InitContainers: []corev1.Container{entrypointInitContainer(images.EntrypointImage, []v1beta1.Step{{Name: "schedule-me"}})}, SchedulerName: "there-scheduler", Volumes: append(implicitVolumes, binVolume, runVolume(0), downwardVolume, corev1.Volume{ Name: "tekton-creds-init-home-0", @@ -1262,7 +1251,7 @@ _EOF_ }, want: &corev1.PodSpec{ RestartPolicy: corev1.RestartPolicyNever, - InitContainers: []corev1.Container{placeToolsInit, tektonDirInit(images.EntrypointImage, []v1beta1.Step{{Name: "image-pull"}})}, + InitContainers: []corev1.Container{entrypointInitContainer(images.EntrypointImage, []v1beta1.Step{{Name: "image-pull"}})}, Volumes: append(implicitVolumes, binVolume, runVolume(0), downwardVolume, corev1.Volume{ Name: "tekton-creds-init-home-0", VolumeSource: corev1.VolumeSource{EmptyDir: &corev1.EmptyDirVolumeSource{Medium: corev1.StorageMediumMemory}}, @@ -1313,7 +1302,7 @@ _EOF_ }, want: &corev1.PodSpec{ RestartPolicy: corev1.RestartPolicyNever, - InitContainers: []corev1.Container{placeToolsInit, tektonDirInit(images.EntrypointImage, []v1beta1.Step{{Name: "host-aliases"}})}, + InitContainers: []corev1.Container{entrypointInitContainer(images.EntrypointImage, []v1beta1.Step{{Name: "host-aliases"}})}, Volumes: append(implicitVolumes, binVolume, runVolume(0), downwardVolume, corev1.Volume{ Name: "tekton-creds-init-home-0", VolumeSource: corev1.VolumeSource{EmptyDir: &corev1.EmptyDirVolumeSource{Medium: corev1.StorageMediumMemory}}, @@ -1362,7 +1351,7 @@ _EOF_ }, want: &corev1.PodSpec{ RestartPolicy: corev1.RestartPolicyNever, - InitContainers: []corev1.Container{placeToolsInit, tektonDirInit(images.EntrypointImage, []v1beta1.Step{{Name: "use-my-hostNetwork"}})}, + InitContainers: []corev1.Container{entrypointInitContainer(images.EntrypointImage, []v1beta1.Step{{Name: "use-my-hostNetwork"}})}, HostNetwork: true, Volumes: append(implicitVolumes, binVolume, runVolume(0), downwardVolume, corev1.Volume{ Name: "tekton-creds-init-home-0", @@ -1406,7 +1395,7 @@ _EOF_ }, want: &corev1.PodSpec{ RestartPolicy: corev1.RestartPolicyNever, - InitContainers: []corev1.Container{placeToolsInit, tektonDirInit(images.EntrypointImage, []v1beta1.Step{{Name: "name"}})}, + InitContainers: []corev1.Container{entrypointInitContainer(images.EntrypointImage, []v1beta1.Step{{Name: "name"}})}, Containers: []corev1.Container{{ Name: "step-name", Image: "image", @@ -1454,7 +1443,7 @@ _EOF_ }, want: &corev1.PodSpec{ RestartPolicy: corev1.RestartPolicyNever, - InitContainers: []corev1.Container{placeToolsInit, tektonDirInit(images.EntrypointImage, []v1beta1.Step{{Name: "name"}})}, + InitContainers: []corev1.Container{entrypointInitContainer(images.EntrypointImage, []v1beta1.Step{{Name: "name"}})}, Containers: []corev1.Container{{ Name: "step-name", Image: "image", @@ -1501,7 +1490,7 @@ _EOF_ }, want: &corev1.PodSpec{ RestartPolicy: corev1.RestartPolicyNever, - InitContainers: []corev1.Container{placeToolsInit, tektonDirInit(images.EntrypointImage, []v1beta1.Step{{Name: "name"}})}, + InitContainers: []corev1.Container{entrypointInitContainer(images.EntrypointImage, []v1beta1.Step{{Name: "name"}})}, Containers: []corev1.Container{{ Name: "step-name", Image: "image", @@ -1541,7 +1530,7 @@ _EOF_ }, want: &corev1.PodSpec{ RestartPolicy: corev1.RestartPolicyNever, - InitContainers: []corev1.Container{placeToolsInit, tektonDirInit(images.EntrypointImage, []v1beta1.Step{{Name: "name"}})}, + InitContainers: []corev1.Container{entrypointInitContainer(images.EntrypointImage, []v1beta1.Step{{Name: "name"}})}, Containers: []corev1.Container{{ Name: "step-name", Image: "image", @@ -1591,7 +1580,7 @@ _EOF_ }, want: &corev1.PodSpec{ RestartPolicy: corev1.RestartPolicyNever, - InitContainers: []corev1.Container{placeToolsInit, tektonDirInit(images.EntrypointImage, []v1beta1.Step{{Name: "name"}})}, + InitContainers: []corev1.Container{entrypointInitContainer(images.EntrypointImage, []v1beta1.Step{{Name: "name"}})}, Containers: []corev1.Container{{ Name: "step-name", Image: "image", @@ -1657,7 +1646,7 @@ _EOF_ }, want: &corev1.PodSpec{ RestartPolicy: corev1.RestartPolicyNever, - InitContainers: []corev1.Container{placeToolsInit, tektonDirInit(images.EntrypointImage, []v1beta1.Step{{Name: "name"}})}, + InitContainers: []corev1.Container{entrypointInitContainer(images.EntrypointImage, []v1beta1.Step{{Name: "name"}})}, Containers: []corev1.Container{{ Name: "step-name", Image: "image", @@ -1702,7 +1691,7 @@ _EOF_ wantPodName: "task-run-0123456789-01234560d38957287bb0283c59440df14069f59-pod", want: &corev1.PodSpec{ RestartPolicy: corev1.RestartPolicyNever, - InitContainers: []corev1.Container{placeToolsInit, tektonDirInit(images.EntrypointImage, []v1beta1.Step{{Name: "name"}})}, + InitContainers: []corev1.Container{entrypointInitContainer(images.EntrypointImage, []v1beta1.Step{{Name: "name"}})}, Containers: []corev1.Container{{ Name: "step-name", Image: "image", @@ -1824,14 +1813,6 @@ _EOF_ } func TestPodBuildwithAlphaAPIEnabled(t *testing.T) { - placeToolsInit := corev1.Container{ - Name: "place-tools", - Image: images.EntrypointImage, - WorkingDir: "/", - Command: []string{"/ko-app/entrypoint", "cp", "/ko-app/entrypoint", "/tekton/bin/entrypoint"}, - VolumeMounts: []corev1.VolumeMount{binMount}, - } - for _, c := range []struct { desc string trs v1beta1.TaskRunSpec @@ -1855,7 +1836,7 @@ func TestPodBuildwithAlphaAPIEnabled(t *testing.T) { }, want: &corev1.PodSpec{ RestartPolicy: corev1.RestartPolicyNever, - InitContainers: []corev1.Container{placeToolsInit, tektonDirInit(images.EntrypointImage, []v1beta1.Step{{Name: "name"}})}, + InitContainers: []corev1.Container{entrypointInitContainer(images.EntrypointImage, []v1beta1.Step{{Name: "name"}})}, Containers: []corev1.Container{{ Name: "step-name", Image: "image", @@ -2071,3 +2052,46 @@ func TestShouldAddReadyAnnotationonPodCreate(t *testing.T) { }) } } + +func TestPrepareInitContainers(t *testing.T) { + tcs := []struct { + name string + steps []v1beta1.Step + want corev1.Container + featureFlags map[string]string + }{{ + name: "nothing-special", + steps: []v1beta1.Step{{ + Name: "foo", + }}, + want: corev1.Container{ + Name: "prepare", + Image: images.EntrypointImage, + WorkingDir: "/", + Command: []string{"/ko-app/entrypoint", "init", "/ko-app/entrypoint", entrypointBinary, "step-foo"}, + VolumeMounts: []corev1.VolumeMount{binMount, internalStepsMount}, + }, + }, { + name: "nothing-special-two-steps", + steps: []v1beta1.Step{{ + Name: "foo", + }, { + Name: "bar", + }}, + want: corev1.Container{ + Name: "prepare", + Image: images.EntrypointImage, + WorkingDir: "/", + Command: []string{"/ko-app/entrypoint", "init", "/ko-app/entrypoint", entrypointBinary, "step-foo", "step-bar"}, + VolumeMounts: []corev1.VolumeMount{binMount, internalStepsMount}, + }, + }} + for _, tc := range tcs { + t.Run(tc.name, func(t *testing.T) { + container := entrypointInitContainer(images.EntrypointImage, tc.steps) + if d := cmp.Diff(tc.want, container); d != "" { + t.Errorf("Diff %s", diff.PrintWantGot(d)) + } + }) + } +} diff --git a/pkg/reconciler/taskrun/taskrun_test.go b/pkg/reconciler/taskrun/taskrun_test.go index 8cb6457bd02..0138deb56b6 100644 --- a/pkg/reconciler/taskrun/taskrun_test.go +++ b/pkg/reconciler/taskrun/taskrun_test.go @@ -50,7 +50,6 @@ import ( "github.com/tektoncd/pipeline/test/names" "github.com/tektoncd/pipeline/test/parse" corev1 "k8s.io/api/core/v1" - v1 "k8s.io/api/core/v1" k8sapierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -355,6 +354,10 @@ var ( EmptyDir: &corev1.EmptyDirVolumeSource{}, }, } + internalStepsMount = corev1.VolumeMount{ + Name: "tekton-internal-steps", + MountPath: pipeline.StepsDir, + } workspaceVolume = corev1.Volume{ Name: "tekton-internal-workspace", @@ -394,21 +397,24 @@ var ( }, } - placeToolsInitContainer = corev1.Container{ - Command: []string{"/ko-app/entrypoint", "cp", "/ko-app/entrypoint", entrypointLocation}, + fakeVersion string + gitResourceSecurityContext = &corev1.SecurityContext{ + RunAsUser: ptr.Int64(0), + } +) + +func placeToolsInitContainer(steps []string) corev1.Container { + return corev1.Container{ + Command: append([]string{"/ko-app/entrypoint", "init", "/ko-app/entrypoint", entrypointLocation}, steps...), VolumeMounts: []corev1.VolumeMount{{ MountPath: "/tekton/bin", Name: "tekton-internal-bin", - }}, + }, internalStepsMount}, WorkingDir: "/", - Name: "place-tools", + Name: "prepare", Image: "override-with-entrypoint:latest", } - fakeVersion string - gitResourceSecurityContext = &corev1.SecurityContext{ - RunAsUser: ptr.Int64(0), - } -) +} var testClock = clock.NewFakePassiveClock(now) @@ -4083,6 +4089,10 @@ type stepForExpectedPod struct { } func expectedPod(podName, taskName, taskRunName, ns, saName string, isClusterTask bool, extraVolumes []corev1.Volume, steps []stepForExpectedPod) *corev1.Pod { + stepNames := make([]string, 0, len(steps)) + for _, s := range steps { + stepNames = append(stepNames, fmt.Sprintf("step-%s", s.name)) + } p := &corev1.Pod{ ObjectMeta: podObjectMeta(podName, taskName, taskRunName, ns, isClusterTask), Spec: corev1.PodSpec{ @@ -4094,25 +4104,13 @@ func expectedPod(podName, taskName, taskRunName, ns, saName string, isClusterTas binVolume, downwardVolume, }, - InitContainers: []corev1.Container{placeToolsInitContainer}, + InitContainers: []corev1.Container{placeToolsInitContainer(stepNames)}, RestartPolicy: corev1.RestartPolicyNever, ActiveDeadlineSeconds: &defaultActiveDeadlineSeconds, ServiceAccountName: saName, }, } - stepNames := make([]string, 0, len(steps)) - for _, s := range steps { - stepNames = append(stepNames, fmt.Sprintf("step-%s", s.name)) - } - p.Spec.InitContainers = []corev1.Container{placeToolsInitContainer, { - Name: "step-init", - Image: images.EntrypointImage, - Command: append([]string{"/ko-app/entrypoint", "step-init"}, stepNames...), - WorkingDir: "/", - VolumeMounts: []v1.VolumeMount{{Name: "tekton-internal-steps", MountPath: "/tekton/steps"}}, - }} - for idx, s := range steps { p.Spec.Volumes = append(p.Spec.Volumes, corev1.Volume{ Name: fmt.Sprintf("tekton-creds-init-home-%d", idx),