Skip to content

Commit

Permalink
revision reachable when active. allow unreachable to scale to 0
Browse files Browse the repository at this point in the history
  • Loading branch information
andrew-delph committed Nov 9, 2023
2 parents 9defe38 + f080333 commit ee655e1
Show file tree
Hide file tree
Showing 5 changed files with 44 additions and 15 deletions.
8 changes: 8 additions & 0 deletions pkg/apis/autoscaling/v1alpha1/pa_lifecycle.go
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,14 @@ func (pa *PodAutoscaler) IsReady() bool {
pas.GetCondition(PodAutoscalerConditionReady).IsTrue()
}

func (pa *PodAutoscaler) IsUnreachable() bool {
return pa.Spec.Reachability == "Unreachable"
}

func (pa *PodAutoscaler) IsReachable() bool {
return pa.Spec.Reachability == "Reachable"
}

// IsActive returns true if the pod autoscaler has finished scaling.
func (pas *PodAutoscalerStatus) IsActive() bool {
return pas.GetCondition(PodAutoscalerConditionActive).IsTrue()
Expand Down
2 changes: 1 addition & 1 deletion pkg/autoscaler/scaling/autoscaler.go
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ func (a *autoscaler) Scale(logger *zap.SugaredLogger, now time.Time) ScaleResult
observedStableValue, observedPanicValue, err = a.metricClient.StableAndPanicConcurrency(metricKey, now)
}

if err != nil {
if err != nil && spec.Reachable {
if errors.Is(err, metrics.ErrNoData) {
logger.Debug("No data to scale on yet")
} else {
Expand Down
18 changes: 14 additions & 4 deletions pkg/reconciler/autoscaling/kpa/kpa.go
Original file line number Diff line number Diff line change
Expand Up @@ -270,14 +270,19 @@ func computeActiveCondition(ctx context.Context, pa *autoscalingv1alpha1.PodAuto
if pa.Status.IsActivating() && minReady > 0 {
// We only ever scale to zero while activating if we fail to activate within the progress deadline.
pa.Status.MarkInactive("TimedOut", "The target could not be activated.")
} else {
} else if !pa.Status.IsInactive() {
pa.Status.MarkInactive(noTrafficReason, "The target is not receiving traffic.")
}

case pc.ready < minReady:
if pc.want > 0 || !pa.Status.IsInactive() {
pa.Status.MarkActivating(
"Queued", "Requests to the target are being buffered as resources are provisioned.")
if pa.IsUnreachable() {
pa.Status.MarkInactive(
"Failed", "The target failed.")
} else {
pa.Status.MarkActivating(
"Queued", "Requests to the target are being buffered as resources are provisioned.")
}
} else {
// This is for the initialScale 0 case. In the first iteration, minReady is 0,
// but for the following iterations, minReady is 1. pc.want will continue being
Expand All @@ -286,7 +291,12 @@ func computeActiveCondition(ctx context.Context, pa *autoscalingv1alpha1.PodAuto
// still need to set it again. Otherwise reconciliation will fail with NewObservedGenFailure
// because we cannot go through one iteration of reconciliation without setting
// some status.
pa.Status.MarkInactive(noTrafficReason, "The target is not receiving traffic.")
if pa.Status.IsInactive() {
cond := pa.Status.GetCondition("Active")
pa.Status.MarkInactive(cond.Reason, cond.Message)
} else {
pa.Status.MarkInactive(noTrafficReason, "The target is not receiving traffic.")
}
}

case pc.ready >= minReady:
Expand Down
24 changes: 16 additions & 8 deletions pkg/reconciler/revision/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,25 +24,27 @@ import (

"go.uber.org/zap"
"golang.org/x/time/rate"
"k8s.io/client-go/tools/cache"
"k8s.io/client-go/util/workqueue"
cachingclient "knative.dev/caching/pkg/client/injection/client"
imageinformer "knative.dev/caching/pkg/client/injection/informers/caching/v1alpha1/image"
netcfg "knative.dev/networking/pkg/config"
"knative.dev/pkg/changeset"
kubeclient "knative.dev/pkg/client/injection/kube/client"
deploymentinformer "knative.dev/pkg/client/injection/kube/informers/apps/v1/deployment"
servingclient "knative.dev/serving/pkg/client/injection/client"
painformer "knative.dev/serving/pkg/client/injection/informers/autoscaling/v1alpha1/podautoscaler"
revisioninformer "knative.dev/serving/pkg/client/injection/informers/serving/v1/revision"
revisionreconciler "knative.dev/serving/pkg/client/injection/reconciler/serving/v1/revision"

"k8s.io/client-go/tools/cache"
"k8s.io/client-go/util/workqueue"
netcfg "knative.dev/networking/pkg/config"
podinformer "knative.dev/pkg/client/injection/kube/informers/core/v1/pod"
"knative.dev/pkg/configmap"
"knative.dev/pkg/controller"
"knative.dev/pkg/logging"
"knative.dev/pkg/metrics"
pkgreconciler "knative.dev/pkg/reconciler"
apisconfig "knative.dev/serving/pkg/apis/config"
"knative.dev/serving/pkg/apis/serving"
v1 "knative.dev/serving/pkg/apis/serving/v1"
servingclient "knative.dev/serving/pkg/client/injection/client"
painformer "knative.dev/serving/pkg/client/injection/informers/autoscaling/v1alpha1/podautoscaler"
revisioninformer "knative.dev/serving/pkg/client/injection/informers/serving/v1/revision"
revisionreconciler "knative.dev/serving/pkg/client/injection/reconciler/serving/v1/revision"
"knative.dev/serving/pkg/deployment"
"knative.dev/serving/pkg/reconciler/revision/config"
)
Expand Down Expand Up @@ -73,6 +75,7 @@ func newControllerWithOptions(
deploymentInformer := deploymentinformer.Get(ctx)
imageInformer := imageinformer.Get(ctx)
paInformer := painformer.Get(ctx)
podsInformer := podinformer.Get(ctx)

c := &Reconciler{
kubeclient: kubeclient.Get(ctx),
Expand Down Expand Up @@ -132,6 +135,11 @@ func newControllerWithOptions(
deploymentInformer.Informer().AddEventHandler(handleMatchingControllers)
paInformer.Informer().AddEventHandler(handleMatchingControllers)

podsInformer.Informer().AddEventHandler(cache.FilteringResourceEventHandler{
FilterFunc: pkgreconciler.LabelExistsFilterFunc(serving.RevisionLabelKey),
Handler: controller.HandleAll(impl.EnqueueLabelOfNamespaceScopedResource("", serving.RevisionLabelKey)),
})

// We don't watch for changes to Image because we don't incorporate any of its
// properties into our own status and should work completely in the absence of
// a functioning Image controller.
Expand Down
7 changes: 5 additions & 2 deletions pkg/reconciler/revision/resources/pa.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,15 +57,18 @@ func reachability(rev *v1.Revision) autoscalingv1alpha1.ReachabilityType {
v1.RevisionConditionContainerHealthy,
}

routingState := rev.GetRoutingState()
if routingState == v1.RoutingStateActive {
return autoscalingv1alpha1.ReachabilityReachable
}

for _, cond := range conds {
if c := rev.Status.GetCondition(cond); c != nil && c.IsFalse() {
return autoscalingv1alpha1.ReachabilityUnreachable
}
}

switch rev.GetRoutingState() {
case v1.RoutingStateActive:
return autoscalingv1alpha1.ReachabilityReachable
case v1.RoutingStateReserve:
return autoscalingv1alpha1.ReachabilityUnreachable
default:
Expand Down

0 comments on commit ee655e1

Please sign in to comment.