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(