RFE to allow dynamic creation of Persistent Volumes from storage classes

[ramgopireddy]

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]

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]

Thanks for your interest.

[haanhvu]

From what I see here:

camel-k/pkg/trait/trait_types.go

Lines 566 to 592 in cb9b886

	func getVolume(volName, storageType, storageName, filterKey, filterValue string) *corev1.Volume {
	items := convertToKeyToPath(filterKey, filterValue)
	volume := corev1.Volume{
	Name: volName,
	VolumeSource: corev1.VolumeSource{},
	}
	switch storageType {
	case "configmap":
	volume.VolumeSource.ConfigMap = &corev1.ConfigMapVolumeSource{
	LocalObjectReference: corev1.LocalObjectReference{
	Name: storageName,
	},
	Items: items,
	}
	case "secret":
	volume.VolumeSource.Secret = &corev1.SecretVolumeSource{
	SecretName: storageName,
	Items: items,
	}
	case "pvc":
	volume.VolumeSource.PersistentVolumeClaim = &corev1.PersistentVolumeClaimVolumeSource{
	ClaimName: storageName,
	}
	}
	return &volume
	}

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

Is that right @squakez @astefanutti ?

[squakez]

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]

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]

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]

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]

@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]

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]

We could leverage the work done in f82d1e2 to dynamically create a volume based on StorageClasses

[tdiesler]

Perhaps I can have a look

[tdiesler]

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

• It uses a pvc subsection for all dynamic PVC stuff. We can later migrate this easily to another trait
• It uses explicit keys for supported PVC props i.e. instead of all in one prop
• It does not change the semantic of existing mount.* properties
• Stuff with the same name naturally belongs together

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]

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]

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]

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?