Merge pull request #173 from reasonerjt/update-get-vsclass

Update the logic to get VSClass for CSI snapshot
vmware-tanzu · May 11, 2023 · db448f3 · db448f3
2 parents b4e5fbb + fe0de37
commit db448f3
Show file tree

Hide file tree

Showing 2 changed files with 37 additions and 7 deletions.
diff --git a/internal/util/util.go b/internal/util/util.go
@@ -121,16 +121,26 @@ func GetVolumeSnapshotClassForStorageClass(provisioner string, snapshotClient sn
 	if err != nil {
 		return nil, errors.Wrap(err, "error listing volumesnapshot classes")
 	}
+	n := 0
+	var vsclass snapshotv1api.VolumeSnapshotClass
 	// We pick the volumesnapshotclass that matches the CSI driver name and has a 'velero.io/csi-volumesnapshot-class'
 	// label. This allows multiple VolumesnapshotClasses for the same driver with different values for the
 	// other fields in the spec.
 	// https://github.com/kubernetes-csi/external-snapshotter/blob/release-4.2/client/config/crd/snapshot.storage.k8s.io_volumesnapshotclasses.yaml
 	for _, sc := range snapshotClasses.Items {
 		_, hasLabelSelector := sc.Labels[VolumeSnapshotClassSelectorLabel]
-		if sc.Driver == provisioner && hasLabelSelector {
-			return &sc, nil
+		if sc.Driver == provisioner {
+			n += 1
+			vsclass = sc
+			if hasLabelSelector {
+				return &sc, nil
+			}
 		}
 	}
+	// If there's only one volumesnapshotclass for the driver, return it.
+	if n == 1 {
+		return &vsclass, nil
+	}
 	return nil, errors.Errorf("failed to get volumesnapshotclass for provisioner %s, ensure that the desired volumesnapshot class has the %s label", provisioner, VolumeSnapshotClassSelectorLabel)
 }
 

diff --git a/internal/util/util_test.go b/internal/util/util_test.go
@@ -657,7 +657,21 @@ func TestGetVolumeSnapshotClassForStorageClass(t *testing.T) {
 		Driver: "baz.csi.k8s.io",
 	}
 
-	objs := []runtime.Object{hostpathClass, fooClass, barClass, bazClass}
+	ambClass1 := &snapshotv1api.VolumeSnapshotClass{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: "amb1",
+		},
+		Driver: "amb.csi.k8s.io",
+	}
+
+	ambClass2 := &snapshotv1api.VolumeSnapshotClass{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: "amb2",
+		},
+		Driver: "amb.csi.k8s.io",
+	}
+
+	objs := []runtime.Object{hostpathClass, fooClass, barClass, bazClass, ambClass1, ambClass2}
 	fakeClient := snapshotFake.NewSimpleClientset(objs...)
 
 	testCases := []struct {
@@ -679,15 +693,21 @@ func TestGetVolumeSnapshotClassForStorageClass(t *testing.T) {
 			expectError: false,
 		},
 		{
-			name:        "should find var volumesnapshotclass",
+			name:        "should find bar volumesnapshotclass",
 			driverName:  "bar.csi.k8s.io",
 			expectedVSC: barClass,
 			expectError: false,
 		},
 		{
-			name:        "should not find foo volumesnapshotclass without \"velero.io/csi-volumesnapshot-class\" label",
+			name:        "should find baz volumesnapshotclass without \"velero.io/csi-volumesnapshot-class\" label, b/c there's only one vsclass matching the driver name",
 			driverName:  "baz.csi.k8s.io",
 			expectedVSC: bazClass,
+			expectError: false,
+		},
+		{
+			name:        "should not find amb volumesnapshotclass without \"velero.io/csi-volumesnapshot-class\" label, b/c there're  more than one vsclass matching the driver name",
+			driverName:  "amb.csi.k8s.io",
+			expectedVSC: nil,
 			expectError: true,
 		},
 		{
@@ -708,8 +728,8 @@ func TestGetVolumeSnapshotClassForStorageClass(t *testing.T) {
 				return
 			}
 
-			assert.Equalf(t, tc.expectedVSC.Name, actualVSC.Name, "unexpected volumesnapshotclass name returned. Want: %s; Got:%s", tc.name, tc.expectedVSC.Name, actualVSC.Name)
-			assert.Equalf(t, tc.expectedVSC.Driver, actualVSC.Driver, "unexpected driver name returned. Want: %s; Got:%s", tc.name, tc.expectedVSC.Driver, actualVSC.Driver)
+			assert.Equalf(t, tc.expectedVSC.Name, actualVSC.Name, "unexpected volumesnapshotclass name returned. Want: %s; Got:%s", tc.expectedVSC.Name, actualVSC.Name)
+			assert.Equalf(t, tc.expectedVSC.Driver, actualVSC.Driver, "unexpected driver name returned. Want: %s; Got:%s", tc.expectedVSC.Driver, actualVSC.Driver)
 		})
 	}
 }