Skip to content

Commit 9d5768a

Browse files
soulebsouleb
committed
Add a TaskTimeout optional field to pipelinerun type
TaskTimeout is used to timeout all dag tasks, finally tasks excluded Validates a TasksTimeout if: - TasksTimeout > 0 - Timeout is specified and TasksTimeout <= Timeout - Timeout not specified and TasksTimeout <= default Timeout Add a builder function for taskTimeout Defines 2 functions to get taskrun timeout One for dag tasks and one specifically for finally tasks
1 parent 02f5f42 commit 9d5768a

File tree

10 files changed

+295
-14
lines changed

10 files changed

+295
-14
lines changed

‎internal/builder/v1beta1/pipeline.go‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -529,6 +529,13 @@ func PipelineRunNilTimeout(prs *v1beta1.PipelineRunSpec) {
529529
prs.Timeout = nil
530530
}
531531

532+
// PipelineRunTasksTimeout sets the timeout to the PipelineRunSpec.
533+
func PipelineRunTasksTimeout(duration time.Duration) PipelineRunSpecOp {
534+
return func(prs *v1beta1.PipelineRunSpec) {
535+
prs.TasksTimeout = &metav1.Duration{Duration: duration}
536+
}
537+
}
538+
532539
// PipelineRunNodeSelector sets the Node selector to the PipelineRunSpec.
533540
func PipelineRunNodeSelector(values map[string]string) PipelineRunSpecOp {
534541
return func(prs *v1beta1.PipelineRunSpec) {

‎internal/builder/v1beta1/pipeline_test.go‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -179,6 +179,7 @@ func TestPipelineRun(t *testing.T) {
179179
tb.PipelineRunParam("first-param-string", "first-value"),
180180
tb.PipelineRunParam("second-param-array", "some", "array"),
181181
tb.PipelineRunTimeout(1*time.Hour),
182+
tb.PipelineRunTasksTimeout(50*time.Minute),
182183
tb.PipelineRunResourceBinding("some-resource", tb.PipelineResourceBindingRef("my-special-resource")),
183184
tb.PipelineRunServiceAccountNameTask("foo", "sa-2"),
184185
tb.PipelineRunPipelineRefBundle("/some/registry"),
@@ -209,7 +210,8 @@ func TestPipelineRun(t *testing.T) {
209210
Name: "second-param-array",
210211
Value: *v1beta1.NewArrayOrString("some", "array"),
211212
}},
212-
Timeout: &metav1.Duration{Duration: 1 * time.Hour},
213+
Timeout: &metav1.Duration{Duration: 1 * time.Hour},
214+
TasksTimeout: &metav1.Duration{Duration: 50 * time.Minute},
213215
Resources: []v1beta1.PipelineResourceBinding{{
214216
Name: "some-resource",
215217
ResourceRef: &v1beta1.PipelineResourceRef{
@@ -244,6 +246,7 @@ func TestPipelineRunWithPodTemplate(t *testing.T) {
244246
tb.PipelineRunParam("first-param-string", "first-value"),
245247
tb.PipelineRunParam("second-param-array", "some", "array"),
246248
tb.PipelineRunTimeout(1*time.Hour),
249+
tb.PipelineRunTasksTimeout(50*time.Minute),
247250
tb.PipelineRunResourceBinding("some-resource", tb.PipelineResourceBindingRef("my-special-resource")),
248251
tb.PipelineRunServiceAccountNameTask("foo", "sa-2"),
249252
tb.PipelineRunNodeSelector(map[string]string{
@@ -276,7 +279,8 @@ func TestPipelineRunWithPodTemplate(t *testing.T) {
276279
Name: "second-param-array",
277280
Value: *v1beta1.NewArrayOrString("some", "array"),
278281
}},
279-
Timeout: &metav1.Duration{Duration: 1 * time.Hour},
282+
Timeout: &metav1.Duration{Duration: 1 * time.Hour},
283+
TasksTimeout: &metav1.Duration{Duration: 50 * time.Minute},
280284
Resources: []v1beta1.PipelineResourceBinding{{
281285
Name: "some-resource",
282286
ResourceRef: &v1beta1.PipelineResourceRef{
@@ -316,6 +320,7 @@ func TestPipelineRunWithResourceSpec(t *testing.T) {
316320
tb.PipelineRunParam("first-param-string", "first-value"),
317321
tb.PipelineRunParam("second-param-array", "some", "array"),
318322
tb.PipelineRunTimeout(1*time.Hour),
323+
tb.PipelineRunTasksTimeout(50*time.Minute),
319324
tb.PipelineRunResourceBinding("some-resource",
320325
tb.PipelineResourceBindingResourceSpec(&resource.PipelineResourceSpec{
321326
Type: v1beta1.PipelineResourceTypeGit,
@@ -351,7 +356,8 @@ func TestPipelineRunWithResourceSpec(t *testing.T) {
351356
Name: "second-param-array",
352357
Value: *v1beta1.NewArrayOrString("some", "array"),
353358
}},
354-
Timeout: &metav1.Duration{Duration: 1 * time.Hour},
359+
Timeout: &metav1.Duration{Duration: 1 * time.Hour},
360+
TasksTimeout: &metav1.Duration{Duration: 50 * time.Minute},
355361
Resources: []v1beta1.PipelineResourceBinding{{
356362
Name: "some-resource",
357363
ResourceSpec: &resource.PipelineResourceSpec{

‎pkg/apis/pipeline/v1beta1/pipelinerun_types.go‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -174,6 +174,10 @@ type PipelineRunSpec struct {
174174
// Used for cancelling a pipelinerun (and maybe more later on)
175175
// +optional
176176
Status PipelineRunSpecStatus `json:"status,omitempty"`
177+
// Time after which the Pipeline tasks time out.
178+
// Finally tasks can run beyond this as they are bound to the pipeline timeout.
179+
// +optional
180+
TasksTimeout *metav1.Duration `json:"tasksTimeout,omitempty"`
177181
// Time after which the Pipeline times out. Defaults to never.
178182
// Refer to Go's ParseDuration documentation for expected format: https://golang.org/pkg/time/#ParseDuration
179183
// +optional

‎pkg/apis/pipeline/v1beta1/pipelinerun_validation.go‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ package v1beta1
1919
import (
2020
"context"
2121
"fmt"
22+
"time"
2223

2324
"github.com/google/go-containerregistry/pkg/name"
2425
"github.com/tektoncd/pipeline/pkg/apis/config"
@@ -82,6 +83,24 @@ func (ps *PipelineRunSpec) Validate(ctx context.Context) (errs *apis.FieldError)
8283
}
8384
}
8485

86+
if ps.TasksTimeout != nil {
87+
// tasksTimeout should be a valid duration of at least 0.
88+
if ps.TasksTimeout.Duration < 0 {
89+
errs = errs.Also(apis.ErrInvalidValue(fmt.Sprintf("%s should be >= 0", ps.TasksTimeout.Duration.String()), "tasksTimeout"))
90+
}
91+
92+
if ps.Timeout != nil {
93+
if ps.TasksTimeout.Duration > ps.Timeout.Duration {
94+
errs = errs.Also(apis.ErrInvalidValue(fmt.Sprintf("%s should be <= timeout duration", ps.TasksTimeout.Duration.String()), "tasksTimeout"))
95+
}
96+
} else {
97+
defaultTimeout := time.Duration(config.FromContextOrDefaults(ctx).Defaults.DefaultTimeoutMinutes)
98+
if ps.TasksTimeout.Duration > defaultTimeout {
99+
errs = errs.Also(apis.ErrInvalidValue(fmt.Sprintf("%s should be <= default timeout duration", ps.TasksTimeout.Duration.String()), "tasksTimeout"))
100+
}
101+
}
102+
}
103+
85104
if ps.Status != "" {
86105
if ps.Status != PipelineRunSpecStatusCancelled && ps.Status != PipelineRunSpecStatusPending {
87106
errs = errs.Also(apis.ErrInvalidValue(fmt.Sprintf("%s should be %s or %s", ps.Status, PipelineRunSpecStatusCancelled, PipelineRunSpecStatusPending), "status"))

‎pkg/apis/pipeline/v1beta1/pipelinerun_validation_test.go‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,35 @@ func TestPipelineRun_Invalid(t *testing.T) {
7979
},
8080
},
8181
want: apis.ErrInvalidValue("-48h0m0s should be >= 0", "spec.timeout"),
82+
}, {
83+
name: "negative pipeline tasksTimeout",
84+
pr: v1beta1.PipelineRun{
85+
ObjectMeta: metav1.ObjectMeta{
86+
Name: "pipelinelineName",
87+
},
88+
Spec: v1beta1.PipelineRunSpec{
89+
PipelineRef: &v1beta1.PipelineRef{
90+
Name: "prname",
91+
},
92+
TasksTimeout: &metav1.Duration{Duration: -48 * time.Hour},
93+
},
94+
},
95+
want: apis.ErrInvalidValue("-48h0m0s should be >= 0", "spec.tasksTimeout"),
96+
}, {
97+
name: "pipeline tasksTimeout > pipeline Timeout",
98+
pr: v1beta1.PipelineRun{
99+
ObjectMeta: metav1.ObjectMeta{
100+
Name: "pipelinelineName",
101+
},
102+
Spec: v1beta1.PipelineRunSpec{
103+
PipelineRef: &v1beta1.PipelineRef{
104+
Name: "prname",
105+
},
106+
TasksTimeout: &metav1.Duration{Duration: 1 * time.Hour},
107+
Timeout: &metav1.Duration{Duration: 25 * time.Minute},
108+
},
109+
},
110+
want: apis.ErrInvalidValue("1h0m0s should be <= Timeout duration", "spec.tasksTimeout"),
82111
}, {
83112
name: "wrong pipelinerun cancel",
84113
pr: v1beta1.PipelineRun{
@@ -206,6 +235,19 @@ func TestPipelineRun_Validate(t *testing.T) {
206235
Timeout: &metav1.Duration{Duration: 0},
207236
},
208237
},
238+
}, {
239+
name: "no tasksTimeout",
240+
pr: v1beta1.PipelineRun{
241+
ObjectMeta: metav1.ObjectMeta{
242+
Name: "pipelinelineName",
243+
},
244+
Spec: v1beta1.PipelineRunSpec{
245+
PipelineRef: &v1beta1.PipelineRef{
246+
Name: "prname",
247+
},
248+
TasksTimeout: &metav1.Duration{Duration: 0},
249+
},
250+
},
209251
}, {
210252
name: "array param with pipelinespec and taskspec",
211253
pr: v1beta1.PipelineRun{

‎pkg/apis/pipeline/v1beta1/zz_generated.deepcopy.go‎

Lines changed: 5 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎pkg/reconciler/pipelinerun/pipelinerun.go‎

Lines changed: 50 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -641,7 +641,11 @@ func (c *Reconciler) runNextSchedulableTask(ctx context.Context, pr *v1beta1.Pip
641641
return fmt.Errorf("error creating Run called %s for PipelineTask %s from PipelineRun %s: %w", rprt.RunName, rprt.PipelineTask.Name, pr.Name, err)
642642
}
643643
} else {
644-
rprt.TaskRun, err = c.createTaskRun(ctx, rprt, pr, as.StorageBasePath(pr))
644+
if rprt.IsFinalTask(pipelineRunFacts) {
645+
rprt.TaskRun, err = c.createFinallyTaskRun(ctx, rprt, pr, as.StorageBasePath(pr))
646+
} else {
647+
rprt.TaskRun, err = c.createTaskRun(ctx, rprt, pr, as.StorageBasePath(pr))
648+
}
645649
if err != nil {
646650
recorder.Eventf(pr, corev1.EventTypeWarning, "TaskRunCreationFailed", "Failed to create TaskRun %q: %v", rprt.TaskRunName, err)
647651
return fmt.Errorf("error creating TaskRun called %s for PipelineTask %s from PipelineRun %s: %w", rprt.TaskRunName, rprt.PipelineTask.Name, pr.Name, err)
@@ -692,7 +696,17 @@ func (c *Reconciler) updateRunsStatusDirectly(pr *v1beta1.PipelineRun) error {
692696
return nil
693697
}
694698

699+
type getTimeoutFunc func(ctx context.Context, pr *v1beta1.PipelineRun, rprt *resources.ResolvedPipelineRunTask) *metav1.Duration
700+
695701
func (c *Reconciler) createTaskRun(ctx context.Context, rprt *resources.ResolvedPipelineRunTask, pr *v1beta1.PipelineRun, storageBasePath string) (*v1beta1.TaskRun, error) {
702+
return c.createTaskRunHelper(ctx, rprt, pr, storageBasePath, getTaskRunTimeout)
703+
}
704+
705+
func (c *Reconciler) createFinallyTaskRun(ctx context.Context, rprt *resources.ResolvedPipelineRunTask, pr *v1beta1.PipelineRun, storageBasePath string) (*v1beta1.TaskRun, error) {
706+
return c.createTaskRunHelper(ctx, rprt, pr, storageBasePath, getFinallyTaskRunTimeout)
707+
}
708+
709+
func (c *Reconciler) createTaskRunHelper(ctx context.Context, rprt *resources.ResolvedPipelineRunTask, pr *v1beta1.PipelineRun, storageBasePath string, getTimeoutFunc getTimeoutFunc) (*v1beta1.TaskRun, error) {
696710
logger := logging.FromContext(ctx)
697711

698712
tr, _ := c.taskRunLister.TaskRuns(pr.Namespace).Get(rprt.TaskRunName)
@@ -719,7 +733,7 @@ func (c *Reconciler) createTaskRun(ctx context.Context, rprt *resources.Resolved
719733
Spec: v1beta1.TaskRunSpec{
720734
Params: rprt.PipelineTask.Params,
721735
ServiceAccountName: taskRunSpec.TaskServiceAccountName,
722-
Timeout: getTaskRunTimeout(ctx, pr, rprt),
736+
Timeout: getTimeoutFunc(ctx, pr, rprt),
723737
PodTemplate: taskRunSpec.TaskPodTemplate,
724738
}}
725739

@@ -926,30 +940,58 @@ func combineTaskRunAndTaskSpecAnnotations(pr *v1beta1.PipelineRun, pipelineTask
926940
return annotations
927941
}
928942

929-
func getTaskRunTimeout(ctx context.Context, pr *v1beta1.PipelineRun, rprt *resources.ResolvedPipelineRunTask) *metav1.Duration {
943+
func getFinallyTaskRunTimeout(ctx context.Context, pr *v1beta1.PipelineRun, rprt *resources.ResolvedPipelineRunTask) *metav1.Duration {
930944
var taskRunTimeout = &metav1.Duration{Duration: apisconfig.NoTimeoutDuration}
931945

932946
var timeout time.Duration
933-
if pr.Spec.Timeout == nil {
947+
948+
if pr.Spec.Timeout != nil {
949+
timeout = pr.Spec.Timeout.Duration
950+
} else {
934951
defaultTimeout := time.Duration(config.FromContextOrDefaults(ctx).Defaults.DefaultTimeoutMinutes)
935952
timeout = defaultTimeout * time.Minute
936-
} else {
953+
}
954+
955+
// If the value of the timeout is 0 for any resource, there is no timeout.
956+
// It is impossible for pr.Spec.Timeout to be nil, since SetDefault always assigns it with a value.
957+
taskRunTimeout = taskRunTimeoutHelper(timeout, pr, taskRunTimeout, rprt)
958+
959+
return taskRunTimeout
960+
}
961+
962+
func getTaskRunTimeout(ctx context.Context, pr *v1beta1.PipelineRun, rprt *resources.ResolvedPipelineRunTask) *metav1.Duration {
963+
var taskRunTimeout = &metav1.Duration{Duration: apisconfig.NoTimeoutDuration}
964+
965+
var timeout time.Duration
966+
967+
// TODO @souleb wrap into alpha feature flag
968+
if pr.Spec.TasksTimeout != nil {
969+
timeout = pr.Spec.TasksTimeout.Duration
970+
} else if pr.Spec.Timeout != nil {
937971
timeout = pr.Spec.Timeout.Duration
972+
} else {
973+
defaultTimeout := time.Duration(config.FromContextOrDefaults(ctx).Defaults.DefaultTimeoutMinutes)
974+
timeout = defaultTimeout * time.Minute
938975
}
939976

940977
// If the value of the timeout is 0 for any resource, there is no timeout.
941978
// It is impossible for pr.Spec.Timeout to be nil, since SetDefault always assigns it with a value.
979+
taskRunTimeout = taskRunTimeoutHelper(timeout, pr, taskRunTimeout, rprt)
980+
981+
return taskRunTimeout
982+
}
983+
984+
func taskRunTimeoutHelper(timeout time.Duration, pr *v1beta1.PipelineRun, taskRunTimeout *metav1.Duration, rprt *resources.ResolvedPipelineRunTask) *metav1.Duration {
942985
if timeout != apisconfig.NoTimeoutDuration {
943986
pTimeoutTime := pr.Status.StartTime.Add(timeout)
944987
if time.Now().After(pTimeoutTime) {
945-
// Just in case something goes awry and we're creating the TaskRun after it should have already timed out,
946-
// set the timeout to 1 second.
988+
947989
taskRunTimeout = &metav1.Duration{Duration: time.Until(pTimeoutTime)}
948990
if taskRunTimeout.Duration < 0 {
949991
taskRunTimeout = &metav1.Duration{Duration: 1 * time.Second}
950992
}
951993
} else {
952-
// check if PipelineTask has a timeout specified
994+
953995
if rprt.PipelineTask.Timeout != nil {
954996
taskRunTimeout = &metav1.Duration{Duration: rprt.PipelineTask.Timeout.Duration}
955997
} else {
@@ -958,11 +1000,9 @@ func getTaskRunTimeout(ctx context.Context, pr *v1beta1.PipelineRun, rprt *resou
9581000
}
9591001
}
9601002

961-
// check if PipelineTask has a timeout specified even if PipelineRun doesn't have a timeout
9621003
if timeout == apisconfig.NoTimeoutDuration && rprt.PipelineTask.Timeout != nil {
9631004
taskRunTimeout = &metav1.Duration{Duration: rprt.PipelineTask.Timeout.Duration}
9641005
}
965-
9661006
return taskRunTimeout
9671007
}
9681008

0 commit comments

Comments
 (0)