Skip to content

Commit 71ee8b5

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

File tree

11 files changed

+372
-120
lines changed

11 files changed

+372
-120
lines changed

vertical-pod-autoscaler/pkg/admission-controller/config.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -157,8 +157,7 @@ func selfRegistration(clientset kubernetes.Interface, caCert []byte, webHookDela
157157
AdmissionReviewVersions: []string{"v1"},
158158
Rules: []admissionregistration.RuleWithOperations{
159159
{
160-
// we're watching for updates now also because of in-place VPA, but we only patch if the update was triggered by the updater
161-
Operations: []admissionregistration.OperationType{admissionregistration.Create, admissionregistration.Update},
160+
Operations: []admissionregistration.OperationType{admissionregistration.Create},
162161
Rule: admissionregistration.Rule{
163162
APIGroups: []string{""},
164163
APIVersions: []string{"v1"},

vertical-pod-autoscaler/pkg/admission-controller/resource/pod/handler.go

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -78,15 +78,6 @@ func (h *resourceHandler) GetPatches(ctx context.Context, ar *admissionv1.Admiss
7878
pod.Namespace = namespace
7979
}
8080

81-
// We're watching for updates now also because of in-place VPA, but we only want to act on the ones
82-
// that were triggered by the updater so we don't violate disruption quotas
83-
if ar.Operation == admissionv1.Update {
84-
// The patcher will remove this annotation, it's just the signal that the updater okayed this
85-
if _, ok := pod.Annotations["autoscaling.k8s.io/resize"]; !ok {
86-
return nil, nil
87-
}
88-
}
89-
9081
klog.V(4).InfoS("Admitting pod", "pod", klog.KObj(&pod))
9182
controllingVpa := h.vpaMatcher.GetMatchingVPA(ctx, &pod)
9283
if controllingVpa == nil {
@@ -102,12 +93,6 @@ func (h *resourceHandler) GetPatches(ctx context.Context, ar *admissionv1.Admiss
10293
if pod.Annotations == nil {
10394
patches = append(patches, patch.GetAddEmptyAnnotationsPatch())
10495
}
105-
// TODO(jkyros): this is weird here, work it out with the above block somehow, but
106-
// we need to have this annotation patched out
107-
if ar.Operation == admissionv1.Update {
108-
// TODO(jkyros): the squiggle is escaping the / in the annotation
109-
patches = append(patches, patch.GetRemoveAnnotationPatch("autoscaling.k8s.io~1resize"))
110-
}
11196

11297
for _, c := range h.patchCalculators {
11398
partialPatches, err := c.CalculatePatches(&pod, controllingVpa)

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

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -39,11 +39,3 @@ func GetAddAnnotationPatch(annotationName, annotationValue string) resource_admi
3939
Value: annotationValue,
4040
}
4141
}
42-
43-
// GetRemoveAnnotationPatch returns a patch that removes the specified annotation.
44-
func GetRemoveAnnotationPatch(annotationName string) resource_admission.PatchRecord {
45-
return resource_admission.PatchRecord{
46-
Op: "remove",
47-
Path: fmt.Sprintf("/metadata/annotations/%s", annotationName),
48-
}
49-
}

vertical-pod-autoscaler/pkg/recommender/input/cluster_feeder.go

Lines changed: 0 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -443,33 +443,6 @@ func (feeder *clusterStateFeeder) LoadPods() {
443443
}
444444
}
445445

446-
// PruneContainers prunes any containers from the intial aggregate states
447-
// that are no longer present in the aggregate states. This is important
448-
// for cases where a container has been renamed or removed, as otherwise
449-
// the VPA will split the recommended resources across containers that
450-
// are no longer present, resulting in the containers that are still
451-
// present being under-resourced
452-
func (feeder *clusterStateFeeder) PruneContainers2() {
453-
454-
var keysPruned int
455-
// Look through all of our VPAs
456-
for _, vpa := range feeder.clusterState.Vpas {
457-
// TODO(jkyros): maybe vpa.PruneInitialAggregateContainerStates() ?
458-
aggregates := vpa.AggregateStateByContainerNameWithoutCheckpoints()
459-
// Check each initial state to see if it's still "real"
460-
for container := range vpa.ContainersInitialAggregateState {
461-
if _, ok := aggregates[container]; !ok {
462-
delete(vpa.ContainersInitialAggregateState, container)
463-
keysPruned = keysPruned + 1
464-
465-
}
466-
}
467-
}
468-
if keysPruned > 0 {
469-
klog.Infof("Pruned %d stale initial aggregate container keys", keysPruned)
470-
}
471-
}
472-
473446
// PruneContainers removes any containers from the aggregates and initial aggregates that are no longer
474447
// present in the feeder's clusterState. Without this, we would be averaging our resource calculations
475448
// over containers that no longer exist, and continuing to update checkpoints that should be left to expire.
Lines changed: 137 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,137 @@
1+
/*
2+
Copyright 2024 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 eviction
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+
// NewProvider constructs the recommendation provider that can be used to determine recommendations for pods.
37+
func NewInPlaceProvider(calculator limitrange.LimitRangeCalculator,
38+
recommendationProcessor vpa_api_util.RecommendationProcessor) recommendation.Provider {
39+
return &inPlaceRecommendationProvider{
40+
limitsRangeCalculator: calculator,
41+
recommendationProcessor: recommendationProcessor,
42+
}
43+
}
44+
45+
// TODO(maxcao13): Strip down function to remove unnecessary stuff, or refactor so that it can be used in the admission controller as well
46+
// GetContainersResources returns the recommended resources for each container in the given pod in the same order they are specified in the pod.Spec.
47+
// If addAll is set to true, containers w/o a recommendation are also added to the list (and their non-recommended requests and limits will always be preserved if present),
48+
// otherwise they're skipped (default behaviour).
49+
func GetContainersResources(pod *core.Pod, vpaResourcePolicy *vpa_types.PodResourcePolicy, podRecommendation vpa_types.RecommendedPodResources, limitRange *core.LimitRangeItem,
50+
addAll bool) []vpa_api_util.ContainerResources {
51+
resources := make([]vpa_api_util.ContainerResources, len(pod.Spec.Containers))
52+
for i, container := range pod.Spec.Containers {
53+
recommendation := vpa_api_util.GetRecommendationForContainer(container.Name, &podRecommendation)
54+
if recommendation == nil {
55+
if !addAll {
56+
klog.V(2).InfoS("No recommendation found for container, skipping", "container", container.Name)
57+
continue
58+
}
59+
klog.V(2).InfoS("No match found for container, using Pod request", "container", container.Name)
60+
resources[i].Requests = container.Resources.Requests
61+
} else {
62+
resources[i].Requests = recommendation.Target
63+
}
64+
defaultLimit := core.ResourceList{}
65+
if limitRange != nil {
66+
defaultLimit = limitRange.Default
67+
}
68+
containerControlledValues := vpa_api_util.GetContainerControlledValues(container.Name, vpaResourcePolicy)
69+
if containerControlledValues == vpa_types.ContainerControlledValuesRequestsAndLimits {
70+
proportionalLimits, _ := vpa_api_util.GetProportionalLimit(container.Resources.Limits, container.Resources.Requests, resources[i].Requests, defaultLimit)
71+
if proportionalLimits != nil {
72+
resources[i].Limits = proportionalLimits
73+
}
74+
}
75+
// If the recommendation only contains CPU or Memory (if the VPA was configured this way), we need to make sure we "backfill" the other.
76+
// Only do this when the addAll flag is true.
77+
if addAll {
78+
cpuRequest, hasCpuRequest := container.Resources.Requests[core.ResourceCPU]
79+
if _, ok := resources[i].Requests[core.ResourceCPU]; !ok && hasCpuRequest {
80+
resources[i].Requests[core.ResourceCPU] = cpuRequest
81+
}
82+
memRequest, hasMemRequest := container.Resources.Requests[core.ResourceMemory]
83+
if _, ok := resources[i].Requests[core.ResourceMemory]; !ok && hasMemRequest {
84+
resources[i].Requests[core.ResourceMemory] = memRequest
85+
}
86+
cpuLimit, hasCpuLimit := container.Resources.Limits[core.ResourceCPU]
87+
if _, ok := resources[i].Limits[core.ResourceCPU]; !ok && hasCpuLimit {
88+
resources[i].Limits[core.ResourceCPU] = cpuLimit
89+
}
90+
memLimit, hasMemLimit := container.Resources.Limits[core.ResourceMemory]
91+
if _, ok := resources[i].Limits[core.ResourceMemory]; !ok && hasMemLimit {
92+
resources[i].Limits[core.ResourceMemory] = memLimit
93+
}
94+
}
95+
}
96+
return resources
97+
}
98+
99+
// GetContainersResourcesForPod returns recommended request for a given pod and associated annotations.
100+
// The returned slice corresponds 1-1 to containers in the Pod.
101+
func (p *inPlaceRecommendationProvider) GetContainersResourcesForPod(pod *core.Pod, vpa *vpa_types.VerticalPodAutoscaler) ([]vpa_api_util.ContainerResources, vpa_api_util.ContainerToAnnotationsMap, error) {
102+
if vpa == nil || pod == nil {
103+
klog.V(2).InfoS("Can't calculate recommendations, one of VPA or Pod is nil", "vpa", vpa, "pod", pod)
104+
return nil, nil, nil
105+
}
106+
klog.V(2).InfoS("Updating requirements for pod", "pod", pod.Name)
107+
108+
recommendedPodResources := &vpa_types.RecommendedPodResources{}
109+
110+
if vpa.Status.Recommendation != nil {
111+
var err error
112+
// ignore annotations as they are not used in the in-place update
113+
recommendedPodResources, _, err = p.recommendationProcessor.Apply(vpa, pod)
114+
if err != nil {
115+
klog.V(2).InfoS("Cannot process recommendation for pod", "pod", klog.KObj(pod))
116+
return nil, nil, err
117+
}
118+
}
119+
containerLimitRange, err := p.limitsRangeCalculator.GetContainerLimitRangeItem(pod.Namespace)
120+
if err != nil {
121+
return nil, nil, fmt.Errorf("error getting containerLimitRange: %s", err)
122+
}
123+
var resourcePolicy *vpa_types.PodResourcePolicy
124+
if vpa.Spec.UpdatePolicy == nil || vpa.Spec.UpdatePolicy.UpdateMode == nil || *vpa.Spec.UpdatePolicy.UpdateMode != vpa_types.UpdateModeOff {
125+
resourcePolicy = vpa.Spec.ResourcePolicy
126+
}
127+
containerResources := GetContainersResources(pod, resourcePolicy, *recommendedPodResources, containerLimitRange, false)
128+
129+
// Ensure that we are not propagating empty resource key if any.
130+
for _, resource := range containerResources {
131+
if resource.RemoveEmptyResourceKeyIfAny() {
132+
klog.InfoS("An empty resource key was found and purged", "pod", klog.KObj(pod), "vpa", klog.KObj(vpa))
133+
}
134+
}
135+
136+
return containerResources, nil, nil
137+
}
Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
/*
2+
Copyright 2024 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 eviction
18+
19+
import (
20+
core "k8s.io/api/core/v1"
21+
resource_admission "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/admission-controller/resource"
22+
"k8s.io/autoscaler/vertical-pod-autoscaler/pkg/admission-controller/resource/pod/patch"
23+
24+
vpa_types "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/apis/autoscaling.k8s.io/v1"
25+
"k8s.io/autoscaler/vertical-pod-autoscaler/pkg/utils/annotations"
26+
)
27+
28+
type observedContainers struct{}
29+
30+
func (*observedContainers) CalculatePatches(pod *core.Pod, _ *vpa_types.VerticalPodAutoscaler) ([]resource_admission.PatchRecord, error) {
31+
vpaObservedContainersValue := annotations.GetVpaObservedContainersValue(pod)
32+
return []resource_admission.PatchRecord{patch.GetAddAnnotationPatch(annotations.VpaObservedContainersLabel, vpaObservedContainersValue)}, nil
33+
}
34+
35+
// NewLastInPlaceUpdateCalculator returns calculator for
36+
// observed containers patches.
37+
func NewLastInPlaceUpdateCalculator() patch.Calculator {
38+
return &observedContainers{}
39+
}

0 commit comments

Comments
 (0)