TEP-0142: Surface step results via termination message #7349

chitrangpatel · 2023-11-10T21:22:51Z

This PR surfaces step results (i.e results written to $(step.results.<resultName>.path)) via termination messages. A followup PR will handle surfacing the results via sidecar logs.
The use of $(step.results.<resultName>.path) is currently only available in StepActions not inlined steps.

This PR is part of issue #7259.

Changes

Submitter Checklist

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

Has Docs if any changes are user facing, including updates to minimum requirements e.g. Kubernetes version bumps
Has Tests included if any functionality added or changed
Follows the commit message standard
Meets the Tekton contributor standards (including functionality, content, code)
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
Release notes block below has been updated with any user facing changes (API changes, bug fixes, changes requiring upgrade notices or deprecation warnings). See some examples of good release notes.
Release notes contains the string "action required" if the change requires additional action from users switching to the new release

Release Notes

[WIP] Surface step results via termination message

/kind feature

chitrangpatel · 2023-11-10T21:23:48Z

/hold wait for #7347 to get merged.

tekton-robot · 2023-11-13T16:04:11Z

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
cmd/entrypoint/main.go	11.6%	11.0%	-0.6
pkg/entrypoint/entrypointer.go	87.6%	80.8%	-6.8
pkg/pod/entrypoint.go	93.4%	76.0%	-17.4
pkg/reconciler/taskrun/resources/apply.go	99.3%	98.1%	-1.3

tekton-robot · 2023-11-13T16:05:59Z

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
cmd/entrypoint/main.go	11.6%	11.0%	-0.6
pkg/entrypoint/entrypointer.go	87.6%	80.8%	-6.8
pkg/pod/entrypoint.go	93.4%	76.0%	-17.4
pkg/reconciler/taskrun/resources/apply.go	99.3%	98.1%	-1.3

tekton-robot · 2023-11-13T16:06:58Z

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
cmd/entrypoint/main.go	11.6%	11.0%	-0.6
pkg/entrypoint/entrypointer.go	87.6%	80.8%	-6.8
pkg/pod/entrypoint.go	93.4%	76.0%	-17.4
pkg/reconciler/taskrun/resources/apply.go	99.3%	98.1%	-1.3

vdemeester

Steps write results somewhere (different than /tekton/results I guess) because:

we don't want to implicitly set them to TaskRun (it would probably cause an error)
we don't want to have on step overwrite the result of another

Am I right ?

Doing that, we end up making /tekton/steps writeable, which I think, is a problem. We should try to find "another place" to write step results. I wonder if we could prefix the result with the step name or something in /tekton/results instead, and somehow filter those by default (if not bound) ?

pkg/pod/pod.go

tekton-robot · 2023-11-13T17:14:36Z

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
cmd/entrypoint/main.go	11.6%	11.0%	-0.6
pkg/entrypoint/entrypointer.go	87.6%	80.8%	-6.8
pkg/pod/entrypoint.go	93.4%	76.0%	-17.4
pkg/reconciler/taskrun/resources/apply.go	99.3%	98.1%	-1.3

tekton-robot · 2023-11-13T17:15:23Z

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
cmd/entrypoint/main.go	11.6%	11.0%	-0.6
pkg/entrypoint/entrypointer.go	87.6%	80.8%	-6.8
pkg/pod/entrypoint.go	93.4%	76.0%	-17.4
pkg/reconciler/taskrun/resources/apply.go	99.3%	98.1%	-1.3

chitrangpatel · 2023-11-13T17:20:37Z

Steps write results somewhere (different than /tekton/results I guess) because:

we don't want to implicitly set them to TaskRun (it would probably cause an error)

we don't want to have on step overwrite the result of another

Am I right ?

Yes, exactly.

Doing that, we end up making /tekton/steps writeable, which I think, is a problem. We should try to find "another place" to write step results. I wonder if we could prefix the result with the step name or something in /tekton/results instead, and somehow filter those by default (if not bound) ?

