Skip to content

Commit

Permalink
Deprecate timeout and promote timeouts in PR
Browse files Browse the repository at this point in the history
Signed-off-by: vinamra28 <jvinamra776@gmail.com>
  • Loading branch information
vinamra28 committed May 8, 2022
1 parent f1ff83f commit 2ad5ac7
Show file tree
Hide file tree
Showing 6 changed files with 27 additions and 38 deletions.
1 change: 1 addition & 0 deletions docs/deprecations.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,3 +25,4 @@ being deprecated.
| [`PipelineRunCancelled` is deprecated and will be removed](https://github.com/tektoncd/pipeline/issues/4611) | [v0.25.0](https://github.com/tektoncd/pipeline/releases/tag/v0.25.0) | Beta | July 12 2022 |
| [`PipelineResources` are deprecated.](https://github.com/tektoncd/community/blob/main/teps/0074-deprecate-pipelineresources.md) | [v0.30.0](https://github.com/tektoncd/pipeline/releases/tag/v0.30.0) | Alpha | Dec 20 2021 |
| [The `PipelineRun.Status.TaskRuns` and `PipelineRun.Status.Runs` fields are deprecated and will be removed.](https://github.com/tektoncd/community/blob/main/teps/0100-embedded-taskruns-and-runs-status-in-pipelineruns.md) | v0.35.0 (to be released) | Beta | Jan 25, 2023 |
| [PipelineRun.Timeout is deprecated and will be removed](https://github.com/tektoncd/community/blob/main/teps/0046-finallytask-execution-post-timeout.md) | v0.36.0 (to be released) | Beta | Feb 25, 2023 |
12 changes: 6 additions & 6 deletions docs/pipelineruns.md
Original file line number Diff line number Diff line change
Expand Up @@ -512,12 +512,6 @@ For more information, see the [`LimitRange` support in Pipeline](./compute-resou

### Configuring a failure timeout

You can use the `timeout` field to set the `PipelineRun's` desired timeout value in minutes.
If you do not specify this value in the `PipelineRun`, the global default timeout value applies.
If you set the timeout to 0, the `PipelineRun` fails immediately upon encountering an error.

> :warning: ** `timeout`will be deprecated in future versions. Consider using `timeouts` instead.

You can use the `timeouts` field to set the `PipelineRun's` desired timeout value in minutes. There are three sub-fields than can be used to specify failures timeout for the entire pipeline, for tasks, and for `finally` tasks.

```yaml
Expand Down Expand Up @@ -551,6 +545,12 @@ spec:
finally: "0h3m0s"
```

You can also use the *Deprecated* `timeout` field to set the `PipelineRun's` desired timeout value in minutes.
If you do not specify this value in the `PipelineRun`, the global default timeout value applies.
If you set the timeout to 0, the `PipelineRun` fails immediately upon encountering an error.

> :warning: ** `timeout` is deprecated and will be removed in future versions. Consider using `timeouts` instead.

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.
Expand Down
3 changes: 0 additions & 3 deletions examples/v1beta1/pipelineruns/no-ci/pipeline-timeout.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -63,9 +63,6 @@ spec:
# 1 hour and half timeout for the pipeline
# 1 hour and fifteen minutes for the pipeline tasks
# 15 minutes for the finally tasks
#
# This field requires enable-api-fields: "alpha" flag
# Check https://github.com/tektoncd/pipeline/blob/main/config/config-feature-flags.yaml
timeouts:
pipeline: 1h30m
tasks: 1h15m
Expand Down
5 changes: 2 additions & 3 deletions pkg/apis/pipeline/v1beta1/pipelinerun_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -219,15 +219,14 @@ type PipelineRunSpec struct {
// Used for cancelling a pipelinerun (and maybe more later on)
// +optional
Status PipelineRunSpecStatus `json:"status,omitempty"`
// This is an alpha field. You must set the "enable-api-fields" feature flag to "alpha"
// for this field to be supported.
//
// Time after which the Pipeline times out.
// Currently three keys are accepted in the map
// pipeline, tasks and finally
// with Timeouts.pipeline >= Timeouts.tasks + Timeouts.finally
// +optional
Timeouts *TimeoutFields `json:"timeouts,omitempty"`

// Timeout Deprecated: use pipelineRunSpec.Timeouts instead
// Time after which the Pipeline times out. Defaults to never.
// Refer to Go's ParseDuration documentation for expected format: https://golang.org/pkg/time/#ParseDuration
// +optional
Expand Down
2 changes: 0 additions & 2 deletions pkg/apis/pipeline/v1beta1/pipelinerun_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,6 @@ func (ps *PipelineRunSpec) Validate(ctx context.Context) (errs *apis.FieldError)
errs = errs.Also(apis.ErrDisallowedFields("timeout", "timeouts"))
}

errs = errs.Also(ValidateEnabledAPIFields(ctx, "timeouts", config.AlphaAPIFields))

// tasks timeout should be a valid duration of at least 0.
errs = errs.Also(validateTimeoutDuration("tasks", ps.Timeouts.Tasks))

Expand Down
42 changes: 18 additions & 24 deletions pkg/apis/pipeline/v1beta1/pipelinerun_validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -707,7 +707,6 @@ func TestPipelineRunWithAlphaFields_Invalid(t *testing.T) {
},
},
want: apis.ErrInvalidValue("-48h0m0s should be >= 0", "spec.timeouts.finally"),
wc: enableAlphaAPIFields,
}, {
name: "pipeline tasks Timeout > pipeline Timeout",
pr: v1beta1.PipelineRun{
Expand All @@ -725,7 +724,6 @@ func TestPipelineRunWithAlphaFields_Invalid(t *testing.T) {
},
},
want: apis.ErrInvalidValue("1h0m0s should be <= pipeline duration", "spec.timeouts.tasks"),
wc: enableAlphaAPIFields,
}, {
name: "pipeline finally Timeout > pipeline Timeout",
pr: v1beta1.PipelineRun{
Expand All @@ -743,7 +741,6 @@ func TestPipelineRunWithAlphaFields_Invalid(t *testing.T) {
},
},
want: apis.ErrInvalidValue("1h0m0s should be <= pipeline duration", "spec.timeouts.finally"),
wc: enableAlphaAPIFields,
}, {
name: "pipeline tasks Timeout + pipeline finally Timeout > pipeline Timeout",
pr: v1beta1.PipelineRun{
Expand All @@ -762,25 +759,6 @@ func TestPipelineRunWithAlphaFields_Invalid(t *testing.T) {
},
},
want: apis.ErrInvalidValue("30m0s + 30m0s should be <= pipeline duration", "spec.timeouts.finally, spec.timeouts.tasks"),
wc: enableAlphaAPIFields,
}, {
name: "Invalid Timeouts when alpha fields not enabled",
pr: v1beta1.PipelineRun{
ObjectMeta: metav1.ObjectMeta{
Name: "pipelinelinename",
},
Spec: v1beta1.PipelineRunSpec{
PipelineRef: &v1beta1.PipelineRef{
Name: "prname",
},
Timeouts: &v1beta1.TimeoutFields{
Pipeline: &metav1.Duration{Duration: 1 * time.Hour},
Tasks: &metav1.Duration{Duration: 30 * time.Minute},
Finally: &metav1.Duration{Duration: 30 * time.Minute},
},
},
},
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{
Expand Down Expand Up @@ -867,7 +845,7 @@ func TestPipelineRunWithAlphaFields_Invalid(t *testing.T) {
}
}

func TestPipelineRunWithAlphaFields_Validate(t *testing.T) {
func TestPipelineRunWithTimeout_Validate(t *testing.T) {
tests := []struct {
name string
pr v1beta1.PipelineRun
Expand All @@ -889,7 +867,23 @@ func TestPipelineRunWithAlphaFields_Validate(t *testing.T) {
},
},
},
wc: enableAlphaAPIFields,
}, {
name: "Invalid Timeouts when alpha fields not enabled",
pr: v1beta1.PipelineRun{
ObjectMeta: metav1.ObjectMeta{
Name: "pipelinelinename",
},
Spec: v1beta1.PipelineRunSpec{
PipelineRef: &v1beta1.PipelineRef{
Name: "prname",
},
Timeouts: &v1beta1.TimeoutFields{
Pipeline: &metav1.Duration{Duration: 1 * time.Hour},
Tasks: &metav1.Duration{Duration: 30 * time.Minute},
Finally: &metav1.Duration{Duration: 30 * time.Minute},
},
},
},
}}

for _, ts := range tests {
Expand Down

0 comments on commit 2ad5ac7

Please sign in to comment.