Skip to content

Commit

Permalink
use isSuperset for comparison in isMonitorVPAReady instead of Equals (#…
Browse files Browse the repository at this point in the history
…415)

* use isSuperset for comparison in isMonitorVPAReady instead of Equals

isMonitorVPAReady now correctly checks that the VPA has recommendations for at least Tortoise defined containers, extra containers doesn't result in a false not ready.

* service_test: added test for defining a subset on Tortoise
  • Loading branch information
Dymanik authored Sep 25, 2024
1 parent b9f4a1c commit 059dd51
Show file tree
Hide file tree
Showing 2 changed files with 43 additions and 1 deletion.
2 changes: 1 addition & 1 deletion pkg/vpa/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ func isMonitorVPAReady(vpa *v1.VerticalPodAutoscaler, tortoise *autoscalingv1bet
}
}

return containerInTortoise.Equal(containerInVPA)
return containerInVPA.IsSuperset(containerInTortoise)
}

func SetAllVerticalContainerResourcePhaseWorking(tortoise *autoscalingv1beta3.Tortoise, now time.Time) *autoscalingv1beta3.Tortoise {
Expand Down
42 changes: 42 additions & 0 deletions pkg/vpa/service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -628,6 +628,48 @@ func Test_isMonitorVPAReady(t *testing.T) {
},
},
},
{
name: "Tortoise defines less containers than VPA, VPA is Ready",
args: args{
vpa: &vpav1.VerticalPodAutoscaler{
Status: vpav1.VerticalPodAutoscalerStatus{
Conditions: []vpav1.VerticalPodAutoscalerCondition{
{
Type: vpav1.RecommendationProvided,
Status: v1.ConditionFalse,
},
},
Recommendation: &vpav1.RecommendedPodResources{
ContainerRecommendations: []vpav1.RecommendedContainerResources{
{
ContainerName: "app",
Target: v1.ResourceList{
v1.ResourceMemory: resource.MustParse("1Gi"),
v1.ResourceCPU: resource.MustParse("1"), // wrong
},
},
{
ContainerName: "istio",
Target: v1.ResourceList{
v1.ResourceMemory: resource.MustParse("1Gi"),
v1.ResourceCPU: resource.MustParse("1"),
},
},
},
},
},
},
tortoise: &autoscalingv1beta3.Tortoise{
Status: autoscalingv1beta3.TortoiseStatus{
AutoscalingPolicy: []autoscalingv1beta3.ContainerAutoscalingPolicy{
{
ContainerName: "app",
},
},
},
},
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
Expand Down

0 comments on commit 059dd51

Please sign in to comment.