-
Notifications
You must be signed in to change notification settings - Fork 1.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add volumeClaimTemplate as a Workspace volume source #2326
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,51 @@ | ||
apiVersion: tekton.dev/v1beta1 | ||
kind: Pipeline | ||
metadata: | ||
name: volume-from-template | ||
spec: | ||
tasks: | ||
- name: writer | ||
taskSpec: | ||
steps: | ||
- name: write | ||
image: ubuntu | ||
script: echo bar > $(workspaces.task-ws.path)/foo | ||
workspaces: | ||
- name: task-ws | ||
workspaces: | ||
- name: task-ws | ||
workspace: ws | ||
- name: reader | ||
runAfter: | ||
- writer | ||
taskSpec: | ||
steps: | ||
- name: read | ||
image: ubuntu | ||
script: cat $(workspaces.myws.path)/foo | grep bar | ||
workspaces: | ||
- name: myws | ||
workspaces: | ||
- name: myws | ||
workspace: ws | ||
workspaces: | ||
- name: ws | ||
--- | ||
apiVersion: tekton.dev/v1beta1 | ||
kind: PipelineRun | ||
metadata: | ||
generateName: run-with-template- | ||
spec: | ||
pipelineRef: | ||
name: volume-from-template | ||
workspaces: | ||
- name: ws | ||
volumeClaimTemplate: | ||
metadata: | ||
name: mypvc | ||
spec: | ||
accessModes: | ||
- ReadWriteOnce | ||
resources: | ||
requests: | ||
storage: 1Gi |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -23,9 +23,18 @@ import ( | |
"github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1" | ||
corev1 "k8s.io/api/core/v1" | ||
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" | ||
"k8s.io/apimachinery/pkg/runtime/schema" | ||
"knative.dev/pkg/apis" | ||
) | ||
|
||
var ( | ||
taskRunGroupVersionKind = schema.GroupVersionKind{ | ||
Group: SchemeGroupVersion.Group, | ||
Version: SchemeGroupVersion.Version, | ||
Kind: pipeline.TaskRunControllerName, | ||
} | ||
) | ||
|
||
// TaskRunSpec defines the desired state of TaskRun | ||
type TaskRunSpec struct { | ||
// +optional | ||
|
@@ -165,6 +174,11 @@ func (tr *TaskRun) GetBuildPodRef() corev1.ObjectReference { | |
} | ||
} | ||
|
||
// GetOwnerReference gets the task run as owner reference for any related objects | ||
func (tr *TaskRun) GetOwnerReference() metav1.OwnerReference { | ||
return *metav1.NewControllerRef(tr, taskRunGroupVersionKind) | ||
} | ||
|
||
// GetPipelineRunPVCName for taskrun gets pipelinerun | ||
func (tr *TaskRun) GetPipelineRunPVCName() string { | ||
if tr == nil { | ||
|
@@ -228,3 +242,14 @@ func (tr *TaskRun) IsPartOfPipeline() (bool, string, string) { | |
|
||
return false, "", "" | ||
} | ||
|
||
// HasVolumeClaimTemplate returns true if TaskRun contains volumeClaimTemplates that is | ||
// used for creating PersistentVolumeClaims with an OwnerReference for each run | ||
func (tr *TaskRun) HasVolumeClaimTemplate() bool { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shouldn't those changes be also on There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good catch! I should add it to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I added the the changes I made on There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Btw, not sure where to ask for this, anyone can help with how would you use it in a CLI with tkn to pass the name of the template which already exists in the system (not template file)? e.g.: I have tried this but it continuously says cannot find the PVC (NOTE: the "default-netapp-disk" exists in OpenShift as a VolumeClaimTemplate and it is the default one as well): _tkn pipeline start pipeline-name -w name=shared-data,claimName=VolumeClaimTemplate,storageClass=default-netapp-disk,accessModes=ReadWriteOnce,storage=1Gi,volumeMode=Filesystem .... OUTPUT: I've also tried using 'claimName=default-netapp-disk' as well, and many other things though nothing works when it comes to sending the VCT and it's details through the tkn CLI... This is very critical to me and I did not really found any useful info during this last week while working on it... |
||
for _, ws := range tr.Spec.Workspaces { | ||
if ws.VolumeClaimTemplate != nil { | ||
return true | ||
} | ||
} | ||
return false | ||
} |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any reason to name it
volumeClaimTemplate
and notpersistentVolumeClaim
?(Mainy reason for this question is if we should be consistent with the
persistentVolumeClaim
field)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I borrowed the whole syntax and idea with
volumeClaimTemplate
from StatefulSet https://kubernetes.io/docs/concepts/workloads/controllers/statefulset/#components and wanted to be consistent with that terminology. ThepersistentVolumeClaim
is still there for workspaces and can be used if you want to use an existing PVC.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fair enough. I don't have a strong opinion on this, although I would tend to prefer be consistent in the naming with our own API than k8s one 😛
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
agree with the persistence part, makes lots of sense...