Skip to content

Commit

Permalink
Add PA condition InitiallyActive (#8354)
Browse files Browse the repository at this point in the history 
* Add PodAutoscaler condition for initial scale

* Set initial active condition

* Use PA status annotation instead of condition

* Rename MarkInitiallyActive -> MarkHasBeenActive
  • Loading branch information
Tara Gu authored Jun 18, 2020
1 parent 5f2c995 commit 7be0f43
Show file tree
Hide file tree
Showing 4 changed files with 55 additions and 17 deletions.
18 changes: 18 additions & 0 deletions pkg/apis/autoscaling/v1alpha1/pa_lifecycle.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ import (
"knative.dev/serving/pkg/apis/autoscaling"
)

const hasBeenActiveAnnotation = "HasBeenActive"

var podCondSet = apis.NewLivingConditionSet(
PodAutoscalerConditionActive,
)
Expand Down Expand Up @@ -172,6 +174,22 @@ 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
}

// 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"
}

// GetCondition gets the condition `t`.
func (pas *PodAutoscalerStatus) GetCondition(t apis.ConditionType) *apis.Condition {
return podCondSet.Manage(pas).GetCondition(t)
Expand Down
11 changes: 11 additions & 0 deletions pkg/apis/autoscaling/v1alpha1/pa_lifecycle_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1171,3 +1171,14 @@ func TestInitialScale(t *testing.T) {
})
}
}

