diff --git a/changelogs/unreleased/8141-shubham-pampattiwar b/changelogs/unreleased/8141-shubham-pampattiwar new file mode 100644 index 0000000000..4550628f70 --- /dev/null +++ b/changelogs/unreleased/8141-shubham-pampattiwar @@ -0,0 +1 @@ +Apply backupPVCConfig to backupPod volume spec diff --git a/pkg/exposer/csi_snapshot.go b/pkg/exposer/csi_snapshot.go index 630a8aad8a..da31a8f662 100644 --- a/pkg/exposer/csi_snapshot.go +++ b/pkg/exposer/csi_snapshot.go @@ -438,7 +438,7 @@ func (e *csiSnapshotExposer) createBackupPod(ctx context.Context, ownerObject co } var gracePeriod int64 = 0 - volumeMounts, volumeDevices, volumePath := kube.MakePodPVCAttachment(volumeName, backupPVC.Spec.VolumeMode) + volumeMounts, volumeDevices, volumePath := kube.MakePodPVCAttachment(volumeName, backupPVC.Spec.VolumeMode, true) volumeMounts = append(volumeMounts, podInfo.volumeMounts...) volumes := []corev1.Volume{{ @@ -446,6 +446,7 @@ func (e *csiSnapshotExposer) createBackupPod(ctx context.Context, ownerObject co VolumeSource: corev1.VolumeSource{ PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ ClaimName: backupPVC.Name, + ReadOnly: true, }, }, }} diff --git a/pkg/exposer/csi_snapshot_test.go b/pkg/exposer/csi_snapshot_test.go index 82044dbb8b..afde0b7f07 100644 --- a/pkg/exposer/csi_snapshot_test.go +++ b/pkg/exposer/csi_snapshot_test.go @@ -155,15 +155,17 @@ func TestExpose(t *testing.T) { } tests := []struct { - name string - snapshotClientObj []runtime.Object - kubeClientObj []runtime.Object - ownerBackup *velerov1.Backup - exposeParam CSISnapshotExposeParam - snapReactors []reactor - kubeReactors []reactor - err string - expectedVolumeSize *resource.Quantity + name string + snapshotClientObj []runtime.Object + kubeClientObj []runtime.Object + ownerBackup *velerov1.Backup + exposeParam CSISnapshotExposeParam + snapReactors []reactor + kubeReactors []reactor + err string + expectedVolumeSize *resource.Quantity + expectedReadOnlyPVC bool + expectedBackupPVCStorageClass string }{ { name: "wait vs ready fail", @@ -390,6 +392,84 @@ func TestExpose(t *testing.T) { }, expectedVolumeSize: resource.NewQuantity(567890, ""), }, + { + name: "backupPod mounts read only backupPVC", + ownerBackup: backup, + exposeParam: CSISnapshotExposeParam{ + SnapshotName: "fake-vs", + SourceNamespace: "fake-ns", + StorageClass: "fake-sc", + AccessMode: AccessModeFileSystem, + OperationTimeout: time.Millisecond, + ExposeTimeout: time.Millisecond, + BackupPVCConfig: map[string]nodeagent.BackupPVC{ + "fake-sc": { + StorageClass: "fake-sc-read-only", + ReadOnly: true, + }, + }, + }, + snapshotClientObj: []runtime.Object{ + vsObject, + vscObj, + }, + kubeClientObj: []runtime.Object{ + daemonSet, + }, + expectedReadOnlyPVC: true, + }, + { + name: "backupPod mounts read only backupPVC and storageClass specified in backupPVC config", + ownerBackup: backup, + exposeParam: CSISnapshotExposeParam{ + SnapshotName: "fake-vs", + SourceNamespace: "fake-ns", + StorageClass: "fake-sc", + AccessMode: AccessModeFileSystem, + OperationTimeout: time.Millisecond, + ExposeTimeout: time.Millisecond, + BackupPVCConfig: map[string]nodeagent.BackupPVC{ + "fake-sc": { + StorageClass: "fake-sc-read-only", + ReadOnly: true, + }, + }, + }, + snapshotClientObj: []runtime.Object{ + vsObject, + vscObj, + }, + kubeClientObj: []runtime.Object{ + daemonSet, + }, + expectedReadOnlyPVC: true, + expectedBackupPVCStorageClass: "fake-sc-read-only", + }, + { + name: "backupPod mounts backupPVC with storageClass specified in backupPVC config", + ownerBackup: backup, + exposeParam: CSISnapshotExposeParam{ + SnapshotName: "fake-vs", + SourceNamespace: "fake-ns", + StorageClass: "fake-sc", + AccessMode: AccessModeFileSystem, + OperationTimeout: time.Millisecond, + ExposeTimeout: time.Millisecond, + BackupPVCConfig: map[string]nodeagent.BackupPVC{ + "fake-sc": { + StorageClass: "fake-sc-read-only", + }, + }, + }, + snapshotClientObj: []runtime.Object{ + vsObject, + vscObj, + }, + kubeClientObj: []runtime.Object{ + daemonSet, + }, + expectedBackupPVCStorageClass: "fake-sc-read-only", + }, } for _, test := range tests { @@ -452,6 +532,20 @@ func TestExpose(t *testing.T) { } else { assert.Equal(t, *resource.NewQuantity(restoreSize, ""), backupPVC.Spec.Resources.Requests[corev1.ResourceStorage]) } + + if test.expectedReadOnlyPVC { + gotReadOnlyAccessMode := false + for _, accessMode := range backupPVC.Spec.AccessModes { + if accessMode == corev1.ReadOnlyMany { + gotReadOnlyAccessMode = true + } + } + assert.Equal(t, test.expectedReadOnlyPVC, gotReadOnlyAccessMode) + } + + if test.expectedBackupPVCStorageClass != "" { + assert.Equal(t, test.expectedBackupPVCStorageClass, *backupPVC.Spec.StorageClassName) + } } else { assert.EqualError(t, err, test.err) } diff --git a/pkg/exposer/generic_restore.go b/pkg/exposer/generic_restore.go index b5c9276027..d498470a77 100644 --- a/pkg/exposer/generic_restore.go +++ b/pkg/exposer/generic_restore.go @@ -310,7 +310,7 @@ func (e *genericRestoreExposer) createRestorePod(ctx context.Context, ownerObjec } var gracePeriod int64 = 0 - volumeMounts, volumeDevices, volumePath := kube.MakePodPVCAttachment(volumeName, targetPVC.Spec.VolumeMode) + volumeMounts, volumeDevices, volumePath := kube.MakePodPVCAttachment(volumeName, targetPVC.Spec.VolumeMode, false) volumeMounts = append(volumeMounts, podInfo.volumeMounts...) volumes := []corev1.Volume{{ diff --git a/pkg/util/kube/pvc_pv.go b/pkg/util/kube/pvc_pv.go index 070e5ed7b9..1811a2c1df 100644 --- a/pkg/util/kube/pvc_pv.go +++ b/pkg/util/kube/pvc_pv.go @@ -346,7 +346,7 @@ func IsPVCBound(pvc *corev1api.PersistentVolumeClaim) bool { } // MakePodPVCAttachment returns the volume mounts and devices for a pod needed to attach a PVC -func MakePodPVCAttachment(volumeName string, volumeMode *corev1api.PersistentVolumeMode) ([]corev1api.VolumeMount, []corev1api.VolumeDevice, string) { +func MakePodPVCAttachment(volumeName string, volumeMode *corev1api.PersistentVolumeMode, readOnly bool) ([]corev1api.VolumeMount, []corev1api.VolumeDevice, string) { var volumeMounts []corev1api.VolumeMount = nil var volumeDevices []corev1api.VolumeDevice = nil volumePath := "/" + volumeName @@ -360,6 +360,7 @@ func MakePodPVCAttachment(volumeName string, volumeMode *corev1api.PersistentVol volumeMounts = []corev1api.VolumeMount{{ Name: volumeName, MountPath: volumePath, + ReadOnly: readOnly, }} } diff --git a/pkg/util/kube/pvc_pv_test.go b/pkg/util/kube/pvc_pv_test.go index 5f6c3be74d..55c31774d9 100644 --- a/pkg/util/kube/pvc_pv_test.go +++ b/pkg/util/kube/pvc_pv_test.go @@ -1386,6 +1386,7 @@ func TestMakePodPVCAttachment(t *testing.T) { name string volumeName string volumeMode corev1api.PersistentVolumeMode + readOnly bool expectedVolumeMount []corev1api.VolumeMount expectedVolumeDevice []corev1api.VolumeDevice expectedVolumePath string @@ -1393,10 +1394,12 @@ func TestMakePodPVCAttachment(t *testing.T) { { name: "no volume mode specified", volumeName: "volume-1", + readOnly: true, expectedVolumeMount: []corev1api.VolumeMount{ { Name: "volume-1", MountPath: "/volume-1", + ReadOnly: true, }, }, expectedVolumePath: "/volume-1", @@ -1405,10 +1408,12 @@ func TestMakePodPVCAttachment(t *testing.T) { name: "fs mode specified", volumeName: "volume-2", volumeMode: corev1api.PersistentVolumeFilesystem, + readOnly: true, expectedVolumeMount: []corev1api.VolumeMount{ { Name: "volume-2", MountPath: "/volume-2", + ReadOnly: true, }, }, expectedVolumePath: "/volume-2", @@ -1425,6 +1430,20 @@ func TestMakePodPVCAttachment(t *testing.T) { }, expectedVolumePath: "/volume-3", }, + { + name: "fs mode specified with readOnly as false", + volumeName: "volume-4", + readOnly: false, + volumeMode: corev1api.PersistentVolumeFilesystem, + expectedVolumeMount: []corev1api.VolumeMount{ + { + Name: "volume-4", + MountPath: "/volume-4", + ReadOnly: false, + }, + }, + expectedVolumePath: "/volume-4", + }, } for _, tc := range testCases { @@ -1434,11 +1453,14 @@ func TestMakePodPVCAttachment(t *testing.T) { volMode = &tc.volumeMode } - mount, device, path := MakePodPVCAttachment(tc.volumeName, volMode) + mount, device, path := MakePodPVCAttachment(tc.volumeName, volMode, tc.readOnly) assert.Equal(t, tc.expectedVolumeMount, mount) assert.Equal(t, tc.expectedVolumeDevice, device) assert.Equal(t, tc.expectedVolumePath, path) + if tc.expectedVolumeMount != nil { + assert.Equal(t, tc.expectedVolumeMount[0].ReadOnly, tc.readOnly) + } }) } }