Merge pull request #7222 from qiuming-best/adjust-bsl-setting-logic

Adjust velero server side default backup location setting logic

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 {

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)

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)
+}

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
 	}

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,
 		},
 		{