Skip to content

Commit

Permalink
OADP-2866: change snapshotMoveData to defaultSnapshotMoveData (#1313) (
Browse files Browse the repository at this point in the history
…#1338)

* change snapshotMoveData to defaultSnapshotMoveData

* get unit tests working

* add test if both default-snapshot-move and default-fs-backup are true

* throw reconcile error if both values are set

* allow both default vars to be set and true in the dpa

* Update controllers/velero.go

fix comment, thanks tiger

Co-authored-by: Tiger Kaovilai <passawit.kaovilai@gmail.com>

* oops fix tests

---------

Co-authored-by: Tiger Kaovilai <passawit.kaovilai@gmail.com>
(cherry picked from commit bb2fb74)

Co-authored-by: Wesley Hayutin <138787+weshayutin@users.noreply.github.com>
  • Loading branch information
shubham-pampattiwar and weshayutin committed Feb 20, 2024
1 parent bbb8d10 commit e91ed09
Show file tree
Hide file tree
Showing 6 changed files with 28 additions and 24 deletions.
2 changes: 1 addition & 1 deletion api/v1alpha1/oadp_types.go
Expand Up @@ -98,7 +98,7 @@ type VeleroConfig struct {
DefaultVolumesToFSBackup *bool `json:"defaultVolumesToFSBackup,omitempty"`
// Specify whether CSI snapshot data should be moved to backup storage by default
// +optional
SnapshotMoveData *bool `json:"snapshotMoveData,omitempty"`
DefaultSnapshotMoveData *bool `json:"defaultSnapshotMoveData,omitempty"`
// Disable informer cache for Get calls on restore. With this enabled, it will speed up restore in cases where there are backup resources which already exist in the cluster, but for very large clusters this will increase velero memory usage. Default is false.
// +optional
DisableInformerCache *bool `json:"disableInformerCache,omitempty"`
Expand Down
4 changes: 2 additions & 2 deletions api/v1alpha1/zz_generated.deepcopy.go

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

Expand Up @@ -647,6 +647,9 @@ spec:
- kubevirt
type: string
type: array
defaultSnapshotMoveData:
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
type: boolean
Expand Down Expand Up @@ -825,9 +828,6 @@ spec:
restoreResourcesVersionPriority:
description: restoreResourceVersionPriority represents a configmap that will be created if defined for use in conjunction with EnableAPIGroupVersions feature flag Defining this field automatically add EnableAPIGroupVersions to the velero server feature flag
type: string
snapshotMoveData:
description: Specify whether CSI snapshot data should be moved to backup storage by default
type: boolean
type: object
type: object
features:
Expand Down
Expand Up @@ -647,6 +647,9 @@ spec:
- kubevirt
type: string
type: array
defaultSnapshotMoveData:
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
type: boolean
Expand Down Expand Up @@ -825,9 +828,6 @@ spec:
restoreResourcesVersionPriority:
description: restoreResourceVersionPriority represents a configmap that will be created if defined for use in conjunction with EnableAPIGroupVersions feature flag Defining this field automatically add EnableAPIGroupVersions to the velero server feature flag
type: string
snapshotMoveData:
description: Specify whether CSI snapshot data should be moved to backup storage by default
type: boolean
type: object
type: object
features:
Expand Down
20 changes: 12 additions & 8 deletions controllers/velero.go
Expand Up @@ -2,12 +2,13 @@ package controllers

import (
"fmt"
"github.com/vmware-tanzu/velero/pkg/util/boolptr"
"os"
"reflect"
"strconv"
"strings"

"github.com/vmware-tanzu/velero/pkg/util/boolptr"

"github.com/google/go-cmp/cmp"
"github.com/openshift/oadp-operator/pkg/credentials"
"github.com/operator-framework/operator-lib/proxy"
Expand Down Expand Up @@ -362,13 +363,16 @@ func (r *DPAReconciler) customizeVeleroDeployment(dpa *oadpv1alpha1.DataProtecti
}

// check for default-snapshot-move-data parameter
snapshotMoveData := getSnapshotMoveDataValue(dpa)
if len(snapshotMoveData) > 0 {
veleroContainer.Args = append(veleroContainer.Args, fmt.Sprintf("--snapshot-move-data=%s", snapshotMoveData))
defaultSnapshotMoveData := getDefaultSnapshotMoveDataValue(dpa)
// check for default-volumes-to-fs-backup
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
defaultVolumesToFSBackup := getDefaultVolumesToFSBackup(dpa)
if len(defaultVolumesToFSBackup) > 0 {
veleroContainer.Args = append(veleroContainer.Args, fmt.Sprintf("--default-volumes-to-fs-backup=%s", defaultVolumesToFSBackup))
}
Expand Down Expand Up @@ -467,12 +471,12 @@ func getFsBackupTimeout(dpa *oadpv1alpha1.DataProtectionApplication) string {
return defaultFsBackupTimeout
}

func getSnapshotMoveDataValue(dpa *oadpv1alpha1.DataProtectionApplication) string {
if dpa.Spec.Configuration.Velero != nil && boolptr.IsSetToTrue(dpa.Spec.Configuration.Velero.SnapshotMoveData) {
func getDefaultSnapshotMoveDataValue(dpa *oadpv1alpha1.DataProtectionApplication) string {
if dpa.Spec.Configuration.Velero != nil && boolptr.IsSetToTrue(dpa.Spec.Configuration.Velero.DefaultSnapshotMoveData) {
return TrueVal
}

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

Expand Down
14 changes: 7 additions & 7 deletions controllers/velero_test.go
Expand Up @@ -828,7 +828,7 @@ func TestDPAReconciler_buildVeleroDeployment(t *testing.T) {
LogLevel: logrus.InfoLevel.String(),
ItemOperationSyncFrequency: "5m",
DefaultItemOperationTimeout: "2h",
SnapshotMoveData: pointer.Bool(false),
DefaultSnapshotMoveData: pointer.Bool(false),
},
},
},
Expand Down Expand Up @@ -910,7 +910,7 @@ func TestDPAReconciler_buildVeleroDeployment(t *testing.T) {
logrus.InfoLevel.String(),
"--item-operation-sync-frequency=5m",
"--default-item-operation-timeout=2h",
"--snapshot-move-data=false",
"--default-snapshot-move-data=false",
defaultDisableInformerCache,
},
VolumeMounts: []corev1.VolumeMount{
Expand Down Expand Up @@ -978,7 +978,7 @@ func TestDPAReconciler_buildVeleroDeployment(t *testing.T) {
},
},
{
name: "given valid DPA CR and Velero Config is defined correctly, SnapshotMovedata is set to true",
name: "given valid DPA CR and Velero Config is defined correctly, defaultSnapshotMovedata is set to true",
veleroDeployment: &appsv1.Deployment{
ObjectMeta: metav1.ObjectMeta{
Name: "test-velero-deployment",
Expand Down Expand Up @@ -1009,7 +1009,7 @@ func TestDPAReconciler_buildVeleroDeployment(t *testing.T) {
LogLevel: logrus.InfoLevel.String(),
ItemOperationSyncFrequency: "5m",
DefaultItemOperationTimeout: "2h",
SnapshotMoveData: pointer.Bool(true),
DefaultSnapshotMoveData: pointer.Bool(true),
},
},
},
Expand Down Expand Up @@ -1091,7 +1091,7 @@ func TestDPAReconciler_buildVeleroDeployment(t *testing.T) {
logrus.InfoLevel.String(),
"--item-operation-sync-frequency=5m",
"--default-item-operation-timeout=2h",
"--snapshot-move-data=true",
"--default-snapshot-move-data=true",
defaultDisableInformerCache,
},
VolumeMounts: []corev1.VolumeMount{
Expand Down Expand Up @@ -2098,7 +2098,7 @@ func TestDPAReconciler_buildVeleroDeployment(t *testing.T) {
ItemOperationSyncFrequency: "5m",
DefaultItemOperationTimeout: "2h",
DefaultVolumesToFSBackup: pointer.Bool(false),
SnapshotMoveData: pointer.Bool(true),
DefaultSnapshotMoveData: pointer.Bool(true),
},
},
},
Expand Down Expand Up @@ -2180,7 +2180,7 @@ func TestDPAReconciler_buildVeleroDeployment(t *testing.T) {
logrus.InfoLevel.String(),
"--item-operation-sync-frequency=5m",
"--default-item-operation-timeout=2h",
"--snapshot-move-data=true",
"--default-snapshot-move-data=true",
"--default-volumes-to-fs-backup=false",
defaultDisableInformerCache,
},
Expand Down

0 comments on commit e91ed09

Please sign in to comment.