Skip to content

Commit

Permalink
Implement 0 as no timeout for TaskRun
Browse files Browse the repository at this point in the history
If the timeout is set to 0, there is no timeout for the TaskRun.
If the timeout is empty, the default timeout 60 mins will be applied.
If the timeout is set to -1, the taskrun is created in pipelinerun after
it timed out.
  • Loading branch information
Vincent Hou committed Jul 3, 2019
1 parent 7768cf4 commit 34354d9
Show file tree
Hide file tree
Showing 4 changed files with 48 additions and 6 deletions.
13 changes: 11 additions & 2 deletions docs/taskruns.md
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,10 @@ following fields:
- [`inputs`] - Specifies [input parameters](#input-parameters) and
[input resources](#providing-resources)
- [`outputs`] - Specifies [output resources](#providing-resources)
- `timeout` - Specifies timeout after which the `TaskRun` will fail. Defaults
to 60 minutes.
- `timeout` - Specifies timeout after which the `TaskRun` will fail. If the value of
`timeout` is empty, the default timeout will be applied. If the value is set to 0,
there is no timeout. You can also follow the instruction [here](#Configuring-default-timeout)
to configure the default timeout.
- [`nodeSelector`] - a selector which must be true for the pod to fit on a
node. The selector which must match a node's labels for the pod to be
scheduled on that node. More info:
Expand Down Expand Up @@ -145,6 +147,13 @@ spec:
value: https://github.com/pivotal-nader-ziada/gohelloworld
```

### Configuring Default Timeout

You can configure the default timeout by changing the value of `default-timeout-minutes`
in [`config/config-defaults.yaml`](./../config/config-defaults.yaml). The default timeout
is 60 minutes, if `default-timeout-minutes` is not available. There is no timeout by
default, if `default-timeout-minutes` is set to 0.

### Service Account

Specifies the `name` of a `ServiceAccount` resource object. Use the
Expand Down
33 changes: 31 additions & 2 deletions pkg/reconciler/timeout_handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -291,15 +291,44 @@ func TestGetTimeout(t *testing.T) {
inputDuration: &metav1.Duration{Duration: 2 * time.Second},
expectedDuration: 2 * time.Second,
},
{
description: "returns 0 if the timeout duration is set to 0 second/minute/hour",
inputDuration: &metav1.Duration{Duration: 0 * time.Second},
expectedDuration: 0 * time.Minute,
},
{
description: "returns -1 if it has timed out",
inputDuration: &metav1.Duration{Duration: -1 * time.Second},
expectedDuration: -1 * time.Second,
},
}
for _, tc := range testCases {
t.Run(tc.description, func(t *testing.T) {
receivedDuration := GetTimeout(tc.inputDuration, 60)
if receivedDuration != tc.expectedDuration {
t.Errorf("expected %q received %q", tc.expectedDuration.String(), receivedDuration.String())
}
})
}
verifyTimeout(0, t)
verifyTimeout(60, t)
}

func verifyTimeout(defaultTimeout int, t *testing.T) {
testCases := []struct {
description string
inputDuration *metav1.Duration
expectedDuration time.Duration
}{
{
description: "returns an end time using the default timeout if a TaskRun's timeout is nil",
inputDuration: nil,
expectedDuration: 60 * time.Minute,
expectedDuration: time.Duration(defaultTimeout) * time.Minute,
},
}
for _, tc := range testCases {
t.Run(tc.description, func(t *testing.T) {
receivedDuration := GetTimeout(tc.inputDuration, 60)
receivedDuration := GetTimeout(tc.inputDuration, defaultTimeout)
if receivedDuration != tc.expectedDuration {
t.Errorf("expected %q received %q", tc.expectedDuration.String(), receivedDuration.String())
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/reconciler/v1alpha1/pipelinerun/pipelinerun.go
Original file line number Diff line number Diff line change
Expand Up @@ -445,10 +445,10 @@ func (c *Reconciler) createTaskRun(logger *zap.SugaredLogger, rprt *resources.Re
pTimeoutTime := pr.Status.StartTime.Add(pr.Spec.Timeout.Duration)
if time.Now().After(pTimeoutTime) {
// Just in case something goes awry and we're creating the TaskRun after it should have already timed out,
// set a timeout of 0.
// set a timeout of -1.
taskRunTimeout = &metav1.Duration{Duration: time.Until(pTimeoutTime)}
if taskRunTimeout.Duration < 0 {
taskRunTimeout = &metav1.Duration{Duration: 0 * time.Second}
taskRunTimeout = &metav1.Duration{Duration: -1 * time.Second}
}
} else {
taskRunTimeout = pr.Spec.Timeout
Expand Down
4 changes: 4 additions & 0 deletions pkg/reconciler/v1alpha1/taskrun/taskrun.go
Original file line number Diff line number Diff line change
Expand Up @@ -542,6 +542,10 @@ func (c *Reconciler) checkTimeout(tr *v1alpha1.TaskRun, ts *v1alpha1.TaskSpec, d

cfg := c.store.Load()
timeout := reconciler.GetTimeout(tr.Spec.Timeout, cfg.ConfigDefault.DefaultTimeoutMinutes)
// If timeout is set to 0 or defaulted to 0, there is no timeout.
if timeout == 0 * time.Minute {
return false, nil
}
runtime := time.Since(tr.Status.StartTime.Time)
c.Logger.Infof("Checking timeout for TaskRun %q (startTime %s, timeout %s, runtime %s)", tr.Name, tr.Status.StartTime, timeout, runtime)

Expand Down

0 comments on commit 34354d9

Please sign in to comment.