Skip to content

Commit

Permalink
Sync up with the autoscaler in serving for 1.15 release (#220)
Browse files Browse the repository at this point in the history
* Sync up with the autoscaler in serving

* Update the deps to 1.15

* Update the branch to release-1.15 for github actions
  • Loading branch information
houshengbo authored Jul 25, 2024
1 parent 529b881 commit 48bc948
Show file tree
Hide file tree
Showing 16 changed files with 579 additions and 153 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/kind-e2e.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ env:
YTT_VERSION: 0.40.1
KO_FLAGS: --platform=linux/amd64
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
KNATIVE_SERVING_BRANCH: main
KNATIVE_SERVING_BRANCH: release-1.15
KNATIVE_DIR: /home/runner/work/serving-progressive-rollout
KNATIVE_SERVING_DIR: /home/runner/work/serving-progressive-rollout/serving

Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ require (
k8s.io/code-generator v0.29.2
k8s.io/kube-openapi v0.0.0-20231010175941-2dd684a91f00
knative.dev/caching v0.0.0-20240716132144-989f54c83776
knative.dev/hack v0.0.0-20240704013904-b9799599afcf
knative.dev/hack v0.0.0-20240719133331-9c9eed6f6679
knative.dev/networking v0.0.0-20240716111826-bab7f2a3e556
knative.dev/pkg v0.0.0-20240716082220-4355f0c73608
knative.dev/serving v0.42.0
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -801,8 +801,8 @@ k8s.io/utils v0.0.0-20240102154912-e7106e64919e h1:eQ/4ljkx21sObifjzXwlPKpdGLrCf
k8s.io/utils v0.0.0-20240102154912-e7106e64919e/go.mod h1:OLgZIPagt7ERELqWJFomSt595RzquPNLL48iOWgYOg0=
knative.dev/caching v0.0.0-20240716132144-989f54c83776 h1:2nINnWuXtb9e2nG/EJxSCeghcmu6qmvmomJ7woiP5Is=
knative.dev/caching v0.0.0-20240716132144-989f54c83776/go.mod h1:Uj74eO9rLiK1eb8wmDBED1hJBZQ7MJ9cvq/d8Ktsm3c=
knative.dev/hack v0.0.0-20240704013904-b9799599afcf h1:n92FmZRywgtHso7pFAku7CW0qvRAs1hXtMQqO0R6eiE=
knative.dev/hack v0.0.0-20240704013904-b9799599afcf/go.mod h1:yk2OjGDsbEnQjfxdm0/HJKS2WqTLEFg/N6nUs6Rqx3Q=
knative.dev/hack v0.0.0-20240719133331-9c9eed6f6679 h1:tvbANb4KIO91DT1FGR4yCLA5E0qAmIeQ3DAGOkZGg4k=
knative.dev/hack v0.0.0-20240719133331-9c9eed6f6679/go.mod h1:R0ritgYtjLDO9527h5vb5X6gfvt5LCrJ55BNbVDsWiY=
knative.dev/networking v0.0.0-20240716111826-bab7f2a3e556 h1:9OTyJkrjiFh/burZiti3WucGv8Qtt91VJTnXfO5dC2g=
knative.dev/networking v0.0.0-20240716111826-bab7f2a3e556/go.mod h1:1PosUDkXqoHNzYxtLIwa7LFqSsIXBShHOseAb6XBeEU=
knative.dev/pkg v0.0.0-20240716082220-4355f0c73608 h1:BOiRzcnRS9Z5ruxlCiS/K1/Hb5bUN0X4W3xCegdcYQE=
Expand Down
31 changes: 11 additions & 20 deletions pkg/reconciler/autoscaling/kpa/kpa.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,8 @@ import (
"fmt"
"math"

goStats "go.opencensus.io/stats"
"go.opencensus.io/stats"
"go.uber.org/zap"
"knative.dev/serving/pkg/autoscaler/config/autoscalerconfig"

nv1alpha1 "knative.dev/networking/pkg/apis/networking/v1alpha1"
"knative.dev/pkg/logging"
Expand Down Expand Up @@ -229,7 +228,15 @@ func (c *Reconciler) reconcileDecider(ctx context.Context, pa *autoscalingv1alph
}

func computeStatus(ctx context.Context, pa *autoscalingv1alpha1.PodAutoscaler, spa *autoscalingv1.StagePodAutoscaler, pc podCounts, logger *zap.SugaredLogger) {
pa.Status.DesiredScale, pa.Status.ActualScale = ptr.Int32(int32(pc.want)), ptr.Int32(int32(pc.ready))
pa.Status.ActualScale = ptr.Int32(int32(pc.ready))

// When the autoscaler just restarted, it does not yet have metrics and would change the desiredScale to -1 and moments
// later back to the correct value. The following condition omits this.
if pc.want == -1 && pa.Status.DesiredScale != nil && *pa.Status.DesiredScale >= 0 {
logger.Debugf("Ignoring change of desiredScale from %d to %d", *pa.Status.DesiredScale, pc.want)
} else {
pa.Status.DesiredScale = ptr.Int32(int32(pc.want))
}

reportMetrics(pa, pc)
computeActiveCondition(ctx, pa, spa, pc)
Expand All @@ -242,7 +249,7 @@ func reportMetrics(pa *autoscalingv1alpha1.PodAutoscaler, pc podCounts) {

ctx := metrics.RevisionContext(pa.Namespace, serviceLabel, configLabel, pa.Name)

stats := []goStats.Measurement{
stats := []stats.Measurement{
actualPodCountM.M(int64(pc.ready)), notReadyPodCountM.M(int64(pc.notReady)),
pendingPodCountM.M(int64(pc.pending)), terminatingPodCountM.M(int64(pc.terminating)),
}
Expand Down Expand Up @@ -358,19 +365,3 @@ func computeNumActivators(readyPods int, decider *scaling.Decider) int32 {

return int32(math.Max(minActivators, math.Ceil(capacityToCover/decider.Spec.ActivatorCapacity)))
}

// GetScaleBounds returns the min and the max scales for the current stage.
func GetScaleBounds(asConfig *autoscalerconfig.Config, pa *autoscalingv1alpha1.PodAutoscaler,
spa *autoscalingv1.StagePodAutoscaler) (int32, int32) {
min, max := pa.ScaleBounds(asConfig)
if spa != nil {
minS, maxS := spa.ScaleBounds()
if minS != nil && min > *minS {
min = *minS
}
if maxS != nil && max > *maxS {
max = *maxS
}
}
return min, max
}
124 changes: 103 additions & 21 deletions pkg/reconciler/autoscaling/kpa/kpa_test.go
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
Copyright 2018 The Knative Authors
Copyright 2023 The Knative Authors
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -43,10 +43,6 @@ import (

networkingclient "knative.dev/networking/pkg/client/injection/client"
filteredinformerfactory "knative.dev/pkg/client/injection/kube/informers/factory/filtered"
_ "knative.dev/pkg/metrics/testing"
_ "knative.dev/pkg/system/testing"
_ "knative.dev/serving-progressive-rollout/pkg/client/injection/informers/serving/v1/rolloutorchestrator/fake"
_ "knative.dev/serving-progressive-rollout/pkg/client/injection/informers/serving/v1/stagepodautoscaler/fake"
servingclient "knative.dev/serving/pkg/client/injection/client"
"knative.dev/serving/pkg/client/injection/ducks/autoscaling/v1alpha1/podscalable"
pareconciler "knative.dev/serving/pkg/client/injection/reconciler/autoscaling/v1alpha1/podautoscaler"
Expand All @@ -73,9 +69,13 @@ import (
"knative.dev/pkg/kmeta"
"knative.dev/pkg/logging"
"knative.dev/pkg/metrics/metricstest"
_ "knative.dev/pkg/metrics/testing"
"knative.dev/pkg/ptr"
"knative.dev/pkg/reconciler"
"knative.dev/pkg/system"
_ "knative.dev/pkg/system/testing"
_ "knative.dev/serving-progressive-rollout/pkg/client/injection/informers/serving/v1/rolloutorchestrator/fake"
_ "knative.dev/serving-progressive-rollout/pkg/client/injection/informers/serving/v1/stagepodautoscaler/fake"
"knative.dev/serving/pkg/apis/autoscaling"
autoscalingv1alpha1 "knative.dev/serving/pkg/apis/autoscaling/v1alpha1"
"knative.dev/serving/pkg/apis/serving"
Expand Down Expand Up @@ -103,9 +103,6 @@ const (
paStableWindow = 45 * time.Second
progressDeadline = 121 * time.Second
stableWindow = 5 * time.Minute
testNamespace = "test-namespace"
testRevision = "test-revision"
key = testNamespace + "/" + testRevision
)

func defaultConfigMapData() map[string]string {
Expand Down Expand Up @@ -606,7 +603,7 @@ 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, WithPASKSNotReady("No Private Service Name"),
WithBufferedTraffic, WithPAMetricsService(privateSvc), withScales(0, unknownScale),
WithBufferedTraffic, WithPAMetricsService(privateSvc), withScales(0, defaultScale),
WithObservedGeneration(1)),
}},
WantCreates: []runtime.Object{
Expand Down Expand Up @@ -1032,7 +1029,7 @@ func TestReconcile(t *testing.T) {
-42 /* ebc */)),
Objects: append([]runtime.Object{
kpa(testNamespace, testRevision, WithPASKSReady, WithBufferedTraffic,
withScales(20, defaultScale), withInitialScaleB(20), WithReachabilityReachable,
withScales(20, defaultScale), withInitialScale(20), WithReachabilityReachable,
WithPAStatusService(testRevision), WithPAMetricsService(privateSvc),
),
sks(testNamespace, testRevision, WithDeployRef(deployName), WithProxyMode, WithSKSReady, WithNumActivators(scaledAct)),
Expand All @@ -1043,7 +1040,7 @@ func TestReconcile(t *testing.T) {
}, makeReadyPods(20, testNamespace, testRevision)...),
WantStatusUpdates: []clientgotesting.UpdateActionImpl{{
Object: kpa(testNamespace, testRevision, WithPASKSReady, WithTraffic,
markScaleTargetInitialized, withScales(20, 20), withInitialScaleB(20), WithReachabilityReachable,
markScaleTargetInitialized, withScales(20, 20), withInitialScale(20), WithReachabilityReachable,
WithPAStatusService(testRevision), WithPAMetricsService(privateSvc), WithObservedGeneration(1),
),
}},
Expand Down Expand Up @@ -1921,10 +1918,6 @@ func TestResolveScrapeTarget(t *testing.T) {
}
}

func withInitialScaleB(initScale int) PodAutoscalerOption {
return withInitialScale(initScale)
}

func withInitialScale(initScale int) PodAutoscalerOption {
return func(pa *autoscalingv1alpha1.PodAutoscaler) {
pa.Annotations = kmeta.UnionMaps(
Expand Down Expand Up @@ -2007,19 +2000,108 @@ func TestComputeActivatorNum(t *testing.T) {
}
}

func TestComputeStatus(t *testing.T) {
cases := []struct {
name string

haveActual *int32
haveDesiredScale *int32

pcReady int
pcWant int

wantActualScale *int32
wantDesiredScale *int32
}{{
name: "initial",

haveActual: nil,
haveDesiredScale: nil,

pcReady: 0,
pcWant: 1,

wantActualScale: ptr.Int32(0),
wantDesiredScale: ptr.Int32(1),
}, {
name: "ready",

haveActual: ptr.Int32(0),
haveDesiredScale: ptr.Int32(1),

pcReady: 1,
pcWant: 1,

wantActualScale: ptr.Int32(1),
wantDesiredScale: ptr.Int32(1),
}, {
name: "stable",

haveActual: ptr.Int32(1),
haveDesiredScale: ptr.Int32(1),

pcReady: 1,
pcWant: 1,

wantActualScale: ptr.Int32(1),
wantDesiredScale: ptr.Int32(1),
}, {
name: "no metrics",

haveActual: ptr.Int32(1),
haveDesiredScale: ptr.Int32(2),

pcReady: 2,
pcWant: -1,

wantActualScale: ptr.Int32(2),
wantDesiredScale: ptr.Int32(2),
}}

tc := &testConfigStore{config: defaultConfig()}

for _, c := range cases {
t.Run(c.name, func(t *testing.T) {
ctx := tc.ToContext(context.Background())

pa := &autoscalingv1alpha1.PodAutoscaler{
Status: autoscalingv1alpha1.PodAutoscalerStatus{
ActualScale: c.haveActual,
DesiredScale: c.haveDesiredScale,
},
}
pc := podCounts{
ready: c.pcReady,
want: c.pcWant,
}

computeStatus(ctx, pa, nil, pc, logging.FromContext(ctx))

if c.wantActualScale == nil && pa.Status.ActualScale != nil || c.wantActualScale != nil && pa.Status.ActualScale == nil {
t.Errorf("Unexpected ActualScale. Want: %v, Got: %v", c.wantActualScale, pa.Status.ActualScale)
}
if c.wantActualScale != nil && pa.Status.ActualScale != nil && *c.wantActualScale != *pa.Status.ActualScale {
t.Errorf("Unexpected ActualScale. Want: %d, Got: %d", *c.wantActualScale, *pa.Status.ActualScale)
}

if c.wantDesiredScale == nil && pa.Status.DesiredScale != nil || c.wantDesiredScale != nil && pa.Status.DesiredScale == nil {
t.Errorf("Unexpected DesiredScale. Want: %v, Got: %v", c.wantDesiredScale, pa.Status.DesiredScale)
}
if c.wantDesiredScale != nil && pa.Status.DesiredScale != nil && *c.wantDesiredScale != *pa.Status.DesiredScale {
t.Errorf("Unexpected DesiredScale. Want: %d, Got: %d", *c.wantDesiredScale, *pa.Status.DesiredScale)
}
})
}
}

func TestReconcileWithNSR(t *testing.T) {
testRevision := "testRevision1"
testNamespace := "testNamespace1"
key := testNamespace + "/" + testRevision
deployName := testRevision + "-deployment"
const (
defaultScale = 19
unknownScale = scaleUnknown
underscale = defaultScale - 1
overscale = defaultScale + 1

defaultAct = 3 // 1-10 ready pods + 200 TBC
scaledAct = 4 // 11 or 12 ready pods + 200 TBC
defaultAct = 3 // 1-10 ready pods + 200 TBC
)
privateSvc := names.PrivateService(testRevision)

Expand Down
22 changes: 21 additions & 1 deletion pkg/reconciler/autoscaling/kpa/scaler.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,14 +25,14 @@ import (
"knative.dev/pkg/apis/duck"
"knative.dev/pkg/injection/clients/dynamicclient"
"knative.dev/pkg/logging"
autoscalingv1 "knative.dev/serving-progressive-rollout/pkg/apis/serving/v1"

netapis "knative.dev/networking/pkg/apis/networking"
netv1alpha1 "knative.dev/networking/pkg/apis/networking/v1alpha1"
nethttp "knative.dev/networking/pkg/http"
netheader "knative.dev/networking/pkg/http/header"
netprober "knative.dev/networking/pkg/prober"
pkgnet "knative.dev/pkg/network"
autoscalingv1 "knative.dev/serving-progressive-rollout/pkg/apis/serving/v1"
"knative.dev/serving/pkg/activator"
autoscalingv1alpha1 "knative.dev/serving/pkg/apis/autoscaling/v1alpha1"
"knative.dev/serving/pkg/autoscaler/config/autoscalerconfig"
Expand Down Expand Up @@ -141,6 +141,10 @@ func activatorProbe(pa *autoscalingv1alpha1.PodAutoscaler, transport http.RoundT
}

func lastPodRetention(pa *autoscalingv1alpha1.PodAutoscaler, cfg *autoscalerconfig.Config) time.Duration {
// if revision is unreachable, no need to account for last pod retention
if pa.Spec.Reachability == autoscalingv1alpha1.ReachabilityUnreachable {
return 0
}
d, ok := pa.ScaleToZeroPodRetention()
if ok {
return d
Expand Down Expand Up @@ -322,6 +326,22 @@ func (ks *scaler) applyScale(ctx context.Context, pa *autoscalingv1alpha1.PodAut
return nil
}

// GetScaleBounds returns the min and the max scales for the current stage.
func GetScaleBounds(asConfig *autoscalerconfig.Config, pa *autoscalingv1alpha1.PodAutoscaler,
spa *autoscalingv1.StagePodAutoscaler) (int32, int32) {
min, max := pa.ScaleBounds(asConfig)
if spa != nil {
minS, maxS := spa.ScaleBounds()
if minS != nil && min > *minS {
min = *minS
}
if maxS != nil && max > *maxS {
max = *maxS
}
}
return min, max
}

// scale attempts to scale the given PA's target reference to the desired scale.
func (ks *scaler) scale(ctx context.Context, pa *autoscalingv1alpha1.PodAutoscaler, spa *autoscalingv1.StagePodAutoscaler, sks *netv1alpha1.ServerlessService, desiredScale int32) (int32, error) {
asConfig := config.FromContext(ctx).Autoscaler
Expand Down
39 changes: 36 additions & 3 deletions pkg/reconciler/autoscaling/kpa/scaler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,10 @@ import (
"testing"
"time"

autoscalingv1 "knative.dev/serving-progressive-rollout/pkg/apis/serving/v1"
"knative.dev/serving/pkg/autoscaler/config/autoscalerconfig"

// These are the fake informers we want setup.
fakedynamicclient "knative.dev/pkg/injection/clients/dynamicclient/fake"
autoscalingv1 "knative.dev/serving-progressive-rollout/pkg/apis/serving/v1"
"knative.dev/serving/pkg/autoscaler/config/autoscalerconfig"
fakeservingclient "knative.dev/serving/pkg/client/injection/client/fake"
podscalable "knative.dev/serving/pkg/client/injection/ducks/autoscaling/v1alpha1/podscalable/fake"

Expand Down Expand Up @@ -65,6 +64,12 @@ import (
. "knative.dev/serving/pkg/testing"
)

const (
testNamespace = "test-namespace"
testRevision = "test-revision"
key = testNamespace + "/" + testRevision
)

func TestScaler(t *testing.T) {
const activationTimeout = progressDeadline + activationTimeoutBuffer
tests := []struct {
Expand Down Expand Up @@ -355,6 +360,33 @@ func TestScaler(t *testing.T) {
paMarkInactive(k, time.Now().Add(-gracePeriod))
WithReachabilityReachable(k)
},
}, {
label: "scale to zero ignore last pod retention if revision is unreachable",
startReplicas: 1,
scaleTo: 0,
wantReplicas: 0,
wantScaling: true,
paMutation: func(k *autoscalingv1alpha1.PodAutoscaler) {
paMarkInactive(k, time.Now().Add(-gracePeriod))
WithReachabilityUnreachable(k)
},
configMutator: func(c *config.Config) {
c.Autoscaler.ScaleToZeroPodRetentionPeriod = 10 * gracePeriod
},
}, {
label: "can't scale to zero if revision is unreachable, but before deadline",
startReplicas: 1,
scaleTo: 0,
wantReplicas: 0,
wantScaling: false,
paMutation: func(k *autoscalingv1alpha1.PodAutoscaler) {
paMarkInactive(k, time.Now().Add(-gracePeriod+time.Second))
WithReachabilityUnreachable(k)
},
configMutator: func(c *config.Config) {
c.Autoscaler.ScaleToZeroPodRetentionPeriod = 10 * gracePeriod
},
wantCBCount: 1,
}, {
label: "ignore minScale if unreachable",
startReplicas: 10,
Expand Down Expand Up @@ -673,6 +705,7 @@ func newRevision(ctx context.Context, t *testing.T, servingClient clientset.Inte

func newDeployment(ctx context.Context, t *testing.T, dynamicClient dynamic.Interface, name string, replicas int) *appsv1.Deployment {
t.Helper()

uns := &unstructured.Unstructured{
Object: map[string]interface{}{
"apiVersion": "apps/v1",
Expand Down
Loading

0 comments on commit 48bc948

Please sign in to comment.