diff --git a/pkg/exposer/csi_snapshot.go b/pkg/exposer/csi_snapshot.go index b632322ad..1435372ef 100644 --- a/pkg/exposer/csi_snapshot.go +++ b/pkg/exposer/csi_snapshot.go @@ -500,7 +500,6 @@ func (e *csiSnapshotExposer) CleanUp(ctx context.Context, ownerObject corev1api. backupPodName := ownerObject.Name backupPVCName := ownerObject.Name backupVSName := ownerObject.Name - backupVSCName := ownerObject.Name kube.DeletePodIfAny(ctx, e.kubeClient.CoreV1(), backupPodName, ownerObject.Namespace, e.log) kube.DeletePVAndPVCIfAny(ctx, e.kubeClient.CoreV1(), backupPVCName, ownerObject.Namespace, cleanUpTimeout, e.log) @@ -512,13 +511,6 @@ func (e *csiSnapshotExposer) CleanUp(ctx context.Context, ownerObject corev1api. csi.DeleteVolumeSnapshotIfAny(ctx, e.csiSnapshotClient, backupVSName, ownerObject.Namespace, e.log) csi.DeleteVolumeSnapshotIfAny(ctx, e.csiSnapshotClient, vsName, sourceNamespace, e.log) - - // The backup VSC is created by Velero as an internal handle to the source - // snapshot. Deleting the backup VS above only cascades to it when its - // deletion policy is Delete, so remove it explicitly to avoid leaking the - // object under a Retain policy. Deleting a Retain VSC drops only the API - // object and leaves the underlying snapshot intact. - csi.DeleteVolumeSnapshotContentIfAny(ctx, e.csiSnapshotClient, backupVSCName, e.log) } func getVolumeModeByAccessMode(accessMode string, dataMover string) (corev1api.PersistentVolumeMode, error) { @@ -583,21 +575,7 @@ func (e *csiSnapshotExposer) createBackupVSC(ctx context.Context, ownerObject co Source: snapshotv1api.VolumeSnapshotContentSource{ SnapshotHandle: snapshotVSC.Status.SnapshotHandle, }, - // The backup VSC is statically provisioned against the same - // snapshot handle as the source VSC, so both objects refer to one - // physical snapshot. Inherit the source's deletion policy instead - // of forcing Delete, otherwise a user who configured Retain on the - // VolumeSnapshotClass still loses the snapshot when the backup VSC - // is cleaned up. - // - // For Case 2 storages per the design (design/block-data-mover/block-data-mover.md, - // e.g. Ceph RBD), inheriting Retain is not just an option but a requirement for - // incrementals to work at all: rbd snap diff needs the base and target snapshots - // in the same clone chain, so Delete destroys the base as soon as this backup - // completes. The next incremental's delta query then fails and degrades to an - // allocated-blocks backup (see the CBT tier ladder) or, without that fix, a full - // whole-device transfer. - DeletionPolicy: snapshotVSC.Spec.DeletionPolicy, + DeletionPolicy: snapshotv1api.VolumeSnapshotContentDelete, Driver: snapshotVSC.Spec.Driver, VolumeSnapshotClassName: snapshotVSC.Spec.VolumeSnapshotClassName, }, diff --git a/pkg/exposer/csi_snapshot_test.go b/pkg/exposer/csi_snapshot_test.go index a2bd3d46c..b72384550 100644 --- a/pkg/exposer/csi_snapshot_test.go +++ b/pkg/exposer/csi_snapshot_test.go @@ -1222,7 +1222,7 @@ func TestExpose(t *testing.T) { assert.Equal(t, anno, backupVSC.Annotations) assert.Equal(t, vscObj.Labels, backupVSC.Labels) - assert.Equal(t, vscObj.Spec.DeletionPolicy, backupVSC.Spec.DeletionPolicy) + assert.Equal(t, snapshotv1api.VolumeSnapshotContentDelete, backupVSC.Spec.DeletionPolicy) assert.Equal(t, vscObj.Spec.Driver, backupVSC.Spec.Driver) assert.Equal(t, *vscObj.Spec.VolumeSnapshotClassName, *backupVSC.Spec.VolumeSnapshotClassName) @@ -2575,17 +2575,16 @@ func TestCreateBackupVSCDeletionPolicy(t *testing.T) { expectedPolicy snapshotv1api.DeletionPolicy }{ { - name: "Delete policy is inherited", + name: "Delete source policy uses Delete", sourcePolicy: snapshotv1api.VolumeSnapshotContentDelete, expectedPolicy: snapshotv1api.VolumeSnapshotContentDelete, }, { - // The backup VSC points at the same snapshot handle as the source - // VSC, so forcing Delete here would destroy a snapshot the user - // asked to keep. - name: "Retain policy is inherited", + // Data mover cleanup must delete the underlying snapshot even when + // the source SnapshotClass uses Retain. + name: "Retain source policy is overridden with Delete", sourcePolicy: snapshotv1api.VolumeSnapshotContentRetain, - expectedPolicy: snapshotv1api.VolumeSnapshotContentRetain, + expectedPolicy: snapshotv1api.VolumeSnapshotContentDelete, }, }