charts icon indicating copy to clipboard operation
charts copied to clipboard

Fix VolumeSnapshot backup capability

Open cam-at-tactiq opened this issue 1 year ago • 5 comments

Hi this is my first contribution so please let me know if I have missed anything.

This PR fixes two issues with VolumeSnapshots for backups:

  • Missing VolumeSnapshotConfiguration block on the Cluster resource
  • Prevents the chart from complaining about missing azure, google, or s3 options when we choose to use volumeSnapshot as a provider
  • Add support for recovery from a VolumeSnapshot

This is achieved by establishing 'volumeSnapshot' as a backups.provider.

There are no breaking changes.

Fixes #334

cam-at-tactiq avatar Jul 24 '24 23:07 cam-at-tactiq

This looks pretty good!

Would you have the time to add the Recovery from VolumeSnapshots functionality as well?

It wouldn't want to add this as a backup method without adding a recovery path, as that could potentially make it more difficult to recover from.

itay-grudev avatar Jul 25 '24 13:07 itay-grudev

Thanks @itay-grudev! I've added support for recovery from snapshots and tested this in a GKE environment.

The recovery config docs also mention being able to use a PVC as a recovery mechanism with the same block on the cluster, but that failed when I tested it, otherwise I would have added that too. It might be a nice to have when that capability stabilises in the operator.

Please let me know if there's anything else that could make this better.

cam-at-tactiq avatar Jul 26 '24 02:07 cam-at-tactiq

Hi @itay-grudev please let me know what blockers if any there are to merging, we are really keen to start using this when it is released :)

cam-at-tactiq avatar Aug 05 '24 02:08 cam-at-tactiq

I not think force user to decide between s3 and snapshot is good idea actually. Provider allow to utilize both S3 and snapshots, why we in Helm should cut off this capabilities, I see this option as: https://github.com/cloudnative-pg/charts/pull/359

dragoangel avatar Aug 26 '24 22:08 dragoangel

I not think force user to decide between s3 and snapshot is good idea actually. Provider allow to utilize both S3 and snapshots, why we in Helm should cut off this capabilities, I see this option as: #359

Hi @dragoangel, I like your PR a lot. Whatever the maintainers prefer I am happy with, I just really want some volume snapshot capability. I've posted a review on your PR with two suggested changes. @itay-grudev please feel free to close this PR if you are going to merge #359 .

cam-at-tactiq avatar Aug 27 '24 04:08 cam-at-tactiq

The solution here is offered freely, but clearly isn't wanted by maintainers at this point in time.

cam-at-tactiq avatar Jan 08 '25 21:01 cam-at-tactiq