diff --git a/changelogs/unreleased/10607-blackpiglet b/changelogs/unreleased/10607-blackpiglet new file mode 100644 index 000000000..d7ee9c0dd --- /dev/null +++ b/changelogs/unreleased/10607-blackpiglet @@ -0,0 +1 @@ +Use GetGlobalResourcePolicies to replace GetResourcePoliciesFromBackup. \ No newline at end of file diff --git a/internal/resourcepolicies/resource_policies.go b/internal/resourcepolicies/resource_policies.go index 9b8b1da7d..6f0687423 100644 --- a/internal/resourcepolicies/resource_policies.go +++ b/internal/resourcepolicies/resource_policies.go @@ -650,7 +650,7 @@ func (p *Policies) GetNamespacedFilterPolicies() []NamespacedFilterPolicy { return p.namespacedFilterPolicies } -func GetResourcePoliciesFromBackup( +func getResourcePoliciesFromBackup( backup velerov1api.Backup, client crclient.Client, logger logrus.FieldLogger, @@ -728,7 +728,7 @@ func GetGlobalResourcePolicies( // GetResourcePoliciesFromBackupWithGlobal builds the effective resource policies for a backup // by merging the backup-referenced resource policies with the global backup volume policies -// (when globalConfigMapName is set). The merged volumePolicies list is the backup-level +// (when globalConfigMapName is set or specified via backup annotations). The merged volumePolicies list is the backup-level // policies followed by the global ones, so the first match wins and a backup can override the // global baseline for a specific volume while still inheriting the rest of the global rules. func GetResourcePoliciesFromBackupWithGlobal( @@ -738,11 +738,18 @@ func GetResourcePoliciesFromBackupWithGlobal( installNamespace string, logger logrus.FieldLogger, ) (*Policies, error) { - backupPolicies, err := GetResourcePoliciesFromBackup(backup, client, logger) + backupPolicies, err := getResourcePoliciesFromBackup(backup, client, logger) if err != nil { return nil, err } + if globalConfigMapName == "" && backup.Annotations != nil { + globalConfigMapName = backup.Annotations[velerov1api.GlobalBackupVolumePolicyConfigMapAnnotation] + } + if installNamespace == "" { + installNamespace = backup.Namespace + } + if globalConfigMapName == "" { return backupPolicies, nil } diff --git a/internal/resourcepolicies/resource_policies_test.go b/internal/resourcepolicies/resource_policies_test.go index 6e87c8fe7..6a17f10e7 100644 --- a/internal/resourcepolicies/resource_policies_test.go +++ b/internal/resourcepolicies/resource_policies_test.go @@ -724,7 +724,7 @@ volumePolicies: for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - resPolicies, err := GetResourcePoliciesFromBackup(tc.backup, client, logger) + resPolicies, err := getResourcePoliciesFromBackup(tc.backup, client, logger) if tc.expectedErr == "" { require.NoError(t, err) if tc.backup.Spec.ResourcePolicy != nil && tc.backup.Spec.ResourcePolicy.Kind == ConfigmapRefType { @@ -3146,6 +3146,8 @@ volumePolicies: globalCMName string globalCM *corev1api.ConfigMap backupRef string + backupAnnotations map[string]string + installNamespace string expectErr bool expectedGp2Action VolumeActionType expectedNumPolicies int @@ -3187,6 +3189,15 @@ volumePolicies: expectedGp2Action: Skip, // only global matches gp2 expectedNumPolicies: 2, }, + { + name: "merge - global from backup annotation with empty params", + backupRef: "backup01", + backupCM: globalPolicyConfigMap("backup01", otherFsBackup), + globalCM: globalPolicyConfigMap("global", gp2Skip), + backupAnnotations: map[string]string{velerov1api.GlobalBackupVolumePolicyConfigMapAnnotation: "global"}, + expectedGp2Action: Skip, // only global matches gp2 + expectedNumPolicies: 2, + }, { name: "global configmap missing - error", globalCMName: "global", @@ -3227,8 +3238,15 @@ volumePolicies: } b := backupWithPolicy(tc.backupRef) + if tc.backupAnnotations != nil { + b.Annotations = tc.backupAnnotations + } - p, err := GetResourcePoliciesFromBackupWithGlobal(b, client, tc.globalCMName, "velero", logrus.New()) + ns := tc.installNamespace + if ns == "" && tc.backupAnnotations == nil { + ns = "velero" + } + p, err := GetResourcePoliciesFromBackupWithGlobal(b, client, tc.globalCMName, ns, logrus.New()) if tc.expectErr { require.Error(t, err) return diff --git a/internal/volumehelper/volume_policy_helper.go b/internal/volumehelper/volume_policy_helper.go index 8d16a4383..435264115 100644 --- a/internal/volumehelper/volume_policy_helper.go +++ b/internal/volumehelper/volume_policy_helper.go @@ -119,7 +119,17 @@ func NewVolumeHelperImplWithCache( pvcPodCache *podvolumeutil.PVCPodCache, pvcMustInclusionTracker vhutil.PVCMustInclusionTracker, ) (vhutil.VolumeHelper, error) { - resourcePolicies, err := resourcepolicies.GetResourcePoliciesFromBackup(backup, client, logger) + var globalCMName string + if backup.Annotations != nil { + globalCMName = backup.Annotations[velerov1api.GlobalBackupVolumePolicyConfigMapAnnotation] + } + resourcePolicies, err := resourcepolicies.GetResourcePoliciesFromBackupWithGlobal( + backup, + client, + globalCMName, + backup.Namespace, + logger, + ) if err != nil { return nil, errors.Wrap(err, "failed to get volume policies from backup") } diff --git a/internal/volumehelper/volume_policy_helper_test.go b/internal/volumehelper/volume_policy_helper_test.go index b8acc68c4..31a2e1e6b 100644 --- a/internal/volumehelper/volume_policy_helper_test.go +++ b/internal/volumehelper/volume_policy_helper_test.go @@ -1121,6 +1121,7 @@ func TestNewVolumeHelperImplWithCache(t *testing.T) { name string backup velerov1api.Backup resourcePolicyConfigMap *corev1api.ConfigMap + globalPolicyConfigMap *corev1api.ConfigMap pvcPodCache bool // whether to pass a cache expectError bool }{ @@ -1205,6 +1206,74 @@ volumePolicies: pvcPodCache: false, expectError: true, }, + { + name: "creates VolumeHelper with global volume policies and per-backup policies", + backup: velerov1api.Backup{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-backup", + Namespace: "velero", + Annotations: map[string]string{ + velerov1api.GlobalBackupVolumePolicyConfigMapAnnotation: "global-policy", + }, + }, + Spec: velerov1api.BackupSpec{ + SnapshotVolumes: ptr.To(true), + ResourcePolicy: &corev1api.TypedLocalObjectReference{ + Kind: "ConfigMap", + Name: "resource-policy", + }, + }, + }, + resourcePolicyConfigMap: &corev1api.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "resource-policy", + Namespace: "velero", + }, + Data: map[string]string{ + "policy": `version: v1 +volumePolicies: +- conditions: + storageClass: + - gp2-csi + action: + type: snapshot`, + }, + }, + globalPolicyConfigMap: &corev1api.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "global-policy", + Namespace: "velero", + }, + Data: map[string]string{ + "policy": `version: v1 +volumePolicies: +- conditions: + storageClass: + - other-csi + action: + type: skip`, + }, + }, + pvcPodCache: true, + expectError: false, + }, + { + name: "fails when global resource policy ConfigMap not found", + backup: velerov1api.Backup{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-backup", + Namespace: "velero", + Annotations: map[string]string{ + velerov1api.GlobalBackupVolumePolicyConfigMapAnnotation: "non-existent-global-policy", + }, + }, + Spec: velerov1api.BackupSpec{ + SnapshotVolumes: ptr.To(true), + }, + }, + pvcPodCache: false, + expectError: true, + }, } for _, tc := range testCases { @@ -1213,6 +1282,9 @@ volumePolicies: if tc.resourcePolicyConfigMap != nil { objs = append(objs, tc.resourcePolicyConfigMap) } + if tc.globalPolicyConfigMap != nil { + objs = append(objs, tc.globalPolicyConfigMap) + } fakeClient := velerotest.NewFakeControllerRuntimeClient(t, objs...) var cache *podvolumeutil.PVCPodCache diff --git a/pkg/backup/actions/csi/pvc_action_test.go b/pkg/backup/actions/csi/pvc_action_test.go index c5d9c33fa..7fb3da213 100644 --- a/pkg/backup/actions/csi/pvc_action_test.go +++ b/pkg/backup/actions/csi/pvc_action_test.go @@ -81,23 +81,24 @@ func (c *errorInjectingClient) Create(ctx context.Context, obj crclient.Object, func TestExecute(t *testing.T) { boolTrue := true tests := []struct { - name string - backup *velerov1api.Backup - pvc *corev1api.PersistentVolumeClaim - pv *corev1api.PersistentVolume - sc *storagev1api.StorageClass - vsClass *snapshotv1api.VolumeSnapshotClass - operationID string - expectedErr error - expectErr bool // Use bool for cases where we just need to check for any error - expectedBackup *velerov1api.Backup - expectedDataUpload *velerov2alpha1.DataUpload - expectedPVC *corev1api.PersistentVolumeClaim - resourcePolicy *corev1api.ConfigMap - extraObjects []runtime.Object - failVSCreate bool - skipVSReadyUpdate bool // New flag to control VS readiness - expectedVSClassName string + name string + backup *velerov1api.Backup + pvc *corev1api.PersistentVolumeClaim + pv *corev1api.PersistentVolume + sc *storagev1api.StorageClass + vsClass *snapshotv1api.VolumeSnapshotClass + operationID string + expectedErr error + expectErr bool // Use bool for cases where we just need to check for any error + expectedBackup *velerov1api.Backup + expectedDataUpload *velerov2alpha1.DataUpload + expectedPVC *corev1api.PersistentVolumeClaim + resourcePolicy *corev1api.ConfigMap + globalResourcePolicy *corev1api.ConfigMap + extraObjects []runtime.Object + failVSCreate bool + skipVSReadyUpdate bool // New flag to control VS readiness + expectedVSClassName string }{ { name: "Skip PVC BIA when backup is in finalizing phase", @@ -229,10 +230,75 @@ func TestExecute(t *testing.T) { vsClass: builder.ForVolumeSnapshotClass("policy-selected-vsclass").Driver("hostpath").Result(), expectedVSClassName: "policy-selected-vsclass", }, + { + name: "Global backup volume policy dataMover overrides backup.Spec.DataMover", + backup: builder.ForBackup("velero", "test"). + SnapshotMoveData(true). + DataMover("velero-fs"). + CSISnapshotTimeout(1 * time.Minute). + ObjectMeta(builder.WithAnnotations(velerov1api.GlobalBackupVolumePolicyConfigMapAnnotation, "global-volume-policy")). + Result(), + pvc: builder.ForPersistentVolumeClaim("velero", "testPVC").VolumeName("testPV").StorageClass("testSC").Phase(corev1api.ClaimBound).Result(), + pv: builder.ForPersistentVolume("testPV").CSI("hostpath", "testVolume").Result(), + sc: builder.ForStorageClass("testSC").Provisioner("hostpath").Result(), + vsClass: builder.ForVolumeSnapshotClass("testVSClass").Driver("hostpath").ObjectMeta(builder.WithLabels(velerov1api.VolumeSnapshotClassSelectorLabel, "")).Result(), + globalResourcePolicy: builder.ForConfigMap("velero", "global-volume-policy"). + Data("policy", `{"version":"v1","volumePolicies":[{"conditions":{"csi":{}},"action":{"type":"snapshot","parameters":{"dataMover":"velero-block"}}}]}`). + Result(), + extraObjects: []runtime.Object{ + &corev1api.Node{ + ObjectMeta: metav1.ObjectMeta{Name: "linux-node", Labels: map[string]string{corev1api.LabelOSStable: "linux"}}, + }, + &appsv1api.DaemonSet{ + ObjectMeta: metav1.ObjectMeta{Namespace: "velero", Name: "node-agent"}, + Status: appsv1api.DaemonSetStatus{NumberReady: 3}, + }, + }, + operationID: ".", + expectedDataUpload: &velerov2alpha1.DataUpload{ + TypeMeta: metav1.TypeMeta{ + Kind: "DataUpload", + APIVersion: velerov2alpha1.SchemeGroupVersion.String(), + }, + ObjectMeta: metav1.ObjectMeta{ + GenerateName: "test-", + Namespace: "velero", + Labels: map[string]string{ + velerov1api.BackupNameLabel: "test", + velerov1api.BackupUIDLabel: "", + velerov1api.PVCUIDLabel: "", + velerov1api.AsyncOperationIDLabel: "du-.", + }, + OwnerReferences: []metav1.OwnerReference{ + { + APIVersion: "velero.io/v1", + Kind: "Backup", + Name: "test", + UID: "", + Controller: &boolTrue, + }, + }, + }, + Spec: velerov2alpha1.DataUploadSpec{ + SnapshotType: velerov2alpha1.SnapshotTypeCSI, + CSISnapshot: &velerov2alpha1.CSISnapshotSpec{ + VolumeSnapshot: "", + StorageClass: "testSC", + SnapshotClass: "testVSClass", + }, + SourcePVC: "testPVC", + SourceNamespace: "velero", + DataMover: "velero-block", + OperationTimeout: metav1.Duration{Duration: 1 * time.Minute}, + ParentSnapshot: "", + }, + }, + }, } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { + t.Parallel() logger := logrus.New() logger.Level = logrus.DebugLevel objects := make([]runtime.Object, 0) @@ -251,6 +317,9 @@ func TestExecute(t *testing.T) { if tc.resourcePolicy != nil { objects = append(objects, tc.resourcePolicy) } + if tc.globalResourcePolicy != nil { + objects = append(objects, tc.globalResourcePolicy) + } objects = append(objects, tc.extraObjects...) var crClient crclient.Client @@ -706,12 +775,13 @@ func TestListGroupedPVCs(t *testing.T) { func TestFilterPVCsByVolumePolicy(t *testing.T) { tests := []struct { - name string - pvcs []corev1api.PersistentVolumeClaim - pvs []corev1api.PersistentVolume - volumePolicyStr string - expectCount int - expectError bool + name string + pvcs []corev1api.PersistentVolumeClaim + pvs []corev1api.PersistentVolume + volumePolicyStr string + globalVolumePolicyStr string + expectCount int + expectError bool }{ { name: "All PVCs should be included when no volume policy", @@ -910,6 +980,65 @@ volumePolicies: volumePolicyStr: ` version: v1 volumePolicies: +- conditions: + nfs: {} + action: + type: skip +`, + expectCount: 1, + }, + { + name: "Filter out NFS PVC using global volume policy annotation", + pvcs: []corev1api.PersistentVolumeClaim{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "pvc-linstor", + Namespace: "ns-1", + Labels: map[string]string{"app.kubernetes.io/instance": "myapp"}, + }, + Spec: corev1api.PersistentVolumeClaimSpec{ + VolumeName: "pv-linstor", + StorageClassName: ptr.To("sc-linstor"), + }, + Status: corev1api.PersistentVolumeClaimStatus{Phase: corev1api.ClaimBound}, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Name: "pvc-nfs", + Namespace: "ns-1", + Labels: map[string]string{"app.kubernetes.io/instance": "myapp"}, + }, + Spec: corev1api.PersistentVolumeClaimSpec{ + VolumeName: "pv-nfs", + StorageClassName: ptr.To("sc-nfs"), + }, + Status: corev1api.PersistentVolumeClaimStatus{Phase: corev1api.ClaimBound}, + }, + }, + pvs: []corev1api.PersistentVolume{ + { + ObjectMeta: metav1.ObjectMeta{Name: "pv-linstor"}, + Spec: corev1api.PersistentVolumeSpec{ + PersistentVolumeSource: corev1api.PersistentVolumeSource{ + CSI: &corev1api.CSIPersistentVolumeSource{Driver: "linstor.csi.linbit.com"}, + }, + }, + }, + { + ObjectMeta: metav1.ObjectMeta{Name: "pv-nfs"}, + Spec: corev1api.PersistentVolumeSpec{ + PersistentVolumeSource: corev1api.PersistentVolumeSource{ + NFS: &corev1api.NFSVolumeSource{ + Server: "nfs-server", + Path: "/export", + }, + }, + }, + }, + }, + globalVolumePolicyStr: ` +version: v1 +volumePolicies: - conditions: nfs: {} action: @@ -936,6 +1065,25 @@ volumePolicies: Spec: velerov1api.BackupSpec{}, } + // Add global volume policy ConfigMap if specified + if tt.globalVolumePolicyStr != "" { + cm := &corev1api.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "global-volume-policy", + Namespace: "velero", + }, + Data: map[string]string{ + "volume-policy": tt.globalVolumePolicyStr, + }, + } + require.NoError(t, client.Create(t.Context(), cm)) + + if backup.Annotations == nil { + backup.Annotations = make(map[string]string) + } + backup.Annotations[velerov1api.GlobalBackupVolumePolicyConfigMapAnnotation] = "global-volume-policy" + } + // Add volume policy ConfigMap if specified if tt.volumePolicyStr != "" { cm := &corev1api.ConfigMap{ @@ -1519,7 +1667,11 @@ func TestWaitForVGSAssociatedVS(t *testing.T) { crClient: client, } - vsMap, err := action.waitForVGSAssociatedVS(t.Context(), tt.groupedPVCs, vgs, 2*time.Second) + timeout := 2 * time.Second + if tt.expectErr { + timeout = 20 * time.Millisecond + } + vsMap, err := action.waitForVGSAssociatedVS(t.Context(), tt.groupedPVCs, vgs, timeout) if tt.expectErr { if err == nil { @@ -1922,7 +2074,11 @@ func TestWaitForVGSCBinding(t *testing.T) { crClient: velerotest.NewFakeControllerRuntimeClientWithVGS(t, tt.vgs.DeepCopy()), } - err := action.waitForVGSCBinding(t.Context(), tt.vgs, 1*time.Second) + timeout := 1 * time.Second + if tt.expectErr { + timeout = 20 * time.Millisecond + } + err := action.waitForVGSCBinding(t.Context(), tt.vgs, timeout) if tt.expectErr { require.Error(t, err) diff --git a/pkg/plugin/utils/volumehelper/volume_policy_helper.go b/pkg/plugin/utils/volumehelper/volume_policy_helper.go index 706e3fa1a..68db740da 100644 --- a/pkg/plugin/utils/volumehelper/volume_policy_helper.go +++ b/pkg/plugin/utils/volumehelper/volume_policy_helper.go @@ -76,9 +76,15 @@ func ShouldPerformSnapshotWithVolumeHelper( } // Otherwise, create a new VolumeHelper (original behavior for third-party plugins) - resourcePolicies, err := resourcepolicies.GetResourcePoliciesFromBackup( + var globalCMName string + if backup.Annotations != nil { + globalCMName = backup.Annotations[velerov1api.GlobalBackupVolumePolicyConfigMapAnnotation] + } + resourcePolicies, err := resourcepolicies.GetResourcePoliciesFromBackupWithGlobal( backup, crClient, + globalCMName, + backup.Namespace, logger, ) if err != nil { diff --git a/pkg/plugin/utils/volumehelper/volume_policy_helper_test.go b/pkg/plugin/utils/volumehelper/volume_policy_helper_test.go index 4ddd49ddf..21c05dd62 100644 --- a/pkg/plugin/utils/volumehelper/volume_policy_helper_test.go +++ b/pkg/plugin/utils/volumehelper/volume_policy_helper_test.go @@ -37,6 +37,7 @@ func TestShouldPerformSnapshotWithBackup(t *testing.T) { name string pvc *corev1api.PersistentVolumeClaim pv *corev1api.PersistentVolume + configMaps []*corev1api.ConfigMap backup *velerov1api.Backup wantSnapshot bool wantError bool @@ -122,12 +123,117 @@ func TestShouldPerformSnapshotWithBackup(t *testing.T) { wantSnapshot: false, wantError: false, }, + { + name: "Honors global volume policy from backup annotation to skip snapshot", + pvc: &corev1api.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pvc", + Namespace: "default", + }, + Spec: corev1api.PersistentVolumeClaimSpec{ + VolumeName: "test-pv", + }, + Status: corev1api.PersistentVolumeClaimStatus{ + Phase: corev1api.ClaimBound, + }, + }, + pv: &corev1api.PersistentVolume{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pv", + }, + Spec: corev1api.PersistentVolumeSpec{ + PersistentVolumeSource: corev1api.PersistentVolumeSource{ + CSI: &corev1api.CSIPersistentVolumeSource{ + Driver: "test-driver", + }, + }, + ClaimRef: &corev1api.ObjectReference{ + Namespace: "default", + Name: "test-pvc", + }, + }, + }, + configMaps: []*corev1api.ConfigMap{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "global-policy", + Namespace: "velero", + }, + Data: map[string]string{ + "policy": `version: v1 +volumePolicies: +- conditions: + csi: + driver: test-driver + action: + type: skip`, + }, + }, + }, + backup: &velerov1api.Backup{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-backup", + Namespace: "velero", + Annotations: map[string]string{ + velerov1api.GlobalBackupVolumePolicyConfigMapAnnotation: "global-policy", + }, + }, + }, + wantSnapshot: false, + wantError: false, + }, + { + name: "Fails when global volume policy ConfigMap not found", + pvc: &corev1api.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pvc", + Namespace: "default", + }, + Spec: corev1api.PersistentVolumeClaimSpec{ + VolumeName: "test-pv", + }, + Status: corev1api.PersistentVolumeClaimStatus{ + Phase: corev1api.ClaimBound, + }, + }, + pv: &corev1api.PersistentVolume{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pv", + }, + Spec: corev1api.PersistentVolumeSpec{ + PersistentVolumeSource: corev1api.PersistentVolumeSource{ + CSI: &corev1api.CSIPersistentVolumeSource{ + Driver: "test-driver", + }, + }, + ClaimRef: &corev1api.ObjectReference{ + Namespace: "default", + Name: "test-pvc", + }, + }, + }, + backup: &velerov1api.Backup{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-backup", + Namespace: "velero", + Annotations: map[string]string{ + velerov1api.GlobalBackupVolumePolicyConfigMapAnnotation: "non-existent-global-policy", + }, + }, + }, + wantSnapshot: false, + wantError: true, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - // Create fake client with PV and PVC - client := velerotest.NewFakeControllerRuntimeClient(t, tt.pv, tt.pvc) + // Create fake client with PV and PVC and ConfigMaps + objs := []runtime.Object{tt.pv, tt.pvc} + for _, cm := range tt.configMaps { + objs = append(objs, cm) + } + client := velerotest.NewFakeControllerRuntimeClient(t, objs...) // Convert PVC to unstructured pvcMap, err := runtime.DefaultUnstructuredConverter.ToUnstructured(tt.pvc)