apache / camel-k

Apache Camel K is a lightweight integration platform, born on Kubernetes, with serverless superpowers
https://camel.apache.org/camel-k
Apache License 2.0
866 stars 348 forks source link

RFE to allow dynamic creation of Persistent Volumes from storage classes #2994

Closed ramgopireddy closed 3 weeks ago

ramgopireddy commented 2 years ago

We would like the option to dynamically create a persistent volume using the operator instead of having to create a Persistent Volume first and using it to mount on the integration. Is it possible to extend the Mount trait to allow provisioning of the PV from a storage class as well?

haanhvu commented 2 years ago

Hi @astefanutti @squakez, I would like to work on this issue. Could you assign it to me? I'm studying the codebase and may ask questions later.

squakez commented 2 years ago

Thanks for your interest.

haanhvu commented 2 years ago

From what I see here: https://github.com/apache/camel-k/blob/cb9b886f964f398e14a02965896267f07190eec2/pkg/trait/trait_types.go#L566-L592 The mount trait will just mount the PVC, no matter if the PVC is statically or dynamically provisioned.

Is that right @squakez @astefanutti ?

squakez commented 2 years ago

The mount trait will just mount the PVC, no matter if the PVC is statically or dynamically provisioned.

Is that right @squakez @astefanutti ?

Yes, that's my understanding too.

haanhvu commented 2 years ago

The mount trait will just mount the PVC, no matter if the PVC is statically or dynamically provisioned. Is that right @squakez @astefanutti ?

Yes, that's my understanding too.

@squakez if that's the case, then this issue already had a solution?

squakez commented 2 years ago

I think the idea of the issue is to have the trait accepting the parameters to create the PVC on the fly. As an example, we can think the following kamel run -t mount.storage-class=vpc-block -t mount.storage-request=30Gi -t mount.storage-mode=ReadWriteOnce ... which is in charge to create the PVC on the cluster, and later, mount accordingly on the Integration.

haanhvu commented 2 years ago

I think the idea of the issue is to have the trait accepting the parameters to create the PVC on the fly. As an example, we can think the following kamel run -t mount.storage-class=vpc-block -t mount.storage-request=30Gi -t mount.storage-mode=ReadWriteOnce ... which is in charge to create the PVC on the cluster, and later, mount accordingly on the Integration.

@squakez should we have another trait for this? I mean, the mount trait should just do what it does: mount the volume. If we need to "create" the volume, should we have another trait for it, like the pvc trait?

squakez commented 2 years ago

@haanhvu we may reason about that for sure. In any case, the business logic would be pretty similar and you'd need to related the mount trait anyway. IMO, we should do a first development within the mount trait, and later figure it out if it makes sense to have a separate trait for that.

github-actions[bot] commented 2 years ago

This issue has been automatically marked as stale due to 90 days of inactivity. It will be closed if no further activity occurs within 15 days. If you think that’s incorrect or the issue should never stale, please simply write any comment. Thanks for your contributions!

squakez commented 1 year ago

We could leverage the work done in https://github.com/apache/camel-k/pull/4025/commits/f82d1e296c4bcd398e03e75764443e0acc6083eb to dynamically create a volume based on StorageClasses

tdiesler commented 4 months ago

Perhaps I can have a look

tdiesler commented 4 months ago

I propose ...

kamel run --dev \
    --trait mount.pvc.accessModes=my-pvc:ReadWriteOnce \
    --trait mount.pvc.storageCapacity=my-pvc:5Mi \
    --trait mount.pvc.storageClass=my-pvc:my-storage \
    --trait mount.volumes=my-pvc:/tmp/data pvc-producer.yaml

An alternative could be ...

kamel run --dev \
    --trait mount.pvc=my-pvc?accessModes=ReadWriteOnce&storageCapacity=5Mi&storageClass=my-storage \
    --trait mount.volumes=my-pvc:/tmp/data pvc-producer.yaml
squakez commented 4 months ago

Consider that modifying the trait is a hard thing to do as it is encapsulated in the Integration API contract. Whatever we chose, we must try to be as stable as possible. The second approach is better, however I'd avoid to add all that redundancy. Probably the user will read the documentation where he check how to compile a given syntax. IMO, following you suggestion it would be better something like: mount.storage=<pvcName>:<storageClass>:<capacity>:<accessModes>:<mountPath> and, by default, the trait would be in charge to mount withouth the need to specify that.

tdiesler commented 4 months ago

Spelled out, this would be ...

kamel run --dev --trait mount.storage=my-pvc:my-storage:5Mi:ReadWriteOnce:/tmp/data pvc-producer.yaml

Using positional arguments (i.e. pvsname=toks[0], class=toks[1], capacity=toks[2], ...) can be problematic when you have optional properties. Name and path are required, so it could perhaps be ...

kamel run --dev 
   --trait mount.storage=my-pvc:/tmp/data?class=my-storage&access=ReadWriteOnce&capacity=5Mi \
   pvc-producer.yaml

This might result in some confusion ...

  1. mount.storage=my-pvc:/tmp/data creates the pvc (with defaults) and mounts it
  2. mount.volumes=my-pvc:/tmp/data requires an existing pvc and mounts it
squakez commented 3 months ago

Hello @tdiesler as this is assigned to you, are you thinking to work on this for next milestone or can we clear your name from the assignees?