VPA: Configurable container limit scaling

vertical-pod-autoscaler/e2e/go.mod

@@ -18,15 +18,14 @@ require (
 	github.com/onsi/gomega v1.5.0
 	github.com/prometheus/procfs v0.0.6 // indirect
 	github.com/spf13/pflag v1.0.5 // indirect
-	golang.org/x/crypto v0.0.0-20200221231518-2aa609cf4a9d // indirect
 	golang.org/x/net v0.0.0-20191112182307-2180aed22343 // indirect
 	golang.org/x/sys v0.0.0-20191112214154-59a1497f0cea // indirect
 	google.golang.org/appengine v1.6.5 // indirect
 	gopkg.in/inf.v0 v0.9.1 // indirect
 	gopkg.in/yaml.v2 v2.2.5 // indirect
 	k8s.io/api v0.0.0
 	k8s.io/apimachinery v0.0.0
-	k8s.io/autoscaler/vertical-pod-autoscaler v0.0.0-20200207133143-e7988a6dd041
+	k8s.io/autoscaler/vertical-pod-autoscaler v0.0.0-20200605154545-936eea18fb1d
 	k8s.io/client-go v11.0.0+incompatible
 	k8s.io/component-base v0.0.0
 	k8s.io/klog v1.0.0

vertical-pod-autoscaler/e2e/go.sum

@@ -545,8 +545,6 @@ golang.org/x/crypto v0.0.0-20190320223903-b7391e95e576/go.mod h1:djNgcEr1/C05ACk
 golang.org/x/crypto v0.0.0-20190510104115-cbcb75029529/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI=
 golang.org/x/crypto v0.0.0-20190611184440-5c40567a22f8 h1:1wopBVtVdWnn03fZelqdXTqk7U7zPQCb+T4rbU9ZEoU=
 golang.org/x/crypto v0.0.0-20190611184440-5c40567a22f8/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI=
-golang.org/x/crypto v0.0.0-20191112222119-e1110fd1c708 h1:pXVtWnwHkrWD9ru3sDxY/qFK/bfc0egRovX91EjWjf4=
-golang.org/x/crypto v0.0.0-20191112222119-e1110fd1c708/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto=
 golang.org/x/crypto v0.0.0-20200221231518-2aa609cf4a9d h1:1ZiEyfaQIg3Qh0EoqpwAakHVhecoE5wlSg5GjnafJGw=
 golang.org/x/crypto v0.0.0-20200221231518-2aa609cf4a9d/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto=
 golang.org/x/exp v0.0.0-20190121172915-509febef88a4/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA=

vertical-pod-autoscaler/e2e/v1/admission_controller.go

@@ -166,6 +166,44 @@ var _ = AdmissionControllerE2eDescribe("Admission-controller", func() {
 		}
 	})
 
