From 30940f8ae27e9b22700d6b720b9e1e3b22ced290 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Johannes=20W=C3=BCrbach?= Date: Tue, 7 Apr 2020 00:20:38 +0200 Subject: [PATCH] Configurable container limit scaling --- .../recommendation/recommendation_provider.go | 30 ++++++++++++++----- .../recommendation_provider_test.go | 10 +++++++ .../resource/vpa/handler.go | 6 ++++ .../resource/vpa/handler_test.go | 19 ++++++++++++ .../pkg/apis/autoscaling.k8s.io/v1/types.go | 17 +++++++++++ .../v1/zz_generated.deepcopy.go | 5 ++++ .../pkg/utils/test/test_vpa.go | 9 ++++++ 7 files changed, 89 insertions(+), 7 deletions(-) diff --git a/vertical-pod-autoscaler/pkg/admission-controller/resource/pod/recommendation/recommendation_provider.go b/vertical-pod-autoscaler/pkg/admission-controller/resource/pod/recommendation/recommendation_provider.go index dd2b092f99e1..d9d480ee003e 100644 --- a/vertical-pod-autoscaler/pkg/admission-controller/resource/pod/recommendation/recommendation_provider.go +++ b/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...) + containerLimitPolicy := GetContainerLimitPolicy(container.Name, vpaResourcePolicy) + if containerLimitPolicy == vpa_types.ContainerLimitScalingModeAuto { + 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 } +// GetContainerLimitPolicy returns container limit scaling policy by container name +func GetContainerLimitPolicy(name string, vpaResourcePolicy *vpa_types.PodResourcePolicy) vpa_types.ContainerLimitScalingMode { + containerPolicy := vpa_api_util.GetContainerResourcePolicy(name, vpaResourcePolicy) + if containerPolicy == nil || containerPolicy.LimitMode == nil { + return vpa_types.ContainerLimitScalingModeAuto + } + return *containerPolicy.LimitMode +} + // 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 } diff --git a/vertical-pod-autoscaler/pkg/admission-controller/resource/pod/recommendation/recommendation_provider_test.go b/vertical-pod-autoscaler/pkg/admission-controller/resource/pod/recommendation/recommendation_provider_test.go index 6f32d42b2681..759b241b2f07 100644 --- a/vertical-pod-autoscaler/pkg/admission-controller/resource/pod/recommendation/recommendation_provider_test.go +++ b/vertical-pod-autoscaler/pkg/admission-controller/resource/pod/recommendation/recommendation_provider_test.go @@ -100,6 +100,8 @@ func TestUpdateResourceRequests(t *testing.T) { vpaWithHighMemory := vpaBuilder.WithTarget("2", "1000Mi").WithMaxAllowed("3", "3Gi").Get() vpaWithExabyteRecommendation := vpaBuilder.WithTarget("1Ei", "1Ei").WithMaxAllowed("1Ei", "1Ei").Get() + limitScalingOffVPA := vpaBuilder.WithLimitMode(vpa_types.ContainerLimitScalingModeOff).Get() + vpaWithEmptyRecommendation := vpaBuilder.Get() vpaWithEmptyRecommendation.Status.Recommendation = &vpa_types.RecommendedPodResources{} vpaWithNilRecommendation := vpaBuilder.Get() @@ -210,6 +212,14 @@ func TestUpdateResourceRequests(t *testing.T) { expectedCPULimit: mustParseResourcePointer("4"), expectedMemLimit: mustParseResourcePointer("400Mi"), }, + { + name: "disabled limit scaling", + pod: podWithDoubleLimit, + vpa: limitScalingOffVPA, + expectedAction: true, + expectedCPU: resource.MustParse("2"), + expectedMem: resource.MustParse("200Mi"), + }, { name: "limit over int64", pod: podWithTenfoldLimit, 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 2a3c720e3fb3..738b08aeb6dd 100644 --- a/vertical-pod-autoscaler/pkg/admission-controller/resource/vpa/handler.go +++ b/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) } } + limitMode := policy.LimitMode + if mode != nil && limitMode != nil { + if *mode == vpa_types.ContainerScalingModeOff && *limitMode == vpa_types.ContainerLimitScalingModeAuto { + return fmt.Errorf("Mode can not be Off when LimitScaling is Auto") + } + } } } 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 be2549c67475..0bb14744c2cf 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 @@ -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 + autoLimitScalingMode := vpa_types.ContainerLimitScalingModeAuto 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 limit scaling auto", + vpa: vpa_types.VerticalPodAutoscaler{ + Spec: vpa_types.VerticalPodAutoscalerSpec{ + ResourcePolicy: &vpa_types.PodResourcePolicy{ + ContainerPolicies: []vpa_types.ContainerResourcePolicy{ + { + ContainerName: "loot box", + Mode: &scalingModeOff, + LimitMode: &autoLimitScalingMode, + }, + }, + }, + }, + }, + expectError: fmt.Errorf("Mode can not be Off when LimitScaling is Auto"), + }, { name: "all valid", vpa: vpa_types.VerticalPodAutoscaler{ diff --git a/vertical-pod-autoscaler/pkg/apis/autoscaling.k8s.io/v1/types.go b/vertical-pod-autoscaler/pkg/apis/autoscaling.k8s.io/v1/types.go index b01bb7d26c77..79b32b477d68 100644 --- a/vertical-pod-autoscaler/pkg/apis/autoscaling.k8s.io/v1/types.go +++ b/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"` + + // Whether autoscaler limit scaling is enabled for the container. The default is "Auto". + // Enabling this requires the autoscaler to be enabled for the container + // +optional + LimitMode *ContainerLimitScalingMode `json:"limitMode,omitempty" protobuf:"bytes,6,rep,name=limitMode"` } const ( @@ -171,6 +176,18 @@ const ( ContainerScalingModeOff ContainerScalingMode = "Off" ) +// ContainerLimitScalingMode controls whether autoscaler limit scaling +// is enabled for a specific container. +type ContainerLimitScalingMode string + +const ( + // ContainerLimitScalingModeAuto means limits are scaled automatically. + // Limit is scaled proportionally to the request. + ContainerLimitScalingModeAuto ContainerLimitScalingMode = "Auto" + // ContainerLimitScalingModeOff means limit scaling is disabled for a container. + ContainerLimitScalingModeOff ContainerLimitScalingMode = "Off" +) + // VerticalPodAutoscalerStatus describes the runtime state of the autoscaler. type VerticalPodAutoscalerStatus struct { // The most recently computed amount of resources recommended by the diff --git a/vertical-pod-autoscaler/pkg/apis/autoscaling.k8s.io/v1/zz_generated.deepcopy.go b/vertical-pod-autoscaler/pkg/apis/autoscaling.k8s.io/v1/zz_generated.deepcopy.go index a54a650106e7..be8df1ab39dd 100644 --- a/vertical-pod-autoscaler/pkg/apis/autoscaling.k8s.io/v1/zz_generated.deepcopy.go +++ b/vertical-pod-autoscaler/pkg/apis/autoscaling.k8s.io/v1/zz_generated.deepcopy.go @@ -57,6 +57,11 @@ func (in *ContainerResourcePolicy) DeepCopyInto(out *ContainerResourcePolicy) { copy(*out, *in) } } + if in.LimitMode != nil { + in, out := &in.LimitMode, &out.LimitMode + *out = new(ContainerLimitScalingMode) + **out = **in + } return } diff --git a/vertical-pod-autoscaler/pkg/utils/test/test_vpa.go b/vertical-pod-autoscaler/pkg/utils/test/test_vpa.go index 359624804338..4269ea89d90d 100644 --- a/vertical-pod-autoscaler/pkg/utils/test/test_vpa.go +++ b/vertical-pod-autoscaler/pkg/utils/test/test_vpa.go @@ -34,6 +34,7 @@ type VerticalPodAutoscalerBuilder interface { WithCreationTimestamp(timestamp time.Time) VerticalPodAutoscalerBuilder WithMinAllowed(cpu, memory string) VerticalPodAutoscalerBuilder WithMaxAllowed(cpu, memory string) VerticalPodAutoscalerBuilder + WithLimitMode(mode vpa_types.ContainerLimitScalingMode) VerticalPodAutoscalerBuilder WithTarget(cpu, memory string) VerticalPodAutoscalerBuilder WithLowerBound(cpu, memory string) VerticalPodAutoscalerBuilder WithTargetRef(targetRef *autoscaling.CrossVersionObjectReference) VerticalPodAutoscalerBuilder @@ -63,6 +64,7 @@ type verticalPodAutoscalerBuilder struct { creationTimestamp time.Time minAllowed core.ResourceList maxAllowed core.ResourceList + limitMode *vpa_types.ContainerLimitScalingMode recommendation RecommendationBuilder conditions []vpa_types.VerticalPodAutoscalerCondition annotations map[string]string @@ -115,6 +117,12 @@ func (b *verticalPodAutoscalerBuilder) WithMaxAllowed(cpu, memory string) Vertic return &c } +func (b *verticalPodAutoscalerBuilder) WithLimitMode(mode vpa_types.ContainerLimitScalingMode) VerticalPodAutoscalerBuilder { + c := *b + c.limitMode = &mode + return &c +} + func (b *verticalPodAutoscalerBuilder) WithTarget(cpu, memory string) VerticalPodAutoscalerBuilder { c := *b c.recommendation = c.recommendation.WithTarget(cpu, memory) @@ -171,6 +179,7 @@ func (b *verticalPodAutoscalerBuilder) Get() *vpa_types.VerticalPodAutoscaler { ContainerName: b.containerName, MinAllowed: b.minAllowed, MaxAllowed: b.maxAllowed, + LimitMode: b.limitMode, }}} recommendation := b.recommendation.WithContainer(b.containerName).Get()