From ab272bff43785a09f1fb2b161d37fe20d8a09161 Mon Sep 17 00:00:00 2001
From: Soule BA <bah.soule@gmail.com>
Date: Thu, 28 Jan 2021 22:22:37 +0100
Subject: [PATCH] Proposal to enable finally tasks to execute when a
 pipelinerun has reached 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.
---
 ...0047-finallytask-execution-post-timeout.md | 265 ++++++++++++++++++
 1 file changed, 265 insertions(+)
 create mode 100644 teps/0047-finallytask-execution-post-timeout.md

diff --git a/teps/0047-finallytask-execution-post-timeout.md b/teps/0047-finallytask-execution-post-timeout.md
new file mode 100644
index 000000000..6a00ee4b8
--- /dev/null
+++ b/teps/0047-finallytask-execution-post-timeout.md
@@ -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.
\ No newline at end of file