Skip to content

Commit

Permalink
Stop capping recommendations to limit
Browse files Browse the repository at this point in the history
Users should use ContainerResourcePolicy if they want to control VPA
recommendation range.
  • Loading branch information
jbartosik committed May 9, 2019
1 parent e8b1d04 commit 8bc98b7
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 27 deletions.
21 changes: 0 additions & 21 deletions vertical-pod-autoscaler/pkg/utils/vpa/capping.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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)
Expand All @@ -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 {
Expand Down
12 changes: 6 additions & 6 deletions vertical-pod-autoscaler/pkg/utils/vpa/capping_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down Expand Up @@ -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)
}

Expand Down

0 comments on commit 8bc98b7

Please sign in to comment.