diff --git a/cmd/entrypoint/main.go b/cmd/entrypoint/main.go index 5352bf3dfac..44c8fe5de3a 100644 --- a/cmd/entrypoint/main.go +++ b/cmd/entrypoint/main.go @@ -17,7 +17,6 @@ limitations under the License. package main import ( - "context" "encoding/json" "errors" "flag" @@ -56,6 +55,7 @@ var ( stdoutPath = flag.String("stdout_path", "", "If specified, file to copy stdout to") stderrPath = flag.String("stderr_path", "", "If specified, file to copy stderr to") breakpointOnFailure = flag.Bool("breakpoint_on_failure", false, "If specified, expect steps to not skip on failure") + debugBeforeStep = flag.Bool("debug_before_step", false, "If specified, wait for a debugger to attach before executing the step") onError = flag.String("on_error", "", "Set to \"continue\" to ignore an error and continue when a container terminates with a non-zero exit code."+ " Set to \"stopAndFail\" to declare a failure with a step error and stop executing the rest of the steps.") stepMetadataDir = flag.String("step_metadata_dir", "", "If specified, create directory to store the step metadata e.g. /tekton/steps//") @@ -66,25 +66,8 @@ var ( const ( defaultWaitPollingInterval = time.Second - breakpointExitSuffix = ".breakpointexit" ) -func checkForBreakpointOnFailure(e entrypoint.Entrypointer, breakpointExitPostFile string) { - if e.BreakpointOnFailure { - if waitErr := e.Waiter.Wait(context.Background(), breakpointExitPostFile, false, false); waitErr != nil { - log.Println("error occurred while waiting for " + breakpointExitPostFile + " : " + waitErr.Error()) - } - // get exitcode from .breakpointexit - exitCode, readErr := e.BreakpointExitCode(breakpointExitPostFile) - // if readErr exists, the exitcode with default to 0 as we would like - // to encourage to continue running the next steps in the taskRun - if readErr != nil { - log.Println("error occurred while reading breakpoint exit code : " + readErr.Error()) - } - os.Exit(exitCode) - } -} - func main() { // Add credential flags originally introduced with our legacy credentials helper // image (creds-init). @@ -172,6 +155,7 @@ func main() { Timeout: timeout, StepWhenExpressions: when, BreakpointOnFailure: *breakpointOnFailure, + DebugBeforeStep: *debugBeforeStep, OnError: *onError, StepMetadataDir: *stepMetadataDir, SpireWorkloadAPI: spireWorkloadAPI, @@ -185,8 +169,10 @@ func main() { } if err := e.Go(); err != nil { - breakpointExitPostFile := e.PostFile + breakpointExitSuffix switch t := err.(type) { //nolint:errorlint // checking for multiple types with errors.As is ugly. + case entrypoint.DebugBeforeStepError: + log.Println("Skipping execute step script because before step breakpoint fail-continue") + os.Exit(1) case entrypoint.SkipError: log.Print("Skipping step because a previous step failed") os.Exit(1) @@ -210,7 +196,7 @@ func main() { // in both cases has an ExitStatus() method with the // same signature. if status, ok := t.Sys().(syscall.WaitStatus); ok { - checkForBreakpointOnFailure(e, breakpointExitPostFile) + e.CheckForBreakpointOnFailure() // ignore a step error i.e. do not exit if a container terminates with a non-zero exit code when onError is set to "continue" if e.OnError != entrypoint.ContinueOnError { os.Exit(status.ExitStatus()) @@ -221,7 +207,7 @@ func main() { log.Fatalf("Error executing command (ExitError): %v", err) } default: - checkForBreakpointOnFailure(e, breakpointExitPostFile) + e.CheckForBreakpointOnFailure() log.Fatalf("Error executing command: %v", err) } } diff --git a/docs/debug.md b/docs/debug.md index ff3d62a9ff4..dae54e86bd7 100644 --- a/docs/debug.md +++ b/docs/debug.md @@ -13,7 +13,8 @@ weight: 108 - [Breakpoint on Failure](#breakpoint-on-failure) - [Failure of a Step](#failure-of-a-step) - [Halting a Step on failure](#halting-a-step-on-failure) - - [Exiting breakpoint](#exiting-breakpoint) + - [Exiting onfailure breakpoint](#exiting-onfailure-breakpoint) + - [Breakpoint before step](#breakpoint-before-step) - [Debug Environment](#debug-environment) - [Mounts](#mounts) - [Debug Scripts](#debug-scripts) @@ -59,12 +60,26 @@ stopping write of the `.err` file and waiting on a signal by the user t In this breakpoint, which is essentially a limbo state the TaskRun finds itself in, the user can interact with the step environment using a CLI or an IDE. -#### Exiting breakpoint +#### Exiting onfailure breakpoint To exit a step which has been paused upon failure, the step would wait on a file similar to `.breakpointexit` which would unpause and exit the step container. eg: Step 0 fails and is paused. Writing `0.breakpointexit` in `/tekton/run` would unpause and exit the step container. +### Breakpoint before step + + +TaskRun will be stuck waiting for user debugging before the step execution. +When beforeStep-Breakpoint takes effect, the user can see the following information +from the corresponding step container log: +``` +debug before step breakpoint has taken effect, waiting for user's decision: +1) continue, use cmd: /tekton/debug/scripts/debug-beforestep-continue +2) fail-continue, use cmd: /tekton/debug/scripts/debug-beforestep-fail-continue +``` +1. Executing /tekton/debug/scripts/debug-beforestep-continue will continue to execute the step program +2. Executing /tekton/debug/scripts/debug-beforestep-fail-continue will not continue to execute the task, and will mark the step as failed + ## Debug Environment Additional environment augmentations made available to the TaskRun Pod to aid in troubleshooting and managing step lifecycle. @@ -80,7 +95,13 @@ to reflect step number. eg: Step 0 will have `/tekton/debug/info/0`, Step 1 will ### Debug Scripts `/tekton/debug/scripts/debug-continue` : Mark the step as completed with success by writing to `/tekton/run`. eg: User wants to exit -breakpoint for failed step 0. Running this script would create `/tekton/run/0` and `/tekton/run/0.breakpointexit`. +onfailure breakpoint for failed step 0. Running this script would create `/tekton/run/0` and `/tekton/run/0/out.breakpointexit`. `/tekton/debug/scripts/debug-fail-continue` : Mark the step as completed with failure by writing to `/tekton/run`. eg: User wants to exit -breakpoint for failed step 0. Running this script would create `/tekton/run/0.err` and `/tekton/run/0.breakpointexit`. +onfailure breakpoint for failed step 0. Running this script would create `/tekton/run/0` and `/tekton/run/0/out.breakpointexit.err`. + +`/tekton/debug/scripts/debug-beforestep-continue` : Mark the step continue to execute by writing to `/tekton/run`. eg: User wants to exit +before step breakpoint for before step 0. Running this script would create `/tekton/run/0` and `/tekton/run/0/out.beforestepexit`. + +`/tekton/debug/scripts/debug-beforestep-fail-continue` : Mark the step not continue to execute by writing to `/tekton/run`. eg: User wants to exit +before step breakpoint for before step 0. Running this script would create `/tekton/run/0` and `/tekton/run/0/out.beforestepexit.err`. diff --git a/docs/developers/taskruns.md b/docs/developers/taskruns.md index 1b702433233..5b35692bde8 100644 --- a/docs/developers/taskruns.md +++ b/docs/developers/taskruns.md @@ -284,4 +284,54 @@ There are known issues with the existing implementation of sidecars: but an Error when the sidecar exits with an error. This is only apparent when using `kubectl` to get the pods of a TaskRun, not when describing the Pod using `kubectl describe pod ...` nor when looking at the TaskRun, but can be - quite confusing. \ No newline at end of file + quite confusing. + +## Breakpoint on Failure + +Halting a TaskRun execution on Failure of a step. + +### Failure of a Step + +The entrypoint binary is used to manage the lifecycle of a step. Steps are aligned beforehand by the TaskRun controller +allowing each step to run in a particular order. This is done using `-wait_file` and the `-post_file` flags. The former +let's the entrypoint binary know that it has to wait on creation of a particular file before starting execution of the step. +And the latter provides information on the step number and signal the next step on completion of the step. + +On success of a step, the `-post-file` is written as is, signalling the next step which would have the same argument given +for `-wait_file` to resume the entrypoint process and move ahead with the step. + +On failure of a step, the `-post_file` is written with appending `.err` to it denoting that the previous step has failed with +and error. The subsequent steps are skipped in this case as well, marking the TaskRun as a failure. + +### Halting a Step on failure + +The failed step writes `.err` to `/tekton/run` and stops running completely. To be able to debug a step we would +need it to continue running (not exit), not skip the next steps and signal health of the step. By disabling step skipping, +stopping write of the `.err` file and waiting on a signal by the user to disable the halt, we would be simulating a +"breakpoint". + +In this breakpoint, which is essentially a limbo state the TaskRun finds itself in, the user can interact with the step +environment using a CLI or an IDE. + +### Exiting onfailure breakpoint + +To exit a step which has been paused upon failure, the step would wait on a file similar to `.breakpointexit` which +would unpause and exit the step container. eg: Step 0 fails and is paused. Writing `0.breakpointexit` in `/tekton/run` +would unpause and exit the step container. + +## Breakpoint before step + +TaskRun will be stuck waiting for user debugging before the step execution. + +### Halting a Step before execution + +The step program will be executed after all the `-wait_file` monitoring ends. If want the user to enter the debugging before the step is executed, +need to pass a parameter `debug_before_step` to `entrypoint`, +and `entrypoint` will end the monitoring of `waitFiles` back pause, +waiting to listen to the `/tekton/run/0/out.beforestepexit` file + +### Exiting before step breakpoint + +`entrypoint` listening `/tekton/run/{{ stepID }}/out.beforestepexit` or `/tekton/run/{{ stepID }}/out.beforestepexit.err` to +decide whether to proceed this step, `out.beforestepexit` means continue with step, +`out.beforestepexit.err` means do not continue with the step. \ No newline at end of file diff --git a/docs/pipeline-api.md b/docs/pipeline-api.md index 270e78c133c..d013bca86cb 100644 --- a/docs/pipeline-api.md +++ b/docs/pipeline-api.md @@ -5142,6 +5142,17 @@ string failed step will not exit

+ + +beforeSteps
+ +[]string + + + +(Optional) + +

TaskKind @@ -14926,6 +14937,17 @@ string failed step will not exit

+ + +beforeSteps
+ +[]string + + + +(Optional) + +

TaskKind diff --git a/docs/taskruns.md b/docs/taskruns.md index 9321bcaa16a..1dd4d040a6c 100644 --- a/docs/taskruns.md +++ b/docs/taskruns.md @@ -909,6 +909,18 @@ spec: onFailure: "enabled" ``` +### Breakpoint before step + +If you want to set a breakpoint before the step is executed, you can add the step name to the `beforeSteps` field in the following way: + +```yaml +spec: + debug: + breakpoints: + beforeSteps: + - {{ stepName }} +``` + Upon failure of a step, the TaskRun Pod execution is halted. If this TaskRun Pod continues to run without any lifecycle change done by the user (running the debug-continue or debug-fail-continue script) the TaskRun would be subject to [TaskRunTimeout](#configuring-the-failure-timeout). @@ -931,6 +943,10 @@ perform :- `debug-fail-continue`: Mark the step as a failure and exit the breakpoint. +`debug-beforestep-continue`: Mark the step continue to execute + +`debug-beforestep-fail-continue`: Mark the step not continue to execute + *More information on the inner workings of debug can be found in the [Debug documentation](debug.md)* ## Code examples diff --git a/pkg/apis/pipeline/v1/openapi_generated.go b/pkg/apis/pipeline/v1/openapi_generated.go index dfdb8f6c04a..17319ab6bc6 100644 --- a/pkg/apis/pipeline/v1/openapi_generated.go +++ b/pkg/apis/pipeline/v1/openapi_generated.go @@ -3567,6 +3567,25 @@ func schema_pkg_apis_pipeline_v1_TaskBreakpoints(ref common.ReferenceCallback) c Format: "", }, }, + "beforeSteps": { + VendorExtensible: spec.VendorExtensible{ + Extensions: spec.Extensions{ + "x-kubernetes-list-type": "atomic", + }, + }, + SchemaProps: spec.SchemaProps{ + Type: []string{"array"}, + Items: &spec.SchemaOrArray{ + Schema: &spec.Schema{ + SchemaProps: spec.SchemaProps{ + Default: "", + Type: []string{"string"}, + Format: "", + }, + }, + }, + }, + }, }, }, }, diff --git a/pkg/apis/pipeline/v1/swagger.json b/pkg/apis/pipeline/v1/swagger.json index 5b01a5ca004..fb8d534248b 100644 --- a/pkg/apis/pipeline/v1/swagger.json +++ b/pkg/apis/pipeline/v1/swagger.json @@ -1825,6 +1825,14 @@ "description": "TaskBreakpoints defines the breakpoint config for a particular Task", "type": "object", "properties": { + "beforeSteps": { + "type": "array", + "items": { + "type": "string", + "default": "" + }, + "x-kubernetes-list-type": "atomic" + }, "onFailure": { "description": "if enabled, pause TaskRun on failure of a step failed step will not exit", "type": "string" diff --git a/pkg/apis/pipeline/v1/taskrun_types.go b/pkg/apis/pipeline/v1/taskrun_types.go index ea6e517649e..2f8848ec615 100644 --- a/pkg/apis/pipeline/v1/taskrun_types.go +++ b/pkg/apis/pipeline/v1/taskrun_types.go @@ -26,6 +26,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/types" + "k8s.io/apimachinery/pkg/util/sets" "k8s.io/utils/clock" "knative.dev/pkg/apis" duckv1 "knative.dev/pkg/apis/duck/v1" @@ -121,6 +122,9 @@ type TaskBreakpoints struct { // failed step will not exit // +optional OnFailure string `json:"onFailure,omitempty"` + // +optional + // +listType=atomic + BeforeSteps []string `json:"beforeSteps,omitempty"` } // NeedsDebugOnFailure return true if the TaskRun is configured to debug on failure @@ -131,14 +135,28 @@ func (trd *TaskRunDebug) NeedsDebugOnFailure() bool { return trd.Breakpoints.OnFailure == EnabledOnFailureBreakpoint } +// NeedsDebugBeforeStep return true if the step is configured to debug before execution +func (trd *TaskRunDebug) NeedsDebugBeforeStep(stepName string) bool { + if trd.Breakpoints == nil { + return false + } + beforeStepSets := sets.NewString(trd.Breakpoints.BeforeSteps...) + return beforeStepSets.Has(stepName) +} + // StepNeedsDebug return true if the step is configured to debug func (trd *TaskRunDebug) StepNeedsDebug(stepName string) bool { - return trd.NeedsDebugOnFailure() + return trd.NeedsDebugOnFailure() || trd.NeedsDebugBeforeStep(stepName) } // NeedsDebug return true if defined onfailure or have any before, after steps func (trd *TaskRunDebug) NeedsDebug() bool { - return trd.NeedsDebugOnFailure() + return trd.NeedsDebugOnFailure() || trd.HaveBeforeSteps() +} + +// HaveBeforeSteps return true if have any before steps +func (trd *TaskRunDebug) HaveBeforeSteps() bool { + return trd.Breakpoints != nil && len(trd.Breakpoints.BeforeSteps) > 0 } // 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 6060c15ebab..a43b8cb2f23 100644 --- a/pkg/apis/pipeline/v1/taskrun_types_test.go +++ b/pkg/apis/pipeline/v1/taskrun_types_test.go @@ -409,6 +409,57 @@ func TestInitializeTaskRunConditions(t *testing.T) { } } +func TestIsDebugBeforeStep(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 before step", + args: args{ + stepName: "step1", + trd: &v1.TaskRunDebug{ + Breakpoints: &v1.TaskBreakpoints{ + BeforeSteps: []string{"step1", "step2"}, + }, + }, + }, + want: true, + }, { + name: "step not in before step breakpoint", + args: args{ + stepName: "step3", + trd: &v1.TaskRunDebug{ + Breakpoints: &v1.TaskBreakpoints{ + BeforeSteps: []string{"step1", "step2"}, + }, + }, + }, + want: false, + }, + } + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + result := tc.args.trd.NeedsDebugBeforeStep(tc.args.stepName) + if d := cmp.Diff(result, tc.want); d != "" { + t.Fatalf(diff.PrintWantGot(d)) + } + }) + } +} + func TestIsStepNeedDebug(t *testing.T) { type args struct { stepName string @@ -437,6 +488,17 @@ func TestIsStepNeedDebug(t *testing.T) { }, }, want: true, + }, { + name: "breakpoint before step", + args: args{ + stepName: "step1", + trd: &v1.TaskRunDebug{ + Breakpoints: &v1.TaskBreakpoints{ + BeforeSteps: []string{"step1"}, + }, + }, + }, + want: true, }, } for _, tc := range testCases { @@ -474,6 +536,16 @@ func TestIsNeedDebug(t *testing.T) { }, }, want: true, + }, { + name: "breakpoint before step", + args: args{ + trd: &v1.TaskRunDebug{ + Breakpoints: &v1.TaskBreakpoints{ + BeforeSteps: []string{"step1"}, + }, + }, + }, + want: true, }, } for _, tc := range testCases { diff --git a/pkg/apis/pipeline/v1/taskrun_validation.go b/pkg/apis/pipeline/v1/taskrun_validation.go index cfdd423e996..e162672a666 100644 --- a/pkg/apis/pipeline/v1/taskrun_validation.go +++ b/pkg/apis/pipeline/v1/taskrun_validation.go @@ -264,6 +264,13 @@ func validateDebug(db *TaskRunDebug) (errs *apis.FieldError) { if db.Breakpoints.OnFailure != "" && db.Breakpoints.OnFailure != EnabledOnFailureBreakpoint { errs = errs.Also(apis.ErrInvalidValue(db.Breakpoints.OnFailure+" is not a valid onFailure breakpoint value, onFailure breakpoint is only allowed to be set as enabled", "breakpoints.onFailure")) } + beforeSteps := sets.NewString() + for i, step := range db.Breakpoints.BeforeSteps { + if beforeSteps.Has(step) { + errs = errs.Also(apis.ErrGeneric(fmt.Sprintf("before step must be unique, the same step: %s is defined multiple times at", step), fmt.Sprintf("breakpoints.beforeSteps[%d]", i))) + } + beforeSteps.Insert(step) + } return errs } diff --git a/pkg/apis/pipeline/v1/taskrun_validation_test.go b/pkg/apis/pipeline/v1/taskrun_validation_test.go index 1b0bca9d825..9d3018b7367 100644 --- a/pkg/apis/pipeline/v1/taskrun_validation_test.go +++ b/pkg/apis/pipeline/v1/taskrun_validation_test.go @@ -705,6 +705,21 @@ func TestTaskRunSpec_Invalidate(t *testing.T) { }, 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: "invalid breakpoint duplicate before steps", + spec: v1.TaskRunSpec{ + TaskRef: &v1.TaskRef{ + Name: "my-task", + }, + Debug: &v1.TaskRunDebug{ + Breakpoints: &v1.TaskBreakpoints{ + BeforeSteps: []string{"step-1", "step-1"}, + OnFailure: "enabled", + }, + }, + }, + wantErr: apis.ErrGeneric("before step must be unique, the same step: step-1 is defined multiple times at", "debug.breakpoints.beforeSteps[1]"), + wc: cfgtesting.EnableAlphaAPIFields, }, { name: "empty onFailure breakpoint", spec: v1.TaskRunSpec{ diff --git a/pkg/apis/pipeline/v1/zz_generated.deepcopy.go b/pkg/apis/pipeline/v1/zz_generated.deepcopy.go index 12dbe03bf8d..d1d34a7554f 100644 --- a/pkg/apis/pipeline/v1/zz_generated.deepcopy.go +++ b/pkg/apis/pipeline/v1/zz_generated.deepcopy.go @@ -1580,6 +1580,11 @@ func (in *Task) DeepCopyObject() runtime.Object { // 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 + if in.BeforeSteps != nil { + in, out := &in.BeforeSteps, &out.BeforeSteps + *out = make([]string, len(*in)) + copy(*out, *in) + } return } @@ -1705,7 +1710,7 @@ func (in *TaskRunDebug) DeepCopyInto(out *TaskRunDebug) { if in.Breakpoints != nil { in, out := &in.Breakpoints, &out.Breakpoints *out = new(TaskBreakpoints) - **out = **in + (*in).DeepCopyInto(*out) } return } diff --git a/pkg/apis/pipeline/v1beta1/openapi_generated.go b/pkg/apis/pipeline/v1beta1/openapi_generated.go index d1048ef49b0..126888e63d7 100644 --- a/pkg/apis/pipeline/v1beta1/openapi_generated.go +++ b/pkg/apis/pipeline/v1beta1/openapi_generated.go @@ -4815,6 +4815,25 @@ func schema_pkg_apis_pipeline_v1beta1_TaskBreakpoints(ref common.ReferenceCallba Format: "", }, }, + "beforeSteps": { + VendorExtensible: spec.VendorExtensible{ + Extensions: spec.Extensions{ + "x-kubernetes-list-type": "atomic", + }, + }, + SchemaProps: spec.SchemaProps{ + Type: []string{"array"}, + Items: &spec.SchemaOrArray{ + Schema: &spec.Schema{ + SchemaProps: spec.SchemaProps{ + Default: "", + Type: []string{"string"}, + Format: "", + }, + }, + }, + }, + }, }, }, }, diff --git a/pkg/apis/pipeline/v1beta1/swagger.json b/pkg/apis/pipeline/v1beta1/swagger.json index fdc0e9752e5..d83792633ab 100644 --- a/pkg/apis/pipeline/v1beta1/swagger.json +++ b/pkg/apis/pipeline/v1beta1/swagger.json @@ -2643,6 +2643,14 @@ "description": "TaskBreakpoints defines the breakpoint config for a particular Task", "type": "object", "properties": { + "beforeSteps": { + "type": "array", + "items": { + "type": "string", + "default": "" + }, + "x-kubernetes-list-type": "atomic" + }, "onFailure": { "description": "if enabled, pause TaskRun on failure of a step failed step will not exit", "type": "string" diff --git a/pkg/apis/pipeline/v1beta1/taskrun_conversion.go b/pkg/apis/pipeline/v1beta1/taskrun_conversion.go index 7b749ac5afb..fb01170254f 100644 --- a/pkg/apis/pipeline/v1beta1/taskrun_conversion.go +++ b/pkg/apis/pipeline/v1beta1/taskrun_conversion.go @@ -214,10 +214,18 @@ func (trd *TaskRunDebug) convertFrom(ctx context.Context, source v1.TaskRunDebug func (tbp TaskBreakpoints) convertTo(ctx context.Context, sink *v1.TaskBreakpoints) { sink.OnFailure = tbp.OnFailure + if len(tbp.BeforeSteps) > 0 { + sink.BeforeSteps = make([]string, 0) + sink.BeforeSteps = append(sink.BeforeSteps, tbp.BeforeSteps...) + } } func (tbp *TaskBreakpoints) convertFrom(ctx context.Context, source v1.TaskBreakpoints) { tbp.OnFailure = source.OnFailure + if len(source.BeforeSteps) > 0 { + tbp.BeforeSteps = make([]string, 0) + tbp.BeforeSteps = append(tbp.BeforeSteps, source.BeforeSteps...) + } } 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 bc7d0247364..ff387e7d03b 100644 --- a/pkg/apis/pipeline/v1beta1/taskrun_conversion_test.go +++ b/pkg/apis/pipeline/v1beta1/taskrun_conversion_test.go @@ -157,7 +157,8 @@ func TestTaskRunConversion(t *testing.T) { Spec: v1beta1.TaskRunSpec{ Debug: &v1beta1.TaskRunDebug{ Breakpoints: &v1beta1.TaskBreakpoints{ - OnFailure: "enabled", + OnFailure: "enabled", + BeforeSteps: []string{"step-1", "step-2"}, }, }, Params: v1beta1.Params{{ @@ -238,21 +239,17 @@ func TestTaskRunConversion(t *testing.T) { }, }, }, - StepOverrides: []v1beta1.TaskRunStepOverride{ - { - Name: "task-1", - Resources: corev1.ResourceRequirements{ - Requests: corev1.ResourceList{corev1.ResourceMemory: corev1resources.MustParse("1Gi")}, - }, - }, + StepOverrides: []v1beta1.TaskRunStepOverride{{ + Name: "task-1", + Resources: corev1.ResourceRequirements{ + Requests: corev1.ResourceList{corev1.ResourceMemory: corev1resources.MustParse("1Gi")}, + }}, }, - SidecarOverrides: []v1beta1.TaskRunSidecarOverride{ - { - Name: "task-1", - Resources: corev1.ResourceRequirements{ - Requests: corev1.ResourceList{corev1.ResourceMemory: corev1resources.MustParse("1Gi")}, - }, - }, + SidecarOverrides: []v1beta1.TaskRunSidecarOverride{{ + Name: "task-1", + Resources: corev1.ResourceRequirements{ + Requests: corev1.ResourceList{corev1.ResourceMemory: corev1resources.MustParse("1Gi")}, + }}, }, ComputeResources: &corev1.ResourceRequirements{ Requests: corev1.ResourceList{ @@ -280,9 +277,7 @@ func TestTaskRunConversion(t *testing.T) { ContainerState: corev1.ContainerState{ Terminated: &corev1.ContainerStateTerminated{ ExitCode: 123, - }, - }, - + }}, Name: "failure", ContainerName: "step-failure", ImageID: "image-id", @@ -293,7 +288,6 @@ func TestTaskRunConversion(t *testing.T) { ExitCode: 123, }, }, - Name: "failure", ContainerName: "step-failure", ImageID: "image-id", diff --git a/pkg/apis/pipeline/v1beta1/taskrun_types.go b/pkg/apis/pipeline/v1beta1/taskrun_types.go index 26707ff337f..2cd76f0e570 100644 --- a/pkg/apis/pipeline/v1beta1/taskrun_types.go +++ b/pkg/apis/pipeline/v1beta1/taskrun_types.go @@ -28,6 +28,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/types" + "k8s.io/apimachinery/pkg/util/sets" "k8s.io/utils/clock" "knative.dev/pkg/apis" duckv1 "knative.dev/pkg/apis/duck/v1" @@ -126,6 +127,9 @@ type TaskBreakpoints struct { // failed step will not exit // +optional OnFailure string `json:"onFailure,omitempty"` + // +optional + // +listType=atomic + BeforeSteps []string `json:"beforeSteps,omitempty"` } // NeedsDebugOnFailure return true if the TaskRun is configured to debug on failure @@ -136,14 +140,28 @@ func (trd *TaskRunDebug) NeedsDebugOnFailure() bool { return trd.Breakpoints.OnFailure == EnabledOnFailureBreakpoint } +// NeedsDebugBeforeStep return true if the step is configured to debug before execution +func (trd *TaskRunDebug) NeedsDebugBeforeStep(stepName string) bool { + if trd.Breakpoints == nil { + return false + } + beforeStepSets := sets.NewString(trd.Breakpoints.BeforeSteps...) + return beforeStepSets.Has(stepName) +} + // StepNeedsDebug return true if the step is configured to debug func (trd *TaskRunDebug) StepNeedsDebug(stepName string) bool { - return trd.NeedsDebugOnFailure() + return trd.NeedsDebugOnFailure() || trd.NeedsDebugBeforeStep(stepName) +} + +// HaveBeforeSteps return true if have any before steps +func (trd *TaskRunDebug) HaveBeforeSteps() bool { + return trd.Breakpoints != nil && len(trd.Breakpoints.BeforeSteps) > 0 } // NeedsDebug return true if defined onfailure or have any before, after steps func (trd *TaskRunDebug) NeedsDebug() bool { - return trd.NeedsDebugOnFailure() + return trd.NeedsDebugOnFailure() || trd.HaveBeforeSteps() } 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 2474740ba9d..debe675868c 100644 --- a/pkg/apis/pipeline/v1beta1/taskrun_types_test.go +++ b/pkg/apis/pipeline/v1beta1/taskrun_types_test.go @@ -467,6 +467,57 @@ func TestInitializeTaskRunConditions(t *testing.T) { } } +func TestIsDebugBeforeStep(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 before step", + args: args{ + stepName: "step1", + trd: &v1beta1.TaskRunDebug{ + Breakpoints: &v1beta1.TaskBreakpoints{ + BeforeSteps: []string{"step1", "step2"}, + }, + }, + }, + want: true, + }, { + name: "step not in before step breakpoint", + args: args{ + stepName: "step3", + trd: &v1beta1.TaskRunDebug{ + Breakpoints: &v1beta1.TaskBreakpoints{ + BeforeSteps: []string{"step1", "step2"}, + }, + }, + }, + want: false, + }, + } + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + result := tc.args.trd.NeedsDebugBeforeStep(tc.args.stepName) + if d := cmp.Diff(result, tc.want); d != "" { + t.Fatalf(diff.PrintWantGot(d)) + } + }) + } +} + func TestIsStepNeedDebug(t *testing.T) { type args struct { stepName string @@ -495,6 +546,17 @@ func TestIsStepNeedDebug(t *testing.T) { }, }, want: true, + }, { + name: "breakpoint before step", + args: args{ + stepName: "step1", + trd: &v1beta1.TaskRunDebug{ + Breakpoints: &v1beta1.TaskBreakpoints{ + BeforeSteps: []string{"step1"}, + }, + }, + }, + want: true, }, } for _, tc := range testCases { @@ -532,6 +594,16 @@ func TestIsNeedDebug(t *testing.T) { }, }, want: true, + }, { + name: "breakpoint before step", + args: args{ + trd: &v1beta1.TaskRunDebug{ + Breakpoints: &v1beta1.TaskBreakpoints{ + BeforeSteps: []string{"step1"}, + }, + }, + }, + want: true, }, } for _, tc := range testCases { diff --git a/pkg/apis/pipeline/v1beta1/taskrun_validation.go b/pkg/apis/pipeline/v1beta1/taskrun_validation.go index a3f37caeee9..ae14965c770 100644 --- a/pkg/apis/pipeline/v1beta1/taskrun_validation.go +++ b/pkg/apis/pipeline/v1beta1/taskrun_validation.go @@ -264,6 +264,13 @@ func validateDebug(db *TaskRunDebug) (errs *apis.FieldError) { if db.Breakpoints.OnFailure != "" && db.Breakpoints.OnFailure != EnabledOnFailureBreakpoint { errs = errs.Also(apis.ErrInvalidValue(db.Breakpoints.OnFailure+" is not a valid onFailure breakpoint value, onFailure breakpoint is only allowed to be set as enabled", "breakpoints.onFailure")) } + beforeSteps := sets.NewString() + for i, step := range db.Breakpoints.BeforeSteps { + if beforeSteps.Has(step) { + errs = errs.Also(apis.ErrGeneric(fmt.Sprintf("before step must be unique, the same step: %s is defined multiple times at", step), fmt.Sprintf("breakpoints.beforeSteps[%d]", i))) + } + beforeSteps.Insert(step) + } return errs } diff --git a/pkg/apis/pipeline/v1beta1/taskrun_validation_test.go b/pkg/apis/pipeline/v1beta1/taskrun_validation_test.go index 0afe02f0372..a80339f31b9 100644 --- a/pkg/apis/pipeline/v1beta1/taskrun_validation_test.go +++ b/pkg/apis/pipeline/v1beta1/taskrun_validation_test.go @@ -700,6 +700,21 @@ func TestTaskRunSpec_Invalidate(t *testing.T) { }, 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: "invalid breakpoint duplicate before steps", + spec: v1beta1.TaskRunSpec{ + TaskRef: &v1beta1.TaskRef{ + Name: "my-task", + }, + Debug: &v1beta1.TaskRunDebug{ + Breakpoints: &v1beta1.TaskBreakpoints{ + BeforeSteps: []string{"step-1", "step-1"}, + OnFailure: "enabled", + }, + }, + }, + wantErr: apis.ErrGeneric("before step must be unique, the same step: step-1 is defined multiple times at", "debug.breakpoints.beforeSteps[1]"), + wc: cfgtesting.EnableAlphaAPIFields, }, { name: "empty onFailure breakpoint", spec: v1beta1.TaskRunSpec{ diff --git a/pkg/apis/pipeline/v1beta1/zz_generated.deepcopy.go b/pkg/apis/pipeline/v1beta1/zz_generated.deepcopy.go index 7852edd8c7c..8c2afe35a6f 100644 --- a/pkg/apis/pipeline/v1beta1/zz_generated.deepcopy.go +++ b/pkg/apis/pipeline/v1beta1/zz_generated.deepcopy.go @@ -2174,6 +2174,11 @@ func (in *Task) DeepCopyObject() runtime.Object { // 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 + if in.BeforeSteps != nil { + in, out := &in.BeforeSteps, &out.BeforeSteps + *out = make([]string, len(*in)) + copy(*out, *in) + } return } @@ -2364,7 +2369,7 @@ func (in *TaskRunDebug) DeepCopyInto(out *TaskRunDebug) { if in.Breakpoints != nil { in, out := &in.Breakpoints, &out.Breakpoints *out = new(TaskBreakpoints) - **out = **in + (*in).DeepCopyInto(*out) } return } diff --git a/pkg/entrypoint/entrypointer.go b/pkg/entrypoint/entrypointer.go index f27ffc32c7c..b3913665a75 100644 --- a/pkg/entrypoint/entrypointer.go +++ b/pkg/entrypoint/entrypointer.go @@ -52,6 +52,22 @@ const ( FailOnError = "stopAndFail" ) +const ( + breakpointExitSuffix = ".breakpointexit" + breakpointBeforeStepSuffix = ".beforestepexit" +) + +// DebugBeforeStepError is an error means mark before step breakpoint failure +type DebugBeforeStepError string + +func (e DebugBeforeStepError) Error() string { + return string(e) +} + +var ( + errDebugBeforeStep = DebugBeforeStepError("before step breakpoint error file, user decided to skip the current step execution") +) + // ScriptDir for testing var ScriptDir = pipeline.ScriptDir @@ -122,6 +138,8 @@ type Entrypointer struct { Timeout *time.Duration // BreakpointOnFailure helps determine if entrypoint execution needs to adapt debugging requirements BreakpointOnFailure bool + // DebugBeforeStep help user attach container before execution + DebugBeforeStep bool // OnError defines exiting behavior of the entrypoint // set it to "stopAndFail" to indicate the entrypoint to exit the taskRun if the container exits with non zero exit code // set it to "continue" to indicate the entrypoint to continue executing the rest of the steps irrespective of the container exit code @@ -201,13 +219,17 @@ func (e Entrypointer) Go() error { } } + var err error + if e.DebugBeforeStep { + err = e.waitBeforeStepDebug() + } + output = append(output, result.RunResult{ Key: "StartedAt", Value: time.Now().Format(timeFormat), ResultType: result.InternalTektonResultType, }) - var err error if e.Timeout != nil && *e.Timeout < time.Duration(0) { err = errors.New("negative timeout specified") } @@ -250,6 +272,8 @@ func (e Entrypointer) Go() error { var ee *exec.ExitError switch { + case err != nil && errors.Is(err, errDebugBeforeStep): + e.WritePostFile(e.PostFile, err) case err != nil && errors.Is(err, ErrContextCanceled): logger.Info("Step was canceling") output = append(output, e.outputRunResult(pod.TerminationReasonCancelled)) @@ -301,25 +325,7 @@ func (e Entrypointer) Go() error { } if e.ResultExtractionMethod == config.ResultExtractionMethodTerminationMessage { - // step artifacts - fp := filepath.Join(e.StepMetadataDir, "artifacts", "provenance.json") - artifacts, err := readArtifacts(fp, result.StepArtifactsResultType) - if err != nil { - logger.Fatalf("Error while handling step artifacts: %s", err) - } - output = append(output, artifacts...) - - artifactsDir := pipeline.ArtifactsDir - // task artifacts - if e.ArtifactsDirectory != "" { - artifactsDir = e.ArtifactsDirectory - } - fp = filepath.Join(artifactsDir, "provenance.json") - artifacts, err = readArtifacts(fp, result.TaskRunArtifactsResultType) - if err != nil { - logger.Fatalf("Error while handling task artifacts: %s", err) - } - output = append(output, artifacts...) + e.appendArtifactOutputs(&output, logger) } return err @@ -336,6 +342,28 @@ func readArtifacts(fp string, resultType result.ResultType) ([]result.RunResult, return []result.RunResult{{Key: fp, Value: string(file), ResultType: resultType}}, nil } +func (e Entrypointer) appendArtifactOutputs(output *[]result.RunResult, logger *zap.SugaredLogger) { + // step artifacts + fp := filepath.Join(e.StepMetadataDir, "artifacts", "provenance.json") + artifacts, err := readArtifacts(fp, result.StepArtifactsResultType) + if err != nil { + logger.Fatalf("Error while handling step artifacts: %s", err) + } + *output = append(*output, artifacts...) + + artifactsDir := pipeline.ArtifactsDir + // task artifacts + if e.ArtifactsDirectory != "" { + artifactsDir = e.ArtifactsDirectory + } + fp = filepath.Join(artifactsDir, "provenance.json") + artifacts, err = readArtifacts(fp, result.TaskRunArtifactsResultType) + if err != nil { + logger.Fatalf("Error while handling task artifacts: %s", err) + } + *output = append(*output, artifacts...) +} + func (e Entrypointer) allowExec() (bool, error) { when := e.StepWhenExpressions m := map[string]bool{} @@ -380,6 +408,18 @@ func (e Entrypointer) allowExec() (bool, error) { return when.AllowsExecution(m), nil } +func (e Entrypointer) waitBeforeStepDebug() error { + log.Println(`debug before step breakpoint has taken effect, waiting for user's decision: +1) continue, use cmd: /tekton/debug/scripts/debug-beforestep-continue +2) fail-continue, use cmd: /tekton/debug/scripts/debug-beforestep-fail-continue`) + breakpointBeforeStepPostFile := e.PostFile + breakpointBeforeStepSuffix + if waitErr := e.Waiter.Wait(context.Background(), breakpointBeforeStepPostFile, false, false); waitErr != nil { + log.Println("error occurred while waiting for " + breakpointBeforeStepPostFile + " : " + errDebugBeforeStep.Error()) + return errDebugBeforeStep + } + return nil +} + func (e Entrypointer) readResultsFromDisk(ctx context.Context, resultDir string, resultType result.ResultType) error { output := []result.RunResult{} results := e.Results @@ -458,6 +498,28 @@ func (e Entrypointer) waitingCancellation(ctx context.Context, cancel context.Ca return nil } +// CheckForBreakpointOnFailure if step up breakpoint on failure +// waiting breakpointExitPostFile to be written +func (e Entrypointer) CheckForBreakpointOnFailure() { + if e.BreakpointOnFailure { + log.Println(`debug onFailure breakpoint has taken effect, waiting for user's decision: +1) continue, use cmd: /tekton/debug/scripts/debug-continue +2) fail-continue, use cmd: /tekton/debug/scripts/debug-fail-continue`) + breakpointExitPostFile := e.PostFile + breakpointExitSuffix + if waitErr := e.Waiter.Wait(context.Background(), breakpointExitPostFile, false, false); waitErr != nil { + log.Println("error occurred while waiting for " + breakpointExitPostFile + " : " + waitErr.Error()) + } + // get exitcode from .breakpointexit + exitCode, readErr := e.BreakpointExitCode(breakpointExitPostFile) + // if readErr exists, the exitcode with default to 0 as we would like + // to encourage to continue running the next steps in the taskRun + if readErr != nil { + log.Println("error occurred while reading breakpoint exit code : " + readErr.Error()) + } + os.Exit(exitCode) + } +} + // loadStepResult reads the step result file and returns the string, array or object result value. func loadStepResult(stepDir string, stepName string, resultName string) (v1.ResultValue, error) { v := v1.ResultValue{} diff --git a/pkg/entrypoint/entrypointer_test.go b/pkg/entrypoint/entrypointer_test.go index 78ad3a70046..b5c423d1376 100644 --- a/pkg/entrypoint/entrypointer_test.go +++ b/pkg/entrypoint/entrypointer_test.go @@ -142,7 +142,9 @@ func TestEntrypointer(t *testing.T) { for _, c := range []struct { desc, entrypoint, postFile, stepDir, stepDirLink string waitFiles, args []string + waitDebugFiles []string breakpointOnFailure bool + debugBeforeStep bool }{{ desc: "do nothing", }, { @@ -173,6 +175,17 @@ func TestEntrypointer(t *testing.T) { }, { desc: "breakpointOnFailure to wait or not to wait ", breakpointOnFailure: true, + }, { + desc: "breakpointBeforeStep to wait or not to wait", + debugBeforeStep: true, + waitFiles: []string{"waitforme"}, + waitDebugFiles: []string{".beforestepexit"}, + }, { + desc: "all breakpoints to wait or not to wait", + breakpointOnFailure: true, + debugBeforeStep: true, + waitFiles: []string{"waitforme", ".beforestepexit"}, + waitDebugFiles: []string{".beforestepexit"}, }} { t.Run(c.desc, func(t *testing.T) { fw, fr, fpw := &fakeWaiter{}, &fakeRunner{}, &fakePostWriter{} @@ -194,6 +207,7 @@ func TestEntrypointer(t *testing.T) { TerminationPath: terminationPath, Timeout: &timeout, BreakpointOnFailure: c.breakpointOnFailure, + DebugBeforeStep: c.debugBeforeStep, StepMetadataDir: c.stepDir, }.Go() if err != nil { @@ -207,7 +221,7 @@ func TestEntrypointer(t *testing.T) { if len(c.waitFiles) > 0 { if fw.waited == nil { t.Error("Wanted waited file, got nil") - } else if !reflect.DeepEqual(fw.waited, c.waitFiles) { + } else if !reflect.DeepEqual(fw.waited, append(c.waitFiles, c.waitDebugFiles...)) { t.Errorf("Waited for %v, want %v", fw.waited, c.waitFiles) } } @@ -266,6 +280,47 @@ func TestEntrypointer(t *testing.T) { } } +func TestCheckForBreakpointOnFailure(t *testing.T) { + testCases := []struct { + name string + breakpointOnFailure bool + }{ + { + name: "set breakpoint on failure and exit with code 0", + breakpointOnFailure: true, + }, + { + name: "unset breakpoint on failure", + breakpointOnFailure: false, + }, + } + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + tmp, err := os.CreateTemp("", "1*.breakpoint") + if err != nil { + t.Fatalf("error while creating temp file for testing exit code written by breakpoint") + } + breakpointFile, err := os.Create(tmp.Name() + breakpointExitSuffix) + if err != nil { + t.Fatalf("failed to create breakpoint waiting file, err: %v", err) + } + // write exit code to file + if err = os.WriteFile(breakpointFile.Name(), []byte("0"), 0700); err != nil { + t.Fatalf("failed writing to temp file create temp file for testing exit code written by breakpoint, err: %v", err) + } + e := Entrypointer{ + BreakpointOnFailure: tc.breakpointOnFailure, + PostFile: tmp.Name(), + Waiter: &fakeWaiter{}, + } + defer func() { + recover() + }() + e.CheckForBreakpointOnFailure() + }) + } +} + func TestReadResultsFromDisk(t *testing.T) { for _, c := range []struct { desc string @@ -428,6 +483,7 @@ func TestEntrypointer_OnError(t *testing.T) { desc, postFile, onError string runner Runner expectedError bool + debugBeforeStep bool }{{ desc: "the step is exiting with 1, ignore the step error when onError is set to continue", runner: &fakeExitErrorRunner{}, @@ -452,6 +508,13 @@ func TestEntrypointer_OnError(t *testing.T) { postFile: "step-one", onError: FailOnError, expectedError: false, + }, { + desc: "the step set debug before step, and before step breakpoint fail-continue", + runner: &fakeRunner{}, + postFile: "step-one", + onError: errDebugBeforeStep.Error(), + debugBeforeStep: true, + expectedError: true, }} { t.Run(c.desc, func(t *testing.T) { fpw := &fakePostWriter{} @@ -462,7 +525,7 @@ func TestEntrypointer_OnError(t *testing.T) { terminationPath = terminationFile.Name() defer os.Remove(terminationFile.Name()) } - err := Entrypointer{ + entry := Entrypointer{ Command: []string{"echo", "some", "args"}, WaitFiles: []string{}, PostFile: c.postFile, @@ -471,12 +534,23 @@ func TestEntrypointer_OnError(t *testing.T) { PostWriter: fpw, TerminationPath: terminationPath, OnError: c.onError, - }.Go() + DebugBeforeStep: c.debugBeforeStep, + } + if c.expectedError && (c.debugBeforeStep) { + entry.Waiter = &fakeErrorWaiter{} + } + err := entry.Go() if c.expectedError && err == nil { t.Fatalf("Entrypointer didn't fail") } + if c.expectedError && (c.debugBeforeStep) { + if err.Error() != c.onError { + t.Errorf("breakpoint fail-continue, want err: %s but got: %s", c.onError, err.Error()) + } + } + if c.onError == ContinueOnError { switch { case fpw.wrote == nil: diff --git a/pkg/pod/entrypoint.go b/pkg/pod/entrypoint.go index 17830a27dab..fd4893bc6ed 100644 --- a/pkg/pod/entrypoint.go +++ b/pkg/pod/entrypoint.go @@ -189,6 +189,9 @@ func orderContainers(ctx context.Context, commonExtraEntrypointArgs []string, st if breakpointConfig != nil && breakpointConfig.NeedsDebugOnFailure() { argsForEntrypoint = append(argsForEntrypoint, "-breakpoint_on_failure") } + if breakpointConfig != nil && breakpointConfig.NeedsDebugBeforeStep(s.Name) { + argsForEntrypoint = append(argsForEntrypoint, "-debug_before_step") + } cmd, args := s.Command, s.Args if len(cmd) > 0 { diff --git a/pkg/pod/entrypoint_test.go b/pkg/pod/entrypoint_test.go index 21926649062..d2923af47ce 100644 --- a/pkg/pod/entrypoint_test.go +++ b/pkg/pod/entrypoint_test.go @@ -256,6 +256,82 @@ func TestOrderContainersWithDebugOnFailure(t *testing.T) { } } +func TestTestOrderContainersWithDebugBeforeStep(t *testing.T) { + steps := []corev1.Container{{ + Name: "my-task", + Image: "step-1", + Command: []string{"cmd"}, + Args: []string{"arg1", "arg2"}, + }} + want := []corev1.Container{{ + Name: "my-task", + Image: "step-1", + Command: []string{entrypointBinary}, + Args: []string{ + "-wait_file", "/tekton/downward/ready", + "-wait_file_content", + "-post_file", "/tekton/run/0/out", + "-termination_path", "/tekton/termination", + "-step_metadata_dir", "/tekton/run/0/status", "-debug_before_step", + "-entrypoint", "cmd", "--", + "arg1", "arg2", + }, + VolumeMounts: []corev1.VolumeMount{downwardMount}, + TerminationMessagePath: "/tekton/termination", + }} + taskRunDebugConfig := &v1.TaskRunDebug{ + Breakpoints: &v1.TaskBreakpoints{ + BeforeSteps: []string{"my-task"}, + }, + } + got, err := orderContainers(context.Background(), []string{}, steps, nil, taskRunDebugConfig, true, false) + if err != nil { + t.Fatalf("orderContainers: %v", err) + } + if d := cmp.Diff(want, got); d != "" { + t.Errorf("Diff %s", diff.PrintWantGot(d)) + } +} + +func TestTestOrderContainersWithAllBreakpoints(t *testing.T) { + steps := []corev1.Container{{ + Name: "my-task", + Image: "step-1", + Command: []string{"cmd"}, + Args: []string{"arg1", "arg2"}, + }} + want := []corev1.Container{{ + Name: "my-task", + Image: "step-1", + Command: []string{entrypointBinary}, + Args: []string{ + "-wait_file", "/tekton/downward/ready", + "-wait_file_content", + "-post_file", "/tekton/run/0/out", + "-termination_path", "/tekton/termination", + "-step_metadata_dir", "/tekton/run/0/status", + "-breakpoint_on_failure", "-debug_before_step", + "-entrypoint", "cmd", "--", + "arg1", "arg2", + }, + VolumeMounts: []corev1.VolumeMount{downwardMount}, + TerminationMessagePath: "/tekton/termination", + }} + taskRunDebugConfig := &v1.TaskRunDebug{ + Breakpoints: &v1.TaskBreakpoints{ + OnFailure: "enabled", + BeforeSteps: []string{"my-task"}, + }, + } + got, err := orderContainers(context.Background(), []string{}, steps, nil, taskRunDebugConfig, true, false) + if err != nil { + t.Fatalf("orderContainers: %v", err) + } + if d := cmp.Diff(want, got); d != "" { + t.Errorf("Diff %s", diff.PrintWantGot(d)) + } +} + func TestOrderContainersWithEnabelKeepPodOnCancel(t *testing.T) { steps := []corev1.Container{{ Image: "step-1", diff --git a/pkg/pod/script.go b/pkg/pod/script.go index 6b0af47467c..70b64ba3472 100644 --- a/pkg/pod/script.go +++ b/pkg/pod/script.go @@ -146,9 +146,7 @@ func convertListOfSteps(steps []v1.Step, initContainer *corev1.Container, debugC } containers = append(containers, *c) } - if debugConfig != nil && debugConfig.NeedsDebugOnFailure() { - placeDebugScriptInContainers(containers, initContainer) - } + placeDebugScriptInContainers(containers, initContainer, debugConfig) return containers } @@ -214,26 +212,48 @@ func encodeScript(script string) string { // placeDebugScriptInContainers inserts debug scripts into containers. It capsules those scripts to files in initContainer, // then executes those scripts in target containers. -func placeDebugScriptInContainers(containers []corev1.Container, initContainer *corev1.Container) { - for i := range len(containers) { +func placeDebugScriptInContainers(containers []corev1.Container, initContainer *corev1.Container, debugConfig *v1.TaskRunDebug) { + if debugConfig == nil || !debugConfig.NeedsDebug() { + return + } + + isDebugOnFailure := debugConfig != nil && debugConfig.NeedsDebugOnFailure() + var needDebugBeforeStep bool + + for i := range containers { debugInfoVolumeMount := corev1.VolumeMount{ Name: debugInfoVolumeName, MountPath: filepath.Join(debugInfoDir, strconv.Itoa(i)), } (&containers[i]).VolumeMounts = append((&containers[i]).VolumeMounts, debugScriptsVolumeMount, debugInfoVolumeMount) + if debugConfig != nil && debugConfig.NeedsDebugBeforeStep(containers[i].Name) { + needDebugBeforeStep = true + } } type script struct { name string content string } - debugScripts := []script{{ - name: "continue", - content: defaultScriptPreamble + fmt.Sprintf(debugContinueScriptTemplate, len(containers), debugInfoDir, RunDir), - }, { - name: "fail-continue", - content: defaultScriptPreamble + fmt.Sprintf(debugFailScriptTemplate, len(containers), debugInfoDir, RunDir), - }} + debugScripts := make([]script, 0) + if isDebugOnFailure { + debugScripts = append(debugScripts, []script{{ + name: "continue", + content: defaultScriptPreamble + fmt.Sprintf(debugContinueScriptTemplate, len(containers), debugInfoDir, RunDir), + }, { + name: "fail-continue", + content: defaultScriptPreamble + fmt.Sprintf(debugFailScriptTemplate, len(containers), debugInfoDir, RunDir), + }}...) + } + if needDebugBeforeStep { + debugScripts = append(debugScripts, []script{{ + name: "beforestep-continue", + content: defaultScriptPreamble + fmt.Sprintf(debugBeforeStepContinueScriptTemplate, len(containers), debugInfoDir, RunDir), + }, { + name: "beforestep-fail-continue", + content: defaultScriptPreamble + fmt.Sprintf(debugBeforeStepFailScriptTemplate, len(containers), debugInfoDir, RunDir), + }}...) + } // Add debug or breakpoint related scripts to /tekton/debug/scripts // Iterate through the debugScripts and add routine for each of them in the initContainer for their creation diff --git a/pkg/pod/script_test.go b/pkg/pod/script_test.go index 93c1ed51ce5..19d0c7523e7 100644 --- a/pkg/pod/script_test.go +++ b/pkg/pod/script_test.go @@ -353,7 +353,7 @@ _EOF_ } } -func TestConvertScripts_WithBreakpoint_OnFailure(t *testing.T) { +func TestConvertScripts_WithBreakpoints(t *testing.T) { names.TestingSeed() preExistingVolumeMounts := []corev1.VolumeMount{{ @@ -363,37 +363,45 @@ func TestConvertScripts_WithBreakpoint_OnFailure(t *testing.T) { Name: "another-one", MountPath: "/another/one", }} - - gotInit, gotSteps, gotSidecars := convertScripts(images.ShellImage, images.ShellImageWin, []v1.Step{{ - Script: `#!/bin/sh + testCases := []struct { + name string + steps []v1.Step + wantInit *corev1.Container + wantSteps []corev1.Container + taskRunDebug *v1.TaskRunDebug + }{ + { + name: "set breakpoint only on failure", + steps: []v1.Step{{ + Script: `#!/bin/sh script-1`, - Image: "step-1", - }, { - // No script to convert here. - Image: "step-2", - }, { - Script: ` + Image: "step-1", + }, { + // No script to convert here. + Image: "step-2", + }, { + Script: ` #!/bin/sh script-3`, - Image: "step-3", - VolumeMounts: preExistingVolumeMounts, - Args: []string{"my", "args"}, - }, { - Script: `no-shebang`, - Image: "step-3", - VolumeMounts: preExistingVolumeMounts, - Args: []string{"my", "args"}, - }}, []v1.Sidecar{}, &v1.TaskRunDebug{ - Breakpoints: &v1.TaskBreakpoints{ - OnFailure: "enabled", - }, - }, true) - - wantInit := &corev1.Container{ - Name: "place-scripts", - Image: images.ShellImage, - Command: []string{"sh"}, - Args: []string{"-c", `scriptfile="/tekton/scripts/script-0-9l9zj" + Image: "step-3", + VolumeMounts: preExistingVolumeMounts, + Args: []string{"my", "args"}, + }, { + Script: `no-shebang`, + Image: "step-3", + VolumeMounts: preExistingVolumeMounts, + Args: []string{"my", "args"}, + }}, + taskRunDebug: &v1.TaskRunDebug{ + Breakpoints: &v1.TaskBreakpoints{ + OnFailure: "enabled", + }, + }, + wantInit: &corev1.Container{ + Name: "place-scripts", + Image: images.ShellImage, + Command: []string{"sh"}, + Args: []string{"-c", `scriptfile="/tekton/scripts/script-0-9l9zj" touch ${scriptfile} && chmod +x ${scriptfile} cat > ${scriptfile} << '_EOF_' IyEvYmluL3NoCnNjcmlwdC0x @@ -456,49 +464,176 @@ else fi debug-fail-continue-heredoc-randomly-generated-6nl7g `}, - VolumeMounts: []corev1.VolumeMount{writeScriptsVolumeMount, binMount, debugScriptsVolumeMount}, - SecurityContext: linuxSecurityContext, - } + VolumeMounts: []corev1.VolumeMount{writeScriptsVolumeMount, binMount, debugScriptsVolumeMount}, + SecurityContext: linuxSecurityContext, + }, + wantSteps: []corev1.Container{{ + Image: "step-1", + Command: []string{"/tekton/scripts/script-0-9l9zj"}, + VolumeMounts: []corev1.VolumeMount{scriptsVolumeMount, debugScriptsVolumeMount, + {Name: debugInfoVolumeName, MountPath: "/tekton/debug/info/0"}}, + }, { + 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"}, + ), + }, { + 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, + {Name: debugInfoVolumeName, MountPath: "/tekton/debug/info/3"}, + }, + }}, + }, { + name: "set all breakpoints with onfailure debugBeforeStep", + steps: []v1.Step{{ + Name: "step-1", + Script: `#!/bin/sh +script-1`, + Image: "step-1", + }}, + taskRunDebug: &v1.TaskRunDebug{ + Breakpoints: &v1.TaskBreakpoints{ + OnFailure: "enabled", + BeforeSteps: []string{"step-1"}, + }, + }, + wantInit: &corev1.Container{ + Name: "place-scripts", + Image: images.ShellImage, + Command: []string{"sh"}, + Args: []string{"-c", `scriptfile="/tekton/scripts/script-0-9l9zj" +touch ${scriptfile} && chmod +x ${scriptfile} +cat > ${scriptfile} << '_EOF_' +IyEvYmluL3NoCnNjcmlwdC0x +_EOF_ +/tekton/bin/entrypoint decode-script "${scriptfile}" +tmpfile="/tekton/debug/scripts/debug-continue" +touch ${tmpfile} && chmod +x ${tmpfile} +cat > ${tmpfile} << 'debug-continue-heredoc-randomly-generated-mz4c7' +#!/bin/sh +set -e - 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"}}, - }, { - 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"}), - }, { - 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, - {Name: debugInfoVolumeName, MountPath: "/tekton/debug/info/3"}, - }, - }} +numberOfSteps=1 +debugInfo=/tekton/debug/info +tektonRun=/tekton/run - if d := cmp.Diff(wantInit, gotInit); d != "" { - t.Errorf("Init Container Diff %s", diff.PrintWantGot(d)) - } +postFile="$(ls ${debugInfo} | grep -E '[0-9]+' | tail -1)" +stepNumber="$(echo ${postFile} | sed 's/[^0-9]*//g')" - if d := cmp.Diff(want, gotSteps); d != "" { - t.Errorf("Containers Diff %s", diff.PrintWantGot(d)) +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-mz4c7 +tmpfile="/tekton/debug/scripts/debug-fail-continue" +touch ${tmpfile} && chmod +x ${tmpfile} +cat > ${tmpfile} << 'debug-fail-continue-heredoc-randomly-generated-mssqb' +#!/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-mssqb +tmpfile="/tekton/debug/scripts/debug-beforestep-continue" +touch ${tmpfile} && chmod +x ${tmpfile} +cat > ${tmpfile} << 'debug-beforestep-continue-heredoc-randomly-generated-78c5n' +#!/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 + echo "0" > ${tektonRun}/${stepNumber}/out.beforestepexit + echo "Executing step $stepNumber..." +else + echo "Last step (no. $stepNumber) has already been executed, before step breakpoint exiting !" + exit 0 +fi +debug-beforestep-continue-heredoc-randomly-generated-78c5n +tmpfile="/tekton/debug/scripts/debug-beforestep-fail-continue" +touch ${tmpfile} && chmod +x ${tmpfile} +cat > ${tmpfile} << 'debug-beforestep-fail-continue-heredoc-randomly-generated-6nl7g' +#!/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 + echo "1" > ${tektonRun}/${stepNumber}/out.beforestepexit.err + echo "Executing step $stepNumber..." +else + echo "Last step (no. $stepNumber) has already been executed, before step breakpoint exiting !" + exit 0 +fi +debug-beforestep-fail-continue-heredoc-randomly-generated-6nl7g +`}, + VolumeMounts: []corev1.VolumeMount{writeScriptsVolumeMount, binMount, debugScriptsVolumeMount}, + SecurityContext: linuxSecurityContext}, + wantSteps: []corev1.Container{{ + Name: "step-1", + Image: "step-1", + Command: []string{"/tekton/scripts/script-0-9l9zj"}, + VolumeMounts: []corev1.VolumeMount{scriptsVolumeMount, debugScriptsVolumeMount, + {Name: debugInfoVolumeName, MountPath: "/tekton/debug/info/0"}}, + }}, + }, } + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + names.TestingSeed() + gotInit, gotSteps, gotSidecars := convertScripts(images.ShellImage, images.ShellImageWin, tc.steps, []v1.Sidecar{}, tc.taskRunDebug, true) + if d := cmp.Diff(tc.wantInit, gotInit); d != "" { + t.Errorf("Init Container Diff %s", diff.PrintWantGot(d)) + } - if len(gotSidecars) != 0 { - t.Errorf("Expected zero sidecars, got %v", len(gotSidecars)) + if d := cmp.Diff(tc.wantSteps, gotSteps); d != "" { + t.Errorf("Containers Diff %s", diff.PrintWantGot(d)) + } + + if len(gotSidecars) != 0 { + t.Errorf("Expected zero sidecars, got %v", len(gotSidecars)) + } + }) } } diff --git a/pkg/pod/scripts_constants.go b/pkg/pod/scripts_constants.go index 8a5867fa0c4..7b018b6c61c 100644 --- a/pkg/pod/scripts_constants.go +++ b/pkg/pod/scripts_constants.go @@ -49,6 +49,36 @@ if [ $stepNumber -lt $numberOfSteps ]; then else echo "Last step (no. $stepNumber) has already been executed, breakpoint exiting !" exit 0 +fi` + debugBeforeStepContinueScriptTemplate = ` +numberOfSteps=%d +debugInfo=%s +tektonRun=%s + +postFile="$(ls ${debugInfo} | grep -E '[0-9]+' | tail -1)" +stepNumber="$(echo ${postFile} | sed 's/[^0-9]*//g')" + +if [ $stepNumber -lt $numberOfSteps ]; then + echo "0" > ${tektonRun}/${stepNumber}/out.beforestepexit + echo "Executing step $stepNumber..." +else + echo "Last step (no. $stepNumber) has already been executed, before step breakpoint exiting !" + exit 0 +fi` + debugBeforeStepFailScriptTemplate = ` +numberOfSteps=%d +debugInfo=%s +tektonRun=%s + +postFile="$(ls ${debugInfo} | grep -E '[0-9]+' | tail -1)" +stepNumber="$(echo ${postFile} | sed 's/[^0-9]*//g')" + +if [ $stepNumber -lt $numberOfSteps ]; then + echo "1" > ${tektonRun}/${stepNumber}/out.beforestepexit.err + echo "Executing step $stepNumber..." +else + echo "Last step (no. $stepNumber) has already been executed, before step breakpoint exiting !" + exit 0 fi` initScriptDirective = `tmpfile="%s" touch ${tmpfile} && chmod +x ${tmpfile}