Yes, or I wonder if we could make the step dir inside /tekton/results and write the results there /tekton/results/stepName/resultFile? Instead of making /tekton/steps writeable and then doing /tekton/steps/stepName/resultFile?

Writing StepAction Results inside step directories even inside /tekton/results will keep it safe from accidental overwrites. e.g $(results.<resultName>.path) would convert to /tekton/results/<resultName> while $(step.results.<resultName>.path) would convert to /tekton/results/<stepName>/<resultName>.

WDYT?

tekton-robot · 2023-11-13T17:52:13Z

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
cmd/entrypoint/main.go	11.6%	10.8%	-0.8
pkg/entrypoint/entrypointer.go	87.6%	80.8%	-6.8
pkg/pod/entrypoint.go	93.4%	76.0%	-17.4
pkg/reconciler/taskrun/resources/apply.go	99.3%	98.1%	-1.3

tekton-robot · 2023-11-13T17:53:11Z

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
cmd/entrypoint/main.go	11.6%	10.8%	-0.8
pkg/entrypoint/entrypointer.go	87.6%	80.8%	-6.8
pkg/pod/entrypoint.go	93.4%	76.0%	-17.4
pkg/reconciler/taskrun/resources/apply.go	99.3%	98.1%	-1.3

tekton-robot · 2023-11-13T18:40:03Z

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
cmd/entrypoint/main.go	11.6%	10.8%	-0.8
pkg/entrypoint/entrypointer.go	87.6%	80.8%	-6.8
pkg/pod/entrypoint.go	93.4%	76.0%	-17.4

tekton-robot · 2023-11-13T18:42:06Z

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
cmd/entrypoint/main.go	11.6%	10.8%	-0.8
pkg/entrypoint/entrypointer.go	87.6%	80.8%	-6.8
pkg/pod/entrypoint.go	93.4%	76.0%	-17.4
pkg/reconciler/taskrun/resources/apply.go	99.3%	98.1%	-1.3

tekton-robot · 2023-11-13T18:42:31Z

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
cmd/entrypoint/main.go	11.6%	10.8%	-0.8
pkg/entrypoint/entrypointer.go	87.6%	80.8%	-6.8
pkg/pod/entrypoint.go	93.4%	76.0%	-17.4
pkg/reconciler/taskrun/resources/apply.go	99.3%	98.1%	-1.3

cmd/entrypoint/main.go

pkg/pod/entrypoint.go

pkg/entrypoint/entrypointer.go

pkg/pod/entrypoint.go

tekton-robot · 2023-11-22T21:36:01Z

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/apis/pipeline/v1beta1/taskrun_conversion.go	93.3%	93.6%	0.3
pkg/entrypoint/entrypointer.go	87.6%	87.0%	-0.6
pkg/pod/entrypoint.go	93.4%	94.1%	0.7
pkg/pod/status.go	91.7%	92.1%	0.4
pkg/reconciler/taskrun/resources/apply.go	99.3%	99.4%	0.0
pkg/result/result.go	85.7%	86.7%	1.0

tekton-robot · 2023-11-22T21:41:23Z

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/apis/pipeline/v1beta1/taskrun_conversion.go	93.3%	93.6%	0.3
pkg/entrypoint/entrypointer.go	87.6%	87.0%	-0.6
pkg/pod/entrypoint.go	93.4%	94.1%	0.7
pkg/pod/status.go	91.7%	92.1%	0.4
pkg/reconciler/taskrun/resources/apply.go	99.3%	99.4%	0.0
pkg/result/result.go	85.7%	86.7%	1.0

tekton-robot · 2023-11-22T21:42:57Z

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/apis/pipeline/v1beta1/taskrun_conversion.go	93.3%	93.6%	0.3
pkg/entrypoint/entrypointer.go	87.6%	87.0%	-0.6
pkg/pod/entrypoint.go	93.4%	94.1%	0.7
pkg/pod/status.go	91.7%	92.1%	0.4
pkg/reconciler/taskrun/resources/apply.go	99.3%	99.4%	0.0
pkg/result/result.go	85.7%	86.7%	1.0

