From b57ed87f6b8a6e110d134daa14a63acae5094ba4 Mon Sep 17 00:00:00 2001 From: Abhayraj Jaiswal Date: Wed, 7 Oct 2026 21:06:54 +0530 Subject: [PATCH] fix(controller): propagate context during backup request preparation (#10621) Pass caller-provided context.Context through cluster API queries during backup request preparation and snapshot location validation, ensuring context cancellation and deadlines are properly respected. Closes #10620 Signed-off-by: Abhayraj Jaiswal --- changelogs/unreleased/10621-abhayrajjais01 | 1 + pkg/controller/backup_controller.go | 16 ++++++------ pkg/controller/backup_controller_test.go | 29 +++++++++++++++++++++- 3 files changed, 37 insertions(+), 9 deletions(-) create mode 100644 changelogs/unreleased/10621-abhayrajjais01 diff --git a/changelogs/unreleased/10621-abhayrajjais01 b/changelogs/unreleased/10621-abhayrajjais01 new file mode 100644 index 000000000..57c7824df --- /dev/null +++ b/changelogs/unreleased/10621-abhayrajjais01 @@ -0,0 +1 @@ +Propagate caller context during backup request preparation and snapshot location validation in backup controller diff --git a/pkg/controller/backup_controller.go b/pkg/controller/backup_controller.go index 8a2761fd3..b12536e30 100644 --- a/pkg/controller/backup_controller.go +++ b/pkg/controller/backup_controller.go @@ -462,7 +462,7 @@ func (b *backupReconciler) prepareBackupRequest(ctx context.Context, backup *vel // TODO(2.0) b.defaultBackupLocation will be deprecated request.Spec.StorageLocation = b.defaultBackupLocation - locationList, err := storage.ListBackupStorageLocations(context.Background(), b.kbClient, request.Namespace) + locationList, err := storage.ListBackupStorageLocations(ctx, b.kbClient, request.Namespace) if err == nil { for _, location := range locationList.Items { if location.Spec.Default { @@ -476,7 +476,7 @@ func (b *backupReconciler) prepareBackupRequest(ctx context.Context, backup *vel // get the storage location, and store the BackupStorageLocation API obj on the request storageLocation := &velerov1api.BackupStorageLocation{} - if err := b.kbClient.Get(context.Background(), kbclient.ObjectKey{ + if err := b.kbClient.Get(ctx, kbclient.ObjectKey{ Namespace: request.Namespace, Name: request.Spec.StorageLocation, }, storageLocation); err != nil { @@ -514,7 +514,7 @@ func (b *backupReconciler) prepareBackupRequest(ctx context.Context, backup *vel // validate and get the backup's VolumeSnapshotLocations, and store the // VolumeSnapshotLocation API objs on the request - if locs, errs := b.validateAndGetSnapshotLocations(request.Backup); len(errs) > 0 { + if locs, errs := b.validateAndGetSnapshotLocations(ctx, request.Backup); len(errs) > 0 { request.Status.ValidationErrors = append(request.Status.ValidationErrors, errs...) } else { request.Spec.VolumeSnapshotLocations = nil @@ -536,7 +536,7 @@ func (b *backupReconciler) prepareBackupRequest(ctx context.Context, backup *vel // Add namespaces with label velero.io/exclude-from-backup=true into request.Spec.ExcludedNamespaces // Essentially, adding the label velero.io/exclude-from-backup=true to a namespace would be equivalent to setting spec.ExcludedNamespaces namespaces := corev1api.NamespaceList{} - if err := b.kbClient.List(context.Background(), &namespaces, kbclient.MatchingLabels{velerov1api.ExcludeFromBackupLabel: "true"}); err == nil { + if err := b.kbClient.List(ctx, &namespaces, kbclient.MatchingLabels{velerov1api.ExcludeFromBackupLabel: "true"}); err == nil { for _, ns := range namespaces.Items { request.Spec.ExcludedNamespaces = append(request.Spec.ExcludedNamespaces, ns.Name) } @@ -778,14 +778,14 @@ func mergeNamespacesByLabel( // it will automatically be used) // // if backup has snapshotVolume disabled then it returns empty VSL -func (b *backupReconciler) validateAndGetSnapshotLocations(backup *velerov1api.Backup) (map[string]*velerov1api.VolumeSnapshotLocation, []string) { +func (b *backupReconciler) validateAndGetSnapshotLocations(ctx context.Context, backup *velerov1api.Backup) (map[string]*velerov1api.VolumeSnapshotLocation, []string) { errors := []string{} providerLocations := make(map[string]*velerov1api.VolumeSnapshotLocation) for _, locationName := range backup.Spec.VolumeSnapshotLocations { // validate each locationName exists as a VolumeSnapshotLocation location := &velerov1api.VolumeSnapshotLocation{} - if err := b.kbClient.Get(context.Background(), kbclient.ObjectKey{Namespace: backup.Namespace, Name: locationName}, location); err != nil { + if err := b.kbClient.Get(ctx, kbclient.ObjectKey{Namespace: backup.Namespace, Name: locationName}, location); err != nil { if apierrors.IsNotFound(err) { errors = append(errors, fmt.Sprintf("a VolumeSnapshotLocation CRD for the location %s with the name specified in the backup spec needs to be created before this snapshot can be executed. Error: %v", locationName, err)) } else { @@ -811,7 +811,7 @@ func (b *backupReconciler) validateAndGetSnapshotLocations(backup *velerov1api.B return nil, errors } volumeSnapshotLocations := &velerov1api.VolumeSnapshotLocationList{} - err := b.kbClient.List(context.Background(), volumeSnapshotLocations, &kbclient.ListOptions{Namespace: backup.Namespace, LabelSelector: labels.Everything()}) + err := b.kbClient.List(ctx, volumeSnapshotLocations, &kbclient.ListOptions{Namespace: backup.Namespace, LabelSelector: labels.Everything()}) if err != nil { errors = append(errors, fmt.Sprintf("error listing volume snapshot locations: %v", err)) return nil, errors @@ -841,7 +841,7 @@ func (b *backupReconciler) validateAndGetSnapshotLocations(backup *velerov1api.B continue } location := &velerov1api.VolumeSnapshotLocation{} - if err := b.kbClient.Get(context.Background(), kbclient.ObjectKey{Namespace: backup.Namespace, Name: defaultLocation}, location); err != nil { + if err := b.kbClient.Get(ctx, kbclient.ObjectKey{Namespace: backup.Namespace, Name: defaultLocation}, location); err != nil { errors = append(errors, fmt.Sprintf("error getting volume snapshot location named %s: %v", defaultLocation, err)) continue } diff --git a/pkg/controller/backup_controller_test.go b/pkg/controller/backup_controller_test.go index 6c7232e2f..1e07fc779 100644 --- a/pkg/controller/backup_controller_test.go +++ b/pkg/controller/backup_controller_test.go @@ -18,6 +18,7 @@ package controller import ( "bytes" + "context" "fmt" "io" "reflect" @@ -46,6 +47,7 @@ import ( ctrl "sigs.k8s.io/controller-runtime" kbclient "sigs.k8s.io/controller-runtime/pkg/client" fakeClient "sigs.k8s.io/controller-runtime/pkg/client/fake" + "sigs.k8s.io/controller-runtime/pkg/client/interceptor" "github.com/vmware-tanzu/velero/internal/resourcepolicies" velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1" @@ -2335,7 +2337,7 @@ func TestValidateAndGetSnapshotLocations(t *testing.T) { require.NoError(t, c.kbClient.Create(t.Context(), location)) } - providerLocations, errs := c.validateAndGetSnapshotLocations(backup) + providerLocations, errs := c.validateAndGetSnapshotLocations(t.Context(), backup) if test.expectedSuccess { for _, err := range errs { require.NoError(t, errors.New(err), "validateAndGetSnapshotLocations unexpected error: %v", err) @@ -2355,6 +2357,31 @@ func TestValidateAndGetSnapshotLocations(t *testing.T) { } }) } + + t.Run("returns error when context is canceled", func(t *testing.T) { + backup := defaultBackup().Phase(velerov1api.BackupPhaseNew).VolumeSnapshotLocations("aws-us-west-1").Result() + ctx, cancel := context.WithCancel(t.Context()) + cancel() + + crClient := velerotest.NewFakeControllerRuntimeClientBuilder(t). + WithInterceptorFuncs(interceptor.Funcs{ + Get: func(ctx context.Context, client kbclient.WithWatch, key kbclient.ObjectKey, obj kbclient.Object, opts ...kbclient.GetOption) error { + if err := ctx.Err(); err != nil { + return err + } + return client.Get(ctx, key, obj, opts...) + }, + }).Build() + + c := &backupReconciler{ + logger: velerotest.NewLogger(), + kbClient: crClient, + } + + _, errs := c.validateAndGetSnapshotLocations(ctx, backup) + require.NotEmpty(t, errs) + require.Contains(t, errs[0], "context canceled") + }) } // Test_getLastSuccessBySchedule verifies that the getLastSuccessBySchedule helper function correctly returns