Skip to content

Commit

Permalink
Merge pull request #2049 from jbartosik/limit-range-limit-support
Browse files Browse the repository at this point in the history 
Limit range limit support
  • Loading branch information
@k8s-ci-robot
k8s-ci-robot authored May 30, 2019
2 parents 7f59902 + 1b0165c commit 63039fb
Show file tree
Hide file tree
Showing 13 changed files with 777 additions and 121 deletions.
109 changes: 32 additions & 77 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 @@ -21,8 +21,7 @@ import (
"k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/resource"
"k8s.io/apimachinery/pkg/labels"
"math"
"math/big"
"k8s.io/autoscaler/vertical-pod-autoscaler/pkg/utils/limitrange"

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"
Expand All @@ -31,104 +30,55 @@ import (
"k8s.io/klog"
)

// ContainerResources holds resources request for container
type ContainerResources struct {
Limits v1.ResourceList
Requests v1.ResourceList
}

func newContainerResources() ContainerResources {
return ContainerResources{
Requests: v1.ResourceList{},
Limits: v1.ResourceList{},
}
}

// RecommendationProvider gets current recommendation, annotations and vpaName for the given pod.
type RecommendationProvider interface {
GetContainersResourcesForPod(pod *v1.Pod) ([]ContainerResources, vpa_api_util.ContainerToAnnotationsMap, string, error)
GetContainersResourcesForPod(pod *v1.Pod) ([]vpa_api_util.ContainerResources, vpa_api_util.ContainerToAnnotationsMap, string, error)
}

type recommendationProvider struct {
vpaLister vpa_lister.VerticalPodAutoscalerLister
limitsRangeCalculator limitrange.LimitRangeCalculator
recommendationProcessor vpa_api_util.RecommendationProcessor
selectorFetcher target.VpaTargetSelectorFetcher
vpaLister vpa_lister.VerticalPodAutoscalerLister
}

// NewRecommendationProvider constructs the recommendation provider that list VPAs and can be used to determine recommendations for pods.
func NewRecommendationProvider(vpaLister vpa_lister.VerticalPodAutoscalerLister, recommendationProcessor vpa_api_util.RecommendationProcessor, selectorFetcher target.VpaTargetSelectorFetcher) *recommendationProvider {
func NewRecommendationProvider(calculator limitrange.LimitRangeCalculator, recommendationProcessor vpa_api_util.RecommendationProcessor,
selectorFetcher target.VpaTargetSelectorFetcher, vpaLister vpa_lister.VerticalPodAutoscalerLister) *recommendationProvider {
return &recommendationProvider{
vpaLister: vpaLister,
limitsRangeCalculator: calculator,
recommendationProcessor: recommendationProcessor,
selectorFetcher: selectorFetcher,
vpaLister: vpaLister,
}
}

func getProportionalLimit(originalLimit, originalRequest, recommendedRequest *resource.Quantity) (limit *resource.Quantity, capped bool) {
// originalLimit not set, don't set limit.
if originalLimit == nil || originalLimit.Value() == 0 {
return nil, false
}
// originalLimit set but originalRequest not set - K8s will treat the pod as if they were equal,
// recommend limit equal to request
if originalRequest == nil || originalRequest.Value() == 0 {
result := *recommendedRequest
return &result, false
}
// originalLimit and originalRequest are set. If they are equal recommend limit equal to request.
if originalRequest.MilliValue() == originalLimit.MilliValue() {
result := *recommendedRequest
return &result, false
}

// Input and output milli values should fit in int64 but intermediate values might be bigger.
originalMilliRequest := big.NewInt(originalRequest.MilliValue())
originalMilliLimit := big.NewInt(originalLimit.MilliValue())
recommendedMilliRequest := big.NewInt(recommendedRequest.MilliValue())
var recommendedMilliLimit big.Int
recommendedMilliLimit.Mul(recommendedMilliRequest, originalMilliLimit)
recommendedMilliLimit.Div(&recommendedMilliLimit, originalMilliRequest)
if recommendedMilliLimit.IsInt64() {
return resource.NewMilliQuantity(recommendedMilliLimit.Int64(), recommendedRequest.Format), false
}
return resource.NewMilliQuantity(math.MaxInt64, recommendedRequest.Format), true
}

// 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, annotations vpa_api_util.ContainerToAnnotationsMap) []ContainerResources {
resources := make([]ContainerResources, len(pod.Spec.Containers))
func GetContainersResources(pod *v1.Pod, podRecommendation vpa_types.RecommendedPodResources, limitRange *v1.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 {
resources[i] = newContainerResources()

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
}
resources[i].Requests = recommendation.Target

cpuLimit, capped := getProportionalLimit(container.Resources.Limits.Cpu(), container.Resources.Requests.Cpu(), resources[i].Requests.Cpu())
if cpuLimit != nil {
resources[i].Limits[v1.ResourceCPU] = *cpuLimit
}
if capped {
annotations[container.Name] = append(
annotations[container.Name],
fmt.Sprintf(
"Failed to keep CPU limit to request proportion of %d to %d with recommended request of %d milliCPU; doesn't fit in int64. Capping limit to MaxInt64",
container.Resources.Limits.Cpu().MilliValue(), container.Resources.Requests.Cpu().MilliValue(), resources[i].Requests.Cpu().MilliValue()))
}
memLimit, capped := getProportionalLimit(container.Resources.Limits.Memory(), container.Resources.Requests.Memory(), resources[i].Requests.Memory())
if memLimit != nil {
resources[i].Limits[v1.ResourceMemory] = *memLimit
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))
}
if capped {
annotations[container.Name] = append(
annotations[container.Name],
fmt.Sprintf(
"Failed to keep memory limit to request proportion of %d to %d with recommended request of %d milliBytes; doesn't fit in int64. Capping limit to MaxInt64",
container.Resources.Limits.Memory().MilliValue(), container.Resources.Requests.Memory().MilliValue(), resources[i].Requests.Memory().MilliValue()))
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))
}
resources[i] = vpa_api_util.ProportionallyCapResourcesToMaxLimit(recommendation.Target, cpuLimit, memLimit, maxCpuLimit, maxMemLimit)
}
return resources
}
Expand Down Expand Up @@ -164,7 +114,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) ([]ContainerResources, vpa_api_util.ContainerToAnnotationsMap, string, error) {
func (p *recommendationProvider) GetContainersResourcesForPod(pod *v1.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 All @@ -183,6 +133,11 @@ func (p *recommendationProvider) GetContainersResourcesForPod(pod *v1.Pod) ([]Co
return nil, annotations, vpaConfig.Name, err
}
}
containerResources := GetContainersResources(pod, *recommendedPodResources, annotations)
podLimitRange, err := p.limitsRangeCalculator.GetContainerLimitRangeItem(pod.Namespace)
// TODO: Support limit range on pod level.
if err != nil {
return nil, nil, "", fmt.Errorf("error getting podLimitRange: %s", err)
}
containerResources := GetContainersResources(pod, *recommendedPodResources, podLimitRange, annotations)
return containerResources, annotations, vpaConfig.Name, nil
}
98 changes: 83 additions & 15 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,6 +18,7 @@ package logic