tekton-robot · 2023-11-22T22:53:36Z

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/apis/pipeline/v1beta1/taskrun_conversion.go	93.3%	93.6%	0.3
pkg/entrypoint/entrypointer.go	87.6%	87.0%	-0.6
pkg/pod/entrypoint.go	93.4%	94.1%	0.7
pkg/pod/status.go	91.7%	92.1%	0.4
pkg/reconciler/taskrun/resources/apply.go	99.3%	99.4%	0.0
pkg/result/result.go	85.7%	86.7%	1.0

tekton-robot · 2023-11-22T22:56:47Z

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/apis/pipeline/v1beta1/taskrun_conversion.go	93.3%	93.6%	0.3
pkg/entrypoint/entrypointer.go	87.6%	87.0%	-0.6
pkg/pod/entrypoint.go	93.4%	94.1%	0.7
pkg/pod/status.go	91.7%	92.1%	0.4
pkg/reconciler/taskrun/resources/apply.go	99.3%	99.4%	0.0
pkg/result/result.go	85.7%	86.7%	1.0

test/stepaction_results_test.go

pkg/pod/status_test.go

pkg/pod/entrypoint.go

tekton-robot · 2023-11-22T23:18:45Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Yongxuanzhang

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:

~~OWNERS~~ [Yongxuanzhang]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

tekton-robot · 2023-11-23T00:32:07Z

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/apis/pipeline/v1beta1/taskrun_conversion.go	93.3%	93.6%	0.3
pkg/entrypoint/entrypointer.go	87.6%	87.0%	-0.6
pkg/pod/entrypoint.go	93.4%	94.1%	0.7
pkg/pod/status.go	91.7%	92.1%	0.4
pkg/reconciler/taskrun/resources/apply.go	99.3%	99.4%	0.0
pkg/result/result.go	85.7%	86.7%	1.0

tekton-robot · 2023-11-23T00:32:07Z

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/apis/pipeline/v1beta1/taskrun_conversion.go	93.3%	93.6%	0.3
pkg/entrypoint/entrypointer.go	87.6%	87.0%	-0.6
pkg/pod/entrypoint.go	93.4%	94.1%	0.7
pkg/pod/status.go	91.7%	92.1%	0.4
pkg/reconciler/taskrun/resources/apply.go	99.3%	99.4%	0.0
pkg/result/result.go	85.7%	86.7%	1.0

tekton-robot · 2023-11-23T02:16:24Z

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/apis/pipeline/v1beta1/taskrun_conversion.go	93.3%	93.6%	0.3
pkg/entrypoint/entrypointer.go	87.6%	87.0%	-0.6
pkg/pod/entrypoint.go	93.4%	94.1%	0.7
pkg/pod/status.go	91.7%	92.1%	0.4
pkg/reconciler/taskrun/resources/apply.go	99.3%	99.4%	0.0
pkg/result/result.go	85.7%	86.7%	1.0

pkg/apis/pipeline/v1/result_types.go

tekton-robot · 2023-11-23T02:19:52Z

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/apis/pipeline/v1beta1/taskrun_conversion.go	93.3%	93.6%	0.3
pkg/entrypoint/entrypointer.go	87.6%	87.0%	-0.6
pkg/pod/entrypoint.go	93.4%	94.1%	0.7
pkg/pod/status.go	91.7%	92.1%	0.4
pkg/reconciler/taskrun/resources/apply.go	99.3%	99.4%	0.0
pkg/result/result.go	85.7%	86.7%	1.0

This PR surfaces step results (i.e results written to $(step.results.<resultName>.path)) via termination messages. A followup PR will handle surfacing the results via sidecar logs.

