Skip to content

Align DPA defaultVolumesToFsBackup with Velero backup spec #2103

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
mail2nadeem92 wants to merge 1 commit into
base: oadp-dev
Choose a base branch
from
+84 −13
Open

Align DPA defaultVolumesToFsBackup with Velero backup spec #2103

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion api/v1alpha1/dataprotectionapplication_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -327,7 +327,11 @@ type VeleroConfig struct {
// How long to wait on asynchronous BackupItemActions and RestoreItemActions to complete before timing out. Default value is 1h.
// +optional
DefaultItemOperationTimeout string `json:"defaultItemOperationTimeout,omitempty"`
// Use pod volume file system backup by default for volumes
// Use pod volume file system backup by default for volumes.
// Matches backup.spec.defaultVolumesToFsBackup in Velero API.
// +optional
DefaultVolumesToFsBackup *bool `json:"defaultVolumesToFsBackup,omitempty"`
// Deprecated: Use defaultVolumesToFsBackup instead (matches Velero backup spec).
// +optional
DefaultVolumesToFSBackup *bool `json:"defaultVolumesToFSBackup,omitempty"`
// DisableFsBackup determines whether the NodeAgent should disable file system backup.
Expand Down
5 changes: 5 additions & 0 deletions api/v1alpha1/zz_generated.deepcopy.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 6 additions & 1 deletion bundle/manifests/oadp.openshift.io_dataprotectionapplications.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1181,7 +1181,12 @@ spec:
description: Specify whether CSI snapshot data should be moved to backup storage by default
type: boolean
defaultVolumesToFSBackup:
description: Use pod volume file system backup by default for volumes
description: 'Deprecated: Use defaultVolumesToFsBackup instead (matches Velero backup spec).'
type: boolean
defaultVolumesToFsBackup:
description: |-
Use pod volume file system backup by default for volumes.
Matches backup.spec.defaultVolumesToFsBackup in Velero API.
type: boolean
disableFsBackup:
default: false
Expand Down
7 changes: 6 additions & 1 deletion config/crd/bases/oadp.openshift.io_dataprotectionapplications.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1181,7 +1181,12 @@ spec:
description: Specify whether CSI snapshot data should be moved to backup storage by default
type: boolean
defaultVolumesToFSBackup:
description: Use pod volume file system backup by default for volumes
description: 'Deprecated: Use defaultVolumesToFsBackup instead (matches Velero backup spec).'
type: boolean
defaultVolumesToFsBackup:
description: |-
Use pod volume file system backup by default for volumes.
Matches backup.spec.defaultVolumesToFsBackup in Velero API.
type: boolean
disableFsBackup:
default: false
Expand Down
23 changes: 15 additions & 8 deletions internal/controller/velero.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -411,16 +411,16 @@ func (r *DataProtectionApplicationReconciler) customizeVeleroDeployment(veleroDe
// check for default-snapshot-move-data parameter
defaultSnapshotMoveData := getDefaultSnapshotMoveDataValue(dpa)
// check for default-volumes-to-fs-backup
defaultVolumesToFSBackup := getDefaultVolumesToFSBackup(dpa)
defaultVolumesToFsBackup := getDefaultVolumesToFsBackup(dpa)

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

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

// check for disable-informer-cache flag
Expand Down Expand Up @@ -726,15 +726,22 @@ func getDefaultSnapshotMoveDataValue(dpa *oadpv1alpha1.DataProtectionApplication
return ""
}

func getDefaultVolumesToFSBackup(dpa *oadpv1alpha1.DataProtectionApplication) string {
if dpa.Spec.Configuration.Velero != nil && boolptr.IsSetToTrue(dpa.Spec.Configuration.Velero.DefaultVolumesToFSBackup) {
func getDefaultVolumesToFsBackup(dpa *oadpv1alpha1.DataProtectionApplication) string {
velero := dpa.Spec.Configuration.Velero
if velero == nil {
return ""
}
// Prefer new field (matches Velero backup.spec.defaultVolumesToFsBackup), fall back to deprecated field
val := velero.DefaultVolumesToFsBackup
if val == nil {
val = velero.DefaultVolumesToFSBackup
}
if boolptr.IsSetToTrue(val) {
return TrueVal
}

if dpa.Spec.Configuration.Velero != nil && boolptr.IsSetToFalse(dpa.Spec.Configuration.Velero.DefaultVolumesToFSBackup) {
if boolptr.IsSetToFalse(val) {
return FalseVal
}

return ""
}

Expand Down
49 changes: 47 additions & 2 deletions internal/controller/velero_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1210,7 +1210,7 @@ func TestDPAReconciler_buildVeleroDeployment(t *testing.T) {
DefaultItemOperationTimeout: "2h",
DefaultSnapshotMoveData: ptr.To(false),
NoDefaultBackupLocation: true,
DefaultVolumesToFSBackup: ptr.To(true),
DefaultVolumesToFsBackup: ptr.To(true),
DefaultPlugins: []oadpv1alpha1.DefaultPlugin{oadpv1alpha1.DefaultPluginCSI},
},
},
Expand Down Expand Up @@ -1278,7 +1278,29 @@ func TestDPAReconciler_buildVeleroDeployment(t *testing.T) {
}),
},
{
name: "valid DPA CR with DefaultVolumesToFSBackup true, Velero Deployment is built with DefaultVolumesToFSBackup true",
name: "valid DPA CR with DefaultVolumesToFsBackup true (new field), Velero Deployment is built with default-volumes-to-fs-backup true",
dpa: createTestDpaWith(
nil,
oadpv1alpha1.DataProtectionApplicationSpec{
Configuration: &oadpv1alpha1.ApplicationConfig{
Velero: &oadpv1alpha1.VeleroConfig{
DefaultVolumesToFsBackup: ptr.To(true),
},
},
},
),
veleroDeployment: testVeleroDeployment.DeepCopy(),
wantVeleroDeployment: createTestBuiltVeleroDeployment(TestBuiltVeleroDeploymentOptions{
args: []string{
defaultFileSystemBackupTimeout,
defaultRestoreResourcePriorities,
"--default-volumes-to-fs-backup=true",
defaultDisableInformerCache,
},
}),
},
{
name: "valid DPA CR with DefaultVolumesToFSBackup true (deprecated field, backwards compat), Velero Deployment is built with default-volumes-to-fs-backup true",
dpa: createTestDpaWith(
nil,
oadpv1alpha1.DataProtectionApplicationSpec{
Expand All @@ -1299,6 +1321,29 @@ func TestDPAReconciler_buildVeleroDeployment(t *testing.T) {
},
}),
},
{
name: "valid DPA CR with both DefaultVolumesToFsBackup and DefaultVolumesToFSBackup set, new field takes precedence",
dpa: createTestDpaWith(
nil,
oadpv1alpha1.DataProtectionApplicationSpec{
Configuration: &oadpv1alpha1.ApplicationConfig{
Velero: &oadpv1alpha1.VeleroConfig{
DefaultVolumesToFsBackup: ptr.To(false),
DefaultVolumesToFSBackup: ptr.To(true), // deprecated, should be ignored
},
},
},
),
veleroDeployment: testVeleroDeployment.DeepCopy(),
wantVeleroDeployment: createTestBuiltVeleroDeployment(TestBuiltVeleroDeploymentOptions{
args: []string{
defaultFileSystemBackupTimeout,
defaultRestoreResourcePriorities,
"--default-volumes-to-fs-backup=false",
defaultDisableInformerCache,
},
}),
},
{
name: "valid DPA CR with DisableInformerCache true, Velero Deployment is built with DisableInformerCache true",
dpa: createTestDpaWith(
Expand Down