Skip to content
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.

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

[1.12] Issue 6647: add default-snapshot-move-data parameter to Velero install #6940

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions changelogs/unreleased/6940-Lyndon-Li
@@ -0,0 +1 @@
Fix issue #6647, add the --default-snapshot-move-data parameter to Velero install, so that users don't need to specify --snapshot-move-data per backup when they want to move snapshot data for all backups
4 changes: 4 additions & 0 deletions pkg/cmd/cli/install/install.go
Expand Up @@ -80,6 +80,7 @@ type Options struct {
Features string
DefaultVolumesToFsBackup bool
UploaderType string
DefaultSnapshotMoveData bool
}

// BindFlags adds command line values to the options struct.
Expand Down Expand Up @@ -120,6 +121,7 @@ func (o *Options) BindFlags(flags *pflag.FlagSet) {
flags.StringVar(&o.Features, "features", o.Features, "Comma separated list of Velero feature flags to be set on the Velero deployment and the node-agent daemonset, if node-agent is enabled")
flags.BoolVar(&o.DefaultVolumesToFsBackup, "default-volumes-to-fs-backup", o.DefaultVolumesToFsBackup, "Bool flag to configure Velero server to use pod volume file system backup by default for all volumes on all backups. Optional.")
flags.StringVar(&o.UploaderType, "uploader-type", o.UploaderType, fmt.Sprintf("The type of uploader to transfer the data of pod volumes, the supported values are '%s', '%s'", uploader.ResticType, uploader.KopiaType))
flags.BoolVar(&o.DefaultSnapshotMoveData, "default-snapshot-move-data", o.DefaultSnapshotMoveData, "Bool flag to configure Velero server to move data by default for all snapshots supporting data movement. Optional.")
}

// NewInstallOptions instantiates a new, default InstallOptions struct.
Expand All @@ -146,6 +148,7 @@ func NewInstallOptions() *Options {
CRDsOnly: false,
DefaultVolumesToFsBackup: false,
UploaderType: uploader.KopiaType,
DefaultSnapshotMoveData: false,
}
}

Expand Down Expand Up @@ -209,6 +212,7 @@ func (o *Options) AsVeleroOptions() (*install.VeleroOptions, error) {
Features: strings.Split(o.Features, ","),
DefaultVolumesToFsBackup: o.DefaultVolumesToFsBackup,
UploaderType: o.UploaderType,
DefaultSnapshotMoveData: o.DefaultSnapshotMoveData,
}, nil
}

Expand Down
4 changes: 4 additions & 0 deletions pkg/cmd/server/server.go
Expand Up @@ -135,6 +135,7 @@
defaultVolumesToFsBackup bool
uploaderType string
maxConcurrentK8SConnections int
defaultSnapshotMoveData bool
}

func NewCommand(f client.Factory) *cobra.Command {
Expand Down Expand Up @@ -163,6 +164,7 @@
defaultVolumesToFsBackup: podvolume.DefaultVolumesToFsBackup,
uploaderType: uploader.ResticType,
maxConcurrentK8SConnections: defaultMaxConcurrentK8SConnections,
defaultSnapshotMoveData: false,
}
)

Expand Down Expand Up @@ -233,6 +235,7 @@
command.Flags().DurationVar(&config.defaultItemOperationTimeout, "default-item-operation-timeout", config.defaultItemOperationTimeout, "How long to wait on asynchronous BackupItemActions and RestoreItemActions to complete before timing out. Default is 4 hours")
command.Flags().DurationVar(&config.resourceTimeout, "resource-timeout", config.resourceTimeout, "How long to wait for resource processes which are not covered by other specific timeout parameters. Default is 10 minutes.")
command.Flags().IntVar(&config.maxConcurrentK8SConnections, "max-concurrent-k8s-connections", config.maxConcurrentK8SConnections, "Max concurrent connections number that Velero can create with kube-apiserver. Default is 30.")
command.Flags().BoolVar(&config.defaultSnapshotMoveData, "default-snapshot-move-data", config.defaultSnapshotMoveData, "Move data by default for all snapshots supporting data movement.")

