Skip to content

Commit

Permalink
Adding a new proposal based on two flags at the pipelinerun spec leveL.
Browse files Browse the repository at this point in the history
Adding alternatives solutions.
  • Loading branch information
souleb committed Feb 10, 2021
1 parent 01a88c8 commit 5c9794a
Showing 1 changed file with 58 additions and 7 deletions.
65 changes: 58 additions & 7 deletions teps/0047-finallytask-execution-post-timeout.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@ authors:
- [Proposal](#proposal)
- [Test Plan](#test-plan)
- [Alternatives](#alternatives)
- [Finally block level timeout flag](#finally-block-level-timeout-flag)
- [Pipelinerun timeout is inclusive of the finally tasks timeout](#pipelinerun-timeout-is-inclusive-of-the-finally-tasks-timeout)
- [Finally Timeout flag at Pipelinerun Spec](#finally-timeout-flag-at-pipelinerun-spec)
<!-- /toc -->

## Summary
Expand Down Expand Up @@ -168,24 +171,43 @@ implementation. The "Design Details" section below is for the real
nitty-gritty.
-->

Enable finally task to run when a pipeline times out. This implies a behavioral change, as finally tasks will run no matter what.
Enable finally task to run when a pipeline times out.

Enable pipeline authors to specify a timeout field for finally tasks. In all normal run, that timeout is not needed and finally tasks execute after non-finally tasks. But in case of timed out pipeline, the finally task execution is bounded by the declared timeout.
We propose to deprecate the current `timeout` as the pipelinerun level. This timeout is as of today used as the timeout of each newly created taskrun by the controller.

Instead we propose to have to new flags:
- tasksTimeout
- runTimeout

`tasksTimeout` will define a timeout for the dag tasks. `runTimeout` will define a timeout for the whole pipelinerun. The finally tasks timeout willbe `runTimeout - tasksTimeout` with `runTimeout > tasksTimeout`.

When `tasksTimeout` is not defined, `runTimeout` is used for the tasks timeout.

This will enable users to manage run time behavior and make sure their finally tasks run as intended by scoping the tasks runtime period.

.
```yaml
spec:
tasks:
tasksTimeout: "1h0m0s"
runTimeout: "2h0m0s"
pipelineSpec:
tasks:
- name: tests
taskRef:
Name: integration-test
finally:
timeout: "0h0m10s"
finally:
- name: cleanup-test
taskRef:
Name: cleanup
```

In order to keep older declaration working, we set the timeout field optional with a `default 20 minutes timeout`.
The default default-timeout-minutes configurable via configmap is kept and will be applied to the `runTimeout`.

In order to keep older declaration working. We will use the [`Feature Flag for all alpha features`](https://github.com/tektoncd/community/pull/280) to enable the new feature.

When we opt-in the feature, we may specify a `tasksTimeout` and a `runTimeout` flags. The will be used to define dags tasks timeout and the pipelinerun timeout. We cannot use the current `timeout` flag. It would result in a validation error.

When we opt-out, the current behaviour is kept. We cannot use `tasksTimeout` and a `runTimeout` flags. It would result in a validation error.


## Test Plan
Expand Down Expand Up @@ -215,4 +237,33 @@ not need to be as detailed as the proposal, but should include enough
information to express the idea and why it was not acceptable.
-->

We could add a new field to decide whether to permits finally tasks to run in case of timeout.
### Finally block level timeout flag

Enable finally task to run when a pipeline times out. This implies a behavioral change, as finally tasks will run no matter what.

Enable pipeline authors to specify a timeout field for finally tasks. In all normal run, that timeout is not needed and finally tasks execute after non-finally tasks. But in case of timed out pipeline, the finally task execution is bounded by the declared timeout.

```yaml
spec:
tasks:
- name: tests
taskRef:
Name: integration-test
finally:
timeout: "0h0m10s"
- name: cleanup-test
taskRef:
Name: cleanup
```

This solution is not backward compatible as the finally tasks are currently defined as a list field in the pipelineRunSpec type.
### Pipelinerun timeout is inclusive of the finally tasks timeout

We could consider that the pipelinerun timeout is inclusive of the finally tasks timeout. So, during execution, we could stop executing dag tasks at some point to give enough time for finally tasks to execute before timing out the pipelinerun (dag tasks timeout = pipelinerun timeout - finally tasks timeout).

This solution was deemed confusing. The user could expect the `timeout` to be for the dag tasks entirely. This is reducing the dagtasks runtime and reduces the user possibilitie sto configure it.


### Finally Timeout flag at Pipelinerun Spec

We could add a new flag at the pipelineRun level `finallyTimeout` similar to the timeout flag. If specified, pipelineRun timeout (default is one hour) applies to dag tasks only. The dag tasks will stop executing once it meets the pipelineRun timeout. The finally tasks starts executing at this point and will be executed until meets the timeout specified in finallyTimeout.

0 comments on commit 5c9794a

Please sign in to comment.