Merge pull request #1100 from leonardoce/informer-magic

Correctly detect newly created Volume[Group]SnapshotClasses
kubernetes-csi · Jun 18, 2024 · af43adc · af43adc
2 parents 3c92522 + 4f6440c
commit af43adc
Show file tree

Hide file tree

Showing 4 changed files with 31 additions and 49 deletions.
diff --git a/pkg/common-controller/groupsnapshot_controller_helper.go b/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)
 		}

diff --git a/pkg/common-controller/snapshot_controller.go b/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)
 		}

diff --git a/pkg/utils/util.go b/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

diff --git a/pkg/utils/util_test.go b/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)
 		}