feat(server): resize pools over the admin API, and two things it needed to work - #122
Open
weilei0120 wants to merge 8 commits into
Open
feat(server): resize pools over the admin API, and two things it needed to work#122weilei0120 wants to merge 8 commits into
weilei0120 wants to merge 8 commits into
Conversation
Changing a pool's size means editing `spec.services.<name>.replicas` on the InferaDeployment. That is a kubectl patch -- fine for a human, awkward for the orchestration layer that usually makes these decisions, which then needs cluster credentials and has to know the CR's shape. GET/POST /v1/admin/scale put that same edit behind the router. The CR is what is written: the generated Deployment and LeaderWorkerSet are derived state, rewritten every reconcile pass, so scaling those succeeds, reports no error, and is undone seconds later. Every named pool moves in one patch, so rebalancing prefill against decode cannot half-apply and leave a deployment lopsided until someone notices. The read reports requested and observed sizes together, which is how a caller tells "not scaled" from "still scaling". Off by default, like profiling: it writes to the cluster and /v1/admin carries no authentication of its own. Scaling a pool to zero is refused -- an empty pool stops serving, and in PD an empty side fails every request rather than only the ones that would have landed there. Retiring a pool stays a kubectl edit, where the intent is unambiguous. The deployment to scale comes from the server's own Pod labels rather than a flag, so the two cannot disagree; a stale name would resize a fleet nobody asked about. The operator's discovery Role gains get/patch on inferadeployments, restricted by ResourceNames to the one it belongs to: every Pod in a deployment shares that identity, and a worker has no business resizing anything. Not a step toward autoscaling. A worker takes about two minutes to become useful, which outlasts most bursts, and Kubernetes offers no way to choose which replica a scale-down removes -- so the one holding the warmest cache is as likely to go as any other. Signed-off-by: leiwei12 <lei.wei@amd.com> Co-authored-by: Cursor <cursoragent@cursor.com>
Kubernetes-native discovery is built on a Pod knowing which Pod it is: a worker registers by patching its own annotation, and the server reads its own labels to find the deployment it belongs to. The operator injects POD_NAME and POD_NAMESPACE for that -- on the path where it renders the pod template itself. A template supplied through extraPodSpec took a different path, which added the server's watch selector and stopped there. Those pods came up without an identity, so worker self-registration could not write its annotation and the scaling API could not find its own CR. Both failed on precisely the deployments the PD example tells people to write, since every service there is an extraPodSpec. Injected for both component types now, and only where absent: a duplicate env name is not an error, the last one wins, and appending ours unconditionally would silently override a value the template's author chose. Signed-off-by: leiwei12 <lei.wei@amd.com> Co-authored-by: Cursor <cursoragent@cursor.com>
This page's claim is that none of its numbers are projected, so the API needs the same treatment as everything else on it: a resize from 1P/2D to 3P/4D reached the Pods within 35s, and reading back mid-flight showed the requested and observed counts disagreeing, which is the whole reason both are reported. The refusals are recorded from the cluster rather than the unit tests, because what matters about a rejected request is that it left nothing behind, and only the cluster can say that. The stand-in caveat is extended rather than repeated: this run used them for a different reason -- the path from request to Pod count does not involve inference at all -- and so it says nothing about a scaled-up worker going on to serve, which the drain measurements above already cover. Signed-off-by: leiwei12 <lei.wei@amd.com> Co-authored-by: Cursor <cursoragent@cursor.com>
A worker binds 0.0.0.0 and advertises something else, because the address it
registers is the one the router dials. Under Kubernetes discovery it takes that
from POD_IP -- the resolution has been there all along and reads the downward
API -- but nothing supplied the variable, so the fallback never fired.
The worker then registered its bind host and looked entirely healthy doing it:
it came up, loaded weights, and appeared in /v1/workers, while the first
inference request came back {"error":"worker 0.0.0.0:8080 unreachable"}. It also
froze the same address into the KV event endpoint.
Injected alongside POD_NAME and POD_NAMESPACE, which it belongs with, on both
the rendered and extraPodSpec paths.
Verified on a cluster with a manifest carrying neither POD_IP nor
--advertise-host: the worker logged "advertising Pod IP 172.16.1.57", registered
that, and served inference. The same deployment previously needed the variable
written by hand.
Signed-off-by: leiwei12 <lei.wei@amd.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
The measurements section reports outcomes -- what ran, what came back. The entries added for the API had drifted into method and rationale: why refusals were exercised against a cluster, what a Pod existing does not prove. That reads as a changelog for the people who wrote it rather than a page for the people running it. Same facts, in the voice of the entries around them. Signed-off-by: leiwei12 <lei.wei@amd.com> Co-authored-by: Cursor <cursoragent@cursor.com>
Both ways of resizing a pool write the InferaDeployment, which a deployment outside Kubernetes does not have: there scaling is starting and stopping worker processes, as the earlier sections describe. The section on editing the CR now says which deployments it is about, and the API note says what a server that the operator did not create answers. Signed-off-by: leiwei12 <lei.wei@amd.com> Co-authored-by: Cursor <cursoragent@cursor.com>
A reader who has just watched two workers register will want a third. On this page's path that is running step 3 again; on Kubernetes it is a call, so both are here rather than only the one this page can demonstrate -- with the note that the API answers 409 on the etcd path, which is exactly where a reader following along would try it. The three replica counts get their own list. They are the reason the read is worth making: a Pod exists in seconds and serves in minutes, and only ready_replicas says which has happened. Signed-off-by: leiwei12 <lei.wei@amd.com> Co-authored-by: Cursor <cursoragent@cursor.com>
The quickstart had grown a second copy of the API's request and response shapes, which is the kind of duplication that goes stale on one side without anyone noticing. It now says what scaling means on the path it demonstrates, and links to the feature page for the path it cannot -- naming the reason, since a reader following that page is on the setup where the API answers 409. The feature page gains what the quickstart had that it lacked: the response shape, the three replica counts and why they differ, and the refusals it only described in prose. Signed-off-by: leiwei12 <lei.wei@amd.com> Co-authored-by: Cursor <cursoragent@cursor.com>
weilei0120
requested review from
JohnQinAMD,
jiejingzhangamd,
limou102 and
xiaobochen-amd
as code owners
August 17, 2026 10:37
Contributor
There was a problem hiding this comment.
Pull request overview
This PR introduces an opt-in admin API for resizing InferaDeployment service pools via the router/server, and makes operator changes needed for Kubernetes-native discovery/registration to work reliably (especially when using extraPodSpec).
Changes:
- Add
GET/POST /v1/admin/scale(guarded by--enable-scaling-api) to read and update per-service replica counts by patching theInferaDeployment. - Fix operator-rendered Pod templates to always provide required downward-API identity (
POD_NAME,POD_NAMESPACE) and routable address (POD_IP), including theextraPodSpecpath. - Expand docs and tests covering scaling semantics, RBAC scoping, and the new HTTP surface.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/unit/server/test_scaling.py | Adds unit tests for scaler behavior (atomic multi-pool patching, validation) and FastAPI endpoints. |
| manual/getting_started/quickstart.md | Adds a “Scale it” section and links to scaling documentation. |
| manual/features/scaling.md | Documents scaling over the admin API, response shape, and constraints (e.g., refusing zero). |
| infera/server/scaling.py | Implements DeploymentScaler and validation for patching InferaDeployment replicas via the in-cluster API. |
| infera/server/args.py | Adds --enable-scaling-api flag (and env var default) to gate the new endpoints. |
| infera/server/app.py | Wires new /v1/admin/scale routes and integrates a DeploymentScaler instance into app initialization. |
| infera/server/main.py | Instantiates and injects the scaler when scaling API is enabled. |
| deploy/operator/internal/controller/scale_paths_test.go | Adds tests asserting discovery Role can patch only its own InferaDeployment (ResourceNames-scoped). |
| deploy/operator/internal/controller/builders.go | Injects POD_IP and ensures identity env vars are present for extraPodSpec; adds inferadeployments RBAC rule. |
| deploy/operator/internal/controller/builders_test.go | Adds tests for extraPodSpec identity injection and POD_IP downward-API wiring. |
Suppressed comments (1)
infera/server/scaling.py:148
- Same as snapshot():
make_client()can raise OSError when in-cluster credentials are missing, which currently escapes as a 500 because only ScalingError is translated by the FastAPI route. Catch OSError around the client creation and return a ScalingError with a clear message/status.
async with make_client() as client:
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+78
to
+84
| resp = await client.get(f"/api/v1/namespaces/{self._namespace}/pods/{self._pod_name}") | ||
| if resp.status_code == 403: | ||
| raise ScalingError( | ||
| "the router's ServiceAccount cannot read its own Pod; re-apply the operator RBAC", | ||
| status=403, | ||
| ) | ||
| resp.raise_for_status() |
Comment on lines
+120
to
+122
| async with make_client() as client: | ||
| name = await self._resolve_deployment(client) | ||
| cr = await self._fetch(client, name) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Where this started
Three questions about scaling a PD deployment, and what the answers turned out
to be:
Can the whole thing scale at once? Partly. Each service has its own
replicas, and for a multi-node service that count is groups rather than Pods.There is no total, and no P:D ratio — 2P4D means setting two numbers.
Can prefill and decode scale independently? Yes, fully. Separate pools,
selected per request. Neither can go to zero: PD dispatch fails closed.
Is there an API? There was not.
kubectl patchon the CR was the only wayin. This adds one.
The API
GET/POSTon/v1/admin/scale, off unless--enable-scaling-api.It writes the
InferaDeployment, which is the only write that survives: thegenerated Deployment and LeaderWorkerSet are derived state, rewritten every
reconcile pass, so scaling those succeeds, reports no error, and is undone
seconds later.
Reads report three counts per pool —
replicas(asked for),current_replicas(Pods that exist),
ready_replicas(workers serving). They converge left toright, and the gap is the point: a Pod appears in seconds and serves in minutes.
Three decisions worth reviewing
Every pool named in one request moves in one write. Rebalancing prefill
against decode cannot half-apply and leave a deployment lopsided until someone
notices.
Zero is refused. An empty pool stops serving, and in PD an empty side fails
every request rather than only the ones that would have landed there.
Retiring a pool stays a
kubectledit, where the intent is unambiguous. Unknownservice names and negative counts are refused the same way, and a refused
request writes nothing.
The deployment comes from the server's own Pod labels, not a flag. The two
cannot disagree that way; a stale name would resize a fleet nobody asked about.
The operator's discovery Role gains
get/patchoninferadeployments,restricted by
resourceNamesto the one it belongs to — every Pod in adeployment shares that identity, and a worker has no business resizing anything.
Two operator fixes this uncovered
Both predate this branch and both broke Kubernetes-native discovery on the
deployments the PD example tells people to write, since every service there uses
extraPodSpec.extraPodSpecpods had no identity.POD_NAMEandPOD_NAMESPACEwereinjected on the path where the operator renders the template itself; a template
supplied by the user took a different path and got only the watch selector. A
worker cannot patch its own annotation to register without them.
Nothing supplied
POD_IP. A worker binds0.0.0.0and advertises somethingelse, resolving it from
POD_IP— logic that has been there all along, readinga variable nobody set. So it registered its bind address and looked healthy
doing it: up, weights loaded, listed in
/v1/workers, and the first inferencerequest came back
{"error":"worker 0.0.0.0:8080 unreachable"}.Not in scope
No autoscaler. This is somewhere to push a decision that has already been made,
not a control loop — nothing here watches load. A worker takes about two minutes
to become useful, which outlasts most bursts, and Kubernetes offers no way to
choose which replica a scale-down removes, so the one holding the warmest cache
is as likely to go as any other.
An
InferaDeploymentalso cannot carry a Kubernetes/scalesubresource:spec.servicesis a map with user-chosen keys, whilespecReplicasPathmust bea static JSONPath and a CRD may declare only one. Naming the service in the
request sidesteps that.
Test plan
tests/unit/server/test_scaling.py) and 5 operator tests;full suites pass. The 6 remaining Python failures are a pre-existing
baseline (sglang/gaie not installed), confirmed unchanged against
main.call: CR updated immediately, Pods within 35 s, and the read distinguished
requested from observed during the window. Scaling to zero, an unknown
service and a negative count were each refused with the CR unchanged; a
server without the flag answered 403. The generated Role was read back and
carried
get/patchscoped to the single deployment.and back over the API. The added worker loaded weights, registered, and
served 4 of the 8 requests issued after it joined — a Pod existing is
not the same as a worker answering.
POD_IPnor--advertise-host: the worker loggedadvertising Pod IP 172.16.1.57,registered that, and served inference. The same deployment previously
needed the variable written by hand.
Made with Cursor