diff --git a/changelogs/unreleased/6481-blackpiglet b/changelogs/unreleased/6481-blackpiglet new file mode 100644 index 000000000..2b54c1416 --- /dev/null +++ b/changelogs/unreleased/6481-blackpiglet @@ -0,0 +1 @@ +Remove PVC's selector in backup's PVC action. \ No newline at end of file diff --git a/pkg/backup/backup_pv_action.go b/pkg/backup/backup_pv_action.go index 56ff1f4b9..a636f83d8 100644 --- a/pkg/backup/backup_pv_action.go +++ b/pkg/backup/backup_pv_action.go @@ -72,6 +72,14 @@ func (a *PVCAction) Execute(item runtime.Unstructured, backup *v1.Backup) (runti pvc.Spec.DataSourceRef = nil } + // When StorageClassName is set to "", it means no StorageClass is specified, + // even the default StorageClass is not used. Only keep the Selector for this case. + // https://kubernetes.io/docs/concepts/storage/persistent-volumes/#reserving-a-persistentvolume + if pvc.Spec.StorageClassName == nil || *pvc.Spec.StorageClassName != "" { + // Clean the selector to make the PVC to dynamically allocate PV. + 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 { diff --git a/pkg/backup/backup_pv_action_test.go b/pkg/backup/backup_pv_action_test.go index 8cb04e221..d9e0a5b33 100644 --- a/pkg/backup/backup_pv_action_test.go +++ b/pkg/backup/backup_pv_action_test.go @@ -21,10 +21,14 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + corev1 "k8s.io/api/core/v1" corev1api "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime" v1 "github.com/vmware-tanzu/velero/pkg/apis/velero/v1" + "github.com/vmware-tanzu/velero/pkg/builder" "github.com/vmware-tanzu/velero/pkg/kuberesource" "github.com/vmware-tanzu/velero/pkg/plugin/velero" velerotest "github.com/vmware-tanzu/velero/pkg/test" @@ -55,6 +59,61 @@ func TestBackupPVAction(t *testing.T) { assert.NoError(t, err) assert.Len(t, additional, 0) + // Action should clean the spec.Selector when the StorageClassName is not set. + input := builder.ForPersistentVolumeClaim("abc", "abc").VolumeName("pv").Selector(&metav1.LabelSelector{MatchLabels: map[string]string{"abc": "abc"}}).Phase(corev1.ClaimBound).Result() + inputUnstructured, err := runtime.DefaultUnstructuredConverter.ToUnstructured(input) + require.NoError(t, err) + item, additional, err := a.Execute(&unstructured.Unstructured{Object: inputUnstructured}, backup) + require.NoError(t, err) + require.Len(t, additional, 1) + modifiedPVC := new(corev1.PersistentVolumeClaim) + require.NoError(t, runtime.DefaultUnstructuredConverter.FromUnstructured(item.UnstructuredContent(), modifiedPVC)) + require.Nil(t, modifiedPVC.Spec.Selector) + + // Action should clean the spec.Selector when the StorageClassName is set to specific StorageClass + input2 := builder.ForPersistentVolumeClaim("abc", "abc").VolumeName("pv").StorageClass("sc1").Selector(&metav1.LabelSelector{MatchLabels: map[string]string{"abc": "abc"}}).Phase(corev1.ClaimBound).Result() + inputUnstructured2, err2 := runtime.DefaultUnstructuredConverter.ToUnstructured(input2) + require.NoError(t, err2) + item2, additional2, err2 := a.Execute(&unstructured.Unstructured{Object: inputUnstructured2}, backup) + require.NoError(t, err2) + require.Len(t, additional2, 1) + modifiedPVC2 := new(corev1.PersistentVolumeClaim) + require.NoError(t, runtime.DefaultUnstructuredConverter.FromUnstructured(item2.UnstructuredContent(), modifiedPVC2)) + require.Nil(t, modifiedPVC2.Spec.Selector) + + // Action should keep the spec.Selector when the StorageClassName is set to "" + input3 := builder.ForPersistentVolumeClaim("abc", "abc").StorageClass("").Selector(&metav1.LabelSelector{MatchLabels: map[string]string{"abc": "abc"}}).VolumeName("pv").Phase(corev1.ClaimBound).Result() + inputUnstructured3, err3 := runtime.DefaultUnstructuredConverter.ToUnstructured(input3) + require.NoError(t, err3) + item3, additional3, err3 := a.Execute(&unstructured.Unstructured{Object: inputUnstructured3}, backup) + require.NoError(t, err3) + require.Len(t, additional3, 1) + modifiedPVC3 := new(corev1.PersistentVolumeClaim) + require.NoError(t, runtime.DefaultUnstructuredConverter.FromUnstructured(item3.UnstructuredContent(), modifiedPVC3)) + require.Equal(t, input3.Spec.Selector, modifiedPVC3.Spec.Selector) + + // Action should delete label started with"velero.io/" from the spec.Selector when the StorageClassName is set to "" + input4 := builder.ForPersistentVolumeClaim("abc", "abc").StorageClass("").Selector(&metav1.LabelSelector{MatchLabels: map[string]string{"velero.io/abc": "abc", "abc": "abc"}}).VolumeName("pv").Phase(corev1.ClaimBound).Result() + inputUnstructured4, err4 := runtime.DefaultUnstructuredConverter.ToUnstructured(input4) + require.NoError(t, err4) + item4, additional4, err4 := a.Execute(&unstructured.Unstructured{Object: inputUnstructured4}, backup) + require.NoError(t, err4) + require.Len(t, additional4, 1) + modifiedPVC4 := new(corev1.PersistentVolumeClaim) + require.NoError(t, runtime.DefaultUnstructuredConverter.FromUnstructured(item4.UnstructuredContent(), modifiedPVC4)) + require.Equal(t, &metav1.LabelSelector{MatchLabels: map[string]string{"abc": "abc"}}, modifiedPVC4.Spec.Selector) + + // Action should clean the spec.Selector when the StorageClassName has value + input5 := builder.ForPersistentVolumeClaim("abc", "abc").StorageClass("sc1").Selector(&metav1.LabelSelector{MatchLabels: map[string]string{"velero.io/abc": "abc", "abc": "abc"}}).VolumeName("pv").Phase(corev1.ClaimBound).Result() + inputUnstructured5, err5 := runtime.DefaultUnstructuredConverter.ToUnstructured(input5) + require.NoError(t, err5) + item5, additional5, err5 := a.Execute(&unstructured.Unstructured{Object: inputUnstructured5}, backup) + require.NoError(t, err5) + require.Len(t, additional5, 1) + modifiedPVC5 := new(corev1.PersistentVolumeClaim) + require.NoError(t, runtime.DefaultUnstructuredConverter.FromUnstructured(item5.UnstructuredContent(), modifiedPVC5)) + require.Nil(t, modifiedPVC5.Spec.Selector) + // non-empty spec.volumeName when status.phase is empty // should result in no error and no additional items pvc.Object["spec"].(map[string]interface{})["volumeName"] = "myVolume"