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 4961384e817f..9e6b05cc2e41 100644 --- a/vertical-pod-autoscaler/pkg/admission-controller/resource/vpa/handler.go +++ b/vertical-pod-autoscaler/pkg/admission-controller/resource/vpa/handler.go @@ -136,19 +136,22 @@ func validateVPA(vpa *vpa_types.VerticalPodAutoscaler, isCreate bool) error { } } for resource, min := range policy.MinAllowed { - if err := validate(resource, min); err != nil { - return err + if err := validateResourceResolution(resource, min); err != nil { + return fmt.Errorf("MinAllowed: %v", err) } max, found := policy.MaxAllowed[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) } } } + + 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 { @@ -169,7 +172,7 @@ func validateVPA(vpa *vpa_types.VerticalPodAutoscaler, isCreate bool) error { return nil } -func validate(name corev1.ResourceName, val apires.Quantity) error { +func validateResourceResolution(name corev1.ResourceName, val apires.Quantity) error { switch name { case corev1.ResourceCPU: return validateCPUResolution(val) 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 6dae07e19fc4..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 @@ -37,7 +37,7 @@ func TestValidateVPA(t *testing.T) { badMinReplicas := int32(0) validMinReplicas := int32(1) badScalingMode := vpa_types.ContainerScalingMode("bad") - badCPUValue := resource.MustParse("187500u") + badCPUResource := resource.MustParse("187500u") validScalingMode := vpa_types.ContainerScalingModeAuto scalingModeOff := vpa_types.ContainerScalingModeOff controlledValuesRequestsAndLimits := vpa_types.ContainerControlledValuesRequestsAndLimits @@ -161,7 +161,7 @@ func TestValidateVPA(t *testing.T) { { ContainerName: "loot box", MinAllowed: apiv1.ResourceList{ - cpu: badCPUValue, + cpu: resource.MustParse("187500u"), }, MaxAllowed: apiv1.ResourceList{ cpu: resource.MustParse("275m"), @@ -171,7 +171,7 @@ func TestValidateVPA(t *testing.T) { }, }, }, - expectError: fmt.Errorf("CPU [%v] must be a whole number of milli CPUs", badCPUValue.String()), + expectError: fmt.Errorf("MinAllowed: CPU [%v] must be a whole number of milli CPUs", badCPUResource.String()), }, { name: "bad minAllowed memory value", @@ -194,28 +194,47 @@ func TestValidateVPA(t *testing.T) { }, }, }, - expectError: fmt.Errorf("Memory [%v] must be a whole number of bytes", resource.MustParse("100m")), + expectError: fmt.Errorf("MinAllowed: Memory [%v] must be a whole number of bytes", resource.MustParse("100m")), }, { - name: "bad manAllowed cpu value", + 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{ - cpu: resource.MustParse("1m"), + 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: badCPUValue, + cpu: resource.MustParse("275m"), + memory: resource.MustParse("500m"), }, }, }, }, }, }, - expectError: fmt.Errorf("CPU [%s] must be a whole number of milli CPUs", badCPUValue.String()), + 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",