Add scale-up circuit breaker for unfulfilled ASG scale-ups

[FocalChord]

Problem

When an ASG can't fulfil a desired-count increase, because an instance type is temporarily out of capacity in an AZ, no instances launch but pod utilisation stays high. Escalator keeps raising the desired count every scan, so it climbs to max_nodes while the running count stays flat, leaving a large and persistent gap between desired and running.

We hit this in production. An ASG sat at desired 600 against an AZ with no capacity, running stayed near 100, and we reset the desired count by hand.

What this does

An opt-in, per-nodegroup circuit breaker that stops raising the ASG desired count once repeated scale-ups fail to add running nodes. Disabled by default.

• A scale-up is counted as failed when the running count has not grown since the previous scale-up, meaning the cloud provider delivered no new capacity in the interim. Judging on "did running grow" rather than "did running reach the previous target" keeps a busy-but-healthy group, whose desired count rises faster than instances launch, from tripping.
• After scale_up_failure_threshold consecutive failures the breaker opens and stops raising the desired count, holding it steady rather than ratcheting it up further.
• There is no cooldown timer. Escalator keeps looping as normal while the breaker is open. It closes and normal scaling resumes as soon as the running count catches up to the current desired count, meaning the cloud provider finally delivered the capacity. The recovery check reads the current desired count, not a value frozen at trip time, so if desired is lowered externally while open, by a scale-down as demand falls or a manual reset, the breaker still recovers rather than staying stuck.
• Untaint of existing nodes is not gated, so already-running capacity is still reused during a crunch.

The gate sits only around the desired-count increase in scaleUpCloudProviderNodeGroup. Note that "running count" here is the ASG instance count (Size()), not the number of Ready Kubernetes nodes. This targets the out of capacity case where no instances launch at all. If instances launch but never join, for example a broken bootstrap, the count grows and the breaker reads that as capacity arriving, so it will not trip on that failure mode.

Worked example

Node group with max_nodes: 600, running 100, scale_up_cool_down_period: 5m, each loop wanting roughly +50 nodes to clear a backlog of pending pods. An instance type is out of capacity in the AZ, so nothing launches and running stays at 100. Breaker set to scale_up_failure_threshold: 3.

