Skip to content

feat: add new --model-server-port to replace --vllm-port #2142

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
elevran merged 3 commits into from
Jul 26, 2026
+117 −39
Merged

feat: add new --model-server-port to replace --vllm-port #2142

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f6cfccf
feat: add new --model-server-port to replace --vllm-port
zdtsw Jul 22, 2026
da94052
update: code review
zdtsw Jul 24, 2026
9569faa
update: code review
zdtsw Jul 24, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 32 additions & 10 deletions pkg/sidecar/proxy/options.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ const (

// Flags
port = "port"
modelServerPort = "model-server-port"
vllmPort = "vllm-port"
dataParallelSize = "data-parallel-size"
kvConnector = "kv-connector"
Expand Down Expand Up @@ -99,6 +100,7 @@ const (
// yamlConfiguration represents structure of YAML configuration for sidecar proxy
type yamlConfiguration struct {
Port int `json:"port,omitempty"`
ModelServerPort int `json:"model-server-port,omitempty"`
VLLMPort int `json:"vllm-port,omitempty"`
MooncakeBootstrapPort int `json:"mooncake-bootstrap-port,omitempty"`
P2PConnectorPort int `json:"p2p-connector-port,omitempty"`
Expand Down Expand Up @@ -130,7 +132,9 @@ type Options struct {
// Fields with direct CLI flags are bound here via embedding; derived fields are set in Complete().
Config

// vllmPort is the port vLLM is listening on; used to compute Config.DecoderURL in Complete().
// modelServerPort is the port the model server (vLLM, SGLang, etc.) is listening on; used to compute Config.DecoderURL in Complete().
modelServerPort string
// vllmPort is the deprecated alias for modelServerPort; migrated in Complete().
vllmPort string
// enableTLS is the list of stages to enable TLS for; used to compute Config.UseTLSFor* in Complete().
enableTLS []string
Expand Down Expand Up @@ -243,8 +247,11 @@ func (opts *Options) AddFlags(fs *pflag.FlagSet) {
// Add Go flags to pflag (for zap options compatibility)
fs.AddGoFlagSet(goFlagSet)
fs.StringVar(&opts.Port, port, opts.Port, "the port the sidecar is listening on")
fs.StringVar(&opts.vllmPort, vllmPort, opts.vllmPort, "the port vLLM is listening on")
fs.IntVar(&opts.DataParallelSize, dataParallelSize, opts.DataParallelSize, "the vLLM DATA-PARALLEL-SIZE value")
fs.StringVar(&opts.modelServerPort, modelServerPort, opts.modelServerPort,
fmt.Sprintf("the port the model server is listening on (default %s)", defaultVLLMPort))
fs.StringVar(&opts.vllmPort, vllmPort, opts.vllmPort, "the port the model server is listening on")
_ = fs.MarkDeprecated(vllmPort, "use --model-server-port instead; --vllm-port will be removed after the deprecation period")
Comment thread
elevran marked this conversation as resolved.
fs.IntVar(&opts.DataParallelSize, dataParallelSize, opts.DataParallelSize, "the model server's data-parallel size")
fs.StringVar(&opts.KVConnector, kvConnector, opts.KVConnector,
"the KV protocol between prefiller and decoder. Supported: "+supportedKVConnectorNamesStr)
fs.StringVar(&opts.ECConnector, ecConnector, opts.ECConnector,
Expand Down Expand Up @@ -310,7 +317,7 @@ func (opts *Options) AddFlags(fs *pflag.FlagSet) {
fs.IntVar(&opts.MaxIdleConnsPerHost, "max-idle-conns-per-host", opts.MaxIdleConnsPerHost, "max idle keep-alive connections per host for reverse proxy transports; set to at least the expected concurrency")
fs.IntVar(&opts.PrefillMaxRetries, prefillMaxRetries, opts.PrefillMaxRetries, "max retry attempts when a prefill request fails with a 5xx error; 0 means no retries (default)")
fs.DurationVar(&opts.PrefillRetryBackoff, prefillRetryBackoff, opts.PrefillRetryBackoff, "delay between prefill retry attempts")
fs.StringVar(&opts.inlineConfiguration, inlineConfiguration, "", "Sidecar configuration in YAML provided as inline specification. Example `--configuration={port: 8085, vllm-port: 8203}. Inline configuration and file configuration are mutually exclusive.`")
fs.StringVar(&opts.inlineConfiguration, inlineConfiguration, "", "Sidecar configuration in YAML provided as inline specification. Example `--configuration={port: 8085, model-server-port: 8203}. Inline configuration and file configuration are mutually exclusive.`")
fs.StringVar(&opts.fileConfiguration, configurationFile, "", "Path to file which contains sidecar configuration in YAML. Example `--configuration-file=/etc/config/sidecar-config.yaml`. Inline configuration and file configuration are mutually exclusive.")
}

Expand All @@ -333,6 +340,13 @@ func (opts *Options) Complete() error {
return err
}

// Resolve the effective model server port with flag-over-config precedence:
// --model-server-port flag > --vllm-port flag > model-server-port YAML > vllm-port YAML > default.
// The deprecated --vllm-port flag must still override a YAML model-server-port.
if (opts.isFlagSet(vllmPort) && !opts.isFlagSet(modelServerPort)) || opts.modelServerPort == "" {
opts.modelServerPort = opts.vllmPort
}

// Parse inferencePool field (namespace/name or just name) into Config.
if opts.inferencePool != "" {
parts := strings.SplitN(opts.inferencePool, "/", 2)
Expand All @@ -353,13 +367,13 @@ func (opts *Options) Complete() error {
opts.InsecureSkipVerifyForEncoder = slices.Contains(opts.tlsInsecureSkipVerify, encodeStage)
opts.InsecureSkipVerifyForDecoder = slices.Contains(opts.tlsInsecureSkipVerify, decodeStage)

// Compute Config.DecoderURL from vllmPort and decoder TLS setting
// Compute Config.DecoderURL from modelServerPort and decoder TLS setting
scheme := "http"
if opts.UseTLSForDecoder {
scheme = schemeHTTPS
}
var err error
opts.DecoderURL, err = url.Parse(scheme + "://localhost:" + opts.vllmPort)
opts.DecoderURL, err = url.Parse(scheme + "://localhost:" + opts.modelServerPort)
if err != nil {
return fmt.Errorf("failed to parse target URL: %w", err)
}
Expand Down Expand Up @@ -472,12 +486,16 @@ func (opts *Options) Validate() error {
return fmt.Errorf("--port %w", err)
}

vllmPort, err := strconv.Atoi(opts.vllmPort)
portFlagName := "--" + modelServerPort
if opts.isFlagSet(vllmPort) && !opts.isFlagSet(modelServerPort) {
portFlagName = "--" + vllmPort
}
msPort, err := strconv.Atoi(opts.modelServerPort)
if err != nil {
return fmt.Errorf("--vllm-port must be a valid integer, got %q", opts.vllmPort)
return fmt.Errorf("%s must be a valid integer, got %q", portFlagName, opts.modelServerPort)
}
if err := validatePortRange(vllmPort, opts.DataParallelSize); err != nil {
return fmt.Errorf("--vllm-port %w", err)
if err := validatePortRange(msPort, opts.DataParallelSize); err != nil {
return fmt.Errorf("%s %w", portFlagName, err)
}

// Validate KV connector
Expand Down Expand Up @@ -650,6 +668,10 @@ func (opts *Options) mergeYAMLConfiguration(cfg yamlConfiguration) {
if cfg.Port != 0 && !opts.isFlagSet(port) {
opts.Port = strconv.Itoa(cfg.Port)
}
// If both keys may be present, Complete() resolves precedence: modelServerPort wins.
if cfg.ModelServerPort != 0 && !opts.isFlagSet(modelServerPort) {
opts.modelServerPort = strconv.Itoa(cfg.ModelServerPort)
}
if cfg.VLLMPort != 0 && !opts.isFlagSet(vllmPort) {
opts.vllmPort = strconv.Itoa(cfg.VLLMPort)
}
Expand Down
112 changes: 84 additions & 28 deletions pkg/sidecar/proxy/options_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ func createConfigWithValidYAML(t *testing.T) string {
t.Helper()
return writeTempYAML(t, "valid.yaml", fmt.Sprintf(`
port: 8100
vllm-port: 8001
model-server-port: 8001
data-parallel-size: 5
kv-connector: %q
ec-connector: %q
Expand Down Expand Up @@ -72,7 +72,7 @@ func createConfigWithUnknownKeys(t *testing.T) string {
t.Helper()
return writeTempYAML(t, "valid.yaml", `
port: 8100
vllm-port: 8001
model-server-port: 8001
unknown-key: 1001
`)
}
Expand All @@ -89,7 +89,7 @@ func TestSidecarConfiguration(t *testing.T) {
// --- inline YAML for testing ---
inlineYAML := fmt.Sprintf(`{
port: 8011,
vllm-port: 8021,
model-server-port: 8021,
data-parallel-size: 3,
kv-connector: %s,
ec-connector: %s,
Expand Down Expand Up @@ -130,7 +130,7 @@ func TestSidecarConfiguration(t *testing.T) {
},
expected: func(o *Options) {
o.Port = "8011"
o.vllmPort = "8021"
o.modelServerPort = "8021"
o.DataParallelSize = 3
o.MaxIdleConnsPerHost = 200
o.MooncakeBootstrapPort = 9001
Expand Down Expand Up @@ -178,7 +178,7 @@ func TestSidecarConfiguration(t *testing.T) {
},
expected: func(o *Options) {
o.Port = "8100"
o.vllmPort = "8001"
o.modelServerPort = "8001"
o.DataParallelSize = 5
o.MaxIdleConnsPerHost = 300
o.MooncakeBootstrapPort = 9000
Expand Down Expand Up @@ -223,7 +223,7 @@ func TestSidecarConfiguration(t *testing.T) {
name: "flags override inline YAML",
inputFlags: map[string]any{
port: "8111",
vllmPort: "8222",
modelServerPort: "8222",
dataParallelSize: 2,
kvConnector: KVConnectorNIXLV2,
ecConnector: ECExampleConnector,
Expand All @@ -240,7 +240,7 @@ func TestSidecarConfiguration(t *testing.T) {
},
expected: func(o *Options) {
o.Port = "8111"
o.vllmPort = "8222"
o.modelServerPort = "8222"
o.DataParallelSize = 2
o.MaxIdleConnsPerHost = 200
o.MooncakeBootstrapPort = 9001
Expand Down Expand Up @@ -287,6 +287,7 @@ func TestSidecarConfiguration(t *testing.T) {
ecConnector: ECConnectorNIXL,
},
expected: func(o *Options) {
o.modelServerPort = defaultVLLMPort
o.KVConnector = KVConnectorNIXLV2
o.ECConnector = ECConnectorNIXL
},
Expand All @@ -296,7 +297,7 @@ func TestSidecarConfiguration(t *testing.T) {
name: "flags override file YAML",
inputFlags: map[string]any{
port: "8111",
vllmPort: "8222",
modelServerPort: "8222",
dataParallelSize: 2,
kvConnector: KVConnectorNIXLV2,
ecConnector: ECExampleConnector,
Expand All @@ -314,7 +315,7 @@ func TestSidecarConfiguration(t *testing.T) {
},
expected: func(o *Options) {
o.Port = "8111"
o.vllmPort = "8222"
o.modelServerPort = "8222"
o.DataParallelSize = 2
o.MaxIdleConnsPerHost = 400
o.MooncakeBootstrapPort = 9002
Expand Down Expand Up @@ -453,7 +454,7 @@ func compareOptions(t *testing.T, expected, actual *Options) {
}

assertEqual(port, expected.Port, actual.Port)
assertEqual(vllmPort, expected.vllmPort, actual.vllmPort)
assertEqual(modelServerPort, expected.modelServerPort, actual.modelServerPort)
assertEqual(dataParallelSize, expected.DataParallelSize, actual.DataParallelSize)
assertEqual(maxIdleConnsPerHost, expected.MaxIdleConnsPerHost, actual.MaxIdleConnsPerHost)

Expand Down Expand Up @@ -492,7 +493,7 @@ func compareOptions(t *testing.T, expected, actual *Options) {
assertEqual(inlineConfiguration, expected.inlineConfiguration, actual.inlineConfiguration)
assertEqual(configurationFile, expected.fileConfiguration, actual.fileConfiguration)

assertEqual("decoderURL", calculateURL(t, expected.UseTLSForDecoder, expected.vllmPort), actual.DecoderURL)
assertEqual("decoderURL", calculateURL(t, expected.UseTLSForDecoder, expected.modelServerPort), actual.DecoderURL)
}

// setEnv sets environment variables for testing and ensures they are cleaned up after the test finishes
Expand Down Expand Up @@ -781,7 +782,7 @@ func TestValidateDataParallelPortRange(t *testing.T) {
tests := []struct {
name string
port string
vllmPort string
modelServerPort string
dataParallelSize int
wantErr bool
}{
Expand All @@ -794,7 +795,7 @@ func TestValidateDataParallelPortRange(t *testing.T) {
t.Run(tt.name, func(t *testing.T) {
opts := NewOptions()
opts.Port = tt.port
opts.vllmPort = tt.vllmPort
opts.modelServerPort = tt.modelServerPort
opts.DataParallelSize = tt.dataParallelSize
_ = opts.Complete()
err := opts.Validate()
Expand All @@ -807,25 +808,25 @@ func TestValidateDataParallelPortRange(t *testing.T) {

func TestValidatePorts(t *testing.T) {
tests := []struct {
name string
port string
vllmPort string
wantErr string
name string
port string
modelServerPort string
wantErr string
}{
{"valid ports", "8000", "8001", ""},
{"invalid port format", "abc", "8001", `--port must be a valid integer, got "abc"`},
{"invalid vllm port format", "8000", "xyz", `--vllm-port must be a valid integer, got "xyz"`},
{"invalid model server port format", "8000", "xyz", `--model-server-port must be a valid integer, got "xyz"`},
{"port too low", "0", "8001", "--port start port 0 is out of valid range [1, 65535]"},
{"port too high", "65536", "8001", "--port start port 65536 is out of valid range [1, 65535]"},
{"vllm port too low", "8000", "0", "--vllm-port start port 0 is out of valid range [1, 65535]"},
{"vllm port too high", "8000", "65536", "--vllm-port start port 65536 is out of valid range [1, 65535]"},
{"model server port too low", "8000", "0", "--model-server-port start port 0 is out of valid range [1, 65535]"},
{"model server port too high", "8000", "65536", "--model-server-port start port 65536 is out of valid range [1, 65535]"},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
opts := NewOptions()
opts.Port = tt.port
opts.vllmPort = tt.vllmPort
opts.modelServerPort = tt.modelServerPort
_ = opts.Complete()
err := opts.Validate()
if tt.wantErr == "" {
Expand Down Expand Up @@ -1130,12 +1131,67 @@ func TestCompleteMoRIIOWriteModeGuards(t *testing.T) {
})
}

// TestModelServerPortMigration verifies that the deprecated vllm-port flag
// migrates to model-server-port.
// Remove when vllm-port is dropped in v0.12 (see issue 2172).
func TestModelServerPortMigration(t *testing.T) {
tests := []struct {
name string
modelServerPort string
vllmPort string
expectedDecoderURL string
}{
{"model-server-port set", "9000", "", "http://localhost:9000"},
{"deprecated vllm-port migrated", "", "9001", "http://localhost:9001"},
{"model-server-port wins over vllm-port when both set", "9000", "9001", "http://localhost:9000"},
{"no model-server-port, default falls back to vllm-port default", "", defaultVLLMPort, "http://localhost:" + defaultVLLMPort},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
opts := NewOptions()
opts.modelServerPort = tt.modelServerPort
opts.vllmPort = tt.vllmPort

require.NoError(t, opts.Complete())
require.NoError(t, opts.Validate())
require.NotNil(t, opts.DecoderURL)
require.Equal(t, tt.expectedDecoderURL, opts.DecoderURL.String())
})
}
}

func TestModelServerPortYAML(t *testing.T) {
opts, testPFlagSet := newTestOptions(t)
yaml := "{model-server-port: 8203}"
setFlag(t, testPFlagSet, inlineConfiguration, &yaml)
require.NoError(t, testPFlagSet.Parse(nil))

require.NoError(t, opts.Complete())
require.NoError(t, opts.Validate())
require.Equal(t, "http://localhost:8203", opts.DecoderURL.String())
}

// A CLI flag overrides YAML config, including the deprecated --vllm-port over a
// model-server-port key.
func TestModelServerPortFlagBeatsYAML(t *testing.T) {
opts, testPFlagSet := newTestOptions(t)
yaml := "{model-server-port: 8203}"
setFlag(t, testPFlagSet, inlineConfiguration, &yaml)
setFlag(t, testPFlagSet, vllmPort, "9001")
require.NoError(t, testPFlagSet.Parse(nil))

require.NoError(t, opts.Complete())
require.NoError(t, opts.Validate())
require.Equal(t, "http://localhost:9001", opts.DecoderURL.String())
}

func TestCompleteTLSConfiguration(t *testing.T) {
tests := []struct {
name string
enableTLS []string
tlsInsecureSkipVerify []string
vllmPort string
modelServerPort string
expectedDecoderURL string
expectedUseTLSForPrefiller bool
expectedUseTLSForDecoder bool
Expand All @@ -1146,7 +1202,7 @@ func TestCompleteTLSConfiguration(t *testing.T) {
name: "no TLS configuration",
enableTLS: []string{},
tlsInsecureSkipVerify: []string{},
vllmPort: "8001",
modelServerPort: "8001",
expectedDecoderURL: "http://localhost:8001",
expectedUseTLSForPrefiller: false,
expectedUseTLSForDecoder: false,
Expand All @@ -1157,7 +1213,7 @@ func TestCompleteTLSConfiguration(t *testing.T) {
name: "prefiller TLS only",
enableTLS: []string{"prefiller"},
tlsInsecureSkipVerify: []string{},
vllmPort: "8001",
modelServerPort: "8001",
expectedDecoderURL: "http://localhost:8001",
expectedUseTLSForPrefiller: true,
expectedUseTLSForDecoder: false,
Expand All @@ -1168,7 +1224,7 @@ func TestCompleteTLSConfiguration(t *testing.T) {
name: "decoder TLS only",
enableTLS: []string{"decoder"},
tlsInsecureSkipVerify: []string{},
vllmPort: "8001",
modelServerPort: "8001",
expectedDecoderURL: "https://localhost:8001",
expectedUseTLSForPrefiller: false,
expectedUseTLSForDecoder: true,
Expand All @@ -1179,7 +1235,7 @@ func TestCompleteTLSConfiguration(t *testing.T) {
name: "both stages TLS",
enableTLS: []string{"prefiller", "decoder"},
tlsInsecureSkipVerify: []string{},
vllmPort: "9000",
modelServerPort: "9000",
expectedDecoderURL: "https://localhost:9000",
expectedUseTLSForPrefiller: true,
expectedUseTLSForDecoder: true,
Expand All @@ -1190,7 +1246,7 @@ func TestCompleteTLSConfiguration(t *testing.T) {
name: "TLS with insecure skip verify",
enableTLS: []string{"prefiller", "decoder"},
tlsInsecureSkipVerify: []string{"prefiller", "decoder"},
vllmPort: "8001",
modelServerPort: "8001",
expectedDecoderURL: "https://localhost:8001",
expectedUseTLSForPrefiller: true,
expectedUseTLSForDecoder: true,
Expand All @@ -1204,7 +1260,7 @@ func TestCompleteTLSConfiguration(t *testing.T) {
opts := NewOptions()
opts.enableTLS = tt.enableTLS
opts.tlsInsecureSkipVerify = tt.tlsInsecureSkipVerify
opts.vllmPort = tt.vllmPort
opts.modelServerPort = tt.modelServerPort

err := opts.Complete()
if err != nil {
Expand Down
Loading
Loading