Skip to content

Commit

Permalink
Hook in allowed resource processor to admission-controller and updater
Browse files Browse the repository at this point in the history
  • Loading branch information
@domenicbozzuto
domenicbozzuto committed Feb 22, 2024
1 parent 19e678a commit 2d7daf7
Show file tree
Hide file tree
Showing 4 changed files with 146 additions and 75 deletions.
11 changes: 10 additions & 1 deletion vertical-pod-autoscaler/pkg/admission-controller/main.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"fmt"
"net/http"
"os"
"strings"
"time"

apiv1 "k8s.io/api/core/v1"
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -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()
Expand Down
190 changes: 117 additions & 73 deletions ...aler/pkg/admission-controller/resource/pod/recommendation/recommendation_provider_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -116,133 +116,147 @@ 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
limitRangeCalcErr error
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"),
},
{
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"),
},
{
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"),
},
{
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",
Expand All @@ -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{
Expand All @@ -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"),
Expand All @@ -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{
Expand All @@ -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,
Expand All @@ -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 {
Expand Down
12 changes: 11 additions & 1 deletion vertical-pod-autoscaler/pkg/updater/main.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"context"
"flag"
"os"
"strings"
"time"

apiv1 "k8s.io/api/core/v1"
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand All @@ -108,7 +118,7 @@ func main() {
*evictionToleranceFraction,
*useAdmissionControllerStatus,
admissionControllerStatusNamespace,
vpa_api_util.NewCappingRecommendationProcessor(limitRangeCalculator),
vpa_api_util.NewDefaultRecommendationProcessor(limitRangeCalculator, allowedResourcesNames),
nil,
targetSelectorFetcher,
priority.NewProcessor(),
Expand Down
8 changes: 8 additions & 0 deletions vertical-pod-autoscaler/pkg/utils/vpa/recommendation_processor.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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 {

Check failure on line 39 in vertical-pod-autoscaler/pkg/utils/vpa/recommendation_processor.go

View workflow job for this annotation

GitHub Actions / test-and-verify 

exported function NewDefaultRecommendationProcessor should have comment or be unexported
return NewSequentialProcessor([]RecommendationProcessor{
NewCappingRecommendationProcessor(limitsRangeCalculator),
NewAllowedResourceFilteringProcessor(allowedResources),
})
}

0 comments on commit 2d7daf7

Please sign in to comment.