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 77b43d7b6a16..52487dd6c6c4 100644 --- a/vertical-pod-autoscaler/pkg/admission-controller/logic/recommendation_provider.go +++ b/vertical-pod-autoscaler/pkg/admission-controller/logic/recommendation_provider.go @@ -18,21 +18,20 @@ package logic import ( "fmt" - "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/api/resource" - "k8s.io/apimachinery/pkg/labels" - "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/utils/limitrange" + core "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/labels" vpa_types "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/apis/autoscaling.k8s.io/v1beta2" vpa_lister "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/client/listers/autoscaling.k8s.io/v1beta2" "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/target" + "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/utils/limitrange" vpa_api_util "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/utils/vpa" "k8s.io/klog" ) // RecommendationProvider gets current recommendation, annotations and vpaName for the given pod. type RecommendationProvider interface { - GetContainersResourcesForPod(pod *v1.Pod) ([]vpa_api_util.ContainerResources, vpa_api_util.ContainerToAnnotationsMap, string, error) + GetContainersResourcesForPod(pod *core.Pod) ([]vpa_api_util.ContainerResources, vpa_api_util.ContainerToAnnotationsMap, string, error) } type recommendationProvider struct { @@ -54,36 +53,32 @@ 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 *v1.Pod, podRecommendation vpa_types.RecommendedPodResources, limitRange *v1.LimitRangeItem, +func GetContainersResources(pod *core.Pod, 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)) - var defaultCpu, defaultMem, maxCpuLimit, maxMemLimit *resource.Quantity - if limitRange != nil { - defaultCpu = limitRange.Default.Cpu() - defaultMem = limitRange.Default.Memory() - maxCpuLimit = limitRange.Max.Cpu() - maxMemLimit = limitRange.Max.Memory() - } for i, container := range pod.Spec.Containers { recommendation := vpa_api_util.GetRecommendationForContainer(container.Name, &podRecommendation) if recommendation == nil { klog.V(2).Infof("no matching recommendation found for container %s", container.Name) continue } - cpuLimit, annotation := vpa_api_util.GetProportionalLimit(container.Resources.Limits.Cpu(), container.Resources.Requests.Cpu(), recommendation.Target.Cpu(), defaultCpu) - if annotation != "" { - annotations[container.Name] = append(annotations[container.Name], fmt.Sprintf("CPU: %s", annotation)) + resources[i].Requests = recommendation.Target + defaultLimit := core.ResourceList{} + if limitRange != nil { + defaultLimit = limitRange.Default } - memLimit, annotation := vpa_api_util.GetProportionalLimit(container.Resources.Limits.Memory(), container.Resources.Requests.Memory(), recommendation.Target.Memory(), defaultMem) - if annotation != "" { - annotations[container.Name] = append(annotations[container.Name], fmt.Sprintf("memory: %s", annotation)) + 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...) + } } - resources[i] = vpa_api_util.ProportionallyCapResourcesToMaxLimit(recommendation.Target, cpuLimit, memLimit, maxCpuLimit, maxMemLimit) } return resources } -func (p *recommendationProvider) getMatchingVPA(pod *v1.Pod) *vpa_types.VerticalPodAutoscaler { +func (p *recommendationProvider) getMatchingVPA(pod *core.Pod) *vpa_types.VerticalPodAutoscaler { configs, err := p.vpaLister.VerticalPodAutoscalers(pod.Namespace).List(labels.Everything()) if err != nil { klog.Errorf("failed to get vpa configs: %v", err) @@ -114,7 +109,7 @@ func (p *recommendationProvider) getMatchingVPA(pod *v1.Pod) *vpa_types.Vertical // GetContainersResourcesForPod returns recommended request for a given pod, annotations and name of controlling VPA. // The returned slice corresponds 1-1 to containers in the Pod. -func (p *recommendationProvider) GetContainersResourcesForPod(pod *v1.Pod) ([]vpa_api_util.ContainerResources, vpa_api_util.ContainerToAnnotationsMap, string, error) { +func (p *recommendationProvider) GetContainersResourcesForPod(pod *core.Pod) ([]vpa_api_util.ContainerResources, vpa_api_util.ContainerToAnnotationsMap, string, error) { klog.V(2).Infof("updating requirements for pod %s.", pod.Name) vpaConfig := p.getMatchingVPA(pod) if vpaConfig == 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 218a42fdea87..9b32bccbee48 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 @@ -18,21 +18,21 @@ package logic import ( "fmt" - "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/utils/limitrange" "math" "testing" - "github.com/golang/mock/gomock" - "github.com/stretchr/testify/assert" apiv1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" vpa_types "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/apis/autoscaling.k8s.io/v1beta2" target_mock "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/target/mock" + "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/utils/limitrange" "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/utils/test" - "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/utils/vpa" vpa_api_util "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/utils/vpa" + + "github.com/golang/mock/gomock" + "github.com/stretchr/testify/assert" ) func parseLabelSelector(selector string) labels.Selector { @@ -262,8 +262,8 @@ func TestUpdateResourceRequests(t *testing.T) { labelSelector: "app = testingApp", annotations: vpa_api_util.ContainerToAnnotationsMap{ containerName: []string{ - "CPU: failed to keep limit to request proportion of 10 to 1 with recommended request of 1Ei; doesn't fit in int64. Capping limit to MaxInt64 milliunits", - "memory: failed to keep limit to request proportion of 1000Mi to 100Mi with recommended request of 1Ei; doesn't fit in int64. Capping limit to MaxInt64 milliunits", + "cpu: failed to keep limit to request ratio; capping limit to int64", + "memory: failed to keep limit to request ratio; capping limit to int64", }, }, }, @@ -293,24 +293,6 @@ func TestUpdateResourceRequests(t *testing.T) { }, }, }, - { - name: "cap limits to max", - pod: limitsMatchRequestsPod, - vpas: []*vpa_types.VerticalPodAutoscaler{vpa}, - expectedAction: true, - expectedCPU: resource.MustParse("1.5"), - expectedMem: resource.MustParse("150Mi"), - expectedCPULimit: mustParseResourcePointer("1.5"), - expectedMemLimit: mustParseResourcePointer("150Mi"), - labelSelector: "app = testingApp", - limitRange: &apiv1.LimitRangeItem{ - Type: apiv1.LimitTypeContainer, - Max: apiv1.ResourceList{ - apiv1.ResourceCPU: resource.MustParse("1.5"), - apiv1.ResourceMemory: resource.MustParse("150Mi"), - }, - }, - }, } for _, tc := range testCases { @@ -330,7 +312,7 @@ func TestUpdateResourceRequests(t *testing.T) { recommendationProvider := &recommendationProvider{ vpaLister: vpaLister, - recommendationProcessor: api.NewCappingRecommendationProcessor(limitrange.NewNoopLimitsCalculator()), + recommendationProcessor: vpa_api_util.NewCappingRecommendationProcessor(limitrange.NewNoopLimitsCalculator()), selectorFetcher: mockSelectorFetcher, limitsRangeCalculator: &fakeLimitRangeCalculator{ tc.limitRange, @@ -366,7 +348,7 @@ func TestUpdateResourceRequests(t *testing.T) { if tc.expectedMemLimit == nil { assert.False(t, memLimitPresent, "expected no memory limit, got %s", memLimit.String()) } else { - if assert.True(t, memLimitPresent, "expected cpu limit, but it's missing") { + if assert.True(t, memLimitPresent, "expected memory limit, but it's missing") { assert.Equal(t, tc.expectedMemLimit.MilliValue(), memLimit.MilliValue(), "memory limit doesn't match") } } diff --git a/vertical-pod-autoscaler/pkg/utils/vpa/capping.go b/vertical-pod-autoscaler/pkg/utils/vpa/capping.go index a8b8bfed3953..b10fbacc19a4 100644 --- a/vertical-pod-autoscaler/pkg/utils/vpa/capping.go +++ b/vertical-pod-autoscaler/pkg/utils/vpa/capping.go @@ -18,6 +18,7 @@ package api import ( "fmt" + apiv1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" vpa_types "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/apis/autoscaling.k8s.io/v1beta2" @@ -34,8 +35,10 @@ func NewCappingRecommendationProcessor(limitsRangeCalculator limitrange.LimitRan type cappingAction string var ( - cappedToMinAllowed cappingAction = "capped to minAllowed" - cappedToMaxAllowed cappingAction = "capped to maxAllowed" + cappedToMinAllowed cappingAction = "capped to minAllowed" + cappedToMaxAllowed cappingAction = "capped to maxAllowed" + cappedProportionallyToMaxLimit cappingAction = "capped to fit Max in container LimitRange" + cappedProportionallyToMinLimit cappingAction = "capped to fit Min in container LimitRange" ) func toCappingAnnotation(resourceName apiv1.ResourceName, action cappingAction) string { @@ -64,19 +67,20 @@ func (c *cappingRecommendationProcessor) Apply( } updatedRecommendations := []vpa_types.RecommendedContainerResources{} containerToAnnotationsMap := ContainerToAnnotationsMap{} - limitAdjustedRecommendation, err := c.capProportionallyToMaxLimit(podRecommendation, pod, containerToAnnotationsMap) - if err != nil { - return nil, nil, err - } - for _, containerRecommendation := range limitAdjustedRecommendation { + for _, containerRecommendation := range podRecommendation.ContainerRecommendations { container := getContainer(containerRecommendation.ContainerName, pod) if container == nil { klog.V(2).Infof("no matching Container found for recommendation %s", containerRecommendation.ContainerName) continue } + + containerLimitRange, err := c.limitsRangeCalculator.GetContainerLimitRangeItem(pod.Namespace) + if err != nil { + klog.Warningf("failed to fetch LimitRange for %v namespace", pod.Namespace) + } updatedContainerResources, containerAnnotations, err := getCappedRecommendationForContainer( - *container, &containerRecommendation, policy) + *container, &containerRecommendation, policy, containerLimitRange) if len(containerAnnotations) != 0 { containerToAnnotationsMap[containerRecommendation.ContainerName] = containerAnnotations @@ -90,58 +94,11 @@ func (c *cappingRecommendationProcessor) Apply( return &vpa_types.RecommendedPodResources{ContainerRecommendations: updatedRecommendations}, containerToAnnotationsMap, nil } -func capSingleRecommendationProportionallyToMaxLimit(recommendation apiv1.ResourceList, container apiv1.Container, podLimitRange *apiv1.LimitRangeItem, containerToAnnotationsMap ContainerToAnnotationsMap) apiv1.ResourceList { - defaultCpu := podLimitRange.Default.Cpu() - defaultMem := podLimitRange.Default.Memory() - maxCpuLimit := podLimitRange.Max.Cpu() - maxMemLimit := podLimitRange.Max.Memory() - cpuLimit, _ := GetProportionalLimit(container.Resources.Limits.Cpu(), container.Resources.Requests.Cpu(), recommendation.Cpu(), defaultCpu) - memLimit, _ := GetProportionalLimit(container.Resources.Limits.Memory(), container.Resources.Requests.Memory(), recommendation.Memory(), defaultMem) - capped := ProportionallyCapResourcesToMaxLimit(recommendation, cpuLimit, memLimit, maxCpuLimit, maxMemLimit) - return apiv1.ResourceList{ - apiv1.ResourceCPU: *capped.Requests.Cpu(), - apiv1.ResourceMemory: *capped.Requests.Memory(), - } -} - -func (c *cappingRecommendationProcessor) capProportionallyToMaxLimit(podRecommendation *vpa_types.RecommendedPodResources, pod *apiv1.Pod, containerToAnnotationsMap ContainerToAnnotationsMap) ([]vpa_types.RecommendedContainerResources, error) { - podLimitRange, err := c.limitsRangeCalculator.GetContainerLimitRangeItem(pod.Namespace) - if err != nil { - return nil, fmt.Errorf("error obtaining limit range: %s", err) - } - if podLimitRange == nil { - return podRecommendation.ContainerRecommendations, nil - } - - updatedRecommendations := []vpa_types.RecommendedContainerResources{} - - for _, container := range pod.Spec.Containers { - recommendation := GetRecommendationForContainer(container.Name, podRecommendation) - if recommendation == nil { - klog.V(2).Infof("no matching recommendation found for container %s", container.Name) - continue - } - - scaledRecommendation := recommendation.DeepCopy() - scaledRecommendation.LowerBound = capSingleRecommendationProportionallyToMaxLimit(recommendation.LowerBound, container, podLimitRange, containerToAnnotationsMap) - scaledRecommendation.Target = capSingleRecommendationProportionallyToMaxLimit(recommendation.Target, container, podLimitRange, containerToAnnotationsMap) - scaledRecommendation.UpperBound = capSingleRecommendationProportionallyToMaxLimit(recommendation.UpperBound, container, podLimitRange, containerToAnnotationsMap) - if scaledRecommendation.Target.Cpu().MilliValue() != recommendation.Target.Cpu().MilliValue() { - containerToAnnotationsMap[container.Name] = append(containerToAnnotationsMap[container.Name], "changed CPU limit to fit within limit range") - } - if scaledRecommendation.Target.Memory().Value() != recommendation.Target.Memory().Value() { - containerToAnnotationsMap[container.Name] = append(containerToAnnotationsMap[container.Name], "changed memory limit to fit within limit range") - } - updatedRecommendations = append(updatedRecommendations, *scaledRecommendation) - } - return updatedRecommendations, nil -} - // getCappedRecommendationForContainer returns a recommendation for the given container, adjusted to obey policy and limits. func getCappedRecommendationForContainer( container apiv1.Container, containerRecommendation *vpa_types.RecommendedContainerResources, - policy *vpa_types.PodResourcePolicy) (*vpa_types.RecommendedContainerResources, []string, error) { + policy *vpa_types.PodResourcePolicy, limitRange *apiv1.LimitRangeItem) (*vpa_types.RecommendedContainerResources, []string, error) { if containerRecommendation == nil { return nil, nil, fmt.Errorf("no recommendation available for container name %v", container.Name) } @@ -153,8 +110,11 @@ func getCappedRecommendationForContainer( cappingAnnotations := make([]string, 0) process := func(recommendation apiv1.ResourceList, genAnnotations bool) { + // TODO: Add anotation if limitRange is conflicting with VPA policy. + limitAnnotations := applyContainerLimitRange(recommendation, container, limitRange) annotations := applyVPAPolicy(recommendation, containerPolicy) if genAnnotations { + cappingAnnotations = append(cappingAnnotations, limitAnnotations...) cappingAnnotations = append(cappingAnnotations, annotations...) } } @@ -173,12 +133,12 @@ func applyVPAPolicy(recommendation apiv1.ResourceList, policy *vpa_types.Contain } annotations := make([]string, 0) for resourceName, recommended := range recommendation { - cappedToMin, isCapped := maybeCapToMin(recommended, resourceName, policy) + cappedToMin, isCapped := maybeCapToPolicyMin(recommended, resourceName, policy) recommendation[resourceName] = cappedToMin if isCapped { annotations = append(annotations, toCappingAnnotation(resourceName, cappedToMinAllowed)) } - cappedToMax, isCapped := maybeCapToMax(cappedToMin, resourceName, policy) + cappedToMax, isCapped := maybeCapToPolicyMax(cappedToMin, resourceName, policy) recommendation[resourceName] = cappedToMax if isCapped { annotations = append(annotations, toCappingAnnotation(resourceName, cappedToMaxAllowed)) @@ -202,9 +162,9 @@ func applyVPAPolicyForContainer(containerName string, process := func(recommendation apiv1.ResourceList) { for resourceName, recommended := range recommendation { - cappedToMin, _ := maybeCapToMin(recommended, resourceName, containerPolicy) + cappedToMin, _ := maybeCapToPolicyMin(recommended, resourceName, containerPolicy) recommendation[resourceName] = cappedToMin - cappedToMax, _ := maybeCapToMax(cappedToMin, resourceName, containerPolicy) + cappedToMax, _ := maybeCapToPolicyMax(cappedToMin, resourceName, containerPolicy) recommendation[resourceName] = cappedToMax } } @@ -216,20 +176,30 @@ func applyVPAPolicyForContainer(containerName string, return cappedRecommendations, nil } -func maybeCapToMin(recommended resource.Quantity, resourceName apiv1.ResourceName, +func maybeCapToPolicyMin(recommended resource.Quantity, resourceName apiv1.ResourceName, containerPolicy *vpa_types.ContainerResourcePolicy) (resource.Quantity, bool) { - min, found := containerPolicy.MinAllowed[resourceName] - if found && !min.IsZero() && recommended.Cmp(min) < 0 { - return min, true + return maybeCapToMin(recommended, resourceName, containerPolicy.MinAllowed) +} + +func maybeCapToPolicyMax(recommended resource.Quantity, resourceName apiv1.ResourceName, + containerPolicy *vpa_types.ContainerResourcePolicy) (resource.Quantity, bool) { + return maybeCapToMax(recommended, resourceName, containerPolicy.MaxAllowed) +} + +func maybeCapToMax(recommended resource.Quantity, resourceName apiv1.ResourceName, + max apiv1.ResourceList) (resource.Quantity, bool) { + maxResource, found := max[resourceName] + if found && !maxResource.IsZero() && recommended.Cmp(maxResource) > 0 { + return maxResource, true } return recommended, false } -func maybeCapToMax(recommended resource.Quantity, resourceName apiv1.ResourceName, - containerPolicy *vpa_types.ContainerResourcePolicy) (resource.Quantity, bool) { - max, found := containerPolicy.MaxAllowed[resourceName] - if found && !max.IsZero() && recommended.Cmp(max) > 0 { - return max, true +func maybeCapToMin(recommended resource.Quantity, resourceName apiv1.ResourceName, + min apiv1.ResourceList) (resource.Quantity, bool) { + minResource, found := min[resourceName] + if found && !minResource.IsZero() && recommended.Cmp(minResource) < 0 { + return minResource, true } return recommended, false } @@ -278,3 +248,55 @@ func getContainer(containerName string, pod *apiv1.Pod) *apiv1.Container { } return nil } + +// applyContainerLimitRange updates recommendation if recommended resources are outside of limits defined in VPA resources policy +func applyContainerLimitRange(recommendation apiv1.ResourceList, container apiv1.Container, limitRange *apiv1.LimitRangeItem) []string { + annotations := make([]string, 0) + if limitRange == nil { + return annotations + } + maxAllowedRecommendation := getMaxAllowedRecommendation(recommendation, container, limitRange) + minAllowedRecommendation := getMinAllowedRecommendation(recommendation, container, limitRange) + for resourceName, recommended := range recommendation { + cappedToMin, isCapped := maybeCapToMin(recommended, resourceName, minAllowedRecommendation) + recommendation[resourceName] = cappedToMin + if isCapped { + annotations = append(annotations, toCappingAnnotation(resourceName, cappedProportionallyToMinLimit)) + } + cappedToMax, isCapped := maybeCapToMax(cappedToMin, resourceName, maxAllowedRecommendation) + recommendation[resourceName] = cappedToMax + if isCapped { + annotations = append(annotations, toCappingAnnotation(resourceName, cappedProportionallyToMaxLimit)) + } + } + return annotations +} + +func getMaxAllowedRecommendation(recommendation apiv1.ResourceList, container apiv1.Container, + podLimitRange *apiv1.LimitRangeItem) apiv1.ResourceList { + if podLimitRange == nil { + return apiv1.ResourceList{} + } + return getBoundaryRecommendation(recommendation, container, podLimitRange.Max, podLimitRange.Default) +} + +func getMinAllowedRecommendation(recommendation apiv1.ResourceList, container apiv1.Container, + podLimitRange *apiv1.LimitRangeItem) apiv1.ResourceList { + if podLimitRange == nil { + return apiv1.ResourceList{} + } + return getBoundaryRecommendation(recommendation, container, podLimitRange.Min, podLimitRange.Default) +} + +func getBoundaryRecommendation(recommendation apiv1.ResourceList, container apiv1.Container, + boundaryLimit, defaultLimit apiv1.ResourceList) apiv1.ResourceList { + if boundaryLimit == nil { + return apiv1.ResourceList{} + } + cpuMaxRequest := GetBoundaryRequest(container.Resources.Requests.Cpu(), container.Resources.Limits.Cpu(), boundaryLimit.Cpu(), defaultLimit.Cpu()) + memMaxRequest := GetBoundaryRequest(container.Resources.Requests.Memory(), container.Resources.Limits.Memory(), boundaryLimit.Memory(), defaultLimit.Memory()) + return apiv1.ResourceList{ + apiv1.ResourceCPU: *cpuMaxRequest, + apiv1.ResourceMemory: *memMaxRequest, + } +} diff --git a/vertical-pod-autoscaler/pkg/utils/vpa/capping_test.go b/vertical-pod-autoscaler/pkg/utils/vpa/capping_test.go index 6fa52d1f6257..efbf4b06a44c 100644 --- a/vertical-pod-autoscaler/pkg/utils/vpa/capping_test.go +++ b/vertical-pod-autoscaler/pkg/utils/vpa/capping_test.go @@ -228,8 +228,10 @@ func TestApplyCapsToLimitRange(t *testing.T) { limitRange := apiv1.LimitRangeItem{ Type: apiv1.LimitTypeContainer, Max: apiv1.ResourceList{ - apiv1.ResourceCPU: resource.MustParse("1"), - apiv1.ResourceMemory: resource.MustParse("1G"), + apiv1.ResourceCPU: resource.MustParse("1"), + }, + Min: apiv1.ResourceList{ + apiv1.ResourceMemory: resource.MustParse("500M"), }, } recommendation := vpa_types.RecommendedPodResources{ @@ -238,7 +240,7 @@ func TestApplyCapsToLimitRange(t *testing.T) { ContainerName: "container", Target: apiv1.ResourceList{ apiv1.ResourceCPU: resource.MustParse("2"), - apiv1.ResourceMemory: resource.MustParse("10G"), + apiv1.ResourceMemory: resource.MustParse("200M"), }, }, }, @@ -266,17 +268,9 @@ func TestApplyCapsToLimitRange(t *testing.T) { ContainerRecommendations: []vpa_types.RecommendedContainerResources{ { ContainerName: "container", - LowerBound: apiv1.ResourceList{ - apiv1.ResourceCPU: *resource.NewQuantity(0, resource.DecimalSI), - apiv1.ResourceMemory: *resource.NewQuantity(0, resource.BinarySI), - }, Target: apiv1.ResourceList{ apiv1.ResourceCPU: resource.MustParse("1000m"), - apiv1.ResourceMemory: resource.MustParse("1000000000000m"), - }, - UpperBound: apiv1.ResourceList{ - apiv1.ResourceCPU: *resource.NewQuantity(0, resource.DecimalSI), - apiv1.ResourceMemory: *resource.NewQuantity(0, resource.BinarySI), + apiv1.ResourceMemory: resource.MustParse("500000000000m"), }, }, }, @@ -286,6 +280,7 @@ func TestApplyCapsToLimitRange(t *testing.T) { processor := NewCappingRecommendationProcessor(&calculator) processedRecommendation, annotations, err := processor.Apply(&recommendation, nil, nil, &pod) assert.NoError(t, err) - assert.Equal(t, map[string][]string{"container": {"changed CPU limit to fit within limit range", "changed memory limit to fit within limit range"}}, annotations) + assert.Contains(t, annotations, "container") + assert.ElementsMatch(t, []string{"cpu capped to fit Max in container LimitRange", "memory capped to fit Min in container LimitRange"}, annotations["container"]) assert.Equal(t, expectedRecommendation, *processedRecommendation) } diff --git a/vertical-pod-autoscaler/pkg/utils/vpa/limit_and_request_scaling.go b/vertical-pod-autoscaler/pkg/utils/vpa/limit_and_request_scaling.go index 2d4c5396aaf6..bbd9cf2c9851 100644 --- a/vertical-pod-autoscaler/pkg/utils/vpa/limit_and_request_scaling.go +++ b/vertical-pod-autoscaler/pkg/utils/vpa/limit_and_request_scaling.go @@ -18,27 +18,51 @@ package api import ( "fmt" - "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/api/resource" "math" "math/big" + + core "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/resource" ) // ContainerResources holds resources request for container type ContainerResources struct { - Limits v1.ResourceList - Requests v1.ResourceList + Limits core.ResourceList + Requests core.ResourceList } func newContainerResources() ContainerResources { return ContainerResources{ - Requests: v1.ResourceList{}, - Limits: v1.ResourceList{}, + Requests: core.ResourceList{}, + Limits: core.ResourceList{}, } } // GetProportionalLimit returns limit that will be in the same proportion to recommended request as original limit had to original request. -func GetProportionalLimit(originalLimit, originalRequest, recommendedRequest, defaultLimit *resource.Quantity) (*resource.Quantity, string) { +func GetProportionalLimit(originalLimit, originalRequest, recommendation, defaultLimit core.ResourceList) (core.ResourceList, []string) { + annotations := []string{} + cpuLimit, annotation := getProportionalResourceLimit(core.ResourceCPU, originalLimit.Cpu(), originalRequest.Cpu(), recommendation.Cpu(), defaultLimit.Cpu()) + if annotation != "" { + annotations = append(annotations, annotation) + } + memLimit, annotation := getProportionalResourceLimit(core.ResourceMemory, originalLimit.Memory(), originalRequest.Memory(), recommendation.Memory(), defaultLimit.Memory()) + if annotation != "" { + annotations = append(annotations, annotation) + } + if memLimit == nil && cpuLimit == nil { + return nil, []string{} + } + result := core.ResourceList{} + if cpuLimit != nil { + result[core.ResourceCPU] = *cpuLimit + } + if memLimit != nil { + result[core.ResourceMemory] = *memLimit + } + return result, annotations +} + +func getProportionalResourceLimit(resourceName core.ResourceName, originalLimit, originalRequest, recommendedRequest, defaultLimit *resource.Quantity) (*resource.Quantity, string) { if originalLimit == nil || originalLimit.Value() == 0 && defaultLimit != nil { originalLimit = defaultLimit } @@ -62,8 +86,25 @@ func GetProportionalLimit(originalLimit, originalRequest, recommendedRequest, de return result, "" } return result, fmt.Sprintf( - "failed to keep limit to request proportion of %s to %s with recommended request of %s; doesn't fit in int64. Capping limit to MaxInt64 milliunits", - originalLimit, originalRequest, recommendedRequest) + "%v: failed to keep limit to request ratio; capping limit to int64", resourceName) +} + +// GetBoundaryRequest returns the boundary (min/max) request that can be specified with +// preserving the original limit to request ratio. Returns nil if no boundary exists +func GetBoundaryRequest(originalRequest, originalLimit, boundaryLimit, defaultLimit *resource.Quantity) *resource.Quantity { + if originalLimit == nil || originalLimit.Value() == 0 && defaultLimit != nil { + originalLimit = defaultLimit + } + // originalLimit not set, no boundary + if originalLimit == nil || originalLimit.Value() == 0 { + return nil + } + // originalLimit set but originalRequest not set - K8s will treat the pod as if they were equal + if originalRequest == nil || originalRequest.Value() == 0 { + return boundaryLimit + } + result, _ := scaleQuantityProportionally(originalRequest /* scaledQuantity */, originalLimit /*scaleBase*/, boundaryLimit /*scaleResult*/) + return result } // scaleQuantityProportionally returns value which has the same proportion to scaledQuantity as scaleResult has to scaleBase @@ -80,31 +121,3 @@ func scaleQuantityProportionally(scaledQuantity, scaleBase, scaleResult *resourc } return resource.NewMilliQuantity(math.MaxInt64, scaledQuantity.Format), true } - -func proportionallyCapLimitToMax(recommendedRequest, recommendedLimit, maxLimit *resource.Quantity) (request, limit *resource.Quantity) { - if recommendedLimit == nil || maxLimit == nil || maxLimit.IsZero() { - return recommendedRequest, recommendedLimit - } - if recommendedLimit.Cmp(*maxLimit) <= 0 { - return recommendedRequest, recommendedLimit - } - scaledRequest, _ := scaleQuantityProportionally(recommendedRequest, recommendedLimit, maxLimit) - return scaledRequest, maxLimit -} - -// ProportionallyCapResourcesToMaxLimit caps CPU and memory limit to maximum and scales requests to maintain limit/request ratio. -func ProportionallyCapResourcesToMaxLimit(recommendedRequests v1.ResourceList, cpuLimit, memLimit, maxCpuLimit, maxMemLimit *resource.Quantity) ContainerResources { - scaledCpuRequest, scaledCpuLimit := proportionallyCapLimitToMax(recommendedRequests.Cpu(), cpuLimit, maxCpuLimit) - scaledMemRequest, scaledMemLimit := proportionallyCapLimitToMax(recommendedRequests.Memory(), memLimit, maxMemLimit) - result := newContainerResources() - - result.Requests[v1.ResourceCPU] = *scaledCpuRequest - result.Requests[v1.ResourceMemory] = *scaledMemRequest - if scaledCpuLimit != nil { - result.Limits[v1.ResourceCPU] = *scaledCpuLimit - } - if scaledMemLimit != nil { - result.Limits[v1.ResourceMemory] = *scaledMemLimit - } - return result -} diff --git a/vertical-pod-autoscaler/pkg/utils/vpa/limit_and_request_scaling_test.go b/vertical-pod-autoscaler/pkg/utils/vpa/limit_and_request_scaling_test.go index 151706f29cb4..c334279c6a4a 100644 --- a/vertical-pod-autoscaler/pkg/utils/vpa/limit_and_request_scaling_test.go +++ b/vertical-pod-autoscaler/pkg/utils/vpa/limit_and_request_scaling_test.go @@ -17,10 +17,13 @@ limitations under the License. package api import ( - "github.com/stretchr/testify/assert" - "k8s.io/apimachinery/pkg/api/resource" "math" "testing" + + core "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/resource" + + "github.com/stretchr/testify/assert" ) func mustParseToPointer(str string) *resource.Quantity { @@ -28,7 +31,7 @@ func mustParseToPointer(str string) *resource.Quantity { return &val } -func TestGetProportionalLimit(t *testing.T) { +func TestGetProportionalResourceLimit(t *testing.T) { tests := []struct { name string originalLimit *resource.Quantity @@ -82,7 +85,7 @@ func TestGetProportionalLimit(t *testing.T) { } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { - gotLimit, gotAnnotation := GetProportionalLimit(tc.originalLimit, tc.originalRequest, tc.recommendedRequest, tc.defaultLimit) + gotLimit, gotAnnotation := getProportionalResourceLimit(core.ResourceCPU, tc.originalLimit, tc.originalRequest, tc.recommendedRequest, tc.defaultLimit) if tc.expectLimit == nil { assert.Nil(t, gotLimit) } else {