Skip to content

Commit 0ee7d8d

Browse files
priyansh17blackpiglet
authored andcommitted
Refactor backup deletion logic to skip CSI snapshot cleanup on tarball download failure
Signed-off-by: Priyansh Choudhary <im1706@gmail.com>
1 parent a086560 commit 0ee7d8d

1 file changed

Lines changed: 1 addition & 35 deletions

File tree

pkg/controller/backup_deletion_controller.go

Lines changed: 1 addition & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -268,15 +268,12 @@ func (r *backupDeletionReconciler) Reconcile(ctx context.Context, req ctrl.Reque
268268

269269
if err != nil {
270270
log.WithError(err).Errorf("Unable to download tarball for backup %s, skipping associated DeleteItemAction plugins", backup.Name)
271-
// Best-effort fallback for pre-v1.15 backups where VS/VSC may still exist in cluster
272-
log.Info("Cleaning up CSI volumesnapshots")
273-
r.deleteCSIVolumeSnapshotsIfAny(ctx, backup, log)
274271
// If the tarball simply does not exist (HTTP 404 / not found), the download
275272
// failure is permanent and not retryable, so we let deletion proceed.
276273
// For transient errors (throttling, auth failures, network issues), record
277274
// the error to fail the deletion so it can be retried later.
278275
if !isTarballNotFoundError(err) {
279-
errs = append(errs, errors.Wrapf(err, "error downloading backup tarball").Error())
276+
errs = append(errs, errors.Wrapf(err, "error downloading backup tarball, CSI snapshot cleanup was skipped").Error())
280277
}
281278
} else {
282279
defer closeAndRemoveFile(backupFile, r.logger)
@@ -532,37 +529,6 @@ func (r *backupDeletionReconciler) deleteCSIVolumeSnapshotsIfAny(ctx context.Con
532529
}
533530
}
534531

535-
// deleteCSIVolumeSnapshotContentsIfAny cleans up VolumeSnapshotContent objects labeled with the backup name.
536-
// This is a best-effort fallback for pre-v1.15 backups where VSC objects may still exist in the cluster
537-
// even when the backup tarball cannot be downloaded (e.g. due to a transient storage error or velero pod restart).
538-
func (r *backupDeletionReconciler) deleteCSIVolumeSnapshotContentsIfAny(ctx context.Context, backup *velerov1api.Backup, log logrus.FieldLogger) {
539-
vscList := snapshotv1api.VolumeSnapshotContentList{}
540-
if err := r.Client.List(ctx, &vscList, &client.ListOptions{
541-
LabelSelector: labels.SelectorFromSet(map[string]string{
542-
velerov1api.BackupNameLabel: label.GetValidName(backup.Name),
543-
}),
544-
}); err != nil {
545-
log.WithError(err).Warnf("Could not list volume snapshot contents, abort")
546-
return
547-
}
548-
for i := range vscList.Items {
549-
vsc := &vscList.Items[i]
550-
// Set DeletionPolicy to Delete so that the underlying cloud snapshot is also removed.
551-
if _, err := csi.SetVolumeSnapshotContentDeletionPolicy(vsc.Name, r.Client, snapshotv1api.VolumeSnapshotContentDelete); err != nil {
552-
log.WithError(err).Warnf("Failed to set DeletionPolicy to Delete for VolumeSnapshotContent %s, will still attempt deletion", vsc.Name)
553-
}
554-
if err := r.Client.Delete(ctx, vsc); err != nil {
555-
if apierrors.IsNotFound(err) {
556-
log.Debugf("VolumeSnapshotContent %s not found, skipping deletion", vsc.Name)
557-
} else {
558-
log.WithError(err).Warnf("Failed to delete VolumeSnapshotContent %s", vsc.Name)
559-
}
560-
} else {
561-
log.Infof("Deleted VolumeSnapshotContent %s", vsc.Name)
562-
}
563-
}
564-
}
565-
566532
func (r *backupDeletionReconciler) deletePodVolumeSnapshots(ctx context.Context, backup *velerov1api.Backup) []error {
567533
if r.repoMgr == nil {
568534
return nil

0 commit comments

Comments
 (0)