From a31ffe45325b7ee367fa416ded29915a3b664d57 Mon Sep 17 00:00:00 2001
From: Tiger Kaovilai <tkaovila@redhat.com>
Date: Thu, 16 Jul 2026 16:16:36 -0400
Subject: [PATCH] Skip DeleteSnapshot when ProviderSnapshotID is empty

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)

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

Co-authored-by: kaovilai <11228024+kaovilai@users.noreply.github.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 ccac5cd85..6f067e6d1 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(