volsync icon indicating copy to clipboard operation
volsync copied to clipboard

Provide option to cleanup temporary objects (PVCs) made by Restore Process

Open reefland opened this issue 11 months ago • 4 comments

Describe the feature you'd like to have. An option added to ReplicationDestination to request volsync to clean-up temporary PVCs created during the restore process which are no longer needed when the restore has completed.

For example:

kind: ReplicationDestination
metadata:
  name: apt-cacher-ng-dst
spec:
  trigger:
    manual: restore-once

Leaves behind 2 snapshots when the restore is completed:

volsync-apt-cacher-ng-dst-cache   Bound    pvc-a2f70adb-2acf-499a-a0d1-707bb55f1e4b   1Gi        RWO            csi-ceph-block   2d18h
volsync-apt-cacher-ng-dst-dest    Bound    pvc-50b3140b-200a-4a26-b093-3cfba92a9313   50Gi       RWO            csi-ceph-block   2d18h

What is the value to the end user? (why is it a priority?)

While the docs state After the restore completes, the ReplicationDestination object can be deleted. It's not clear there can be multiple objects to cleanup. Why not allow the cleanup process to be automated by volsync instead of the user needing to remember to do this manually?

How will we know we have a good solution? (acceptance criteria) Upon completion of a successful restore, if configured to do so, volsync will cleanup temporary objects such as PVCs no longer needed.

Additional context

My preference is this should be enabled by default unless you have a reason to keep the objects. However, I understand this could be considered a breaking change if users expect/want these objects to be left behind. I'm fine if disabled by default, just need to the option to enable it.

reefland avatar Mar 07 '24 15:03 reefland

I think the main hesitation about implementing this feature would be that we'd need to specifically document not to use the flag in most situations. Many use-cases benefit from these PVCs not being cleaned up as it means you don't need to re-download the entire data on each replication cycle (and this is crucial for things like rsync). When users are done the expectation is that they can remove their replicationdestination if syncs are no longer required, and at that point the pvcs would be cleaned up as well.

tesshuflower avatar Mar 07 '24 21:03 tesshuflower

Oh I was unaware removing the replicationdestination would clean up the PVCs, makes sense though. However since I have that declarative in my Flux / GitOps configuration it's not great to remove it via kubectl delete... because Flux will just put it back. I have it this way so I don't need to do any comment/uncomment commit dancing when re-provisioning my cluster.

onedr0p avatar Mar 07 '24 21:03 onedr0p

hmm. I like having the ReplicationDestination defined within gitops for bootstrapping the application PVC with backed up data. I want to leave this in-place and just have PVCs cleaned up. :)

If this is something specific to Restic restore, and doesn't make sense elsewhere, then can we only apply to ReplicationDestination .spec.restric and not .spec.rsync ?

reefland avatar Mar 07 '24 22:03 reefland

Seems related to #1129

reefland avatar Mar 09 '24 14:03 reefland