Conversation
Add fields to the Volume Component type to allow specifying: * Whether persistent storage should be used for this Volume (enabling provisioning of emptyDir volumes) * An external volume that should exist outside of the lifecycle of container components; allows specifying an existing PersistentVolumeClaim, ConfigMap, or Secret to be attached to the container component. Signed-off-by: Angel Misevski <[email protected]>
Signed-off-by: Angel Misevski <[email protected]>
sleshchenko
left a comment
sleshchenko
left a comment
There was a problem hiding this comment.
I'm OK with the made proposal
|
|
||
| const ( | ||
| // StorageVolumeType specifies persistent storage, e.g. a PersistentVolumeClaim | ||
| StorageVolumeType ExistingVolumeType = "storage" |
There was a problem hiding this comment.
I wonder what is drawback if we name it with k8s term (as we do for other types), e.g. pvc, or persistentvolumeclaim (I know it's too long).
There was a problem hiding this comment.
PVC probably makes sense to make clear what we're planning to do with the information provided. I chose "storage" as it's more generic, but it may complicate things if we need to extend this in the future.
| // delete. | ||
| // +optional | ||
| // +kubebuilder:default=true | ||
| Persistent bool `json:"persistent,omitempty"` |
There was a problem hiding this comment.
Can it be used together with External? Should we deny it on the implementation level?
There was a problem hiding this comment.
Yeah I agree. One thing we discussed is that semantically it only makes sense to have either [persistent, size] or [external] but not both.
I also need to remove the readonly parameter for now, it doesn't make sense in its current spot.
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: amisevsk, sleshchenko The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
4 participants
What does this PR do?
Implements extensions to Volume components according to proposal in #310. Not a final design; I'm hoping for more discussion before merge. I've added this topic to the upcoming call to discuss more widely.
What issues does this PR fix or reference?
Closes #310
Is your PR tested? Consider putting some instruction how to test your changes
N/A?