diff --git a/sgl-router/py_src/sglang_router/router_args.py b/sgl-router/py_src/sglang_router/router_args.py index d2d44542c..04077b9de 100644 --- a/sgl-router/py_src/sglang_router/router_args.py +++ b/sgl-router/py_src/sglang_router/router_args.py @@ -677,12 +677,9 @@ class RouterArgs: def _validate_router_args(self): # Validate configuration based on mode if self.pd_disaggregation: - # Validate PD configuration - skip URL requirements if using service discovery - if not self.service_discovery: - if not self.prefill_urls: - raise ValueError("PD disaggregation mode requires --prefill") - if not self.decode_urls: - raise ValueError("PD disaggregation mode requires --decode") + # Allow empty URLs even without service discovery to support dynamic worker addition + # URLs will be validated separately if provided + pass # Warn about policy usage in PD mode if self.prefill_policy and self.decode_policy and self.policy: diff --git a/sgl-router/py_test/unit/test_router_config.py b/sgl-router/py_test/unit/test_router_config.py index 6343d3b9e..5ba91cb1d 100644 --- a/sgl-router/py_test/unit/test_router_config.py +++ b/sgl-router/py_test/unit/test_router_config.py @@ -53,8 +53,8 @@ class TestRouterConfigValidation: assert args.decode_urls == ["http://decode1:8001", "http://decode2:8001"] assert args.policy == "cache_aware" - def test_pd_config_without_urls_raises_error(self): - """Test that PD mode without URLs raises validation error.""" + def test_pd_config_without_urls_allowed(self): + """Test that PD mode without URLs is now allowed (URLs are optional).""" args = RouterArgs( pd_disaggregation=True, prefill_urls=[], @@ -62,11 +62,14 @@ class TestRouterConfigValidation: service_discovery=False, ) - # This should raise an error when trying to launch - with pytest.raises( - ValueError, match="PD disaggregation mode requires --prefill" - ): + # Should not raise validation error - URLs are now optional + with patch("sglang_router.launch_router.Router") as router_mod: + mock_router_instance = MagicMock() + router_mod.from_args = MagicMock(return_value=mock_router_instance) + + # This should succeed without raising an error launch_router(args) + router_mod.from_args.assert_called_once() def test_pd_config_with_service_discovery_allows_empty_urls(self): """Test that PD mode with service discovery allows empty URLs.""" diff --git a/sgl-router/py_test/unit/test_startup_sequence.py b/sgl-router/py_test/unit/test_startup_sequence.py index 11799627b..6a40a67f4 100644 --- a/sgl-router/py_test/unit/test_startup_sequence.py +++ b/sgl-router/py_test/unit/test_startup_sequence.py @@ -464,7 +464,7 @@ class TestStartupValidation: def test_pd_mode_validation_during_startup(self): """Test PD mode validation during startup.""" - # PD mode without URLs should fail + # PD mode without URLs is now allowed (URLs are optional) args = RouterArgs( pd_disaggregation=True, prefill_urls=[], @@ -472,10 +472,14 @@ class TestStartupValidation: service_discovery=False, ) - with pytest.raises( - ValueError, match="PD disaggregation mode requires --prefill" - ): + # Should not raise validation error - URLs are now optional + with patch("sglang_router.launch_router.Router") as router_mod: + mock_router_instance = MagicMock() + router_mod.from_args = MagicMock(return_value=mock_router_instance) + + # This should succeed without raising an error launch_router(args) + router_mod.from_args.assert_called_once() def test_pd_mode_with_service_discovery_validation(self): """Test PD mode with service discovery validation during startup.""" diff --git a/sgl-router/py_test/unit/test_validation.py b/sgl-router/py_test/unit/test_validation.py index e6eef3102..587cd9504 100644 --- a/sgl-router/py_test/unit/test_validation.py +++ b/sgl-router/py_test/unit/test_validation.py @@ -400,9 +400,9 @@ class TestConfigurationValidation: class TestLaunchValidation: """Test launch-time validation logic.""" - def test_pd_mode_requires_urls(self): - """Test that PD mode requires prefill and decode URLs.""" - # PD mode without URLs should fail + def test_pd_mode_allows_empty_urls(self): + """Test that PD mode now allows empty URLs (URLs are optional).""" + # PD mode without URLs is now allowed args = RouterArgs( pd_disaggregation=True, prefill_urls=[], @@ -410,10 +410,14 @@ class TestLaunchValidation: service_discovery=False, ) - with pytest.raises( - ValueError, match="PD disaggregation mode requires --prefill" - ): + # Should not raise validation error - URLs are now optional + with patch("sglang_router.launch_router.Router") as router_mod: + mock_router_instance = MagicMock() + router_mod.from_args = MagicMock(return_value=mock_router_instance) + + # This should succeed without raising an error launch_router(args) + router_mod.from_args.assert_called_once() def test_pd_mode_with_service_discovery_allows_empty_urls(self): """Test that PD mode with service discovery allows empty URLs.""" diff --git a/sgl-router/src/config/validation.rs b/sgl-router/src/config/validation.rs index b56a116c6..d9905e766 100644 --- a/sgl-router/src/config/validation.rs +++ b/sgl-router/src/config/validation.rs @@ -6,9 +6,7 @@ pub struct ConfigValidator; impl ConfigValidator { pub fn validate(config: &RouterConfig) -> ConfigResult<()> { - let has_service_discovery = config.discovery.as_ref().is_some_and(|d| d.enabled); - - Self::validate_mode(&config.mode, has_service_discovery)?; + Self::validate_mode(&config.mode)?; Self::validate_policy(&config.policy)?; Self::validate_server_settings(config)?; @@ -89,7 +87,7 @@ impl ConfigValidator { Ok(()) } - fn validate_mode(mode: &RoutingMode, has_service_discovery: bool) -> ConfigResult<()> { + fn validate_mode(mode: &RoutingMode) -> ConfigResult<()> { match mode { RoutingMode::Regular { worker_urls } => { if !worker_urls.is_empty() { @@ -103,19 +101,8 @@ impl ConfigValidator { prefill_policy, decode_policy, } => { - if !has_service_discovery { - if prefill_urls.is_empty() { - return Err(ConfigError::ValidationFailed { - reason: "PD mode requires at least one prefill worker URL".to_string(), - }); - } - if decode_urls.is_empty() { - return Err(ConfigError::ValidationFailed { - reason: "PD mode requires at least one decode worker URL".to_string(), - }); - } - } - + // Allow empty URLs even without service discovery to support dynamic worker addition + // URLs will be validated if provided if !prefill_urls.is_empty() { let prefill_url_strings: Vec = prefill_urls.iter().map(|(url, _)| url.clone()).collect(); @@ -145,12 +132,11 @@ impl ConfigValidator { } } RoutingMode::OpenAI { worker_urls } => { - if worker_urls.is_empty() { - return Err(ConfigError::ValidationFailed { - reason: "OpenAI mode requires at least one --worker-urls entry".to_string(), - }); + // Allow empty URLs to support dynamic worker addition + // URLs will be validated if provided + if !worker_urls.is_empty() { + Self::validate_urls(worker_urls)?; } - Self::validate_urls(worker_urls)?; } } Ok(()) @@ -888,6 +874,45 @@ mod tests { ); } + #[test] + fn test_validate_empty_urls_allowed_without_service_discovery() { + // Test that empty URLs are now allowed in PD mode + let config = RouterConfig::new( + RoutingMode::PrefillDecode { + prefill_urls: vec![], + decode_urls: vec![], + prefill_policy: None, + decode_policy: None, + }, + PolicyConfig::Random, + ); + + // Should pass validation even with empty URLs + assert!(ConfigValidator::validate(&config).is_ok()); + + // Test that empty URLs are allowed in Regular mode + let config = RouterConfig::new( + RoutingMode::Regular { + worker_urls: vec![], + }, + PolicyConfig::Random, + ); + + // Should pass validation even with empty URLs + assert!(ConfigValidator::validate(&config).is_ok()); + + // Test that empty URLs are allowed in OpenAI mode + let config = RouterConfig::new( + RoutingMode::OpenAI { + worker_urls: vec![], + }, + PolicyConfig::Random, + ); + + // Should pass validation even with empty URLs + assert!(ConfigValidator::validate(&config).is_ok()); + } + #[test] fn test_validate_grpc_requires_tokenizer() { let mut config = RouterConfig::new( diff --git a/sgl-router/src/main.rs b/sgl-router/src/main.rs index e87b57139..34c9c4609 100644 --- a/sgl-router/src/main.rs +++ b/sgl-router/src/main.rs @@ -478,12 +478,7 @@ impl CliArgs { } else if self.pd_disaggregation { let decode_urls = self.decode.clone(); - if !self.service_discovery && (prefill_urls.is_empty() || decode_urls.is_empty()) { - return Err(ConfigError::ValidationFailed { - reason: "PD disaggregation mode requires --prefill and --decode URLs when not using service discovery".to_string(), - }); - } - + // Allow empty URLs to support dynamic worker addition RoutingMode::PrefillDecode { prefill_urls, decode_urls, @@ -491,12 +486,7 @@ impl CliArgs { decode_policy: self.decode_policy.as_ref().map(|p| self.parse_policy(p)), } } else { - if !self.service_discovery && self.worker_urls.is_empty() { - return Err(ConfigError::ValidationFailed { - reason: "Regular mode requires --worker-urls when not using service discovery" - .to_string(), - }); - } + // Allow empty URLs to support dynamic worker addition RoutingMode::Regular { worker_urls: self.worker_urls.clone(), }