From 0e7fce240c77d66a67b1aedd5813799cd2eb2e4a Mon Sep 17 00:00:00 2001 From: Vibhav Bobade Date: Mon, 21 Dec 2020 17:13:36 +0530 Subject: [PATCH] Add debug with breakpoint onFailure to TaskRun Spec Based on TEP-0042, this commit adds the ability for the user to debug TaskRuns with the spec.debug delaration where the user can provide breakpoints in spec.debug.breakpoint. With this commit the only breakpoint supported right now is the "onFailure" breakpoint which pauses the TaskRun during a step failure so the user can get shell access to the failed step container and debug. The following additions have been done. - Add -breakpoint_on_failure to entrypointer which disables exit of container upon failure - Add debug helper scripts to /tekton/debug/scripts which contains continuing the TaskRun and marking the step as a success or a failure by writing .breakpointexit file to /tekton/tools - Add /tekton/debug/info/ mount which is used by helper scripts to understand which step they are running in where denotes the step number eg: First step = 0, Second step = 1 and so on. - Exit code is propogated from .breakpointexit to end of TaskRun - Disable step skipError if breakpoint enabled - Add webhook validation for debug - Add Debug as a alpha feature Signed-off-by: Vincent Demeester Signed-off-by: Vibhav Bobade --- cmd/entrypoint/main.go | 62 ++++-- cmd/entrypoint/waiter.go | 12 +- cmd/entrypoint/waiter_test.go | 64 +++++- docs/debug.md | 82 +++++++ docs/taskruns.md | 39 ++++ .../pipeline/v1beta1/openapi_generated.go | 35 ++- pkg/apis/pipeline/v1beta1/swagger.json | 16 ++ pkg/apis/pipeline/v1beta1/taskrun_types.go | 8 + .../pipeline/v1beta1/taskrun_validation.go | 21 ++ .../v1beta1/taskrun_validation_test.go | 23 ++ .../pipeline/v1beta1/zz_generated.deepcopy.go | 26 +++ pkg/entrypoint/entrypointer.go | 32 ++- pkg/entrypoint/entrypointer_test.go | 47 +++- pkg/pod/entrypoint.go | 15 +- pkg/pod/entrypoint_test.go | 41 +++- pkg/pod/pod.go | 28 ++- pkg/pod/pod_test.go | 207 ++++++++++++++++++ pkg/pod/script.go | 82 ++++++- pkg/pod/script_test.go | 154 ++++++++++++- pkg/pod/scripts_constants.go | 43 ++++ 20 files changed, 971 insertions(+), 66 deletions(-) create mode 100644 docs/debug.md create mode 100644 pkg/pod/scripts_constants.go diff --git a/cmd/entrypoint/main.go b/cmd/entrypoint/main.go index be95770e567..d87197d673a 100644 --- a/cmd/entrypoint/main.go +++ b/cmd/entrypoint/main.go @@ -34,16 +34,36 @@ import ( ) var ( - ep = flag.String("entrypoint", "", "Original specified entrypoint to execute") - waitFiles = flag.String("wait_file", "", "Comma-separated list of paths to wait for") - waitFileContent = flag.Bool("wait_file_content", false, "If specified, expect wait_file to have content") - postFile = flag.String("post_file", "", "If specified, file to write upon completion") - terminationPath = flag.String("termination_path", "/tekton/termination", "If specified, file to write upon termination") - results = flag.String("results", "", "If specified, list of file names that might contain task results") - timeout = flag.Duration("timeout", time.Duration(0), "If specified, sets timeout for step") + ep = flag.String("entrypoint", "", "Original specified entrypoint to execute") + waitFiles = flag.String("wait_file", "", "Comma-separated list of paths to wait for") + waitFileContent = flag.Bool("wait_file_content", false, "If specified, expect wait_file to have content") + postFile = flag.String("post_file", "", "If specified, file to write upon completion") + terminationPath = flag.String("termination_path", "/tekton/termination", "If specified, file to write upon termination") + results = flag.String("results", "", "If specified, list of file names that might contain task results") + timeout = flag.Duration("timeout", time.Duration(0), "If specified, sets timeout for step") + breakpointOnFailure = flag.Bool("breakpoint_on_failure", false, "If specified, expect steps to not skip on failure") ) -const defaultWaitPollingInterval = time.Second +const ( + defaultWaitPollingInterval = time.Second + breakpointExitSuffix = ".breakpointexit" +) + +func checkForBreakpointOnFailure(e entrypoint.Entrypointer, breakpointExitPostFile string) { + if e.BreakpointOnFailure { + if waitErr := e.Waiter.Wait(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 @@ -75,17 +95,18 @@ func main() { } e := entrypoint.Entrypointer{ - Entrypoint: *ep, - WaitFiles: strings.Split(*waitFiles, ","), - WaitFileContent: *waitFileContent, - PostFile: *postFile, - TerminationPath: *terminationPath, - Args: flag.Args(), - Waiter: &realWaiter{waitPollingInterval: defaultWaitPollingInterval}, - Runner: &realRunner{}, - PostWriter: &realPostWriter{}, - Results: strings.Split(*results, ","), - Timeout: timeout, + Entrypoint: *ep, + WaitFiles: strings.Split(*waitFiles, ","), + WaitFileContent: *waitFileContent, + PostFile: *postFile, + TerminationPath: *terminationPath, + Args: flag.Args(), + Waiter: &realWaiter{waitPollingInterval: defaultWaitPollingInterval, breakpointOnFailure: *breakpointOnFailure}, + Runner: &realRunner{}, + PostWriter: &realPostWriter{}, + Results: strings.Split(*results, ","), + Timeout: timeout, + BreakpointOnFailure: *breakpointOnFailure, } // Copy any creds injected by the controller into the $HOME directory of the current @@ -95,6 +116,7 @@ func main() { } if err := e.Go(); err != nil { + breakpointExitPostFile := e.PostFile + breakpointExitSuffix switch t := err.(type) { case skipError: log.Print("Skipping step because a previous step failed") @@ -110,10 +132,12 @@ func main() { // in both cases has an ExitStatus() method with the // same signature. if status, ok := t.Sys().(syscall.WaitStatus); ok { + checkForBreakpointOnFailure(e, breakpointExitPostFile) os.Exit(status.ExitStatus()) } log.Fatalf("Error executing command (ExitError): %v", err) default: + checkForBreakpointOnFailure(e, breakpointExitPostFile) log.Fatalf("Error executing command: %v", err) } } diff --git a/cmd/entrypoint/waiter.go b/cmd/entrypoint/waiter.go index 2edef6a4bd8..fbf82bd2792 100644 --- a/cmd/entrypoint/waiter.go +++ b/cmd/entrypoint/waiter.go @@ -11,6 +11,7 @@ import ( // realWaiter actually waits for files, by polling. type realWaiter struct { waitPollingInterval time.Duration + breakpointOnFailure bool } var _ entrypoint.Waiter = (*realWaiter)(nil) @@ -30,7 +31,7 @@ func (rw *realWaiter) setWaitPollingInterval(pollingInterval time.Duration) *rea // // If a file of the same name with a ".err" extension exists then this Wait // will end with a skipError. -func (rw *realWaiter) Wait(file string, expectContent bool) error { +func (rw *realWaiter) Wait(file string, expectContent bool, breakpointOnFailure bool) error { if file == "" { return nil } @@ -42,7 +43,16 @@ func (rw *realWaiter) Wait(file string, expectContent bool) error { } else if !os.IsNotExist(err) { return fmt.Errorf("waiting for %q: %w", file, err) } + // When a .err file is read by this step, it means that a previous step has failed + // We wouldn't want this step to stop executing because the previous step failed during debug + // That is counterproductive to debugging + // Hence we disable skipError here so that the other steps in the failed taskRun can continue + // executing if breakpointOnFailure is enabled for the taskRun + // TLDR: Do not return skipError when breakpointOnFailure is enabled as it breaks execution of the TaskRun if _, err := os.Stat(file + ".err"); err == nil { + if breakpointOnFailure { + return nil + } return skipError("error file present, bail and skip the step") } } diff --git a/cmd/entrypoint/waiter_test.go b/cmd/entrypoint/waiter_test.go index bd66fe3b901..5c2585f5274 100644 --- a/cmd/entrypoint/waiter_test.go +++ b/cmd/entrypoint/waiter_test.go @@ -19,6 +19,7 @@ package main import ( "io/ioutil" "os" + "strings" "testing" "time" ) @@ -37,7 +38,7 @@ func TestRealWaiterWaitMissingFile(t *testing.T) { rw := realWaiter{} doneCh := make(chan struct{}) go func() { - err := rw.setWaitPollingInterval(testWaitPollingInterval).Wait(tmp.Name(), false) + err := rw.setWaitPollingInterval(testWaitPollingInterval).Wait(tmp.Name(), false, false) if err != nil { t.Errorf("error waiting on tmp file %q", tmp.Name()) } @@ -60,7 +61,7 @@ func TestRealWaiterWaitWithFile(t *testing.T) { rw := realWaiter{} doneCh := make(chan struct{}) go func() { - err := rw.setWaitPollingInterval(testWaitPollingInterval).Wait(tmp.Name(), false) + err := rw.setWaitPollingInterval(testWaitPollingInterval).Wait(tmp.Name(), false, false) if err != nil { t.Errorf("error waiting on tmp file %q", tmp.Name()) } @@ -83,7 +84,7 @@ func TestRealWaiterWaitMissingContent(t *testing.T) { rw := realWaiter{} doneCh := make(chan struct{}) go func() { - err := rw.setWaitPollingInterval(testWaitPollingInterval).Wait(tmp.Name(), true) + err := rw.setWaitPollingInterval(testWaitPollingInterval).Wait(tmp.Name(), true, false) if err != nil { t.Errorf("error waiting on tmp file %q", tmp.Name()) } @@ -106,7 +107,7 @@ func TestRealWaiterWaitWithContent(t *testing.T) { rw := realWaiter{} doneCh := make(chan struct{}) go func() { - err := rw.setWaitPollingInterval(testWaitPollingInterval).Wait(tmp.Name(), true) + err := rw.setWaitPollingInterval(testWaitPollingInterval).Wait(tmp.Name(), true, false) if err != nil { t.Errorf("error waiting on tmp file %q", tmp.Name()) } @@ -122,3 +123,58 @@ func TestRealWaiterWaitWithContent(t *testing.T) { t.Errorf("expected Wait() to have detected a non-zero file size by now") } } + +func TestRealWaiterWaitWithErrorWaitfile(t *testing.T) { + tmp, err := ioutil.TempFile("", "real_waiter_test_file*.err") + if err != nil { + t.Errorf("error creating temp file: %v", err) + } + tmpFileName := strings.Replace(tmp.Name(), ".err", "", 1) + defer os.Remove(tmp.Name()) + rw := realWaiter{} + doneCh := make(chan struct{}) + go func() { + // error of type skipError is returned after encountering a error waitfile + err := rw.setWaitPollingInterval(testWaitPollingInterval).Wait(tmpFileName, false, false) + if err == nil { + t.Errorf("expected skipError upon encounter error waitfile") + } + switch typ := err.(type) { + case skipError: + close(doneCh) + default: + t.Errorf("unexpected error type %T", typ) + } + }() + select { + case <-doneCh: + // Success + case <-time.After(2 * testWaitPollingInterval): + t.Errorf("expected Wait() to have detected a non-zero file size by now") + } +} + +func TestRealWaiterWaitWithBreakpointOnFailure(t *testing.T) { + tmp, err := ioutil.TempFile("", "real_waiter_test_file*.err") + if err != nil { + t.Errorf("error creating temp file: %v", err) + } + tmpFileName := strings.Replace(tmp.Name(), ".err", "", 1) + defer os.Remove(tmp.Name()) + rw := realWaiter{} + doneCh := make(chan struct{}) + go func() { + // When breakpoint on failure is enabled skipError shouldn't be returned for a error waitfile + err := rw.setWaitPollingInterval(testWaitPollingInterval).Wait(tmpFileName, false, true) + if err != nil { + t.Errorf("error waiting on tmp file %q", tmp.Name()) + } + close(doneCh) + }() + select { + case <-doneCh: + // Success + case <-time.After(2 * testWaitPollingInterval): + t.Errorf("expected Wait() to have detected a non-zero file size by now") + } +} diff --git a/docs/debug.md b/docs/debug.md new file mode 100644 index 00000000000..28822caa9ce --- /dev/null +++ b/docs/debug.md @@ -0,0 +1,82 @@ + +# Debug + +- [Overview](#overview) +- [Debugging TaskRuns](#debugging-taskruns) + - [Adding Breakpoints](#adding-breakpoints) + - [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) +- [Debug Environment](#debug-environment) + - [Mounts](#mounts) + - [Debug Scripts](#debug-scripts) + + +## Overview + +`Debug` spec is used for troubleshooting and breakpointing runtime resources. This doc helps understand the inner +workings of debug in Tekton. Currently only the `TaskRun` resource is supported. + +## Debugging TaskRuns + +The following provides explanation on how Debugging TaskRuns is possible through Tekton. To understand how to use +the debug spec for TaskRuns follow the [TaskRun Debugging Documentation](taskruns.md#debugging-a-taskrun). + +### 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/tools` 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 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/tools` +would unpause and exit the step container. + +## Debug Environment + +Additional environment augmentations made available to the TaskRun Pod to aid in troubleshooting and managing step lifecycle. + +### Mounts + +`/tekton/debug/scripts` : Contains scripts which the user can run to mark the step as a success, failure or exit the breakpoint. +Shared between all the containers. + +`/tekton/debug/info/` : Contains information about the step. Single EmptyDir shared between all step containers, but renamed +to reflect step number. eg: Step 0 will have `/tekton/debug/info/0`, Step 1 will have `/tekton/debug/info/1` etc. + +### Debug Scripts + +`/tekton/debug/scripts/debug-continue` : Mark the step as completed with success by writing to `/tekton/tools`. eg: User wants to exit +breakpoint for failed step 0. Running this script would create `/tekton/tools/0` and `/tekton/tools/0.breakpointexit`. + +`/tekton/debug/scripts/debug-fail-continue` : Mark the step as completed with failure by writing to `/tekton/tools`. eg: User wants to exit +breakpoint for failed step 0. Running this script would create `/tekton/tools/0.err` and `/tekton/tools/0.breakpointexit`. \ No newline at end of file diff --git a/docs/taskruns.md b/docs/taskruns.md index aec2b9848cd..867cfb12d6d 100644 --- a/docs/taskruns.md +++ b/docs/taskruns.md @@ -23,6 +23,7 @@ weight: 300 - [Monitoring `Steps`](#monitoring-steps) - [Monitoring `Results`](#monitoring-results) - [Cancelling a `TaskRun`](#cancelling-a-taskrun) +- [Debugging a `TaskRun`](#debugging-a-taskrun) - [Events](events.md#taskruns) - [Running a TaskRun Hermetically](hermetic.md) - [Code examples](#code-examples) @@ -30,6 +31,7 @@ weight: 300 - [Example `TaskRun` with an embedded `Task`](#example-taskrun-with-an-embedded-task) - [Reusing a `Task`](#reusing-a-task) - [Using custom `ServiceAccount` credentials](#using-custom-serviceaccount-credentials) + - [Running step containers as a non-root user](#running-step-containers-as-a-non-root-user) # Overview @@ -447,6 +449,43 @@ spec: status: "TaskRunCancelled" ``` + +### Debugging a `TaskRun` + +#### Breakpoint on Failure + +TaskRuns can be halted on failure for troubleshooting by providing the following spec patch as seen below. + +```yaml +spec: + debug: + breakpoint: ["onFailure"] +``` + +Upon failure of a step, the TaskRun Pod execution is halted. If ths 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). +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-w5qrn -c step-print-date-human-readable +``` + +#### Debug Environment + +After the user/client has access to the container environment, they can scour for any missing parts because of which +their step might have failed. + +To control the lifecycle of the step to mark it as a success or a failure or close the breakpoint, there are scripts +provided in the `/tekton/debug/scripts` directory in the container. The following are the scripts and the tasks they +perform :- + +`debug-continue`: Mark the step as a success and exit the breakpoint. + +`debug-fail-continue`: Mark the step as a failure and exit the breakpoint. + +*More information on the inner workings of debug can be found in the [Debug documentation](debug.md)* + ## Code examples To better understand `TaskRuns`, study the following code examples: diff --git a/pkg/apis/pipeline/v1beta1/openapi_generated.go b/pkg/apis/pipeline/v1beta1/openapi_generated.go index ba8c2613c84..60edf78d6b6 100644 --- a/pkg/apis/pipeline/v1beta1/openapi_generated.go +++ b/pkg/apis/pipeline/v1beta1/openapi_generated.go @@ -86,6 +86,7 @@ func GetOpenAPIDefinitions(ref common.ReferenceCallback) map[string]common.OpenA "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.TaskResources": schema_pkg_apis_pipeline_v1beta1_TaskResources(ref), "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.TaskResult": schema_pkg_apis_pipeline_v1beta1_TaskResult(ref), "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.TaskRun": schema_pkg_apis_pipeline_v1beta1_TaskRun(ref), + "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.TaskRunDebug": schema_pkg_apis_pipeline_v1beta1_TaskRunDebug(ref), "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.TaskRunInputs": schema_pkg_apis_pipeline_v1beta1_TaskRunInputs(ref), "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.TaskRunList": schema_pkg_apis_pipeline_v1beta1_TaskRunList(ref), "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.TaskRunOutputs": schema_pkg_apis_pipeline_v1beta1_TaskRunOutputs(ref), @@ -3501,6 +3502,33 @@ func schema_pkg_apis_pipeline_v1beta1_TaskRun(ref common.ReferenceCallback) comm } } +func schema_pkg_apis_pipeline_v1beta1_TaskRunDebug(ref common.ReferenceCallback) common.OpenAPIDefinition { + return common.OpenAPIDefinition{ + Schema: spec.Schema{ + SchemaProps: spec.SchemaProps{ + Description: "TaskRunDebug defines the breakpoint config for a particular TaskRun", + Type: []string{"object"}, + Properties: map[string]spec.Schema{ + "breakpoint": { + SchemaProps: spec.SchemaProps{ + Type: []string{"array"}, + Items: &spec.SchemaOrArray{ + Schema: &spec.Schema{ + SchemaProps: spec.SchemaProps{ + Default: "", + Type: []string{"string"}, + Format: "", + }, + }, + }, + }, + }, + }, + }, + }, + } +} + func schema_pkg_apis_pipeline_v1beta1_TaskRunInputs(ref common.ReferenceCallback) common.OpenAPIDefinition { return common.OpenAPIDefinition{ Schema: spec.Schema{ @@ -3699,6 +3727,11 @@ func schema_pkg_apis_pipeline_v1beta1_TaskRunSpec(ref common.ReferenceCallback) Description: "TaskRunSpec defines the desired state of TaskRun", Type: []string{"object"}, Properties: map[string]spec.Schema{ + "debug": { + SchemaProps: spec.SchemaProps{ + Ref: ref("github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.TaskRunDebug"), + }, + }, "params": { SchemaProps: spec.SchemaProps{ Type: []string{"array"}, @@ -3772,7 +3805,7 @@ func schema_pkg_apis_pipeline_v1beta1_TaskRunSpec(ref common.ReferenceCallback) }, }, Dependencies: []string{ - "github.com/tektoncd/pipeline/pkg/apis/pipeline/pod.Template", "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.Param", "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.TaskRef", "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.TaskRunResources", "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.TaskSpec", "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.WorkspaceBinding", "k8s.io/apimachinery/pkg/apis/meta/v1.Duration"}, + "github.com/tektoncd/pipeline/pkg/apis/pipeline/pod.Template", "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.Param", "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.TaskRef", "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.TaskRunDebug", "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.TaskRunResources", "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.TaskSpec", "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.WorkspaceBinding", "k8s.io/apimachinery/pkg/apis/meta/v1.Duration"}, } } diff --git a/pkg/apis/pipeline/v1beta1/swagger.json b/pkg/apis/pipeline/v1beta1/swagger.json index 76ed7bb9aa3..91e566fa452 100644 --- a/pkg/apis/pipeline/v1beta1/swagger.json +++ b/pkg/apis/pipeline/v1beta1/swagger.json @@ -2086,6 +2086,19 @@ } } }, + "v1beta1.TaskRunDebug": { + "description": "TaskRunDebug defines the breakpoint config for a particular TaskRun", + "type": "object", + "properties": { + "breakpoint": { + "type": "array", + "items": { + "type": "string", + "default": "" + } + } + } + }, "v1beta1.TaskRunInputs": { "description": "TaskRunInputs holds the input values that this task was invoked with.", "type": "object", @@ -2193,6 +2206,9 @@ "description": "TaskRunSpec defines the desired state of TaskRun", "type": "object", "properties": { + "debug": { + "$ref": "#/definitions/v1beta1.TaskRunDebug" + }, "params": { "type": "array", "items": { diff --git a/pkg/apis/pipeline/v1beta1/taskrun_types.go b/pkg/apis/pipeline/v1beta1/taskrun_types.go index 0cc46ae5045..debcbc68207 100644 --- a/pkg/apis/pipeline/v1beta1/taskrun_types.go +++ b/pkg/apis/pipeline/v1beta1/taskrun_types.go @@ -42,6 +42,8 @@ var ( // TaskRunSpec defines the desired state of TaskRun type TaskRunSpec struct { + // +optional + Debug *TaskRunDebug `json:"debug,omitempty"` // +optional Params []Param `json:"params,omitempty"` // +optional @@ -77,6 +79,12 @@ const ( TaskRunSpecStatusCancelled = "TaskRunCancelled" ) +// TaskRunDebug defines the breakpoint config for a particular TaskRun +type TaskRunDebug struct { + // +optional + Breakpoint []string `json:"breakpoint,omitempty"` +} + // TaskRunInputs holds the input values that this task was invoked with. type TaskRunInputs struct { // +optional diff --git a/pkg/apis/pipeline/v1beta1/taskrun_validation.go b/pkg/apis/pipeline/v1beta1/taskrun_validation.go index b2d6bf50bc4..f47ea927959 100644 --- a/pkg/apis/pipeline/v1beta1/taskrun_validation.go +++ b/pkg/apis/pipeline/v1beta1/taskrun_validation.go @@ -78,6 +78,13 @@ func (ts *TaskRunSpec) Validate(ctx context.Context) (errs *apis.FieldError) { errs = errs.Also(validateParameters(ts.Params).ViaField("params")) errs = errs.Also(validateWorkspaceBindings(ctx, ts.Workspaces).ViaField("workspaces")) errs = errs.Also(ts.Resources.Validate(ctx).ViaField("resources")) + if cfg.FeatureFlags.EnableAPIFields == config.AlphaAPIFields { + if ts.Debug != nil { + errs = errs.Also(validateDebug(ts.Debug).ViaField("debug")) + } + } else if ts.Debug != nil { + errs = errs.Also(apis.ErrDisallowedFields("debug")) + } if ts.Status != "" { if ts.Status != TaskRunSpecStatusCancelled { @@ -94,6 +101,20 @@ func (ts *TaskRunSpec) Validate(ctx context.Context) (errs *apis.FieldError) { return errs } +// validateDebug +func validateDebug(db *TaskRunDebug) (errs *apis.FieldError) { + breakpointOnFailure := "onFailure" + validBreakpoints := sets.NewString() + validBreakpoints.Insert(breakpointOnFailure) + + for _, b := range db.Breakpoint { + if !validBreakpoints.Has(b) { + errs = errs.Also(apis.ErrInvalidValue(fmt.Sprintf("%s is not a valid breakpoint. Available valid breakpoints include %s", b, validBreakpoints.List()), "breakpoint")) + } + } + return errs +} + // validateWorkspaceBindings makes sure the volumes provided for the Task's declared workspaces make sense. func validateWorkspaceBindings(ctx context.Context, wb []WorkspaceBinding) (errs *apis.FieldError) { seen := sets.NewString() diff --git a/pkg/apis/pipeline/v1beta1/taskrun_validation_test.go b/pkg/apis/pipeline/v1beta1/taskrun_validation_test.go index e0fbd3a59a1..c7c9e5a2c8b 100644 --- a/pkg/apis/pipeline/v1beta1/taskrun_validation_test.go +++ b/pkg/apis/pipeline/v1beta1/taskrun_validation_test.go @@ -239,6 +239,29 @@ func TestTaskRunSpec_Invalidate(t *testing.T) { }, wantErr: apis.ErrInvalidValue("invalid bundle reference (could not parse reference: invalid reference)", "taskref.bundle"), wc: enableTektonOCIBundles(t), + }, { + name: "using debug when apifields stable", + spec: v1beta1.TaskRunSpec{ + TaskRef: &v1beta1.TaskRef{ + Name: "my-task", + }, + Debug: &v1beta1.TaskRunDebug{ + Breakpoint: []string{"bReaKdAnCe"}, + }, + }, + wantErr: apis.ErrDisallowedFields("debug"), + }, { + name: "invalid breakpoint", + spec: v1beta1.TaskRunSpec{ + TaskRef: &v1beta1.TaskRef{ + Name: "my-task", + }, + Debug: &v1beta1.TaskRunDebug{ + Breakpoint: []string{"breakito"}, + }, + }, + wantErr: apis.ErrInvalidValue("breakito is not a valid breakpoint. Available valid breakpoints include [onFailure]", "debug.breakpoint"), + wc: enableAlphaAPIFields, }} for _, ts := range tests { t.Run(ts.name, func(t *testing.T) { diff --git a/pkg/apis/pipeline/v1beta1/zz_generated.deepcopy.go b/pkg/apis/pipeline/v1beta1/zz_generated.deepcopy.go index da60c5b31c5..97bd2956b6f 100644 --- a/pkg/apis/pipeline/v1beta1/zz_generated.deepcopy.go +++ b/pkg/apis/pipeline/v1beta1/zz_generated.deepcopy.go @@ -1482,6 +1482,27 @@ func (in *TaskRun) DeepCopyObject() runtime.Object { return nil } +// 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) + } + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new TaskRunDebug. +func (in *TaskRunDebug) DeepCopy() *TaskRunDebug { + if in == nil { + return nil + } + out := new(TaskRunDebug) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *TaskRunInputs) DeepCopyInto(out *TaskRunInputs) { *out = *in @@ -1617,6 +1638,11 @@ func (in *TaskRunResult) DeepCopy() *TaskRunResult { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *TaskRunSpec) DeepCopyInto(out *TaskRunSpec) { *out = *in + if in.Debug != nil { + in, out := &in.Debug, &out.Debug + *out = new(TaskRunDebug) + (*in).DeepCopyInto(*out) + } if in.Params != nil { in, out := &in.Params, &out.Params *out = make([]Param, len(*in)) diff --git a/pkg/entrypoint/entrypointer.go b/pkg/entrypoint/entrypointer.go index d4f7d8ae1df..be2e4f41b99 100644 --- a/pkg/entrypoint/entrypointer.go +++ b/pkg/entrypoint/entrypointer.go @@ -20,8 +20,11 @@ import ( "context" "fmt" "io/ioutil" + "log" "os" "path/filepath" + "strconv" + "strings" "time" "github.com/tektoncd/pipeline/pkg/apis/pipeline" @@ -66,12 +69,14 @@ type Entrypointer struct { Results []string // Timeout is an optional user-specified duration within which the Step must complete Timeout *time.Duration + // BreakpointOnFailure helps determine if entrypoint execution needs to adapt debugging requirements + BreakpointOnFailure bool } // Waiter encapsulates waiting for files to exist. type Waiter interface { // Wait blocks until the specified file exists. - Wait(file string, expectContent bool) error + Wait(file string, expectContent bool, breakpointOnFailure bool) error } // Runner encapsulates running commands. @@ -100,10 +105,13 @@ func (e Entrypointer) Go() error { }() for _, f := range e.WaitFiles { - if err := e.Waiter.Wait(f, e.WaitFileContent); err != nil { + if err := e.Waiter.Wait(f, e.WaitFileContent, e.BreakpointOnFailure); err != nil { // An error happened while waiting, so we bail // *but* we write postfile to make next steps bail too. - e.WritePostFile(e.PostFile, err) + // In case of breakpoint on failure do not write post file. + if !e.BreakpointOnFailure { + e.WritePostFile(e.PostFile, err) + } output = append(output, v1beta1.PipelineResourceResult{ Key: "StartedAt", Value: time.Now().Format(timeFormat), @@ -145,8 +153,11 @@ func (e Entrypointer) Go() error { } } - // Write the post file *no matter what* - e.WritePostFile(e.PostFile, err) + if err != nil && e.BreakpointOnFailure { + logger.Info("Skipping writing to PostFile") + } else { + e.WritePostFile(e.PostFile, err) + } // strings.Split(..) with an empty string returns an array that contains one element, an empty string. // This creates an error when trying to open the result folder as a file. @@ -187,6 +198,17 @@ func (e Entrypointer) readResultsFromDisk() error { return nil } +func (e Entrypointer) BreakpointExitCode(breakpointExitPostFile string) (int, error) { + exitCode, err := ioutil.ReadFile(breakpointExitPostFile) + if os.IsNotExist(err) { + return 0, fmt.Errorf("breakpoint postfile %s not found", breakpointExitPostFile) + } + strExitCode := strings.TrimSuffix(string(exitCode), "\n") + log.Println("Breakpoint exiting with exit code " + strExitCode) + + return strconv.Atoi(strExitCode) +} + // WritePostFile write the postfile func (e Entrypointer) WritePostFile(postFile string, err error) { if err != nil && postFile != "" { diff --git a/pkg/entrypoint/entrypointer_test.go b/pkg/entrypoint/entrypointer_test.go index 50bf1dd24fb..988be23526a 100644 --- a/pkg/entrypoint/entrypointer_test.go +++ b/pkg/entrypoint/entrypointer_test.go @@ -20,6 +20,7 @@ import ( "context" "encoding/json" "errors" + "fmt" "io/ioutil" "os" "reflect" @@ -120,6 +121,7 @@ func TestEntrypointer(t *testing.T) { for _, c := range []struct { desc, entrypoint, postFile string waitFiles, args []string + breakpointOnFailure bool }{{ desc: "do nothing", }, { @@ -145,20 +147,24 @@ func TestEntrypointer(t *testing.T) { }, { desc: "multiple wait files", waitFiles: []string{"waitforme", "metoo", "methree"}, + }, { + desc: "breakpointOnFailure to wait or not to wait ", + breakpointOnFailure: true, }} { t.Run(c.desc, func(t *testing.T) { fw, fr, fpw := &fakeWaiter{}, &fakeRunner{}, &fakePostWriter{} timeout := time.Duration(0) err := Entrypointer{ - Entrypoint: c.entrypoint, - WaitFiles: c.waitFiles, - PostFile: c.postFile, - Args: c.args, - Waiter: fw, - Runner: fr, - PostWriter: fpw, - TerminationPath: "termination", - Timeout: &timeout, + Entrypoint: c.entrypoint, + WaitFiles: c.waitFiles, + PostFile: c.postFile, + Args: c.args, + Waiter: fw, + Runner: fr, + PostWriter: fpw, + TerminationPath: "termination", + Timeout: &timeout, + BreakpointOnFailure: c.breakpointOnFailure, }.Go() if err != nil { t.Fatalf("Entrypointer failed: %v", err) @@ -225,9 +231,28 @@ func TestEntrypointer(t *testing.T) { } } +func TestEntrypointer_ReadBreakpointExitCodeFromDisk(t *testing.T) { + expectedExitCode := 1 + // setup test + tmp, err := ioutil.TempFile("", "1*.err") + if err != nil { + t.Errorf("error while creating temp file for testing exit code written by breakpoint") + } + // write exit code to file + if err = ioutil.WriteFile(tmp.Name(), []byte(fmt.Sprintf("%d", expectedExitCode)), 0700); err != nil { + t.Errorf("error while writing to temp file create temp file for testing exit code written by breakpoint") + } + e := Entrypointer{} + // test reading the exit code from error waitfile + actualExitCode, err := e.BreakpointExitCode(tmp.Name()) + if actualExitCode != expectedExitCode { + t.Errorf("error while parsing exit code. want %d , got %d", expectedExitCode, actualExitCode) + } +} + type fakeWaiter struct{ waited []string } -func (f *fakeWaiter) Wait(file string, _ bool) error { +func (f *fakeWaiter) Wait(file string, _ bool, _ bool) error { f.waited = append(f.waited, file) return nil } @@ -245,7 +270,7 @@ func (f *fakePostWriter) Write(file string) { f.wrote = &file } type fakeErrorWaiter struct{ waited *string } -func (f *fakeErrorWaiter) Wait(file string, expectContent bool) error { +func (f *fakeErrorWaiter) Wait(file string, expectContent bool, breakpointOnFailure bool) error { f.waited = &file return errors.New("waiter failed") } diff --git a/pkg/pod/entrypoint.go b/pkg/pod/entrypoint.go index 165e52a5490..fd1bfcd3943 100644 --- a/pkg/pod/entrypoint.go +++ b/pkg/pod/entrypoint.go @@ -48,6 +48,8 @@ const ( stepPrefix = "step-" sidecarPrefix = "sidecar-" + + BreakpointOnFailure = "onFailure" ) var ( @@ -91,7 +93,7 @@ var ( // command, we must have fetched the image's ENTRYPOINT before calling this // method, using entrypoint_lookup.go. // Additionally, Step timeouts are added as entrypoint flag. -func orderContainers(entrypointImage string, commonExtraEntrypointArgs []string, steps []corev1.Container, taskSpec *v1beta1.TaskSpec) (corev1.Container, []corev1.Container, error) { +func orderContainers(entrypointImage string, commonExtraEntrypointArgs []string, steps []corev1.Container, taskSpec *v1beta1.TaskSpec, breakpointConfig *v1beta1.TaskRunDebug) (corev1.Container, []corev1.Container, error) { initContainer := corev1.Container{ Name: "place-tools", Image: entrypointImage, @@ -145,6 +147,17 @@ func orderContainers(entrypointImage string, commonExtraEntrypointArgs []string, args = append(cmd[1:], args...) cmd = []string{cmd[0]} } + + 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") + } + } + } + argsForEntrypoint = append(argsForEntrypoint, "-entrypoint", cmd[0], "--") argsForEntrypoint = append(argsForEntrypoint, args...) diff --git a/pkg/pod/entrypoint_test.go b/pkg/pod/entrypoint_test.go index ca0e54f6a3c..19c61d461c9 100644 --- a/pkg/pod/entrypoint_test.go +++ b/pkg/pod/entrypoint_test.go @@ -88,7 +88,7 @@ func TestOrderContainers(t *testing.T) { VolumeMounts: []corev1.VolumeMount{toolsMount}, TerminationMessagePath: "/tekton/termination", }} - gotInit, got, err := orderContainers(images.EntrypointImage, []string{}, steps, nil) + gotInit, got, err := orderContainers(images.EntrypointImage, []string{}, steps, nil, nil) if err != nil { t.Fatalf("orderContainers: %v", err) } @@ -108,6 +108,39 @@ func TestOrderContainers(t *testing.T) { } } +func TestOrderContainersWithDebugOnFailure(t *testing.T) { + steps := []corev1.Container{{ + Image: "step-1", + Command: []string{"cmd"}, + Args: []string{"arg1", "arg2"}, + }} + want := []corev1.Container{{ + Image: "step-1", + Command: []string{entrypointBinary}, + Args: []string{ + "-wait_file", "/tekton/downward/ready", + "-wait_file_content", + "-post_file", "/tekton/tools/0", + "-termination_path", "/tekton/termination", + "-breakpoint_on_failure", + "-entrypoint", "cmd", "--", + "arg1", "arg2", + }, + VolumeMounts: []corev1.VolumeMount{toolsMount, downwardMount}, + TerminationMessagePath: "/tekton/termination", + }} + taskRunDebugConfig := &v1beta1.TaskRunDebug{ + Breakpoint: []string{"onFailure"}, + } + _, got, err := orderContainers(images.EntrypointImage, []string{}, steps, nil, taskRunDebugConfig) + if err != nil { + t.Fatalf("orderContainers: %v", err) + } + if d := cmp.Diff(want, got); d != "" { + t.Errorf("Diff %s", diff.PrintWantGot(d)) + } +} + func TestEntryPointResults(t *testing.T) { taskSpec := v1beta1.TaskSpec{ Results: []v1beta1.TaskResult{{ @@ -175,7 +208,7 @@ func TestEntryPointResults(t *testing.T) { VolumeMounts: []corev1.VolumeMount{toolsMount}, TerminationMessagePath: "/tekton/termination", }} - _, got, err := orderContainers(images.EntrypointImage, []string{}, steps, &taskSpec) + _, got, err := orderContainers(images.EntrypointImage, []string{}, steps, &taskSpec, nil) if err != nil { t.Fatalf("orderContainers: %v", err) } @@ -215,7 +248,7 @@ func TestEntryPointResultsSingleStep(t *testing.T) { VolumeMounts: []corev1.VolumeMount{toolsMount, downwardMount}, TerminationMessagePath: "/tekton/termination", }} - _, got, err := orderContainers(images.EntrypointImage, []string{}, steps, &taskSpec) + _, got, err := orderContainers(images.EntrypointImage, []string{}, steps, &taskSpec, nil) if err != nil { t.Fatalf("orderContainers: %v", err) } @@ -251,7 +284,7 @@ func TestEntryPointSingleResultsSingleStep(t *testing.T) { VolumeMounts: []corev1.VolumeMount{toolsMount, downwardMount}, TerminationMessagePath: "/tekton/termination", }} - _, got, err := orderContainers(images.EntrypointImage, []string{}, steps, &taskSpec) + _, got, err := orderContainers(images.EntrypointImage, []string{}, steps, &taskSpec, nil) if err != nil { t.Fatalf("orderContainers: %v", err) } diff --git a/pkg/pod/pod.go b/pkg/pod/pod.go index 8ca2659a1ef..6d5e140a88a 100644 --- a/pkg/pod/pod.go +++ b/pkg/pod/pod.go @@ -94,10 +94,15 @@ type Builder struct { // and TaskSpec provided in its arguments. An error is returned if there are // any problems during the conversion. func (b *Builder) Build(ctx context.Context, taskRun *v1beta1.TaskRun, taskSpec v1beta1.TaskSpec) (*corev1.Pod, error) { - var initContainers []corev1.Container - var volumes []corev1.Volume - var volumeMounts []corev1.VolumeMount + var ( + scriptsInit *corev1.Container + entrypointInit corev1.Container + initContainers, stepContainers, sidecarContainers []corev1.Container + volumes []corev1.Volume + volumeMounts []corev1.VolumeMount + ) implicitEnvVars := []corev1.EnvVar{} + alphaAPIEnabled := config.FromContextOrDefaults(ctx).FeatureFlags.EnableAPIFields == config.AlphaAPIFields // Add our implicit volumes first, so they can be overridden by the user if they prefer. volumes = append(volumes, implicitVolumes...) @@ -129,12 +134,20 @@ func (b *Builder) Build(ctx context.Context, taskRun *v1beta1.TaskRun, taskSpec // Convert any steps with Script to command+args. // If any are found, append an init container to initialize scripts. - scriptsInit, stepContainers, sidecarContainers := convertScripts(b.Images.ShellImage, steps, taskSpec.Sidecars) + if alphaAPIEnabled { + scriptsInit, stepContainers, sidecarContainers = convertScripts(b.Images.ShellImage, steps, taskSpec.Sidecars, taskRun.Spec.Debug) + } else { + scriptsInit, stepContainers, sidecarContainers = convertScripts(b.Images.ShellImage, steps, taskSpec.Sidecars, nil) + } if scriptsInit != nil { initContainers = append(initContainers, *scriptsInit) volumes = append(volumes, scriptsVolume) } + if alphaAPIEnabled && taskRun.Spec.Debug != nil { + volumes = append(volumes, debugScriptsVolume, debugInfoVolume) + } + // Initialize any workingDirs under /workspace. if workingDirInit := workingDirInit(b.Images.ShellImage, stepContainers); workingDirInit != nil { initContainers = append(initContainers, *workingDirInit) @@ -149,7 +162,11 @@ func (b *Builder) Build(ctx context.Context, taskRun *v1beta1.TaskRun, taskSpec // Rewrite steps with entrypoint binary. Append the entrypoint init // container to place the entrypoint binary. Also add timeout flags // to entrypoint binary. - entrypointInit, stepContainers, err := orderContainers(b.Images.EntrypointImage, credEntrypointArgs, stepContainers, &taskSpec) + if alphaAPIEnabled { + entrypointInit, stepContainers, err = orderContainers(b.Images.EntrypointImage, credEntrypointArgs, stepContainers, &taskSpec, taskRun.Spec.Debug) + } else { + entrypointInit, stepContainers, err = orderContainers(b.Images.EntrypointImage, credEntrypointArgs, stepContainers, &taskSpec, nil) + } if err != nil { return nil, err } @@ -177,7 +194,6 @@ func (b *Builder) Build(ctx context.Context, taskRun *v1beta1.TaskRun, taskSpec } // Add env var if hermetic execution was requested & if the alpha API is enabled - alphaAPIEnabled := config.FromContextOrDefaults(ctx).FeatureFlags.EnableAPIFields == config.AlphaAPIFields if taskRun.Annotations[ExecutionModeAnnotation] == ExecutionModeHermetic && alphaAPIEnabled { for i, s := range stepContainers { // Add it at the end so it overrides diff --git a/pkg/pod/pod_test.go b/pkg/pod/pod_test.go index 83773cc7f2b..16147e44518 100644 --- a/pkg/pod/pod_test.go +++ b/pkg/pod/pod_test.go @@ -126,6 +126,51 @@ func TestPodBuild(t *testing.T) { VolumeSource: corev1.VolumeSource{EmptyDir: &corev1.EmptyDirVolumeSource{Medium: corev1.StorageMediumMemory}}, }), }, + }, { + desc: "simple with breakpoint onFailure enabled, alpha api fields disabled", + trs: v1beta1.TaskRunSpec{ + Debug: &v1beta1.TaskRunDebug{ + Breakpoint: []string{BreakpointOnFailure}, + }, + }, + ts: v1beta1.TaskSpec{ + Steps: []v1beta1.Step{{Container: corev1.Container{ + Name: "name", + Image: "image", + Command: []string{"cmd"}, // avoid entrypoint lookup. + }}}, + }, + want: &corev1.PodSpec{ + RestartPolicy: corev1.RestartPolicyNever, + InitContainers: []corev1.Container{placeToolsInit}, + Containers: []corev1.Container{{ + Name: "step-name", + Image: "image", + Command: []string{"/tekton/tools/entrypoint"}, + Args: []string{ + "-wait_file", + "/tekton/downward/ready", + "-wait_file_content", + "-post_file", + "/tekton/tools/0", + "-termination_path", + "/tekton/termination", + "-entrypoint", + "cmd", + "--", + }, + VolumeMounts: append([]corev1.VolumeMount{toolsMount, downwardMount, { + Name: "tekton-creds-init-home-0", + MountPath: "/tekton/creds", + }}, implicitVolumeMounts...), + Resources: corev1.ResourceRequirements{Requests: allZeroQty()}, + TerminationMessagePath: "/tekton/termination", + }}, + Volumes: append(implicitVolumes, toolsVolume, downwardVolume, corev1.Volume{ + Name: "tekton-creds-init-home-0", + VolumeSource: corev1.VolumeSource{EmptyDir: &corev1.EmptyDirVolumeSource{Medium: corev1.StorageMediumMemory}}, + }), + }, }, { desc: "simple with running-in-environment-with-injected-sidecar set to false", ts: v1beta1.TaskSpec{ @@ -1472,6 +1517,168 @@ _EOF_ } } +func TestPodBuildwithAlphaAPIEnabled(t *testing.T) { + placeToolsInit := corev1.Container{ + Name: "place-tools", + Image: images.EntrypointImage, + WorkingDir: "/", + Command: []string{"/ko-app/entrypoint", "cp", "/ko-app/entrypoint", "/tekton/tools/entrypoint"}, + VolumeMounts: []corev1.VolumeMount{toolsMount}, + } + + for _, c := range []struct { + desc string + trs v1beta1.TaskRunSpec + trAnnotation map[string]string + ts v1beta1.TaskSpec + featureFlags map[string]string + overrideHomeEnv *bool + want *corev1.PodSpec + wantAnnotations map[string]string + }{{ + desc: "simple with debug breakpoint onFailure", + trs: v1beta1.TaskRunSpec{ + Debug: &v1beta1.TaskRunDebug{ + Breakpoint: []string{BreakpointOnFailure}, + }, + }, + ts: v1beta1.TaskSpec{ + Steps: []v1beta1.Step{{Container: corev1.Container{ + Name: "name", + Image: "image", + Command: []string{"cmd"}, // avoid entrypoint lookup. + }}}, + }, + want: &corev1.PodSpec{ + RestartPolicy: corev1.RestartPolicyNever, + InitContainers: []corev1.Container{placeToolsInit}, + Containers: []corev1.Container{{ + Name: "step-name", + Image: "image", + Command: []string{"/tekton/tools/entrypoint"}, + Args: []string{ + "-wait_file", + "/tekton/downward/ready", + "-wait_file_content", + "-post_file", + "/tekton/tools/0", + "-termination_path", + "/tekton/termination", + "-breakpoint_on_failure", + "-entrypoint", + "cmd", + "--", + }, + VolumeMounts: append([]corev1.VolumeMount{toolsMount, downwardMount, { + Name: "tekton-creds-init-home-0", + MountPath: "/tekton/creds", + }}, implicitVolumeMounts...), + Resources: corev1.ResourceRequirements{Requests: allZeroQty()}, + TerminationMessagePath: "/tekton/termination", + }}, + Volumes: append(implicitVolumes, debugScriptsVolume, debugInfoVolume, toolsVolume, downwardVolume, corev1.Volume{ + Name: "tekton-creds-init-home-0", + VolumeSource: corev1.VolumeSource{EmptyDir: &corev1.EmptyDirVolumeSource{Medium: corev1.StorageMediumMemory}}, + }), + }, + }} { + t.Run(c.desc, func(t *testing.T) { + names.TestingSeed() + store := config.NewStore(logtesting.TestLogger(t)) + store.OnConfigChanged( + &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{Name: config.GetFeatureFlagsConfigName(), Namespace: system.Namespace()}, + Data: c.featureFlags, + }, + ) + kubeclient := fakek8s.NewSimpleClientset( + &corev1.ServiceAccount{ObjectMeta: metav1.ObjectMeta{Name: "default", Namespace: "default"}}, + &corev1.ServiceAccount{ObjectMeta: metav1.ObjectMeta{Name: "service-account", Namespace: "default"}, + Secrets: []corev1.ObjectReference{{ + Name: "multi-creds", + }}, + }, + &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "multi-creds", + Namespace: "default", + Annotations: map[string]string{ + "tekton.dev/docker-0": "https://us.gcr.io", + "tekton.dev/docker-1": "https://docker.io", + "tekton.dev/git-0": "github.com", + "tekton.dev/git-1": "gitlab.com", + }}, + Type: "kubernetes.io/basic-auth", + Data: map[string][]byte{ + "username": []byte("foo"), + "password": []byte("BestEver"), + }, + }, + ) + var trAnnotations map[string]string + if c.trAnnotation == nil { + trAnnotations = map[string]string{ + ReleaseAnnotation: version.PipelineVersion, + } + } else { + trAnnotations = c.trAnnotation + trAnnotations[ReleaseAnnotation] = version.PipelineVersion + } + tr := &v1beta1.TaskRun{ + ObjectMeta: metav1.ObjectMeta{ + Name: "taskrun-name", + Namespace: "default", + Annotations: trAnnotations, + }, + Spec: c.trs, + } + + // No entrypoints should be looked up. + entrypointCache := fakeCache{} + + overrideHomeEnv := false + if s, ok := c.featureFlags[featureFlagDisableHomeEnvKey]; ok { + var err error = nil + if overrideHomeEnv, err = strconv.ParseBool(s); err != nil { + t.Fatalf("error parsing bool from %s feature flag: %v", featureFlagDisableHomeEnvKey, err) + } + } + builder := Builder{ + Images: images, + KubeClient: kubeclient, + EntrypointCache: entrypointCache, + OverrideHomeEnv: overrideHomeEnv, + } + + featureFlags, _ := config.NewFeatureFlagsFromMap(map[string]string{ + "enable-api-fields": "alpha", + }) + cfg := &config.Config{ + FeatureFlags: featureFlags, + } + ctx := config.ToContext(context.Background(), cfg) + got, err := builder.Build(ctx, tr, c.ts) + if err != nil { + t.Fatalf("builder.Build: %v", err) + } + + if !strings.HasPrefix(got.Name, "taskrun-name-pod-") { + t.Errorf("Pod name %q should have prefix 'taskrun-name-pod-'", got.Name) + } + + if d := cmp.Diff(c.want, &got.Spec, resourceQuantityCmp); d != "" { + t.Errorf("Diff %s", diff.PrintWantGot(d)) + } + + if c.wantAnnotations != nil { + if d := cmp.Diff(c.wantAnnotations, got.ObjectMeta.Annotations, cmpopts.IgnoreMapEntries(ignoreReleaseAnnotation)); d != "" { + t.Errorf("Annotation Diff(-want, +got):\n%s", d) + } + } + }) + } +} + func TestMakeLabels(t *testing.T) { taskRunName := "task-run-name" want := map[string]string{ diff --git a/pkg/pod/script.go b/pkg/pod/script.go index 67f3719a1b6..ec1f3881e58 100644 --- a/pkg/pod/script.go +++ b/pkg/pod/script.go @@ -29,9 +29,13 @@ import ( ) const ( - scriptsVolumeName = "tekton-internal-scripts" - scriptsDir = "/tekton/scripts" - defaultScriptPreamble = "#!/bin/sh\nset -xe\n" + scriptsVolumeName = "tekton-internal-scripts" + debugScriptsVolumeName = "tekton-internal-debug-scripts" + debugInfoVolumeName = "tekton-internal-debug-info" + scriptsDir = "/tekton/scripts" + debugScriptsDir = "/tekton/debug/scripts" + defaultScriptPreamble = "#!/bin/sh\nset -xe\n" + debugInfoDir = "/tekton/debug/info" ) var ( @@ -45,6 +49,18 @@ var ( Name: scriptsVolumeName, MountPath: scriptsDir, } + debugScriptsVolume = corev1.Volume{ + Name: debugScriptsVolumeName, + VolumeSource: corev1.VolumeSource{EmptyDir: &corev1.EmptyDirVolumeSource{}}, + } + debugScriptsVolumeMount = corev1.VolumeMount{ + Name: debugScriptsVolumeName, + MountPath: debugScriptsDir, + } + debugInfoVolume = corev1.Volume{ + Name: debugInfoVolumeName, + VolumeSource: corev1.VolumeSource{EmptyDir: &corev1.EmptyDirVolumeSource{}}, + } ) // convertScripts converts any steps and sidecars that specify a Script field into a normal Container. @@ -52,8 +68,11 @@ var ( // It does this by prepending a container that writes specified Script bodies // to executable files in a shared volumeMount, then produces Containers that // simply run those executable files. -func convertScripts(shellImage string, steps []v1beta1.Step, sidecars []v1beta1.Sidecar) (*corev1.Container, []corev1.Container, []corev1.Container) { +func convertScripts(shellImage string, steps []v1beta1.Step, sidecars []v1beta1.Sidecar, debugConfig *v1beta1.TaskRunDebug) (*corev1.Container, []corev1.Container, []corev1.Container) { placeScripts := false + // Place scripts is an init container used for creating scripts in the + // /tekton/scripts directory which would be later used by the step containers + // as a Command placeScriptsInit := corev1.Container{ Name: "place-scripts", Image: shellImage, @@ -62,8 +81,7 @@ func convertScripts(shellImage string, steps []v1beta1.Step, sidecars []v1beta1. VolumeMounts: []corev1.VolumeMount{scriptsVolumeMount, toolsMount}, } - convertedStepContainers := convertListOfSteps(steps, &placeScriptsInit, &placeScripts, "script") - + breakpoints := []string{} sideCarSteps := []v1beta1.Step{} for _, step := range sidecars { sidecarStep := v1beta1.Step{ @@ -73,8 +91,17 @@ func convertScripts(shellImage string, steps []v1beta1.Step, sidecars []v1beta1. } sideCarSteps = append(sideCarSteps, sidecarStep) } - sidecarContainers := convertListOfSteps(sideCarSteps, &placeScriptsInit, &placeScripts, "sidecar-script") + // Add mounts for debug + if debugConfig != nil && len(debugConfig.Breakpoint) > 0 { + breakpoints = debugConfig.Breakpoint + placeScriptsInit.VolumeMounts = append(placeScriptsInit.VolumeMounts, debugScriptsVolumeMount) + } + + convertedStepContainers := convertListOfSteps(steps, &placeScriptsInit, &placeScripts, breakpoints, "script") + + // Pass empty breakpoint list in "sidecar step to container" converter to not rewrite the scripts and add breakpoints to sidecar + sidecarContainers := convertListOfSteps(sideCarSteps, &placeScriptsInit, &placeScripts, []string{}, "sidecar-script") if placeScripts { return &placeScriptsInit, convertedStepContainers, sidecarContainers } @@ -85,7 +112,7 @@ func convertScripts(shellImage string, steps []v1beta1.Step, sidecars []v1beta1. // // It iterates through the list of steps (or sidecars), generates the script file name and heredoc termination string, // adds an entry to the init container args, sets up the step container to run the script, and sets the volume mounts. -func convertListOfSteps(steps []v1beta1.Step, initContainer *corev1.Container, placeScripts *bool, namePrefix string) []corev1.Container { +func convertListOfSteps(steps []v1beta1.Step, initContainer *corev1.Container, placeScripts *bool, breakpoints []string, namePrefix string) []corev1.Container { containers := []corev1.Container{} for i, s := range steps { if s.Script == "" { @@ -129,8 +156,47 @@ cat > ${scriptfile} << '%s' // we'll clear out the Args and overwrite Command. steps[i].Command = []string{scriptFile} steps[i].VolumeMounts = append(steps[i].VolumeMounts, scriptsVolumeMount) + + // Add debug mounts if breakpoints are present + if len(breakpoints) > 0 { + debugInfoVolumeMount := corev1.VolumeMount{ + Name: debugInfoVolumeName, + MountPath: filepath.Join(debugInfoDir, fmt.Sprintf("%d", i)), + } + steps[i].VolumeMounts = append(steps[i].VolumeMounts, debugScriptsVolumeMount, debugInfoVolumeMount) + } containers = append(containers, steps[i].Container) } + + // Place debug scripts if breakpoints are enabled + if len(breakpoints) > 0 { + // Debug script names + debugContinueScriptName := "continue" + debugFailContinueScriptName := "fail-continue" + + // debugContinueScript can be used by the user to mark the failing step as a success + debugContinueScript := defaultScriptPreamble + fmt.Sprintf(debugContinueScriptTemplate, len(steps), debugInfoDir, mountPoint) + + // debugFailContinueScript can be used by the user to mark the failing step as a failure + debugFailContinueScript := defaultScriptPreamble + fmt.Sprintf(debugFailScriptTemplate, len(steps), debugInfoDir, mountPoint) + + // Script names and their content + debugScripts := map[string]string{ + debugContinueScriptName: debugContinueScript, + debugFailContinueScriptName: debugFailContinueScript, + } + + // 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 + for debugScriptName, debugScript := range debugScripts { + tmpFile := filepath.Join(debugScriptsDir, fmt.Sprintf("%s-%s", "debug", debugScriptName)) + heredoc := names.SimpleNameGenerator.RestrictLengthWithRandomSuffix(fmt.Sprintf("%s-%s-heredoc-randomly-generated", "debug", debugScriptName)) + + initContainer.Args[1] += fmt.Sprintf(initScriptDirective, tmpFile, heredoc, debugScript, heredoc) + } + + } + return containers } diff --git a/pkg/pod/script_test.go b/pkg/pod/script_test.go index 4eaa723ec6e..0fc7ebf6d7a 100644 --- a/pkg/pod/script_test.go +++ b/pkg/pod/script_test.go @@ -35,7 +35,7 @@ func TestConvertScripts_NothingToConvert_EmptySidecars(t *testing.T) { Container: corev1.Container{ Image: "step-2", }, - }}, []v1beta1.Sidecar{}) + }}, []v1beta1.Sidecar{}, nil) want := []corev1.Container{{ Image: "step-1", }, { @@ -62,7 +62,7 @@ func TestConvertScripts_NothingToConvert_NilSidecars(t *testing.T) { Container: corev1.Container{ Image: "step-2", }, - }}, nil) + }}, nil, nil) want := []corev1.Container{{ Image: "step-1", }, { @@ -93,7 +93,7 @@ func TestConvertScripts_NothingToConvert_WithSidecar(t *testing.T) { Container: corev1.Container{ Image: "sidecar-1", }, - }}) + }}, nil) want := []corev1.Container{{ Image: "step-1", }, { @@ -153,7 +153,7 @@ script-3`, VolumeMounts: preExistingVolumeMounts, Args: []string{"my", "args"}, }, - }}, []v1beta1.Sidecar{}) + }}, []v1beta1.Sidecar{}, nil) wantInit := &corev1.Container{ Name: "place-scripts", Image: images.ShellImage, @@ -212,7 +212,7 @@ _EOF_ } } -func TestConvertScripts_WithSidecar(t *testing.T) { +func TestConvertScripts_WithBreakpoint_OnFailure(t *testing.T) { names.TestingSeed() preExistingVolumeMounts := []corev1.VolumeMount{{ @@ -230,6 +230,148 @@ script-1`, }, { // No script to convert here. Container: corev1.Container{Image: "step-2"}, + }, { + Script: ` +#!/bin/sh +script-3`, + Container: corev1.Container{ + Image: "step-3", + VolumeMounts: preExistingVolumeMounts, + Args: []string{"my", "args"}, + }, + }, { + Script: `no-shebang`, + Container: corev1.Container{ + Image: "step-3", + VolumeMounts: preExistingVolumeMounts, + Args: []string{"my", "args"}, + }, + }}, []v1beta1.Sidecar{}, &v1beta1.TaskRunDebug{ + Breakpoint: []string{BreakpointOnFailure}, + }) + 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/tools/entrypoint decode-script "${scriptfile}" +scriptfile="/tekton/scripts/script-2-mz4c7" +touch ${scriptfile} && chmod +x ${scriptfile} +cat > ${scriptfile} << '_EOF_' +CiMhL2Jpbi9zaApzY3JpcHQtMw== +_EOF_ +/tekton/tools/entrypoint decode-script "${scriptfile}" +scriptfile="/tekton/scripts/script-3-mssqb" +touch ${scriptfile} && chmod +x ${scriptfile} +cat > ${scriptfile} << '_EOF_' +IyEvYmluL3NoCnNldCAteGUKbm8tc2hlYmFuZw== +_EOF_ +/tekton/tools/entrypoint decode-script "${scriptfile}" +tmpfile="/tekton/debug/scripts/debug-continue" +touch ${tmpfile} && chmod +x ${tmpfile} +cat > ${tmpfile} << 'debug-continue-heredoc-randomly-generated-78c5n' +#!/bin/sh +set -xe + +numberOfSteps=4 +debugInfo=/tekton/debug/info +tektonTools=/tekton/tools + +postFile="$(ls ${debugInfo} | grep -E '[0-9]+' | tail -1)" +stepNumber="$(echo ${postFile} | sed 's/[^0-9]*//g')" + +if [ $stepNumber -lt $numberOfSteps ]; then + touch ${tektonTools}/${stepNumber} # Mark step as success + echo "0" > ${tektonTools}/${stepNumber}.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-78c5n +tmpfile="/tekton/debug/scripts/debug-fail-continue" +touch ${tmpfile} && chmod +x ${tmpfile} +cat > ${tmpfile} << 'debug-fail-continue-heredoc-randomly-generated-6nl7g' +#!/bin/sh +set -xe + +numberOfSteps=4 +debugInfo=/tekton/debug/info +tektonTools=/tekton/tools + +postFile="$(ls ${debugInfo} | grep -E '[0-9]+' | tail -1)" +stepNumber="$(echo ${postFile} | sed 's/[^0-9]*//g')" + +if [ $stepNumber -lt $numberOfSteps ]; then + touch ${tektonTools}/${stepNumber}.err # Mark step as a failure + echo "1" > ${tektonTools}/${stepNumber}.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-6nl7g +`}, + VolumeMounts: []corev1.VolumeMount{scriptsVolumeMount, toolsMount, debugScriptsVolumeMount}, + } + 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", + }, { + 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"}, + }, + }} + if d := cmp.Diff(wantInit, gotInit); d != "" { + t.Errorf("Init Container Diff %s", diff.PrintWantGot(d)) + } + if d := cmp.Diff(want, gotSteps); d != "" { + t.Errorf("Containers Diff %s", diff.PrintWantGot(d)) + } + + if len(gotSidecars) != 0 { + t.Errorf("Expected zero sidecars, got %v", len(gotSidecars)) + } +} + +func TestConvertScripts_WithSidecar(t *testing.T) { + names.TestingSeed() + + preExistingVolumeMounts := []corev1.VolumeMount{{ + Name: "pre-existing-volume-mount", + MountPath: "/mount/path", + }, { + Name: "another-one", + MountPath: "/another/one", + }} + + gotInit, gotSteps, gotSidecars := convertScripts(images.ShellImage, []v1beta1.Step{{ + Script: `#!/bin/sh +script-1`, + Container: corev1.Container{Image: "step-1"}, + }, { + // No script to convert here.: + Container: corev1.Container{Image: "step-2"}, }, { Script: `#!/bin/sh script-3`, @@ -242,7 +384,7 @@ script-3`, Script: `#!/bin/sh sidecar-1`, Container: corev1.Container{Image: "sidecar-1"}, - }}) + }}, nil) wantInit := &corev1.Container{ Name: "place-scripts", Image: images.ShellImage, diff --git a/pkg/pod/scripts_constants.go b/pkg/pod/scripts_constants.go new file mode 100644 index 00000000000..c9c0069ee58 --- /dev/null +++ b/pkg/pod/scripts_constants.go @@ -0,0 +1,43 @@ +package pod + +// TODO(#3972): Use text/template templating instead of %s based templating +const ( + debugContinueScriptTemplate = ` +numberOfSteps=%d +debugInfo=%s +tektonTools=%s + +postFile="$(ls ${debugInfo} | grep -E '[0-9]+' | tail -1)" +stepNumber="$(echo ${postFile} | sed 's/[^0-9]*//g')" + +if [ $stepNumber -lt $numberOfSteps ]; then + touch ${tektonTools}/${stepNumber} # Mark step as success + echo "0" > ${tektonTools}/${stepNumber}.breakpointexit + echo "Executing step $stepNumber..." +else + echo "Last step (no. $stepNumber) has already been executed, breakpoint exiting !" + exit 0 +fi` + debugFailScriptTemplate = ` +numberOfSteps=%d +debugInfo=%s +tektonTools=%s + +postFile="$(ls ${debugInfo} | grep -E '[0-9]+' | tail -1)" +stepNumber="$(echo ${postFile} | sed 's/[^0-9]*//g')" + +if [ $stepNumber -lt $numberOfSteps ]; then + touch ${tektonTools}/${stepNumber}.err # Mark step as a failure + echo "1" > ${tektonTools}/${stepNumber}.breakpointexit + echo "Executing step $stepNumber..." +else + echo "Last step (no. $stepNumber) has already been executed, breakpoint exiting !" + exit 0 +fi` + initScriptDirective = `tmpfile="%s" +touch ${tmpfile} && chmod +x ${tmpfile} +cat > ${tmpfile} << '%s' +%s +%s +` +)