import (
"fmt"
"k8s.io/autoscaler/vertical-pod-autoscaler/pkg/utils/limitrange"
"math"
"testing"

Expand All @@ -30,7 +31,7 @@ import (
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/test"
api "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/utils/vpa"
"k8s.io/autoscaler/vertical-pod-autoscaler/pkg/utils/vpa"
vpa_api_util "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/utils/vpa"
)

Expand All @@ -45,6 +46,15 @@ func mustParseResourcePointer(val string) *resource.Quantity {
return &q
}

type fakeLimitRangeCalculator struct {
limitRange *apiv1.LimitRangeItem
err error
}

func (nlrc *fakeLimitRangeCalculator) GetContainerLimitRangeItem(namespace string) (*apiv1.LimitRangeItem, error) {
return nlrc.limitRange, nlrc.err
}

func TestUpdateResourceRequests(t *testing.T) {
containerName := "container1"
vpaName := "vpa1"
Expand All @@ -62,7 +72,7 @@ func TestUpdateResourceRequests(t *testing.T) {
WithLabels(labels).Get()

initializedContainer := test.Container().WithName(containerName).
WithCPURequest(resource.MustParse("1")).WithMemRequest(resource.MustParse("100Mi")).Get()
WithCPURequest(resource.MustParse("1")).WithCPURequest(resource.MustParse("2")).WithMemRequest(resource.MustParse("100Mi")).Get()
initialized := test.Pod().WithName("test_initialized").
AddContainer(initializedContainer).WithLabels(labels).Get()

Expand Down Expand Up @@ -102,16 +112,19 @@ func TestUpdateResourceRequests(t *testing.T) {
vpaWithNilRecommendation.Status.Recommendation = nil

testCases := []struct {
name string
pod *apiv1.Pod
vpas []*vpa_types.VerticalPodAutoscaler
expectedAction bool
expectedMem resource.Quantity
expectedCPU resource.Quantity
expectedCPULimit *resource.Quantity
expectedMemLimit *resource.Quantity
annotations vpa_api_util.ContainerToAnnotationsMap
labelSelector string
name string
pod *apiv1.Pod
vpas []*vpa_types.VerticalPodAutoscaler
expectedAction bool
expectedError error
expectedMem resource.Quantity
expectedCPU resource.Quantity
expectedCPULimit *resource.Quantity
expectedMemLimit *resource.Quantity
limitRange *apiv1.LimitRangeItem
limitRangeCalcErr error
annotations vpa_api_util.ContainerToAnnotationsMap
labelSelector string
}{
{
name: "uninitialized pod",
Expand Down Expand Up @@ -249,12 +262,57 @@ func TestUpdateResourceRequests(t *testing.T) {
labelSelector: "app = testingApp",
annotations: vpa_api_util.ContainerToAnnotationsMap{
containerName: []string{
"Failed to keep CPU limit to request proportion of 10000 to 1000 with recommended request of -9223372036854775808 milliCPU; doesn't fit in int64. Capping limit to MaxInt64",
"Failed to keep memory limit to request proportion of 1048576000000 to 104857600000 with recommended request of -9223372036854775808 milliBytes; doesn't fit in int64. Capping limit to MaxInt64",
"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",
},
},
},
{
name: "limit range calculation error",
pod: initialized,
vpas: []*vpa_types.VerticalPodAutoscaler{vpa},
limitRangeCalcErr: fmt.Errorf("oh no"),
expectedAction: false,
expectedError: fmt.Errorf("error getting podLimitRange: oh no"),
},
{
name: "proportional limit from default",
pod: initialized,
vpas: []*vpa_types.VerticalPodAutoscaler{vpa},
expectedAction: true,
expectedCPU: resource.MustParse("2"),
expectedMem: resource.MustParse("200Mi"),
expectedCPULimit: mustParseResourcePointer("2"),
expectedMemLimit: mustParseResourcePointer("200Mi"),
labelSelector: "app = testingApp",
limitRange: &apiv1.LimitRangeItem{
Type: apiv1.LimitTypeContainer,
Default: apiv1.ResourceList{
apiv1.ResourceCPU: resource.MustParse("2"),
apiv1.ResourceMemory: resource.MustParse("100Mi"),
},
},
},
{
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 {
t.Run(fmt.Sprintf(tc.name), func(t *testing.T) {
ctrl := gomock.NewController(t)
Expand All @@ -272,8 +330,12 @@ func TestUpdateResourceRequests(t *testing.T) {

recommendationProvider := &recommendationProvider{
vpaLister: vpaLister,
recommendationProcessor: api.NewCappingRecommendationProcessor(),
recommendationProcessor: api.NewCappingRecommendationProcessor(limitrange.NewNoopLimitsCalculator()),
selectorFetcher: mockSelectorFetcher,
limitsRangeCalculator: &fakeLimitRangeCalculator{
tc.limitRange,
tc.limitRangeCalcErr,
},
}

resources, annotations, name, err := recommendationProvider.GetContainersResourcesForPod(tc.pod)
Expand Down Expand Up @@ -320,6 +382,12 @@ func TestUpdateResourceRequests(t *testing.T) {
}
} else {
assert.Empty(t, resources)
if tc.expectedError != nil {
assert.Error(t, err)
assert.Equal(t, tc.expectedError.Error(), err.Error())
} else {
assert.NoError(t, err)
}
}

})
Expand Down
11 changes: 7 additions & 4 deletions vertical-pod-autoscaler/pkg/admission-controller/logic/server.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"encoding/json"
"fmt"
"io/ioutil"
"k8s.io/autoscaler/vertical-pod-autoscaler/pkg/utils/limitrange"
"net/http"

"strings"
Expand All @@ -39,11 +40,12 @@ type AdmissionServer struct {
recommendationProvider RecommendationProvider
podPreProcessor PodPreProcessor
vpaPreProcessor VpaPreProcessor
limitsChecker limitrange.LimitRangeCalculator
}

// NewAdmissionServer constructs new AdmissionServer
func NewAdmissionServer(recommendationProvider RecommendationProvider, podPreProcessor PodPreProcessor, vpaPreProcessor VpaPreProcessor) *AdmissionServer {
return &AdmissionServer{recommendationProvider, podPreProcessor, vpaPreProcessor}
func NewAdmissionServer(recommendationProvider RecommendationProvider, podPreProcessor PodPreProcessor, vpaPreProcessor VpaPreProcessor, limitsChecker limitrange.LimitRangeCalculator) *AdmissionServer {
return &AdmissionServer{recommendationProvider, podPreProcessor, vpaPreProcessor, limitsChecker}
}

type patchRecord struct {
Expand Down Expand Up @@ -73,10 +75,11 @@ func (s *AdmissionServer) getPatchesForPodResourceRequest(raw []byte, namespace
if annotationsPerContainer == nil {
annotationsPerContainer = vpa_api_util.ContainerToAnnotationsMap{}
}

patches := []patchRecord{}
updatesAnnotation := []string{}
for i, containerResources := range containersResources {
newPatches, newUpdatesAnnotation := s.getContainerPatch(pod, i, "requests", annotationsPerContainer, containerResources)
newPatches, newUpdatesAnnotation := s.getContainerPatch(pod, i, annotationsPerContainer, containerResources)
patches = append(patches, newPatches...)
updatesAnnotation = append(updatesAnnotation, newUpdatesAnnotation)
}
Expand Down Expand Up @@ -120,7 +123,7 @@ func getAddResourceRequirementValuePatch(i int, kind string, resource v1.Resourc
Value: quantity.String()}
}

func (s *AdmissionServer) getContainerPatch(pod v1.Pod, i int, patchKind string, annotationsPerContainer vpa_api_util.ContainerToAnnotationsMap, containerResources ContainerResources) ([]patchRecord, string) {
func (s *AdmissionServer) getContainerPatch(pod v1.Pod, i int, annotationsPerContainer vpa_api_util.ContainerToAnnotationsMap, containerResources vpa_api_util.ContainerResources) ([]patchRecord, string) {
var patches []patchRecord
// Add empty resources object if missing
if pod.Spec.Containers[i].Resources.Limits == nil &&
Expand Down
Loading

0 comments on commit 63039fb

Please sign in to comment.