return command
}
Expand Down Expand Up @@ -757,6 +760,7 @@
s.csiSnapshotClient,
s.credentialFileStore,
s.config.maxConcurrentK8SConnections,
s.config.defaultSnapshotMoveData,

Check warning on line 763 in pkg/cmd/server/server.go

View check run for this annotation

Codecov / codecov/patch

pkg/cmd/server/server.go#L763

Added line #L763 was not covered by tests
).SetupWithManager(s.mgr); err != nil {
s.logger.Fatal(err, "unable to create controller", "controller", controller.Backup)
}
Expand Down
7 changes: 7 additions & 0 deletions pkg/controller/backup_controller.go
Expand Up @@ -88,6 +88,7 @@
volumeSnapshotClient snapshotterClientSet.Interface
credentialFileStore credentials.FileStore
maxConcurrentK8SConnections int
defaultSnapshotMoveData bool
}

func NewBackupReconciler(
Expand All @@ -113,6 +114,7 @@
volumeSnapshotClient snapshotterClientSet.Interface,
credentialStore credentials.FileStore,
maxConcurrentK8SConnections int,
defaultSnapshotMoveData bool,
) *backupReconciler {
b := &backupReconciler{
ctx: ctx,
Expand All @@ -138,6 +140,7 @@
volumeSnapshotClient: volumeSnapshotClient,
credentialFileStore: credentialStore,
maxConcurrentK8SConnections: maxConcurrentK8SConnections,
defaultSnapshotMoveData: defaultSnapshotMoveData,

Check warning on line 143 in pkg/controller/backup_controller.go

View check run for this annotation

Codecov / codecov/patch

pkg/controller/backup_controller.go#L143

Added line #L143 was not covered by tests
}
b.updateTotalBackupMetric()
return b
Expand Down Expand Up @@ -353,6 +356,10 @@
request.Spec.DefaultVolumesToFsBackup = &b.defaultVolumesToFsBackup
}

if request.Spec.SnapshotMoveData == nil {
request.Spec.SnapshotMoveData = &b.defaultSnapshotMoveData
}

// find which storage location to use
var serverSpecified bool
if request.Spec.StorageLocation == "" {
Expand Down
137 changes: 137 additions & 0 deletions pkg/controller/backup_controller_test.go
Expand Up @@ -583,6 +583,7 @@ func TestProcessBackupCompletions(t *testing.T) {
backup *velerov1api.Backup
backupLocation *velerov1api.BackupStorageLocation
defaultVolumesToFsBackup bool
defaultSnapshotMoveData bool
expectedResult *velerov1api.Backup
backupExists bool
existenceCheckError error
Expand Down Expand Up @@ -615,6 +616,7 @@ func TestProcessBackupCompletions(t *testing.T) {
Spec: velerov1api.BackupSpec{
StorageLocation: defaultBackupLocation.Name,
DefaultVolumesToFsBackup: boolptr.True(),
SnapshotMoveData: boolptr.False(),
},
Status: velerov1api.BackupStatus{
Phase: velerov1api.BackupPhaseFinalizing,
Expand Down Expand Up @@ -651,6 +653,7 @@ func TestProcessBackupCompletions(t *testing.T) {
Spec: velerov1api.BackupSpec{
StorageLocation: "alt-loc",
DefaultVolumesToFsBackup: boolptr.False(),
SnapshotMoveData: boolptr.False(),
},
Status: velerov1api.BackupStatus{
Phase: velerov1api.BackupPhaseFinalizing,
Expand Down Expand Up @@ -690,6 +693,7 @@ func TestProcessBackupCompletions(t *testing.T) {
Spec: velerov1api.BackupSpec{
StorageLocation: "read-write",
DefaultVolumesToFsBackup: boolptr.True(),
SnapshotMoveData: boolptr.False(),
},
Status: velerov1api.BackupStatus{
Phase: velerov1api.BackupPhaseFinalizing,
Expand Down Expand Up @@ -727,6 +731,7 @@ func TestProcessBackupCompletions(t *testing.T) {
TTL: metav1.Duration{Duration: 10 * time.Minute},
StorageLocation: defaultBackupLocation.Name,
DefaultVolumesToFsBackup: boolptr.False(),
SnapshotMoveData: boolptr.False(),
},
Status: velerov1api.BackupStatus{
Phase: velerov1api.BackupPhaseFinalizing,
Expand Down Expand Up @@ -764,6 +769,7 @@ func TestProcessBackupCompletions(t *testing.T) {
Spec: velerov1api.BackupSpec{
StorageLocation: defaultBackupLocation.Name,
DefaultVolumesToFsBackup: boolptr.True(),
SnapshotMoveData: boolptr.False(),
},
Status: velerov1api.BackupStatus{
Phase: velerov1api.BackupPhaseFinalizing,
Expand Down Expand Up @@ -802,6 +808,7 @@ func TestProcessBackupCompletions(t *testing.T) {
Spec: velerov1api.BackupSpec{
StorageLocation: defaultBackupLocation.Name,
DefaultVolumesToFsBackup: boolptr.False(),
SnapshotMoveData: boolptr.False(),
},
Status: velerov1api.BackupStatus{
Phase: velerov1api.BackupPhaseFinalizing,
Expand Down Expand Up @@ -840,6 +847,7 @@ func TestProcessBackupCompletions(t *testing.T) {
Spec: velerov1api.BackupSpec{
StorageLocation: defaultBackupLocation.Name,
DefaultVolumesToFsBackup: boolptr.True(),
SnapshotMoveData: boolptr.False(),
},
Status: velerov1api.BackupStatus{
Phase: velerov1api.BackupPhaseFinalizing,
Expand Down Expand Up @@ -878,6 +886,7 @@ func TestProcessBackupCompletions(t *testing.T) {
Spec: velerov1api.BackupSpec{
StorageLocation: defaultBackupLocation.Name,
DefaultVolumesToFsBackup: boolptr.True(),
SnapshotMoveData: boolptr.False(),
},
Status: velerov1api.BackupStatus{
Phase: velerov1api.BackupPhaseFinalizing,
Expand Down Expand Up @@ -916,6 +925,7 @@ func TestProcessBackupCompletions(t *testing.T) {
Spec: velerov1api.BackupSpec{
StorageLocation: defaultBackupLocation.Name,
DefaultVolumesToFsBackup: boolptr.False(),
SnapshotMoveData: boolptr.False(),
},
Status: velerov1api.BackupStatus{
Phase: velerov1api.BackupPhaseFinalizing,
Expand Down Expand Up @@ -955,6 +965,7 @@ func TestProcessBackupCompletions(t *testing.T) {
Spec: velerov1api.BackupSpec{
StorageLocation: defaultBackupLocation.Name,
DefaultVolumesToFsBackup: boolptr.True(),
SnapshotMoveData: boolptr.False(),
},
Status: velerov1api.BackupStatus{
Phase: velerov1api.BackupPhaseFailed,
Expand Down Expand Up @@ -994,6 +1005,7 @@ func TestProcessBackupCompletions(t *testing.T) {
Spec: velerov1api.BackupSpec{
StorageLocation: defaultBackupLocation.Name,
DefaultVolumesToFsBackup: boolptr.True(),
SnapshotMoveData: boolptr.False(),
},
Status: velerov1api.BackupStatus{
Phase: velerov1api.BackupPhaseFailed,
Expand Down Expand Up @@ -1113,6 +1125,89 @@ func TestProcessBackupCompletions(t *testing.T) {
Spec: velerov1api.BackupSpec{
StorageLocation: defaultBackupLocation.Name,
DefaultVolumesToFsBackup: boolptr.False(),
SnapshotMoveData: boolptr.False(),
},
Status: velerov1api.BackupStatus{
Phase: velerov1api.BackupPhaseFinalizing,
Version: 1,
FormatVersion: "1.1.0",
StartTimestamp: &timestamp,
Expiration: &timestamp,
CSIVolumeSnapshotsAttempted: 1,
CSIVolumeSnapshotsCompleted: 0,
},
},
volumeSnapshot: builder.ForVolumeSnapshot("velero", "testVS").ObjectMeta(builder.WithLabels(velerov1api.BackupNameLabel, "backup-1")).Result(),
},
{
name: "backup with snapshot data movement set to true and defaultSnapshotMoveData set to false",
backup: defaultBackup().SnapshotMoveData(true).Result(),
backupLocation: defaultBackupLocation,
defaultVolumesToFsBackup: false,
defaultSnapshotMoveData: false,
expectedResult: &velerov1api.Backup{
TypeMeta: metav1.TypeMeta{
Kind: "Backup",
APIVersion: "velero.io/v1",
},
ObjectMeta: metav1.ObjectMeta{
Namespace: velerov1api.DefaultNamespace,
Name: "backup-1",
Annotations: map[string]string{
"velero.io/source-cluster-k8s-major-version": "1",
"velero.io/source-cluster-k8s-minor-version": "16",
"velero.io/source-cluster-k8s-gitversion": "v1.16.4",
"velero.io/resource-timeout": "0s",
},
Labels: map[string]string{
"velero.io/storage-location": "loc-1",
},
},
Spec: velerov1api.BackupSpec{
StorageLocation: defaultBackupLocation.Name,
DefaultVolumesToFsBackup: boolptr.False(),
SnapshotMoveData: boolptr.True(),
},
Status: velerov1api.BackupStatus{
Phase: velerov1api.BackupPhaseFinalizing,
Version: 1,
FormatVersion: "1.1.0",
StartTimestamp: &timestamp,
Expiration: &timestamp,
CSIVolumeSnapshotsAttempted: 0,
CSIVolumeSnapshotsCompleted: 0,
},
},
volumeSnapshot: builder.ForVolumeSnapshot("velero", "testVS").ObjectMeta(builder.WithLabels(velerov1api.BackupNameLabel, "backup-1")).Result(),
},
{
name: "backup with snapshot data movement set to false and defaultSnapshotMoveData set to true",
backup: defaultBackup().SnapshotMoveData(false).Result(),
backupLocation: defaultBackupLocation,
defaultVolumesToFsBackup: false,
defaultSnapshotMoveData: true,
expectedResult: &velerov1api.Backup{
TypeMeta: metav1.TypeMeta{
Kind: "Backup",
APIVersion: "velero.io/v1",
},
ObjectMeta: metav1.ObjectMeta{
Namespace: velerov1api.DefaultNamespace,
Name: "backup-1",
Annotations: map[string]string{
"velero.io/source-cluster-k8s-major-version": "1",
"velero.io/source-cluster-k8s-minor-version": "16",
"velero.io/source-cluster-k8s-gitversion": "v1.16.4",
"velero.io/resource-timeout": "0s",
},
Labels: map[string]string{
"velero.io/storage-location": "loc-1",
},
},
Spec: velerov1api.BackupSpec{
StorageLocation: defaultBackupLocation.Name,
DefaultVolumesToFsBackup: boolptr.False(),
SnapshotMoveData: boolptr.False(),
},
Status: velerov1api.BackupStatus{
Phase: velerov1api.BackupPhaseFinalizing,
Expand All @@ -1126,6 +1221,47 @@ func TestProcessBackupCompletions(t *testing.T) {
},
volumeSnapshot: builder.ForVolumeSnapshot("velero", "testVS").ObjectMeta(builder.WithLabels(velerov1api.BackupNameLabel, "backup-1")).Result(),
},
{
name: "backup with snapshot data movement not set and defaultSnapshotMoveData set to true",
backup: defaultBackup().Result(),
backupLocation: defaultBackupLocation,
defaultVolumesToFsBackup: false,
defaultSnapshotMoveData: true,
expectedResult: &velerov1api.Backup{
TypeMeta: metav1.TypeMeta{
Kind: "Backup",
APIVersion: "velero.io/v1",
},
ObjectMeta: metav1.ObjectMeta{
Namespace: velerov1api.DefaultNamespace,
Name: "backup-1",
Annotations: map[string]string{
"velero.io/source-cluster-k8s-major-version": "1",
"velero.io/source-cluster-k8s-minor-version": "16",
"velero.io/source-cluster-k8s-gitversion": "v1.16.4",
"velero.io/resource-timeout": "0s",
},
Labels: map[string]string{
"velero.io/storage-location": "loc-1",
},
},
Spec: velerov1api.BackupSpec{
StorageLocation: defaultBackupLocation.Name,
DefaultVolumesToFsBackup: boolptr.False(),
SnapshotMoveData: boolptr.True(),
},
Status: velerov1api.BackupStatus{
Phase: velerov1api.BackupPhaseFinalizing,
Version: 1,
FormatVersion: "1.1.0",
StartTimestamp: &timestamp,
Expiration: &timestamp,
CSIVolumeSnapshotsAttempted: 0,
CSIVolumeSnapshotsCompleted: 0,
},
},
volumeSnapshot: builder.ForVolumeSnapshot("velero", "testVS").ObjectMeta(builder.WithLabels(velerov1api.BackupNameLabel, "backup-1")).Result(),
},
}

for _, test := range tests {
Expand Down Expand Up @@ -1178,6 +1314,7 @@ func TestProcessBackupCompletions(t *testing.T) {
kbClient: fakeClient,
defaultBackupLocation: defaultBackupLocation.Name,
defaultVolumesToFsBackup: test.defaultVolumesToFsBackup,
defaultSnapshotMoveData: test.defaultSnapshotMoveData,
backupTracker: NewBackupTracker(),
metrics: metrics.NewServerMetrics(),
clock: testclocks.NewFakeClock(now),
Expand Down
11 changes: 11 additions & 0 deletions pkg/install/deployment.go
Expand Up @@ -47,6 +47,7 @@
serviceAccountName string
uploaderType string
privilegedNodeAgent bool
defaultSnapshotMoveData bool
}

func WithImage(image string) podTemplateOption {
Expand Down Expand Up @@ -137,6 +138,12 @@
}
}

func WithDefaultSnapshotMoveData() podTemplateOption {
return func(c *podTemplateConfig) {
c.defaultSnapshotMoveData = true
}

Check warning on line 144 in pkg/install/deployment.go

View check run for this annotation

Codecov / codecov/patch

pkg/install/deployment.go#L141-L144

Added lines #L141 - L144 were not covered by tests
}

func WithServiceAccountName(sa string) podTemplateOption {
return func(c *podTemplateConfig) {
c.serviceAccountName = sa
Expand Down Expand Up @@ -174,6 +181,10 @@
args = append(args, "--default-volumes-to-fs-backup=true")
}

if c.defaultSnapshotMoveData {
args = append(args, "--default-snapshot-move-data=true")
}

Check warning on line 186 in pkg/install/deployment.go

View check run for this annotation

Codecov / codecov/patch

pkg/install/deployment.go#L185-L186

Added lines #L185 - L186 were not covered by tests

if len(c.uploaderType) > 0 {
args = append(args, fmt.Sprintf("--uploader-type=%s", c.uploaderType))
}
Expand Down
5 changes: 5 additions & 0 deletions pkg/install/resources.go
Expand Up @@ -252,6 +252,7 @@
Features []string
DefaultVolumesToFsBackup bool
UploaderType string
DefaultSnapshotMoveData bool
}

func AllCRDs() *unstructured.UnstructuredList {
Expand Down Expand Up @@ -352,6 +353,10 @@
deployOpts = append(deployOpts, WithDefaultVolumesToFsBackup())
}

if o.DefaultSnapshotMoveData {
deployOpts = append(deployOpts, WithDefaultSnapshotMoveData())
}

Check warning on line 358 in pkg/install/resources.go

View check run for this annotation

Codecov / codecov/patch

pkg/install/resources.go#L357-L358

Added lines #L357 - L358 were not covered by tests

deploy := Deployment(o.Namespace, deployOpts...)

if err := appendUnstructured(resources, deploy); err != nil {
Expand Down