TEP-0097 breakpoints for taskrun

make TaskBreakpoints struct for taskrun debug. breakpoint on failure of a step was moved to `breakpoints.onFailure` spec and onFailure breakpoint are now enabled by setting `taskRun.spec.debug.breakpoints.onFailure` to `enabled`. Signed-off-by: chengjoey <zchengjoey@gmail.com>
tektoncd · Oct 10, 2023 · b507fa5 · b507fa5
1 parent f84c7cf
commit b507fa5
Show file tree

Hide file tree

Showing 23 changed files with 476 additions and 121 deletions.
diff --git a/docs/pipeline-api.md b/docs/pipeline-api.md
@@ -4607,6 +4607,37 @@ More info: <a href="https://kubernetes.io/docs/tasks/configure-pod-container/sec
 </tr>
 </tbody>
 </table>
+<h3 id="tekton.dev/v1.TaskBreakpoints">TaskBreakpoints
+</h3>
+<p>
+(<em>Appears on:</em><a href="#tekton.dev/v1.TaskRunDebug">TaskRunDebug</a>)
+</p>
+<div>
+<p>TaskBreakpoints defines the breakpoint config for a particular Task</p>
+</div>
+<table>
+<thead>
+<tr>
+<th>Field</th>
+<th>Description</th>
+</tr>
+</thead>
+<tbody>
+<tr>
+<td>
+<code>onFailure</code><br/>
+<em>
+string
+</em>
+</td>
+<td>
+<em>(Optional)</em>
+<p>if enabled, pause TaskRun on failure of a step
+failed step will not exit</p>
+</td>
+</tr>
+</tbody>
+</table>
 <h3 id="tekton.dev/v1.TaskKind">TaskKind
 (<code>string</code> alias)</h3>
 <p>
@@ -4792,9 +4823,11 @@ string
 <tbody>
 <tr>
 <td>
-<code>breakpoint</code><br/>
+<code>breakpoints</code><br/>
 <em>
-[]string
+<a href="#tekton.dev/v1.TaskBreakpoints">
+TaskBreakpoints
+</a>
 </em>
 </td>
 <td>
@@ -13069,6 +13102,37 @@ Default is false.</p>
 </tr>
 </tbody>
 </table>
+<h3 id="tekton.dev/v1beta1.TaskBreakpoints">TaskBreakpoints
+</h3>
+<p>
+(<em>Appears on:</em><a href="#tekton.dev/v1beta1.TaskRunDebug">TaskRunDebug</a>)
+</p>
+<div>
+<p>TaskBreakpoints defines the breakpoint config for a particular Task</p>
+</div>
+<table>
+<thead>
+<tr>
+<th>Field</th>
+<th>Description</th>
+</tr>
+</thead>
+<tbody>
+<tr>
+<td>
+<code>onFailure</code><br/>
+<em>
+string
+</em>
+</td>
+<td>
+<em>(Optional)</em>
+<p>if enabled, pause TaskRun on failure of a step
+failed step will not exit</p>
+</td>
+</tr>
+</tbody>
+</table>
 <h3 id="tekton.dev/v1beta1.TaskKind">TaskKind
 (<code>string</code> alias)</h3>
 <p>
@@ -13404,9 +13468,11 @@ conditions such as one used in spire results verification</p>
 <tbody>
 <tr>
 <td>
-<code>breakpoint</code><br/>
+<code>breakpoints</code><br/>
 <em>
-[]string
+<a href="#tekton.dev/v1beta1.TaskBreakpoints">
+TaskBreakpoints
+</a>
 </em>
 </td>
 <td>

diff --git a/docs/taskruns.md b/docs/taskruns.md
@@ -932,7 +932,8 @@ TaskRuns can be halted on failure for troubleshooting by providing the following
 ```yaml
 spec:
   debug:
-    breakpoint: ["onFailure"]
+    breakpoints:
+      onFailure: "enabled"
 ```
 
 Upon failure of a step, the TaskRun Pod execution is halted. If this TaskRun Pod continues to run without any lifecycle
@@ -941,7 +942,7 @@ change done by the user (running the debug-continue or debug-fail-continue scrip
 During this time, the user/client can get remote shell access to the step container with a command such as the following.
 
 ```bash
-kubectl exec -it print-date-d7tj5-pod -c step-print-date-human-readable
+kubectl exec -it print-date-d7tj5-pod -c step-print-date-human-readable sh
 ```
 
 ### Debug Environment

diff --git a/pkg/apis/pipeline/v1/openapi_generated.go b/pkg/apis/pipeline/v1/openapi_generated.go
diff --git a/pkg/apis/pipeline/v1/swagger.json b/pkg/apis/pipeline/v1/swagger.json
@@ -1645,6 +1645,16 @@
         }
       }
     },
