don't validate skipped task results for pipeline results

[Yongxuanzhang]

Changes

This commit skip the validation for skipped task results in pipeline results. This fixes #6139. The skipped task results are not emitted by design thus we shouldn't validate if they are emitted. Pipeline results referenced to skipped task will remain as the original unsubstituted references.

/kind bug

Signed-off-by: Yongxuan Zhang [email protected]

Submitter Checklist

As the author of this PR, please check off the items in this checklist:

• [ ] Has Docs included if any changes are user facing
• [x] Has Tests included if any functionality added or changed
• [x] Follows the commit message standard
• [x] Meets the Tekton contributor standards (including functionality, content, code)
• [x] Has a kind label. You can add one by adding a comment on this PR that contains /kind <type>. Valid types are bug, cleanup, design, documentation, feature, flake, misc, question, tep
• [x] Release notes block below has been updated with any user facing changes (API changes, bug fixes, changes requiring upgrade notices or deprecation warnings)
• [ ] Release notes contains the string "action required" if the change requires additional action from users switching to the new release

Release Notes

NONE

[tekton-robot]

Skipping CI for Draft Pull Request. If you want CI signal for your change, please convert it to an actual PR. You can still manually trigger a test run with /test all

[tekton-robot]

The following is the coverage report on the affected files. Say /test pull-tekton-pipeline-go-coverage-df to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/reconciler/pipelinerun/resources/apply.go	97.5%	97.5%	0.1

[tekton-robot]

The following is the coverage report on the affected files. Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/reconciler/pipelinerun/resources/apply.go	97.5%	97.5%	0.1

[Yongxuanzhang]

Hi @pritidesai @jerop , could you take a look at this PR?

[jerop]

please fix the typo in the pr title and commit message

[tekton-robot]

The following is the coverage report on the affected files. Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/reconciler/pipelinerun/resources/apply.go	97.5%	97.5%	0.1

[tekton-robot]

The following is the coverage report on the affected files. Say /test pull-tekton-pipeline-go-coverage-df to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/reconciler/pipelinerun/resources/apply.go	97.5%	97.5%	0.1

[Yongxuanzhang]

could it be more confusing for the pipeline result to remain as is - an unsubstituted reference to a task result? what if users assume pipeline results were produced by the pipeline, but then some are just the original references? wondering if we should just wait for tektoncd/community#954?

also @Yongxuanzhang please add to the commit message that this change is not only removing validation, but also leaving pipeline results as the original unsubstituted references to task results when the task was skipped

This commit skips the validation of "skipped tasks", my understanding is that if users use "when expressions" on tasks and they should be aware of some results cannot be replaced if tasks are skipped as mentioned in https://tekton.dev/docs/pipelines/pipelines/#emitting-results-from-a-pipeline

[jerop]

This commit skips the validation of "skipped tasks", my understanding is that if users use "when expressions" on tasks and they should be aware of some results cannot be replaced if tasks are skipped as mentioned in https://tekton.dev/docs/pipelines/pipelines/#emitting-results-from-a-pipeline

in that case, please update the documentation explaining it to users so it's not surprising to see unsubstituted references to task results in the pipeline results

[Yongxuanzhang]

could it be more confusing for the pipeline result to remain as is - an unsubstituted reference to a task result? what if users assume pipeline results were produced by the pipeline, but then some are just the original references? wondering if we should just wait for tektoncd/community#954?

also @Yongxuanzhang please add to the commit message that this change is not only removing validation, but also leaving pipeline results as the original unsubstituted references to task results when the task was skipped

This commit should be compatible with https://github.com/tektoncd/community/pull/954? Another reason is it may take unexpected longer time to wait https://github.com/tektoncd/community/pull/954 is implemented

[tekton-robot]

The following is the coverage report on the affected files. Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/reconciler/pipelinerun/resources/apply.go	97.5%	97.5%	0.1

[Yongxuanzhang]

This commit skips the validation of "skipped tasks", my understanding is that if users use "when expressions" on tasks and they should be aware of some results cannot be replaced if tasks are skipped as mentioned in https://tekton.dev/docs/pipelines/pipelines/#emitting-results-from-a-pipeline

in that case, please update the documentation explaining it to users so it's not surprising to see unsubstituted references to task results in the pipeline results

That makes sense! I added a note into the doc I linked, plz take a look.

[tekton-robot]

The following is the coverage report on the affected files. Say /test pull-tekton-pipeline-go-coverage-df to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/reconciler/pipelinerun/resources/apply.go	97.5%	97.5%	0.1

[tekton-robot]

The following is the coverage report on the affected files. Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/reconciler/pipelinerun/resources/apply.go	97.5%	97.5%	0.1

[tekton-robot]

The following is the coverage report on the affected files. Say /test pull-tekton-pipeline-go-coverage-df to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/reconciler/pipelinerun/resources/apply.go	97.5%	97.5%	0.1

[tekton-robot]

The following is the coverage report on the affected files. Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/reconciler/pipelinerun/resources/apply.go	97.5%	97.6%	0.1

[tekton-robot]

The following is the coverage report on the affected files. Say /test pull-tekton-pipeline-go-coverage-df to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/reconciler/pipelinerun/resources/apply.go	97.5%	97.6%	0.1

[Yongxuanzhang]

Thanks for this. It might be worth adding something in the release notes for this change. The code looks good, the only small question I have is about the log at info level 🙏

Thanks! I will also bring this PR to the slack and/or our wg. I think we need to collect more feedback of how to deal with skipped task results.

[kyubisation]

Is it possible to provide an estimate on when this will be merged? Unfortunately due to #6139 we are blocked in upgrading our Tekton setup and we would be very grateful for this fix.

[Yongxuanzhang]

Is it possible to provide an estimate on when this will be merged? Unfortunately due to #6139 we are blocked in upgrading our Tekton setup and we would be very grateful for this fix.

Thanks for the reminder! I will bring this up today during the community working group.

[tekton-robot]

The following is the coverage report on the affected files. Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/reconciler/pipelinerun/resources/apply.go	97.5%	97.5%	0.1

[tekton-robot]

The following is the coverage report on the affected files. Say /test pull-tekton-pipeline-go-coverage-df to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/reconciler/pipelinerun/resources/apply.go	97.5%	97.5%	0.1

[pritidesai]

Please update the release notes elaborating its a fix for a regression issue, thanks!

[tekton-robot]

The following is the coverage report on the affected files. Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/reconciler/pipelinerun/resources/apply.go	97.5%	97.5%	0.1

[tekton-robot]

The following is the coverage report on the affected files. Say /test pull-tekton-pipeline-go-coverage-df to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/reconciler/pipelinerun/resources/apply.go	97.5%	97.5%	0.1

[tekton-robot]

The following is the coverage report on the affected files. Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/reconciler/pipelinerun/resources/apply.go	98.0%	98.1%	0.1

[tekton-robot]

The following is the coverage report on the affected files. Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/reconciler/pipelinerun/resources/apply.go	98.0%	98.1%	0.1

[tekton-robot]

The following is the coverage report on the affected files. Say /test pull-tekton-pipeline-go-coverage-df to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/reconciler/pipelinerun/resources/apply.go	98.0%	98.1%	0.1

[tekton-robot]

The following is the coverage report on the affected files. Say /test pull-tekton-pipeline-go-coverage-df to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/reconciler/pipelinerun/resources/apply.go	98.0%	98.1%	0.1

[tekton-robot]

The following is the coverage report on the affected files. Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/reconciler/pipelinerun/resources/apply.go	98.0%	98.1%	0.1