TEP-0011: Add StdoutConfig and StderrConfig to steps. #4882

bradbeck · 2022-05-17T15:11:43Z

Changes

Implements Option 1 of TEP-0011

Resurrects #3103

Closes #2925

Signed-off-by: Brad Beck [email protected]

Submitter Checklist

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

Docs included if any changes are user facing
Tests included if any functionality added or changed
Follows the commit message standard
Meets the Tekton contributor standards (including
functionality, content, code)
Release notes block below has been filled in
(if there are no user facing changes, use release note "NONE")

Release Notes

Users can specify `stdoutConfig` and `stderrConfig` in steps to capture steps' stdout and stderr to local files. This feature can be used to capture stdout and stderr into task results.

tekton-robot · 2022-05-17T15:11:58Z

Hi @bradbeck. Thanks for your PR.

I'm waiting for a tektoncd member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

pritidesai · 2022-05-17T15:31:09Z

/ok-to-test

tekton-robot · 2022-05-17T15:33:19Z

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/io.go	Do not exist	86.8%
cmd/entrypoint/runner.go	81.5%	81.0%	-0.4
pkg/apis/pipeline/v1beta1/step_replacements.go	40.0%	66.7%	26.7

tekton-robot · 2022-05-17T22:11:44Z

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/io.go	Do not exist	86.8%
cmd/entrypoint/runner.go	81.5%	81.0%	-0.4
pkg/apis/pipeline/v1beta1/step_replacements.go	40.0%	66.7%	26.7
pkg/pod/entrypoint.go	88.3%	88.9%	0.6

tekton-robot · 2022-05-17T22:35:51Z

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/io.go	Do not exist	86.8%
cmd/entrypoint/runner.go	81.5%	81.0%	-0.4
pkg/apis/pipeline/v1beta1/step_replacements.go	40.0%	66.7%	26.7
pkg/pod/entrypoint.go	88.3%	88.9%	0.6

tekton-robot · 2022-05-19T14:30:34Z

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/io.go	Do not exist	86.8%
cmd/entrypoint/runner.go	81.5%	79.3%	-2.2
pkg/apis/pipeline/v1beta1/step_replacements.go	40.0%	66.7%	26.7
pkg/pod/entrypoint.go	88.3%	88.9%	0.6

tekton-robot · 2022-05-19T16:10:50Z

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/io.go	Do not exist	86.8%
cmd/entrypoint/runner.go	81.5%	84.7%	3.2
pkg/apis/pipeline/v1beta1/step_replacements.go	40.0%	66.7%	26.7
pkg/pod/entrypoint.go	88.3%	88.9%	0.6

tekton-robot · 2022-05-19T18:38:18Z

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/io.go	Do not exist	86.8%
cmd/entrypoint/runner.go	81.5%	83.6%	2.1
pkg/apis/pipeline/v1beta1/step_replacements.go	40.0%	66.7%	26.7
pkg/pod/entrypoint.go	88.3%	88.9%	0.6

bradbeck · 2022-05-19T20:53:28Z

/test pull-tekton-pipeline-alpha-integration-tests

tekton-robot · 2022-06-09T18:20: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
cmd/entrypoint/io.go	Do not exist	86.8%
cmd/entrypoint/runner.go	81.5%	83.6%	2.1
pkg/apis/pipeline/v1beta1/step_replacements.go	40.0%	66.7%	26.7
pkg/pod/entrypoint.go	88.3%	88.9%	0.6

tekton-robot · 2022-06-09T22:03:46Z

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/io.go	Do not exist	86.8%
cmd/entrypoint/runner.go	77.8%	83.6%	5.8
pkg/apis/pipeline/v1beta1/step_replacements.go	40.0%	66.7%	26.7
pkg/pod/entrypoint.go	88.3%	88.9%	0.6

bradbeck · 2022-06-16T17:38:26Z

@vdemeester @imjasonh Please take a look.

pritidesai · 2022-06-21T23:00:05Z

cmd/entrypoint/main.go

