Skip to content

Commit 7e53a85

Browse files
openshift-merge-bot[bot]openshift-merge-bot[bot]
authored
Merge pull request #2103 from mail2nadeem92/fix/dpa-defaultVolumesToFsBackup
Align DPA defaultVolumesToFsBackup with Velero backup spec
2 parents d4bbbd3 + 0bffba7 commit 7e53a85

File tree

6 files changed

+84
-13
lines changed

6 files changed

+84
-13
lines changed

‎api/v1alpha1/dataprotectionapplication_types.go‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -327,7 +327,11 @@ type VeleroConfig struct {
327327
// How long to wait on asynchronous BackupItemActions and RestoreItemActions to complete before timing out. Default value is 1h.
328328
// +optional
329329
DefaultItemOperationTimeout string `json:"defaultItemOperationTimeout,omitempty"`
330-
// Use pod volume file system backup by default for volumes
330+
// Use pod volume file system backup by default for volumes.
331+
// Matches backup.spec.defaultVolumesToFsBackup in Velero API.
332+
// +optional
333+
DefaultVolumesToFsBackup *bool `json:"defaultVolumesToFsBackup,omitempty"`
334+
// Deprecated: Use defaultVolumesToFsBackup instead (matches Velero backup spec).
331335
// +optional
332336
DefaultVolumesToFSBackup *bool `json:"defaultVolumesToFSBackup,omitempty"`
333337
// DisableFsBackup determines whether the NodeAgent should disable file system backup.

‎api/v1alpha1/zz_generated.deepcopy.go‎

Lines changed: 5 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎bundle/manifests/oadp.openshift.io_dataprotectionapplications.yaml‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1214,7 +1214,12 @@ spec:
12141214
description: Specify whether CSI snapshot data should be moved to backup storage by default
12151215
type: boolean
12161216
defaultVolumesToFSBackup:
1217-
description: Use pod volume file system backup by default for volumes
1217+
description: 'Deprecated: Use defaultVolumesToFsBackup instead (matches Velero backup spec).'
1218+
type: boolean
1219+
defaultVolumesToFsBackup:
1220+
description: |-
1221+
Use pod volume file system backup by default for volumes.
1222+
Matches backup.spec.defaultVolumesToFsBackup in Velero API.
12181223
type: boolean
12191224
disableFsBackup:
12201225
default: false

‎config/crd/bases/oadp.openshift.io_dataprotectionapplications.yaml‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1214,7 +1214,12 @@ spec:
12141214
description: Specify whether CSI snapshot data should be moved to backup storage by default
12151215
type: boolean
12161216
defaultVolumesToFSBackup:
1217-
description: Use pod volume file system backup by default for volumes
1217+
description: 'Deprecated: Use defaultVolumesToFsBackup instead (matches Velero backup spec).'
1218+
type: boolean
1219+
defaultVolumesToFsBackup:
1220+
description: |-
1221+
Use pod volume file system backup by default for volumes.
1222+
Matches backup.spec.defaultVolumesToFsBackup in Velero API.
12181223
type: boolean
12191224
disableFsBackup:
12201225
default: false

‎internal/controller/velero.go‎

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -420,16 +420,16 @@ func (r *DataProtectionApplicationReconciler) customizeVeleroDeployment(veleroDe
420420
// check for default-snapshot-move-data parameter
421421
defaultSnapshotMoveData := getDefaultSnapshotMoveDataValue(dpa)
422422
// check for default-volumes-to-fs-backup
423-
defaultVolumesToFSBackup := getDefaultVolumesToFSBackup(dpa)
423+
defaultVolumesToFsBackup := getDefaultVolumesToFsBackup(dpa)
424424

425425
// check for default-snapshot-move-data
426426
if len(defaultSnapshotMoveData) > 0 {
427427
veleroContainer.Args = append(veleroContainer.Args, fmt.Sprintf("--default-snapshot-move-data=%s", defaultSnapshotMoveData))
428428
}
429429

430430
// check for default-volumes-to-fs-backup
431-
if len(defaultVolumesToFSBackup) > 0 {
432-
veleroContainer.Args = append(veleroContainer.Args, fmt.Sprintf("--default-volumes-to-fs-backup=%s", defaultVolumesToFSBackup))
431+
if len(defaultVolumesToFsBackup) > 0 {
432+
veleroContainer.Args = append(veleroContainer.Args, fmt.Sprintf("--default-volumes-to-fs-backup=%s", defaultVolumesToFsBackup))
433433
}
434434

435435
// check for disable-informer-cache flag
@@ -735,15 +735,22 @@ func getDefaultSnapshotMoveDataValue(dpa *oadpv1alpha1.DataProtectionApplication
735735
return ""
736736
}
737737

738-
func getDefaultVolumesToFSBackup(dpa *oadpv1alpha1.DataProtectionApplication) string {
739-
if dpa.Spec.Configuration.Velero != nil && boolptr.IsSetToTrue(dpa.Spec.Configuration.Velero.DefaultVolumesToFSBackup) {
738+
func getDefaultVolumesToFsBackup(dpa *oadpv1alpha1.DataProtectionApplication) string {
739+
velero := dpa.Spec.Configuration.Velero
740+
if velero == nil {
741+
return ""
742+
}
743+
// Prefer new field (matches Velero backup.spec.defaultVolumesToFsBackup), fall back to deprecated field
744+
val := velero.DefaultVolumesToFsBackup
745+
if val == nil {
746+
val = velero.DefaultVolumesToFSBackup
747+
}
748+
if boolptr.IsSetToTrue(val) {
740749
return TrueVal
741750
}
742-
743-
if dpa.Spec.Configuration.Velero != nil && boolptr.IsSetToFalse(dpa.Spec.Configuration.Velero.DefaultVolumesToFSBackup) {
751+
if boolptr.IsSetToFalse(val) {
744752
return FalseVal
745753
}
746-
747754
return ""
748755
}
749756

‎internal/controller/velero_test.go‎

Lines changed: 47 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1261,7 +1261,7 @@ func TestDPAReconciler_buildVeleroDeployment(t *testing.T) {
12611261
DefaultItemOperationTimeout: "2h",
12621262
DefaultSnapshotMoveData: ptr.To(false),
12631263
NoDefaultBackupLocation: true,
1264-
DefaultVolumesToFSBackup: ptr.To(true),
1264+
DefaultVolumesToFsBackup: ptr.To(true),
12651265
DefaultPlugins: []oadpv1alpha1.DefaultPlugin{oadpv1alpha1.DefaultPluginCSI},
12661266
},
12671267
},
@@ -1329,7 +1329,29 @@ func TestDPAReconciler_buildVeleroDeployment(t *testing.T) {
13291329
}),
13301330
},
13311331
{
1332-
name: "valid DPA CR with DefaultVolumesToFSBackup true, Velero Deployment is built with DefaultVolumesToFSBackup true",
1332+
name: "valid DPA CR with DefaultVolumesToFsBackup true (new field), Velero Deployment is built with default-volumes-to-fs-backup true",
1333+
dpa: createTestDpaWith(
1334+
nil,
1335+
oadpv1alpha1.DataProtectionApplicationSpec{
1336+
Configuration: &oadpv1alpha1.ApplicationConfig{
1337+
Velero: &oadpv1alpha1.VeleroConfig{
1338+
DefaultVolumesToFsBackup: ptr.To(true),
1339+
},
1340+
},
1341+
},
1342+
),
1343+
veleroDeployment: testVeleroDeployment.DeepCopy(),
1344+
wantVeleroDeployment: createTestBuiltVeleroDeployment(TestBuiltVeleroDeploymentOptions{
1345+
args: []string{
1346+
defaultFileSystemBackupTimeout,
1347+
defaultRestoreResourcePriorities,
1348+
"--default-volumes-to-fs-backup=true",
1349+
defaultDisableInformerCache,
1350+
},
1351+
}),
1352+
},
1353+
{
1354+
name: "valid DPA CR with DefaultVolumesToFSBackup true (deprecated field, backwards compat), Velero Deployment is built with default-volumes-to-fs-backup true",
13331355
dpa: createTestDpaWith(
13341356
nil,
13351357
oadpv1alpha1.DataProtectionApplicationSpec{
@@ -1350,6 +1372,29 @@ func TestDPAReconciler_buildVeleroDeployment(t *testing.T) {
13501372
},
13511373
}),
13521374
},
1375+
{
1376+
name: "valid DPA CR with both DefaultVolumesToFsBackup and DefaultVolumesToFSBackup set, new field takes precedence",
1377+
dpa: createTestDpaWith(
1378+
nil,
1379+
oadpv1alpha1.DataProtectionApplicationSpec{
1380+
Configuration: &oadpv1alpha1.ApplicationConfig{
1381+
Velero: &oadpv1alpha1.VeleroConfig{
1382+
DefaultVolumesToFsBackup: ptr.To(false),
1383+
DefaultVolumesToFSBackup: ptr.To(true), // deprecated, should be ignored
1384+
},
1385+
},
1386+
},
1387+
),
1388+
veleroDeployment: testVeleroDeployment.DeepCopy(),
1389+
wantVeleroDeployment: createTestBuiltVeleroDeployment(TestBuiltVeleroDeploymentOptions{
1390+
args: []string{
1391+
defaultFileSystemBackupTimeout,
1392+
defaultRestoreResourcePriorities,
1393+
"--default-volumes-to-fs-backup=false",
1394+
defaultDisableInformerCache,
1395+
},
1396+
}),
1397+
},
13531398
{
13541399
name: "valid DPA CR with DisableInformerCache true, Velero Deployment is built with DisableInformerCache true",
13551400
dpa: createTestDpaWith(

0 commit comments

Comments
 (0)