Skip to content

Commit a7cc025

Browse files
committed
VPA: Allow VPA updater to actuate recommendations in-place
Signed-off-by: Max Cao <[email protected]>
1 parent d345286 commit a7cc025

File tree

13 files changed

+534
-151
lines changed

13 files changed

+534
-151
lines changed

vertical-pod-autoscaler/deploy/vpa-rbac.yaml

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,32 @@ rules:
121121
- create
122122
---
123123
apiVersion: rbac.authorization.k8s.io/v1
124+
kind: ClusterRole
125+
metadata:
126+
name: system:vpa-updater-in-place
127+
rules:
128+
- apiGroups:
129+
- ""
130+
resources:
131+
- pods/resize
132+
- pods
133+
verbs:
134+
- patch
135+
---
136+
apiVersion: rbac.authorization.k8s.io/v1
137+
kind: ClusterRoleBinding
138+
metadata:
139+
name: system:vpa-updater-in-place-binding
140+
roleRef:
141+
apiGroup: rbac.authorization.k8s.io
142+
kind: ClusterRole
143+
name: system:vpa-updater-in-place
144+
subjects:
145+
- kind: ServiceAccount
146+
name: vpa-updater
147+
namespace: kube-system
148+
---
149+
apiVersion: rbac.authorization.k8s.io/v1
124150
kind: ClusterRoleBinding
125151
metadata:
126152
name: system:metrics-reader