+	ginkgo.It("keeps limits unchanged when container controlled values is requests only", func() {
+		d := NewHamsterDeploymentWithResourcesAndLimits(f,
+			ParseQuantityOrDie("100m") /*cpu request*/, ParseQuantityOrDie("100Mi"), /*memory request*/
+			ParseQuantityOrDie("500m") /*cpu limit*/, ParseQuantityOrDie("500Mi") /*memory limit*/)
+
+		ginkgo.By("Setting up a VPA CRD")
+		vpaCRD := NewVPA(f, "hamster-vpa", hamsterTargetRef)
+		vpaCRD.Status.Recommendation = &vpa_types.RecommendedPodResources{
+			ContainerRecommendations: []vpa_types.RecommendedContainerResources{{
+				ContainerName: "hamster",
+				Target: apiv1.ResourceList{
+					apiv1.ResourceCPU:    ParseQuantityOrDie("250m"),
+					apiv1.ResourceMemory: ParseQuantityOrDie("200Mi"),
+				},
+			}},
+		}
+		containerControlledValuesRequestsOnly := vpa_types.ContainerControlledValuesRequestsOnly
+		vpaCRD.Spec.ResourcePolicy = &vpa_types.PodResourcePolicy{
+			ContainerPolicies: []vpa_types.ContainerResourcePolicy{{
+				ContainerName:    "hamster",
+				ControlledValues: &containerControlledValuesRequestsOnly,
+			}},
+		}
+		InstallVPA(f, vpaCRD)
+
+		ginkgo.By("Setting up a hamster deployment")
+		podList := startDeploymentPods(f, d)
+
+		// Originally Pods had 100m CPU, 100Mi of memory, but admission controller
+		// should change it to 250m CPU and 200Mi of memory. Limits should stay unchanged.
+		for _, pod := range podList.Items {
+			gomega.Expect(*pod.Spec.Containers[0].Resources.Requests.Cpu()).To(gomega.Equal(ParseQuantityOrDie("250m")))
+			gomega.Expect(*pod.Spec.Containers[0].Resources.Requests.Memory()).To(gomega.Equal(ParseQuantityOrDie("200Mi")))
+			gomega.Expect(*pod.Spec.Containers[0].Resources.Limits.Cpu()).To(gomega.Equal(ParseQuantityOrDie("500m")))
+			gomega.Expect(*pod.Spec.Containers[0].Resources.Limits.Memory()).To(gomega.Equal(ParseQuantityOrDie("500Mi")))
+		}
+	})
+
 	ginkgo.It("caps request according to container max limit set in LimitRange", func() {
 		startCpuRequest := ParseQuantityOrDie("100m")
 		startCpuLimit := ParseQuantityOrDie("150m")

vertical-pod-autoscaler/e2e/vendor/modules.txt

@@ -925,7 +925,7 @@ k8s.io/apiserver/pkg/util/webhook
 k8s.io/apiserver/pkg/util/wsstream
 k8s.io/apiserver/plugin/pkg/authenticator/token/webhook
 k8s.io/apiserver/plugin/pkg/authorizer/webhook
-# k8s.io/autoscaler/vertical-pod-autoscaler v0.0.0-20200207133143-e7988a6dd041 => ../
+# k8s.io/autoscaler/vertical-pod-autoscaler v0.0.0-20200605154545-936eea18fb1d => ../
 k8s.io/autoscaler/vertical-pod-autoscaler/pkg/apis/autoscaling.k8s.io/v1
 k8s.io/autoscaler/vertical-pod-autoscaler/pkg/apis/autoscaling.k8s.io/v1beta1
 k8s.io/autoscaler/vertical-pod-autoscaler/pkg/apis/autoscaling.k8s.io/v1beta2

vertical-pod-autoscaler/pkg/admission-controller/resource/pod/recommendation/recommendation_provider.go

@@ -46,7 +46,7 @@ func NewProvider(calculator limitrange.LimitRangeCalculator,
 }
 
 // GetContainersResources returns the recommended resources for each container in the given pod in the same order they are specified in the pod.Spec.
-func GetContainersResources(pod *core.Pod, podRecommendation vpa_types.RecommendedPodResources, limitRange *core.LimitRangeItem,
+func GetContainersResources(pod *core.Pod, vpaResourcePolicy *vpa_types.PodResourcePolicy, podRecommendation vpa_types.RecommendedPodResources, limitRange *core.LimitRangeItem,
 	annotations vpa_api_util.ContainerToAnnotationsMap) []vpa_api_util.ContainerResources {
 	resources := make([]vpa_api_util.ContainerResources, len(pod.Spec.Containers))
 	for i, container := range pod.Spec.Containers {
@@ -60,17 +60,29 @@ func GetContainersResources(pod *core.Pod, podRecommendation vpa_types.Recommend
 		if limitRange != nil {
 			defaultLimit = limitRange.Default
 		}
-		proportionalLimits, limitAnnotations := vpa_api_util.GetProportionalLimit(container.Resources.Limits, container.Resources.Requests, recommendation.Target, defaultLimit)
-		if proportionalLimits != nil {
-			resources[i].Limits = proportionalLimits
-			if len(limitAnnotations) > 0 {
-				annotations[container.Name] = append(annotations[container.Name], limitAnnotations...)
+		containerControlledValues := GetContainerControlledValues(container.Name, vpaResourcePolicy)
+		if containerControlledValues == vpa_types.ContainerControlledValuesRequestsAndLimits {
+			proportionalLimits, limitAnnotations := vpa_api_util.GetProportionalLimit(container.Resources.Limits, container.Resources.Requests, recommendation.Target, defaultLimit)
+			if proportionalLimits != nil {
+				resources[i].Limits = proportionalLimits
+				if len(limitAnnotations) > 0 {
+					annotations[container.Name] = append(annotations[container.Name], limitAnnotations...)
+				}
 			}
 		}
 	}
 	return resources
 }
 
+// GetContainerControlledValues returns controlled resource values
+func GetContainerControlledValues(name string, vpaResourcePolicy *vpa_types.PodResourcePolicy) vpa_types.ContainerControlledValues {
+	containerPolicy := vpa_api_util.GetContainerResourcePolicy(name, vpaResourcePolicy)
+	if containerPolicy == nil || containerPolicy.ControlledValues == nil {
+		return vpa_types.ContainerControlledValuesRequestsAndLimits
+	}
+	return *containerPolicy.ControlledValues
+}
+
 // GetContainersResourcesForPod returns recommended request for a given pod and associated annotations.
 // The returned slice corresponds 1-1 to containers in the Pod.
 func (p *recommendationProvider) GetContainersResourcesForPod(pod *core.Pod, vpa *vpa_types.VerticalPodAutoscaler) ([]vpa_api_util.ContainerResources, vpa_api_util.ContainerToAnnotationsMap, error) {
@@ -95,6 +107,10 @@ func (p *recommendationProvider) GetContainersResourcesForPod(pod *core.Pod, vpa
 	if err != nil {
 		return nil, nil, fmt.Errorf("error getting containerLimitRange: %s", err)
 	}
-	containerResources := GetContainersResources(pod, *recommendedPodResources, containerLimitRange, annotations)
+	var resourcePolicy *vpa_types.PodResourcePolicy
+	if vpa.Spec.UpdatePolicy == nil || vpa.Spec.UpdatePolicy.UpdateMode == nil || *vpa.Spec.UpdatePolicy.UpdateMode != vpa_types.UpdateModeOff {
+		resourcePolicy = vpa.Spec.ResourcePolicy
+	}
+	containerResources := GetContainersResources(pod, resourcePolicy, *recommendedPodResources, containerLimitRange, annotations)
 	return containerResources, annotations, nil
 }

vertical-pod-autoscaler/pkg/admission-controller/resource/pod/recommendation/recommendation_provider_test.go

@@ -100,6 +100,11 @@ func TestUpdateResourceRequests(t *testing.T) {
 	vpaWithHighMemory := vpaBuilder.WithTarget("2", "1000Mi").WithMaxAllowed("3", "3Gi").Get()
 	vpaWithExabyteRecommendation := vpaBuilder.WithTarget("1Ei", "1Ei").WithMaxAllowed("1Ei", "1Ei").Get()
 
+	resourceRequestsAndLimitsVPA := vpaBuilder.WithControlledValues(vpa_types.ContainerControlledValuesRequestsAndLimits).Get()
+	resourceRequestsOnlyVPA := vpaBuilder.WithControlledValues(vpa_types.ContainerControlledValuesRequestsOnly).Get()
+	resourceRequestsOnlyVPAHighTarget := vpaBuilder.WithControlledValues(vpa_types.ContainerControlledValuesRequestsOnly).
+		WithTarget("3", "500Mi").WithMaxAllowed("5", "1Gi").Get()
+
 	vpaWithEmptyRecommendation := vpaBuilder.Get()
 	vpaWithEmptyRecommendation.Status.Recommendation = &vpa_types.RecommendedPodResources{}
 	vpaWithNilRecommendation := vpaBuilder.Get()
@@ -201,7 +206,7 @@ func TestUpdateResourceRequests(t *testing.T) {
 			expectedMemLimit: mustParseResourcePointer("200Mi"),
 		},
 		{
-			name:             "proportional limit",
+			name:             "proportional limit - as default",
 			pod:              podWithDoubleLimit,
 			vpa:              vpa,
 			expectedAction:   true,
@@ -210,6 +215,38 @@ func TestUpdateResourceRequests(t *testing.T) {
 			expectedCPULimit: mustParseResourcePointer("4"),
 			expectedMemLimit: mustParseResourcePointer("400Mi"),
 		},
+		{
+			name:             "proportional limit - set explicit",
+			pod:              podWithDoubleLimit,
+			vpa:              resourceRequestsAndLimitsVPA,
+			expectedAction:   true,
+			expectedCPU:      resource.MustParse("2"),
+			expectedMem:      resource.MustParse("200Mi"),
+			expectedCPULimit: mustParseResourcePointer("4"),
+			expectedMemLimit: mustParseResourcePointer("400Mi"),
+		},
+		{
+			name:           "disabled limit scaling",
+			pod:            podWithDoubleLimit,
+			vpa:            resourceRequestsOnlyVPA,
+			expectedAction: true,
+			expectedCPU:    resource.MustParse("2"),
+			expectedMem:    resource.MustParse("200Mi"),
+		},
+		{
+			name:           "disabled limit scaling - requests capped at limit",
+			pod:            podWithDoubleLimit,
+			vpa:            resourceRequestsOnlyVPAHighTarget,
+			expectedAction: true,
+			expectedCPU:    resource.MustParse("2"),
+			expectedMem:    resource.MustParse("200Mi"),
+			annotations: vpa_api_util.ContainerToAnnotationsMap{
+				containerName: []string{
+					"cpu capped to container limit",
+					"memory capped to container limit",
+				},
+			},
+		},
 		{
 			name:             "limit over int64",
 			pod:              podWithTenfoldLimit,

vertical-pod-autoscaler/pkg/admission-controller/resource/vpa/handler.go

@@ -134,6 +134,12 @@ func validateVPA(vpa *vpa_types.VerticalPodAutoscaler, isCreate bool) error {
 					return fmt.Errorf("max resource for %v is lower than min", resource)
 				}
 			}
+			ControlledValues := policy.ControlledValues
+			if mode != nil && ControlledValues != nil {
+				if *mode == vpa_types.ContainerScalingModeOff && *ControlledValues == vpa_types.ContainerControlledValuesRequestsAndLimits {
+					return fmt.Errorf("ControlledValues shouldn't be specified if container scaling mode is off.")
+				}
+			}
 		}
 	}
 

vertical-pod-autoscaler/pkg/admission-controller/resource/vpa/handler_test.go

@@ -36,6 +36,8 @@ func TestValidateVPA(t *testing.T) {
 	validUpdateMode := vpa_types.UpdateModeOff
 	badScalingMode := vpa_types.ContainerScalingMode("bad")
 	validScalingMode := vpa_types.ContainerScalingModeAuto
+	scalingModeOff := vpa_types.ContainerScalingModeOff
+	controlledValuesRequestsAndLimits := vpa_types.ContainerControlledValuesRequestsAndLimits
 	tests := []struct {
 		name        string
 		vpa         vpa_types.VerticalPodAutoscaler
@@ -120,6 +122,23 @@ func TestValidateVPA(t *testing.T) {
 			},
 			expectError: fmt.Errorf("max resource for cpu is lower than min"),
 		},
+		{
+			name: "scaling off with controlled values requests and limits",
+			vpa: vpa_types.VerticalPodAutoscaler{
+				Spec: vpa_types.VerticalPodAutoscalerSpec{
+					ResourcePolicy: &vpa_types.PodResourcePolicy{
+						ContainerPolicies: []vpa_types.ContainerResourcePolicy{
+							{
+								ContainerName:    "loot box",
+								Mode:             &scalingModeOff,
+								ControlledValues: &controlledValuesRequestsAndLimits,
+							},
+						},
+					},
+				},
+			},
+			expectError: fmt.Errorf("ControlledValues shouldn't be specified if container scaling mode is off."),
+		},
 		{
 			name: "all valid",
 			vpa: vpa_types.VerticalPodAutoscaler{

vertical-pod-autoscaler/pkg/apis/autoscaling.k8s.io/v1/types.go

@@ -152,6 +152,11 @@ type ContainerResourcePolicy struct {
 	// (and possibly applied) by VPA.
 	// If not specified, the default of [ResourceCPU, ResourceMemory] will be used.
 	ControlledResources *[]v1.ResourceName `json:"controlledResources,omitempty" patchStrategy:"merge" protobuf:"bytes,5,rep,name=controlledResources"`
+
+	// Specifies which resource values should be controlled.
+	// The default is "RequestsAndLimits".
+	// +optional
+	ControlledValues *ContainerControlledValues `json:"controlledValues,omitempty" protobuf:"bytes,6,rep,name=controlledValues"`
 }
 
 const (
@@ -171,6 +176,17 @@ const (
 	ContainerScalingModeOff ContainerScalingMode = "Off"
 )
 
+// ContainerControlledValues controls which resource value should be autoscaled.
+type ContainerControlledValues string
+
+const (
+	// ContainerControlledValuesRequestsAndLimits means resource request and limits
+	// are scaled automatically. The limit is scaled proportionally to the request.
+	ContainerControlledValuesRequestsAndLimits ContainerControlledValues = "RequestsAndLimits"
+	// ContainerControlledValuesRequestsOnly means only requested resource is autoscaled.
+	ContainerControlledValuesRequestsOnly ContainerControlledValues = "RequestsOnly"
+)
+
 // VerticalPodAutoscalerStatus describes the runtime state of the autoscaler.
 type VerticalPodAutoscalerStatus struct {
 	// The most recently computed amount of resources recommended by the