diff --git a/pkg/controller/backup_controller.go b/pkg/controller/backup_controller.go index 22762bff7..90085fa1f 100644 --- a/pkg/controller/backup_controller.go +++ b/pkg/controller/backup_controller.go @@ -36,7 +36,11 @@ import ( "k8s.io/apimachinery/pkg/util/wait" "k8s.io/utils/clock" ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/builder" kbclient "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/controller" + "sigs.k8s.io/controller-runtime/pkg/event" + "sigs.k8s.io/controller-runtime/pkg/predicate" "github.com/vmware-tanzu/velero/internal/credentials" "github.com/vmware-tanzu/velero/internal/resourcepolicies" @@ -161,7 +165,7 @@ func NewBackupReconciler( maxConcurrentK8SConnections: maxConcurrentK8SConnections, defaultSnapshotMoveData: defaultSnapshotMoveData, itemBlockWorkerCount: itemBlockWorkerCount, - concurrentBackups: concurrentBackups, + concurrentBackups: max(concurrentBackups, 1), globalCRClient: globalCRClient, workerPool: pkgbackup.StartItemBlockWorkerPool(ctx, itemBlockWorkerCount, logger), } @@ -171,7 +175,24 @@ func NewBackupReconciler( func (b *backupReconciler) SetupWithManager(mgr ctrl.Manager) error { return ctrl.NewControllerManagedBy(mgr). - For(&velerov1api.Backup{}). + For(&velerov1api.Backup{}, builder.WithPredicates(predicate.Funcs{ + UpdateFunc: func(ue event.UpdateEvent) bool { + backup := ue.ObjectNew.(*velerov1api.Backup) + return backup.Status.Phase == velerov1api.BackupPhaseReadyToStart + }, + CreateFunc: func(ce event.CreateEvent) bool { + return false + }, + DeleteFunc: func(de event.DeleteEvent) bool { + return false + }, + GenericFunc: func(ge event.GenericEvent) bool { + return false + }, + })). + WithOptions(controller.Options{ + MaxConcurrentReconciles: b.concurrentBackups, + }). Named(constant.ControllerBackup). Complete(b) } @@ -257,8 +278,8 @@ func (b *backupReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr // InProgress, we still need this check so we can return nil to indicate we've finished processing // this key (even though it was a no-op). switch original.Status.Phase { - case "", velerov1api.BackupPhaseNew: - // only process new backups + case velerov1api.BackupPhaseReadyToStart: + // only process ReadytToStart backups default: b.logger.WithFields(logrus.Fields{ "backup": kubeutil.NamespaceAndName(original), diff --git a/pkg/controller/backup_controller_test.go b/pkg/controller/backup_controller_test.go index 715f5068a..524db4b24 100644 --- a/pkg/controller/backup_controller_test.go +++ b/pkg/controller/backup_controller_test.go @@ -95,7 +95,11 @@ func (b *fakeBackupper) FinalizeBackup( } func defaultBackup() *builder.BackupBuilder { - return builder.ForBackup(velerov1api.DefaultNamespace, "backup-1") + return builder.ForBackup(velerov1api.DefaultNamespace, "backup-1").Phase(velerov1api.BackupPhaseReadyToStart) +} + +func namedBackup(name string) *builder.BackupBuilder { + return builder.ForBackup(velerov1api.DefaultNamespace, name).Phase(velerov1api.BackupPhaseReadyToStart) } func TestProcessBackupNonProcessedItems(t *testing.T) { @@ -104,6 +108,16 @@ func TestProcessBackupNonProcessedItems(t *testing.T) { key string backup *velerov1api.Backup }{ + { + name: "New backup is not processed", + key: "velero/backup-1", + backup: defaultBackup().Phase(velerov1api.BackupPhaseNew).Result(), + }, + { + name: "Queued backup is not processed", + key: "velero/backup-1", + backup: defaultBackup().Phase(velerov1api.BackupPhaseQueued).Result(), + }, { name: "FailedValidation backup is not processed", key: "velero/backup-1", @@ -331,7 +345,7 @@ func Test_prepareBackupRequest_BackupStorageLocation(t *testing.T) { }{ { name: "BackupLocation is specified in backup CR'spec and it can be found in ApiServer", - backup: builder.ForBackup("velero", "backup-1").Result(), + backup: defaultBackup().Result(), backupLocationNameInBackup: "test-backup-location", backupLocationInAPIServer: builder.ForBackupStorageLocation("velero", "test-backup-location").Result(), defaultBackupLocationInAPIServer: builder.ForBackupStorageLocation("velero", "default-location").Result(), @@ -340,7 +354,7 @@ func Test_prepareBackupRequest_BackupStorageLocation(t *testing.T) { }, { name: "BackupLocation is specified in backup CR'spec and it can't be found in ApiServer", - backup: builder.ForBackup("velero", "backup-1").Result(), + backup: defaultBackup().Result(), backupLocationNameInBackup: "test-backup-location", backupLocationInAPIServer: nil, defaultBackupLocationInAPIServer: nil, @@ -349,7 +363,7 @@ func Test_prepareBackupRequest_BackupStorageLocation(t *testing.T) { }, { name: "Using default BackupLocation and it can be found in ApiServer", - backup: builder.ForBackup("velero", "backup-1").Result(), + backup: defaultBackup().Result(), backupLocationNameInBackup: "", backupLocationInAPIServer: builder.ForBackupStorageLocation("velero", "test-backup-location").Result(), defaultBackupLocationInAPIServer: builder.ForBackupStorageLocation("velero", "default-location").Result(), @@ -358,7 +372,7 @@ func Test_prepareBackupRequest_BackupStorageLocation(t *testing.T) { }, { name: "Using default BackupLocation and it can't be found in ApiServer", - backup: builder.ForBackup("velero", "backup-1").Result(), + backup: defaultBackup().Result(), backupLocationNameInBackup: "", backupLocationInAPIServer: nil, defaultBackupLocationInAPIServer: nil, @@ -497,7 +511,7 @@ func TestPrepareBackupRequest_SetsVGSLabelKey(t *testing.T) { }{ { name: "backup with spec label key set", - backup: builder.ForBackup("velero", "backup-1"). + backup: defaultBackup(). VolumeGroupSnapshotLabelKey("spec-key"). Result(), serverFlagKey: "server-key", @@ -505,13 +519,13 @@ func TestPrepareBackupRequest_SetsVGSLabelKey(t *testing.T) { }, { name: "backup with no spec key, uses server flag", - backup: builder.ForBackup("velero", "backup-2").Result(), + backup: namedBackup("backup-2").Result(), serverFlagKey: "server-key", expectedLabelKey: "server-key", }, { name: "backup with no spec or server flag, uses default", - backup: builder.ForBackup("velero", "backup-3").Result(), + backup: namedBackup("backup-3").Result(), serverFlagKey: velerov1api.DefaultVGSLabelKey, expectedLabelKey: velerov1api.DefaultVGSLabelKey, }, @@ -1390,7 +1404,7 @@ func TestProcessBackupCompletions(t *testing.T) { }, { name: "backup with namespace-scoped and cluster-scoped resource filters", - backup: builder.ForBackup(velerov1api.DefaultNamespace, "backup-1"). + backup: defaultBackup(). ExcludedClusterScopedResources("clusterroles"). IncludedClusterScopedResources("storageclasses"). ExcludedNamespaceScopedResources("secrets"). @@ -1439,7 +1453,7 @@ func TestProcessBackupCompletions(t *testing.T) { }, { name: "backup's include filter overlap with default exclude resources", - backup: builder.ForBackup(velerov1api.DefaultNamespace, "backup-1"). + backup: defaultBackup(). ExcludedClusterScopedResources("clusterroles"). IncludedClusterScopedResources("storageclasses", "volumesnapshotcontents.snapshot.storage.k8s.io"). ExcludedNamespaceScopedResources("secrets"). diff --git a/pkg/controller/backup_queue_controller.go b/pkg/controller/backup_queue_controller.go index 8fa679542..7fe295da3 100644 --- a/pkg/controller/backup_queue_controller.go +++ b/pkg/controller/backup_queue_controller.go @@ -64,7 +64,7 @@ func NewBackupQueueReconciler( Client: client, Scheme: scheme, logger: logger, - concurrentBackups: concurrentBackups, + concurrentBackups: max(concurrentBackups, 1), frequency: defaultQueuedBackupRecheckFrequency, } }