feat: support configure BSL CR to indicate which one is the default (#3092)

* Add default field to BSL CRD

Signed-off-by: JenTing Hsiao <jenting.hsiao@suse.com>

* Add a new flag `--default` under `velero backup-location create`

add a new flag `--default` under `velero backup-location create`
to specify this new location to be the new default BSL.

Signed-off-by: JenTing Hsiao <jenting.hsiao@suse.com>

* Add a new default field under `velero backup-location get`

add a new default field under `velero backup-location get` to indicate
which BSL is the default one.

Signed-off-by: JenTing Hsiao <jenting.hsiao@suse.com>

* Add a new sub-command and flag under `velero backup-location`

Add a new sub-command called `velero backup-location set` sub-command
and a new flag `velero backup-cation set --default` to configure which
BSL is the default one.

Signed-off-by: JenTing Hsiao <jenting.hsiao@suse.com>

* Add new flag to get the default backup-location

Add a new flag `--default` under `velero backup-location get`
to displays the current default BSL.

Signed-off-by: JenTing Hsiao <jenting.hsiao@suse.com>

* Configures default BSL in BSL controller

When upgrade the BSL CRDs, none of the BSL has been labeled as default.
Sets the BSL default field to true if the BSL name matches to the default BSL setting.

Signed-off-by: JenTing Hsiao <jenting.hsiao@suse.com>

* Configures the default BSL in BSL controller for velero upgrade

When upgrade the BSL CRDs, none of the BSL be marked as the default.
Sets the BSL `.spec.default: true` if the BSL name matches against the
`velero server --default-backup-storage-location`.

Signed-off-by: JenTing Hsiao <jenting.hsiao@suse.com>

* Add unit test to test default BSL behavior

Signed-off-by: JenTing Hsiao <jenting.hsiao@suse.com>

* Update check which one is the default BSL in backup/backup_sync/restore controller

Signed-off-by: JenTing Hsiao <jenting.hsiao@suse.com>

* Add changelog

Signed-off-by: JenTing Hsiao <jenting.hsiao@suse.com>

* Update docs locations.md and upgrade-to-1.6.md

Signed-off-by: JenTing Hsiao <jenting.hsiao@suse.com>

pkg/controller/backup_controller.go

@@ -41,6 +41,7 @@ import (
 	snapshotv1beta1api "github.com/kubernetes-csi/external-snapshotter/v2/pkg/apis/volumesnapshot/v1beta1"
 	snapshotv1beta1listers "github.com/kubernetes-csi/external-snapshotter/v2/pkg/client/listers/volumesnapshot/v1beta1"
 
+	"github.com/vmware-tanzu/velero/internal/storage"
 	velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1"
 	pkgbackup "github.com/vmware-tanzu/velero/pkg/backup"
 	"github.com/vmware-tanzu/velero/pkg/discovery"
@@ -345,6 +346,16 @@ func (c *backupController) prepareBackupRequest(backup *velerov1api.Backup) *pkg
 	// default storage location if not specified
 	if request.Spec.StorageLocation == "" {
 		request.Spec.StorageLocation = c.defaultBackupLocation
+
+		locationList, err := storage.ListBackupStorageLocations(context.Background(), c.kbClient, request.Namespace)
+		if err == nil {
+			for _, location := range locationList.Items {
+				if location.Spec.Default {
+					request.Spec.StorageLocation = location.Name
+					break
+				}
+			}
+		}
 	}
 
 	if request.Spec.DefaultVolumesToRestic == nil {

pkg/controller/backup_controller_test.go

@@ -336,7 +336,7 @@ func TestDefaultBackupTTL(t *testing.T) {
 }
 
 func TestProcessBackupCompletions(t *testing.T) {
-	defaultBackupLocation := builder.ForBackupStorageLocation("velero", "loc-1").Bucket("store-1").Result()
+	defaultBackupLocation := builder.ForBackupStorageLocation("velero", "loc-1").Default(true).Bucket("store-1").Result()
 
 	now, err := time.Parse(time.RFC1123Z, time.RFC1123Z)
 	require.NoError(t, err)

pkg/controller/backup_storage_location_controller.go

@@ -67,13 +67,25 @@ func (r *BackupStorageLocationReconciler) Reconcile(req ctrl.Request) (ctrl.Resu
 	defer pluginManager.CleanupClients()
 
 	var defaultFound bool
+	for _, location := range locationList.Items {
+		if location.Spec.Default {
+			defaultFound = true
+			break
+		}
+	}
+
 	var unavailableErrors []string
 	var anyVerified bool
 	for i := range locationList.Items {
 		location := &locationList.Items[i]
+		isDefault := location.Spec.Default
 		log := r.Log.WithField("controller", BackupStorageLocation).WithField(BackupStorageLocation, location.Name)
 
-		if location.Name == r.DefaultBackupLocationInfo.StorageLocation {
+		if !defaultFound && location.Name == r.DefaultBackupLocationInfo.StorageLocation {
+			// For backward-compatible, to configure the backup storage location as the default if
+			// none of the BSLs be marked as the default and the BSL name matches against the
+			// "velero server --default-backup-storage-location".
+			isDefault = true
 			defaultFound = true
 		}
 
@@ -95,29 +107,28 @@ func (r *BackupStorageLocationReconciler) Reconcile(req ctrl.Request) (ctrl.Resu
 			continue
 		}
 
+		// updates the default backup location
+		location.Spec.Default = isDefault
+
 		log.Info("Validating backup storage location")
 		anyVerified = true
 		if err := backupStore.IsValid(); err != nil {
-			log.Info("Backup location is invalid, marking as unavailable")
-			unavailableErrors = append(unavailableErrors, errors.Wrapf(err, "Backup location %q is unavailable", location.Name).Error())
-
-			if location.Name == r.DefaultBackupLocationInfo.StorageLocation {
-				log.Warnf("The specified default backup location named %q is unavailable; for convenience, be sure to configure it properly or make another backup location that is available the default", r.DefaultBackupLocationInfo.StorageLocation)
-			}
-
+			log.Info("Backup storage location is invalid, marking as unavailable")
+			unavailableErrors = append(unavailableErrors, errors.Wrapf(err, "Backup storage location %q is unavailable", location.Name).Error())
 			location.Status.Phase = velerov1api.BackupStorageLocationPhaseUnavailable
 		} else {
-			log.Info("Backup location valid, marking as available")
+			log.Info("Backup storage location valid, marking as available")
 			location.Status.Phase = velerov1api.BackupStorageLocationPhaseAvailable
 		}
 		location.Status.LastValidationTime = &metav1.Time{Time: time.Now().UTC()}
+
 		if err := patchHelper.Patch(r.Ctx, location); err != nil {
-			log.WithError(err).Error("Error updating backup location phase")
+			log.WithError(err).Error("Error updating backup storage location phase")
 		}
 	}
 
 	if !anyVerified {
-		log.Debug("No backup locations needed to be validated")
+		log.Debug("No backup storage locations needed to be validated")
 	}
 
 	r.logReconciledPhase(defaultFound, locationList, unavailableErrors)
@@ -154,11 +165,11 @@ func (r *BackupStorageLocationReconciler) logReconciledPhase(defaultFound bool,
 			log.Errorf("Current backup storage locations available/unavailable/unknown: %v/%v/%v)", numAvailable, numUnavailable, numUnknown)
 		}
 	} else if numUnavailable > 0 { // some but not all BSL unavailable
-		log.Warnf("Invalid backup locations detected: available/unavailable/unknown: %v/%v/%v, %s)", numAvailable, numUnavailable, numUnknown, strings.Join(errs, "; "))
+		log.Warnf("Invalid backup storage locations detected: available/unavailable/unknown: %v/%v/%v, %s)", numAvailable, numUnavailable, numUnknown, strings.Join(errs, "; "))
 	}
 
 	if !defaultFound {
-		log.Warnf("The specified default backup location named %q was not found; for convenience, be sure to create one or make another backup location that is available the default", r.DefaultBackupLocationInfo.StorageLocation)
+		log.Warn("The default backup storage location was not found; for convenience, be sure to create one or make another backup location that is designated as the default")
 	}
 }
 

pkg/controller/backup_storage_location_controller_test.go

@@ -46,19 +46,91 @@ var _ = Describe("Backup Storage Location Reconciler", func() {
 
 	It("Should successfully patch a backup storage location object status phase according to whether its storage is valid or not", func() {
 		tests := []struct {
-			backupLocation *velerov1api.BackupStorageLocation
-			isValidError   error
-			expectedPhase  velerov1api.BackupStorageLocationPhase
+			backupLocation    *velerov1api.BackupStorageLocation
+			isValidError      error
+			expectedIsDefault bool
+			expectedPhase     velerov1api.BackupStorageLocationPhase
 		}{
 			{
-				backupLocation: builder.ForBackupStorageLocation("ns-1", "location-1").ValidationFrequency(1 * time.Second).Result(),
-				isValidError:   nil,
-				expectedPhase:  velerov1api.BackupStorageLocationPhaseAvailable,
+				backupLocation:    builder.ForBackupStorageLocation("ns-1", "location-1").ValidationFrequency(1 * time.Second).Result(),
+				isValidError:      nil,
+				expectedIsDefault: true,
+				expectedPhase:     velerov1api.BackupStorageLocationPhaseAvailable,
 			},
 			{
-				backupLocation: builder.ForBackupStorageLocation("ns-1", "location-2").ValidationFrequency(1 * time.Second).Result(),
-				isValidError:   errors.New("an error"),
-				expectedPhase:  velerov1api.BackupStorageLocationPhaseUnavailable,
+				backupLocation:    builder.ForBackupStorageLocation("ns-1", "location-2").ValidationFrequency(1 * time.Second).Result(),
+				isValidError:      errors.New("an error"),
+				expectedIsDefault: false,
+				expectedPhase:     velerov1api.BackupStorageLocationPhaseUnavailable,
+			},
+		}
+
+		// Setup
+		var (
+			pluginManager = &pluginmocks.Manager{}
+			backupStores  = make(map[string]*persistencemocks.BackupStore)
+		)
+		pluginManager.On("CleanupClients").Return(nil)
+
+		locations := new(velerov1api.BackupStorageLocationList)
+		for i, test := range tests {
+			location := test.backupLocation
+			locations.Items = append(locations.Items, *location)
+			backupStores[location.Name] = &persistencemocks.BackupStore{}
+			backupStore := backupStores[location.Name]
+			backupStore.On("IsValid").Return(tests[i].isValidError)
+		}
+
+		// Setup reconciler
+		Expect(velerov1api.AddToScheme(scheme.Scheme)).To(Succeed())
+		r := BackupStorageLocationReconciler{
+			Ctx:    ctx,
+			Client: fake.NewFakeClientWithScheme(scheme.Scheme, locations),
+			DefaultBackupLocationInfo: storage.DefaultBackupLocationInfo{
+				StorageLocation:          "location-1",
+				StoreValidationFrequency: 0,
+			},
+			NewPluginManager: func(logrus.FieldLogger) clientmgmt.Manager { return pluginManager },
+			NewBackupStore: func(loc *velerov1api.BackupStorageLocation, _ persistence.ObjectStoreGetter, _ logrus.FieldLogger) (persistence.BackupStore, error) {
+				// this gets populated just below, prior to exercising the method under test
+				return backupStores[loc.Name], nil
+			},
+			Log: velerotest.NewLogger(),
+		}
+
+		actualResult, err := r.Reconcile(ctrl.Request{
+			NamespacedName: types.NamespacedName{Namespace: "ns-1"},
+		})
+
+		Expect(actualResult).To(BeEquivalentTo(ctrl.Result{Requeue: true}))
+		Expect(err).To(BeNil())
+
+		// Assertions
+		for i, location := range locations.Items {
+			key := client.ObjectKey{Name: location.Name, Namespace: location.Namespace}
+			instance := &velerov1api.BackupStorageLocation{}
+			err := r.Client.Get(ctx, key, instance)
+			Expect(err).To(BeNil())
+			Expect(instance.Spec.Default).To(BeIdenticalTo(tests[i].expectedIsDefault))
+			Expect(instance.Status.Phase).To(BeIdenticalTo(tests[i].expectedPhase))
+		}
+	})
+
+	It("Should successfully patch a backup storage location object spec default if the BSL is the default one", func() {
+		tests := []struct {
+			backupLocation    *velerov1api.BackupStorageLocation
+			isValidError      error
+			expectedIsDefault bool
+		}{
+			{
+				backupLocation:    builder.ForBackupStorageLocation("ns-1", "location-1").ValidationFrequency(1 * time.Second).Default(false).Result(),
+				isValidError:      nil,
+				expectedIsDefault: false,
+			},
+			{
+				backupLocation:    builder.ForBackupStorageLocation("ns-1", "location-2").ValidationFrequency(1 * time.Second).Default(true).Result(),
+				isValidError:      nil,
+				expectedIsDefault: true,
 			},
 		}
 
@@ -108,28 +180,31 @@ var _ = Describe("Backup Storage Location Reconciler", func() {
 			instance := &velerov1api.BackupStorageLocation{}
 			err := r.Client.Get(ctx, key, instance)
 			Expect(err).To(BeNil())
-			Expect(instance.Status.Phase).To(BeIdenticalTo(tests[i].expectedPhase))
+			Expect(instance.Spec.Default).To(BeIdenticalTo(tests[i].expectedIsDefault))
 		}
 	})
 
 	It("Should not patch a backup storage location object status phase if the location's validation frequency is specifically set to zero", func() {
 		tests := []struct {
-			backupLocation *velerov1api.BackupStorageLocation
-			isValidError   error
-			expectedPhase  velerov1api.BackupStorageLocationPhase
-			wantErr        bool
+			backupLocation    *velerov1api.BackupStorageLocation
+			isValidError      error
+			expectedIsDefault bool
+			expectedPhase     velerov1api.BackupStorageLocationPhase
+			wantErr           bool
 		}{
 			{
-				backupLocation: builder.ForBackupStorageLocation("ns-1", "location-1").ValidationFrequency(0).LastValidationTime(time.Now()).Result(),
-				isValidError:   nil,
-				expectedPhase:  "",
-				wantErr:        false,
+				backupLocation:    builder.ForBackupStorageLocation("ns-1", "location-1").ValidationFrequency(0).LastValidationTime(time.Now()).Result(),
+				isValidError:      nil,
+				expectedIsDefault: false,
+				expectedPhase:     "",
+				wantErr:           false,
 			},
 			{
-				backupLocation: builder.ForBackupStorageLocation("ns-1", "location-2").ValidationFrequency(0).LastValidationTime(time.Now()).Result(),
-				isValidError:   nil,
-				expectedPhase:  "",
-				wantErr:        false,
+				backupLocation:    builder.ForBackupStorageLocation("ns-1", "location-2").ValidationFrequency(0).LastValidationTime(time.Now()).Result(),
+				isValidError:      nil,
+				expectedIsDefault: false,
+				expectedPhase:     "",
+				wantErr:           false,
 			},
 		}
 
@@ -178,6 +253,7 @@ var _ = Describe("Backup Storage Location Reconciler", func() {
 			instance := &velerov1api.BackupStorageLocation{}
 			err := r.Client.Get(ctx, key, instance)
 			Expect(err).To(BeNil())
+			Expect(instance.Spec.Default).To(BeIdenticalTo(tests[i].expectedIsDefault))
 			Expect(instance.Status.Phase).To(BeIdenticalTo(tests[i].expectedPhase))
 		}
 	})

pkg/controller/backup_sync_controller.go

@@ -1,5 +1,5 @@
 /*
-Copyright 2017, 2020 the Velero contributors.
+Copyright 2020 the Velero contributors.
 
 Licensed under the Apache License, Version 2.0 (the "License");
 you may not use this file except in compliance with the License.
@@ -129,7 +129,13 @@ func (c *backupSyncController) run() {
 		return
 	}
 
-	// sync the default location first, if it exists
+	// sync the default backup storage location first, if it exists
+	for _, location := range locationList.Items {
+		if location.Spec.Default {
+			c.defaultBackupLocation = location.Name
+			break
+		}
+	}
 	locations := orderedBackupLocations(&locationList, c.defaultBackupLocation)
 
 	pluginManager := c.newPluginManager(c.logger)

pkg/controller/backup_sync_controller_test.go

@@ -55,6 +55,7 @@ func defaultLocationsList(namespace string) []*velerov1api.BackupStorageLocation
 						Bucket: "bucket-1",
 					},
 				},
+				Default: true,
 			},
 		},
 		{

pkg/controller/restore_controller.go

@@ -1,5 +1,5 @@
 /*
-Copyright 2017, 2019 the Velero contributors.
+Copyright 2020 the Velero contributors.
 
 Licensed under the Apache License, Version 2.0 (the "License");
 you may not use this file except in compliance with the License.
@@ -109,7 +109,6 @@ func NewRestoreController(
 	logger logrus.FieldLogger,
 	restoreLogLevel logrus.Level,
 	newPluginManager func(logrus.FieldLogger) clientmgmt.Manager,
-	defaultBackupLocation string,
 	metrics *metrics.ServerMetrics,
 	logFormat logging.Format,
 ) Interface {
@@ -124,7 +123,6 @@ func NewRestoreController(
 		kbClient:               kbClient,
 		snapshotLocationLister: snapshotLocationLister,
 		restoreLogLevel:        restoreLogLevel,
-		defaultBackupLocation:  defaultBackupLocation,
 		metrics:                metrics,
 		logFormat:              logFormat,
 		clock:                  &clock.RealClock{},

pkg/controller/restore_controller_test.go

@@ -1,5 +1,5 @@
 /*
-Copyright 2017, 2019 the Velero contributors.
+Copyright 2020 the Velero contributors.
 
 Licensed under the Apache License, Version 2.0 (the "License");
 you may not use this file except in compliance with the License.
@@ -117,7 +117,6 @@ func TestFetchBackupInfo(t *testing.T) {
 				logger,
 				logrus.InfoLevel,
 				func(logrus.FieldLogger) clientmgmt.Manager { return pluginManager },
-				"default",
 				metrics.NewServerMetrics(),
 				formatFlag,
 			).(*restoreController)
@@ -213,7 +212,6 @@ func TestProcessQueueItemSkips(t *testing.T) {
 				logger,
 				logrus.InfoLevel,
 				nil,
-				"default",
 				metrics.NewServerMetrics(),
 				formatFlag,
 			).(*restoreController)
@@ -440,7 +438,6 @@ func TestProcessQueueItem(t *testing.T) {
 				logger,
 				logrus.InfoLevel,
 				func(logrus.FieldLogger) clientmgmt.Manager { return pluginManager },
-				"default",
 				metrics.NewServerMetrics(),
 				formatFlag,
 			).(*restoreController)
@@ -673,7 +670,6 @@ func TestvalidateAndCompleteWhenScheduleNameSpecified(t *testing.T) {
 		logger,
 		logrus.DebugLevel,
 		nil,
-		"default",
 		nil,
 		formatFlag,
 	).(*restoreController)