From e6c7eb06955d74af5f42f167bccd3b39488ad924 Mon Sep 17 00:00:00 2001 From: yuzhipeng Date: Sun, 8 May 2022 20:46:26 +0800 Subject: [PATCH] Update debug script mount logic When the step script is empty then the logic which mounts the debug script will skip for the container and the signal of placeScripts will be the default value false which causes no debug script volume will be mounted. Now it will check the breakpoint first, if the breakpoint is not null then will add volume Mount for container no matter weather the script is empty or not. And if the breakpoint is not null then will set the signal placeScripts to be true while it used to be true only when the script is not empty. related issue: https://github.com/tektoncd/pipeline/issues/4613 Signed-off-by: yuzhipeng --- pkg/pod/pod.go | 4 +-- pkg/pod/pod_test.go | 72 ++++++++++++++++++++++++++++++++++++++---- pkg/pod/script.go | 23 ++++++++------ pkg/pod/script_test.go | 19 ++++++++--- 4 files changed, 96 insertions(+), 22 deletions(-) diff --git a/pkg/pod/pod.go b/pkg/pod/pod.go index b038942933a..1f0d3eb856b 100644 --- a/pkg/pod/pod.go +++ b/pkg/pod/pod.go @@ -371,7 +371,7 @@ func makeLabels(s *v1beta1.TaskRun) map[string]string { return labels } -// shouldAddReadyAnnotationonPodCreate returns a bool indicating whether the +// shouldAddReadyAnnotationOnPodCreate returns a bool indicating whether the // controller should add the `Ready` annotation when creating the Pod. We cannot // add the annotation if Tekton is running in a cluster with injected sidecars // or if the Task specifies any sidecars. @@ -401,7 +401,7 @@ func runVolume(i int) corev1.Volume { } } -// prepareInitContainers generates a few init containers based of a set of command (in images) and volumes to run +// entrypointInitContainer 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 diff --git a/pkg/pod/pod_test.go b/pkg/pod/pod_test.go index bc7aee52359..c0a23c4298a 100644 --- a/pkg/pod/pod_test.go +++ b/pkg/pod/pod_test.go @@ -1813,6 +1813,69 @@ _EOF_ } func TestPodBuildwithAlphaAPIEnabled(t *testing.T) { + + placeScriptsContainer := corev1.Container{ + Name: "place-scripts", + Image: "busybox", + Command: []string{"sh"}, + VolumeMounts: []corev1.VolumeMount{writeScriptsVolumeMount, binMount, debugScriptsVolumeMount}, + Args: []string{"-c", `tmpfile="/tekton/debug/scripts/debug-continue" +touch ${tmpfile} && chmod +x ${tmpfile} +cat > ${tmpfile} << 'debug-continue-heredoc-randomly-generated-9l9zj' +#!/bin/sh +set -e + +numberOfSteps=1 +debugInfo=/tekton/debug/info +tektonRun=/tekton/run + +postFile="$(ls ${debugInfo} | grep -E '[0-9]+' | tail -1)" +stepNumber="$(echo ${postFile} | sed 's/[^0-9]*//g')" + +if [ $stepNumber -lt $numberOfSteps ]; then + touch ${tektonRun}/${stepNumber}/out # Mark step as success + echo "0" > ${tektonRun}/${stepNumber}/out.breakpointexit + echo "Executing step $stepNumber..." +else + echo "Last step (no. $stepNumber) has already been executed, breakpoint exiting !" + exit 0 +fi +debug-continue-heredoc-randomly-generated-9l9zj +tmpfile="/tekton/debug/scripts/debug-fail-continue" +touch ${tmpfile} && chmod +x ${tmpfile} +cat > ${tmpfile} << 'debug-fail-continue-heredoc-randomly-generated-mz4c7' +#!/bin/sh +set -e + +numberOfSteps=1 +debugInfo=/tekton/debug/info +tektonRun=/tekton/run + +postFile="$(ls ${debugInfo} | grep -E '[0-9]+' | tail -1)" +stepNumber="$(echo ${postFile} | sed 's/[^0-9]*//g')" + +if [ $stepNumber -lt $numberOfSteps ]; then + touch ${tektonRun}/${stepNumber}/out.err # Mark step as a failure + echo "1" > ${tektonRun}/${stepNumber}/out.breakpointexit + echo "Executing step $stepNumber..." +else + echo "Last step (no. $stepNumber) has already been executed, breakpoint exiting !" + exit 0 +fi +debug-fail-continue-heredoc-randomly-generated-mz4c7 +`}, + } + + containersVolumeMounts := append([]corev1.VolumeMount{binROMount, runMount(0, false), downwardMount, { + Name: "tekton-creds-init-home-0", + MountPath: "/tekton/creds", + }}, implicitVolumeMounts...) + containersVolumeMounts = append(containersVolumeMounts, debugScriptsVolumeMount) + containersVolumeMounts = append(containersVolumeMounts, corev1.VolumeMount{ + Name: debugInfoVolumeName, + MountPath: filepath.Join(debugInfoDir, fmt.Sprintf("%d", 0)), + }) + for _, c := range []struct { desc string trs v1beta1.TaskRunSpec @@ -1836,7 +1899,7 @@ func TestPodBuildwithAlphaAPIEnabled(t *testing.T) { }, want: &corev1.PodSpec{ RestartPolicy: corev1.RestartPolicyNever, - InitContainers: []corev1.Container{entrypointInitContainer(images.EntrypointImage, []v1beta1.Step{{Name: "name"}})}, + InitContainers: []corev1.Container{entrypointInitContainer(images.EntrypointImage, []v1beta1.Step{{Name: "name"}}), placeScriptsContainer}, Containers: []corev1.Container{{ Name: "step-name", Image: "image", @@ -1856,13 +1919,10 @@ func TestPodBuildwithAlphaAPIEnabled(t *testing.T) { "cmd", "--", }, - VolumeMounts: append([]corev1.VolumeMount{binROMount, runMount(0, false), downwardMount, { - Name: "tekton-creds-init-home-0", - MountPath: "/tekton/creds", - }}, implicitVolumeMounts...), + VolumeMounts: containersVolumeMounts, TerminationMessagePath: "/tekton/termination", }}, - Volumes: append(implicitVolumes, debugScriptsVolume, debugInfoVolume, binVolume, runVolume(0), downwardVolume, corev1.Volume{ + Volumes: append(implicitVolumes, debugScriptsVolume, debugInfoVolume, binVolume, scriptsVolume, 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.go index 8ca0043f41b..019d4a3e30d 100644 --- a/pkg/pod/script.go +++ b/pkg/pod/script.go @@ -130,9 +130,18 @@ func convertScripts(shellImageLinux string, shellImageWin string, steps []v1beta func convertListOfSteps(steps []v1beta1.Step, initContainer *corev1.Container, placeScripts *bool, breakpoints []string, namePrefix string) []corev1.Container { containers := []corev1.Container{} for i, s := range steps { + // Add debug mounts if breakpoints are present + if len(breakpoints) > 0 { + debugInfoVolumeMount := corev1.VolumeMount{ + Name: debugInfoVolumeName, + MountPath: filepath.Join(debugInfoDir, fmt.Sprintf("%d", i)), + } + steps[i].VolumeMounts = append(steps[i].VolumeMounts, debugScriptsVolumeMount, debugInfoVolumeMount) + } + if s.Script == "" { // Nothing to convert. - containers = append(containers, *s.ToK8sContainer()) + containers = append(containers, *steps[i].ToK8sContainer()) continue } @@ -187,19 +196,15 @@ cat > ${scriptfile} << '%s' } steps[i].VolumeMounts = append(steps[i].VolumeMounts, scriptsVolumeMount) - // Add debug mounts if breakpoints are present - if len(breakpoints) > 0 { - debugInfoVolumeMount := corev1.VolumeMount{ - Name: debugInfoVolumeName, - MountPath: filepath.Join(debugInfoDir, fmt.Sprintf("%d", i)), - } - steps[i].VolumeMounts = append(steps[i].VolumeMounts, debugScriptsVolumeMount, debugInfoVolumeMount) - } containers = append(containers, *steps[i].ToK8sContainer()) } // Place debug scripts if breakpoints are enabled if len(breakpoints) > 0 { + // If breakpoint is not nil then should add the init container + // to write debug script files + *placeScripts = true + type script struct { name string content string diff --git a/pkg/pod/script_test.go b/pkg/pod/script_test.go index 6ab62821422..0aa51d5144b 100644 --- a/pkg/pod/script_test.go +++ b/pkg/pod/script_test.go @@ -227,6 +227,7 @@ script-3`, }}, []v1beta1.Sidecar{}, &v1beta1.TaskRunDebug{ Breakpoint: []string{breakpointOnFailure}, }) + wantInit := &corev1.Container{ Name: "place-scripts", Image: images.ShellImage, @@ -296,19 +297,24 @@ debug-fail-continue-heredoc-randomly-generated-6nl7g `}, VolumeMounts: []corev1.VolumeMount{writeScriptsVolumeMount, binMount, debugScriptsVolumeMount}, } + want := []corev1.Container{{ Image: "step-1", Command: []string{"/tekton/scripts/script-0-9l9zj"}, - VolumeMounts: []corev1.VolumeMount{scriptsVolumeMount, debugScriptsVolumeMount, - {Name: debugInfoVolumeName, MountPath: "/tekton/debug/info/0"}}, + VolumeMounts: []corev1.VolumeMount{debugScriptsVolumeMount, + {Name: debugInfoVolumeName, MountPath: "/tekton/debug/info/0"}, scriptsVolumeMount}, }, { Image: "step-2", + VolumeMounts: []corev1.VolumeMount{ + debugScriptsVolumeMount, {Name: debugInfoVolumeName, MountPath: "/tekton/debug/info/1"}, + }, }, { Image: "step-3", Command: []string{"/tekton/scripts/script-2-mz4c7"}, Args: []string{"my", "args"}, - VolumeMounts: append(preExistingVolumeMounts, scriptsVolumeMount, debugScriptsVolumeMount, - corev1.VolumeMount{Name: debugInfoVolumeName, MountPath: "/tekton/debug/info/2"}), + VolumeMounts: append(preExistingVolumeMounts, debugScriptsVolumeMount, + corev1.VolumeMount{Name: debugInfoVolumeName, MountPath: "/tekton/debug/info/2"}, + scriptsVolumeMount), }, { Image: "step-3", Command: []string{"/tekton/scripts/script-3-mssqb"}, @@ -316,13 +322,16 @@ 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, + debugScriptsVolumeMount, {Name: debugInfoVolumeName, MountPath: "/tekton/debug/info/3"}, + scriptsVolumeMount, }, }} + if d := cmp.Diff(wantInit, gotInit); d != "" { t.Errorf("Init Container Diff %s", diff.PrintWantGot(d)) } + if d := cmp.Diff(want, gotSteps); d != "" { t.Errorf("Containers Diff %s", diff.PrintWantGot(d)) }