From 9ca0dd2de89b63dd9eea664afdbd429a04c96bcb Mon Sep 17 00:00:00 2001
From: Tiger Kaovilai <tkaovila@redhat.com>
Date: Mon, 24 Aug 2026 10:27:34 -0400
Subject: [PATCH] Skip DeleteSnapshot when ProviderSnapshotID is empty (#9795)

When CreateSnapshot fails (e.g. quota limit), the snapshot is recorded
with an empty ProviderSnapshotID. During backup deletion, velero was
calling DeleteSnapshot("") which produces unnecessary 404 API calls.

Skip the DeleteSnapshot call when ProviderSnapshotID is empty and log
a warning instead.

Fixes #9429

Generated with [Claude Code](https://claude.ai/code)
via [Happy](https://happy.engineering)

Signed-off-by: Tiger Kaovilai <tkaovila@redhat.com>
Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: Happy <yesreply@happy.engineering>
(cherry picked from commit 09c1656df9a7dc4f276a9bd0693733c9c9dbc2b4)

Co-authored-by: kaovilai <11228024+kaovilai@users.noreply.github.com>
Signed-off-by: Tiger Kaovilai <tkaovila@redhat.com>
---
 changelogs/unreleased/9795-kaovilai           |  1 +
 pkg/controller/backup_deletion_controller.go  |  4 ++
 .../backup_deletion_controller_test.go        | 68 +++++++++++++++++++
 3 files changed, 73 insertions(+)
 create mode 100644 changelogs/unreleased/9795-kaovilai

diff --git a/changelogs/unreleased/9795-kaovilai b/changelogs/unreleased/9795-kaovilai
new file mode 100644
index 000000000..6394ff4d3
--- /dev/null
+++ b/changelogs/unreleased/9795-kaovilai
@@ -0,0 +1 @@
+Skip DeleteSnapshot when ProviderSnapshotID is empty
diff --git a/pkg/controller/backup_deletion_controller.go b/pkg/controller/backup_deletion_controller.go
index 5fe29c5f1..c1c643f0e 100644
--- a/pkg/controller/backup_deletion_controller.go
+++ b/pkg/controller/backup_deletion_controller.go
@@ -321,6 +321,10 @@ func (r *backupDeletionReconciler) Reconcile(ctx context.Context, req ctrl.Reque
 					volumeSnapshotters[snapshot.Spec.Location] = volumeSnapshotter
 				}
 
+				if snapshot.Status.ProviderSnapshotID == "" {
+					log.WithField("volumeSnapshot", snapshot.Spec.PersistentVolumeName).Warn("Skipping snapshot deletion: empty ProviderSnapshotID")
+					continue
+				}
 				if err := volumeSnapshotter.DeleteSnapshot(snapshot.Status.ProviderSnapshotID); err != nil {
 					errs = append(errs, errors.Wrapf(err, "error deleting snapshot %s", snapshot.Status.ProviderSnapshotID).Error())
 				}
diff --git a/pkg/controller/backup_deletion_controller_test.go b/pkg/controller/backup_deletion_controller_test.go
index dfca4f137..3b8307a44 100644
--- a/pkg/controller/backup_deletion_controller_test.go
+++ b/pkg/controller/backup_deletion_controller_test.go
@@ -397,6 +397,74 @@ func TestBackupDeletionControllerReconcile(t *testing.T) {
 		// Make sure snapshot was deleted
 		assert.Equal(t, 0, td.volumeSnapshotter.SnapshotsTaken.Len())
 	})
+	t.Run("empty ProviderSnapshotID skips DeleteSnapshot call", func(t *testing.T) {
+		input := defaultTestDbr()
+
+		backup := builder.ForBackup(velerov1api.DefaultNamespace, input.Spec.BackupName).Result()
+		backup.UID = "uid"
+		backup.Spec.StorageLocation = "primary"
+
+		restore1 := builder.ForRestore(backup.Namespace, "restore-1").
+			Phase(velerov1api.RestorePhaseCompleted).
+			Backup(backup.Name).
+			Result()
+
+		location := &velerov1api.BackupStorageLocation{
+			ObjectMeta: metav1.ObjectMeta{
+				Namespace: backup.Namespace,
+				Name:      "primary",
+			},
+			Spec: velerov1api.BackupStorageLocationSpec{
+				Provider: "objStoreProvider",
+				StorageType: velerov1api.StorageType{
+					ObjectStorage: &velerov1api.ObjectStorageLocation{
+						Bucket: "bucket",
+					},
+				},
+			},
+			Status: velerov1api.BackupStorageLocationStatus{
+				Phase: velerov1api.BackupStorageLocationPhaseAvailable,
+			},
+		}
+
+		snapshotLocation := &velerov1api.VolumeSnapshotLocation{
+			ObjectMeta: metav1.ObjectMeta{
+				Namespace: backup.Namespace,
+				Name:      "vsl-1",
+			},
+			Spec: velerov1api.VolumeSnapshotLocationSpec{
+				Provider: "provider-1",
+			},
+		}
+		td := setupBackupDeletionControllerTest(t, input, backup, restore1, location, snapshotLocation)
+
+		snapshots := []*volume.Snapshot{
+			{
+				Spec: volume.SnapshotSpec{
+					Location:             "vsl-1",
+					PersistentVolumeName: "pv-1",
+				},
+				Status: volume.SnapshotStatus{
+					ProviderSnapshotID: "",
+				},
+			},
+		}
+
+		pluginManager := &pluginmocks.Manager{}
+		pluginManager.On("GetVolumeSnapshotter", "provider-1").Return(td.volumeSnapshotter, nil)
+		pluginManager.On("GetDeleteItemActions").Return(nil, nil)
+		pluginManager.On("CleanupClients")
+		td.controller.newPluginManager = func(logrus.FieldLogger) clientmgmt.Manager { return pluginManager }
+
+		td.backupStore.On("GetBackupVolumeSnapshots", input.Spec.BackupName).Return(snapshots, nil)
+		td.backupStore.On("GetBackupContents", input.Spec.BackupName).Return(io.NopCloser(bytes.NewReader([]byte("hello world"))), nil)
+		td.backupStore.On("DeleteBackup", input.Spec.BackupName).Return(nil)
+
+		_, err := td.controller.Reconcile(t.Context(), td.req)
+		require.NoError(t, err)
+
+		td.backupStore.AssertCalled(t, "DeleteBackup", input.Spec.BackupName)
+	})
 	t.Run("full delete, no errors, with backup name greater than 63 chars", func(t *testing.T) {
 		backup := defaultBackup().
 			ObjectMeta(