diff --git a/docs/pipeline-api.md b/docs/pipeline-api.md index de0deb66868..c4267286f6e 100644 --- a/docs/pipeline-api.md +++ b/docs/pipeline-api.md @@ -4607,6 +4607,37 @@ More info: TaskBreakpoints + +

+(Appears on:TaskRunDebug) +

+
+

TaskBreakpoints defines the breakpoint config for a particular Task

+
+ + + + + + + + + + + + + +
FieldDescription
+onFailure
+ +string + +
+(Optional) +

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

+

TaskKind (string alias)

@@ -4792,9 +4823,11 @@ string -breakpoint
+breakpoints
-[]string + +TaskBreakpoints + @@ -13069,6 +13102,37 @@ Default is false.

+

TaskBreakpoints +

+

+(Appears on:TaskRunDebug) +

+
+

TaskBreakpoints defines the breakpoint config for a particular Task

+
+ + + + + + + + + + + + + +
FieldDescription
+onFailure
+ +string + +
+(Optional) +

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

+

TaskKind (string alias)

@@ -13404,9 +13468,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 35b0cfaab18..4cb81ed0bb1 100644 --- 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 index c3011075abc..a48fd8bc68e 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), @@ -3209,6 +3210,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{"string"}, + Format: "", + }, + }, + }, + }, + }, + } +} + func schema_pkg_apis_pipeline_v1_TaskList(ref common.ReferenceCallback) common.OpenAPIDefinition { return common.OpenAPIDefinition{ Schema: spec.Schema{ @@ -3399,28 +3420,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 e2f82a64734..43cc932c969 100644 --- 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 index 30095d53525..cc273950a91 100644 --- 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 index 88fd3d488fe..d4f1fefc4c9 100644 --- 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 index c818a5760c2..dbbeb1c0201 100644 --- 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 } diff --git a/pkg/apis/pipeline/v1/taskrun_validation_test.go b/pkg/apis/pipeline/v1/taskrun_validation_test.go index 7e0863c9695..8dae7195b2f 100644 --- a/pkg/apis/pipeline/v1/taskrun_validation_test.go +++ b/pkg/apis/pipeline/v1/taskrun_validation_test.go @@ -638,22 +638,26 @@ func TestTaskRunSpec_Invalidate(t *testing.T) { Name: "my-task", }, Debug: &v1.TaskRunDebug{ - Breakpoint: []string{"onFailure"}, + Breakpoints: &v1.TaskBreakpoints{ + OnFailure: "enabled", + }, }, }, wc: cfgtesting.EnableStableAPIFields, wantErr: apis.ErrGeneric("debug requires \"enable-api-fields\" feature gate to be \"alpha\" but it is \"stable\""), }, { - name: "invalid breakpoint", + name: "invalid onFailure breakpoint", spec: v1.TaskRunSpec{ TaskRef: &v1.TaskRef{ Name: "my-task", }, Debug: &v1.TaskRunDebug{ - Breakpoint: []string{"breakito"}, + Breakpoints: &v1.TaskBreakpoints{ + OnFailure: "turnOn", + }, }, }, - wantErr: apis.ErrInvalidValue("breakito is not a valid breakpoint. Available valid breakpoints include [onFailure]", "debug.breakpoint"), + wantErr: apis.ErrInvalidValue("turnOn is not a valid onFailure breakpoint value, onFailure breakpoint is only allowed to be set as enabled", "debug.breakpoints.onFailure"), wc: cfgtesting.EnableAlphaAPIFields, }, { name: "stepSpecs disallowed without alpha feature gate", diff --git a/pkg/apis/pipeline/v1/zz_generated.deepcopy.go b/pkg/apis/pipeline/v1/zz_generated.deepcopy.go index 484d08a4341..d47e242753c 100644 --- a/pkg/apis/pipeline/v1/zz_generated.deepcopy.go +++ b/pkg/apis/pipeline/v1/zz_generated.deepcopy.go @@ -1394,6 +1394,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 @@ -1498,10 +1514,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 71844dd01d9..a1ac309a000 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), @@ -4256,6 +4257,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{"string"}, + Format: "", + }, + }, + }, + }, + }, + } +} + func schema_pkg_apis_pipeline_v1beta1_TaskList(ref common.ReferenceCallback) common.OpenAPIDefinition { return common.OpenAPIDefinition{ Schema: spec.Schema{ @@ -4611,28 +4632,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 47aafa9a098..8002b1f1b0b 100644 --- a/pkg/apis/pipeline/v1beta1/swagger.json +++ b/pkg/apis/pipeline/v1beta1/swagger.json @@ -2359,6 +2359,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": "string" + } + } + }, "v1beta1.TaskList": { "description": "TaskList contains a list of Task", "type": "object", @@ -2552,13 +2562,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 59a33850f9b..ed3fcc5856e 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 eb530d34c5a..13a870e2728 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{} @@ -115,7 +111,9 @@ func TestTaskRunConversion(t *testing.T) { }, Spec: v1beta1.TaskRunSpec{ Debug: &v1beta1.TaskRunDebug{ - Breakpoint: []string{breakpointOnFailure}, + Breakpoints: &v1beta1.TaskBreakpoints{ + OnFailure: "enabled", + }, }, 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 134e1c8137c..2b869121d2c 100644 --- a/pkg/apis/pipeline/v1beta1/taskrun_types.go +++ b/pkg/apis/pipeline/v1beta1/taskrun_types.go @@ -108,11 +108,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() } 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 81e4a748d4d..7412e783109 100644 --- a/pkg/apis/pipeline/v1beta1/taskrun_types_test.go +++ b/pkg/apis/pipeline/v1beta1/taskrun_types_test.go @@ -467,6 +467,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: "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 *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: "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 := 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 300fd4aa930..22ef336c95d 100644 --- a/pkg/apis/pipeline/v1beta1/taskrun_validation.go +++ b/pkg/apis/pipeline/v1beta1/taskrun_validation.go @@ -213,16 +213,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 } diff --git a/pkg/apis/pipeline/v1beta1/taskrun_validation_test.go b/pkg/apis/pipeline/v1beta1/taskrun_validation_test.go index f933df00dcf..1aaa409741c 100644 --- a/pkg/apis/pipeline/v1beta1/taskrun_validation_test.go +++ b/pkg/apis/pipeline/v1beta1/taskrun_validation_test.go @@ -626,22 +626,26 @@ func TestTaskRunSpec_Invalidate(t *testing.T) { Name: "my-task", }, Debug: &v1beta1.TaskRunDebug{ - Breakpoint: []string{"onFailure"}, + Breakpoints: &v1beta1.TaskBreakpoints{ + OnFailure: "enabled", + }, }, }, wc: cfgtesting.EnableStableAPIFields, wantErr: apis.ErrGeneric("debug requires \"enable-api-fields\" feature gate to be \"alpha\" but it is \"stable\""), }, { - name: "invalid breakpoint", + name: "invalid onFailure breakpoint", spec: v1beta1.TaskRunSpec{ TaskRef: &v1beta1.TaskRef{ Name: "my-task", }, Debug: &v1beta1.TaskRunDebug{ - Breakpoint: []string{"breakito"}, + Breakpoints: &v1beta1.TaskBreakpoints{ + OnFailure: "turnOn", + }, }, }, - wantErr: apis.ErrInvalidValue("breakito is not a valid breakpoint. Available valid breakpoints include [onFailure]", "debug.breakpoint"), + wantErr: apis.ErrInvalidValue("turnOn is not a valid onFailure breakpoint value, onFailure breakpoint is only allowed to be set as enabled", "debug.breakpoints.onFailure"), wc: cfgtesting.EnableAlphaAPIFields, }, { name: "stepOverride disallowed without alpha feature gate", diff --git a/pkg/apis/pipeline/v1beta1/zz_generated.deepcopy.go b/pkg/apis/pipeline/v1beta1/zz_generated.deepcopy.go index c158be1779f..aa44da6184e 100644 --- a/pkg/apis/pipeline/v1beta1/zz_generated.deepcopy.go +++ b/pkg/apis/pipeline/v1beta1/zz_generated.deepcopy.go @@ -1891,6 +1891,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 @@ -2060,10 +2076,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 aebd06cafbd..f1cb453dc55 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 38ac79b68ba..766d9d3c1c4 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 := &v1.TaskRunDebug{ - Breakpoint: []string{"onFailure"}, + Breakpoints: &v1.TaskBreakpoints{ + OnFailure: "enabled", + }, } 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 a0929a5835b..ff5ee7db064 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: v1.TaskRunSpec{ Debug: &v1.TaskRunDebug{ - Breakpoint: []string{breakpointOnFailure}, + Breakpoints: &v1.TaskBreakpoints{ + OnFailure: "enabled", + }, }, }, ts: v1.TaskSpec{ @@ -2247,7 +2249,9 @@ debug-fail-continue-heredoc-randomly-generated-mz4c7 desc: "simple with debug breakpoint onFailure", trs: v1.TaskRunSpec{ Debug: &v1.TaskRunDebug{ - Breakpoint: []string{breakpointOnFailure}, + Breakpoints: &v1.TaskBreakpoints{ + OnFailure: "enabled", + }, }, }, ts: v1.TaskSpec{ diff --git a/pkg/pod/script.go b/pkg/pod/script.go index 9c4af99f9c8..32ee47ae2e6 100644 --- a/pkg/pod/script.go +++ b/pkg/pod/script.go @@ -106,18 +106,15 @@ func convertScripts(shellImageLinux string, shellImageWin string, steps []v1.Ste placeScriptsInit.SecurityContext = securityContext } - breakpoints := []string{} - // 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, breakpoints, "script") + convertedStepContainers := convertListOfSteps(steps, &placeScriptsInit, debugConfig, "script") sidecarContainers := convertListOfSidecars(sidecars, &placeScriptsInit, "sidecar-script") - if hasScripts(steps, sidecars, breakpoints) { + if hasScripts(steps, sidecars, debugConfig) { return &placeScriptsInit, convertedStepContainers, sidecarContainers } return nil, convertedStepContainers, sidecarContainers @@ -139,7 +136,7 @@ func convertListOfSidecars(sidecars []v1.Sidecar, initContainer *corev1.Containe // convertListOfSteps iterates through the list of steps, 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 []v1.Step, initContainer *corev1.Container, breakpoints []string, namePrefix string) []corev1.Container { +func convertListOfSteps(steps []v1.Step, initContainer *corev1.Container, debugConfig *v1.TaskRunDebug, namePrefix string) []corev1.Container { containers := []corev1.Container{} for i, s := range steps { c := steps[i].ToK8sContainer() @@ -148,7 +145,7 @@ func convertListOfSteps(steps []v1.Step, initContainer *corev1.Container, breakp } containers = append(containers, *c) } - if len(breakpoints) > 0 { + if debugConfig != nil && debugConfig.NeedsDebugOnFailure() { placeDebugScriptInContainers(containers, initContainer) } return containers @@ -248,7 +245,7 @@ func placeDebugScriptInContainers(containers []corev1.Container, initContainer * } // hasScripts determines if we need to generate scripts in InitContainer given steps, sidecars and breakpoints. -func hasScripts(steps []v1.Step, sidecars []v1.Sidecar, breakpoints []string) bool { +func hasScripts(steps []v1.Step, sidecars []v1.Sidecar, debugConfig *v1.TaskRunDebug) bool { for _, s := range steps { if s.Script != "" { return true @@ -259,7 +256,7 @@ func hasScripts(steps []v1.Step, sidecars []v1.Sidecar, breakpoints []string) bo return true } } - return len(breakpoints) > 0 + return debugConfig != nil && debugConfig.NeedsDebug() } func checkWindowsRequirement(steps []v1.Step, sidecars []v1.Sidecar) bool { diff --git a/pkg/pod/script_test.go b/pkg/pod/script_test.go index 668c2a9061b..93c1ed51ce5 100644 --- a/pkg/pod/script_test.go +++ b/pkg/pod/script_test.go @@ -384,7 +384,9 @@ script-3`, VolumeMounts: preExistingVolumeMounts, Args: []string{"my", "args"}, }}, []v1.Sidecar{}, &v1.TaskRunDebug{ - Breakpoint: []string{breakpointOnFailure}, + Breakpoints: &v1.TaskBreakpoints{ + OnFailure: "enabled", + }, }, true) wantInit := &corev1.Container{