From 6bb73bae0682d58ffb611d174cc6e51b2b26a9a1 Mon Sep 17 00:00:00 2001 From: Kushagra Date: Thu, 1 Sep 2022 11:18:37 +0000 Subject: [PATCH 1/4] FIX: hpa scale down with target >= 100 --- pkg/controller/podautoscaler/replica_calculator.go | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/pkg/controller/podautoscaler/replica_calculator.go b/pkg/controller/podautoscaler/replica_calculator.go index b065fce3743..c97c5f51140 100644 --- a/pkg/controller/podautoscaler/replica_calculator.go +++ b/pkg/controller/podautoscaler/replica_calculator.go @@ -105,9 +105,10 @@ func (c *ReplicaCalculator) GetResourceReplicas(ctx context.Context, currentRepl if len(missingPods) > 0 { if usageRatio < 1.0 { - // on a scale-down, treat missing pods as using 100% of the resource request + // on a scale-down, treat missing pods as using 100% (all) of the resource request + maxTargetUtilization := int64(max(100, int32(targetUtilization))) for podName := range missingPods { - metrics[podName] = metricsclient.PodMetric{Value: requests[podName]} + metrics[podName] = metricsclient.PodMetric{Value: requests[podName] * maxTargetUtilization / 100} } } else if usageRatio > 1.0 { // on a scale-up, treat missing pods as using 0% of the resource request @@ -208,9 +209,10 @@ func (c *ReplicaCalculator) calcPlainMetricReplicas(metrics metricsclient.PodMet if len(missingPods) > 0 { if usageRatio < 1.0 { - // on a scale-down, treat missing pods as using 100% of the resource request + // on a scale-down, treat missing pods as using 100% (all) of the resource request + maxTargetUtilization := int64(max(100, int32(targetUtilization))) for podName := range missingPods { - metrics[podName] = metricsclient.PodMetric{Value: targetUtilization} + metrics[podName] = metricsclient.PodMetric{Value: maxTargetUtilization} } } else { // on a scale-up, treat missing pods as using 0% of the resource request From bb735bf68954f90d3cb86af158a2380068095a6c Mon Sep 17 00:00:00 2001 From: Kushagra Date: Thu, 1 Sep 2022 13:01:36 +0000 Subject: [PATCH 2/4] revert for non-utilization metrics --- pkg/controller/podautoscaler/replica_calculator.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/pkg/controller/podautoscaler/replica_calculator.go b/pkg/controller/podautoscaler/replica_calculator.go index c97c5f51140..fe86a8111f6 100644 --- a/pkg/controller/podautoscaler/replica_calculator.go +++ b/pkg/controller/podautoscaler/replica_calculator.go @@ -210,9 +210,8 @@ func (c *ReplicaCalculator) calcPlainMetricReplicas(metrics metricsclient.PodMet if len(missingPods) > 0 { if usageRatio < 1.0 { // on a scale-down, treat missing pods as using 100% (all) of the resource request - maxTargetUtilization := int64(max(100, int32(targetUtilization))) for podName := range missingPods { - metrics[podName] = metricsclient.PodMetric{Value: maxTargetUtilization} + metrics[podName] = metricsclient.PodMetric{Value: targetUtilization} } } else { // on a scale-up, treat missing pods as using 0% of the resource request From b75fbda0ed29f5ffa6efe61879f3f9509682bd49 Mon Sep 17 00:00:00 2001 From: Kushagra Date: Fri, 2 Sep 2022 04:35:10 +0000 Subject: [PATCH 3/4] requested changes --- pkg/controller/podautoscaler/replica_calculator.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/pkg/controller/podautoscaler/replica_calculator.go b/pkg/controller/podautoscaler/replica_calculator.go index fe86a8111f6..049aa61f31c 100644 --- a/pkg/controller/podautoscaler/replica_calculator.go +++ b/pkg/controller/podautoscaler/replica_calculator.go @@ -106,9 +106,10 @@ func (c *ReplicaCalculator) GetResourceReplicas(ctx context.Context, currentRepl if len(missingPods) > 0 { if usageRatio < 1.0 { // on a scale-down, treat missing pods as using 100% (all) of the resource request - maxTargetUtilization := int64(max(100, int32(targetUtilization))) + // or the utilization target for targets higher than 100% + fallbackUtilization := int64(max(100, targetUtilization)) for podName := range missingPods { - metrics[podName] = metricsclient.PodMetric{Value: requests[podName] * maxTargetUtilization / 100} + metrics[podName] = metricsclient.PodMetric{Value: requests[podName] * fallbackUtilization / 100} } } else if usageRatio > 1.0 { // on a scale-up, treat missing pods as using 0% of the resource request @@ -209,7 +210,7 @@ func (c *ReplicaCalculator) calcPlainMetricReplicas(metrics metricsclient.PodMet if len(missingPods) > 0 { if usageRatio < 1.0 { - // on a scale-down, treat missing pods as using 100% (all) of the resource request + // on a scale-down, treat missing pods as using exactly the target amount for podName := range missingPods { metrics[podName] = metricsclient.PodMetric{Value: targetUtilization} } From de8245e9528b238f09692a3b275bd3c60a4ed1c2 Mon Sep 17 00:00:00 2001 From: Kushagra Date: Mon, 5 Sep 2022 14:56:24 +0000 Subject: [PATCH 4/4] added ut for the change --- .../podautoscaler/replica_calculator_test.go | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/pkg/controller/podautoscaler/replica_calculator_test.go b/pkg/controller/podautoscaler/replica_calculator_test.go index 2f401d8b7a6..5d915d1897d 100644 --- a/pkg/controller/podautoscaler/replica_calculator_test.go +++ b/pkg/controller/podautoscaler/replica_calculator_test.go @@ -1464,6 +1464,24 @@ func TestReplicaCalcMissingMetricsUnreadyScaleDown(t *testing.T) { tc.runTest(t) } +func TestReplicaCalcMissingMetricsScaleDownTargetOver100(t *testing.T) { + tc := replicaCalcTestCase{ + currentReplicas: 4, + expectedReplicas: 2, + podReadiness: []v1.ConditionStatus{v1.ConditionFalse, v1.ConditionTrue, v1.ConditionTrue, v1.ConditionTrue}, + resource: &resourceInfo{ + name: v1.ResourceCPU, + requests: []resource.Quantity{resource.MustParse("1.0"), resource.MustParse("1.0"), resource.MustParse("2.0"), resource.MustParse("2.0")}, + levels: makePodMetricLevels(200, 100, 100), + + targetUtilization: 300, + expectedUtilization: 6, + expectedValue: numContainersPerPod * 100, + }, + } + tc.runTest(t) +} + func TestReplicaCalcDuringRollingUpdateWithMaxSurge(t *testing.T) { tc := replicaCalcTestCase{ currentReplicas: 2,