[Feat] Add multi-node multiprocessing deployment via LeaderWorkerSet - #1007
[Feat] Add multi-node multiprocessing deployment via LeaderWorkerSet#1007ardecode wants to merge 5 commits into
Conversation
…rSet Adds a first-class multi-node path for the serving engine that uses vLLM's multiprocessing (MP) distributed backend instead of Ray, addressing vllm-project#930. A new modelSpec.lwsSpec renders a LeaderWorkerSet (leaderworkerset.x-k8s.io/v1): the leader (node-rank 0) serves the OpenAI API and workers run with --headless, wired via the LWS-injected LWS_GROUP_SIZE / LWS_WORKER_INDEX / LWS_LEADER_ADDRESS values (--nnodes / --node-rank / --master-addr). Works with the stock vllm/vllm-openai image; Ray is not required. LWS-enabled models are excluded from the plain Deployment and KEDA paths, and only the leader carries the engine discovery labels so the router targets it. Signed-off-by: ardecode <desaiarijit@gmail.com>
There was a problem hiding this comment.
Code Review
This pull request introduces support for deploying models across multiple nodes using Kubernetes LeaderWorkerSet (LWS) and vLLM's multiprocessing distributed backend as an alternative to Ray. It adds the new lws-vllm.yaml template and updates values, schemas, and other templates to support this deployment mode. The review feedback highlights several critical feature parity gaps and integration issues in the new LWS template, including missing LMCache configurations and environment variables, lack of support for emptyDir in pvcStorage, missing support for sharedPvcStorage, and several missing CLI arguments and environment variables (such as envFromSecret, --runner, --convert, and explicit prefix caching flags).
| {{- if $modelSpec.chatTemplate }} | ||
| - "--chat-template" | ||
| - {{ $modelSpec.chatTemplate | quote }} | ||
| {{- end }} |
There was a problem hiding this comment.
LMCache integration is completely broken in LWS mode because the --kv-transfer-config argument is not passed to the vllm serve command. Add the configuration argument to ensure LMCache is initialized correctly.
{{- if $modelSpec.chatTemplate }}
- "--chat-template"
- {{ $modelSpec.chatTemplate | quote }}
{{- end }}
{{- $kv_role := "kv_both" }}
{{- if and $modelSpec.lmcacheConfig $modelSpec.lmcacheConfig.kvRole }}
{{- $kv_role = $modelSpec.lmcacheConfig.kvRole }}
{{- end }}
{{- if $modelSpec.lmcacheConfig }}
{{- if $modelSpec.lmcacheConfig.enabled }}
{{- if hasKey $modelSpec.lmcacheConfig "enablePD" }}
- "--kv-transfer-config"
- '{"kv_connector":"LMCacheConnectorV1","kv_role":"{{ $kv_role }}","kv_connector_extra_config":{"discard_partial_chunks": false, "lmcache_rpc_port": {{ $modelSpec.lmcacheConfig.nixlRole | quote }}}}'
{{- else if and $modelSpec.vllmConfig (hasKey $modelSpec.vllmConfig "v0") (eq (toString $modelSpec.vllmConfig.v0) "1") }}
- "--kv-transfer-config"
- '{"kv_connector":"LMCacheConnector","kv_role":"{{ $kv_role }}"}'
{{- else }}
- "--kv-transfer-config"
- '{"kv_connector":"LMCacheConnectorV1","kv_role":"{{ $kv_role }}"}'
{{- end }}
{{- end }}
{{- end }}| {{- with $root.Values.servingEngineSpec.env }} | ||
| {{- toYaml . | nindent 4 }} | ||
| {{- end }} | ||
| {{- with $modelSpec.env }} | ||
| {{- toYaml . | nindent 4 }} | ||
| {{- end }} |
There was a problem hiding this comment.
| {{- if hasKey $modelSpec "pvcStorage" }} | ||
| - name: {{ $root.Release.Name }}-storage | ||
| persistentVolumeClaim: | ||
| claimName: "{{ $root.Release.Name }}-{{ $modelSpec.name }}-storage-claim" | ||
| {{- end }} |
There was a problem hiding this comment.
The pvcStorage configuration can be defined as an emptyDir map in values.yaml (similar to how it is handled in deployment-vllm-multi.yaml). Currently, lws-vllm.yaml assumes pvcStorage is always a persistent volume claim, which will cause the LeaderWorkerSet deployment to fail if emptyDir is used.
{{- if hasKey $modelSpec "pvcStorage" }}
- name: {{ $root.Release.Name }}-storage
{{- if and (kindIs "map" $modelSpec.pvcStorage) (hasKey $modelSpec.pvcStorage "emptyDir") }}
emptyDir:
{{- toYaml $modelSpec.pvcStorage.emptyDir | nindent 4 }}
{{- else }}
persistentVolumeClaim:
claimName: "{{ $root.Release.Name }}-{{ $modelSpec.name }}-storage-claim"
{{- end }}
{{- end }}| {{- if and $modelSpec.vllmConfig (hasKey $modelSpec.vllmConfig "tensorParallelSize") }} | ||
| - name: shm | ||
| emptyDir: | ||
| medium: Memory | ||
| sizeLimit: {{ default "20Gi" $modelSpec.shmSize }} | ||
| {{- end }} |
There was a problem hiding this comment.
Add support for sharedPvcStorage in chart.lwsVolumes to ensure feature parity with the standard Deployment template. This allows LWS pods to mount the shared storage volume when enabled.
{{- if $root.Values.sharedPvcStorage.enabled }}
- name: {{ $root.Release.Name }}-shared-pvc-storage
persistentVolumeClaim:
claimName: "{{ $root.Release.Name }}-shared-pvc-storage-claim"
{{- end }}
{{- if and $modelSpec.vllmConfig (hasKey $modelSpec.vllmConfig "tensorParallelSize") }}
- name: shm
emptyDir:
medium: Memory
sizeLimit: {{ default "20Gi" $modelSpec.shmSize }}
{{- end }}| imagePullSecrets: | ||
| - name: {{ $modelSpec.imagePullSecret }} | ||
| {{- end }} | ||
| {{- $vols := or (hasKey $modelSpec "pvcStorage") (and $modelSpec.vllmConfig (hasKey $modelSpec.vllmConfig "tensorParallelSize")) $modelSpec.extraVolumes }} |
There was a problem hiding this comment.
Include sharedPvcStorage.enabled in the $vols check to ensure volumes are rendered when shared storage is enabled.
{{- $vols := or (hasKey $modelSpec "pvcStorage") (and $modelSpec.vllmConfig (hasKey $modelSpec.vllmConfig "tensorParallelSize")) $modelSpec.extraVolumes $root.Values.sharedPvcStorage.enabled }}| - name: HF_HOME | ||
| {{- if hasKey $modelSpec "pvcStorage" }} | ||
| value: /data | ||
| {{- else }} | ||
| value: /tmp | ||
| {{- end }} |
There was a problem hiding this comment.
Configure HF_HOME to point to the shared PVC storage path when sharedPvcStorage is enabled, matching the behavior in deployment-vllm-multi.yaml.
- name: HF_HOME
{{- if $root.Values.sharedPvcStorage.enabled }}
value: /data/shared-pvc-storage
{{- else if hasKey $modelSpec "pvcStorage" }}
value: /data
{{- else }}
value: /tmp
{{- end }}| {{- $mountVolumes := or (hasKey $modelSpec "pvcStorage") (and $modelSpec.vllmConfig (hasKey $modelSpec.vllmConfig "tensorParallelSize")) $modelSpec.extraVolumeMounts }} | ||
| {{- if $mountVolumes }} | ||
| volumeMounts: | ||
| {{- if hasKey $modelSpec "pvcStorage" }} | ||
| - name: {{ $root.Release.Name }}-storage | ||
| mountPath: /data | ||
| {{- end }} |
There was a problem hiding this comment.
Mount the shared PVC storage volume in the container volume mounts when sharedPvcStorage is enabled.
{{- $mountVolumes := or (hasKey $modelSpec "pvcStorage") (and $modelSpec.vllmConfig (hasKey $modelSpec.vllmConfig "tensorParallelSize")) $modelSpec.extraVolumeMounts $root.Values.sharedPvcStorage.enabled }}
{{- if $mountVolumes }}
volumeMounts:
{{- if hasKey $modelSpec "pvcStorage" }}
- name: {{ $root.Release.Name }}-storage
mountPath: /data
{{- end }}
{{- if $root.Values.sharedPvcStorage.enabled }}
- name: {{ $root.Release.Name }}-shared-pvc-storage
mountPath: /data/shared-pvc-storage
{{- end }}| {{- if $root.Values.servingEngineSpec.configs }} | ||
| envFrom: | ||
| - configMapRef: | ||
| name: "{{ $root.Release.Name }}-configs" | ||
| {{- end }} |
There was a problem hiding this comment.
Add support for envFromSecret in envFrom to allow loading environment variables from secrets in LWS mode.
{{- if or $root.Values.servingEngineSpec.configs $modelSpec.envFromSecret }}
envFrom:
{{- if $root.Values.servingEngineSpec.configs }}
- configMapRef:
name: "{{ $root.Release.Name }}-configs"
{{- end }}
{{- if $modelSpec.envFromSecret }}
- secretRef:
name: {{ $modelSpec.envFromSecret.name }}
{{- end }
{{- end }}| {{- if hasKey . "maxLoras" }} | ||
| - "--max-loras" | ||
| - {{ .maxLoras | quote }} | ||
| {{- end }} | ||
| {{- range .extraArgs }} | ||
| - {{ . | quote }} | ||
| {{- end }} |
There was a problem hiding this comment.
Add support for --runner and --convert command arguments in chart.lwsServeCommand to match the configuration options available in the standard Deployment template.
{{- if hasKey . "maxLoras" }}
- "--max-loras"
- {{ .maxLoras | quote }}
{{- end }}
{{- if hasKey . "runner" }}
- "--runner"
- {{ .runner | quote }}
{{- end }}
{{- if hasKey . "convert" }}
- "--convert"
- {{ .convert | quote }}
{{- end }}
{{- range .extraArgs }}
- {{ . | quote }}
{{- end }}| {{- if .enablePrefixCaching }} | ||
| - "--enable-prefix-caching" | ||
| {{- end }} |
There was a problem hiding this comment.
…arity for LWS Bring the LeaderWorkerSet template to feature parity with deployment-vllm-multi.yaml per PR review: - wire LMCache --kv-transfer-config and the full LMCACHE_* env block - support sharedPvcStorage (volume, mount, HF_HOME) and pvcStorage emptyDir - add envFromSecret, --runner/--convert, and explicit --no-enable-prefix-caching Signed-off-by: ardecode <desaiarijit@gmail.com>
c664751 to
5856883
Compare
|
@ruizhang0101 , ready for review! |
Summary
Implements first-class multi-node multiprocessing (MP) deployment support.
Fixes #930.
The only distributed path today is
ray-cluster.yaml, which requires the KubeRay operator and hardcodes--distributed-executor-backend ray. Since vLLM v0.19.0 dropped Ray from the default image, this adds an MP alternative modeled on the upstream vLLM LeaderWorkerSet example (vllm-project/vllm#39400).Changes
modelSpec.lwsSpec(enabled,size,restartPolicy) +values.schema.jsonentryhelm/templates/lws-vllm.yamlrendering aLeaderWorkerSet:--headless--nnodes/--node-rank/--master-addrfrom$(LWS_GROUP_SIZE)/$(LWS_WORKER_INDEX)/$(LWS_LEADER_ADDRESS)raySpec)Notes
helm template,helm lint, and structural YAML checks; not yet exercised on a live multi-node GPU cluster.Checklist
[Feat])pre-commit run --all-files; I could not run thehelm-schemagenerator locally)