Repository: knative/serving
Pull request #8583 (2020-07-08 21:41:02)

Switch to a common hash func in pkg
- use the moved hash function
- adapt to the new api
- remove the obsolete files

/assign mattmoor @yanweiguo  

knative-prow-robot: [APPROVALNOTIFIER] This PR is **APPROVED**

This pull-request has been approved by: *<a href="https://github.com/knative/serving/pull/8583#" title="Author self-approved">vagababov</a>*

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=knative%2Fserving).

The pull request process is described [here](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process)

<details >
Needs approval from an approver in each of these files:

- ~~[OWNERS](https://github.com/knative/serving/blob/master/OWNERS)~~ [vagababov]

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":[]} -->
yanweiguo: /lgtm
knative-prow-robot: @vagababov: The following test **failed**, say `/retest` to rerun all failed tests:

Test name | Commit | Details | Rerun command
--- | --- | --- | ---
pull-knative-serving-integration-tests | 64fa2db8a6bc45648da7d96b2ba27a8bbdfe8dfc | [link](https://prow.knative.dev/view/gcs/knative-prow/pr-logs/pull/knative_serving/8583/pull-knative-serving-integration-tests/1280980371319558146) | `/test pull-knative-serving-integration-tests`

[Full PR test history](https://gubernator.knative.dev/pr/knative_serving/8583). [Your PR dashboard](https://gubernator.knative.dev/pr/vagababov).

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->
vagababov: /retest
seems unrelated

Patch:
diff --git a/pkg/apis/autoscaling/v1alpha1/pa_lifecycle.go b/pkg/apis/autoscaling/v1alpha1/pa_lifecycle.go
index c4d7ce1e2..2d424874f 100644
--- a/pkg/apis/autoscaling/v1alpha1/pa_lifecycle.go
+++ b/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`.
diff --git a/pkg/apis/autoscaling/v1alpha1/pa_types.go b/pkg/apis/autoscaling/v1alpha1/pa_types.go
index 02fc64084..163189eca 100644
--- a/pkg/apis/autoscaling/v1alpha1/pa_types.go
+++ b/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"
 )
diff --git a/pkg/reconciler/autoscaling/hpa/hpa.go b/pkg/reconciler/autoscaling/hpa/hpa.go
index 98e2a92be..fb48c93eb 100644
--- a/pkg/reconciler/autoscaling/hpa/hpa.go
+++ b/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()
 	}
 
diff --git a/pkg/reconciler/autoscaling/kpa/kpa.go b/pkg/reconciler/autoscaling/kpa/kpa.go
index f9fc38d69..aaa08106c 100644
--- a/pkg/reconciler/autoscaling/kpa/kpa.go
+++ b/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()
 		}
diff --git a/pkg/reconciler/serverlessservice/serverlessservice.go b/pkg/reconciler/serverlessservice/serverlessservice.go
index ee20f5e45..b976e5132 100644
--- a/pkg/reconciler/serverlessservice/serverlessservice.go
+++ b/pkg/reconciler/serverlessservice/serverlessservice.go
@@ -30,6 +30,7 @@ import (
 	apierrs "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/labels"
+	"k8s.io/apimachinery/pkg/util/sets"
 	"k8s.io/client-go/kubernetes"
 	corev1listers "k8s.io/client-go/listers/core/v1"
 	sksreconciler "knative.dev/networking/pkg/client/injection/reconciler/networking/v1alpha1/serverlessservice"
@@ -37,6 +38,7 @@ import (
 	"knative.dev/networking/pkg/apis/networking"
 	netv1alpha1 "knative.dev/networking/pkg/apis/networking/v1alpha1"
 	"knative.dev/pkg/apis/duck"
+	"knative.dev/pkg/hash"
 	"knative.dev/pkg/logging"
 	pkgreconciler "knative.dev/pkg/reconciler"
 	"knative.dev/pkg/system"
@@ -132,10 +134,10 @@ func subsetEndpoints(eps *corev1.Endpoints, target string, n int) *corev1.Endpoi
 		return eps
 	}
 
-	addrs := make([]string, 0, n)
+	addrs := sets.NewString()
 	for _, ss := range eps.Subsets {
 		for _, addr := range ss.Addresses {
-			addrs = append(addrs, addr.IP)
+			addrs.Insert(addr.IP)
 		}
 	}
 
@@ -144,7 +146,7 @@ func subsetEndpoints(eps *corev1.Endpoints, target string, n int) *corev1.Endpoi
 		return eps
 	}
 
-	selection := chooseSubset(addrs, n, target)
+	selection := hash.ChooseSubset(addrs, n, target)
 
 	// Copy the informer's copy, so we can filter it out.
 	neps := eps.DeepCopy()
diff --git a/vendor/knative.dev/pkg/hash/doc.go b/vendor/knative.dev/pkg/hash/doc.go
new file mode 100644
index 000000000..36569b84a
--- /dev/null
+++ b/vendor/knative.dev/pkg/hash/doc.go
@@ -0,0 +1,22 @@
+/*
+Copyright 2020 The Knative Authors
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    https://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+// Package hash contains various Knative specific hashing utilities.
+//
+// - ChooseSubset is a consistent hashing/mapping function providing
+//   a  consistent selection of N keys from M (N<=M) keys for a given
+//   target.
+package hash
diff --git a/pkg/reconciler/serverlessservice/subsetter.go b/vendor/knative.dev/pkg/hash/hash.go
similarity index 92%
rename from pkg/reconciler/serverlessservice/subsetter.go
rename to vendor/knative.dev/pkg/hash/hash.go
index 130da4023..9de437709 100644
--- a/pkg/reconciler/serverlessservice/subsetter.go
+++ b/vendor/knative.dev/pkg/hash/hash.go
@@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package serverlessservice
+package hash
 
 // This file contains the implementation of the subsetting algorithm for
 // choosing a subset of input values in a consistent manner.
@@ -72,12 +72,14 @@ func (hd *hashData) nameForHIndex(hi int) string {
 	return hd.nameLookup[hd.hashPool[hi]]
 }
 
-func buildHashes(from []string, target string) *hashData {
+func buildHashes(in sets.String, target string) *hashData {
 	// Any one changing this function must execute
 	// `go test -run=TestOverlay -count=200`.
 	// This is to ensure there is no regression in the selection
 	// algorithm.
 
+	// Sorted list to ensure consistent results every time.
+	from := in.List()
 	// Write in two pieces, so we don't allocate temp string which is sum of both.
 	buf := bytes.NewBufferString(target)
 	buf.WriteString(startSalt)
@@ -119,14 +121,14 @@ func buildHashes(from []string, target string) *hashData {
 	return hd
 }
 
-// chooseSubset consistently chooses n items from `from`, using
+// ChooseSubset consistently chooses n items from `from`, using
 // `target` as a seed value.
-// chooseSubset is an internal function and presumes sanitized inputs.
+// ChooseSubset is an internal function and presumes sanitized inputs.
 // TODO(vagababov): once initial impl is ready, think about how to cache
 // the prepared data.
-func chooseSubset(from []string, n int, target string) sets.String {
+func ChooseSubset(from sets.String, n int, target string) sets.String {
 	if n >= len(from) {
-		return sets.NewString(from...)
+		return from
 	}
 
 	hashData := buildHashes(from, target)
diff --git a/vendor/modules.txt b/vendor/modules.txt
index 46f084e44..19fc48bd0 100644
--- a/vendor/modules.txt
+++ b/vendor/modules.txt
@@ -1081,6 +1081,7 @@ knative.dev/pkg/configmap/hash-gen
 knative.dev/pkg/configmap/testing
 knative.dev/pkg/controller
 knative.dev/pkg/hack
+knative.dev/pkg/hash
 knative.dev/pkg/injection
 knative.dev/pkg/injection/clients/dynamicclient
 knative.dev/pkg/injection/clients/dynamicclient/fake


Test patch:
diff --git a/pkg/apis/autoscaling/v1alpha1/pa_lifecycle_test.go b/pkg/apis/autoscaling/v1alpha1/pa_lifecycle_test.go
index 87b78db82..f7bd77060 100644
--- a/pkg/apis/autoscaling/v1alpha1/pa_lifecycle_test.go
+++ b/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)
 	}
 }
diff --git a/pkg/reconciler/autoscaling/hpa/hpa_test.go b/pkg/reconciler/autoscaling/hpa/hpa_test.go
index dfd4fd262..a931750dd 100644
--- a/pkg/reconciler/autoscaling/hpa/hpa_test.go
+++ b/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)
 	}
diff --git a/pkg/reconciler/autoscaling/kpa/kpa_test.go b/pkg/reconciler/autoscaling/kpa/kpa_test.go
index 6b9deb7e7..766c410b6 100644
--- a/pkg/reconciler/autoscaling/kpa/kpa_test.go
+++ b/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)),
diff --git a/pkg/reconciler/revision/revision_test.go b/pkg/reconciler/revision/revision_test.go
index 71d6bbb53..7cfe27f1d 100644
--- a/pkg/reconciler/revision/revision_test.go
+++ b/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
 }
diff --git a/pkg/reconciler/revision/table_test.go b/pkg/reconciler/revision/table_test.go
index 75153ddef..8761b318a 100644
--- a/pkg/reconciler/revision/table_test.go
+++ b/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"),
 		},
diff --git a/pkg/reconciler/serverlessservice/subsetter_test.go b/pkg/reconciler/serverlessservice/subsetter_test.go
deleted file mode 100644
index 441c3d70a..000000000
--- a/pkg/reconciler/serverlessservice/subsetter_test.go
+++ /dev/null
@@ -1,167 +0,0 @@
-/*
-Copyright 2020 The Knative Authors
-
-Licensed under the Apache License, Version 2.0 (the "License");
-you may not use this file except in compliance with the License.
-You may obtain a copy of the License at
-
-    https://www.apache.org/licenses/LICENSE-2.0
-
-Unless required by applicable law or agreed to in writing, software
-distributed under the License is distributed on an "AS IS" BASIS,
-WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-See the License for the specific language governing permissions and
-limitations under the License.
-*/
-
-package serverlessservice
-
-import (
-	"fmt"
-	"hash/fnv"
-	"math"
-	"sort"
-	"testing"
-
-	"github.com/davecgh/go-spew/spew"
-	"github.com/google/go-cmp/cmp"
-	"github.com/google/uuid"
-	"k8s.io/apimachinery/pkg/util/sets"
-)
-
-func TestBuildHashes(t *testing.T) {
-	const target = "a target to remember"
-	set := []string{"a", "b", "c", "e", "f"}
-
-	hd1 := buildHashes(set, target)
-	hd2 := buildHashes(set, target)
-	t.Log("HashData = ", spew.Sprintf("%+v", hd1))
-
-	if !cmp.Equal(hd1, hd2, cmp.AllowUnexported(hashData{})) {
-		t.Errorf("buildHashe is not consistent: diff(-want,+got):\n%s",
-			cmp.Diff(hd1, hd2, cmp.AllowUnexported(hashData{})))
-	}
-	if !sort.SliceIsSorted(hd1.hashPool, func(i, j int) bool {
-		return hd1.hashPool[i] < hd1.hashPool[j]
-	}) {
-		t.Errorf("From list is not sorted: %v", hd1.hashPool)
-	}
-}
-
-func TestChooseSubset(t *testing.T) {
-	tests := []struct {
-		name    string
-		from    []string
-		target  string
-		wantNum int
-		want    sets.String
-	}{{
-		name:    "return all",
-		from:    []string{"sun", "moon", "mars", "mercury"},
-		target:  "a target!",
-		wantNum: 4,
-		want:    sets.NewString("sun", "moon", "mars", "mercury"),
-	}, {
-		name:    "subset 1",
-		from:    []string{"sun", "moon", "mars", "mercury"},
-		target:  "a target!",
-		wantNum: 2,
-		want:    sets.NewString("mercury", "moon"),
-	}, {
-		name:    "subset 2",
-		from:    []string{"sun", "moon", "mars", "mercury"},
-		target:  "something else entirely",
-		wantNum: 2,
-		want:    sets.NewString("mercury", "mars"),
-	}, {
-		name:    "select 3",
-		from:    []string{"sun", "moon", "mars", "mercury"},
-		target:  "something else entirely",
-		wantNum: 3,
-		want:    sets.NewString("mars", "mercury", "sun"),
-	}}
-
-	for _, tc := range tests {
-		t.Run(tc.name, func(t *testing.T) {
-			got := chooseSubset(tc.from, tc.wantNum, tc.target)
-			if !got.Equal(tc.want) {
-				t.Errorf("Chose = %v, want = %v, diff(-want,+got):\n%s", got, tc.want, cmp.Diff(tc.want, got))
-			}
-		})
-	}
-}
-
-func TestCollisionHandling(t *testing.T) {
-	const (
-		key1   = "b08006d4-81f9-42ee-808b-ea18a39cbd83"
-		key2   = "c9dc8df4-8c8d-4077-8750-6d2c2113a23b"
-		target = "e68a64e1-19d8-4855-9ffa-04f49223a059"
-	)
-	// Verify baseline, that they collide.
-	hasher := fnv.New64a()
-	h1 := computeHash([]byte(key1+target), hasher) % universe
-	hasher.Reset()
-	h2 := computeHash([]byte(key2+target), hasher) % universe
-	if h1 != h2 {
-		t.Fatalf("Baseline incorrect keys don't collide %d != %d", h1, h2)
-	}
-	hd := buildHashes([]string{key1, key2}, target)
-	if got, want := len(hd.nameLookup), 2; got != want {
-		t.Error("Did not resolve collision, only 1 key in the map")
-	}
-}
-
-func TestOverlay(t *testing.T) {
-	// Execute
-	// `go test -run=TestOverlay -count=200`
-	// To ensure assignments are still not skewed.
-	const (
-		sources   = 50
-		samples   = 30000
-		selection = 10
-		want      = samples * selection / sources
-		threshold = want / 5 // 20%
-	)
-	from := make([]string, sources)
-	for i := 0; i < sources; i++ {
-		from[i] = uuid.New().String()
-	}
-	freqs := make(map[string]int, sources)
-
-	for i := 0; i < samples; i++ {
-		target := uuid.New().String()
-		got := chooseSubset(from, selection, target)
-		for k := range got {
-			freqs[k]++
-		}
-	}
-
-	totalDiff := 0.
-	for _, v := range freqs {
-		diff := float64(v - want)
-		adiff := math.Abs(diff)
-		totalDiff += adiff
-		if adiff > threshold {
-			t.Errorf("Diff for %d is %v, larger than threshold: %d", v, diff, threshold)
-		}
-	}
-	t.Log(totalDiff / float64(len(freqs)))
-}
-
-func BenchmarkSelection(b *testing.B) {
-	const maxSet = 200
-	from := make([]string, maxSet)
-	for i := 0; i < maxSet; i++ {
-		from[i] = uuid.New().String()
-	}
-	for _, v := range []int{5, 10, 25, 50, 100, 150, maxSet} {
-		for _, ss := range []int{1, 5, 10, 15, 20, 25} {
-			b.Run(fmt.Sprintf("pool-%d-subset-%d", v, ss), func(b *testing.B) {
-				target := uuid.New().String()
-				for i := 0; i < b.N; i++ {
-					chooseSubset(from[:v], 10, target)
-				}
-			})
-		}
-	}
-}
diff --git a/pkg/testing/functional.go b/pkg/testing/functional.go
index 4a9103f6a..2b8ef3fa4 100644
--- a/pkg/testing/functional.go
+++ b/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) {