Any reason why backup=0 is hardcoded?

sergelogvinov / proxmox-csi-plugin

Proxmox CSI Plugin

Apache License 2.0

268 stars 25 forks source link

Any reason why backup=0 is hardcoded? #201

Open j0nm1 opened 3 months ago

j0nm1 commented 3 months ago

Hi, first of all, thanks for the great project! I really like the management of the pvcs directly in Proxmox. Currently, I am just wondering if there is any specific reason that backup is explicitly set to 0 in https://github.com/sergelogvinov/proxmox-csi-plugin/blob/76c899e4e1c8f2287854a4e08fbbdd8a1c7360a9/pkg/csi/controller.go#L352?

I am using a PBS to automatically backup my VMs and would enjoy including the pvcs as well. Or is there any technical limitation that I forgot here?

Thanks! Jonas

sergelogvinov commented 3 months ago

My thoughts was:

There are many cloud-native solutions available to backup PVCs.
A PV does not belong to a VM, and backing up a VM should not affect the PV
During the recovery process, we might encounter unpredictable behavior. For example, if the PV exists and is attached to another VM, what will Proxmox do in this case? Or, if we have already deleted the PV resource in Kubernetes, we might end up with a block device that the CSI plugin cannot handle properly, as it does not register in Kubernetes automatically.

kowalski7cc commented 2 months ago

Could be this make an option? It can be userful for small setups using proxmox backup, especially when you just want to restore a single file using the file restore option. It's true that there are kube native solutions for making backups of PVCs, but some of them (like K10) rely on PVC snapshots, which are not implemented yet.

sergelogvinov commented 2 months ago

I appreciate curious individuals who enjoy experimenting. Yep, we could add a CLI flag that disabled all checks, for example.

vehagn commented 2 months ago

I think this possibly goes against original intention of this project and perhaps the Noflake Manifesto, but it would be convenient (at least in my case) to bind a volume to a VM by name. E.g. instead of creating the VM disk vm-9999-pvc-<UUID> you create the VM disk vm-100-pvc-<namespace>-<volumeName> bound to VM 100 and turn on backup. If a disk with the name exists you bind to it, and if not you create it. I'm not sure if this is feasible though and I don't think it should be the default behaviour.

Looking at @sergelogvinov's terraform-talos proxmox example here, it might be possible to create a volume manually in Proxmox and bind to i using the pv spec.csi.volumeHandle attribute? This is all just speculation on my part though.

sergelogvinov commented 2 weeks ago

to switch off the BACKUP=0 param - use UNSAFEMOUNT="true" environment. I do not want to documented it, i believe the better way to fix it is using VolumeAttributesClass resource (not implemented yet)