diff --git a/docs/pipelineruns.md b/docs/pipelineruns.md index fe97934e5ca..33f2ab91ef9 100644 --- a/docs/pipelineruns.md +++ b/docs/pipelineruns.md @@ -523,6 +523,7 @@ spec: If you do not specify the `timeout` value or `timeouts.pipeline` in the `PipelineRun`, the global default timeout value applies. If you set the `timeout` value or `timeouts.pipeline` to 0, the `PipelineRun` fails immediately upon encountering an error. +If `timeouts.tasks` or `timeouts.finally` is set to 0, `timeouts.pipeline` must also be set to 0. The global default timeout is set to 60 minutes when you first install Tekton. You can set a different global default timeout value using the `default-timeout-minutes` field in diff --git a/pkg/apis/pipeline/v1beta1/pipelinerun_validation.go b/pkg/apis/pipeline/v1beta1/pipelinerun_validation.go index f829a8aa2e3..b5f2809bd07 100644 --- a/pkg/apis/pipeline/v1beta1/pipelinerun_validation.go +++ b/pkg/apis/pipeline/v1beta1/pipelinerun_validation.go @@ -23,6 +23,7 @@ import ( "github.com/google/go-containerregistry/pkg/name" "github.com/tektoncd/pipeline/pkg/apis/config" + apisconfig "github.com/tektoncd/pipeline/pkg/apis/config" "github.com/tektoncd/pipeline/pkg/apis/validate" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "knative.dev/pkg/apis" @@ -169,12 +170,34 @@ func validateTimeoutDuration(field string, d *metav1.Duration) (errs *apis.Field } func (ps *PipelineRunSpec) validatePipelineTimeout(timeout time.Duration, errorMsg string) (errs *apis.FieldError) { - if ps.Timeouts.Tasks != nil && ps.Timeouts.Tasks.Duration > timeout { - errs = errs.Also(apis.ErrInvalidValue(fmt.Sprintf("%s %s", ps.Timeouts.Tasks.Duration.String(), errorMsg), "timeouts.tasks")) + if ps.Timeouts.Tasks != nil { + tasksTimeoutErr := false + tasksTimeoutStr := ps.Timeouts.Tasks.Duration.String() + if ps.Timeouts.Tasks.Duration > timeout { + tasksTimeoutErr = true + } + if ps.Timeouts.Tasks.Duration == apisconfig.NoTimeoutDuration && timeout != apisconfig.NoTimeoutDuration { + tasksTimeoutErr = true + tasksTimeoutStr += " (no timeout)" + } + if tasksTimeoutErr { + errs = errs.Also(apis.ErrInvalidValue(fmt.Sprintf("%s %s", tasksTimeoutStr, errorMsg), "timeouts.tasks")) + } } - if ps.Timeouts.Finally != nil && ps.Timeouts.Finally.Duration > timeout { - errs = errs.Also(apis.ErrInvalidValue(fmt.Sprintf("%s %s", ps.Timeouts.Finally.Duration.String(), errorMsg), "timeouts.finally")) + if ps.Timeouts.Finally != nil { + finallyTimeoutErr := false + finallyTimeoutStr := ps.Timeouts.Finally.Duration.String() + if ps.Timeouts.Finally.Duration > timeout { + finallyTimeoutErr = true + } + if ps.Timeouts.Finally.Duration == apisconfig.NoTimeoutDuration && timeout != apisconfig.NoTimeoutDuration { + finallyTimeoutErr = true + finallyTimeoutStr += " (no timeout)" + } + if finallyTimeoutErr { + errs = errs.Also(apis.ErrInvalidValue(fmt.Sprintf("%s %s", finallyTimeoutStr, errorMsg), "timeouts.finally")) + } } if ps.Timeouts.Tasks != nil && ps.Timeouts.Finally != nil { diff --git a/pkg/apis/pipeline/v1beta1/pipelinerun_validation_test.go b/pkg/apis/pipeline/v1beta1/pipelinerun_validation_test.go index c777ecdd8cf..0d4ab91c3bd 100644 --- a/pkg/apis/pipeline/v1beta1/pipelinerun_validation_test.go +++ b/pkg/apis/pipeline/v1beta1/pipelinerun_validation_test.go @@ -18,7 +18,6 @@ package v1beta1_test import ( "context" - "fmt" "testing" "time" @@ -598,7 +597,77 @@ func TestPipelineRunWithAlphaFields_Invalid(t *testing.T) { }, }, }, - want: apis.ErrGeneric(fmt.Sprintf(`timeouts requires "enable-api-fields" feature gate to be "alpha" but it is "stable"`)), + want: apis.ErrGeneric(`timeouts requires "enable-api-fields" feature gate to be "alpha" but it is "stable"`), + }, { + name: "Tasks timeout = 0 but Pipeline timeout not set", + pr: v1beta1.PipelineRun{ + ObjectMeta: metav1.ObjectMeta{ + Name: "pipelinelinename", + }, + Spec: v1beta1.PipelineRunSpec{ + PipelineRef: &v1beta1.PipelineRef{ + Name: "prname", + }, + Timeouts: &v1beta1.TimeoutFields{ + Tasks: &metav1.Duration{Duration: 0 * time.Minute}, + }, + }, + }, + wc: enableAlphaAPIFields, + want: apis.ErrInvalidValue(`0s (no timeout) should be <= default timeout duration`, "spec.timeouts.tasks"), + }, { + name: "Tasks timeout = 0 but Pipeline timeout is not 0", + pr: v1beta1.PipelineRun{ + ObjectMeta: metav1.ObjectMeta{ + Name: "pipelinelinename", + }, + Spec: v1beta1.PipelineRunSpec{ + PipelineRef: &v1beta1.PipelineRef{ + Name: "prname", + }, + Timeouts: &v1beta1.TimeoutFields{ + Pipeline: &metav1.Duration{Duration: 10 * time.Minute}, + Tasks: &metav1.Duration{Duration: 0 * time.Minute}, + }, + }, + }, + wc: enableAlphaAPIFields, + want: apis.ErrInvalidValue(`0s (no timeout) should be <= pipeline duration`, "spec.timeouts.tasks"), + }, { + name: "Finally timeout = 0 but Pipeline timeout not set", + pr: v1beta1.PipelineRun{ + ObjectMeta: metav1.ObjectMeta{ + Name: "pipelinelinename", + }, + Spec: v1beta1.PipelineRunSpec{ + PipelineRef: &v1beta1.PipelineRef{ + Name: "prname", + }, + Timeouts: &v1beta1.TimeoutFields{ + Finally: &metav1.Duration{Duration: 0 * time.Minute}, + }, + }, + }, + wc: enableAlphaAPIFields, + want: apis.ErrInvalidValue(`0s (no timeout) should be <= default timeout duration`, "spec.timeouts.finally"), + }, { + name: "Finally timeout = 0 but Pipeline timeout is not 0", + pr: v1beta1.PipelineRun{ + ObjectMeta: metav1.ObjectMeta{ + Name: "pipelinelinename", + }, + Spec: v1beta1.PipelineRunSpec{ + PipelineRef: &v1beta1.PipelineRef{ + Name: "prname", + }, + Timeouts: &v1beta1.TimeoutFields{ + Pipeline: &metav1.Duration{Duration: 10 * time.Minute}, + Finally: &metav1.Duration{Duration: 0 * time.Minute}, + }, + }, + }, + wc: enableAlphaAPIFields, + want: apis.ErrInvalidValue(`0s (no timeout) should be <= pipeline duration`, "spec.timeouts.finally"), }} for _, tc := range tests {