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: #4613 Signed-off-by: yuzhipeng <[email protected]>
tektoncd · May 20, 2022 · e6c7eb0 · e6c7eb0
1 parent 7aef957
commit e6c7eb0
Show file tree

Hide file tree

Showing 4 changed files with 96 additions and 22 deletions.
diff --git 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
@@ -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
@@ -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
@@ -227,6 +227,7 @@ script-3`,
 	}}, []v1beta1.Sidecar{}, &v1beta1.TaskRunDebug{
 		Breakpoint: []string{breakpointOnFailure},
 	})
+
 	wantInit := &corev1.Container{
 		Name:    "place-scripts",
 		Image:   images.ShellImage,
@@ -296,33 +297,41 @@ 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"},
 		Args:    []string{"my", "args"},
 		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))
 	}