vertical-pod-autoscaler/pkg/admission-controller/resource/pod/patch/resource_updates.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ func getContainerPatch(pod *core.Pod, i int, annotationsPerContainer vpa_api_uti
7777
// Add empty resources object if missing.
7878
if pod.Spec.Containers[i].Resources.Limits == nil &&
7979
pod.Spec.Containers[i].Resources.Requests == nil {
80-
patches = append(patches, getPatchInitializingEmptyResources(i))
80+
patches = append(patches, GetPatchInitializingEmptyResources(i))
8181
}
8282

8383
annotations, found := annotationsPerContainer[pod.Spec.Containers[i].Name]
@@ -95,31 +95,31 @@ func getContainerPatch(pod *core.Pod, i int, annotationsPerContainer vpa_api_uti
9595
func appendPatchesAndAnnotations(patches []resource_admission.PatchRecord, annotations []string, current core.ResourceList, containerIndex int, resources core.ResourceList, fieldName, resourceName string) ([]resource_admission.PatchRecord, []string) {
9696
// Add empty object if it's missing and we're about to fill it.
9797
if current == nil && len(resources) > 0 {
98-
patches = append(patches, getPatchInitializingEmptyResourcesSubfield(containerIndex, fieldName))
98+
patches = append(patches, GetPatchInitializingEmptyResourcesSubfield(containerIndex, fieldName))
9999
}
100100
for resource, request := range resources {
101-
patches = append(patches, getAddResourceRequirementValuePatch(containerIndex, fieldName, resource, request))
101+
patches = append(patches, GetAddResourceRequirementValuePatch(containerIndex, fieldName, resource, request))
102102
annotations = append(annotations, fmt.Sprintf("%s %s", resource, resourceName))
103103
}
104104
return patches, annotations
105105
}
106106

107-
func getAddResourceRequirementValuePatch(i int, kind string, resource core.ResourceName, quantity resource.Quantity) resource_admission.PatchRecord {
107+
func GetAddResourceRequirementValuePatch(i int, kind string, resource core.ResourceName, quantity resource.Quantity) resource_admission.PatchRecord {
108108
return resource_admission.PatchRecord{
109109
Op: "add",
110110
Path: fmt.Sprintf("/spec/containers/%d/resources/%s/%s", i, kind, resource),
111111
Value: quantity.String()}
112112
}
113113

114-
func getPatchInitializingEmptyResources(i int) resource_admission.PatchRecord {
114+
func GetPatchInitializingEmptyResources(i int) resource_admission.PatchRecord {
115115
return resource_admission.PatchRecord{
116116
Op: "add",
117117
Path: fmt.Sprintf("/spec/containers/%d/resources", i),
118118
Value: core.ResourceRequirements{},
119119
}
120120
}
121121

122-
func getPatchInitializingEmptyResourcesSubfield(i int, kind string) resource_admission.PatchRecord {
122+
func GetPatchInitializingEmptyResourcesSubfield(i int, kind string) resource_admission.PatchRecord {
123123
return resource_admission.PatchRecord{
124124
Op: "add",
125125
Path: fmt.Sprintf("/spec/containers/%d/resources/%s", i, kind),

vertical-pod-autoscaler/pkg/updater/eviction/pods_eviction_restriction.go

Lines changed: 120 additions & 67 deletions
Large diffs are not rendered by default.

vertical-pod-autoscaler/pkg/updater/eviction/pods_eviction_restriction_test.go

Lines changed: 27 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ import (
2828
batchv1 "k8s.io/api/batch/v1"
2929
apiv1 "k8s.io/api/core/v1"
3030
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
31+
"k8s.io/autoscaler/vertical-pod-autoscaler/pkg/admission-controller/resource/pod/patch"
3132
vpa_types "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/apis/autoscaling.k8s.io/v1"
3233
"k8s.io/autoscaler/vertical-pod-autoscaler/pkg/utils/test"
3334
appsinformer "k8s.io/client-go/informers/apps/v1"
@@ -46,6 +47,10 @@ func getBasicVpa() *vpa_types.VerticalPodAutoscaler {
4647
return test.VerticalPodAutoscaler().WithContainer("any").Get()
4748
}
4849

50+
func getNoopPatchCalculators() []patch.Calculator {
51+
return []patch.Calculator{}
52+
}
53+
4954
func TestEvictReplicatedByController(t *testing.T) {
5055
rc := apiv1.ReplicationController{
5156
ObjectMeta: metav1.ObjectMeta{
@@ -72,13 +77,15 @@ func TestEvictReplicatedByController(t *testing.T) {
7277
replicas int32
7378
evictionTolerance float64
7479
vpa *vpa_types.VerticalPodAutoscaler
80+
calculators []patch.Calculator
7581
pods []podWithExpectations
7682
}{
7783
{
7884
name: "Evict only first pod (half of 3).",
7985
replicas: 3,
8086
evictionTolerance: 0.5,
8187
vpa: getBasicVpa(),
88+
calculators: getNoopPatchCalculators(),
8289
pods: []podWithExpectations{
8390
{
8491
pod: generatePod().Get(),
@@ -102,6 +109,7 @@ func TestEvictReplicatedByController(t *testing.T) {
102109
replicas: 4,
103110
evictionTolerance: 0.5,
104111
vpa: getBasicVpa(),
112+
calculators: getNoopPatchCalculators(),
105113
pods: []podWithExpectations{
106114
{
107115

@@ -131,6 +139,7 @@ func TestEvictReplicatedByController(t *testing.T) {
131139
replicas: 4,
132140
evictionTolerance: 0.5,
133141
vpa: getBasicVpa(),
142+
calculators: getNoopPatchCalculators(),
134143
pods: []podWithExpectations{
135144
{
136145
pod: generatePod().Get(),
@@ -154,6 +163,7 @@ func TestEvictReplicatedByController(t *testing.T) {
154163
replicas: 3,
155164
evictionTolerance: 0.1,
156165
vpa: getBasicVpa(),
166+
calculators: getNoopPatchCalculators(),
157167
pods: []podWithExpectations{
158168
{
159169
pod: generatePod().Get(),
@@ -177,6 +187,7 @@ func TestEvictReplicatedByController(t *testing.T) {
177187
replicas: 3,
178188
evictionTolerance: 0.1,
179189
vpa: getBasicVpa(),
190+
calculators: getNoopPatchCalculators(),
180191
pods: []podWithExpectations{
181192
{
182193
pod: generatePod().Get(),
@@ -195,6 +206,7 @@ func TestEvictReplicatedByController(t *testing.T) {
195206
replicas: 3,
196207
evictionTolerance: 0.5,
197208
vpa: getBasicVpa(),
209+
calculators: getNoopPatchCalculators(),
198210
pods: []podWithExpectations{
199211
{
200212
pod: generatePod().Get(),
@@ -218,6 +230,7 @@ func TestEvictReplicatedByController(t *testing.T) {
218230
replicas: 4,
219231
evictionTolerance: 0.5,
220232
vpa: getBasicVpa(),
233+
calculators: getNoopPatchCalculators(),
221234
pods: []podWithExpectations{
222235
{
223236
pod: generatePod().Get(),
@@ -246,6 +259,7 @@ func TestEvictReplicatedByController(t *testing.T) {
246259
replicas: 1,
247260
evictionTolerance: 0.5,
248261
vpa: getBasicVpa(),
262+
calculators: getNoopPatchCalculators(),
249263
pods: []podWithExpectations{
250264
{
251265
pod: generatePod().Get(),
@@ -259,6 +273,7 @@ func TestEvictReplicatedByController(t *testing.T) {
259273
replicas: 1,
260274
evictionTolerance: 0.5,
261275
vpa: vpaSingleReplica,
276+
calculators: getNoopPatchCalculators(),
262277
pods: []podWithExpectations{
263278
{
264279
pod: generatePod().Get(),
@@ -278,7 +293,7 @@ func TestEvictReplicatedByController(t *testing.T) {
278293
pods = append(pods, p.pod)
279294
}
280295
factory, _ := getEvictionRestrictionFactory(&rc, nil, nil, nil, 2, testCase.evictionTolerance)
281-
eviction := factory.NewPodsEvictionRestriction(pods, testCase.vpa)
296+
eviction := factory.NewPodsEvictionRestriction(pods, testCase.vpa, testCase.calculators)
282297
for i, p := range testCase.pods {
283298
assert.Equalf(t, p.canEvict, eviction.CanEvict(p.pod), "TC %v - unexpected CanEvict result for pod-%v %#v", testCase.name, i, p.pod)
284299
}
@@ -317,7 +332,7 @@ func TestEvictReplicatedByReplicaSet(t *testing.T) {
317332

318333
basicVpa := getBasicVpa()
319334
factory, _ := getEvictionRestrictionFactory(nil, &rs, nil, nil, 2, 0.5)
320-
eviction := factory.NewPodsEvictionRestriction(pods, basicVpa)
335+
eviction := factory.NewPodsEvictionRestriction(pods, basicVpa, getNoopPatchCalculators())
321336

322337
for _, pod := range pods {
323338
assert.True(t, eviction.CanEvict(pod))
@@ -357,7 +372,7 @@ func TestEvictReplicatedByStatefulSet(t *testing.T) {
357372

358373
basicVpa := getBasicVpa()
359374
factory, _ := getEvictionRestrictionFactory(nil, nil, &ss, nil, 2, 0.5)
360-
eviction := factory.NewPodsEvictionRestriction(pods, basicVpa)
375+
eviction := factory.NewPodsEvictionRestriction(pods, basicVpa, getNoopPatchCalculators())
361376

362377
for _, pod := range pods {
363378
assert.True(t, eviction.CanEvict(pod))
@@ -396,7 +411,7 @@ func TestEvictReplicatedByDaemonSet(t *testing.T) {
396411

397412
basicVpa := getBasicVpa()
398413
factory, _ := getEvictionRestrictionFactory(nil, nil, nil, &ds, 2, 0.5)
399-
eviction := factory.NewPodsEvictionRestriction(pods, basicVpa)
414+
eviction := factory.NewPodsEvictionRestriction(pods, basicVpa, getNoopPatchCalculators())
400415

401416
for _, pod := range pods {
402417
assert.True(t, eviction.CanEvict(pod))
@@ -432,7 +447,7 @@ func TestEvictReplicatedByJob(t *testing.T) {
432447

433448
basicVpa := getBasicVpa()
434449
factory, _ := getEvictionRestrictionFactory(nil, nil, nil, nil, 2, 0.5)
435-
eviction := factory.NewPodsEvictionRestriction(pods, basicVpa)
450+
eviction := factory.NewPodsEvictionRestriction(pods, basicVpa, getNoopPatchCalculators())
436451

437452
for _, pod := range pods {
438453
assert.True(t, eviction.CanEvict(pod))
@@ -472,7 +487,7 @@ func TestEvictTooFewReplicas(t *testing.T) {
472487

473488
basicVpa := getBasicVpa()
474489
factory, _ := getEvictionRestrictionFactory(&rc, nil, nil, nil, 10, 0.5)
475-
eviction := factory.NewPodsEvictionRestriction(pods, basicVpa)
490+
eviction := factory.NewPodsEvictionRestriction(pods, basicVpa, getNoopPatchCalculators())
476491

477492
for _, pod := range pods {
478493
assert.False(t, eviction.CanEvict(pod))
@@ -509,7 +524,7 @@ func TestEvictionTolerance(t *testing.T) {
509524

510525
basicVpa := getBasicVpa()
511526
factory, _ := getEvictionRestrictionFactory(&rc, nil, nil, nil, 2 /*minReplicas*/, tolerance)
512-
eviction := factory.NewPodsEvictionRestriction(pods, basicVpa)
527+
eviction := factory.NewPodsEvictionRestriction(pods, basicVpa, getNoopPatchCalculators())
513528

514529
for _, pod := range pods {
515530
assert.True(t, eviction.CanEvict(pod))
@@ -550,7 +565,7 @@ func TestEvictAtLeastOne(t *testing.T) {
550565

551566
basicVpa := getBasicVpa()
552567
factory, _ := getEvictionRestrictionFactory(&rc, nil, nil, nil, 2, tolerance)
553-
eviction := factory.NewPodsEvictionRestriction(pods, basicVpa)
568+
eviction := factory.NewPodsEvictionRestriction(pods, basicVpa, getNoopPatchCalculators())
554569

555570
for _, pod := range pods {
556571
assert.True(t, eviction.CanEvict(pod))
@@ -590,6 +605,7 @@ func TestEvictEmitEvent(t *testing.T) {
590605
replicas int32
591606
evictionTolerance float64
592607
vpa *vpa_types.VerticalPodAutoscaler
608+
calculators []patch.Calculator
593609
pods []podWithExpectations
594610
errorExpected bool
595611
}{
@@ -598,6 +614,7 @@ func TestEvictEmitEvent(t *testing.T) {
598614
replicas: 4,
599615
evictionTolerance: 0.5,
600616
vpa: basicVpa,
617+
calculators: getNoopPatchCalculators(),
601618
pods: []podWithExpectations{
602619
{
603620
pod: generatePod().WithPhase(apiv1.PodPending).Get(),
@@ -617,6 +634,7 @@ func TestEvictEmitEvent(t *testing.T) {
617634
replicas: 4,
618635
evictionTolerance: 0.5,
619636
vpa: basicVpa,
637+
calculators: getNoopPatchCalculators(),
620638
pods: []podWithExpectations{
621639

622640
{
@@ -638,7 +656,7 @@ func TestEvictEmitEvent(t *testing.T) {
638656
pods = append(pods, p.pod)
639657
}
640658
factory, _ := getEvictionRestrictionFactory(&rc, nil, nil, nil, 2, testCase.evictionTolerance)
641-
eviction := factory.NewPodsEvictionRestriction(pods, testCase.vpa)
659+
eviction := factory.NewPodsEvictionRestriction(pods, testCase.vpa, testCase.calculators)
642660

643661
for _, p := range testCase.pods {
644662
mockRecorder := test.MockEventRecorder()
Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
/*
2+
Copyright 2025 The Kubernetes Authors.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package inplace
18+
19+
import (
20+
"fmt"
21+
22+
core "k8s.io/api/core/v1"
23+
"k8s.io/autoscaler/vertical-pod-autoscaler/pkg/admission-controller/resource/pod/recommendation"
24+
"k8s.io/klog/v2"
25+
26+
vpa_types "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/apis/autoscaling.k8s.io/v1"
27+
"k8s.io/autoscaler/vertical-pod-autoscaler/pkg/utils/limitrange"
28+
vpa_api_util "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/utils/vpa"
29+
)
30+
31+
type inPlaceRecommendationProvider struct {
32+
limitsRangeCalculator limitrange.LimitRangeCalculator
33+
recommendationProcessor vpa_api_util.RecommendationProcessor
34+
}
35+
36+
// NewInPlaceRecommendationProvider constructs the recommendation provider that can be used to determine recommendations for pods.
37+
func NewInPlaceRecommendationProvider(calculator limitrange.LimitRangeCalculator,
38+
recommendationProcessor vpa_api_util.RecommendationProcessor) recommendation.Provider {
39+
return &inPlaceRecommendationProvider{
40+
limitsRangeCalculator: calculator,
41+
recommendationProcessor: recommendationProcessor,
42+
}
43+
}
44+
45+
// GetContainersResourcesForPod returns recommended request for a given pod.
46+
// The returned slice corresponds 1-1 to containers in the Pod.
47+
func (p *inPlaceRecommendationProvider) GetContainersResourcesForPod(pod *core.Pod, vpa *vpa_types.VerticalPodAutoscaler) ([]vpa_api_util.ContainerResources, vpa_api_util.ContainerToAnnotationsMap, error) {
48+
if vpa == nil || pod == nil {
49+
klog.V(2).InfoS("Can't calculate recommendations, one of VPA or Pod is nil", "vpa", vpa, "pod", pod)
50+
return nil, nil, nil
51+
}
52+
klog.V(2).InfoS("Updating requirements for pod", "pod", pod.Name)
53+
54+
recommendedPodResources := &vpa_types.RecommendedPodResources{}
55+
56+
if vpa.Status.Recommendation != nil {
57+
var err error
58+
// ignore annotations as they are cannot be used when patching resize subresource
59+
recommendedPodResources, _, err = p.recommendationProcessor.Apply(vpa, pod)
60+
if err != nil {
61+
klog.V(2).InfoS("Cannot process recommendation for pod", "pod", klog.KObj(pod))
62+
return nil, nil, err
63+
}
64+
}
65+
containerLimitRange, err := p.limitsRangeCalculator.GetContainerLimitRangeItem(pod.Namespace)
66+
if err != nil {
67+
return nil, nil, fmt.Errorf("error getting containerLimitRange: %s", err)
68+
}
69+
var resourcePolicy *vpa_types.PodResourcePolicy
70+
if vpa.Spec.UpdatePolicy == nil || vpa.Spec.UpdatePolicy.UpdateMode == nil || *vpa.Spec.UpdatePolicy.UpdateMode != vpa_types.UpdateModeOff {
71+
resourcePolicy = vpa.Spec.ResourcePolicy
72+
}
73+
containerResources := recommendation.GetContainersResources(pod, resourcePolicy, *recommendedPodResources, containerLimitRange, false, nil)
74+
75+
// Ensure that we are not propagating empty resource key if any.
76+
for _, resource := range containerResources {
77+
if resource.RemoveEmptyResourceKeyIfAny() {
78+
klog.InfoS("An empty resource key was found and purged", "pod", klog.KObj(pod), "vpa", klog.KObj(vpa))
79+
}
80+
}
81+
82+
return containerResources, nil, nil
83+
}

0 commit comments

Comments
 (0)