From e1245833d2d81bcb4cf6af2fa1ebb9958b66f74b Mon Sep 17 00:00:00 2001 From: Vibhav Bobade Date: Fri, 2 Feb 2024 14:46:39 +0530 Subject: [PATCH] Implement breakpoint_after, breakpoint_before to step entrypoint breakpoint_on_failure runs at the end of a step, this commit adds a manual breakpoint -breakpoint_after to run after the step ends whether it fails or not. The checks for this have been added in the same place as breakpoint on failure breakpoint_before sets a breakpoint before the entrypointer runs the command --- cmd/entrypoint/main.go | 21 ++++++---- docs/pipeline-api.md | 42 +++++++++++++++++++ pkg/apis/pipeline/v1/openapi_generated.go | 28 +++++++++++++ pkg/apis/pipeline/v1/swagger.json | 14 +++++++ pkg/apis/pipeline/v1/taskrun_types.go | 18 +++++--- pkg/apis/pipeline/v1/zz_generated.deepcopy.go | 12 +++++- .../pipeline/v1beta1/openapi_generated.go | 28 +++++++++++++ pkg/apis/pipeline/v1beta1/swagger.json | 14 +++++++ pkg/apis/pipeline/v1beta1/taskrun_types.go | 9 +++- .../pipeline/v1beta1/zz_generated.deepcopy.go | 12 +++++- pkg/entrypoint/entrypointer.go | 4 ++ pkg/pod/entrypoint.go | 6 +++ pkg/pod/script.go | 2 +- 13 files changed, 193 insertions(+), 17 deletions(-) diff --git a/cmd/entrypoint/main.go b/cmd/entrypoint/main.go index dead6fa6581..3935fdd1987 100644 --- a/cmd/entrypoint/main.go +++ b/cmd/entrypoint/main.go @@ -54,6 +54,8 @@ 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") + breakpointBefore = flag.Bool("breakpoint_before", false, "If specified, expect steps to not skip before the specified step") + breakpointAfter = flag.Bool("breakpoint_after", false, "If specified, expect steps to not skip after the specified 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//") @@ -67,9 +69,9 @@ const ( breakpointExitSuffix = ".breakpointexit" ) -func checkForBreakpointOnFailure(e entrypoint.Entrypointer, breakpointExitPostFile string) { - if e.BreakpointOnFailure { - if waitErr := e.Waiter.Wait(context.Background(), breakpointExitPostFile, false, false); waitErr != nil { +func checkForBreakpoint(e entrypoint.Entrypointer, breakpointExitPostFile string) { + if e.BreakpointOnFailure || e.BreakpointAfter || e.BreakpointBefore { + if waitErr := e.Waiter.Wait(context.Background(), breakpointExitPostFile, false, e.BreakpointOnFailure); waitErr != nil { log.Println("error occurred while waiting for " + breakpointExitPostFile + " : " + waitErr.Error()) } // get exitcode from .breakpointexit @@ -163,6 +165,8 @@ func main() { StepResults: strings.Split(*stepResults, ","), Timeout: timeout, BreakpointOnFailure: *breakpointOnFailure, + BreakpointAfter: *breakpointAfter, + BreakpointBefore: *breakpointBefore, OnError: *onError, StepMetadataDir: *stepMetadataDir, SpireWorkloadAPI: spireWorkloadAPI, @@ -174,9 +178,10 @@ func main() { if err := credentials.CopyCredsToHome(credentials.CredsInitCredentials); err != nil { log.Printf("non-fatal error copying credentials: %q", err) } - + breakpointExitPostFile := e.PostFile + breakpointExitSuffix + // check if there is a breakpoint set before the entrypoint runs + checkForBreakpoint(e, breakpointExitPostFile) 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 skipError: log.Print("Skipping step because a previous step failed") @@ -201,7 +206,8 @@ func main() { // in both cases has an ExitStatus() method with the // same signature. if status, ok := t.Sys().(syscall.WaitStatus); ok { - checkForBreakpointOnFailure(e, breakpointExitPostFile) + // check if there is a breakpoint set after the command errors out + checkForBreakpoint(e, breakpointExitPostFile) // 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()) @@ -212,7 +218,8 @@ func main() { log.Fatalf("Error executing command (ExitError): %v", err) } default: - checkForBreakpointOnFailure(e, breakpointExitPostFile) + // check if there is a breakpoint set after the command has finished running + checkForBreakpoint(e, breakpointExitPostFile) log.Fatalf("Error executing command: %v", err) } } diff --git a/docs/pipeline-api.md b/docs/pipeline-api.md index f7e0439660c..4bcd08d1ae9 100644 --- a/docs/pipeline-api.md +++ b/docs/pipeline-api.md @@ -4869,6 +4869,26 @@ string failed step will not exit

+ + +afterBreakpoints
+ +[]string + + + + + + + +beforeBreakpoints
+ +[]string + + + + +

TaskKind @@ -13957,6 +13977,28 @@ string failed step will not exit

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

