Add Validation for VPA

kubernetes · Apr 16, 2022 · 93febe6 · 93febe6
1 parent 1fa0716
commit 93febe6
Show file tree

Hide file tree

Showing 2 changed files with 105 additions and 3 deletions.
diff --git 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
+}
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
@@ -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{