From aedc0fe5e25fd8446af13c4e17d6b51043dbb60f Mon Sep 17 00:00:00 2001 From: Scott Seago Date: Fri, 3 Oct 2025 14:04:38 -0400 Subject: [PATCH] make update, changelog Signed-off-by: Scott Seago --- changelogs/unreleased/9307-sseago | 1 + pkg/backup/backup_test.go | 4 +- pkg/controller/backup_queue_controller.go | 41 +++++++++--------- .../backup_queue_controller_test.go | 43 ++++++++++--------- 4 files changed, 46 insertions(+), 43 deletions(-) create mode 100644 changelogs/unreleased/9307-sseago diff --git a/changelogs/unreleased/9307-sseago b/changelogs/unreleased/9307-sseago new file mode 100644 index 000000000..c69891510 --- /dev/null +++ b/changelogs/unreleased/9307-sseago @@ -0,0 +1 @@ +Concurrent backup processing diff --git a/pkg/backup/backup_test.go b/pkg/backup/backup_test.go index 1db3cff6b..f9351245c 100644 --- a/pkg/backup/backup_test.go +++ b/pkg/backup/backup_test.go @@ -1428,8 +1428,8 @@ func TestBackupItemActionsForSkippedPV(t *testing.T) { }, includedPVs: map[string]struct{}{}, }, - BackedUpItems: NewBackedUpItemsMap(), - WorkerPool: itemBlockPool, + BackedUpItems: NewBackedUpItemsMap(), + WorkerPool: itemBlockPool, }, apiResources: []*test.APIResource{ test.PVCs( diff --git a/pkg/controller/backup_queue_controller.go b/pkg/controller/backup_queue_controller.go index 93ef08713..2aff4e105 100644 --- a/pkg/controller/backup_queue_controller.go +++ b/pkg/controller/backup_queue_controller.go @@ -85,7 +85,7 @@ func queuePositionOrderFunc(objList client.ObjectList) client.ObjectList { // SetupWithManager adds the reconciler to the manager func (r *backupQueueReconciler) SetupWithManager(mgr ctrl.Manager) error { - // For periodic requeue, only consider Queued backups, order by QueuePosition + // For periodic requeue, only consider Queued backups, order by QueuePosition gp := kube.NewGenericEventPredicate(func(object client.Object) bool { backup := object.(*velerov1api.Backup) return backup.Status.Phase == velerov1api.BackupPhaseQueued @@ -93,7 +93,7 @@ func (r *backupQueueReconciler) SetupWithManager(mgr ctrl.Manager) error { s := kube.NewPeriodicalEnqueueSource(r.logger.WithField("controller", constant.ControllerBackupQueue), mgr.GetClient(), &velerov1api.BackupList{}, r.frequency, kube.PeriodicalEnqueueSourceOption{ Predicates: []predicate.Predicate{gp}, - OrderFunc: queuePositionOrderFunc, + OrderFunc: queuePositionOrderFunc, }) return ctrl.NewControllerManagedBy(mgr). @@ -123,7 +123,7 @@ func (r *backupQueueReconciler) SetupWithManager(mgr ctrl.Manager) error { return oldBackup.Status.Phase == velerov1api.BackupPhaseInProgress && newBackup.Status.Phase != velerov1api.BackupPhaseInProgress || oldBackup.Status.Phase != velerov1api.BackupPhaseQueued && - newBackup.Status.Phase == velerov1api.BackupPhaseQueued + newBackup.Status.Phase == velerov1api.BackupPhaseQueued }, CreateFunc: func(event.CreateEvent) bool { return false @@ -148,9 +148,9 @@ func (r *backupQueueReconciler) listQueuedBackups(ctx context.Context, ns string return nil, err } for _, backup := range backupList.Items { - if backup.Status.Phase == velerov1api.BackupPhaseQueued { + if backup.Status.Phase == velerov1api.BackupPhaseQueued { backups = append(backups, backup) - } + } } return backups, nil @@ -164,17 +164,17 @@ func (r *backupQueueReconciler) listEarlierBackups(ctx context.Context, ns strin return nil, 0, err } for _, backup := range backupList.Items { - if backup.Status.Phase == velerov1api.BackupPhaseInProgress || + if backup.Status.Phase == velerov1api.BackupPhaseInProgress || backup.Status.Phase == velerov1api.BackupPhaseReadyToStart || (backup.Status.Phase == velerov1api.BackupPhaseQueued && - backup.Status.QueuePosition < queuePos) { + backup.Status.QueuePosition < queuePos) { backups = append(backups, backup) - } + } // InProgress and ReadyToStart backups count towards the concurrentBackups limit - if backup.Status.Phase == velerov1api.BackupPhaseInProgress || + if backup.Status.Phase == velerov1api.BackupPhaseInProgress || backup.Status.Phase == velerov1api.BackupPhaseReadyToStart { runningCount += 1 - } + } } return backups, runningCount, nil } @@ -216,6 +216,7 @@ func detectNSConflictsInternal(ctx context.Context, backup *velerov1api.Backup, } return false, "" } + // Returns true if there are backups ahead of the current backup that are runnable // This could happen if velero just reconciled the one earlier in the queue and rejected it // due to too many running backups, but a backup completed in between that reconcile and this one @@ -261,9 +262,9 @@ func (r *backupQueueReconciler) orderedQueuedBackups(ctx context.Context, backup } orderedBackupList := queuePositionOrderFunc(backupList).(*velerov1api.BackupList) for _, item := range orderedBackupList.Items { - if item.Status.Phase == velerov1api.BackupPhaseQueued { - returnList = append(returnList, item) - } + if item.Status.Phase == velerov1api.BackupPhaseQueued { + returnList = append(returnList, item) + } } return returnList, nil } @@ -272,13 +273,13 @@ func (r *backupQueueReconciler) findQueuedBackupsToRequeue(ctx context.Context, requests := []reconcile.Request{} backups, _ := r.orderedQueuedBackups(ctx, backup) for _, item := range backups { - requests = append(requests, reconcile.Request{ - NamespacedName: types.NamespacedName{ - Namespace: item.GetNamespace(), - Name: item.GetName(), - }, - }) - } + requests = append(requests, reconcile.Request{ + NamespacedName: types.NamespacedName{ + Namespace: item.GetNamespace(), + Name: item.GetName(), + }, + }) + } return requests } diff --git a/pkg/controller/backup_queue_controller_test.go b/pkg/controller/backup_queue_controller_test.go index 1a5eb582f..c559dc8e0 100644 --- a/pkg/controller/backup_queue_controller_test.go +++ b/pkg/controller/backup_queue_controller_test.go @@ -23,10 +23,12 @@ import ( "github.com/sirupsen/logrus" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + //metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" ctrl "sigs.k8s.io/controller-runtime" + //"sigs.k8s.io/controller-runtime/pkg/client/fake" velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1" @@ -60,7 +62,7 @@ func TestBackupQueueReconciler(t *testing.T) { expectPhase: velerov1api.BackupPhaseInProgress, }, { - name: "Second New Backup gets queued with queuePosition 2", + name: "Second New Backup gets queued with queuePosition 2", priorBackups: []*velerov1api.Backup{ builder.ForBackup(velerov1api.DefaultNamespace, "backup-11").Phase(velerov1api.BackupPhaseQueued).QueuePosition(1).Result(), }, @@ -74,7 +76,7 @@ func TestBackupQueueReconciler(t *testing.T) { expectPhase: velerov1api.BackupPhaseReadyToStart, }, { - name: "Queued Backup remains queued if no spaces available", + name: "Queued Backup remains queued if no spaces available", priorBackups: []*velerov1api.Backup{ builder.ForBackup(velerov1api.DefaultNamespace, "backup-11").Phase(velerov1api.BackupPhaseInProgress).Result(), builder.ForBackup(velerov1api.DefaultNamespace, "backup-12").Phase(velerov1api.BackupPhaseInProgress).Result(), @@ -85,7 +87,7 @@ func TestBackupQueueReconciler(t *testing.T) { expectQueuePosition: 1, }, { - name: "Queued Backup remains queued if no spaces available including ReadyToStart", + name: "Queued Backup remains queued if no spaces available including ReadyToStart", priorBackups: []*velerov1api.Backup{ builder.ForBackup(velerov1api.DefaultNamespace, "backup-11").Phase(velerov1api.BackupPhaseInProgress).Result(), builder.ForBackup(velerov1api.DefaultNamespace, "backup-12").Phase(velerov1api.BackupPhaseReadyToStart).Result(), @@ -96,7 +98,7 @@ func TestBackupQueueReconciler(t *testing.T) { expectQueuePosition: 1, }, { - name: "Queued Backup remains queued if earlier runnable backup is also queued", + name: "Queued Backup remains queued if earlier runnable backup is also queued", priorBackups: []*velerov1api.Backup{ builder.ForBackup(velerov1api.DefaultNamespace, "backup-11").Phase(velerov1api.BackupPhaseInProgress).Result(), builder.ForBackup(velerov1api.DefaultNamespace, "backup-12").Phase(velerov1api.BackupPhaseQueued).QueuePosition(1).Result(), @@ -107,7 +109,7 @@ func TestBackupQueueReconciler(t *testing.T) { expectQueuePosition: 2, }, { - name: "Queued Backup remains queued if in conflict with running backup", + name: "Queued Backup remains queued if in conflict with running backup", priorBackups: []*velerov1api.Backup{ builder.ForBackup(velerov1api.DefaultNamespace, "backup-11").Phase(velerov1api.BackupPhaseInProgress).IncludedNamespaces("foo").Result(), }, @@ -118,7 +120,7 @@ func TestBackupQueueReconciler(t *testing.T) { expectQueuePosition: 1, }, { - name: "Queued Backup remains queued if in conflict with ReadyToStart backup", + name: "Queued Backup remains queued if in conflict with ReadyToStart backup", priorBackups: []*velerov1api.Backup{ builder.ForBackup(velerov1api.DefaultNamespace, "backup-11").Phase(velerov1api.BackupPhaseReadyToStart).IncludedNamespaces("foo").Result(), }, @@ -129,7 +131,7 @@ func TestBackupQueueReconciler(t *testing.T) { expectQueuePosition: 1, }, { - name: "Queued Backup remains queued if in conflict with earlier queued backup", + name: "Queued Backup remains queued if in conflict with earlier queued backup", priorBackups: []*velerov1api.Backup{ builder.ForBackup(velerov1api.DefaultNamespace, "backup-11").Phase(velerov1api.BackupPhaseQueued).QueuePosition(1).IncludedNamespaces("foo").Result(), }, @@ -140,7 +142,7 @@ func TestBackupQueueReconciler(t *testing.T) { expectQueuePosition: 2, }, { - name: "Queued Backup remains queued if earlier non-ns-conflict backup exists", + name: "Queued Backup remains queued if earlier non-ns-conflict backup exists", priorBackups: []*velerov1api.Backup{ builder.ForBackup(velerov1api.DefaultNamespace, "backup-11").Phase(velerov1api.BackupPhaseInProgress).IncludedNamespaces("bar").Result(), builder.ForBackup(velerov1api.DefaultNamespace, "backup-12").Phase(velerov1api.BackupPhaseQueued).QueuePosition(1).IncludedNamespaces("foo").Result(), @@ -152,7 +154,7 @@ func TestBackupQueueReconciler(t *testing.T) { expectQueuePosition: 2, }, { - name: "Running all-namespace backup conflicts with queued one-namespace backup ", + name: "Running all-namespace backup conflicts with queued one-namespace backup ", priorBackups: []*velerov1api.Backup{ builder.ForBackup(velerov1api.DefaultNamespace, "backup-11").Phase(velerov1api.BackupPhaseInProgress).IncludedNamespaces("*").Result(), }, @@ -163,7 +165,7 @@ func TestBackupQueueReconciler(t *testing.T) { expectQueuePosition: 1, }, { - name: "Running one-namespace backup conflicts with queued all-namespace backup ", + name: "Running one-namespace backup conflicts with queued all-namespace backup ", priorBackups: []*velerov1api.Backup{ builder.ForBackup(velerov1api.DefaultNamespace, "backup-11").Phase(velerov1api.BackupPhaseInProgress).IncludedNamespaces("bar").Result(), }, @@ -174,24 +176,24 @@ func TestBackupQueueReconciler(t *testing.T) { expectQueuePosition: 1, }, { - name: "Queued Backup moves to ReadyToStart if running count < concurrentBackups", + name: "Queued Backup moves to ReadyToStart if running count < concurrentBackups", priorBackups: []*velerov1api.Backup{ builder.ForBackup(velerov1api.DefaultNamespace, "backup-11").Phase(velerov1api.BackupPhaseInProgress).Result(), builder.ForBackup(velerov1api.DefaultNamespace, "backup-12").Phase(velerov1api.BackupPhaseInProgress).Result(), }, - concurrentBackups: 3, - backup: builder.ForBackup(velerov1api.DefaultNamespace, "backup-20").Phase(velerov1api.BackupPhaseQueued).QueuePosition(1).Result(), - expectPhase: velerov1api.BackupPhaseReadyToStart, + concurrentBackups: 3, + backup: builder.ForBackup(velerov1api.DefaultNamespace, "backup-20").Phase(velerov1api.BackupPhaseQueued).QueuePosition(1).Result(), + expectPhase: velerov1api.BackupPhaseReadyToStart, }, { - name: "Queued Backup moves to ReadyToStart if running count < concurrentBackups and no ns conflict found", + name: "Queued Backup moves to ReadyToStart if running count < concurrentBackups and no ns conflict found", priorBackups: []*velerov1api.Backup{ builder.ForBackup(velerov1api.DefaultNamespace, "backup-11").Phase(velerov1api.BackupPhaseReadyToStart).IncludedNamespaces("foo").Result(), }, - namespaces: []string{"foo", "bar"}, - concurrentBackups: 3, - backup: builder.ForBackup(velerov1api.DefaultNamespace, "backup-20").Phase(velerov1api.BackupPhaseQueued).QueuePosition(1).IncludedNamespaces("bar").Result(), - expectPhase: velerov1api.BackupPhaseReadyToStart, + namespaces: []string{"foo", "bar"}, + concurrentBackups: 3, + backup: builder.ForBackup(velerov1api.DefaultNamespace, "backup-20").Phase(velerov1api.BackupPhaseQueued).QueuePosition(1).IncludedNamespaces("bar").Result(), + expectPhase: velerov1api.BackupPhaseReadyToStart, }, } @@ -214,7 +216,7 @@ func TestBackupQueueReconciler(t *testing.T) { logger := logrus.New() log := logger.WithField("controller", "backup-queue-test") r := NewBackupQueueReconciler(fakeClient, scheme, log, test.concurrentBackups) - req := ctrl.Request{NamespacedName: types.NamespacedName{Namespace: test.backup.Namespace,Name: test.backup.Name}} + req := ctrl.Request{NamespacedName: types.NamespacedName{Namespace: test.backup.Namespace, Name: test.backup.Name}} res, err := r.Reconcile(context.Background(), req) gotErr := err != nil require.NoError(t, err) @@ -232,5 +234,4 @@ func TestBackupQueueReconciler(t *testing.T) { }) } - }