Skip to content

Commit ee28452

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request kubernetes#8946 from iamzili/fix-scaleQuantityProportionallyCPU-func
Fix scaleQuantityProportionallyCPU function
2 parents 346bab1 + 3aa3592 commit ee28452

5 files changed

Lines changed: 378 additions & 24 deletions

File tree

‎vertical-pod-autoscaler/e2e/v1/admission_controller.go‎

Lines changed: 178 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -689,6 +689,184 @@ var _ = AdmissionControllerE2eDescribe("Admission-controller", func() {
689689
}
690690
})
691691

692+
ginkgo.It("raises cpu requests and limits according to pod min limit set in LimitRange", func() {
693+
d := utils.NewNHamstersDeployment(f, 3)
694+
695+
d.Spec.Template.Spec.Containers[0].Resources.Requests = apiv1.ResourceList{
696+
apiv1.ResourceCPU: ParseQuantityOrDie("1m"),
697+
apiv1.ResourceMemory: ParseQuantityOrDie("100Mi"),
698+
}
699+
d.Spec.Template.Spec.Containers[0].Resources.Limits = apiv1.ResourceList{
700+
apiv1.ResourceCPU: ParseQuantityOrDie("1m"),
701+
apiv1.ResourceMemory: ParseQuantityOrDie("100Mi"),
702+
}
703+
d.Spec.Template.Spec.Containers[1].Resources.Requests = apiv1.ResourceList{
704+
apiv1.ResourceCPU: ParseQuantityOrDie("1m"),
705+
apiv1.ResourceMemory: ParseQuantityOrDie("100Mi"),
706+
}
707+
d.Spec.Template.Spec.Containers[1].Resources.Limits = apiv1.ResourceList{
708+
apiv1.ResourceCPU: ParseQuantityOrDie("1m"),
709+
apiv1.ResourceMemory: ParseQuantityOrDie("100Mi"),
710+
}
711+
d.Spec.Template.Spec.Containers[2].Resources.Requests = apiv1.ResourceList{
712+
apiv1.ResourceCPU: ParseQuantityOrDie("1m"),
713+
apiv1.ResourceMemory: ParseQuantityOrDie("100Mi"),
714+
}
715+
d.Spec.Template.Spec.Containers[2].Resources.Limits = apiv1.ResourceList{
716+
apiv1.ResourceCPU: ParseQuantityOrDie("1m"),
717+
apiv1.ResourceMemory: ParseQuantityOrDie("100Mi"),
718+
}
719+
720+
container1Name := utils.GetHamsterContainerNameByIndex(0)
721+
container2Name := utils.GetHamsterContainerNameByIndex(1)
722+
container3Name := utils.GetHamsterContainerNameByIndex(2)
723+
724+
ginkgo.By("Setting up a VPA CRD")
725+
vpaCRD := test.VerticalPodAutoscaler().
726+
WithName("hamster-vpa").
727+
WithNamespace(f.Namespace.Name).
728+
WithTargetRef(utils.HamsterTargetRef).
729+
WithContainer(container1Name).
730+
AppendRecommendation(
731+
test.Recommendation().
732+
WithContainer(container1Name).
733+
WithTarget("4m", "100Mi").
734+
WithLowerBound("4m", "100Mi").
735+
WithUpperBound("4m", "100Mi").
736+
GetContainerResources()).
737+
WithContainer(container2Name).
738+
AppendRecommendation(
739+
test.Recommendation().
740+
WithContainer(container2Name).
741+
WithTarget("21m", "100Mi").
742+
WithLowerBound("21m", "100Mi").
743+
WithUpperBound("21m", "100Mi").
744+
GetContainerResources()).
745+
WithContainer(container3Name).
746+
AppendRecommendation(
747+
test.Recommendation().
748+
WithContainer(container3Name).
749+
WithTarget("90m", "100Mi").
750+
WithLowerBound("90m", "100Mi").
751+
WithUpperBound("90m", "100Mi").
752+
GetContainerResources()).
753+
Get()
754+
755+
utils.InstallVPA(f, vpaCRD)
756+
757+
minCpu := ParseQuantityOrDie("150m")
758+
installLimitRange(f, &minCpu, nil, nil, nil, apiv1.LimitTypePod)
759+
760+
ginkgo.By("Setting up a hamster deployment")
761+
podList := utils.StartDeploymentPods(f, d)
762+
763+
expectedRequestsLimits := map[string]string{
764+
container1Name: "6m", // ceil((4*150)/115) = ceil(5.22) = 6; for more details check PR #8946
765+
container2Name: "28m", // ceil((21*150)/115)
766+
container3Name: "118m", // ceil((90*150)/115)
767+
}
768+
769+
for _, pod := range podList.Items {
770+
for _, container := range pod.Spec.Containers {
771+
gomega.Expect(*container.Resources.Requests.Cpu()).To(gomega.Equal(ParseQuantityOrDie(expectedRequestsLimits[container.Name])))
772+
gomega.Expect(*container.Resources.Requests.Memory()).To(gomega.Equal(ParseQuantityOrDie("100Mi")))
773+
gomega.Expect(*container.Resources.Limits.Cpu()).To(gomega.Equal(ParseQuantityOrDie(expectedRequestsLimits[container.Name])))
774+
gomega.Expect(*container.Resources.Limits.Memory()).To(gomega.Equal(ParseQuantityOrDie("100Mi")))
775+
gomega.Expect(float64(container.Resources.Limits.Cpu().MilliValue()) / float64(container.Resources.Requests.Cpu().MilliValue())).To(gomega.BeNumerically("~", 1))
776+
gomega.Expect(float64(container.Resources.Limits.Memory().Value()) / float64(container.Resources.Requests.Memory().Value())).To(gomega.BeNumerically("~", 1))
777+
}
778+
}
779+
})
780+
781+
ginkgo.It("caps cpu requests and limits according to pod max limit set in LimitRange", func() {
782+
d := utils.NewNHamstersDeployment(f, 3)
783+
784+
d.Spec.Template.Spec.Containers[0].Resources.Requests = apiv1.ResourceList{
785+
apiv1.ResourceCPU: ParseQuantityOrDie("1m"),
786+
apiv1.ResourceMemory: ParseQuantityOrDie("100Mi"),
787+
}
788+
d.Spec.Template.Spec.Containers[0].Resources.Limits = apiv1.ResourceList{
789+
apiv1.ResourceCPU: ParseQuantityOrDie("1m"),
790+
apiv1.ResourceMemory: ParseQuantityOrDie("100Mi"),
791+
}
792+
d.Spec.Template.Spec.Containers[1].Resources.Requests = apiv1.ResourceList{
793+
apiv1.ResourceCPU: ParseQuantityOrDie("1m"),
794+
apiv1.ResourceMemory: ParseQuantityOrDie("100Mi"),
795+
}
796+
d.Spec.Template.Spec.Containers[1].Resources.Limits = apiv1.ResourceList{
797+
apiv1.ResourceCPU: ParseQuantityOrDie("1m"),
798+
apiv1.ResourceMemory: ParseQuantityOrDie("100Mi"),
799+
}
800+
d.Spec.Template.Spec.Containers[2].Resources.Requests = apiv1.ResourceList{
801+
apiv1.ResourceCPU: ParseQuantityOrDie("1m"),
802+
apiv1.ResourceMemory: ParseQuantityOrDie("100Mi"),
803+
}
804+
d.Spec.Template.Spec.Containers[2].Resources.Limits = apiv1.ResourceList{
805+
apiv1.ResourceCPU: ParseQuantityOrDie("1m"),
806+
apiv1.ResourceMemory: ParseQuantityOrDie("100Mi"),
807+
}
808+
809+
container1Name := utils.GetHamsterContainerNameByIndex(0)
810+
container2Name := utils.GetHamsterContainerNameByIndex(1)
811+
container3Name := utils.GetHamsterContainerNameByIndex(2)
812+
813+
ginkgo.By("Setting up a VPA CRD")
814+
vpaCRD := test.VerticalPodAutoscaler().
815+
WithName("hamster-vpa").
816+
WithNamespace(f.Namespace.Name).
817+
WithTargetRef(utils.HamsterTargetRef).
818+
WithContainer(container1Name).
819+
AppendRecommendation(
820+
test.Recommendation().
821+
WithContainer(container1Name).
822+
WithTarget("4m", "100Mi").
823+
WithLowerBound("4m", "100Mi").
824+
WithUpperBound("4m", "100Mi").
825+
GetContainerResources()).
826+
WithContainer(container2Name).
827+
AppendRecommendation(
828+
test.Recommendation().
829+
WithContainer(container2Name).
830+
WithTarget("21m", "100Mi").
831+
WithLowerBound("21m", "100Mi").
832+
WithUpperBound("21m", "100Mi").
833+
GetContainerResources()).
834+
WithContainer(container3Name).
835+
AppendRecommendation(
836+
test.Recommendation().
837+
WithContainer(container3Name).
838+
WithTarget("90m", "100Mi").
839+
WithLowerBound("90m", "100Mi").
840+
WithUpperBound("90m", "100Mi").
841+
GetContainerResources()).
842+
Get()
843+
844+
utils.InstallVPA(f, vpaCRD)
845+
846+
maxCpu := ParseQuantityOrDie("80m")
847+
installLimitRange(f, nil, nil, &maxCpu, nil, apiv1.LimitTypePod)
848+
849+
ginkgo.By("Setting up a hamster deployment")
850+
podList := utils.StartDeploymentPods(f, d)
851+
852+
expectedRequestsLimits := map[string]string{
853+
container1Name: "2m", // floor((4*80)/115), for more details check PR #8946
854+
container2Name: "14m", // floor((21*80)/115)
855+
container3Name: "62m", // floor((90*80)/115)
856+
}
857+
858+
for _, pod := range podList.Items {
859+
for _, container := range pod.Spec.Containers {
860+
gomega.Expect(*container.Resources.Requests.Cpu()).To(gomega.Equal(ParseQuantityOrDie(expectedRequestsLimits[container.Name])))
861+
gomega.Expect(*container.Resources.Requests.Memory()).To(gomega.Equal(ParseQuantityOrDie("100Mi")))
862+
gomega.Expect(*container.Resources.Limits.Cpu()).To(gomega.Equal(ParseQuantityOrDie(expectedRequestsLimits[container.Name])))
863+
gomega.Expect(*container.Resources.Limits.Memory()).To(gomega.Equal(ParseQuantityOrDie("100Mi")))
864+
gomega.Expect(float64(container.Resources.Limits.Cpu().MilliValue()) / float64(container.Resources.Requests.Cpu().MilliValue())).To(gomega.BeNumerically("~", 1))
865+
gomega.Expect(float64(container.Resources.Limits.Memory().Value()) / float64(container.Resources.Requests.Memory().Value())).To(gomega.BeNumerically("~", 1))
866+
}
867+
}
868+
})
869+
692870
ginkgo.It("raises request according to pod min limit set in LimitRange", func() {
693871
d := NewHamsterDeploymentWithResourcesAndLimits(f,
694872
ParseQuantityOrDie("100m") /*cpu request*/, ParseQuantityOrDie("200Mi"), /*memory request*/

‎vertical-pod-autoscaler/pkg/utils/vpa/capping.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -443,7 +443,7 @@ func applyPodLimitRange(resources []vpa_types.RecommendedContainerResources,
443443
if resourceName == corev1.ResourceMemory {
444444
cappedContainerRequest, _ = scaleQuantityProportionallyMem(&request, &sumRecommendation, &minLimit, roundUpToFullUnit)
445445
} else {
446-
cappedContainerRequest, _ = scaleQuantityProportionallyCPU(&request, &sumRecommendation, &minLimit, noRounding)
446+
cappedContainerRequest, _ = scaleQuantityProportionallyCPU(&request, &sumRecommendation, &minLimit, roundUpToFullUnit)
447447
}
448448
(*fieldGetter(*containerWithRecommendation.recommendation))[resourceName] = *cappedContainerRequest
449449
}
@@ -475,7 +475,7 @@ func applyPodLimitRange(resources []vpa_types.RecommendedContainerResources,
475475
if resourceName == corev1.ResourceMemory {
476476
cappedContainerRequest, _ = scaleQuantityProportionallyMem(&limit, &sumLimit, &targetTotalLimit, roundDownToFullUnit)
477477
} else {
478-
cappedContainerRequest, _ = scaleQuantityProportionallyCPU(&limit, &sumLimit, &targetTotalLimit, noRounding)
478+
cappedContainerRequest, _ = scaleQuantityProportionallyCPU(&limit, &sumLimit, &targetTotalLimit, roundDownToFullUnit)
479479
}
480480
(*fieldGetter(*containerWithRecommendation.recommendation))[resourceName] = *cappedContainerRequest
481481
}

‎vertical-pod-autoscaler/pkg/utils/vpa/capping_test.go‎

Lines changed: 130 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1084,6 +1084,136 @@ func TestApplyPodLimitRange(t *testing.T) {
10841084
},
10851085
},
10861086
},
1087+
{
1088+
name: "cap target cpu to pod min",
1089+
resources: []vpa_types.RecommendedContainerResources{
1090+
{
1091+
ContainerName: "container1",
1092+
Target: corev1.ResourceList{
1093+
corev1.ResourceCPU: resource.MustParse("15m"),
1094+
},
1095+
},
1096+
{
1097+
ContainerName: "container2",
1098+
Target: corev1.ResourceList{
1099+
corev1.ResourceCPU: resource.MustParse("100m"),
1100+
},
1101+
},
1102+
},
1103+
pod: corev1.Pod{
1104+
Spec: corev1.PodSpec{
1105+
Containers: []corev1.Container{
1106+
{
1107+
Name: "container1",
1108+
Resources: corev1.ResourceRequirements{
1109+
Requests: corev1.ResourceList{
1110+
corev1.ResourceCPU: resource.MustParse("1m"),
1111+
},
1112+
Limits: corev1.ResourceList{
1113+
corev1.ResourceCPU: resource.MustParse("1m"),
1114+
},
1115+
},
1116+
},
1117+
{
1118+
Name: "container2",
1119+
Resources: corev1.ResourceRequirements{
1120+
Requests: corev1.ResourceList{
1121+
corev1.ResourceCPU: resource.MustParse("1m"),
1122+
},
1123+
Limits: corev1.ResourceList{
1124+
corev1.ResourceCPU: resource.MustParse("1m"),
1125+
},
1126+
},
1127+
},
1128+
},
1129+
},
1130+
},
1131+
limitRange: corev1.LimitRangeItem{
1132+
Min: corev1.ResourceList{
1133+
corev1.ResourceCPU: resource.MustParse("150m"),
1134+
},
1135+
},
1136+
resourceName: corev1.ResourceCPU,
1137+
expect: []vpa_types.RecommendedContainerResources{
1138+
{
1139+
ContainerName: "container1",
1140+
Target: corev1.ResourceList{
1141+
corev1.ResourceCPU: *resource.NewMilliQuantity(20, resource.DecimalSI), // ceil((15*150)/115), for more details check PR #8946
1142+
},
1143+
},
1144+
{
1145+
ContainerName: "container2",
1146+
Target: corev1.ResourceList{
1147+
corev1.ResourceCPU: *resource.NewMilliQuantity(131, resource.DecimalSI), // ceil((100*150)/115)
1148+
},
1149+
},
1150+
},
1151+
},
1152+
{
1153+
name: "cap target cpu to pod max",
1154+
resources: []vpa_types.RecommendedContainerResources{
1155+
{
1156+
ContainerName: "container1",
1157+
Target: corev1.ResourceList{
1158+
corev1.ResourceCPU: resource.MustParse("15m"),
1159+
},
1160+
},
1161+
{
1162+
ContainerName: "container2",
1163+
Target: corev1.ResourceList{
1164+
corev1.ResourceCPU: resource.MustParse("100m"),
1165+
},
1166+
},
1167+
},
1168+
pod: corev1.Pod{
1169+
Spec: corev1.PodSpec{
1170+
Containers: []corev1.Container{
1171+
{
1172+
Name: "container1",
1173+
Resources: corev1.ResourceRequirements{
1174+
Requests: corev1.ResourceList{
1175+
corev1.ResourceCPU: resource.MustParse("1m"),
1176+
},
1177+
Limits: corev1.ResourceList{
1178+
corev1.ResourceCPU: resource.MustParse("1m"),
1179+
},
1180+
},
1181+
},
1182+
{
1183+
Name: "container2",
1184+
Resources: corev1.ResourceRequirements{
1185+
Requests: corev1.ResourceList{
1186+
corev1.ResourceCPU: resource.MustParse("1m"),
1187+
},
1188+
Limits: corev1.ResourceList{
1189+
corev1.ResourceCPU: resource.MustParse("1m"),
1190+
},
1191+
},
1192+
},
1193+
},
1194+
},
1195+
},
1196+
limitRange: corev1.LimitRangeItem{
1197+
Max: corev1.ResourceList{
1198+
corev1.ResourceCPU: resource.MustParse("90m"),
1199+
},
1200+
},
1201+
resourceName: corev1.ResourceCPU,
1202+
expect: []vpa_types.RecommendedContainerResources{
1203+
{
1204+
ContainerName: "container1",
1205+
Target: corev1.ResourceList{
1206+
corev1.ResourceCPU: *resource.NewMilliQuantity(11, resource.DecimalSI), // floor((15*90)/115), for more details check PR #8946
1207+
},
1208+
},
1209+
{
1210+
ContainerName: "container2",
1211+
Target: corev1.ResourceList{
1212+
corev1.ResourceCPU: *resource.NewMilliQuantity(78, resource.DecimalSI), // floor((100*90)/115)
1213+
},
1214+
},
1215+
},
1216+
},
10871217
}
10881218
getTarget := func(rl vpa_types.RecommendedContainerResources) *corev1.ResourceList { return &rl.Target }
10891219
for _, tc := range tests {

0 commit comments

Comments
 (0)