Skip to content

Commit 37dc76f

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

File tree

13 files changed

+543
-151
lines changed

13 files changed

+543
-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: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ func getContainerPatch(pod *core.Pod, i int, annotationsPerContainer vpa_api_uti
7878
// Add empty resources object if missing.
7979
if pod.Spec.Containers[i].Resources.Limits == nil &&
8080
pod.Spec.Containers[i].Resources.Requests == nil {
81-
patches = append(patches, getPatchInitializingEmptyResources(i))
81+
patches = append(patches, GetPatchInitializingEmptyResources(i))
8282
}
8383

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

108-
func getAddResourceRequirementValuePatch(i int, kind string, resource core.ResourceName, quantity resource.Quantity) resource_admission.PatchRecord {
108+
// GetAddResourceRequirementValuePatch returns a patch record to add resource requirements to a container.
109+
func GetAddResourceRequirementValuePatch(i int, kind string, resource core.ResourceName, quantity resource.Quantity) resource_admission.PatchRecord {
109110
return resource_admission.PatchRecord{
110111
Op: "add",
111112
Path: fmt.Sprintf("/spec/containers/%d/resources/%s/%s", i, kind, resource),
112113
Value: quantity.String()}
113114
}
114115

115-
func getPatchInitializingEmptyResources(i int) resource_admission.PatchRecord {
116+
// GetPatchInitializingEmptyResources returns a patch record to initialize an empty resources object for a container.
117+
func GetPatchInitializingEmptyResources(i int) resource_admission.PatchRecord {
116118
return resource_admission.PatchRecord{
117119
Op: "add",
118120
Path: fmt.Sprintf("/spec/containers/%d/resources", i),
119121
Value: core.ResourceRequirements{},
120122
}
121123
}
122124

123-
func getPatchInitializingEmptyResourcesSubfield(i int, kind string) resource_admission.PatchRecord {
125+
// GetPatchInitializingEmptyResourcesSubfield returns a patch record to initialize an empty subfield
126+
// (e.g., "requests" or "limits") within a container's resources object.
127+
func GetPatchInitializingEmptyResourcesSubfield(i int, kind string) resource_admission.PatchRecord {
124128
return resource_admission.PatchRecord{
125129
Op: "add",
126130
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
appsinformer "k8s.io/client-go/informers/apps/v1"
3233
coreinformer "k8s.io/client-go/informers/core/v1"
3334
"k8s.io/client-go/kubernetes/fake"
@@ -47,6 +48,10 @@ func getBasicVpa() *vpa_types.VerticalPodAutoscaler {
4748
return test.VerticalPodAutoscaler().WithContainer("any").Get()
4849
}
4950

51+
func getNoopPatchCalculators() []patch.Calculator {
52+
return []patch.Calculator{}
53+
}
54+
5055
func TestEvictReplicatedByController(t *testing.T) {
5156
rc := apiv1.ReplicationController{
5257
ObjectMeta: metav1.ObjectMeta{
@@ -73,13 +78,15 @@ func TestEvictReplicatedByController(t *testing.T) {
7378
replicas int32
7479
evictionTolerance float64
7580
vpa *vpa_types.VerticalPodAutoscaler
81+
calculators []patch.Calculator
7682
pods []podWithExpectations
7783
}{
7884
{
7985
name: "Evict only first pod (half of 3).",
8086
replicas: 3,
8187
evictionTolerance: 0.5,
8288
vpa: getBasicVpa(),
89+
calculators: getNoopPatchCalculators(),
8390
pods: []podWithExpectations{
8491
{
8592
pod: generatePod().Get(),
@@ -103,6 +110,7 @@ func TestEvictReplicatedByController(t *testing.T) {
103110
replicas: 4,
104111
evictionTolerance: 0.5,
105112
vpa: getBasicVpa(),
113+
calculators: getNoopPatchCalculators(),
106114
pods: []podWithExpectations{
107115
{
108116

@@ -132,6 +140,7 @@ func TestEvictReplicatedByController(t *testing.T) {
132140
replicas: 4,
133141
evictionTolerance: 0.5,
134142
vpa: getBasicVpa(),
143+
calculators: getNoopPatchCalculators(),
135144
pods: []podWithExpectations{
136145
{
137146
pod: generatePod().Get(),
@@ -155,6 +164,7 @@ func TestEvictReplicatedByController(t *testing.T) {
155164
replicas: 3,
156165
evictionTolerance: 0.1,
157166
vpa: getBasicVpa(),
167+
calculators: getNoopPatchCalculators(),
158168
pods: []podWithExpectations{
159169
{
160170
pod: generatePod().Get(),
@@ -178,6 +188,7 @@ func TestEvictReplicatedByController(t *testing.T) {
178188
replicas: 3,
179189
evictionTolerance: 0.1,
180190
vpa: getBasicVpa(),
191+
calculators: getNoopPatchCalculators(),
181192
pods: []podWithExpectations{
182193
{
183194
pod: generatePod().Get(),
@@ -196,6 +207,7 @@ func TestEvictReplicatedByController(t *testing.T) {
196207
replicas: 3,
197208
evictionTolerance: 0.5,
198209
vpa: getBasicVpa(),
210+
calculators: getNoopPatchCalculators(),
199211
pods: []podWithExpectations{
200212
{
201213
pod: generatePod().Get(),
@@ -219,6 +231,7 @@ func TestEvictReplicatedByController(t *testing.T) {
219231
replicas: 4,
220232
evictionTolerance: 0.5,
221233
vpa: getBasicVpa(),
234+
calculators: getNoopPatchCalculators(),
222235
pods: []podWithExpectations{
223236
{
224237
pod: generatePod().Get(),
@@ -247,6 +260,7 @@ func TestEvictReplicatedByController(t *testing.T) {
247260
replicas: 1,
248261
evictionTolerance: 0.5,
249262
vpa: getBasicVpa(),
263+
calculators: getNoopPatchCalculators(),
250264
pods: []podWithExpectations{
251265
{
252266
pod: generatePod().Get(),
@@ -260,6 +274,7 @@ func TestEvictReplicatedByController(t *testing.T) {
260274
replicas: 1,
261275
evictionTolerance: 0.5,
262276
vpa: vpaSingleReplica,
277+
calculators: getNoopPatchCalculators(),
263278
pods: []podWithExpectations{
264279
{
265280
pod: generatePod().Get(),
@@ -279,7 +294,7 @@ func TestEvictReplicatedByController(t *testing.T) {
279294
pods = append(pods, p.pod)
280295
}
281296
factory, _ := getEvictionRestrictionFactory(&rc, nil, nil, nil, 2, testCase.evictionTolerance)
282-
eviction := factory.NewPodsEvictionRestriction(pods, testCase.vpa)
297+
eviction := factory.NewPodsEvictionRestriction(pods, testCase.vpa, testCase.calculators)
283298
for i, p := range testCase.pods {
284299
assert.Equalf(t, p.canEvict, eviction.CanEvict(p.pod), "TC %v - unexpected CanEvict result for pod-%v %#v", testCase.name, i, p.pod)
285300
}
@@ -318,7 +333,7 @@ func TestEvictReplicatedByReplicaSet(t *testing.T) {
318333

319334
basicVpa := getBasicVpa()
320335
factory, _ := getEvictionRestrictionFactory(nil, &rs, nil, nil, 2, 0.5)
321-
eviction := factory.NewPodsEvictionRestriction(pods, basicVpa)
336+
eviction := factory.NewPodsEvictionRestriction(pods, basicVpa, getNoopPatchCalculators())
322337

323338
for _, pod := range pods {
324339
assert.True(t, eviction.CanEvict(pod))
@@ -358,7 +373,7 @@ func TestEvictReplicatedByStatefulSet(t *testing.T) {
358373

359374
basicVpa := getBasicVpa()
360375
factory, _ := getEvictionRestrictionFactory(nil, nil, &ss, nil, 2, 0.5)
361-
eviction := factory.NewPodsEvictionRestriction(pods, basicVpa)
376+
eviction := factory.NewPodsEvictionRestriction(pods, basicVpa, getNoopPatchCalculators())
362377

363378
for _, pod := range pods {
364379
assert.True(t, eviction.CanEvict(pod))
@@ -397,7 +412,7 @@ func TestEvictReplicatedByDaemonSet(t *testing.T) {
397412

398413
basicVpa := getBasicVpa()
399414
factory, _ := getEvictionRestrictionFactory(nil, nil, nil, &ds, 2, 0.5)
400-
eviction := factory.NewPodsEvictionRestriction(pods, basicVpa)
415+
eviction := factory.NewPodsEvictionRestriction(pods, basicVpa, getNoopPatchCalculators())
401416

402417
for _, pod := range pods {
403418
assert.True(t, eviction.CanEvict(pod))
@@ -433,7 +448,7 @@ func TestEvictReplicatedByJob(t *testing.T) {
433448

434449
basicVpa := getBasicVpa()
435450
factory, _ := getEvictionRestrictionFactory(nil, nil, nil, nil, 2, 0.5)
436-
eviction := factory.NewPodsEvictionRestriction(pods, basicVpa)
451+
eviction := factory.NewPodsEvictionRestriction(pods, basicVpa, getNoopPatchCalculators())
437452

438453
for _, pod := range pods {
439454
assert.True(t, eviction.CanEvict(pod))
@@ -473,7 +488,7 @@ func TestEvictTooFewReplicas(t *testing.T) {
473488

474489
basicVpa := getBasicVpa()
475490
factory, _ := getEvictionRestrictionFactory(&rc, nil, nil, nil, 10, 0.5)
476-
eviction := factory.NewPodsEvictionRestriction(pods, basicVpa)
491+
eviction := factory.NewPodsEvictionRestriction(pods, basicVpa, getNoopPatchCalculators())
477492

478493
for _, pod := range pods {
479494
assert.False(t, eviction.CanEvict(pod))
@@ -510,7 +525,7 @@ func TestEvictionTolerance(t *testing.T) {
510525

511526
basicVpa := getBasicVpa()
512527
factory, _ := getEvictionRestrictionFactory(&rc, nil, nil, nil, 2 /*minReplicas*/, tolerance)
513-
eviction := factory.NewPodsEvictionRestriction(pods, basicVpa)
528+
eviction := factory.NewPodsEvictionRestriction(pods, basicVpa, getNoopPatchCalculators())
514529

515530
for _, pod := range pods {
516531
assert.True(t, eviction.CanEvict(pod))
@@ -551,7 +566,7 @@ func TestEvictAtLeastOne(t *testing.T) {
551566

552567
basicVpa := getBasicVpa()
553568
factory, _ := getEvictionRestrictionFactory(&rc, nil, nil, nil, 2, tolerance)
554-
eviction := factory.NewPodsEvictionRestriction(pods, basicVpa)
569+
eviction := factory.NewPodsEvictionRestriction(pods, basicVpa, getNoopPatchCalculators())
555570

556571
for _, pod := range pods {
557572
assert.True(t, eviction.CanEvict(pod))
@@ -591,6 +606,7 @@ func TestEvictEmitEvent(t *testing.T) {
591606
replicas int32
592607
evictionTolerance float64
593608
vpa *vpa_types.VerticalPodAutoscaler
609+
calculators []patch.Calculator
594610
pods []podWithExpectations
595611
errorExpected bool
596612
}{
@@ -599,6 +615,7 @@ func TestEvictEmitEvent(t *testing.T) {
599615
replicas: 4,
600616
evictionTolerance: 0.5,
601617
vpa: basicVpa,
618+
calculators: getNoopPatchCalculators(),
602619
pods: []podWithExpectations{
603620
{
604621
pod: generatePod().WithPhase(apiv1.PodPending).Get(),
@@ -618,6 +635,7 @@ func TestEvictEmitEvent(t *testing.T) {
618635
replicas: 4,
619636
evictionTolerance: 0.5,
620637
vpa: basicVpa,
638+
calculators: getNoopPatchCalculators(),
621639
pods: []podWithExpectations{
622640

623641
{
@@ -639,7 +657,7 @@ func TestEvictEmitEvent(t *testing.T) {
639657
pods = append(pods, p.pod)
640658
}
641659
factory, _ := getEvictionRestrictionFactory(&rc, nil, nil, nil, 2, testCase.evictionTolerance)
642-
eviction := factory.NewPodsEvictionRestriction(pods, testCase.vpa)
660+
eviction := factory.NewPodsEvictionRestriction(pods, testCase.vpa, testCase.calculators)
643661

644662
for _, p := range testCase.pods {
645663
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)