Skip to content

Commit f6cfccf

Browse files
zdtswzdtsw
committed
feat: add new --model-server-port to replace --vllm-port
- disagg-sidecar expose vllm specific flag name which should be model server neutral. add --model-server-port to replace --vllm-port which stays as a deprecated alias until final removal. - update usage text on --data-parallel-size Signed-off-by: Wen Zhou <wenzhou@redhat.com>
1 parent a0a1d55 commit f6cfccf

2 files changed

Lines changed: 72 additions & 13 deletions

File tree

‎pkg/sidecar/proxy/options.go‎

Lines changed: 31 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@ const (
5454

5555
// Flags
5656
port = "port"
57+
modelServerPort = "model-server-port"
5758
vllmPort = "vllm-port"
5859
dataParallelSize = "data-parallel-size"
5960
kvConnector = "kv-connector"
@@ -99,6 +100,7 @@ const (
99100
// yamlConfiguration represents structure of YAML configuration for sidecar proxy
100101
type yamlConfiguration struct {
101102
Port int `json:"port,omitempty"`
103+
ModelServerPort int `json:"model-server-port,omitempty"`
102104
VLLMPort int `json:"vllm-port,omitempty"`
103105
MooncakeBootstrapPort int `json:"mooncake-bootstrap-port,omitempty"`
104106
P2PConnectorPort int `json:"p2p-connector-port,omitempty"`
@@ -130,7 +132,9 @@ type Options struct {
130132
// Fields with direct CLI flags are bound here via embedding; derived fields are set in Complete().
131133
Config
132134

133-
// vllmPort is the port vLLM is listening on; used to compute Config.DecoderURL in Complete().
135+
// modelServerPort is the port the model server(vLLM, SGLang etc) is listening on; used to compute Config.DecoderURL in Complete().
136+
modelServerPort string
137+
// vllmPort is the deprecated alias for modelServerPort; migrated in Complete().
134138
vllmPort string
135139
// enableTLS is the list of stages to enable TLS for; used to compute Config.UseTLSFor* in Complete().
136140
enableTLS []string
@@ -243,8 +247,10 @@ func (opts *Options) AddFlags(fs *pflag.FlagSet) {
243247
// Add Go flags to pflag (for zap options compatibility)
244248
fs.AddGoFlagSet(goFlagSet)
245249
fs.StringVar(&opts.Port, port, opts.Port, "the port the sidecar is listening on")
246-
fs.StringVar(&opts.vllmPort, vllmPort, opts.vllmPort, "the port vLLM is listening on")
247-
fs.IntVar(&opts.DataParallelSize, dataParallelSize, opts.DataParallelSize, "the vLLM DATA-PARALLEL-SIZE value")
250+
fs.StringVar(&opts.modelServerPort, modelServerPort, opts.modelServerPort, "the port the model server is listening on")
251+
fs.StringVar(&opts.vllmPort, vllmPort, opts.vllmPort, "the port the model server is listening on")
252+
_ = fs.MarkDeprecated(vllmPort, "use --model-server-port instead; --vllm-port will be removed after the deprecation period")
253+
fs.IntVar(&opts.DataParallelSize, dataParallelSize, opts.DataParallelSize, "the model server's data-parallel size")
248254
fs.StringVar(&opts.KVConnector, kvConnector, opts.KVConnector,
249255
"the KV protocol between prefiller and decoder. Supported: "+supportedKVConnectorNamesStr)
250256
fs.StringVar(&opts.ECConnector, ecConnector, opts.ECConnector,
@@ -310,7 +316,7 @@ func (opts *Options) AddFlags(fs *pflag.FlagSet) {
310316
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")
311317
fs.IntVar(&opts.PrefillMaxRetries, prefillMaxRetries, opts.PrefillMaxRetries, "max retry attempts when a prefill request fails with a 5xx error; 0 means no retries (default)")
312318
fs.DurationVar(&opts.PrefillRetryBackoff, prefillRetryBackoff, opts.PrefillRetryBackoff, "delay between prefill retry attempts")
313-
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.`")
319+
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.`")
314320
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.")
315321
}
316322

@@ -333,6 +339,13 @@ func (opts *Options) Complete() error {
333339
return err
334340
}
335341

342+
// Migrate deprecated --vllm-port to --model-server-port.
343+
// defaults to empty, so an unset value means neither flag nor YAML provided a --model-server-port
344+
// fall back to the --vllm-port value.
345+
if opts.modelServerPort == "" {
346+
opts.modelServerPort = opts.vllmPort
347+
}
348+
336349
// Parse inferencePool field (namespace/name or just name) into Config.
337350
if opts.inferencePool != "" {
338351
parts := strings.SplitN(opts.inferencePool, "/", 2)
@@ -353,13 +366,13 @@ func (opts *Options) Complete() error {
353366
opts.InsecureSkipVerifyForEncoder = slices.Contains(opts.tlsInsecureSkipVerify, encodeStage)
354367
opts.InsecureSkipVerifyForDecoder = slices.Contains(opts.tlsInsecureSkipVerify, decodeStage)
355368

356-
// Compute Config.DecoderURL from vllmPort and decoder TLS setting
369+
// Compute Config.DecoderURL from modelServerPort and decoder TLS setting
357370
scheme := "http"
358371
if opts.UseTLSForDecoder {
359372
scheme = schemeHTTPS
360373
}
361374
var err error
362-
opts.DecoderURL, err = url.Parse(scheme + "://localhost:" + opts.vllmPort)
375+
opts.DecoderURL, err = url.Parse(scheme + "://localhost:" + opts.modelServerPort)
363376
if err != nil {
364377
return fmt.Errorf("failed to parse target URL: %w", err)
365378
}
@@ -472,12 +485,16 @@ func (opts *Options) Validate() error {
472485
return fmt.Errorf("--port %w", err)
473486
}
474487

475-
vllmPort, err := strconv.Atoi(opts.vllmPort)
488+
portFlagName := "--" + modelServerPort
489+
if opts.isFlagSet(vllmPort) && !opts.isFlagSet(modelServerPort) {
490+
portFlagName = "--" + vllmPort
491+
}
492+
msPort, err := strconv.Atoi(opts.modelServerPort)
476493
if err != nil {
477-
return fmt.Errorf("--vllm-port must be a valid integer, got %q", opts.vllmPort)
494+
return fmt.Errorf("%s must be a valid integer, got %q", portFlagName, opts.modelServerPort)
478495
}
479-
if err := validatePortRange(vllmPort, opts.DataParallelSize); err != nil {
480-
return fmt.Errorf("--vllm-port %w", err)
496+
if err := validatePortRange(msPort, opts.DataParallelSize); err != nil {
497+
return fmt.Errorf("%s %w", portFlagName, err)
481498
}
482499

483500
// Validate KV connector
@@ -650,6 +667,10 @@ func (opts *Options) mergeYAMLConfiguration(cfg yamlConfiguration) {
650667
if cfg.Port != 0 && !opts.isFlagSet(port) {
651668
opts.Port = strconv.Itoa(cfg.Port)
652669
}
670+
// If both keys may be present, Complete() resolves precedence: modelServerPort wins.
671+
if cfg.ModelServerPort != 0 && !opts.isFlagSet(modelServerPort) {
672+
opts.modelServerPort = strconv.Itoa(cfg.ModelServerPort)
673+
}
653674
if cfg.VLLMPort != 0 && !opts.isFlagSet(vllmPort) {
654675
opts.vllmPort = strconv.Itoa(cfg.VLLMPort)
655676
}

‎pkg/sidecar/proxy/options_test.go‎

Lines changed: 41 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -814,11 +814,11 @@ func TestValidatePorts(t *testing.T) {
814814
}{
815815
{"valid ports", "8000", "8001", ""},
816816
{"invalid port format", "abc", "8001", `--port must be a valid integer, got "abc"`},
817-
{"invalid vllm port format", "8000", "xyz", `--vllm-port must be a valid integer, got "xyz"`},
817+
{"invalid model server port format", "8000", "xyz", `--model-server-port must be a valid integer, got "xyz"`},
818818
{"port too low", "0", "8001", "--port start port 0 is out of valid range [1, 65535]"},
819819
{"port too high", "65536", "8001", "--port start port 65536 is out of valid range [1, 65535]"},
820-
{"vllm port too low", "8000", "0", "--vllm-port start port 0 is out of valid range [1, 65535]"},
821-
{"vllm port too high", "8000", "65536", "--vllm-port start port 65536 is out of valid range [1, 65535]"},
820+
{"model server port too low", "8000", "0", "--model-server-port start port 0 is out of valid range [1, 65535]"},
821+
{"model server port too high", "8000", "65536", "--model-server-port start port 65536 is out of valid range [1, 65535]"},
822822
}
823823

824824
for _, tt := range tests {
@@ -1130,6 +1130,44 @@ func TestCompleteMoRIIOWriteModeGuards(t *testing.T) {
11301130
})
11311131
}
11321132

1133+
func TestModelServerPortMigration(t *testing.T) {
1134+
tests := []struct {
1135+
name string
1136+
modelServerPort string
1137+
vllmPort string
1138+
expectedDecoderURL string
1139+
}{
1140+
{"model-server-port set", "9000", "", "http://localhost:9000"},
1141+
{"deprecated vllm-port migrated", "", "9001", "http://localhost:9001"},
1142+
{"model-server-port wins over vllm-port when both set", "9000", "9001", "http://localhost:9000"},
1143+
{"no model-server-port, default falls back to vllm-port default", "", defaultVLLMPort, "http://localhost:" + defaultVLLMPort},
1144+
}
1145+
1146+
for _, tt := range tests {
1147+
t.Run(tt.name, func(t *testing.T) {
1148+
opts := NewOptions()
1149+
opts.modelServerPort = tt.modelServerPort
1150+
opts.vllmPort = tt.vllmPort
1151+
1152+
require.NoError(t, opts.Complete())
1153+
require.NoError(t, opts.Validate())
1154+
require.NotNil(t, opts.DecoderURL)
1155+
require.Equal(t, tt.expectedDecoderURL, opts.DecoderURL.String())
1156+
})
1157+
}
1158+
}
1159+
1160+
func TestModelServerPortYAML(t *testing.T) {
1161+
opts, testPFlagSet := newTestOptions(t)
1162+
yaml := "{model-server-port: 8203}"
1163+
setFlag(t, testPFlagSet, inlineConfiguration, &yaml)
1164+
require.NoError(t, testPFlagSet.Parse(nil))
1165+
1166+
require.NoError(t, opts.Complete())
1167+
require.NoError(t, opts.Validate())
1168+
require.Equal(t, "http://localhost:8203", opts.DecoderURL.String())
1169+
}
1170+
11331171
func TestCompleteTLSConfiguration(t *testing.T) {
11341172
tests := []struct {
11351173
name string

0 commit comments

Comments
 (0)