Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

VPA: Limit containers to maxAllowed if no limit is present #2362

Closed
wants to merge 1 commit into from
Closed

VPA: Limit containers to maxAllowed if no limit is present #2362

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 15 additions & 2 deletions vertical-pod-autoscaler/pkg/admission-controller/logic/recommendation_provider.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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 {
Expand All @@ -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
Expand Down Expand Up @@ -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
}
92 changes: 52 additions & 40 deletions vertical-pod-autoscaler/pkg/admission-controller/logic/recommendation_provider_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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",
Expand Down