Merge pull request #113238 from pacoxu/storage-quantity-warning

pvc storage request warning for fractional byte value

pkg/api/persistentvolumeclaim/util.go

@@ -17,6 +17,9 @@ limitations under the License.
 package persistentvolumeclaim
 
 import (
+	"fmt"
+
+	"k8s.io/apimachinery/pkg/util/validation/field"
 	utilfeature "k8s.io/apiserver/pkg/util/feature"
 	"k8s.io/kubernetes/pkg/apis/core"
 	"k8s.io/kubernetes/pkg/features"
@@ -151,3 +154,29 @@ func allocatedResourcesInUse(oldPVC *core.PersistentVolumeClaim) bool {
 
 	return false
 }
+
+func GetWarningsForPersistentVolumeClaim(pv *core.PersistentVolumeClaim) []string {
+	if pv == nil {
+		return nil
+	}
+
+	return GetWarningsForPersistentVolumeClaimSpec(field.NewPath("spec"), pv.Spec)
+}
+
+func GetWarningsForPersistentVolumeClaimSpec(fieldPath *field.Path, pvSpec core.PersistentVolumeClaimSpec) []string {
+
+	var warnings []string
+	requestValue := pvSpec.Resources.Requests[core.ResourceStorage]
+	if requestValue.MilliValue()%int64(1000) != int64(0) {
+		warnings = append(warnings, fmt.Sprintf(
+			"%s: fractional byte value %q is invalid, must be an integer",
+			fieldPath.Child("resources").Child("requests").Key(core.ResourceStorage.String()), requestValue.String()))
+	}
+	limitValue := pvSpec.Resources.Limits[core.ResourceStorage]
+	if limitValue.MilliValue()%int64(1000) != int64(0) {
+		warnings = append(warnings, fmt.Sprintf(
+			"%s: fractional byte value %q is invalid, must be an integer",
+			fieldPath.Child("resources").Child("limits").Key(core.ResourceStorage.String()), limitValue.String()))
+	}
+	return warnings
+}

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,79 @@ 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 requests no warning",
+			template: &core.PersistentVolumeClaim{
+				Spec: core.PersistentVolumeClaimSpec{
+					Resources: core.ResourceRequirements{
+						Requests: core.ResourceList{
+							core.ResourceStorage: resource.MustParse("200Mi"),
+						},
+						Limits: 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"),
+						},
+						Limits: core.ResourceList{
+							core.ResourceStorage: resource.MustParse("100m"),
+						},
+					},
+				},
+			},
+			expected: []string{
+				`spec.resources.requests[storage]: fractional byte value "200m" is invalid, must be an integer`,
+				`spec.resources.limits[storage]: fractional byte value "100m" 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)
+			}
+		})
+
+	}
+}

pkg/api/pod/warnings.go

@@ -25,6 +25,7 @@ import (
 	"k8s.io/apimachinery/pkg/util/sets"
 	"k8s.io/apimachinery/pkg/util/validation/field"
 	nodeapi "k8s.io/kubernetes/pkg/api/node"
+	pvcutil "k8s.io/kubernetes/pkg/api/persistentvolumeclaim"
 	api "k8s.io/kubernetes/pkg/apis/core"
 	"k8s.io/kubernetes/pkg/apis/core/pods"
 )
@@ -151,6 +152,9 @@ func warningsForPodSpecAndMeta(fieldPath *field.Path, podSpec *api.PodSpec, meta
 		if v.Glusterfs != nil {
 			warnings = append(warnings, fmt.Sprintf("%s: deprecated in v1.25, this feature will be removed soon after in a subsequent release", fieldPath.Child("spec", "volumes").Index(i).Child("glusterfs")))
 		}
+		if v.Ephemeral != nil && v.Ephemeral.VolumeClaimTemplate != nil {
+			warnings = append(warnings, pvcutil.GetWarningsForPersistentVolumeClaimSpec(fieldPath.Child("spec", "volumes").Index(i).Child("ephemeral").Child("volumeClaimTemplate").Child("spec"), v.Ephemeral.VolumeClaimTemplate.Spec)...)
+		}
 	}
 
 	// duplicate hostAliases (#91670, #58477)

pkg/api/pod/warnings_test.go

@@ -459,6 +459,36 @@ func TestWarnings(t *testing.T) {
 			},
 			expected: []string{},
 		},
+		{
+			name: "pod with ephemeral volume source 200Mi",
+			template: &api.PodTemplateSpec{
+				ObjectMeta: metav1.ObjectMeta{},
+				Spec: api.PodSpec{Volumes: []api.Volume{
+					{Name: "ephemeral-volume", VolumeSource: api.VolumeSource{Ephemeral: &api.EphemeralVolumeSource{
+						VolumeClaimTemplate: &api.PersistentVolumeClaimTemplate{
+							Spec: api.PersistentVolumeClaimSpec{Resources: api.ResourceRequirements{
+								Requests: api.ResourceList{api.ResourceStorage: resource.MustParse("200Mi")}}},
+						},
+					}}}}},
+			},
+			expected: []string{},
+		},
+		{
+			name: "pod with ephemeral volume source 200m",
+			template: &api.PodTemplateSpec{
+				ObjectMeta: metav1.ObjectMeta{},
+				Spec: api.PodSpec{Volumes: []api.Volume{
+					{Name: "ephemeral-volume", VolumeSource: api.VolumeSource{Ephemeral: &api.EphemeralVolumeSource{
+						VolumeClaimTemplate: &api.PersistentVolumeClaimTemplate{
+							Spec: api.PersistentVolumeClaimSpec{Resources: api.ResourceRequirements{
+								Requests: api.ResourceList{api.ResourceStorage: resource.MustParse("200m")}}},
+						},
+					}}}}},
+			},
+			expected: []string{
+				`spec.volumes[0].ephemeral.volumeClaimTemplate.spec.resources.requests[storage]: fractional byte value "200m" is invalid, must be an integer`,
+			},
+		},
 	}
 
 	for _, tc := range testcases {

pkg/registry/apps/statefulset/strategy.go

@@ -26,6 +26,7 @@ import (
 	"k8s.io/apiserver/pkg/storage/names"
 	utilfeature "k8s.io/apiserver/pkg/util/feature"
 	"k8s.io/kubernetes/pkg/api/legacyscheme"
+	pvcutil "k8s.io/kubernetes/pkg/api/persistentvolumeclaim"
 	"k8s.io/kubernetes/pkg/api/pod"
 	"k8s.io/kubernetes/pkg/apis/apps"
 	"k8s.io/kubernetes/pkg/apis/apps/validation"
@@ -139,7 +140,11 @@ func (statefulSetStrategy) Validate(ctx context.Context, obj runtime.Object) fie
 // WarningsOnCreate returns warnings for the creation of the given object.
 func (statefulSetStrategy) WarningsOnCreate(ctx context.Context, obj runtime.Object) []string {
 	newStatefulSet := obj.(*apps.StatefulSet)
-	return pod.GetWarningsForPodTemplate(ctx, field.NewPath("spec", "template"), &newStatefulSet.Spec.Template, nil)
+	warnings := pod.GetWarningsForPodTemplate(ctx, field.NewPath("spec", "template"), &newStatefulSet.Spec.Template, nil)
+	for i, pvc := range newStatefulSet.Spec.VolumeClaimTemplates {
+		warnings = append(warnings, pvcutil.GetWarningsForPersistentVolumeClaimSpec(field.NewPath("spec", "volumeClaimTemplates").Index(i), pvc.Spec)...)
+	}
+	return warnings
 }
 
 // Canonicalize normalizes the object after validation.
@@ -168,6 +173,10 @@ func (statefulSetStrategy) WarningsOnUpdate(ctx context.Context, obj, old runtim
 	if newStatefulSet.Generation != oldStatefulSet.Generation {
 		warnings = pod.GetWarningsForPodTemplate(ctx, field.NewPath("spec", "template"), &newStatefulSet.Spec.Template, &oldStatefulSet.Spec.Template)
 	}
+	for i, pvc := range newStatefulSet.Spec.VolumeClaimTemplates {
+		warnings = append(warnings, pvcutil.GetWarningsForPersistentVolumeClaimSpec(field.NewPath("spec", "volumeClaimTemplates").Index(i).Child("Spec"), pvc.Spec)...)
+	}
+
 	return warnings
 }
 

pkg/registry/core/persistentvolumeclaim/strategy.go

@@ -86,7 +86,7 @@ func (persistentvolumeclaimStrategy) Validate(ctx context.Context, obj runtime.O
 
 // WarningsOnCreate returns warnings for the creation of the given object.
 func (persistentvolumeclaimStrategy) WarningsOnCreate(ctx context.Context, obj runtime.Object) []string {
-	return nil
+	return pvcutil.GetWarningsForPersistentVolumeClaim(obj.(*api.PersistentVolumeClaim))
 }
 
 // Canonicalize normalizes the object after validation.
@@ -128,7 +128,7 @@ func (persistentvolumeclaimStrategy) ValidateUpdate(ctx context.Context, obj, ol
 
 // WarningsOnUpdate returns warnings for the given update.
 func (persistentvolumeclaimStrategy) WarningsOnUpdate(ctx context.Context, obj, old runtime.Object) []string {
-	return nil
+	return pvcutil.GetWarningsForPersistentVolumeClaim(obj.(*api.PersistentVolumeClaim))
 }
 
 func (persistentvolumeclaimStrategy) AllowUnconditionalUpdate() bool {

test/e2e/framework/.import-restrictions

@@ -8,6 +8,7 @@ rules:
       - k8s.io/kubernetes/pkg/api/v1/service
       - k8s.io/kubernetes/pkg/api/pod
       - k8s.io/kubernetes/pkg/api/node
+      - k8s.io/kubernetes/pkg/api/persistentvolumeclaim
       - k8s.io/kubernetes/pkg/apis/apps
       - k8s.io/kubernetes/pkg/apis/apps/validation
       - k8s.io/kubernetes/pkg/apis/autoscaling