Add subPath to WorkspacePipelineTaskBinding #2491

jlpettersson · 2020-04-24T20:59:57Z

Changes

Use case: Use two instances of a task that writes to the same workspace volume - but
they write to different path of the volume. A typical use case is two git-clone tasks
that clone two git repositories, but the files should be located in two different
directories on the workspace volume.

This can currently be solved when using PersistentVolumeClaim based workspace, by declaring multiple workspaces using the same PVC but different subPath (in the WorkspaceBinding of *Run). But that does not work when using a VolumeClaimTemplate based workspace.

This commit solves this by adding the subPath field to the WorkspacePipelineTaskBinding.

If the WorkspaceBinding in PipelineRun also has a subPath set, they will be appended, e.g. subPath/subPath. This was discussed in the API WG.

Example log output from the example:

$ tkn pr logs pr-gj4l9
[writer-1 : write] + echo bar

[writer-2 : write] + echo bar

[read-all : read-1] + cat /workspace/local-ws/dir-1/foo
[read-all : read-1] + grep bar
[read-all : read-1] bar

[read-all : read-2] + cat /workspace/local-ws/dir-2/foo
[read-all : read-2] + grep bar
[read-all : read-2] bar

Fixes #2490

Submitter Checklist

These are the criteria that every PR should meet, please check them off as you
review them:

Includes tests (if functionality changed/added)
Includes docs (if user facing)
Commit messages follow commit message best practices

See the contribution guide for more details.

Double check this list of stuff that's easy to miss:

If you are adding a new binary/image to the cmd dir, please update
the release Task to build and release this image.

Reviewer Notes

If API changes are included, additive changes must be approved by at least two OWNERS and backwards incompatible changes must be approved by more than 50% of the OWNERS, and they must first be added in a backwards compatible way.

tekton-robot · 2020-04-24T21:00:12Z

Hi @jlpettersson. 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.

jlpettersson · 2020-04-25T18:31:06Z

This should probably be on hold until it is discussed

vdemeester · 2020-04-27T09:10:56Z

/ok-to-test
/hold

tekton-robot · 2020-04-27T09:13:12Z

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

File	Old Coverage	New Coverage	Delta
pkg/apis/pipeline/v1beta1/pipelinerun_validation.go	96.0%	96.3%	0.3

tekton-robot · 2020-04-27T11:01:49Z

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

File	Old Coverage	New Coverage	Delta
pkg/apis/pipeline/v1beta1/pipelinerun_validation.go	96.0%	96.3%	0.3

tekton-robot · 2020-04-27T19:42: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
pkg/reconciler/pipelinerun/pipelinerun.go	72.6%	72.8%	0.2

jlpettersson · 2020-04-27T19:45:28Z

/retest

tekton-robot · 2020-04-27T19:49:27Z

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	72.6%	72.8%	0.2

jlpettersson · 2020-04-27T19:55:34Z

This was discussed in the API WG today, and the code is now updated to reflect the discussion, e.g. append subPath from PipelineRun subPath and PipelineTask subPath if both is set.

I think we can remove the hold now.

ghost

Suggest including some info in docs/workspaces.md too, in the "Using Workspaces in Pipelines` section. Something like:

Include a `subPath` in the workspace binding to mount different parts of the same volume for different Tasks. See [a full example of this kind of Pipeline](../examples/v1beta1/pipelineruns/pipelinerun-using-different-subpaths-of-workspace.yaml) which writes data to two adjacent directories on the same Volume.

The `subPath` specified in a `Pipeline` will be appended to any `subPath` specified as part of the `PipelineRun` workspace declaration. So a `PipelineRun` declaring a Workspace with `subPath` of `/foo` for a `Pipeline` who binds it to a `Task` with `subPath` of `/bar` will end up mounting the `Volume`'s `/foo/bar` directory.

pkg/reconciler/pipelinerun/pipelinerun.go

ghost · 2020-04-27T19:59:31Z

/hold cancel

tekton-robot · 2020-04-27T20:23:03Z

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	72.6%	72.8%	0.2

Use case: Use two instances of a task that writes to the same workspace volume - but they write to different path of the volume. A typical use case is two git-clone tasks that clone two git repositories, but the files should be located in two different directories on the workspace volume. This commit solves this by adding the `subPath` field to the WorkspacePipelineTaskBinding.

tekton-robot · 2020-04-27T21:26:00Z

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	72.6%	72.8%	0.2

vdemeester

/lgtm
/meow

tekton-robot · 2020-04-28T13:06:43Z

@vdemeester:

In response to this:

/lgtm
/meow

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.

tekton-robot · 2020-04-28T13:06:46Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: sbwsg, vdemeester

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~~ [sbwsg,vdemeester]

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

jlpettersson · 2020-04-28T15:39:08Z

/retest

