Update IsFailure method to reflect Run retries #4625
Conversation
Custom Tasks support retries; however, there are some places in the code base that consider retries only for TaskRuns. This commit updates ResolvedPipelineRunTask.IsFailure to recognize Run retries and adds tests for this method. The function PipelineRunState.getNextTasks will be updated in a future commit. No functional changes for TaskRuns.
tekton-robot
added
release-note
tekton-robot
added
the
size/L
|
The following is the coverage report on the affected files.
|
| @@ -136,16 +136,46 @@ func (t ResolvedPipelineRunTask) IsSuccessful() bool { | |||
|
|
|||
| // IsFailure returns true only if the run has failed and will not be retried. | |||
| func (t ResolvedPipelineRunTask) IsFailure() bool { | |||
| if t.IsCustomTask() { | |||
| return t.Run != nil && t.Run.IsDone() && !t.Run.IsSuccessful() | |||
| if t.IsCancelled() { | |||
There was a problem hiding this comment.
one weird bit of maybe undefined behavior is that for TaskRuns, if it is marked as cancelled but has status "unknown" (rather than "false"), this is considered not failed and not done. I decided to use the same behavior for Runs as for TaskRuns. If we want to change this behavior I think it belongs in a separate commit.
There was a problem hiding this comment.
A taskRun is considered as Cancelled if status is false and reason is set to TaskRunCancelled. Are you saying the status is set to unknown when a taskRun is cancelled on demand? I didnt quite follow how thats happening.
There was a problem hiding this comment.
What I mean is, if a TaskRun has status "unknown" with reason "cancelled", it will be marked as not failed. It's unclear to me whether such a condition can occur and what the reconciler's behavior should be in this scenario. Our tests implicitly depended on this behavior prior to this commit, and now make this behavior explicit.
|
/test tekton-pipeline-unit-tests |
|
[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
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
tekton-robot
added
the
approved
|
thank you @lbernick 👍 |
Changes
Custom Tasks support retries (as of #4327); however, there are some places in the code base that consider
retries only for TaskRuns. This commit updates ResolvedPipelineRunTask.IsFailure to recognize
Run retries and adds tests for this method. The function PipelineRunState.getNextTasks will
be updated in a future commit. No functional changes for TaskRuns.
/kind bug
Submitter Checklist
As the author of this PR, please check off the items in this checklist:
functionality, content, code)
Release Notes