From 22c88ba330fe8bb481b28512aabfb3a315748510 Mon Sep 17 00:00:00 2001
From: Lyndon-Li <lyonghui@vmware.com>
Date: Tue, 8 Aug 2023 12:05:56 +0800
Subject: [PATCH] data mover fail earlier for snapshot creation error

Signed-off-by: Lyndon-Li <lyonghui@vmware.com>
---
 pkg/util/csi/volume_snapshot.go      | 11 ++++++-
 pkg/util/csi/volume_snapshot_test.go | 43 ++++++++++++++++++++++++++--
 pkg/util/stringptr/stringptr.go      | 27 +++++++++++++++++
 3 files changed, 78 insertions(+), 3 deletions(-)
 create mode 100644 pkg/util/stringptr/stringptr.go

diff --git a/pkg/util/csi/volume_snapshot.go b/pkg/util/csi/volume_snapshot.go
index b4585e3ed9..91e560d4f6 100644
--- a/pkg/util/csi/volume_snapshot.go
+++ b/pkg/util/csi/volume_snapshot.go
@@ -29,6 +29,7 @@ import (
 	"k8s.io/apimachinery/pkg/util/wait"
 
 	"github.com/vmware-tanzu/velero/pkg/util/boolptr"
+	"github.com/vmware-tanzu/velero/pkg/util/stringptr"
 
 	snapshotv1api "github.com/kubernetes-csi/external-snapshotter/client/v4/apis/volumesnapshot/v1"
 	snapshotter "github.com/kubernetes-csi/external-snapshotter/client/v4/clientset/versioned/typed/volumesnapshot/v1"
@@ -53,7 +54,15 @@ func WaitVolumeSnapshotReady(ctx context.Context, snapshotClient snapshotter.Sna
 			return false, errors.Wrapf(err, fmt.Sprintf("error to get volumesnapshot %s/%s", volumeSnapshotNS, volumeSnapshot))
 		}
 
-		if tmpVS.Status == nil || tmpVS.Status.BoundVolumeSnapshotContentName == nil || !boolptr.IsSetToTrue(tmpVS.Status.ReadyToUse) || tmpVS.Status.RestoreSize == nil {
+		if tmpVS.Status == nil {
+			return false, nil
+		}
+
+		if tmpVS.Status.Error != nil {
+			return false, errors.Errorf("volume snapshot creation error %s", stringptr.GetString(tmpVS.Status.Error.Message))
+		}
+
+		if !boolptr.IsSetToTrue(tmpVS.Status.ReadyToUse) {
 			return false, nil
 		}
 
diff --git a/pkg/util/csi/volume_snapshot_test.go b/pkg/util/csi/volume_snapshot_test.go
index 37f202ef97..7a678deb94 100644
--- a/pkg/util/csi/volume_snapshot_test.go
+++ b/pkg/util/csi/volume_snapshot_test.go
@@ -31,6 +31,7 @@ import (
 	clientTesting "k8s.io/client-go/testing"
 
 	"github.com/vmware-tanzu/velero/pkg/util/boolptr"
+	"github.com/vmware-tanzu/velero/pkg/util/stringptr"
 
 	velerotest "github.com/vmware-tanzu/velero/pkg/test"
 )
@@ -55,6 +56,8 @@ func TestWaitVolumeSnapshotReady(t *testing.T) {
 		},
 	}
 
+	errMessage := "fake-snapshot-creation-error"
+
 	tests := []struct {
 		name      string
 		clientObj []runtime.Object
@@ -116,7 +119,7 @@ func TestWaitVolumeSnapshotReady(t *testing.T) {
 			err: "timed out waiting for the condition",
 		},
 		{
-			name:      "restore size is nil in status",
+			name:      "ready to use is false",
 			vsName:    "fake-vs",
 			namespace: "fake-ns",
 			clientObj: []runtime.Object{
@@ -127,12 +130,48 @@ func TestWaitVolumeSnapshotReady(t *testing.T) {
 					},
 					Status: &snapshotv1api.VolumeSnapshotStatus{
 						BoundVolumeSnapshotContentName: &vscName,
-						ReadyToUse:                     boolptr.True(),
+						ReadyToUse:                     boolptr.False(),
 					},
 				},
 			},
 			err: "timed out waiting for the condition",
 		},
+		{
+			name:      "snapshot creation error with message",
+			vsName:    "fake-vs",
+			namespace: "fake-ns",
+			clientObj: []runtime.Object{
+				&snapshotv1api.VolumeSnapshot{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:      "fake-vs",
+						Namespace: "fake-ns",
+					},
+					Status: &snapshotv1api.VolumeSnapshotStatus{
+						Error: &snapshotv1api.VolumeSnapshotError{
+							Message: &errMessage,
+						},
+					},
+				},
+			},
+			err: "volume snapshot creation error fake-snapshot-creation-error",
+		},
+		{
+			name:      "snapshot creation error without message",
+			vsName:    "fake-vs",
+			namespace: "fake-ns",
+			clientObj: []runtime.Object{
+				&snapshotv1api.VolumeSnapshot{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:      "fake-vs",
+						Namespace: "fake-ns",
+					},
+					Status: &snapshotv1api.VolumeSnapshotStatus{
+						Error: &snapshotv1api.VolumeSnapshotError{},
+					},
+				},
+			},
+			err: "volume snapshot creation error " + stringptr.NilString,
+		},
 		{
 			name:      "success",
 			vsName:    "fake-vs",
diff --git a/pkg/util/stringptr/stringptr.go b/pkg/util/stringptr/stringptr.go
new file mode 100644
index 0000000000..faee7b99d1
--- /dev/null
+++ b/pkg/util/stringptr/stringptr.go
@@ -0,0 +1,27 @@
+/*
+Copyright 2017 the Velero contributors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package stringptr
+
+const NilString = "<nil>"
+
+func GetString(str *string) string {
+	if str == nil {
+		return NilString
+	} else {
+		return *str
+	}
+}