feat(processor): tenant-scoped model_gateways lookups (route_key_by_tenant) - #630
Conversation
…enant) When multiple tenants share one batch-apiserver, their models may carry identical runtime model names (--served-model-name) pointing at different inference backends. Gateway lookup previously keyed only on the in-request model ID, so two tenants advertising the same model name collapse onto a single model_gateways entry and cross-route. Add an opt-in processor flag `route_key_by_tenant`. When enabled, the gateway lookup key becomes "<tenantID>/<modelID>" — built from the tenant ID the apiserver already persists per job (header X-MaaS-Username by default, remappable via input_headers) — while the request body is still forwarded verbatim, so runtimes keep validating the bare served model name. Default false preserves the exact previous behavior. - worker: routeKey helper shared by both lookup sites - plan-file source: resolve ModelID/objective header via the scoped key - preprocessor: per-model registration check via the scoped key - helm: render route_key_by_tenant when processor.config.routeKeyByTenant Signed-off-by: todayim <74353553+todayim@users.noreply.github.com>
37f39d2 to
f091fe3
Compare
|
can we have an issue created to track this change? |
|
Good point — tracking issue created: #634. Linked it to this PR. |
|
So the core requirement here is: This is an L7 routing problem, so HTTPRoute rules might be a good fit. I haven't validated this on a real cluster yet, but a practical approach might be: 1) Ensure tenant header is passed through by batch-gateway apiserver: 2) Point processor to a shared inference gateway url. 3) Use HTTPRoute to dispatch to differnt inference pool by tenant header Adding a new tenant then becomes adding a new HTTPRoute match rule. no processor code change, and usually no restart. |
|
Thanks @yizhaodev for the detailed write-up! We verified the pass-through chain end to end (apiserver stores configured headers in job tags with the The scenario motivating this PR is a different deployment shape, though: single cluster, multi-tenant (multi-project), with per-namespace entry points — each project has its own gateway/hostname, and there is no shared gateway in front of all InferencePools. In that shape:
So I'd frame these as complementary patterns for different topologies:
Would you be open to keeping the opt-in lookup for the second shape? I'm also happy to add a docs note describing the HTTPRoute pattern as the alternative for single-gateway deployments, so users can pick per topology. |
|
Hi @yizhaodev, following up on the route-key discussion. We confirmed the HTTPRoute approach works for single-gateway clusters, while this tenant-scoped lookup remains needed for per-project/per-namespace entry paths. The PR is CI-green and mergeable; could you please confirm whether you have any remaining concerns or approve the current direction? |
|
@todayim need resolve conflict from main branch first ^ |
|
Unsigned commits detected! Please sign your commits. For instructions on how to set up GPG/SSH signing and verify your commits, please see GitHub Documentation. |
|
Resolved by merging current main in dcb039e. The only conflict was the processor chart values block; both the async-dispatch documentation and routeKeyByTenant setting are preserved. Verified with helm lint, 100 Helm chart tests, and processor config Go tests. |
dcb039e to
ee936ce
Compare
…enant Signed-off-by: todayim <809634488@qq.com> # Conflicts: # charts/batch-gateway/values.yaml
ee936ce to
a07aede
Compare
|
Follow-up: the conflict-resolution merge is now DCO signed and cryptographically verified in a07aede. All required checks are green; the PR remains mergeable with the existing approval. |
|
Thanks @zdtsw for updating the branch. The new head ef2704b is mergeable and the update tree matches the locally verified merge (processor tests, helm lint, and 100 chart tests all pass). The required Pre-commit workflow is currently blocked as action_required with no jobs: https://github.com/llm-d/llm-d-batch-gateway/actions/runs/31375249393. Could you approve the fork workflow run when convenient? |
|
the pr may introduce side effect. Currently the processor assume model ID and endpoint are 1-to-1, it will use model ID as the key for:
Now Only 3) was updated to use the tenant-scoped key ("tenant-a/qwen3"), while 1) and 2) still use the bare model ID ("qwen3"). This mismatch means the other model ID based features are silently disabled. |
|
Additionally, consider replacing the boolean route_key_by_tenant with a route_key_method enum for extensibility: route_key_method: tenant --> // in the future, for any new requirement, e.g |
Why is this PR needed?
In multi-tenant deployments that share a single batch-apiserver, different tenants may serve models under identical runtime model names (
--served-model-name) backed by different inference gateways — e.g. per-InferencePool admission where each tenant's gateway is reached on its own hostname.Today the processor keys
model_gatewayslookups solely on thebody.modelfield of each input line. Two tenants advertising the same model name therefore collapse onto a single map entry: the second configuration silently wins, and one tenant's requests are routed to the other tenant's backend. The only workaround is requiring globally-unique model names, which operators cannot enforce when users bring their own serving images.The apiserver already extracts and persists a tenant ID per job (
X-MaaS-Usernameheader by default, remappable viainput_headers.tenant), and that tenant ID already reaches the processor's job context — but it is not used for gateway selection.What does this PR do?
Adds an opt-in processor flag
route_key_by_tenant. When enabled, the gateway lookup key becomes<tenantID>/<modelID>:model_gatewaysentries keyed accordingly (e.g.team-a/qwen3,team-b/qwen3), each tenant's requests route to its own gateway even though the in-request model name is identical.false: lookups use the bare model ID exactly as before. Existing deployments and configs are unaffected (the flag is needed because the apiserver always assigns a tenant ID, defaulting todefault— an implicit always-on change would silently alter routing for existing users).Implementation:
worker.routeKey(byTenant, tenantID, modelID)— single helper used by both lookup sitessource_planfile.go): resolvesRequestItem.ModelIDand the objective header via the scoped key; plan grouping and error messages keep the raw model namepreprocessor.go): per-model registration check uses the scoped key (the job'sTenantIDis already in scope)route_key_by_tenant: trueinto the processor config whenprocessor.config.routeKeyByTenantis setHow was this tested?
TestRouteKey: disabled / enabled-with-empty-tenant / enabled-scoped casesTestPlanFileSource_Produce_TenantScopedLookup: with the flag on and ateam/modelgateway entry, produced items carry the scoped ModelID while the body keeps the raw model name and the objective header resolves via the scoped key; the same test re-runs with the flag off (default) to assert the bare-model behavior is preservedroute_key_by_tenant; Helm tests cover both default omission and enabled rendering (99/99pass)go test ./...passes;make test-regressionpasses;go vet/gofmtcleanTestExecuteJob_SLOExpiredDuringDispatchandTestCancelInProgressThrottledflake intermittently on both this branch and cleanmain(pre-existing timing sensitivity, unrelated to this change — the code paths they exercise do not touch gateway-key resolution)helm templateverified rendering ofroute_key_by_tenant: trueand slash-containingmodel_gatewayskeys;helm lintpassesChecklist
git commit -s) per DCOmake ci)make test-e2e) — not run locally; relies on CIRelated Issues
Tracks #634 — related to multi-tenant operation of a shared batch-apiserver; complements (does not replace) name uniqueness conventions.