Repository: kubernetes-csi/external-snapshotter (go)

Issue:
Use driver name from PV instead of from StorageClass to validate
In current implementation, [SetDefaultSnapshotClass](https://github.com/kubernetes-csi/external-snapshotter/blob/master/pkg/common-controller/snapshot_controller.go#L1189) relies on PVC's StorageClass to find the driver name. Instead its more appropriate to rely on PV's driver name as StorageClass can be updated after a volume has been provisioned.

Per discussion in https://github.com/kubernetes/kubernetes/issues/94312

Pull request description:
using csi driver from PV instead of from StorageClass
<!--  Thanks for sending a pull request!  Here are some tips for you:

1. If this is your first time, please read our contributor guidelines: https://git.k8s.io/community/contributors/guide#your-first-contribution and developer guide https://git.k8s.io/community/contributors/devel/development.md#development-guide
2. Follow the instructions for writing a release note: https://git.k8s.io/community/contributors/guide/release-notes.md
3. If the PR is unfinished, see how to mark it: https://git.k8s.io/community/contributors/guide/pull-requests.md#marking-unfinished-pull-requests
-->

**What type of PR is this?**
> Uncomment only one ` /kind <>` line, hit enter to put that in a new line, and remove leading whitespaces from that line:
>

/kind cleanup

**What this PR does / why we need it**:
In current implementation, SetDefaultSnapshotClass relies on PVC's StorageClass to find the driver name. Instead it's more appropriate to rely on PV's driver name as StorageClass can be updated after a volume has been provisioned.

This also removes the dependency on storage/v1

**Which issue(s) this PR fixes**:
<!--
*Automatically closes linked issue when PR is merged.
Usage: `Fixes #<issue number>`, or `Fixes (paste link of issue)`.
_If PR is about `failing-tests or flakes`, please post the related issues/tests in a comment and do not use `Fixes`_*
-->
Fixes #404

**Special notes for your reviewer**:

**Does this PR introduce a user-facing change?**:
<!--
If no, just write "NONE" in the release-note block below.
If yes, a release note is required:
Enter your extended release note in the block below. If the PR requires additional action from users switching to the new release, include the string "action required".
-->
```release-note
NONE

```


Patch:
diff --git a/pkg/common-controller/snapshot_controller.go b/pkg/common-controller/snapshot_controller.go
index 6ebbbd759..34b627743 100644
--- a/pkg/common-controller/snapshot_controller.go
+++ b/pkg/common-controller/snapshot_controller.go
@@ -23,7 +23,6 @@ import (
 	"time"
 
 	v1 "k8s.io/api/core/v1"
-	storagev1 "k8s.io/api/storage/v1"
 	apierrs "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/api/resource"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -1149,28 +1148,20 @@ func (ctrl *csiSnapshotCommonController) isVolumeBoundToClaim(volume *v1.Persist
 	return true
 }
 
-func (ctrl *csiSnapshotCommonController) getStorageClassFromVolumeSnapshot(snapshot *crdv1.VolumeSnapshot) (*storagev1.StorageClass, error) {
-	// Get storage class from PVC or PV
-	pvc, err := ctrl.getClaimFromVolumeSnapshot(snapshot)
+// pvDriverFromSnapshot is a helper function to get the CSI driver name from the targeted PersistentVolume.
+// It looks up the PVC from which the snapshot is specified to be created from, and looks for the PVC's corresponding
+// PV. Bi-directional binding will be verified between PVC and PV before the PV's CSI driver is returned.
+// For an non-CSI volume, it returns an error immediately as it's not supported.
+func (ctrl *csiSnapshotCommonController) pvDriverFromSnapshot(snapshot *crdv1.VolumeSnapshot) (string, error) {
+	pv, err := ctrl.getVolumeFromVolumeSnapshot(snapshot)
 	if err != nil {
-		return nil, err
-	}
-	storageclassName := *pvc.Spec.StorageClassName
-	if len(storageclassName) == 0 {
-		volume, err := ctrl.getVolumeFromVolumeSnapshot(snapshot)
-		if err != nil {
-			return nil, err
-		}
-		storageclassName = volume.Spec.StorageClassName
-	}
-	if len(storageclassName) == 0 {
-		return nil, fmt.Errorf("cannot figure out the snapshot class automatically, please specify one in snapshot spec")
+		return "", err
 	}
-	storageclass, err := ctrl.client.StorageV1().StorageClasses().Get(context.TODO(), storageclassName, metav1.GetOptions{})
-	if err != nil {
-		return nil, err
+	// supports ONLY CSI volumes
+	if pv.Spec.PersistentVolumeSource.CSI == nil {
+		return "", fmt.Errorf("snapshotting non-CSI volumes is not supported, snapshot:%s/%s", snapshot.Namespace, snapshot.Name)
 	}
-	return storageclass, nil
+	return pv.Spec.PersistentVolumeSource.CSI.Driver, nil
 }
 
 // getSnapshotClass is a helper function to get snapshot class from the class name.
@@ -1186,8 +1177,10 @@ func (ctrl *csiSnapshotCommonController) getSnapshotClass(className string) (*cr
 	return class, nil
 }
 
-// SetDefaultSnapshotClass is a helper function to figure out the default snapshot class from
-// PVC/PV StorageClass and update VolumeSnapshot with this snapshot class name.
+// SetDefaultSnapshotClass is a helper function to figure out the default snapshot class.
+// For pre-provisioned case, it's an no-op.
+// For dynamic provisioning, it gets the default SnapshotClasses in the system if there is any(could be multiple),
+// and finds the one with the same CSI Driver as the PV from which a snapshot will be taken.
 func (ctrl *csiSnapshotCommonController) SetDefaultSnapshotClass(snapshot *crdv1.VolumeSnapshot) (*crdv1.VolumeSnapshotClass, *crdv1.VolumeSnapshot, error) {
 	klog.V(5).Infof("SetDefaultSnapshotClass for snapshot [%s]", snapshot.Name)
 
@@ -1197,21 +1190,23 @@ func (ctrl *csiSnapshotCommonController) SetDefaultSnapshotClass(snapshot *crdv1
 		return nil, snapshot, nil
 	}
 
-	storageclass, err := ctrl.getStorageClassFromVolumeSnapshot(snapshot)
+	// Find default snapshot class if available
+	list, err := ctrl.classLister.List(labels.Everything())
 	if err != nil {
 		return nil, nil, err
 	}
-	// Find default snapshot class if available
-	list, err := ctrl.classLister.List(labels.Everything())
+
+	pvDriver, err := ctrl.pvDriverFromSnapshot(snapshot)
 	if err != nil {
+		klog.Errorf("failed to get pv csi driver from snapshot %s/%s: %q", snapshot.Namespace, snapshot.Name, err)
 		return nil, nil, err
 	}
-	defaultClasses := []*crdv1.VolumeSnapshotClass{}
 
+	defaultClasses := []*crdv1.VolumeSnapshotClass{}
 	for _, class := range list {
-		if utils.IsDefaultAnnotation(class.ObjectMeta) && storageclass.Provisioner == class.Driver { //&& ctrl.snapshotterName == class.Snapshotter {
+		if utils.IsDefaultAnnotation(class.ObjectMeta) && pvDriver == class.Driver {
 			defaultClasses = append(defaultClasses, class)
-			klog.V(5).Infof("get defaultClass added: %s", class.Name)
+			klog.V(5).Infof("get defaultClass added: %s, driver: %s", class.Name, pvDriver)
 		}
 	}
 	if len(defaultClasses) == 0 {


Test patch:
diff --git a/pkg/common-controller/framework_test.go b/pkg/common-controller/framework_test.go
index bc481c91f..62c85d66c 100644
--- a/pkg/common-controller/framework_test.go
+++ b/pkg/common-controller/framework_test.go
@@ -36,7 +36,6 @@ import (
 	storagelisters "github.com/kubernetes-csi/external-snapshotter/client/v3/listers/volumesnapshot/v1beta1"
 	"github.com/kubernetes-csi/external-snapshotter/v3/pkg/utils"
 	v1 "k8s.io/api/core/v1"
-	storagev1 "k8s.io/api/storage/v1"
 	"k8s.io/apimachinery/pkg/api/resource"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
@@ -94,8 +93,6 @@ type controllerTest struct {
 	initialVolumes []*v1.PersistentVolume
 	// Initial content of controller claim cache.
 	initialClaims []*v1.PersistentVolumeClaim
-	// Initial content of controller StorageClass cache.
-	initialStorageClasses []*storagev1.StorageClass
 	// Initial content of controller Secret cache.
 	initialSecrets []*v1.Secret
 	// Expected events - any event with prefix will pass, we don't check full
@@ -138,7 +135,6 @@ var noerrors = []reactorError{}
 //   the list.
 type snapshotReactor struct {
 	secrets              map[string]*v1.Secret
-	storageClasses       map[string]*storagev1.StorageClass
 	volumes              map[string]*v1.PersistentVolume
 	claims               map[string]*v1.PersistentVolumeClaim
 	contents             map[string]*crdv1.VolumeSnapshotContent
@@ -372,16 +368,6 @@ func (r *snapshotReactor) React(action core.Action) (handled bool, ret runtime.O
 		klog.V(4).Infof("saved updated claim %s", claim.Name)
 		return true, claim, nil
 
-	case action.Matches("get", "storageclasses"):
-		name := action.(core.GetAction).GetName()
-		storageClass, found := r.storageClasses[name]
-		if found {
-			klog.V(4).Infof("GetStorageClass: found %s", storageClass.Name)
-			return true, storageClass, nil
-		}
-		klog.V(4).Infof("GetStorageClass: storageClass %s not found", name)
-		return true, nil, fmt.Errorf("cannot find storageClass %s", name)
-
 	case action.Matches("get", "secrets"):
 		name := action.(core.GetAction).GetName()
 		secret, found := r.secrets[name]
@@ -710,7 +696,6 @@ func (r *snapshotReactor) addSnapshotEvent(snapshot *crdv1.VolumeSnapshot) {
 func newSnapshotReactor(kubeClient *kubefake.Clientset, client *fake.Clientset, ctrl *csiSnapshotCommonController, fakeVolumeWatch, fakeClaimWatch *watch.FakeWatcher, errors []reactorError) *snapshotReactor {
 	reactor := &snapshotReactor{
 		secrets:           make(map[string]*v1.Secret),
-		storageClasses:    make(map[string]*storagev1.StorageClass),
 		volumes:           make(map[string]*v1.PersistentVolume),
 		claims:            make(map[string]*v1.PersistentVolumeClaim),
 		snapshotClasses:   make(map[string]*crdv1.VolumeSnapshotClass),
@@ -735,7 +720,6 @@ func newSnapshotReactor(kubeClient *kubefake.Clientset, client *fake.Clientset,
 	kubeClient.AddReactor("get", "persistentvolumeclaims", reactor.React)
 	kubeClient.AddReactor("update", "persistentvolumeclaims", reactor.React)
 	kubeClient.AddReactor("get", "persistentvolumes", reactor.React)
-	kubeClient.AddReactor("get", "storageclasses", reactor.React)
 	kubeClient.AddReactor("get", "secrets", reactor.React)
 
 	return reactor
@@ -1293,9 +1277,6 @@ func runSyncTests(t *testing.T, tests []controllerTest, snapshotClasses []*crdv1
 		for _, volume := range test.initialVolumes {
 			reactor.volumes[volume.Name] = volume
 		}
-		for _, storageClass := range test.initialStorageClasses {
-			reactor.storageClasses[storageClass.Name] = storageClass
-		}
 		for _, secret := range test.initialSecrets {
 			reactor.secrets[secret.Name] = secret
 		}
@@ -1358,9 +1339,6 @@ func runFinalizerTests(t *testing.T, tests []controllerTest, snapshotClasses []*
 		for _, volume := range test.initialVolumes {
 			reactor.volumes[volume.Name] = volume
 		}
-		for _, storageClass := range test.initialStorageClasses {
-			reactor.storageClasses[storageClass.Name] = storageClass
-		}
 		for _, secret := range test.initialSecrets {
 			reactor.secrets[secret.Name] = secret
 		}
@@ -1498,9 +1476,6 @@ func runUpdateSnapshotClassTests(t *testing.T, tests []controllerTest, snapshotC
 		for _, volume := range test.initialVolumes {
 			reactor.volumes[volume.Name] = volume
 		}
-		for _, storageClass := range test.initialStorageClasses {
-			reactor.storageClasses[storageClass.Name] = storageClass
-		}
 		for _, secret := range test.initialSecrets {
 			reactor.secrets[secret.Name] = secret
 		}
diff --git a/pkg/common-controller/snapshot_create_test.go b/pkg/common-controller/snapshot_create_test.go
index 6acc23362..0eb27e8f4 100644
--- a/pkg/common-controller/snapshot_create_test.go
+++ b/pkg/common-controller/snapshot_create_test.go
@@ -23,7 +23,6 @@ import (
 
 	crdv1 "github.com/kubernetes-csi/external-snapshotter/client/v3/apis/volumesnapshot/v1beta1"
 	v1 "k8s.io/api/core/v1"
-	storage "k8s.io/api/storage/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
@@ -39,27 +38,6 @@ var metaTimeNowUnix = &metav1.Time{
 var defaultSize int64 = 1000
 var deletePolicy = crdv1.VolumeSnapshotContentDelete
 var retainPolicy = crdv1.VolumeSnapshotContentRetain
-var sameDriverStorageClass = &storage.StorageClass{
-	TypeMeta: metav1.TypeMeta{
-		Kind: "StorageClass",
-	},
-	ObjectMeta: metav1.ObjectMeta{
-		Name: sameDriver,
-	},
-	Provisioner: mockDriverName,
-	Parameters:  class1Parameters,
-}
-
-var diffDriverStorageClass = &storage.StorageClass{
-	TypeMeta: metav1.TypeMeta{
-		Kind: "StorageClass",
-	},
-	ObjectMeta: metav1.ObjectMeta{
-		Name: diffDriver,
-	},
-	Provisioner: mockDriverName,
-	Parameters:  class1Parameters,
-}
 
 // Test single call to SyncSnapshot, expecting create snapshot to happen.
 // 1. Fill in the controller with initial data
@@ -73,8 +51,8 @@ func TestCreateSnapshotSync(t *testing.T) {
 			expectedContents:  newContentArrayNoStatus("snapcontent-snapuid6-1", "snapuid6-1", "snap6-1", "sid6-1", classGold, "", "pv-handle6-1", deletionPolicy, nil, nil, false, false),
 			initialSnapshots:  newSnapshotArray("snap6-1", "snapuid6-1", "claim6-1", "", classGold, "", &False, nil, nil, nil, false, true, nil),
 			expectedSnapshots: newSnapshotArray("snap6-1", "snapuid6-1", "claim6-1", "", classGold, "snapcontent-snapuid6-1", &False, nil, nil, nil, false, true, nil),
-			initialClaims:     newClaimArray("claim6-1", "pvc-uid6-1", "1Gi", "volume6-1", v1.ClaimBound, &classEmpty),
-			initialVolumes:    newVolumeArray("volume6-1", "pv-uid6-1", "pv-handle6-1", "1Gi", "pvc-uid6-1", "claim6-1", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classEmpty),
+			initialClaims:     newClaimArray("claim6-1", "pvc-uid6-1", "1Gi", "volume6-1", v1.ClaimBound, &classGold),
+			initialVolumes:    newVolumeArray("volume6-1", "pv-uid6-1", "pv-handle6-1", "1Gi", "pvc-uid6-1", "claim6-1", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classGold),
 			errors:            noerrors,
 			test:              testSyncSnapshot,
 		},
@@ -108,18 +86,17 @@ func TestCreateSnapshotSync(t *testing.T) {
 			test:              testSyncSnapshot,
 		},
 		{
-			name:                  "7-3 - fail to create snapshot without snapshot class ",
-			initialContents:       nocontents,
-			expectedContents:      nocontents,
-			initialSnapshots:      newSnapshotArray("snap7-3", "snapuid7-3", "claim7-3", "", "", "", &False, nil, nil, nil, false, true, nil),
-			expectedSnapshots:     newSnapshotArray("snap7-3", "snapuid7-3", "claim7-3", "", "", "", &False, nil, nil, newVolumeError("Failed to create snapshot content with error failed to get input parameters to create snapshot snap7-3: \"failed to take snapshot snap7-3 without a snapshot class\""), false, true, nil),
-			initialClaims:         newClaimArray("claim7-3", "pvc-uid7-3", "1Gi", "volume7-3", v1.ClaimBound, &classEmpty),
-			initialVolumes:        newVolumeArray("volume7-3", "pv-uid7-3", "pv-handle7-3", "1Gi", "pvc-uid7-3", "claim7-3", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classEmpty),
-			initialStorageClasses: []*storage.StorageClass{diffDriverStorageClass},
-			expectedEvents:        []string{"Warning SnapshotContentCreationFailed"},
-			errors:                noerrors,
-			expectSuccess:         false,
-			test:                  testSyncSnapshot,
+			name:              "7-3 - fail to create snapshot without snapshot class ",
+			initialContents:   nocontents,
+			expectedContents:  nocontents,
+			initialSnapshots:  newSnapshotArray("snap7-3", "snapuid7-3", "claim7-3", "", "", "", &False, nil, nil, nil, false, true, nil),
+			expectedSnapshots: newSnapshotArray("snap7-3", "snapuid7-3", "claim7-3", "", "", "", &False, nil, nil, newVolumeError("Failed to create snapshot content with error failed to get input parameters to create snapshot snap7-3: \"failed to take snapshot snap7-3 without a snapshot class\""), false, true, nil),
+			initialClaims:     newClaimArray("claim7-3", "pvc-uid7-3", "1Gi", "volume7-3", v1.ClaimBound, &classEmpty),
+			initialVolumes:    newVolumeArray("volume7-3", "pv-uid7-3", "pv-handle7-3", "1Gi", "pvc-uid7-3", "claim7-3", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classEmpty),
+			expectedEvents:    []string{"Warning SnapshotContentCreationFailed"},
+			errors:            noerrors,
+			expectSuccess:     false,
+			test:              testSyncSnapshot,
 		},
 		{
 			name:              "7-4 - fail create snapshot with no-existing claim",
@@ -127,7 +104,7 @@ func TestCreateSnapshotSync(t *testing.T) {
 			expectedContents:  nocontents,
 			initialSnapshots:  newSnapshotArray("snap7-4", "snapuid7-4", "claim7-4", "", classGold, "", &False, nil, nil, nil, false, true, nil),
 			expectedSnapshots: newSnapshotArray("snap7-4", "snapuid7-4", "claim7-4", "", classGold, "", &False, nil, nil, newVolumeError("Failed to create snapshot content with error snapshot controller failed to update snap7-4 on API server: cannot get claim from snapshot"), false, true, nil),
-			initialVolumes:    newVolumeArray("volume7-4", "pv-uid7-4", "pv-handle7-4", "1Gi", "pvc-uid7-4", "claim7-4", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classEmpty),
+			initialVolumes:    newVolumeArray("volume7-4", "pv-uid7-4", "pv-handle7-4", "1Gi", "pvc-uid7-4", "claim7-4", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classGold),
 			expectedEvents:    []string{"Warning SnapshotContentCreationFailed"},
 			errors:            noerrors,
 			expectSuccess:     false,
@@ -139,7 +116,7 @@ func TestCreateSnapshotSync(t *testing.T) {
 			expectedContents:  nocontents,
 			initialSnapshots:  newSnapshotArray("snap7-5", "snapuid7-5", "claim7-5", "", classGold, "", &False, nil, nil, nil, false, true, nil),
 			expectedSnapshots: newSnapshotArray("snap7-5", "snapuid7-5", "claim7-5", "", classGold, "", &False, nil, nil, newVolumeError("Failed to create snapshot content with error failed to get input parameters to create snapshot snap7-5: \"failed to retrieve PV volume7-5 from the API server: \\\"cannot find volume volume7-5\\\"\""), false, true, nil),
-			initialClaims:     newClaimArray("claim7-5", "pvc-uid7-5", "1Gi", "volume7-5", v1.ClaimBound, &classEmpty),
+			initialClaims:     newClaimArray("claim7-5", "pvc-uid7-5", "1Gi", "volume7-5", v1.ClaimBound, &classGold),
 			expectedEvents:    []string{"Warning SnapshotContentCreationFailed"},
 			errors:            noerrors,
 			expectSuccess:     false,
@@ -152,7 +129,7 @@ func TestCreateSnapshotSync(t *testing.T) {
 			expectedContents:  nocontents,
 			initialSnapshots:  newSnapshotArray("snap7-6", "snapuid7-6", "claim7-6", "", classGold, "", &False, nil, nil, nil, false, true, nil),
 			expectedSnapshots: newSnapshotArray("snap7-6", "snapuid7-6", "claim7-6", "", classGold, "", &False, nil, nil, newVolumeError("Failed to create snapshot content with error failed to get input parameters to create snapshot snap7-6: \"the PVC claim7-6 is not yet bound to a PV, will not attempt to take a snapshot\""), false, true, nil),
-			initialClaims:     newClaimArray("claim7-6", "pvc-uid7-6", "1Gi", "", v1.ClaimPending, &classEmpty),
+			initialClaims:     newClaimArray("claim7-6", "pvc-uid7-6", "1Gi", "", v1.ClaimPending, &classGold),
 			expectedEvents:    []string{"Warning SnapshotContentCreationFailed"},
 			errors:            noerrors,
 			expectSuccess:     false,
@@ -165,8 +142,8 @@ func TestCreateSnapshotSync(t *testing.T) {
 			expectedContents:  newContentArray("snapcontent-snapuid7-7", "snapuid7-7", "snap7-7", "sid7-7", classGold, "", "pv-handle7-7", deletionPolicy, nil, nil, false),
 			initialSnapshots:  newSnapshotArray("snap7-7", "snapuid7-7", "claim7-7", "", classGold, "snapcontent-snapuid7-7", &True, nil, nil, nil, false, true, nil),
 			expectedSnapshots: newSnapshotArray("snap7-7", "snapuid7-7", "claim7-7", "", classGold, "snapcontent-snapuid7-7", &True, nil, nil, nil, false, true, nil),
-			initialClaims:     newClaimArrayFinalizer("claim7-7", "pvc-uid7-7", "1Gi", "volume7-7", v1.ClaimBound, &classEmpty),
-			initialVolumes:    newVolumeArray("volume7-7", "pv-uid7-7", "pv-handle7-7", "1Gi", "pvc-uid7-7", "claim7-7", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classEmpty),
+			initialClaims:     newClaimArrayFinalizer("claim7-7", "pvc-uid7-7", "1Gi", "volume7-7", v1.ClaimBound, &classGold),
+			initialVolumes:    newVolumeArray("volume7-7", "pv-uid7-7", "pv-handle7-7", "1Gi", "pvc-uid7-7", "claim7-7", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classGold),
 			errors: []reactorError{
 				{"update", "persistentvolumeclaims", errors.New("mock update error")},
 				{"update", "persistentvolumeclaims", errors.New("mock update error")},
@@ -181,8 +158,8 @@ func TestCreateSnapshotSync(t *testing.T) {
 			expectedContents:  newContentArrayNoStatus("snapcontent-snapuid7-9", "snapuid7-9", "snap7-9", "sid7-9", classGold, "", "pv-handle7-9", deletionPolicy, nil, nil, false, false),
 			initialSnapshots:  newSnapshotArray("snap7-9", "snapuid7-9", "claim7-9", "", classGold, "", &False, nil, nil, nil, false, true, nil),
 			expectedSnapshots: newSnapshotArray("snap7-9", "snapuid7-9", "claim7-9", "", classGold, "", &False, nil, nil, nil, false, true, nil),
-			initialClaims:     newClaimArray("claim7-9", "pvc-uid7-9", "1Gi", "volume7-9", v1.ClaimBound, &classEmpty),
-			initialVolumes:    newVolumeArray("volume7-9", "pv-uid7-9", "pv-handle7-9", "1Gi", "pvc-uid7-9", "claim7-9", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classEmpty),
+			initialClaims:     newClaimArray("claim7-9", "pvc-uid7-9", "1Gi", "volume7-9", v1.ClaimBound, &classGold),
+			initialVolumes:    newVolumeArray("volume7-9", "pv-uid7-9", "pv-handle7-9", "1Gi", "pvc-uid7-9", "claim7-9", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classGold),
 			errors: []reactorError{
 				{"update", "volumesnapshots", errors.New("mock update error")},
 				{"update", "volumesnapshots", errors.New("mock update error")},
@@ -204,17 +181,13 @@ func TestCreateSnapshotSync(t *testing.T) {
 			test:              testSyncSnapshot,
 		},
 		{
-			// TODO(xiangqian): this test case needs to be
-			// revisited the scenario
-			// of VolumeSnapshotContent saving failure. Since there will be no content object
-			// in API server, it could potentially cause leaking issue
 			name:              "7-11 - fail create snapshot due to cannot save snapshot content",
 			initialContents:   nocontents,
 			expectedContents:  nocontents,
 			initialSnapshots:  newSnapshotArray("snap7-11", "snapuid7-11", "claim7-11", "", classGold, "", &False, nil, nil, nil, false, true, nil),
 			expectedSnapshots: newSnapshotArray("snap7-11", "snapuid7-11", "claim7-11", "", classGold, "", &False, nil, nil, newVolumeError("Failed to create snapshot content with error snapshot controller failed to update default/snap7-11 on API server: mock create error"), false, true, nil),
-			initialClaims:     newClaimArray("claim7-11", "pvc-uid7-11", "1Gi", "volume7-11", v1.ClaimBound, &classEmpty),
-			initialVolumes:    newVolumeArray("volume7-11", "pv-uid7-11", "pv-handle7-11", "1Gi", "pvc-uid7-11", "claim7-11", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classEmpty),
+			initialClaims:     newClaimArray("claim7-11", "pvc-uid7-11", "1Gi", "volume7-11", v1.ClaimBound, &classGold),
+			initialVolumes:    newVolumeArray("volume7-11", "pv-uid7-11", "pv-handle7-11", "1Gi", "pvc-uid7-11", "claim7-11", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classGold),
 			errors: []reactorError{
 				{"create", "volumesnapshotcontents", errors.New("mock create error")},
 				{"create", "volumesnapshotcontents", errors.New("mock create error")},
diff --git a/pkg/common-controller/snapshotclass_test.go b/pkg/common-controller/snapshotclass_test.go
index 1930c6c84..1ff32ae3c 100644
--- a/pkg/common-controller/snapshotclass_test.go
+++ b/pkg/common-controller/snapshotclass_test.go
@@ -17,11 +17,7 @@ limitations under the License.
 package common_controller
 
 import (
-	"errors"
-
 	v1 "k8s.io/api/core/v1"
-	storage "k8s.io/api/storage/v1"
-
 	"testing"
 )
 
@@ -34,70 +30,51 @@ func TestUpdateSnapshotClass(t *testing.T) {
 	tests := []controllerTest{
 		{
 			// default snapshot class name should be set
-			name:                  "1-1 - default snapshot class name should be set",
-			initialContents:       nocontents,
-			initialSnapshots:      newSnapshotArray("snap1-1", "snapuid1-1", "claim1-1", "", "", "", &True, nil, nil, nil, false, true, nil),
-			expectedSnapshots:     newSnapshotArray("snap1-1", "snapuid1-1", "claim1-1", "", defaultClass, "", &True, nil, nil, nil, false, true, nil),
-			initialClaims:         newClaimArray("claim1-1", "pvc-uid1-1", "1Gi", "volume1-1", v1.ClaimBound, &sameDriver),
-			initialVolumes:        newVolumeArray("volume1-1", "pv-uid1-1", "pv-handle1-1", "1Gi", "pvc-uid1-1", "claim1-1", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classEmpty),
-			initialStorageClasses: []*storage.StorageClass{sameDriverStorageClass},
-			expectedEvents:        noevents,
-			errors:                noerrors,
-			test:                  testUpdateSnapshotClass,
+			name:              "1-1 - default snapshot class name should be set",
+			initialContents:   nocontents,
+			initialSnapshots:  newSnapshotArray("snap1-1", "snapuid1-1", "claim1-1", "", "", "", &True, nil, nil, nil, false, true, nil),
+			expectedSnapshots: newSnapshotArray("snap1-1", "snapuid1-1", "claim1-1", "", defaultClass, "", &True, nil, nil, nil, false, true, nil),
+			initialClaims:     newClaimArray("claim1-1", "pvc-uid1-1", "1Gi", "volume1-1", v1.ClaimBound, &sameDriver),
+			initialVolumes:    newVolumeArray("volume1-1", "pv-uid1-1", "pv-handle1-1", "1Gi", "pvc-uid1-1", "claim1-1", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, sameDriver),
+			expectedEvents:    noevents,
+			errors:            noerrors,
+			test:              testUpdateSnapshotClass,
 		},
 		{
 			// snapshot class name already set
-			name:                  "1-2 - snapshot class name already set",
-			initialContents:       nocontents,
-			initialSnapshots:      newSnapshotArray("snap1-2", "snapuid1-2", "claim1-2", "", defaultClass, "", &True, nil, nil, nil, false, true, nil),
-			expectedSnapshots:     newSnapshotArray("snap1-2", "snapuid1-2", "claim1-2", "", defaultClass, "", &True, nil, nil, nil, false, true, nil),
-			initialClaims:         newClaimArray("claim1-2", "pvc-uid1-2", "1Gi", "volume1-2", v1.ClaimBound, &sameDriver),
-			initialVolumes:        newVolumeArray("volume1-2", "pv-uid1-2", "pv-handle1-2", "1Gi", "pvc-uid1-2", "claim1-2", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classEmpty),
-			initialStorageClasses: []*storage.StorageClass{sameDriverStorageClass},
-			expectedEvents:        noevents,
-			errors:                noerrors,
-			test:                  testUpdateSnapshotClass,
+			name:              "1-2 - snapshot class name already set",
+			initialContents:   nocontents,
+			initialSnapshots:  newSnapshotArray("snap1-2", "snapuid1-2", "claim1-2", "", defaultClass, "", &True, nil, nil, nil, false, true, nil),
+			expectedSnapshots: newSnapshotArray("snap1-2", "snapuid1-2", "claim1-2", "", defaultClass, "", &True, nil, nil, nil, false, true, nil),
+			initialClaims:     newClaimArray("claim1-2", "pvc-uid1-2", "1Gi", "volume1-2", v1.ClaimBound, &sameDriver),
+			initialVolumes:    newVolumeArray("volume1-2", "pv-uid1-2", "pv-handle1-2", "1Gi", "pvc-uid1-2", "claim1-2", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, sameDriver),
+			expectedEvents:    noevents,
+			errors:            noerrors,
+			test:              testUpdateSnapshotClass,
 		},
 		{
 			// default snapshot class not found
-			name:                  "1-3 - snapshot class name not found",
-			initialContents:       nocontents,
-			initialSnapshots:      newSnapshotArray("snap1-3", "snapuid1-3", "claim1-3", "", "missing-class", "", &True, nil, nil, nil, false, true, nil),
-			expectedSnapshots:     newSnapshotArray("snap1-3", "snapuid1-3", "claim1-3", "", "missing-class", "", &False, nil, nil, newVolumeError("Failed to get snapshot class with error volumesnapshotclass.snapshot.storage.k8s.io \"missing-class\" not found"), false, true, nil),
-			initialClaims:         newClaimArray("claim1-3", "pvc-uid1-3", "1Gi", "volume1-3", v1.ClaimBound, &sameDriver),
-			initialVolumes:        newVolumeArray("volume1-3", "pv-uid1-3", "pv-handle1-3", "1Gi", "pvc-uid1-3", "claim1-3", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classEmpty),
-			initialStorageClasses: []*storage.StorageClass{sameDriverStorageClass},
-			expectedEvents:        []string{"Warning GetSnapshotClassFailed"},
-			errors:                noerrors,
-			test:                  testUpdateSnapshotClass,
-		},
-		{
-			// failed to get snapshot class from name
-			name:                  "1-4 - snapshot update with default class name failed because storageclass not found",
-			initialContents:       nocontents,
-			initialSnapshots:      newSnapshotArray("snap1-4", "snapuid1-4", "claim1-4", "", "", "", &True, nil, nil, nil, false, true, nil),
-			expectedSnapshots:     newSnapshotArray("snap1-4", "snapuid1-4", "claim1-4", "", "", "", &False, nil, nil, newVolumeError("Failed to set default snapshot class with error mock update error"), false, true, nil),
-			initialClaims:         newClaimArray("claim1-4", "pvc-uid1-4", "1Gi", "volume1-4", v1.ClaimBound, &sameDriver),
-			initialVolumes:        newVolumeArray("volume1-4", "pv-uid1-4", "pv-handle1-4", "1Gi", "pvc-uid1-4", "claim1-4", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classEmpty),
-			initialStorageClasses: []*storage.StorageClass{sameDriverStorageClass},
-			expectedEvents:        []string{"Warning SetDefaultSnapshotClassFailed"},
-			errors: []reactorError{
-				{"get", "storageclasses", errors.New("mock update error")},
-			},
-			test: testUpdateSnapshotClass,
+			name:              "1-3 - snapshot class name not found",
+			initialContents:   nocontents,
+			initialSnapshots:  newSnapshotArray("snap1-3", "snapuid1-3", "claim1-3", "", "missing-class", "", &True, nil, nil, nil, false, true, nil),
+			expectedSnapshots: newSnapshotArray("snap1-3", "snapuid1-3", "claim1-3", "", "missing-class", "", &False, nil, nil, newVolumeError("Failed to get snapshot class with error volumesnapshotclass.snapshot.storage.k8s.io \"missing-class\" not found"), false, true, nil),
+			initialClaims:     newClaimArray("claim1-3", "pvc-uid1-3", "1Gi", "volume1-3", v1.ClaimBound, &sameDriver),
+			initialVolumes:    newVolumeArray("volume1-3", "pv-uid1-3", "pv-handle1-3", "1Gi", "pvc-uid1-3", "claim1-3", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, sameDriver),
+			expectedEvents:    []string{"Warning GetSnapshotClassFailed"},
+			errors:            noerrors,
+			test:              testUpdateSnapshotClass,
 		},
 		{
 			// PVC does not exist
-			name:                  "1-5 - snapshot update with default class name failed because PVC was not found",
-			initialContents:       nocontents,
-			initialSnapshots:      newSnapshotArray("snap1-5", "snapuid1-5", "claim1-5", "", "", "", &True, nil, nil, nil, false, true, nil),
-			expectedSnapshots:     newSnapshotArray("snap1-5", "snapuid1-5", "claim1-5", "", "", "", &False, nil, nil, newVolumeError("Failed to set default snapshot class with error failed to retrieve PVC claim1-5 from the lister: \"persistentvolumeclaim \\\"claim1-5\\\" not found\""), false, true, nil),
-			initialClaims:         nil,
-			initialVolumes:        nil,
-			initialStorageClasses: []*storage.StorageClass{},
-			expectedEvents:        []string{"Warning SetDefaultSnapshotClassFailed"},
-			errors:                noerrors,
-			test:                  testUpdateSnapshotClass,
+			name:              "1-5 - snapshot update with default class name failed because PVC was not found",
+			initialContents:   nocontents,
+			initialSnapshots:  newSnapshotArray("snap1-5", "snapuid1-5", "claim1-5", "", "", "", &True, nil, nil, nil, false, true, nil),
+			expectedSnapshots: newSnapshotArray("snap1-5", "snapuid1-5", "claim1-5", "", "", "", &False, nil, nil, newVolumeError("Failed to set default snapshot class with error failed to retrieve PVC claim1-5 from the lister: \"persistentvolumeclaim \\\"claim1-5\\\" not found\""), false, true, nil),
+			initialClaims:     nil,
+			initialVolumes:    nil,
+			expectedEvents:    []string{"Warning SetDefaultSnapshotClassFailed"},
+			errors:            noerrors,
+			test:              testUpdateSnapshotClass,
 		},
 	}