From 75c38777ad164862baba831baaafd71d6bb886b9 Mon Sep 17 00:00:00 2001
From: mattjmcnaughton <mattjmcnaughton@gmail.com>
Date: Tue, 10 Oct 2017 21:48:34 -0400
Subject: [PATCH] Fix hpa scaling above max replicas w/ scaleUpLimit

Fix #53670

Fix a bug where `desiredReplicas` could be greater than `maxReplicas`
if the original value for `desiredReplicas > scaleUpLimit` and
`scaleUpLimit > maxReplicas`. Previously, when that happened, we would
scale up to `scaleUpLimit`, and then in the next auto-scaling run, scale
down to `maxReplicas`. Address this issue and introduce a regression
test.
---
 pkg/controller/podautoscaler/horizontal.go    |  9 +++++++
 .../podautoscaler/horizontal_test.go          | 24 +++++++++++++++++++
 2 files changed, 33 insertions(+)

diff --git a/pkg/controller/podautoscaler/horizontal.go b/pkg/controller/podautoscaler/horizontal.go
index e9b1fb432f6..438261cc1c4 100644
--- a/pkg/controller/podautoscaler/horizontal.go
+++ b/pkg/controller/podautoscaler/horizontal.go
@@ -422,6 +422,15 @@ func (a *HorizontalController) reconcileAutoscaler(hpav1Shared *autoscalingv1.Ho
 		case desiredReplicas > scaleUpLimit:
 			setCondition(hpa, autoscalingv2.ScalingLimited, v1.ConditionTrue, "ScaleUpLimit", "the desired replica count is increasing faster than the maximum scale rate")
 			desiredReplicas = scaleUpLimit
+
+			// Ensure that even if the scaleUpLimit is greater
+			// than the maximum number of replicas, we only
+			// set the max number of replicas as desired.
+			if scaleUpLimit > hpa.Spec.MaxReplicas {
+				setCondition(hpa, autoscalingv2.ScalingLimited, v1.ConditionTrue, "TooManyReplicas", "the desired replica count was more than the maximum replica count")
+				desiredReplicas = hpa.Spec.MaxReplicas
+			}
+
 		case hpa.Spec.MinReplicas != nil && desiredReplicas < *hpa.Spec.MinReplicas:
 			// make sure we aren't below our minimum
 			var statusMsg string
diff --git a/pkg/controller/podautoscaler/horizontal_test.go b/pkg/controller/podautoscaler/horizontal_test.go
index 2d708318a3f..b4c7ad5846c 100644
--- a/pkg/controller/podautoscaler/horizontal_test.go
+++ b/pkg/controller/podautoscaler/horizontal_test.go
@@ -1219,6 +1219,30 @@ func TestUpscaleCap(t *testing.T) {
 	tc.runTest(t)
 }
 
+func TestUpscaleCapGreaterThanMaxReplicas(t *testing.T) {
+	tc := testCase{
+		minReplicas:     1,
+		maxReplicas:     20,
+		initialReplicas: 3,
+		// desiredReplicas would be 24 without maxReplicas
+		desiredReplicas:     20,
+		CPUTarget:           10,
+		reportedLevels:      []uint64{100, 200, 300},
+		reportedCPURequests: []resource.Quantity{resource.MustParse("0.1"), resource.MustParse("0.1"), resource.MustParse("0.1")},
+		useMetricsAPI:       true,
+		expectedConditions: statusOkWithOverrides(autoscalingv2.HorizontalPodAutoscalerCondition{
+			Type:   autoscalingv2.ScalingLimited,
+			Status: v1.ConditionTrue,
+			Reason: "ScaleUpLimit",
+		}, autoscalingv2.HorizontalPodAutoscalerCondition{
+			Type:   autoscalingv2.ScalingLimited,
+			Status: v1.ConditionTrue,
+			Reason: "TooManyReplicas",
+		}),
+	}
+	tc.runTest(t)
+}
+
 func TestConditionInvalidSelectorMissing(t *testing.T) {
 	tc := testCase{
 		minReplicas:         1,