@@ -45,6 +45,8 @@ var (
 	terminationPath     = flag.String("termination_path", "/tekton/termination", "If specified, file to write upon termination")
 	results             = flag.String("results", "", "If specified, list of file names that might contain task results")
 	timeout             = flag.Duration("timeout", time.Duration(0), "If specified, sets timeout for step")
+	stdoutPath          = flag.String("stdout_path", "", "If specified, file to copy stdout to")
+	stderrPath          = flag.String("stderr_path", "", "If specified, file to copy stdout to")


s/stdout/stderr/

vdemeester

LGTM, the only question I have is wether we want to add this behind a feature-flag or not — as it is additive only, I think it is fine to add it directly, but maybe we want to hide it behind the alpha feature flag.

@bradbeck 👍🏼

bradbeck · 2022-06-23T02:00:15Z

LGTM, the only question I have is wether we want to add this behind a feature-flag or not — as it is additive only, I think it is fine to add it directly, but maybe we want to hide it behind the alpha feature flag.

@bradbeck 👍🏼

@vdemeester Thanks for the feedback. Working on understanding what it takes to put this behind the alpha feature flag.

tekton-robot · 2022-06-23T02:14:53Z

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/io.go	Do not exist	86.8%
cmd/entrypoint/runner.go	77.8%	82.1%	4.3
pkg/apis/pipeline/v1beta1/step_replacements.go	40.0%	66.7%	26.7
pkg/pod/entrypoint.go	88.3%	88.9%	0.6

tekton-robot · 2022-06-23T20:45: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
cmd/entrypoint/io.go	Do not exist	86.8%
cmd/entrypoint/runner.go	81.5%	83.6%	2.1
pkg/apis/pipeline/v1beta1/step_replacements.go	40.0%	66.7%	26.7
pkg/pod/entrypoint.go	88.3%	88.9%	0.6

tekton-robot · 2022-07-12T17:33:08Z

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/io.go	Do not exist	86.8%
cmd/entrypoint/runner.go	77.8%	81.7%	3.9
pkg/apis/pipeline/v1beta1/step_replacements.go	40.0%	66.7%	26.7
pkg/apis/pipeline/v1beta1/task_validation.go	97.6%	97.7%	0.0
pkg/pod/entrypoint.go	88.8%	89.3%	0.5

tekton-robot · 2022-07-12T17:55:11Z

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/io.go	Do not exist	86.8%
cmd/entrypoint/runner.go	77.8%	81.7%	3.9
pkg/apis/pipeline/v1beta1/step_replacements.go	40.0%	66.7%	26.7
pkg/apis/pipeline/v1beta1/task_validation.go	97.6%	97.7%	0.0
pkg/pod/entrypoint.go	88.8%	89.3%	0.5

tekton-robot · 2022-07-12T20:18:15Z

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/io.go	Do not exist	86.8%
cmd/entrypoint/runner.go	77.8%	81.7%	3.9
pkg/apis/pipeline/v1beta1/step_replacements.go	40.0%	66.7%	26.7
pkg/apis/pipeline/v1beta1/task_validation.go	97.6%	97.7%	0.0
pkg/pod/entrypoint.go	88.8%	89.4%	0.7

pkg/pod/entrypoint.go

tekton-robot · 2022-07-13T17:07:59Z

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/io.go	Do not exist	86.8%
cmd/entrypoint/runner.go	77.8%	80.3%	2.5
pkg/apis/pipeline/v1beta1/step_replacements.go	40.0%	66.7%	26.7
pkg/apis/pipeline/v1beta1/task_validation.go	97.6%	97.7%	0.0
pkg/pod/entrypoint.go	88.8%	89.3%	0.5

bradbeck · 2022-07-13T20:27:51Z

/retest

tekton-robot · 2022-07-14T17:48:35Z

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/io.go	Do not exist	86.8%
cmd/entrypoint/runner.go	81.5%	81.7%	0.2
pkg/apis/pipeline/v1beta1/step_replacements.go	40.0%	66.7%	26.7
pkg/apis/pipeline/v1beta1/task_validation.go	97.6%	97.7%	0.0
pkg/pod/entrypoint.go	88.8%	89.3%	0.5

tekton-robot · 2022-07-14T18:25:37Z

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/io.go	Do not exist	86.8%
cmd/entrypoint/runner.go	81.5%	81.7%	0.2
pkg/apis/pipeline/v1beta1/step_replacements.go	40.0%	66.7%	26.7
pkg/apis/pipeline/v1beta1/task_validation.go	97.6%	97.7%	0.0
pkg/pod/entrypoint.go	88.8%	89.3%	0.5

tekton-robot · 2022-07-14T18:42:50Z

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/io.go	Do not exist	86.8%
cmd/entrypoint/runner.go	81.5%	81.7%	0.2
pkg/apis/pipeline/v1beta1/step_replacements.go	40.0%	66.7%	26.7
pkg/apis/pipeline/v1beta1/task_validation.go	97.6%	97.7%	0.0
pkg/pod/entrypoint.go	88.8%	89.3%	0.5

tekton-robot · 2022-07-14T18:55:55Z

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/io.go	Do not exist	83.0%
cmd/entrypoint/runner.go	81.5%	78.9%	-2.6
pkg/apis/pipeline/v1beta1/step_replacements.go	40.0%	66.7%	26.7
pkg/apis/pipeline/v1beta1/task_validation.go	97.6%	97.7%	0.0
pkg/pod/entrypoint.go	88.8%	89.3%	0.5

Implements Option 1 of [TEP-0011](https://github.com/tektoncd/community/blob/master/teps/0011-redirecting-step-output-streams.md) Resurrects [#3103](#3103) Closes [#2925](#2925) Signed-off-by: Brad Beck <[email protected]>

tekton-robot · 2022-07-14T19:45:23Z

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/io.go	Do not exist	86.8%
cmd/entrypoint/runner.go	81.5%	81.7%	0.2
pkg/apis/pipeline/v1beta1/step_replacements.go	40.0%	66.7%	26.7
pkg/apis/pipeline/v1beta1/task_validation.go	97.6%	97.7%	0.0
pkg/pod/entrypoint.go	88.8%	89.3%	0.5

afrittoli · 2022-07-14T21:53:18Z

yay, tests passing now!! 😎

afrittoli · 2022-07-15T10:27:21Z

pkg/reconciler/taskrun/taskrun_test.go

@@ -1239,6 +1239,171 @@ spec:
 	}
 }

