Check parent task's status when calculating pipelinerun status #1178
Conversation
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: dibyom 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 |
|
The following is the coverage report on pkg/.
|
|
The following is the coverage report on pkg/.
|
This logic is currently embedded inside the large `GetPipelineConditionStatus` function. Extracting it will help simplify it. Signed-off-by: Dibyo Mukherjee <dibyo@google.com>
|
The following is the coverage report on pkg/.
|
|
The following is the coverage report on pkg/.
|
|
The following is the coverage report on pkg/.
|
|
/test pull-tekton-pipeline-integration-tests |
|
The following is the coverage report on pkg/.
|
Before conditionals, a pipelinerun was marked successful only if all tasks were successfully run. Now, a pipelinerun is successful if all tasks were either successfully run or skipped due to a condition check failure. Currently while updating status, we only see if a task's own condition check failed. This is not sufficient if that task (B) is dependent on another task (A) using `from` or `runAfter`. If A is skipped due to a condition failure, task B is not scheduled to be execute, and there are no other tasks left to run. However, the pipeline status is never updated from `Running` to `Successful`. To fix this, this commit changes the `GetPipelineConditionStatus` function to recursively look at a task's parent tasks to see if they are skipped. If any single parent task is skipped, we skip the task. If all tasks in a pipeline are either successful or skipped, we update the pipeline run status to successful. Signed-off-by: Dibyo Mukherjee <dibyo@google.com>
|
The following is the coverage report on pkg/.
|
| logger.Infof("TaskRun %s has failed, so PipelineRun %s has failed, retries done: %b", rprt.TaskRunName, prName, len(rprt.TaskRun.Status.RetriesStatus)) | ||
| // A single failed task mean we fail the pipeline | ||
| for _, rprt := range state { | ||
| if rprt.IsFailure() { //IsDone ensures we have crossed the retry limit |
There was a problem hiding this comment.
I'm a bit unclear what the IsDone comment is referring to here. Did you mean IsFailure ensures we have crossed...?
There was a problem hiding this comment.
Yeah old comment. Will fix
Changes
Before conditionals, a pipelinerun was marked successful only
if all tasks were successfully run. Now, a pipelinerun is successful
if all tasks were either successfully run or skipped due to a condition
check failure.
Currently while updating status, we only see if a task's own condition
check failed. This is not sufficient if that task (B) is dependent on another
task (A) using
fromorrunAfter. If A is skipped due to a condition failure,task B is not scheduled to be execute, and there are no other tasks left to run.
However, the pipeline status is never updated from
RunningtoSuccessful.To fix this, this commit changes the
GetPipelineConditionStatusfunction torecursively look at a task's parent tasks to see if they are skipped. If any single
parent task is skipped, we skip the task. If all tasks in a pipeline are either
successful or skipped, we update the pipeline run status to successful.
Fixes #1173
Submitter Checklist
These are the criteria that every PR should meet, please check them off as you
review them: