Skip to content

Commit

Permalink
Proposal to enable finally tasks to execute when a pipelinerun has re…
Browse files Browse the repository at this point in the history 
…ached timeout.

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-finallytasks. But in case of timed out pipeline, the finally task execution is bounded by the declared timeout.
  • Loading branch information
@souleb
souleb committed Jan 28, 2021
1 parent 4ebddd4 commit ab272bf
Showing 1 changed file with 265 additions and 0 deletions.
265 changes: 265 additions & 0 deletions teps/0047-finallytask-execution-post-timeout.md
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,265 @@
---
status: proposed
title: Finally tasks execution post pipelinerun timeout
creation-date: '2021-01-26'
last-updated: '2021-01-26'
authors:
- '@souleb'
---

# TEP-0047: Finally tasks execution post pipelinerun timeout
---


<!--
**Note:** When your TEP is complete, all of these comment blocks should be removed.
To get started with this template:
- [ ] **Fill out this file as best you can.**
At minimum, you should fill in the "Summary", and "Motivation" sections.
These should be easy if you've preflighted the idea of the TEP with the
appropriate Working Group.
- [ ] **Create a PR for this TEP.**
Assign it to people in the SIG that are sponsoring this process.
- [ ] **Merge early and iterate.**
Avoid getting hung up on specific details and instead aim to get the goals of
the TEP clarified and merged quickly. The best way to do this is to just
start with the high-level sections and fill out details incrementally in
subsequent PRs.
Just because a TEP is merged does not mean it is complete or approved. Any TEP
marked as a `proposed` is a working document and subject to change. You can
denote sections that are under active debate as follows:
```
<<[UNRESOLVED optional short context or usernames ]>>
Stuff that is being argued.
<<[/UNRESOLVED]>>
```
When editing TEPS, aim for tightly-scoped, single-topic PRs to keep discussions
focused. If you disagree with what is already in a document, open a new PR
with suggested changes.
If there are new details that belong in the TEP, edit the TEP. Once a
feature has become "implemented", major changes should get new TEPs.
The canonical place for the latest set of instructions (and the likely source
of this file) is [here](/teps/NNNN-TEP-template/README.md).
-->

<!--
This is the title of your TEP. Keep it short, simple, and descriptive. A good
title can help communicate what the TEP is and should be considered as part of
any review.
-->

<!--
A table of contents is helpful for quickly jumping to sections of a TEP and for
highlighting any additional information provided beyond the standard TEP
template.
Ensure the TOC is wrapped with
<code>&lt;!-- toc --&rt;&lt;!-- /toc --&rt;</code>
tags, and then generate with `hack/update-toc.sh`.
-->

