Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

WIP - implementing pipeline level finally #2651

Closed
wants to merge 1 commit into from
Closed

WIP - implementing pipeline level finally #2651

wants to merge 1 commit into from

Conversation

pritidesai
Copy link
Member

Changes

We can now specify a list of tasks needs to be executed just before
pipeline exits (either after finishing all non-final tasks successfully or after
a single failure)

Most useful for tasks such as report test results, cleanup cluster resources, etc

apiVersion: tekton.dev/v1beta1
kind: Pipeline
metadata:
  name: pipeline-with-final-tasks
spec:
  tasks:
    - name: pre-work
      taskRef:
        Name: some-pre-work
    - name: unit-test
      taskRef:
        Name: run-unit-test
      runAfter:
        - pre-work
    - name: integration-test
      taskRef:
        Name: run-integration-test
      runAfter:
        - unit-test
  finally:
    - name: cleanup-test
      taskRef:
        Name: cleanup-cluster
    - name: report-results
      taskRef:
        Name: report-test-results

Design doc: https://docs.google.com/document/d/1lxpYQHppiWOxsn4arqbwAFDo4T0-LCqpNa6p-TJdHrw/edit#

Part of #1684
Fixes #2446

Depends on: #2650

Submitter Checklist

These are the criteria that every PR should meet, please check them off as you
review them:

See the contribution guide for more details.

Double check this list of stuff that's easy to miss:

Reviewer Notes

If API changes are included, additive changes must be approved by at least two OWNERS and backwards incompatible changes must be approved by more than 50% of the OWNERS, and they must first be added in a backwards compatible way.

Release Notes

Users can now specify Tasks within a Pipeline that will always execute, even if Tasks fail, via the new `finally` clause

@tekton-robot tekton-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label May 20, 2020
@tekton-robot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
To complete the pull request process, please assign dibyom
You can assign the PR to them by writing /assign @dibyom in a comment when ready.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@tekton-robot tekton-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label May 20, 2020
@tekton-robot
Copy link
Collaborator

This PR cannot be merged: expecting exactly one kind/ label

Available kind/ labels are:

kind/bug: Categorizes issue or PR as related to a bug.
kind/flake: Categorizes issue or PR as related to a flakey test
kind/cleanup: Categorizes issue or PR as related to cleaning up code, process, or technical debt.
kind/design: Categorizes issue or PR as related to design.
kind/documentation: Categorizes issue or PR as related to documentation.
kind/feature: Categorizes issue or PR as related to a new feature.
kind/misc: Categorizes issue or PR as a miscellaneuous one.

2 similar comments
@tekton-robot
Copy link
Collaborator

This PR cannot be merged: expecting exactly one kind/ label

Available kind/ labels are:

kind/bug: Categorizes issue or PR as related to a bug.
kind/flake: Categorizes issue or PR as related to a flakey test
kind/cleanup: Categorizes issue or PR as related to cleaning up code, process, or technical debt.
kind/design: Categorizes issue or PR as related to design.
kind/documentation: Categorizes issue or PR as related to documentation.
kind/feature: Categorizes issue or PR as related to a new feature.
kind/misc: Categorizes issue or PR as a miscellaneuous one.

@tekton-robot
Copy link
Collaborator

This PR cannot be merged: expecting exactly one kind/ label

Available kind/ labels are:

kind/bug: Categorizes issue or PR as related to a bug.
kind/flake: Categorizes issue or PR as related to a flakey test
kind/cleanup: Categorizes issue or PR as related to cleaning up code, process, or technical debt.
kind/design: Categorizes issue or PR as related to design.
kind/documentation: Categorizes issue or PR as related to documentation.
kind/feature: Categorizes issue or PR as related to a new feature.
kind/misc: Categorizes issue or PR as a miscellaneuous one.

@tekton-robot
Copy link
Collaborator

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File Old Coverage New Coverage Delta
pkg/reconciler/pipelinerun/pipelinerun.go 73.5% 73.6% 0.1
pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go 90.5% 93.1% 2.6

@pritidesai
Copy link
Member Author

/kind feature

@tekton-robot tekton-robot added the kind/feature Categorizes issue or PR as related to a new feature. label May 20, 2020
@pritidesai pritidesai force-pushed the finally-implementation branch from bab4c27 to f75dff1 Compare May 20, 2020 07:10
@tekton-robot
Copy link
Collaborator

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File Old Coverage New Coverage Delta
pkg/reconciler/pipelinerun/pipelinerun.go 73.5% 73.6% 0.1
pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go 90.5% 93.1% 2.6

