Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

pkg/pod: simplify orderContainers returns #4244

Merged
merged 1 commit into from
Sep 21, 2021
Merged

pkg/pod: simplify orderContainers returns #4244

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 5 additions & 20 deletions pkg/pod/entrypoint.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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...)
Expand Down Expand Up @@ -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 {
Expand Down
23 changes: 6 additions & 17 deletions pkg/pod/entrypoint_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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)
}
Expand Down
18 changes: 15 additions & 3 deletions pkg/pod/pod.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down