refactor: streamline VolumeSnapshotContent deletion process and remove readiness checks

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

internal/delete/actions/csi/volumesnapshotcontent_action.go

@@ -115,25 +115,8 @@ func (p *volumeSnapshotContentDeleteItemAction) Execute(
 	}
 	p.log.Infof("Created temp VolumeSnapshotContent %s with DeletionPolicy=Delete to trigger cloud snapshot cleanup", snapCont.Name)
 
-	// Check if the VSC is ready before proceeding to deletion.
-	ready, err := checkVSCReadiness(context.TODO(), &snapCont, p.crClient)
-	if err != nil || !ready {
-		// Clean up the VSC we created since it isn't ready
-		if err != nil {
-			p.log.WithError(err).Warnf("Temp VolumeSnapshotContent %s is not ready, cleaning up", snapCont.Name)
-		} else {
-			p.log.Warnf("Temp VolumeSnapshotContent %s is not ready, cleaning up", snapCont.Name)
-		}
-		if deleteErr := p.crClient.Delete(context.TODO(), &snapCont); deleteErr != nil && !apierrors.IsNotFound(deleteErr) {
-			p.log.WithError(deleteErr).Errorf("Failed to clean up temp VolumeSnapshotContent %s", snapCont.Name)
-		}
-		if err != nil {
-			return errors.Wrapf(err, "VolumeSnapshotContent %s is not ready", snapCont.Name)
-		}
-		return errors.Errorf("VolumeSnapshotContent %s is not ready", snapCont.Name)
-	}
-
-	p.log.Infof("Temp VolumeSnapshotContent %s is ready, deleting to trigger cloud snapshot removal", snapCont.Name)
+	// Delete the temp VSC immediately to trigger cloud snapshot removal.
+	// The CSI driver will handle the actual cloud snapshot deletion.
 	if err := p.crClient.Delete(
 		context.TODO(),
 		&snapCont,
@@ -188,35 +171,6 @@ func (p *volumeSnapshotContentDeleteItemAction) tryDeleteOriginalVSC(
 	return true
 }
 
-// checkVSCReadiness checks if the given VolumeSnapshotContent has a SnapshotHandle in its status,
-// which indicates that the CSI driver has processed the VSC and it's ready for deletion.
-// It also checks for any permanent errors reported by the CSI driver and fails fast if such an error is found.
-var checkVSCReadiness = func(
-	ctx context.Context,
-	vsc *snapshotv1api.VolumeSnapshotContent,
-	client crclient.Client,
-) (bool, error) {
-	tmpVSC := new(snapshotv1api.VolumeSnapshotContent)
-	if err := client.Get(ctx, crclient.ObjectKeyFromObject(vsc), tmpVSC); err != nil {
-		return false, errors.Wrapf(
-			err, "failed to get VolumeSnapshotContent %s", vsc.Name,
-		)
-	}
-
-	if tmpVSC.Status != nil && tmpVSC.Status.SnapshotHandle != nil {
-		return true, nil
-	}
-
-	// Fail fast on permanent CSI driver errors (e.g., InvalidSnapshot.NotFound)
-	if tmpVSC.Status != nil && tmpVSC.Status.Error != nil && tmpVSC.Status.Error.Message != nil {
-		return false, errors.Errorf(
-			"VolumeSnapshotContent %s has error: %s", vsc.Name, *tmpVSC.Status.Error.Message,
-		)
-	}
-
-	return false, nil
-}
-
 func NewVolumeSnapshotContentDeleteItemAction(
 	f client.Factory,
 ) plugincommon.HandlerInitializer {

internal/delete/actions/csi/volumesnapshotcontent_action_test.go

@@ -21,8 +21,7 @@ import (
 	"fmt"
 	"testing"
 
-	snapshotv1api "github.com/kubernetes-csi/external-snapshotter/client/v7/apis/volumesnapshot/v1"
-	"github.com/pkg/errors"
+	snapshotv1api "github.com/kubernetes-csi/external-snapshotter/client/v8/apis/volumesnapshot/v1"
 	"github.com/sirupsen/logrus"
 	"github.com/stretchr/testify/require"
 	corev1api "k8s.io/api/core/v1"
@@ -76,12 +75,7 @@ func TestVSCExecute(t *testing.T) {
 		vsc            *snapshotv1api.VolumeSnapshotContent
 		backup         *velerov1api.Backup
 		preExistingVSC *snapshotv1api.VolumeSnapshotContent
-		function       func(
-			ctx context.Context,
-			vsc *snapshotv1api.VolumeSnapshotContent,
-			client crclient.Client,
-		) (bool, error)
-		expectErr bool
+		expectErr      bool
 	}{
 		{
 			name: "VolumeSnapshotContent doesn't have backup label",
@@ -105,26 +99,6 @@ func TestVSCExecute(t *testing.T) {
 			vsc:       builder.ForVolumeSnapshotContent("bar").ObjectMeta(builder.WithLabelsMap(map[string]string{velerov1api.BackupNameLabel: "backup"})).VolumeSnapshotClassName("volumesnapshotclass").Status(&snapshotv1api.VolumeSnapshotContentStatus{SnapshotHandle: &snapshotHandleStr}).Result(),
 			backup:    builder.ForBackup("velero", "backup").Result(),
 			expectErr: false,
-			function: func(
-				ctx context.Context,
-				vsc *snapshotv1api.VolumeSnapshotContent,
-				client crclient.Client,
-			) (bool, error) {
-				return true, nil
-			},
-		},
-		{
-			name:      "Normal case, VolumeSnapshot should be deleted",
-			vsc:       builder.ForVolumeSnapshotContent("bar").ObjectMeta(builder.WithLabelsMap(map[string]string{velerov1api.BackupNameLabel: "backup"})).Status(&snapshotv1api.VolumeSnapshotContentStatus{SnapshotHandle: &snapshotHandleStr}).Result(),
-			backup:    builder.ForBackup("velero", "backup").Result(),
-			expectErr: true,
-			function: func(
-				ctx context.Context,
-				vsc *snapshotv1api.VolumeSnapshotContent,
-				client crclient.Client,
-			) (bool, error) {
-				return false, errors.Errorf("test error case")
-			},
 		},
 		{
 			name:      "Original VSC exists in cluster, cleaned up directly",
@@ -141,26 +115,12 @@ func TestVSCExecute(t *testing.T) {
 				},
 			},
 		},
-		{
-			name:      "Error case with CSI error, dangling VSC should be cleaned up",
-			vsc:       builder.ForVolumeSnapshotContent("bar").ObjectMeta(builder.WithLabelsMap(map[string]string{velerov1api.BackupNameLabel: "backup"})).Status(&snapshotv1api.VolumeSnapshotContentStatus{SnapshotHandle: &snapshotHandleStr}).Result(),
-			backup:    builder.ForBackup("velero", "backup").Result(),
-			expectErr: true,
-			function: func(
-				ctx context.Context,
-				vsc *snapshotv1api.VolumeSnapshotContent,
-				client crclient.Client,
-			) (bool, error) {
-				return false, errors.Errorf("VolumeSnapshotContent %s has error: InvalidSnapshot.NotFound", vsc.Name)
-			},
-		},
 	}
 
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
 			crClient := velerotest.NewFakeControllerRuntimeClient(t)
 			logger := logrus.StandardLogger()
-			checkVSCReadiness = test.function
 
 			if test.preExistingVSC != nil {
 				require.NoError(t, crClient.Create(context.Background(), test.preExistingVSC))
@@ -222,75 +182,6 @@ func TestNewVolumeSnapshotContentDeleteItemAction(t *testing.T) {
 	require.NoError(t, err1)
 }
 
-func TestCheckVSCReadiness(t *testing.T) {
-	tests := []struct {
-		name      string
-		vsc       *snapshotv1api.VolumeSnapshotContent
-		createVSC bool
-		expectErr bool
-		ready     bool
-	}{
-		{
-			name: "VSC not exist",
-			vsc: &snapshotv1api.VolumeSnapshotContent{
-				ObjectMeta: metav1.ObjectMeta{
-					Name:      "vsc-1",
-					Namespace: "velero",
-				},
-			},
-			createVSC: false,
-			expectErr: true,
-			ready:     false,
-		},
-		{
-			name: "VSC not ready",
-			vsc: &snapshotv1api.VolumeSnapshotContent{
-				ObjectMeta: metav1.ObjectMeta{
-					Name:      "vsc-1",
-					Namespace: "velero",
-				},
-			},
-			createVSC: true,
-			expectErr: false,
-			ready:     false,
-		},
-		{
-			name: "VSC has error from CSI driver",
-			vsc: &snapshotv1api.VolumeSnapshotContent{
-				ObjectMeta: metav1.ObjectMeta{
-					Name:      "vsc-1",
-					Namespace: "velero",
-				},
-				Status: &snapshotv1api.VolumeSnapshotContentStatus{
-					ReadyToUse: boolPtr(false),
-					Error: &snapshotv1api.VolumeSnapshotError{
-						Message: stringPtr("InvalidSnapshot.NotFound: The snapshot 'snap-0abc123' does not exist."),
-					},
-				},
-			},
-			createVSC: true,
-			expectErr: true,
-			ready:     false,
-		},
-	}
-
-	for _, test := range tests {
-		t.Run(test.name, func(t *testing.T) {
-			ctx := context.TODO()
-			crClient := velerotest.NewFakeControllerRuntimeClient(t)
-			if test.createVSC {
-				require.NoError(t, crClient.Create(ctx, test.vsc))
-			}
-
-			ready, err := checkVSCReadiness(ctx, test.vsc, crClient)
-			require.Equal(t, test.ready, ready)
-			if test.expectErr {
-				require.Error(t, err)
-			}
-		})
-	}
-}
-
 func TestTryDeleteOriginalVSC(t *testing.T) {
 	tests := []struct {
 		name      string