Add "volume" PipelineResource 🔊 #1416
Conversation
This will allow copying content either into or out of a `TaskRun`, either to an existing volume or a newly created volume. The immediate use case is for copying a pipeline's workspace to be made available as the input for another pipeline's workspace without needing to deal with uploading everything to a bucket. The volume, whether already existing or created, will not be deleted at the end of the `PipelineRun`, unlike the artifact storage PVC. The Volume resource is a sub-type of the general Storage resource. Since this type will require the creation of a PVC to function (to be configurable later), this commit adds a Setup interface that PipelineResources can implement if they need to do setup that involves instantiating objects in Kube. This could be a place to later add features like caching, and also is the sort of design we'd expect once PipelineResources are extensible (PipelineResources will be free to do whatever setup they need). The behavior of this volume resource is: 1. For inputs, copy data _from_ the PVC to the workspace path 2. For outputs, copy data _to_ the PVC from the workspace path If a user does want to control where the data is copied from, they can: 1. Add a step that copies from the location they want to copy from on disk to /workspace/whatever 2. Use the "targetPath" argument in the PipelineResource to control the location the data is copied to (still relative to targetPath https://github.com/tektoncd/pipeline/blob/master/docs/resources.md#controlling-where-resources-are-mounted) 3. Use `path` https://github.com/tektoncd/pipeline/blob/master/docs/resources.md#overriding-where-resources-are-copied-from (tbd if we want to keep this feature post PVC) The underlying PVC will need to be created by the Task reonciler, if only a TaskRun is being used, or by the PipelineRun reconciler if a Pipeline is being used. The PipelineRun reconciler cannot delegate this to the TaskRun reconciler b/c when two different reconcilers create PVCs and Tekton is running on a regional GKE cluster, they can get created in different zones, resulting in a pod that tries to use both being unschedulable. In order to actually schedule a pod using two volume resources, we had to: - Use a storage class that can be scheduled in a GKE regional cluster https://cloud.google.com/kubernetes-engine/docs/how-to/persistent-volumes/regional-pd - Either use the same storage class for the PVC attached automatically for input/output linking or don't use the PVC (chose the latter!) This commit removes automatic PVC copying for input output linking of the VolumeResource b/c since it itself is a PVC, there is no need to copy between an intermediate PVCs. This makes it simpler to make a Task using the VolumeResource schedulable, removes redundant copying, and removes a side effect where if a VolumeResources output was linked to an input, the Task with the input would see _only_ the changes made by the output and none of the other contents of the PVC. Also removing the docs on the `paths` param (i.e. "overriding where resources are copied from") because it was implemented such that it would only work in the output -> input linking PVC case and can't actually be used by users and it will be removed in tektoncd#1284. fixes tektoncd#1062 Co-authored-by: Dan Lorenc <lorenc.d@gmail.com> Co-authored-by: Christie Wilson <bobcatfish@gmail.com>
|
We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google. ℹ️ Googlers: Go here for more info. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
tekton-robot
added
the
size/XXL
bobcatfish
added
cla: yes
|
A Googler has manually verified that the CLAs look good. (Googler, please make sure the reason for overriding the CLA status is clearly documented in these comments.) ℹ️ Googlers: Go here for more info. |
|
What's the behavior vis a vis cleanup of the volumes? I forget if we're doing that currently, but if we are automatically cleaning up these new volumes, we want to make sure that's optional. |
Currently they aren't cleaned up at all! 😓 |
|
|
|
... i cant push to dan's fork now. I think i need to reopen this one more time. |
|
heheheh Also, could we get an example of using a pre-existing volume? |
|
@bobcatfish: The following tests failed, say
Full PR test history. Your PR dashboard. 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. I understand the commands that are listed here. |
Changes
This will allow copying content either into or out of a
TaskRun,either to an existing volume or a newly created volume. The immediate
use case is for copying a pipeline's workspace to be made available as
the input for another pipeline's workspace without needing to deal
with uploading everything to a bucket. The volume, whether already
existing or created, will not be deleted at the end of the
PipelineRun, unlike the artifact storage PVC.The Volume resource is a sub-type of the general Storage resource.
Since this type will require the creation of a PVC to function (to be
configurable later), this commit adds a Setup interface that
PipelineResources can implement if they need to do setup that involves
instantiating objects in Kube. This could be a place to later add
features like caching, and also is the sort of design we'd expect once
PipelineResources are extensible (PipelineResources will be free to do
whatever setup they need).
The behavior of this volume resource is:
If a user does want to control where the data is copied from, they can:
disk to /workspace/whatever
location the data is copied to (still relative to targetPath
https://github.com/tektoncd/pipeline/blob/master/docs/resources.md#controlling-where-resources-are-mounted)
pathhttps://github.com/tektoncd/pipeline/blob/master/docs/resources.md#overriding-where-resources-are-copied-from(tbd if we want to keep this feature post PVC)
The underlying PVC will need to be created by the Task reonciler, if
only a TaskRun is being used, or by the PipelineRun reconciler if a
Pipeline is being used. The PipelineRun reconciler cannot delegate this
to the TaskRun reconciler b/c when two different reconcilers create PVCs
and Tekton is running on a regional GKE cluster, they can get created in
different zones, resulting in a pod that tries to use both being
unschedulable.
In order to actually schedule a pod using two volume resources, we had
to:
https://cloud.google.com/kubernetes-engine/docs/how-to/persistent-volumes/regional-pd
for input/output linking or don't use the PVC (chose the latter!)
This commit removes automatic PVC copying for input output linking of
the VolumeResource b/c since it itself is a PVC, there is no need to
copy between an intermediate PVCs. This makes it simpler to make a Task
using the VolumeResource schedulable, removes redundant copying, and
removes a side effect where if a VolumeResources output was linked to an
input, the Task with the input would see only the changes made by the
output and none of the other contents of the PVC.
Also removing the docs on the
pathsparam (i.e. "overriding whereresources are copied from") because it was implemented such that it
would only work in the output -> input linking PVC case and can't
actually be used by users and it will be removed in #1284.
fixes #1062
(Continuing #1184)
Submitter Checklist
These are the criteria that every PR should meet, please check them off as you
review them:
See the contribution guide for more details.
Double check this list of stuff that's easy to miss:
cmddir, please updatethe 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.
Release Notes