mirror of
https://github.com/vmware-tanzu/velero.git
synced 2026-10-22 06:13:00 +00:00
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 09c1656df9)
Co-authored-by: kaovilai <11228024+kaovilai@users.noreply.github.com>
Signed-off-by: Tiger Kaovilai <tkaovila@redhat.com>
This commit is contained in:
co-authored by
kaovilai
parent
cae8768c8f
commit
9ca0dd2de8
@@ -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