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

[reefland]

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.

[tesshuflower]

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.

[onedr0p]

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.

[reefland]

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]

Seems related to #1129

[michaelbeaumont]

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.

But that's ok to just document this, I mean the use case is basically a "single use ReplicationDestination," which I have to imagine is relatively common. "Delete the RD" is not really feasible for GitOps workflows and IMO the continued existence of the RD is useful, since it serves to document the origin of the PVC that points to it and can be marked as "single use" via a flag or similar.

@tesshuflower what's the best way to move this forward? does it make sense to open a PR immediately adding the new behavior?

[tesshuflower]

We've had a discussion and think that there's been enough interest for us to implement the delete option for the temp PVC on a replicationdestination.

Notes for implementation:

• This will most likely be available to all movers
• VolSync will only delete the temp PVC if it has created it, volsync will not delete user-created PVCs provided in spec.<mover>.destinationPVC.
• The default for the option will be false (i.e. do not clean up the temp PVC) to maintain current behavior
• Targeting this for the v0.11.0 release.