fix: fail backup early when CSI VolumeSnapshot error persists beyond CSISnapshotTimeout

Signed-off-by: Priyansh Choudhary <im1706@gmail.com>

Author: priyansh17

changelogs/unreleased/9673-priyansh17

@@ -0,0 +1 @@
+Fix backup stuck in WaitingForPluginOperations when CSI VolumeSnapshot has a persistent error beyond CSISnapshotTimeout

pkg/apis/velero/v1/labels_annotations.go

@@ -178,4 +178,9 @@ const (
 
 	// Annotation prefix on Backup to override VGS class per CSI driver
 	VolumeGroupSnapshotClassAnnotationBackupPrefix = "velero.io/csi-volumegroupsnapshot-class_"
+
+	// VSErrorFirstObservedTimeAnnotation is the annotation key used to record the time
+	// when a VolumeSnapshot error was first observed during an async backup operation.
+	// This is used by Progress() to detect errors that persist beyond CSISnapshotTimeout.
+	VSErrorFirstObservedTimeAnnotation = "velero.io/vs-error-first-observed-time"
 )

pkg/backup/actions/csi/volumesnapshot_action.go

@@ -294,8 +294,45 @@ func (p *volumeSnapshotBackupItemAction) Progress(
 		if vs.Status.Error.Message != nil {
 			errorMessage = *vs.Status.Error.Message
 		}
-		p.log.Warnf("VolumeSnapshot has a temporary error %s. Snapshot controller will retry later.",
-			errorMessage)
+
+		now := time.Now()
+		firstObservedTime := now
+
+		if existingTime, ok := vs.Annotations[velerov1api.VSErrorFirstObservedTimeAnnotation]; ok {
+			if t, err := time.Parse(time.RFC3339, existingTime); err == nil {
+				firstObservedTime = t
+			} else {
+				p.log.Warnf("VolumeSnapshot %s/%s has an unparseable %s annotation value %q, resetting timer: %v",
+					vs.Namespace, vs.Name, velerov1api.VSErrorFirstObservedTimeAnnotation, existingTime, err)
+			}
+		} else {
+			// First time we observe this error — stamp the annotation onto the VS.
+			originVS := vs.DeepCopy()
+			kubeutil.AddAnnotations(&vs.ObjectMeta, map[string]string{
+				velerov1api.VSErrorFirstObservedTimeAnnotation: now.Format(time.RFC3339),
+			})
+			if patchErr := p.crClient.Patch(
+				context.Background(), vs, crclient.MergeFrom(originVS),
+			); patchErr != nil {
+				p.log.Warnf("Failed to patch VolumeSnapshot %s/%s with error first observed time: %v",
+					vs.Namespace, vs.Name, patchErr)
+			}
+		}
+
+		timeout := backup.Spec.CSISnapshotTimeout.Duration
+		if timeout > 0 && now.Sub(firstObservedTime) >= timeout {
+			p.log.Errorf(
+				"VolumeSnapshot %s/%s has a persistent error beyond CSISnapshotTimeout (%s): %s",
+				vs.Namespace, vs.Name, timeout, errorMessage)
+			progress.Completed = true
+			progress.Updated = now
+			progress.Err = fmt.Sprintf("VolumeSnapshot %s/%s has a persistent error: %s",
+				vs.Namespace, vs.Name, errorMessage)
+			return progress, nil
+		}
+
+		p.log.Warnf("VolumeSnapshot %s/%s has an error within the CSISnapshotTimeout window: %s. Snapshot controller will retry later.",
+			vs.Namespace, vs.Name, errorMessage)
 
 		return progress, nil
 	}

pkg/backup/actions/csi/volumesnapshot_action_test.go

@@ -19,6 +19,7 @@ package csi
 import (
 	"fmt"
 	"testing"
+	"time"
 
 	"github.com/google/go-cmp/cmp"
 	"github.com/google/go-cmp/cmp/cmpopts"
@@ -219,7 +220,7 @@ func TestVSProgress(t *testing.T) {
 			expectedErr: false,
 		},
 		{
-			name:        "VS status has error",
+			name:        "VS status has error, no prior annotation, no CSISnapshotTimeout configured",
 			operationID: "ns/name/2024-04-11T18:49:00+08:00",
 			vs: builder.ForVolumeSnapshot("ns", "name").Status().
 				StatusError(snapshotv1api.VolumeSnapshotError{
@@ -228,6 +229,38 @@ func TestVSProgress(t *testing.T) {
 			backup:      builder.ForBackup("velero", "backup").Result(),
 			expectedErr: false,
 		},
+		{
+			name:        "VS status has error, annotation within CSISnapshotTimeout",
+			operationID: "ns/name/2024-04-11T18:49:00+08:00",
+			vs: builder.ForVolumeSnapshot("ns", "name").
+				ObjectMeta(builder.WithAnnotations(
+					velerov1api.VSErrorFirstObservedTimeAnnotation, time.Now().Format(time.RFC3339),
+				)).
+				Status().
+				StatusError(snapshotv1api.VolumeSnapshotError{
+					Message: &errorStr,
+				}).Result(),
+			backup:      builder.ForBackup("velero", "backup").CSISnapshotTimeout(10 * time.Minute).Result(),
+			expectedErr: false,
+		},
+		{
+			name:        "VS status has persistent error beyond CSISnapshotTimeout",
+			operationID: "ns/name/2024-04-11T18:49:00+08:00",
+			vs: builder.ForVolumeSnapshot("ns", "name").
+				ObjectMeta(builder.WithAnnotations(
+					velerov1api.VSErrorFirstObservedTimeAnnotation, "2024-04-11T18:49:00+08:00",
+				)).
+				Status().
+				StatusError(snapshotv1api.VolumeSnapshotError{
+					Message: &errorStr,
+				}).Result(),
+			backup:      builder.ForBackup("velero", "backup").CSISnapshotTimeout(10 * time.Minute).Result(),
+			expectedErr: false,
+			expectedProgress: &velero.OperationProgress{
+				Completed: true,
+				Err:       fmt.Sprintf("VolumeSnapshot ns/name has a persistent error: %s", errorStr),
+			},
+		},
 		{
 			name:        "Fail to get VSC",
 			operationID: "ns/name/2024-04-11T18:49:00+08:00",