From 6ab92f1793bb49a36cdf34b740e648f241968706 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Johannes=20W=C3=BCrbach?= Date: Thu, 19 Sep 2019 13:19:16 +0200 Subject: [PATCH] VPA: Limit containers to maxAllowed if no limit is present Instead of allow containers to use any amount of resources, if no limit is provided. Cap them at maxAllowed so maxAllowed is actually enforced. --- .../logic/recommendation_provider.go | 17 +++- .../logic/recommendation_provider_test.go | 92 +++++++++++-------- 2 files changed, 67 insertions(+), 42 deletions(-) diff --git a/vertical-pod-autoscaler/pkg/admission-controller/logic/recommendation_provider.go b/vertical-pod-autoscaler/pkg/admission-controller/logic/recommendation_provider.go index 7d81c28b93a3..de888591ba9c 100644 --- a/vertical-pod-autoscaler/pkg/admission-controller/logic/recommendation_provider.go +++ b/vertical-pod-autoscaler/pkg/admission-controller/logic/recommendation_provider.go @@ -53,7 +53,7 @@ func NewRecommendationProvider(calculator limitrange.LimitRangeCalculator, recom } // 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 { @@ -62,6 +62,13 @@ func GetContainersResources(pod *core.Pod, podRecommendation vpa_types.Recommend klog.V(2).Infof("no matching recommendation found for container %s", container.Name) continue } + + containerPolicy := vpa_api_util.GetContainerResourcePolicy(container.Name, vpaResourcePolicy) + if containerPolicy == nil { + klog.V(2).Infof("no matching resource policy found for container %s", container.Name) + continue + } + resources[i].Requests = recommendation.Target defaultLimit := core.ResourceList{} if limitRange != nil { @@ -73,6 +80,8 @@ func GetContainersResources(pod *core.Pod, podRecommendation vpa_types.Recommend if len(limitAnnotations) > 0 { annotations[container.Name] = append(annotations[container.Name], limitAnnotations...) } + } else if containerPolicy != nil && containerPolicy.MaxAllowed != nil { + resources[i].Limits = containerPolicy.MaxAllowed } } return resources @@ -132,6 +141,10 @@ func (p *recommendationProvider) GetContainersResourcesForPod(pod *core.Pod) ([] 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 vpaConfig.Spec.UpdatePolicy == nil || vpaConfig.Spec.UpdatePolicy.UpdateMode == nil || *vpaConfig.Spec.UpdatePolicy.UpdateMode != vpa_types.UpdateModeOff { + resourcePolicy = vpaConfig.Spec.ResourcePolicy + } + containerResources := GetContainersResources(pod, resourcePolicy, *recommendedPodResources, containerLimitRange, annotations) return containerResources, annotations, vpaConfig.Name, nil } diff --git a/vertical-pod-autoscaler/pkg/admission-controller/logic/recommendation_provider_test.go b/vertical-pod-autoscaler/pkg/admission-controller/logic/recommendation_provider_test.go index a11928a12a2c..bff86672a1e2 100644 --- a/vertical-pod-autoscaler/pkg/admission-controller/logic/recommendation_provider_test.go +++ b/vertical-pod-autoscaler/pkg/admission-controller/logic/recommendation_provider_test.go @@ -133,55 +133,65 @@ func TestUpdateResourceRequests(t *testing.T) { labelSelector string }{ { - name: "uninitialized pod", - pod: uninitialized, - vpas: []*vpa_types.VerticalPodAutoscaler{vpa}, - expectedAction: true, - expectedMem: resource.MustParse("200Mi"), - expectedCPU: resource.MustParse("2"), - labelSelector: "app = testingApp", + name: "uninitialized pod", + pod: uninitialized, + vpas: []*vpa_types.VerticalPodAutoscaler{vpa}, + expectedAction: true, + expectedMem: resource.MustParse("200Mi"), + expectedCPU: resource.MustParse("2"), + expectedCPULimit: mustParseResourcePointer("3"), + expectedMemLimit: mustParseResourcePointer("1Gi"), + labelSelector: "app = testingApp", }, { - name: "target below min", - pod: uninitialized, - vpas: []*vpa_types.VerticalPodAutoscaler{targetBelowMinVPA}, - expectedAction: true, - expectedMem: resource.MustParse("300Mi"), // MinMemory is expected to be used - expectedCPU: resource.MustParse("4"), // MinCpu is expected to be used + name: "target below min", + pod: uninitialized, + vpas: []*vpa_types.VerticalPodAutoscaler{targetBelowMinVPA}, + expectedAction: true, + expectedMem: resource.MustParse("300Mi"), // MinMemory is expected to be used + expectedCPU: resource.MustParse("4"), // MinCpu is expected to be used + expectedCPULimit: mustParseResourcePointer("5"), + expectedMemLimit: mustParseResourcePointer("1Gi"), annotations: vpa_api_util.ContainerToAnnotationsMap{ containerName: []string{"cpu capped to minAllowed", "memory capped to minAllowed"}, }, labelSelector: "app = testingApp", }, { - name: "target above max", - pod: uninitialized, - vpas: []*vpa_types.VerticalPodAutoscaler{targetAboveMaxVPA}, - expectedAction: true, - expectedMem: resource.MustParse("1Gi"), // MaxMemory is expected to be used - expectedCPU: resource.MustParse("5"), // MaxCpu is expected to be used + name: "target above max", + pod: uninitialized, + vpas: []*vpa_types.VerticalPodAutoscaler{targetAboveMaxVPA}, + expectedAction: true, + expectedMem: resource.MustParse("1Gi"), // MaxMemory is expected to be used + expectedCPU: resource.MustParse("5"), // MaxCpu is expected to be used + expectedCPULimit: mustParseResourcePointer("5"), + expectedMemLimit: mustParseResourcePointer("1Gi"), annotations: vpa_api_util.ContainerToAnnotationsMap{ containerName: []string{"cpu capped to maxAllowed", "memory capped to maxAllowed"}, }, labelSelector: "app = testingApp", }, { - name: "initialized pod", - pod: initialized, - vpas: []*vpa_types.VerticalPodAutoscaler{vpa}, - expectedAction: true, - expectedMem: resource.MustParse("200Mi"), - expectedCPU: resource.MustParse("2"), - labelSelector: "app = testingApp", + name: "initialized pod", + pod: initialized, + vpas: []*vpa_types.VerticalPodAutoscaler{vpa}, + expectedAction: true, + expectedMem: resource.MustParse("200Mi"), + expectedCPU: resource.MustParse("2"), + expectedCPULimit: mustParseResourcePointer("3"), + expectedMemLimit: mustParseResourcePointer("1Gi"), + labelSelector: "app = testingApp", }, { - name: "high memory", - pod: initialized, - vpas: []*vpa_types.VerticalPodAutoscaler{vpaWithHighMemory}, - expectedAction: true, - expectedMem: resource.MustParse("1000Mi"), - expectedCPU: resource.MustParse("2"), - labelSelector: "app = testingApp", + name: "high memory", + pod: initialized, + vpas: []*vpa_types.VerticalPodAutoscaler{vpaWithHighMemory}, + expectedAction: true, + expectedMem: resource.MustParse("1000Mi"), + expectedCPU: resource.MustParse("2"), + expectedCPULimit: mustParseResourcePointer("3"), + expectedMemLimit: mustParseResourcePointer("3Gi"), + labelSelector: "app = testingApp", }, { name: "not matching selecetor", @@ -198,13 +208,15 @@ func TestUpdateResourceRequests(t *testing.T) { labelSelector: "app = testingApp", }, { - name: "two vpas one in off mode", - pod: uninitialized, - vpas: []*vpa_types.VerticalPodAutoscaler{offVPA, vpa}, - expectedAction: true, - expectedMem: resource.MustParse("200Mi"), - expectedCPU: resource.MustParse("2"), - labelSelector: "app = testingApp", + name: "two vpas one in off mode", + pod: uninitialized, + vpas: []*vpa_types.VerticalPodAutoscaler{offVPA, vpa}, + expectedAction: true, + expectedMem: resource.MustParse("200Mi"), + expectedCPU: resource.MustParse("2"), + expectedCPULimit: mustParseResourcePointer("3"), + expectedMemLimit: mustParseResourcePointer("1Gi"), + labelSelector: "app = testingApp", }, { name: "empty recommendation",