Jonas has recently become a regularly contributor. He started with adding a minor [_missing_ `omitempty`](tektoncd/pipeline#2301) and then [proposed some ideas](tektoncd/pipeline#1986 (comment)) around workspaces and PersistentVolumeClaim creation and continued to [elaborate around those ideas](tektoncd/pipeline#1986 (comment)). A sunny day a few days later, he also submitted an [extensive implementation for volumeClaimTemplate](tektoncd/pipeline#2326), corresponding to the idea discussions. A few days later submitted a [small refactoring PR](tektoncd/pipeline#2392), and he also listened to community members that [proposed changes](tektoncd/pipeline#2450) to his implementation about volumeClaimTemplates and did an [implementation for that proposal](tektoncd/pipeline#2453). A rainy day, he also wrote [technical documentation about PVCs](tektoncd/pipeline#2521) including adding an example that caused _flaky_ integration tests for the whole community during multiple days. When he understood his mistake, he submitted a [removal of the example](tektoncd/pipeline#2546) that caused flaky tests. He has also put his toe into Tekton Catalog and [contributed to the buildah task](tektoncd/pipeline#2546). This has followed, mostly with more PRs to the Pipeline project: - tektoncd/pipeline#2460 - tektoncd/pipeline#2491 - tektoncd/pipeline#2502 - tektoncd/pipeline#2506 - tektoncd/pipeline#2632 - tektoncd/pipeline#2633 - tektoncd/pipeline#2634 - tektoncd/pipeline#2636 - tektoncd/pipeline#2601 - tektoncd/pipeline#2630 Jonas is excited about the great community around Tekton and the project! He now would like to join the org.

tekton-robot requested review from afrittoli and a user April 24, 2020 21:00

tekton-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Apr 24, 2020

jlpettersson force-pushed the add_subpath_to_pipelinetask branch from 480899d to 30ee0ed Compare April 24, 2020 23:08

jlpettersson force-pushed the add_subpath_to_pipelinetask branch from 9f9d1bd to 62bde62 Compare April 27, 2020 19:39

ghost reviewed Apr 27, 2020

View reviewed changes

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

tekton-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Apr 27, 2020

jlpettersson force-pushed the add_subpath_to_pipelinetask branch from 62bde62 to 3f3d21c Compare April 27, 2020 20:16

jlpettersson force-pushed the add_subpath_to_pipelinetask branch from 3f3d21c to 328e551 Compare April 27, 2020 21:22

ghost approved these changes Apr 28, 2020

View reviewed changes

tekton-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 28, 2020

vdemeester approved these changes Apr 28, 2020

View reviewed changes

tekton-robot assigned vdemeester Apr 28, 2020

tekton-robot added the lgtm Indicates that a PR is ready to be merged. label Apr 28, 2020

tekton-robot merged commit a593e32 into tektoncd:master Apr 28, 2020

afrittoli added the kind/feature Categorizes issue or PR as related to a new feature. label Apr 30, 2020

jlpettersson mentioned this pull request May 19, 2020

Add Jonas to the org tektoncd/community#106

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add subPath to WorkspacePipelineTaskBinding #2491

Add subPath to WorkspacePipelineTaskBinding #2491

jlpettersson commented Apr 24, 2020 •

edited

Loading

tekton-robot commented Apr 24, 2020

jlpettersson commented Apr 25, 2020

vdemeester commented Apr 27, 2020

tekton-robot commented Apr 27, 2020

tekton-robot commented Apr 27, 2020

tekton-robot commented Apr 27, 2020

jlpettersson commented Apr 27, 2020

tekton-robot commented Apr 27, 2020

jlpettersson commented Apr 27, 2020

ghost left a comment •

edited by ghost

Loading

ghost commented Apr 27, 2020

tekton-robot commented Apr 27, 2020

tekton-robot commented Apr 27, 2020

vdemeester left a comment

tekton-robot commented Apr 28, 2020

tekton-robot commented Apr 28, 2020

jlpettersson commented Apr 28, 2020

Add subPath to WorkspacePipelineTaskBinding #2491

Add subPath to WorkspacePipelineTaskBinding #2491

Conversation

jlpettersson commented Apr 24, 2020 • edited Loading

Changes

Submitter Checklist

Reviewer Notes

tekton-robot commented Apr 24, 2020

jlpettersson commented Apr 25, 2020

vdemeester commented Apr 27, 2020

tekton-robot commented Apr 27, 2020

tekton-robot commented Apr 27, 2020

tekton-robot commented Apr 27, 2020

jlpettersson commented Apr 27, 2020

tekton-robot commented Apr 27, 2020

jlpettersson commented Apr 27, 2020

ghost left a comment • edited by ghost Loading

Choose a reason for hiding this comment

ghost commented Apr 27, 2020

tekton-robot commented Apr 27, 2020

tekton-robot commented Apr 27, 2020

vdemeester left a comment

Choose a reason for hiding this comment

tekton-robot commented Apr 28, 2020

tekton-robot commented Apr 28, 2020

jlpettersson commented Apr 28, 2020

jlpettersson commented Apr 24, 2020 •

edited

Loading

ghost left a comment •

edited by ghost

Loading