Add a BSL controller to handle validation + update BSL status phase

changelogs/unreleased/2490-carlisia

@@ -0,0 +1 @@
+No longer block backups if there's even 1 only 1 invalid BSL: only log and set BSL status accordingly.

pkg/apis/velero/v1/backup_storage_location.go

@@ -89,10 +89,16 @@ type BackupStorageLocationSpec struct {
 	// +optional
 	// +nullable
 	BackupSyncPeriod *metav1.Duration `json:"backupSyncPeriod,omitempty"`
+
+	// StoreValidationFrequency defines how frequently to validate the corresponding object storage. A value of 0 disables validation.
+	// +optional
+	// +nullable
+	ValidationFrequency *metav1.Duration `json:"validationFrequency,omitempty"`
 }
 
 // BackupStorageLocationPhase is the lifecyle phase of a Velero BackupStorageLocation.
-// +kubebuilder:validation:Enum=Available;Unavailable
+// +kubebuilder:validation:Enum=Available;Unavailable;Unverified
+// +kubebuilder:default=Unverified
 type BackupStorageLocationPhase string
 
 const (
@@ -101,6 +107,9 @@ const (
 
 	// BackupStorageLocationPhaseUnavailable means the location is unavailable to read and write from.
 	BackupStorageLocationPhaseUnavailable BackupStorageLocationPhase = "Unavailable"
+
+	// BackupStorageLocationPhaseUnverified means the location was unverifiable if available to read and write from.
+	BackupStorageLocationPhaseUnverified BackupStorageLocationPhase = "Unverified"
 )
 
 // BackupStorageLocationAccessMode represents the permissions for a BackupStorageLocation.
@@ -124,6 +133,12 @@ type BackupStorageLocationStatus struct {
 	// +optional
 	Phase BackupStorageLocationPhase `json:"phase,omitempty"`
 
+	// LastValidationTime is the last time the backup store location was validated
+	// the cluster.
+	// +optional
+	// +nullable
+	LastValidationTime *metav1.Time `json:"lastValidationTime,omitempty"`
+
 	// LastSyncedTime is the last time the contents of the location were synced into
 	// the cluster.
 	// +optional

pkg/cmd/cli/backuplocation/create.go

@@ -55,14 +55,15 @@ func NewCreateCommand(f client.Factory, use string) *cobra.Command {
 }
 
 type CreateOptions struct {
-	Name             string
-	Provider         string
-	Bucket           string
-	Prefix           string
-	BackupSyncPeriod time.Duration
-	Config           flag.Map
-	Labels           flag.Map
-	AccessMode       *flag.Enum
+	Name                string
+	Provider            string
+	Bucket              string
+	Prefix              string
+	BackupSyncPeriod    time.Duration
+	ValidationFrequency time.Duration
+	Config              flag.Map
+	Labels              flag.Map
+	AccessMode          *flag.Enum
 }
 
 func NewCreateOptions() *CreateOptions {
@@ -81,6 +82,7 @@ func (o *CreateOptions) BindFlags(flags *pflag.FlagSet) {
 	flags.StringVar(&o.Bucket, "bucket", o.Bucket, "name of the object storage bucket where backups should be stored")
 	flags.StringVar(&o.Prefix, "prefix", o.Prefix, "prefix under which all Velero data should be stored within the bucket. Optional.")
 	flags.DurationVar(&o.BackupSyncPeriod, "backup-sync-period", o.BackupSyncPeriod, "how often to ensure all Velero backups in object storage exist as Backup API objects in the cluster. Optional. Set this to `0s` to disable sync")
+	flags.DurationVar(&o.ValidationFrequency, "storage-validation-period", o.ValidationFrequency, "how often to validate the backup storage location. Optional. Set this to `0s` to disable validation")
 	flags.Var(&o.Config, "config", "configuration key-value pairs")
 	flags.Var(&o.Labels, "labels", "labels to apply to the backup storage location")
 	flags.Var(
@@ -107,6 +109,10 @@ func (o *CreateOptions) Validate(c *cobra.Command, args []string, f client.Facto
 		return errors.New("--backup-sync-period must be non-negative")
 	}
 
+	if o.ValidationFrequency < 0 {
+		return errors.New("--storage-validation-period must be non-negative")
+	}
+
 	return nil
 }
 
@@ -116,12 +122,16 @@ func (o *CreateOptions) Complete(args []string, f client.Factory) error {
 }
 
 func (o *CreateOptions) Run(c *cobra.Command, f client.Factory) error {
-	var backupSyncPeriod *metav1.Duration
+	var backupSyncPeriod, storageValidationPeriod *metav1.Duration
 
 	if c.Flags().Changed("backup-sync-period") {
 		backupSyncPeriod = &metav1.Duration{Duration: o.BackupSyncPeriod}
 	}
 
+	if c.Flags().Changed("storage-validation-period") {
+		storageValidationPeriod = &metav1.Duration{Duration: o.ValidationFrequency}
+	}
+
 	backupStorageLocation := &velerov1api.BackupStorageLocation{
 		ObjectMeta: metav1.ObjectMeta{
 			Namespace: f.Namespace(),
@@ -136,9 +146,10 @@ func (o *CreateOptions) Run(c *cobra.Command, f client.Factory) error {
 					Prefix: o.Prefix,
 				},
 			},
-			Config:           o.Config.Data(),
-			AccessMode:       velerov1api.BackupStorageLocationAccessMode(o.AccessMode.String()),
-			BackupSyncPeriod: backupSyncPeriod,
+			Config:              o.Config.Data(),
+			AccessMode:          velerov1api.BackupStorageLocationAccessMode(o.AccessMode.String()),
+			BackupSyncPeriod:    backupSyncPeriod,
+			ValidationFrequency: storageValidationPeriod,
 		},
 	}
 

pkg/cmd/server/server.go

@@ -63,7 +63,6 @@ import (
 	clientset "github.com/vmware-tanzu/velero/pkg/generated/clientset/versioned"
 	informers "github.com/vmware-tanzu/velero/pkg/generated/informers/externalversions"
 	"github.com/vmware-tanzu/velero/pkg/metrics"
-	"github.com/vmware-tanzu/velero/pkg/persistence"
 	"github.com/vmware-tanzu/velero/pkg/plugin/clientmgmt"
 	"github.com/vmware-tanzu/velero/pkg/podexec"
 	"github.com/vmware-tanzu/velero/pkg/restic"
@@ -76,6 +75,7 @@ const (
 	defaultMetricsAddress = ":8085"
 
 	defaultBackupSyncPeriod           = time.Minute
+	defaultStoreValidationPeriod      = time.Minute
 	defaultPodVolumeOperationTimeout  = 60 * time.Minute
 	defaultResourceTerminatingTimeout = 10 * time.Minute
 
@@ -86,15 +86,16 @@ const (
 	defaultProfilerAddress = "localhost:6060"
 
 	// keys used to map out available controllers with disable-controllers flag
-	BackupControllerKey              = "backup"
-	BackupSyncControllerKey          = "backup-sync"
-	ScheduleControllerKey            = "schedule"
-	GcControllerKey                  = "gc"
-	BackupDeletionControllerKey      = "backup-deletion"
-	RestoreControllerKey             = "restore"
-	DownloadRequestControllerKey     = "download-request"
-	ResticRepoControllerKey          = "restic-repo"
-	ServerStatusRequestControllerKey = "server-status-request"
+	BackupControllerKey                = "backup"
+	BackupSyncControllerKey            = "backup-sync"
+	ScheduleControllerKey              = "schedule"
+	GcControllerKey                    = "gc"
+	BackupDeletionControllerKey        = "backup-deletion"
+	RestoreControllerKey               = "restore"
+	DownloadRequestControllerKey       = "download-request"
+	ResticRepoControllerKey            = "restic-repo"
+	ServerStatusRequestControllerKey   = "server-status-request"
+	BackupStorageLocationControllerKey = "backup-storage-location"
 
 	defaultControllerWorkers = 1
 	// the default TTL for a backup
@@ -112,12 +113,13 @@ var disableControllerList = []string{
 	DownloadRequestControllerKey,
 	ResticRepoControllerKey,
 	ServerStatusRequestControllerKey,
+	BackupStorageLocationControllerKey,
 }
 
 type serverConfig struct {
 	pluginDir, metricsAddress, defaultBackupLocation                        string
 	backupSyncPeriod, podVolumeOperationTimeout, resourceTerminatingTimeout time.Duration
-	defaultBackupTTL                                                        time.Duration
+	defaultBackupTTL, defaultStoreValidationPeriod                          time.Duration
 	restoreResourcePriorities                                               []string
 	defaultVolumeSnapshotLocations                                          map[string]string
 	restoreOnly                                                             bool
@@ -144,6 +146,7 @@ func NewCommand(f client.Factory) *cobra.Command {
 			defaultBackupLocation:             "default",
 			defaultVolumeSnapshotLocations:    make(map[string]string),
 			backupSyncPeriod:                  defaultBackupSyncPeriod,
+			defaultStoreValidationPeriod:      defaultStoreValidationPeriod,
 			defaultBackupTTL:                  defaultBackupTTL,
 			podVolumeOperationTimeout:         defaultPodVolumeOperationTimeout,
 			restoreResourcePriorities:         defaultRestorePriorities,
@@ -336,15 +339,6 @@ func (s *server) run() error {
 		return err
 	}
 
-	if err := s.validateBackupStorageLocations(); err != nil {
-		return err
-	}
-
-	if _, err := s.veleroClient.VeleroV1().BackupStorageLocations(s.namespace).Get(s.config.defaultBackupLocation, metav1.GetOptions{}); err != nil {
-		s.logger.WithError(errors.WithStack(err)).
-			Warnf("A backup storage location named %s has been specified for the server to use by default, but no corresponding backup storage location exists. Backups with a location not matching the default will need to explicitly specify an existing location", s.config.defaultBackupLocation)
-	}
-
 	if err := s.initRestic(); err != nil {
 		return err
 	}
@@ -432,39 +426,6 @@ func (s *server) veleroResourcesExist() error {
 	return nil
 }
 
-// validateBackupStorageLocations checks to ensure all backup storage locations exist
-// and have a compatible layout, and returns an error if not.
-func (s *server) validateBackupStorageLocations() error {
-	s.logger.Info("Checking that all backup storage locations are valid")
-
-	pluginManager := clientmgmt.NewManager(s.logger, s.logLevel, s.pluginRegistry)
-	defer pluginManager.CleanupClients()
-
-	locations, err := s.veleroClient.VeleroV1().BackupStorageLocations(s.namespace).List(metav1.ListOptions{})
-	if err != nil {
-		return errors.WithStack(err)
-	}
-
-	var invalid []string
-	for _, location := range locations.Items {
-		backupStore, err := persistence.NewObjectBackupStore(&location, pluginManager, s.logger)
-		if err != nil {
-			invalid = append(invalid, errors.Wrapf(err, "error getting backup store for location %q", location.Name).Error())
-			continue
-		}
-
-		if err := backupStore.IsValid(); err != nil {
-			invalid = append(invalid, errors.Wrapf(err, "backup store for location %q is invalid", location.Name).Error())
-		}
-	}
-
-	if len(invalid) > 0 {
-		return errors.Errorf("some backup storage locations are invalid: %s", strings.Join(invalid, "; "))
-	}
-
-	return nil
-}
-
 // - Custom Resource Definitions come before Custom Resource so that they can be
 //   restored with their corresponding CRD.
 // - Namespaces go second because all namespaced resources depend on them.
@@ -803,16 +764,35 @@ func (s *server) runControllers(defaultVolumeSnapshotLocations map[string]string
 		}
 	}
 
+	backupStorageLocationControllerRunInfo := func() controllerRunInfo {
+		backupStorageLocationController := controller.NewBackupStorageLocationController(
+			s.namespace,
+			s.config.defaultBackupLocation,
+			s.config.defaultStoreValidationPeriod,
+			s.sharedInformerFactory.Velero().V1().BackupStorageLocations(),
+			s.veleroClient.VeleroV1(),
+			s.sharedInformerFactory.Velero().V1().BackupStorageLocations().Lister(),
+			newPluginManager,
+			s.logger,
+		)
+
+		return controllerRunInfo{
+			controller: backupStorageLocationController,
+			numWorkers: defaultControllerWorkers,
+		}
+	}
+
 	enabledControllers := map[string]func() controllerRunInfo{
-		BackupSyncControllerKey:          backupSyncControllerRunInfo,
-		BackupControllerKey:              backupControllerRunInfo,
-		ScheduleControllerKey:            scheduleControllerRunInfo,
-		GcControllerKey:                  gcControllerRunInfo,
-		BackupDeletionControllerKey:      deletionControllerRunInfo,
-		RestoreControllerKey:             restoreControllerRunInfo,
-		ResticRepoControllerKey:          resticRepoControllerRunInfo,
-		DownloadRequestControllerKey:     downloadrequestControllerRunInfo,
-		ServerStatusRequestControllerKey: serverStatusRequestControllerRunInfo,
+		BackupSyncControllerKey:            backupSyncControllerRunInfo,
+		BackupControllerKey:                backupControllerRunInfo,
+		ScheduleControllerKey:              scheduleControllerRunInfo,
+		GcControllerKey:                    gcControllerRunInfo,
+		BackupDeletionControllerKey:        deletionControllerRunInfo,
+		RestoreControllerKey:               restoreControllerRunInfo,
+		ResticRepoControllerKey:            resticRepoControllerRunInfo,
+		DownloadRequestControllerKey:       downloadrequestControllerRunInfo,
+		ServerStatusRequestControllerKey:   serverStatusRequestControllerRunInfo,
+		BackupStorageLocationControllerKey: backupStorageLocationControllerRunInfo,
 	}
 
 	if s.config.restoreOnly {