From 9790600ced7db28ed70a3a7f1b372aff8a5ca049 Mon Sep 17 00:00:00 2001 From: joey Date: Tue, 4 Apr 2023 11:46:31 +0800 Subject: [PATCH] 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 --- docs/pipeline-api.md | 74 +++++++++++++++++- docs/taskruns.md | 5 +- pkg/apis/pipeline/v1/openapi_generated.go | 41 ++++++---- pkg/apis/pipeline/v1/swagger.json | 19 +++-- pkg/apis/pipeline/v1/taskrun_types.go | 29 ++++++- pkg/apis/pipeline/v1/taskrun_types_test.go | 77 +++++++++++++++++++ pkg/apis/pipeline/v1/taskrun_validation.go | 9 --- .../pipeline/v1/taskrun_validation_test.go | 16 +--- pkg/apis/pipeline/v1/zz_generated.deepcopy.go | 24 +++++- .../pipeline/v1beta1/openapi_generated.go | 41 ++++++---- pkg/apis/pipeline/v1beta1/swagger.json | 19 +++-- .../pipeline/v1beta1/taskrun_conversion.go | 19 ++++- .../v1beta1/taskrun_conversion_test.go | 8 +- pkg/apis/pipeline/v1beta1/taskrun_types.go | 29 ++++++- .../pipeline/v1beta1/taskrun_types_test.go | 77 +++++++++++++++++++ .../pipeline/v1beta1/taskrun_validation.go | 9 --- .../v1beta1/taskrun_validation_test.go | 16 +--- .../pipeline/v1beta1/zz_generated.deepcopy.go | 24 +++++- pkg/pod/entrypoint.go | 12 +-- pkg/pod/entrypoint_test.go | 4 +- pkg/pod/pod_test.go | 8 +- pkg/pod/script.go | 17 ++-- pkg/pod/script_test.go | 4 +- 23 files changed, 443 insertions(+), 138 deletions(-) diff --git a/docs/pipeline-api.md b/docs/pipeline-api.md index 7c080cb4f7a..99142c428fe 100644 --- a/docs/pipeline-api.md +++ b/docs/pipeline-api.md @@ -4493,6 +4493,37 @@ More info: TaskBreakpoints + +

+(Appears on:TaskRunDebug) +

+
+

TaskBreakpoints defines the breakpoint config for a particular Task

+
+ + + + + + + + + + + + + +
FieldDescription
+onFailure
+ +bool + +		 +(Optional) +

if enabled, pause TaskRun on failure of a step +failed step will not exit

+

TaskKind (string alias)

@@ -4674,9 +4705,11 @@ string -breakpoint
+breakpoints
-[]string + +TaskBreakpoints + @@ -12931,6 +12964,37 @@ Default is false.

+

TaskBreakpoints +

+

+(Appears on:TaskRunDebug) +

+
+

TaskBreakpoints defines the breakpoint config for a particular Task

+
+ + + + + + + + + + + + + +
FieldDescription
+onFailure
+ +bool + +		 +(Optional) +

if enabled, pause TaskRun on failure of a step +failed step will not exit

+

TaskKind (string alias)

@@ -13266,9 +13330,11 @@ conditions such as one used in spire results verification