<!-- toc -->
- [# TEP-0047: Finally tasks execution post pipelinerun timeout](#-tep-0047-finally-tasks-execution-post-pipelinerun-timeout)
- [Summary](#summary)
- [Motivation](#motivation)
- [Goals](#goals)
- [Non-Goals](#non-goals)
- [Proposal](#proposal)
- [Test Plan](#test-plan)
- [Alternatives](#alternatives)
<!-- /toc -->

## Summary

<!--
This section is incredibly important for producing high quality user-focused
documentation such as release notes or a development roadmap. It should be
possible to collect this information before implementation begins in order to
avoid requiring implementors to split their attention between writing release
notes and implementing the feature itself.
A good summary is probably at least a paragraph in length.
Both in this section and below, follow the guidelines of the [documentation
style guide]. In particular, wrap lines to a reasonable length, to make it
easier for reviewers to cite specific portions, and to minimize diff churn on
updates.
[documentation style guide]: https://github.com/kubernetes/community/blob/master/contributors/guide/style-guide.md
-->

This TEP adresses issue [`#2980`](https://github.com/tektoncd/pipeline/issues/2989).

The proposal is to enable finally tasks to execute when a pipelinerun has reached timeout.

## Motivation

<!--
This section is for explicitly listing the motivation, goals and non-goals of
this TEP. Describe why the change is important and the benefits to users. The
motivation section can optionally provide links to [experience reports][] to
demonstrate the interest in a TEP within the wider Tekton community.
[experience reports]: https://github.com/golang/go/wiki/ExperienceReports
-->

The finally task [`design document`](https://docs.google.com/document/d/1lxpYQHppiWOxsn4arqbwAFDo4T0-LCqpNa6p-TJdHrw/edit#heading=h.w51ed6k2inef) list the following use cases :

- Cleanup cluster resources after finishing (with success/failure) integration tests (Dogfooding Scenario)
- Update Pull Request with what happened overall in the pipeline (pipeline level)
- Report Test Results at the end of the test pipeline (Notifications Scenario)

Unfortunately if a pipeline's execution reaches the defined timeout value, the pipelinerun stop and reports a failed status.

Here is an example pipeline run with a finally task:

```yaml
apiVersion: tekton.dev/v1beta1
kind: PipelineRun
metadata:
name: hello-world-pipeline-run-with-timeout
spec:
timeout: "0h0m60s"
pipelineSpec:
tasks:
- name: task1
timeout: "0h0m30s"
taskSpec:
steps:
- name: hello
image: ubuntu
script: |
echo "Hello World!"
sleep 10
finally:
- name: task2
params:
- name: echoStatus
value: "$(tasks.task1.status)"
taskSpec:
params:
- name: echoStatus
steps:
- name: verify-status
image: ubuntu
script: |
if [ $(params.echoStatus) == "Succeeded" ]
then
echo " Hello World echoed successfully"
fi
```
The finally task runs after the task completion and both execute normally.
| NAME | TASK NAME | STARTED | DURATION | STATUS |
|----------------------------------------------------------------|------------------|----------------|------------|------------------------|
| ∙ hello-world-pipeline-run-with-timeout-task1-tp9dq | task1 | 17 seconds ago | 4 seconds | Succeeded |
| ∙ hello-world-pipeline-run-with-timeout-task2-zpzfm | task2 | 47 seconds ago | 30 seconds | Succeeded |
| | | | | |
Now if we change the task script in order to have it exceed its timeout (30s), we get the following status report:
| NAME | TASK NAME | STARTED | DURATION | STATUS |
|-----------------------------------------------------|-----------|----------------|------------|------------------------|
| ∙ hello-world-pipeline-run-with-timeout-task2-44tsb | task2 | 8 seconds ago | 5 seconds | Succeeded |
| ∙ hello-world-pipeline-run-with-timeout-task1-wgcq7 | task1 | 38 seconds ago | 30 seconds | Failed(TaskRunTimeout) |
| | | | | |
The finally task still executes after the task failure.
Finally if we reduce the pipelinerun timeout to 10s, our status report shows:
`PipelineRun "hello-world-pipeline-run-with-timeout" failed to finish within "10s" (TaskRun "hello-world-pipeline-run-with-timeout-task1-q7fw4" failed to finish within "30s")`

| NAME | TASK NAME | STARTED | DURATION | STATUS |
|-----------------------------------------------------|-----------|---------------|------------|------------------------|
| ∙ hello-world-pipeline-run-with-timeout-task1-q7fw4 | task1 | 2 minutes ago | 30 seconds | Failed(TaskRunTimeout) |
| | | | | |
| | | | | |


The pipelinerun timeout take precedence over the task timeout. After 10s the task fails... And the finally task does not get the chance to execute.


For this reason, it is currently not possible to rely on Finally tasks for any of the aforementioned use cases.

### Goals

<!--
List the specific goals of the TEP. What is it trying to achieve? How will we
know that this has succeeded?
-->

Enable the uses cases :

- Cleanup cluster resources after finishing (with success/failure) integration tests (Dogfooding Scenario)
- Update Pull Request with what happened overall in the pipeline (pipeline level)
- Report Test Results at the end of the test pipeline (Notifications Scenario)

When a pipelinerun times out.

### Non-Goals

<!--
What is out of scope for this TEP? Listing non-goals helps to focus discussion
and make progress.
-->

## Proposal

<!--
This is where we get down to the specifics of what the proposal actually is.
This should have enough detail that reviewers can understand exactly what
you're proposing, but should not include things like API designs or
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 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
```

In order to keep older declaration working, we set the timeout field optional with a `default 20 minutes timeout`.


## Test Plan

<!--
**Note:** *Not required until targeted at a release.*
Consider the following in developing a test plan for this enhancement:
- Will there be e2e and integration tests, in addition to unit tests?
- How will it be tested in isolation vs with other components?
No need to outline all of the test cases, just the general strategy. Anything
that would count as tricky in the implementation and anything particularly
challenging to test should be called out.
All code is expected to have adequate tests (eventually with coverage
expectations).
-->

- Unit tests
- End-to-end tests
- Examples


## Alternatives

<!--
What other approaches did you consider and why did you rule them out? These do
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.

0 comments on commit ab272bf

Please sign in to comment.