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 <andrew.bayer@gmail.com>
Signed-off-by: Vincent Demeester <vdemeest@redhat.com>
  • Loading branch information
vdemeester committed May 4, 2022
1 parent 2ffa9a5 commit 2b67426
Show file tree
Hide file tree
Showing 7 changed files with 234 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[2:]
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
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
68 changes: 29 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 @@ -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{}

Expand All @@ -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 {
Expand All @@ -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.
Expand Down Expand Up @@ -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
}
Loading

0 comments on commit 2b67426

Please sign in to comment.