Skip to content

Commit

Permalink
Merge pull request #2079 from bskiba/min-limit-cont
Browse files Browse the repository at this point in the history 
Account for min Limit from LimitRange.
  • Loading branch information
@k8s-ci-robot
k8s-ci-robot authored May 31, 2019
2 parents 3f57084 + 2fd9d33 commit 8bb4cb1
Show file tree
Hide file tree
Showing 6 changed files with 181 additions and 171 deletions.
39 changes: 17 additions & 22 deletions vertical-pod-autoscaler/pkg/admission-controller/logic/recommendation_provider.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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)
Expand Down Expand Up @@ -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 {
Expand Down
34 changes: 8 additions & 26 deletions vertical-pod-autoscaler/pkg/admission-controller/logic/recommendation_provider_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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",
},
},
},
Expand Down Expand Up @@ -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 {
Expand All @@ -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,
Expand Down Expand Up @@ -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")
}
}
Expand Down
Loading

0 comments on commit 8bb4cb1

Please sign in to comment.