Skip to content

Commit c0e3048

Browse files
zdtswzdtsw
authored
feat: add new --model-server-port to replace --vllm-port (#2142)
* 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> * update: code review - update test and config which still set to old --vllm-port to new flag Signed-off-by: Wen Zhou <wenzhou@redhat.com> * update: code review - old --vllm-port set as flag should override model-server-port in Yaml - wording fix Signed-off-by: Wen Zhou <wenzhou@redhat.com> --------- Signed-off-by: Wen Zhou <wenzhou@redhat.com>
1 parent f56f3bd commit c0e3048

3 files changed

Lines changed: 117 additions & 39 deletions

File tree

‎pkg/sidecar/proxy/options.go‎

Lines changed: 32 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,11 @@ 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,
251+
fmt.Sprintf("the port the model server is listening on (default %s)", defaultVLLMPort))
252+
fs.StringVar(&opts.vllmPort, vllmPort, opts.vllmPort, "the port the model server is listening on")
253+
_ = fs.MarkDeprecated(vllmPort, "use --model-server-port instead; --vllm-port will be removed after the deprecation period")
254+
fs.IntVar(&opts.DataParallelSize, dataParallelSize, opts.DataParallelSize, "the model server's data-parallel size")
248255
fs.StringVar(&opts.KVConnector, kvConnector, opts.KVConnector,
249256
"the KV protocol between prefiller and decoder. Supported: "+supportedKVConnectorNamesStr)
250257
fs.StringVar(&opts.ECConnector, ecConnector, opts.ECConnector,
@@ -310,7 +317,7 @@ func (opts *Options) AddFlags(fs *pflag.FlagSet) {
310317
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")
311318
fs.IntVar(&opts.PrefillMaxRetries, prefillMaxRetries, opts.PrefillMaxRetries, "max retry attempts when a prefill request fails with a 5xx error; 0 means no retries (default)")
312319
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.`")
320+
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.`")
314321
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.")
315322
}
316323

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

343+
// Resolve the effective model server port with flag-over-config precedence:
344+
// --model-server-port flag > --vllm-port flag > model-server-port YAML > vllm-port YAML > default.
345+
// The deprecated --vllm-port flag must still override a YAML model-server-port.
346+
if (opts.isFlagSet(vllmPort) && !opts.isFlagSet(modelServerPort)) || opts.modelServerPort == "" {
347+
opts.modelServerPort = opts.vllmPort
348+
}
349+
336350
// Parse inferencePool field (namespace/name or just name) into Config.
337351
if opts.inferencePool != "" {
338352
parts := strings.SplitN(opts.inferencePool, "/", 2)
@@ -353,13 +367,13 @@ func (opts *Options) Complete() error {
353367
opts.InsecureSkipVerifyForEncoder = slices.Contains(opts.tlsInsecureSkipVerify, encodeStage)
354368
opts.InsecureSkipVerifyForDecoder = slices.Contains(opts.tlsInsecureSkipVerify, decodeStage)
355369

356-
// Compute Config.DecoderURL from vllmPort and decoder TLS setting
370+
// Compute Config.DecoderURL from modelServerPort and decoder TLS setting
357371
scheme := "http"
358372
if opts.UseTLSForDecoder {
359373
scheme = schemeHTTPS
360374
}
361375
var err error
362-
opts.DecoderURL, err = url.Parse(scheme + "://localhost:" + opts.vllmPort)
376+
opts.DecoderURL, err = url.Parse(scheme + "://localhost:" + opts.modelServerPort)
363377
if err != nil {
364378
return fmt.Errorf("failed to parse target URL: %w", err)
365379
}
@@ -472,12 +486,16 @@ func (opts *Options) Validate() error {
472486
return fmt.Errorf("--port %w", err)
473487
}
474488

475-
vllmPort, err := strconv.Atoi(opts.vllmPort)
489+
portFlagName := "--" + modelServerPort
490+
if opts.isFlagSet(vllmPort) && !opts.isFlagSet(modelServerPort) {
491+
portFlagName = "--" + vllmPort
492+
}
493+
msPort, err := strconv.Atoi(opts.modelServerPort)
476494
if err != nil {
477-
return fmt.Errorf("--vllm-port must be a valid integer, got %q", opts.vllmPort)
495+
return fmt.Errorf("%s must be a valid integer, got %q", portFlagName, opts.modelServerPort)
478496
}
479-
if err := validatePortRange(vllmPort, opts.DataParallelSize); err != nil {
480-
return fmt.Errorf("--vllm-port %w", err)
497+
if err := validatePortRange(msPort, opts.DataParallelSize); err != nil {
498+
return fmt.Errorf("%s %w", portFlagName, err)
481499
}
482500

483501
// Validate KV connector
@@ -650,6 +668,10 @@ func (opts *Options) mergeYAMLConfiguration(cfg yamlConfiguration) {
650668
if cfg.Port != 0 && !opts.isFlagSet(port) {
651669
opts.Port = strconv.Itoa(cfg.Port)
652670
}
671+
// If both keys may be present, Complete() resolves precedence: modelServerPort wins.
672+
if cfg.ModelServerPort != 0 && !opts.isFlagSet(modelServerPort) {
673+
opts.modelServerPort = strconv.Itoa(cfg.ModelServerPort)
674+
}
653675
if cfg.VLLMPort != 0 && !opts.isFlagSet(vllmPort) {
654676
opts.vllmPort = strconv.Itoa(cfg.VLLMPort)
655677
}

‎pkg/sidecar/proxy/options_test.go‎

Lines changed: 84 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ func createConfigWithValidYAML(t *testing.T) string {
4343
t.Helper()
4444
return writeTempYAML(t, "valid.yaml", fmt.Sprintf(`
4545
port: 8100
46-
vllm-port: 8001
46+
model-server-port: 8001
4747
data-parallel-size: 5
4848
kv-connector: %q
4949
ec-connector: %q
@@ -72,7 +72,7 @@ func createConfigWithUnknownKeys(t *testing.T) string {
7272
t.Helper()
7373
return writeTempYAML(t, "valid.yaml", `
7474
port: 8100
75-
vllm-port: 8001
75+
model-server-port: 8001
7676
unknown-key: 1001
7777
`)
7878
}
@@ -89,7 +89,7 @@ func TestSidecarConfiguration(t *testing.T) {
8989
// --- inline YAML for testing ---
9090
inlineYAML := fmt.Sprintf(`{
9191
port: 8011,
92-
vllm-port: 8021,
92+
model-server-port: 8021,
9393
data-parallel-size: 3,
9494
kv-connector: %s,
9595
ec-connector: %s,
@@ -130,7 +130,7 @@ func TestSidecarConfiguration(t *testing.T) {
130130
},
131131
expected: func(o *Options) {
132132
o.Port = "8011"
133-
o.vllmPort = "8021"
133+
o.modelServerPort = "8021"
134134
o.DataParallelSize = 3
135135
o.MaxIdleConnsPerHost = 200
136136
o.MooncakeBootstrapPort = 9001
@@ -178,7 +178,7 @@ func TestSidecarConfiguration(t *testing.T) {
178178
},
179179
expected: func(o *Options) {
180180
o.Port = "8100"
181-
o.vllmPort = "8001"
181+
o.modelServerPort = "8001"
182182
o.DataParallelSize = 5
183183
o.MaxIdleConnsPerHost = 300
184184
o.MooncakeBootstrapPort = 9000
@@ -223,7 +223,7 @@ func TestSidecarConfiguration(t *testing.T) {
223223
name: "flags override inline YAML",
224224
inputFlags: map[string]any{
225225
port: "8111",
226-
vllmPort: "8222",
226+
modelServerPort: "8222",
227227
dataParallelSize: 2,
228228
kvConnector: KVConnectorNIXLV2,
229229
ecConnector: ECExampleConnector,
@@ -240,7 +240,7 @@ func TestSidecarConfiguration(t *testing.T) {
240240
},
241241
expected: func(o *Options) {
242242
o.Port = "8111"
243-
o.vllmPort = "8222"
243+
o.modelServerPort = "8222"
244244
o.DataParallelSize = 2
245245
o.MaxIdleConnsPerHost = 200
246246
o.MooncakeBootstrapPort = 9001
@@ -287,6 +287,7 @@ func TestSidecarConfiguration(t *testing.T) {
287287
ecConnector: ECConnectorNIXL,
288288
},
289289
expected: func(o *Options) {
290+
o.modelServerPort = defaultVLLMPort
290291
o.KVConnector = KVConnectorNIXLV2
291292
o.ECConnector = ECConnectorNIXL
292293
},
@@ -296,7 +297,7 @@ func TestSidecarConfiguration(t *testing.T) {
296297
name: "flags override file YAML",
297298
inputFlags: map[string]any{
298299
port: "8111",
299-
vllmPort: "8222",
300+
modelServerPort: "8222",
300301
dataParallelSize: 2,
301302
kvConnector: KVConnectorNIXLV2,
302303
ecConnector: ECExampleConnector,
@@ -314,7 +315,7 @@ func TestSidecarConfiguration(t *testing.T) {
314315
},
315316
expected: func(o *Options) {
316317
o.Port = "8111"
317-
o.vllmPort = "8222"
318+
o.modelServerPort = "8222"
318319
o.DataParallelSize = 2
319320
o.MaxIdleConnsPerHost = 400
320321
o.MooncakeBootstrapPort = 9002
@@ -453,7 +454,7 @@ func compareOptions(t *testing.T, expected, actual *Options) {
453454
}
454455

455456
assertEqual(port, expected.Port, actual.Port)
456-
assertEqual(vllmPort, expected.vllmPort, actual.vllmPort)
457+
assertEqual(modelServerPort, expected.modelServerPort, actual.modelServerPort)
457458
assertEqual(dataParallelSize, expected.DataParallelSize, actual.DataParallelSize)
458459
assertEqual(maxIdleConnsPerHost, expected.MaxIdleConnsPerHost, actual.MaxIdleConnsPerHost)
459460

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

495-
assertEqual("decoderURL", calculateURL(t, expected.UseTLSForDecoder, expected.vllmPort), actual.DecoderURL)
496+
assertEqual("decoderURL", calculateURL(t, expected.UseTLSForDecoder, expected.modelServerPort), actual.DecoderURL)
496497
}
497498

498499
// setEnv sets environment variables for testing and ensures they are cleaned up after the test finishes
@@ -799,7 +800,7 @@ func TestValidateDataParallelPortRange(t *testing.T) {
799800
tests := []struct {
800801
name string
801802
port string
802-
vllmPort string
803+
modelServerPort string
803804
dataParallelSize int
804805
wantErr bool
805806
}{
@@ -812,7 +813,7 @@ func TestValidateDataParallelPortRange(t *testing.T) {
812813
t.Run(tt.name, func(t *testing.T) {
813814
opts := NewOptions()
814815
opts.Port = tt.port
815-
opts.vllmPort = tt.vllmPort
816+
opts.modelServerPort = tt.modelServerPort
816817
opts.DataParallelSize = tt.dataParallelSize
817818
_ = opts.Complete()
818819
err := opts.Validate()
@@ -825,25 +826,25 @@ func TestValidateDataParallelPortRange(t *testing.T) {
825826

826827
func TestValidatePorts(t *testing.T) {
827828
tests := []struct {
828-
name string
829-
port string
830-
vllmPort string
831-
wantErr string
829+
name string
830+
port string
831+
modelServerPort string
832+
wantErr string
832833
}{
833834
{"valid ports", "8000", "8001", ""},
834835
{"invalid port format", "abc", "8001", `--port must be a valid integer, got "abc"`},
835-
{"invalid vllm port format", "8000", "xyz", `--vllm-port must be a valid integer, got "xyz"`},
836+
{"invalid model server port format", "8000", "xyz", `--model-server-port must be a valid integer, got "xyz"`},
836837
{"port too low", "0", "8001", "--port start port 0 is out of valid range [1, 65535]"},
837838
{"port too high", "65536", "8001", "--port start port 65536 is out of valid range [1, 65535]"},
838-
{"vllm port too low", "8000", "0", "--vllm-port start port 0 is out of valid range [1, 65535]"},
839-
{"vllm port too high", "8000", "65536", "--vllm-port start port 65536 is out of valid range [1, 65535]"},
839+
{"model server port too low", "8000", "0", "--model-server-port start port 0 is out of valid range [1, 65535]"},
840+
{"model server port too high", "8000", "65536", "--model-server-port start port 65536 is out of valid range [1, 65535]"},
840841
}
841842

842843
for _, tt := range tests {
843844
t.Run(tt.name, func(t *testing.T) {
844845
opts := NewOptions()
845846
opts.Port = tt.port
846-
opts.vllmPort = tt.vllmPort
847+
opts.modelServerPort = tt.modelServerPort
847848
_ = opts.Complete()
848849
err := opts.Validate()
849850
if tt.wantErr == "" {
@@ -1148,12 +1149,67 @@ func TestCompleteMoRIIOWriteModeGuards(t *testing.T) {
11481149
})
11491150
}
11501151

1152+
// TestModelServerPortMigration verifies that the deprecated vllm-port flag
1153+
// migrates to model-server-port.
1154+
// Remove when vllm-port is dropped in v0.12 (see issue 2172).
1155+
func TestModelServerPortMigration(t *testing.T) {
1156+
tests := []struct {
1157+
name string
1158+
modelServerPort string
1159+
vllmPort string
1160+
expectedDecoderURL string
1161+
}{
1162+
{"model-server-port set", "9000", "", "http://localhost:9000"},
1163+
{"deprecated vllm-port migrated", "", "9001", "http://localhost:9001"},
1164+
{"model-server-port wins over vllm-port when both set", "9000", "9001", "http://localhost:9000"},
1165+
{"no model-server-port, default falls back to vllm-port default", "", defaultVLLMPort, "http://localhost:" + defaultVLLMPort},
1166+
}
1167+
1168+
for _, tt := range tests {
1169+
t.Run(tt.name, func(t *testing.T) {
1170+
opts := NewOptions()
1171+
opts.modelServerPort = tt.modelServerPort
1172+
opts.vllmPort = tt.vllmPort
1173+
1174+
require.NoError(t, opts.Complete())
1175+
require.NoError(t, opts.Validate())
1176+
require.NotNil(t, opts.DecoderURL)
1177+
require.Equal(t, tt.expectedDecoderURL, opts.DecoderURL.String())
1178+
})
1179+
}
1180+
}
1181+
1182+
func TestModelServerPortYAML(t *testing.T) {
1183+
opts, testPFlagSet := newTestOptions(t)
1184+
yaml := "{model-server-port: 8203}"
1185+
setFlag(t, testPFlagSet, inlineConfiguration, &yaml)
1186+
require.NoError(t, testPFlagSet.Parse(nil))
1187+
1188+
require.NoError(t, opts.Complete())
1189+
require.NoError(t, opts.Validate())
1190+
require.Equal(t, "http://localhost:8203", opts.DecoderURL.String())
1191+
}
1192+
1193+
// A CLI flag overrides YAML config, including the deprecated --vllm-port over a
1194+
// model-server-port key.
1195+
func TestModelServerPortFlagBeatsYAML(t *testing.T) {
1196+
opts, testPFlagSet := newTestOptions(t)
1197+
yaml := "{model-server-port: 8203}"
1198+
setFlag(t, testPFlagSet, inlineConfiguration, &yaml)
1199+
setFlag(t, testPFlagSet, vllmPort, "9001")
1200+
require.NoError(t, testPFlagSet.Parse(nil))
1201+
1202+
require.NoError(t, opts.Complete())
1203+
require.NoError(t, opts.Validate())
1204+
require.Equal(t, "http://localhost:9001", opts.DecoderURL.String())
1205+
}
1206+
11511207
func TestCompleteTLSConfiguration(t *testing.T) {
11521208
tests := []struct {
11531209
name string
11541210
enableTLS []string
11551211
tlsInsecureSkipVerify []string
1156-
vllmPort string
1212+
modelServerPort string
11571213
expectedDecoderURL string
11581214
expectedUseTLSForPrefiller bool
11591215
expectedUseTLSForDecoder bool
@@ -1164,7 +1220,7 @@ func TestCompleteTLSConfiguration(t *testing.T) {
11641220
name: "no TLS configuration",
11651221
enableTLS: []string{},
11661222
tlsInsecureSkipVerify: []string{},
1167-
vllmPort: "8001",
1223+
modelServerPort: "8001",
11681224
expectedDecoderURL: "http://localhost:8001",
11691225
expectedUseTLSForPrefiller: false,
11701226
expectedUseTLSForDecoder: false,
@@ -1175,7 +1231,7 @@ func TestCompleteTLSConfiguration(t *testing.T) {
11751231
name: "prefiller TLS only",
11761232
enableTLS: []string{"prefiller"},
11771233
tlsInsecureSkipVerify: []string{},
1178-
vllmPort: "8001",
1234+
modelServerPort: "8001",
11791235
expectedDecoderURL: "http://localhost:8001",
11801236
expectedUseTLSForPrefiller: true,
11811237
expectedUseTLSForDecoder: false,
@@ -1186,7 +1242,7 @@ func TestCompleteTLSConfiguration(t *testing.T) {
11861242
name: "decoder TLS only",
11871243
enableTLS: []string{"decoder"},
11881244
tlsInsecureSkipVerify: []string{},
1189-
vllmPort: "8001",
1245+
modelServerPort: "8001",
11901246
expectedDecoderURL: "https://localhost:8001",
11911247
expectedUseTLSForPrefiller: false,
11921248
expectedUseTLSForDecoder: true,
@@ -1197,7 +1253,7 @@ func TestCompleteTLSConfiguration(t *testing.T) {
11971253
name: "both stages TLS",
11981254
enableTLS: []string{"prefiller", "decoder"},
11991255
tlsInsecureSkipVerify: []string{},
1200-
vllmPort: "9000",
1256+
modelServerPort: "9000",
12011257
expectedDecoderURL: "https://localhost:9000",
12021258
expectedUseTLSForPrefiller: true,
12031259
expectedUseTLSForDecoder: true,
@@ -1208,7 +1264,7 @@ func TestCompleteTLSConfiguration(t *testing.T) {
12081264
name: "TLS with insecure skip verify",
12091265
enableTLS: []string{"prefiller", "decoder"},
12101266
tlsInsecureSkipVerify: []string{"prefiller", "decoder"},
1211-
vllmPort: "8001",
1267+
modelServerPort: "8001",
12121268
expectedDecoderURL: "https://localhost:8001",
12131269
expectedUseTLSForPrefiller: true,
12141270
expectedUseTLSForDecoder: true,
@@ -1222,7 +1278,7 @@ func TestCompleteTLSConfiguration(t *testing.T) {
12221278
opts := NewOptions()
12231279
opts.enableTLS = tt.enableTLS
12241280
opts.tlsInsecureSkipVerify = tt.tlsInsecureSkipVerify
1225-
opts.vllmPort = tt.vllmPort
1281+
opts.modelServerPort = tt.modelServerPort
12261282

12271283
err := opts.Complete()
12281284
if err != nil {

0 commit comments

Comments
 (0)