From dde06f5591b0d7daa7d5946d07f0a9f75fd8e528 Mon Sep 17 00:00:00 2001 From: nikimanoledaki Date: Wed, 17 Apr 2024 14:04:35 +0200 Subject: [PATCH 1/7] Add more precise logging for VPA resource recommendations --- .../pkg/updater/priority/update_priority_calculator.go | 2 +- vertical-pod-autoscaler/pkg/utils/vpa/capping.go | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/vertical-pod-autoscaler/pkg/updater/priority/update_priority_calculator.go b/vertical-pod-autoscaler/pkg/updater/priority/update_priority_calculator.go index ed72e710f5fd..d4337ac27ac0 100644 --- a/vertical-pod-autoscaler/pkg/updater/priority/update_priority_calculator.go +++ b/vertical-pod-autoscaler/pkg/updater/priority/update_priority_calculator.go @@ -140,7 +140,7 @@ func (calc *UpdatePriorityCalculator) AddPod(pod *apiv1.Pod, now time.Time) { klog.V(4).Infof("not updating pod %v/%v because resource would not change", pod.Namespace, pod.Name) return } - klog.V(2).Infof("pod accepted for update %v/%v with priority %v", pod.Namespace, pod.Name, updatePriority.ResourceDiff) + klog.V(2).Infof("pod accepted for update %v/%v with priority %v", pod.Namespace, pod.Name, updatePriority.ResourceDiff, processedRecommendation) calc.pods = append(calc.pods, prioritizedPod{ pod: pod, priority: updatePriority, diff --git a/vertical-pod-autoscaler/pkg/utils/vpa/capping.go b/vertical-pod-autoscaler/pkg/utils/vpa/capping.go index ad2578b175a6..858ca904e40f 100644 --- a/vertical-pod-autoscaler/pkg/utils/vpa/capping.go +++ b/vertical-pod-autoscaler/pkg/utils/vpa/capping.go @@ -130,6 +130,7 @@ func getCappedRecommendationForContainer( cappingAnnotations = append(cappingAnnotations, annotations...) } } + klog.V(3).Infof("processed capped recommendations for %s: %v", container.Name, cappingAnnotations) } process(cappedRecommendations.Target, true) From 33a22200cb20fcc349c292e47a466fd585517bea Mon Sep 17 00:00:00 2001 From: nikimanoledaki Date: Wed, 15 May 2024 12:55:15 +0200 Subject: [PATCH 2/7] Add func with targets of processed recommendations --- .../updater/priority/update_priority_calculator.go | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/vertical-pod-autoscaler/pkg/updater/priority/update_priority_calculator.go b/vertical-pod-autoscaler/pkg/updater/priority/update_priority_calculator.go index d4337ac27ac0..e4cf95a58247 100644 --- a/vertical-pod-autoscaler/pkg/updater/priority/update_priority_calculator.go +++ b/vertical-pod-autoscaler/pkg/updater/priority/update_priority_calculator.go @@ -18,7 +18,9 @@ package priority import ( "flag" + "fmt" "sort" + "strings" "time" apiv1 "k8s.io/api/core/v1" @@ -140,7 +142,7 @@ func (calc *UpdatePriorityCalculator) AddPod(pod *apiv1.Pod, now time.Time) { klog.V(4).Infof("not updating pod %v/%v because resource would not change", pod.Namespace, pod.Name) return } - klog.V(2).Infof("pod accepted for update %v/%v with priority %v", pod.Namespace, pod.Name, updatePriority.ResourceDiff, processedRecommendation) + klog.V(2).Infof("pod accepted for update %v/%v with priority %v - processed recommendations:\n%v", pod.Namespace, pod.Name, updatePriority.ResourceDiff, getProcessedRecommendationTargets(processedRecommendation)) calc.pods = append(calc.pods, prioritizedPod{ pod: pod, priority: updatePriority, @@ -177,6 +179,14 @@ func parseVpaObservedContainers(pod *apiv1.Pod) (bool, sets.String) { return hasObservedContainers, vpaContainerSet } +func getProcessedRecommendationTargets(r *vpa_types.RecommendedPodResources) string { + sb := &strings.Builder{} + for _, cr := range r.ContainerRecommendations { + sb.WriteString(fmt.Sprintf("%s: [ target: %sK, %vm; uncappedTarget: %sK, %vm ]\n", cr.ContainerName, cr.Target.Memory().AsDec(), cr.Target.Cpu().MilliValue(), cr.UncappedTarget.Memory().AsDec(), cr.UncappedTarget.Cpu().MilliValue())) + } + return sb.String() +} + type prioritizedPod struct { pod *apiv1.Pod priority PodPriority From 1277ff600b7440d39e7b53024dbef6f479202999 Mon Sep 17 00:00:00 2001 From: nikimanoledaki Date: Wed, 15 May 2024 14:41:08 +0200 Subject: [PATCH 3/7] Remove capped recommendation log --- vertical-pod-autoscaler/pkg/utils/vpa/capping.go | 1 - 1 file changed, 1 deletion(-) diff --git a/vertical-pod-autoscaler/pkg/utils/vpa/capping.go b/vertical-pod-autoscaler/pkg/utils/vpa/capping.go index 858ca904e40f..ad2578b175a6 100644 --- a/vertical-pod-autoscaler/pkg/utils/vpa/capping.go +++ b/vertical-pod-autoscaler/pkg/utils/vpa/capping.go @@ -130,7 +130,6 @@ func getCappedRecommendationForContainer( cappingAnnotations = append(cappingAnnotations, annotations...) } } - klog.V(3).Infof("processed capped recommendations for %s: %v", container.Name, cappingAnnotations) } process(cappedRecommendations.Target, true) From 093618ed58ddcb68c68f2a273c0f13348284a38f Mon Sep 17 00:00:00 2001 From: nikimanoledaki Date: Wed, 15 May 2024 15:02:17 +0200 Subject: [PATCH 4/7] Handle nil values for VPA targets log --- .../pkg/updater/priority/update_priority_calculator.go | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/vertical-pod-autoscaler/pkg/updater/priority/update_priority_calculator.go b/vertical-pod-autoscaler/pkg/updater/priority/update_priority_calculator.go index e4cf95a58247..a7433f8da274 100644 --- a/vertical-pod-autoscaler/pkg/updater/priority/update_priority_calculator.go +++ b/vertical-pod-autoscaler/pkg/updater/priority/update_priority_calculator.go @@ -182,7 +182,14 @@ func parseVpaObservedContainers(pod *apiv1.Pod) (bool, sets.String) { func getProcessedRecommendationTargets(r *vpa_types.RecommendedPodResources) string { sb := &strings.Builder{} for _, cr := range r.ContainerRecommendations { - sb.WriteString(fmt.Sprintf("%s: [ target: %sK, %vm; uncappedTarget: %sK, %vm ]\n", cr.ContainerName, cr.Target.Memory().AsDec(), cr.Target.Cpu().MilliValue(), cr.UncappedTarget.Memory().AsDec(), cr.UncappedTarget.Cpu().MilliValue())) + sb.WriteString(fmt.Sprintf("%s:", cr.ContainerName)) + if cr.Target != nil { + sb.WriteString(fmt.Sprintf("target: %sK, %vm;", cr.Target.Memory().AsDec(), cr.Target.Cpu().MilliValue())) + } + if cr.UncappedTarget != nil { + sb.WriteString(fmt.Sprintf("uncappedTarget: %sK, %vm;", cr.UncappedTarget.Memory().AsDec(), cr.UncappedTarget.Cpu().MilliValue())) + } + sb.WriteString("\n") } return sb.String() } From 1800a6e8fa8959ea713e72e9c778c5fe00b54b93 Mon Sep 17 00:00:00 2001 From: nikimanoledaki Date: Mon, 27 May 2024 15:59:59 +0200 Subject: [PATCH 5/7] autoscaler: only log non-zero values for processed recommendations --- .../priority/update_priority_calculator.go | 44 ++++++++++++------- .../update_priority_calculator_test.go | 36 +++++++++++++++ 2 files changed, 64 insertions(+), 16 deletions(-) diff --git a/vertical-pod-autoscaler/pkg/updater/priority/update_priority_calculator.go b/vertical-pod-autoscaler/pkg/updater/priority/update_priority_calculator.go index a7433f8da274..cf0b398e834b 100644 --- a/vertical-pod-autoscaler/pkg/updater/priority/update_priority_calculator.go +++ b/vertical-pod-autoscaler/pkg/updater/priority/update_priority_calculator.go @@ -142,7 +142,7 @@ func (calc *UpdatePriorityCalculator) AddPod(pod *apiv1.Pod, now time.Time) { klog.V(4).Infof("not updating pod %v/%v because resource would not change", pod.Namespace, pod.Name) return } - klog.V(2).Infof("pod accepted for update %v/%v with priority %v - processed recommendations:\n%v", pod.Namespace, pod.Name, updatePriority.ResourceDiff, getProcessedRecommendationTargets(processedRecommendation)) + klog.V(2).Infof("pod accepted for update %v/%v with priority %v - processed recommendations:\n%v", pod.Namespace, pod.Name, updatePriority.ResourceDiff, calc.GetProcessedRecommendationTargets(processedRecommendation)) calc.pods = append(calc.pods, prioritizedPod{ pod: pod, priority: updatePriority, @@ -165,6 +165,33 @@ func (calc *UpdatePriorityCalculator) GetSortedPods(admission PodEvictionAdmissi return result } +func (calc *UpdatePriorityCalculator) GetProcessedRecommendationTargets(r *vpa_types.RecommendedPodResources) string { + sb := &strings.Builder{} + for _, cr := range r.ContainerRecommendations { + sb.WriteString(fmt.Sprintf("%s: ", cr.ContainerName)) + if cr.Target != nil { + sb.WriteString("target: ") + if !cr.Target.Memory().IsZero() { + sb.WriteString(fmt.Sprintf("%sK ", cr.Target.Memory().AsDec())) + } + if !cr.Target.Cpu().IsZero() { + sb.WriteString(fmt.Sprintf("%vm; ", cr.Target.Cpu().MilliValue())) + } + } + if cr.UncappedTarget != nil { + sb.WriteString("uncappedTarget: ") + if !cr.UncappedTarget.Memory().IsZero() { + sb.WriteString(fmt.Sprintf("%sK ", cr.UncappedTarget.Memory().AsDec())) + } + if !cr.UncappedTarget.Cpu().IsZero() { + sb.WriteString(fmt.Sprintf("%vm;", cr.UncappedTarget.Cpu().MilliValue())) + } + } + sb.WriteString("\n") + } + return sb.String() +} + func parseVpaObservedContainers(pod *apiv1.Pod) (bool, sets.String) { observedContainers, hasObservedContainers := pod.GetAnnotations()[annotations.VpaObservedContainersLabel] vpaContainerSet := sets.NewString() @@ -179,21 +206,6 @@ func parseVpaObservedContainers(pod *apiv1.Pod) (bool, sets.String) { return hasObservedContainers, vpaContainerSet } -func getProcessedRecommendationTargets(r *vpa_types.RecommendedPodResources) string { - sb := &strings.Builder{} - for _, cr := range r.ContainerRecommendations { - sb.WriteString(fmt.Sprintf("%s:", cr.ContainerName)) - if cr.Target != nil { - sb.WriteString(fmt.Sprintf("target: %sK, %vm;", cr.Target.Memory().AsDec(), cr.Target.Cpu().MilliValue())) - } - if cr.UncappedTarget != nil { - sb.WriteString(fmt.Sprintf("uncappedTarget: %sK, %vm;", cr.UncappedTarget.Memory().AsDec(), cr.UncappedTarget.Cpu().MilliValue())) - } - sb.WriteString("\n") - } - return sb.String() -} - type prioritizedPod struct { pod *apiv1.Pod priority PodPriority diff --git a/vertical-pod-autoscaler/pkg/updater/priority/update_priority_calculator_test.go b/vertical-pod-autoscaler/pkg/updater/priority/update_priority_calculator_test.go index 94b149f8c877..4af15a5e7c4e 100644 --- a/vertical-pod-autoscaler/pkg/updater/priority/update_priority_calculator_test.go +++ b/vertical-pod-autoscaler/pkg/updater/priority/update_priority_calculator_test.go @@ -555,3 +555,39 @@ func TestLessPodPriority(t *testing.T) { } } + +func TestAddPodLogs(t *testing.T) { + testCases := []struct { + name string + givenRec *vpa_types.RecommendedPodResources + expectedLog string + }{ + { + name: "container with target and uncappedTarget", + givenRec: test.Recommendation().WithContainer(containerName).WithTarget("4", "10M").Get(), + expectedLog: "container1: target: 10000000K 4000m; uncappedTarget: 10000000K 4000m;\n", + }, + { + name: "container with cpu only", + givenRec: test.Recommendation().WithContainer(containerName).WithTarget("8", "").Get(), + expectedLog: "container1: target: 8000m; uncappedTarget: 8000m;\n", + }, + { + name: "container with memory only", + givenRec: test.Recommendation().WithContainer(containerName).WithTarget("", "10M").Get(), + expectedLog: "container1: target: 10000000K uncappedTarget: 10000000K \n", + }, + } + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + vpa := test.VerticalPodAutoscaler().WithContainer(containerName).WithTarget("10", "").Get() + priorityProcessor := NewFakeProcessor(map[string]PodPriority{ + "POD1": {ScaleUp: true, ResourceDiff: 4.0}}) + calculator := NewUpdatePriorityCalculator(vpa, nil, + &test.FakeRecommendationProcessor{}, priorityProcessor) + + actualLog := calculator.GetProcessedRecommendationTargets(tc.givenRec) + assert.Equal(t, tc.expectedLog, actualLog) + }) + } +} From 390b06149ec684f1b2ee231ea461f0979b5ce255 Mon Sep 17 00:00:00 2001 From: nikimanoledaki Date: Mon, 27 May 2024 17:11:42 +0200 Subject: [PATCH 6/7] Add doc comment for VPA Updater's GetProcessedRecommendationTargets func --- .../pkg/updater/priority/update_priority_calculator.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/vertical-pod-autoscaler/pkg/updater/priority/update_priority_calculator.go b/vertical-pod-autoscaler/pkg/updater/priority/update_priority_calculator.go index cf0b398e834b..f7e3729a9bb8 100644 --- a/vertical-pod-autoscaler/pkg/updater/priority/update_priority_calculator.go +++ b/vertical-pod-autoscaler/pkg/updater/priority/update_priority_calculator.go @@ -165,6 +165,8 @@ func (calc *UpdatePriorityCalculator) GetSortedPods(admission PodEvictionAdmissi return result } +// GetProcessedRecommendationTargets takes a RecommendedPodResources object and returns a formatted string +// with the recommended pod resources. Specifically, it formats the target and uncapped target CPU and memory. func (calc *UpdatePriorityCalculator) GetProcessedRecommendationTargets(r *vpa_types.RecommendedPodResources) string { sb := &strings.Builder{} for _, cr := range r.ContainerRecommendations { From 9a57ecd597858c538e2bb8b5465195bb935d969b Mon Sep 17 00:00:00 2001 From: nikimanoledaki Date: Wed, 29 May 2024 10:13:35 +0200 Subject: [PATCH 7/7] Change mem value to kilobyte --- .../pkg/updater/priority/update_priority_calculator.go | 5 +++-- .../pkg/updater/priority/update_priority_calculator_test.go | 4 ++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/vertical-pod-autoscaler/pkg/updater/priority/update_priority_calculator.go b/vertical-pod-autoscaler/pkg/updater/priority/update_priority_calculator.go index f7e3729a9bb8..d47a4ae9f516 100644 --- a/vertical-pod-autoscaler/pkg/updater/priority/update_priority_calculator.go +++ b/vertical-pod-autoscaler/pkg/updater/priority/update_priority_calculator.go @@ -24,6 +24,7 @@ import ( "time" apiv1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/resource" "k8s.io/apimachinery/pkg/util/sets" vpa_types "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/apis/autoscaling.k8s.io/v1" "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/utils/annotations" @@ -174,7 +175,7 @@ func (calc *UpdatePriorityCalculator) GetProcessedRecommendationTargets(r *vpa_t if cr.Target != nil { sb.WriteString("target: ") if !cr.Target.Memory().IsZero() { - sb.WriteString(fmt.Sprintf("%sK ", cr.Target.Memory().AsDec())) + sb.WriteString(fmt.Sprintf("%dk ", cr.Target.Memory().ScaledValue(resource.Kilo))) } if !cr.Target.Cpu().IsZero() { sb.WriteString(fmt.Sprintf("%vm; ", cr.Target.Cpu().MilliValue())) @@ -183,7 +184,7 @@ func (calc *UpdatePriorityCalculator) GetProcessedRecommendationTargets(r *vpa_t if cr.UncappedTarget != nil { sb.WriteString("uncappedTarget: ") if !cr.UncappedTarget.Memory().IsZero() { - sb.WriteString(fmt.Sprintf("%sK ", cr.UncappedTarget.Memory().AsDec())) + sb.WriteString(fmt.Sprintf("%dk ", cr.UncappedTarget.Memory().ScaledValue(resource.Kilo))) } if !cr.UncappedTarget.Cpu().IsZero() { sb.WriteString(fmt.Sprintf("%vm;", cr.UncappedTarget.Cpu().MilliValue())) diff --git a/vertical-pod-autoscaler/pkg/updater/priority/update_priority_calculator_test.go b/vertical-pod-autoscaler/pkg/updater/priority/update_priority_calculator_test.go index 4af15a5e7c4e..2d169d38afad 100644 --- a/vertical-pod-autoscaler/pkg/updater/priority/update_priority_calculator_test.go +++ b/vertical-pod-autoscaler/pkg/updater/priority/update_priority_calculator_test.go @@ -565,7 +565,7 @@ func TestAddPodLogs(t *testing.T) { { name: "container with target and uncappedTarget", givenRec: test.Recommendation().WithContainer(containerName).WithTarget("4", "10M").Get(), - expectedLog: "container1: target: 10000000K 4000m; uncappedTarget: 10000000K 4000m;\n", + expectedLog: "container1: target: 10000k 4000m; uncappedTarget: 10000k 4000m;\n", }, { name: "container with cpu only", @@ -575,7 +575,7 @@ func TestAddPodLogs(t *testing.T) { { name: "container with memory only", givenRec: test.Recommendation().WithContainer(containerName).WithTarget("", "10M").Get(), - expectedLog: "container1: target: 10000000K uncappedTarget: 10000000K \n", + expectedLog: "container1: target: 10000k uncappedTarget: 10000k \n", }, } for _, tc := range testCases {