-breakpoint
+breakpoints
-[]string + +TaskBreakpoints + diff --git a/docs/taskruns.md b/docs/taskruns.md index 9894d1f352c..b6f10a091eb 100644 --- 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 index c3b755875be..303801a4705 100644 --- a/pkg/apis/pipeline/v1/openapi_generated.go +++ b/pkg/apis/pipeline/v1/openapi_generated.go @@ -72,6 +72,7 @@ func GetOpenAPIDefinitions(ref common.ReferenceCallback) map[string]common.OpenA "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1.StepState": schema_pkg_apis_pipeline_v1_StepState(ref), "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1.StepTemplate": schema_pkg_apis_pipeline_v1_StepTemplate(ref), "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1.Task": schema_pkg_apis_pipeline_v1_Task(ref), + "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1.TaskBreakpoints": schema_pkg_apis_pipeline_v1_TaskBreakpoints(ref), "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1.TaskList": schema_pkg_apis_pipeline_v1_TaskList(ref), "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1.TaskRef": schema_pkg_apis_pipeline_v1_TaskRef(ref), "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1.TaskResult": schema_pkg_apis_pipeline_v1_TaskResult(ref), @@ -3197,6 +3198,26 @@ func schema_pkg_apis_pipeline_v1_Task(ref common.ReferenceCallback) common.OpenA } } +func schema_pkg_apis_pipeline_v1_TaskBreakpoints(ref common.ReferenceCallback) common.OpenAPIDefinition { + return common.OpenAPIDefinition{ + Schema: spec.Schema{ + SchemaProps: spec.SchemaProps{ + Description: "TaskBreakpoints defines the breakpoint config for a particular Task", + Type: []string{"object"}, + Properties: map[string]spec.Schema{ + "onFailure": { + SchemaProps: spec.SchemaProps{ + Description: "if enabled, pause TaskRun on failure of a step failed step will not exit", + Type: []string{"boolean"}, + Format: "", + }, + }, + }, + }, + }, + } +} + func schema_pkg_apis_pipeline_v1_TaskList(ref common.ReferenceCallback) common.OpenAPIDefinition { return common.OpenAPIDefinition{ Schema: spec.Schema{ @@ -3387,28 +3408,16 @@ func schema_pkg_apis_pipeline_v1_TaskRunDebug(ref common.ReferenceCallback) comm Description: "TaskRunDebug defines the breakpoint config for a particular TaskRun", Type: []string{"object"}, Properties: map[string]spec.Schema{ - "breakpoint": { - VendorExtensible: spec.VendorExtensible{ - Extensions: spec.Extensions{ - "x-kubernetes-list-type": "atomic", - }, - }, + "breakpoints": { SchemaProps: spec.SchemaProps{ - Type: []string{"array"}, - Items: &spec.SchemaOrArray{ - Schema: &spec.Schema{ - SchemaProps: spec.SchemaProps{ - Default: "", - Type: []string{"string"}, - Format: "", - }, - }, - }, + Ref: ref("github.com/tektoncd/pipeline/pkg/apis/pipeline/v1.TaskBreakpoints"), }, }, }, }, }, + Dependencies: []string{ + "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1.TaskBreakpoints"}, } } diff --git a/pkg/apis/pipeline/v1/swagger.json b/pkg/apis/pipeline/v1/swagger.json index 154d596bbc3..ba72a0e4a29 100644 --- 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 index 3ab15ffe319..ab4c4a4ee7a 100644 --- 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 index eb8f717098c..d9bc89f1887 100644 --- 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 index 055094bb6a1..f24ac932d78 100644 --- 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 index c49db14dda1..24c441925fa 100644 --- 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{ diff --git a/pkg/apis/pipeline/v1/zz_generated.deepcopy.go b/pkg/apis/pipeline/v1/zz_generated.deepcopy.go index ebd05545106..b01aff13ac6 100644 --- a/pkg/apis/pipeline/v1/zz_generated.deepcopy.go +++ b/pkg/apis/pipeline/v1/zz_generated.deepcopy.go @@ -1384,6 +1384,22 @@ func (in *Task) DeepCopyObject() runtime.Object { return nil } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *TaskBreakpoints) DeepCopyInto(out *TaskBreakpoints) { + *out = *in + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new TaskBreakpoints. +func (in *TaskBreakpoints) DeepCopy() *TaskBreakpoints { + if in == nil { + return nil + } + out := new(TaskBreakpoints) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *TaskList) DeepCopyInto(out *TaskList) { *out = *in @@ -1488,10 +1504,10 @@ func (in *TaskRun) DeepCopyObject() runtime.Object { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *TaskRunDebug) DeepCopyInto(out *TaskRunDebug) { *out = *in - if in.Breakpoint != nil { - in, out := &in.Breakpoint, &out.Breakpoint - *out = make([]string, len(*in)) - copy(*out, *in) + if in.Breakpoints != nil { + in, out := &in.Breakpoints, &out.Breakpoints + *out = new(TaskBreakpoints) + **out = **in } return } diff --git a/pkg/apis/pipeline/v1beta1/openapi_generated.go b/pkg/apis/pipeline/v1beta1/openapi_generated.go index dd0dec17bdf..be38e318e74 100644 --- a/pkg/apis/pipeline/v1beta1/openapi_generated.go +++ b/pkg/apis/pipeline/v1beta1/openapi_generated.go @@ -87,6 +87,7 @@ func GetOpenAPIDefinitions(ref common.ReferenceCallback) map[string]common.OpenA "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.StepState": schema_pkg_apis_pipeline_v1beta1_StepState(ref), "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.StepTemplate": schema_pkg_apis_pipeline_v1beta1_StepTemplate(ref), "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.Task": schema_pkg_apis_pipeline_v1beta1_Task(ref), + "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.TaskBreakpoints": schema_pkg_apis_pipeline_v1beta1_TaskBreakpoints(ref), "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.TaskList": schema_pkg_apis_pipeline_v1beta1_TaskList(ref), "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.TaskRef": schema_pkg_apis_pipeline_v1beta1_TaskRef(ref), "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.TaskResource": schema_pkg_apis_pipeline_v1beta1_TaskResource(ref), @@ -4236,6 +4237,26 @@ func schema_pkg_apis_pipeline_v1beta1_Task(ref common.ReferenceCallback) common. } } +func schema_pkg_apis_pipeline_v1beta1_TaskBreakpoints(ref common.ReferenceCallback) common.OpenAPIDefinition { + return common.OpenAPIDefinition{ + Schema: spec.Schema{ + SchemaProps: spec.SchemaProps{ + Description: "TaskBreakpoints defines the breakpoint config for a particular Task", + Type: []string{"object"}, + Properties: map[string]spec.Schema{ + "onFailure": { + SchemaProps: spec.SchemaProps{ + Description: "if enabled, pause TaskRun on failure of a step failed step will not exit", + Type: []string{"boolean"}, + Format: "", + }, + }, + }, + }, + }, + } +} + func schema_pkg_apis_pipeline_v1beta1_TaskList(ref common.ReferenceCallback) common.OpenAPIDefinition { return common.OpenAPIDefinition{ Schema: spec.Schema{ @@ -4591,28 +4612,16 @@ func schema_pkg_apis_pipeline_v1beta1_TaskRunDebug(ref common.ReferenceCallback) Description: "TaskRunDebug defines the breakpoint config for a particular TaskRun", Type: []string{"object"}, Properties: map[string]spec.Schema{ - "breakpoint": { - VendorExtensible: spec.VendorExtensible{ - Extensions: spec.Extensions{ - "x-kubernetes-list-type": "atomic", - }, - }, + "breakpoints": { SchemaProps: spec.SchemaProps{ - Type: []string{"array"}, - Items: &spec.SchemaOrArray{ - Schema: &spec.Schema{ - SchemaProps: spec.SchemaProps{ - Default: "", - Type: []string{"string"}, - Format: "", - }, - }, - }, + Ref: ref("github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.TaskBreakpoints"), }, }, }, }, }, + Dependencies: []string{ + "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.TaskBreakpoints"}, } } diff --git a/pkg/apis/pipeline/v1beta1/swagger.json b/pkg/apis/pipeline/v1beta1/swagger.json index bacfd7bcb46..8d1d9f0872a 100644 --- a/pkg/apis/pipeline/v1beta1/swagger.json +++ b/pkg/apis/pipeline/v1beta1/swagger.json @@ -2341,6 +2341,16 @@ } } }, + "v1beta1.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" + } + } + }, "v1beta1.TaskList": { "description": "TaskList contains a list of Task", "type": "object", @@ -2534,13 +2544,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/v1beta1.TaskBreakpoints" } } }, diff --git a/pkg/apis/pipeline/v1beta1/taskrun_conversion.go b/pkg/apis/pipeline/v1beta1/taskrun_conversion.go index 726448ab104..350078c1c24 100644 --- a/pkg/apis/pipeline/v1beta1/taskrun_conversion.go +++ b/pkg/apis/pipeline/v1beta1/taskrun_conversion.go @@ -185,11 +185,26 @@ func (trs *TaskRunSpec) ConvertFrom(ctx context.Context, source *v1.TaskRunSpec) } func (trd TaskRunDebug) convertTo(ctx context.Context, sink *v1.TaskRunDebug) { - sink.Breakpoint = trd.Breakpoint + if trd.Breakpoints != nil { + sink.Breakpoints = &v1.TaskBreakpoints{} + trd.Breakpoints.convertTo(ctx, sink.Breakpoints) + } } func (trd *TaskRunDebug) convertFrom(ctx context.Context, source v1.TaskRunDebug) { - trd.Breakpoint = source.Breakpoint + if source.Breakpoints != nil { + newBreakpoints := TaskBreakpoints{} + newBreakpoints.convertFrom(ctx, *source.Breakpoints) + trd.Breakpoints = &newBreakpoints + } +} + +func (tbp TaskBreakpoints) convertTo(ctx context.Context, sink *v1.TaskBreakpoints) { + sink.OnFailure = tbp.OnFailure +} + +func (tbp *TaskBreakpoints) convertFrom(ctx context.Context, source v1.TaskBreakpoints) { + tbp.OnFailure = source.OnFailure } func (trso TaskRunStepOverride) convertTo(ctx context.Context, sink *v1.TaskRunStepSpec) { diff --git a/pkg/apis/pipeline/v1beta1/taskrun_conversion_test.go b/pkg/apis/pipeline/v1beta1/taskrun_conversion_test.go index 53cf3219b2d..cf06652f9c7 100644 --- a/pkg/apis/pipeline/v1beta1/taskrun_conversion_test.go +++ b/pkg/apis/pipeline/v1beta1/taskrun_conversion_test.go @@ -34,10 +34,6 @@ import ( duckv1 "knative.dev/pkg/apis/duck/v1" ) -const ( - breakpointOnFailure = "onFailure" -) - func TestTaskRunConversionBadType(t *testing.T) { good, bad := &v1beta1.TaskRun{}, &v1beta1.Task{} @@ -74,7 +70,9 @@ func TestTaskRunConversion(t *testing.T) { }, Spec: v1beta1.TaskRunSpec{ Debug: &v1beta1.TaskRunDebug{ - Breakpoint: []string{breakpointOnFailure}, + Breakpoints: &v1beta1.TaskBreakpoints{ + OnFailure: true, + }, }, Params: v1beta1.Params{{ Name: "param-task-1", diff --git a/pkg/apis/pipeline/v1beta1/taskrun_types.go b/pkg/apis/pipeline/v1beta1/taskrun_types.go index 005084b51cb..d638dff7596 100644 --- a/pkg/apis/pipeline/v1beta1/taskrun_types.go +++ b/pkg/apis/pipeline/v1beta1/taskrun_types.go @@ -112,8 +112,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() } var taskRunCondSet = apis.NewBatchConditionSet() diff --git a/pkg/apis/pipeline/v1beta1/taskrun_types_test.go b/pkg/apis/pipeline/v1beta1/taskrun_types_test.go index 8b11dad0fc8..89862cf1a63 100644 --- a/pkg/apis/pipeline/v1beta1/taskrun_types_test.go +++ b/pkg/apis/pipeline/v1beta1/taskrun_types_test.go @@ -391,6 +391,83 @@ func TestInitializeTaskRunConditions(t *testing.T) { } } +func TestIsStepNeedDebug(t *testing.T) { + type args struct { + stepName string + trd *v1beta1.TaskRunDebug + } + testCases := []struct { + name string + args args + want bool + }{ + { + name: "empty breakpoints", + args: args{ + stepName: "step1", + trd: &v1beta1.TaskRunDebug{}, + }, + want: false, + }, { + name: "breakpoint on failure", + args: args{ + stepName: "step1", + trd: &v1beta1.TaskRunDebug{ + Breakpoints: &v1beta1.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 *v1beta1.TaskRunDebug + } + testCases := []struct { + name string + args args + want bool + }{ + { + name: "empty breakpoints", + args: args{ + trd: &v1beta1.TaskRunDebug{}, + }, + want: false, + }, { + name: "breakpoint on failure", + args: args{ + trd: &v1beta1.TaskRunDebug{ + Breakpoints: &v1beta1.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 := v1beta1.TaskRunStatus{} retryStatus.SetCondition(&apis.Condition{ diff --git a/pkg/apis/pipeline/v1beta1/taskrun_validation.go b/pkg/apis/pipeline/v1beta1/taskrun_validation.go index ef414612b43..54a00f16d98 100644 --- a/pkg/apis/pipeline/v1beta1/taskrun_validation.go +++ b/pkg/apis/pipeline/v1beta1/taskrun_validation.go @@ -216,15 +216,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/v1beta1/taskrun_validation_test.go b/pkg/apis/pipeline/v1beta1/taskrun_validation_test.go index c99b8afeb10..481542559ca 100644 --- a/pkg/apis/pipeline/v1beta1/taskrun_validation_test.go +++ b/pkg/apis/pipeline/v1beta1/taskrun_validation_test.go @@ -633,22 +633,12 @@ func TestTaskRunSpec_Invalidate(t *testing.T) { Name: "my-task", }, Debug: &v1beta1.TaskRunDebug{ - Breakpoint: []string{"onFailure"}, + Breakpoints: &v1beta1.TaskBreakpoints{ + OnFailure: true, + }, }, }, wantErr: apis.ErrGeneric("debug requires \"enable-api-fields\" feature gate to be \"alpha\" but it is \"stable\""), - }, { - name: "invalid breakpoint", - spec: v1beta1.TaskRunSpec{ - TaskRef: &v1beta1.TaskRef{ - Name: "my-task", - }, - Debug: &v1beta1.TaskRunDebug{ - Breakpoint: []string{"breakito"}, - }, - }, - wantErr: apis.ErrInvalidValue("breakito is not a valid breakpoint. Available valid breakpoints include [onFailure]", "debug.breakpoint"), - wc: config.EnableAlphaAPIFields, }, { name: "stepOverride disallowed without alpha feature gate", spec: v1beta1.TaskRunSpec{ diff --git a/pkg/apis/pipeline/v1beta1/zz_generated.deepcopy.go b/pkg/apis/pipeline/v1beta1/zz_generated.deepcopy.go index c90b2f1b169..d42ff8b08e7 100644 --- a/pkg/apis/pipeline/v1beta1/zz_generated.deepcopy.go +++ b/pkg/apis/pipeline/v1beta1/zz_generated.deepcopy.go @@ -1868,6 +1868,22 @@ func (in *Task) DeepCopyObject() runtime.Object { return nil } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *TaskBreakpoints) DeepCopyInto(out *TaskBreakpoints) { + *out = *in + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new TaskBreakpoints. +func (in *TaskBreakpoints) DeepCopy() *TaskBreakpoints { + if in == nil { + return nil + } + out := new(TaskBreakpoints) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *TaskList) DeepCopyInto(out *TaskList) { *out = *in @@ -2037,10 +2053,10 @@ func (in *TaskRun) DeepCopyObject() runtime.Object { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *TaskRunDebug) DeepCopyInto(out *TaskRunDebug) { *out = *in - if in.Breakpoint != nil { - in, out := &in.Breakpoint, &out.Breakpoint - *out = make([]string, len(*in)) - copy(*out, *in) + if in.Breakpoints != nil { + in, out := &in.Breakpoints, &out.Breakpoints + *out = new(TaskBreakpoints) + **out = **in } return } diff --git a/pkg/pod/entrypoint.go b/pkg/pod/entrypoint.go index e62b04b6fc3..e50e3500310 100644 --- a/pkg/pod/entrypoint.go +++ b/pkg/pod/entrypoint.go @@ -59,8 +59,6 @@ const ( stepPrefix = "step-" sidecarPrefix = "sidecar-" - - breakpointOnFailure = "onFailure" ) var ( @@ -162,14 +160,8 @@ func orderContainers(commonExtraEntrypointArgs []string, steps []corev1.Containe argsForEntrypoint = append(argsForEntrypoint, resultArgument(steps, taskSpec.Results)...) } - if breakpointConfig != nil && len(breakpointConfig.Breakpoint) > 0 { - breakpoints := breakpointConfig.Breakpoint - for _, b := range breakpoints { - // TODO(TEP #0042): Add other breakpoints - if b == breakpointOnFailure { - argsForEntrypoint = append(argsForEntrypoint, "-breakpoint_on_failure") - } - } + if breakpointConfig != nil && breakpointConfig.NeedsDebugOnFailure() { + argsForEntrypoint = append(argsForEntrypoint, "-breakpoint_on_failure") } cmd, args := s.Command, s.Args diff --git a/pkg/pod/entrypoint_test.go b/pkg/pod/entrypoint_test.go index 261862f1360..dfde6043dfa 100644 --- a/pkg/pod/entrypoint_test.go +++ b/pkg/pod/entrypoint_test.go @@ -241,7 +241,9 @@ func TestOrderContainersWithDebugOnFailure(t *testing.T) { TerminationMessagePath: "/tekton/termination", }} taskRunDebugConfig := &v1beta1.TaskRunDebug{ - Breakpoint: []string{"onFailure"}, + Breakpoints: &v1beta1.TaskBreakpoints{ + OnFailure: true, + }, } got, err := orderContainers([]string{}, steps, nil, taskRunDebugConfig, true) if err != nil { diff --git a/pkg/pod/pod_test.go b/pkg/pod/pod_test.go index 37f62924c00..e612e50232c 100644 --- a/pkg/pod/pod_test.go +++ b/pkg/pod/pod_test.go @@ -143,7 +143,9 @@ func TestPodBuild(t *testing.T) { desc: "simple with breakpoint onFailure enabled, alpha api fields disabled", trs: v1beta1.TaskRunSpec{ Debug: &v1beta1.TaskRunDebug{ - Breakpoint: []string{breakpointOnFailure}, + Breakpoints: &v1beta1.TaskBreakpoints{ + OnFailure: true, + }, }, }, ts: v1beta1.TaskSpec{ @@ -2133,7 +2135,9 @@ debug-fail-continue-heredoc-randomly-generated-mz4c7 desc: "simple with debug breakpoint onFailure", trs: v1beta1.TaskRunSpec{ Debug: &v1beta1.TaskRunDebug{ - Breakpoint: []string{breakpointOnFailure}, + Breakpoints: &v1beta1.TaskBreakpoints{ + OnFailure: true, + }, }, }, ts: v1beta1.TaskSpec{ diff --git a/pkg/pod/script.go b/pkg/pod/script.go index 23498a0b0d9..828f9dd27fb 100644 --- a/pkg/pod/script.go +++ b/pkg/pod/script.go @@ -95,7 +95,6 @@ func convertScripts(shellImageLinux string, shellImageWin string, steps []v1beta VolumeMounts: []corev1.VolumeMount{writeScriptsVolumeMount, binMount}, } - breakpoints := []string{} sideCarSteps := []v1beta1.Step{} for _, sidecar := range sidecars { c := sidecar.ToK8sContainer() @@ -108,15 +107,14 @@ func convertScripts(shellImageLinux string, shellImageWin string, steps []v1beta } // Add mounts for debug - if debugConfig != nil && len(debugConfig.Breakpoint) > 0 { - breakpoints = debugConfig.Breakpoint + if debugConfig != nil && debugConfig.NeedsDebug() { placeScriptsInit.VolumeMounts = append(placeScriptsInit.VolumeMounts, debugScriptsVolumeMount) } - convertedStepContainers := convertListOfSteps(steps, &placeScriptsInit, &placeScripts, breakpoints, "script") + convertedStepContainers := convertListOfSteps(steps, &placeScriptsInit, &placeScripts, debugConfig, "script") // Pass empty breakpoint list in "sidecar step to container" converter to not rewrite the scripts and add breakpoints to sidecar - sidecarContainers := convertListOfSteps(sideCarSteps, &placeScriptsInit, &placeScripts, []string{}, "sidecar-script") + sidecarContainers := convertListOfSteps(sideCarSteps, &placeScriptsInit, &placeScripts, nil, "sidecar-script") if placeScripts { return &placeScriptsInit, convertedStepContainers, sidecarContainers } @@ -127,11 +125,12 @@ func convertScripts(shellImageLinux string, shellImageWin string, steps []v1beta // // It iterates through the list of steps (or sidecars), generates the script file name and heredoc termination string, // adds an entry to the init container args, sets up the step container to run the script, and sets the volume mounts. -func convertListOfSteps(steps []v1beta1.Step, initContainer *corev1.Container, placeScripts *bool, breakpoints []string, namePrefix string) []corev1.Container { +func convertListOfSteps(steps []v1beta1.Step, initContainer *corev1.Container, placeScripts *bool, debugConfig *v1beta1.TaskRunDebug, namePrefix string) []corev1.Container { containers := []corev1.Container{} + isDebugOnFailure := debugConfig != nil && debugConfig.NeedsDebugOnFailure() for i, s := range steps { // Add debug mounts if breakpoints are present - if len(breakpoints) > 0 { + if isDebugOnFailure { debugInfoVolumeMount := corev1.VolumeMount{ Name: debugInfoVolumeName, MountPath: filepath.Join(debugInfoDir, fmt.Sprintf("%d", i)), @@ -178,7 +177,7 @@ func convertListOfSteps(steps []v1beta1.Step, initContainer *corev1.Container, p // Only encode the script for linux scripts // The decode-script subcommand of the entrypoint does not work under windows script = encodeScript(script) - heredoc := "_EOF_" // underscores because base64 doesnt include them in its alphabet + heredoc := "_EOF_" // underscores because base64 doesn't include them in its alphabet initContainer.Args[1] += fmt.Sprintf(`scriptfile="%s" touch ${scriptfile} && chmod +x ${scriptfile} cat > ${scriptfile} << '%s' @@ -200,7 +199,7 @@ cat > ${scriptfile} << '%s' } // Place debug scripts if breakpoints are enabled - if len(breakpoints) > 0 { + if isDebugOnFailure { // If breakpoint is not nil then should add the init container // to write debug script files *placeScripts = true diff --git a/pkg/pod/script_test.go b/pkg/pod/script_test.go index f341be86d42..950953ec101 100644 --- a/pkg/pod/script_test.go +++ b/pkg/pod/script_test.go @@ -225,7 +225,9 @@ script-3`, VolumeMounts: preExistingVolumeMounts, Args: []string{"my", "args"}, }}, []v1beta1.Sidecar{}, &v1beta1.TaskRunDebug{ - Breakpoint: []string{breakpointOnFailure}, + Breakpoints: &v1beta1.TaskBreakpoints{ + OnFailure: true, + }, }) wantInit := &corev1.Container{