From 8bc98b75b1ee2c1f23a5221207916e332eeab75a Mon Sep 17 00:00:00 2001 From: Joachim Bartosik Date: Thu, 9 May 2019 16:48:36 +0200 Subject: [PATCH] Stop capping recommendations to limit Users should use ContainerResourcePolicy if they want to control VPA recommendation range. --- .../pkg/utils/vpa/capping.go | 21 ------------------- .../pkg/utils/vpa/capping_test.go | 12 +++++------ 2 files changed, 6 insertions(+), 27 deletions(-) diff --git a/vertical-pod-autoscaler/pkg/utils/vpa/capping.go b/vertical-pod-autoscaler/pkg/utils/vpa/capping.go index 38cd39352eb6..0703e90123d3 100644 --- a/vertical-pod-autoscaler/pkg/utils/vpa/capping.go +++ b/vertical-pod-autoscaler/pkg/utils/vpa/capping.go @@ -37,7 +37,6 @@ type cappingAction string var ( cappedToMinAllowed cappingAction = "capped to minAllowed" cappedToMaxAllowed cappingAction = "capped to maxAllowed" - cappedToLimit cappingAction = "capped to container limit" ) func toCappingAnnotation(resourceName apiv1.ResourceName, action cappingAction) string { @@ -105,11 +104,6 @@ func getCappedRecommendationForContainer( if genAnnotations { cappingAnnotations = append(cappingAnnotations, annotations...) } - // TODO: If limits and policy are conflicting, set some condition on the VPA. - annotations = capRecommendationToContainerLimit(recommendation, container) - if genAnnotations { - cappingAnnotations = append(cappingAnnotations, annotations...) - } } process(cappedRecommendations.Target, true) @@ -119,21 +113,6 @@ func getCappedRecommendationForContainer( return cappedRecommendations, cappingAnnotations, nil } -// capRecommendationToContainerLimit makes sure recommendation is not above current limit for the container. -// If this function makes adjustments appropriate annotations are returned. -func capRecommendationToContainerLimit(recommendation apiv1.ResourceList, container apiv1.Container) []string { - annotations := make([]string, 0) - // Iterate over limits set in the container. Unset means Infinite limit. - for resourceName, limit := range container.Resources.Limits { - recommendedValue, found := recommendation[resourceName] - if found && recommendedValue.MilliValue() > limit.MilliValue() { - recommendation[resourceName] = limit - annotations = append(annotations, toCappingAnnotation(resourceName, cappedToLimit)) - } - } - return annotations -} - // applyVPAPolicy updates recommendation if recommended resources are outside of limits defined in VPA resources policy func applyVPAPolicy(recommendation apiv1.ResourceList, policy *vpa_types.ContainerResourcePolicy) []string { if policy == nil { diff --git a/vertical-pod-autoscaler/pkg/utils/vpa/capping_test.go b/vertical-pod-autoscaler/pkg/utils/vpa/capping_test.go index 847332a0b992..e0ad3d342a83 100644 --- a/vertical-pod-autoscaler/pkg/utils/vpa/capping_test.go +++ b/vertical-pod-autoscaler/pkg/utils/vpa/capping_test.go @@ -47,7 +47,7 @@ func TestRecommendationNotAvailable(t *testing.T) { assert.Empty(t, res.ContainerRecommendations) } -func TestRecommendationCappedToLimit(t *testing.T) { +func TestRecommendationNotCappedToLimit(t *testing.T) { pod := test.Pod().WithName("pod1").AddContainer(test.BuildTestContainer("ctr-name", "", "")).Get() pod.Spec.Containers[0].Resources.Limits = apiv1.ResourceList{ @@ -75,16 +75,16 @@ func TestRecommendationCappedToLimit(t *testing.T) { res, annotations, err := NewCappingRecommendationProcessor().Apply(&podRecommendation, &policy, nil, pod) assert.Nil(t, err) assert.Equal(t, apiv1.ResourceList{ - apiv1.ResourceCPU: *resource.NewScaledQuantity(3, 1), - apiv1.ResourceMemory: *resource.NewScaledQuantity(7000, 1), + apiv1.ResourceCPU: *resource.NewScaledQuantity(10, 1), + apiv1.ResourceMemory: *resource.NewScaledQuantity(8000, 1), }, res.ContainerRecommendations[0].Target) - assert.Contains(t, annotations, "ctr-name") - assert.Contains(t, annotations["ctr-name"], "memory capped to container limit") + assert.NotContains(t, annotations, "ctr-name") + assert.NotContains(t, annotations["ctr-name"], "memory capped to container limit") assert.Equal(t, apiv1.ResourceList{ apiv1.ResourceCPU: *resource.NewScaledQuantity(2, 1), - apiv1.ResourceMemory: *resource.NewScaledQuantity(7000, 1), + apiv1.ResourceMemory: *resource.NewScaledQuantity(9000, 1), }, res.ContainerRecommendations[0].UpperBound) }