Prevent a PodAutoscaler's DesiredScale to turn to -1 (#14866)

* Prevent a PodAutoscaler's DesiredScale to turn to -1

* use correct ptr package

---------

Co-authored-by: dprotaso <dprotaso@gmail.com>

pkg/reconciler/autoscaling/kpa/kpa.go

@@ -219,7 +219,15 @@ func (c *Reconciler) reconcileDecider(ctx context.Context, pa *autoscalingv1alph
 }
 
 func computeStatus(ctx context.Context, pa *autoscalingv1alpha1.PodAutoscaler, pc podCounts, logger *zap.SugaredLogger) {
-	pa.Status.DesiredScale, pa.Status.ActualScale = ptr.Int32(int32(pc.want)), ptr.Int32(int32(pc.ready))
+	pa.Status.ActualScale = ptr.Int32(int32(pc.ready))
+
+	// When the autoscaler just restarted, it does not yet have metrics and would change the desiredScale to -1 and moments
+	// later back to the correct value. The following condition omits this.
+	if pc.want == -1 && pa.Status.DesiredScale != nil && *pa.Status.DesiredScale >= 0 {
+		logger.Debugf("Ignoring change of desiredScale from %d to %d", *pa.Status.DesiredScale, pc.want)
+	} else {
+		pa.Status.DesiredScale = ptr.Int32(int32(pc.want))
+	}
 
 	reportMetrics(pa, pc)
 	computeActiveCondition(ctx, pa, pc)

pkg/reconciler/autoscaling/kpa/kpa_test.go

@@ -601,7 +601,7 @@ func TestReconcile(t *testing.T) {
 		WantStatusUpdates: []clientgotesting.UpdateActionImpl{{
 			// SKS does not exist, so we're just creating and have no status.
 			Object: kpa(testNamespace, testRevision, WithPASKSNotReady("No Private Service Name"),
-				WithBufferedTraffic, WithPAMetricsService(privateSvc), withScales(0, unknownScale),
+				WithBufferedTraffic, WithPAMetricsService(privateSvc), withScales(0, defaultScale),
 				WithObservedGeneration(1)),
 		}},
 		WantCreates: []runtime.Object{
@@ -1996,3 +1996,97 @@ func TestComputeActivatorNum(t *testing.T) {
 		})
 	}
 }
+
+func TestComputeStatus(t *testing.T) {
+	cases := []struct {
+		name string
+
+		haveActual       *int32
+		haveDesiredScale *int32
+
+		pcReady int
+		pcWant  int
+
+		wantActualScale  *int32
+		wantDesiredScale *int32
+	}{{
+		name: "initial",
+
+		haveActual:       nil,
+		haveDesiredScale: nil,
+
+		pcReady: 0,
+		pcWant:  1,
+
+		wantActualScale:  ptr.Int32(0),
+		wantDesiredScale: ptr.Int32(1),
+	}, {
+		name: "ready",
+
+		haveActual:       ptr.Int32(0),
+		haveDesiredScale: ptr.Int32(1),
+
+		pcReady: 1,
+		pcWant:  1,
+
+		wantActualScale:  ptr.Int32(1),
+		wantDesiredScale: ptr.Int32(1),
+	}, {
+		name: "stable",
+
+		haveActual:       ptr.Int32(1),
+		haveDesiredScale: ptr.Int32(1),
+
+		pcReady: 1,
+		pcWant:  1,
+
+		wantActualScale:  ptr.Int32(1),
+		wantDesiredScale: ptr.Int32(1),
+	}, {
+		name: "no metrics",
+
+		haveActual:       ptr.Int32(1),
+		haveDesiredScale: ptr.Int32(2),
+
+		pcReady: 2,
+		pcWant:  -1,
+
+		wantActualScale:  ptr.Int32(2),
+		wantDesiredScale: ptr.Int32(2),
+	}}
+
+	tc := &testConfigStore{config: defaultConfig()}
+
+	for _, c := range cases {
+		t.Run(c.name, func(t *testing.T) {
+			ctx := tc.ToContext(context.Background())
+
+			pa := &autoscalingv1alpha1.PodAutoscaler{
+				Status: autoscalingv1alpha1.PodAutoscalerStatus{
+					ActualScale:  c.haveActual,
+					DesiredScale: c.haveDesiredScale,
+				},
+			}
+			pc := podCounts{
+				ready: c.pcReady,
+				want:  c.pcWant,
+			}
+
+			computeStatus(ctx, pa, pc, logging.FromContext(ctx))
+
+			if c.wantActualScale == nil && pa.Status.ActualScale != nil || c.wantActualScale != nil && pa.Status.ActualScale == nil {
+				t.Errorf("Unexpected ActualScale. Want: %v, Got: %v", c.wantActualScale, pa.Status.ActualScale)
+			}
+			if c.wantActualScale != nil && pa.Status.ActualScale != nil && *c.wantActualScale != *pa.Status.ActualScale {
+				t.Errorf("Unexpected ActualScale. Want: %d, Got: %d", *c.wantActualScale, *pa.Status.ActualScale)
+			}
+
+			if c.wantDesiredScale == nil && pa.Status.DesiredScale != nil || c.wantDesiredScale != nil && pa.Status.DesiredScale == nil {
+				t.Errorf("Unexpected DesiredScale. Want: %v, Got: %v", c.wantDesiredScale, pa.Status.DesiredScale)
+			}
+			if c.wantDesiredScale != nil && pa.Status.DesiredScale != nil && *c.wantDesiredScale != *pa.Status.DesiredScale {
+				t.Errorf("Unexpected DesiredScale. Want: %d, Got: %d", *c.wantDesiredScale, *pa.Status.DesiredScale)
+			}
+		})
+	}
+}