Skip to content

Commit

Permalink
Merge pull request #8141 from shubham-pampattiwar/fix-backup-pvc-config
Browse files Browse the repository at this point in the history 
Apply backupPVCConfig to backupPod volume spec
  • Loading branch information
@Lyndon-Li
Lyndon-Li authored Aug 30, 2024
2 parents b5c9921 + f6e2b01 commit 3408ffe
Show file tree
Hide file tree
Showing 6 changed files with 132 additions and 13 deletions.
1 change: 1 addition & 0 deletions changelogs/unreleased/8141-shubham-pampattiwar
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Apply backupPVCConfig to backupPod volume spec
3 changes: 2 additions & 1 deletion pkg/exposer/csi_snapshot.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -438,14 +438,15 @@ 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{{
Name: volumeName,
VolumeSource: corev1.VolumeSource{
PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{
ClaimName: backupPVC.Name,
ReadOnly: true,
},
},
}}
Expand Down
112 changes: 103 additions & 9 deletions pkg/exposer/csi_snapshot_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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)
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/exposer/generic_restore.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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{{
Expand Down
3 changes: 2 additions & 1 deletion pkg/util/kube/pvc_pv.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -360,6 +360,7 @@ func MakePodPVCAttachment(volumeName string, volumeMode *corev1api.PersistentVol
volumeMounts = []corev1api.VolumeMount{{
Name: volumeName,
MountPath: volumePath,
ReadOnly: readOnly,
}}
}

Expand Down
24 changes: 23 additions & 1 deletion pkg/util/kube/pvc_pv_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1386,17 +1386,20 @@ func TestMakePodPVCAttachment(t *testing.T) {
name string
volumeName string
volumeMode corev1api.PersistentVolumeMode
readOnly bool
expectedVolumeMount []corev1api.VolumeMount
expectedVolumeDevice []corev1api.VolumeDevice
expectedVolumePath string
}{
{
name: "no volume mode specified",
volumeName: "volume-1",
readOnly: true,
expectedVolumeMount: []corev1api.VolumeMount{
{
Name: "volume-1",
MountPath: "/volume-1",
ReadOnly: true,
},
},
expectedVolumePath: "/volume-1",
Expand All @@ -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",
Expand All @@ -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 {
Expand All @@ -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)
}
})
}
}

0 comments on commit 3408ffe

Please sign in to comment.