Skip to content

Commit

Permalink
Implement 0 as no timeout for TaskRun and PipelineRun
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 and PipelineRun.
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 8, 2019
1 parent 196a945 commit 566012a
Show file tree
Hide file tree
Showing 8 changed files with 33 additions and 12 deletions.
2 changes: 1 addition & 1 deletion config/config-defaults.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -35,5 +35,5 @@ data:
# to actually change the configuration.
# default-timeout-minutes contains the default number of
# minutes to use for TaskRun, if none is specified.
# minutes to use for TaskRun and PipelineRun, if none is specified.
default-timeout-minutes: "60" # 60 minutes
6 changes: 5 additions & 1 deletion docs/pipelineruns.md
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,11 @@ following fields:
information.
- [`serviceAccounts`](#service-accounts) - Specifies a list of `ServiceAccount`
and `PipelineTask` pairs that enable you to overwrite `ServiceAccount` for concrete `PipelineTask`.
- `timeout` - Specifies timeout after which the `PipelineRun` will fail.
- [`timeout`] - Specifies timeout after which the `PipelineRun` 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. `PipelineRun` shares the same default timouet as `TaskRun`. You can
follow the instruction [here](taskruns.md#Configuring-default-timeout) to configure the
default timeout, the same way as `TaskRun`.
- [`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
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
2 changes: 2 additions & 0 deletions pkg/apis/config/default.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ package config
import (
"fmt"
"strconv"
"time"

corev1 "k8s.io/api/core/v1"
)
Expand All @@ -27,6 +28,7 @@ const (
// ConfigName is the name of the configmap
DefaultsConfigName = "config-defaults"
DefaultTimeoutMinutes = 60
NoTimeoutDuration = 0 * time.Minute
defaultTimeoutMinutesKey = "default-timeout-minutes"
)

Expand Down
4 changes: 2 additions & 2 deletions pkg/apis/pipeline/v1alpha1/pipelinerun_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,8 @@ func (ps *PipelineRunSpec) Validate(ctx context.Context) *apis.FieldError {

if ps.Timeout != nil {
// timeout should be a valid duration of at least 0.
if ps.Timeout.Duration <= 0 {
return apis.ErrInvalidValue(fmt.Sprintf("%s should be > 0", ps.Timeout.Duration.String()), "spec.timeout")
if ps.Timeout.Duration < 0 {
return apis.ErrInvalidValue(fmt.Sprintf("%s should be >= 0", ps.Timeout.Duration.String()), "spec.timeout")
}
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/apis/pipeline/v1alpha1/pipelinerun_validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ func TestPipelineRun_Invalidate(t *testing.T) {
Timeout: &metav1.Duration{Duration: -48 * time.Hour},
},
},
want: apis.ErrInvalidValue("-48h0m0s should be > 0", "spec.timeout"),
want: apis.ErrInvalidValue("-48h0m0s should be >= 0", "spec.timeout"),
},
}

Expand Down
11 changes: 6 additions & 5 deletions pkg/reconciler/v1alpha1/pipelinerun/pipelinerun.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (
"github.com/knative/pkg/configmap"
"github.com/knative/pkg/controller"
"github.com/knative/pkg/tracker"
apisconfig "github.com/tektoncd/pipeline/pkg/apis/config"
"github.com/tektoncd/pipeline/pkg/apis/pipeline"
"github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1"
artifacts "github.com/tektoncd/pipeline/pkg/artifacts"
Expand Down Expand Up @@ -441,20 +442,20 @@ func (c *Reconciler) updateTaskRunsStatusDirectly(pr *v1alpha1.PipelineRun) erro
func (c *Reconciler) createTaskRun(logger *zap.SugaredLogger, rprt *resources.ResolvedPipelineRunTask, pr *v1alpha1.PipelineRun, storageBasePath string) (*v1alpha1.TaskRun, error) {
var taskRunTimeout = &metav1.Duration{Duration: 0 * time.Second}

if pr.Spec.Timeout != nil {
// If the value of the timeout is 0 for any resource, there is no timeout.
// It is impossible for pr.Spec.Timeout to be nil, since SetDefault always assigns it with a value.
if pr.Spec.Timeout.Duration != apisconfig.NoTimeoutDuration {
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
}
} else {
taskRunTimeout = nil
}

// If service account is configured for a given PipelineTask, override PipelineRun's seviceAccount
Expand Down
5 changes: 5 additions & 0 deletions pkg/reconciler/v1alpha1/taskrun/taskrun.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (
"github.com/knative/pkg/apis"
"github.com/knative/pkg/controller"
"github.com/knative/pkg/tracker"
apisconfig "github.com/tektoncd/pipeline/pkg/apis/config"
"github.com/tektoncd/pipeline/pkg/apis/pipeline"
"github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1"
informers "github.com/tektoncd/pipeline/pkg/client/informers/externalversions/pipeline/v1alpha1"
Expand Down Expand Up @@ -534,6 +535,10 @@ func (c *Reconciler) checkTimeout(tr *v1alpha1.TaskRun, ts *v1alpha1.TaskSpec, d
}

timeout := tr.Spec.Timeout.Duration
// If timeout is set to 0 or defaulted to 0, there is no timeout.
if timeout == apisconfig.NoTimeoutDuration {
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 566012a

Please sign in to comment.