mirrors/velero
1
0
Fork 0
mirror of https://github.com/vmware-tanzu/velero.git synced 2026-08-25 00:26:08 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

Double check the label for backup when deleting VSC (#10346)
e2e-test-kind.yaml / extract (push) Failing after 8s
Details
Run the E2E test on kind / get-go-version (push) Failing after 9s
Details
Run the E2E test on kind / build (push) Skipped
Details
Run the E2E test on kind / setup-test-matrix (push) Successful in 3s
Details
Run the E2E test on kind / run-e2e-test (push) Skipped
Details
push.yml / extract (push) Failing after 7s
Details
Main CI / get-go-version (push) Failing after 8s
Details
Main CI / Build (push) Skipped
Details

Browse Source 
This commit double checks the label of the VSC on the cluster before
deleting it to avoid mis-deletion.

Signed-off-by: Daniel Jiang <daniel.jiang@broadcom.com>
This commit is contained in:
Daniel Jiang
2026-08-21 11:19:57 +08:00
committed by GitHub
parent f27a4ad8c0
commit 8bbd546167
3 changed files with 100 additions and 22 deletions
Download Patch File Download Diff File Expand all files Collapse all files
changelogs/unreleased/10346-reasonerjt
+1
View File
@@ -0,0 +1 @@
Double check the label for backup when deleting VSC- #10346
internal/delete/actions/csi/volumesnapshotcontent_action.go
+12 -2
View File
@@ -86,7 +86,7 @@ func (p *volumeSnapshotContentDeleteItemAction) Execute(
// This handles legacy (pre-1.15) backups where the original VSC
// with DeletionPolicy=Retain still exists in the cluster.
originalVSCName := snapCont.Name
if cleaned := p.tryDeleteOriginalVSC(context.TODO(), originalVSCName); cleaned {
if cleaned := p.tryDeleteOriginalVSC(context.TODO(), originalVSCName, input.Backup.Name); cleaned {
p.log.Infof("Successfully deleted original VolumeSnapshotContent %s from cluster, skipping temp VSC creation", originalVSCName)
return nil
}
@@ -149,10 +149,11 @@ func (p *volumeSnapshotContentDeleteItemAction) Execute(
// the cluster (legacy pre-1.15 backups). It patches the DeletionPolicy to
// Delete so the CSI driver also removes the cloud snapshot, then deletes
// the VSC object itself.
// Returns true if the original VSC was found and deletion was initiated.
// Returns true if the original VSC was found, carries the backup label, and deletion was initiated.
func (p *volumeSnapshotContentDeleteItemAction) tryDeleteOriginalVSC(
ctx context.Context,
vscName string,
backupName string,
) bool {
existing := new(snapshotv1api.VolumeSnapshotContent)
if err := p.crClient.Get(ctx, crclient.ObjectKey{Name: vscName}, existing); err != nil {
@@ -164,6 +165,15 @@ func (p *volumeSnapshotContentDeleteItemAction) tryDeleteOriginalVSC(
return false
}
if !kubeutil.HasBackupLabel(&existing.ObjectMeta, backupName) {
p.log.Warnf(
"Original VolumeSnapshotContent %s in cluster does not belong to backup %s, skipping direct deletion",
vscName,
backupName,
)
return false
}
p.log.Debugf("Found original VolumeSnapshotContent %s in cluster (legacy backup), cleaning up directly", vscName)
// Patch DeletionPolicy to Delete so the CSI driver removes the cloud snapshot
internal/delete/actions/csi/volumesnapshotcontent_action_test.go
+87 -20
View File
@@ -122,7 +122,29 @@ func TestVSCExecute(t *testing.T) {
backup: builder.ForBackup("velero", "backup").Result(),
expectErr: false,
preExistingVSC: &snapshotv1api.VolumeSnapshotContent{
ObjectMeta: metav1.ObjectMeta{Name: "bar"},
ObjectMeta: metav1.ObjectMeta{
Name: "bar",
Labels: map[string]string{
velerov1api.BackupNameLabel: "backup",
},
},
Spec: snapshotv1api.VolumeSnapshotContentSpec{
DeletionPolicy: snapshotv1api.VolumeSnapshotContentRetain,
Driver: "disk.csi.azure.com",
Source: snapshotv1api.VolumeSnapshotContentSource{SnapshotHandle: stringPtr("snap-123")},
VolumeSnapshotRef: corev1api.ObjectReference{Name: "vs-1", Namespace: "default"},
},
},
},
{
name: "Original VSC exists in cluster without backup label, falls through to temp VSC flow",
vsc: builder.ForVolumeSnapshotContent("bar").ObjectMeta(builder.WithLabelsMap(map[string]string{velerov1api.BackupNameLabel: "backup"})).Status(&snapshotv1api.VolumeSnapshotContentStatus{SnapshotHandle: &snapshotHandleStr}).Result(),
backup: builder.ForBackup("velero", "backup").Result(),
expectErr: false,
preExistingVSC: &snapshotv1api.VolumeSnapshotContent{
ObjectMeta: metav1.ObjectMeta{
Name: "bar",
},
Spec: snapshotv1api.VolumeSnapshotContentSpec{
DeletionPolicy: snapshotv1api.VolumeSnapshotContentRetain,
Driver: "disk.csi.azure.com",
@@ -200,22 +222,51 @@ func TestNewVolumeSnapshotContentDeleteItemAction(t *testing.T) {
func TestTryDeleteOriginalVSC(t *testing.T) {
tests := []struct {
name string
vscName string
existing *snapshotv1api.VolumeSnapshotContent
createIt bool
expectRet bool
name string
vscName string
backupName string
existing *snapshotv1api.VolumeSnapshotContent
createIt bool
expectRet bool
}{
{
name: "VSC not found in cluster, returns false",
vscName: "not-found",
name: "VSC not found in cluster, returns false",
vscName: "not-found",
backupName: "test-backup",
expectRet: false,
},
{
name: "VSC found in cluster without backup label, returns false",
vscName: "unlabeled-vsc",
backupName: "test-backup",
existing: &snapshotv1api.VolumeSnapshotContent{
ObjectMeta: metav1.ObjectMeta{Name: "unlabeled-vsc"},
Spec: snapshotv1api.VolumeSnapshotContentSpec{
DeletionPolicy: snapshotv1api.VolumeSnapshotContentRetain,
Driver: "disk.csi.azure.com",
Source: snapshotv1api.VolumeSnapshotContentSource{
SnapshotHandle: stringPtr("snap-123"),
},
VolumeSnapshotRef: corev1api.ObjectReference{
Name: "vs-1",
Namespace: "default",
},
},
},
createIt: true,
expectRet: false,
},
{
name: "VSC found with Retain policy, patches and deletes",
vscName: "legacy-vsc",
name: "VSC found with Retain policy and matching backup label, patches and deletes",
vscName: "legacy-vsc",
backupName: "test-backup",
existing: &snapshotv1api.VolumeSnapshotContent{
ObjectMeta: metav1.ObjectMeta{Name: "legacy-vsc"},
ObjectMeta: metav1.ObjectMeta{
Name: "legacy-vsc",
Labels: map[string]string{
velerov1api.BackupNameLabel: "test-backup",
},
},
Spec: snapshotv1api.VolumeSnapshotContentSpec{
DeletionPolicy: snapshotv1api.VolumeSnapshotContentRetain,
Driver: "disk.csi.azure.com",
@@ -232,10 +283,16 @@ func TestTryDeleteOriginalVSC(t *testing.T) {
expectRet: true,
},
{
name: "VSC found with Delete policy already, just deletes",
vscName: "already-delete-vsc",
name: "VSC found with Delete policy and matching backup label, just deletes",
vscName: "already-delete-vsc",
backupName: "test-backup",
existing: &snapshotv1api.VolumeSnapshotContent{
ObjectMeta: metav1.ObjectMeta{Name: "already-delete-vsc"},
ObjectMeta: metav1.ObjectMeta{
Name: "already-delete-vsc",
Labels: map[string]string{
velerov1api.BackupNameLabel: "test-backup",
},
},
Spec: snapshotv1api.VolumeSnapshotContentSpec{
DeletionPolicy: snapshotv1api.VolumeSnapshotContentDelete,
Driver: "disk.csi.azure.com",
@@ -266,7 +323,7 @@ func TestTryDeleteOriginalVSC(t *testing.T) {
require.NoError(t, crClient.Create(t.Context(), test.existing))
}
result := p.tryDeleteOriginalVSC(t.Context(), test.vscName)
result := p.tryDeleteOriginalVSC(t.Context(), test.vscName, test.backupName)
require.Equal(t, test.expectRet, result)
// If cleanup succeeded, verify the VSC is gone
@@ -289,13 +346,18 @@ func TestTryDeleteOriginalVSC(t *testing.T) {
log: logrus.StandardLogger(),
crClient: errClient,
}
require.False(t, p.tryDeleteOriginalVSC(t.Context(), "some-vsc"))
require.False(t, p.tryDeleteOriginalVSC(t.Context(), "some-vsc", "test-backup"))
})
t.Run("Patch fails, returns false", func(t *testing.T) {
realClient := velerotest.NewFakeControllerRuntimeClient(t)
vsc := &snapshotv1api.VolumeSnapshotContent{
ObjectMeta: metav1.ObjectMeta{Name: "patch-fail-vsc"},
ObjectMeta: metav1.ObjectMeta{
Name: "patch-fail-vsc",
Labels: map[string]string{
velerov1api.BackupNameLabel: "test-backup",
},
},
Spec: snapshotv1api.VolumeSnapshotContentSpec{
DeletionPolicy: snapshotv1api.VolumeSnapshotContentRetain,
Driver: "disk.csi.azure.com",
@@ -313,13 +375,18 @@ func TestTryDeleteOriginalVSC(t *testing.T) {
log: logrus.StandardLogger(),
crClient: errClient,
}
require.False(t, p.tryDeleteOriginalVSC(t.Context(), "patch-fail-vsc"))
require.False(t, p.tryDeleteOriginalVSC(t.Context(), "patch-fail-vsc", "test-backup"))
})
t.Run("Delete fails, returns false", func(t *testing.T) {
realClient := velerotest.NewFakeControllerRuntimeClient(t)
vsc := &snapshotv1api.VolumeSnapshotContent{
ObjectMeta: metav1.ObjectMeta{Name: "delete-fail-vsc"},
ObjectMeta: metav1.ObjectMeta{
Name: "delete-fail-vsc",
Labels: map[string]string{
velerov1api.BackupNameLabel: "test-backup",
},
},
Spec: snapshotv1api.VolumeSnapshotContentSpec{
DeletionPolicy: snapshotv1api.VolumeSnapshotContentDelete,
Driver: "disk.csi.azure.com",
@@ -337,7 +404,7 @@ func TestTryDeleteOriginalVSC(t *testing.T) {
log: logrus.StandardLogger(),
crClient: errClient,
}
require.False(t, p.tryDeleteOriginalVSC(t.Context(), "delete-fail-vsc"))
require.False(t, p.tryDeleteOriginalVSC(t.Context(), "delete-fail-vsc", "test-backup"))
})
}