From 2d7daf74fc9da7b0fc3014a234c9d56218623103 Mon Sep 17 00:00:00 2001 From: "dom.bozzuto" Date: Thu, 22 Feb 2024 17:23:45 -0500 Subject: [PATCH] Hook in allowed resource processor to admission-controller and updater --- .../pkg/admission-controller/main.go | 11 +- .../recommendation_provider_test.go | 190 +++++++++++------- vertical-pod-autoscaler/pkg/updater/main.go | 12 +- .../pkg/utils/vpa/recommendation_processor.go | 8 + 4 files changed, 146 insertions(+), 75 deletions(-) diff --git a/vertical-pod-autoscaler/pkg/admission-controller/main.go b/vertical-pod-autoscaler/pkg/admission-controller/main.go index d8904643601d..cc359c59a7ab 100644 --- a/vertical-pod-autoscaler/pkg/admission-controller/main.go +++ b/vertical-pod-autoscaler/pkg/admission-controller/main.go @@ -21,6 +21,7 @@ import ( "fmt" "net/http" "os" + "strings" "time" apiv1 "k8s.io/api/core/v1" @@ -69,6 +70,7 @@ var ( registerWebhook = flag.Bool("register-webhook", true, "If set to true, admission webhook object will be created on start up to register with the API server.") registerByURL = flag.Bool("register-by-url", false, "If set to true, admission webhook will be registered by URL (webhookAddress:webhookPort) instead of by service name") vpaObjectNamespace = flag.String("vpa-object-namespace", apiv1.NamespaceAll, "Namespace to search for VPA objects. Empty means all namespaces will be used.") + allowedResources = flag.String("allowed-resources", strings.Join([]string{string(apiv1.ResourceCPU), string(apiv1.ResourceMemory)}, ","), "Comma-separated list of resources that can be applied from a VPA.") ) func main() { @@ -96,7 +98,14 @@ func main() { klog.Errorf("Failed to create limitRangeCalculator, falling back to not checking limits. Error message: %s", err) limitRangeCalculator = limitrange.NewNoopLimitsCalculator() } - recommendationProvider := recommendation.NewProvider(limitRangeCalculator, vpa_api_util.NewCappingRecommendationProcessor(limitRangeCalculator)) + + var allowedResourcesNames []apiv1.ResourceName + for _, resourceName := range strings.Split(*allowedResources, ",") { + if resourceName != "" { + allowedResourcesNames = append(allowedResourcesNames, apiv1.ResourceName(resourceName)) + } + } + recommendationProvider := recommendation.NewProvider(limitRangeCalculator, vpa_api_util.NewDefaultRecommendationProcessor(limitRangeCalculator, allowedResourcesNames)) vpaMatcher := vpa.NewMatcher(vpaLister, targetSelectorFetcher) hostname, err := os.Hostname() 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 f58e98f1a88e..d77598fc2986 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 @@ -116,10 +116,11 @@ func TestUpdateResourceRequests(t *testing.T) { name string pod *apiv1.Pod vpa *vpa_types.VerticalPodAutoscaler + allowedResources []apiv1.ResourceName expectedAction bool expectedError error - expectedMem resource.Quantity - expectedCPU resource.Quantity + expectedMem *resource.Quantity + expectedCPU *resource.Quantity expectedCPULimit *resource.Quantity expectedMemLimit *resource.Quantity limitRange *apiv1.LimitRangeItem @@ -127,74 +128,82 @@ func TestUpdateResourceRequests(t *testing.T) { annotations vpa_api_util.ContainerToAnnotationsMap }{ { - name: "uninitialized pod", - pod: uninitialized, - vpa: vpa, - expectedAction: true, - expectedMem: resource.MustParse("200Mi"), - expectedCPU: resource.MustParse("2"), + name: "uninitialized pod", + pod: uninitialized, + vpa: vpa, + allowedResources: []apiv1.ResourceName{apiv1.ResourceCPU, apiv1.ResourceMemory}, + expectedAction: true, + expectedMem: mustParseResourcePointer("200Mi"), + expectedCPU: mustParseResourcePointer("2"), }, { - name: "target below min", - pod: uninitialized, - vpa: targetBelowMinVPA, - expectedAction: true, - expectedMem: resource.MustParse("300Mi"), // MinMemory is expected to be used - expectedCPU: resource.MustParse("4"), // MinCpu is expected to be used + name: "target below min", + pod: uninitialized, + vpa: targetBelowMinVPA, + allowedResources: []apiv1.ResourceName{apiv1.ResourceCPU, apiv1.ResourceMemory}, + expectedAction: true, + expectedMem: mustParseResourcePointer("300Mi"), // MinMemory is expected to be used + expectedCPU: mustParseResourcePointer("4"), // MinCpu is expected to be used annotations: vpa_api_util.ContainerToAnnotationsMap{ containerName: []string{"cpu capped to minAllowed", "memory capped to minAllowed"}, }, }, { - name: "target above max", - pod: uninitialized, - vpa: targetAboveMaxVPA, - expectedAction: true, - expectedMem: resource.MustParse("1Gi"), // MaxMemory is expected to be used - expectedCPU: resource.MustParse("5"), // MaxCpu is expected to be used + name: "target above max", + pod: uninitialized, + vpa: targetAboveMaxVPA, + allowedResources: []apiv1.ResourceName{apiv1.ResourceCPU, apiv1.ResourceMemory}, + expectedAction: true, + expectedMem: mustParseResourcePointer("1Gi"), // MaxMemory is expected to be used + expectedCPU: mustParseResourcePointer("5"), // MaxCpu is expected to be used annotations: vpa_api_util.ContainerToAnnotationsMap{ containerName: []string{"cpu capped to maxAllowed", "memory capped to maxAllowed"}, }, }, { - name: "initialized pod", - pod: initialized, - vpa: vpa, - expectedAction: true, - expectedMem: resource.MustParse("200Mi"), - expectedCPU: resource.MustParse("2"), + name: "initialized pod", + pod: initialized, + vpa: vpa, + allowedResources: []apiv1.ResourceName{apiv1.ResourceCPU, apiv1.ResourceMemory}, + expectedAction: true, + expectedMem: mustParseResourcePointer("200Mi"), + expectedCPU: mustParseResourcePointer("2"), }, { - name: "high memory", - pod: initialized, - vpa: vpaWithHighMemory, - expectedAction: true, - expectedMem: resource.MustParse("1000Mi"), - expectedCPU: resource.MustParse("2"), + name: "high memory", + pod: initialized, + vpa: vpaWithHighMemory, + allowedResources: []apiv1.ResourceName{apiv1.ResourceCPU, apiv1.ResourceMemory}, + expectedAction: true, + expectedMem: mustParseResourcePointer("1000Mi"), + expectedCPU: mustParseResourcePointer("2"), }, { - name: "empty recommendation", - pod: initialized, - vpa: vpaWithEmptyRecommendation, - expectedAction: true, - expectedMem: resource.MustParse("0"), - expectedCPU: resource.MustParse("0"), + name: "empty recommendation", + pod: initialized, + vpa: vpaWithEmptyRecommendation, + allowedResources: []apiv1.ResourceName{apiv1.ResourceCPU, apiv1.ResourceMemory}, + expectedAction: true, + expectedMem: nil, + expectedCPU: nil, }, { - name: "nil recommendation", - pod: initialized, - vpa: vpaWithNilRecommendation, - expectedAction: true, - expectedMem: resource.MustParse("0"), - expectedCPU: resource.MustParse("0"), + name: "nil recommendation", + pod: initialized, + vpa: vpaWithNilRecommendation, + allowedResources: []apiv1.ResourceName{apiv1.ResourceCPU, apiv1.ResourceMemory}, + expectedAction: true, + expectedMem: nil, + expectedCPU: nil, }, { name: "guaranteed resources", pod: limitsMatchRequestsPod, vpa: vpa, + allowedResources: []apiv1.ResourceName{apiv1.ResourceCPU, apiv1.ResourceMemory}, expectedAction: true, - expectedMem: resource.MustParse("200Mi"), - expectedCPU: resource.MustParse("2"), + expectedMem: mustParseResourcePointer("200Mi"), + expectedCPU: mustParseResourcePointer("2"), expectedCPULimit: mustParseResourcePointer("2"), expectedMemLimit: mustParseResourcePointer("200Mi"), }, @@ -202,9 +211,10 @@ func TestUpdateResourceRequests(t *testing.T) { name: "guaranteed resources - no request", pod: limitsNoRequestsPod, vpa: vpa, + allowedResources: []apiv1.ResourceName{apiv1.ResourceCPU, apiv1.ResourceMemory}, expectedAction: true, - expectedMem: resource.MustParse("200Mi"), - expectedCPU: resource.MustParse("2"), + expectedMem: mustParseResourcePointer("200Mi"), + expectedCPU: mustParseResourcePointer("2"), expectedCPULimit: mustParseResourcePointer("2"), expectedMemLimit: mustParseResourcePointer("200Mi"), }, @@ -212,9 +222,10 @@ func TestUpdateResourceRequests(t *testing.T) { name: "proportional limit - as default", pod: podWithDoubleLimit, vpa: vpa, + allowedResources: []apiv1.ResourceName{apiv1.ResourceCPU, apiv1.ResourceMemory}, expectedAction: true, - expectedCPU: resource.MustParse("2"), - expectedMem: resource.MustParse("200Mi"), + expectedCPU: mustParseResourcePointer("2"), + expectedMem: mustParseResourcePointer("200Mi"), expectedCPULimit: mustParseResourcePointer("4"), expectedMemLimit: mustParseResourcePointer("400Mi"), }, @@ -222,27 +233,30 @@ func TestUpdateResourceRequests(t *testing.T) { name: "proportional limit - set explicit", pod: podWithDoubleLimit, vpa: resourceRequestsAndLimitsVPA, + allowedResources: []apiv1.ResourceName{apiv1.ResourceCPU, apiv1.ResourceMemory}, expectedAction: true, - expectedCPU: resource.MustParse("2"), - expectedMem: resource.MustParse("200Mi"), + expectedCPU: mustParseResourcePointer("2"), + expectedMem: mustParseResourcePointer("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", + pod: podWithDoubleLimit, + vpa: resourceRequestsOnlyVPA, + allowedResources: []apiv1.ResourceName{apiv1.ResourceCPU, apiv1.ResourceMemory}, + expectedAction: true, + expectedCPU: mustParseResourcePointer("2"), + expectedMem: mustParseResourcePointer("200Mi"), }, { - name: "disabled limit scaling - requests capped at limit", - pod: podWithDoubleLimit, - vpa: resourceRequestsOnlyVPAHighTarget, - expectedAction: true, - expectedCPU: resource.MustParse("2"), - expectedMem: resource.MustParse("200Mi"), + name: "disabled limit scaling - requests capped at limit", + pod: podWithDoubleLimit, + vpa: resourceRequestsOnlyVPAHighTarget, + allowedResources: []apiv1.ResourceName{apiv1.ResourceCPU, apiv1.ResourceMemory}, + expectedAction: true, + expectedCPU: mustParseResourcePointer("2"), + expectedMem: mustParseResourcePointer("200Mi"), annotations: vpa_api_util.ContainerToAnnotationsMap{ containerName: []string{ "cpu capped to container limit", @@ -254,9 +268,10 @@ func TestUpdateResourceRequests(t *testing.T) { name: "limit over int64", pod: podWithTenfoldLimit, vpa: vpaWithExabyteRecommendation, + allowedResources: []apiv1.ResourceName{apiv1.ResourceCPU, apiv1.ResourceMemory}, expectedAction: true, - expectedCPU: resource.MustParse("1Ei"), - expectedMem: resource.MustParse("1Ei"), + expectedCPU: mustParseResourcePointer("1Ei"), + expectedMem: mustParseResourcePointer("1Ei"), expectedCPULimit: resource.NewMilliQuantity(math.MaxInt64, resource.DecimalExponent), expectedMemLimit: resource.NewQuantity(math.MaxInt64, resource.DecimalExponent), annotations: vpa_api_util.ContainerToAnnotationsMap{ @@ -270,6 +285,7 @@ func TestUpdateResourceRequests(t *testing.T) { name: "limit range calculation error", pod: initialized, vpa: vpa, + allowedResources: []apiv1.ResourceName{apiv1.ResourceCPU, apiv1.ResourceMemory}, limitRangeCalcErr: fmt.Errorf("oh no"), expectedAction: false, expectedError: fmt.Errorf("error getting containerLimitRange: oh no"), @@ -279,8 +295,8 @@ func TestUpdateResourceRequests(t *testing.T) { pod: initialized, vpa: vpa, expectedAction: true, - expectedCPU: resource.MustParse("2"), - expectedMem: resource.MustParse("200Mi"), + expectedCPU: mustParseResourcePointer("2"), + expectedMem: mustParseResourcePointer("200Mi"), expectedCPULimit: mustParseResourcePointer("2"), expectedMemLimit: mustParseResourcePointer("200Mi"), limitRange: &apiv1.LimitRangeItem{ @@ -291,12 +307,28 @@ func TestUpdateResourceRequests(t *testing.T) { }, }, }, + { + name: "memory recommendation filtered out", + pod: initialized, + vpa: vpa, + allowedResources: []apiv1.ResourceName{apiv1.ResourceCPU}, + expectedAction: true, + expectedCPU: mustParseResourcePointer("2"), + }, + { + name: "cpu recommendation filtered out", + pod: initialized, + vpa: vpa, + allowedResources: []apiv1.ResourceName{apiv1.ResourceMemory}, + expectedAction: true, + expectedMem: mustParseResourcePointer("200Mi"), + }, } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { recommendationProvider := &recommendationProvider{ - recommendationProcessor: vpa_api_util.NewCappingRecommendationProcessor(limitrange.NewNoopLimitsCalculator()), + recommendationProcessor: vpa_api_util.NewDefaultRecommendationProcessor(limitrange.NewNoopLimitsCalculator(), tc.allowedResources), limitsRangeCalculator: &fakeLimitRangeCalculator{ containerLimitRange: tc.limitRange, containerErr: tc.limitRangeCalcErr, @@ -314,11 +346,23 @@ func TestUpdateResourceRequests(t *testing.T) { _, foundEmpty := resources[0].Requests[""] assert.Equal(t, foundEmpty, false, "empty resourceKey have not been purged") - cpuRequest := resources[0].Requests[apiv1.ResourceCPU] - assert.Equal(t, tc.expectedCPU.Value(), cpuRequest.Value(), "cpu request doesn't match") + cpuRequest, cpuRequestPresent := resources[0].Requests[apiv1.ResourceCPU] + if tc.expectedCPU == nil { + assert.False(t, cpuRequestPresent, "expected no cpu request, got %s", cpuRequest.String()) + } else { + if assert.True(t, cpuRequestPresent, "expected cpu request, but it's missing") { + assert.Equal(t, tc.expectedCPU.Value(), cpuRequest.Value(), "cpu request doesn't match") + } + } - memoryRequest := resources[0].Requests[apiv1.ResourceMemory] - assert.Equal(t, tc.expectedMem.Value(), memoryRequest.Value(), "memory request doesn't match") + memoryRequest, memoryRequestPresent := resources[0].Requests[apiv1.ResourceMemory] + if tc.expectedMem == nil { + assert.False(t, memoryRequestPresent, "expected no mem request, got %s", memoryRequest.String()) + } else { + if assert.True(t, memoryRequestPresent, "expected memory request, but it's missing") { + assert.Equal(t, tc.expectedMem.Value(), memoryRequest.Value(), "memory request doesn't match") + } + } cpuLimit, cpuLimitPresent := resources[0].Limits[apiv1.ResourceCPU] if tc.expectedCPULimit == nil { diff --git a/vertical-pod-autoscaler/pkg/updater/main.go b/vertical-pod-autoscaler/pkg/updater/main.go index 1df3d0e0a84e..3877ca5dd98a 100644 --- a/vertical-pod-autoscaler/pkg/updater/main.go +++ b/vertical-pod-autoscaler/pkg/updater/main.go @@ -20,6 +20,7 @@ import ( "context" "flag" "os" + "strings" "time" apiv1 "k8s.io/api/core/v1" @@ -69,6 +70,7 @@ var ( vpaObjectNamespace = flag.String("vpa-object-namespace", apiv1.NamespaceAll, "Namespace to search for VPA objects. Empty means all namespaces will be used.") podLabelSelector = flag.String("pod-label-selector", "", "Label selector for pods that are eligible for the Updater") + allowedResources = flag.String("allowed-resources", strings.Join([]string{string(apiv1.ResourceCPU), string(apiv1.ResourceMemory)}, ","), "Comma-separated list of resources that can be applied from a VPA.") ) const defaultResyncPeriod time.Duration = 10 * time.Minute @@ -98,6 +100,14 @@ func main() { if namespace != "" { admissionControllerStatusNamespace = namespace } + + var allowedResourcesNames []apiv1.ResourceName + for _, resourceName := range strings.Split(*allowedResources, ",") { + if resourceName != "" { + allowedResourcesNames = append(allowedResourcesNames, apiv1.ResourceName(resourceName)) + } + } + // TODO: use SharedInformerFactory in updater updater, err := updater.NewUpdater( kubeClient, @@ -108,7 +118,7 @@ func main() { *evictionToleranceFraction, *useAdmissionControllerStatus, admissionControllerStatusNamespace, - vpa_api_util.NewCappingRecommendationProcessor(limitRangeCalculator), + vpa_api_util.NewDefaultRecommendationProcessor(limitRangeCalculator, allowedResourcesNames), nil, targetSelectorFetcher, priority.NewProcessor(), diff --git a/vertical-pod-autoscaler/pkg/utils/vpa/recommendation_processor.go b/vertical-pod-autoscaler/pkg/utils/vpa/recommendation_processor.go index 6e4b08f69007..a976f8ea8fe8 100644 --- a/vertical-pod-autoscaler/pkg/utils/vpa/recommendation_processor.go +++ b/vertical-pod-autoscaler/pkg/utils/vpa/recommendation_processor.go @@ -19,6 +19,7 @@ package api import ( "k8s.io/api/core/v1" vpa_types "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/apis/autoscaling.k8s.io/v1" + "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/utils/limitrange" ) // ContainerToAnnotationsMap contains annotations per container. @@ -34,3 +35,10 @@ type RecommendationProcessor interface { conditions []vpa_types.VerticalPodAutoscalerCondition, pod *v1.Pod) (*vpa_types.RecommendedPodResources, ContainerToAnnotationsMap, error) } + +func NewDefaultRecommendationProcessor(limitsRangeCalculator limitrange.LimitRangeCalculator, allowedResources []v1.ResourceName) RecommendationProcessor { + return NewSequentialProcessor([]RecommendationProcessor{ + NewCappingRecommendationProcessor(limitsRangeCalculator), + NewAllowedResourceFilteringProcessor(allowedResources), + }) +}