TEP-0097 breakpoints for taskrun

make TaskBreakpoints struct for taskrun debug. breakpoint on failure of a step was moved to `breakpoints.onFailure` spec and change field type from string to boolean Signed-off-by: chengjoey <zchengjoey@gmail.com>
tektoncd · Apr 5, 2023 · 9790600 · 9790600
1 parent 8cb2529
commit 9790600
Show file tree

Hide file tree

Showing 23 changed files with 443 additions and 138 deletions.
diff --git a/docs/pipeline-api.md b/docs/pipeline-api.md
@@ -4493,6 +4493,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>
+bool
+</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>
@@ -4674,9 +4705,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>
@@ -12931,6 +12964,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>
+bool
+</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>
@@ -13266,9 +13330,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
@@ -827,7 +827,8 @@ TaskRuns can be halted on failure for troubleshooting by providing the following
 ```yaml
 spec:
   debug:
-    breakpoint: ["onFailure"]
+    breakpoints:
+      onFailure: true
 ```
 
 Upon failure of a step, the TaskRun Pod execution is halted. If this TaskRun Pod continues to run without any lifecycle
@@ -836,7 +837,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
@@ -1637,6 +1637,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": "boolean"
+        }
+      }
+    },
     "v1.TaskList": {
       "description": "TaskList contains a list of Task",
       "type": "object",
@@ -1743,13 +1753,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
@@ -106,8 +106,33 @@ const (
 // 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 bool `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
+}
+
+// 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
@@ -331,6 +331,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: true,
+					},
+				},
+			},
+			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: true,
+					},
+				},
+			},
+			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
@@ -214,15 +214,6 @@ func combineParamSpec(p ParamSpec, paramSpecForValidation map[string]ParamSpec)
 
 // validateDebug
 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"))
-		}
-	}
 	return errs
 }
 

diff --git a/pkg/apis/pipeline/v1/taskrun_validation_test.go b/pkg/apis/pipeline/v1/taskrun_validation_test.go
@@ -637,22 +637,12 @@ func TestTaskRunSpec_Invalidate(t *testing.T) {
 				Name: "my-task",
 			},
 			Debug: &v1.TaskRunDebug{
-				Breakpoint: []string{"onFailure"},
+				Breakpoints: &v1.TaskBreakpoints{
+					OnFailure: true,
+				},
 			},
 		},
 		wantErr: apis.ErrGeneric("debug requires \"enable-api-fields\" feature gate to be \"alpha\" but it is \"stable\""),
-	}, {
-		name: "invalid breakpoint",
-		spec: v1.TaskRunSpec{
-			TaskRef: &v1.TaskRef{
-				Name: "my-task",
-			},
-			Debug: &v1.TaskRunDebug{
-				Breakpoint: []string{"breakito"},
-			},
-		},
-		wantErr: apis.ErrInvalidValue("breakito is not a valid breakpoint. Available valid breakpoints include [onFailure]", "debug.breakpoint"),
-		wc:      config.EnableAlphaAPIFields,
 	}, {
 		name: "stepSpecs disallowed without alpha feature gate",
 		spec: v1.TaskRunSpec{