From 3a3c91aa2ee03f2af90e2ab05c5053f1899a2fd7 Mon Sep 17 00:00:00 2001 From: Vincent Demeester Date: Tue, 21 Sep 2021 13:19:07 +0200 Subject: [PATCH] pkg/pod: simplify orderContainers returns This simplifies a bit the `orderContainers` function to remove an unecessary return that can be extracted outside. It simplifies the functino a bit reduces its responsability. Signed-off-by: Vincent Demeester --- pkg/pod/entrypoint.go | 25 +++++-------------------- pkg/pod/entrypoint_test.go | 23 ++++++----------------- pkg/pod/pod.go | 18 +++++++++++++++--- 3 files changed, 26 insertions(+), 40 deletions(-) diff --git a/pkg/pod/entrypoint.go b/pkg/pod/entrypoint.go index b39952d329a..e6b80241226 100644 --- a/pkg/pod/entrypoint.go +++ b/pkg/pod/entrypoint.go @@ -103,30 +103,15 @@ var ( ) // orderContainers returns the specified steps, modified so that they are -// executed in order by overriding the entrypoint binary. It also returns the -// init container that places the entrypoint binary pulled from the -// entrypointImage. +// executed in order by overriding the entrypoint binary. // // Containers must have Command specified; if the user didn't specify a // command, we must have fetched the image's ENTRYPOINT before calling this // method, using entrypoint_lookup.go. // Additionally, Step timeouts are added as entrypoint flag. -func orderContainers(entrypointImage string, commonExtraEntrypointArgs []string, steps []corev1.Container, taskSpec *v1beta1.TaskSpec, breakpointConfig *v1beta1.TaskRunDebug) (corev1.Container, []corev1.Container, error) { - initContainer := corev1.Container{ - Name: "place-tools", - Image: 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}, - } - +func orderContainers(commonExtraEntrypointArgs []string, steps []corev1.Container, taskSpec *v1beta1.TaskSpec, breakpointConfig *v1beta1.TaskRunDebug) ([]corev1.Container, error) { if len(steps) == 0 { - return corev1.Container{}, nil, errors.New("No steps specified") + return nil, errors.New("No steps specified") } for i, s := range steps { @@ -169,7 +154,7 @@ func orderContainers(entrypointImage string, commonExtraEntrypointArgs []string, cmd, args := s.Command, s.Args if len(cmd) == 0 { - return corev1.Container{}, nil, fmt.Errorf("Step %d did not specify command", i) + return nil, fmt.Errorf("Step %d did not specify command", i) } if len(cmd) > 1 { args = append(cmd[1:], args...) @@ -197,7 +182,7 @@ func orderContainers(entrypointImage string, commonExtraEntrypointArgs []string, // Mount the Downward volume into the first step container. steps[0].VolumeMounts = append(steps[0].VolumeMounts, downwardMount) - return initContainer, steps, nil + return steps, nil } func resultArgument(steps []corev1.Container, results []v1beta1.TaskResult) []string { diff --git a/pkg/pod/entrypoint_test.go b/pkg/pod/entrypoint_test.go index 7fae5477fbe..cd912edfe21 100644 --- a/pkg/pod/entrypoint_test.go +++ b/pkg/pod/entrypoint_test.go @@ -95,24 +95,13 @@ func TestOrderContainers(t *testing.T) { VolumeMounts: []corev1.VolumeMount{binROMount, runMount}, TerminationMessagePath: "/tekton/termination", }} - gotInit, got, err := orderContainers(images.EntrypointImage, []string{}, steps, nil, nil) + got, err := orderContainers([]string{}, steps, nil, nil) if err != nil { t.Fatalf("orderContainers: %v", err) } if d := cmp.Diff(want, got); d != "" { t.Errorf("Diff %s", diff.PrintWantGot(d)) } - - wantInit := corev1.Container{ - Name: "place-tools", - Image: images.EntrypointImage, - WorkingDir: "/", - Command: []string{"/ko-app/entrypoint", "cp", "/ko-app/entrypoint", entrypointBinary}, - VolumeMounts: []corev1.VolumeMount{binMount}, - } - if d := cmp.Diff(wantInit, gotInit); d != "" { - t.Errorf("Init Container Diff %s", diff.PrintWantGot(d)) - } } func TestOrderContainersWithDebugOnFailure(t *testing.T) { @@ -141,7 +130,7 @@ func TestOrderContainersWithDebugOnFailure(t *testing.T) { taskRunDebugConfig := &v1beta1.TaskRunDebug{ Breakpoint: []string{"onFailure"}, } - _, got, err := orderContainers(images.EntrypointImage, []string{}, steps, nil, taskRunDebugConfig) + got, err := orderContainers([]string{}, steps, nil, taskRunDebugConfig) if err != nil { t.Fatalf("orderContainers: %v", err) } @@ -223,7 +212,7 @@ func TestEntryPointResults(t *testing.T) { VolumeMounts: []corev1.VolumeMount{binROMount, runMount}, TerminationMessagePath: "/tekton/termination", }} - _, got, err := orderContainers(images.EntrypointImage, []string{}, steps, &taskSpec, nil) + got, err := orderContainers([]string{}, steps, &taskSpec, nil) if err != nil { t.Fatalf("orderContainers: %v", err) } @@ -265,7 +254,7 @@ func TestEntryPointResultsSingleStep(t *testing.T) { VolumeMounts: []corev1.VolumeMount{binROMount, runMount, downwardMount}, TerminationMessagePath: "/tekton/termination", }} - _, got, err := orderContainers(images.EntrypointImage, []string{}, steps, &taskSpec, nil) + got, err := orderContainers([]string{}, steps, &taskSpec, nil) if err != nil { t.Fatalf("orderContainers: %v", err) } @@ -303,7 +292,7 @@ func TestEntryPointSingleResultsSingleStep(t *testing.T) { VolumeMounts: []corev1.VolumeMount{binROMount, runMount, downwardMount}, TerminationMessagePath: "/tekton/termination", }} - _, got, err := orderContainers(images.EntrypointImage, []string{}, steps, &taskSpec, nil) + got, err := orderContainers([]string{}, steps, &taskSpec, nil) if err != nil { t.Fatalf("orderContainers: %v", err) } @@ -363,7 +352,7 @@ func TestEntryPointOnError(t *testing.T) { VolumeMounts: []corev1.VolumeMount{binROMount, runMount}, TerminationMessagePath: "/tekton/termination", }} - _, got, err := orderContainers(images.EntrypointImage, []string{}, steps, &taskSpec, nil) + got, err := orderContainers([]string{}, steps, &taskSpec, nil) if err != nil { t.Fatalf("orderContainers: %v", err) } diff --git a/pkg/pod/pod.go b/pkg/pod/pod.go index 04edb48693d..0e0616851bb 100644 --- a/pkg/pod/pod.go +++ b/pkg/pod/pod.go @@ -105,7 +105,6 @@ type Transformer func(*corev1.Pod) (*corev1.Pod, error) func (b *Builder) Build(ctx context.Context, taskRun *v1beta1.TaskRun, taskSpec v1beta1.TaskSpec, transformers ...Transformer) (*corev1.Pod, error) { var ( scriptsInit *corev1.Container - entrypointInit corev1.Container initContainers, stepContainers, sidecarContainers []corev1.Container volumes []corev1.Volume volumeMounts []corev1.VolumeMount @@ -171,10 +170,23 @@ func (b *Builder) Build(ctx context.Context, taskRun *v1beta1.TaskRun, taskSpec // 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 { - entrypointInit, stepContainers, err = orderContainers(b.Images.EntrypointImage, credEntrypointArgs, stepContainers, &taskSpec, taskRun.Spec.Debug) + stepContainers, err = orderContainers(credEntrypointArgs, stepContainers, &taskSpec, taskRun.Spec.Debug) } else { - entrypointInit, stepContainers, err = orderContainers(b.Images.EntrypointImage, credEntrypointArgs, stepContainers, &taskSpec, nil) + stepContainers, err = orderContainers(credEntrypointArgs, stepContainers, &taskSpec, nil) } if err != nil { return nil, err