Skip to content

Commit

Permalink
Add new KPA status for dependencies (#8809)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
vagababov authored Jul 26, 2020
1 parent a342c9f commit 1642dd1
Show file tree
Hide file tree
Showing 7 changed files with 157 additions and 67 deletions.
13 changes: 12 additions & 1 deletion pkg/apis/autoscaling/v1alpha1/pa_lifecycle.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import (
var podCondSet = apis.NewLivingConditionSet(
PodAutoscalerConditionActive,
PodAutoscalerConditionScaleTargetInitialized,
PodAutoscalerSKSReady,
)

// GetConditionSet retrieves the condition set for this resource. Implements the KRShaped interface.
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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 {
Expand Down
24 changes: 23 additions & 1 deletion pkg/apis/autoscaling/v1alpha1/pa_lifecycle_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -251,6 +251,7 @@ func TestInactiveFor(t *testing.T) {
})
}
}

func TestActiveFor(t *testing.T) {
now := time.Now()
cases := []struct {
Expand Down Expand Up @@ -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)
Expand All @@ -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) {
Expand Down
2 changes: 2 additions & 0 deletions pkg/apis/autoscaling/v1alpha1/pa_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Expand Down
8 changes: 7 additions & 1 deletion pkg/reconciler/autoscaling/kpa/kpa.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down Expand Up @@ -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,
Expand Down
Loading

0 comments on commit 1642dd1

Please sign in to comment.