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.

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

Prevent a PodAutoscaler's DesiredScale to turn to -1 #14866

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
10 changes: 9 additions & 1 deletion pkg/reconciler/autoscaling/kpa/kpa.go
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,15 @@ func (c *Reconciler) reconcileDecider(ctx context.Context, pa *autoscalingv1alph
}

func computeStatus(ctx context.Context, pa *autoscalingv1alpha1.PodAutoscaler, 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))
Copy link
Member

@dprotaso dprotaso Mar 26, 2024

Choose a reason for hiding this comment

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

I was wondering why ActualScale is different then I saw we are deriving this number from querying the informer cache which is sync'd at startup

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Correct, that value is never incorrect.


// 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)
dprotaso marked this conversation as resolved.
Show resolved Hide resolved
} else {
pa.Status.DesiredScale = ptr.Int32(int32(pc.want))
}

reportMetrics(pa, pc)
computeActiveCondition(ctx, pa, pc)
Expand Down
96 changes: 95 additions & 1 deletion pkg/reconciler/autoscaling/kpa/kpa_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -601,7 +601,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 @@ -1996,3 +1996,97 @@ 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, 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)
}
})
}
}
Loading