diff --git a/pkg/cmd/cli/backuplocation/set.go b/pkg/cmd/cli/backuplocation/set.go index bc0dfd1cd9..8aa018e292 100644 --- a/pkg/cmd/cli/backuplocation/set.go +++ b/pkg/cmd/cli/backuplocation/set.go @@ -125,7 +125,13 @@ func (o *SetOptions) Run(c *cobra.Command, f client.Factory) error { return errors.WithStack(err) } if len(defalutBSLs.Items) > 0 { - return errors.New("there is already a default backup storage location") + if len(defalutBSLs.Items) == 1 && defalutBSLs.Items[0].Name == o.Name { + // the default backup storage location is the one we want to set + // so we do not need to do anything + fmt.Printf("Backup storage location %q is already the default backup storage location.\n", o.Name) + return nil + } + return errors.New("there are already exist default backup storage locations, please unset them first") } } } else { diff --git a/pkg/cmd/server/server.go b/pkg/cmd/server/server.go index eb5db9ed9d..8593013736 100644 --- a/pkg/cmd/server/server.go +++ b/pkg/cmd/server/server.go @@ -431,7 +431,9 @@ func (s *server) run() error { return err } - markInProgressCRsFailed(s.ctx, s.mgr.GetConfig(), s.mgr.GetScheme(), s.namespace, s.logger) + if err := s.setupBeforeControllerRun(); err != nil { + return err + } if err := s.runControllers(s.config.defaultVolumeSnapshotLocations); err != nil { return err @@ -440,6 +442,46 @@ func (s *server) run() error { return nil } +// setupBeforeControllerRun do any setup that needs to happen before the controllers are started. +func (s *server) setupBeforeControllerRun() error { + client, err := ctrlclient.New(s.mgr.GetConfig(), ctrlclient.Options{Scheme: s.mgr.GetScheme()}) + // the function is called before starting the controller manager, the embedded client isn't ready to use, so create a new one here + if err != nil { + return errors.WithStack(err) + } + + markInProgressCRsFailed(s.ctx, client, s.namespace, s.logger) + + if err := setDefaultBackupLocation(s.ctx, client, s.namespace, s.config.defaultBackupLocation, s.logger); err != nil { + return err + } + return nil +} + +// setDefaultBackupLocation set the BSL that matches the "velero server --default-backup-storage-location" +func setDefaultBackupLocation(ctx context.Context, client ctrlclient.Client, namespace, defaultBackupLocation string, logger logrus.FieldLogger) error { + if defaultBackupLocation == "" { + logger.Debug("No default backup storage location specified. Velero will not automatically select a backup storage location for new backups.") + return nil + } + + backupLocation := &velerov1api.BackupStorageLocation{} + if err := client.Get(ctx, types.NamespacedName{Namespace: namespace, Name: defaultBackupLocation}, backupLocation); err != nil { + return errors.WithStack(err) + } + + if !backupLocation.Spec.Default { + backupLocation.Spec.Default = true + if err := client.Update(ctx, backupLocation); err != nil { + return errors.WithStack(err) + } + + logger.WithField("backupStorageLocation", defaultBackupLocation).Info("Set backup storage location as default") + } + + return nil +} + // namespaceExists returns nil if namespace can be successfully // gotten from the Kubernetes API, or an error otherwise. func (s *server) namespaceExists(namespace string) error { @@ -979,14 +1021,7 @@ func (s *server) runProfiler() { // if there is a restarting during the reconciling of backups/restores/etc, these CRs may be stuck in progress status // markInProgressCRsFailed tries to mark the in progress CRs as failed when starting the server to avoid the issue -func markInProgressCRsFailed(ctx context.Context, cfg *rest.Config, scheme *runtime.Scheme, namespace string, log logrus.FieldLogger) { - // the function is called before starting the controller manager, the embedded client isn't ready to use, so create a new one here - client, err := ctrlclient.New(cfg, ctrlclient.Options{Scheme: scheme}) - if err != nil { - log.WithError(errors.WithStack(err)).Error("failed to create client") - return - } - +func markInProgressCRsFailed(ctx context.Context, client ctrlclient.Client, namespace string, log logrus.FieldLogger) { markInProgressBackupsFailed(ctx, client, namespace, log) markInProgressRestoresFailed(ctx, client, namespace, log) diff --git a/pkg/cmd/server/server_test.go b/pkg/cmd/server/server_test.go index fc487d3d1c..e8c8c9b23c 100644 --- a/pkg/cmd/server/server_test.go +++ b/pkg/cmd/server/server_test.go @@ -376,3 +376,37 @@ func Test_markInProgressRestoresFailed(t *testing.T) { require.Nil(t, c.Get(context.Background(), client.ObjectKey{Namespace: "velero", Name: "restore02"}, restore02)) assert.Equal(t, velerov1api.RestorePhaseCompleted, restore02.Status.Phase) } + +func Test_setDefaultBackupLocation(t *testing.T) { + scheme := runtime.NewScheme() + velerov1api.AddToScheme(scheme) + + c := fake.NewClientBuilder(). + WithScheme(scheme). + WithLists(&velerov1api.BackupStorageLocationList{ + Items: []velerov1api.BackupStorageLocation{ + { + ObjectMeta: metav1.ObjectMeta{ + Namespace: "velero", + Name: "default", + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Namespace: "velero", + Name: "non-default", + }, + }, + }, + }). + Build() + setDefaultBackupLocation(context.Background(), c, "velero", "default", logrus.New()) + + defaultLocation := &velerov1api.BackupStorageLocation{} + require.Nil(t, c.Get(context.Background(), client.ObjectKey{Namespace: "velero", Name: "default"}, defaultLocation)) + assert.True(t, defaultLocation.Spec.Default) + + nonDefaultLocation := &velerov1api.BackupStorageLocation{} + require.Nil(t, c.Get(context.Background(), client.ObjectKey{Namespace: "velero", Name: "non-default"}, nonDefaultLocation)) + assert.False(t, nonDefaultLocation.Spec.Default) +} diff --git a/pkg/controller/backup_storage_location_controller.go b/pkg/controller/backup_storage_location_controller.go index d2899a4b1f..b12f92e9de 100644 --- a/pkg/controller/backup_storage_location_controller.go +++ b/pkg/controller/backup_storage_location_controller.go @@ -250,26 +250,7 @@ func (r *backupStorageLocationReconciler) ensureSingleDefaultBSL(locationList ve } } } else if len(defaultBSLs) == 0 { // no default BSL - // find the BSL that matches the "velero server --default-backup-storage-location" - var defaultBSL *velerov1api.BackupStorageLocation - for i, location := range locationList.Items { - if location.Name == r.defaultBackupLocationInfo.StorageLocation { - defaultBSL = &locationList.Items[i] - break - } - } - - // set the default BSL - if defaultBSL != nil { - defaultBSL.Spec.Default = true - defaultFound = true - if err := r.client.Update(r.ctx, defaultBSL); err != nil { - return defaultFound, errors.Wrapf(err, "failed to set default backup storage location %q", defaultBSL.Name) - } - r.log.Debugf("update default backup storage location %q to true", defaultBSL.Name) - } else { - defaultFound = false - } + defaultFound = false } else { // only 1 default BSL defaultFound = true } diff --git a/pkg/controller/backup_storage_location_controller_test.go b/pkg/controller/backup_storage_location_controller_test.go index f7a6f27f91..5742c36887 100644 --- a/pkg/controller/backup_storage_location_controller_test.go +++ b/pkg/controller/backup_storage_location_controller_test.go @@ -51,7 +51,7 @@ var _ = Describe("Backup Storage Location Reconciler", func() { expectedPhase velerov1api.BackupStorageLocationPhase }{ { - backupLocation: builder.ForBackupStorageLocation("ns-1", "location-1").ValidationFrequency(1 * time.Second).Result(), + backupLocation: builder.ForBackupStorageLocation("ns-1", "location-1").ValidationFrequency(1 * time.Second).Default(true).Result(), isValidError: nil, expectedIsDefault: true, expectedPhase: velerov1api.BackupStorageLocationPhaseAvailable, @@ -206,7 +206,7 @@ func TestEnsureSingleDefaultBSL(t *testing.T) { defaultBackupInfo: storage.DefaultBackupLocationInfo{ StorageLocation: "location-2", }, - expectedDefaultSet: true, + expectedDefaultSet: false, expectedError: nil, }, {