task results reassertion

[pritidesai]

• TaskResultsNotProduced: TaskRun for a task should result in failure if task results are not populated. Tasks are declared successful today even before initializing task results from container status. updateTaskRunResourceResult is executed when a taskRun is declared successful. TaskRun is declared complete and successful here, we should add a check on task results even before updating taskRun.status.

• InvalidTaskResultReference: Task execution order is set such that the task producing a result is executed before the task referencing that result. In case of a failure of a referenced task, pipeline exits without executing the rest of the pipeline tasks. But when a referenced task succeeds and does not instantiate the result, the task referencing that result is not executed and pipeline results in failure. Pipeline exits with PipelineValidationFailed since the result did not exist. Replace PipelineValidationFailed with something more specific InvalidTaskResultReference.

An example of such task and pipeline execution here.

I think, we might not even get to this kind of error if TaskResultsNotProduced is implemented.

• MissingTaskResultReference: A task might be set to produce task results but these results are not referenced in any of the pipeline tasks. This scenario is ignored today, does not impact task execution order and does not produce any failure. Pipeline should result in failure if task results are not referenced at all.

Please feel free to suggest better naming if these does not make sense.

[bobcatfish]

Hey @pritidesai ! I think I'm missing a bit of context for this one: is this about changing the error "reason" fields in Status conditions?

[pritidesai]

yes changing the reason field from generic PipelineValidationFailed to specific InvalidTaskResultReference and also changing the existing behavior where taskRun is marked successful without checking if task results were initialized/populated or not ... change TaskRun reconciler such that, check on task results are done before changing taskRun status to sucess.

[nikhil-thomas]

/area api

[tekton-robot]

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.
If this issue is safe to close now please do so with /close.

/lifecycle rotten

Send feedback to tektoncd/plumbing.

[tekton-robot]

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

/close

Send feedback to tektoncd/plumbing.

[tekton-robot]

@tekton-robot: Closing this issue.

In response to this:

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

/close

Send feedback to tektoncd/plumbing.

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.

[vdemeester]

/remove-lifecycle rotten
/remove-lifecycle stale
/reopen

[tekton-robot]

@vdemeester: Reopened this issue.

In response to this:

/remove-lifecycle rotten
/remove-lifecycle stale
/reopen

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.

[bobcatfish]

@pritidesai looking at this more closely would you say #3497 is a duplicate of this issue? It sounds like this issue is about trying to add more validation around expectations for Task results?

[pritidesai]

This issue is about documenting and implementing the expected behavior in cases such as TaskResultsNotProduced i.e. a task must fail if the result is not populated (same as issue #3497), and InvalidTaskResultReference which was implemented with PR #2538.

MissingTaskResultReference is something not discussed much but does not warrant any changes in pipeline. A task is allowed to produce a number of results but pipeline does not enforce all of them being utilized by other tasks.

Yup, we can close this as a duplicate of #3497