From d2e9d01bbe65925cee33972bc96bf2fded0b0b1d Mon Sep 17 00:00:00 2001 From: Vincent Demeester Date: Wed, 13 Oct 2021 10:52:02 +0200 Subject: [PATCH] =?UTF-8?q?pkg/pod:=20extracting=20function=20from=20the?= =?UTF-8?q?=20package=E2=80=A6?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit … to smaller packages. Signed-off-by: Vincent Demeester --- pkg/internal/sidecars/sidecars.go | 114 +++++++++++++++++ pkg/internal/sidecars/sidecars_test.go | 146 ++++++++++++++++++++++ pkg/pod/entrypoint.go | 73 ++--------- pkg/pod/entrypoint_test.go | 114 ----------------- pkg/pod/pod.go | 14 ++- pkg/pod/pod_test.go | 27 ++-- pkg/pod/{ => script}/script.go | 45 ++++--- pkg/pod/{ => script}/script_test.go | 72 ++++++----- pkg/pod/{ => script}/scripts_constants.go | 4 +- pkg/pod/{ => status}/status.go | 41 ++---- pkg/pod/{ => status}/status_test.go | 3 +- pkg/reconciler/taskrun/taskrun.go | 42 ++++--- pkg/reconciler/taskrun/taskrun_test.go | 29 ++--- test/tektonbundles_test.go | 7 +- 14 files changed, 415 insertions(+), 316 deletions(-) create mode 100644 pkg/internal/sidecars/sidecars.go create mode 100644 pkg/internal/sidecars/sidecars_test.go rename pkg/pod/{ => script}/script.go (87%) rename pkg/pod/{ => script}/script_test.go (88%) rename pkg/pod/{ => script}/scripts_constants.go (95%) rename pkg/pod/{ => status}/status.go (94%) rename pkg/pod/{ => status}/status_test.go (99%) diff --git a/pkg/internal/sidecars/sidecars.go b/pkg/internal/sidecars/sidecars.go new file mode 100644 index 00000000000..ae760f21826 --- /dev/null +++ b/pkg/internal/sidecars/sidecars.go @@ -0,0 +1,114 @@ +/* +Copyright 2021 The Tekton Authors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ +package sidecars + +import ( + "context" + "fmt" + "strings" + + "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1" + "github.com/tektoncd/pipeline/pkg/pod" + corev1 "k8s.io/api/core/v1" + k8serrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/kubernetes" +) + +const ( + sidecarPrefix = "sidecar-" +) + +// Ready returns true if all of the Pod's sidecars are Ready or +// Terminated. +func Ready(podStatus corev1.PodStatus) bool { + if podStatus.Phase != corev1.PodRunning { + return false + } + for _, s := range podStatus.ContainerStatuses { + // If the step indicates that it's a step, skip it. + // An injected sidecar might not have the "sidecar-" prefix, so + // we can't just look for that prefix, we need to look at any + // non-step container. + if pod.IsContainerStep(s.Name) { + continue + } + if s.State.Running != nil && s.Ready { + continue + } + if s.State.Terminated != nil { + continue + } + return false + } + return true +} + +// Stop updates sidecar containers in the Pod to a nop image, which +// exits successfully immediately. +func Stop(ctx context.Context, nopImage string, kubeclient kubernetes.Interface, namespace, name string) (*corev1.Pod, error) { + newPod, err := kubeclient.CoreV1().Pods(namespace).Get(ctx, name, metav1.GetOptions{}) + if k8serrors.IsNotFound(err) { + // return NotFound as-is, since the K8s error checks don't handle wrapping. + return nil, err + } else if err != nil { + return nil, fmt.Errorf("error getting Pod %q when stopping sidecars: %w", name, err) + } + + updated := false + if newPod.Status.Phase == corev1.PodRunning { + for _, s := range newPod.Status.ContainerStatuses { + // Stop any running container that isn't a step. + // An injected sidecar container might not have the + // "sidecar-" prefix, so we can't just look for that + // prefix. + if !pod.IsContainerStep(s.Name) && s.State.Running != nil { + for j, c := range newPod.Spec.Containers { + if c.Name == s.Name && c.Image != nopImage { + updated = true + newPod.Spec.Containers[j].Image = nopImage + } + } + } + } + } + if updated { + if newPod, err = kubeclient.CoreV1().Pods(newPod.Namespace).Update(ctx, newPod, metav1.UpdateOptions{}); err != nil { + return nil, fmt.Errorf("error stopping sidecars of Pod %q: %w", name, err) + } + } + return newPod, nil +} + +// IsRunning determines if any SidecarStatus on a TaskRun +// is still running. +func IsRunning(tr *v1beta1.TaskRun) bool { + for _, sidecar := range tr.Status.Sidecars { + if sidecar.Terminated == nil { + return true + } + } + + return false +} + +// IsContainerSidecar returns true if the container name indicates that it +// represents a sidecar. +func IsContainerSidecar(name string) bool { return strings.HasPrefix(name, sidecarPrefix) } + +// TrimPrefix returns the container name, stripped of its sidecar +// prefix. +func TrimPrefix(name string) string { return strings.TrimPrefix(name, sidecarPrefix) } diff --git a/pkg/internal/sidecars/sidecars_test.go b/pkg/internal/sidecars/sidecars_test.go new file mode 100644 index 00000000000..f06ffae9f77 --- /dev/null +++ b/pkg/internal/sidecars/sidecars_test.go @@ -0,0 +1,146 @@ +/* +Copyright 2021 The Tekton Authors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ +package sidecars_test + +import ( + "context" + "testing" + "time" + + "github.com/google/go-cmp/cmp" + "github.com/tektoncd/pipeline/pkg/internal/sidecars" + "github.com/tektoncd/pipeline/test/diff" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + fakek8s "k8s.io/client-go/kubernetes/fake" +) + +const ( + stepPrefix = "step-" // FIXME(vdemeester) deduplicate this + sidecarPrefix = "sidecar-" // FIXME(vdemeester) deduplicate this + nopImage = "nop-image" +) + +// TestStopSidecars tests stopping sidecars by updating their images to a nop +// image. +func TestStopSidecars(t *testing.T) { + stepContainer := corev1.Container{ + Name: stepPrefix + "my-step", + Image: "foo", + } + sidecarContainer := corev1.Container{ + Name: sidecarPrefix + "my-sidecar", + Image: "original-image", + } + stoppedSidecarContainer := corev1.Container{ + Name: sidecarContainer.Name, + Image: nopImage, + } + + // This is a container that doesn't start with the "sidecar-" prefix, + // which indicates it was injected into the Pod by a Mutating Webhook + // Admission Controller. Injected sidecars should also be stopped if + // they're running. + injectedSidecar := corev1.Container{ + Name: "injected", + Image: "original-injected-image", + } + stoppedInjectedSidecar := corev1.Container{ + Name: injectedSidecar.Name, + Image: nopImage, + } + + for _, c := range []struct { + desc string + pod corev1.Pod + wantContainers []corev1.Container + }{{ + desc: "Running sidecars (incl injected) should be stopped", + pod: corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pod", + }, + Spec: corev1.PodSpec{ + Containers: []corev1.Container{stepContainer, sidecarContainer, injectedSidecar}, + }, + Status: corev1.PodStatus{ + Phase: corev1.PodRunning, + ContainerStatuses: []corev1.ContainerStatus{{ + // Step state doesn't matter. + }, { + Name: sidecarContainer.Name, + // Sidecar is running. + State: corev1.ContainerState{Running: &corev1.ContainerStateRunning{StartedAt: metav1.NewTime(time.Now())}}, + }, { + Name: injectedSidecar.Name, + // Injected sidecar is running. + State: corev1.ContainerState{Running: &corev1.ContainerStateRunning{StartedAt: metav1.NewTime(time.Now())}}, + }}, + }, + }, + wantContainers: []corev1.Container{stepContainer, stoppedSidecarContainer, stoppedInjectedSidecar}, + }, { + desc: "Pending Pod should not be updated", + pod: corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{Name: "test-pod"}, + Spec: corev1.PodSpec{ + Containers: []corev1.Container{stepContainer, sidecarContainer}, + }, + Status: corev1.PodStatus{ + Phase: corev1.PodPending, + // Container states don't matter. + }, + }, + wantContainers: []corev1.Container{stepContainer, sidecarContainer}, + }, { + desc: "Non-Running sidecars should not be stopped", + pod: corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pod", + }, + Spec: corev1.PodSpec{ + Containers: []corev1.Container{stepContainer, sidecarContainer, injectedSidecar}, + }, + Status: corev1.PodStatus{ + Phase: corev1.PodRunning, + ContainerStatuses: []corev1.ContainerStatus{{ + // Step state doesn't matter. + }, { + Name: sidecarContainer.Name, + // Sidecar is waiting. + State: corev1.ContainerState{Waiting: &corev1.ContainerStateWaiting{}}, + }, { + Name: injectedSidecar.Name, + // Injected sidecar is waiting. + State: corev1.ContainerState{Waiting: &corev1.ContainerStateWaiting{}}, + }}, + }, + }, + wantContainers: []corev1.Container{stepContainer, sidecarContainer, injectedSidecar}, + }} { + t.Run(c.desc, func(t *testing.T) { + ctx := context.Background() + ctx, cancel := context.WithCancel(ctx) + defer cancel() + kubeclient := fakek8s.NewSimpleClientset(&c.pod) + if got, err := sidecars.Stop(ctx, nopImage, kubeclient, c.pod.Namespace, c.pod.Name); err != nil { + t.Errorf("error stopping sidecar: %v", err) + } else if d := cmp.Diff(c.wantContainers, got.Spec.Containers); d != "" { + t.Errorf("Containers Diff %s", diff.PrintWantGot(d)) + } + }) + } +} diff --git a/pkg/pod/entrypoint.go b/pkg/pod/entrypoint.go index b9cec5e5d85..9de2f0f4317 100644 --- a/pkg/pod/entrypoint.go +++ b/pkg/pod/entrypoint.go @@ -28,21 +28,21 @@ import ( "github.com/tektoncd/pipeline/pkg/apis/pipeline" "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1" + "github.com/tektoncd/pipeline/pkg/pod/script" "gomodules.xyz/jsonpatch/v2" corev1 "k8s.io/api/core/v1" - k8serrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/kubernetes" ) const ( - binVolumeName = "tekton-internal-bin" - binDir = "/tekton/bin" + binVolumeName = "tekton-internal-bin" // FIXME(vdemeester) remove duplication with script package + binDir = "/tekton/bin" // FIXME(vdemeester) remove duplication with script package entrypointBinary = binDir + "/entrypoint" runVolumeName = "tekton-internal-run" - runDir = "/tekton/run" + runDir = "/tekton/run" // FIXME(vdemeester) remove duplication with script package downwardVolumeName = "tekton-internal-downward" downwardMountPoint = "/tekton/downward" @@ -53,12 +53,11 @@ const ( stepPrefix = "step-" sidecarPrefix = "sidecar-" - - breakpointOnFailure = "onFailure" ) var ( // TODO(#1605): Generate volumeMount names, to avoid collisions. + // FIXME(vdemeester) remove duplication with script package binMount = corev1.VolumeMount{ Name: binVolumeName, MountPath: binDir, @@ -160,7 +159,7 @@ func orderContainers(commonExtraEntrypointArgs []string, steps []corev1.Containe breakpoints := breakpointConfig.Breakpoint for _, b := range breakpoints { // TODO(TEP #0042): Add other breakpoints - if b == breakpointOnFailure { + if b == script.BreakpointOnFailure { argsForEntrypoint = append(argsForEntrypoint, "-breakpoint_on_failure") } } @@ -219,68 +218,12 @@ func UpdateReady(ctx context.Context, kubeclient kubernetes.Interface, pod corev return err } -// StopSidecars updates sidecar containers in the Pod to a nop image, which -// exits successfully immediately. -func StopSidecars(ctx context.Context, nopImage string, kubeclient kubernetes.Interface, namespace, name string) (*corev1.Pod, error) { - newPod, err := kubeclient.CoreV1().Pods(namespace).Get(ctx, name, metav1.GetOptions{}) - if k8serrors.IsNotFound(err) { - // return NotFound as-is, since the K8s error checks don't handle wrapping. - return nil, err - } else if err != nil { - return nil, fmt.Errorf("error getting Pod %q when stopping sidecars: %w", name, err) - } - - updated := false - if newPod.Status.Phase == corev1.PodRunning { - for _, s := range newPod.Status.ContainerStatuses { - // Stop any running container that isn't a step. - // An injected sidecar container might not have the - // "sidecar-" prefix, so we can't just look for that - // prefix. - if !IsContainerStep(s.Name) && s.State.Running != nil { - for j, c := range newPod.Spec.Containers { - if c.Name == s.Name && c.Image != nopImage { - updated = true - newPod.Spec.Containers[j].Image = nopImage - } - } - } - } - } - if updated { - if newPod, err = kubeclient.CoreV1().Pods(newPod.Namespace).Update(ctx, newPod, metav1.UpdateOptions{}); err != nil { - return nil, fmt.Errorf("error stopping sidecars of Pod %q: %w", name, err) - } - } - return newPod, nil -} - -// IsSidecarStatusRunning determines if any SidecarStatus on a TaskRun -// is still running. -func IsSidecarStatusRunning(tr *v1beta1.TaskRun) bool { - for _, sidecar := range tr.Status.Sidecars { - if sidecar.Terminated == nil { - return true - } - } - - return false -} - -// IsContainerStep returns true if the container name indicates that it +// isContainerStep returns true if the container name indicates that it // represents a step. func IsContainerStep(name string) bool { return strings.HasPrefix(name, stepPrefix) } -// isContainerSidecar returns true if the container name indicates that it -// represents a sidecar. -func isContainerSidecar(name string) bool { return strings.HasPrefix(name, sidecarPrefix) } - // trimStepPrefix returns the container name, stripped of its step prefix. -func trimStepPrefix(name string) string { return strings.TrimPrefix(name, stepPrefix) } - -// TrimSidecarPrefix returns the container name, stripped of its sidecar -// prefix. -func TrimSidecarPrefix(name string) string { return strings.TrimPrefix(name, sidecarPrefix) } +func TrimStepPrefix(name string) string { return strings.TrimPrefix(name, stepPrefix) } // StepName returns the step name after adding "step-" prefix to the actual step name or // returns "step-unnamed-" if not specified diff --git a/pkg/pod/entrypoint_test.go b/pkg/pod/entrypoint_test.go index f41544261ed..a6ba3ff1f91 100644 --- a/pkg/pod/entrypoint_test.go +++ b/pkg/pod/entrypoint_test.go @@ -19,7 +19,6 @@ package pod import ( "context" "testing" - "time" "github.com/google/go-cmp/cmp" "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1" @@ -436,116 +435,3 @@ func TestUpdateReady(t *testing.T) { }) } } - -const nopImage = "nop-image" - -// TestStopSidecars tests stopping sidecars by updating their images to a nop -// image. -func TestStopSidecars(t *testing.T) { - stepContainer := corev1.Container{ - Name: stepPrefix + "my-step", - Image: "foo", - } - sidecarContainer := corev1.Container{ - Name: sidecarPrefix + "my-sidecar", - Image: "original-image", - } - stoppedSidecarContainer := corev1.Container{ - Name: sidecarContainer.Name, - Image: nopImage, - } - - // This is a container that doesn't start with the "sidecar-" prefix, - // which indicates it was injected into the Pod by a Mutating Webhook - // Admission Controller. Injected sidecars should also be stopped if - // they're running. - injectedSidecar := corev1.Container{ - Name: "injected", - Image: "original-injected-image", - } - stoppedInjectedSidecar := corev1.Container{ - Name: injectedSidecar.Name, - Image: nopImage, - } - - for _, c := range []struct { - desc string - pod corev1.Pod - wantContainers []corev1.Container - }{{ - desc: "Running sidecars (incl injected) should be stopped", - pod: corev1.Pod{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-pod", - }, - Spec: corev1.PodSpec{ - Containers: []corev1.Container{stepContainer, sidecarContainer, injectedSidecar}, - }, - Status: corev1.PodStatus{ - Phase: corev1.PodRunning, - ContainerStatuses: []corev1.ContainerStatus{{ - // Step state doesn't matter. - }, { - Name: sidecarContainer.Name, - // Sidecar is running. - State: corev1.ContainerState{Running: &corev1.ContainerStateRunning{StartedAt: metav1.NewTime(time.Now())}}, - }, { - Name: injectedSidecar.Name, - // Injected sidecar is running. - State: corev1.ContainerState{Running: &corev1.ContainerStateRunning{StartedAt: metav1.NewTime(time.Now())}}, - }}, - }, - }, - wantContainers: []corev1.Container{stepContainer, stoppedSidecarContainer, stoppedInjectedSidecar}, - }, { - desc: "Pending Pod should not be updated", - pod: corev1.Pod{ - ObjectMeta: metav1.ObjectMeta{Name: "test-pod"}, - Spec: corev1.PodSpec{ - Containers: []corev1.Container{stepContainer, sidecarContainer}, - }, - Status: corev1.PodStatus{ - Phase: corev1.PodPending, - // Container states don't matter. - }, - }, - wantContainers: []corev1.Container{stepContainer, sidecarContainer}, - }, { - desc: "Non-Running sidecars should not be stopped", - pod: corev1.Pod{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-pod", - }, - Spec: corev1.PodSpec{ - Containers: []corev1.Container{stepContainer, sidecarContainer, injectedSidecar}, - }, - Status: corev1.PodStatus{ - Phase: corev1.PodRunning, - ContainerStatuses: []corev1.ContainerStatus{{ - // Step state doesn't matter. - }, { - Name: sidecarContainer.Name, - // Sidecar is waiting. - State: corev1.ContainerState{Waiting: &corev1.ContainerStateWaiting{}}, - }, { - Name: injectedSidecar.Name, - // Injected sidecar is waiting. - State: corev1.ContainerState{Waiting: &corev1.ContainerStateWaiting{}}, - }}, - }, - }, - wantContainers: []corev1.Container{stepContainer, sidecarContainer, injectedSidecar}, - }} { - t.Run(c.desc, func(t *testing.T) { - ctx := context.Background() - ctx, cancel := context.WithCancel(ctx) - defer cancel() - kubeclient := fakek8s.NewSimpleClientset(&c.pod) - if got, err := StopSidecars(ctx, nopImage, kubeclient, c.pod.Namespace, c.pod.Name); err != nil { - t.Errorf("error stopping sidecar: %v", err) - } else if d := cmp.Diff(c.wantContainers, got.Spec.Containers); d != "" { - t.Errorf("Containers Diff %s", diff.PrintWantGot(d)) - } - }) - } -} diff --git a/pkg/pod/pod.go b/pkg/pod/pod.go index 86abf51f541..161429f0722 100644 --- a/pkg/pod/pod.go +++ b/pkg/pod/pod.go @@ -27,6 +27,7 @@ import ( "github.com/tektoncd/pipeline/pkg/apis/pipeline/pod" "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1" "github.com/tektoncd/pipeline/pkg/names" + "github.com/tektoncd/pipeline/pkg/pod/script" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime/schema" @@ -134,18 +135,21 @@ func (b *Builder) Build(ctx context.Context, taskRun *v1beta1.TaskRun, taskSpec // Convert any steps with Script to command+args. // If any are found, append an init container to initialize scripts. + // TODO(vdemeester) get one list of steps, return 1 list of containers + // TODO(vdemeester) extract breakpoint from scripts if alphaAPIEnabled { - scriptsInit, stepContainers, sidecarContainers = convertScripts(b.Images.ShellImage, b.Images.ShellImageWin, steps, taskSpec.Sidecars, taskRun.Spec.Debug) + scriptsInit, stepContainers, sidecarContainers = script.Convert(b.Images, steps, taskSpec.Sidecars, taskRun.Spec.Debug) } else { - scriptsInit, stepContainers, sidecarContainers = convertScripts(b.Images.ShellImage, "", steps, taskSpec.Sidecars, nil) + scriptsInit, stepContainers, sidecarContainers = script.Convert(b.Images, steps, taskSpec.Sidecars, nil) } + // scriptsInit, stepContainers = breakpoint.Transform(stepContainers, taskRun.Spec.Debug, scriptsInit) if scriptsInit != nil { initContainers = append(initContainers, *scriptsInit) - volumes = append(volumes, scriptsVolume) + volumes = append(volumes, script.ScriptsVolume) } if alphaAPIEnabled && taskRun.Spec.Debug != nil { - volumes = append(volumes, debugScriptsVolume, debugInfoVolume) + volumes = append(volumes, script.DebugScriptsVolume, script.DebugInfoVolume) } // Initialize any workingDirs under /workspace. @@ -159,7 +163,7 @@ func (b *Builder) Build(ctx context.Context, taskRun *v1beta1.TaskRun, taskSpec return nil, err } - // Rewrite steps with entrypoint binary. Append the entrypoint init + // Rewrite steps with entrypoint binàary. Append the entrypoint init // container to place the entrypoint binary. Also add timeout flags // to entrypoint binary. entrypointInit := corev1.Container{ diff --git a/pkg/pod/pod_test.go b/pkg/pod/pod_test.go index 088529c612d..4fd5a57f96f 100644 --- a/pkg/pod/pod_test.go +++ b/pkg/pod/pod_test.go @@ -31,6 +31,7 @@ import ( "github.com/tektoncd/pipeline/pkg/apis/pipeline" "github.com/tektoncd/pipeline/pkg/apis/pipeline/pod" "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1" + "github.com/tektoncd/pipeline/pkg/pod/script" "github.com/tektoncd/pipeline/test/diff" "github.com/tektoncd/pipeline/test/names" corev1 "k8s.io/api/core/v1" @@ -151,7 +152,7 @@ func TestPodBuild(t *testing.T) { desc: "simple with breakpoint onFailure enabled, alpha api fields disabled", trs: v1beta1.TaskRunSpec{ Debug: &v1beta1.TaskRunDebug{ - Breakpoint: []string{breakpointOnFailure}, + Breakpoint: []string{script.BreakpointOnFailure}, }, }, ts: v1beta1.TaskSpec{ @@ -604,7 +605,7 @@ func TestPodBuild(t *testing.T) { Name: "place-scripts", Image: "busybox", Command: []string{"sh"}, - VolumeMounts: []corev1.VolumeMount{writeScriptsVolumeMount, binMount}, + VolumeMounts: []corev1.VolumeMount{script.WriteScriptsVolumeMount, binMount}, Args: []string{"-c", `scriptfile="/tekton/scripts/sidecar-script-0-9l9zj" touch ${scriptfile} && chmod +x ${scriptfile} cat > ${scriptfile} << '_EOF_' @@ -643,9 +644,9 @@ _EOF_ Name: "sidecar-sc-name", Image: "sidecar-image", Command: []string{"/tekton/scripts/sidecar-script-0-9l9zj"}, - VolumeMounts: []corev1.VolumeMount{scriptsVolumeMount}, + VolumeMounts: []corev1.VolumeMount{script.ScriptsVolumeMount}, }}, - Volumes: append(implicitVolumes, scriptsVolume, binVolume, runVolume(0), downwardVolume, corev1.Volume{ + Volumes: append(implicitVolumes, script.ScriptsVolume, binVolume, runVolume(0), downwardVolume, corev1.Volume{ Name: "tekton-creds-init-home-0", VolumeSource: corev1.VolumeSource{EmptyDir: &corev1.EmptyDirVolumeSource{Medium: corev1.StorageMediumMemory}}, }), @@ -855,7 +856,7 @@ IyEvdXNyL2Jpbi9lbnYgcHl0aG9uCnByaW50KCJIZWxsbyBmcm9tIFB5dGhvbiIp _EOF_ /tekton/bin/entrypoint decode-script "${scriptfile}" `}, - VolumeMounts: []corev1.VolumeMount{writeScriptsVolumeMount, binMount}, + VolumeMounts: []corev1.VolumeMount{script.WriteScriptsVolumeMount, binMount}, }, }, Containers: []corev1.Container{{ @@ -881,7 +882,7 @@ _EOF_ "args", }, Env: []corev1.EnvVar{{Name: "FOO", Value: "bar"}}, - VolumeMounts: append([]corev1.VolumeMount{scriptsVolumeMount, binROMount, runMount(0, false), runMount(1, true), runMount(2, true), downwardMount, { + VolumeMounts: append([]corev1.VolumeMount{script.ScriptsVolumeMount, binROMount, runMount(0, false), runMount(1, true), runMount(2, true), downwardMount, { Name: "tekton-creds-init-home-0", MountPath: "/tekton/creds", }}, implicitVolumeMounts...), @@ -908,7 +909,7 @@ _EOF_ "args", }, Env: []corev1.EnvVar{{Name: "FOO", Value: "bar"}}, - VolumeMounts: append([]corev1.VolumeMount{{Name: "i-have-a-volume-mount"}, scriptsVolumeMount, binROMount, runMount(0, true), runMount(1, false), runMount(2, true), { + VolumeMounts: append([]corev1.VolumeMount{{Name: "i-have-a-volume-mount"}, script.ScriptsVolumeMount, binROMount, runMount(0, true), runMount(1, false), runMount(2, true), { Name: "tekton-creds-init-home-1", MountPath: "/tekton/creds", }}, implicitVolumeMounts...), @@ -942,7 +943,7 @@ _EOF_ }}, implicitVolumeMounts...), TerminationMessagePath: "/tekton/termination", }}, - Volumes: append(implicitVolumes, scriptsVolume, binVolume, runVolume(0), runVolume(1), runVolume(2), downwardVolume, corev1.Volume{ + Volumes: append(implicitVolumes, script.ScriptsVolume, binVolume, runVolume(0), runVolume(1), runVolume(2), downwardVolume, corev1.Volume{ Name: "tekton-creds-init-home-0", VolumeSource: corev1.VolumeSource{EmptyDir: &corev1.EmptyDirVolumeSource{Medium: corev1.StorageMediumMemory}}, }, corev1.Volume{ @@ -978,7 +979,7 @@ IyEvYmluL3NoCiQk _EOF_ /tekton/bin/entrypoint decode-script "${scriptfile}" `}, - VolumeMounts: []corev1.VolumeMount{writeScriptsVolumeMount, binMount}, + VolumeMounts: []corev1.VolumeMount{script.WriteScriptsVolumeMount, binMount}, }}, Containers: []corev1.Container{{ Name: "step-one", @@ -1000,13 +1001,13 @@ _EOF_ "/tekton/scripts/script-0-9l9zj", "--", }, - VolumeMounts: append([]corev1.VolumeMount{scriptsVolumeMount, binROMount, runMount(0, false), downwardMount, { + VolumeMounts: append([]corev1.VolumeMount{script.ScriptsVolumeMount, binROMount, runMount(0, false), downwardMount, { Name: "tekton-creds-init-home-0", MountPath: "/tekton/creds", }}, implicitVolumeMounts...), TerminationMessagePath: "/tekton/termination", }}, - Volumes: append(implicitVolumes, scriptsVolume, binVolume, runVolume(0), downwardVolume, corev1.Volume{ + Volumes: append(implicitVolumes, script.ScriptsVolume, binVolume, runVolume(0), downwardVolume, corev1.Volume{ Name: "tekton-creds-init-home-0", VolumeSource: corev1.VolumeSource{EmptyDir: &corev1.EmptyDirVolumeSource{Medium: corev1.StorageMediumMemory}}, }), @@ -1526,7 +1527,7 @@ func TestPodBuildwithAlphaAPIEnabled(t *testing.T) { desc: "simple with debug breakpoint onFailure", trs: v1beta1.TaskRunSpec{ Debug: &v1beta1.TaskRunDebug{ - Breakpoint: []string{breakpointOnFailure}, + Breakpoint: []string{script.BreakpointOnFailure}, }, }, ts: v1beta1.TaskSpec{ @@ -1566,7 +1567,7 @@ func TestPodBuildwithAlphaAPIEnabled(t *testing.T) { }}, implicitVolumeMounts...), TerminationMessagePath: "/tekton/termination", }}, - Volumes: append(implicitVolumes, debugScriptsVolume, debugInfoVolume, binVolume, runVolume(0), downwardVolume, corev1.Volume{ + Volumes: append(implicitVolumes, script.DebugScriptsVolume, script.DebugInfoVolume, binVolume, runVolume(0), downwardVolume, corev1.Volume{ Name: "tekton-creds-init-home-0", VolumeSource: corev1.VolumeSource{EmptyDir: &corev1.EmptyDirVolumeSource{Medium: corev1.StorageMediumMemory}}, }), diff --git a/pkg/pod/script.go b/pkg/pod/script/script.go similarity index 87% rename from pkg/pod/script.go rename to pkg/pod/script/script.go index accbe3767eb..396fca6c301 100644 --- a/pkg/pod/script.go +++ b/pkg/pod/script/script.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package pod +package script import ( "encoding/base64" @@ -22,6 +22,7 @@ import ( "path/filepath" "strings" + "github.com/tektoncd/pipeline/pkg/apis/pipeline" "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1" "github.com/tektoncd/pipeline/pkg/names" corev1 "k8s.io/api/core/v1" @@ -36,53 +37,67 @@ const ( debugScriptsDir = "/tekton/debug/scripts" defaultScriptPreamble = "#!/bin/sh\nset -xe\n" debugInfoDir = "/tekton/debug/info" + + binVolumeName = "tekton-internal-bin" + binDir = "/tekton/bin" + runDir = "/tekton/run" ) var ( + binMount = corev1.VolumeMount{ + Name: binVolumeName, + MountPath: binDir, + } // Volume definition attached to Pods generated from TaskRuns that have // steps that specify a Script. - scriptsVolume = corev1.Volume{ + // FIXME(vdemeester) unexport + ScriptsVolume = corev1.Volume{ Name: scriptsVolumeName, VolumeSource: corev1.VolumeSource{EmptyDir: &corev1.EmptyDirVolumeSource{}}, } - scriptsVolumeMount = corev1.VolumeMount{ + // FIXME(vdemeester) unexport + ScriptsVolumeMount = corev1.VolumeMount{ Name: scriptsVolumeName, MountPath: scriptsDir, ReadOnly: true, } - writeScriptsVolumeMount = corev1.VolumeMount{ + // FIXME(vdemeester): unexport + WriteScriptsVolumeMount = corev1.VolumeMount{ Name: scriptsVolumeName, MountPath: scriptsDir, ReadOnly: false, } - debugScriptsVolume = corev1.Volume{ + // FIXME(vdemeester) unexport + DebugScriptsVolume = corev1.Volume{ Name: debugScriptsVolumeName, VolumeSource: corev1.VolumeSource{EmptyDir: &corev1.EmptyDirVolumeSource{}}, } - debugScriptsVolumeMount = corev1.VolumeMount{ + // FIXME(vdemeester) unexport + DebugScriptsVolumeMount = corev1.VolumeMount{ Name: debugScriptsVolumeName, MountPath: debugScriptsDir, } - debugInfoVolume = corev1.Volume{ + // FIXME(vdemeester) unexport + DebugInfoVolume = corev1.Volume{ Name: debugInfoVolumeName, VolumeSource: corev1.VolumeSource{EmptyDir: &corev1.EmptyDirVolumeSource{}}, } ) -// convertScripts converts any steps and sidecars that specify a Script field into a normal Container. -func convertScripts(shellImageLinux string, shellImageWin string, steps []v1beta1.Step, sidecars []v1beta1.Sidecar, debugConfig *v1beta1.TaskRunDebug) (*corev1.Container, []corev1.Container, []corev1.Container) { +// Convert converts any steps and sidecars that specify a Script field into a normal Container. +func Convert(images pipeline.Images, steps []v1beta1.Step, sidecars []v1beta1.Sidecar, debugConfig *v1beta1.TaskRunDebug) (*corev1.Container, []corev1.Container, []corev1.Container) { placeScripts := false // Place scripts is an init container used for creating scripts in the // /tekton/scripts directory which would be later used by the step containers // as a Command requiresWindows := checkWindowsRequirement(steps, sidecars) - shellImage := shellImageLinux + shellImage := images.ShellImage shellCommand := "sh" shellArg := "-c" // Set windows variants for Image, Command and Args if requiresWindows { - shellImage = shellImageWin + shellImage = images.ShellImageWin shellCommand = "pwsh" shellArg = "-Command" } @@ -92,7 +107,7 @@ func convertScripts(shellImageLinux string, shellImageWin string, steps []v1beta Image: shellImage, Command: []string{shellCommand}, Args: []string{shellArg, ""}, - VolumeMounts: []corev1.VolumeMount{writeScriptsVolumeMount, binMount}, + VolumeMounts: []corev1.VolumeMount{WriteScriptsVolumeMount, binMount}, } breakpoints := []string{} @@ -109,7 +124,7 @@ func convertScripts(shellImageLinux string, shellImageWin string, steps []v1beta // Add mounts for debug if debugConfig != nil && len(debugConfig.Breakpoint) > 0 { breakpoints = debugConfig.Breakpoint - placeScriptsInit.VolumeMounts = append(placeScriptsInit.VolumeMounts, debugScriptsVolumeMount) + placeScriptsInit.VolumeMounts = append(placeScriptsInit.VolumeMounts, DebugScriptsVolumeMount) } convertedStepContainers := convertListOfSteps(steps, &placeScriptsInit, &placeScripts, breakpoints, "script") @@ -184,7 +199,7 @@ cat > ${scriptfile} << '%s' // we'll clear out the Args and overwrite Command. steps[i].Command = []string{scriptFile} } - steps[i].VolumeMounts = append(steps[i].VolumeMounts, scriptsVolumeMount) + steps[i].VolumeMounts = append(steps[i].VolumeMounts, ScriptsVolumeMount) // Add debug mounts if breakpoints are present if len(breakpoints) > 0 { @@ -192,7 +207,7 @@ cat > ${scriptfile} << '%s' Name: debugInfoVolumeName, MountPath: filepath.Join(debugInfoDir, fmt.Sprintf("%d", i)), } - steps[i].VolumeMounts = append(steps[i].VolumeMounts, debugScriptsVolumeMount, debugInfoVolumeMount) + steps[i].VolumeMounts = append(steps[i].VolumeMounts, DebugScriptsVolumeMount, debugInfoVolumeMount) } containers = append(containers, steps[i].Container) } diff --git a/pkg/pod/script_test.go b/pkg/pod/script/script_test.go similarity index 88% rename from pkg/pod/script_test.go rename to pkg/pod/script/script_test.go index 8211837e99a..728b8a7cd84 100644 --- a/pkg/pod/script_test.go +++ b/pkg/pod/script/script_test.go @@ -14,20 +14,28 @@ See the License for the specific language governing permissions and limitations under the License. */ -package pod +package script import ( "testing" "github.com/google/go-cmp/cmp" + "github.com/tektoncd/pipeline/pkg/apis/pipeline" "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1" "github.com/tektoncd/pipeline/test/diff" "github.com/tektoncd/pipeline/test/names" corev1 "k8s.io/api/core/v1" ) +var ( + images = pipeline.Images{ + EntrypointImage: "entrypoint-image", + ShellImage: "busybox", + } +) + func TestConvertScripts_NothingToConvert_EmptySidecars(t *testing.T) { - gotInit, gotScripts, gotSidecars := convertScripts(images.ShellImage, images.ShellImageWin, []v1beta1.Step{{ + gotInit, gotScripts, gotSidecars := Convert(images, []v1beta1.Step{{ Container: corev1.Container{ Image: "step-1", }, @@ -54,7 +62,7 @@ func TestConvertScripts_NothingToConvert_EmptySidecars(t *testing.T) { } func TestConvertScripts_NothingToConvert_NilSidecars(t *testing.T) { - gotInit, gotScripts, gotSidecars := convertScripts(images.ShellImage, images.ShellImageWin, []v1beta1.Step{{ + gotInit, gotScripts, gotSidecars := Convert(images, []v1beta1.Step{{ Container: corev1.Container{ Image: "step-1", }, @@ -81,7 +89,7 @@ func TestConvertScripts_NothingToConvert_NilSidecars(t *testing.T) { } func TestConvertScripts_NothingToConvert_WithSidecar(t *testing.T) { - gotInit, gotScripts, gotSidecars := convertScripts(images.ShellImage, images.ShellImageWin, []v1beta1.Step{{ + gotInit, gotScripts, gotSidecars := Convert(images, []v1beta1.Step{{ Container: corev1.Container{ Image: "step-1", }, @@ -130,7 +138,7 @@ func TestConvertScripts(t *testing.T) { MountPath: "/another/one", }} - gotInit, gotSteps, gotSidecars := convertScripts(images.ShellImage, images.ShellImageWin, []v1beta1.Step{{ + gotInit, gotSteps, gotSidecars := Convert(images, []v1beta1.Step{{ Script: `#!/bin/sh script-1`, Container: corev1.Container{Image: "step-1"}, @@ -177,19 +185,19 @@ IyEvYmluL3NoCnNldCAteGUKbm8tc2hlYmFuZw== _EOF_ /tekton/bin/entrypoint decode-script "${scriptfile}" `}, - VolumeMounts: []corev1.VolumeMount{writeScriptsVolumeMount, binMount}, + VolumeMounts: []corev1.VolumeMount{WriteScriptsVolumeMount, binMount}, } want := []corev1.Container{{ Image: "step-1", Command: []string{"/tekton/scripts/script-0-9l9zj"}, - VolumeMounts: []corev1.VolumeMount{scriptsVolumeMount}, + VolumeMounts: []corev1.VolumeMount{ScriptsVolumeMount}, }, { Image: "step-2", }, { Image: "step-3", Command: []string{"/tekton/scripts/script-2-mz4c7"}, Args: []string{"my", "args"}, - VolumeMounts: append(preExistingVolumeMounts, scriptsVolumeMount), + VolumeMounts: append(preExistingVolumeMounts, ScriptsVolumeMount), }, { Image: "step-3", Command: []string{"/tekton/scripts/script-3-mssqb"}, @@ -197,7 +205,7 @@ _EOF_ VolumeMounts: []corev1.VolumeMount{ {Name: "pre-existing-volume-mount", MountPath: "/mount/path"}, {Name: "another-one", MountPath: "/another/one"}, - scriptsVolumeMount, + ScriptsVolumeMount, }, }} if d := cmp.Diff(wantInit, gotInit); d != "" { @@ -223,7 +231,7 @@ func TestConvertScripts_WithBreakpoint_OnFailure(t *testing.T) { MountPath: "/another/one", }} - gotInit, gotSteps, gotSidecars := convertScripts(images.ShellImage, images.ShellImageWin, []v1beta1.Step{{ + gotInit, gotSteps, gotSidecars := Convert(images, []v1beta1.Step{{ Script: `#!/bin/sh script-1`, Container: corev1.Container{Image: "step-1"}, @@ -316,12 +324,12 @@ else fi debug-fail-continue-heredoc-randomly-generated-6nl7g `}, - VolumeMounts: []corev1.VolumeMount{writeScriptsVolumeMount, binMount, debugScriptsVolumeMount}, + VolumeMounts: []corev1.VolumeMount{WriteScriptsVolumeMount, binMount, DebugScriptsVolumeMount}, } want := []corev1.Container{{ Image: "step-1", Command: []string{"/tekton/scripts/script-0-9l9zj"}, - VolumeMounts: []corev1.VolumeMount{scriptsVolumeMount, debugScriptsVolumeMount, + VolumeMounts: []corev1.VolumeMount{ScriptsVolumeMount, DebugScriptsVolumeMount, {Name: debugInfoVolumeName, MountPath: "/tekton/debug/info/0"}}, }, { Image: "step-2", @@ -329,7 +337,7 @@ debug-fail-continue-heredoc-randomly-generated-6nl7g Image: "step-3", Command: []string{"/tekton/scripts/script-2-mz4c7"}, Args: []string{"my", "args"}, - VolumeMounts: append(preExistingVolumeMounts, scriptsVolumeMount, debugScriptsVolumeMount, + VolumeMounts: append(preExistingVolumeMounts, ScriptsVolumeMount, DebugScriptsVolumeMount, corev1.VolumeMount{Name: debugInfoVolumeName, MountPath: "/tekton/debug/info/2"}), }, { Image: "step-3", @@ -338,7 +346,7 @@ debug-fail-continue-heredoc-randomly-generated-6nl7g VolumeMounts: []corev1.VolumeMount{ {Name: "pre-existing-volume-mount", MountPath: "/mount/path"}, {Name: "another-one", MountPath: "/another/one"}, - scriptsVolumeMount, debugScriptsVolumeMount, + ScriptsVolumeMount, DebugScriptsVolumeMount, {Name: debugInfoVolumeName, MountPath: "/tekton/debug/info/3"}, }, }} @@ -365,7 +373,7 @@ func TestConvertScripts_WithSidecar(t *testing.T) { MountPath: "/another/one", }} - gotInit, gotSteps, gotSidecars := convertScripts(images.ShellImage, images.ShellImageWin, []v1beta1.Step{{ + gotInit, gotSteps, gotSidecars := Convert(images, []v1beta1.Step{{ Script: `#!/bin/sh script-1`, Container: corev1.Container{Image: "step-1"}, @@ -408,12 +416,12 @@ IyEvYmluL3NoCnNpZGVjYXItMQ== _EOF_ /tekton/bin/entrypoint decode-script "${scriptfile}" `}, - VolumeMounts: []corev1.VolumeMount{writeScriptsVolumeMount, binMount}, + VolumeMounts: []corev1.VolumeMount{WriteScriptsVolumeMount, binMount}, } want := []corev1.Container{{ Image: "step-1", Command: []string{"/tekton/scripts/script-0-9l9zj"}, - VolumeMounts: []corev1.VolumeMount{scriptsVolumeMount}, + VolumeMounts: []corev1.VolumeMount{ScriptsVolumeMount}, }, { Image: "step-2", }, { @@ -423,14 +431,14 @@ _EOF_ VolumeMounts: []corev1.VolumeMount{ {Name: "pre-existing-volume-mount", MountPath: "/mount/path"}, {Name: "another-one", MountPath: "/another/one"}, - scriptsVolumeMount, + ScriptsVolumeMount, }, }} wantSidecars := []corev1.Container{{ Image: "sidecar-1", Command: []string{"/tekton/scripts/sidecar-script-0-mssqb"}, - VolumeMounts: []corev1.VolumeMount{scriptsVolumeMount}, + VolumeMounts: []corev1.VolumeMount{ScriptsVolumeMount}, }} if d := cmp.Diff(wantInit, gotInit); d != "" { t.Errorf("Init Container Diff %s", diff.PrintWantGot(d)) @@ -459,7 +467,7 @@ func TestConvertScripts_Windows(t *testing.T) { MountPath: "/another/one", }} - gotInit, gotSteps, gotSidecars := convertScripts(images.ShellImage, images.ShellImageWin, []v1beta1.Step{{ + gotInit, gotSteps, gotSidecars := Convert(images, []v1beta1.Step{{ Script: `#!win pwsh -File script-1`, Container: corev1.Container{Image: "step-1"}, @@ -499,20 +507,20 @@ script-3 no-shebang "@ | Out-File -FilePath /tekton/scripts/script-3-mssqb.cmd `}, - VolumeMounts: []corev1.VolumeMount{writeScriptsVolumeMount, binMount}, + VolumeMounts: []corev1.VolumeMount{WriteScriptsVolumeMount, binMount}, } want := []corev1.Container{{ Image: "step-1", Command: []string{"pwsh"}, Args: []string{"-File", "/tekton/scripts/script-0-9l9zj"}, - VolumeMounts: []corev1.VolumeMount{scriptsVolumeMount}, + VolumeMounts: []corev1.VolumeMount{ScriptsVolumeMount}, }, { Image: "step-2", }, { Image: "step-3", Command: []string{"powershell"}, Args: []string{"-File", "/tekton/scripts/script-2-mz4c7.ps1", "my", "args"}, - VolumeMounts: append(preExistingVolumeMounts, scriptsVolumeMount), + VolumeMounts: append(preExistingVolumeMounts, ScriptsVolumeMount), }, { Image: "step-3", Command: []string{"/tekton/scripts/script-3-mssqb.cmd"}, @@ -520,7 +528,7 @@ no-shebang VolumeMounts: []corev1.VolumeMount{ {Name: "pre-existing-volume-mount", MountPath: "/mount/path"}, {Name: "another-one", MountPath: "/another/one"}, - scriptsVolumeMount, + ScriptsVolumeMount, }, }} if d := cmp.Diff(wantInit, gotInit); d != "" { @@ -546,7 +554,7 @@ func TestConvertScripts_Windows_WithSidecar(t *testing.T) { MountPath: "/another/one", }} - gotInit, gotSteps, gotSidecars := convertScripts(images.ShellImage, images.ShellImageWin, []v1beta1.Step{{ + gotInit, gotSteps, gotSidecars := Convert(images, []v1beta1.Step{{ Script: `#!win pwsh -File script-1`, Container: corev1.Container{Image: "step-1"}, @@ -583,13 +591,13 @@ script-3 sidecar-1 "@ | Out-File -FilePath /tekton/scripts/sidecar-script-0-mssqb `}, - VolumeMounts: []corev1.VolumeMount{writeScriptsVolumeMount, binMount}, + VolumeMounts: []corev1.VolumeMount{WriteScriptsVolumeMount, binMount}, } want := []corev1.Container{{ Image: "step-1", Command: []string{"pwsh"}, Args: []string{"-File", "/tekton/scripts/script-0-9l9zj"}, - VolumeMounts: []corev1.VolumeMount{scriptsVolumeMount}, + VolumeMounts: []corev1.VolumeMount{ScriptsVolumeMount}, }, { Image: "step-2", }, { @@ -599,7 +607,7 @@ sidecar-1 VolumeMounts: []corev1.VolumeMount{ {Name: "pre-existing-volume-mount", MountPath: "/mount/path"}, {Name: "another-one", MountPath: "/another/one"}, - scriptsVolumeMount, + ScriptsVolumeMount, }, }} @@ -607,7 +615,7 @@ sidecar-1 Image: "sidecar-1", Command: []string{"pwsh"}, Args: []string{"-File", "/tekton/scripts/sidecar-script-0-mssqb"}, - VolumeMounts: []corev1.VolumeMount{scriptsVolumeMount}, + VolumeMounts: []corev1.VolumeMount{ScriptsVolumeMount}, }} if d := cmp.Diff(wantInit, gotInit); d != "" { t.Errorf("Init Container Diff %s", diff.PrintWantGot(d)) @@ -628,7 +636,7 @@ sidecar-1 func TestConvertScripts_Windows_SidecarOnly(t *testing.T) { names.TestingSeed() - gotInit, gotSteps, gotSidecars := convertScripts(images.ShellImage, images.ShellImageWin, []v1beta1.Step{{ + gotInit, gotSteps, gotSidecars := Convert(images, []v1beta1.Step{{ // No script to convert here.: Container: corev1.Container{Image: "step-1"}, }}, []v1beta1.Sidecar{{ @@ -645,7 +653,7 @@ sidecar-1`, sidecar-1 "@ | Out-File -FilePath /tekton/scripts/sidecar-script-0-9l9zj `}, - VolumeMounts: []corev1.VolumeMount{writeScriptsVolumeMount, binMount}, + VolumeMounts: []corev1.VolumeMount{WriteScriptsVolumeMount, binMount}, } want := []corev1.Container{{ Image: "step-1", @@ -655,7 +663,7 @@ sidecar-1 Image: "sidecar-1", Command: []string{"python"}, Args: []string{"/tekton/scripts/sidecar-script-0-9l9zj"}, - VolumeMounts: []corev1.VolumeMount{scriptsVolumeMount}, + VolumeMounts: []corev1.VolumeMount{ScriptsVolumeMount}, }} if d := cmp.Diff(wantInit, gotInit); d != "" { t.Errorf("Init Container Diff %s", diff.PrintWantGot(d)) diff --git a/pkg/pod/scripts_constants.go b/pkg/pod/script/scripts_constants.go similarity index 95% rename from pkg/pod/scripts_constants.go rename to pkg/pod/script/scripts_constants.go index c9c0069ee58..dd9fd607db4 100644 --- a/pkg/pod/scripts_constants.go +++ b/pkg/pod/script/scripts_constants.go @@ -1,7 +1,9 @@ -package pod +package script // TODO(#3972): Use text/template templating instead of %s based templating const ( + BreakpointOnFailure = "onFailure" + debugContinueScriptTemplate = ` numberOfSteps=%d debugInfo=%s diff --git a/pkg/pod/status.go b/pkg/pod/status/status.go similarity index 94% rename from pkg/pod/status.go rename to pkg/pod/status/status.go index 9e2d9c7d727..3cf3b970502 100644 --- a/pkg/pod/status.go +++ b/pkg/pod/status/status.go @@ -25,6 +25,8 @@ import ( "github.com/hashicorp/go-multierror" "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1" + "github.com/tektoncd/pipeline/pkg/internal/sidecars" + podconvert "github.com/tektoncd/pipeline/pkg/pod" "github.com/tektoncd/pipeline/pkg/termination" "go.uber.org/zap" corev1 "k8s.io/api/core/v1" @@ -71,31 +73,6 @@ const ( const oomKilled = "OOMKilled" -// SidecarsReady returns true if all of the Pod's sidecars are Ready or -// Terminated. -func SidecarsReady(podStatus corev1.PodStatus) bool { - if podStatus.Phase != corev1.PodRunning { - return false - } - for _, s := range podStatus.ContainerStatuses { - // If the step indicates that it's a step, skip it. - // An injected sidecar might not have the "sidecar-" prefix, so - // we can't just look for that prefix, we need to look at any - // non-step container. - if IsContainerStep(s.Name) { - continue - } - if s.State.Running != nil && s.Ready { - continue - } - if s.State.Terminated != nil { - continue - } - return false - } - return true -} - // MakeTaskRunStatus returns a TaskRunStatus based on the Pod's status. func MakeTaskRunStatus(logger *zap.SugaredLogger, tr v1beta1.TaskRun, pod *corev1.Pod) (v1beta1.TaskRunStatus, error) { trs := &tr.Status @@ -121,9 +98,9 @@ func MakeTaskRunStatus(logger *zap.SugaredLogger, tr v1beta1.TaskRun, pod *corev var stepStatuses []corev1.ContainerStatus var sidecarStatuses []corev1.ContainerStatus for _, s := range pod.Status.ContainerStatuses { - if IsContainerStep(s.Name) { + if podconvert.IsContainerStep(s.Name) { stepStatuses = append(stepStatuses, s) - } else if isContainerSidecar(s.Name) { + } else if sidecars.IsContainerSidecar(s.Name) { sidecarStatuses = append(sidecarStatuses, s) } } @@ -185,7 +162,7 @@ func setTaskRunStatusBasedOnStepStatus(logger *zap.SugaredLogger, stepStatuses [ } trs.Steps = append(trs.Steps, v1beta1.StepState{ ContainerState: *s.State.DeepCopy(), - Name: trimStepPrefix(s.Name), + Name: podconvert.TrimStepPrefix(s.Name), ContainerName: s.Name, ImageID: s.ImageID, }) @@ -199,7 +176,7 @@ func setTaskRunStatusBasedOnSidecarStatus(sidecarStatuses []corev1.ContainerStat for _, s := range sidecarStatuses { trs.Sidecars = append(trs.Sidecars, v1beta1.SidecarState{ ContainerState: *s.State.DeepCopy(), - Name: TrimSidecarPrefix(s.Name), + Name: sidecars.TrimPrefix(s.Name), ContainerName: s.Name, ImageID: s.ImageID, }) @@ -324,7 +301,7 @@ func updateIncompleteTaskRunStatus(trs *v1beta1.TaskRunStatus, pod *corev1.Pod) func DidTaskRunFail(pod *corev1.Pod) bool { f := pod.Status.Phase == corev1.PodFailed for _, s := range pod.Status.ContainerStatuses { - if IsContainerStep(s.Name) { + if podconvert.IsContainerStep(s.Name) { if s.State.Terminated != nil { f = f || s.State.Terminated.ExitCode != 0 || isOOMKilled(s) } @@ -336,7 +313,7 @@ func DidTaskRunFail(pod *corev1.Pod) bool { func areStepsComplete(pod *corev1.Pod) bool { stepsComplete := len(pod.Status.ContainerStatuses) > 0 && pod.Status.Phase == corev1.PodRunning for _, s := range pod.Status.ContainerStatuses { - if IsContainerStep(s.Name) { + if podconvert.IsContainerStep(s.Name) { if s.State.Terminated == nil { stepsComplete = false } @@ -374,7 +351,7 @@ func getFailureMessage(logger *zap.SugaredLogger, pod *corev1.Pod) string { } for _, s := range pod.Status.ContainerStatuses { - if IsContainerStep(s.Name) { + if podconvert.IsContainerStep(s.Name) { if s.State.Terminated != nil { if isOOMKilled(s) { return oomKilled diff --git a/pkg/pod/status_test.go b/pkg/pod/status/status_test.go similarity index 99% rename from pkg/pod/status_test.go rename to pkg/pod/status/status_test.go index b7075199622..aa71d604667 100644 --- a/pkg/pod/status_test.go +++ b/pkg/pod/status/status_test.go @@ -23,6 +23,7 @@ import ( "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1" + "github.com/tektoncd/pipeline/pkg/internal/sidecars" "github.com/tektoncd/pipeline/test/diff" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -1176,7 +1177,7 @@ func TestSidecarsReady(t *testing.T) { want: false, }} { t.Run(c.desc, func(t *testing.T) { - got := SidecarsReady(corev1.PodStatus{ + got := sidecars.Ready(corev1.PodStatus{ Phase: corev1.PodRunning, ContainerStatuses: c.statuses, }) diff --git a/pkg/reconciler/taskrun/taskrun.go b/pkg/reconciler/taskrun/taskrun.go index ea21695c778..f543c475f23 100644 --- a/pkg/reconciler/taskrun/taskrun.go +++ b/pkg/reconciler/taskrun/taskrun.go @@ -41,7 +41,9 @@ import ( "github.com/tektoncd/pipeline/pkg/internal/affinityassistant" "github.com/tektoncd/pipeline/pkg/internal/deprecated" "github.com/tektoncd/pipeline/pkg/internal/limitrange" + "github.com/tektoncd/pipeline/pkg/internal/sidecars" podconvert "github.com/tektoncd/pipeline/pkg/pod" + podstatus "github.com/tektoncd/pipeline/pkg/pod/status" tknreconciler "github.com/tektoncd/pipeline/pkg/reconciler" "github.com/tektoncd/pipeline/pkg/reconciler/events" "github.com/tektoncd/pipeline/pkg/reconciler/events/cloudevent" @@ -234,11 +236,11 @@ func (c *Reconciler) stopSidecars(ctx context.Context, tr *v1beta1.TaskRun) (*co } } - pod, err := podconvert.StopSidecars(ctx, c.Images.NopImage, c.KubeClientSet, tr.Namespace, tr.Status.PodName) + pod, err := sidecars.Stop(ctx, c.Images.NopImage, c.KubeClientSet, tr.Namespace, tr.Status.PodName) if err == nil { // Check if any SidecarStatuses are still shown as Running after stopping // Sidecars. If any Running, update SidecarStatuses based on Pod ContainerStatuses. - if podconvert.IsSidecarStatusRunning(tr) { + if sidecars.IsRunning(tr) { err = updateStoppedSidecarStatus(ctx, pod, tr, c) } } @@ -249,7 +251,7 @@ func (c *Reconciler) stopSidecars(ctx context.Context, tr *v1beta1.TaskRun) (*co return nil, controller.NewPermanentError(err) } else if err != nil { logger.Errorf("Error stopping sidecars for TaskRun %q: %v", tr.Name, err) - tr.Status.MarkResourceFailed(podconvert.ReasonFailedResolution, err) + tr.Status.MarkResourceFailed(podstatus.ReasonFailedResolution, err) } return pod, nil } @@ -299,7 +301,7 @@ func (c *Reconciler) prepare(ctx context.Context, tr *v1beta1.TaskRun) (*v1beta1 tr.Status.SetCondition(&apis.Condition{ Type: apis.ConditionSucceeded, Status: corev1.ConditionFalse, - Reason: podconvert.ReasonFailedResolution, + Reason: podstatus.ReasonFailedResolution, Message: err.Error(), }) return nil, nil, err @@ -308,7 +310,7 @@ func (c *Reconciler) prepare(ctx context.Context, tr *v1beta1.TaskRun) (*v1beta1 taskMeta, taskSpec, err := resources.GetTaskData(ctx, tr, getTaskfunc) if err != nil { logger.Errorf("Failed to determine Task spec to use for taskrun %s: %v", tr.Name, err) - tr.Status.MarkResourceFailed(podconvert.ReasonFailedResolution, err) + tr.Status.MarkResourceFailed(podstatus.ReasonFailedResolution, err) return nil, nil, controller.NewPermanentError(err) } @@ -353,37 +355,37 @@ func (c *Reconciler) prepare(ctx context.Context, tr *v1beta1.TaskRun) (*v1beta1 // Instead return an (non-permanent) error, which will prompt the // controller to requeue the key with backoff. logger.Warnf("References for taskrun %s not found: %v", tr.Name, err) - tr.Status.MarkResourceOngoing(podconvert.ReasonFailedResolution, + tr.Status.MarkResourceOngoing(podstatus.ReasonFailedResolution, fmt.Sprintf("Unable to resolve dependencies for %q: %v", tr.Name, err)) return nil, nil, err } logger.Errorf("Failed to resolve references for taskrun %s: %v", tr.Name, err) - tr.Status.MarkResourceFailed(podconvert.ReasonFailedResolution, err) + tr.Status.MarkResourceFailed(podstatus.ReasonFailedResolution, err) return nil, nil, controller.NewPermanentError(err) } if err := ValidateResolvedTaskResources(ctx, tr.Spec.Params, rtr); err != nil { logger.Errorf("TaskRun %q resources are invalid: %v", tr.Name, err) - tr.Status.MarkResourceFailed(podconvert.ReasonFailedValidation, err) + tr.Status.MarkResourceFailed(podstatus.ReasonFailedValidation, err) return nil, nil, controller.NewPermanentError(err) } if err := c.updateTaskRunWithDefaultWorkspaces(ctx, tr, taskSpec); err != nil { logger.Errorf("Failed to update taskrun %s with default workspace: %v", tr.Name, err) - tr.Status.MarkResourceFailed(podconvert.ReasonFailedResolution, err) + tr.Status.MarkResourceFailed(podstatus.ReasonFailedResolution, err) return nil, nil, controller.NewPermanentError(err) } if err := workspace.ValidateBindings(taskSpec.Workspaces, tr.Spec.Workspaces); err != nil { logger.Errorf("TaskRun %q workspaces are invalid: %v", tr.Name, err) - tr.Status.MarkResourceFailed(podconvert.ReasonFailedValidation, err) + tr.Status.MarkResourceFailed(podstatus.ReasonFailedValidation, err) return nil, nil, controller.NewPermanentError(err) } if _, usesAssistant := tr.Annotations[workspace.AnnotationAffinityAssistantName]; usesAssistant { if err := workspace.ValidateOnlyOnePVCIsUsed(tr.Spec.Workspaces); err != nil { logger.Errorf("TaskRun %q workspaces incompatible with Affinity Assistant: %v", tr.Name, err) - tr.Status.MarkResourceFailed(podconvert.ReasonFailedValidation, err) + tr.Status.MarkResourceFailed(podstatus.ReasonFailedValidation, err) return nil, nil, controller.NewPermanentError(err) } } @@ -435,7 +437,7 @@ func (c *Reconciler) reconcile(ctx context.Context, tr *v1beta1.TaskRun, rtr *re } for index := range pos.Items { po := pos.Items[index] - if metav1.IsControlledBy(&po, tr) && !podconvert.DidTaskRunFail(&po) { + if metav1.IsControlledBy(&po, tr) && !podstatus.DidTaskRunFail(&po) { pod = &po } } @@ -464,18 +466,18 @@ func (c *Reconciler) reconcile(ctx context.Context, tr *v1beta1.TaskRun, rtr *re } } - if podconvert.IsPodExceedingNodeResources(pod) { - recorder.Eventf(tr, corev1.EventTypeWarning, podconvert.ReasonExceededNodeResources, "Insufficient resources to schedule pod %q", pod.Name) + if podstatus.IsPodExceedingNodeResources(pod) { + recorder.Eventf(tr, corev1.EventTypeWarning, podstatus.ReasonExceededNodeResources, "Insufficient resources to schedule pod %q", pod.Name) } - if podconvert.SidecarsReady(pod.Status) { + if sidecars.Ready(pod.Status) { if err := podconvert.UpdateReady(ctx, c.KubeClientSet, *pod); err != nil { return err } } // Convert the Pod's status to the equivalent TaskRun Status. - tr.Status, err = podconvert.MakeTaskRunStatus(logger, *tr, pod) + tr.Status, err = podstatus.MakeTaskRunStatus(logger, *tr, pod) if err != nil { return err } @@ -544,11 +546,11 @@ func (c *Reconciler) handlePodCreationError(ctx context.Context, tr *v1beta1.Tas tr.Status.SetCondition(&apis.Condition{ Type: apis.ConditionSucceeded, Status: corev1.ConditionUnknown, - Reason: podconvert.ReasonExceededResourceQuota, + Reason: podstatus.ReasonExceededResourceQuota, Message: fmt.Sprint("TaskRun Pod exceeded available resources: ", err), }) case isTaskRunValidationFailed(err): - tr.Status.MarkResourceFailed(podconvert.ReasonFailedValidation, err) + tr.Status.MarkResourceFailed(podstatus.ReasonFailedValidation, err) default: // The pod creation failed with unknown reason. The most likely // reason is that something is wrong with the spec of the Task, that we could @@ -560,7 +562,7 @@ func (c *Reconciler) handlePodCreationError(ctx context.Context, tr *v1beta1.Tas msg += "invalid TaskSpec" } err = controller.NewPermanentError(errors.New(msg)) - tr.Status.MarkResourceFailed(podconvert.ReasonCouldntGetTask, err) + tr.Status.MarkResourceFailed(podstatus.ReasonCouldntGetTask, err) } return err } @@ -768,7 +770,7 @@ func updateStoppedSidecarStatus(ctx context.Context, pod *corev1.Pod, tr *v1beta tr.Status.Sidecars = append(tr.Status.Sidecars, v1beta1.SidecarState{ ContainerState: *sidecarState.DeepCopy(), - Name: podconvert.TrimSidecarPrefix(s.Name), + Name: sidecars.TrimPrefix(s.Name), ContainerName: s.Name, ImageID: s.ImageID, }) diff --git a/pkg/reconciler/taskrun/taskrun_test.go b/pkg/reconciler/taskrun/taskrun_test.go index b9f4baed34b..bb0dd65dc22 100644 --- a/pkg/reconciler/taskrun/taskrun_test.go +++ b/pkg/reconciler/taskrun/taskrun_test.go @@ -41,6 +41,7 @@ import ( resourcev1alpha1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1" "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1" podconvert "github.com/tektoncd/pipeline/pkg/pod" + podstatus "github.com/tektoncd/pipeline/pkg/pod/status" "github.com/tektoncd/pipeline/pkg/reconciler/events/cloudevent" "github.com/tektoncd/pipeline/pkg/reconciler/taskrun/resources" ttesting "github.com/tektoncd/pipeline/pkg/reconciler/testing" @@ -1679,7 +1680,7 @@ func TestReconcileInvalidTaskRuns(t *testing.T) { }{{ name: "task run with no task", taskRun: noTaskRun, - reason: podconvert.ReasonFailedResolution, + reason: podstatus.ReasonFailedResolution, wantEvents: []string{ "Normal Started", "Warning Failed", @@ -1688,7 +1689,7 @@ func TestReconcileInvalidTaskRuns(t *testing.T) { }, { name: "task run with wrong ref", taskRun: withWrongRef, - reason: podconvert.ReasonFailedResolution, + reason: podstatus.ReasonFailedResolution, wantEvents: []string{ "Normal Started", "Warning Failed", @@ -1756,7 +1757,7 @@ func TestReconcileTaskRunWithPermanentError(t *testing.T) { apis.Condition{ Type: apis.ConditionSucceeded, Status: corev1.ConditionFalse, - Reason: podconvert.ReasonFailedResolution, + Reason: podstatus.ReasonFailedResolution, Message: "error when listing tasks for taskRun taskrun-failure: tasks.tekton.dev \"notask\" not found", }, }, @@ -1803,8 +1804,8 @@ func TestReconcileTaskRunWithPermanentError(t *testing.T) { if condition == nil || condition.Status != corev1.ConditionFalse { t.Errorf("Expected invalid TaskRun to have failed status, but had %v", condition) } - if condition != nil && condition.Reason != podconvert.ReasonFailedResolution { - t.Errorf("Expected failure to be because of reason %q but was %s", podconvert.ReasonFailedResolution, condition.Reason) + if condition != nil && condition.Reason != podstatus.ReasonFailedResolution { + t.Errorf("Expected failure to be because of reason %q but was %s", podstatus.ReasonFailedResolution, condition.Reason) } } @@ -2533,19 +2534,19 @@ func TestHandlePodCreationError(t *testing.T) { err: k8sapierrors.NewForbidden(k8sruntimeschema.GroupResource{Group: "foo", Resource: "bar"}, "baz", errors.New("exceeded quota")), expectedType: apis.ConditionSucceeded, expectedStatus: corev1.ConditionUnknown, - expectedReason: podconvert.ReasonExceededResourceQuota, + expectedReason: podstatus.ReasonExceededResourceQuota, }, { description: "taskrun validation failed", err: errors.New("TaskRun validation failed"), expectedType: apis.ConditionSucceeded, expectedStatus: corev1.ConditionFalse, - expectedReason: podconvert.ReasonFailedValidation, + expectedReason: podstatus.ReasonFailedValidation, }, { description: "errors other than exceeded quota fail the taskrun", err: errors.New("this is a fatal error"), expectedType: apis.ConditionSucceeded, expectedStatus: corev1.ConditionFalse, - expectedReason: podconvert.ReasonCouldntGetTask, + expectedReason: podstatus.ReasonCouldntGetTask, }} for _, tc := range testcases { t.Run(tc.description, func(t *testing.T) { @@ -3149,7 +3150,7 @@ func TestReconcileWorkspaceMissing(t *testing.T) { failedCorrectly := false for _, c := range tr.Status.Conditions { - if c.Type == apis.ConditionSucceeded && c.Status == corev1.ConditionFalse && c.Reason == podconvert.ReasonFailedValidation { + if c.Type == apis.ConditionSucceeded && c.Status == corev1.ConditionFalse && c.Reason == podstatus.ReasonFailedValidation { failedCorrectly = true } } @@ -3227,7 +3228,7 @@ func TestReconcileValidDefaultWorkspace(t *testing.T) { } for _, c := range tr.Status.Conditions { - if c.Type == apis.ConditionSucceeded && c.Status == corev1.ConditionFalse && c.Reason == podconvert.ReasonFailedValidation { + if c.Type == apis.ConditionSucceeded && c.Status == corev1.ConditionFalse && c.Reason == podstatus.ReasonFailedValidation { t.Errorf("Expected TaskRun to pass Validation by using the default workspace but it did not. Final conditions were:\n%#v", tr.Status.Conditions) } } @@ -3432,7 +3433,7 @@ func TestReconcileTaskResourceResolutionAndValidation(t *testing.T) { ClusterTasks: nil, PipelineResources: nil, }, - wantFailedReason: podconvert.ReasonFailedResolution, + wantFailedReason: podstatus.ReasonFailedResolution, wantEvents: []string{ "Normal Started ", "Warning Failed", // Event about the TaskRun state changed @@ -3466,7 +3467,7 @@ func TestReconcileTaskResourceResolutionAndValidation(t *testing.T) { ClusterTasks: nil, PipelineResources: nil, }, - wantFailedReason: podconvert.ReasonFailedValidation, + wantFailedReason: podstatus.ReasonFailedValidation, wantEvents: []string{ "Normal Started ", "Warning Failed", // Event about the TaskRun state changed @@ -3600,8 +3601,8 @@ func TestReconcileWithWorkspacesIncompatibleWithAffinityAssistant(t *testing.T) } for _, cond := range ttt.Status.Conditions { - if cond.Reason != podconvert.ReasonFailedValidation { - t.Errorf("unexpected Reason on the Condition, expected: %s, got: %s", podconvert.ReasonFailedValidation, cond.Reason) + if cond.Reason != podstatus.ReasonFailedValidation { + t.Errorf("unexpected Reason on the Condition, expected: %s, got: %s", podstatus.ReasonFailedValidation, cond.Reason) } } } diff --git a/test/tektonbundles_test.go b/test/tektonbundles_test.go index 938ddc90543..fe3996bf993 100644 --- a/test/tektonbundles_test.go +++ b/test/tektonbundles_test.go @@ -30,8 +30,6 @@ import ( "strings" "testing" - "github.com/tektoncd/pipeline/test/parse" - "github.com/ghodss/yaml" "github.com/google/go-containerregistry/pkg/name" v1 "github.com/google/go-containerregistry/pkg/v1" @@ -39,8 +37,9 @@ import ( "github.com/google/go-containerregistry/pkg/v1/layout" "github.com/google/go-containerregistry/pkg/v1/mutate" "github.com/google/go-containerregistry/pkg/v1/tarball" - "github.com/tektoncd/pipeline/pkg/pod" + podstatus "github.com/tektoncd/pipeline/pkg/pod/status" "github.com/tektoncd/pipeline/pkg/reconciler/pipelinerun" + "github.com/tektoncd/pipeline/test/parse" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "knative.dev/pkg/apis" @@ -266,7 +265,7 @@ spec: t.Logf("Waiting for PipelineRun in namespace %s to finish", namespace) if err := WaitForPipelineRunState(ctx, c, pipelineRunName, timeout, Chain( - FailedWithReason(pod.ReasonCouldntGetTask, pipelineRunName), + FailedWithReason(podstatus.ReasonCouldntGetTask, pipelineRunName), FailedWithMessage("does not contain a dev.tekton.image.apiVersion annotation", pipelineRunName), ), "PipelineRunFailed"); err != nil { t.Fatalf("Error waiting for PipelineRun to finish with expected error: %s", err)