mirror of
https://github.com/vmware-tanzu/velero.git
synced 2026-08-25 00:26:08 +00:00
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 f84c9766ca)
Co-authored-by: kaovilai <11228024+kaovilai@users.noreply.github.com>
This commit is contained in:
committed by
GitHub
co-authored by
kaovilai
parent
b6c763badf
commit
a31ffe4532
@@ -0,0 +1 @@
|
||||
Skip DeleteSnapshot when ProviderSnapshotID is empty
|
||||
@@ -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())
|
||||
}
|
||||
|
||||
@@ -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(
|
||||
|
||||
Reference in New Issue
Block a user