diff --git a/pkg/apis/pipeline/v1alpha1/run_types.go b/pkg/apis/pipeline/v1alpha1/run_types.go index 7889cfa8b8f..8f7bd9ba561 100644 --- a/pkg/apis/pipeline/v1alpha1/run_types.go +++ b/pkg/apis/pipeline/v1alpha1/run_types.go @@ -79,6 +79,10 @@ type RunSpec struct { // Workspaces is a list of WorkspaceBindings from volumes to workspaces. // +optional Workspaces []v1beta1.WorkspaceBinding `json:"workspaces,omitempty"` + + // TimeoutFromParent is set to true if this Run's timeout was set by a parent PipelineRun. + // +optional + TimeoutFromParent bool `json:"timeoutFromParent,omitempty"` } // RunSpecStatus defines the taskrun spec status the user can provide diff --git a/pkg/apis/pipeline/v1beta1/openapi_generated.go b/pkg/apis/pipeline/v1beta1/openapi_generated.go index 3fdc788544f..f999c06af72 100644 --- a/pkg/apis/pipeline/v1beta1/openapi_generated.go +++ b/pkg/apis/pipeline/v1beta1/openapi_generated.go @@ -4489,6 +4489,13 @@ func schema_pkg_apis_pipeline_v1beta1_TaskRunSpec(ref common.ReferenceCallback) Ref: ref("k8s.io/api/core/v1.ResourceRequirements"), }, }, + "timeoutFromParent": { + SchemaProps: spec.SchemaProps{ + Description: "TimeoutFromParent is set to true if this TaskRun's timeout was set by a parent PipelineRun.", + Type: []string{"boolean"}, + Format: "", + }, + }, }, }, }, diff --git a/pkg/apis/pipeline/v1beta1/swagger.json b/pkg/apis/pipeline/v1beta1/swagger.json index 556a95ecd0e..ad2cc9c2bb2 100644 --- a/pkg/apis/pipeline/v1beta1/swagger.json +++ b/pkg/apis/pipeline/v1beta1/swagger.json @@ -2507,6 +2507,10 @@ "description": "Time after which the build times out. Defaults to 1 hour. Specified build timeout should be less than 24h. Refer Go's ParseDuration documentation for expected format: https://golang.org/pkg/time/#ParseDuration", "$ref": "#/definitions/v1.Duration" }, + "timeoutFromParent": { + "description": "TimeoutFromParent is set to true if this TaskRun's timeout was set by a parent PipelineRun.", + "type": "boolean" + }, "workspaces": { "description": "Workspaces is a list of WorkspaceBindings from volumes to workspaces.", "type": "array", diff --git a/pkg/apis/pipeline/v1beta1/taskrun_types.go b/pkg/apis/pipeline/v1beta1/taskrun_types.go index 44a0f02b816..61cf0763b25 100644 --- a/pkg/apis/pipeline/v1beta1/taskrun_types.go +++ b/pkg/apis/pipeline/v1beta1/taskrun_types.go @@ -58,6 +58,7 @@ type TaskRunSpec struct { // +optional Timeout *metav1.Duration `json:"timeout,omitempty"` // PodTemplate holds pod specific configuration + // +optional PodTemplate *PodTemplate `json:"podTemplate,omitempty"` // Workspaces is a list of WorkspaceBindings from volumes to workspaces. // +optional @@ -78,7 +79,11 @@ type TaskRunSpec struct { // +listType=atomic SidecarOverrides []TaskRunSidecarOverride `json:"sidecarOverrides,omitempty"` // Compute resources to use for this TaskRun + // +optional ComputeResources *corev1.ResourceRequirements `json:"computeResources,omitempty"` + // TimeoutFromParent is set to true if this TaskRun's timeout was set by a parent PipelineRun. + // +optional + TimeoutFromParent bool `json:"timeoutFromParent,omitempty"` } // TaskRunSpecStatus defines the taskrun spec status the user can provide diff --git a/pkg/reconciler/pipelinerun/pipelinerun.go b/pkg/reconciler/pipelinerun/pipelinerun.go index 19038cd6cb7..82d281ae11d 100644 --- a/pkg/reconciler/pipelinerun/pipelinerun.go +++ b/pkg/reconciler/pipelinerun/pipelinerun.go @@ -770,7 +770,7 @@ func (c *Reconciler) updateRunsStatusDirectly(pr *v1beta1.PipelineRun) error { return nil } -type getTimeoutFunc func(ctx context.Context, pr *v1beta1.PipelineRun, rpt *resources.ResolvedPipelineTask, c clock.PassiveClock) *metav1.Duration +type getTimeoutFunc func(ctx context.Context, pr *v1beta1.PipelineRun, rpt *resources.ResolvedPipelineTask, c clock.PassiveClock) (*metav1.Duration, bool) func (c *Reconciler) createTaskRuns(ctx context.Context, rpt *resources.ResolvedPipelineTask, pr *v1beta1.PipelineRun, storageBasePath string, getTimeoutFunc getTimeoutFunc) ([]*v1beta1.TaskRun, error) { var taskRuns []*v1beta1.TaskRun @@ -804,6 +804,7 @@ func (c *Reconciler) createTaskRun(ctx context.Context, taskRunName string, para rpt.PipelineTask = resources.ApplyPipelineTaskContexts(rpt.PipelineTask) taskRunSpec := pr.GetTaskRunSpec(rpt.PipelineTask.Name) params = append(params, rpt.PipelineTask.Params...) + trTimeout, timeoutFromParent := getTimeoutFunc(ctx, pr, rpt, c.Clock) tr = &v1beta1.TaskRun{ ObjectMeta: metav1.ObjectMeta{ Name: taskRunName, @@ -815,10 +816,11 @@ func (c *Reconciler) createTaskRun(ctx context.Context, taskRunName string, para Spec: v1beta1.TaskRunSpec{ Params: params, ServiceAccountName: taskRunSpec.TaskServiceAccountName, - Timeout: getTimeoutFunc(ctx, pr, rpt, c.Clock), + Timeout: trTimeout, PodTemplate: taskRunSpec.TaskPodTemplate, StepOverrides: taskRunSpec.StepOverrides, SidecarOverrides: taskRunSpec.SidecarOverrides, + TimeoutFromParent: timeoutFromParent, }} if rpt.ResolvedTaskResources.TaskName != "" { @@ -862,6 +864,7 @@ func (c *Reconciler) createRun(ctx context.Context, runName string, params []v1b logger := logging.FromContext(ctx) taskRunSpec := pr.GetTaskRunSpec(rpt.PipelineTask.Name) params = append(params, rpt.PipelineTask.Params...) + runTimeout, timeoutFromParent := getTimeoutFunc(ctx, pr, rpt, c.Clock) r := &v1alpha1.Run{ ObjectMeta: metav1.ObjectMeta{ Name: runName, @@ -875,8 +878,9 @@ func (c *Reconciler) createRun(ctx context.Context, runName string, params []v1b Ref: rpt.PipelineTask.TaskRef, Params: params, ServiceAccountName: taskRunSpec.TaskServiceAccountName, - Timeout: getTimeoutFunc(ctx, pr, rpt, c.Clock), + Timeout: runTimeout, PodTemplate: taskRunSpec.TaskPodTemplate, + TimeoutFromParent: timeoutFromParent, }, } @@ -1099,34 +1103,38 @@ func addMetadataByPrecedence(metadata map[string]string, addedMetadata map[strin } // getFinallyTaskRunTimeout returns the timeout to set when creating the ResolvedPipelineTask, which is a finally Task. -// If there is no timeout for the finally TaskRun, returns 0. +// If there is no timeout for the finally TaskRun, returns 0 and false, since finally tasks are not covered by the // If pipeline level timeouts have already been exceeded, returns 1 second. -func getFinallyTaskRunTimeout(ctx context.Context, pr *v1beta1.PipelineRun, rpt *resources.ResolvedPipelineTask, c clock.PassiveClock) *metav1.Duration { - taskRunTimeout := calculateTaskRunTimeout(pr.PipelineTimeout(ctx), pr, rpt, c) +// If the timeout for the TaskRun is determined based on the PipelineRun's own timeout, also returns true. +func getFinallyTaskRunTimeout(ctx context.Context, pr *v1beta1.PipelineRun, rpt *resources.ResolvedPipelineTask, c clock.PassiveClock) (*metav1.Duration, bool) { + taskRunTimeout, timeoutFromPR := calculateTaskRunTimeout(pr.PipelineTimeout(ctx), pr, rpt, c, true) finallyTimeout := pr.FinallyTimeout() // Return the smaller of taskRunTimeout and finallyTimeout // This works because all finally tasks run in parallel, so there is no need to consider time spent by other finally tasks // TODO(#4071): Account for time spent since finally task was first started (i.e. retries) if finallyTimeout == nil || finallyTimeout.Duration == apisconfig.NoTimeoutDuration { - return taskRunTimeout + return taskRunTimeout, timeoutFromPR } if finallyTimeout.Duration < taskRunTimeout.Duration { - return finallyTimeout + return finallyTimeout, false } - return taskRunTimeout + return taskRunTimeout, timeoutFromPR } // getTaskRunTimeout returns the timeout to set when creating the ResolvedPipelineTask. // If there is no timeout for the TaskRun, returns 0. // If pipeline level timeouts have already been exceeded, returns 1 second. -func getTaskRunTimeout(ctx context.Context, pr *v1beta1.PipelineRun, rpt *resources.ResolvedPipelineTask, c clock.PassiveClock) *metav1.Duration { +// If the timeout for the TaskRun is determined based on the PipelineRun's own timeout, also returns true. +func getTaskRunTimeout(ctx context.Context, pr *v1beta1.PipelineRun, rpt *resources.ResolvedPipelineTask, c clock.PassiveClock) (*metav1.Duration, bool) { var timeout time.Duration + timeoutFromPR := false if pr.TasksTimeout() != nil { timeout = pr.TasksTimeout().Duration } else { timeout = pr.PipelineTimeout(ctx) + timeoutFromPR = true } - return calculateTaskRunTimeout(timeout, pr, rpt, c) + return calculateTaskRunTimeout(timeout, pr, rpt, c, timeoutFromPR) } // calculateTaskRunTimeout returns the timeout to set when creating the ResolvedPipelineTask. @@ -1135,24 +1143,26 @@ func getTaskRunTimeout(ctx context.Context, pr *v1beta1.PipelineRun, rpt *resour // - If ResolvedPipelineTask is a Finally Task, `timeout` is the Pipeline Timeout // If there is no timeout for the TaskRun, returns 0. // If pipeline level timeouts have already been exceeded, returns 1 second. -func calculateTaskRunTimeout(timeout time.Duration, pr *v1beta1.PipelineRun, rpt *resources.ResolvedPipelineTask, c clock.PassiveClock) *metav1.Duration { +// If the timeout for the TaskRun is determined based on the PipelineRun's own timeout, also returns true. +func calculateTaskRunTimeout(timeout time.Duration, pr *v1beta1.PipelineRun, rpt *resources.ResolvedPipelineTask, c clock.PassiveClock, isPRTimeout bool) (*metav1.Duration, bool) { if timeout != apisconfig.NoTimeoutDuration { pElapsedTime := c.Since(pr.Status.StartTime.Time) if pElapsedTime > timeout { - return &metav1.Duration{Duration: 1 * time.Second} + return &metav1.Duration{Duration: 1 * time.Second}, false } timeRemaining := (timeout - pElapsedTime) // Return the smaller of timeRemaining and rpt.pipelineTask.timeout if rpt.PipelineTask.Timeout != nil && rpt.PipelineTask.Timeout.Duration < timeRemaining { - return &metav1.Duration{Duration: rpt.PipelineTask.Timeout.Duration} + return &metav1.Duration{Duration: rpt.PipelineTask.Timeout.Duration}, false } - return &metav1.Duration{Duration: timeRemaining} + // If the time + return &metav1.Duration{Duration: timeRemaining}, timeRemaining <= timeout && isPRTimeout } if timeout == apisconfig.NoTimeoutDuration && rpt.PipelineTask.Timeout != nil { - return &metav1.Duration{Duration: rpt.PipelineTask.Timeout.Duration} + return &metav1.Duration{Duration: rpt.PipelineTask.Timeout.Duration}, false } - return &metav1.Duration{Duration: apisconfig.NoTimeoutDuration} + return &metav1.Duration{Duration: apisconfig.NoTimeoutDuration}, false } func (c *Reconciler) updateLabelsAndAnnotations(ctx context.Context, pr *v1beta1.PipelineRun) (*v1beta1.PipelineRun, error) { diff --git a/pkg/reconciler/pipelinerun/pipelinerun_test.go b/pkg/reconciler/pipelinerun/pipelinerun_test.go index c3f8779db2f..036066ff4f1 100644 --- a/pkg/reconciler/pipelinerun/pipelinerun_test.go +++ b/pkg/reconciler/pipelinerun/pipelinerun_test.go @@ -566,6 +566,7 @@ spec: taskRef: name: unit-test-task timeout: 1h0m0s + timeoutFromParent: true `) // ignore IgnoreUnexported ignore both after and before steps fields if d := cmp.Diff(expectedTaskRun, actual, ignoreTypeMeta, cmpopts.SortSlices(lessTaskResourceBindings)); d != "" { @@ -638,6 +639,7 @@ spec: retries: 3 serviceAccountName: default timeout: 1h0m0s + timeoutFromParent: true ` tcs := []struct { @@ -700,6 +702,7 @@ spec: field1: 123 field2: value timeout: 1h0m0s + timeoutFromParent: true `), }, { name: "custom task with workspace", @@ -738,6 +741,7 @@ spec: kind: Example serviceAccountName: default timeout: 1h0m0s + timeoutFromParent: true workspaces: - name: taskws persistentVolumeClaim: @@ -872,6 +876,7 @@ spec: image: myimage serviceAccountName: %s timeout: 1h0m0s + timeoutFromParent: true `, config.DefaultServiceAccountValue)) expectedTaskRun.ObjectMeta = taskRunObjectMeta("test-pipeline-run-success-unit-test-task-spec", "foo", "test-pipeline-run-success", "test-pipeline", "unit-test-task-spec", false) @@ -2786,6 +2791,7 @@ spec: taskRef: name: hello-world timeout: 1h0m0s + timeoutFromParent: true `) d := test.Data{ @@ -2923,6 +2929,7 @@ spec: taskRef: name: hello-world-task timeout: 1h0m0s + timeoutFromParent: true `), mustParseTaskRunWithObjectMeta(t, taskRunObjectMeta(taskRunNames[1], "foo", "test-pipeline-run-different-service-accs", "test-pipeline", "hello-world-1", false), @@ -2933,6 +2940,7 @@ spec: taskRef: name: hello-world-task timeout: 1h0m0s + timeoutFromParent: true `), } @@ -3110,12 +3118,13 @@ func TestGetTaskRunTimeout(t *testing.T) { p := "pipeline" tcs := []struct { - name string - timeoutDuration *metav1.Duration - timeoutFields *v1beta1.TimeoutFields - startTime time.Time - rpt *resources.ResolvedPipelineTask - expected *metav1.Duration + name string + timeoutDuration *metav1.Duration + timeoutFields *v1beta1.TimeoutFields + startTime time.Time + rpt *resources.ResolvedPipelineTask + expected *metav1.Duration + isTimeoutFromParent bool }{{ name: "nil timeout duration", startTime: now, @@ -3124,7 +3133,8 @@ func TestGetTaskRunTimeout(t *testing.T) { Timeout: nil, }, }, - expected: &metav1.Duration{Duration: 60 * time.Minute}, + expected: &metav1.Duration{Duration: 60 * time.Minute}, + isTimeoutFromParent: true, }, { name: "timeout specified in pr", timeoutDuration: &metav1.Duration{Duration: 20 * time.Minute}, @@ -3134,7 +3144,8 @@ func TestGetTaskRunTimeout(t *testing.T) { Timeout: nil, }, }, - expected: &metav1.Duration{Duration: 20 * time.Minute}, + expected: &metav1.Duration{Duration: 20 * time.Minute}, + isTimeoutFromParent: true, }, { name: "0 timeout duration", timeoutDuration: &metav1.Duration{Duration: 0 * time.Minute}, @@ -3279,6 +3290,26 @@ func TestGetTaskRunTimeout(t *testing.T) { }, }, expected: &metav1.Duration{Duration: 10 * time.Minute}, + }, { + name: "taskrun with elapsed time; timeouts.pipeline applies", + timeoutFields: &v1beta1.TimeoutFields{ + Pipeline: &metav1.Duration{Duration: 20 * time.Minute}, + }, + startTime: now.Add(-10 * time.Minute), + rpt: &resources.ResolvedPipelineTask{ + PipelineTask: &v1beta1.PipelineTask{ + Timeout: &metav1.Duration{Duration: 15 * time.Minute}, + }, + TaskRun: &v1beta1.TaskRun{ + Status: v1beta1.TaskRunStatus{ + TaskRunStatusFields: v1beta1.TaskRunStatusFields{ + StartTime: nil, + }, + }, + }, + }, + expected: &metav1.Duration{Duration: 10 * time.Minute}, + isTimeoutFromParent: true, }} for _, tc := range tcs { @@ -3296,9 +3327,13 @@ func TestGetTaskRunTimeout(t *testing.T) { }, }, } - if d := cmp.Diff(getTaskRunTimeout(context.TODO(), pr, tc.rpt, testClock), tc.expected); d != "" { + trTimeout, timeoutFromParent := getTaskRunTimeout(context.TODO(), pr, tc.rpt, testClock) + if d := cmp.Diff(trTimeout, tc.expected); d != "" { t.Errorf("Unexpected task run timeout. Diff %s", diff.PrintWantGot(d)) } + if tc.isTimeoutFromParent != timeoutFromParent { + t.Errorf("Expected whether task run timeout is determined by PipelineRun timeout to be %t, but is %t", tc.isTimeoutFromParent, timeoutFromParent) + } }) } } @@ -3309,20 +3344,22 @@ func TestGetFinallyTaskRunTimeout(t *testing.T) { p := "pipeline" tcs := []struct { - name string - timeoutDuration *metav1.Duration - timeoutFields *v1beta1.TimeoutFields - startTime time.Time - pr *v1beta1.PipelineRun - rpt *resources.ResolvedPipelineTask - expected *metav1.Duration + name string + timeoutDuration *metav1.Duration + timeoutFields *v1beta1.TimeoutFields + startTime time.Time + pr *v1beta1.PipelineRun + rpt *resources.ResolvedPipelineTask + expected *metav1.Duration + isTimeoutFromParent bool }{{ name: "nil timeout duration", startTime: now, rpt: &resources.ResolvedPipelineTask{ PipelineTask: &v1beta1.PipelineTask{}, }, - expected: &metav1.Duration{Duration: 60 * time.Minute}, + expected: &metav1.Duration{Duration: 60 * time.Minute}, + isTimeoutFromParent: true, }, { name: "timeout specified in pr", timeoutDuration: &metav1.Duration{Duration: 20 * time.Minute}, @@ -3330,7 +3367,8 @@ func TestGetFinallyTaskRunTimeout(t *testing.T) { rpt: &resources.ResolvedPipelineTask{ PipelineTask: &v1beta1.PipelineTask{}, }, - expected: &metav1.Duration{Duration: 20 * time.Minute}, + expected: &metav1.Duration{Duration: 20 * time.Minute}, + isTimeoutFromParent: true, }, { name: "taskrun being created after timeout expired", timeoutDuration: &metav1.Duration{Duration: 1 * time.Minute}, @@ -3449,7 +3487,8 @@ func TestGetFinallyTaskRunTimeout(t *testing.T) { }, }, }, - expected: &metav1.Duration{Duration: 11 * time.Minute}, + expected: &metav1.Duration{Duration: 11 * time.Minute}, + isTimeoutFromParent: true, }} for _, tc := range tcs { @@ -3467,9 +3506,13 @@ func TestGetFinallyTaskRunTimeout(t *testing.T) { }, }, } - if d := cmp.Diff(tc.expected, getFinallyTaskRunTimeout(context.TODO(), pr, tc.rpt, testClock)); d != "" { + trTimeout, timeoutFromParent := getFinallyTaskRunTimeout(context.TODO(), pr, tc.rpt, testClock) + if d := cmp.Diff(tc.expected, trTimeout); d != "" { t.Errorf("Unexpected finally task run timeout. Diff %s", diff.PrintWantGot(d)) } + if tc.isTimeoutFromParent != timeoutFromParent { + t.Errorf("Expected whether finally task run timeout is determined by PipelineRun timeout to be %t, but is %t", tc.isTimeoutFromParent, timeoutFromParent) + } }) } } @@ -3531,6 +3574,7 @@ spec: taskRef: name: hello-world timeout: 1h0m0s + timeoutFromParent: true `) if d := cmp.Diff(actual, expectedTaskRun, ignoreTypeMeta); d != "" { @@ -3724,6 +3768,7 @@ spec: taskRef: name: hello-world timeout: 1h0m0s + timeoutFromParent: false status: conditions: - status: "True" @@ -3759,6 +3804,7 @@ spec: taskRef: name: b-task timeout: 1h0m0s + timeoutFromParent: true `) // Check that the expected TaskRun was created actual, err := clients.Pipeline.TektonV1beta1().TaskRuns("foo").List(prt.TestAssets.Ctx, metav1.ListOptions{ @@ -3950,6 +3996,7 @@ spec: taskRef: name: %s timeout: 1h0m0s + timeoutFromParent: true `, taskName)) actual, err := clients.Pipeline.TektonV1beta1().TaskRuns("foo").List(prt.TestAssets.Ctx, metav1.ListOptions{ @@ -4628,6 +4675,7 @@ spec: taskRef: name: b-task timeout: 1h0m0s + timeoutFromParent: true `) // Check that the expected TaskRun was created actual, err := clients.Pipeline.TektonV1beta1().TaskRuns("foo").List(prt.TestAssets.Ctx, metav1.ListOptions{ @@ -4718,6 +4766,7 @@ spec: kind: Task name: a-task timeout: 1h0m0s + timeoutFromParent: true `) // Check that the expected TaskRun was created (only) actual, err := clients.Pipeline.TektonV1beta1().TaskRuns("foo").List(prt.TestAssets.Ctx, metav1.ListOptions{}) @@ -6228,6 +6277,7 @@ spec: taskRef: name: mytask timeout: 1h0m0s + timeoutFromParent: true status: conditions: - reason: Failed @@ -6259,6 +6309,7 @@ spec: taskRef: name: finaltask timeout: 1h0m0s + timeoutFromParent: true `) // Check that the expected TaskRun was created actual, err := clients.Pipeline.TektonV1beta1().TaskRuns("foo").List(prt.TestAssets.Ctx, metav1.ListOptions{ @@ -6438,6 +6489,7 @@ spec: taskRef: name: final-task timeout: 1h0m0s + timeoutFromParent: true `) // Check that the expected TaskRun was created @@ -6645,6 +6697,7 @@ spec: kind: Task name: unit-test-task timeout: 1h0m0s + timeoutFromParent: true `, ref)) if d := cmp.Diff(expectedTaskRun, actual, ignoreTypeMeta, cmpopts.SortSlices(lessTaskResourceBindings)); d != "" { @@ -6751,6 +6804,7 @@ spec: - name: ws optional: true timeout: 1h0m0s + timeoutFromParent: true `) if d := cmp.Diff(expectedTaskRun, actual, ignoreTypeMeta, cmpopts.SortSlices(lessTaskResourceBindings)); d != "" { @@ -7170,6 +7224,7 @@ func getTaskRunWithTaskSpec(tr, pr, p, t string, labels, annotations map[string] ServiceAccountName: config.DefaultServiceAccountValue, Resources: &v1beta1.TaskRunResources{}, Timeout: &metav1.Duration{Duration: config.DefaultTimeoutMinutes * time.Minute}, + TimeoutFromParent: true, }, } } @@ -7575,6 +7630,7 @@ spec: taskRef: name: hello-world timeout: 1h0m0s + timeoutFromParent: true `) if d := cmp.Diff(actual, expectedTaskRun, ignoreTypeMeta); d != "" { @@ -7648,6 +7704,7 @@ spec: - name: foo-step image: foo-image timeout: 1h0m0s + timeoutFromParent: true `) if d := cmp.Diff(actual, expectedTaskRun, ignoreTypeMeta); d != "" { @@ -7692,6 +7749,7 @@ spec: taskRef: name: mytask timeout: 1h0m0s + timeoutFromParent: true `), mustParseTaskRunWithObjectMeta(t, taskRunObjectMeta("pr-platforms-and-browsers-1", "foo", @@ -7710,6 +7768,7 @@ spec: taskRef: name: mytask timeout: 1h0m0s + timeoutFromParent: true `), mustParseTaskRunWithObjectMeta(t, taskRunObjectMeta("pr-platforms-and-browsers-2", "foo", @@ -7728,6 +7787,7 @@ spec: taskRef: name: mytask timeout: 1h0m0s + timeoutFromParent: true `), mustParseTaskRunWithObjectMeta(t, taskRunObjectMeta("pr-platforms-and-browsers-3", "foo", @@ -7746,6 +7806,7 @@ spec: taskRef: name: mytask timeout: 1h0m0s + timeoutFromParent: true `), mustParseTaskRunWithObjectMeta(t, taskRunObjectMeta("pr-platforms-and-browsers-4", "foo", @@ -7764,6 +7825,7 @@ spec: taskRef: name: mytask timeout: 1h0m0s + timeoutFromParent: true `), mustParseTaskRunWithObjectMeta(t, taskRunObjectMeta("pr-platforms-and-browsers-5", "foo", @@ -7782,6 +7844,7 @@ spec: taskRef: name: mytask timeout: 1h0m0s + timeoutFromParent: true `), mustParseTaskRunWithObjectMeta(t, taskRunObjectMeta("pr-platforms-and-browsers-6", "foo", @@ -7800,6 +7863,7 @@ spec: taskRef: name: mytask timeout: 1h0m0s + timeoutFromParent: true `), mustParseTaskRunWithObjectMeta(t, taskRunObjectMeta("pr-platforms-and-browsers-7", "foo", @@ -7818,6 +7882,7 @@ spec: taskRef: name: mytask timeout: 1h0m0s + timeoutFromParent: true `), mustParseTaskRunWithObjectMeta(t, taskRunObjectMeta("pr-platforms-and-browsers-8", "foo", @@ -7836,6 +7901,7 @@ spec: taskRef: name: mytask timeout: 1h0m0s + timeoutFromParent: true `), } cms := []*corev1.ConfigMap{withEmbeddedStatus(withEnabledAlphaAPIFields(newFeatureFlagsConfigMap()), config.MinimalEmbeddedStatus)} @@ -8223,6 +8289,7 @@ spec: taskRef: name: mytask timeout: 1h0m0s + timeoutFromParent: true `), mustParseTaskRunWithObjectMeta(t, taskRunObjectMeta("pr-platforms-and-browsers-1", "foo", @@ -8241,6 +8308,7 @@ spec: taskRef: name: mytask timeout: 1h0m0s + timeoutFromParent: true `), mustParseTaskRunWithObjectMeta(t, taskRunObjectMeta("pr-platforms-and-browsers-2", "foo", @@ -8259,6 +8327,7 @@ spec: taskRef: name: mytask timeout: 1h0m0s + timeoutFromParent: true `), mustParseTaskRunWithObjectMeta(t, taskRunObjectMeta("pr-platforms-and-browsers-3", "foo", @@ -8277,6 +8346,7 @@ spec: taskRef: name: mytask timeout: 1h0m0s + timeoutFromParent: true `), mustParseTaskRunWithObjectMeta(t, taskRunObjectMeta("pr-platforms-and-browsers-4", "foo", @@ -8295,6 +8365,7 @@ spec: taskRef: name: mytask timeout: 1h0m0s + timeoutFromParent: true `), mustParseTaskRunWithObjectMeta(t, taskRunObjectMeta("pr-platforms-and-browsers-5", "foo", @@ -8313,6 +8384,7 @@ spec: taskRef: name: mytask timeout: 1h0m0s + timeoutFromParent: true `), mustParseTaskRunWithObjectMeta(t, taskRunObjectMeta("pr-platforms-and-browsers-6", "foo", @@ -8331,6 +8403,7 @@ spec: taskRef: name: mytask timeout: 1h0m0s + timeoutFromParent: true `), mustParseTaskRunWithObjectMeta(t, taskRunObjectMeta("pr-platforms-and-browsers-7", "foo", @@ -8349,6 +8422,7 @@ spec: taskRef: name: mytask timeout: 1h0m0s + timeoutFromParent: true `), mustParseTaskRunWithObjectMeta(t, taskRunObjectMeta("pr-platforms-and-browsers-8", "foo", @@ -8367,6 +8441,7 @@ spec: taskRef: name: mytask timeout: 1h0m0s + timeoutFromParent: true `), } cms := []*corev1.ConfigMap{withEmbeddedStatus(withEnabledAlphaAPIFields(newFeatureFlagsConfigMap()), config.MinimalEmbeddedStatus)} @@ -8798,6 +8873,7 @@ spec: taskRef: name: mytask timeout: 1h0m0s + timeoutFromParent: true `), mustParseRunWithObjectMeta(t, taskRunObjectMeta("pr-platforms-and-browsers-1", "foo", @@ -8819,6 +8895,7 @@ spec: taskRef: name: mytask timeout: 1h0m0s + timeoutFromParent: true `), mustParseRunWithObjectMeta(t, taskRunObjectMeta("pr-platforms-and-browsers-2", "foo", @@ -8840,6 +8917,7 @@ spec: taskRef: name: mytask timeout: 1h0m0s + timeoutFromParent: true `), mustParseRunWithObjectMeta(t, taskRunObjectMeta("pr-platforms-and-browsers-3", "foo", @@ -8861,6 +8939,7 @@ spec: taskRef: name: mytask timeout: 1h0m0s + timeoutFromParent: true `), mustParseRunWithObjectMeta(t, taskRunObjectMeta("pr-platforms-and-browsers-4", "foo", @@ -8882,6 +8961,7 @@ spec: taskRef: name: mytask timeout: 1h0m0s + timeoutFromParent: true `), mustParseRunWithObjectMeta(t, taskRunObjectMeta("pr-platforms-and-browsers-5", "foo", @@ -8903,6 +8983,7 @@ spec: taskRef: name: mytask timeout: 1h0m0s + timeoutFromParent: true `), mustParseRunWithObjectMeta(t, taskRunObjectMeta("pr-platforms-and-browsers-6", "foo", @@ -8924,6 +9005,7 @@ spec: taskRef: name: mytask timeout: 1h0m0s + timeoutFromParent: true `), mustParseRunWithObjectMeta(t, taskRunObjectMeta("pr-platforms-and-browsers-7", "foo", @@ -8945,6 +9027,7 @@ spec: taskRef: name: mytask timeout: 1h0m0s + timeoutFromParent: true `), mustParseRunWithObjectMeta(t, taskRunObjectMeta("pr-platforms-and-browsers-8", "foo", @@ -8966,6 +9049,7 @@ spec: taskRef: name: mytask timeout: 1h0m0s + timeoutFromParent: true `), } cms := []*corev1.ConfigMap{withEmbeddedStatus(withEnabledAlphaAPIFields(newFeatureFlagsConfigMap()), config.MinimalEmbeddedStatus)} diff --git a/pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go b/pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go index 55fc974bbaf..4f651beeb6b 100644 --- a/pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go +++ b/pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go @@ -30,6 +30,7 @@ import ( "github.com/tektoncd/pipeline/pkg/reconciler/taskrun/resources" "github.com/tektoncd/pipeline/pkg/remote" kerrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/util/clock" "knative.dev/pkg/apis" "knative.dev/pkg/kmeta" ) @@ -195,6 +196,43 @@ func (t ResolvedPipelineTask) isFailure() bool { return isDone && c.IsFalse() && !t.hasRemainingRetries() } +// isTimedOutDueToPipelineRun returns true only if the run has timed out, and its timeout was set to the PipelineRun's timeout. +// If the PipelineTask has a Matrix, isSuccessful returns true if any of the runs satisfied this condition. +func (t ResolvedPipelineTask) isTimedOutDueToPipelineRun(ctx context.Context, c clock.PassiveClock) bool { + switch { + case t.IsCustomTask() && t.IsMatrixed(): + if len(t.Runs) == 0 { + return false + } + for _, run := range t.Runs { + if run.HasTimedOut(c) && run.Spec.TimeoutFromParent { + return true + } + } + return false + case t.IsCustomTask(): + if t.Run == nil { + return false + } + return t.Run.HasTimedOut(c) && t.Run.Spec.TimeoutFromParent + case t.IsMatrixed(): + if len(t.TaskRuns) == 0 { + return false + } + for _, taskRun := range t.TaskRuns { + if taskRun.HasTimedOut(ctx, c) && taskRun.Spec.TimeoutFromParent { + return true + } + } + return false + default: + if t.TaskRun == nil { + return false + } + return t.TaskRun.HasTimedOut(ctx, c) && t.TaskRun.Spec.TimeoutFromParent + } +} + // hasRemainingRetries returns true only when the number of retries already attempted // is less than the number of retries allowed. func (t ResolvedPipelineTask) hasRemainingRetries() bool { diff --git a/pkg/reconciler/pipelinerun/resources/pipelinerunresolution_test.go b/pkg/reconciler/pipelinerun/resources/pipelinerunresolution_test.go index 02a2f14819d..db55e12d03d 100644 --- a/pkg/reconciler/pipelinerun/resources/pipelinerunresolution_test.go +++ b/pkg/reconciler/pipelinerun/resources/pipelinerunresolution_test.go @@ -22,6 +22,7 @@ import ( "fmt" "sort" "testing" + "time" "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" @@ -274,6 +275,33 @@ func makeFailed(tr v1beta1.TaskRun) *v1beta1.TaskRun { return newTr } +func makeFailedDueToPipelineRunTimeout(tr v1beta1.TaskRun, startTime time.Time) *v1beta1.TaskRun { + newTr := newTaskRun(tr) + newTr.Spec.Timeout = &metav1.Duration{Duration: 3 * time.Second} + newTr.Spec.TimeoutFromParent = true + newTr.Status.Conditions[0].Status = corev1.ConditionFalse + newTr.Status.StartTime = &metav1.Time{Time: startTime} + return newTr +} + +func allFinishedWithOneTimedOutDueToPipelineRunState(startTime time.Time) PipelineRunState { + return PipelineRunState{{ + PipelineTask: &pts[0], + TaskRunName: "pipelinerun-mytask1", + TaskRun: makeFailedDueToPipelineRunTimeout(trs[0], startTime), + ResolvedTaskResources: &resources.ResolvedTaskResources{ + TaskSpec: &task.Spec, + }, + }, { + PipelineTask: &pts[1], + TaskRunName: "pipelinerun-mytask2", + TaskRun: makeSucceeded(trs[0]), + ResolvedTaskResources: &resources.ResolvedTaskResources{ + TaskSpec: &task.Spec, + }, + }} +} + func makeRunFailed(run v1alpha1.Run) *v1alpha1.Run { newRun := newRun(run) newRun.Status.Conditions[0].Status = corev1.ConditionFalse diff --git a/pkg/reconciler/pipelinerun/resources/pipelinerunstate.go b/pkg/reconciler/pipelinerun/resources/pipelinerunstate.go index 365593c80d3..2d97ac7d44f 100644 --- a/pkg/reconciler/pipelinerun/resources/pipelinerunstate.go +++ b/pkg/reconciler/pipelinerun/resources/pipelinerunstate.go @@ -79,6 +79,8 @@ type pipelineRunStatusCount struct { Cancelled int // number of tasks which are still pending, have not executed Incomplete int + // number of tasks which timed out due to the PipelineRun's timeout + TimedOutDueToPipelineRun int } // ResetSkippedCache resets the skipped cache in the facts map @@ -449,7 +451,7 @@ func (facts *PipelineRunFacts) GetPipelineConditionStatus(ctx context.Context, p // report the count in PipelineRun Status // get the count of successful tasks, failed tasks, cancelled tasks, skipped task, and incomplete tasks - s := facts.getPipelineTasksCount() + s := facts.getPipelineTasksCount(ctx, c) // completed task is a collection of successful, failed, cancelled tasks (skipped tasks are reported separately) cmTasks := s.Succeeded + s.Failed + s.Cancelled @@ -469,6 +471,11 @@ func (facts *PipelineRunFacts) GetPipelineConditionStatus(ctx context.Context, p } switch { + case s.TimedOutDueToPipelineRun > 0: + // Set reason to ReasonTimedOut - At least one timed out due to the PipelineRun's timeout + reason = v1beta1.PipelineRunReasonTimedOut.String() + status = corev1.ConditionFalse + message = fmt.Sprintf("PipelineRun %q failed to finish within %q", pr.Name, pr.PipelineTimeout(ctx).String()) case s.Failed > 0: // Set reason to ReasonFailed - At least one failed reason = v1beta1.PipelineRunReasonFailed.String() @@ -630,19 +637,23 @@ func (facts *PipelineRunFacts) checkFinalTasksDone() bool { } // getPipelineTasksCount returns the count of successful tasks, failed tasks, cancelled tasks, skipped task, and incomplete tasks -func (facts *PipelineRunFacts) getPipelineTasksCount() pipelineRunStatusCount { +func (facts *PipelineRunFacts) getPipelineTasksCount(ctx context.Context, c clock.PassiveClock) pipelineRunStatusCount { s := pipelineRunStatusCount{ - Skipped: 0, - Succeeded: 0, - Failed: 0, - Cancelled: 0, - Incomplete: 0, + Skipped: 0, + Succeeded: 0, + Failed: 0, + Cancelled: 0, + Incomplete: 0, + TimedOutDueToPipelineRun: 0, } for _, t := range facts.State { switch { // increment success counter since the task is successful case t.isSuccessful(): s.Succeeded++ + // increment timed out counter due to PipelineRun counters if the task timed out due to the PipelineRun timeout + case t.isTimedOutDueToPipelineRun(ctx, c): + s.TimedOutDueToPipelineRun++ // increment cancelled counter since the task is cancelled case t.isCancelled(): s.Cancelled++ diff --git a/pkg/reconciler/pipelinerun/resources/pipelinerunstate_test.go b/pkg/reconciler/pipelinerun/resources/pipelinerunstate_test.go index f269dd27f08..c3125d57eab 100644 --- a/pkg/reconciler/pipelinerun/resources/pipelinerunstate_test.go +++ b/pkg/reconciler/pipelinerun/resources/pipelinerunstate_test.go @@ -1962,6 +1962,35 @@ func TestGetPipelineConditionStatus_PipelineTimeoutDeprecated(t *testing.T) { } } +// pipeline should result in timeout if all tasks are completed and at least one of them timed out due to the PipelineRun timeout +func TestGetPipelineConditionStatus_PipelineTimeoutDeprecated_PRTimeoutTask(t *testing.T) { + testState := allFinishedWithOneTimedOutDueToPipelineRunState(now.Add(-4 * time.Second)) + d, err := dagFromState(testState) + if err != nil { + t.Fatalf("Unexpected error while building DAG for state %v: %v", testState, err) + } + pr := &v1beta1.PipelineRun{ + ObjectMeta: metav1.ObjectMeta{Name: "pipelinerun-no-tasks-started"}, + Spec: v1beta1.PipelineRunSpec{ + Timeout: &metav1.Duration{Duration: 121 * time.Second}, + }, + Status: v1beta1.PipelineRunStatus{ + PipelineRunStatusFields: v1beta1.PipelineRunStatusFields{ + StartTime: &metav1.Time{Time: now.Add(-2 * time.Minute)}, + }, + }, + } + facts := PipelineRunFacts{ + State: testState, + 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, testState) + } +} + // 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) @@ -1992,6 +2021,37 @@ func TestGetPipelineConditionStatus_PipelineTimeouts(t *testing.T) { } } +// pipeline should result in timeout if all tasks are completed and at least one of them timed out due to the PipelineRun timeout +func TestGetPipelineConditionStatus_PipelineTimeouts_PRTimeoutTask(t *testing.T) { + testState := allFinishedWithOneTimedOutDueToPipelineRunState(now.Add(-4 * time.Second)) + d, err := dagFromState(testState) + if err != nil { + t.Fatalf("Unexpected error while building DAG for state %v: %v", testState, err) + } + pr := &v1beta1.PipelineRun{ + ObjectMeta: metav1.ObjectMeta{Name: "pipelinerun-no-tasks-started"}, + Spec: v1beta1.PipelineRunSpec{ + Timeouts: &v1beta1.TimeoutFields{ + Pipeline: &metav1.Duration{Duration: 121 * time.Second}, + }, + }, + Status: v1beta1.PipelineRunStatus{ + PipelineRunStatusFields: v1beta1.PipelineRunStatusFields{ + StartTime: &metav1.Time{Time: now.Add(-2 * time.Minute)}, + }, + }, + } + facts := PipelineRunFacts{ + State: testState, + 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, testState) + } +} + func TestAdjustStartTime(t *testing.T) { baseline := metav1.Time{Time: now}