fix: do not inject default port into vllm if it is set in the prefill or decode

[zdtsw]

Description

• before this PR, it always inject 8000 for prefill and 8000 for sidecar 8200 for decode main, even we set it in the container.args with "--port"
• this only apply to modelCommand is called vllmServe or imageDefault

Ref

it was from a converstaion from llm-d/llm-d#605 (comment)
it should only get --port 8300 not both in the result

Test

values.yaml

modelArtifacts:
  name: test-model
  uri: "hf://microsoft/DialoGPT-small"

# Prefill pod with custom port to test the fix
prefill:
  create: true
  replicas: 1
  containers:
  - name: "vllm"
    image: ghcr.io/llm-d/llm-d-xpu:v0.2.0
    modelCommand: vllmServe  # Uses the fixed template
    args:
      - "--port"
      - "8300"  # Custom port - should NOT get duplicate with default 8000
    resources:
      limits:
        memory: 16Gi
        cpu: "4"
    mountModelVolume: true

# Decode pod with default port (no custom --port)
decode:
  create: true
  replicas: 1
  containers:
  - name: "vllm"
    image: ghcr.io/llm-d/llm-d-xpu:v0.2.0
    modelCommand: vllmServe
    resources:
      limits:
        memory: 16Gi
        cpu: "4"
    mountModelVolume: true

result

...
apiVersion: apps/v1
kind: Deployment
metadata:
  name: test-port-fix-llm-d-modelservice-prefill
  labels:
    helm.sh/chart: llm-d-modelservice-v0.4.0
    app.kubernetes.io/version: "v0.3.0"
    app.kubernetes.io/managed-by: Helm
spec:
  replicas: 1
  selector:
    matchLabels:
      llm-d.ai/inferenceServing: "true"
      llm-d.ai/model: random_model
      llm-d.ai/role: prefill
  template:
    metadata:
      labels:
        llm-d.ai/inferenceServing: "true"
        llm-d.ai/model: random_model
        llm-d.ai/role: prefill
    spec:
    
      serviceAccountName: test-port-fix-llm-d-modelservice
      
      volumes:
        - emptyDir: {}
          name: metrics-volume
      
        - name: model-storage
          emptyDir:
            sizeLimit: 5Mi
        
      
      containers:
        - name: vllm
          image: ghcr.io/llm-d/llm-d-xpu:v0.2.0
          
          command: ["vllm", "serve"]
          args:
            - microsoft/DialoGPT-small
            - --served-model-name
            - "test-model"
            
            
            - --port
            - "8300"                    # use new port value, no 8000
          env:
          - name: DP_SIZE
            value: "1"
          - name: TP_SIZE
            value: "1"
          - name: DP_SIZE_LOCAL
            value: "1"
          
          - name: HF_HOME
            value: /model-cache
          
          
          resources:
            limits:
              cpu: "4"
              memory: 16Gi
              nvidia.com/gpu: "1"
            requests:
              nvidia.com/gpu: "1"
          
          volumeMounts:
            - name: model-storage
              mountPath: /model-cache

[zdtsw]

@jgchn / @kalantar if you have time to take a look at this?

[jgchn]

I think this change LGTM. I'm wondering if it's necessary to expose this port in ports.containerPort? Or should that be left to the user to declare if necessary?

[zdtsw]

I thought about this before, the reason did not go that way:
if user want to use a different port in prefill, they need set it explictliy in the prefill.containers with both args and ports. surely this can be done on the fly by chart, but then the logic will be more complicated. e.g if it is not vllmServer.
if we add a new spec like prefill.port that normally wont be useful without prefill.containers.args set accordingly.

[kalantar]

port can be set using routing.servicePort and proxy.targetPort, can't it?

[zdtsw]

port can be set using routing.servicePort and proxy.targetPort, can't it?

If i understand this correctly:
when we enable P/D,

• routing.servicePort is used for prefill pod
• routing.proxy.targetport is for main contianer of decode pod
• rrouting.servicePort is also used for sidecar in decode pod

the current case/example is: prefill on 8300, sidecar on 8000, decode on 8200

[kalantar]

port can be set using routing.servicePort and proxy.targetPort, can't it?

If i understand this correctly: when we enable P/D,

* `routing.servicePort` is used for prefill pod

* ` routing.proxy.targetport` is for main contianer of decode pod

* `rrouting.servicePort` is also used for sidecar in decode pod

the current case/example is: prefill on 8300, sidecar on 8000, decode on 8200

Your understanding of how the variables are used is correct. However, I don't understand the configuration that allows the prefill pod to have a port different from the sidecar (ie, the external port of the decode pod). Can you explain the configuration of the components that enables this?

[zdtsw]

port can be set using routing.servicePort and proxy.targetPort, can't it?

If i understand this correctly: when we enable P/D,

* `routing.servicePort` is used for prefill pod

* ` routing.proxy.targetport` is for main contianer of decode pod

* `rrouting.servicePort` is also used for sidecar in decode pod

the current case/example is: prefill on 8300, sidecar on 8000, decode on 8200

Your understanding of how the variables are used is correct. However, I don't understand the configuration that allows the prefill pod to have a port different from the sidecar (ie, the external port of the decode pod). Can you explain the configuration of the components that enables this?

The idea is
if user want to use 8300 for prefill, and keep 8000/8200 for decode, they could do something like this in values.yaml

prefill:
  create: true
  replicas: 1
  containers:
  - name: "vllm"
    image: ghcr.io/llm-d/llm-d-xpu:v0.2.0
    modelCommand: vllmServe                                # need vllmServe
    args:
      - "--port"
      - "8300"                                                                 # Custom port
    resources:
      limits:
        memory: 16Gi
        cpu: "4"
    mountModelVolume: true
    ports: 
      - containerPort: 8300                                         # Custom port
        protocal: TCP