+    "v1.TaskBreakpoints": {
+      "description": "TaskBreakpoints defines the breakpoint config for a particular Task",
+      "type": "object",
+      "properties": {
+        "onFailure": {
+          "description": "if enabled, pause TaskRun on failure of a step failed step will not exit",
+          "type": "string"
+        }
+      }
+    },
     "v1.TaskList": {
       "description": "TaskList contains a list of Task",
       "type": "object",
@@ -1751,13 +1761,8 @@
       "description": "TaskRunDebug defines the breakpoint config for a particular TaskRun",
       "type": "object",
       "properties": {
-        "breakpoint": {
-          "type": "array",
-          "items": {
-            "type": "string",
-            "default": ""
-          },
-          "x-kubernetes-list-type": "atomic"
+        "breakpoints": {
+          "$ref": "#/definitions/v1.TaskBreakpoints"
         }
       }
     },

diff --git a/pkg/apis/pipeline/v1/taskrun_types.go b/pkg/apis/pipeline/v1/taskrun_types.go
@@ -102,11 +102,41 @@ const (
 	TaskRunCancelledByPipelineTimeoutMsg TaskRunSpecStatusMessage = "TaskRun cancelled as the PipelineRun it belongs to has timed out."
 )
 
+const (
+	// EnabledOnFailureBreakpoint is the value for TaskRunDebug.Breakpoints.OnFailure that means the breakpoint onFailure is enabled
+	EnabledOnFailureBreakpoint = "enabled"
+)
+
 // TaskRunDebug defines the breakpoint config for a particular TaskRun
 type TaskRunDebug struct {
 	// +optional
-	// +listType=atomic
-	Breakpoint []string `json:"breakpoint,omitempty"`
+	Breakpoints *TaskBreakpoints `json:"breakpoints,omitempty"`
+}
+
+// TaskBreakpoints defines the breakpoint config for a particular Task
+type TaskBreakpoints struct {
+	// if enabled, pause TaskRun on failure of a step
+	// failed step will not exit
+	// +optional
+	OnFailure string `json:"onFailure,omitempty"`
+}
+
+// NeedsDebugOnFailure return true if the TaskRun is configured to debug on failure
+func (trd *TaskRunDebug) NeedsDebugOnFailure() bool {
+	if trd.Breakpoints == nil {
+		return false
+	}
+	return trd.Breakpoints.OnFailure == EnabledOnFailureBreakpoint
+}
+
+// StepNeedsDebug return true if the step is configured to debug
+func (trd *TaskRunDebug) StepNeedsDebug(stepName string) bool {
+	return trd.NeedsDebugOnFailure()
+}
+
+// NeedsDebug return true if defined onfailure or have any before, after steps
+func (trd *TaskRunDebug) NeedsDebug() bool {
+	return trd.NeedsDebugOnFailure()
 }
 
 // TaskRunInputs holds the input values that this task was invoked with.

diff --git a/pkg/apis/pipeline/v1/taskrun_types_test.go b/pkg/apis/pipeline/v1/taskrun_types_test.go
@@ -407,6 +407,83 @@ func TestInitializeTaskRunConditions(t *testing.T) {
 	}
 }
 
+func TestIsStepNeedDebug(t *testing.T) {
+	type args struct {
+		stepName string
+		trd      *v1.TaskRunDebug
+	}
+	testCases := []struct {
+		name string
+		args args
+		want bool
+	}{
+		{
+			name: "empty breakpoints",
+			args: args{
+				stepName: "step1",
+				trd:      &v1.TaskRunDebug{},
+			},
+			want: false,
+		}, {
+			name: "breakpoint on failure",
+			args: args{
+				stepName: "step1",
+				trd: &v1.TaskRunDebug{
+					Breakpoints: &v1.TaskBreakpoints{
+						OnFailure: "enabled",
+					},
+				},
+			},
+			want: true,
+		},
+	}
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			result := tc.args.trd.StepNeedsDebug(tc.args.stepName)
+			if d := cmp.Diff(result, tc.want); d != "" {
+				t.Fatalf(diff.PrintWantGot(d))
+			}
+		})
+	}
+}
+
+func TestIsNeedDebug(t *testing.T) {
+	type args struct {
+		trd *v1.TaskRunDebug
+	}
+	testCases := []struct {
+		name string
+		args args
+		want bool
+	}{
+		{
+			name: "empty breakpoints",
+			args: args{
+				trd: &v1.TaskRunDebug{},
+			},
+			want: false,
+		}, {
+			name: "breakpoint on failure",
+			args: args{
+				trd: &v1.TaskRunDebug{
+					Breakpoints: &v1.TaskBreakpoints{
+						OnFailure: "enabled",
+					},
+				},
+			},
+			want: true,
+		},
+	}
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			result := tc.args.trd.NeedsDebug()
+			if d := cmp.Diff(result, tc.want); d != "" {
+				t.Fatalf(diff.PrintWantGot(d))
+			}
+		})
+	}
+}
+
 func TestTaskRunIsRetriable(t *testing.T) {
 	retryStatus := v1.TaskRunStatus{}
 	retryStatus.SetCondition(&apis.Condition{

diff --git a/pkg/apis/pipeline/v1/taskrun_validation.go b/pkg/apis/pipeline/v1/taskrun_validation.go
@@ -216,16 +216,14 @@ func combineParamSpec(p ParamSpec, paramSpecForValidation map[string]ParamSpec)
 	return paramSpecForValidation, nil
 }
 
-// validateDebug
+// validateDebug validates the debug section of the TaskRun.
+// if set, onFailure breakpoint must be "enabled"
 func validateDebug(db *TaskRunDebug) (errs *apis.FieldError) {
-	breakpointOnFailure := "onFailure"
-	validBreakpoints := sets.NewString()
-	validBreakpoints.Insert(breakpointOnFailure)
-
-	for _, b := range db.Breakpoint {
-		if !validBreakpoints.Has(b) {
-			errs = errs.Also(apis.ErrInvalidValue(fmt.Sprintf("%s is not a valid breakpoint. Available valid breakpoints include %s", b, validBreakpoints.List()), "breakpoint"))
-		}
+	if db == nil || db.Breakpoints == nil {
+		return errs
+	}
+	if db.Breakpoints.OnFailure != "" && db.Breakpoints.OnFailure != EnabledOnFailureBreakpoint {
+		errs = errs.Also(apis.ErrInvalidValue(fmt.Sprintf("%s is not a valid onFailure breakpoint value, onFailure breakpoint is only allowed to be set as enabled", db.Breakpoints.OnFailure), "breakpoints.onFailure"))
 	}
 	return errs
 }