Skip to content

Commit

Permalink
Add a command line option to limit which resources can be applied
Browse files Browse the repository at this point in the history
  • Loading branch information
@domenicbozzuto
domenicbozzuto committed Feb 22, 2024
1 parent 25a8a96 commit c3213da
Show file tree
Hide file tree
Showing 3 changed files with 148 additions and 64 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.NewCappingRecommendationProcessor(limitRangeCalculator), allowedResourcesNames)
vpaMatcher := vpa.NewMatcher(vpaLister, targetSelectorFetcher)

hostname, err := os.Hostname()
Expand Down
41 changes: 36 additions & 5 deletions ...utoscaler/pkg/admission-controller/resource/pod/recommendation/recommendation_provider.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ package recommendation

import (
"fmt"

core "k8s.io/api/core/v1"
"k8s.io/klog/v2"

Expand All @@ -35,24 +34,27 @@ type Provider interface {
type recommendationProvider struct {
limitsRangeCalculator limitrange.LimitRangeCalculator
recommendationProcessor vpa_api_util.RecommendationProcessor
allowedResources []core.ResourceName
}

// NewProvider constructs the recommendation provider that can be used to determine recommendations for pods.
func NewProvider(calculator limitrange.LimitRangeCalculator,
recommendationProcessor vpa_api_util.RecommendationProcessor) Provider {
recommendationProcessor vpa_api_util.RecommendationProcessor,
allowedResources []core.ResourceName) Provider {
return &recommendationProvider{
limitsRangeCalculator: calculator,
recommendationProcessor: recommendationProcessor,
allowedResources: allowedResources,
}
}

// GetContainersResources returns the recommended resources for each container in the given pod in the same order they are specified in the pod.Spec.
// If addAll is set to true, containers w/o a recommendation are also added to the list, otherwise they're skipped (default behaviour).
func GetContainersResources(pod *core.Pod, vpaResourcePolicy *vpa_types.PodResourcePolicy, podRecommendation vpa_types.RecommendedPodResources, limitRange *core.LimitRangeItem,
addAll bool, annotations vpa_api_util.ContainerToAnnotationsMap) []vpa_api_util.ContainerResources {
addAll bool, annotations vpa_api_util.ContainerToAnnotationsMap, allowedResources []core.ResourceName) []vpa_api_util.ContainerResources {
resources := make([]vpa_api_util.ContainerResources, len(pod.Spec.Containers))
for i, container := range pod.Spec.Containers {
recommendation := vpa_api_util.GetRecommendationForContainer(container.Name, &podRecommendation)
recommendation := filterAllowedContainerResources(vpa_api_util.GetRecommendationForContainer(container.Name, &podRecommendation), allowedResources)
if recommendation == nil {
if !addAll {
klog.V(2).Infof("no matching recommendation found for container %s, skipping", container.Name)
Expand Down Expand Up @@ -109,7 +111,7 @@ func (p *recommendationProvider) GetContainersResourcesForPod(pod *core.Pod, vpa
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, false, annotations)
containerResources := GetContainersResources(pod, resourcePolicy, *recommendedPodResources, containerLimitRange, false, annotations, p.allowedResources)

// Ensure that we are not propagating empty resource key if any.
for _, resource := range containerResources {
Expand All @@ -120,3 +122,32 @@ func (p *recommendationProvider) GetContainersResourcesForPod(pod *core.Pod, vpa

return containerResources, annotations, nil
}

func filterAllowedContainerResources(recommendation *vpa_types.RecommendedContainerResources, allowedResources []core.ResourceName) *vpa_types.RecommendedContainerResources {
if recommendation != nil {
recommendation.Target = filterResourceList(recommendation.Target, allowedResources)
recommendation.LowerBound = filterResourceList(recommendation.LowerBound, allowedResources)
recommendation.UpperBound = filterResourceList(recommendation.UpperBound, allowedResources)
recommendation.UncappedTarget = filterResourceList(recommendation.UncappedTarget, allowedResources)
}
return recommendation
}

func filterResourceList(resourceList core.ResourceList, allowedResources []core.ResourceName) core.ResourceList {
filteredResourceList := core.ResourceList{}
for resourceName, resourceValue := range resourceList {
if containsResource(allowedResources, resourceName) {
filteredResourceList[resourceName] = resourceValue
}
}
return filteredResourceList
}

func containsResource(resourceNames []core.ResourceName, target core.ResourceName) bool {
for _, resourceName := range resourceNames {
if target == resourceName {
return true
}
}
return false
}
160 changes: 102 additions & 58 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,6 +116,7 @@ func TestUpdateResourceRequests(t *testing.T) {
name string
pod *apiv1.Pod
vpa *vpa_types.VerticalPodAutoscaler
allowedResources []apiv1.ResourceName
expectedAction bool
expectedError error
expectedMem resource.Quantity
Expand All @@ -127,71 +128,79 @@ 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: resource.MustParse("200Mi"),
expectedCPU: resource.MustParse("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: resource.MustParse("300Mi"), // MinMemory is expected to be used
expectedCPU: resource.MustParse("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: resource.MustParse("1Gi"), // MaxMemory is expected to be used
expectedCPU: resource.MustParse("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: resource.MustParse("200Mi"),
expectedCPU: resource.MustParse("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: resource.MustParse("1000Mi"),
expectedCPU: resource.MustParse("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: resource.MustParse("0"),
expectedCPU: resource.MustParse("0"),
},
{
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: resource.MustParse("0"),
expectedCPU: resource.MustParse("0"),
},
{
name: "guaranteed resources",
pod: limitsMatchRequestsPod,
vpa: vpa,
allowedResources: []apiv1.ResourceName{apiv1.ResourceCPU, apiv1.ResourceMemory},
expectedAction: true,
expectedMem: resource.MustParse("200Mi"),
expectedCPU: resource.MustParse("2"),
Expand All @@ -202,6 +211,7 @@ 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"),
Expand All @@ -212,6 +222,7 @@ 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"),
Expand All @@ -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"),
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: 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"),
name: "disabled limit scaling - requests capped at limit",
pod: podWithDoubleLimit,
vpa: resourceRequestsOnlyVPAHighTarget,
allowedResources: []apiv1.ResourceName{apiv1.ResourceCPU, apiv1.ResourceMemory},
expectedAction: true,
expectedCPU: resource.MustParse("2"),
expectedMem: resource.MustParse("200Mi"),
annotations: vpa_api_util.ContainerToAnnotationsMap{
containerName: []string{
"cpu capped to container limit",
Expand All @@ -254,6 +268,7 @@ 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"),
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 @@ -278,6 +294,7 @@ func TestUpdateResourceRequests(t *testing.T) {
name: "proportional limit from default",
pod: initialized,
vpa: vpa,
allowedResources: []apiv1.ResourceName{apiv1.ResourceCPU, apiv1.ResourceMemory},
expectedAction: true,
expectedCPU: resource.MustParse("2"),
expectedMem: resource.MustParse("200Mi"),
Expand All @@ -291,6 +308,22 @@ func TestUpdateResourceRequests(t *testing.T) {
},
},
},
{
name: "memory recommendation filtered out",
pod: initialized,
vpa: vpa,
allowedResources: []apiv1.ResourceName{apiv1.ResourceCPU},
expectedAction: true,
expectedCPU: resource.MustParse("2"),
},
{
name: "cpu recommendation filtered out",
pod: initialized,
vpa: vpa,
allowedResources: []apiv1.ResourceName{apiv1.ResourceMemory},
expectedAction: true,
expectedMem: resource.MustParse("200Mi"),
},
}

for _, tc := range testCases {
Expand All @@ -301,6 +334,7 @@ func TestUpdateResourceRequests(t *testing.T) {
containerLimitRange: tc.limitRange,
containerErr: tc.limitRangeCalcErr,
},
allowedResources: tc.allowedResources,
}

resources, annotations, err := recommendationProvider.GetContainersResourcesForPod(tc.pod, tc.vpa)
Expand All @@ -314,11 +348,21 @@ 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")
if containsResource(tc.allowedResources, apiv1.ResourceCPU) {
cpuRequest := resources[0].Requests[apiv1.ResourceCPU]
assert.Equal(t, tc.expectedCPU.Value(), cpuRequest.Value(), "cpu request doesn't match")
} else {
_, ok := resources[0].Requests[apiv1.ResourceCPU]
assert.False(t, ok)
}

memoryRequest := resources[0].Requests[apiv1.ResourceMemory]
assert.Equal(t, tc.expectedMem.Value(), memoryRequest.Value(), "memory request doesn't match")
if containsResource(tc.allowedResources, apiv1.ResourceMemory) {
memoryRequest := resources[0].Requests[apiv1.ResourceMemory]
assert.Equal(t, tc.expectedMem.Value(), memoryRequest.Value(), "memory request doesn't match")
} else {
_, ok := resources[0].Requests[apiv1.ResourceMemory]
assert.False(t, ok)
}

cpuLimit, cpuLimitPresent := resources[0].Limits[apiv1.ResourceCPU]
if tc.expectedCPULimit == nil {
Expand Down

0 comments on commit c3213da

Please sign in to comment.