Add pending status support for TaskRun (parity with PipelineRun)#9464
Add pending status support for TaskRun (parity with PipelineRun)#9464sahilleth wants to merge 4 commits intotektoncd:mainfrom
Conversation
Release Notes |
|
@sahilleth Can you use the same PR format as other PRs? |
|
@sahilleth Thank you for this PR. I would like to echo @khrm s request here as well. Please update this and other PRs of yours based on the pull request template which is available and do go through the contribution guidelines as well. Again ! Thank you for the contrib ! |
|
@sahilleth can you please rebase this PR ? Thank you. |
16720c5 to
fb30378
Compare
This comment was marked as duplicate.
This comment was marked as duplicate.
|
@waveywaves - Thanks for the guidance, and for reviewing this PR! I’ve updated this PR (and my other open PRs) to follow the pull request template and have gone through the contribution guidelines to make sure I’m aligned with the project’s expectations. Please let me know if there’s anything I should further adjust. |
|
@sahilleth is it fixing or closing an existing issue ? |
|
@vdemeester - Yes, this is intended to fix the pending TaskRun support gap tracked in #9376. I’ve updated the PR description to include “Fixes #9376” so it will be closed automatically when this is merged. |
There was a problem hiding this comment.
Reviewed the full diff. Implementation is clean and correctly mirrors the existing PipelineRun pending pattern:
- API types (v1 and v1beta1): constants,
IsPending()helper — consistent withPipelineRunPending - Validation: properly rejects pending on already-started TaskRuns
- Reconciler: correct ordering (done → cancelled → pending → timeout → pod creation), skips
InitializeConditionsfor pending, returns early withMarkResourceOngoing - Timeout handling is safe:
HasTimedOut()returns false whenStartTimeis zero, so pending TaskRuns won't spuriously time out - Tests cover the pending reconciliation path and validation for both valid and invalid cases
No issues found.
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: vdemeester The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
A couple of housekeeping items before this can merge:
Code-wise this looks good to go 👍 |
|
/kind feature |
|
@vdemeester - I’ve added |
fb30378 to
7546e79
Compare
|
I’ve updated the branch to be up to date with main. Please let me know if anything else is needed. |
|
/assign @vdemeester Thanks for the review ! I believe all feedback has been addressed and the PR is ready for merge. |
|
Updated the release-note block to match the required format. |
4220fd5 to
7546e79
Compare
TaskRun now supports spec.status: TaskRunPending to defer execution. When pending, no Pod is created and StartTime is not set. Clearing spec.status starts execution; setting TaskRunCancelled cancels without running. - Add TaskRunSpecStatusPending and TaskRunReasonPending to v1 and v1beta1 - Update validation to accept TaskRunPending and reject it after start - Update reconciler to handle pending (no Pod creation, set Unknown condition) - Add docs/taskruns.md Pending TaskRuns section - Add reconcile and validation tests Fixes tektoncd#9376
7546e79 to
6efc4fb
Compare
|
@vdemeester - |
|
Hi @twoGiants, just a gentle follow-up - I’ve updated the release-note block to match the required format. |
|
@sahilleth lgtm needs to be set by someone else than me indeed 😛 |
|
Ahh got it @vdemeester - thanks! |
waveywaves
left a comment
There was a problem hiding this comment.
Code Review
3 inline comments. The concept is correct and mirrors PipelineRun's pending support. A few things to consider around reconciler ordering, test coverage, and timeout behavior.
Covers Pending->Running and Pending->Cancelled transitions to ensure reconciler behavior stays correct.
There was a problem hiding this comment.
Thanks for the review and the detailed feedback @waveywaves !
I’ve added follow-up tests to cover the missing lifecycle transitions:
- pending → running (clearing spec.status resumes execution and creates the Pod)
- pending → cancelled (ensures it cancels without starting)
- I also updated the docs to clarify that the timeout does not start while the TaskRun is pending and only begins once execution starts.
Please let me know if you’d like any further refinements.
|
/retest |
|
This indicates |
|
Hello @waveywaves , |
|
/retest |
Changes
Adds pending status support for
TaskRunto match the existingPipelineRunpending behavior.Fixes #9376.
TaskRunPending(spec.status: TaskRunPending) in both v1 and v1beta1 APIs.TaskRunReasonPendingand anIsPending()helper.TaskRunPendingon new TaskRuns.TaskRunPendingonce the TaskRun has already started.status.startTimeis not set.spec.statusstarts execution.TaskRunCancelledcancels without running.docs/taskruns.mdwith use cases and examples.TestReconcileOnPendingTaskRun.Submitter Checklist
As the author of this PR, please check off the items in this checklist:
/kind feature. Valid types are bug, cleanup, design, documentation, feature, flake, misc, question, tepRelease Notes