feat(disaggregatedset): per-role autoscaling via DisaggregatedSetRoleScaler (#922)
* feat(disaggregatedset): per-role autoscaling via DisaggregatedSetRoleScaler
Adds KEP-849 alpha implementation of per-role external scaling for
DisaggregatedSet. A role opts in via scaling.mode: External; the DS
controller then auto-creates a DisaggregatedSetRoleScaler named
"<ds>-<role>" that exposes /scale for HPA, KEDA, or any /scale-aware
controller. Replicas flow: autoscaler → scaler.spec.replicas → LWS.
New CRD DisaggregatedSetRoleScaler with the /scale subresource, aggregate
status.selector across revisions, and controller ownerRef for GC. The
executor resolves per-role targets from the scaler map; a no-shrink guard
prevents the new-revision target from shrinking mid-rollout if HPA writes a
smaller value. Webhook warns on External + spec.replicas > 1 and rejects
External + spec.slices > 1 (alpha restriction).
* test(disaggregatedset): e2e regression guard for /scale on a fresh scaler
Adds test/e2e/disaggregatedset/hpa_test.go covering three invariants that
unit tests can't reach because they need a real apiserver in the loop:
1. GET /scale on a freshly auto-created scaler must return 200. This is
the regression guard for yankay/lws#15 — HPA reads /scale before its
first write, and if the CRD scale handler errors because .spec.replicas
is missing, HPA parks in AbleToScale=False / FailedGetScale forever.
2. kubectl scale on the scaler propagates spec.replicas to the underlying
LWS (guards controller wiring end-to-end).
3. Deleting the parent DS garbage-collects the auto-created scaler via
the controller ownerRef (fake client doesn't model GC).
Extends test/testutils/disaggregatedset/fixtures.Role with an External bool
that emits scaling.mode: External on the role.
Spec #1 currently fails on this branch as expected — spec.replicas is a
*int32 that stays nil on auto-created scalers, and the apiserver's scale
handler cannot extract a missing JSONPath. Fix in a follow-up commit.
* fix(disaggregatedset): materialise scaler spec.replicas so HPA can attach
Kubernetes' CRD /scale handler extracts .spec.replicas at GET time and
returns HTTP 500 ("the spec replicas field does not exist") when the
JSONPath resolves to nothing. HPA reads /scale before its first write to
learn the current replica count, so a scaler created with spec.replicas
unset deadlocks the HPA loop in AbleToScale=False / FailedGetScale.
Repro'd end-to-end via the hpa_test.go GET /scale spec added in the
previous commit. Root-cause analysis and reproducer:
https://github.com/yankay/lws/issues/15
Changes:
- DisaggregatedSetRoleScalerSpec.Replicas: *int32 -> int32 with
+kubebuilder:default=0. The field is now always materialised at rest.
- ScalerManager.Reconcile takes a seedFor callback and passes the seed
into ScalerManager.create so a newly-created scaler carries an explicit
initial value. The DS controller supplies the aggregate LWS spec.replicas
for the role so a Static->External flip on a running role does not drain
it to 0.
- getTargetReplicas: drop the nil branch and always read scaler.Spec.Replicas.
- setReady / statusEqual: replaced with apimeta.SetStatusCondition. The
Ready condition is now always True once the controller has observed the
scaler; the WaitingForScaler-via-nil semantic went away with the type
change. Also addresses the copilot bot review comments about stale
observedGeneration.
Tests:
- Unit resolution matrix updated for the new type; scaler-missing case
still exercises the transient fallback to currentNew.
- test/e2e/disaggregatedset/hpa_test.go all three specs pass.
* docs(disaggregatedset): mark scaler spec/status as +optional
Addresses Copilot review on PR #922: the reference docs generated by
genref were labelling both spec and status on DisaggregatedSetRoleScaler
as [Required] because the struct fields carried neither +optional markers
nor field descriptions. Both were misleading — status is server-populated
and spec has all-optional fields with defaults.
Adds +optional and a one-line description on Spec, Status, and ObjectMeta
to match the shape used on DisaggregatedSet itself. Regenerated docs drop
the [Required] labels and populate the description column.
* chore: unblock CI on merged upstream/main
Two pre-existing issues surfaced by the upstream merge — fixing here so
this PR can be merged; both are worth their own upstream PRs eventually.
- site/content/en/docs/installation/_index.md: CHART_VERSION=0.8.0 was
hard-coded on the "safe Helm upgrade path" snippet added by #883.
pull-lws-verify-main runs a sed pattern that rewrites
CHART_VERSION=X.Y.Z to the current release (v0.9.0) and asserts a clean
diff, so any older number tanks verify. Bumped to 0.9.0.
- go.mod: google.golang.org/grpc v1.79.3 → v1.82.1 to clear
GHSA-hrxh-6v49-42gf (xDS RBAC + HTTP/2 vulns). Trivy DB started
reporting this HIGH advisory 2026-07-22, after upstream/main's last
push. The bump also pulls oauth2 v0.36.0 and genproto through
go mod tidy.
* fix(disaggregatedset): seed fresh External scaler at 1 so vanilla HPA can attach
Vanilla HPA v2 parks in ScalingDisabled=True when it reads current=0 from
/scale, regardless of minReplicas — scale-from-zero requires the
HPAScaleToZero alpha feature gate or a scaler that has its own scale-
from-zero path (KEDA). The previous seed of 0 for a fresh External role
therefore deadlocked the vanilla-HPA bootstrap.
Change seedForRole to return 1 when no LWS exists for the role, and keep
the aggregate LWS spec.replicas seed for Static->External flips (still
correctly preserving 0 for a role that was Static at 0).
KEP-849 prose updated in three places to match:
- Proposal: describe the seed-at-creation behavior explicitly.
- Story 1 (vanilla HPA): drop the "bootstraps from zero" claim and
reference the seeding note.
- Risks and Mitigations: rewrite the "held at 0" bullet to describe
seed-at-1 for fresh roles and current-count for Static->External.
Reported by @yankay at https://github.com/yankay/lws/issues/16
* fix(disaggregatedset): count pods (groups × size) in scaler status.replicas
KEP-849 specifies scaler.status.replicas as the observed pod count, but
LWS status.replicas counts groups. When leaderWorkerTemplate.size > 1
the two diverge and vanilla HPA's per-pod-metric averaging inflates by
size: HPA divides the metric sum by the reported count (groups) while
the selector matches groups × size pods. The KEDA external-metrics path
is unaffected because it does not average per pod.
Multiply LWS status.replicas by leaderWorkerTemplate.size (default 1)
when aggregating across revisions.
Reported by @yankay at https://github.com/yankay/lws/issues/17
* docs(kep-849): describe the External+replicas>1 admission warning correctly
The Risks and Mitigations section claimed a per-role CEL rule forbidding
spec.replicas > 0 when scaling.mode == External. That rule was never
implemented and is not implementable as-is: LeaderWorkerSetSpec.Replicas
carries +kubebuilder:default=1, and API-server defaulting runs before
CEL, so every External role would arrive at CEL validation with
spec.replicas == 1 and be rejected. The impl uses a webhook admission
warning (not a rejection) when an explicit value > 1 is set. Rewrite
the paragraph to describe the actual behavior and the defaulting reason
that forced the choice.
* fix(disaggregatedset): scale on leader pods; supersedes 4c988d2
The prior fix multiplied status.replicas by leaderWorkerTemplate.size to
report pod count. That breaks HPA's ratio math when size > 1: HPA writes
spec.replicas in units of LWS groups (that's what the controller forwards
to LWS.spec.replicas), but reads status.replicas in units of pods. HPA
computes desiredReplicas = currentReplicas × (currentMetric/targetMetric)
without knowing the units differ, so a scale event lands in the wrong unit
and the loop diverges — e.g. HPA reads current=6 (pods), computes
desired=12, writes 12 to spec.replicas (which the controller creates as
12 groups = 36 pods), then next tick reads 36 and continues overshooting.
Correct approach: keep status.replicas as LWS groups (matching spec.replicas)
and refine status.selector to match one pod per group (the leader), so
HPA's per-pod-metric averaging divides its metric sum by the group count.
Selector becomes:
disaggregatedset.x-k8s.io/name=<ds>,
disaggregatedset.x-k8s.io/role=<role>,
leaderworkerset.sigs.k8s.io/worker-index=0
This mirrors LWS's own /scale semantics (also leader-only). Scaling is
now driven by the leader's per-pod metric — the right signal for the
common LWS shape where the leader handles ingress and workers are
downstream compute.
KEP-849 prose updated in three places (Scale Subresource Status Fields,
Rolling Update Interaction, Interaction with Slices) to describe the
leader-only selector and the group-unit invariant.
Reported by @yankay at https://github.com/yankay/lws/issues/17
* address non-blocking review follow-ups
Two small fixes flagged in the review:
1. Add missing Helm manager RBAC for disaggregatedsetrolescalers and
disaggregatedsetrolescalers/status. The kubebuilder-generated
config/rbac/role.yaml already grants these (via the +kubebuilder:rbac
markers on the DS controller), but the Helm chart's clusterrole
template was still missing them, so chart-installed controllers
would 403 the moment they try to List / Create / UpdateStatus a
DisaggregatedSetRoleScaler. Merged into the existing DS blocks
since verb sets match.
2. Fix DisaggregatedSetRoleScalerSpec.Replicas doc: the comment said
the controller seeds a fresh role at 0, but the implementation
(seedForRole in disaggregatedset_controller.go) actually seeds at
1 to keep vanilla HPA from parking in ScalingDisabled. Updated the
source comment plus the generated CRD YAMLs (config/crd/bases and
charts/lws/crds) and the generated reference docs
(site/content/en/docs/reference/disaggregatedset.v1.md) so they
all match. M
Mathis Felardos committed
03e24b07c40a44d58e6894be72a9c1ffc7c7ed15
Parent: d78df5e
Committed by GitHub <noreply@github.com>
on 7/29/2026, 3:08:03 PM