autoscaler: ignore ScaleDownDelay if not reachable (#15831)

* autoscaler: ignore ScaleDownDelay if not reachable

* gofumpt

---------

Co-authored-by: Dave Protasowski <[email protected]>

Author: scottjmaddox

Diff for: pkg/autoscaler/scaling/autoscaler.go

@@ -247,13 +247,13 @@ func (a *autoscaler) Scale(logger *zap.SugaredLogger, now time.Time) ScaleResult
 		logger.Debug("Operating in stable mode.")
 	}
 
-	// Delay scale down decisions, if a ScaleDownDelay was specified.
+	// Delay scale down decisions if reachable and if a ScaleDownDelay was specified.
 	// We only do this if there's a non-nil delayWindow because although a
 	// one-element delay window is _almost_ the same as no delay at all, it is
 	// not the same in the case where two Scale()s happen in the same time
 	// interval (because the largest will be picked rather than the most recent
 	// in that case).
-	if a.delayWindow != nil {
+	if a.deciderSpec.Reachable && a.delayWindow != nil {
 		a.delayWindow.Record(now, desiredPodCount)
 		delayedPodCount := a.delayWindow.Current()
 		if delayedPodCount != desiredPodCount {

Diff for: pkg/autoscaler/scaling/autoscaler_test.go

@@ -76,6 +76,7 @@ func TestAutoscalerScaleDownDelay(t *testing.T) {
 		MaxScaleUpRate:   10,
 		PanicThreshold:   100,
 		ScaleDownDelay:   5 * time.Minute,
+		Reachable:        true,
 	}
 	as := New(context.Background(), testNamespace, testRevision, metrics, pc, spec)
 
@@ -128,6 +129,50 @@ func TestAutoscalerScaleDownDelay(t *testing.T) {
 	})
 }
 
+func TestAutoscalerScaleDownDelayNotReachable(t *testing.T) {
+	pc := &fakePodCounter{}
+	metrics := &metricClient{}
+	spec := &DeciderSpec{
+		TargetValue:      10,
+		MaxScaleDownRate: 10,
+		MaxScaleUpRate:   10,
+		PanicThreshold:   100,
+		ScaleDownDelay:   5 * time.Minute,
+		Reachable:        true,
+	}
+	as := New(context.Background(), testNamespace, testRevision, metrics, pc, spec)
+
+	now := time.Time{}
+
+	// scale up.
+	metrics.SetStableAndPanicConcurrency(40, 40)
+	expectScale(t, as, now.Add(2*time.Second), ScaleResult{
+		ScaleValid:      true,
+		DesiredPodCount: 4,
+	})
+	// one minute passes at reduced concurrency - should not scale down (less than delay).
+	metrics.SetStableAndPanicConcurrency(0, 0)
+	expectScale(t, as, now.Add(1*time.Minute), ScaleResult{
+		ScaleValid:      true,
+		DesiredPodCount: 4,
+	})
+	// mark as unreachable to simulate another revision coming up
+	unreachableSpec := &DeciderSpec{
+		TargetValue:      10,
+		MaxScaleDownRate: 10,
+		MaxScaleUpRate:   10,
+		PanicThreshold:   100,
+		ScaleDownDelay:   5 * time.Minute,
+		Reachable:        false,
+	}
+	as.Update(unreachableSpec)
+	// 2 seconds pass at reduced concurrency - now we scale down.
+	expectScale(t, as, now.Add(2*time.Second), ScaleResult{
+		ScaleValid:      true,
+		DesiredPodCount: 0,
+	})
+}
+
 func TestAutoscalerScaleDownDelayZero(t *testing.T) {
 	pc := &fakePodCounter{}
 	metrics := &metricClient{}