From bab77cd33848aafe1e3bd4655c98c962c7a8ee88 Mon Sep 17 00:00:00 2001 From: Ismail Alidzhikov <9372594+ialidzhikov@users.noreply.github.com> Date: Tue, 11 Feb 2025 17:22:11 +0200 Subject: [PATCH] Address review feedback from plkokanov --- vertical-pod-autoscaler/pkg/utils/vpa/capping.go | 4 +++- vertical-pod-autoscaler/pkg/utils/vpa/capping_test.go | 5 +++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/vertical-pod-autoscaler/pkg/utils/vpa/capping.go b/vertical-pod-autoscaler/pkg/utils/vpa/capping.go index 080dc8de68c7..815caa37048c 100644 --- a/vertical-pod-autoscaler/pkg/utils/vpa/capping.go +++ b/vertical-pod-autoscaler/pkg/utils/vpa/capping.go @@ -200,7 +200,9 @@ func applyVPAPolicyForContainer(containerName string, var maxAllowed apiv1.ResourceList if containerPolicy != nil { - maxAllowed = containerPolicy.MaxAllowed + // Deep copy containerPolicy.MaxAllowed as maxAllowed can later on be merged with globalMaxAllowed. + // Deep copy is needed to prevent unwanted modifications to containerPolicy.MaxAllowed. + maxAllowed = containerPolicy.MaxAllowed.DeepCopy() } if maxAllowed == nil { maxAllowed = globalMaxAllowed diff --git a/vertical-pod-autoscaler/pkg/utils/vpa/capping_test.go b/vertical-pod-autoscaler/pkg/utils/vpa/capping_test.go index 825f31356b95..9fd890cb4e5e 100644 --- a/vertical-pod-autoscaler/pkg/utils/vpa/capping_test.go +++ b/vertical-pod-autoscaler/pkg/utils/vpa/capping_test.go @@ -569,9 +569,14 @@ func TestApplyVPAPolicy(t *testing.T) { for _, tt := range tests { t.Run(tt.Name, func(t *testing.T) { + resourcePolicyCopy := tt.ResourcePolicy.DeepCopy() + actual, err := ApplyVPAPolicy(tt.PodRecommendation, tt.ResourcePolicy, tt.GlobalMaxAllowed) assert.Nil(t, err) assert.Equal(t, tt.Expected, actual) + + // Make sure that the func does not have a side affect and does not modify the passed resource policy. + assert.Equal(t, resourcePolicyCopy, tt.ResourcePolicy) }) } }