feat(discovery)!: tagged discovery config with one runtime conversion

[XinyueZhang369]

Description

Problem

RouterConfig.discovery is the validated, serializable discovery configuration, but discovery doesn't run from it. The Rust CLI and the Python binding each build a second, runtime ServiceDiscoveryConfig beside it, mostly straight from the flags again, and server::startup starts discovery from that copy:

CLI flags ──to_router_config()──► RouterConfig.discovery                (validated)
    └──────to_server_config()───► ServerConfig.service_discovery_config  (run)

Nothing ties the two together, so a flag wired into only one is silently lost on the other: #2359 had to wire the KV annotation flags into both. The mesh router config is assembled separately on each path too.

There is also no way to name a provider. --service-discovery is a Kubernetes boolean and DiscoveryConfig is a flat Kubernetes struct with an enabled flag, so the file provider that comes next has nowhere to plug in.

Solution

RouterConfig.discovery becomes one tagged provider configuration, and every input surface reaches the runtime through one conversion, RuntimeDiscoveryConfig::from_config. Kubernetes is still the only provider.

# Reads as before: no `provider` means Kubernetes; `enabled: false` means off.
discovery:
  enabled: true
  selector: { app: sglang }
  # ...

# Written from now on: the tag, and no `enabled` (presence means on).
discovery:
  provider: kubernetes
  selector: { app: sglang }
  # ...

--discovery-provider kubernetes selects the provider on the command line, with --service-discovery kept as its legacy spelling.

Changes

Configuration (config/types.rs). DiscoveryConfig is an enum tagged by provider with one variant, Kubernetes(KubernetesDiscoveryConfig): the old fields minus enabled. RouterConfig.discovery reads through deserialize_discovery, which accepts the flat legacy object: a missing provider means Kubernetes, and enabled: false becomes None at deserialization, so writing the config back can't turn discovery on. Validation dispatches per provider; the Kubernetes checks are unchanged.

One conversion (service_discovery/runtime.rs). RuntimeDiscoveryConfig::from_config(&DiscoveryConfig, &RoutingMode) builds the runtime config. It derives what the runtime needs beyond the configuration: the interval as a Duration, the disaggregated flag from the routing mode, and the parsed ModelIdSource. main.rs and the Python binding both call it in place of their hand-built copies, and MeshDiscoveryConfig::from_discovery likewise replaces the two hand-built mesh configs. start_service_discovery dispatches on the provider.

CLI. --discovery-provider <kubernetes> conflicts with --service-discovery; giving both is a usage error rather than a precedence rule. IGW auto-enable and the worker-auto-recovery default follow the selected provider, whichever spelling chose it.

Python. --discovery-provider joins the Python CLI in a mutually exclusive group with --service-discovery. RouterArgs.discovery_provider is appended at the tail, since the field order is frozen for positional callers. Router.from_args passes Kubernetes on as service_discovery=True and raises for a provider it can't pass on, so a provider added to the CLI choices without wiring fails loudly instead of starting the router without discovery. The binding gains a keyword-only discovery= mapping, read by deserialize_discovery like RouterConfig.discovery; passing it together with service_discovery=True raises ValueError.

kind E2E. The in-cluster gateway now starts through --discovery-provider kubernetes, and the host-process journey keeps --service-discovery, so every run covers both spellings.

Compatibility

CLI flags, Python arguments and serialized configs all keep working. Rust callers need a small migration:

Before	After
DiscoveryConfig { enabled: true, .. }	DiscoveryConfig::Kubernetes(KubernetesDiscoveryConfig { .. }), or .into()
DiscoveryConfig::default()	KubernetesDiscoveryConfig::default()
ServiceDiscoveryConfig { enabled, .. }	enabled removed; None around it means off
ServerConfig.service_discovery_config: Option<ServiceDiscoveryConfig>	Option<RuntimeDiscoveryConfig>

DiscoveryConfig deliberately has no Default: the old default was disabled, and an enum default would silently mean enabled Kubernetes.

One serialized edge: YAML read into RouterConfig must quote label values that look like numbers or booleans (version: "1"), as a Kubernetes manifest already must. The flat struct let serde_yaml read version: 1 into a String. A tagged union is buffered before its fields are read, so a YAML scalar keeps the type YAML gave it. No in-tree path reads RouterConfig from YAML.

