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..b423558c74f8 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,11 +136,20 @@ func validateVPA(vpa *vpa_types.VerticalPodAutoscaler, isCreate bool) error { } } for resource, min := range policy.MinAllowed { + if err := validateResourceResolution(resource, min); err != nil { + return fmt.Errorf("MinAllowed: %v", err) + } max, found := policy.MaxAllowed[resource] if found && max.Cmp(min) < 0 { return fmt.Errorf("max resource for %v is lower than min", resource) } } + + for resource, max := range policy.MaxAllowed { + if err := validateResourceResolution(resource, max); err != nil { + return fmt.Errorf("MaxAllowed: %v", err) + } + } ControlledValues := policy.ControlledValues if mode != nil && ControlledValues != nil { if *mode == vpa_types.ContainerScalingModeOff && *ControlledValues == vpa_types.ContainerControlledValuesRequestsAndLimits { @@ -157,3 +169,27 @@ func validateVPA(vpa *vpa_types.VerticalPodAutoscaler, isCreate bool) error { return nil } + +func validateResourceResolution(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 +} 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..0fe710e5f3ca 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") + badCPUResource := resource.MustParse("187500u") validScalingMode := vpa_types.ContainerScalingModeAuto scalingModeOff := vpa_types.ContainerScalingModeOff controlledValuesRequestsAndLimits := vpa_types.ContainerControlledValuesRequestsAndLimits @@ -150,6 +152,90 @@ 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: resource.MustParse("187500u"), + }, + MaxAllowed: apiv1.ResourceList{ + cpu: resource.MustParse("275m"), + }, + }, + }, + }, + }, + }, + expectError: fmt.Errorf("MinAllowed: CPU [%v] must be a whole number of milli CPUs", badCPUResource.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("MinAllowed: Memory [%v] must be a whole number of bytes", resource.MustParse("100m")), + }, + { + name: "bad maxAllowed cpu value", + vpa: vpa_types.VerticalPodAutoscaler{ + Spec: vpa_types.VerticalPodAutoscalerSpec{ + ResourcePolicy: &vpa_types.PodResourcePolicy{ + ContainerPolicies: []vpa_types.ContainerResourcePolicy{ + { + ContainerName: "loot box", + MinAllowed: apiv1.ResourceList{}, + MaxAllowed: apiv1.ResourceList{ + cpu: resource.MustParse("187500u"), + }, + }, + }, + }, + }, + }, + expectError: fmt.Errorf("MaxAllowed: CPU [%s] must be a whole number of milli CPUs", badCPUResource.String()), + }, + { + name: "bad maxAllowed 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")}, + MaxAllowed: apiv1.ResourceList{ + cpu: resource.MustParse("275m"), + memory: resource.MustParse("500m"), + }, + }, + }, + }, + }, + }, + expectError: fmt.Errorf("MaxAllowed: Memory [%v] must be a whole number of bytes", resource.MustParse("500m")), + }, { name: "scaling off with controlled values requests and limits", vpa: vpa_types.VerticalPodAutoscaler{