From 62f133818832748bcc16d7ef2da6e90f5bf18166 Mon Sep 17 00:00:00 2001 From: Vincent Demeester Date: Wed, 4 May 2022 15:15:26 +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 +++ pkg/pod/entrypoint.go | 5 + pkg/pod/pod.go | 68 ++++++------ pkg/pod/pod_test.go | 120 +++++++++++++--------- pkg/reconciler/taskrun/taskrun_test.go | 44 ++++---- 7 files changed, 234 insertions(+), 110 deletions(-) create mode 100644 cmd/entrypoint/subcommands/init.go create mode 100644 cmd/entrypoint/subcommands/init_test.go 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/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..5a91987dce8 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 ( @@ -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 } diff --git a/pkg/pod/pod_test.go b/pkg/pod/pod_test.go index e573cebe606..282ad9f1254 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{prepareInitContainer(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{prepareInitContainer(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{prepareInitContainer(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{prepareInitContainer(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{prepareInitContainer(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{prepareInitContainer(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{prepareInitContainer(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"}}), + prepareInitContainer(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{prepareInitContainer(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"}}), + prepareInitContainer(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{prepareInitContainer(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{prepareInitContainer(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{prepareInitContainer(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{prepareInitContainer(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{prepareInitContainer(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{ + prepareInitContainer(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{prepareInitContainer(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{prepareInitContainer(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{prepareInitContainer(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{prepareInitContainer(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{prepareInitContainer(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{prepareInitContainer(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{prepareInitContainer(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{prepareInitContainer(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{prepareInitContainer(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{prepareInitContainer(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{prepareInitContainer(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{prepareInitContainer(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{prepareInitContainer(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 := prepareInitContainer(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),