From c90abe6dd0b811b0ef0e81fe0b868b8ba5641299 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Johannes=20W=C3=BCrbach?= Date: Sat, 6 Jun 2020 15:19:16 +0200 Subject: [PATCH 1/5] vpa: add container controlled values to types --- .../pkg/apis/autoscaling.k8s.io/v1/types.go | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/vertical-pod-autoscaler/pkg/apis/autoscaling.k8s.io/v1/types.go b/vertical-pod-autoscaler/pkg/apis/autoscaling.k8s.io/v1/types.go index b01bb7d26c77..1b25eff4919b 100644 --- a/vertical-pod-autoscaler/pkg/apis/autoscaling.k8s.io/v1/types.go +++ b/vertical-pod-autoscaler/pkg/apis/autoscaling.k8s.io/v1/types.go @@ -152,6 +152,11 @@ type ContainerResourcePolicy struct { // (and possibly applied) by VPA. // If not specified, the default of [ResourceCPU, ResourceMemory] will be used. ControlledResources *[]v1.ResourceName `json:"controlledResources,omitempty" patchStrategy:"merge" protobuf:"bytes,5,rep,name=controlledResources"` + + // Specifies which resource values should be controlled. + // The default is "RequestsAndLimits". + // +optional + ControlledValues *ContainerControlledValues `json:"controlledValues,omitempty" protobuf:"bytes,6,rep,name=controlledValues"` } const ( @@ -171,6 +176,17 @@ const ( ContainerScalingModeOff ContainerScalingMode = "Off" ) +// ContainerControlledValues controls which resource value should be autoscaled. +type ContainerControlledValues string + +const ( + // ContainerControlledValuesRequestsAndLimits means resource request and limits + // are scaled automatically. The limit is scaled proportionally to the request. + ContainerControlledValuesRequestsAndLimits ContainerControlledValues = "RequestsAndLimits" + // ContainerControlledValuesRequestsOnly means only requested resource is autoscaled. + ContainerControlledValuesRequestsOnly ContainerControlledValues = "RequestsOnly" +) + // VerticalPodAutoscalerStatus describes the runtime state of the autoscaler. type VerticalPodAutoscalerStatus struct { // The most recently computed amount of resources recommended by the From 33eab5ef26c33f2112a467ebfe4da5c1742c882e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Johannes=20W=C3=BCrbach?= Date: Sat, 6 Jun 2020 15:19:52 +0200 Subject: [PATCH 2/5] vpa: update-codegen types --- .../pkg/apis/autoscaling.k8s.io/v1/zz_generated.deepcopy.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/vertical-pod-autoscaler/pkg/apis/autoscaling.k8s.io/v1/zz_generated.deepcopy.go b/vertical-pod-autoscaler/pkg/apis/autoscaling.k8s.io/v1/zz_generated.deepcopy.go index a54a650106e7..dfd2eadd99f7 100644 --- a/vertical-pod-autoscaler/pkg/apis/autoscaling.k8s.io/v1/zz_generated.deepcopy.go +++ b/vertical-pod-autoscaler/pkg/apis/autoscaling.k8s.io/v1/zz_generated.deepcopy.go @@ -57,6 +57,11 @@ func (in *ContainerResourcePolicy) DeepCopyInto(out *ContainerResourcePolicy) { copy(*out, *in) } } + if in.ControlledValues != nil { + in, out := &in.ControlledValues, &out.ControlledValues + *out = new(ContainerControlledValues) + **out = **in + } return } From 4fdd927ade184d3bff6463ebd4639116ad3247ce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Johannes=20W=C3=BCrbach?= Date: Sat, 6 Jun 2020 15:20:48 +0200 Subject: [PATCH 3/5] vpa: container controlled values --- .../recommendation/recommendation_provider.go | 30 ++- .../recommendation_provider_test.go | 39 +++- .../resource/vpa/handler.go | 6 + .../resource/vpa/handler_test.go | 19 ++ .../pkg/utils/test/test_vpa.go | 15 +- .../pkg/utils/vpa/capping.go | 21 ++ .../pkg/utils/vpa/capping_test.go | 192 ++++++++++++++---- 7 files changed, 270 insertions(+), 52 deletions(-) diff --git a/vertical-pod-autoscaler/pkg/admission-controller/resource/pod/recommendation/recommendation_provider.go b/vertical-pod-autoscaler/pkg/admission-controller/resource/pod/recommendation/recommendation_provider.go index dd2b092f99e1..ba22f2d39c9a 100644 --- a/vertical-pod-autoscaler/pkg/admission-controller/resource/pod/recommendation/recommendation_provider.go +++ b/vertical-pod-autoscaler/pkg/admission-controller/resource/pod/recommendation/recommendation_provider.go @@ -46,7 +46,7 @@ func NewProvider(calculator limitrange.LimitRangeCalculator, } // 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 { @@ -60,17 +60,29 @@ func GetContainersResources(pod *core.Pod, podRecommendation vpa_types.Recommend if limitRange != nil { defaultLimit = limitRange.Default } - 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...) + containerControlledValues := GetContainerControlledValues(container.Name, vpaResourcePolicy) + if containerControlledValues == vpa_types.ContainerControlledValuesRequestsAndLimits { + 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...) + } } } } return resources } +// GetContainerControlledValues returns controlled resource values +func GetContainerControlledValues(name string, vpaResourcePolicy *vpa_types.PodResourcePolicy) vpa_types.ContainerControlledValues { + containerPolicy := vpa_api_util.GetContainerResourcePolicy(name, vpaResourcePolicy) + if containerPolicy == nil || containerPolicy.ControlledValues == nil { + return vpa_types.ContainerControlledValuesRequestsAndLimits + } + return *containerPolicy.ControlledValues +} + // GetContainersResourcesForPod returns recommended request for a given pod and associated annotations. // The returned slice corresponds 1-1 to containers in the Pod. func (p *recommendationProvider) GetContainersResourcesForPod(pod *core.Pod, vpa *vpa_types.VerticalPodAutoscaler) ([]vpa_api_util.ContainerResources, vpa_api_util.ContainerToAnnotationsMap, error) { @@ -95,6 +107,10 @@ func (p *recommendationProvider) GetContainersResourcesForPod(pod *core.Pod, vpa 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 vpa.Spec.UpdatePolicy == nil || vpa.Spec.UpdatePolicy.UpdateMode == nil || *vpa.Spec.UpdatePolicy.UpdateMode != vpa_types.UpdateModeOff { + resourcePolicy = vpa.Spec.ResourcePolicy + } + containerResources := GetContainersResources(pod, resourcePolicy, *recommendedPodResources, containerLimitRange, annotations) return containerResources, annotations, nil } diff --git a/vertical-pod-autoscaler/pkg/admission-controller/resource/pod/recommendation/recommendation_provider_test.go b/vertical-pod-autoscaler/pkg/admission-controller/resource/pod/recommendation/recommendation_provider_test.go index 6f32d42b2681..5f48f076443f 100644 --- a/vertical-pod-autoscaler/pkg/admission-controller/resource/pod/recommendation/recommendation_provider_test.go +++ b/vertical-pod-autoscaler/pkg/admission-controller/resource/pod/recommendation/recommendation_provider_test.go @@ -100,6 +100,11 @@ func TestUpdateResourceRequests(t *testing.T) { vpaWithHighMemory := vpaBuilder.WithTarget("2", "1000Mi").WithMaxAllowed("3", "3Gi").Get() vpaWithExabyteRecommendation := vpaBuilder.WithTarget("1Ei", "1Ei").WithMaxAllowed("1Ei", "1Ei").Get() + resourceRequestsAndLimitsVPA := vpaBuilder.WithControlledValues(vpa_types.ContainerControlledValuesRequestsAndLimits).Get() + resourceRequestsOnlyVPA := vpaBuilder.WithControlledValues(vpa_types.ContainerControlledValuesRequestsOnly).Get() + resourceRequestsOnlyVPAHighTarget := vpaBuilder.WithControlledValues(vpa_types.ContainerControlledValuesRequestsOnly). + WithTarget("3", "500Mi").WithMaxAllowed("5", "1Gi").Get() + vpaWithEmptyRecommendation := vpaBuilder.Get() vpaWithEmptyRecommendation.Status.Recommendation = &vpa_types.RecommendedPodResources{} vpaWithNilRecommendation := vpaBuilder.Get() @@ -201,7 +206,7 @@ func TestUpdateResourceRequests(t *testing.T) { expectedMemLimit: mustParseResourcePointer("200Mi"), }, { - name: "proportional limit", + name: "proportional limit - as default", pod: podWithDoubleLimit, vpa: vpa, expectedAction: true, @@ -210,6 +215,38 @@ func TestUpdateResourceRequests(t *testing.T) { expectedCPULimit: mustParseResourcePointer("4"), expectedMemLimit: mustParseResourcePointer("400Mi"), }, + { + name: "proportional limit - set explicit", + pod: podWithDoubleLimit, + vpa: resourceRequestsAndLimitsVPA, + expectedAction: true, + expectedCPU: resource.MustParse("2"), + expectedMem: resource.MustParse("200Mi"), + expectedCPULimit: mustParseResourcePointer("4"), + expectedMemLimit: mustParseResourcePointer("400Mi"), + }, + { + name: "disabled limit scaling", + pod: podWithDoubleLimit, + vpa: resourceRequestsOnlyVPA, + expectedAction: true, + expectedCPU: resource.MustParse("2"), + expectedMem: resource.MustParse("200Mi"), + }, + { + name: "disabled limit scaling - requests capped at limit", + pod: podWithDoubleLimit, + vpa: resourceRequestsOnlyVPAHighTarget, + expectedAction: true, + expectedCPU: resource.MustParse("2"), + expectedMem: resource.MustParse("200Mi"), + annotations: vpa_api_util.ContainerToAnnotationsMap{ + containerName: []string{ + "cpu capped to container limit", + "memory capped to container limit", + }, + }, + }, { name: "limit over int64", pod: podWithTenfoldLimit, diff --git a/vertical-pod-autoscaler/pkg/admission-controller/resource/vpa/handler.go b/vertical-pod-autoscaler/pkg/admission-controller/resource/vpa/handler.go index 2a3c720e3fb3..0b159068eed6 100644 --- a/vertical-pod-autoscaler/pkg/admission-controller/resource/vpa/handler.go +++ b/vertical-pod-autoscaler/pkg/admission-controller/resource/vpa/handler.go @@ -134,6 +134,12 @@ func validateVPA(vpa *vpa_types.VerticalPodAutoscaler, isCreate bool) error { return fmt.Errorf("max resource for %v is lower than min", resource) } } + ControlledValues := policy.ControlledValues + if mode != nil && ControlledValues != nil { + if *mode == vpa_types.ContainerScalingModeOff && *ControlledValues == vpa_types.ContainerControlledValuesRequestsAndLimits { + return fmt.Errorf("ControlledValues shouldn't be specified if container scaling mode is off.") + } + } } } diff --git a/vertical-pod-autoscaler/pkg/admission-controller/resource/vpa/handler_test.go b/vertical-pod-autoscaler/pkg/admission-controller/resource/vpa/handler_test.go index be2549c67475..80ecb7207499 100644 --- a/vertical-pod-autoscaler/pkg/admission-controller/resource/vpa/handler_test.go +++ b/vertical-pod-autoscaler/pkg/admission-controller/resource/vpa/handler_test.go @@ -36,6 +36,8 @@ func TestValidateVPA(t *testing.T) { validUpdateMode := vpa_types.UpdateModeOff badScalingMode := vpa_types.ContainerScalingMode("bad") validScalingMode := vpa_types.ContainerScalingModeAuto + scalingModeOff := vpa_types.ContainerScalingModeOff + controlledValuesRequestsAndLimits := vpa_types.ContainerControlledValuesRequestsAndLimits tests := []struct { name string vpa vpa_types.VerticalPodAutoscaler @@ -120,6 +122,23 @@ func TestValidateVPA(t *testing.T) { }, expectError: fmt.Errorf("max resource for cpu is lower than min"), }, + { + name: "scaling off with controlled values requests and limits", + vpa: vpa_types.VerticalPodAutoscaler{ + Spec: vpa_types.VerticalPodAutoscalerSpec{ + ResourcePolicy: &vpa_types.PodResourcePolicy{ + ContainerPolicies: []vpa_types.ContainerResourcePolicy{ + { + ContainerName: "loot box", + Mode: &scalingModeOff, + ControlledValues: &controlledValuesRequestsAndLimits, + }, + }, + }, + }, + }, + expectError: fmt.Errorf("ControlledValues shouldn't be specified if container scaling mode is off."), + }, { name: "all valid", vpa: vpa_types.VerticalPodAutoscaler{ diff --git a/vertical-pod-autoscaler/pkg/utils/test/test_vpa.go b/vertical-pod-autoscaler/pkg/utils/test/test_vpa.go index 359624804338..1da3a076aff3 100644 --- a/vertical-pod-autoscaler/pkg/utils/test/test_vpa.go +++ b/vertical-pod-autoscaler/pkg/utils/test/test_vpa.go @@ -34,6 +34,7 @@ type VerticalPodAutoscalerBuilder interface { WithCreationTimestamp(timestamp time.Time) VerticalPodAutoscalerBuilder WithMinAllowed(cpu, memory string) VerticalPodAutoscalerBuilder WithMaxAllowed(cpu, memory string) VerticalPodAutoscalerBuilder + WithControlledValues(mode vpa_types.ContainerControlledValues) VerticalPodAutoscalerBuilder WithTarget(cpu, memory string) VerticalPodAutoscalerBuilder WithLowerBound(cpu, memory string) VerticalPodAutoscalerBuilder WithTargetRef(targetRef *autoscaling.CrossVersionObjectReference) VerticalPodAutoscalerBuilder @@ -63,6 +64,7 @@ type verticalPodAutoscalerBuilder struct { creationTimestamp time.Time minAllowed core.ResourceList maxAllowed core.ResourceList + ControlledValues *vpa_types.ContainerControlledValues recommendation RecommendationBuilder conditions []vpa_types.VerticalPodAutoscalerCondition annotations map[string]string @@ -115,6 +117,12 @@ func (b *verticalPodAutoscalerBuilder) WithMaxAllowed(cpu, memory string) Vertic return &c } +func (b *verticalPodAutoscalerBuilder) WithControlledValues(mode vpa_types.ContainerControlledValues) VerticalPodAutoscalerBuilder { + c := *b + c.ControlledValues = &mode + return &c +} + func (b *verticalPodAutoscalerBuilder) WithTarget(cpu, memory string) VerticalPodAutoscalerBuilder { c := *b c.recommendation = c.recommendation.WithTarget(cpu, memory) @@ -168,9 +176,10 @@ func (b *verticalPodAutoscalerBuilder) Get() *vpa_types.VerticalPodAutoscaler { panic("Must call WithContainer() before Get()") } resourcePolicy := vpa_types.PodResourcePolicy{ContainerPolicies: []vpa_types.ContainerResourcePolicy{{ - ContainerName: b.containerName, - MinAllowed: b.minAllowed, - MaxAllowed: b.maxAllowed, + ContainerName: b.containerName, + MinAllowed: b.minAllowed, + MaxAllowed: b.maxAllowed, + ControlledValues: b.ControlledValues, }}} recommendation := b.recommendation.WithContainer(b.containerName).Get() diff --git a/vertical-pod-autoscaler/pkg/utils/vpa/capping.go b/vertical-pod-autoscaler/pkg/utils/vpa/capping.go index d8b39857c2d9..985a32d4245f 100644 --- a/vertical-pod-autoscaler/pkg/utils/vpa/capping.go +++ b/vertical-pod-autoscaler/pkg/utils/vpa/capping.go @@ -37,6 +37,7 @@ type cappingAction string var ( cappedToMinAllowed cappingAction = "capped to minAllowed" cappedToMaxAllowed cappingAction = "capped to maxAllowed" + cappedToLimit cappingAction = "capped to container limit" cappedProportionallyToMaxLimit cappingAction = "capped to fit Max in container LimitRange" cappedProportionallyToMinLimit cappingAction = "capped to fit Min in container LimitRange" ) @@ -121,6 +122,11 @@ func getCappedRecommendationForContainer( cappingAnnotations = append(cappingAnnotations, limitAnnotations...) 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) @@ -130,6 +136,21 @@ 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 f74b0364721f..6c8d2eb5a97d 100644 --- a/vertical-pod-autoscaler/pkg/utils/vpa/capping_test.go +++ b/vertical-pod-autoscaler/pkg/utils/vpa/capping_test.go @@ -47,6 +47,45 @@ func TestRecommendationNotAvailable(t *testing.T) { assert.Empty(t, res.ContainerRecommendations) } +func TestRecommendationCappedToLimit(t *testing.T) { + pod := test.Pod().WithName("pod1").AddContainer(test.BuildTestContainer("ctr-name", "", "")).Get() + pod.Spec.Containers[0].Resources.Limits = + apiv1.ResourceList{ + apiv1.ResourceCPU: *resource.NewScaledQuantity(3, 1), + apiv1.ResourceMemory: *resource.NewScaledQuantity(7000, 1), + } + podRecommendation := vpa_types.RecommendedPodResources{ + ContainerRecommendations: []vpa_types.RecommendedContainerResources{ + { + ContainerName: "ctr-name", + Target: apiv1.ResourceList{ + apiv1.ResourceCPU: *resource.NewScaledQuantity(10, 1), + apiv1.ResourceMemory: *resource.NewScaledQuantity(8000, 1), + }, + UpperBound: apiv1.ResourceList{ + apiv1.ResourceCPU: *resource.NewScaledQuantity(2, 1), + apiv1.ResourceMemory: *resource.NewScaledQuantity(9000, 1), + }, + }, + }, + } + policy := vpa_types.PodResourcePolicy{} + res, annotations, err := NewCappingRecommendationProcessor(&fakeLimitRangeCalculator{}).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), + }, res.ContainerRecommendations[0].Target) + + assert.Contains(t, annotations, "ctr-name") + assert.Contains(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), + }, res.ContainerRecommendations[0].UpperBound) +} + func TestRecommendationCappedToMinMaxPolicy(t *testing.T) { pod := test.Pod().WithName("pod1").AddContainer(test.BuildTestContainer("ctr-name", "", "")).Get() podRecommendation := vpa_types.RecommendedPodResources{ @@ -566,59 +605,130 @@ func TestApplyPodLimitRange(t *testing.T) { } func TestApplyLimitRangeMinToRequest(t *testing.T) { - limitRange := apiv1.LimitRangeItem{ - Type: apiv1.LimitTypeContainer, - Min: apiv1.ResourceList{ - apiv1.ResourceMemory: resource.MustParse("500M"), - }, - } - recommendation := vpa_types.RecommendedPodResources{ - ContainerRecommendations: []vpa_types.RecommendedContainerResources{ - { - ContainerName: "container", - Target: apiv1.ResourceList{ - apiv1.ResourceCPU: resource.MustParse("1"), - apiv1.ResourceMemory: resource.MustParse("200M"), + tests := []struct { + name string + resources vpa_types.RecommendedPodResources + pod apiv1.Pod + limitRange apiv1.LimitRangeItem + expect vpa_types.RecommendedPodResources + expectAnnotations []string + }{ + { + name: "caps to min range if above container limit", + resources: vpa_types.RecommendedPodResources{ + ContainerRecommendations: []vpa_types.RecommendedContainerResources{ + { + ContainerName: "container", + Target: apiv1.ResourceList{ + apiv1.ResourceCPU: resource.MustParse("1"), + apiv1.ResourceMemory: resource.MustParse("200M"), + }, + }, }, }, - }, - } - pod := apiv1.Pod{ - Spec: apiv1.PodSpec{ - Containers: []apiv1.Container{ - { - Name: "container", - Resources: apiv1.ResourceRequirements{ - Requests: apiv1.ResourceList{ + pod: apiv1.Pod{ + Spec: apiv1.PodSpec{ + Containers: []apiv1.Container{ + { + Name: "container", + Resources: apiv1.ResourceRequirements{ + Requests: apiv1.ResourceList{ + apiv1.ResourceCPU: resource.MustParse("1"), + apiv1.ResourceMemory: resource.MustParse("50M"), + }, + Limits: apiv1.ResourceList{ + apiv1.ResourceCPU: resource.MustParse("1"), + apiv1.ResourceMemory: resource.MustParse("600M"), + }, + }, + }, + }, + }, + }, + limitRange: apiv1.LimitRangeItem{ + Type: apiv1.LimitTypeContainer, + Min: apiv1.ResourceList{ + apiv1.ResourceMemory: resource.MustParse("500M"), + }, + }, + expect: vpa_types.RecommendedPodResources{ + ContainerRecommendations: []vpa_types.RecommendedContainerResources{ + { + ContainerName: "container", + Target: apiv1.ResourceList{ apiv1.ResourceCPU: resource.MustParse("1"), - apiv1.ResourceMemory: resource.MustParse("50M"), + apiv1.ResourceMemory: resource.MustParse("500M"), }, - Limits: apiv1.ResourceList{ + }, + }, + }, + expectAnnotations: []string{ + "memory capped to fit Min in container LimitRange", + }, + }, { + name: "caps to container limit if below container limit", + resources: vpa_types.RecommendedPodResources{ + ContainerRecommendations: []vpa_types.RecommendedContainerResources{ + { + ContainerName: "container", + Target: apiv1.ResourceList{ apiv1.ResourceCPU: resource.MustParse("1"), - apiv1.ResourceMemory: resource.MustParse("100M"), + apiv1.ResourceMemory: resource.MustParse("200M"), }, }, }, }, - }, - } - expectedRecommendation := vpa_types.RecommendedPodResources{ - ContainerRecommendations: []vpa_types.RecommendedContainerResources{ - { - ContainerName: "container", - Target: apiv1.ResourceList{ - apiv1.ResourceCPU: resource.MustParse("1"), + pod: apiv1.Pod{ + Spec: apiv1.PodSpec{ + Containers: []apiv1.Container{ + { + Name: "container", + Resources: apiv1.ResourceRequirements{ + Requests: apiv1.ResourceList{ + apiv1.ResourceCPU: resource.MustParse("1"), + apiv1.ResourceMemory: resource.MustParse("50M"), + }, + Limits: apiv1.ResourceList{ + apiv1.ResourceCPU: resource.MustParse("1"), + apiv1.ResourceMemory: resource.MustParse("100M"), + }, + }, + }, + }, + }, + }, + limitRange: apiv1.LimitRangeItem{ + Type: apiv1.LimitTypeContainer, + Min: apiv1.ResourceList{ apiv1.ResourceMemory: resource.MustParse("500M"), }, }, + expect: vpa_types.RecommendedPodResources{ + ContainerRecommendations: []vpa_types.RecommendedContainerResources{ + { + ContainerName: "container", + Target: apiv1.ResourceList{ + apiv1.ResourceCPU: resource.MustParse("1"), + apiv1.ResourceMemory: resource.MustParse("100M"), + }, + }, + }, + }, + expectAnnotations: []string{ + "memory capped to fit Min in container LimitRange", + "memory capped to container limit", + }, }, } - - calculator := fakeLimitRangeCalculator{containerLimitRange: limitRange} - processor := NewCappingRecommendationProcessor(&calculator) - processedRecommendation, annotations, err := processor.Apply(&recommendation, nil, nil, &pod) - assert.NoError(t, err) - assert.Contains(t, annotations, "container") - assert.ElementsMatch(t, []string{"memory capped to fit Min in container LimitRange"}, annotations["container"]) - assert.Equal(t, expectedRecommendation, *processedRecommendation) + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + calculator := fakeLimitRangeCalculator{containerLimitRange: tc.limitRange} + processor := NewCappingRecommendationProcessor(&calculator) + processedRecommendation, annotations, err := processor.Apply(&tc.resources, nil, nil, &tc.pod) + assert.NoError(t, err) + assert.Contains(t, annotations, "container") + assert.ElementsMatch(t, tc.expectAnnotations, annotations["container"]) + assert.Equal(t, tc.expect, *processedRecommendation) + }) + } } From f251c9c10e6dd0d0d50b2f974a1c291d6dc75008 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Johannes=20W=C3=BCrbach?= Date: Sat, 6 Jun 2020 15:27:49 +0200 Subject: [PATCH 4/5] vpa: e2e update vpa --- vertical-pod-autoscaler/e2e/go.mod | 3 +-- vertical-pod-autoscaler/e2e/go.sum | 2 -- .../pkg/apis/autoscaling.k8s.io/v1/types.go | 21 +++++++++++++++++++ .../v1/zz_generated.deepcopy.go | 18 ++++++++++++++-- .../v1beta1/zz_generated.deepcopy.go | 4 ++-- .../v1beta2/zz_generated.deepcopy.go | 4 ++-- .../v1alpha1/zz_generated.deepcopy.go | 4 ++-- .../e2e/vendor/modules.txt | 2 +- 8 files changed, 45 insertions(+), 13 deletions(-) diff --git a/vertical-pod-autoscaler/e2e/go.mod b/vertical-pod-autoscaler/e2e/go.mod index 3b8878c3c6f2..32095be59107 100644 --- a/vertical-pod-autoscaler/e2e/go.mod +++ b/vertical-pod-autoscaler/e2e/go.mod @@ -18,7 +18,6 @@ require ( github.com/onsi/gomega v1.5.0 github.com/prometheus/procfs v0.0.6 // indirect github.com/spf13/pflag v1.0.5 // indirect - golang.org/x/crypto v0.0.0-20200221231518-2aa609cf4a9d // indirect golang.org/x/net v0.0.0-20191112182307-2180aed22343 // indirect golang.org/x/sys v0.0.0-20191112214154-59a1497f0cea // indirect google.golang.org/appengine v1.6.5 // indirect @@ -26,7 +25,7 @@ require ( gopkg.in/yaml.v2 v2.2.5 // indirect k8s.io/api v0.0.0 k8s.io/apimachinery v0.0.0 - k8s.io/autoscaler/vertical-pod-autoscaler v0.0.0-20200207133143-e7988a6dd041 + k8s.io/autoscaler/vertical-pod-autoscaler v0.0.0-20200605154545-936eea18fb1d k8s.io/client-go v11.0.0+incompatible k8s.io/component-base v0.0.0 k8s.io/klog v1.0.0 diff --git a/vertical-pod-autoscaler/e2e/go.sum b/vertical-pod-autoscaler/e2e/go.sum index b2fe2e0f4219..a91ea6b3d213 100644 --- a/vertical-pod-autoscaler/e2e/go.sum +++ b/vertical-pod-autoscaler/e2e/go.sum @@ -545,8 +545,6 @@ golang.org/x/crypto v0.0.0-20190320223903-b7391e95e576/go.mod h1:djNgcEr1/C05ACk golang.org/x/crypto v0.0.0-20190510104115-cbcb75029529/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI= golang.org/x/crypto v0.0.0-20190611184440-5c40567a22f8 h1:1wopBVtVdWnn03fZelqdXTqk7U7zPQCb+T4rbU9ZEoU= golang.org/x/crypto v0.0.0-20190611184440-5c40567a22f8/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI= -golang.org/x/crypto v0.0.0-20191112222119-e1110fd1c708 h1:pXVtWnwHkrWD9ru3sDxY/qFK/bfc0egRovX91EjWjf4= -golang.org/x/crypto v0.0.0-20191112222119-e1110fd1c708/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto= golang.org/x/crypto v0.0.0-20200221231518-2aa609cf4a9d h1:1ZiEyfaQIg3Qh0EoqpwAakHVhecoE5wlSg5GjnafJGw= golang.org/x/crypto v0.0.0-20200221231518-2aa609cf4a9d/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto= golang.org/x/exp v0.0.0-20190121172915-509febef88a4/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA= diff --git a/vertical-pod-autoscaler/e2e/vendor/k8s.io/autoscaler/vertical-pod-autoscaler/pkg/apis/autoscaling.k8s.io/v1/types.go b/vertical-pod-autoscaler/e2e/vendor/k8s.io/autoscaler/vertical-pod-autoscaler/pkg/apis/autoscaling.k8s.io/v1/types.go index f2be65511b87..1b25eff4919b 100644 --- a/vertical-pod-autoscaler/e2e/vendor/k8s.io/autoscaler/vertical-pod-autoscaler/pkg/apis/autoscaling.k8s.io/v1/types.go +++ b/vertical-pod-autoscaler/e2e/vendor/k8s.io/autoscaler/vertical-pod-autoscaler/pkg/apis/autoscaling.k8s.io/v1/types.go @@ -147,6 +147,16 @@ type ContainerResourcePolicy struct { // for the container. The default is no maximum. // +optional MaxAllowed v1.ResourceList `json:"maxAllowed,omitempty" protobuf:"bytes,4,rep,name=maxAllowed,casttype=ResourceList,castkey=ResourceName"` + + // Specifies the type of recommendations that will be computed + // (and possibly applied) by VPA. + // If not specified, the default of [ResourceCPU, ResourceMemory] will be used. + ControlledResources *[]v1.ResourceName `json:"controlledResources,omitempty" patchStrategy:"merge" protobuf:"bytes,5,rep,name=controlledResources"` + + // Specifies which resource values should be controlled. + // The default is "RequestsAndLimits". + // +optional + ControlledValues *ContainerControlledValues `json:"controlledValues,omitempty" protobuf:"bytes,6,rep,name=controlledValues"` } const ( @@ -166,6 +176,17 @@ const ( ContainerScalingModeOff ContainerScalingMode = "Off" ) +// ContainerControlledValues controls which resource value should be autoscaled. +type ContainerControlledValues string + +const ( + // ContainerControlledValuesRequestsAndLimits means resource request and limits + // are scaled automatically. The limit is scaled proportionally to the request. + ContainerControlledValuesRequestsAndLimits ContainerControlledValues = "RequestsAndLimits" + // ContainerControlledValuesRequestsOnly means only requested resource is autoscaled. + ContainerControlledValuesRequestsOnly ContainerControlledValues = "RequestsOnly" +) + // VerticalPodAutoscalerStatus describes the runtime state of the autoscaler. type VerticalPodAutoscalerStatus struct { // The most recently computed amount of resources recommended by the diff --git a/vertical-pod-autoscaler/e2e/vendor/k8s.io/autoscaler/vertical-pod-autoscaler/pkg/apis/autoscaling.k8s.io/v1/zz_generated.deepcopy.go b/vertical-pod-autoscaler/e2e/vendor/k8s.io/autoscaler/vertical-pod-autoscaler/pkg/apis/autoscaling.k8s.io/v1/zz_generated.deepcopy.go index b7baaf21e90d..dfd2eadd99f7 100644 --- a/vertical-pod-autoscaler/e2e/vendor/k8s.io/autoscaler/vertical-pod-autoscaler/pkg/apis/autoscaling.k8s.io/v1/zz_generated.deepcopy.go +++ b/vertical-pod-autoscaler/e2e/vendor/k8s.io/autoscaler/vertical-pod-autoscaler/pkg/apis/autoscaling.k8s.io/v1/zz_generated.deepcopy.go @@ -48,6 +48,20 @@ func (in *ContainerResourcePolicy) DeepCopyInto(out *ContainerResourcePolicy) { (*out)[key] = val.DeepCopy() } } + if in.ControlledResources != nil { + in, out := &in.ControlledResources, &out.ControlledResources + *out = new([]corev1.ResourceName) + if **in != nil { + in, out := *in, *out + *out = make([]corev1.ResourceName, len(*in)) + copy(*out, *in) + } + } + if in.ControlledValues != nil { + in, out := &in.ControlledValues, &out.ControlledValues + *out = new(ContainerControlledValues) + **out = **in + } return } @@ -256,7 +270,7 @@ func (in *VerticalPodAutoscalerCheckpoint) DeepCopyObject() runtime.Object { func (in *VerticalPodAutoscalerCheckpointList) DeepCopyInto(out *VerticalPodAutoscalerCheckpointList) { *out = *in out.TypeMeta = in.TypeMeta - out.ListMeta = in.ListMeta + in.ListMeta.DeepCopyInto(&out.ListMeta) if in.Items != nil { in, out := &in.Items, &out.Items *out = make([]VerticalPodAutoscalerCheckpoint, len(*in)) @@ -343,7 +357,7 @@ func (in *VerticalPodAutoscalerCondition) DeepCopy() *VerticalPodAutoscalerCondi func (in *VerticalPodAutoscalerList) DeepCopyInto(out *VerticalPodAutoscalerList) { *out = *in out.TypeMeta = in.TypeMeta - out.ListMeta = in.ListMeta + in.ListMeta.DeepCopyInto(&out.ListMeta) if in.Items != nil { in, out := &in.Items, &out.Items *out = make([]VerticalPodAutoscaler, len(*in)) diff --git a/vertical-pod-autoscaler/e2e/vendor/k8s.io/autoscaler/vertical-pod-autoscaler/pkg/apis/autoscaling.k8s.io/v1beta1/zz_generated.deepcopy.go b/vertical-pod-autoscaler/e2e/vendor/k8s.io/autoscaler/vertical-pod-autoscaler/pkg/apis/autoscaling.k8s.io/v1beta1/zz_generated.deepcopy.go index 17414894a2b2..74a0f94d1be2 100644 --- a/vertical-pod-autoscaler/e2e/vendor/k8s.io/autoscaler/vertical-pod-autoscaler/pkg/apis/autoscaling.k8s.io/v1beta1/zz_generated.deepcopy.go +++ b/vertical-pod-autoscaler/e2e/vendor/k8s.io/autoscaler/vertical-pod-autoscaler/pkg/apis/autoscaling.k8s.io/v1beta1/zz_generated.deepcopy.go @@ -256,7 +256,7 @@ func (in *VerticalPodAutoscalerCheckpoint) DeepCopyObject() runtime.Object { func (in *VerticalPodAutoscalerCheckpointList) DeepCopyInto(out *VerticalPodAutoscalerCheckpointList) { *out = *in out.TypeMeta = in.TypeMeta - out.ListMeta = in.ListMeta + in.ListMeta.DeepCopyInto(&out.ListMeta) if in.Items != nil { in, out := &in.Items, &out.Items *out = make([]VerticalPodAutoscalerCheckpoint, len(*in)) @@ -343,7 +343,7 @@ func (in *VerticalPodAutoscalerCondition) DeepCopy() *VerticalPodAutoscalerCondi func (in *VerticalPodAutoscalerList) DeepCopyInto(out *VerticalPodAutoscalerList) { *out = *in out.TypeMeta = in.TypeMeta - out.ListMeta = in.ListMeta + in.ListMeta.DeepCopyInto(&out.ListMeta) if in.Items != nil { in, out := &in.Items, &out.Items *out = make([]VerticalPodAutoscaler, len(*in)) diff --git a/vertical-pod-autoscaler/e2e/vendor/k8s.io/autoscaler/vertical-pod-autoscaler/pkg/apis/autoscaling.k8s.io/v1beta2/zz_generated.deepcopy.go b/vertical-pod-autoscaler/e2e/vendor/k8s.io/autoscaler/vertical-pod-autoscaler/pkg/apis/autoscaling.k8s.io/v1beta2/zz_generated.deepcopy.go index c7f00f124267..255d419bd9e1 100644 --- a/vertical-pod-autoscaler/e2e/vendor/k8s.io/autoscaler/vertical-pod-autoscaler/pkg/apis/autoscaling.k8s.io/v1beta2/zz_generated.deepcopy.go +++ b/vertical-pod-autoscaler/e2e/vendor/k8s.io/autoscaler/vertical-pod-autoscaler/pkg/apis/autoscaling.k8s.io/v1beta2/zz_generated.deepcopy.go @@ -256,7 +256,7 @@ func (in *VerticalPodAutoscalerCheckpoint) DeepCopyObject() runtime.Object { func (in *VerticalPodAutoscalerCheckpointList) DeepCopyInto(out *VerticalPodAutoscalerCheckpointList) { *out = *in out.TypeMeta = in.TypeMeta - out.ListMeta = in.ListMeta + in.ListMeta.DeepCopyInto(&out.ListMeta) if in.Items != nil { in, out := &in.Items, &out.Items *out = make([]VerticalPodAutoscalerCheckpoint, len(*in)) @@ -343,7 +343,7 @@ func (in *VerticalPodAutoscalerCondition) DeepCopy() *VerticalPodAutoscalerCondi func (in *VerticalPodAutoscalerList) DeepCopyInto(out *VerticalPodAutoscalerList) { *out = *in out.TypeMeta = in.TypeMeta - out.ListMeta = in.ListMeta + in.ListMeta.DeepCopyInto(&out.ListMeta) if in.Items != nil { in, out := &in.Items, &out.Items *out = make([]VerticalPodAutoscaler, len(*in)) diff --git a/vertical-pod-autoscaler/e2e/vendor/k8s.io/autoscaler/vertical-pod-autoscaler/pkg/apis/poc.autoscaling.k8s.io/v1alpha1/zz_generated.deepcopy.go b/vertical-pod-autoscaler/e2e/vendor/k8s.io/autoscaler/vertical-pod-autoscaler/pkg/apis/poc.autoscaling.k8s.io/v1alpha1/zz_generated.deepcopy.go index 01e3f9181f14..f7c2b1640d7c 100644 --- a/vertical-pod-autoscaler/e2e/vendor/k8s.io/autoscaler/vertical-pod-autoscaler/pkg/apis/poc.autoscaling.k8s.io/v1alpha1/zz_generated.deepcopy.go +++ b/vertical-pod-autoscaler/e2e/vendor/k8s.io/autoscaler/vertical-pod-autoscaler/pkg/apis/poc.autoscaling.k8s.io/v1alpha1/zz_generated.deepcopy.go @@ -249,7 +249,7 @@ func (in *VerticalPodAutoscalerCheckpoint) DeepCopyObject() runtime.Object { func (in *VerticalPodAutoscalerCheckpointList) DeepCopyInto(out *VerticalPodAutoscalerCheckpointList) { *out = *in out.TypeMeta = in.TypeMeta - out.ListMeta = in.ListMeta + in.ListMeta.DeepCopyInto(&out.ListMeta) if in.Items != nil { in, out := &in.Items, &out.Items *out = make([]VerticalPodAutoscalerCheckpoint, len(*in)) @@ -336,7 +336,7 @@ func (in *VerticalPodAutoscalerCondition) DeepCopy() *VerticalPodAutoscalerCondi func (in *VerticalPodAutoscalerList) DeepCopyInto(out *VerticalPodAutoscalerList) { *out = *in out.TypeMeta = in.TypeMeta - out.ListMeta = in.ListMeta + in.ListMeta.DeepCopyInto(&out.ListMeta) if in.Items != nil { in, out := &in.Items, &out.Items *out = make([]VerticalPodAutoscaler, len(*in)) diff --git a/vertical-pod-autoscaler/e2e/vendor/modules.txt b/vertical-pod-autoscaler/e2e/vendor/modules.txt index 25bf0b9f2235..e0120c502ad2 100644 --- a/vertical-pod-autoscaler/e2e/vendor/modules.txt +++ b/vertical-pod-autoscaler/e2e/vendor/modules.txt @@ -925,7 +925,7 @@ k8s.io/apiserver/pkg/util/webhook k8s.io/apiserver/pkg/util/wsstream k8s.io/apiserver/plugin/pkg/authenticator/token/webhook k8s.io/apiserver/plugin/pkg/authorizer/webhook -# k8s.io/autoscaler/vertical-pod-autoscaler v0.0.0-20200207133143-e7988a6dd041 => ../ +# k8s.io/autoscaler/vertical-pod-autoscaler v0.0.0-20200605154545-936eea18fb1d => ../ k8s.io/autoscaler/vertical-pod-autoscaler/pkg/apis/autoscaling.k8s.io/v1 k8s.io/autoscaler/vertical-pod-autoscaler/pkg/apis/autoscaling.k8s.io/v1beta1 k8s.io/autoscaler/vertical-pod-autoscaler/pkg/apis/autoscaling.k8s.io/v1beta2 From 727dab656e4be2615de89f6fb51b168b99e172a2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Johannes=20W=C3=BCrbach?= Date: Sat, 6 Jun 2020 15:28:05 +0200 Subject: [PATCH 5/5] vpa: container controlled values e2e --- .../e2e/v1/admission_controller.go | 38 +++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/vertical-pod-autoscaler/e2e/v1/admission_controller.go b/vertical-pod-autoscaler/e2e/v1/admission_controller.go index 8885aa394e6f..0a21914ecd40 100644 --- a/vertical-pod-autoscaler/e2e/v1/admission_controller.go +++ b/vertical-pod-autoscaler/e2e/v1/admission_controller.go @@ -166,6 +166,44 @@ var _ = AdmissionControllerE2eDescribe("Admission-controller", func() { } }) + ginkgo.It("keeps limits unchanged when container controlled values is requests only", func() { + d := NewHamsterDeploymentWithResourcesAndLimits(f, + ParseQuantityOrDie("100m") /*cpu request*/, ParseQuantityOrDie("100Mi"), /*memory request*/ + ParseQuantityOrDie("500m") /*cpu limit*/, ParseQuantityOrDie("500Mi") /*memory limit*/) + + ginkgo.By("Setting up a VPA CRD") + vpaCRD := NewVPA(f, "hamster-vpa", hamsterTargetRef) + vpaCRD.Status.Recommendation = &vpa_types.RecommendedPodResources{ + ContainerRecommendations: []vpa_types.RecommendedContainerResources{{ + ContainerName: "hamster", + Target: apiv1.ResourceList{ + apiv1.ResourceCPU: ParseQuantityOrDie("250m"), + apiv1.ResourceMemory: ParseQuantityOrDie("200Mi"), + }, + }}, + } + containerControlledValuesRequestsOnly := vpa_types.ContainerControlledValuesRequestsOnly + vpaCRD.Spec.ResourcePolicy = &vpa_types.PodResourcePolicy{ + ContainerPolicies: []vpa_types.ContainerResourcePolicy{{ + ContainerName: "hamster", + ControlledValues: &containerControlledValuesRequestsOnly, + }}, + } + InstallVPA(f, vpaCRD) + + ginkgo.By("Setting up a hamster deployment") + podList := startDeploymentPods(f, d) + + // Originally Pods had 100m CPU, 100Mi of memory, but admission controller + // should change it to 250m CPU and 200Mi of memory. Limits should stay unchanged. + for _, pod := range podList.Items { + gomega.Expect(*pod.Spec.Containers[0].Resources.Requests.Cpu()).To(gomega.Equal(ParseQuantityOrDie("250m"))) + gomega.Expect(*pod.Spec.Containers[0].Resources.Requests.Memory()).To(gomega.Equal(ParseQuantityOrDie("200Mi"))) + gomega.Expect(*pod.Spec.Containers[0].Resources.Limits.Cpu()).To(gomega.Equal(ParseQuantityOrDie("500m"))) + gomega.Expect(*pod.Spec.Containers[0].Resources.Limits.Memory()).To(gomega.Equal(ParseQuantityOrDie("500Mi"))) + } + }) + ginkgo.It("caps request according to container max limit set in LimitRange", func() { startCpuRequest := ParseQuantityOrDie("100m") startCpuLimit := ParseQuantityOrDie("150m")