the change in the PR, is mainly to use the value if args. has --port set there
the ports.containerPort comes from user, this PR does not handle that part

[kalantar]

There must be configuration changes on the sidecar and endpointpicker as well, no? It is this configuration that makes me think the ports can't be different.

[jgchn]

@kalantar @zdtsw is that the --port argument in the sidecar?

llm-d-modelservice/charts/llm-d-modelservice/templates/_helpers.tpl

Line 99 in 29e3dcb

- --port={{ default 8000 .servicePort }}

.

I also found an example here:

• https://github.com/llm-d/llm-d-routing-sidecar/blob/78051b86ff13c4511fb1c8c7def56e04b0850b67/test/config/nixl/qwen-decoder-pod.yaml#L27
• https://github.com/llm-d/llm-d-routing-sidecar/blob/78051b86ff13c4511fb1c8c7def56e04b0850b67/test/config/nixl/qwen-prefiller-pod.yaml#L28

Or is --port just the port that the sidecar is exposed on?

[zdtsw]

@kalantar @zdtsw is that the --port argument in the sidecar?

llm-d-modelservice/charts/llm-d-modelservice/templates/_helpers.tpl

Line 99 in 29e3dcb

- --port={{ default 8000 .servicePort }}

.
I also found an example here:

• https://github.com/llm-d/llm-d-routing-sidecar/blob/78051b86ff13c4511fb1c8c7def56e04b0850b67/test/config/nixl/qwen-decoder-pod.yaml#L27
• https://github.com/llm-d/llm-d-routing-sidecar/blob/78051b86ff13c4511fb1c8c7def56e04b0850b67/test/config/nixl/qwen-prefiller-pod.yaml#L28

Or is --port just the port that the sidecar is exposed on?

Sorry, I must have missed your comments.
from sidecar these two args --port 8000 is for the inbound to rounting, and --vllm-port 8001 (i think we normally use 8200) for decode vllm container. . this matches decode example
as for the prefill example , --port 8000 is the one in my case 8300 should be configable.

as for the llm-d-modelservice/charts/llm-d-modelservice/templates/_helpers.tpl yes this is the servicePort which is being used for both prefill port and decode routing port. https://github.com/llm-d-incubation/llm-d-modelservice/blob/main/charts/llm-d-modelservice/templates/_helpers.tpl#L221-L231

Probably two options:

1. as in this PR, user need to provide the port they want for prefill by passing both --args to vllm and containerports. Basically it is all on themselves. but chart should not set default --port 8000 there
2. could be done by adding a new parameter e.g prefillServicePort. some logic like, if it is not set by reuse the same port as the current servicePort, if explicitly set then use this value.

routing:
    servicePort: 8000        # routing proxy listens here
    proxy:
      targetPort: 8200       # vLLM container listens here (decode only)
   ...
prefill:
    servicePort: 8300  # new field
    ...
``` still we should not inject `--port 8000`

[zdtsw]

@kalantar @zdtsw is that the --port argument in the sidecar?

llm-d-modelservice/charts/llm-d-modelservice/templates/_helpers.tpl

Line 99 in 29e3dcb

- --port={{ default 8000 .servicePort }}

.
I also found an example here:

• https://github.com/llm-d/llm-d-routing-sidecar/blob/78051b86ff13c4511fb1c8c7def56e04b0850b67/test/config/nixl/qwen-decoder-pod.yaml#L27
• https://github.com/llm-d/llm-d-routing-sidecar/blob/78051b86ff13c4511fb1c8c7def56e04b0850b67/test/config/nixl/qwen-prefiller-pod.yaml#L28

Or is --port just the port that the sidecar is exposed on?

Sorry, I must have missed your comments. from sidecar these two args --port 8000 is for the inbound to rounting, and --vllm-port 8001 (i think we normally use 8200) for decode vllm container. . this matches decode example as for the prefill example , --port 8000 is the one in my case 8300 should be configable.

as for the llm-d-modelservice/charts/llm-d-modelservice/templates/_helpers.tpl yes this is the servicePort which is being used for both prefill port and decode routing port. https://github.com/llm-d-incubation/llm-d-modelservice/blob/main/charts/llm-d-modelservice/templates/_helpers.tpl#L221-L231

Probably two options:

1. as in this PR, user need to provide the port they want for prefill by passing both --args to vllm and containerports. Basically it is all on themselves. but chart should not set default --port 8000 there
2. could be done by adding a new parameter e.g prefillServicePort. some logic like, if it is not set by reuse the same port as the current servicePort, if explicitly set then use this value.

routing:
    servicePort: 8000        # routing proxy listens here
    proxy:
      targetPort: 8200       # vLLM container listens here (decode only)
   ...
prefill:
    servicePort: 8300  # new field
    ...
``` still we should not inject `--port 8000`

i made a change for the option 2 #200 maybe this is more reasonable than this option 1 ?

[jgchn]

In that case I would prefer if prefillServicePort is under routing as that section contains all the networking logic. And the the default for it should be 8000 in values.yaml

[zdtsw]

In that case I would prefer if prefillServicePort is under routing as that section contains all the networking logic. And the the default for it should be 8000 in values.yaml

you are right, have prefillServicePort under routing is easier than under prefill.
let me close this PR and rework #200 with routing

[zdtsw]

close this as the current implementation of "scheduler" does not support that prefill port is different from decode routing port(in P/D) case.