-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
allow pipeline runs whose task/custom runs have been deleted still timeout #7557
Conversation
The following is the coverage report on the affected files.
|
The following is the coverage report on the affected files.
|
/kind bug |
9c6795d
to
a398e22
Compare
The following is the coverage report on the affected files.
|
The following is the coverage report on the affected files.
|
/assign @vdemeester |
@gabemontero: GitHub didn't allow me to assign the following users: khrm. 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. In response to this:
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. |
/assign @abayer |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/approve
Please review and give lgtm here.
@afrittoli
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: khrm, vdemeester The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
…meout Back with PR 5134 Tekton started using the cancel function for cleaning up underlying TaskRuns of PipelineRuns that are timing out, though separate timeout functions were used (presumably in case the timeout behavior needed to be tweaked from the cancel behavior later on). Then, in PR 5288, improvements to the baseline cancel function were made to still complete cancel processing of a PipelineRun if the underlying TaskRuns were deleted. However, that same accomodation was not made for the timeout path. This change addresses that, but still keeps the timeout codepaths separate from the 'base' cancel codepaths.
a398e22
to
e79a236
Compare
The following is the coverage report on the affected files.
|
The following is the coverage report on the affected files.
|
rebase completed @khrm @vdemeester @afrittoli and CI clean .... PTAL |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
/lgtm
/cherry-pick release-v0.56.x |
@vdemeester: #7557 failed to apply on top of branch "release-v0.56.x":
In response to this:
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. |
Fixes #7556
Changes
Back with PR 5134 Tekton started using the cancel function for cleaning up underlying TaskRuns of PipelineRuns that are timing out, though separate timeout functions were used (presumably in case the timeout behavior needed to be tweaked from the cancel behavior later on). Then, in PR 5288, improvements to the baseline cancel function were made to still complete cancel processing of a PipelineRun if the underlying TaskRuns were deleted. However, that same accomodation was not made for the timeout path. This change addresses that, but still keeps the timeout codepaths separate from the 'base' cancel codepaths.
Submitter Checklist
As the author of this PR, please check off the items in this checklist:
/kind <type>
. Valid types are bug, cleanup, design, documentation, feature, flake, misc, question, tepRelease Notes
@vdemeester @afrittoli @abayer PTAL - thanks