Skip to content

Commit

Permalink
Merge place-tools and step-init together
Browse files Browse the repository at this point in the history
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 <[email protected]>
Signed-off-by: Vincent Demeester <[email protected]>
  • Loading branch information
vdemeester authored and tekton-robot committed May 4, 2022
1 parent 50d89bb commit 7059cfd
Show file tree
Hide file tree
Showing 8 changed files with 327 additions and 110 deletions.
13 changes: 13 additions & 0 deletions cmd/entrypoint/subcommands/init.go
Original file line number Diff line number Diff line change
@@ -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)
}
82 changes: 82 additions & 0 deletions cmd/entrypoint/subcommands/init_test.go
Original file line number Diff line number Diff line change
@@ -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)
}
})
}
}
12 changes: 12 additions & 0 deletions cmd/entrypoint/subcommands/subcommands.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
95 changes: 95 additions & 0 deletions examples/v1beta1/pipelineruns/4808-regression.yaml
Original file line number Diff line number Diff line change
@@ -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"
5 changes: 5 additions & 0 deletions pkg/pod/entrypoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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.
Expand Down
66 changes: 27 additions & 39 deletions pkg/pod/pod.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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 (
Expand Down Expand Up @@ -147,22 +146,25 @@ 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 {
scriptsInit, stepContainers, sidecarContainers = convertScripts(b.Images.ShellImage, b.Images.ShellImageWin, steps, sidecars, taskRun.Spec.Debug)
} 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)
Expand All @@ -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 {
Expand All @@ -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.
Expand Down Expand Up @@ -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
}
Loading

0 comments on commit 7059cfd

Please sign in to comment.