+func TestAlphaReconcile(t *testing.T) {


afrittoli · 2022-07-15T10:31:03Z

docs/tasks.md

+    volumeMounts:
+    - name: data
+      mountPath: /data
+  volumes:
+  - name: data


We generally discourage use of volumes in favour of workspaces which is the preferred authoring time abstraction. Could you change the example here to use workspaces instead, as users will often copy examples from the reference docs. Thank you!

afrittoli · 2022-07-15T10:32:31Z

docs/tasks.md

+
+> NOTE:
+>
+> - If the intent is to share output between `Step`s via a file, the user must ensure that the paths provided are shared between the `Step`s (e.g via `volumes`).


Users should use a workspace rather than a volume. The workspace can be bound at runtime to an emptyDir is the required sharing is only within the Task.

afrittoli

Thank you so much for this feature! 🎉

I would very much like to see the suggested update to the docs before this is merged, but otherwise everything LGTM, code, docs and test coverage.

/approve

tekton-robot · 2022-07-15T12:21:10Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: afrittoli

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~~ [afrittoli]

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

vdemeester

/lgtm

pritidesai · 2022-07-15T16:56:50Z

Very excited to try this out 🎉 Thank you @bradbeck for implementing this 🙏

Thank you @afrittoli and @vdemeester for the reviews 🙏

TEP-0011 was implemented in tektoncd/pipeline#4882. This change updates the TEP state from `implementable` to `implemented`.

tekton-robot added the release-note Denotes a PR that will be considered when it comes time to generate release notes. label May 17, 2022

tekton-robot requested review from bobcatfish and imjasonh May 17, 2022 15:11

tekton-robot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label May 17, 2022

tekton-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label May 17, 2022

tekton-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels May 17, 2022

vdemeester self-assigned this May 18, 2022

tekton-robot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Jun 9, 2022

pritidesai added this to the Pipelines v0.38 milestone Jun 13, 2022

tekton-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 17, 2022

pritidesai reviewed Jun 21, 2022

View reviewed changes

vdemeester reviewed Jun 22, 2022

View reviewed changes

tekton-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 23, 2022

afrittoli reviewed Jul 12, 2022

View reviewed changes

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

TEP-0011: Add StdoutConfig and StderrConfig to steps.

3c1689c

Implements Option 1 of [TEP-0011](https://github.com/tektoncd/community/blob/master/teps/0011-redirecting-step-output-streams.md) Resurrects [#3103](#3103) Closes [#2925](#2925) Signed-off-by: Brad Beck <[email protected]>

afrittoli reviewed Jul 15, 2022

View reviewed changes

pkg/reconciler/taskrun/taskrun_test.go

@@ -1239,6 +1239,171 @@ spec:

}

}