tekton-robot · 2023-11-23T05:02:07Z

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/apis/pipeline/v1beta1/taskrun_conversion.go	93.3%	93.6%	0.3
pkg/entrypoint/entrypointer.go	87.6%	87.0%	-0.6
pkg/pod/entrypoint.go	93.4%	94.1%	0.7
pkg/pod/status.go	91.7%	92.1%	0.4
pkg/reconciler/taskrun/resources/apply.go	99.3%	99.4%	0.0
pkg/result/result.go	85.7%	86.7%	1.0

tekton-robot · 2023-11-23T05:09:20Z

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/apis/pipeline/v1beta1/taskrun_conversion.go	93.3%	93.6%	0.3
pkg/entrypoint/entrypointer.go	87.6%	87.0%	-0.6
pkg/pod/entrypoint.go	93.4%	94.1%	0.7
pkg/pod/status.go	91.7%	92.1%	0.4
pkg/reconciler/taskrun/resources/apply.go	99.3%	99.4%	0.0
pkg/result/result.go	85.7%	86.7%	1.0

jerop

🎉

/lgtm

tekton-robot added kind/feature Categorizes issue or PR as related to a new feature. release-note Denotes a PR that will be considered when it comes time to generate release notes. labels Nov 10, 2023

chitrangpatel requested a review from Yongxuanzhang November 10, 2023 21:22

tekton-robot requested review from pritidesai and vdemeester November 10, 2023 21:23

tekton-robot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Nov 10, 2023

tekton-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 10, 2023

chitrangpatel mentioned this pull request Nov 10, 2023

TEP-0142: Enable Step Reusability - Implementation #7259

Closed

20 tasks

chitrangpatel force-pushed the step-action-results-termination-message branch 2 times, most recently from 751dda7 to ad58095 Compare November 13, 2023 15:57

tekton-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Nov 13, 2023

vdemeester reviewed Nov 13, 2023

View reviewed changes

pkg/pod/pod.go Outdated Show resolved Hide resolved

chitrangpatel force-pushed the step-action-results-termination-message branch from ad58095 to 56e6fe9 Compare November 13, 2023 17:07

chitrangpatel force-pushed the step-action-results-termination-message branch from 56e6fe9 to d0c55e0 Compare November 13, 2023 17:45

chitrangpatel force-pushed the step-action-results-termination-message branch from 8a9cbbe to f8732b5 Compare November 13, 2023 18:34

wlynch requested changes Nov 13, 2023

View reviewed changes

cmd/entrypoint/main.go Outdated Show resolved Hide resolved

pkg/pod/entrypoint.go Outdated Show resolved Hide resolved

pkg/entrypoint/entrypointer.go Outdated Show resolved Hide resolved

chitrangpatel force-pushed the step-action-results-termination-message branch from f8732b5 to 2f16970 Compare November 14, 2023 04:57

jerop reviewed Nov 22, 2023

View reviewed changes

pkg/pod/entrypoint.go Outdated Show resolved Hide resolved

jerop reviewed Nov 22, 2023

View reviewed changes

pkg/pod/entrypoint.go Show resolved Hide resolved

chitrangpatel force-pushed the step-action-results-termination-message branch from 86ddebf to 3da292c Compare November 22, 2023 21:34

chitrangpatel force-pushed the step-action-results-termination-message branch from 3da292c to 9a970dd Compare November 22, 2023 22:46

tekton-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Nov 22, 2023

chitrangpatel requested a review from jerop November 22, 2023 22:49

Yongxuanzhang approved these changes Nov 22, 2023

View reviewed changes

test/stepaction_results_test.go Outdated Show resolved Hide resolved

pkg/pod/status_test.go Show resolved Hide resolved

pkg/pod/entrypoint.go Show resolved Hide resolved

chitrangpatel force-pushed the step-action-results-termination-message branch from 9a970dd to 9ec9359 Compare November 23, 2023 00:24