TaskKind diff --git a/pkg/apis/pipeline/v1/openapi_generated.go b/pkg/apis/pipeline/v1/openapi_generated.go index c6a73d7afc3..9ffc10c796f 100644 --- a/pkg/apis/pipeline/v1/openapi_generated.go +++ b/pkg/apis/pipeline/v1/openapi_generated.go @@ -3377,6 +3377,34 @@ func schema_pkg_apis_pipeline_v1_TaskBreakpoints(ref common.ReferenceCallback) c Format: "", }, }, + "afterBreakpoints": { + SchemaProps: spec.SchemaProps{ + Type: []string{"array"}, + Items: &spec.SchemaOrArray{ + Schema: &spec.Schema{ + SchemaProps: spec.SchemaProps{ + Default: "", + Type: []string{"string"}, + Format: "", + }, + }, + }, + }, + }, + "beforeBreakpoints": { + 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 1424609e3b6..9ac1d97303e 100644 --- a/pkg/apis/pipeline/v1/swagger.json +++ b/pkg/apis/pipeline/v1/swagger.json @@ -1729,6 +1729,20 @@ "description": "TaskBreakpoints defines the breakpoint config for a particular Task", "type": "object", "properties": { + "afterBreakpoints": { + "type": "array", + "items": { + "type": "string", + "default": "" + } + }, + "beforeBreakpoints": { + "type": "array", + "items": { + "type": "string", + "default": "" + } + }, "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 ff94ed6c330..96fd9948e06 100644 --- a/pkg/apis/pipeline/v1/taskrun_types.go +++ b/pkg/apis/pipeline/v1/taskrun_types.go @@ -16,6 +16,7 @@ package v1 import ( "context" "fmt" + "k8s.io/utils/strings/slices" "time" "github.com/tektoncd/pipeline/pkg/apis/config" @@ -119,7 +120,9 @@ type TaskBreakpoints struct { // if enabled, pause TaskRun on failure of a step // failed step will not exit // +optional - OnFailure string `json:"onFailure,omitempty"` + OnFailure string `json:"onFailure,omitempty"` + AfterBreakpoints []string `json:"afterBreakpoints,omitempty"` + BeforeBreakpoints []string `json:"beforeBreakpoints,omitempty"` } // NeedsDebugOnFailure return true if the TaskRun is configured to debug on failure @@ -130,14 +133,19 @@ func (trd *TaskRunDebug) NeedsDebugOnFailure() bool { 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() +// StepNeedsDebugAfter return true if the step is configured to have a breakpoint after it's execution +func (trd *TaskRunDebug) StepNeedsDebugAfter(stepName string) bool { + return slices.Contains(trd.Breakpoints.AfterBreakpoints, stepName) +} + +// StepNeedsDebugBefore return true if the step is configured to have a breakpoint before it's execution +func (trd *TaskRunDebug) StepNeedsDebugBefore(stepName string) bool { + return slices.Contains(trd.Breakpoints.BeforeBreakpoints, stepName) } // NeedsDebug return true if defined onfailure or have any before, after steps func (trd *TaskRunDebug) NeedsDebug() bool { - return trd.NeedsDebugOnFailure() + return trd.NeedsDebugOnFailure() || len(trd.Breakpoints.AfterBreakpoints) > 0 || len(trd.Breakpoints.BeforeBreakpoints) > 0 } // TaskRunInputs holds the input values that this task was invoked with. diff --git a/pkg/apis/pipeline/v1/zz_generated.deepcopy.go b/pkg/apis/pipeline/v1/zz_generated.deepcopy.go index db26c7d4c5e..4078482a71b 100644 --- a/pkg/apis/pipeline/v1/zz_generated.deepcopy.go +++ b/pkg/apis/pipeline/v1/zz_generated.deepcopy.go @@ -1473,6 +1473,16 @@ 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.AfterBreakpoints != nil { + in, out := &in.AfterBreakpoints, &out.AfterBreakpoints + *out = make([]string, len(*in)) + copy(*out, *in) + } + if in.BeforeBreakpoints != nil { + in, out := &in.BeforeBreakpoints, &out.BeforeBreakpoints + *out = make([]string, len(*in)) + copy(*out, *in) + } return } @@ -1598,7 +1608,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 c55315ac140..8a62066d6d8 100644 --- a/pkg/apis/pipeline/v1beta1/openapi_generated.go +++ b/pkg/apis/pipeline/v1beta1/openapi_generated.go @@ -4370,6 +4370,34 @@ func schema_pkg_apis_pipeline_v1beta1_TaskBreakpoints(ref common.ReferenceCallba Format: "", }, }, + "beforeSteps": { + SchemaProps: spec.SchemaProps{ + Type: []string{"array"}, + Items: &spec.SchemaOrArray{ + Schema: &spec.Schema{ + SchemaProps: spec.SchemaProps{ + Default: "", + Type: []string{"string"}, + Format: "", + }, + }, + }, + }, + }, + "afterSteps": { + 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 b8624cf62ce..bc63a05a64e 100644 --- a/pkg/apis/pipeline/v1beta1/swagger.json +++ b/pkg/apis/pipeline/v1beta1/swagger.json @@ -2413,6 +2413,20 @@ "description": "TaskBreakpoints defines the breakpoint config for a particular Task", "type": "object", "properties": { + "afterSteps": { + "type": "array", + "items": { + "type": "string", + "default": "" + } + }, + "beforeSteps": { + "type": "array", + "items": { + "type": "string", + "default": "" + } + }, "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_types.go b/pkg/apis/pipeline/v1beta1/taskrun_types.go index a12676acb0a..79a0db20df7 100644 --- a/pkg/apis/pipeline/v1beta1/taskrun_types.go +++ b/pkg/apis/pipeline/v1beta1/taskrun_types.go @@ -19,6 +19,7 @@ package v1beta1 import ( "context" "fmt" + "k8s.io/utils/strings/slices" "time" "github.com/tektoncd/pipeline/pkg/apis/config" @@ -125,6 +126,10 @@ type TaskBreakpoints struct { // failed step will not exit // +optional OnFailure string `json:"onFailure,omitempty"` + // +optional + BeforeSteps []string `json:"beforeSteps,omitempty"` + // +optional + AfterSteps []string `json:"afterSteps,omitempty"` } // NeedsDebugOnFailure return true if the TaskRun is configured to debug on failure @@ -137,12 +142,12 @@ func (trd *TaskRunDebug) NeedsDebugOnFailure() bool { // StepNeedsDebug return true if the step is configured to debug func (trd *TaskRunDebug) StepNeedsDebug(stepName string) bool { - return trd.NeedsDebugOnFailure() + return trd.NeedsDebugOnFailure() || slices.Contains(trd.Breakpoints.BeforeSteps, stepName) || slices.Contains(trd.Breakpoints.AfterSteps, stepName) } // NeedsDebug return true if defined onfailure or have any before, after steps func (trd *TaskRunDebug) NeedsDebug() bool { - return trd.NeedsDebugOnFailure() + return trd.NeedsDebugOnFailure() || len(trd.Breakpoints.BeforeSteps) > 0 || len(trd.Breakpoints.AfterSteps) > 0 } var taskRunCondSet = apis.NewBatchConditionSet() diff --git a/pkg/apis/pipeline/v1beta1/zz_generated.deepcopy.go b/pkg/apis/pipeline/v1beta1/zz_generated.deepcopy.go index 68cce2d2643..e0e0c91ffa8 100644 --- a/pkg/apis/pipeline/v1beta1/zz_generated.deepcopy.go +++ b/pkg/apis/pipeline/v1beta1/zz_generated.deepcopy.go @@ -1948,6 +1948,16 @@ 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) + } + if in.AfterSteps != nil { + in, out := &in.AfterSteps, &out.AfterSteps + *out = make([]string, len(*in)) + copy(*out, *in) + } return } @@ -2138,7 +2148,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 d27d3554682..d7d331fe253 100644 --- a/pkg/entrypoint/entrypointer.go +++ b/pkg/entrypoint/entrypointer.go @@ -106,6 +106,10 @@ type Entrypointer struct { Timeout *time.Duration // BreakpointOnFailure helps determine if entrypoint execution needs to adapt debugging requirements BreakpointOnFailure bool + // BreakpointAfter helps determine if entrypoint execution needs to halt after for debugging + BreakpointAfter bool + // BreakpointBefore helps determine if entrypoint execution needs to halt before for debugging + BreakpointBefore 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 diff --git a/pkg/pod/entrypoint.go b/pkg/pod/entrypoint.go index f8c73d7491b..e3d54634e4d 100644 --- a/pkg/pod/entrypoint.go +++ b/pkg/pod/entrypoint.go @@ -179,6 +179,12 @@ func orderContainers(ctx context.Context, commonExtraEntrypointArgs []string, st if breakpointConfig != nil && breakpointConfig.NeedsDebugOnFailure() { argsForEntrypoint = append(argsForEntrypoint, "-breakpoint_on_failure") } + if breakpointConfig != nil && breakpointConfig.StepNeedsDebugAfter(s.Name) { + argsForEntrypoint = append(argsForEntrypoint, "-breakpoint_after") + } + if breakpointConfig != nil && breakpointConfig.StepNeedsDebugBefore(s.Name) { + argsForEntrypoint = append(argsForEntrypoint, "-breakpoint_before") + } cmd, args := s.Command, s.Args if len(cmd) > 0 { diff --git a/pkg/pod/script.go b/pkg/pod/script.go index 32ee47ae2e6..fe1a1ca39e8 100644 --- a/pkg/pod/script.go +++ b/pkg/pod/script.go @@ -145,7 +145,7 @@ func convertListOfSteps(steps []v1.Step, initContainer *corev1.Container, debugC } containers = append(containers, *c) } - if debugConfig != nil && debugConfig.NeedsDebugOnFailure() { + if debugConfig != nil && debugConfig.NeedsDebug() { placeDebugScriptInContainers(containers, initContainer) } return containers