Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add new KPA status for dependencies #8809

Merged
merged 9 commits into from
Jul 26, 2020
Merged

Add new KPA status for dependencies #8809

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
2692df5
Add new Kpa status for dependencies
vagababov Jul 24, 2020
ad7fba5
t Merge branch 'master' into 20200724-kpa-status
vagababov Jul 25, 2020
7b79765
more files to fix
vagababov Jul 25, 2020
a8ba223
review
vagababov Jul 25, 2020
22d40be
restore tests
vagababov Jul 25, 2020
983c811
Merge branch 'master' into 20200724-kpa-status
vagababov Jul 26, 2020
9a2048c
rewrite as sks only cond
vagababov Jul 26, 2020
3dbfdd2
holdout
vagababov Jul 26, 2020
4b1ca7d
more fixes
vagababov Jul 26, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 12 additions & 1 deletion pkg/apis/autoscaling/v1alpha1/pa_lifecycle.go
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can return nil, so I'd guess this will make our controllers crash loop 😬

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one cannot: we get here only if PrivateServiceName is set, which means SKS reconciled this at least once, which means InitializeConditions has been invoked and sks updated the status.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i see the sks.Status.PrivateServiceName guard, which ensures things have been initialized to reach this point.

} 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