pipeline api: add volumClaimTemplate #513
Conversation
✅ Deploy Preview for kurator-dev canceled.
|
Signed-off-by: Xieql <[email protected]>
|
/label tide/merge-method-squash |
pkg/apis/pipeline/v1alpha1/type.go
Outdated
| type VolumeClaimTemplate struct { | ||
| // AccessMode determines the access modes for the volume, e.g., ReadWriteOnce. | ||
| // This affects how the volume can be mounted. | ||
| AccessMode string `json:"accessMode,omitempty"` |
There was a problem hiding this comment.
Donot use string, please refer to k8s https://kubernetes.io/docs/concepts/storage/persistent-volumes/#access-modes
And its API: AccessModes []PersistentVolumeAccessMode , PersistentVolumeAccessMode is a type aliase
pkg/apis/pipeline/v1alpha1/type.go
Outdated
|
|
||
| // RequestsStorage defines the storage size required for this PVC, e.g., 1Gi, 100Mi. | ||
| // It specifies the storage capacity needed as part of ResourceRequirements. | ||
| RequestsStorage string `json:"requestsStorage,omitempty"` |
There was a problem hiding this comment.
How do you validate string with Gi, Mi
There was a problem hiding this comment.
we can use Pattern like // +kubebuilder:validation:Pattern="^[0-9]+(\.[0-9]+)?(Gi|Mi)$"
pkg/apis/pipeline/v1alpha1/type.go
Outdated
|
|
||
| // RequestsStorage defines the storage size required for this PVC, e.g., 1Gi, 100Mi. | ||
| // It specifies the storage capacity needed as part of ResourceRequirements. | ||
| RequestsStorage string `json:"requestsStorage,omitempty"` |
There was a problem hiding this comment.
s/RequestsStorage/StorageRequest, it is singular
pkg/apis/pipeline/v1alpha1/type.go
Outdated
|
|
||
| // VolumeMode specifies whether the volume should be used with a formatted filesystem (Filesystem) | ||
| // or remain in raw block state (Block). The Filesystem value is implied when not included. | ||
| VolumeMode string `json:"volumeMode,omitempty"` |
There was a problem hiding this comment.
Same as above donot use a generic string type
kurator-bot
added
size/XXL
and removed
do-not-merge/contains-merge-commits
size/L
labels
kurator-bot
added
do-not-merge/contains-merge-commits
size/L
and removed
size/XXL
labels
Signed-off-by: Xieql <[email protected]>
|
@hzxuzhonghu PTAL |
| // If not set, Kurator will create a PVC named Pipeline.name using default config | ||
| // +optional | ||
| SharedWorkspace *string `json:"sharedWorkspace,omitempty"` | ||
| SharedWorkspace *VolumeClaimTemplate `json:"sharedWorkspace,omitempty"` |
There was a problem hiding this comment.
I did not realize: isn't this a PVC spec. why do we redefine a new one?
There was a problem hiding this comment.
Using a pre-created PVC (Persistent Volume Claim) in a pipeline then references it name is not recommended, as PVCs are meant to be linked with each specific pipeline execution instance, not the pipeline itself.
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: hzxuzhonghu 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:
Approvers can indicate their approval by writing |
What type of PR is this?
/kind api-change
What this PR does:
Add complete pvc config about
SharedWorkspace，rather than a string.Why we need it:
Using a pre-created PVC (Persistent Volume Claim) in a pipeline then references it name is not recommended, as PVCs are meant to be linked with each specific pipeline execution instance, not the pipeline itself.
To simplify the user experience, only fields that have appeared in the Tekton Hub are included.
In total, there are only four configurations, among which almost only the
RequestsStorageis a meaningful setting. The remaining three, even if configured, are almost always default settings.