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.

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

PipelineRun Timeouts #4460

Closed
jerop opened this issue Jan 10, 2022 · 9 comments · Fixed by #4813
Closed

PipelineRun Timeouts #4460

jerop opened this issue Jan 10, 2022 · 9 comments · Fixed by #4813
Assignees
Labels
area/roadmap Issues that are part of the project (or organization) roadmap (usually an epic) help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. kind/feature Categorizes issue or PR as related to a new feature.

Comments

@jerop
Copy link
Member

jerop commented Jan 10, 2022

In TEP-0046: Finally Tasks Execution Post PipelineRun Timeout, we introduced new timeouts fields to replace the timeout field.

# Before
kind: PipelineRun
spec:
  timeout: "0h0m60s"

# After
kind: PipelineRun
spec:
  timeouts:
    pipeline: "0h4m0s"
    tasks: "0h1m0s"
    finally: "0h3m0s"

As a follow up to that work, we need to:

  • Promote PipelineRun.Timeouts from Alpha to Beta
  • Deprecate PipelineRun.Timeout

This work should be included in our V1 effort.

/kind feature
/cc @lbernick @souleb @pritidesai

@tekton-robot tekton-robot added the kind/feature Categorizes issue or PR as related to a new feature. label Jan 10, 2022
@pritidesai
Copy link
Member

thank you @jerop 👍

Yes, definitely, the pipelineRun.timeouts was introduced in 0.25 about 6 releases back - https://github.com/tektoncd/pipeline/blob/main/docs/install.md#alpha-features. We can work towards promoting it to beta and making it part of V1 efforts. We also updated our docs with a warning of deprecating pipelineRun.timeout in the same release which should buy us enough time for the deprecation.

@lbernick
Copy link
Member

Before promoting pipelinerun.timeouts to beta, we should clearly document and validate what happens if a user wants no timeout for pipelinerun.timeouts.tasks and pipelinerun.timeouts.finally. Right now, it's possible to set those timeouts to 0 (no timeout) without setting pipelinerun.timeouts.pipeline to no timeout, which I would consider an invalid configuration.

@jerop jerop added the area/roadmap Issues that are part of the project (or organization) roadmap (usually an epic) label Feb 17, 2022
@jerop jerop moved this to In Progress in Tekton Pipelines Roadmap Feb 17, 2022
@lbernick lbernick moved this to Todo in Pipelines V1 Feb 23, 2022
@jerop
Copy link
Member Author

jerop commented Mar 8, 2022

/help wanted

@jerop jerop added the help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. label Mar 8, 2022
@pritidesai
Copy link
Member

/assign @vinamra28

Please feel free to unassign yourself if needed

@tekton-robot
Copy link
Collaborator

@pritidesai: GitHub didn't allow me to assign the following users: vinamra28.

Note that only tektoncd members, repo collaborators and people who have commented on this issue/PR can be assigned. Additionally, issues/PRs can only have 10 assignees at the same time.
For more information please see the contributor guide

In response to this:

/assign @vinamra28

Please feel free to unassign yourself if needed

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.

@jerop
Copy link
Member Author

jerop commented Mar 22, 2022

@vinamra28 mentioned at Pipeline working group we discussed this and you're willing to take this on after #4611

@vinamra28
Copy link
Member

/assign

@lbernick
Copy link
Member

lbernick commented Apr 7, 2022

#4071 is relevant here-- not sure if it's a blocker for moving timeouts to beta but it's a pretty gnarly bug

@lbernick
Copy link
Member

Just talked with Vinamra; I think it would be OK to promote timeouts to beta and deprecate Timeout despite this bug. Reasoning here is that this bug doesn't affect Timeouts.Pipeline (which replaces Timeout), and we are confident we want Timeouts to be part of the API rather than Timeout.

@lbernick lbernick moved this from Todo to In Progress in Pipelines V1 May 5, 2022
Repository owner moved this from In Progress to Done in Tekton Pipelines Roadmap May 12, 2022
Repository owner moved this from In Progress to Done in Pipelines V1 May 12, 2022
jerop added a commit to jerop/pipeline that referenced this issue Jun 11, 2022
`PipelineRun` timeouts were promoted to beta in this pull request:
tektoncd#4813.

In this change, we update the alpha features table to remove
`PipelineRun` timeouts.

Related issue: tektoncd#4460.
jerop added a commit to jerop/pipeline that referenced this issue Jun 16, 2022
`PipelineRun` timeouts were promoted to Beta. This change removes
the listing of `PipelineRun` timeouts from the alpha features.

Issue: tektoncd#4460
PR: tektoncd#4813
jerop added a commit to jerop/pipeline that referenced this issue Jun 16, 2022
`PipelineRun` timeouts were promoted to Beta. This change removes
the listing of `PipelineRun` timeouts from the alpha features.

Issue: tektoncd#4460
PR: tektoncd#4813
tekton-robot pushed a commit that referenced this issue Jun 17, 2022
`PipelineRun` timeouts were promoted to Beta. This change removes
the listing of `PipelineRun` timeouts from the alpha features.

Issue: #4460
PR: #4813
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/roadmap Issues that are part of the project (or organization) roadmap (usually an epic) help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. kind/feature Categorizes issue or PR as related to a new feature.
Projects
Status: Done
Status: Done
Development

Successfully merging a pull request may close this issue.

5 participants