TEP-0111 - Propagating workspaces in taskruns #5081

chitrangpatel · 2022-07-05T18:46:54Z

The verbosity of writing specifications in Tekton Pipelines is a common pain point that creates difficulties in getting-started scenarios. In addition, the verbosity leads to long specifications that are error-prone, harder to maintain, and reach the etcd size limits for CRDs. Automatically propagating Workspaces will lead to better user experience. This addresses features proposed in TEP-0111. This PR addresses propagating workspaces in TaskRuns. A follow up PR is addressing propagating workspaces in PipelineRuns.

Changes

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

Workspaces are propagated in embedded specifications without mutations.

tekton-robot · 2022-07-05T18:52: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
pkg/reconciler/pipelinerun/pipelinerun.go	86.2%	84.7%	-1.5
pkg/reconciler/taskrun/taskrun.go	80.9%	80.9%	-0.0
pkg/workspace/apply.go	100.0%	98.8%	-1.2
pkg/workspace/validate.go	100.0%	96.3%	-3.7

tekton-robot · 2022-07-05T19:07:40Z

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/pipelinerun.go	86.2%	84.7%	-1.5
pkg/reconciler/taskrun/taskrun.go	80.9%	80.9%	-0.0
pkg/workspace/apply.go	100.0%	98.8%	-1.2
pkg/workspace/validate.go	100.0%	96.3%	-3.7

tekton-robot · 2022-07-11T16:29:09Z

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/pipelinerun.go	86.2%	84.7%	-1.5
pkg/reconciler/taskrun/taskrun.go	80.9%	80.9%	-0.0
pkg/workspace/apply.go	100.0%	98.8%	-1.2
pkg/workspace/validate.go	100.0%	96.3%	-3.7

chitrangpatel · 2022-07-11T16:41:45Z

/retest

jerop · 2022-07-11T20:26:45Z

/kind feature

jerop · 2022-07-11T20:26:58Z

/retest

jerop · 2022-07-12T12:17:06Z

/retest

chitrangpatel · 2022-07-12T14:03:23Z

/test check-pr-has-kind-label

tekton-robot · 2022-07-12T14:03:26Z

@chitrangpatel: The specified target(s) for /test were not found.
The following commands are available to trigger required jobs:

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

The following commands are available to trigger optional jobs:

/test pull-tekton-pipeline-go-coverage
/test pull-tekton-pipeline-kind-alpha-integration-tests
/test pull-tekton-pipeline-kind-alpha-yaml-tests
/test pull-tekton-pipeline-kind-integration-tests
/test pull-tekton-pipeline-kind-yaml-tests

Use /test all to run the following jobs that were automatically triggered:

pull-tekton-pipeline-alpha-integration-tests
pull-tekton-pipeline-build-tests
pull-tekton-pipeline-go-coverage
pull-tekton-pipeline-integration-tests
pull-tekton-pipeline-unit-tests

In response to this:

/test check-pr-has-kind-label

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.

chitrangpatel · 2022-07-12T18:12:59Z

/kind feature

tekton-robot · 2022-07-12T18:17: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/reconciler/pipelinerun/pipelinerun.go	86.2%	84.7%	-1.5
pkg/reconciler/taskrun/taskrun.go	80.9%	80.9%	-0.0
pkg/workspace/apply.go	100.0%	98.8%	-1.2
pkg/workspace/validate.go	100.0%	96.3%	-3.7

chitrangpatel · 2022-07-12T18:50:29Z

/retest

jerop · 2022-07-12T18:54:48Z

/assign @dibyom

tekton-robot · 2022-09-09T15:52:45Z

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/v1/task_types.go	0.0%	96.4%	96.4
pkg/apis/pipeline/v1beta1/task_types.go	0.0%	87.5%	87.5
pkg/reconciler/taskrun/taskrun.go	80.7%	81.0%	0.3
pkg/substitution/substitution.go	62.2%	63.0%	0.8

lbernick

Thank you Chitrang for splitting into multiple PRs! it's a big help

pkg/reconciler/taskrun/taskrun.go

pkg/substitution/substitution.go

docs/tasks.md

tekton-robot · 2022-09-09T17:49: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
pkg/apis/pipeline/v1/task_types.go	0.0%	96.4%	96.4
pkg/apis/pipeline/v1beta1/task_types.go	0.0%	87.5%	87.5
pkg/reconciler/taskrun/taskrun.go	80.7%	81.0%	0.3
pkg/substitution/substitution.go	62.2%	63.0%	0.8

tekton-robot · 2022-09-09T17:57: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
pkg/apis/pipeline/v1/task_types.go	0.0%	96.4%	96.4
pkg/apis/pipeline/v1beta1/task_types.go	0.0%	87.5%	87.5
pkg/reconciler/taskrun/taskrun.go	80.7%	81.0%	0.3

tekton-robot · 2022-09-09T18:03:10Z

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/v1/task_types.go	0.0%	96.4%	96.4
pkg/apis/pipeline/v1beta1/task_types.go	0.0%	87.5%	87.5
pkg/reconciler/taskrun/taskrun.go	80.7%	81.0%	0.3