func TestHasBeenActive(t *testing.T) {
p := PodAutoscaler{}
if got, want := p.Status.HasBeenActive(), 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 {
t.Errorf("after marking initially active: got: %v, want: %v", got, want)
}
}
1 change: 1 addition & 0 deletions pkg/reconciler/autoscaling/kpa/kpa.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -259,6 +259,7 @@ func computeActiveCondition(pa *pav1alpha1.PodAutoscaler, pc podCounts) {

case pc.ready >= minReady:
if pc.want > 0 || !pa.Status.IsInactive() {
pa.Status.MarkHasBeenActive()
// SKS should already be active.
pa.Status.MarkActive()
}
Expand Down
42 changes: 25 additions & 17 deletions pkg/reconciler/autoscaling/kpa/kpa_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,10 @@ func markInactive(pa *asv1a1.PodAutoscaler) {
pa.Status.MarkInactive("NoTraffic", "The target is not receiving traffic.")
}

func markHasBeenActive(pa *asv1a1.PodAutoscaler) {
pa.Status.MarkHasBeenActive()
}

func kpa(ns, n string, opts ...PodAutoscalerOption) *asv1a1.PodAutoscaler {
rev := newTestRevision(ns, n)
kpa := revisionresources.MakePA(rev)
Expand Down Expand Up @@ -248,16 +252,20 @@ func TestReconcile(t *testing.T) {
WithObservedGeneration(1),
)
}
activatingKPAMinScale := func(g int32) *asv1a1.PodAutoscaler {
return kpa(
activatingKPAMinScale := func(g int32, opts ...PodAutoscalerOption) *asv1a1.PodAutoscaler {
kpa := kpa(
testNamespace, testRevision, markActivating, withScales(g, defaultScale), WithReachabilityReachable,
withMinScale(defaultScale), WithPAStatusService(testRevision), WithPAMetricsService(privateSvc),
WithObservedGeneration(1),
)
for _, opt := range opts {
opt(kpa)
}
return kpa
}
activeKPAMinScale := func(g, w int32) *asv1a1.PodAutoscaler {
return kpa(
testNamespace, testRevision, markActive, withScales(g, w), WithReachabilityReachable,
testNamespace, testRevision, markActive, markHasBeenActive, withScales(g, w), WithReachabilityReachable,
withMinScale(defaultScale), WithPAStatusService(testRevision), WithPAMetricsService(privateSvc),
WithObservedGeneration(1),
)
Expand Down Expand Up @@ -293,7 +301,7 @@ func TestReconcile(t *testing.T) {
Name: "steady state",
Key: key,
Objects: []runtime.Object{
kpa(testNamespace, testRevision, markActive, WithPAMetricsService(privateSvc),
kpa(testNamespace, testRevision, markActive, markHasBeenActive, WithPAMetricsService(privateSvc),
withScales(1, defaultScale), WithPAStatusService(testRevision), WithObservedGeneration(1)),
sks(testNamespace, testRevision, WithDeployRef(deployName), WithSKSReady),
metric(testNamespace, testRevision),
Expand Down Expand Up @@ -375,14 +383,14 @@ func TestReconcile(t *testing.T) {
Name: "create metric",
Key: key,
Objects: []runtime.Object{
kpa(testNamespace, testRevision, markActive,
kpa(testNamespace, testRevision, markActive, markHasBeenActive,
withScales(1, defaultScale), WithPAStatusService(testRevision)),
defaultSKS,
defaultDeployment,
defaultEndpoints,
},
WantStatusUpdates: []clientgotesting.UpdateActionImpl{{
Object: kpa(testNamespace, testRevision, markActive, withScales(1, defaultScale),
Object: kpa(testNamespace, testRevision, markActive, markHasBeenActive, withScales(1, defaultScale),
WithPAStatusService(testRevision), WithPAMetricsService(privateSvc), WithObservedGeneration(1)),
}},
WantCreates: []runtime.Object{
Expand All @@ -392,7 +400,7 @@ func TestReconcile(t *testing.T) {
Name: "scale up deployment",
Key: key,
Objects: []runtime.Object{
kpa(testNamespace, testRevision, markActive, WithPAMetricsService(privateSvc),
kpa(testNamespace, testRevision, markActive, markHasBeenActive, WithPAMetricsService(privateSvc),
withScales(1, defaultScale), WithPAStatusService(testRevision), WithObservedGeneration(1)),
defaultSKS,
metric(testNamespace, testRevision),
Expand Down Expand Up @@ -493,7 +501,7 @@ func TestReconcile(t *testing.T) {
defaultDeployment, defaultEndpoints,
},
WantStatusUpdates: []clientgotesting.UpdateActionImpl{{
Object: kpa(testNamespace, testRevision, markActive, WithPAStatusService(testRevision),
Object: kpa(testNamespace, testRevision, markActive, markHasBeenActive, WithPAStatusService(testRevision),
WithPAMetricsService(privateSvc), withScales(1, defaultScale), WithObservedGeneration(1)),
}},
}, {
Expand Down Expand Up @@ -537,7 +545,7 @@ func TestReconcile(t *testing.T) {
defaultDeployment, defaultEndpoints,
},
WantStatusUpdates: []clientgotesting.UpdateActionImpl{{
Object: kpa(testNamespace, testRevision, markActive, withMinScale(2), WithPAMetricsService(privateSvc),
Object: kpa(testNamespace, testRevision, markActive, markHasBeenActive, withMinScale(2), WithPAMetricsService(privateSvc),
withScales(1, defaultScale), WithPAStatusService(testRevision), WithReachabilityUnreachable,
WithObservedGeneration(1)),
}},
Expand All @@ -553,7 +561,7 @@ func TestReconcile(t *testing.T) {
makeSKSPrivateEndpoints(2, testNamespace, testRevision),
},
WantStatusUpdates: []clientgotesting.UpdateActionImpl{{
Object: kpa(testNamespace, testRevision, markActive, withMinScale(2), WithPAMetricsService(privateSvc),
Object: kpa(testNamespace, testRevision, markActive, markHasBeenActive, withMinScale(2), WithPAMetricsService(privateSvc),
withScales(2, defaultScale), WithPAStatusService(testRevision), WithReachabilityReachable,
WithObservedGeneration(1)),
}},
Expand All @@ -569,7 +577,7 @@ func TestReconcile(t *testing.T) {
makeSKSPrivateEndpoints(2, testNamespace, testRevision),
},
WantStatusUpdates: []clientgotesting.UpdateActionImpl{{
Object: kpa(testNamespace, testRevision, markActive, withMinScale(2), WithPAMetricsService(privateSvc),
Object: kpa(testNamespace, testRevision, markActive, markHasBeenActive, withMinScale(2), WithPAMetricsService(privateSvc),
withScales(2, defaultScale), WithPAStatusService(testRevision), WithReachabilityUnknown,
WithObservedGeneration(1)),
}},
Expand Down Expand Up @@ -748,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, withScales(1, 1),
kpa(testNamespace, testRevision, markActive, markHasBeenActive, withScales(1, 1),
WithPAStatusService(testRevision), WithPAMetricsService(privateSvc), WithObservedGeneration(1)),
defaultSKS,
metric(testNamespace, testRevision),
Expand Down Expand Up @@ -845,7 +853,7 @@ func TestReconcile(t *testing.T) {
minScalePatch,
},
WantStatusUpdates: []clientgotesting.UpdateActionImpl{{
Object: activatingKPAMinScale(underscale),
Object: activatingKPAMinScale(underscale, markHasBeenActive),
}},
}, {
// Scale to `minScale` and mark PA "active"
Expand Down Expand Up @@ -935,7 +943,7 @@ func TestReconcile(t *testing.T) {
decider(testNamespace, testRevision, defaultScale, /* desiredScale */
-42 /* ebc */, scaling.MinActivators)),
Objects: []runtime.Object{
kpa(testNamespace, testRevision, markActive, WithPAMetricsService(privateSvc),
kpa(testNamespace, testRevision, markActive, markHasBeenActive, WithPAMetricsService(privateSvc),
withScales(1, defaultScale), WithPAStatusService(testRevision), WithObservedGeneration(1)),
sks(testNamespace, testRevision, WithDeployRef(deployName), WithProxyMode, WithSKSReady),
metric(testNamespace, testRevision),
Expand All @@ -948,7 +956,7 @@ func TestReconcile(t *testing.T) {
decider(testNamespace, testRevision, defaultScale, /* desiredScale */
-42 /* ebc */, 1982 /*numActivators*/)),
Objects: []runtime.Object{
kpa(testNamespace, testRevision, markActive, WithPAMetricsService(privateSvc),
kpa(testNamespace, testRevision, markActive, markHasBeenActive, WithPAMetricsService(privateSvc),
withScales(1, defaultScale), WithPAStatusService(testRevision), WithObservedGeneration(1)),
sks(testNamespace, testRevision, WithDeployRef(deployName),
WithProxyMode, WithSKSReady, WithNumActivators(4)),
Expand All @@ -966,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, WithPAMetricsService(privateSvc),
kpa(testNamespace, testRevision, markActive, markHasBeenActive, WithPAMetricsService(privateSvc),
withScales(1, defaultScale), WithPAStatusService(testRevision), WithObservedGeneration(1)),
sks(testNamespace, testRevision, WithDeployRef(deployName), WithSKSReady,
WithNumActivators(2)),
Expand All @@ -984,7 +992,7 @@ func TestReconcile(t *testing.T) {
decider(testNamespace, testRevision, defaultScale, /* desiredScale */
1 /* ebc */, scaling.MinActivators)),
Objects: []runtime.Object{
kpa(testNamespace, testRevision, markActive, WithPAMetricsService(privateSvc),
kpa(testNamespace, testRevision, markActive, markHasBeenActive, WithPAMetricsService(privateSvc),
withScales(1, defaultScale), WithPAStatusService(testRevision), WithObservedGeneration(1)),
sks(testNamespace, testRevision, WithDeployRef(deployName), WithSKSReady, WithProxyMode,
WithNumActivators(3)),
Expand Down

0 comments on commit 7be0f43

Please sign in to comment.