From ca94a89414618a93f3e3dcfc1e5afb5b0209bec1 Mon Sep 17 00:00:00 2001
From: Paco Xu <paco.xu@daocloud.io>
Date: Fri, 21 Oct 2022 13:43:40 +0800
Subject: [PATCH] pvc warning for storage request: add unit test

---
 pkg/api/persistentvolumeclaim/util.go      | 10 ++--
 pkg/api/persistentvolumeclaim/util_test.go | 70 ++++++++++++++++++++++
 2 files changed, 74 insertions(+), 6 deletions(-)

diff --git a/pkg/api/persistentvolumeclaim/util.go b/pkg/api/persistentvolumeclaim/util.go
index b8cacf83a4c..519aaeb18cf 100644
--- a/pkg/api/persistentvolumeclaim/util.go
+++ b/pkg/api/persistentvolumeclaim/util.go
@@ -19,7 +19,6 @@ package persistentvolumeclaim
 import (
 	"fmt"
 
-	"k8s.io/apimachinery/pkg/api/resource"
 	"k8s.io/apimachinery/pkg/util/validation/field"
 	utilfeature "k8s.io/apiserver/pkg/util/feature"
 	"k8s.io/kubernetes/pkg/apis/core"
@@ -161,13 +160,12 @@ func GetWarningsForPersistentVolumeClaim(pv *core.PersistentVolumeClaim) []strin
 		return nil
 	}
 	storageValue := pv.Spec.Resources.Requests[core.ResourceStorage]
-	return warningsForPersistentVolumeClaimResources(field.NewPath("spec").Child("Resources").Child("Requests").Key(core.ResourceStorage.String()), storageValue)
-}
-
-func warningsForPersistentVolumeClaimResources(fieldPath *field.Path, storageValue resource.Quantity) []string {
 	var warnings []string
 	if storageValue.MilliValue()%int64(1000) != int64(0) {
-		warnings = append(warnings, fmt.Sprintf("%s: fractional byte value %q is invalid, must be an integer", fieldPath.String(), storageValue.String()))
+		warnings = append(warnings, fmt.Sprintf(
+			"%s: fractional byte value %q is invalid, must be an integer",
+			field.NewPath("spec").Child("resources").Child("requests").Key(core.ResourceStorage.String()),
+			storageValue.String()))
 	}
 	return warnings
 }
diff --git a/pkg/api/persistentvolumeclaim/util_test.go b/pkg/api/persistentvolumeclaim/util_test.go
index 8462cde39f9..1b7d260b7d3 100644
--- a/pkg/api/persistentvolumeclaim/util_test.go
+++ b/pkg/api/persistentvolumeclaim/util_test.go
@@ -23,6 +23,7 @@ import (
 
 	"github.com/google/go-cmp/cmp"
 	"k8s.io/apimachinery/pkg/api/resource"
+	"k8s.io/apimachinery/pkg/util/sets"
 
 	utilfeature "k8s.io/apiserver/pkg/util/feature"
 	featuregatetesting "k8s.io/component-base/featuregate/testing"
@@ -397,3 +398,72 @@ func withResizeStatus(status core.PersistentVolumeClaimResizeStatus) *core.Persi
 		},
 	}
 }
+
+func TestWarnings(t *testing.T) {
+	testcases := []struct {
+		name     string
+		template *core.PersistentVolumeClaim
+		expected []string
+	}{
+		{
+			name:     "null",
+			template: nil,
+			expected: nil,
+		},
+		{
+			name: "200Mi no warning",
+			template: &core.PersistentVolumeClaim{
+				Spec: core.PersistentVolumeClaimSpec{
+					Resources: core.ResourceRequirements{
+						Requests: core.ResourceList{
+							core.ResourceStorage: resource.MustParse("200Mi"),
+						},
+					},
+				},
+			},
+			expected: nil,
+		},
+		{
+			name: "200m warning",
+			template: &core.PersistentVolumeClaim{
+				Spec: core.PersistentVolumeClaimSpec{
+					Resources: core.ResourceRequirements{
+						Requests: core.ResourceList{
+							core.ResourceStorage: resource.MustParse("200m"),
+						},
+					},
+				},
+			},
+			expected: []string{
+				`spec.resources.requests[storage]: fractional byte value "200m" is invalid, must be an integer`,
+			},
+		},
+		{
+			name: "integer no warning",
+			template: &core.PersistentVolumeClaim{
+				Spec: core.PersistentVolumeClaimSpec{
+					Resources: core.ResourceRequirements{
+						Requests: core.ResourceList{
+							core.ResourceStorage: resource.MustParse("200"),
+						},
+					},
+				},
+			},
+			expected: nil,
+		},
+	}
+
+	for _, tc := range testcases {
+		t.Run("pvcspec_"+tc.name, func(t *testing.T) {
+			actual := sets.NewString(GetWarningsForPersistentVolumeClaim(tc.template)...)
+			expected := sets.NewString(tc.expected...)
+			for _, missing := range expected.Difference(actual).List() {
+				t.Errorf("missing: %s", missing)
+			}
+			for _, extra := range actual.Difference(expected).List() {
+				t.Errorf("extra: %s", extra)
+			}
+		})
+
+	}
+}