func TestAlphaReconcile(t *testing.T) {

Copy link

Member

afrittoli Jul 15, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤩

afrittoli reviewed Jul 15, 2022

View reviewed changes

tekton-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 15, 2022

vdemeester reviewed Jul 15, 2022

View reviewed changes

tekton-robot added the lgtm Indicates that a PR is ready to be merged. label Jul 15, 2022

tekton-robot merged commit a0c7d31 into tektoncd:main Jul 15, 2022

bradbeck deleted the tep-0011-stdoutconfig-stderrconfig branch July 15, 2022 12:54

abayer mentioned this pull request Jul 20, 2022

Release Pipeline failing #5174

Closed

This was referenced Jul 22, 2022

Add StdoutConfig and StderrorConfig to v1 Step #5199

Merged

flake: TestRealRunnerStdoutAndStderr* unit tests #5152

Closed

jerop added a commit to jerop/community that referenced this pull request Mar 21, 2023

TEP-0011: Redirecting Step Output Streams - Mark as Implemented

93022c3

TEP-0011 was implemented in tektoncd/pipeline#4882. This change updates the TEP state from `implementable` to `implemented`.

jerop mentioned this pull request Mar 21, 2023

TEP-0011: Redirecting Step Output Streams - Mark as Implemented tektoncd/community#985

Merged

TEP-0011: Add StdoutConfig and StderrConfig to steps. #4882

TEP-0011: Add StdoutConfig and StderrConfig to steps. #4882

Conversation

bradbeck commented May 17, 2022 • edited Loading

Changes

Submitter Checklist

Release Notes

tekton-robot commented May 17, 2022

pritidesai commented May 17, 2022

tekton-robot commented May 17, 2022

tekton-robot commented May 17, 2022

tekton-robot commented May 17, 2022

tekton-robot commented May 19, 2022

tekton-robot commented May 19, 2022

tekton-robot commented May 19, 2022

bradbeck commented May 19, 2022

tekton-robot commented Jun 9, 2022

tekton-robot commented Jun 9, 2022

bradbeck commented Jun 16, 2022

pritidesai Jun 21, 2022

Choose a reason for hiding this comment

vdemeester left a comment

Choose a reason for hiding this comment

bradbeck commented Jun 23, 2022

tekton-robot commented Jun 23, 2022

tekton-robot commented Jun 23, 2022

tekton-robot commented Jul 12, 2022

tekton-robot commented Jul 12, 2022

tekton-robot commented Jul 12, 2022

tekton-robot commented Jul 13, 2022

bradbeck commented Jul 13, 2022

tekton-robot commented Jul 14, 2022

tekton-robot commented Jul 14, 2022

tekton-robot commented Jul 14, 2022

tekton-robot commented Jul 14, 2022

tekton-robot commented Jul 14, 2022

afrittoli commented Jul 14, 2022

afrittoli Jul 15, 2022

Choose a reason for hiding this comment

afrittoli Jul 15, 2022

Choose a reason for hiding this comment

afrittoli Jul 15, 2022

Choose a reason for hiding this comment

afrittoli left a comment

Choose a reason for hiding this comment

tekton-robot commented Jul 15, 2022

vdemeester left a comment

Choose a reason for hiding this comment

pritidesai commented Jul 15, 2022

bradbeck commented May 17, 2022 •

edited

Loading