chitrangpatel force-pushed the step-action-results-termination-message branch from 9ec9359 to e505a85 Compare November 23, 2023 02:09

jerop reviewed Nov 23, 2023

View reviewed changes

pkg/apis/pipeline/v1/result_types.go Outdated Show resolved Hide resolved

TEP-0142: Surface step results via termination message

39af932

This PR surfaces step results (i.e results written to $(step.results.<resultName>.path)) via termination messages. A followup PR will handle surfacing the results via sidecar logs.

chitrangpatel force-pushed the step-action-results-termination-message branch from e505a85 to 39af932 Compare November 23, 2023 04:54

chitrangpatel requested a review from jerop November 23, 2023 05:09

jerop reviewed Nov 23, 2023

View reviewed changes

tekton-robot assigned jerop Nov 23, 2023

tekton-robot added the lgtm Indicates that a PR is ready to be merged. label Nov 23, 2023

tekton-robot merged commit b395663 into tektoncd:main Nov 23, 2023
13 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

TEP-0142: Surface step results via termination message #7349

TEP-0142: Surface step results via termination message #7349

chitrangpatel commented Nov 10, 2023 •

edited

Loading

chitrangpatel commented Nov 10, 2023

tekton-robot commented Nov 13, 2023

tekton-robot commented Nov 13, 2023

tekton-robot commented Nov 13, 2023

vdemeester left a comment

tekton-robot commented Nov 13, 2023

tekton-robot commented Nov 13, 2023

chitrangpatel commented Nov 13, 2023 •

edited

Loading

tekton-robot commented Nov 13, 2023

tekton-robot commented Nov 13, 2023

tekton-robot commented Nov 13, 2023

tekton-robot commented Nov 13, 2023

tekton-robot commented Nov 13, 2023

tekton-robot commented Nov 22, 2023

tekton-robot commented Nov 22, 2023

tekton-robot commented Nov 22, 2023

tekton-robot commented Nov 22, 2023

tekton-robot commented Nov 22, 2023

tekton-robot commented Nov 22, 2023

tekton-robot commented Nov 23, 2023

tekton-robot commented Nov 23, 2023

tekton-robot commented Nov 23, 2023

tekton-robot commented Nov 23, 2023

tekton-robot commented Nov 23, 2023

tekton-robot commented Nov 23, 2023

jerop left a comment

TEP-0142: Surface step results via termination message #7349

TEP-0142: Surface step results via termination message #7349

Conversation

chitrangpatel commented Nov 10, 2023 • edited Loading

Changes

Submitter Checklist

Release Notes

chitrangpatel commented Nov 10, 2023

tekton-robot commented Nov 13, 2023

tekton-robot commented Nov 13, 2023

tekton-robot commented Nov 13, 2023

vdemeester left a comment

Choose a reason for hiding this comment

tekton-robot commented Nov 13, 2023

tekton-robot commented Nov 13, 2023

chitrangpatel commented Nov 13, 2023 • edited Loading

tekton-robot commented Nov 13, 2023

tekton-robot commented Nov 13, 2023

tekton-robot commented Nov 13, 2023

tekton-robot commented Nov 13, 2023

tekton-robot commented Nov 13, 2023

tekton-robot commented Nov 22, 2023

tekton-robot commented Nov 22, 2023

tekton-robot commented Nov 22, 2023

tekton-robot commented Nov 22, 2023

tekton-robot commented Nov 22, 2023

tekton-robot commented Nov 22, 2023

tekton-robot commented Nov 23, 2023

tekton-robot commented Nov 23, 2023

tekton-robot commented Nov 23, 2023

tekton-robot commented Nov 23, 2023

tekton-robot commented Nov 23, 2023

tekton-robot commented Nov 23, 2023

jerop left a comment

Choose a reason for hiding this comment

chitrangpatel commented Nov 10, 2023 •

edited

Loading

chitrangpatel commented Nov 13, 2023 •

edited

Loading