Deviations from the plan

• Only the Kubernetes variant. File, Slurm and Consul variants land with their providers, so the CLI never accepts a provider that can't run.
• No kubernetes_legacy() constructor. From<KubernetesDiscoveryConfig> covers the migration.
• Helm and independent mesh-router discovery are left for a follow-up: the discovery.provider selector with RBAC gated on the providers in use, and a router-discovery surface that runs without worker discovery. Router discovery is still configured inside Kubernetes discovery, as today.

Test Plan

New tests:

• config/types.rs: legacy_flat_discovery_reads_as_kubernetes, untagged_discovery_without_enabled_reads_as_kubernetes, tagged_kubernetes_discovery_reads_as_kubernetes, disabled_legacy_discovery_reads_as_none_and_stays_none, discovery_serializes_tagged_without_enabled, legacy_flat_discovery_reads_from_yaml, unknown_discovery_provider_is_rejected, non_boolean_discovery_enabled_is_rejected.
• service_discovery/runtime.rs: kubernetes_settings_reach_the_runtime (every field, the KV annotations included), disaggregated_modes_select_role_selectors, invalid_model_id_source_is_a_config_error.
• main.rs: discovery_provider_kubernetes_matches_service_discovery (both spellings build identical router, worker-discovery and mesh configs), service_discovery_and_discovery_provider_conflict, selecting_a_discovery_provider_enables_igw; worker_auto_recovery_follows_service_discovery_by_default now covers the new spelling.
• Python: test_parse_discovery_provider_kubernetes, test_service_discovery_and_discovery_provider_are_exclusive, test_unknown_discovery_provider_is_rejected, test_selected_discovery_provider_programmatic, test_discovery_provider_kubernetes_enables_igw, test_from_args_passes_discovery_provider_as_service_discovery, test_from_args_rejects_a_provider_it_cannot_pass (fails against a plain == "kubernetes" mapping), and TestDiscoveryMapping (tagged and untagged mappings accepted, conflict with service_discovery=True, unknown provider rejected).

Gate	Result
cargo +nightly fmt --all -- --check	clean
cargo clippy --workspace --all-targets --all-features -- -D warnings	clean¹
lib	2072 passed
bins (smg, amg)	44 passed each
k8s_discovery_test	21 passed
Python bindings/python/tests against a wheel built from this branch	334 passed, 4 skipped
pre-commit hooks on the changed files	pass

¹ Run on Rust 1.95, which also flags nonminimal_bool at routers/common/kv_transfer.rs:342 on main; that lint was allowed for the run. CI's pinned 1.98 doesn't flag it.

Checklist

• cargo +nightly fmt passes
• cargo clippy --all-targets --all-features -- -D warnings passes
• (Optional) Documentation updated
• (Optional) Please join us on Slack #sig-smg to discuss, review, and merge PRs

[coderabbitai]

Note

Currently processing new changes in this PR. This may take a few minutes, please wait...

⚙️ Run configuration

• Configuration used: Organization UI
• Review profile: CHILL
• Plan: Team
• Run ID: 217e6811-bc10-49c6-bdde-31e2ca7ed8df

📥 Commits

Reviewing files that changed from the base of the PR and between 2cddd26 and 12051b0.

📒 Files selected for processing (2)

• bindings/python/src/smg/router.py
• bindings/python/tests/test_startup_sequence.py

 __________________________________________________________
< Not just a pretty face, but a pretty good code reviewer! >
 ----------------------------------------------------------
  \
   \   \
        \ /\
        ( )
      .( o ).

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Organization UI
• Review profile: CHILL
• Plan: Team
• Run ID: 39f9deb3-6b8d-4321-86fe-7b18003751ad

📥 Commits

Reviewing files that changed from the base of the PR and between 9f4bce9 and 2cddd26.

📒 Files selected for processing (19)

