diff --git a/changelogs/unreleased/9206-Joeavaikath b/changelogs/unreleased/9206-Joeavaikath new file mode 100644 index 000000000..d080ddf8b --- /dev/null +++ b/changelogs/unreleased/9206-Joeavaikath @@ -0,0 +1 @@ +Remove labels associated with previous backups diff --git a/pkg/backup/actions/backup_pv_action.go b/pkg/backup/actions/backup_pv_action.go index 4b8a44ef6..c3f378fac 100644 --- a/pkg/backup/actions/backup_pv_action.go +++ b/pkg/backup/actions/backup_pv_action.go @@ -76,14 +76,8 @@ func (a *PVCAction) Execute(item runtime.Unstructured, backup *v1.Backup) (runti pvc.Spec.Selector = nil } - // remove label selectors with "velero.io/" prefixing in the key which is left by Velero restore - if pvc.Spec.Selector != nil && pvc.Spec.Selector.MatchLabels != nil { - for k := range pvc.Spec.Selector.MatchLabels { - if strings.HasPrefix(k, "velero.io/") { - delete(pvc.Spec.Selector.MatchLabels, k) - } - } - } + // Clean stale Velero labels from PVC metadata and selector + a.cleanupStaleVeleroLabels(pvc, backup) pvcMap, err := runtime.DefaultUnstructuredConverter.ToUnstructured(&pvc) if err != nil { @@ -92,3 +86,50 @@ func (a *PVCAction) Execute(item runtime.Unstructured, backup *v1.Backup) (runti return &unstructured.Unstructured{Object: pvcMap}, actionhelpers.RelatedItemsForPVC(pvc, a.log), nil } + +// cleanupStaleVeleroLabels removes stale Velero labels from both the PVC metadata +// and the selector's match labels to ensure clean backups +func (a *PVCAction) cleanupStaleVeleroLabels(pvc *corev1api.PersistentVolumeClaim, backup *v1.Backup) { + // Clean stale Velero labels from selector match labels + if pvc.Spec.Selector != nil && pvc.Spec.Selector.MatchLabels != nil { + for k := range pvc.Spec.Selector.MatchLabels { + if strings.HasPrefix(k, "velero.io/") { + a.log.Infof("Deleting stale Velero label %s from PVC %s selector", k, pvc.Name) + delete(pvc.Spec.Selector.MatchLabels, k) + } + } + } + + // Clean stale Velero labels from main metadata + if pvc.Labels != nil { + for k, v := range pvc.Labels { + // Only remove labels that are clearly stale from previous operations + shouldRemove := false + + // Always remove restore-name labels as these are from previous restores + if k == v1.RestoreNameLabel { + shouldRemove = true + } + + if k == v1.MustIncludeAdditionalItemAnnotation { + shouldRemove = true + } + + // Remove backup-name labels that don't match current backup + if k == v1.BackupNameLabel && v != backup.Name { + shouldRemove = true + } + + // Remove volume-snapshot-name labels from previous CSI backups + // Note: If this backup creates new CSI snapshots, the CSI action will add them back + if k == v1.VolumeSnapshotLabel { + shouldRemove = true + } + + if shouldRemove { + a.log.Infof("Deleting stale Velero label %s=%s from PVC %s", k, v, pvc.Name) + delete(pvc.Labels, k) + } + } + } +} diff --git a/pkg/backup/actions/backup_pv_action_test.go b/pkg/backup/actions/backup_pv_action_test.go index 7c8f596b3..41af9e9bf 100644 --- a/pkg/backup/actions/backup_pv_action_test.go +++ b/pkg/backup/actions/backup_pv_action_test.go @@ -149,3 +149,176 @@ func TestBackupPVAction(t *testing.T) { require.NoError(t, err) assert.Empty(t, additional) } + +func TestCleanupStaleVeleroLabels(t *testing.T) { + tests := []struct { + name string + inputPVC *corev1api.PersistentVolumeClaim + backup *v1.Backup + expectedLabels map[string]string + expectedSelector *metav1.LabelSelector + }{ + { + name: "removes restore-name labels", + inputPVC: &corev1api.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pvc", + Labels: map[string]string{ + "velero.io/restore-name": "old-restore", + "app": "myapp", + }, + }, + }, + backup: &v1.Backup{ObjectMeta: metav1.ObjectMeta{Name: "current-backup"}}, + expectedLabels: map[string]string{ + "app": "myapp", + }, + }, + { + name: "removes backup-name labels that don't match current backup", + inputPVC: &corev1api.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pvc", + Labels: map[string]string{ + "velero.io/backup-name": "old-backup", + "app": "myapp", + }, + }, + }, + backup: &v1.Backup{ObjectMeta: metav1.ObjectMeta{Name: "current-backup"}}, + expectedLabels: map[string]string{ + "app": "myapp", + }, + }, + { + name: "keeps backup-name labels that match current backup", + inputPVC: &corev1api.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pvc", + Labels: map[string]string{ + "velero.io/backup-name": "current-backup", + "app": "myapp", + }, + }, + }, + backup: &v1.Backup{ObjectMeta: metav1.ObjectMeta{Name: "current-backup"}}, + expectedLabels: map[string]string{ + "velero.io/backup-name": "current-backup", + "app": "myapp", + }, + }, + { + name: "removes volume-snapshot-name labels", + inputPVC: &corev1api.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pvc", + Labels: map[string]string{ + "velero.io/volume-snapshot-name": "old-snapshot", + "app": "myapp", + }, + }, + }, + backup: &v1.Backup{ObjectMeta: metav1.ObjectMeta{Name: "current-backup"}}, + expectedLabels: map[string]string{ + "app": "myapp", + }, + }, + { + name: "removes velero labels from selector match labels", + inputPVC: &corev1api.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pvc", + }, + Spec: corev1api.PersistentVolumeClaimSpec{ + Selector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + "velero.io/restore-name": "old-restore", + "velero.io/backup-name": "old-backup", + "app": "myapp", + }, + }, + }, + }, + backup: &v1.Backup{ObjectMeta: metav1.ObjectMeta{Name: "current-backup"}}, + expectedLabels: nil, + expectedSelector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + "app": "myapp", + }, + }, + }, + { + name: "handles PVC with no labels", + inputPVC: &corev1api.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pvc", + }, + }, + backup: &v1.Backup{ObjectMeta: metav1.ObjectMeta{Name: "current-backup"}}, + expectedLabels: nil, + }, + { + name: "handles PVC with no selector", + inputPVC: &corev1api.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pvc", + Labels: map[string]string{ + "app": "myapp", + }, + }, + }, + backup: &v1.Backup{ObjectMeta: metav1.ObjectMeta{Name: "current-backup"}}, + expectedLabels: map[string]string{ + "app": "myapp", + }, + expectedSelector: nil, + }, + { + name: "removes multiple stale velero labels", + inputPVC: &corev1api.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pvc", + Labels: map[string]string{ + "velero.io/restore-name": "old-restore", + "velero.io/backup-name": "old-backup", + "velero.io/volume-snapshot-name": "old-snapshot", + "app": "myapp", + "env": "prod", + }, + }, + Spec: corev1api.PersistentVolumeClaimSpec{ + Selector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + "velero.io/restore-name": "old-restore", + "app": "myapp", + }, + }, + }, + }, + backup: &v1.Backup{ObjectMeta: metav1.ObjectMeta{Name: "current-backup"}}, + expectedLabels: map[string]string{ + "app": "myapp", + "env": "prod", + }, + expectedSelector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + "app": "myapp", + }, + }, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + action := NewPVCAction(velerotest.NewLogger()) + + // Create a copy of the input PVC to avoid modifying the test case + pvcCopy := tc.inputPVC.DeepCopy() + + action.cleanupStaleVeleroLabels(pvcCopy, tc.backup) + + assert.Equal(t, tc.expectedLabels, pvcCopy.Labels, "Labels should match expected values") + assert.Equal(t, tc.expectedSelector, pvcCopy.Spec.Selector, "Selector should match expected values") + }) + } +}