Time	Desired (breaker disabled)	Desired (breaker enabled)	Running
00:00	150	150	100
05:00	200	200 (failure 1, running didn't grow)	100
10:00	250	250 (failure 2)	100
15:00	300	250 (opens on failure 3, desired held)	100
25:00	400	250 (held)	100
45:00	600 (max)	250 (held)	100
AZ recovers	600 (500-node overshoot)	250 delivered, breaker closes	250

With the breaker disabled the desired count climbs to the max of 600 while only 100 nodes run, a gap of 500 nodes that all try to launch at once when the AZ recovers and overshoot real demand. With the breaker it caps at 250 and holds there, with no cooldown to wait out. When the AZ recovers, the 250 requested nodes launch, the running count catches up to the held desired count of 250, the breaker closes, and scaling resumes against real demand.

Configuration

The default scale_up_failure_threshold of 0 disables the breaker, so existing configs are unchanged.

scale_up_failure_threshold: 5

Judging on whether the running count grows means a group that launches instances steadily, even slowly, keeps making progress and does not accrue failures, so slow launch latency does not by itself trip the breaker.

Metrics

• node_group_scale_up_circuit_breaker_open, a gauge that is 1 when open and 0 when closed. It is seeded to 0 when an enabled breaker is built, so an enabled group that never trips still emits a baseline.
• node_group_scale_up_failed_scale_events, a counter of scale-ups whose running count did not grow.

Verification

go test ./pkg/... passes. Unit tests cover the state machine: disabled, reset when running grows, no trip on a healthy group whose desired count keeps rising while running climbs, trip after threshold consecutive no-growth scale-ups, stays open until running reaches desired, recovery once running catches up, recovery when desired is lowered externally while open, and failed-event counting. An integration test runs scaleUpCloudProviderNodeGroup against a mock ASG that raises the desired count but never delivers nodes, asserts the desired count freezes once the breaker trips, and then that scaling resumes once the running count reaches the frozen target.

[tomwwright]

That's a long time (I know this is just example config). Short cooldown isn't an issue, because we still half-probe and fail out if we still haven't caught up to desired

[FocalChord]

Ack'd

[tomwwright]

We might not even need the cooldown... The load from checking isn't hurting us, and introducing a cooldown will only introduce potential latency during scaling

We just want to open the circuit breaker, keep looping as normal, but only do scale activities when actual matches desired again

[FocalChord]

Ack'd

[awprice]

Looking good - just some tweaks needed

[awprice]

String() doesn't look used anywhere. It's also the only value receiver on the type (allow, recordScaleUp and enabled are all pointer receivers), so dropping it also makes the receivers consistent.

[FocalChord]

Removed in 06ed774. It had no callers and was the only value receiver on the type, so dropping it also lets the fmt import go and keeps every method on a pointer receiver.

[awprice]

This reads the target back out of the cache after IncreaseSize, but the AWS provider's IncreaseSize only calls SetDesiredCapacity (aws.go:359); it never updates the local asg, which is only refreshed on the next Refresh/DescribeAutoScalingGroups. So on real AWS this records the previous desired count, not the one we just asked for. Tests don't catch it because the mock's IncreaseSize updates targetSize inline (cloud_provider.go:139).

Two effects: the breaker opens a cycle late (threshold + 1 stuck cooldowns instead of threshold), and the frozen target ends up one step behind the real DesiredCapacity, so a partial recovery can close it too early. I confirmed both by running the real provider against a stub: with threshold: 3 it opened after 4 scale-ups, and froze at 14 while AWS desired was already 18.

Grabbing the target before the increase fixes it for both mock and AWS:

requestedTarget := cloudProviderNodeGroup.TargetSize() + nodesToAdd
err := cloudProviderNodeGroup.IncreaseSize(nodesToAdd)
...
opts.nodeGroup.scaleUpCircuitBreaker.recordScaleUp(requestedTarget)

[FocalChord]

Good catch, thanks for running it against the real provider.

I reworked this so it no longer reads the target back after IncreaseSize. The failure signal is now based on whether the running count grew rather than whether it reached the requested target, so recordScaleUp records Size() captured at the start of the scan, and Size() is not touched by IncreaseSize. That takes the stale read you described out of the picture.

Size() and TargetSize() are both read once up front now, which on AWS reflects the values from the scan's refresh.

[awprice]

This only passes because the mock updates targetSize synchronously in IncreaseSize, which AWS doesn't; desired isn't visible until the next Refresh. If the mock only exposed the new desired after a Refresh (a SetDecoupleTarget knob alongside SetDecoupleActual would do it), it'd have caught the stale read above.

[FocalChord]

Tied to the change on the stale read above. Since recordScaleUp no longer reads TargetSize() back after the increase, the stale value that knob would have exposed can no longer occur, so I left SetDecoupleTarget out rather than add scaffolding for a path that isn't there anymore.

SetDecoupleActual still drives this test: the running count stays pinned, so it never grows and the breaker trips after the threshold. If you'd still like the mock to model deferred desired visibility for future tests I'm happy to add it.

[awprice]

lastRequestedTarget is frozen when we trip and we only close once running reaches it. If desired drops below that number while we're open (a scale-down when demand falls off during a long outage, or someone resetting desired by hand, which is what we did in the incident), running can never get back to the frozen value and we're stuck open until a restart. That's the manual intervention we're trying to remove.

Could we compare against the live TargetSize() instead, closing once running has caught up to whatever desired currently is? While open we're not raising desired, so it's the same number in the normal case; it only differs when something external lowers it, which is exactly when we want to recover.

[FocalChord]

Agreed. Switched the recovery check to the live TargetSize().

allow now takes both the running and desired counts and closes once running reaches the current desired, not the value frozen at trip time. So if desired is lowered while the breaker is open, whether by a scale-down as demand falls or the manual reset we did in the incident, it recovers on the next scan. Added TestCircuitBreakerRecoversWhenDesiredDropsWhileOpen to cover that.

[awprice]

Want to make sure we're happy with this: we count a failure whenever running hasn't hit the previous target yet. If an ASG is genuinely launching but always a step behind a rising desired (busy cluster, steady-but-slow launches), running never catches the previous target and we trip on a healthy group. The docs cover slow launches vs cooldown, but this is slightly different: it's the target moving every round. Would keying off "did running go up at all since last time" be safer? Not a blocker since it's opt-in.

[FocalChord]

Went with your suggestion. A scale-up now counts as failed only when the running count did not grow since the previous one, rather than when it fell short of the previous target. A group whose desired count climbs faster than instances launch keeps making progress each scan and no longer accrues failures. The out of capacity case this targets still trips because the running count stays flat.

One tradeoff worth naming: a group that delivers a slow trickle while wanting a lot more will not trip. My read is that degrades to today's behaviour rather than under provisioning a healthy group, which felt like the safer default for an opt-in feature. Covered by TestCircuitBreakerDoesNotTripWhenRunningKeepsGrowing.

[awprice]

Flagging that Size() is len(asg.Instances) (aws.go:240), i.e. instances the ASG has, not Ready k8s nodes. If instances come up but never join (bad bootstrap), we'll think capacity arrived and never trip. Fine for the out-of-capacity case this targets, but the "running node count" wording in the docs/metric help is a bit optimistic.

[FocalChord]

You're right that Size() is len(asg.Instances) and not Ready k8s nodes. I kept it as the signal since this targets the out of capacity case where no instances launch at all, but updated the docs to say instance count rather than running node count, and called out the failure mode you described: if instances launch but never join, the count grows and the breaker reads that as capacity arriving, so it won't trip on a broken bootstrap.

[awprice]

worth a checkThat(nodegroup.ScaleUpFailureThreshold >= 0, ...) to match the rest of the validation; a negative value just silently disables it right now.

[FocalChord]

Added checkThat(nodegroup.ScaleUpFailureThreshold >= 0, ...) alongside the rest of the validation, so a negative value now errors rather than silently disabling.

[awprice]

this gauge only gets Set on a transition, so an enabled group that never trips never emits it and there's no 0 line for dashboards to sit on. A Set(0) when we build the breaker gives it a baseline.

[FocalChord]

Added a newScaleUpCircuitBreaker constructor that sets the gauge to 0 when the breaker is enabled, so an enabled group emits a 0 baseline from startup even if it never trips. Wired it into both construction sites.

[awprice]

LGTM!