Skip to content

feat(modeldeployment): simplify elastic Ray admission - #749

Merged
thxCode merged 4 commits into
mainfrom
fix/elastic-max-dp-admission
Oct 8, 2026
Merged

thxCode merged 4 commits into
mainfrom
fix/elastic-max-dp-admission

Conversation

@thxCode

@thxCode thxCode commented Oct 8, 2026 •

Copy link
Copy Markdown
Collaborator

What type of PR is this?

/kind enhancement
/kind api-change
/area worker

What this PR does / why we need it:

Elastic EP requires a CPU InstanceType and Kueue queue for its Ray head, although the head holds no GPU engine.
GPU members also declare Ray GPU capacity explicitly instead of discovering their allocated devices.

Remove elasticEp.headInstanceType and render the independent head as an auxiliary Pod with no resource requests or limits.
Keep its owner, Service, role identity, shared-memory policy, explicit privilege and uncached creation guard.
Remove --num-gpus from GPU master/worker commands while preserving GPU requests and fixed TP.
Keep --num-gpus=0 on the CPU head and reuse the existing Ray capacity and allocation checks.

Read vLLM's startup --elastic-ep-max-dp-size from extraArgs through the existing parser.
Reject target widths above an explicit capacity and changes to that effective startup capacity after creation.
Omitted capacity preserves legacy admission. Regenerate all API artifacts and update the owning guide and resize spec.

Which issue(s) this PR links to:

Relates #748

Special notes for your reviewer:

  • Existing Elastic Pods retain their templates.
  • Namespace LimitRange or admission webhooks can inject resources independently of the generated template.
  • Starting with vLLM 0.30.0, omitted startup capacity equals initial DP. Set a larger explicit capacity before startup for online expansion.
  • Source permits fixed TP greater than one. Static TP=2 inference previously passed; dynamic TP>1 inference and container GPU auto-discovery still require runtime verification.
  • SGLang ordinary replica scaling remains supported. Its native elastic protocol is not integrated in this profile.
  • make test passed for 99 packages with race, shuffle and coverage. Code/docs lint and post-lint generation passed. Independent Standards and Spec review found no confirmed defects. PR review identified a missing privileged setting; its regression test failed before the fix and now passes. The complete 99-package suite passed again after that fix.
  • No paid cluster or GPU workload was started for this change.

Does this PR introduce a user-facing change?

Elastic Ray heads render as independent auxiliary Pods without InstanceType, queue admission or resource requests and limits. GPU members let Ray discover allocated devices. An explicit --elastic-ep-max-dp-size in extraArgs now bounds elasticEp.width and is fixed at creation.

Signed-off-by: thxCode <thxcode0824@gmail.com>
@gpustack-code-review

gpustack-code-review Bot commented Oct 8, 2026 •

Copy link
Copy Markdown

✅ OpenCodeReview: Review complete: 0 finding(s) across 1 selected item(s).

Reviewed 4163fe4..385ad03 only; earlier commits in this PR were reviewed in a previous run.

Comment thread api/worker/v1alpha1/model_deployment.go Outdated
Signed-off-by: thxCode <thxcode0824@gmail.com>
@thxCode

thxCode commented Oct 8, 2026

Copy link
Copy Markdown
Collaborator Author

Review round 1 read all feedback buckets: 1 submitted review, 2 inline comments and 1 summary comment. All 5 emitted items were checked.

  • API field removal: retained as explicitly requested for an unreleased feature. No optional compatibility entry or migration note is added. Protobuf tag 2 is not reused.
  • Head settings: fixed the role's privileged setting being dropped. The focused test reproduced the failure and now passes. CPU heads previously received no accelerator RuntimeClass; the TIME-WAIT sysctl is limited to SGLang prefills and did not apply to the vLLM takeover head.
  • Termination grace: no behavior change. The old head was a takeover role, which kept Kubernetes' 30-second default when unset; the operator default is also 30 seconds. Explicit role values remain preserved. Regression cases cover both forms.
  • Webhook/API reminders: generated artifacts and 24 capacity admission cases were already included. Generation and lint were rerun for this round.

The corrected head still has no resource requests/limits and no CPU InstanceType or queue dependency. Full CPU validation results are recorded in the PR description. No GPU runtime result is claimed.

Signed-off-by: thxCode <thxcode0824@gmail.com>
Comment thread docs/modules/model-deployment/elastic-ep.md
Comment thread docs/modules/model-deployment/elastic-ep.md Outdated
Signed-off-by: thxCode <thxcode0824@gmail.com>
@thxCode

thxCode commented Oct 8, 2026

Copy link
Copy Markdown
Collaborator Author

Review round 3 read all feedback buckets: 4 submitted reviews, 6 inline comments including prior replies, and 2 conversation comments. The latest review emitted 2 findings. No suppressed findings were present.

  • Version requirement: verified against the vLLM v0.30.0 release commit. The flag exists in its CLI, and the reviewed spec already linked the release source. That link now uses the fixed commit. The 0.30.0 requirement is retained.
  • Omitted capacity: accepted the wording ambiguity. The guide now explicitly scopes this behavior to vLLM 0.30.0 and later. The operator's legacy admission behavior is unchanged.

The usage guide keeps configuration and version requirements. Source evidence stays in the design record, as requested. Documentation lint and API generation were rerun. The runtime source is unchanged from the previously tested branch.

@thxCode
thxCode merged commit 6b82694 into main Oct 8, 2026
8 checks passed
@thxCode
thxCode deleted the fix/elastic-max-dp-admission branch October 8, 2026 06:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant