From 140502af8cbe4de0b78265f4ba025a5ada805849 Mon Sep 17 00:00:00 2001 From: Paco Xu Date: Fri, 21 Oct 2022 22:31:48 +0800 Subject: [PATCH] add warning for PVC template in statefulset and in pod ephemeral volume source --- pkg/api/persistentvolumeclaim/util.go | 19 +++++++++++--- pkg/api/persistentvolumeclaim/util_test.go | 9 ++++++- pkg/api/pod/warnings.go | 4 +++ pkg/api/pod/warnings_test.go | 30 ++++++++++++++++++++++ pkg/registry/apps/statefulset/strategy.go | 11 +++++++- test/e2e/framework/.import-restrictions | 1 + 6 files changed, 68 insertions(+), 6 deletions(-) diff --git a/pkg/api/persistentvolumeclaim/util.go b/pkg/api/persistentvolumeclaim/util.go index 519aaeb18cf..2f24c645b66 100644 --- a/pkg/api/persistentvolumeclaim/util.go +++ b/pkg/api/persistentvolumeclaim/util.go @@ -159,13 +159,24 @@ func GetWarningsForPersistentVolumeClaim(pv *core.PersistentVolumeClaim) []strin if pv == nil { return nil } - storageValue := pv.Spec.Resources.Requests[core.ResourceStorage] + + return GetWarningsForPersistentVolumeClaimSpec(field.NewPath("spec"), pv.Spec) +} + +func GetWarningsForPersistentVolumeClaimSpec(fieldPath *field.Path, pvSpec core.PersistentVolumeClaimSpec) []string { + var warnings []string - if storageValue.MilliValue()%int64(1000) != int64(0) { + 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", - field.NewPath("spec").Child("resources").Child("requests").Key(core.ResourceStorage.String()), - storageValue.String())) + 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 } diff --git a/pkg/api/persistentvolumeclaim/util_test.go b/pkg/api/persistentvolumeclaim/util_test.go index 1b7d260b7d3..a0e4f5fec63 100644 --- a/pkg/api/persistentvolumeclaim/util_test.go +++ b/pkg/api/persistentvolumeclaim/util_test.go @@ -411,13 +411,16 @@ func TestWarnings(t *testing.T) { expected: nil, }, { - name: "200Mi no warning", + 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"), + }, }, }, }, @@ -431,11 +434,15 @@ func TestWarnings(t *testing.T) { 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`, }, }, { diff --git a/pkg/api/pod/warnings.go b/pkg/api/pod/warnings.go index 146f0cca61d..f2e5d8fa01d 100644 --- a/pkg/api/pod/warnings.go +++ b/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) diff --git a/pkg/api/pod/warnings_test.go b/pkg/api/pod/warnings_test.go index 0b9bb00c915..6bab0367c69 100644 --- a/pkg/api/pod/warnings_test.go +++ b/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 { diff --git a/pkg/registry/apps/statefulset/strategy.go b/pkg/registry/apps/statefulset/strategy.go index 6d689bc5e14..7b63faa7513 100644 --- a/pkg/registry/apps/statefulset/strategy.go +++ b/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 } diff --git a/test/e2e/framework/.import-restrictions b/test/e2e/framework/.import-restrictions index be280ae1d91..8e171cee315 100644 --- a/test/e2e/framework/.import-restrictions +++ b/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