From 1642dd12db11c65a63e0abfac9181f1dde4721a8 Mon Sep 17 00:00:00 2001 From: Victor Agababov Date: Sun, 26 Jul 2020 13:49:59 -0700 Subject: [PATCH] Add new KPA status for dependencies (#8809) * Add new Kpa status for dependencies This initial version only deals with SKS and sets deps not ready if SKS is not ready and ready otherwise. Lots of test churn... :-) * more files to fix * review * restore tests * rewrite as sks only cond * holdout * more fixes --- pkg/apis/autoscaling/v1alpha1/pa_lifecycle.go | 13 +- .../autoscaling/v1alpha1/pa_lifecycle_test.go | 24 ++- pkg/apis/autoscaling/v1alpha1/pa_types.go | 2 + pkg/reconciler/autoscaling/kpa/kpa.go | 8 +- pkg/reconciler/autoscaling/kpa/kpa_test.go | 153 +++++++++++------- pkg/reconciler/revision/table_test.go | 12 +- pkg/testing/functional.go | 12 ++ 7 files changed, 157 insertions(+), 67 deletions(-) diff --git a/pkg/apis/autoscaling/v1alpha1/pa_lifecycle.go b/pkg/apis/autoscaling/v1alpha1/pa_lifecycle.go index 2d424874f27e..cc63731e8697 100644 --- a/pkg/apis/autoscaling/v1alpha1/pa_lifecycle.go +++ b/pkg/apis/autoscaling/v1alpha1/pa_lifecycle.go @@ -31,6 +31,7 @@ import ( var podCondSet = apis.NewLivingConditionSet( PodAutoscalerConditionActive, PodAutoscalerConditionScaleTargetInitialized, + PodAutoscalerSKSReady, ) // GetConditionSet retrieves the condition set for this resource. Implements the KRShaped interface. @@ -185,6 +186,16 @@ func (pas *PodAutoscalerStatus) MarkScaleTargetInitialized() { podCondSet.Manage(pas).MarkTrue(PodAutoscalerConditionScaleTargetInitialized) } +// MarkSKSReady marks the PA condition denoting that SKS is ready. +func (pas *PodAutoscalerStatus) MarkSKSReady() { + podCondSet.Manage(pas).MarkTrue(PodAutoscalerSKSReady) +} + +// MarkSKSNotReady marks the PA condation that SKS is not yet ready. +func (pas *PodAutoscalerStatus) MarkSKSNotReady(mes string) { + podCondSet.Manage(pas).MarkUnknown(PodAutoscalerSKSReady, "NotReady", mes) +} + // GetCondition gets the condition `t`. func (pas *PodAutoscalerStatus) GetCondition(t apis.ConditionType) *apis.Condition { return podCondSet.Manage(pas).GetCondition(t) @@ -248,7 +259,7 @@ func (pas *PodAutoscalerStatus) CanFailActivation(now time.Time, idlePeriod time // inStatusFor returns positive duration if the PodAutoscalerStatus's Active condition has stayed in // the specified status for at least the specified duration. Otherwise it returns negative duration, -// including when the status is undetermined (Active condition is not found.) +// including when the status is undetermined. func (pas *PodAutoscalerStatus) inStatusFor(status corev1.ConditionStatus, now time.Time, dur time.Duration) time.Duration { cond := pas.GetCondition(PodAutoscalerConditionActive) if cond == nil || cond.Status != status { diff --git a/pkg/apis/autoscaling/v1alpha1/pa_lifecycle_test.go b/pkg/apis/autoscaling/v1alpha1/pa_lifecycle_test.go index f7bd77060b68..1fcca085bfe5 100644 --- a/pkg/apis/autoscaling/v1alpha1/pa_lifecycle_test.go +++ b/pkg/apis/autoscaling/v1alpha1/pa_lifecycle_test.go @@ -251,6 +251,7 @@ func TestInactiveFor(t *testing.T) { }) } } + func TestActiveFor(t *testing.T) { now := time.Now() cases := []struct { @@ -977,21 +978,41 @@ func TestTypicalFlow(t *testing.T) { r.InitializeConditions() apistest.CheckConditionOngoing(r, PodAutoscalerConditionActive, t) apistest.CheckConditionOngoing(r, PodAutoscalerConditionReady, t) + apistest.CheckConditionOngoing(r, PodAutoscalerSKSReady, t) + apistest.CheckConditionOngoing(r, PodAutoscalerConditionScaleTargetInitialized, t) + + r.MarkActive() + r.MarkSKSReady() + apistest.CheckConditionSucceeded(r, PodAutoscalerSKSReady, t) + apistest.CheckConditionSucceeded(r, PodAutoscalerConditionActive, t) + apistest.CheckConditionOngoing(r, PodAutoscalerConditionReady, t) // When we see traffic, mark ourselves active. r.MarkActive() r.MarkScaleTargetInitialized() apistest.CheckConditionSucceeded(r, PodAutoscalerConditionScaleTargetInitialized, t) + apistest.CheckConditionSucceeded(r, PodAutoscalerSKSReady, t) apistest.CheckConditionSucceeded(r, PodAutoscalerConditionActive, t) apistest.CheckConditionSucceeded(r, PodAutoscalerConditionReady, t) // Check idempotency. r.MarkActive() r.MarkScaleTargetInitialized() + r.MarkSKSReady() apistest.CheckConditionSucceeded(r, PodAutoscalerConditionScaleTargetInitialized, t) apistest.CheckConditionSucceeded(r, PodAutoscalerConditionActive, t) + apistest.CheckConditionSucceeded(r, PodAutoscalerSKSReady, t) apistest.CheckConditionSucceeded(r, PodAutoscalerConditionReady, t) + r.MarkSKSNotReady("something something private") + apistest.CheckConditionSucceeded(r, PodAutoscalerConditionScaleTargetInitialized, t) + apistest.CheckConditionSucceeded(r, PodAutoscalerConditionActive, t) + apistest.CheckConditionOngoing(r, PodAutoscalerSKSReady, t) + apistest.CheckConditionOngoing(r, PodAutoscalerConditionReady, t) + + // Restore. + r.MarkSKSReady() + // When we stop seeing traffic, mark ourselves inactive. r.MarkInactive("TheReason", "the message") apistest.CheckConditionSucceeded(r, PodAutoscalerConditionScaleTargetInitialized, t) @@ -1014,9 +1035,10 @@ 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, PodAutoscalerSKSReady, t) apistest.CheckConditionSucceeded(r, PodAutoscalerConditionReady, t) + apistest.CheckConditionSucceeded(r, PodAutoscalerConditionScaleTargetInitialized, t) } func TestTargetBC(t *testing.T) { diff --git a/pkg/apis/autoscaling/v1alpha1/pa_types.go b/pkg/apis/autoscaling/v1alpha1/pa_types.go index 163189ecaf7d..ac8dd277b854 100644 --- a/pkg/apis/autoscaling/v1alpha1/pa_types.go +++ b/pkg/apis/autoscaling/v1alpha1/pa_types.go @@ -117,6 +117,8 @@ const ( PodAutoscalerConditionScaleTargetInitialized apis.ConditionType = "ScaleTargetInitialized" // PodAutoscalerConditionActive is set when the PodAutoscaler's ScaleTargetRef is receiving traffic. PodAutoscalerConditionActive apis.ConditionType = "Active" + // PodAutoscalerCondtionDependenciesReady is set when SKS is ready. + PodAutoscalerSKSReady = "SKSReady" ) // PodAutoscalerStatus communicates the observed state of the PodAutoscaler (from the controller). diff --git a/pkg/reconciler/autoscaling/kpa/kpa.go b/pkg/reconciler/autoscaling/kpa/kpa.go index 1ab38fd4e80f..3a487578d9b3 100644 --- a/pkg/reconciler/autoscaling/kpa/kpa.go +++ b/pkg/reconciler/autoscaling/kpa/kpa.go @@ -79,12 +79,13 @@ func (c *Reconciler) ReconcileKind(ctx context.Context, pa *pav1alpha1.PodAutosc } // Having an SKS and its PrivateServiceName is a prerequisite for all upcoming steps. - if sks == nil || (sks != nil && sks.Status.PrivateServiceName == "") { + if sks == nil || sks.Status.PrivateServiceName == "" { // Before we can reconcile decider and get real number of activators // we start with default of 2. if _, err = c.ReconcileSKS(ctx, pa, nv1alpha1.SKSOperationModeServe, 0 /*numActivators == all*/); err != nil { return fmt.Errorf("error reconciling SKS: %w", err) } + pa.Status.MarkSKSNotReady("No Private Service Name") // In both cases this is true. return computeStatus(ctx, pa, podCounts{want: scaleUnknown}, logger) } @@ -138,7 +139,12 @@ func (c *Reconciler) ReconcileKind(ctx context.Context, pa *pav1alpha1.PodAutosc // If SKS is not ready — ensure we're not becoming ready. // TODO: see if we can perhaps propagate the SKS state to computing active status. if !sks.IsReady() { + logger.Debug("SKS is not ready, marking SKS status not ready") ready = 0 + pa.Status.MarkSKSNotReady(sks.Status.GetCondition(nv1alpha1.ServerlessServiceConditionReady).Message) + } else { + logger.Debug("SKS is ready, marking SKS status ready") + pa.Status.MarkSKSReady() } logger.Infof("PA scale got=%d, want=%d, desiredPods=%d ebc=%d", ready, want, diff --git a/pkg/reconciler/autoscaling/kpa/kpa_test.go b/pkg/reconciler/autoscaling/kpa/kpa_test.go index e0ed53cca33d..49f88f6f7a7c 100644 --- a/pkg/reconciler/autoscaling/kpa/kpa_test.go +++ b/pkg/reconciler/autoscaling/kpa/kpa_test.go @@ -58,6 +58,7 @@ import ( "golang.org/x/sync/errgroup" nv1a1 "knative.dev/networking/pkg/apis/networking/v1alpha1" + duckv1 "knative.dev/pkg/apis/duck/v1" "knative.dev/pkg/configmap" "knative.dev/pkg/controller" "knative.dev/pkg/kmeta" @@ -161,9 +162,14 @@ func metric(ns, n string, opts ...metricOption) *asv1a1.Metric { return m } +func sksNoConds(s *nv1a1.ServerlessService) { + s.Status.Status = duckv1.Status{} +} + func sks(ns, n string, so ...SKSOption) *nv1a1.ServerlessService { kpa := kpa(ns, n) s := aresources.MakeSKS(kpa, nv1a1.SKSOperationModeServe, scaling.MinActivators) + s.Status.InitializeConditions() for _, opt := range so { opt(s) } @@ -241,14 +247,16 @@ func TestReconcile(t *testing.T) { inactiveKPAMinScale := func(g int32) *asv1a1.PodAutoscaler { return kpa( - testNamespace, testRevision, markInactive, withScales(g, unknownScale), WithReachabilityReachable, - withMinScale(defaultScale), WithPAStatusService(testRevision), WithPAMetricsService(privateSvc), - WithObservedGeneration(1), + testNamespace, testRevision, WithPASKSNotReady(""), + markInactive, withScales(g, unknownScale), + WithReachabilityReachable, withMinScale(defaultScale), WithPAStatusService(testRevision), + WithPAMetricsService(privateSvc), WithObservedGeneration(1), ) } activatingKPAMinScale := func(g int32, opts ...PodAutoscalerOption) *asv1a1.PodAutoscaler { kpa := kpa( - testNamespace, testRevision, markActivating, withScales(g, defaultScale), WithReachabilityReachable, + testNamespace, testRevision, WithPASKSNotReady(""), + markActivating, withScales(g, defaultScale), WithReachabilityReachable, withMinScale(defaultScale), WithPAStatusService(testRevision), WithPAMetricsService(privateSvc), WithObservedGeneration(1), ) @@ -259,7 +267,8 @@ func TestReconcile(t *testing.T) { } activeKPAMinScale := func(g, w int32) *asv1a1.PodAutoscaler { return kpa( - testNamespace, testRevision, markActive, markScaleTargetInitialized, withScales(g, w), WithReachabilityReachable, + testNamespace, testRevision, WithPASKSReady, markActive, markScaleTargetInitialized, + withScales(g, w), WithReachabilityReachable, withMinScale(defaultScale), WithPAStatusService(testRevision), WithPAMetricsService(privateSvc), WithObservedGeneration(1), ) @@ -294,7 +303,8 @@ func TestReconcile(t *testing.T) { Name: "steady state", Key: key, Objects: append([]runtime.Object{ - kpa(testNamespace, testRevision, markActive, markScaleTargetInitialized, WithPAMetricsService(privateSvc), + kpa(testNamespace, testRevision, WithPASKSReady, markActive, + markScaleTargetInitialized, WithPAMetricsService(privateSvc), withScales(1, defaultScale), WithPAStatusService(testRevision), WithObservedGeneration(1)), sks(testNamespace, testRevision, WithDeployRef(deployName), WithSKSReady), metric(testNamespace, testRevision), @@ -320,12 +330,12 @@ func TestReconcile(t *testing.T) { }, WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ Object: kpa(testNamespace, testRevision, markActive, - markScaleTargetInitialized, + markScaleTargetInitialized, WithPASKSReady, WithPAMetricsService(privateSvc), withScales(1, defaultScale), WithPAStatusService(testRevision), WithObservedGeneration(1)), }, { Object: kpa(testNamespace, testRevision, markActive, - markScaleTargetInitialized, + markScaleTargetInitialized, WithPASKSReady, WithPAMetricsService(privateSvc), withScales(1, defaultScale), WithPAStatusService(testRevision), WithObservedGeneration(1)), }}, @@ -378,8 +388,9 @@ func TestReconcile(t *testing.T) { defaultSKS, defaultDeployment, }, defaultReady...), WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ - Object: kpa(testNamespace, testRevision, markActive, markScaleTargetInitialized, withScales(1, defaultScale), - WithPAStatusService(testRevision), WithPAMetricsService(privateSvc), WithObservedGeneration(1)), + Object: kpa(testNamespace, testRevision, markActive, markScaleTargetInitialized, + withScales(1, defaultScale), WithPASKSReady, WithPAStatusService(testRevision), + WithPAMetricsService(privateSvc), WithObservedGeneration(1)), }}, WantCreates: []runtime.Object{ metric(testNamespace, testRevision), @@ -388,8 +399,9 @@ func TestReconcile(t *testing.T) { Name: "scale up deployment", Key: key, Objects: append([]runtime.Object{ - kpa(testNamespace, testRevision, markActive, markScaleTargetInitialized, WithPAMetricsService(privateSvc), - withScales(1, defaultScale), WithPAStatusService(testRevision), WithObservedGeneration(1)), + kpa(testNamespace, testRevision, WithPASKSReady, markActive, markScaleTargetInitialized, + WithPAMetricsService(privateSvc), withScales(1, defaultScale), WithPAStatusService(testRevision), + WithObservedGeneration(1)), defaultSKS, metric(testNamespace, testRevision), deploy(testNamespace, testRevision), @@ -443,7 +455,8 @@ func TestReconcile(t *testing.T) { }}, WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ Object: kpa(testNamespace, testRevision, markActivating, withScales(0, defaultScale), - WithPAMetricsService(privateSvc), WithPAStatusService(testRevision), WithObservedGeneration(1)), + WithPASKSReady, WithPAMetricsService(privateSvc), + WithPAStatusService(testRevision), WithObservedGeneration(1)), }}, }, { Name: "sks is still not ready", @@ -457,21 +470,23 @@ func TestReconcile(t *testing.T) { defaultDeployment, }, defaultReady...), WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ - Object: kpa(testNamespace, testRevision, markActivating, withScales(0, defaultScale), + Object: kpa(testNamespace, testRevision, withScales(0, defaultScale), + WithPASKSNotReady(""), markActivating, WithPAMetricsService(privateSvc), WithPAStatusService(testRevision), WithObservedGeneration(1)), }}, }, { Name: "sks becomes ready", Key: key, Objects: append([]runtime.Object{ - kpa(testNamespace, testRevision, WithPAMetricsService(privateSvc)), - defaultSKS, - metric(testNamespace, testRevision), - defaultDeployment, - }, defaultReady...), + kpa(testNamespace, testRevision, WithPASKSNotReady(""), + markActivating, WithPAMetricsService(privateSvc), withScales(0, unknownScale), + WithObservedGeneration(1)), + defaultSKS, defaultDeployment, metric(testNamespace, testRevision), + }), WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ - Object: kpa(testNamespace, testRevision, markActive, markScaleTargetInitialized, WithPAStatusService(testRevision), - WithPAMetricsService(privateSvc), withScales(1, defaultScale), WithObservedGeneration(1)), + Object: kpa(testNamespace, testRevision, WithPASKSReady, WithPAStatusService(testRevision), + markActivating, WithPAMetricsService(privateSvc), withScales(0, defaultScale), + WithObservedGeneration(1)), }}, }, { Name: "kpa does not become ready without minScale endpoints when reachable", @@ -484,7 +499,8 @@ func TestReconcile(t *testing.T) { defaultDeployment, }, defaultReady...), WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ - Object: kpa(testNamespace, testRevision, markActivating, withMinScale(2), WithPAMetricsService(privateSvc), + Object: kpa(testNamespace, testRevision, WithPASKSReady, + markActivating, withMinScale(2), WithPAMetricsService(privateSvc), withScales(1, defaultScale), WithPAStatusService(testRevision), WithReachabilityReachable, WithObservedGeneration(1)), }}, @@ -499,7 +515,8 @@ func TestReconcile(t *testing.T) { defaultDeployment, }, defaultReady...), WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ - Object: kpa(testNamespace, testRevision, markActivating, withMinScale(2), WithPAMetricsService(privateSvc), + Object: kpa(testNamespace, testRevision, WithPASKSReady, + markActivating, withMinScale(2), WithPAMetricsService(privateSvc), withScales(1, defaultScale), WithPAStatusService(testRevision), WithReachabilityUnknown, WithObservedGeneration(1)), }}, @@ -514,7 +531,8 @@ func TestReconcile(t *testing.T) { defaultDeployment, }, defaultReady...), WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ - Object: kpa(testNamespace, testRevision, markActive, markScaleTargetInitialized, withMinScale(2), WithPAMetricsService(privateSvc), + Object: kpa(testNamespace, testRevision, WithPASKSReady, + markActive, markScaleTargetInitialized, withMinScale(2), WithPAMetricsService(privateSvc), withScales(1, defaultScale), WithPAStatusService(testRevision), WithReachabilityUnreachable, WithObservedGeneration(1)), }}, @@ -529,7 +547,8 @@ func TestReconcile(t *testing.T) { defaultDeployment, }, makeReadyPods(2, testNamespace, testRevision)...), WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ - Object: kpa(testNamespace, testRevision, markActive, markScaleTargetInitialized, withMinScale(2), WithPAMetricsService(privateSvc), + Object: kpa(testNamespace, testRevision, WithPASKSReady, + markActive, markScaleTargetInitialized, withMinScale(2), WithPAMetricsService(privateSvc), withScales(2, defaultScale), WithPAStatusService(testRevision), WithReachabilityReachable, WithObservedGeneration(1)), }}, @@ -544,7 +563,8 @@ func TestReconcile(t *testing.T) { defaultDeployment, }, makeReadyPods(2, testNamespace, testRevision)...), WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ - Object: kpa(testNamespace, testRevision, markActive, markScaleTargetInitialized, withMinScale(2), WithPAMetricsService(privateSvc), + Object: kpa(testNamespace, testRevision, WithPASKSReady, + markActive, markScaleTargetInitialized, withMinScale(2), WithPAMetricsService(privateSvc), withScales(2, defaultScale), WithPAStatusService(testRevision), WithReachabilityUnknown, WithObservedGeneration(1)), }}, @@ -557,26 +577,28 @@ 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, markActivating, WithPAMetricsService(privateSvc), withScales(0, unknownScale), + Object: kpa(testNamespace, testRevision, WithPASKSNotReady("No Private Service Name"), + markActivating, WithPAMetricsService(privateSvc), withScales(0, unknownScale), WithObservedGeneration(1)), }}, WantCreates: []runtime.Object{ - sks(testNamespace, testRevision, WithDeployRef(deployName), WithNumActivators(0)), + sks(testNamespace, testRevision, WithDeployRef(deployName), WithNumActivators(0), sksNoConds), }, }, { Name: "sks is out of whack", Key: key, Objects: []runtime.Object{ - kpa(testNamespace, testRevision, withScales(0, defaultScale), WithPAMetricsService(privateSvc), markActive), + kpa(testNamespace, testRevision, WithPASKSReady, + withScales(0, defaultScale), WithPAMetricsService(privateSvc), markActive), sks(testNamespace, testRevision, WithDeployRef("bar"), WithPubService, WithPrivateService), metric(testNamespace, testRevision), defaultDeployment, }, WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ // SKS just got updated and we don't have up to date status. - Object: kpa(testNamespace, testRevision, markActivating, - withScales(0, defaultScale), WithPAStatusService(testRevision), WithPAMetricsService(privateSvc), - WithObservedGeneration(1)), + Object: kpa(testNamespace, testRevision, WithPASKSNotReady(""), + markActivating, withScales(0, defaultScale), WithPAStatusService(testRevision), + WithPAMetricsService(privateSvc), WithObservedGeneration(1)), }}, WantUpdates: []clientgotesting.UpdateActionImpl{{ Object: sks(testNamespace, testRevision, WithPubService, WithPrivateService, @@ -595,7 +617,7 @@ func TestReconcile(t *testing.T) { }, WantErr: true, WantCreates: []runtime.Object{ - sks(testNamespace, testRevision, WithDeployRef(deployName), WithNumActivators(0)), + sks(testNamespace, testRevision, WithDeployRef(deployName), WithNumActivators(0), sksNoConds), }, WantEvents: []string{ Eventf(corev1.EventTypeWarning, "InternalError", @@ -663,7 +685,8 @@ func TestReconcile(t *testing.T) { Objects: []runtime.Object{ kpa(testNamespace, testRevision, withScales(0, 0), WithNoTraffic("NoTraffic", "The target is not receiving traffic."), - markOld, WithPAStatusService(testRevision), WithPAMetricsService(privateSvc), WithObservedGeneration(1)), + WithPASKSReady, markOld, WithPAStatusService(testRevision), + WithPAMetricsService(privateSvc), WithObservedGeneration(1)), sks(testNamespace, testRevision, WithDeployRef(deployName), WithProxyMode, WithSKSReady), metric(testNamespace, testRevision), deploy(testNamespace, testRevision, func(d *appsv1.Deployment) { @@ -678,7 +701,8 @@ func TestReconcile(t *testing.T) { Objects: []runtime.Object{ kpa(testNamespace, testRevision, withScales(0, 0), WithNoTraffic("NoTraffic", "The target is not receiving traffic."), - markOld, WithPAStatusService(testRevision), WithPAMetricsService(privateSvc), WithObservedGeneration(1)), + WithPASKSReady, markOld, WithPAStatusService(testRevision), + WithPAMetricsService(privateSvc), WithObservedGeneration(1)), sks(testNamespace, testRevision, WithDeployRef(deployName), WithProxyMode, WithSKSReady), metric(testNamespace, testRevision), deploy(testNamespace, testRevision), @@ -696,15 +720,15 @@ func TestReconcile(t *testing.T) { Ctx: context.WithValue(context.Background(), deciderKey, decider(testNamespace, testRevision, 0 /* desiredScale */, 0 /* ebc */, scaling.MinActivators)), Objects: append([]runtime.Object{ - kpa(testNamespace, testRevision, markActive, markOld, withScales(0, 0), - WithPAStatusService(testRevision), WithPAMetricsService(privateSvc)), + kpa(testNamespace, testRevision, WithPASKSReady, markActive, markOld, + withScales(0, 0), WithPAStatusService(testRevision), WithPAMetricsService(privateSvc)), defaultSKS, metric(testNamespace, testRevision), deploy(testNamespace, testRevision), }, defaultReady...), WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ Object: kpa(testNamespace, testRevision, withScales(1, 0), - WithPAMetricsService(privateSvc), + WithPASKSReady, WithPAMetricsService(privateSvc), WithNoTraffic("NoTraffic", "The target is not receiving traffic."), WithPAStatusService(testRevision), WithPAMetricsService(privateSvc), WithObservedGeneration(1)), @@ -719,7 +743,8 @@ func TestReconcile(t *testing.T) { Ctx: context.WithValue(context.Background(), deciderKey, decider(testNamespace, testRevision, 0 /* desiredScale */, 0 /* ebc */, scaling.MinActivators)), Objects: append([]runtime.Object{ - kpa(testNamespace, testRevision, markActive, markScaleTargetInitialized, withScales(1, 1), + kpa(testNamespace, testRevision, WithPASKSReady, markActive, + markScaleTargetInitialized, withScales(1, 1), WithPAStatusService(testRevision), WithPAMetricsService(privateSvc), WithObservedGeneration(1)), defaultSKS, metric(testNamespace, testRevision), @@ -731,7 +756,7 @@ func TestReconcile(t *testing.T) { Ctx: context.WithValue(context.Background(), deciderKey, decider(testNamespace, testRevision, 0 /* desiredScale */, 0 /* ebc */, scaling.MinActivators)), Objects: append([]runtime.Object{ - kpa(testNamespace, testRevision, markActivating, markOld, + kpa(testNamespace, testRevision, WithPASKSReady, markActivating, markOld, WithPAStatusService(testRevision), withScales(0, 0), WithPAMetricsService(privateSvc)), defaultSKS, @@ -739,7 +764,7 @@ func TestReconcile(t *testing.T) { deploy(testNamespace, testRevision), }, defaultReady...), WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ - Object: kpa(testNamespace, testRevision, WithPAMetricsService(privateSvc), + Object: kpa(testNamespace, testRevision, WithPASKSReady, WithPAMetricsService(privateSvc), WithNoTraffic("TimedOut", "The target could not be activated."), withScales(1, 0), WithPAStatusService(testRevision), WithPAMetricsService(privateSvc), WithObservedGeneration(1)), @@ -796,7 +821,7 @@ func TestReconcile(t *testing.T) { Ctx: context.WithValue(context.Background(), deciderKey, decider(testNamespace, testRevision, 2 /*autoscaler desired scale*/, 0 /* ebc */, scaling.MinActivators)), Objects: append([]runtime.Object{ - activatingKPAMinScale(underscale), underscaledDeployment, + activatingKPAMinScale(underscale, WithPASKSReady), underscaledDeployment, defaultSKS, defaultMetric, }, underscaledReady...), WantPatches: []clientgotesting.PatchActionImpl{ @@ -816,7 +841,7 @@ func TestReconcile(t *testing.T) { minScalePatch, }, WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ - Object: activatingKPAMinScale(underscale, markScaleTargetInitialized), + Object: activatingKPAMinScale(underscale, markScaleTargetInitialized, WithPASKSReady), }}, }, { // Scale to `minScale` and mark PA "active" @@ -893,12 +918,13 @@ func TestReconcile(t *testing.T) { decider(testNamespace, testRevision, unknownScale, /* desiredScale */ 0 /* ebc */, scaling.MinActivators)), Objects: append([]runtime.Object{ - kpa(testNamespace, testRevision, markInactive, WithPAMetricsService(privateSvc), + kpa(testNamespace, testRevision, WithPASKSReady, markInactive, WithPAMetricsService(privateSvc), withScales(0, -1), WithPAStatusService(testRevision), WithObservedGeneration(1)), - sks(testNamespace, testRevision, WithDeployRef(deployName), WithProxyMode, WithPubService, WithPrivateService), + sks(testNamespace, testRevision, WithDeployRef(deployName), + WithProxyMode, WithSKSReady), metric(testNamespace, testRevision), defaultDeployment, - }, defaultReady...), + }), }, { Name: "steady not enough capacity", Key: key, @@ -906,7 +932,8 @@ func TestReconcile(t *testing.T) { decider(testNamespace, testRevision, defaultScale, /* desiredScale */ -42 /* ebc */, scaling.MinActivators)), Objects: append([]runtime.Object{ - kpa(testNamespace, testRevision, markActive, markScaleTargetInitialized, WithPAMetricsService(privateSvc), + kpa(testNamespace, testRevision, WithPASKSReady, markActive, + markScaleTargetInitialized, WithPAMetricsService(privateSvc), withScales(1, defaultScale), WithPAStatusService(testRevision), WithObservedGeneration(1)), sks(testNamespace, testRevision, WithDeployRef(deployName), WithProxyMode, WithSKSReady), metric(testNamespace, testRevision), @@ -919,7 +946,8 @@ func TestReconcile(t *testing.T) { decider(testNamespace, testRevision, defaultScale, /* desiredScale */ -42 /* ebc */, 1982 /*numActivators*/)), Objects: append([]runtime.Object{ - kpa(testNamespace, testRevision, markActive, markScaleTargetInitialized, WithPAMetricsService(privateSvc), + kpa(testNamespace, testRevision, WithPASKSReady, markActive, + markScaleTargetInitialized, WithPAMetricsService(privateSvc), withScales(1, defaultScale), WithPAStatusService(testRevision), WithObservedGeneration(1)), sks(testNamespace, testRevision, WithDeployRef(deployName), WithProxyMode, WithSKSReady, WithNumActivators(4)), @@ -937,8 +965,9 @@ func TestReconcile(t *testing.T) { decider(testNamespace, testRevision, defaultScale, /* desiredScale */ -18 /* ebc */, scaling.MinActivators+1)), Objects: append([]runtime.Object{ - kpa(testNamespace, testRevision, markActive, markScaleTargetInitialized, WithPAMetricsService(privateSvc), - withScales(1, defaultScale), WithPAStatusService(testRevision), WithObservedGeneration(1)), + kpa(testNamespace, testRevision, WithPASKSReady, markActive, markScaleTargetInitialized, + WithPAMetricsService(privateSvc), withScales(1, defaultScale), + WithPAStatusService(testRevision), WithObservedGeneration(1)), sks(testNamespace, testRevision, WithDeployRef(deployName), WithSKSReady, WithNumActivators(2)), metric(testNamespace, testRevision), @@ -955,8 +984,9 @@ func TestReconcile(t *testing.T) { decider(testNamespace, testRevision, defaultScale, /* desiredScale */ 1 /* ebc */, scaling.MinActivators)), Objects: append([]runtime.Object{ - kpa(testNamespace, testRevision, markActive, markScaleTargetInitialized, WithPAMetricsService(privateSvc), - withScales(1, defaultScale), WithPAStatusService(testRevision), WithObservedGeneration(1)), + kpa(testNamespace, testRevision, WithPASKSReady, markActive, markScaleTargetInitialized, + WithPAMetricsService(privateSvc), withScales(1, defaultScale), + WithPAStatusService(testRevision), WithObservedGeneration(1)), sks(testNamespace, testRevision, WithDeployRef(deployName), WithSKSReady, WithProxyMode, WithNumActivators(3)), metric(testNamespace, testRevision), @@ -973,14 +1003,15 @@ func TestReconcile(t *testing.T) { decider(testNamespace, testRevision, defaultScale, /* desiredScale */ -42 /* ebc */, scaling.MinActivators)), Objects: append([]runtime.Object{ - kpa(testNamespace, testRevision, markActivating, withScales(defaultScale, defaultScale), WithReachabilityReachable, + kpa(testNamespace, testRevision, WithPASKSReady, markActivating, + withScales(defaultScale, defaultScale), WithReachabilityReachable, withMinScale(defaultScale), withInitialScale(20), WithPAStatusService(testRevision), WithPAMetricsService(privateSvc)), - sks(testNamespace, testRevision, WithDeployRef(deployName), WithProxyMode, WithSKSReady, - WithPubService, WithPrivateService), + sks(testNamespace, testRevision, WithDeployRef(deployName), WithProxyMode, WithSKSReady), defaultMetric, defaultDeployment, }, makeReadyPods(defaultScale, testNamespace, testRevision)...), WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ - Object: kpa(testNamespace, testRevision, markActivating, withScales(defaultScale, 20), WithReachabilityReachable, + Object: kpa(testNamespace, testRevision, WithPASKSReady, markActivating, + withScales(defaultScale, 20), WithReachabilityReachable, withMinScale(defaultScale), withInitialScale(20), WithPAStatusService(testRevision), WithPAMetricsService(privateSvc), WithObservedGeneration(1), ), @@ -997,18 +1028,19 @@ func TestReconcile(t *testing.T) { decider(testNamespace, testRevision, defaultScale, /* desiredScale */ -42 /* ebc */, scaling.MinActivators)), Objects: append([]runtime.Object{ - kpa(testNamespace, testRevision, markActivating, withScales(20, defaultScale), withInitialScale(20), WithReachabilityReachable, + kpa(testNamespace, testRevision, WithPASKSReady, markActivating, + withScales(20, defaultScale), withInitialScale(20), WithReachabilityReachable, WithPAStatusService(testRevision), WithPAMetricsService(privateSvc), ), - sks(testNamespace, testRevision, WithDeployRef(deployName), WithProxyMode, WithSKSReady, - WithPubService, WithPrivateService), + sks(testNamespace, testRevision, WithDeployRef(deployName), WithProxyMode, WithSKSReady), defaultMetric, deploy(testNamespace, testRevision, func(d *appsv1.Deployment) { d.Spec.Replicas = ptr.Int32(defaultScale) }), }, makeReadyPods(20, testNamespace, testRevision)...), WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ - Object: kpa(testNamespace, testRevision, markActive, markScaleTargetInitialized, withScales(20, 20), withInitialScale(20), WithReachabilityReachable, + Object: kpa(testNamespace, testRevision, WithPASKSReady, markActive, + markScaleTargetInitialized, withScales(20, 20), withInitialScale(20), WithReachabilityReachable, WithPAStatusService(testRevision), WithPAMetricsService(privateSvc), WithObservedGeneration(1), ), }}, @@ -1137,6 +1169,7 @@ func TestGlobalResyncOnUpdateAutoscalerConfigMap(t *testing.T) { kpa := revisionresources.MakePA(rev) sks := aresources.MakeSKS(kpa, nv1a1.SKSOperationModeServe, scaling.MinActivators) sks.Status.PrivateServiceName = "bogus" + sks.Status.InitializeConditions() fakenetworkingclient.Get(ctx).NetworkingV1alpha1().ServerlessServices(testNamespace).Create(sks) fakesksinformer.Get(ctx).Informer().GetIndexer().Add(sks) diff --git a/pkg/reconciler/revision/table_test.go b/pkg/reconciler/revision/table_test.go index 8761b318a027..70a17f2aa82c 100644 --- a/pkg/reconciler/revision/table_test.go +++ b/pkg/reconciler/revision/table_test.go @@ -247,7 +247,8 @@ func TestReconcile(t *testing.T) { Objects: []runtime.Object{ rev("foo", "pa-ready", withK8sServiceName("old-stuff"), WithLogURL, AllUnknownConditions), - pa("foo", "pa-ready", WithTraffic, WithScaleTargetInitialized, WithPAStatusService("new-stuff"), WithReachabilityUnknown), + pa("foo", "pa-ready", WithPASKSReady, WithTraffic, + WithScaleTargetInitialized, WithPAStatusService("new-stuff"), WithReachabilityUnknown), deploy(t, "foo", "pa-ready"), image("foo", "pa-ready"), }, @@ -343,7 +344,8 @@ 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, WithScaleTargetInitialized, WithPAStatusService("fix-mutated-pa")), + WithTraffic, WithPASKSReady, WithScaleTargetInitialized, + WithPAStatusService("fix-mutated-pa")), deploy(t, "foo", "fix-mutated-pa"), image("foo", "fix-mutated-pa"), }, @@ -357,7 +359,8 @@ func TestReconcile(t *testing.T) { withDefaultContainerStatuses(), withObservedGeneration(1)), }}, WantUpdates: []clientgotesting.UpdateActionImpl{{ - Object: pa("foo", "fix-mutated-pa", WithTraffic, WithScaleTargetInitialized, + Object: pa("foo", "fix-mutated-pa", WithPASKSReady, + WithTraffic, WithScaleTargetInitialized, WithPAStatusService("fix-mutated-pa"), WithReachabilityReachable), }}, WantEvents: []string{ @@ -511,7 +514,8 @@ 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, WithScaleTargetInitialized, WithPAStatusService("steadier-even")), + pa("foo", "steady-ready", WithPASKSReady, WithTraffic, + WithScaleTargetInitialized, WithPAStatusService("steadier-even")), deploy(t, "foo", "steady-ready"), image("foo", "steady-ready"), }, diff --git a/pkg/testing/functional.go b/pkg/testing/functional.go index 2b8ef3fa4998..2e1f2a5aebbc 100644 --- a/pkg/testing/functional.go +++ b/pkg/testing/functional.go @@ -84,6 +84,18 @@ func WithTraffic(pa *asv1a1.PodAutoscaler) { pa.Status.MarkActive() } +// WithPASKSReady marks PA status that all its deps are ready. +func WithPASKSReady(pa *asv1a1.PodAutoscaler) { + pa.Status.MarkSKSReady() +} + +// WithPADepsReady marks PA status that at least one of its deps is not ready. +func WithPASKSNotReady(m string) PodAutoscalerOption { + return func(pa *asv1a1.PodAutoscaler) { + pa.Status.MarkSKSNotReady(m) + } +} + // WithScaleTargetInitialized updates the PA to reflect it having initialized its // ScaleTarget. func WithScaleTargetInitialized(pa *asv1a1.PodAutoscaler) {