diff --git a/cmd/entrypoint/main.go b/cmd/entrypoint/main.go index c8437a8a4e4..1b3256606da 100644 --- a/cmd/entrypoint/main.go +++ b/cmd/entrypoint/main.go @@ -41,6 +41,7 @@ var ( 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") waitPollingInterval = time.Second + timeout = flag.Duration("timeout", time.Duration(0), "If specified, sets timeout for step") ) func cp(src, dst string) error { @@ -103,6 +104,7 @@ func main() { Runner: &realRunner{}, PostWriter: &realPostWriter{}, Results: strings.Split(*results, ","), + Timeout: timeout, } // Copy any creds injected by the controller into the $HOME directory of the current diff --git a/cmd/entrypoint/runner.go b/cmd/entrypoint/runner.go index a8082dfa533..6350a33e445 100644 --- a/cmd/entrypoint/runner.go +++ b/cmd/entrypoint/runner.go @@ -1,6 +1,7 @@ package main import ( + "context" "os" "os/exec" "os/signal" @@ -19,7 +20,7 @@ type realRunner struct { var _ entrypoint.Runner = (*realRunner)(nil) -func (rr *realRunner) Run(args ...string) error { +func (rr *realRunner) Run(ctx context.Context, args ...string) error { if len(args) == 0 { return nil } @@ -33,7 +34,7 @@ func (rr *realRunner) Run(args ...string) error { signal.Notify(rr.signals) defer signal.Reset() - cmd := exec.Command(name, args...) + cmd := exec.CommandContext(ctx, name, args...) cmd.Stdout = os.Stdout cmd.Stderr = os.Stderr // dedicated PID group used to forward signals to @@ -42,6 +43,9 @@ func (rr *realRunner) Run(args ...string) error { // Start defined command if err := cmd.Start(); err != nil { + if ctx.Err() == context.DeadlineExceeded { + return context.DeadlineExceeded + } return err } @@ -57,6 +61,9 @@ func (rr *realRunner) Run(args ...string) error { // Wait for command to exit if err := cmd.Wait(); err != nil { + if ctx.Err() == context.DeadlineExceeded { + return context.DeadlineExceeded + } return err } diff --git a/cmd/entrypoint/runner_test.go b/cmd/entrypoint/runner_test.go index 399ab0d9ec6..27d4674b6c4 100644 --- a/cmd/entrypoint/runner_test.go +++ b/cmd/entrypoint/runner_test.go @@ -1,9 +1,11 @@ package main import ( + "context" "os" "syscall" "testing" + "time" ) // TestRealRunnerSignalForwarding will artificially put an interrupt signal (SIGINT) in the rr.signals chan. @@ -14,9 +16,24 @@ func TestRealRunnerSignalForwarding(t *testing.T) { rr := realRunner{} rr.signals = make(chan os.Signal, 1) rr.signals <- syscall.SIGINT - if err := rr.Run("sleep", "3600"); err.Error() == "signal: interrupt" { + if err := rr.Run(context.Background(), "sleep", "3600"); err.Error() == "signal: interrupt" { t.Logf("SIGINT forwarded to Entrypoint") } else { t.Fatalf("Unexpected error received: %v", err) } } + +// TestRealRunnerTimeout tests whether cmd is killed after a millisecond even though it's supposed to sleep for 10 milliseconds. +func TestRealRunnerTimeout(t *testing.T) { + rr := realRunner{} + timeout := time.Millisecond + ctx, cancel := context.WithTimeout(context.Background(), timeout) + defer cancel() + if err := rr.Run(ctx, "sleep", "0.01"); err != nil { + if err != context.DeadlineExceeded { + t.Fatalf("unexpected error received: %v", err) + } + } else { + t.Fatalf("step didn't timeout") + } +} diff --git a/docs/tasks.md b/docs/tasks.md index 17d74264b86..a48667b5e4d 100644 --- a/docs/tasks.md +++ b/docs/tasks.md @@ -12,6 +12,7 @@ weight: 1 - [Defining `Steps`](#defining-steps) - [Reserved directories](#reserved-directories) - [Running scripts within `Steps`](#running-scripts-within-steps) + - [Specifying a timeout](#specifying-a-timeout) - [Specifying `Parameters`](#specifying-parameters) - [Specifying `Resources`](#specifying-resources) - [Specifying `Workspaces`](#specifying-workspaces) @@ -241,7 +242,27 @@ steps: #!/usr/bin/env bash /bin/my-binary ``` +#### Specifying a timeout +A `Step` can specify a `timeout` field. +If the `Step` execution time exceeds the specified timeout, the `Step` kills +its running process and any subsequent `Steps` in the `TaskRun` will not be +executed. The `TaskRun` is placed into a `Failed` condition. An accompanying log +describing which `Step` timed out is written as the `Failed` condition's message. + +The timeout specification follows the duration format as specified in the [Go time package](https://golang.org/pkg/time/#ParseDuration) (e.g. 1s or 1ms). + +The example `Step` below is supposed to sleep for 60 seconds but will be canceled by the specified 5 second timeout. +```yaml +steps: + - name: sleep-then-timeout + image: ubuntu + script: | + #!/usr/bin/env bash + echo "I am supposed to sleep for 60 seconds!" + sleep 60 + timeout: 5s +``` ### Specifying `Parameters` You can specify parameters, such as compilation flags or artifact names, that you want to supply to the `Task` at execution time. diff --git a/examples/v1beta1/taskruns/workspace-in-sidecar.yaml b/examples/v1beta1/taskruns/workspace-in-sidecar.yaml index 026fde1ea81..3c758107bd6 100644 --- a/examples/v1beta1/taskruns/workspace-in-sidecar.yaml +++ b/examples/v1beta1/taskruns/workspace-in-sidecar.yaml @@ -10,7 +10,7 @@ apiVersion: tekton.dev/v1beta1 metadata: generateName: workspace-in-sidecar- spec: - timeout: 30s + timeout: 60s workspaces: - name: signals emptyDir: {} diff --git a/pkg/apis/pipeline/v1beta1/task_types.go b/pkg/apis/pipeline/v1beta1/task_types.go index 405527f8b32..6899a62980c 100644 --- a/pkg/apis/pipeline/v1beta1/task_types.go +++ b/pkg/apis/pipeline/v1beta1/task_types.go @@ -125,10 +125,12 @@ type Step struct { // // If Script is not empty, the Step cannot have an Command or Args. Script string `json:"script,omitempty"` + // Timeout is the time after which the step times out. Defaults to never. + // Refer to Go's ParseDuration documentation for expected format: https://golang.org/pkg/time/#ParseDuration + Timeout *metav1.Duration `json:"timeout,omitempty"` } -// Sidecar embeds the Container type, which allows it to include fields not -// provided by Container. +// Sidecar has nearly the same data structure as Step, consisting of a Container and an optional Script, but does not have the ability to timeout. type Sidecar struct { corev1.Container `json:",inline"` diff --git a/pkg/apis/pipeline/v1beta1/task_validation.go b/pkg/apis/pipeline/v1beta1/task_validation.go index 9615129e6a1..434345cf290 100644 --- a/pkg/apis/pipeline/v1beta1/task_validation.go +++ b/pkg/apis/pipeline/v1beta1/task_validation.go @@ -21,6 +21,7 @@ import ( "fmt" "path/filepath" "strings" + "time" "github.com/tektoncd/pipeline/pkg/apis/validate" "github.com/tektoncd/pipeline/pkg/substitution" @@ -159,6 +160,12 @@ func validateStep(s Step, names sets.String) (errs *apis.FieldError) { names.Insert(s.Name) } + if s.Timeout != nil { + if s.Timeout.Duration < time.Duration(0) { + return apis.ErrInvalidValue(s.Timeout.Duration, "negative timeout") + } + } + for j, vm := range s.VolumeMounts { if strings.HasPrefix(vm.MountPath, "/tekton/") && !strings.HasPrefix(vm.MountPath, "/tekton/home") { diff --git a/pkg/apis/pipeline/v1beta1/task_validation_test.go b/pkg/apis/pipeline/v1beta1/task_validation_test.go index 6cbd1488f32..562d4596092 100644 --- a/pkg/apis/pipeline/v1beta1/task_validation_test.go +++ b/pkg/apis/pipeline/v1beta1/task_validation_test.go @@ -19,12 +19,14 @@ package v1beta1_test import ( "context" "testing" + "time" "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1" "github.com/tektoncd/pipeline/test/diff" corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "knative.dev/pkg/apis" ) @@ -930,6 +932,17 @@ func TestTaskSpecValidateError(t *testing.T) { Message: `non-existent variable in "\n\t\t\t\t#!/usr/bin/env bash\n\t\t\t\thello \"$(context.task.missing)\""`, Paths: []string{"steps[0].script"}, }, + }, { + name: "negative timeout string", + fields: fields{ + Steps: []v1beta1.Step{{ + Timeout: &metav1.Duration{Duration: -10 * time.Second}, + }}, + }, + expectedError: apis.FieldError{ + Message: "invalid value: -10s", + Paths: []string{"steps[0].negative timeout"}, + }, }} for _, tt := range tests { t.Run(tt.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 9b7f057dc54..f2c14543989 100644 --- a/pkg/apis/pipeline/v1beta1/zz_generated.deepcopy.go +++ b/pkg/apis/pipeline/v1beta1/zz_generated.deepcopy.go @@ -1205,6 +1205,11 @@ func (in *SkippedTask) DeepCopy() *SkippedTask { func (in *Step) DeepCopyInto(out *Step) { *out = *in in.Container.DeepCopyInto(&out.Container) + if in.Timeout != nil { + in, out := &in.Timeout, &out.Timeout + *out = new(metav1.Duration) + **out = **in + } return } diff --git a/pkg/entrypoint/entrypointer.go b/pkg/entrypoint/entrypointer.go index 6c9ed4edbc4..db582f88e2b 100644 --- a/pkg/entrypoint/entrypointer.go +++ b/pkg/entrypoint/entrypointer.go @@ -17,6 +17,7 @@ limitations under the License. package entrypoint import ( + "context" "fmt" "io/ioutil" "os" @@ -63,6 +64,8 @@ type Entrypointer struct { // Results is the set of files that might contain task results Results []string + // Timeout is an optional user-specified duration within which the Step must complete + Timeout *time.Duration } // Waiter encapsulates waiting for files to exist. @@ -73,7 +76,7 @@ type Waiter interface { // Runner encapsulates running commands. type Runner interface { - Run(args ...string) error + Run(ctx context.Context, args ...string) error } // PostWriter encapsulates writing a file when complete. @@ -106,7 +109,6 @@ func (e Entrypointer) Go() error { Value: time.Now().Format(timeFormat), ResultType: v1beta1.InternalTektonResultType, }) - return err } } @@ -114,13 +116,34 @@ func (e Entrypointer) Go() error { if e.Entrypoint != "" { e.Args = append([]string{e.Entrypoint}, e.Args...) } + output = append(output, v1beta1.PipelineResourceResult{ Key: "StartedAt", Value: time.Now().Format(timeFormat), ResultType: v1beta1.InternalTektonResultType, }) - err := e.Runner.Run(e.Args...) + var err error + if e.Timeout != nil && *e.Timeout < time.Duration(0) { + err = fmt.Errorf("negative timeout specified") + } + + if err == nil { + ctx := context.Background() + var cancel context.CancelFunc + if e.Timeout != nil && *e.Timeout != time.Duration(0) { + ctx, cancel = context.WithTimeout(ctx, *e.Timeout) + defer cancel() + } + err = e.Runner.Run(ctx, e.Args...) + if err == context.DeadlineExceeded { + output = append(output, v1beta1.PipelineResourceResult{ + Key: "Reason", + Value: "TimeoutExceeded", + ResultType: v1beta1.InternalTektonResultType, + }) + } + } // Write the post file *no matter what* e.WritePostFile(e.PostFile, err) diff --git a/pkg/entrypoint/entrypointer_test.go b/pkg/entrypoint/entrypointer_test.go index bff9821a4bb..50bf1dd24fb 100644 --- a/pkg/entrypoint/entrypointer_test.go +++ b/pkg/entrypoint/entrypointer_test.go @@ -17,12 +17,14 @@ limitations under the License. package entrypoint import ( + "context" "encoding/json" "errors" "io/ioutil" "os" "reflect" "testing" + "time" "github.com/google/go-cmp/cmp" "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1" @@ -36,26 +38,41 @@ func TestEntrypointerFailures(t *testing.T) { waiter Waiter runner Runner expectedError string + timeout time.Duration }{{ - desc: "failing runner with no postFile", - runner: &fakeErrorRunner{}, - expectedError: "runner failed", - }, { desc: "failing runner with postFile", runner: &fakeErrorRunner{}, expectedError: "runner failed", postFile: "foo", + timeout: time.Duration(0), }, { desc: "failing waiter with no postFile", waitFiles: []string{"foo"}, waiter: &fakeErrorWaiter{}, expectedError: "waiter failed", + timeout: time.Duration(0), }, { desc: "failing waiter with postFile", waitFiles: []string{"foo"}, waiter: &fakeErrorWaiter{}, expectedError: "waiter failed", postFile: "bar", + timeout: time.Duration(0), + }, { + desc: "negative timeout", + runner: &fakeErrorRunner{}, + timeout: -10 * time.Second, + expectedError: `negative timeout specified`, + }, { + desc: "zero timeout string does not time out", + runner: &fakeZeroTimeoutRunner{}, + timeout: time.Duration(0), + expectedError: `runner failed`, + }, { + desc: "timeout leads to runner", + runner: &fakeTimeoutRunner{}, + timeout: 1 * time.Millisecond, + expectedError: `runner failed`, }} { t.Run(c.desc, func(t *testing.T) { fw := c.waiter @@ -76,6 +93,7 @@ func TestEntrypointerFailures(t *testing.T) { Runner: fr, PostWriter: fpw, TerminationPath: "termination", + Timeout: &c.timeout, }.Go() if err == nil { t.Fatalf("Entrypointer didn't fail") @@ -130,6 +148,7 @@ func TestEntrypointer(t *testing.T) { }} { 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, @@ -139,6 +158,7 @@ func TestEntrypointer(t *testing.T) { Runner: fr, PostWriter: fpw, TerminationPath: "termination", + Timeout: &timeout, }.Go() if err != nil { t.Fatalf("Entrypointer failed: %v", err) @@ -214,7 +234,7 @@ func (f *fakeWaiter) Wait(file string, _ bool) error { type fakeRunner struct{ args *[]string } -func (f *fakeRunner) Run(args ...string) error { +func (f *fakeRunner) Run(ctx context.Context, args ...string) error { f.args = &args return nil } @@ -232,7 +252,27 @@ func (f *fakeErrorWaiter) Wait(file string, expectContent bool) error { type fakeErrorRunner struct{ args *[]string } -func (f *fakeErrorRunner) Run(args ...string) error { +func (f *fakeErrorRunner) Run(ctx context.Context, args ...string) error { f.args = &args return errors.New("runner failed") } + +type fakeZeroTimeoutRunner struct{ args *[]string } + +func (f *fakeZeroTimeoutRunner) Run(ctx context.Context, args ...string) error { + f.args = &args + if _, ok := ctx.Deadline(); ok == true { + return errors.New("context deadline should not be set with a zero timeout duration") + } + return errors.New("runner failed") +} + +type fakeTimeoutRunner struct{ args *[]string } + +func (f *fakeTimeoutRunner) Run(ctx context.Context, args ...string) error { + f.args = &args + if _, ok := ctx.Deadline(); ok == false { + return errors.New("context deadline should have been set because of a timeout") + } + return errors.New("runner failed") +} diff --git a/pkg/pod/entrypoint.go b/pkg/pod/entrypoint.go index 74685a86171..4df660b0d3e 100644 --- a/pkg/pod/entrypoint.go +++ b/pkg/pod/entrypoint.go @@ -85,9 +85,8 @@ var ( // Containers must have Command specified; if the user didn't specify a // command, we must have fetched the image's ENTRYPOINT before calling this // method, using entrypoint_lookup.go. -// -// TODO(#1605): Also use entrypoint injection to order sidecar start/stop. -func orderContainers(entrypointImage string, extraEntrypointArgs []string, steps []corev1.Container, results []v1beta1.TaskResult) (corev1.Container, []corev1.Container, error) { +// 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) { initContainer := corev1.Container{ Name: "place-tools", Image: entrypointImage, @@ -121,8 +120,13 @@ func orderContainers(entrypointImage string, extraEntrypointArgs []string, steps "-termination_path", terminationPath, } } - argsForEntrypoint = append(argsForEntrypoint, extraEntrypointArgs...) - argsForEntrypoint = append(argsForEntrypoint, resultArgument(steps, results)...) + argsForEntrypoint = append(argsForEntrypoint, commonExtraEntrypointArgs...) + if taskSpec != nil { + if taskSpec.Steps != nil && len(taskSpec.Steps) >= i+1 && taskSpec.Steps[i].Timeout != nil { + argsForEntrypoint = append(argsForEntrypoint, "-timeout", taskSpec.Steps[i].Timeout.Duration.String()) + } + argsForEntrypoint = append(argsForEntrypoint, resultArgument(steps, taskSpec.Results)...) + } cmd, args := s.Command, s.Args if len(cmd) == 0 { @@ -239,6 +243,6 @@ func isContainerSidecar(name string) bool { return strings.HasPrefix(name, sidec // trimStepPrefix returns the container name, stripped of its step prefix. func trimStepPrefix(name string) string { return strings.TrimPrefix(name, stepPrefix) } -// trimSidecarPrefix returns the container name, stripped of its sidecar +// TrimSidecarPrefix returns the container name, stripped of its sidecar // prefix. func TrimSidecarPrefix(name string) string { return strings.TrimPrefix(name, sidecarPrefix) } diff --git a/pkg/pod/entrypoint_test.go b/pkg/pod/entrypoint_test.go index 4b9beccd8fb..8c83d3efa53 100644 --- a/pkg/pod/entrypoint_test.go +++ b/pkg/pod/entrypoint_test.go @@ -108,13 +108,15 @@ func TestOrderContainers(t *testing.T) { } func TestEntryPointResults(t *testing.T) { - results := []v1beta1.TaskResult{{ - Name: "sum", - Description: "This is the sum result of the task", - }, { - Name: "sub", - Description: "This is the sub result of the task", - }} + taskSpec := v1beta1.TaskSpec{ + Results: []v1beta1.TaskResult{{ + Name: "sum", + Description: "This is the sum result of the task", + }, { + Name: "sub", + Description: "This is the sub result of the task", + }}, + } steps := []corev1.Container{{ Image: "step-1", @@ -172,7 +174,7 @@ func TestEntryPointResults(t *testing.T) { VolumeMounts: []corev1.VolumeMount{toolsMount}, TerminationMessagePath: "/tekton/termination", }} - _, got, err := orderContainers(images.EntrypointImage, []string{}, steps, results) + _, got, err := orderContainers(images.EntrypointImage, []string{}, steps, &taskSpec) if err != nil { t.Fatalf("orderContainers: %v", err) } @@ -182,13 +184,15 @@ func TestEntryPointResults(t *testing.T) { } func TestEntryPointResultsSingleStep(t *testing.T) { - results := []v1beta1.TaskResult{{ - Name: "sum", - Description: "This is the sum result of the task", - }, { - Name: "sub", - Description: "This is the sub result of the task", - }} + taskSpec := v1beta1.TaskSpec{ + Results: []v1beta1.TaskResult{{ + Name: "sum", + Description: "This is the sum result of the task", + }, { + Name: "sub", + Description: "This is the sub result of the task", + }}, + } steps := []corev1.Container{{ Image: "step-1", @@ -210,7 +214,7 @@ func TestEntryPointResultsSingleStep(t *testing.T) { VolumeMounts: []corev1.VolumeMount{toolsMount, downwardMount}, TerminationMessagePath: "/tekton/termination", }} - _, got, err := orderContainers(images.EntrypointImage, []string{}, steps, results) + _, got, err := orderContainers(images.EntrypointImage, []string{}, steps, &taskSpec) if err != nil { t.Fatalf("orderContainers: %v", err) } @@ -219,10 +223,12 @@ func TestEntryPointResultsSingleStep(t *testing.T) { } } func TestEntryPointSingleResultsSingleStep(t *testing.T) { - results := []v1beta1.TaskResult{{ - Name: "sum", - Description: "This is the sum result of the task", - }} + taskSpec := v1beta1.TaskSpec{ + Results: []v1beta1.TaskResult{{ + Name: "sum", + Description: "This is the sum result of the task", + }}, + } steps := []corev1.Container{{ Image: "step-1", @@ -244,7 +250,7 @@ func TestEntryPointSingleResultsSingleStep(t *testing.T) { VolumeMounts: []corev1.VolumeMount{toolsMount, downwardMount}, TerminationMessagePath: "/tekton/termination", }} - _, got, err := orderContainers(images.EntrypointImage, []string{}, steps, results) + _, got, err := orderContainers(images.EntrypointImage, []string{}, steps, &taskSpec) if err != nil { t.Fatalf("orderContainers: %v", err) } diff --git a/pkg/pod/pod.go b/pkg/pod/pod.go index e038f9034c2..66c4601a14f 100644 --- a/pkg/pod/pod.go +++ b/pkg/pod/pod.go @@ -140,8 +140,9 @@ 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. - entrypointInit, stepContainers, err := orderContainers(b.Images.EntrypointImage, credEntrypointArgs, stepContainers, taskSpec.Results) + // container to place the entrypoint binary. Also add timeout flags + // to entrypoint binary. + entrypointInit, stepContainers, err := orderContainers(b.Images.EntrypointImage, credEntrypointArgs, stepContainers, &taskSpec) if err != nil { return nil, err } diff --git a/pkg/pod/pod_test.go b/pkg/pod/pod_test.go index f6645a0cf7f..7e45fbee61c 100644 --- a/pkg/pod/pod_test.go +++ b/pkg/pod/pod_test.go @@ -22,6 +22,7 @@ import ( "path/filepath" "strings" "testing" + "time" "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" @@ -1084,7 +1085,54 @@ script-heredoc-randomly-generated-78c5n Resources: corev1.ResourceRequirements{Requests: allZeroQty()}, TerminationMessagePath: "/tekton/termination", }}, - }}} { + }, + }, { + desc: "step-with-timeout", + ts: v1beta1.TaskSpec{ + Steps: []v1beta1.Step{{Container: corev1.Container{ + Name: "name", + Image: "image", + Command: []string{"cmd"}, // avoid entrypoint lookup. + }, + Timeout: &metav1.Duration{Duration: time.Second}, + }}, + }, + 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", + "-timeout", + "1s", + "-entrypoint", + "cmd", + "--", + }, + Env: implicitEnvVars, + VolumeMounts: append([]corev1.VolumeMount{toolsMount, downwardMount, { + Name: "tekton-creds-init-home-9l9zj", + MountPath: "/tekton/creds", + }}, implicitVolumeMounts...), + WorkingDir: pipeline.WorkspaceDir, + Resources: corev1.ResourceRequirements{Requests: allZeroQty()}, + TerminationMessagePath: "/tekton/termination", + }}, + Volumes: append(implicitVolumes, toolsVolume, downwardVolume, corev1.Volume{ + Name: "tekton-creds-init-home-9l9zj", + 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)) diff --git a/pkg/pod/script.go b/pkg/pod/script.go index ec57bb251bc..288c6875cc1 100644 --- a/pkg/pod/script.go +++ b/pkg/pod/script.go @@ -24,6 +24,7 @@ import ( "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1" "github.com/tektoncd/pipeline/pkg/names" corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) const ( @@ -61,13 +62,13 @@ func convertScripts(shellImage string, steps []v1beta1.Step, sidecars []v1beta1. } convertedStepContainers := convertListOfSteps(steps, &placeScriptsInit, &placeScripts, "script") - // convertListOfSteps operates on overlapping fields across Step and Sidecar, hence a conversion - // from Sidecar into Step + sideCarSteps := []v1beta1.Step{} for _, step := range sidecars { sidecarStep := v1beta1.Step{ - step.Container, - step.Script, + Container: step.Container, + Script: step.Script, + Timeout: &metav1.Duration{}, } sideCarSteps = append(sideCarSteps, sidecarStep) } diff --git a/pkg/pod/status.go b/pkg/pod/status.go index be8efc9e31a..1a34fc5e7a5 100644 --- a/pkg/pod/status.go +++ b/pkg/pod/status.go @@ -109,9 +109,9 @@ func MakeTaskRunStatus(logger *zap.SugaredLogger, tr v1beta1.TaskRun, pod *corev complete := areStepsComplete(pod) || pod.Status.Phase == corev1.PodSucceeded || pod.Status.Phase == corev1.PodFailed if complete { - updateCompletedTaskRun(trs, pod) + updateCompletedTaskRunStatus(logger, trs, pod) } else { - updateIncompleteTaskRun(trs, pod) + updateIncompleteTaskRunStatus(trs, pod) } trs.PodName = pod.Name @@ -269,9 +269,9 @@ func extractStartedAtTimeFromResults(results []v1beta1.PipelineResourceResult) ( return nil, nil } -func updateCompletedTaskRun(trs *v1beta1.TaskRunStatus, pod *corev1.Pod) { +func updateCompletedTaskRunStatus(logger *zap.SugaredLogger, trs *v1beta1.TaskRunStatus, pod *corev1.Pod) { if DidTaskRunFail(pod) { - msg := getFailureMessage(pod) + msg := getFailureMessage(logger, pod) MarkStatusFailure(trs, msg) } else { MarkStatusSuccess(trs) @@ -281,7 +281,7 @@ func updateCompletedTaskRun(trs *v1beta1.TaskRunStatus, pod *corev1.Pod) { trs.CompletionTime = &metav1.Time{Time: time.Now()} } -func updateIncompleteTaskRun(trs *v1beta1.TaskRunStatus, pod *corev1.Pod) { +func updateIncompleteTaskRunStatus(trs *v1beta1.TaskRunStatus, pod *corev1.Pod) { switch pod.Status.Phase { case corev1.PodRunning: MarkStatusRunning(trs, v1beta1.TaskRunReasonRunning.String(), "Not all Steps in the Task have finished executing") @@ -327,15 +327,27 @@ func areStepsComplete(pod *corev1.Pod) bool { return stepsComplete } -func getFailureMessage(pod *corev1.Pod) string { +func getFailureMessage(logger *zap.SugaredLogger, pod *corev1.Pod) string { // First, try to surface an error about the actual build step that failed. for _, status := range pod.Status.ContainerStatuses { term := status.State.Terminated - if term != nil && term.ExitCode != 0 { - // Newline required at end to prevent yaml parser from breaking the log help text at 80 chars - return fmt.Sprintf("%q exited with code %d (image: %q); for logs run: kubectl -n %s logs %s -c %s\n", - status.Name, term.ExitCode, status.ImageID, - pod.Namespace, pod.Name, status.Name) + if term != nil { + msg := status.State.Terminated.Message + r, _ := termination.ParseMessage(logger, msg) + for _, result := range r { + if result.ResultType == v1beta1.InternalTektonResultType && result.Key == "Reason" && result.Value == "TimeoutExceeded" { + // Newline required at end to prevent yaml parser from breaking the log help text at 80 chars + return fmt.Sprintf("%q exited because the step exceeded the specified timeout limit; for logs run: kubectl -n %s logs %s -c %s\n", + status.Name, + pod.Namespace, pod.Name, status.Name) + } + } + if term.ExitCode != 0 { + // Newline required at end to prevent yaml parser from breaking the log help text at 80 chars + return fmt.Sprintf("%q exited with code %d (image: %q); for logs run: kubectl -n %s logs %s -c %s\n", + status.Name, term.ExitCode, status.ImageID, + pod.Namespace, pod.Name, status.Name) + } } } // Next, return the Pod's status message if it has one. diff --git a/test/timeout_test.go b/test/timeout_test.go index 2ecdb29af41..34c2a943556 100644 --- a/test/timeout_test.go +++ b/test/timeout_test.go @@ -168,6 +168,79 @@ func TestPipelineRunTimeout(t *testing.T) { } } +// TestStepTimeout is an integration test that will verify a Step can be timed out. +func TestStepTimeout(t *testing.T) { + ctx := context.Background() + ctx, cancel := context.WithCancel(ctx) + defer cancel() + c, namespace := setup(ctx, t) + t.Parallel() + + knativetest.CleanupOnInterrupt(func() { tearDown(ctx, t, c, namespace) }, t.Logf) + defer tearDown(ctx, t, c, namespace) + + t.Logf("Creating Task with Step step-no-timeout, Step step-timeout, and Step step-canceled in namespace %s", namespace) + + taskrunName := "run-timeout" + + t.Logf("Creating TaskRun %s in namespace %s", taskrunName, namespace) + taskRun := &v1beta1.TaskRun{ + ObjectMeta: metav1.ObjectMeta{Name: taskrunName, Namespace: namespace}, + Spec: v1beta1.TaskRunSpec{ + TaskSpec: &v1beta1.TaskSpec{ + Steps: []v1beta1.Step{{ + Container: corev1.Container{ + Name: "no-timeout", + Image: "busybox", + }, + Script: "sleep 1", + Timeout: &metav1.Duration{Duration: 2 * time.Second}, + }, { + Container: corev1.Container{ + Name: "timeout", + Image: "busybox", + }, + Script: "sleep 1", + Timeout: &metav1.Duration{Duration: time.Millisecond}, + }, { + Container: corev1.Container{ + Name: "canceled", + Image: "busybox", + }, + Script: "sleep 1", + }, + }, + }, + }, + } + if _, err := c.TaskRunClient.Create(ctx, taskRun, metav1.CreateOptions{}); err != nil { + t.Fatalf("Failed to create TaskRun `%s`: %s", taskrunName, err) + } + + failMsg := "\"step-timeout\" exited because the step exceeded the specified timeout limit" + t.Logf("Waiting for %s in namespace %s to time out", "step-timeout", namespace) + if err := WaitForTaskRunState(ctx, c, taskrunName, FailedWithMessage(failMsg, "run-timeout"), "StepTimeout"); err != nil { + t.Logf("Error in taskRun %s status: %s\n", taskrunName, err) + t.Errorf("Expected: %s", failMsg) + } + + tr, err := c.TaskRunClient.Get(ctx, taskrunName, metav1.GetOptions{}) + if err != nil { + t.Errorf("Error getting Taskrun: %v", err) + } + if tr.Status.Steps[0].Terminated == nil { + if tr.Status.Steps[0].Terminated.Reason != "Completed" { + t.Errorf("step-no-timeout should not have been terminated") + } + } + if tr.Status.Steps[2].Terminated == nil { + t.Errorf("step-canceled should have been canceled after step-timeout timed out") + } else if exitcode := tr.Status.Steps[2].Terminated.ExitCode; exitcode != 1 { + t.Logf("step-canceled exited with exit code %d, expected exit code 1", exitcode) + } + +} + // TestTaskRunTimeout is an integration test that will verify a TaskRun can be timed out. func TestTaskRunTimeout(t *testing.T) { ctx := context.Background()