• bindings/python/src/lib.rs
• bindings/python/src/smg/launch_router.py
• bindings/python/src/smg/router.py
• bindings/python/src/smg/router_args.py
• bindings/python/tests/test_arg_parser.py
• bindings/python/tests/test_router_config.py
• bindings/python/tests/test_startup_sequence.py
• e2e_test/kind_discovery/conftest.py
• e2e_test/kind_discovery/in_cluster.yaml
• model_gateway/src/config/builder.rs
• model_gateway/src/config/types.rs
• model_gateway/src/config/validation.rs
• model_gateway/src/main.rs
• model_gateway/src/mesh_discovery/kubernetes.rs
• model_gateway/src/server.rs
• model_gateway/src/service_discovery/kubernetes.rs
• model_gateway/src/service_discovery/mod.rs
• model_gateway/src/service_discovery/runtime.rs
• model_gateway/tests/k8s_discovery_test.rs

💤 Files with no reviewable changes (1)

• model_gateway/tests/k8s_discovery_test.rs

Included review availability: This review used your included allowance. 9 included reviews remain after this review. Your included PR review attempts over the past 7 days set your current allowance at 10 reviews per hour.

---

📝 Summary

Summary by CodeRabbit

• New Features
  • Added Kubernetes discovery provider configuration through discovery settings or the --discovery-provider kubernetes option. Omitting the provider defaults to Kubernetes.
  • The existing --service-discovery option remains supported as an alternative.
  • Enabling discovery now automatically enables IGW when it is not already enabled and defaults unhealthy-worker recovery to on.
• Bug Fixes
  • Invalid discovery settings and conflicting option combinations now return clear errors.

Walkthrough

The change introduces provider-tagged Kubernetes discovery configuration and adds provider selection to the Python and CLI interfaces. Startup derives worker and mesh discovery settings from the selected router configuration.

Changes

Kubernetes discovery provider

Layer / File(s)	Summary
Discovery configuration contract model_gateway/src/config/*, bindings/python/src/lib.rs, bindings/python/tests/test_router_config.py	Discovery configuration now uses a Kubernetes provider variant and accepts tagged or legacy flat input. Builders, validation, and the Python router mapping use the updated configuration.
Provider selection and router entry points model_gateway/src/main.rs, bindings/python/src/smg/*, bindings/python/tests/test_arg_parser.py, bindings/python/tests/test_startup_sequence.py, e2e_test/kind_discovery/*	The CLI accepts --discovery-provider kubernetes alongside the legacy --service-discovery spelling, and rejects using both. Python argument handling resolves the provider and forwards discovery settings to the Rust router.
Runtime conversion and discovery startup model_gateway/src/service_discovery/*, model_gateway/src/mesh_discovery/kubernetes.rs, model_gateway/src/server.rs, model_gateway/src/main.rs, bindings/python/src/lib.rs, model_gateway/tests/k8s_discovery_test.rs	Startup converts router discovery settings into runtime Kubernetes configuration. Worker discovery starts through the runtime dispatcher, and mesh discovery settings derive from the router discovery configuration.

Priority: ➖ Normal

Estimated code review effort: 4 (Complex) | ~45 minutes

Change: Feature

Sequence Diagram(s)

sequenceDiagram
  participant RouterArgs
  participant RouterFromArgs
  participant RustRouter
  participant RuntimeDiscoveryConfig
  participant KubernetesDiscovery
  RouterArgs->>RouterFromArgs: resolve selected discovery provider
  RouterFromArgs->>RustRouter: pass Kubernetes discovery configuration
  RustRouter->>RuntimeDiscoveryConfig: convert settings for routing mode
  RuntimeDiscoveryConfig->>KubernetesDiscovery: start provider discovery

Loading

Suggested reviewers: slin1237

Merge Risk: ⚪ Minimal · up to 2cddd

No actionable issue remains from these findings; the change is mergeable after normal checks.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 63.27% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 98 functions across 17 files. (1 skipped:…	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title clearly summarizes the main changes: tagged discovery configuration and a unified runtime conversion.
Description check	✅ Passed	The description explains the discovery configuration changes, runtime conversion, CLI and Python updates, compatibility, and tests.

Full details: Docstring Coverage

Explanation

Docstring coverage is 63.27% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 98 functions across 17 files. (1 skipped: 1 unsupported.)

• Fix all pre-merge checks with AI

✨ Finishing Touches

📝 Generate docstrings

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

---

_{Comment @coderabbitai help to get the list of available commands.}