diff --git a/vertical-pod-autoscaler/pkg/admission-controller/resource/vpa/handler.go b/vertical-pod-autoscaler/pkg/admission-controller/resource/vpa/handler.go index 37465073aa4f..553dfaab2fdc 100644 --- a/vertical-pod-autoscaler/pkg/admission-controller/resource/vpa/handler.go +++ b/vertical-pod-autoscaler/pkg/admission-controller/resource/vpa/handler.go @@ -19,7 +19,10 @@ package vpa import ( "encoding/json" "fmt" + v1 "k8s.io/api/admission/v1" + corev1 "k8s.io/api/core/v1" + apires "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/admission-controller/resource" @@ -133,9 +136,17 @@ func validateVPA(vpa *vpa_types.VerticalPodAutoscaler, isCreate bool) error { } } for resource, min := range policy.MinAllowed { + if err := validate(resource, min); err != nil { + return err + } max, found := policy.MaxAllowed[resource] - if found && max.Cmp(min) < 0 { - return fmt.Errorf("max resource for %v is lower than min", resource) + if found { + if err := validate(resource, max); err != nil { + return err + } + if max.Cmp(min) < 0 { + return fmt.Errorf("max resource for %v is lower than min", resource) + } } } ControlledValues := policy.ControlledValues @@ -157,3 +168,27 @@ func validateVPA(vpa *vpa_types.VerticalPodAutoscaler, isCreate bool) error { return nil } + +func validate(name corev1.ResourceName, val apires.Quantity) error { + switch name { + case corev1.ResourceCPU: + return validateCPUResolution(val) + case corev1.ResourceMemory: + return validateMemoryResolution(val) + } + return nil +} + +func validateCPUResolution(val apires.Quantity) error { + if _, precissionPreserved := val.AsScale(apires.Milli); !precissionPreserved { + return fmt.Errorf("CPU [%s] must be a whole number of milli CPUs", val.String()) + } + return nil +} + +func validateMemoryResolution(val apires.Quantity) error { + if _, precissionPreserved := val.AsScale(0); !precissionPreserved { + return fmt.Errorf("Memory [%v] must be a whole number of bytes", val) + } + return nil +} \ No newline at end of file diff --git a/vertical-pod-autoscaler/pkg/admission-controller/resource/vpa/handler_test.go b/vertical-pod-autoscaler/pkg/admission-controller/resource/vpa/handler_test.go index cacadfa44523..df925c15f33d 100644 --- a/vertical-pod-autoscaler/pkg/admission-controller/resource/vpa/handler_test.go +++ b/vertical-pod-autoscaler/pkg/admission-controller/resource/vpa/handler_test.go @@ -27,7 +27,8 @@ import ( ) const ( - cpu = apiv1.ResourceCPU + cpu = apiv1.ResourceCPU + memory = apiv1.ResourceMemory ) func TestValidateVPA(t *testing.T) { @@ -36,6 +37,7 @@ func TestValidateVPA(t *testing.T) { badMinReplicas := int32(0) validMinReplicas := int32(1) badScalingMode := vpa_types.ContainerScalingMode("bad") + badCPUValue := resource.MustParse("187500u") validScalingMode := vpa_types.ContainerScalingModeAuto scalingModeOff := vpa_types.ContainerScalingModeOff controlledValuesRequestsAndLimits := vpa_types.ContainerControlledValuesRequestsAndLimits @@ -150,6 +152,71 @@ func TestValidateVPA(t *testing.T) { }, expectError: fmt.Errorf("max resource for cpu is lower than min"), }, + { + name: "bad minAllowed cpu value", + vpa: vpa_types.VerticalPodAutoscaler{ + Spec: vpa_types.VerticalPodAutoscalerSpec{ + ResourcePolicy: &vpa_types.PodResourcePolicy{ + ContainerPolicies: []vpa_types.ContainerResourcePolicy{ + { + ContainerName: "loot box", + MinAllowed: apiv1.ResourceList{ + cpu: badCPUValue, + }, + MaxAllowed: apiv1.ResourceList{ + cpu: resource.MustParse("275m"), + }, + }, + }, + }, + }, + }, + expectError: fmt.Errorf("CPU [%v] must be a whole number of milli CPUs", badCPUValue.String()), + }, + { + name: "bad minAllowed memory value", + vpa: vpa_types.VerticalPodAutoscaler{ + Spec: vpa_types.VerticalPodAutoscalerSpec{ + ResourcePolicy: &vpa_types.PodResourcePolicy{ + ContainerPolicies: []vpa_types.ContainerResourcePolicy{ + { + ContainerName: "loot box", + MinAllowed: apiv1.ResourceList{ + cpu: resource.MustParse("1m"), + memory: resource.MustParse("100m"), + }, + MaxAllowed: apiv1.ResourceList{ + cpu: resource.MustParse("275m"), + memory: resource.MustParse("500M"), + }, + }, + }, + }, + }, + }, + expectError: fmt.Errorf("Memory [%v] must be a whole number of bytes", resource.MustParse("100m")), + }, + { + name: "bad manAllowed cpu value", + vpa: vpa_types.VerticalPodAutoscaler{ + Spec: vpa_types.VerticalPodAutoscalerSpec{ + ResourcePolicy: &vpa_types.PodResourcePolicy{ + ContainerPolicies: []vpa_types.ContainerResourcePolicy{ + { + ContainerName: "loot box", + MinAllowed: apiv1.ResourceList{ + cpu: resource.MustParse("1m"), + }, + MaxAllowed: apiv1.ResourceList{ + cpu: badCPUValue, + }, + }, + }, + }, + }, + }, + expectError: fmt.Errorf("CPU [%s] must be a whole number of milli CPUs", badCPUValue.String()), + }, { name: "scaling off with controlled values requests and limits", vpa: vpa_types.VerticalPodAutoscaler{