Correctly detect newly created Volume[Group]SnapshotClasses

The controller is using an informer to get VolumeGroupSnapshotClasses
and VolumeSnapshotClasses. Unfortunately, objects returned by that will
not have the API Kind info set.

This prevented the IsDefaultAnnotation function from working correctly.

This patch avoid relying on the object Kind to know whether a
VolumeGroupSnapshotClass or a VolumeSnapshotClass is set as default.

See: kubernetes/client-go#308

Signed-off-by: Leonardo Cecchi <[email protected]>

Author: leonardoce

pkg/common-controller/groupsnapshot_controller_helper.go

@@ -141,7 +141,7 @@ func (ctrl *csiSnapshotCommonController) SetDefaultGroupSnapshotClass(groupSnaps
 
 	defaultClasses := []*crdv1alpha1.VolumeGroupSnapshotClass{}
 	for _, groupSnapshotClass := range list {
-		if utils.IsDefaultAnnotation(groupSnapshotClass.TypeMeta, groupSnapshotClass.ObjectMeta) && pvDriver == groupSnapshotClass.Driver {
+		if utils.IsVolumeGroupSnapshotClassDefaultAnnotation(groupSnapshotClass.ObjectMeta) && pvDriver == groupSnapshotClass.Driver {
 			defaultClasses = append(defaultClasses, groupSnapshotClass)
 			klog.V(5).Infof("get defaultGroupClass added: %s, driver: %s", groupSnapshotClass.Name, pvDriver)
 		}

pkg/common-controller/snapshot_controller.go

@@ -1400,7 +1400,7 @@ func (ctrl *csiSnapshotCommonController) SetDefaultSnapshotClass(snapshot *crdv1
 
 	defaultClasses := []*crdv1.VolumeSnapshotClass{}
 	for _, class := range list {
-		if utils.IsDefaultAnnotation(class.TypeMeta, class.ObjectMeta) && pvDriver == class.Driver {
+		if utils.IsVolumeSnapshotClassDefaultAnnotation(class.ObjectMeta) && pvDriver == class.Driver {
 			defaultClasses = append(defaultClasses, class)
 			klog.V(5).Infof("get defaultClass added: %s, driver: %s", class.Name, pvDriver)
 		}

pkg/utils/util.go

@@ -274,21 +274,16 @@ func GetDynamicSnapshotContentNameForSnapshot(snapshot *crdv1.VolumeSnapshot) st
 	return "snapcontent-" + string(snapshot.UID)
 }
 
-// IsDefaultAnnotation returns a boolean if
-// the annotation is set
-func IsDefaultAnnotation(tm metav1.TypeMeta, obj metav1.ObjectMeta) bool {
-	switch tm.Kind {
-	case "VolumeSnapshotClass":
-		if obj.Annotations[IsDefaultSnapshotClassAnnotation] == "true" {
-			return true
-		}
-	case "VolumeGroupSnapshotClass":
-		if obj.Annotations[IsDefaultGroupSnapshotClassAnnotation] == "true" {
-			return true
-		}
-	}
+// IsVolumeSnapshotClassDefaultAnnotation returns a true boolean if
+// a VolumeSnapshotClass is marked as the default one
+func IsVolumeSnapshotClassDefaultAnnotation(obj metav1.ObjectMeta) bool {
+	return obj.Annotations[IsDefaultSnapshotClassAnnotation] == "true"
+}
 
-	return false
+// IsVolumeGroupSnapshotClassDefaultAnnotation returns a true boolean if
+// a VolumeGroupSnapshotClass is marked as the default one
+func IsVolumeGroupSnapshotClassDefaultAnnotation(obj metav1.ObjectMeta) bool {
+	return obj.Annotations[IsDefaultGroupSnapshotClassAnnotation] == "true"
 }
 
 // verifyAndGetSecretNameAndNamespaceTemplate gets the values (templates) associated

pkg/utils/util_test.go

@@ -201,28 +201,21 @@ func TestRemovePrefixedCSIParams(t *testing.T) {
 	}
 }
 
-func TestIsDefaultAnnotation(t *testing.T) {
+func TestIsVolumeSnapshotClassDefaultAnnotation(t *testing.T) {
 	testcases := []struct {
 		name       string
-		typeMeta   metav1.TypeMeta
 		objectMeta metav1.ObjectMeta
 		isDefault  bool
 	}{
 		{
 			name: "no default annotation in snapshot class",
-			typeMeta: metav1.TypeMeta{
-				Kind: "VolumeSnapshotClass",
-			},
 			objectMeta: metav1.ObjectMeta{
-				Annotations: map[string]string{},
+				Annotations: nil,
 			},
 			isDefault: false,
 		},
 		{
 			name: "with default annotation in snapshot class",
-			typeMeta: metav1.TypeMeta{
-				Kind: "VolumeSnapshotClass",
-			},
 			objectMeta: metav1.ObjectMeta{
 				Annotations: map[string]string{
 					IsDefaultSnapshotClassAnnotation: "true",
@@ -232,31 +225,38 @@ func TestIsDefaultAnnotation(t *testing.T) {
 		},
 		{
 			name: "with default=false annotation in snapshot class",
-			typeMeta: metav1.TypeMeta{
-				Kind: "VolumeSnapshotClass",
-			},
 			objectMeta: metav1.ObjectMeta{
 				Annotations: map[string]string{
 					IsDefaultSnapshotClassAnnotation: "false",
 				},
 			},
 			isDefault: false,
 		},
+	}
+	for _, tc := range testcases {
+		t.Logf("test: %s", tc.name)
+		isDefault := IsVolumeSnapshotClassDefaultAnnotation(tc.objectMeta)
+		if tc.isDefault != isDefault {
+			t.Fatalf("default annotation on class incorrectly detected: %v != %v", isDefault, tc.isDefault)
+		}
+	}
+}
+
+func TestIsVolumeGroupSnapshotClassDefaultAnnotation(t *testing.T) {
+	testcases := []struct {
+		name       string
+		objectMeta metav1.ObjectMeta
+		isDefault  bool
+	}{
 		{
 			name: "no default annotation in group snapshot class",
-			typeMeta: metav1.TypeMeta{
-				Kind: "VolumeGroupSnapshotClass",
-			},
 			objectMeta: metav1.ObjectMeta{
-				Annotations: map[string]string{},
+				Annotations: nil,
 			},
 			isDefault: false,
 		},
 		{
 			name: "with default annotation in group snapshot class",
-			typeMeta: metav1.TypeMeta{
-				Kind: "VolumeGroupSnapshotClass",
-			},
 			objectMeta: metav1.ObjectMeta{
 				Annotations: map[string]string{
 					IsDefaultGroupSnapshotClassAnnotation: "true",
@@ -266,30 +266,17 @@ func TestIsDefaultAnnotation(t *testing.T) {
 		},
 		{
 			name: "with default=false annotation in group snapshot class",
-			typeMeta: metav1.TypeMeta{
-				Kind: "VolumeGroupSnapshotClass",
-			},
 			objectMeta: metav1.ObjectMeta{
 				Annotations: map[string]string{
 					IsDefaultGroupSnapshotClassAnnotation: "false",
 				},
 			},
 			isDefault: false,
 		},
-		{
-			name: "unknown kind, not a snapshot or group snapshot class",
-			typeMeta: metav1.TypeMeta{
-				Kind: "PersistentVolume",
-			},
-			objectMeta: metav1.ObjectMeta{
-				Annotations: map[string]string{},
-			},
-			isDefault: false,
-		},
 	}
 	for _, tc := range testcases {
 		t.Logf("test: %s", tc.name)
-		isDefault := IsDefaultAnnotation(tc.typeMeta, tc.objectMeta)
+		isDefault := IsVolumeGroupSnapshotClassDefaultAnnotation(tc.objectMeta)
 		if tc.isDefault != isDefault {
 			t.Fatalf("default annotation on class incorrectly detected: %v != %v", isDefault, tc.isDefault)
 		}