lbernick · 2022-09-12T14:52:42Z

pkg/workspace/apply_test.go

+ Sidecars: []v1beta1.Sidecar{{
+ Name: "conflicting volume mount sidecar",
+ VolumeMounts: []corev1.VolumeMount{{
+ Name: "mount-path-conflicts",


it looks like this test case is for conflicting mount paths, can you rename to reflect what's being tested here?

No it wasn't. Its my mistake. Copy and pasting the above test and then modifying it lead to this confusion. I apologize. I will update it to better reflect the test its actually doing.

I made this into its own test to check the functionality I added in apply.go

lbernick · 2022-09-12T14:54:11Z

pkg/apis/pipeline/v1beta1/task_types.go

+}
+
+// FindWorkspaceSubstitutionLocationsInTask returns a set of all the locations in the TaskSpec where we can apply substitutions.
+func (ts *TaskSpec) FindWorkspaceSubstitutionLocationsInTask() sets.String {


Where is this function being used? It looks like this is functionality for replacing workspaces.foo.path syntax but I don't see it being used anywhere-- is this replacement happening as part of this PR?
If this function does need to be in this PR, I don't think it should be exported.

Good point. Even though it is written in task_types, it might be being called from the PR handling propagated workspaces in pipelinerun.

In that case, how is this PR performing substitutions for task scripts/args containing $(workspaces.foo.path)?

This function was being called in the pipelinerun reconciler to parse the pipelineTask and identify which workspaces are being called. The reconciler then propagated those workspaces to the pipelineTasks by creating task bindings.

The taskRun reconciler (reconciler/taskrun/taskrun.go) does a similar thing and propagates workspaces to the taskSpec.
The substitutions happen the way they did before. i.e. via https://github.com/chitrangpatel/pipeline/blob/e302587a9dcbc7af8cc1475dfa24c3c35a6715f9/pkg/reconciler/pipelinerun/resources/apply.go#L173-L185.

tekton-robot · 2022-09-12T15:39:21Z

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/taskrun/taskrun.go	80.7%	81.0%	0.3

docs/tasks.md

pkg/reconciler/taskrun/taskrun_test.go

docs/taskruns.md

tekton-robot · 2022-09-12T20:56:32Z

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/taskrun/taskrun.go	80.7%	81.2%	0.5

This POC illustrates propagating workspaces defined at the `pipelinerun` stage and directly referred at the `spec`. There is no need to specify it in the `pipelinespec` followed by `tasks` and `taskspec`.

tekton-robot · 2022-09-13T15:50:31Z

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/taskrun/taskrun.go	80.7%	81.3%	0.6

chitrangpatel · 2022-09-13T15:53:15Z

/retest

chitrangpatel · 2022-09-13T16:09:38Z

/retest

lbernick

/lgtm

lbernick · 2022-09-14T20:57:02Z

pkg/reconciler/taskrun/taskrun_test.go

@@ -2545,8 +2590,14 @@ spec:
 Kind: "Task",
 TaskSpec: &v1beta1.TaskSpec{Steps: simpleTask.Spec.Steps, Workspaces: simpleTask.Spec.Workspaces},
 }
- taskSpec := updateTaskSpecParamsContextsResults(context.Background(), taskRun, rtr)
- pod, err := r.createPod(testAssets.Ctx, taskSpec, taskRun, rtr)
+ ctx := config.EnableAlphaAPIFields(context.Background())


would you mind creating an issue to refactor these tests to call Reconcile rather than all of these helpers?

Sure. I can do that.

tekton-robot added the do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. label Jul 5, 2022

tekton-robot requested review from dibyom and jerop July 5, 2022 18:47

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

chitrangpatel force-pushed the TEP-0111-Propagating-Workspaces branch from b77584a to 2000cb1 Compare July 5, 2022 19:04

chitrangpatel force-pushed the TEP-0111-Propagating-Workspaces branch from 2000cb1 to 5fe2d48 Compare July 11, 2022 16:24

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

tekton-robot added the kind/feature Categorizes issue or PR as related to a new feature. label Jul 11, 2022

chitrangpatel closed this Jul 12, 2022

chitrangpatel reopened this Jul 12, 2022

chitrangpatel changed the title ~~TEP-0111 - Propagating workspaces~~ TEP-0111 - Propagating workspaces in task runs Sep 9, 2022

chitrangpatel changed the title ~~TEP-0111 - Propagating workspaces in task runs~~ TEP-0111 - Propagating workspaces in taskruns Sep 9, 2022

chitrangpatel mentioned this pull request Sep 9, 2022

TEP-0111 - Propagating workspaces in pipelinerun #5463

Merged

7 tasks

lbernick reviewed Sep 9, 2022

View reviewed changes

pkg/reconciler/taskrun/taskrun.go Outdated Show resolved Hide resolved

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

docs/tasks.md Outdated Show resolved Hide resolved

chitrangpatel force-pushed the TEP-0111-Propagating-Workspaces branch from 93a8d35 to 56228ef Compare September 9, 2022 17:44

chitrangpatel force-pushed the TEP-0111-Propagating-Workspaces branch from 56228ef to 13ef6d8 Compare September 9, 2022 17:51

chitrangpatel force-pushed the TEP-0111-Propagating-Workspaces branch from 13ef6d8 to 2de5f8b Compare September 9, 2022 17:58

lbernick reviewed Sep 12, 2022

View reviewed changes

chitrangpatel force-pushed the TEP-0111-Propagating-Workspaces branch from 2de5f8b to e302587 Compare September 12, 2022 15:34

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 Sep 12, 2022

lbernick reviewed Sep 12, 2022

View reviewed changes

docs/tasks.md Outdated Show resolved Hide resolved

pkg/reconciler/taskrun/taskrun_test.go Show resolved Hide resolved

pkg/reconciler/taskrun/taskrun_test.go Show resolved Hide resolved

docs/taskruns.md Outdated Show resolved Hide resolved

docs/taskruns.md Show resolved Hide resolved

chitrangpatel force-pushed the TEP-0111-Propagating-Workspaces branch from e302587 to bad5e4b Compare September 12, 2022 20:46

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

chitrangpatel force-pushed the TEP-0111-Propagating-Workspaces branch from bad5e4b to a51171d Compare September 12, 2022 20:52

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

TEP-0111 - Propagating workspaces in Taskrun

62f0f52

This POC illustrates propagating workspaces defined at the `pipelinerun` stage and directly referred at the `spec`. There is no need to specify it in the `pipelinespec` followed by `tasks` and `taskspec`.

chitrangpatel force-pushed the TEP-0111-Propagating-Workspaces branch from a51171d to 62f0f52 Compare September 13, 2022 15:45

lbernick reviewed Sep 14, 2022

View reviewed changes

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

tekton-robot merged commit 0b3d1e7 into tektoncd:main Sep 14, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

TEP-0111 - Propagating workspaces in taskruns #5081

TEP-0111 - Propagating workspaces in taskruns #5081

chitrangpatel commented Jul 5, 2022 •

edited

Loading

tekton-robot commented Jul 5, 2022

tekton-robot commented Jul 5, 2022

tekton-robot commented Jul 11, 2022

chitrangpatel commented Jul 11, 2022

jerop commented Jul 11, 2022

jerop commented Jul 11, 2022

jerop commented Jul 12, 2022

chitrangpatel commented Jul 12, 2022

tekton-robot commented Jul 12, 2022

chitrangpatel commented Jul 12, 2022

tekton-robot commented Jul 12, 2022

chitrangpatel commented Jul 12, 2022

jerop commented Jul 12, 2022

tekton-robot commented Sep 9, 2022

lbernick left a comment

tekton-robot commented Sep 9, 2022

tekton-robot commented Sep 9, 2022

tekton-robot commented Sep 9, 2022

lbernick Sep 12, 2022

chitrangpatel Sep 12, 2022

chitrangpatel Sep 12, 2022

lbernick Sep 12, 2022

chitrangpatel Sep 12, 2022

lbernick Sep 12, 2022

chitrangpatel Sep 12, 2022

tekton-robot commented Sep 12, 2022

tekton-robot commented Sep 12, 2022

tekton-robot commented Sep 13, 2022

chitrangpatel commented Sep 13, 2022

chitrangpatel commented Sep 13, 2022

lbernick left a comment

lbernick Sep 14, 2022

chitrangpatel Sep 15, 2022

TEP-0111 - Propagating workspaces in taskruns #5081

TEP-0111 - Propagating workspaces in taskruns #5081

Conversation

chitrangpatel commented Jul 5, 2022 • edited Loading

Changes

Submitter Checklist

Release Notes

tekton-robot commented Jul 5, 2022

tekton-robot commented Jul 5, 2022

tekton-robot commented Jul 11, 2022

chitrangpatel commented Jul 11, 2022

jerop commented Jul 11, 2022

jerop commented Jul 11, 2022

jerop commented Jul 12, 2022

chitrangpatel commented Jul 12, 2022

tekton-robot commented Jul 12, 2022

chitrangpatel commented Jul 12, 2022

tekton-robot commented Jul 12, 2022

chitrangpatel commented Jul 12, 2022

jerop commented Jul 12, 2022

tekton-robot commented Sep 9, 2022

lbernick left a comment

Choose a reason for hiding this comment

tekton-robot commented Sep 9, 2022

tekton-robot commented Sep 9, 2022

tekton-robot commented Sep 9, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tekton-robot commented Sep 12, 2022

tekton-robot commented Sep 12, 2022

tekton-robot commented Sep 13, 2022

chitrangpatel commented Sep 13, 2022

chitrangpatel commented Sep 13, 2022

lbernick left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chitrangpatel commented Jul 5, 2022 •

edited

Loading