Propagate HasBeenActive to a terminal condition as "ScaleTargetInitialized".

pkg/apis/autoscaling/v1alpha1/pa_lifecycle.go

@@ -28,10 +28,9 @@ import (
 	"knative.dev/serving/pkg/apis/autoscaling"
 )
 
-const hasBeenActiveAnnotation = "HasBeenActive"
-
 var podCondSet = apis.NewLivingConditionSet(
 	PodAutoscalerConditionActive,
+	PodAutoscalerConditionScaleTargetInitialized,
 )
 
 // GetConditionSet retrieves the condition set for this resource. Implements the KRShaped interface.
@@ -174,20 +173,16 @@ func (pas *PodAutoscalerStatus) IsInactive() bool {
 	return pas.GetCondition(PodAutoscalerConditionActive).IsFalse()
 }
 
-// HasBeenActive returns true if the pod autoscaler has reached its initial scale.
-func (pas *PodAutoscalerStatus) HasBeenActive() bool {
-	if val, ok := pas.Annotations[hasBeenActiveAnnotation]; !ok || val != "true" {
-		return false
-	}
-	return true
+// IsScaleTargetInitialized returns true if the PodAutoscaler's scale target has been
+// initialized successfully.
+func (pas *PodAutoscalerStatus) IsScaleTargetInitialized() bool {
+	return pas.GetCondition(PodAutoscalerConditionScaleTargetInitialized).IsTrue()
 }
 
-// MarkHasBeenActive marks the PA's PodAutoscalerConditionInitiallyActive condition true.
-func (pas *PodAutoscalerStatus) MarkHasBeenActive() {
-	if pas.Annotations == nil {
-		pas.Annotations = map[string]string{}
-	}
-	pas.Annotations[hasBeenActiveAnnotation] = "true"
+// MarkScaleTargetInitialized marks the PA's PodAutoscalerConditionScaleTargetInitialized
+// condition true.
+func (pas *PodAutoscalerStatus) MarkScaleTargetInitialized() {
+	podCondSet.Manage(pas).MarkTrue(PodAutoscalerConditionScaleTargetInitialized)
 }
 
 // GetCondition gets the condition `t`.

pkg/apis/autoscaling/v1alpha1/pa_lifecycle_test.go

@@ -980,16 +980,21 @@ func TestTypicalFlow(t *testing.T) {
 
 	// When we see traffic, mark ourselves active.
 	r.MarkActive()
+	r.MarkScaleTargetInitialized()
+	apistest.CheckConditionSucceeded(r, PodAutoscalerConditionScaleTargetInitialized, t)
 	apistest.CheckConditionSucceeded(r, PodAutoscalerConditionActive, t)
 	apistest.CheckConditionSucceeded(r, PodAutoscalerConditionReady, t)
 
 	// Check idempotency.
 	r.MarkActive()
+	r.MarkScaleTargetInitialized()
+	apistest.CheckConditionSucceeded(r, PodAutoscalerConditionScaleTargetInitialized, t)
 	apistest.CheckConditionSucceeded(r, PodAutoscalerConditionActive, t)
 	apistest.CheckConditionSucceeded(r, PodAutoscalerConditionReady, t)
 
 	// When we stop seeing traffic, mark ourselves inactive.
 	r.MarkInactive("TheReason", "the message")
+	apistest.CheckConditionSucceeded(r, PodAutoscalerConditionScaleTargetInitialized, t)
 	apistest.CheckConditionFailed(r, PodAutoscalerConditionActive, t)
 	if !r.IsInactive() {
 		t.Error("IsInactive was not set.")
@@ -999,6 +1004,7 @@ func TestTypicalFlow(t *testing.T) {
 	// When traffic hits the activator and we scale up the deployment we mark
 	// ourselves as activating.
 	r.MarkActivating("Activating", "Red team, GO!")
+	apistest.CheckConditionSucceeded(r, PodAutoscalerConditionScaleTargetInitialized, t)
 	apistest.CheckConditionOngoing(r, PodAutoscalerConditionActive, t)
 	apistest.CheckConditionOngoing(r, PodAutoscalerConditionReady, t)
 
@@ -1008,6 +1014,8 @@ func TestTypicalFlow(t *testing.T) {
 	if !r.IsActive() {
 		t.Error("Active was not set.")
 	}
+	apistest.CheckConditionSucceeded(r, PodAutoscalerConditionScaleTargetInitialized, t)
+	apistest.CheckConditionSucceeded(r, PodAutoscalerConditionActive, t)
 	apistest.CheckConditionSucceeded(r, PodAutoscalerConditionReady, t)
 }
 
@@ -1172,13 +1180,13 @@ func TestInitialScale(t *testing.T) {
 	}
 }
 
-func TestHasBeenActive(t *testing.T) {
+func TestIsScaleTargetInitialized(t *testing.T) {
 	p := PodAutoscaler{}
-	if got, want := p.Status.HasBeenActive(), false; got != want {
+	if got, want := p.Status.IsScaleTargetInitialized(), false; got != want {
 		t.Errorf("before marking initially active: got: %v, want: %v", got, want)
 	}
-	p.Status.MarkHasBeenActive()
-	if got, want := p.Status.HasBeenActive(), true; got != want {
+	p.Status.MarkScaleTargetInitialized()
+	if got, want := p.Status.IsScaleTargetInitialized(), true; got != want {
 		t.Errorf("after marking initially active: got: %v, want: %v", got, want)
 	}
 }

pkg/apis/autoscaling/v1alpha1/pa_types.go

@@ -112,6 +112,9 @@ const (
 	// PodAutoscalerConditionReady is set when the revision is starting to materialize
 	// runtime resources, and becomes true when those resources are ready.
 	PodAutoscalerConditionReady = apis.ConditionReady
+	// PodAutoscalerConditionScaleTargetInitialized is set when the PodAutoscaler's
+	// ScaleTargetRef was ready to serve traffic once.
+	PodAutoscalerConditionScaleTargetInitialized apis.ConditionType = "ScaleTargetInitialized"
 	// PodAutoscalerConditionActive is set when the PodAutoscaler's ScaleTargetRef is receiving traffic.
 	PodAutoscalerConditionActive apis.ConditionType = "Active"
 )

pkg/reconciler/autoscaling/hpa/hpa.go

@@ -91,6 +91,7 @@ func (c *Reconciler) ReconcileKind(ctx context.Context, pa *pav1alpha1.PodAutosc
 	if !sks.IsReady() {
 		pa.Status.MarkInactive("ServicesNotReady", "SKS Services are not ready yet")
 	} else {
+		pa.Status.MarkScaleTargetInitialized()
 		pa.Status.MarkActive()
 	}
 

pkg/reconciler/autoscaling/hpa/hpa_test.go

@@ -134,8 +134,8 @@ func TestReconcile(t *testing.T) {
 		Name: "no op",
 		Objects: []runtime.Object{
 			hpa(pa(testNamespace, testRevision, WithHPAClass, WithMetricAnnotation("cpu"))),
-			pa(testNamespace, testRevision, WithHPAClass, WithTraffic, WithPAStatusService(testRevision),
-				WithPAMetricsService(privateSvc), withScales(0, 0)),
+			pa(testNamespace, testRevision, WithHPAClass, WithTraffic, WithScaleTargetInitialized,
+				WithPAStatusService(testRevision), WithPAMetricsService(privateSvc), withScales(0, 0)),
 			deploy(testNamespace, testRevision),
 			sks(testNamespace, testRevision, WithDeployRef(deployName), WithSKSReady),
 		},
@@ -193,7 +193,7 @@ func TestReconcile(t *testing.T) {
 		},
 		WantStatusUpdates: []ktesting.UpdateActionImpl{{
 			Object: pa(testNamespace, testRevision, WithHPAClass, withScales(0, 0),
-				WithTraffic, WithPAStatusService(testRevision), WithPAMetricsService(privateSvc)),
+				WithTraffic, WithScaleTargetInitialized, WithPAStatusService(testRevision), WithPAMetricsService(privateSvc)),
 		}},
 		Key: key(testNamespace, testRevision),
 	}, {
@@ -205,7 +205,7 @@ func TestReconcile(t *testing.T) {
 			sks(testNamespace, testRevision, WithDeployRef("bar"), WithSKSReady),
 		},
 		WantStatusUpdates: []ktesting.UpdateActionImpl{{
-			Object: pa(testNamespace, testRevision, WithHPAClass, WithTraffic, withScales(5, 3),
+			Object: pa(testNamespace, testRevision, WithHPAClass, WithTraffic, WithScaleTargetInitialized, withScales(5, 3),
 				WithPAStatusService(testRevision), WithPAMetricsService(privateSvc)),
 		}},
 		Key: key(testNamespace, testRevision),
@@ -319,7 +319,7 @@ func TestReconcile(t *testing.T) {
 		},
 		WantStatusUpdates: []ktesting.UpdateActionImpl{{
 			Object: pa(testNamespace, testRevision, WithHPAClass, withScales(19, 18),
-				WithTraffic, WithPAStatusService(testRevision), WithPAMetricsService(privateSvc)),
+				WithTraffic, WithScaleTargetInitialized, WithPAStatusService(testRevision), WithPAMetricsService(privateSvc)),
 		}},
 		Key:     key(testNamespace, testRevision),
 		WantErr: true,
@@ -352,7 +352,7 @@ func TestReconcile(t *testing.T) {
 	}, {
 		Name: "update hpa with target usage",
 		Objects: []runtime.Object{
-			pa(testNamespace, testRevision, WithHPAClass, WithTraffic, withScales(0, 0),
+			pa(testNamespace, testRevision, WithHPAClass, WithTraffic, WithScaleTargetInitialized, withScales(0, 0),
 				WithPAStatusService(testRevision), WithPAMetricsService(privateSvc), WithTargetAnnotation("1")),
 			hpa(pa(testNamespace, testRevision, WithHPAClass, WithMetricAnnotation("cpu"))),
 			deploy(testNamespace, testRevision),
@@ -442,6 +442,7 @@ func pa(namespace, name string, options ...PodAutoscalerOption) *asv1a1.PodAutos
 			ProtocolType: networking.ProtocolHTTP1,
 		},
 	}
+	pa.Status.InitializeConditions()
 	for _, opt := range options {
 		opt(pa)
 	}

pkg/reconciler/autoscaling/kpa/kpa.go

@@ -259,7 +259,7 @@ func computeActiveCondition(pa *pav1alpha1.PodAutoscaler, pc podCounts) {
 
 	case pc.ready >= minReady:
 		if pc.want > 0 || !pa.Status.IsInactive() {
-			pa.Status.MarkHasBeenActive()
+			pa.Status.MarkScaleTargetInitialized()
 			// SKS should already be active.
 			pa.Status.MarkActive()
 		}

pkg/reconciler/autoscaling/kpa/kpa_test.go

@@ -192,8 +192,8 @@ func markInactive(pa *asv1a1.PodAutoscaler) {
 	pa.Status.MarkInactive("NoTraffic", "The target is not receiving traffic.")
 }
 
-func markHasBeenActive(pa *asv1a1.PodAutoscaler) {
-	pa.Status.MarkHasBeenActive()
+func markScaleTargetInitialized(pa *asv1a1.PodAutoscaler) {
+	pa.Status.MarkScaleTargetInitialized()
 }
 
 func kpa(ns, n string, opts ...PodAutoscalerOption) *asv1a1.PodAutoscaler {
@@ -202,6 +202,7 @@ func kpa(ns, n string, opts ...PodAutoscalerOption) *asv1a1.PodAutoscaler {
 	kpa.Generation = 1
 	kpa.Annotations["autoscaling.knative.dev/class"] = "kpa.autoscaling.knative.dev"
 	kpa.Annotations["autoscaling.knative.dev/metric"] = "concurrency"
+	kpa.Status.InitializeConditions()
 	for _, opt := range opts {
 		opt(kpa)
 	}
@@ -264,7 +265,7 @@ func TestReconcile(t *testing.T) {
 	}
 	activeKPAMinScale := func(g, w int32) *asv1a1.PodAutoscaler {
 		return kpa(
-			testNamespace, testRevision, markActive, markHasBeenActive, withScales(g, w), WithReachabilityReachable,
+			testNamespace, testRevision, markActive, markScaleTargetInitialized, withScales(g, w), WithReachabilityReachable,
 			withMinScale(defaultScale), WithPAStatusService(testRevision), WithPAMetricsService(privateSvc),
 			WithObservedGeneration(1),
 		)
@@ -300,7 +301,7 @@ func TestReconcile(t *testing.T) {
 		Name: "steady state",
 		Key:  key,
 		Objects: []runtime.Object{
-			kpa(testNamespace, testRevision, markActive, markHasBeenActive, WithPAMetricsService(privateSvc),
+			kpa(testNamespace, testRevision, markActive, markScaleTargetInitialized, WithPAMetricsService(privateSvc),
 				withScales(1, defaultScale), WithPAStatusService(testRevision), WithObservedGeneration(1)),
 			sks(testNamespace, testRevision, WithDeployRef(deployName), WithSKSReady),
 			metric(testNamespace, testRevision),
@@ -382,14 +383,14 @@ func TestReconcile(t *testing.T) {
 		Name: "create metric",
 		Key:  key,
 		Objects: []runtime.Object{
-			kpa(testNamespace, testRevision, markActive, markHasBeenActive,
+			kpa(testNamespace, testRevision, markActive, markScaleTargetInitialized,
 				withScales(1, defaultScale), WithPAStatusService(testRevision)),
 			defaultSKS,
 			defaultDeployment,
 			defaultEndpoints,
 		},
 		WantStatusUpdates: []clientgotesting.UpdateActionImpl{{
-			Object: kpa(testNamespace, testRevision, markActive, markHasBeenActive, withScales(1, defaultScale),
+			Object: kpa(testNamespace, testRevision, markActive, markScaleTargetInitialized, withScales(1, defaultScale),
 				WithPAStatusService(testRevision), WithPAMetricsService(privateSvc), WithObservedGeneration(1)),
 		}},
 		WantCreates: []runtime.Object{
@@ -399,7 +400,7 @@ func TestReconcile(t *testing.T) {
 		Name: "scale up deployment",
 		Key:  key,
 		Objects: []runtime.Object{
-			kpa(testNamespace, testRevision, markActive, markHasBeenActive, WithPAMetricsService(privateSvc),
+			kpa(testNamespace, testRevision, markActive, markScaleTargetInitialized, WithPAMetricsService(privateSvc),
 				withScales(1, defaultScale), WithPAStatusService(testRevision), WithObservedGeneration(1)),
 			defaultSKS,
 			metric(testNamespace, testRevision),
@@ -500,7 +501,7 @@ func TestReconcile(t *testing.T) {
 			defaultDeployment, defaultEndpoints,
 		},
 		WantStatusUpdates: []clientgotesting.UpdateActionImpl{{
-			Object: kpa(testNamespace, testRevision, markActive, markHasBeenActive, WithPAStatusService(testRevision),
+			Object: kpa(testNamespace, testRevision, markActive, markScaleTargetInitialized, WithPAStatusService(testRevision),
 				WithPAMetricsService(privateSvc), withScales(1, defaultScale), WithObservedGeneration(1)),
 		}},
 	}, {
@@ -544,7 +545,7 @@ func TestReconcile(t *testing.T) {
 			defaultDeployment, defaultEndpoints,
 		},
 		WantStatusUpdates: []clientgotesting.UpdateActionImpl{{
-			Object: kpa(testNamespace, testRevision, markActive, markHasBeenActive, withMinScale(2), WithPAMetricsService(privateSvc),
+			Object: kpa(testNamespace, testRevision, markActive, markScaleTargetInitialized, withMinScale(2), WithPAMetricsService(privateSvc),
 				withScales(1, defaultScale), WithPAStatusService(testRevision), WithReachabilityUnreachable,
 				WithObservedGeneration(1)),
 		}},
@@ -560,7 +561,7 @@ func TestReconcile(t *testing.T) {
 			makeSKSPrivateEndpoints(2, testNamespace, testRevision),
 		},
 		WantStatusUpdates: []clientgotesting.UpdateActionImpl{{
-			Object: kpa(testNamespace, testRevision, markActive, markHasBeenActive, withMinScale(2), WithPAMetricsService(privateSvc),
+			Object: kpa(testNamespace, testRevision, markActive, markScaleTargetInitialized, withMinScale(2), WithPAMetricsService(privateSvc),
 				withScales(2, defaultScale), WithPAStatusService(testRevision), WithReachabilityReachable,
 				WithObservedGeneration(1)),
 		}},
@@ -576,7 +577,7 @@ func TestReconcile(t *testing.T) {
 			makeSKSPrivateEndpoints(2, testNamespace, testRevision),
 		},
 		WantStatusUpdates: []clientgotesting.UpdateActionImpl{{
-			Object: kpa(testNamespace, testRevision, markActive, markHasBeenActive, withMinScale(2), WithPAMetricsService(privateSvc),
+			Object: kpa(testNamespace, testRevision, markActive, markScaleTargetInitialized, withMinScale(2), WithPAMetricsService(privateSvc),
 				withScales(2, defaultScale), WithPAStatusService(testRevision), WithReachabilityUnknown,
 				WithObservedGeneration(1)),
 		}},
@@ -755,7 +756,7 @@ func TestReconcile(t *testing.T) {
 		Ctx: context.WithValue(context.Background(), deciderKey,
 			decider(testNamespace, testRevision, 0 /* desiredScale */, 0 /* ebc */, scaling.MinActivators)),
 		Objects: []runtime.Object{
-			kpa(testNamespace, testRevision, markActive, markHasBeenActive, withScales(1, 1),
+			kpa(testNamespace, testRevision, markActive, markScaleTargetInitialized, withScales(1, 1),
 				WithPAStatusService(testRevision), WithPAMetricsService(privateSvc), WithObservedGeneration(1)),
 			defaultSKS,
 			metric(testNamespace, testRevision),
@@ -852,7 +853,7 @@ func TestReconcile(t *testing.T) {
 			minScalePatch,
 		},
 		WantStatusUpdates: []clientgotesting.UpdateActionImpl{{
-			Object: activatingKPAMinScale(underscale, markHasBeenActive),
+			Object: activatingKPAMinScale(underscale, markScaleTargetInitialized),
 		}},
 	}, {
 		// Scale to `minScale` and mark PA "active"
@@ -942,7 +943,7 @@ func TestReconcile(t *testing.T) {
 			decider(testNamespace, testRevision, defaultScale, /* desiredScale */
 				-42 /* ebc */, scaling.MinActivators)),
 		Objects: []runtime.Object{
-			kpa(testNamespace, testRevision, markActive, markHasBeenActive, WithPAMetricsService(privateSvc),
+			kpa(testNamespace, testRevision, markActive, markScaleTargetInitialized, WithPAMetricsService(privateSvc),
 				withScales(1, defaultScale), WithPAStatusService(testRevision), WithObservedGeneration(1)),
 			sks(testNamespace, testRevision, WithDeployRef(deployName), WithProxyMode, WithSKSReady),
 			metric(testNamespace, testRevision),
@@ -955,7 +956,7 @@ func TestReconcile(t *testing.T) {
 			decider(testNamespace, testRevision, defaultScale, /* desiredScale */
 				-42 /* ebc */, 1982 /*numActivators*/)),
 		Objects: []runtime.Object{
-			kpa(testNamespace, testRevision, markActive, markHasBeenActive, WithPAMetricsService(privateSvc),
+			kpa(testNamespace, testRevision, markActive, markScaleTargetInitialized, WithPAMetricsService(privateSvc),
 				withScales(1, defaultScale), WithPAStatusService(testRevision), WithObservedGeneration(1)),
 			sks(testNamespace, testRevision, WithDeployRef(deployName),
 				WithProxyMode, WithSKSReady, WithNumActivators(4)),
@@ -973,7 +974,7 @@ func TestReconcile(t *testing.T) {
 			decider(testNamespace, testRevision, defaultScale, /* desiredScale */
 				-18 /* ebc */, scaling.MinActivators+1)),
 		Objects: []runtime.Object{
-			kpa(testNamespace, testRevision, markActive, markHasBeenActive, WithPAMetricsService(privateSvc),
+			kpa(testNamespace, testRevision, markActive, markScaleTargetInitialized, WithPAMetricsService(privateSvc),
 				withScales(1, defaultScale), WithPAStatusService(testRevision), WithObservedGeneration(1)),
 			sks(testNamespace, testRevision, WithDeployRef(deployName), WithSKSReady,
 				WithNumActivators(2)),
@@ -991,7 +992,7 @@ func TestReconcile(t *testing.T) {
 			decider(testNamespace, testRevision, defaultScale, /* desiredScale */
 				1 /* ebc */, scaling.MinActivators)),
 		Objects: []runtime.Object{
-			kpa(testNamespace, testRevision, markActive, markHasBeenActive, WithPAMetricsService(privateSvc),
+			kpa(testNamespace, testRevision, markActive, markScaleTargetInitialized, WithPAMetricsService(privateSvc),
 				withScales(1, defaultScale), WithPAStatusService(testRevision), WithObservedGeneration(1)),
 			sks(testNamespace, testRevision, WithDeployRef(deployName), WithSKSReady, WithProxyMode,
 				WithNumActivators(3)),

pkg/reconciler/revision/revision_test.go

@@ -108,6 +108,7 @@ func testReadyPA(rev *v1.Revision) *av1alpha1.PodAutoscaler {
 	pa := resources.MakePA(rev)
 	pa.Status.InitializeConditions()
 	pa.Status.MarkActive()
+	pa.Status.MarkScaleTargetInitialized()
 	pa.Status.ServiceName = serviceName(rev.Name)
 	return pa
 }

pkg/reconciler/revision/table_test.go

@@ -247,7 +247,7 @@ func TestReconcile(t *testing.T) {
 		Objects: []runtime.Object{
 			rev("foo", "pa-ready",
 				withK8sServiceName("old-stuff"), WithLogURL, AllUnknownConditions),
-			pa("foo", "pa-ready", WithTraffic, WithPAStatusService("new-stuff"), WithReachabilityUnknown),
+			pa("foo", "pa-ready", WithTraffic, WithScaleTargetInitialized, WithPAStatusService("new-stuff"), WithReachabilityUnknown),
 			deploy(t, "foo", "pa-ready"),
 			image("foo", "pa-ready"),
 		},
@@ -343,7 +343,7 @@ func TestReconcile(t *testing.T) {
 				withK8sServiceName("ill-follow-the-sun"), WithLogURL, MarkRevisionReady,
 				WithRevisionLabel(serving.RouteLabelKey, "foo")),
 			pa("foo", "fix-mutated-pa", WithProtocolType(networking.ProtocolH2C),
-				WithTraffic, WithPAStatusService("fix-mutated-pa")),
+				WithTraffic, WithScaleTargetInitialized, WithPAStatusService("fix-mutated-pa")),
 			deploy(t, "foo", "fix-mutated-pa"),
 			image("foo", "fix-mutated-pa"),
 		},
@@ -357,7 +357,7 @@ func TestReconcile(t *testing.T) {
 				withDefaultContainerStatuses(), withObservedGeneration(1)),
 		}},
 		WantUpdates: []clientgotesting.UpdateActionImpl{{
-			Object: pa("foo", "fix-mutated-pa", WithTraffic,
+			Object: pa("foo", "fix-mutated-pa", WithTraffic, WithScaleTargetInitialized,
 				WithPAStatusService("fix-mutated-pa"), WithReachabilityReachable),
 		}},
 		WantEvents: []string{
@@ -511,7 +511,7 @@ func TestReconcile(t *testing.T) {
 		// This signal should make our Reconcile mark the Revision as Ready.
 		Objects: []runtime.Object{
 			rev("foo", "steady-ready", withK8sServiceName("very-steady"), WithLogURL),
-			pa("foo", "steady-ready", WithTraffic, WithPAStatusService("steadier-even")),
+			pa("foo", "steady-ready", WithTraffic, WithScaleTargetInitialized, WithPAStatusService("steadier-even")),
 			deploy(t, "foo", "steady-ready"),
 			image("foo", "steady-ready"),
 		},

pkg/testing/functional.go

@@ -84,6 +84,12 @@ func WithTraffic(pa *asv1a1.PodAutoscaler) {
 	pa.Status.MarkActive()
 }
 
+// WithScaleTargetInitialized updates the PA to reflect it having initialized its
+// ScaleTarget.
+func WithScaleTargetInitialized(pa *asv1a1.PodAutoscaler) {
+	pa.Status.MarkScaleTargetInitialized()
+}
+
 // WithPAStatusService annotates PA Status with the provided service name.
 func WithPAStatusService(svc string) PodAutoscalerOption {
 	return func(pa *asv1a1.PodAutoscaler) {