diff --git a/docs/deprecations.md b/docs/deprecations.md index aa502437ffb..014066db51a 100644 --- a/docs/deprecations.md +++ b/docs/deprecations.md @@ -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 | diff --git a/docs/pipelineruns.md b/docs/pipelineruns.md index 2848e2c93d3..4adb42ee5fc 100644 --- a/docs/pipelineruns.md +++ b/docs/pipelineruns.md @@ -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 @@ -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. diff --git a/examples/v1beta1/pipelineruns/no-ci/pipeline-timeout.yaml b/examples/v1beta1/pipelineruns/no-ci/pipeline-timeout.yaml index c3fdbba710f..c5fc67ac20f 100644 --- a/examples/v1beta1/pipelineruns/no-ci/pipeline-timeout.yaml +++ b/examples/v1beta1/pipelineruns/no-ci/pipeline-timeout.yaml @@ -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 diff --git a/pkg/apis/pipeline/v1beta1/openapi_generated.go b/pkg/apis/pipeline/v1beta1/openapi_generated.go index 9d547860475..0d0443ea230 100644 --- a/pkg/apis/pipeline/v1beta1/openapi_generated.go +++ b/pkg/apis/pipeline/v1beta1/openapi_generated.go @@ -1779,13 +1779,13 @@ func schema_pkg_apis_pipeline_v1beta1_PipelineRunSpec(ref common.ReferenceCallba }, "timeouts": { SchemaProps: spec.SchemaProps{ - Description: "This is an alpha field. You must set the \"enable-api-fields\" feature flag to \"alpha\" for this field to be supported.\n\nTime 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", + Description: "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", Ref: ref("github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.TimeoutFields"), }, }, "timeout": { SchemaProps: spec.SchemaProps{ - Description: "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", + Description: "Timeout Deprecated: use pipelineRunSpec.Timeouts.Pipeline 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", Ref: ref("k8s.io/apimachinery/pkg/apis/meta/v1.Duration"), }, }, diff --git a/pkg/apis/pipeline/v1beta1/pipelinerun_types.go b/pkg/apis/pipeline/v1beta1/pipelinerun_types.go index 55531db0b79..31dd1d94ea0 100644 --- a/pkg/apis/pipeline/v1beta1/pipelinerun_types.go +++ b/pkg/apis/pipeline/v1beta1/pipelinerun_types.go @@ -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.Pipeline 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 diff --git a/pkg/apis/pipeline/v1beta1/pipelinerun_validation.go b/pkg/apis/pipeline/v1beta1/pipelinerun_validation.go index 0a9dc702855..482cda177eb 100644 --- a/pkg/apis/pipeline/v1beta1/pipelinerun_validation.go +++ b/pkg/apis/pipeline/v1beta1/pipelinerun_validation.go @@ -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)) diff --git a/pkg/apis/pipeline/v1beta1/pipelinerun_validation_test.go b/pkg/apis/pipeline/v1beta1/pipelinerun_validation_test.go index c9ae415cc57..6fc9f58c306 100644 --- a/pkg/apis/pipeline/v1beta1/pipelinerun_validation_test.go +++ b/pkg/apis/pipeline/v1beta1/pipelinerun_validation_test.go @@ -651,12 +651,11 @@ func TestPipelineRunSpec_Validate(t *testing.T) { } } -func TestPipelineRunWithAlphaFields_Invalid(t *testing.T) { +func TestPipelineRun_InvalidTimeouts(t *testing.T) { tests := []struct { name string pr v1beta1.PipelineRun want *apis.FieldError - wc func(context.Context) context.Context }{{ name: "negative pipeline timeouts", pr: v1beta1.PipelineRun{ @@ -673,7 +672,6 @@ func TestPipelineRunWithAlphaFields_Invalid(t *testing.T) { }, }, want: apis.ErrInvalidValue("-48h0m0s should be >= 0", "spec.timeouts.pipeline"), - wc: enableAlphaAPIFields, }, { name: "negative pipeline tasks Timeout", pr: v1beta1.PipelineRun{ @@ -690,7 +688,6 @@ func TestPipelineRunWithAlphaFields_Invalid(t *testing.T) { }, }, want: apis.ErrInvalidValue("-48h0m0s should be >= 0", "spec.timeouts.tasks"), - wc: enableAlphaAPIFields, }, { name: "negative pipeline finally Timeout", pr: v1beta1.PipelineRun{ @@ -707,7 +704,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{ @@ -725,7 +721,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{ @@ -743,7 +738,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{ @@ -762,25 +756,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{ @@ -796,7 +771,6 @@ func TestPipelineRunWithAlphaFields_Invalid(t *testing.T) { }, }, }, - 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", @@ -814,7 +788,6 @@ func TestPipelineRunWithAlphaFields_Invalid(t *testing.T) { }, }, }, - wc: enableAlphaAPIFields, want: apis.ErrInvalidValue(`0s (no timeout) should be <= pipeline duration`, "spec.timeouts.tasks"), }, { name: "Finally timeout = 0 but Pipeline timeout not set", @@ -831,7 +804,6 @@ func TestPipelineRunWithAlphaFields_Invalid(t *testing.T) { }, }, }, - 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", @@ -849,16 +821,29 @@ func TestPipelineRunWithAlphaFields_Invalid(t *testing.T) { }, }, }, - wc: enableAlphaAPIFields, want: apis.ErrInvalidValue(`0s (no timeout) should be <= pipeline duration`, "spec.timeouts.finally"), + }, { + name: "Timeout and Timeouts both are set", + pr: v1beta1.PipelineRun{ + ObjectMeta: metav1.ObjectMeta{ + Name: "pipelinelinename", + }, + Spec: v1beta1.PipelineRunSpec{ + PipelineRef: &v1beta1.PipelineRef{ + Name: "prname", + }, + Timeout: &metav1.Duration{Duration: 10 * time.Minute}, + Timeouts: &v1beta1.TimeoutFields{ + Pipeline: &metav1.Duration{Duration: 10 * time.Minute}, + }, + }, + }, + want: apis.ErrDisallowedFields("spec.timeout", "spec.timeouts"), }} for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { ctx := context.Background() - if tc.wc != nil { - ctx = tc.wc(ctx) - } err := tc.pr.Validate(ctx) if d := cmp.Diff(err.Error(), tc.want.Error()); d != "" { t.Error(diff.PrintWantGot(d)) @@ -867,7 +852,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 @@ -889,7 +874,23 @@ func TestPipelineRunWithAlphaFields_Validate(t *testing.T) { }, }, }, - wc: enableAlphaAPIFields, + }, { + name: "Timeouts set for all three Task, Finally and Pipeline", + 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 { diff --git a/pkg/apis/pipeline/v1beta1/swagger.json b/pkg/apis/pipeline/v1beta1/swagger.json index 99ef08835e6..6de55917cb9 100644 --- a/pkg/apis/pipeline/v1beta1/swagger.json +++ b/pkg/apis/pipeline/v1beta1/swagger.json @@ -1088,11 +1088,11 @@ "x-kubernetes-list-type": "atomic" }, "timeout": { - "description": "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", + "description": "Timeout Deprecated: use pipelineRunSpec.Timeouts.Pipeline 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", "$ref": "#/definitions/v1.Duration" }, "timeouts": { - "description": "This is an alpha field. You must set the \"enable-api-fields\" feature flag to \"alpha\" for this field to be supported.\n\nTime after which the Pipeline times out. Currently three keys are accepted in the map pipeline, tasks and finally with Timeouts.pipeline \u003e= Timeouts.tasks + Timeouts.finally", + "description": "Time after which the Pipeline times out. Currently three keys are accepted in the map pipeline, tasks and finally with Timeouts.pipeline \u003e= Timeouts.tasks + Timeouts.finally", "$ref": "#/definitions/v1beta1.TimeoutFields" }, "workspaces": { diff --git a/pkg/reconciler/pipelinerun/pipelinerun_test.go b/pkg/reconciler/pipelinerun/pipelinerun_test.go index 91a52a1d9d1..1ce7f34c393 100644 --- a/pkg/reconciler/pipelinerun/pipelinerun_test.go +++ b/pkg/reconciler/pipelinerun/pipelinerun_test.go @@ -2432,8 +2432,8 @@ spec: } } -func TestReconcileWithTimeout(t *testing.T) { - // TestReconcileWithTimeout runs "Reconcile" on a PipelineRun that has timed out. +func TestReconcileWithTimeoutDeprecated(t *testing.T) { + // TestReconcileWithTimeoutDeprecated runs "Reconcile" on a PipelineRun that has timed out. // It verifies that reconcile is successful, the pipeline status updated and events generated. ps := []*v1beta1.Pipeline{simpleHelloWorldPipeline} prs := []*v1beta1.PipelineRun{parse.MustParsePipelineRun(t, ` @@ -2481,6 +2481,56 @@ status: } } +func TestReconcileWithTimeouts(t *testing.T) { + // TestReconcileWithTimeouts runs "Reconcile" on a PipelineRun that has timed out. + // It verifies that reconcile is successful, the pipeline status updated and events generated. + ps := []*v1beta1.Pipeline{simpleHelloWorldPipeline} + prs := []*v1beta1.PipelineRun{parse.MustParsePipelineRun(t, ` +metadata: + name: test-pipeline-run-with-timeout + namespace: foo +spec: + pipelineRef: + name: test-pipeline + serviceAccountName: test-sa + timeouts: + pipeline: 12h0m0s +status: + startTime: "2021-12-31T00:00:00Z" +`)} + ts := []*v1beta1.Task{simpleHelloWorldTask} + + d := test.Data{ + PipelineRuns: prs, + Pipelines: ps, + Tasks: ts, + } + prt := newPipelineRunTest(d, t) + defer prt.Cancel() + + wantEvents := []string{ + "Warning Failed PipelineRun \"test-pipeline-run-with-timeout\" failed to finish within \"12h0m0s\"", + } + reconciledRun, clients := prt.reconcileRun("foo", "test-pipeline-run-with-timeout", wantEvents, false) + + if reconciledRun.Status.CompletionTime == nil { + t.Errorf("Expected a CompletionTime on invalid PipelineRun but was nil") + } + + // The PipelineRun should be timed out. + if reconciledRun.Status.GetCondition(apis.ConditionSucceeded).Reason != "PipelineRunTimeout" { + t.Errorf("Expected PipelineRun to be timed out, but condition reason is %s", reconciledRun.Status.GetCondition(apis.ConditionSucceeded)) + } + + // Check that the expected TaskRun was created + actual := getTaskRunCreations(t, clients.Pipeline.Actions(), 2)[0] + + // The TaskRun timeout should be less than or equal to the PipelineRun timeout. + if actual.Spec.Timeout.Duration > prs[0].Spec.Timeouts.Pipeline.Duration { + t.Errorf("TaskRun timeout %s should be less than or equal to PipelineRun timeout %s", actual.Spec.Timeout.Duration.String(), prs[0].Spec.Timeouts.Pipeline.Duration.String()) + } +} + func TestReconcileWithoutPVC(t *testing.T) { // TestReconcileWithoutPVC runs "Reconcile" on a PipelineRun that has two unrelated tasks. // It verifies that reconcile is successful and that no PVC is created diff --git a/pkg/reconciler/pipelinerun/resources/pipelinerunstate_test.go b/pkg/reconciler/pipelinerun/resources/pipelinerunstate_test.go index 3e08d02570c..ba2bf3b4162 100644 --- a/pkg/reconciler/pipelinerun/resources/pipelinerunstate_test.go +++ b/pkg/reconciler/pipelinerun/resources/pipelinerunstate_test.go @@ -1587,7 +1587,7 @@ func TestGetPipelineConditionStatus_WithFinalTasks(t *testing.T) { } // pipeline should result in timeout if its runtime exceeds its spec.Timeout based on its status.Timeout -func TestGetPipelineConditionStatus_PipelineTimeouts(t *testing.T) { +func TestGetPipelineConditionStatus_PipelineTimeoutDeprecated(t *testing.T) { d, err := dagFromState(oneFinishedState) if err != nil { t.Fatalf("Unexpected error while building DAG for state %v: %v", oneFinishedState, err) @@ -1614,6 +1614,36 @@ func TestGetPipelineConditionStatus_PipelineTimeouts(t *testing.T) { } } +// pipeline should result in timeout if its runtime exceeds its spec.Timeouts.Pipeline based on its status.Timeout +func TestGetPipelineConditionStatus_PipelineTimeouts(t *testing.T) { + d, err := dagFromState(oneFinishedState) + if err != nil { + t.Fatalf("Unexpected error while building DAG for state %v: %v", oneFinishedState, err) + } + pr := &v1beta1.PipelineRun{ + ObjectMeta: metav1.ObjectMeta{Name: "pipelinerun-no-tasks-started"}, + Spec: v1beta1.PipelineRunSpec{ + Timeouts: &v1beta1.TimeoutFields{ + Pipeline: &metav1.Duration{Duration: 1 * time.Minute}, + }, + }, + Status: v1beta1.PipelineRunStatus{ + PipelineRunStatusFields: v1beta1.PipelineRunStatusFields{ + StartTime: &metav1.Time{Time: now.Add(-2 * time.Minute)}, + }, + }, + } + facts := PipelineRunFacts{ + State: oneFinishedState, + TasksGraph: d, + FinalTasksGraph: &dag.Graph{}, + } + c := facts.GetPipelineConditionStatus(context.Background(), pr, zap.NewNop().Sugar(), testClock) + if c.Status != corev1.ConditionFalse && c.Reason != v1beta1.PipelineRunReasonTimedOut.String() { + t.Fatalf("Expected to get status %s but got %s for state %v", corev1.ConditionFalse, c.Status, oneFinishedState) + } +} + func TestAdjustStartTime(t *testing.T) { baseline := metav1.Time{Time: now}