@pritidesai pritidesai mentioned this pull request May 20, 2020
Closed
3 tasks
@pritidesai pritidesai force-pushed the finally-implementation branch from f75dff1 to f62c998 Compare May 20, 2020 07:25
@tekton-robot
Copy link
Collaborator

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File Old Coverage New Coverage Delta
pkg/reconciler/pipelinerun/pipelinerun.go 73.5% 73.4% -0.1
pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go 90.5% 92.6% 2.1

@pritidesai pritidesai force-pushed the finally-implementation branch from f62c998 to 71ea79d Compare May 20, 2020 07:30
@tekton-robot
Copy link
Collaborator

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File Old Coverage New Coverage Delta
pkg/reconciler/pipelinerun/pipelinerun.go 73.5% 73.3% -0.2
pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go 90.5% 92.6% 2.1

@pritidesai pritidesai force-pushed the finally-implementation branch from 71ea79d to 7c4b81c Compare May 20, 2020 07:56
@tekton-robot
Copy link
Collaborator

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File Old Coverage New Coverage Delta
pkg/reconciler/pipelinerun/pipelinerun.go 73.5% 73.3% -0.2
pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go 90.5% 92.6% 2.1

@pritidesai pritidesai force-pushed the finally-implementation branch from 7c4b81c to 4327cd1 Compare May 20, 2020 08:04
@tekton-robot
Copy link
Collaborator

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File Old Coverage New Coverage Delta
pkg/reconciler/pipelinerun/pipelinerun.go 73.5% 73.3% -0.2
pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go 90.5% 92.6% 2.1

@pritidesai pritidesai changed the title wip - implementing pipeline level finally WIP - implementing pipeline level finally May 20, 2020
@pritidesai pritidesai force-pushed the finally-implementation branch from 4327cd1 to d9a8784 Compare May 20, 2020 22:34
@tekton-robot
Copy link
Collaborator

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File Old Coverage New Coverage Delta
pkg/reconciler/pipelinerun/pipelinerun.go 73.5% 73.3% -0.2
pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go 90.5% 92.6% 2.1

@pritidesai pritidesai force-pushed the finally-implementation branch from d9a8784 to 0c466e7 Compare May 20, 2020 23:22
@tekton-robot
Copy link
Collaborator

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File Old Coverage New Coverage Delta
pkg/reconciler/pipelinerun/pipelinerun.go 73.5% 73.3% -0.2
pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go 90.5% 92.6% 2.1

@pritidesai pritidesai force-pushed the finally-implementation branch from 0c466e7 to 41e5a0e Compare May 20, 2020 23:49
@tekton-robot
Copy link
Collaborator

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File Old Coverage New Coverage Delta
pkg/reconciler/pipelinerun/pipelinerun.go 73.5% 74.4% 0.9
pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go 90.5% 92.6% 2.1

@pritidesai
pipeline level finally - implementation
b26f852 
We can now specify a list of tasks needs to be executed just before
pipeline exits (either after finishing all non-final tasks successfully or after
a single failure)

Most useful for tasks such as report test results, cleanup cluster resources, etc

```
apiVersion: tekton.dev/v1beta1
kind: Pipeline
metadata:
  name: pipeline-with-final-tasks
spec:
  tasks:
    - name: pre-work
      taskRef:
        Name: some-pre-work
    - name: unit-test
      taskRef:
        Name: run-unit-test
      runAfter:
        - pre-work
    - name: integration-test
      taskRef:
        Name: run-integration-test
      runAfter:
        - unit-test
  finally:
    - name: cleanup-test
      taskRef:
        Name: cleanup-cluster
    - name: report-results
      taskRef:
        Name: report-test-results
```
@pritidesai pritidesai force-pushed the finally-implementation branch from 41e5a0e to b26f852 Compare May 21, 2020 00:18
@tekton-robot
Copy link
Collaborator

@pritidesai: The following tests failed, say /retest to rerun them all:

Test name Commit Details Rerun command
pull-tekton-pipeline-build-tests b26f852 link /test pull-tekton-pipeline-build-tests
pull-tekton-pipeline-unit-tests b26f852 link /test tekton-pipeline-unit-tests
pull-tekton-pipeline-integration-tests b26f852 link /test pull-tekton-pipeline-integration-tests
pull-tekton-pipeline-go-coverage b26f852 link /test pull-tekton-pipeline-go-coverage

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@pritidesai
Copy link
Member Author

Messed up resolving conflicts, opened a new PR #2661 😭

@pritidesai pritidesai closed this May 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bobcatfish bobcatfish Awaiting requested review from bobcatfish

@dlorenc dlorenc Awaiting requested review from dlorenc

Assignees
No one assigned
Labels
do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. kind/feature Categorizes issue or PR as related to a new feature. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

"Finally" tasks for Pipeline
2 participants
@pritidesai @tekton-robot