From 79b48fa4c80551e41586dfd449790c5474f2876e Mon Sep 17 00:00:00 2001 From: Ishan Dhanani Date: Mon, 21 Sep 2026 16:59:14 -0700 Subject: [PATCH 1/7] refactor(frontends): registry, required_backend, and validate(config) on the protocol frontend.type now resolves through a registry populated by @register_frontend on each implementation, imported from the package __init__; the FrontendType literal and the if-chain in get_frontend are gone. Each frontend declares required_backend and its recipe rules in validate(config), so SrtConfig has one _validate_frontend that checks the type is registered, enforces the pairing generically, and reports the frontend's ValueError as a load-time ValidationError. The four per-frontend schema validators (trtllm_serve, vllm, sglang, static routers) move into their classes with their messages unchanged; the direct sglang pairing message becomes the shared form. Adding a frontend is now one module under srtctl/frontends plus one import. Dry-run output for all 26 example recipes is unchanged. Signed-off-by: Ishan Dhanani --- CLAUDE.md | 8 +- src/srtctl/core/schema.py | 186 +++----------------------- src/srtctl/frontends/__init__.py | 13 +- src/srtctl/frontends/base.py | 98 ++++++++------ src/srtctl/frontends/dynamo.py | 11 +- src/srtctl/frontends/sglang.py | 4 +- src/srtctl/frontends/sglang_direct.py | 32 ++++- src/srtctl/frontends/static_router.py | 19 ++- src/srtctl/frontends/trtllm_serve.py | 18 ++- src/srtctl/frontends/vllm.py | 23 +++- src/srtctl/frontends/vllm_router.py | 82 +++++++++++- tests/test_frontends.py | 86 +++++++++++- tests/test_sglang_direct_frontend.py | 2 +- 13 files changed, 359 insertions(+), 223 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 78534e854..38e2ed033 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -57,7 +57,7 @@ Read these before adding a feature. Each rule names the existing pattern to reus - **One resolver per overridable setting.** A setting the recipe can set at engine level and override per role (`roles..args.connector`, DP size) has one accessor on the backend in the `get_config_for_mode` style, and every consumer uses it: command builder, process env, frontend, and schema validator. Two readers of the raw fields disagree the moment a role override appears. - **Frontends own readiness; backends own worker commands and ports.** `core/health.py` and the stage mixins contain no `frontend_type == "..."` checks and no `getattr(frontend, "hook", fallback)` probing. The frontend implements the protocol hook; if a hook is missing, add it to `FrontendProtocol`. A frontend asks a backend a question through a method (`backend.is_grpc_mode(mode)`), never by reading its fields by name. - **Every listener a process opens comes from the allocator.** Two processes can share a node in this repo (`nodes: colocate`, DP endpoints), so any port a worker binds (HTTP, bootstrap, side channel, handshake, notify, metrics) is allocated by `NodePortAllocator` and carried on `Process`. An upstream default port left in a generated config is a collision on the first colocated recipe. See Ports below. -- **Modes are not types.** A new `frontend.type`, `services[].type`, or `engine.type` is for a different process with its own launch, health API, and registration model. A different CLI shape, transport, or discovery mode of the same binary is an override inside the existing class: `trtllm_serve` handles aggregate and disaggregated in one type, `sglang-router` picks http or grpc per mode. A new type string is checked in a dozen places (`FrontendType`, `get_frontend`, the schema pairing map, health expectations, telemetry targets, the backend command builder); count them before choosing. +- **Modes are not types.** A new `frontend.type`, `services[].type`, or `engine.type` is for a different process with its own launch, health API, and registration model. A different CLI shape, transport, or discovery mode of the same binary is an override inside the existing class: `trtllm_serve` handles aggregate and disaggregated in one type, `sglang-router` picks http or grpc per mode. A new frontend type is one registered module, but the name is still read by health expectations, telemetry targets, and the backend command builder; count those sites before choosing. - **Check upstream before working around it.** When a change encodes an upstream behavior (what a health endpoint returns, which keys a connector reads, what a flag does), read the upstream source at the version the container ships and cite the commit in the PR. Do not add a probe, shim, or port-scan workaround for something upstream already handles. - **Reuse the machinery before adding a mechanism.** Services plus `placement` before a bespoke launcher, `roles..restart` before a wrapper loop, `host_setup` before a setup script that needs the host. The smallest diff that rides existing machinery beats a self-contained new module. - **A user-visible feature ships complete.** A `tests/` case (dry-run for visible config, mock orchestrator for behavior), a `docs/` page or section, an example recipe under `examples/`, and regenerated `docs/schema-reference.md`. In a stacked PR, a test lives in the layer that introduces the behavior it asserts. @@ -105,9 +105,9 @@ For aggregated mode, pass `expected_prefill=0, expected_decode=num_agg`. ### Frontends -The frontend is the process that owns the public OpenAI port (`FRONTEND_PUBLIC_PORT`) and decides when the job is ready. `frontend.type` selects an implementation through `get_frontend()` in `src/srtctl/frontends/base.py`: `dynamo` (etcd/NATS discovery), `sglang-router` and `vllm-router` (static URL routers, both built on `StaticRouterFrontend`), `trtllm_serve` (direct aggregate or the disaggregated orchestrator), `sglang` and `vllm` (one direct worker owns the port), `none` (services-only, no gate). +The frontend is the process that owns the public OpenAI port (`FRONTEND_PUBLIC_PORT`) and decides when the job is ready. Implementations live under `src/srtctl/frontends/`, register with `@register_frontend("")`, and are imported from the package `__init__`; `frontend.type` resolves through that registry (`get_frontend`, `list_frontend_types`) and nowhere else. Registered today: `dynamo` (etcd/NATS discovery), `sglang-router` and `vllm-router` (static URL routers, both built on `StaticRouterFrontend`), `trtllm_serve` (direct aggregate or the disaggregated orchestrator), `sglang` and `vllm` (one direct worker owns the port); `none` is the services-only job with no implementation and no gate. -`FrontendProtocol` hooks: `health_endpoint` and `parse_health` (readiness), `get_backend_health_urls` (second gate: every advertised worker URL must answer 200 before traffic), `start_frontends` (launch on `topology.frontend_nodes`, one `ManagedProcess` per node with a `step_name`), `get_frontend_args_list` (`frontend.args` to CLI). +`FrontendProtocol` hooks: `required_backend` and `validate(config)` (recipe rules, run by `SrtConfig._validate_frontend` at load so dry-run catches them; raise `ValueError` with the user-facing message), `health_endpoint` and `parse_health` (readiness), `get_backend_health_urls` (second gate: every advertised worker URL must answer 200 before traffic), `start_frontends` (launch on `topology.frontend_nodes`, one `ManagedProcess` per node with a `step_name`), `get_frontend_args_list` (`frontend.args` to CLI). The schema knows no individual frontend: pairing and per-type rules come from these two members. `StaticRouterFrontend` (`frontends/static_router.py`) is the base for routers that take worker URLs on the command line. Subclasses set `executable`, `pd_flag`, `process_name` and override only what differs: `worker_scheme` (http or grpc per mode), `worker_bootstrap_port` (the P/D port advertised next to a prefill URL), `resolve_worker_host`, `get_managed_frontend_args` (arguments derived from the allocated topology; a conflicting user value raises instead of being overwritten), `build_bash_preamble`, `build_router_command`, `start_process` (test seam). `collect_workers` treats a positive `Process.http_port` as the definition of a routable worker. @@ -375,7 +375,7 @@ with patch.dict(os.environ, H100Rack.slurm_env()): ### Adding a Router Mode or a New Frontend Type -Decide first whether it is a mode or a type (see Design Rules). A mode of an existing router (a discovery flag, another connector, a transport) is an override in the existing frontend class, keyed on a backend method that resolves the effective per-role setting, with `StaticRouterFrontend` left unchanged. A new type is a different process: add it to `FrontendType` and `get_frontend` in `frontends/base.py`, the backend pairing map in `SrtConfig._validate_static_router_frontend` or its own validator, `_get_health_expectations` if it reports counts differently, and the telemetry scrape targets in `core/telemetry.py`; then a `tests/test__frontend.py` using `start_process` as the seam, a `docs/.md` page, and an `examples/` recipe. +Decide first whether it is a mode or a type (see Design Rules). A mode of an existing router (a discovery flag, another connector, a transport) is an override in the existing frontend class, keyed on a backend method that resolves the effective per-role setting, with `StaticRouterFrontend` left unchanged. A new type is a different process: one module under `frontends/` decorated with `@register_frontend("")`, imported from `frontends/__init__.py`, carrying `required_backend` and its recipe rules in `validate(config)` (no schema edits); then `_get_health_expectations` if it reports counts differently and the telemetry scrape targets in `core/telemetry.py` until those move onto the protocol; then a `tests/test__frontend.py` using `start_process` as the seam, a `docs/.md` page, and an `examples/` recipe. ### Adding a New Benchmark diff --git a/src/srtctl/core/schema.py b/src/srtctl/core/schema.py index 04189319a..001e219dd 100755 --- a/src/srtctl/core/schema.py +++ b/src/srtctl/core/schema.py @@ -2294,10 +2294,7 @@ def __post_init__(self): self._validate_het_jobs() self._validate_colocated_decode() self._validate_dedicated_node_placement() - self._validate_trtllm_serve() - self._validate_vllm_frontend() - self._validate_sglang_direct_frontend() - self._validate_static_router_frontend() + self._validate_frontend() self._validate_dynamo_sidecar() self._validate_vllm_failover() self._validate_host_setup() @@ -2529,176 +2526,33 @@ def _validate_frontend_worker_selection(self): "or DYN_ROUTER_POLICY_CONFIG in frontend.env/environment" ) - def _validate_trtllm_serve(self): - """Catch trtllm_serve misconfigurations at load time (dry-run) instead of - failing mid-job at the frontend stage. + def _validate_frontend(self) -> None: + """``frontend.type`` must be registered, pair with the backend, and pass its own rules. - The trtllm_serve frontend supports either one direct aggregate worker or a - single ``trtllm-serve disaggregated`` orchestrator. Both use the - single-frontend path (no nginx/multi-frontend). + The registry in ``srtctl.frontends`` is the only list of frontend types. + Each implementation carries ``required_backend`` and ``validate``, so this + schema does not know individual frontends. ``none`` is the services-only + job and is covered by ``_validate_services_only``. """ - if self.frontend.type != "trtllm_serve": - return - if self.backend_type != "trtllm": - raise ValidationError( - f"frontend.type: trtllm_serve requires backend.type: trtllm; got {self.backend_type!r}" - ) - if self.frontend.enable_multiple_frontends: - raise ValidationError( - "frontend.type: trtllm_serve uses one public endpoint; set frontend.enable_multiple_frontends: false" - ) - if not self.resources.is_disaggregated and self.resources.num_agg != 1: - raise ValidationError( - "frontend.type: trtllm_serve aggregate mode requires exactly one " - "aggregate worker (set resources.agg_workers: 1)" - ) - - def _validate_vllm_frontend(self): - """Catch direct-vLLM frontend misconfigurations at load time. - - Direct vLLM means the aggregate `vllm serve` worker owns the OpenAI port - itself. It is not a disaggregated router and does not support the nginx - multi-frontend path. - """ - if self.frontend.type != "vllm": - return - if self.backend_type != "vllm": - raise ValidationError(f"frontend.type: vllm requires backend.type: vllm; got {self.backend_type!r}") - if self.frontend.enable_multiple_frontends: - raise ValidationError( - "frontend.type: vllm binds vllm serve directly; set frontend.enable_multiple_frontends: false" - ) - if self.resources.is_disaggregated: - raise ValidationError("frontend.type: vllm supports aggregate jobs only, not disaggregated layouts") - if self.resources.num_agg != 1: - raise ValidationError( - f"frontend.type: vllm supports exactly one aggregate worker, got {self.resources.num_agg}. " - "vllm serve owns the public port directly and there is no router to load-balance " - "replicas, so extra workers would either idle or collide on the port. " - "Use frontend.type: dynamo to run multiple aggregate workers, or scale a single " - "worker across nodes with resources.agg_nodes." - ) - - def _validate_sglang_direct_frontend(self): - """Catch direct-SGLang frontend misconfigurations at load time. - - ``frontend.type: sglang`` means the one aggregate ``sglang.launch_server`` - owns the public port itself. Several replicas or a prefill/decode layout - need ``sglang-router`` (or ``dynamo``); a schema 2 recipe that still says - ``sglang`` for those is an old router recipe and is rejected rather than - silently run unbalanced. - """ - if self.frontend.type != "sglang": - return - if self.backend_type != "sglang": - raise ValidationError(f"frontend.type: sglang requires engine sglang; got {self.backend_type!r}") - if self.frontend.enable_multiple_frontends: - raise ValidationError( - "frontend.type: sglang binds sglang.launch_server directly; set frontend.enable_multiple_frontends: false" - ) - if self.resources.is_disaggregated: - raise ValidationError( - "frontend.type: sglang supports one aggregate worker only, not a prefill/decode layout. " - "The SGLang router is frontend.type: sglang-router (renamed in 2.0; `srtctl migrate` rewrites " - "schema 1 recipes)." - ) - if self.resources.num_agg != 1: - raise ValidationError( - f"frontend.type: sglang supports exactly one aggregate worker, got {self.resources.num_agg}. " - "sglang.launch_server owns the public port directly and there is no router to balance " - "replicas. Use frontend.type: sglang-router (the SGLang Model Gateway, renamed in 2.0) or dynamo." - ) - if self.dynamo.sidecar: - raise ValidationError("frontend.type: sglang does not support dynamo.sidecar; use frontend.type: dynamo") - - def _validate_static_router_frontend(self): - """Validate static-router/backend pairings and vLLM DP ownership.""" - required_backend = {"sglang-router": "sglang", "vllm-router": "vllm"}.get(self.frontend.type) - if required_backend is None: - return - if self.backend_type != required_backend: - raise ValidationError( - f"frontend.type: {self.frontend.type} requires backend.type: {required_backend}; " - f"got {self.backend_type!r}" - ) - - if self.frontend.type != "vllm-router": + if self.frontend.type == "none": return - if not isinstance(self.backend, VLLMProtocol): - raise ValidationError(f"frontend.type: vllm-router requires backend.type: vllm; got {self.backend_type!r}") - backend = self.backend - - endpoint_gpu_counts: dict[Literal["prefill", "decode", "agg"], int] = { - "prefill": self.resources.gpus_per_prefill if self.resources.num_prefill else 0, - "decode": self.resources.gpus_per_decode if self.resources.num_decode else 0, - "agg": self.resources.gpus_per_agg if self.resources.num_agg else 0, - } - if backend.find_dp_modes() and backend.dp_launch_mode != "per_node": - raise ValidationError( - "frontend.type: vllm-router with data-parallel-size requires " - "backend.dp_launch_mode: per_node; deprecated per_gpu processes are " - "Dynamo registrations, not independently routable vLLM API servers" - ) + from srtctl.frontends import get_frontend, list_frontend_types - expansion_by_mode: dict[str, int] = {} - for mode, gpu_count in endpoint_gpu_counts.items(): - if gpu_count <= 0: - continue - if not backend._is_dp_mode(mode): - expansion_by_mode[mode] = 1 - continue - try: - configured_dp_size = backend._get_dp_size(mode) - dp_size = int(configured_dp_size) if configured_dp_size is not None else 1 - if dp_size < 1: - raise ValueError( - f"vLLM {mode} data-parallel-size must be a positive integer; got {configured_dp_size!r}" - ) - replica_size = backend._get_model_parallel_size(mode) - except (TypeError, ValueError) as exc: - raise ValidationError(str(exc)) from exc - - required_gpus = dp_size * replica_size - if required_gpus != gpu_count: - raise ValidationError( - f"vLLM Router {mode} parallelism requires DP*TP*PP*PCP=" - f"{dp_size}*{replica_size}={required_gpus} GPUs, " - f"but resources allocate {gpu_count} GPUs per worker" - ) - - local_gpu_count = min(gpu_count, self.resources.gpus_per_node) - if replica_size > local_gpu_count: - expansion_by_mode[mode] = 1 - else: - try: - expansion_by_mode[mode] = backend._get_local_dp_size(mode, local_gpu_count) - except ValueError as exc: - raise ValidationError(str(exc)) from exc - - expansions = set(expansion_by_mode.values()) - if len(expansions) > 1: - detail = ", ".join(f"{mode}={size}" for mode, size in expansion_by_mode.items()) - raise ValidationError( - "vLLM Router has one --intra-node-data-parallel-size for all worker pools, " - f"but the allocated topology derives different expansion factors: {detail}" - ) - - configured_expansion = (self.frontend.args or {}).get( - "intra-node-data-parallel-size", - (self.frontend.args or {}).get("intra_node_data_parallel_size"), - ) - derived_expansion = next(iter(expansions), 1) try: - configured_expansion_value = int(configured_expansion) if configured_expansion is not None else None - except (TypeError, ValueError) as exc: + frontend = get_frontend(self.frontend.type) + except ValueError: raise ValidationError( - f"frontend.args.intra-node-data-parallel-size must be an integer; got {configured_expansion!r}" - ) from exc - if configured_expansion_value is not None and configured_expansion_value != derived_expansion: + f"Unknown frontend.type {self.frontend.type!r}. Available: {', '.join(list_frontend_types())}" + ) from None + required = frontend.required_backend + if required is not None and self.backend_type != required: raise ValidationError( - "frontend.args.intra-node-data-parallel-size conflicts with the allocated vLLM topology: " - f"configured {configured_expansion}, derived {derived_expansion}" + f"frontend.type: {self.frontend.type} requires backend.type: {required}; got {self.backend_type!r}" ) + try: + frontend.validate(self) + except ValueError as exc: + raise ValidationError(str(exc)) from exc def _validate_het_jobs(self): """When ``resources.het_jobs`` is set to True, enforce supported shape. diff --git a/src/srtctl/frontends/__init__.py b/src/srtctl/frontends/__init__.py index d4ab7f432..4310d33e2 100644 --- a/src/srtctl/frontends/__init__.py +++ b/src/srtctl/frontends/__init__.py @@ -4,18 +4,25 @@ """ Frontend implementations for routing requests to backend workers. +Each module registers its implementation with ``@register_frontend("")``; +importing it from here is what makes ``frontend.type: `` resolvable. +``list_frontend_types()`` is the registry, ``none`` included. + Supported frontend types: - dynamo: Dynamo frontend with NATS/etcd communication - sglang: Direct sglang.launch_server for a single aggregate worker (no router) - sglang-router: SGLang Model Gateway router in front of static workers +- trtllm_serve: Direct trtllm-serve worker or the disaggregated orchestrator - vllm: Direct vLLM OpenAI server for aggregate jobs - vllm-router: Official vLLM Router with static aggregate or P/D workers """ from srtctl.frontends.base import ( + FRONTEND_NONE, FrontendProtocol, - FrontendType, get_frontend, + list_frontend_types, + register_frontend, ) from srtctl.frontends.dynamo import DynamoFrontend from srtctl.frontends.sglang import SGLangRouterFrontend @@ -25,13 +32,15 @@ from srtctl.frontends.vllm_router import VLLMRouterFrontend __all__ = [ + "FRONTEND_NONE", "DynamoFrontend", "FrontendProtocol", - "FrontendType", "SGLangFrontend", "SGLangRouterFrontend", "TRTLLMServeFrontend", "VLLMFrontend", "VLLMRouterFrontend", "get_frontend", + "list_frontend_types", + "register_frontend", ] diff --git a/src/srtctl/frontends/base.py b/src/srtctl/frontends/base.py index a99c34b48..5ee4befa0 100644 --- a/src/srtctl/frontends/base.py +++ b/src/srtctl/frontends/base.py @@ -11,7 +11,7 @@ """ import threading -from typing import TYPE_CHECKING, Any, Literal, Protocol +from typing import TYPE_CHECKING, Any, ClassVar, Protocol if TYPE_CHECKING: from srtctl.core.health import WorkerHealthResult @@ -19,11 +19,11 @@ from srtctl.core.runtime import RuntimeContext from srtctl.core.topology import Process -# Supported frontend types - extensible by adding new literals. ``none`` is a -# services-only job: no router process, no OpenAI endpoint, no worker-count -# health gate (see SrtConfig._validate_services_only); it has no implementation -# and every stage short-circuits on it before calling get_frontend(). -FrontendType = Literal["dynamo", "sglang", "sglang-router", "trtllm_serve", "vllm", "vllm-router", "none"] +# ``frontend.type: none`` is a services-only job: no router process, no OpenAI +# endpoint, no worker-count health gate (see SrtConfig._validate_services_only). +# It has no implementation and every stage short-circuits on it before calling +# get_frontend(). +FRONTEND_NONE = "none" class FrontendProtocol(Protocol): @@ -33,13 +33,32 @@ class FrontendProtocol(Protocol): 1. Starting router/frontend processes on designated nodes 2. Providing health check endpoint and response parsing 3. Building CLI arguments from config + 4. Its own recipe-level rules (``required_backend``, ``validate``) + + An implementation registers with ``@register_frontend("")``; the + recipe's ``frontend.type`` is resolved through that registry and nowhere + else, so adding a frontend is one module under ``srtctl/frontends/`` + imported from the package ``__init__``. """ + #: Backend type this frontend requires, or ``None`` for any backend. + #: ``SrtConfig._validate_frontend`` enforces it at config load. + required_backend: ClassVar[str | None] + @property def type(self) -> str: """Frontend type identifier (e.g., 'dynamo', 'sglang').""" ... + def validate(self, config: Any) -> None: + """Recipe-level rules for this frontend. + + Raise ``ValueError`` with the user-facing message; the schema reports it + as a load-time ValidationError so ``srtctl dry-run`` catches it before an + allocation is spent. The backend pairing is checked before this runs. + """ + ... + @property def health_endpoint(self) -> str: """HTTP endpoint for health checks (e.g., '/health', '/workers').""" @@ -93,45 +112,48 @@ def get_frontend_args_list(self, args: dict[str, Any] | None) -> list[str]: ... -def get_frontend(frontend_type: str) -> FrontendProtocol: - """Get frontend implementation by type. +_FRONTENDS: dict[str, type] = {} + + +def register_frontend(name: str): + """Class decorator registering a frontend implementation under ``frontend.type: ``.""" - Args: - frontend_type: Frontend type string (e.g., 'dynamo', 'sglang') + def decorator(cls): + _FRONTENDS[name] = cls + return cls - Returns: - Instantiated frontend implementation + return decorator + + +def _load_registry() -> None: + # The package __init__ imports every implementation module, which registers + # it. Imported lazily: implementations import core modules that import this + # one, and get_frontend() is only ever called at run time. + import srtctl.frontends # noqa: F401 + + +def list_frontend_types() -> list[str]: + """Every accepted ``frontend.type``, including ``none``.""" + _load_registry() + return sorted([*_FRONTENDS, FRONTEND_NONE]) + + +def get_frontend(frontend_type: str) -> FrontendProtocol: + """Instantiate the registered frontend implementation for ``frontend_type``. Raises: - ValueError: If frontend type is unknown + ValueError: for ``none`` (which has no implementation) and for unknown types """ - # Import here to avoid circular imports - from srtctl.frontends.dynamo import DynamoFrontend - from srtctl.frontends.sglang import SGLangRouterFrontend - from srtctl.frontends.sglang_direct import SGLangFrontend - from srtctl.frontends.trtllm_serve import TRTLLMServeFrontend - from srtctl.frontends.vllm import VLLMFrontend - from srtctl.frontends.vllm_router import VLLMRouterFrontend - - if frontend_type == "dynamo": - return DynamoFrontend() - elif frontend_type == "sglang": - return SGLangFrontend() - elif frontend_type == "sglang-router": - return SGLangRouterFrontend() - elif frontend_type == "trtllm_serve": - return TRTLLMServeFrontend() - elif frontend_type == "vllm": - return VLLMFrontend() - elif frontend_type == "vllm-router": - return VLLMRouterFrontend() - elif frontend_type == "none": + _load_registry() + if frontend_type == FRONTEND_NONE: raise ValueError( "frontend.type 'none' has no frontend implementation: services-only jobs skip the frontend layer " "and the health gate, so nothing should ask for one" ) - else: + try: + implementation = _FRONTENDS[frontend_type] + except KeyError: raise ValueError( - f"Unknown frontend type: {frontend_type!r}. Supported: dynamo, sglang, sglang-router, trtllm_serve, " - "vllm, vllm-router, none" - ) + f"Unknown frontend type: {frontend_type!r}. Supported: {', '.join(list_frontend_types())}" + ) from None + return implementation() diff --git a/src/srtctl/frontends/dynamo.py b/src/srtctl/frontends/dynamo.py index 497cfc106..6271573f8 100644 --- a/src/srtctl/frontends/dynamo.py +++ b/src/srtctl/frontends/dynamo.py @@ -10,7 +10,7 @@ import logging import shlex import threading -from typing import TYPE_CHECKING, Any +from typing import TYPE_CHECKING, Any, ClassVar import yaml @@ -18,6 +18,7 @@ from srtctl.core.observability_nsys import wrap_observability_nsys from srtctl.core.schema import build_otel_env from srtctl.core.slurm import CONTAINER_REMAP_ROOT_EXPORT, start_srun_process +from srtctl.frontends.base import register_frontend from srtctl.services.implicit import discovery_env if TYPE_CHECKING: @@ -31,6 +32,7 @@ ROUTER_POLICY_CONFIG_CONTAINER_PATH = f"/logs/{ROUTER_POLICY_CONFIG_FILENAME}" +@register_frontend("dynamo") class DynamoFrontend: """Dynamo frontend implementation. @@ -38,10 +40,17 @@ class DynamoFrontend: Health checks via /health endpoint. """ + # Dynamo fronts every engine; the dynamo.* rules (sidecar, failover, + # worker_selection) are dynamo-config validations and stay in the schema. + required_backend: ClassVar[str | None] = None + @property def type(self) -> str: return "dynamo" + def validate(self, config: Any) -> None: + del config + @property def health_endpoint(self) -> str: return "/health" diff --git a/src/srtctl/frontends/sglang.py b/src/srtctl/frontends/sglang.py index a318b4e4e..20d0f8dc2 100644 --- a/src/srtctl/frontends/sglang.py +++ b/src/srtctl/frontends/sglang.py @@ -10,6 +10,7 @@ from srtctl.core.slurm import get_hostname_ip, start_srun_process from srtctl.core.topology import Process +from srtctl.frontends.base import register_frontend from srtctl.frontends.static_router import StaticRouterFrontend from srtctl.ports import SGLANG_ROUTER_METRICS_PORT @@ -23,11 +24,12 @@ def router_metrics_port(frontend_args: dict[str, Any] | None) -> int: return SGLANG_ROUTER_METRICS_PORT +@register_frontend("sglang-router") class SGLangRouterFrontend(StaticRouterFrontend): """SGLang Model Gateway static router (`frontend.type: sglang-router`).""" type: ClassVar[str] = "sglang-router" - backend_type: ClassVar[str] = "sglang" + required_backend: ClassVar[str | None] = "sglang" executable: ClassVar[tuple[str, ...]] = ("python", "-m", "sglang_router.launch_router") pd_flag: ClassVar[str] = "--pd-disaggregation" process_name: ClassVar[str] = "sglang_router" diff --git a/src/srtctl/frontends/sglang_direct.py b/src/srtctl/frontends/sglang_direct.py index 7d70de6ec..aa688da50 100644 --- a/src/srtctl/frontends/sglang_direct.py +++ b/src/srtctl/frontends/sglang_direct.py @@ -13,9 +13,10 @@ import logging import threading -from typing import TYPE_CHECKING, Any +from typing import TYPE_CHECKING, Any, ClassVar from srtctl.core.health import WorkerHealthResult +from srtctl.frontends.base import register_frontend if TYPE_CHECKING: from srtctl.core.processes import ManagedProcess @@ -25,6 +26,7 @@ logger = logging.getLogger(__name__) +@register_frontend("sglang") class SGLangFrontend: """Direct SGLang OpenAI server frontend. @@ -32,10 +34,38 @@ class SGLangFrontend: port itself. Readiness is the worker's ``/health`` plus ``/v1/models``. """ + required_backend: ClassVar[str | None] = "sglang" + @property def type(self) -> str: return "sglang" + def validate(self, config: Any) -> None: + """One aggregate ``sglang.launch_server`` owns the public port. + + Several replicas or a prefill/decode layout need ``sglang-router`` (or + ``dynamo``); a schema 2 recipe that still says ``sglang`` for those is an + old router recipe and is rejected rather than silently run unbalanced. + """ + if config.frontend.enable_multiple_frontends: + raise ValueError( + "frontend.type: sglang binds sglang.launch_server directly; set frontend.enable_multiple_frontends: false" + ) + if config.resources.is_disaggregated: + raise ValueError( + "frontend.type: sglang supports one aggregate worker only, not a prefill/decode layout. " + "The SGLang router is frontend.type: sglang-router (renamed in 2.0; `srtctl migrate` rewrites " + "schema 1 recipes)." + ) + if config.resources.num_agg != 1: + raise ValueError( + f"frontend.type: sglang supports exactly one aggregate worker, got {config.resources.num_agg}. " + "sglang.launch_server owns the public port directly and there is no router to balance " + "replicas. Use frontend.type: sglang-router (the SGLang Model Gateway, renamed in 2.0) or dynamo." + ) + if config.dynamo.sidecar: + raise ValueError("frontend.type: sglang does not support dynamo.sidecar; use frontend.type: dynamo") + @property def health_endpoint(self) -> str: return "/health" diff --git a/src/srtctl/frontends/static_router.py b/src/srtctl/frontends/static_router.py index e6868f3e0..9e43f77c7 100644 --- a/src/srtctl/frontends/static_router.py +++ b/src/srtctl/frontends/static_router.py @@ -32,10 +32,14 @@ class RouterWorker: class StaticRouterFrontend: - """Base class for routers whose worker topology is supplied on the CLI.""" + """Base class for routers whose worker topology is supplied on the CLI. + + A subclass sets the class attributes, registers with ``@register_frontend``, + and overrides only the hooks whose behavior differs. + """ type: ClassVar[str] - backend_type: ClassVar[str] + required_backend: ClassVar[str | None] executable: ClassVar[tuple[str, ...]] pd_flag: ClassVar[str] process_name: ClassVar[str] @@ -46,6 +50,10 @@ class StaticRouterFrontend: def health_endpoint(self) -> str: return "/workers" + def validate(self, config: Any) -> None: + """Recipe-level rules beyond the backend pairing; none by default.""" + del config + def parse_health( self, response_json: dict, @@ -185,10 +193,11 @@ def start_frontends( del stop_event # Static routers return immediately after launch. from srtctl.core.processes import FRONTEND_TERMINATE_TIMEOUT_SECONDS, ManagedProcess - configured_backend = getattr(getattr(config, "backend", None), "type", self.backend_type) - if configured_backend != self.backend_type: + configured_backend = getattr(getattr(config, "backend", None), "type", self.required_backend) + if configured_backend != self.required_backend: raise ValueError( - f"frontend.type: {self.type} requires backend.type: {self.backend_type} (got {configured_backend!r})" + f"frontend.type: {self.type} requires backend.type: {self.required_backend} " + f"(got {configured_backend!r})" ) workers = self.collect_workers(backend, backend_processes, runtime.network_interface) diff --git a/src/srtctl/frontends/trtllm_serve.py b/src/srtctl/frontends/trtllm_serve.py index 8e8dbb81f..4de4a94e0 100644 --- a/src/srtctl/frontends/trtllm_serve.py +++ b/src/srtctl/frontends/trtllm_serve.py @@ -14,12 +14,13 @@ import logging import shlex import threading -from typing import TYPE_CHECKING, Any +from typing import TYPE_CHECKING, Any, ClassVar import yaml from srtctl.core.health import WorkerHealthResult, check_trtllm_serve_health, wait_for_health from srtctl.core.slurm import get_hostname_ip, start_srun_process +from srtctl.frontends.base import register_frontend if TYPE_CHECKING: from srtctl.core.processes import ManagedProcess @@ -29,6 +30,7 @@ logger = logging.getLogger(__name__) +@register_frontend("trtllm_serve") class TRTLLMServeFrontend: """Direct aggregate or disaggregated trtllm-serve frontend. @@ -37,10 +39,24 @@ class TRTLLMServeFrontend: ser.yaml` on the head node. Health is exposed at /health in both modes. """ + required_backend: ClassVar[str | None] = "trtllm" + @property def type(self) -> str: return "trtllm_serve" + def validate(self, config: Any) -> None: + """One direct aggregate worker or one disaggregated orchestrator; either way one public endpoint.""" + if config.frontend.enable_multiple_frontends: + raise ValueError( + "frontend.type: trtllm_serve uses one public endpoint; set frontend.enable_multiple_frontends: false" + ) + if not config.resources.is_disaggregated and config.resources.num_agg != 1: + raise ValueError( + "frontend.type: trtllm_serve aggregate mode requires exactly one " + "aggregate worker (set resources.agg_workers: 1)" + ) + @property def health_endpoint(self) -> str: return "/health" diff --git a/src/srtctl/frontends/vllm.py b/src/srtctl/frontends/vllm.py index cb396fb4d..5f74cc298 100644 --- a/src/srtctl/frontends/vllm.py +++ b/src/srtctl/frontends/vllm.py @@ -12,9 +12,10 @@ import logging import threading -from typing import TYPE_CHECKING, Any +from typing import TYPE_CHECKING, Any, ClassVar from srtctl.core.health import WorkerHealthResult +from srtctl.frontends.base import register_frontend if TYPE_CHECKING: from srtctl.core.processes import ManagedProcess @@ -24,6 +25,7 @@ logger = logging.getLogger(__name__) +@register_frontend("vllm") class VLLMFrontend: """Direct vLLM OpenAI server frontend. @@ -33,10 +35,29 @@ class VLLMFrontend: as Dynamo, since nothing here load-balances between endpoints. """ + required_backend: ClassVar[str | None] = "vllm" + @property def type(self) -> str: return "vllm" + def validate(self, config: Any) -> None: + """The one aggregate ``vllm serve`` owns the public port: no nginx fan-out, no P/D, one worker.""" + if config.frontend.enable_multiple_frontends: + raise ValueError( + "frontend.type: vllm binds vllm serve directly; set frontend.enable_multiple_frontends: false" + ) + if config.resources.is_disaggregated: + raise ValueError("frontend.type: vllm supports aggregate jobs only, not disaggregated layouts") + if config.resources.num_agg != 1: + raise ValueError( + f"frontend.type: vllm supports exactly one aggregate worker, got {config.resources.num_agg}. " + "vllm serve owns the public port directly and there is no router to load-balance " + "replicas, so extra workers would either idle or collide on the port. " + "Use frontend.type: dynamo to run multiple aggregate workers, or scale a single " + "worker across nodes with resources.agg_nodes." + ) + @property def health_endpoint(self) -> str: return "/health" diff --git a/src/srtctl/frontends/vllm_router.py b/src/srtctl/frontends/vllm_router.py index a0dc8ecf4..40875c48e 100644 --- a/src/srtctl/frontends/vllm_router.py +++ b/src/srtctl/frontends/vllm_router.py @@ -8,6 +8,7 @@ import shlex from typing import TYPE_CHECKING, Any, ClassVar +from srtctl.frontends.base import register_frontend from srtctl.frontends.static_router import StaticRouterFrontend if TYPE_CHECKING: @@ -46,15 +47,94 @@ def node_local_data_parallel_size(backend: Any, backend_processes: list[Process] return next(iter(routed_sizes), 1) +@register_frontend("vllm-router") class VLLMRouterFrontend(StaticRouterFrontend): """Route aggregate or P/D traffic to direct vLLM API servers.""" type: ClassVar[str] = "vllm-router" - backend_type: ClassVar[str] = "vllm" + required_backend: ClassVar[str | None] = "vllm" executable: ClassVar[tuple[str, ...]] = ("vllm-router",) pd_flag: ClassVar[str] = "--vllm-pd-disaggregation" process_name: ClassVar[str] = "vllm_router" + def validate(self, config: Any) -> None: + """Router expands each advertised URL by one node-local DP factor, so the vLLM topology must be uniform. + + Every routable worker must be an independently addressable ``vllm serve`` + (``per_node`` DP), its GPU count must equal DP*TP*PP*PCP, and every pool + must derive the same ``--intra-node-data-parallel-size``. + """ + backend = config.backend + resources = config.resources + endpoint_gpu_counts: dict[str, int] = { + "prefill": resources.gpus_per_prefill if resources.num_prefill else 0, + "decode": resources.gpus_per_decode if resources.num_decode else 0, + "agg": resources.gpus_per_agg if resources.num_agg else 0, + } + if backend.find_dp_modes() and backend.dp_launch_mode != "per_node": + raise ValueError( + "frontend.type: vllm-router with data-parallel-size requires " + "backend.dp_launch_mode: per_node; deprecated per_gpu processes are " + "Dynamo registrations, not independently routable vLLM API servers" + ) + + expansion_by_mode: dict[str, int] = {} + for mode, gpu_count in endpoint_gpu_counts.items(): + if gpu_count <= 0: + continue + if not backend._is_dp_mode(mode): + expansion_by_mode[mode] = 1 + continue + try: + configured_dp_size = backend._get_dp_size(mode) + dp_size = int(configured_dp_size) if configured_dp_size is not None else 1 + if dp_size < 1: + raise ValueError( + f"vLLM {mode} data-parallel-size must be a positive integer; got {configured_dp_size!r}" + ) + replica_size = backend._get_model_parallel_size(mode) + except (TypeError, ValueError) as exc: + raise ValueError(str(exc)) from exc + + required_gpus = dp_size * replica_size + if required_gpus != gpu_count: + raise ValueError( + f"vLLM Router {mode} parallelism requires DP*TP*PP*PCP=" + f"{dp_size}*{replica_size}={required_gpus} GPUs, " + f"but resources allocate {gpu_count} GPUs per worker" + ) + + local_gpu_count = min(gpu_count, resources.gpus_per_node) + if replica_size > local_gpu_count: + expansion_by_mode[mode] = 1 + else: + expansion_by_mode[mode] = backend._get_local_dp_size(mode, local_gpu_count) + + expansions = set(expansion_by_mode.values()) + if len(expansions) > 1: + detail = ", ".join(f"{mode}={size}" for mode, size in expansion_by_mode.items()) + raise ValueError( + "vLLM Router has one --intra-node-data-parallel-size for all worker pools, " + f"but the allocated topology derives different expansion factors: {detail}" + ) + + frontend_args = config.frontend.args or {} + configured_expansion = frontend_args.get( + "intra-node-data-parallel-size", frontend_args.get("intra_node_data_parallel_size") + ) + derived_expansion = next(iter(expansions), 1) + try: + configured_expansion_value = int(configured_expansion) if configured_expansion is not None else None + except (TypeError, ValueError) as exc: + raise ValueError( + f"frontend.args.intra-node-data-parallel-size must be an integer; got {configured_expansion!r}" + ) from exc + if configured_expansion_value is not None and configured_expansion_value != derived_expansion: + raise ValueError( + "frontend.args.intra-node-data-parallel-size conflicts with the allocated vLLM topology: " + f"configured {configured_expansion}, derived {derived_expansion}" + ) + def build_bash_preamble(self, config: Any) -> str | None: """Run the recipe setup script in the vLLM Router container.""" setup_script = getattr(config, "setup_script", None) diff --git a/tests/test_frontends.py b/tests/test_frontends.py index f3d76fb74..743757958 100644 --- a/tests/test_frontends.py +++ b/tests/test_frontends.py @@ -12,7 +12,15 @@ import yaml from srtctl.core.schema import ObservabilityConfig -from srtctl.frontends import DynamoFrontend, SGLangFrontend, SGLangRouterFrontend, VLLMFrontend, get_frontend +from srtctl.frontends import ( + DynamoFrontend, + SGLangFrontend, + SGLangRouterFrontend, + VLLMFrontend, + get_frontend, + list_frontend_types, + register_frontend, +) # ============================================================================ # get_frontend() Tests @@ -53,6 +61,82 @@ def test_get_unknown_frontend_raises(self): get_frontend("invalid") +class TestFrontendRegistry: + """frontend.type resolves through the registry and nowhere else.""" + + def test_registry_lists_every_frontend_type(self): + assert list_frontend_types() == [ + "dynamo", + "none", + "sglang", + "sglang-router", + "trtllm_serve", + "vllm", + "vllm-router", + ] + for name in list_frontend_types(): + if name == "none": + continue + frontend = get_frontend(name) + assert frontend.type == name + assert hasattr(frontend, "required_backend") + assert callable(frontend.validate) + + def test_register_frontend_makes_a_type_resolvable(self, monkeypatch): + from srtctl.frontends import base + + monkeypatch.setattr(base, "_FRONTENDS", dict(base._FRONTENDS)) + + @register_frontend("toy-router") + class ToyRouter: + required_backend = "vllm" + + @property + def type(self) -> str: + return "toy-router" + + def validate(self, config) -> None: + del config + + assert isinstance(get_frontend("toy-router"), ToyRouter) + assert "toy-router" in list_frontend_types() + + def test_schema_rejects_unknown_type_at_load(self): + from marshmallow import ValidationError + + from srtctl.backends import SGLangProtocol + from srtctl.core.schema import FrontendConfig, ResourceConfig, SrtConfig + + with pytest.raises(ValidationError, match="Unknown frontend.type 'toy-router'.*Available: dynamo, none"): + SrtConfig( + name="toy", + model={"path": "model", "container": "image", "precision": "fp8"}, + resources=ResourceConfig(gpu_type="h100", gpus_per_node=8, agg_nodes=1, agg_workers=1), + frontend=FrontendConfig(type="toy-router", enable_multiple_frontends=False), + backend=SGLangProtocol(), + ) + + @pytest.mark.parametrize( + ("frontend_type", "required"), + [("sglang", "sglang"), ("sglang-router", "sglang"), ("vllm", "vllm"), ("vllm-router", "vllm")], + ) + def test_schema_enforces_required_backend_generically(self, frontend_type, required): + from marshmallow import ValidationError + + from srtctl.backends import TRTLLMProtocol + from srtctl.core.schema import FrontendConfig, ResourceConfig, SrtConfig + + assert get_frontend(frontend_type).required_backend == required + with pytest.raises(ValidationError, match=f"frontend.type: {frontend_type} requires backend.type: {required}"): + SrtConfig( + name="pairing", + model={"path": "model", "container": "image", "precision": "fp8"}, + resources=ResourceConfig(gpu_type="h100", gpus_per_node=8, agg_nodes=1, agg_workers=1), + frontend=FrontendConfig(type=frontend_type, enable_multiple_frontends=False), + backend=TRTLLMProtocol(), + ) + + # ============================================================================ # Frontend Properties Tests # ============================================================================ diff --git a/tests/test_sglang_direct_frontend.py b/tests/test_sglang_direct_frontend.py index 266fb819f..cb4429632 100644 --- a/tests/test_sglang_direct_frontend.py +++ b/tests/test_sglang_direct_frontend.py @@ -57,7 +57,7 @@ def test_direct_rejects_replicas_disagg_nginx_and_other_engines() -> None: ) with pytest.raises(ValidationError, match="enable_multiple_frontends: false"): _load(_recipe(frontend={"type": "sglang", "enable_multiple_frontends": True})) - with pytest.raises(ValidationError, match="requires engine sglang"): + with pytest.raises(ValidationError, match="frontend.type: sglang requires backend.type: sglang"): _load(_recipe(engine="vllm")) From 9a075fb179f81a5b7d86b1d8c8691afcf4aa4288 Mon Sep 17 00:00:00 2001 From: Ishan Dhanani Date: Mon, 21 Sep 2026 17:06:05 -0700 Subject: [PATCH 2/7] refactor(frontends): worker shape on the protocol, backends stop comparing frontend names Every backend decided the worker shape by comparing the frontend name: Dynamo registration versus a direct server, whether the server binds the public port or its allocated http_port, and whether a router expands per-node hybrid-LB DP pools. Those are frontend properties, so they move onto the protocol as worker_launch, worker_api_port(mode), and expands_node_local_dp. build_worker_command and endpoints_to_processes still take frontend_type and resolve it through get_frontend; the eleven name comparisons in the vLLM, SGLang, and TRT-LLM backends and the one in the nsys prefix builder read those members instead. Two messages now name the frontend generically. Tests: a per-frontend contract test for the three members; the nsys prefix tests use the registered router name instead of a legacy spelling. Dry-run output for all 26 example recipes is unchanged. Signed-off-by: Ishan Dhanani --- CLAUDE.md | 6 ++-- src/srtctl/backends/sglang.py | 15 +++++---- src/srtctl/backends/trtllm.py | 10 ++++-- src/srtctl/backends/vllm.py | 48 ++++++++++++++++----------- src/srtctl/core/schema.py | 10 ++++-- src/srtctl/frontends/base.py | 24 +++++++++++++- src/srtctl/frontends/dynamo.py | 9 ++++- src/srtctl/frontends/sglang_direct.py | 9 ++++- src/srtctl/frontends/static_router.py | 10 +++++- src/srtctl/frontends/trtllm_serve.py | 8 ++++- src/srtctl/frontends/vllm.py | 9 ++++- src/srtctl/frontends/vllm_router.py | 2 ++ tests/test_frontends.py | 26 +++++++++++++++ tests/test_profiling.py | 8 ++--- 14 files changed, 151 insertions(+), 43 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 38e2ed033..accb94a93 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -57,7 +57,7 @@ Read these before adding a feature. Each rule names the existing pattern to reus - **One resolver per overridable setting.** A setting the recipe can set at engine level and override per role (`roles..args.connector`, DP size) has one accessor on the backend in the `get_config_for_mode` style, and every consumer uses it: command builder, process env, frontend, and schema validator. Two readers of the raw fields disagree the moment a role override appears. - **Frontends own readiness; backends own worker commands and ports.** `core/health.py` and the stage mixins contain no `frontend_type == "..."` checks and no `getattr(frontend, "hook", fallback)` probing. The frontend implements the protocol hook; if a hook is missing, add it to `FrontendProtocol`. A frontend asks a backend a question through a method (`backend.is_grpc_mode(mode)`), never by reading its fields by name. - **Every listener a process opens comes from the allocator.** Two processes can share a node in this repo (`nodes: colocate`, DP endpoints), so any port a worker binds (HTTP, bootstrap, side channel, handshake, notify, metrics) is allocated by `NodePortAllocator` and carried on `Process`. An upstream default port left in a generated config is a collision on the first colocated recipe. See Ports below. -- **Modes are not types.** A new `frontend.type`, `services[].type`, or `engine.type` is for a different process with its own launch, health API, and registration model. A different CLI shape, transport, or discovery mode of the same binary is an override inside the existing class: `trtllm_serve` handles aggregate and disaggregated in one type, `sglang-router` picks http or grpc per mode. A new frontend type is one registered module, but the name is still read by health expectations, telemetry targets, and the backend command builder; count those sites before choosing. +- **Modes are not types.** A new `frontend.type`, `services[].type`, or `engine.type` is for a different process with its own launch, health API, and registration model. A different CLI shape, transport, or discovery mode of the same binary is an override inside the existing class: `trtllm_serve` handles aggregate and disaggregated in one type, `sglang-router` picks http or grpc per mode. A new frontend type is one registered module, but the name is still read by health expectations and telemetry targets; count those sites before choosing. - **Check upstream before working around it.** When a change encodes an upstream behavior (what a health endpoint returns, which keys a connector reads, what a flag does), read the upstream source at the version the container ships and cite the commit in the PR. Do not add a probe, shim, or port-scan workaround for something upstream already handles. - **Reuse the machinery before adding a mechanism.** Services plus `placement` before a bespoke launcher, `roles..restart` before a wrapper loop, `host_setup` before a setup script that needs the host. The smallest diff that rides existing machinery beats a self-contained new module. - **A user-visible feature ships complete.** A `tests/` case (dry-run for visible config, mock orchestrator for behavior), a `docs/` page or section, an example recipe under `examples/`, and regenerated `docs/schema-reference.md`. In a stacked PR, a test lives in the layer that introduces the behavior it asserts. @@ -111,7 +111,9 @@ The frontend is the process that owns the public OpenAI port (`FRONTEND_PUBLIC_P `StaticRouterFrontend` (`frontends/static_router.py`) is the base for routers that take worker URLs on the command line. Subclasses set `executable`, `pd_flag`, `process_name` and override only what differs: `worker_scheme` (http or grpc per mode), `worker_bootstrap_port` (the P/D port advertised next to a prefill URL), `resolve_worker_host`, `get_managed_frontend_args` (arguments derived from the allocated topology; a conflicting user value raises instead of being overwritten), `build_bash_preamble`, `build_router_command`, `start_process` (test seam). `collect_workers` treats a positive `Process.http_port` as the definition of a routable worker. -Readiness runs in `BenchmarkStageMixin._wait_for_service_ready`: `wait_for_model` polls `health_endpoint` for `health_check.max_attempts * interval_seconds` and hands the response to `parse_health` with counts from `_get_health_expectations`, then `get_backend_health_urls` are polled with `wait_for_http_endpoints`. Everything the frontend knows about readiness belongs in those hooks. The backend chooses the worker shape from `frontend_type` inside `build_worker_command` (Dynamo registration versus a direct server), so a new router mode usually touches the backend's command builder and one frontend override, nothing else. +Readiness runs in `BenchmarkStageMixin._wait_for_service_ready`: `wait_for_model` polls `health_endpoint` for `health_check.max_attempts * interval_seconds` and hands the response to `parse_health` with counts from `_get_health_expectations`, then `get_backend_health_urls` are polled with `wait_for_http_endpoints`. Everything the frontend knows about readiness belongs in those hooks. + +The frontend also owns the worker shape, and backends read it instead of comparing names: `worker_launch` (`dynamo` workers register with the Dynamo runtime, `direct` workers are the engine's own server), `worker_api_port(mode)` (`public` when the worker is itself the endpoint and binds `runtime.frontend_port`, `allocated` when a router fronts it on `Process.http_port`), and `expands_node_local_dp` (vLLM Router expands per-node hybrid-LB pools, so the vLLM backend launches one API per node and refuses `per_gpu`). `build_worker_command` still takes `frontend_type` and resolves it through `get_frontend`; a new router mode is therefore one frontend override plus, at most, a new attribute on the protocol. ### Ports diff --git a/src/srtctl/backends/sglang.py b/src/srtctl/backends/sglang.py index 81289ee46..fa803f387 100644 --- a/src/srtctl/backends/sglang.py +++ b/src/srtctl/backends/sglang.py @@ -311,12 +311,15 @@ def build_worker_command( dump_config_path: Path to dump config JSON """ from srtctl.core.slurm import get_hostname_ip + from srtctl.frontends import get_frontend mode = process.endpoint_mode + # The frontend owns the worker shape; nothing below compares frontend names. + frontend = get_frontend(frontend_type) sidecar_config = get_dynamo_sidecar_config(runtime) if sidecar_config is not None: - if frontend_type != "dynamo": + if frontend.worker_launch != "dynamo": raise ValueError("SGLang sidecar mode requires frontend.type: dynamo") return self._build_sidecar_command( process=process, @@ -348,8 +351,8 @@ def build_worker_command( leader_ip = get_hostname_ip(endpoint_nodes[0]) dist_init_port = SGLANG_DIST_INIT_PORT_BASE - # Choose Python module based on frontend type - use_sglang = frontend_type in ("sglang", "sglang-router") + # Direct frontends run the native server; Dynamo frontends run the registering worker. + use_sglang = frontend.worker_launch == "direct" python_module = "sglang.launch_server" if use_sglang else "dynamo.sglang" # Get served model name from config @@ -378,9 +381,9 @@ def build_worker_command( ) # Always pass --port when using sglang.launch_server or dynamo.sglang. - # Direct mode (frontend.type: sglang): the single aggregate worker is the - # public endpoint, so it binds the frontend port instead of its own. - api_port = runtime.frontend_port if frontend_type == "sglang" and mode == "agg" else process.http_port + # A worker that is itself the public endpoint (frontend.type: sglang) + # binds the frontend port instead of its own. + api_port = runtime.frontend_port if frontend.worker_api_port(mode) == "public" else process.http_port cmd.extend(["--port", str(api_port)]) cmd.extend(["--nccl-port", str(nccl_port)]) diff --git a/src/srtctl/backends/trtllm.py b/src/srtctl/backends/trtllm.py index 250015819..75bd6d5f6 100644 --- a/src/srtctl/backends/trtllm.py +++ b/src/srtctl/backends/trtllm.py @@ -296,12 +296,16 @@ def build_worker_command( ) -> list[str]: """Build the command to start a TRTLLM worker process.""" + from srtctl.frontends import get_frontend + mode = process.endpoint_mode config = self.get_config_for_mode(mode) + # The frontend owns the worker shape; nothing below compares frontend names. + frontend = get_frontend(frontend_type) sidecar_config = get_dynamo_sidecar_config(runtime) if sidecar_config is not None: - if frontend_type != "dynamo": + if frontend.worker_launch != "dynamo": raise ValueError("TensorRT-LLM sidecar mode requires frontend.type: dynamo") if mode != "agg": raise ValueError("TensorRT-LLM sidecar mode supports aggregated workers only") @@ -342,8 +346,8 @@ def build_worker_command( # worker is also the public frontend, so it binds runtime.frontend_port. # There is no Dynamo request plane and no --disaggregation-mode: a disagg # worker is prefill or decode purely by which list it appears in in ser.yaml. - if frontend_type == "trtllm_serve": - http_port = runtime.frontend_port if mode == "agg" else process.http_port + if frontend.worker_launch == "direct": + http_port = runtime.frontend_port if frontend.worker_api_port(mode) == "public" else process.http_port cmd = base_prefix + [ "trtllm-serve", model_arg, diff --git a/src/srtctl/backends/vllm.py b/src/srtctl/backends/vllm.py index 3be0da2a6..406e04c49 100644 --- a/src/srtctl/backends/vllm.py +++ b/src/srtctl/backends/vllm.py @@ -907,8 +907,11 @@ def endpoints_to_processes( For standard TP mode, creates one process per node. """ from srtctl.core.topology import NodePortAllocator, Process, endpoints_to_processes + from srtctl.frontends import get_frontend - if frontend_type == "vllm": + if get_frontend(frontend_type).worker_api_port("agg") == "public": + # The worker is the public endpoint: one `vllm serve` per node owns + # its local DP ranks, so the standard topology applies. return endpoints_to_processes(endpoints, base_sys_port=base_sys_port, port_allocator=port_allocator) # Check if any endpoint uses DP mode @@ -1123,9 +1126,16 @@ def build_worker_command( profiling: Profiling config; drives --profiler-config for iteration-based nsys """ from srtctl.core.slurm import get_hostname_ip + from srtctl.frontends import get_frontend mode = process.endpoint_mode config = self.get_config_for_mode(mode) + # The frontend owns the worker shape: Dynamo registration versus a direct + # server, which port that server binds, and whether a router expands + # node-local DP pools. Nothing below compares frontend names. + frontend = get_frontend(frontend_type) + direct_workers = frontend.worker_launch == "direct" + binds_public_port = frontend.worker_api_port(mode) == "public" # Determine if multi-node endpoint_nodes = list(dict.fromkeys(p.node for p in endpoint_processes)) @@ -1133,7 +1143,7 @@ def build_worker_command( # Native vLLM rendezvous must use the configured interface, including # native engines behind a Dynamo sidecar. - if frontend_type in {"vllm", "vllm-router"} or get_dynamo_sidecar_config(runtime) is not None: + if direct_workers or get_dynamo_sidecar_config(runtime) is not None: leader_ip = get_hostname_ip(endpoint_nodes[0], runtime.network_interface) else: leader_ip = get_hostname_ip(endpoint_nodes[0]) @@ -1162,7 +1172,7 @@ def build_worker_command( sidecar_config = get_dynamo_sidecar_config(runtime) if sidecar_config is not None: - if frontend_type != "dynamo": + if frontend.worker_launch != "dynamo": raise ValueError("vLLM sidecar mode requires frontend.type: dynamo") process_ip = get_hostname_ip(process.node, getattr(runtime, "network_interface", None)) return self._build_sidecar_command( @@ -1177,23 +1187,23 @@ def build_worker_command( sidecar_config=sidecar_config, ) - if frontend_type in {"vllm", "vllm-router"}: - if frontend_type == "vllm" and mode != "agg": - raise ValueError("frontend.type: vllm supports aggregate vLLM jobs only") + if direct_workers: + if binds_public_port and mode != "agg": + raise ValueError(f"frontend.type: {frontend.type} supports aggregate vLLM jobs only") overridden = pop_vllm_orchestration_flags(config) config.setdefault("served-model-name", served_model_name) - if frontend_type == "vllm": - config.pop("connector", None) - else: - mode_connector = config.pop("connector", None) - connector = mode_connector if mode_connector is not None else self.connector - if mode in {"prefill", "decode"} and connector and connector not in ("null", "none", None): - config.setdefault("kv-transfer-config", _connector_to_kv_transfer_config(connector)) + # A prefill/decode worker gets its KV connector; an aggregate worker has none. + mode_connector = config.pop("connector", None) + connector = mode_connector if mode_connector is not None else self.connector + if mode in {"prefill", "decode"} and connector and connector not in ("null", "none", None): + config.setdefault("kv-transfer-config", _connector_to_kv_transfer_config(connector)) node_rank = endpoint_nodes.index(process.node) - serve_binary = self.vllm_serve_binary if frontend_type == "vllm" else "vllm" + # The worker that is itself the public endpoint may run the alternate + # OpenAI frontend binary (vllm-rs); routed workers run vllm. + serve_binary = self.vllm_serve_binary if binds_public_port else "vllm" cmd.extend([serve_binary, "serve", model_arg]) # Collected as the command is built so the override report below can # name the value srtslurm actually passed for each flag it took over. @@ -1203,20 +1213,20 @@ def build_worker_command( local_gpu_count = len(process.gpu_indices) spans_nodes = replica_size > local_gpu_count is_router_local_dp = ( - frontend_type == "vllm-router" + frontend.expands_node_local_dp and is_multi_node and is_dp_mode and self.dp_launch_mode == "per_node" and not spans_nodes ) - if frontend_type == "vllm-router" and is_dp_mode and self.dp_launch_mode != "per_node": + if frontend.expands_node_local_dp and is_dp_mode and self.dp_launch_mode != "per_node": raise ValueError( - "frontend.type: vllm-router with data-parallel-size requires backend.dp_launch_mode: per_node" + f"frontend.type: {frontend.type} with data-parallel-size requires backend.dp_launch_mode: per_node" ) if node_rank == 0 or is_router_local_dp: - api_port = runtime.frontend_port if frontend_type == "vllm" else process.http_port + api_port = runtime.frontend_port if binds_public_port else process.http_port cmd.extend(["--host", "0.0.0.0", "--port", str(api_port)]) srtslurm_owned["host"] = "0.0.0.0" srtslurm_owned["port"] = str(api_port) @@ -1261,7 +1271,7 @@ def build_worker_command( srtslurm_owned["master-addr"] = leader_ip srtslurm_owned["nnodes"] = str(len(endpoint_nodes)) srtslurm_owned["node-rank"] = str(node_rank) - if frontend_type == "vllm-router" and is_dp_mode: + if frontend.expands_node_local_dp and is_dp_mode: for key in list(config): if normalize_vllm_config_key(key) in { "data-parallel-address", diff --git a/src/srtctl/core/schema.py b/src/srtctl/core/schema.py index 001e219dd..02446ba7f 100755 --- a/src/srtctl/core/schema.py +++ b/src/srtctl/core/schema.py @@ -1151,8 +1151,9 @@ def get_nsys_prefix( Args: output_file: Path for nsys output file (without extension) - frontend_type: Frontend type (e.g., "dynamo", "sglang"). When set to "dynamo" - with a non-trtllm backend, adds --trace-fork-before-exec=true. + frontend_type: Frontend type (e.g., "dynamo", "sglang"). For a frontend whose + workers are Dynamo processes (``worker_launch == "dynamo"``) with a + non-trtllm backend, adds --trace-fork-before-exec=true. backend_type: Backend type (e.g., "trtllm", "sglang"). When set to "trtllm", uses TRTLLM-specific nsys flags (ucx traces, --kill none, --wait all). @@ -1167,7 +1168,10 @@ def get_nsys_prefix( trace_fork_before_exec = self.trace_fork_before_exec if trace_fork_before_exec is None: - trace_fork_before_exec = frontend_type == "dynamo" + # Dynamo workers fork the engine after exec; direct servers do not. + from srtctl.frontends import get_frontend + + trace_fork_before_exec = frontend_type is not None and get_frontend(frontend_type).worker_launch == "dynamo" # Time-based capture for non-TRTLLM backends (vllm, sglang). if self.is_nsys_time: diff --git a/src/srtctl/frontends/base.py b/src/srtctl/frontends/base.py index 5ee4befa0..2796fa31b 100644 --- a/src/srtctl/frontends/base.py +++ b/src/srtctl/frontends/base.py @@ -11,7 +11,7 @@ """ import threading -from typing import TYPE_CHECKING, Any, ClassVar, Protocol +from typing import TYPE_CHECKING, Any, ClassVar, Literal, Protocol if TYPE_CHECKING: from srtctl.core.health import WorkerHealthResult @@ -45,11 +45,33 @@ class FrontendProtocol(Protocol): #: ``SrtConfig._validate_frontend`` enforces it at config load. required_backend: ClassVar[str | None] + #: How this frontend's workers are launched. ``dynamo`` workers are + #: ``dynamo.`` processes that register with the Dynamo runtime; + #: ``direct`` workers are the engine's own OpenAI server (``vllm serve``, + #: ``sglang.launch_server``, ``trtllm-serve``). Backends read this instead + #: of comparing frontend names. + worker_launch: ClassVar[Literal["dynamo", "direct"]] + + #: The router expands each advertised URL into its node-local hybrid-LB DP + #: ranks (vLLM Router). The vLLM backend launches one hybrid-LB API per node + #: for such a frontend and refuses the deprecated per_gpu layout. + expands_node_local_dp: ClassVar[bool] + @property def type(self) -> str: """Frontend type identifier (e.g., 'dynamo', 'sglang').""" ... + def worker_api_port(self, mode: str) -> Literal["public", "allocated"]: + """Which port a direct worker of ``mode`` binds. + + ``public``: the worker is the endpoint itself and binds + ``runtime.frontend_port``. ``allocated``: a router fronts it and it binds + its own ``Process.http_port``. Dynamo workers serve no HTTP API and never + consult this. + """ + ... + def validate(self, config: Any) -> None: """Recipe-level rules for this frontend. diff --git a/src/srtctl/frontends/dynamo.py b/src/srtctl/frontends/dynamo.py index 6271573f8..b381b617e 100644 --- a/src/srtctl/frontends/dynamo.py +++ b/src/srtctl/frontends/dynamo.py @@ -10,7 +10,7 @@ import logging import shlex import threading -from typing import TYPE_CHECKING, Any, ClassVar +from typing import TYPE_CHECKING, Any, ClassVar, Literal import yaml @@ -43,6 +43,8 @@ class DynamoFrontend: # Dynamo fronts every engine; the dynamo.* rules (sidecar, failover, # worker_selection) are dynamo-config validations and stay in the schema. required_backend: ClassVar[str | None] = None + worker_launch: ClassVar[Literal["dynamo", "direct"]] = "dynamo" + expands_node_local_dp: ClassVar[bool] = False @property def type(self) -> str: @@ -51,6 +53,11 @@ def type(self) -> str: def validate(self, config: Any) -> None: del config + def worker_api_port(self, mode: str) -> Literal["public", "allocated"]: + """Dynamo workers register over the request plane; they bind no OpenAI port.""" + del mode + return "allocated" + @property def health_endpoint(self) -> str: return "/health" diff --git a/src/srtctl/frontends/sglang_direct.py b/src/srtctl/frontends/sglang_direct.py index aa688da50..507e34385 100644 --- a/src/srtctl/frontends/sglang_direct.py +++ b/src/srtctl/frontends/sglang_direct.py @@ -13,7 +13,7 @@ import logging import threading -from typing import TYPE_CHECKING, Any, ClassVar +from typing import TYPE_CHECKING, Any, ClassVar, Literal from srtctl.core.health import WorkerHealthResult from srtctl.frontends.base import register_frontend @@ -35,11 +35,18 @@ class SGLangFrontend: """ required_backend: ClassVar[str | None] = "sglang" + worker_launch: ClassVar[Literal["dynamo", "direct"]] = "direct" + expands_node_local_dp: ClassVar[bool] = False @property def type(self) -> str: return "sglang" + def worker_api_port(self, mode: str) -> Literal["public", "allocated"]: + """The one ``sglang.launch_server`` is the endpoint, so it binds the public port.""" + del mode + return "public" + def validate(self, config: Any) -> None: """One aggregate ``sglang.launch_server`` owns the public port. diff --git a/src/srtctl/frontends/static_router.py b/src/srtctl/frontends/static_router.py index 9e43f77c7..0755f4a36 100644 --- a/src/srtctl/frontends/static_router.py +++ b/src/srtctl/frontends/static_router.py @@ -9,7 +9,7 @@ import shlex import threading from dataclasses import dataclass -from typing import TYPE_CHECKING, Any, ClassVar +from typing import TYPE_CHECKING, Any, ClassVar, Literal from srtctl.core.health import WorkerHealthResult, check_static_router_health from srtctl.core.slurm import get_hostname_ip, start_srun_process @@ -45,6 +45,9 @@ class StaticRouterFrontend: process_name: ClassVar[str] log_label: ClassVar[str | None] = None allow_empty_workers: ClassVar[bool] = False + # Workers are the engines' own servers, each on its allocated HTTP port. + worker_launch: ClassVar[Literal["dynamo", "direct"]] = "direct" + expands_node_local_dp: ClassVar[bool] = False @property def health_endpoint(self) -> str: @@ -54,6 +57,11 @@ def validate(self, config: Any) -> None: """Recipe-level rules beyond the backend pairing; none by default.""" del config + def worker_api_port(self, mode: str) -> Literal["public", "allocated"]: + """A routed worker binds its own allocated port; the router owns the public one.""" + del mode + return "allocated" + def parse_health( self, response_json: dict, diff --git a/src/srtctl/frontends/trtllm_serve.py b/src/srtctl/frontends/trtllm_serve.py index 4de4a94e0..19fdeacbe 100644 --- a/src/srtctl/frontends/trtllm_serve.py +++ b/src/srtctl/frontends/trtllm_serve.py @@ -14,7 +14,7 @@ import logging import shlex import threading -from typing import TYPE_CHECKING, Any, ClassVar +from typing import TYPE_CHECKING, Any, ClassVar, Literal import yaml @@ -40,11 +40,17 @@ class TRTLLMServeFrontend: """ required_backend: ClassVar[str | None] = "trtllm" + worker_launch: ClassVar[Literal["dynamo", "direct"]] = "direct" + expands_node_local_dp: ClassVar[bool] = False @property def type(self) -> str: return "trtllm_serve" + def worker_api_port(self, mode: str) -> Literal["public", "allocated"]: + """The aggregate worker is the endpoint; P/D workers sit behind the disaggregated orchestrator.""" + return "public" if mode == "agg" else "allocated" + def validate(self, config: Any) -> None: """One direct aggregate worker or one disaggregated orchestrator; either way one public endpoint.""" if config.frontend.enable_multiple_frontends: diff --git a/src/srtctl/frontends/vllm.py b/src/srtctl/frontends/vllm.py index 5f74cc298..3ae53f2cd 100644 --- a/src/srtctl/frontends/vllm.py +++ b/src/srtctl/frontends/vllm.py @@ -12,7 +12,7 @@ import logging import threading -from typing import TYPE_CHECKING, Any, ClassVar +from typing import TYPE_CHECKING, Any, ClassVar, Literal from srtctl.core.health import WorkerHealthResult from srtctl.frontends.base import register_frontend @@ -36,11 +36,18 @@ class VLLMFrontend: """ required_backend: ClassVar[str | None] = "vllm" + worker_launch: ClassVar[Literal["dynamo", "direct"]] = "direct" + expands_node_local_dp: ClassVar[bool] = False @property def type(self) -> str: return "vllm" + def worker_api_port(self, mode: str) -> Literal["public", "allocated"]: + """The one ``vllm serve`` is the endpoint, so it binds the public port in every mode it runs.""" + del mode + return "public" + def validate(self, config: Any) -> None: """The one aggregate ``vllm serve`` owns the public port: no nginx fan-out, no P/D, one worker.""" if config.frontend.enable_multiple_frontends: diff --git a/src/srtctl/frontends/vllm_router.py b/src/srtctl/frontends/vllm_router.py index 40875c48e..c8e529962 100644 --- a/src/srtctl/frontends/vllm_router.py +++ b/src/srtctl/frontends/vllm_router.py @@ -53,6 +53,8 @@ class VLLMRouterFrontend(StaticRouterFrontend): type: ClassVar[str] = "vllm-router" required_backend: ClassVar[str | None] = "vllm" + # Router expands each node-local hybrid-LB pool into its DP ranks itself. + expands_node_local_dp: ClassVar[bool] = True executable: ClassVar[tuple[str, ...]] = ("vllm-router",) pd_flag: ClassVar[str] = "--vllm-pd-disaggregation" process_name: ClassVar[str] = "vllm_router" diff --git a/tests/test_frontends.py b/tests/test_frontends.py index 743757958..bd2f4c587 100644 --- a/tests/test_frontends.py +++ b/tests/test_frontends.py @@ -82,6 +82,26 @@ def test_registry_lists_every_frontend_type(self): assert hasattr(frontend, "required_backend") assert callable(frontend.validate) + @pytest.mark.parametrize( + ("frontend_type", "launch", "agg_port", "pd_port", "expands"), + [ + ("dynamo", "dynamo", "allocated", "allocated", False), + ("sglang", "direct", "public", "public", False), + ("sglang-router", "direct", "allocated", "allocated", False), + ("trtllm_serve", "direct", "public", "allocated", False), + ("vllm", "direct", "public", "public", False), + ("vllm-router", "direct", "allocated", "allocated", True), + ], + ) + def test_worker_shape_contract(self, frontend_type, launch, agg_port, pd_port, expands): + """Backends read these instead of comparing frontend names.""" + frontend = get_frontend(frontend_type) + assert frontend.worker_launch == launch + assert frontend.worker_api_port("agg") == agg_port + assert frontend.worker_api_port("prefill") == pd_port + assert frontend.worker_api_port("decode") == pd_port + assert frontend.expands_node_local_dp is expands + def test_register_frontend_makes_a_type_resolvable(self, monkeypatch): from srtctl.frontends import base @@ -90,6 +110,8 @@ def test_register_frontend_makes_a_type_resolvable(self, monkeypatch): @register_frontend("toy-router") class ToyRouter: required_backend = "vllm" + worker_launch = "direct" + expands_node_local_dp = False @property def type(self) -> str: @@ -98,6 +120,10 @@ def type(self) -> str: def validate(self, config) -> None: del config + def worker_api_port(self, mode: str) -> str: + del mode + return "allocated" + assert isinstance(get_frontend("toy-router"), ToyRouter) assert "toy-router" in list_frontend_types() diff --git a/tests/test_profiling.py b/tests/test_profiling.py index c22c04258..579a47dd3 100644 --- a/tests/test_profiling.py +++ b/tests/test_profiling.py @@ -77,10 +77,10 @@ def test_nsys_profiling(self): assert "profile" in prefix assert "/output/test" in prefix - # Dynamo frontend requires trace-fork-before-exec, sglangrouter does not. + # Dynamo frontend requires trace-fork-before-exec, sglang-router does not. prefix_dynamo = profiling.get_nsys_prefix("/output/test", frontend_type="dynamo") assert "--trace-fork-before-exec=true" in prefix_dynamo - prefix_router = profiling.get_nsys_prefix("/output/test", frontend_type="sglangrouter") + prefix_router = profiling.get_nsys_prefix("/output/test", frontend_type="sglang-router") assert "--trace-fork-before-exec=true" not in prefix_router def test_nsys_profiling_with_extra_args(self): @@ -194,8 +194,8 @@ def test_nsys_time_vllm_dynamo_path(self, monkeypatch): # Output file is the last token (-o ). assert prefix[-1] == "/out/w0" - # sglangrouter / non-dynamo frontend omits the fork flag. - prefix_router = profiling.get_nsys_prefix("/out/w0", frontend_type="sglangrouter", backend_type="vllm") + # sglang-router / non-dynamo frontend omits the fork flag. + prefix_router = profiling.get_nsys_prefix("/out/w0", frontend_type="sglang-router", backend_type="vllm") assert "--trace-fork-before-exec=true" not in prefix_router def test_nsys_binary_override(self, monkeypatch): From 6cbe13fa29dc8aad1d74afee198bcf9ff71abeb3 Mon Sep 17 00:00:00 2001 From: Ishan Dhanani Date: Mon, 21 Sep 2026 17:15:51 -0700 Subject: [PATCH 3/7] refactor(frontends): worker endpoint, metrics, and profiling ports on the protocol Four consumers decided which rank serves what by comparing the frontend name, each with slightly different rules: tachometer scrape targets (core/telemetry.py, 9 sites), the benchmark's logical worker endpoints, the AIPerf server metrics URLs, and the profiling control endpoints in benchmark_stage.py, plus the sequential-start readiness port in worker_stage.py. Those become frontend members: metrics_path, worker_metrics_port, worker_endpoint_port, profiling_control_port, profiling_control_is_leader_only, direct_endpoint_nodes, and worker_ready_port, implemented per class from the rules each site encoded. No frontend-name comparison remains in telemetry.py or the benchmark stage; BenchmarkStageMixin.frontend is None for a services-only job. Behavior changes, all corrections of dead ports the old per-site rules produced: the trtllm_serve aggregate worker's logical endpoint is the public port it binds rather than its unbound http_port; the direct sglang nsys control endpoint is the leader's public port rather than http_port; built-in AIPerf metrics for sglang-router target the worker leaders' HTTP ports rather than Dynamo system ports no sglang worker serves; and direct frontends no longer return early from the metrics env, so KVBM and CPU power exporter URLs are appended for them like every other frontend. The vLLM direct telemetry test fake gains the frontend_port it relies on. Tests: a per-frontend contract test over a four-process topology for every new member, and the Dynamo sidecar case. Dry-run output for all 26 example recipes is unchanged. Signed-off-by: Ishan Dhanani --- CLAUDE.md | 6 +- src/srtctl/cli/mixins/benchmark_stage.py | 141 ++++++++++------------- src/srtctl/cli/mixins/worker_stage.py | 11 +- src/srtctl/core/telemetry.py | 60 ++++------ src/srtctl/frontends/base.py | 51 ++++++++ src/srtctl/frontends/dynamo.py | 32 +++++ src/srtctl/frontends/sglang_direct.py | 29 ++++- src/srtctl/frontends/static_router.py | 30 +++++ src/srtctl/frontends/trtllm_serve.py | 33 ++++++ src/srtctl/frontends/vllm.py | 29 ++++- src/srtctl/frontends/vllm_router.py | 10 ++ tests/test_frontends.py | 44 +++++++ tests/test_telemetry.py | 2 + 13 files changed, 350 insertions(+), 128 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index accb94a93..af3f50435 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -57,7 +57,7 @@ Read these before adding a feature. Each rule names the existing pattern to reus - **One resolver per overridable setting.** A setting the recipe can set at engine level and override per role (`roles..args.connector`, DP size) has one accessor on the backend in the `get_config_for_mode` style, and every consumer uses it: command builder, process env, frontend, and schema validator. Two readers of the raw fields disagree the moment a role override appears. - **Frontends own readiness; backends own worker commands and ports.** `core/health.py` and the stage mixins contain no `frontend_type == "..."` checks and no `getattr(frontend, "hook", fallback)` probing. The frontend implements the protocol hook; if a hook is missing, add it to `FrontendProtocol`. A frontend asks a backend a question through a method (`backend.is_grpc_mode(mode)`), never by reading its fields by name. - **Every listener a process opens comes from the allocator.** Two processes can share a node in this repo (`nodes: colocate`, DP endpoints), so any port a worker binds (HTTP, bootstrap, side channel, handshake, notify, metrics) is allocated by `NodePortAllocator` and carried on `Process`. An upstream default port left in a generated config is a collision on the first colocated recipe. See Ports below. -- **Modes are not types.** A new `frontend.type`, `services[].type`, or `engine.type` is for a different process with its own launch, health API, and registration model. A different CLI shape, transport, or discovery mode of the same binary is an override inside the existing class: `trtllm_serve` handles aggregate and disaggregated in one type, `sglang-router` picks http or grpc per mode. A new frontend type is one registered module, but the name is still read by health expectations and telemetry targets; count those sites before choosing. +- **Modes are not types.** A new `frontend.type`, `services[].type`, or `engine.type` is for a different process with its own launch, health API, and registration model. A different CLI shape, transport, or discovery mode of the same binary is an override inside the existing class: `trtllm_serve` handles aggregate and disaggregated in one type, `sglang-router` picks http or grpc per mode. A new frontend type is one registered module, but the name is still read by the health expectations and the readiness loop; count those sites before choosing. - **Check upstream before working around it.** When a change encodes an upstream behavior (what a health endpoint returns, which keys a connector reads, what a flag does), read the upstream source at the version the container ships and cite the commit in the PR. Do not add a probe, shim, or port-scan workaround for something upstream already handles. - **Reuse the machinery before adding a mechanism.** Services plus `placement` before a bespoke launcher, `roles..restart` before a wrapper loop, `host_setup` before a setup script that needs the host. The smallest diff that rides existing machinery beats a self-contained new module. - **A user-visible feature ships complete.** A `tests/` case (dry-run for visible config, mock orchestrator for behavior), a `docs/` page or section, an example recipe under `examples/`, and regenerated `docs/schema-reference.md`. In a stacked PR, a test lives in the layer that introduces the behavior it asserts. @@ -115,6 +115,8 @@ Readiness runs in `BenchmarkStageMixin._wait_for_service_ready`: `wait_for_model The frontend also owns the worker shape, and backends read it instead of comparing names: `worker_launch` (`dynamo` workers register with the Dynamo runtime, `direct` workers are the engine's own server), `worker_api_port(mode)` (`public` when the worker is itself the endpoint and binds `runtime.frontend_port`, `allocated` when a router fronts it on `Process.http_port`), and `expands_node_local_dp` (vLLM Router expands per-node hybrid-LB pools, so the vLLM backend launches one API per node and refuses `per_gpu`). `build_worker_command` still takes `frontend_type` and resolves it through `get_frontend`; a new router mode is therefore one frontend override plus, at most, a new attribute on the protocol. +Which rank serves what is the frontend's call too, one method per consumer: `worker_metrics_port(process, runtime)` and `metrics_path` feed the tachometer scrape targets and the AIPerf metrics URLs (Dynamo: every rank on its system port; native servers: the leader or each routable pool on its HTTP port), `worker_endpoint_port(process, config, runtime)` feeds the `PREFILL_IPS`-style benchmark env (one per logical worker), `profiling_control_port` and `profiling_control_is_leader_only` feed iteration-triggered nsys control, `direct_endpoint_nodes(processes)` names the nodes whose worker is itself the public endpoint, and `worker_ready_port(process)` is what sequential endpoint start polls. `core/telemetry.py` and the benchmark stage contain no frontend-name checks; `BenchmarkStageMixin.frontend` is `None` for a services-only job. + ### Ports Every fixed port and base lives in `src/srtctl/ports.py` with a comment naming its owner. Per-process ports come from `NodePortAllocator` in `src/srtctl/core/topology.py`: per-node counters for what upstream binds on a known port (`next_http_port`, `next_bootstrap_port`, `next_dp_rpc_port`) and global counters for side channels (`next_kv_events_port`, `next_nixl_port`), with `_block(size)` variants when upstream adds a rank offset to a base (`VLLM_NIXL_SIDE_CHANNEL_PORT + dp_rank`). The backend's `endpoints_to_processes` stores them on `Process` (`sys_port`, `http_port`, `bootstrap_port`, `kv_events_port`, `nixl_port`, `dp_rpc_port`), and `get_process_environment` / `build_worker_command` turn them into env vars or flags. @@ -377,7 +379,7 @@ with patch.dict(os.environ, H100Rack.slurm_env()): ### Adding a Router Mode or a New Frontend Type -Decide first whether it is a mode or a type (see Design Rules). A mode of an existing router (a discovery flag, another connector, a transport) is an override in the existing frontend class, keyed on a backend method that resolves the effective per-role setting, with `StaticRouterFrontend` left unchanged. A new type is a different process: one module under `frontends/` decorated with `@register_frontend("")`, imported from `frontends/__init__.py`, carrying `required_backend` and its recipe rules in `validate(config)` (no schema edits); then `_get_health_expectations` if it reports counts differently and the telemetry scrape targets in `core/telemetry.py` until those move onto the protocol; then a `tests/test__frontend.py` using `start_process` as the seam, a `docs/.md` page, and an `examples/` recipe. +Decide first whether it is a mode or a type (see Design Rules). A mode of an existing router (a discovery flag, another connector, a transport) is an override in the existing frontend class, keyed on a backend method that resolves the effective per-role setting, with `StaticRouterFrontend` left unchanged. A new type is a different process: one module under `frontends/` decorated with `@register_frontend("")`, imported from `frontends/__init__.py`, carrying `required_backend` and its recipe rules in `validate(config)` (no schema edits); then `_get_health_expectations` if it reports counts differently, until that moves onto the protocol; then a `tests/test__frontend.py` using `start_process` as the seam, a `docs/.md` page, and an `examples/` recipe. ### Adding a New Benchmark diff --git a/src/srtctl/cli/mixins/benchmark_stage.py b/src/srtctl/cli/mixins/benchmark_stage.py index b762fe622..cae7f96be 100644 --- a/src/srtctl/cli/mixins/benchmark_stage.py +++ b/src/srtctl/cli/mixins/benchmark_stage.py @@ -28,6 +28,7 @@ from srtctl.core.processes import terminate_and_reap from srtctl.core.slurm import get_hostname_ip, start_srun_process from srtctl.core.status import JobStage, JobStatus, StatusReporter +from srtctl.frontends import FRONTEND_NONE, get_frontend from srtctl.ports import FRONTEND_PUBLIC_PORT, SGLANG_HTTP_PORT_BASE from srtctl.runtime_scripts.nsys_window import finish as finish_nsys_windows @@ -43,6 +44,7 @@ from srtctl.core.runtime import RuntimeContext from srtctl.core.schema import SrtConfig from srtctl.core.topology import Endpoint, Process + from srtctl.frontends import FrontendProtocol logger = logging.getLogger(__name__) @@ -208,15 +210,19 @@ def _orchestrator_node(self) -> str: self.backend_processes, placement, self.runtime.nodes.head, kind="frontend.orchestrator_placement" ) + @property + def frontend(self) -> "FrontendProtocol | None": + """The frontend implementation for ``frontend.type``; ``None`` for a services-only job.""" + if self.config.frontend.type == FRONTEND_NONE: + return None + return get_frontend(self.config.frontend.type) + def _public_api_node(self) -> str: """Node hosting the public OpenAI HTTP endpoint clients should probe.""" - if self.config.frontend.type in ("vllm", "sglang") and self.config.resources.num_agg > 0: - agg_leaders = sorted( - (p for p in self.backend_processes if p.endpoint_mode == "agg" and p.is_leader), - key=lambda p: p.endpoint_index, - ) - if len(agg_leaders) == 1: - return agg_leaders[0].node + frontend = self.frontend + direct_nodes = frontend.direct_endpoint_nodes(self.backend_processes) if frontend is not None else [] + if len(direct_nodes) == 1: + return direct_nodes[0] return self._orchestrator_node() def _benchmark_node(self) -> str: @@ -245,17 +251,13 @@ def _logical_worker_endpoints(self) -> list[tuple[str, str, int]]: vLLM exposes aggregate metrics on the public frontend port, while other frontends expose them on the worker HTTP port. """ + frontend = self.frontend + if frontend is None: + return [] endpoints: list[tuple[str, str, int]] = [] for process in self.backend_processes: - if self.config.frontend.type != "vllm-router" and not process.is_leader: - continue - if self.config.frontend.type == "dynamo" and not self.config.dynamo.sidecar: - port = process.sys_port - elif self.config.frontend.type in ("vllm", "sglang"): - port = self.runtime.frontend_port - else: - port = process.http_port - if port <= 0: + port = frontend.worker_endpoint_port(process, self.config, self.runtime) + if port is None: continue host = get_hostname_ip(process.node, self.runtime.network_interface) endpoints.append((process.endpoint_mode, host, port)) @@ -273,6 +275,10 @@ def _profiling_worker_endpoints(self) -> list[tuple[str, str, int]]: if not profiling.is_nsys or profiling.is_nsys_time or self.config.backend_type == "trtllm": return self._logical_worker_endpoints() + frontend = self.frontend + if frontend is None: + return [] + leader_only_control = frontend.profiling_control_is_leader_only(self.config) endpoints: list[tuple[str, str, int]] = [] selected_modes: set[str] = set() for process in self.backend_processes: @@ -286,9 +292,6 @@ def _profiling_worker_endpoints(self) -> list[tuple[str, str, int]]: ): continue - leader_only_control = self.config.frontend.type == "vllm" or ( - self.config.frontend.type == "dynamo" and self.config.dynamo.sidecar - ) if leader_only_control and not process.is_leader: # The direct-vLLM server and a Dynamo sidecar expose one # control server per logical endpoint, on its leader only. @@ -303,13 +306,8 @@ def _profiling_worker_endpoints(self) -> list[tuple[str, str, int]]: f"worker_rank={worker_rank}" ) - if self.config.frontend.type == "dynamo": - port = process.sys_port - elif self.config.frontend.type == "vllm": - port = self.runtime.frontend_port - else: - port = process.http_port - if port <= 0: + port = frontend.profiling_control_port(process, self.config, self.runtime) + if port is None: # Native distributed servers expose one HTTP control endpoint # for multiple physical processes. Wrap every process, but send # only the routable leader endpoint to the benchmark. @@ -368,8 +366,6 @@ def _wait_for_service_ready(self, stop_event: threading.Event) -> bool: ): return False - from srtctl.frontends import get_frontend - frontend = get_frontend(self.config.frontend.type) backend_health_urls = frontend.get_backend_health_urls( self.config.backend, @@ -807,18 +803,20 @@ def _get_aiperf_server_metrics_env( ranks are not advertised as separate engines. """ urls: list[str] = [] + frontend = self.frontend + if frontend is None: + # Services-only job: no workers serve engine metrics. + return {} + is_trtllm = self.config.backend_type == "trtllm" dynamo_trtllm_metrics_disabled = ( - self.config.frontend.type == "dynamo" - and self.config.backend_type == "trtllm" + frontend.worker_launch == "dynamo" + and is_trtllm and not ( (not self.config.dynamo.sidecar and getattr(self.config.backend, "dynamo_metrics_flags", ())) or getattr(self.config.backend, "publish_events_and_metrics", False) ) ) - # trtllm-serve serves Prometheus at /prometheus/metrics on the worker - # OpenAI port (GET /metrics there is JSON iteration stats, not - # exposition text); every other frontend serves it at /metrics. - metrics_path = "/prometheus/metrics" if self.config.frontend.type == "trtllm_serve" else "/metrics" + metrics_path = frontend.metrics_path if logical_workers_only: if logical_endpoints is None: logical_endpoints = self._logical_worker_endpoints() @@ -826,51 +824,36 @@ def _get_aiperf_server_metrics_env( # control their existing logical-worker URL discovery. if self.config.dynamo.sidecar or not dynamo_trtllm_metrics_disabled: urls = [f"http://{host}:{port}{metrics_path}" for _, host, port in logical_endpoints] - else: - if self.config.frontend.type in {"vllm", "sglang", "vllm-router"}: - for process in self.backend_processes: - if ( - self.config.frontend.type in {"vllm", "sglang"} - and process.endpoint_mode == "agg" - and process.is_leader - ): - host = get_hostname_ip(process.node, self.runtime.network_interface) - urls.append(f"http://{host}:{FRONTEND_PUBLIC_PORT}/metrics") - elif self.config.frontend.type == "vllm-router" and process.http_port > 0: - host = get_hostname_ip(process.node, self.runtime.network_interface) - urls.append(f"http://{host}:{process.http_port}/metrics") - if urls: - return {"AIPERF_SERVER_METRICS_URLS": ",".join(sorted(set(urls)))} - - # trtllm-serve workers bind only their OpenAI http_port (leaders) — - # the DYN_SYSTEM_PORT sys-port endpoints are never created in this - # mode, so advertising them would point the client at dead ports. - # trtllm-serve mounts the Prometheus route only when the engine - # runs with return_perf_metrics (expand_trtllm_serve_defaults sets - # it on every trtllm_serve recipe; an explicit false opts out), so - # gate each worker on its own effective engine config -- - # publish_events_and_metrics is a dynamo.trtllm flag that never - # reaches a trtllm-serve worker. - if self.config.frontend.type == "trtllm_serve": - for process in self.backend_processes: - if process.endpoint_mode == "agg" or process.http_port <= 0: - continue - engine_config = self.config.backend.get_config_for_mode(process.endpoint_mode) - if not engine_config.get("return_perf_metrics"): - continue - host = get_hostname_ip(process.node, self.runtime.network_interface) - urls.append(f"http://{host}:{process.http_port}{metrics_path}") - # Dynamo TRT-LLM engine metrics require either the metrics-only - # flag (the default) or the legacy combined flag (also enabled by - # observability). Retain the existing sidecar gate because sidecars - # do not receive --publish-metrics. An explicit legacy False disables - # both flags. Runtime-only metrics may still exist with publication disabled, - # but must not be advertised as an engine-metrics capture. - elif not dynamo_trtllm_metrics_disabled: - for process in self.backend_processes: - if process.sys_port > 0: - host = get_hostname_ip(process.node, self.runtime.network_interface) - urls.append(f"http://{host}:{process.sys_port}/metrics") + elif frontend.worker_launch == "direct": + # Every rank the frontend says serves metrics. trtllm-serve mounts its + # Prometheus route only when the engine runs with return_perf_metrics + # (expand_trtllm_serve_defaults sets it on every trtllm_serve recipe; + # an explicit false opts out), so gate each worker on its own engine + # config -- publish_events_and_metrics is a dynamo.trtllm flag that + # never reaches a trtllm-serve worker. + for process in self.backend_processes: + port = frontend.worker_metrics_port(process, self.runtime) + if port is None: + continue + if is_trtllm and not self.config.backend.get_config_for_mode(process.endpoint_mode).get( + "return_perf_metrics" + ): + continue + host = get_hostname_ip(process.node, self.runtime.network_interface) + urls.append(f"http://{host}:{port}{metrics_path}") + # Dynamo TRT-LLM engine metrics require either the metrics-only + # flag (the default) or the legacy combined flag (also enabled by + # observability). Retain the existing sidecar gate because sidecars + # do not receive --publish-metrics. An explicit legacy False disables + # both flags. Runtime-only metrics may still exist with publication disabled, + # but must not be advertised as an engine-metrics capture. + elif not dynamo_trtllm_metrics_disabled: + for process in self.backend_processes: + port = frontend.worker_metrics_port(process, self.runtime) + if port is None: + continue + host = get_hostname_ip(process.node, self.runtime.network_interface) + urls.append(f"http://{host}:{port}{metrics_path}") # Add KVBM metrics endpoints for prefill processes with DYN_KVBM_METRICS_PORT prefill_env = getattr(self.config.backend, "prefill_environment", {}) diff --git a/src/srtctl/cli/mixins/worker_stage.py b/src/srtctl/cli/mixins/worker_stage.py index b6d29c0b0..e76b077af 100644 --- a/src/srtctl/cli/mixins/worker_stage.py +++ b/src/srtctl/cli/mixins/worker_stage.py @@ -643,12 +643,13 @@ def _wait_for_worker_ready(self, leader: "Process") -> None: and exposes GET /health → 200 {"status":"ready"} once the model is loaded and the NATS/TCP request endpoint is registered. """ - health_cfg = self.config.health_check - frontend_type = self.config.frontend.type + from srtctl.frontends import get_frontend - # dynamo.trtllm: DYN_SYSTEM_PORT is set to sys_port in start_endpoint_worker, - # which enables the per-worker axum HTTP server on that same port. - port = leader.http_port if frontend_type == "trtllm_serve" else leader.sys_port + health_cfg = self.config.health_check + # The frontend knows which port a worker reports its own health on: + # trtllm-serve's OpenAI port, or DYN_SYSTEM_PORT (set to sys_port in + # start_endpoint_worker) where the Dynamo runtime serves /health. + port = get_frontend(self.config.frontend.type).worker_ready_port(leader) logger.info( "Sequential node start: waiting for worker %s:%d to be ready", diff --git a/src/srtctl/core/telemetry.py b/src/srtctl/core/telemetry.py index 9d91aeea3..5fd64a544 100644 --- a/src/srtctl/core/telemetry.py +++ b/src/srtctl/core/telemetry.py @@ -11,7 +11,6 @@ from srtctl.core.ip_utils import url_host from srtctl.core.slurm import get_hostname_ip -from srtctl.ports import FRONTEND_PUBLIC_PORT if TYPE_CHECKING: from collections.abc import Sequence @@ -103,9 +102,12 @@ def generate_tachometer_config( its own listener (``frontend_metrics_port``, ``--prometheus-port``), not on the routing port. """ - # trtllm-serve (worker and disagg orchestrator alike) exposes Prometheus - # text at /prometheus/metrics; every other frontend/backend uses /metrics. - metrics_path = "/prometheus/metrics" if frontend_type == "trtllm_serve" else "/metrics" + from srtctl.frontends import FRONTEND_NONE, get_frontend + + # The frontend says which rank serves metrics on which port and at what path; + # a services-only job has no frontend and no worker processes. + frontend = None if frontend_type == FRONTEND_NONE else get_frontend(frontend_type) + metrics_path = frontend.metrics_path if frontend is not None else "/metrics" endpoints: list[TelemetryEndpoint] = [] # Per-GPU worker labels, attached to DCGM targets so GPU rows carry the rank that owns the GPU. gpu_metadata_by_node: dict[str, dict[str, dict[str, str]]] = {} @@ -118,31 +120,15 @@ def generate_tachometer_config( } for process in sorted(processes, key=lambda p: (p.endpoint_mode, p.endpoint_index, p.node_rank, p.node)): - # Every rank is a target (vLLM agg followers excepted below): follower - # metadata columns keep rows distinguishable, and rank coverage is - # exactly what the physical-process client list provides for vLLM DP. - if frontend_type in ("vllm", "sglang") and process.endpoint_mode == "agg" and not process.is_leader: - continue - if frontend_type == "vllm-router" and process.http_port <= 0: - continue - if frontend_type == "trtllm_serve" and (process.endpoint_mode == "agg" or process.http_port <= 0): - # trtllm-serve workers bind only the leader's OpenAI http_port; - # follower ranks serve nothing. Aggregate mode is out of scope - # (the one agg worker binds the public frontend port instead of - # process.http_port). - continue - if frontend_type == "sglang-router" and (not process.is_leader or process.http_port <= 0): - # Native sglang.launch_server: only the leader rank of a worker binds - # the HTTP server that carries /metrics. + # Every rank that serves metrics is a target: follower metadata columns + # keep rows distinguishable, and rank coverage is exactly what the + # physical-process client list provides for vLLM DP. Which ranks serve + # (Dynamo: every rank on its system port; native servers: the leader or + # each routable pool on its HTTP port) is the frontend's call. + port = frontend.worker_metrics_port(process, runtime) if frontend is not None else None + if port is None: continue node_ip = get_hostname_ip(process.node, runtime.network_interface) - if frontend_type in ("vllm", "sglang") and process.endpoint_mode == "agg": - # Direct modes: the aggregate leader binds the public port itself. - port = FRONTEND_PUBLIC_PORT - elif frontend_type in ("vllm-router", "trtllm_serve", "sglang-router"): - port = process.http_port - else: - port = process.sys_port url = f"http://{url_host(node_ip)}:{port}{metrics_path}" node_metadata = { "hostname": process.node, @@ -161,19 +147,13 @@ def generate_tachometer_config( ) ) - # A services-only job has no frontend process, so nothing listens on the frontend port. - frontend_nodes = [] if frontend_type == "none" else list(frontend_topology.frontend_nodes) - if frontend_type in ("vllm", "sglang"): - # Direct vLLM / SGLang have no separate frontend process. The public endpoint is - # the aggregate leader, which may differ from the Slurm/orchestrator - # head recorded in FrontendTopology. - agg_leader_nodes = [ - process.node - for process in sorted(processes, key=lambda p: (p.endpoint_index, p.node_rank, p.node)) - if process.endpoint_mode == "agg" and process.is_leader - ] - if agg_leader_nodes: - frontend_nodes = list(dict.fromkeys(agg_leader_nodes)) + # A services-only job has no frontend process, so nothing listens on the + # frontend port. A frontend whose worker is itself the endpoint (direct vLLM + # or SGLang) has no separate process either: the endpoint is the aggregate + # leader's node, which may differ from the orchestrator head in FrontendTopology. + frontend_nodes: list[str] = [] + if frontend is not None: + frontend_nodes = frontend.direct_endpoint_nodes(processes) or list(frontend_topology.frontend_nodes) for frontend_index, node in enumerate(frontend_nodes): node_ip = get_hostname_ip(node, runtime.network_interface) diff --git a/src/srtctl/frontends/base.py b/src/srtctl/frontends/base.py index 2796fa31b..d1b0984a7 100644 --- a/src/srtctl/frontends/base.py +++ b/src/srtctl/frontends/base.py @@ -72,6 +72,47 @@ def worker_api_port(self, mode: str) -> Literal["public", "allocated"]: """ ... + #: Path where this frontend's workers and router serve Prometheus metrics. + metrics_path: ClassVar[str] + + def worker_metrics_port(self, process: "Process", runtime: "RuntimeContext") -> int | None: + """Port on ``process.node`` serving Prometheus metrics at ``metrics_path`` for this rank. + + ``None`` when the rank serves none: a follower of a native multi-node + server, or a layout the frontend does not scrape. Every rank of a Dynamo + worker serves its own system port; this is the telemetry view. + """ + ... + + def worker_endpoint_port(self, process: "Process", config: Any, runtime: "RuntimeContext") -> int | None: + """Port a benchmark addresses this worker's HTTP endpoint on, one per logical worker. + + ``None`` for a rank that is not addressable on its own (followers behind + a leader). Feeds the ``PREFILL_IPS``-style benchmark env and custom + benchmarks' metrics URLs. + """ + ... + + def profiling_control_port(self, process: "Process", config: Any, runtime: "RuntimeContext") -> int | None: + """Port carrying this rank's profiler control routes for iteration-triggered captures, or ``None``.""" + ... + + def profiling_control_is_leader_only(self, config: Any) -> bool: + """Whether one control server per logical endpoint, on its leader, fronts every rank.""" + ... + + def direct_endpoint_nodes(self, processes: list["Process"]) -> list[str]: + """Nodes whose worker is itself the public endpoint, in topology order. + + Empty when a router process owns the public port; then the frontend + topology's nodes are the endpoint. + """ + ... + + def worker_ready_port(self, process: "Process") -> int: + """Port polled for a worker's own ``/health`` during sequential endpoint start.""" + ... + def validate(self, config: Any) -> None: """Recipe-level rules for this frontend. @@ -134,6 +175,16 @@ def get_frontend_args_list(self, args: dict[str, Any] | None) -> list[str]: ... +def agg_leader_nodes(processes: list["Process"]) -> list[str]: + """Nodes of the aggregate workers' leader ranks, in topology order, without repeats. + + For a frontend whose one aggregate worker is the public endpoint, these are + the endpoint nodes. + """ + ordered = sorted(processes, key=lambda p: (p.endpoint_index, p.node_rank, p.node)) + return list(dict.fromkeys(p.node for p in ordered if p.endpoint_mode == "agg" and p.is_leader)) + + _FRONTENDS: dict[str, type] = {} diff --git a/src/srtctl/frontends/dynamo.py b/src/srtctl/frontends/dynamo.py index b381b617e..7c8699a41 100644 --- a/src/srtctl/frontends/dynamo.py +++ b/src/srtctl/frontends/dynamo.py @@ -58,6 +58,38 @@ def worker_api_port(self, mode: str) -> Literal["public", "allocated"]: del mode return "allocated" + metrics_path: ClassVar[str] = "/metrics" + + def worker_metrics_port(self, process: "Process", runtime: "RuntimeContext") -> int | None: + """Every rank runs the Dynamo system status server (health, metrics) on its system port.""" + del runtime + return process.sys_port if process.sys_port > 0 else None + + def worker_endpoint_port(self, process: "Process", config: Any, runtime: "RuntimeContext") -> int | None: + """One endpoint per logical worker: the leader's system port, or the native engine's port behind a sidecar.""" + del runtime + if not process.is_leader: + return None + port = process.http_port if config.dynamo.sidecar else process.sys_port + return port if port > 0 else None + + def profiling_control_port(self, process: "Process", config: Any, runtime: "RuntimeContext") -> int | None: + """Iteration-triggered captures are controlled per rank on the system port.""" + del config, runtime + return process.sys_port if process.sys_port > 0 else None + + def profiling_control_is_leader_only(self, config: Any) -> bool: + """A Dynamo sidecar exposes one control server per logical endpoint, on its leader.""" + return bool(config.dynamo.sidecar) + + def direct_endpoint_nodes(self, processes: list["Process"]) -> list[str]: + del processes + return [] + + def worker_ready_port(self, process: "Process") -> int: + """DYN_SYSTEM_PORT: the per-worker axum server reports /health once registered.""" + return process.sys_port + @property def health_endpoint(self) -> str: return "/health" diff --git a/src/srtctl/frontends/sglang_direct.py b/src/srtctl/frontends/sglang_direct.py index 507e34385..89a3f96ae 100644 --- a/src/srtctl/frontends/sglang_direct.py +++ b/src/srtctl/frontends/sglang_direct.py @@ -16,7 +16,7 @@ from typing import TYPE_CHECKING, Any, ClassVar, Literal from srtctl.core.health import WorkerHealthResult -from srtctl.frontends.base import register_frontend +from srtctl.frontends.base import agg_leader_nodes, register_frontend if TYPE_CHECKING: from srtctl.core.processes import ManagedProcess @@ -47,6 +47,33 @@ def worker_api_port(self, mode: str) -> Literal["public", "allocated"]: del mode return "public" + metrics_path: ClassVar[str] = "/metrics" + + def worker_metrics_port(self, process: Process, runtime: RuntimeContext) -> int | None: + """The aggregate leader binds the public port; its followers serve nothing.""" + if process.endpoint_mode == "agg" and process.is_leader: + return runtime.frontend_port + return None + + def worker_endpoint_port(self, process: Process, config: Any, runtime: RuntimeContext) -> int | None: + del config + return runtime.frontend_port if process.is_leader else None + + def profiling_control_port(self, process: Process, config: Any, runtime: RuntimeContext) -> int | None: + """The leader's server on the public port carries the control routes; followers have none.""" + del config + return runtime.frontend_port if process.is_leader else None + + def profiling_control_is_leader_only(self, config: Any) -> bool: + del config + return False + + def direct_endpoint_nodes(self, processes: list[Process]) -> list[str]: + return agg_leader_nodes(processes) + + def worker_ready_port(self, process: Process) -> int: + return process.sys_port + def validate(self, config: Any) -> None: """One aggregate ``sglang.launch_server`` owns the public port. diff --git a/src/srtctl/frontends/static_router.py b/src/srtctl/frontends/static_router.py index 0755f4a36..e2c7510b1 100644 --- a/src/srtctl/frontends/static_router.py +++ b/src/srtctl/frontends/static_router.py @@ -62,6 +62,36 @@ def worker_api_port(self, mode: str) -> Literal["public", "allocated"]: del mode return "allocated" + metrics_path: ClassVar[str] = "/metrics" + + def worker_metrics_port(self, process: Process, runtime: RuntimeContext) -> int | None: + """A native server's leader rank binds the HTTP server that carries /metrics; followers serve nothing.""" + del runtime + if process.is_leader and process.http_port > 0: + return process.http_port + return None + + def worker_endpoint_port(self, process: Process, config: Any, runtime: RuntimeContext) -> int | None: + del config, runtime + if process.is_leader and process.http_port > 0: + return process.http_port + return None + + def profiling_control_port(self, process: Process, config: Any, runtime: RuntimeContext) -> int | None: + del config, runtime + return process.http_port if process.http_port > 0 else None + + def profiling_control_is_leader_only(self, config: Any) -> bool: + del config + return False + + def direct_endpoint_nodes(self, processes: list[Process]) -> list[str]: + del processes + return [] + + def worker_ready_port(self, process: Process) -> int: + return process.sys_port + def parse_health( self, response_json: dict, diff --git a/src/srtctl/frontends/trtllm_serve.py b/src/srtctl/frontends/trtllm_serve.py index 19fdeacbe..b3ae643b8 100644 --- a/src/srtctl/frontends/trtllm_serve.py +++ b/src/srtctl/frontends/trtllm_serve.py @@ -51,6 +51,39 @@ def worker_api_port(self, mode: str) -> Literal["public", "allocated"]: """The aggregate worker is the endpoint; P/D workers sit behind the disaggregated orchestrator.""" return "public" if mode == "agg" else "allocated" + # trtllm-serve (worker and disaggregated orchestrator alike) serves Prometheus + # text at /prometheus/metrics; GET /metrics on a worker is JSON iteration stats. + metrics_path: ClassVar[str] = "/prometheus/metrics" + + def worker_metrics_port(self, process: "Process", runtime: "RuntimeContext") -> int | None: + """P/D leaders serve Prometheus on their OpenAI port; followers bind nothing. Aggregate is out of scope.""" + del runtime + if process.endpoint_mode == "agg" or process.http_port <= 0: + return None + return process.http_port + + def worker_endpoint_port(self, process: "Process", config: Any, runtime: "RuntimeContext") -> int | None: + del config + if not process.is_leader: + return None + port = runtime.frontend_port if self.worker_api_port(process.endpoint_mode) == "public" else process.http_port + return port if port > 0 else None + + def profiling_control_port(self, process: "Process", config: Any, runtime: "RuntimeContext") -> int | None: + return self.worker_endpoint_port(process, config, runtime) + + def profiling_control_is_leader_only(self, config: Any) -> bool: + del config + return False + + def direct_endpoint_nodes(self, processes: list["Process"]) -> list[str]: + del processes + return [] + + def worker_ready_port(self, process: "Process") -> int: + """A trtllm-serve worker reports /health on its own OpenAI port.""" + return process.http_port + def validate(self, config: Any) -> None: """One direct aggregate worker or one disaggregated orchestrator; either way one public endpoint.""" if config.frontend.enable_multiple_frontends: diff --git a/src/srtctl/frontends/vllm.py b/src/srtctl/frontends/vllm.py index 3ae53f2cd..2d0b6a1fe 100644 --- a/src/srtctl/frontends/vllm.py +++ b/src/srtctl/frontends/vllm.py @@ -15,7 +15,7 @@ from typing import TYPE_CHECKING, Any, ClassVar, Literal from srtctl.core.health import WorkerHealthResult -from srtctl.frontends.base import register_frontend +from srtctl.frontends.base import agg_leader_nodes, register_frontend if TYPE_CHECKING: from srtctl.core.processes import ManagedProcess @@ -48,6 +48,33 @@ def worker_api_port(self, mode: str) -> Literal["public", "allocated"]: del mode return "public" + metrics_path: ClassVar[str] = "/metrics" + + def worker_metrics_port(self, process: Process, runtime: RuntimeContext) -> int | None: + """The aggregate leader binds the public port; its followers serve nothing.""" + if process.endpoint_mode == "agg" and process.is_leader: + return runtime.frontend_port + return None + + def worker_endpoint_port(self, process: Process, config: Any, runtime: RuntimeContext) -> int | None: + del config + return runtime.frontend_port if process.is_leader else None + + def profiling_control_port(self, process: Process, config: Any, runtime: RuntimeContext) -> int | None: + """One control server for the whole worker, on the public port.""" + del process, config + return runtime.frontend_port + + def profiling_control_is_leader_only(self, config: Any) -> bool: + del config + return True + + def direct_endpoint_nodes(self, processes: list[Process]) -> list[str]: + return agg_leader_nodes(processes) + + def worker_ready_port(self, process: Process) -> int: + return process.sys_port + def validate(self, config: Any) -> None: """The one aggregate ``vllm serve`` owns the public port: no nginx fan-out, no P/D, one worker.""" if config.frontend.enable_multiple_frontends: diff --git a/src/srtctl/frontends/vllm_router.py b/src/srtctl/frontends/vllm_router.py index c8e529962..1b26252a3 100644 --- a/src/srtctl/frontends/vllm_router.py +++ b/src/srtctl/frontends/vllm_router.py @@ -204,3 +204,13 @@ def worker_bootstrap_port(self, backend: Any, process: Process) -> int | None: """Advertise vLLM's NIXL side-channel port for P/D routing.""" del backend return process.nixl_port + + def worker_metrics_port(self, process: Process, runtime: Any) -> int | None: + """Every node-local hybrid-LB pool has its own API and /metrics; a positive http_port marks one.""" + del runtime + return process.http_port if process.http_port > 0 else None + + def worker_endpoint_port(self, process: Process, config: Any, runtime: Any) -> int | None: + """Router-facing pools are addressable whether or not they are the endpoint's leader rank.""" + del config, runtime + return process.http_port if process.http_port > 0 else None diff --git a/tests/test_frontends.py b/tests/test_frontends.py index bd2f4c587..0b0bc73d3 100644 --- a/tests/test_frontends.py +++ b/tests/test_frontends.py @@ -102,6 +102,50 @@ def test_worker_shape_contract(self, frontend_type, launch, agg_port, pd_port, e assert frontend.worker_api_port("decode") == pd_port assert frontend.expands_node_local_dp is expands + @pytest.mark.parametrize( + ("frontend_type", "metrics_path", "metrics", "endpoint", "direct_nodes", "ready"), + [ + # metrics/endpoint: ports for (agg leader, agg follower, routed decode pool, prefill leader) + ("dynamo", "/metrics", (7500, 7501, 7501, 7502), (7500, None, None, 7502), [], 7500), + ("vllm", "/metrics", (8000, None, None, None), (8000, None, None, 8000), ["n0"], 7500), + ("sglang", "/metrics", (8000, None, None, None), (8000, None, None, 8000), ["n0"], 7500), + ("sglang-router", "/metrics", (6100, None, None, 6100), (6100, None, None, 6100), [], 7500), + ("vllm-router", "/metrics", (6100, None, 6132, 6100), (6100, None, 6132, 6100), [], 7500), + ("trtllm_serve", "/prometheus/metrics", (None, None, 6132, 6100), (8000, None, None, 6100), [], 6100), + ], + ) + def test_worker_port_contract(self, frontend_type, metrics_path, metrics, endpoint, direct_nodes, ready): + """Telemetry, the benchmark env, and sequential start read these instead of comparing names.""" + from srtctl.core.topology import Process + + agg_leader = Process("n0", frozenset({0}), 7500, 6100, "agg", 0, node_rank=0) + agg_follower = Process("n1", frozenset({0}), 7501, 0, "agg", 0, node_rank=1) + routed_pool = Process("n1", frozenset({0}), 7501, 6132, "decode", 0, node_rank=1) + prefill_leader = Process("n2", frozenset({0}), 7502, 6100, "prefill", 0, node_rank=0) + processes = [agg_leader, agg_follower, routed_pool, prefill_leader] + runtime = SimpleNamespace(frontend_port=8000, network_interface=None) + config = SimpleNamespace(dynamo=SimpleNamespace(sidecar=False)) + + frontend = get_frontend(frontend_type) + assert frontend.metrics_path == metrics_path + assert tuple(frontend.worker_metrics_port(p, runtime) for p in processes) == metrics + assert tuple(frontend.worker_endpoint_port(p, config, runtime) for p in processes) == endpoint + assert frontend.direct_endpoint_nodes(processes) == direct_nodes + assert frontend.worker_ready_port(agg_leader) == ready + assert isinstance(frontend.profiling_control_is_leader_only(config), bool) + + def test_dynamo_sidecar_moves_the_endpoint_to_the_engine_port(self): + from srtctl.core.topology import Process + + leader = Process("n0", frozenset({0}), 7500, 6100, "agg", 0, node_rank=0) + runtime = SimpleNamespace(frontend_port=8000, network_interface=None) + dynamo = get_frontend("dynamo") + assert ( + dynamo.worker_endpoint_port(leader, SimpleNamespace(dynamo=SimpleNamespace(sidecar=True)), runtime) == 6100 + ) + assert dynamo.profiling_control_is_leader_only(SimpleNamespace(dynamo=SimpleNamespace(sidecar=True))) is True + assert dynamo.profiling_control_is_leader_only(SimpleNamespace(dynamo=SimpleNamespace(sidecar=False))) is False + def test_register_frontend_makes_a_type_resolvable(self, monkeypatch): from srtctl.frontends import base diff --git a/tests/test_telemetry.py b/tests/test_telemetry.py index 323709c23..fdb170150 100644 --- a/tests/test_telemetry.py +++ b/tests/test_telemetry.py @@ -1098,6 +1098,8 @@ def test_vllm_frontend_targets_only_agg_leader_metrics(self, mock_get_hostname_i runtime.job_id = "12345" runtime.run_name = "test_12345" runtime.network_interface = "eth0" + # The direct worker binds the public frontend port itself. + runtime.frontend_port = 8000 processes = [ Process( node="node-a", From 80ae05f88fe436dc2eebe41b0d6e430d156f5df8 Mon Sep 17 00:00:00 2001 From: Ishan Dhanani Date: Mon, 21 Sep 2026 17:32:37 -0700 Subject: [PATCH 4/7] refactor(frontends): DynamicFrontend base for registration-based frontends The counterpart of StaticRouterFrontend. A static router is launched with its worker URLs on the command line; a dynamic frontend is launched with no worker list, workers announce themselves over a discovery plane, and readiness is the frontend's own registration count. What every such frontend shares moves into the base: it fronts any engine (required_backend None), needs no per-worker URL gate, no worker is the public endpoint, workers bind allocated ports, /health is the registration endpoint, and frontend.args pass through with keys verbatim. DynamoFrontend subclasses it and keeps only what is Dynamo's: the dynamo.frontend launch, /health parsing, and the system-port rules. A router binary that can also take static URLs (vLLM Router's ZMQ discovery mode) remains a mode of a static router. Signed-off-by: Ishan Dhanani --- CLAUDE.md | 4 +- src/srtctl/frontends/__init__.py | 2 + src/srtctl/frontends/dynamic_frontend.py | 80 ++++++++++++++++++++++++ src/srtctl/frontends/dynamo.py | 56 +++-------------- tests/test_frontends.py | 28 +++++++++ 5 files changed, 119 insertions(+), 51 deletions(-) create mode 100644 src/srtctl/frontends/dynamic_frontend.py diff --git a/CLAUDE.md b/CLAUDE.md index af3f50435..07877d44e 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -109,7 +109,7 @@ The frontend is the process that owns the public OpenAI port (`FRONTEND_PUBLIC_P `FrontendProtocol` hooks: `required_backend` and `validate(config)` (recipe rules, run by `SrtConfig._validate_frontend` at load so dry-run catches them; raise `ValueError` with the user-facing message), `health_endpoint` and `parse_health` (readiness), `get_backend_health_urls` (second gate: every advertised worker URL must answer 200 before traffic), `start_frontends` (launch on `topology.frontend_nodes`, one `ManagedProcess` per node with a `step_name`), `get_frontend_args_list` (`frontend.args` to CLI). The schema knows no individual frontend: pairing and per-type rules come from these two members. -`StaticRouterFrontend` (`frontends/static_router.py`) is the base for routers that take worker URLs on the command line. Subclasses set `executable`, `pd_flag`, `process_name` and override only what differs: `worker_scheme` (http or grpc per mode), `worker_bootstrap_port` (the P/D port advertised next to a prefill URL), `resolve_worker_host`, `get_managed_frontend_args` (arguments derived from the allocated topology; a conflicting user value raises instead of being overwritten), `build_bash_preamble`, `build_router_command`, `start_process` (test seam). `collect_workers` treats a positive `Process.http_port` as the definition of a routable worker. +Two base classes cover the two ways a frontend learns about its workers. `DynamicFrontend` (`frontends/dynamic_frontend.py`) is for frontends whose workers register themselves over a discovery plane: it fronts any engine, needs no per-worker URL gate, and no worker is the public endpoint; Dynamo is its only implementation. A router binary that can also take static URLs (vLLM Router's ZMQ discovery mode) is a mode of a static router, not a dynamic frontend. `StaticRouterFrontend` (`frontends/static_router.py`) is the base for routers that take worker URLs on the command line. Subclasses set `executable`, `pd_flag`, `process_name` and override only what differs: `worker_scheme` (http or grpc per mode), `worker_bootstrap_port` (the P/D port advertised next to a prefill URL), `resolve_worker_host`, `get_managed_frontend_args` (arguments derived from the allocated topology; a conflicting user value raises instead of being overwritten), `build_bash_preamble`, `build_router_command`, `start_process` (test seam). `collect_workers` treats a positive `Process.http_port` as the definition of a routable worker. Readiness runs in `BenchmarkStageMixin._wait_for_service_ready`: `wait_for_model` polls `health_endpoint` for `health_check.max_attempts * interval_seconds` and hands the response to `parse_health` with counts from `_get_health_expectations`, then `get_backend_health_urls` are polled with `wait_for_http_endpoints`. Everything the frontend knows about readiness belongs in those hooks. @@ -379,7 +379,7 @@ with patch.dict(os.environ, H100Rack.slurm_env()): ### Adding a Router Mode or a New Frontend Type -Decide first whether it is a mode or a type (see Design Rules). A mode of an existing router (a discovery flag, another connector, a transport) is an override in the existing frontend class, keyed on a backend method that resolves the effective per-role setting, with `StaticRouterFrontend` left unchanged. A new type is a different process: one module under `frontends/` decorated with `@register_frontend("")`, imported from `frontends/__init__.py`, carrying `required_backend` and its recipe rules in `validate(config)` (no schema edits); then `_get_health_expectations` if it reports counts differently, until that moves onto the protocol; then a `tests/test__frontend.py` using `start_process` as the seam, a `docs/.md` page, and an `examples/` recipe. +Decide first whether it is a mode or a type (see Design Rules). A mode of an existing router (a discovery flag, another connector, a transport) is an override in the existing frontend class, keyed on a backend method that resolves the effective per-role setting, with `StaticRouterFrontend` left unchanged. A new type is a different process: one module under `frontends/` decorated with `@register_frontend("")`, imported from `frontends/__init__.py`, subclassing `StaticRouterFrontend` when the router takes worker URLs or `DynamicFrontend` when workers register themselves, carrying `required_backend` and its recipe rules in `validate(config)` (no schema edits); then `_get_health_expectations` if it reports counts differently, until that moves onto the protocol; then a `tests/test__frontend.py` using `start_process` as the seam, a `docs/.md` page, and an `examples/` recipe. ### Adding a New Benchmark diff --git a/src/srtctl/frontends/__init__.py b/src/srtctl/frontends/__init__.py index 4310d33e2..ec4920d98 100644 --- a/src/srtctl/frontends/__init__.py +++ b/src/srtctl/frontends/__init__.py @@ -24,6 +24,7 @@ list_frontend_types, register_frontend, ) +from srtctl.frontends.dynamic_frontend import DynamicFrontend from srtctl.frontends.dynamo import DynamoFrontend from srtctl.frontends.sglang import SGLangRouterFrontend from srtctl.frontends.sglang_direct import SGLangFrontend @@ -33,6 +34,7 @@ __all__ = [ "FRONTEND_NONE", + "DynamicFrontend", "DynamoFrontend", "FrontendProtocol", "SGLangFrontend", diff --git a/src/srtctl/frontends/dynamic_frontend.py b/src/srtctl/frontends/dynamic_frontend.py new file mode 100644 index 000000000..f3f9257f3 --- /dev/null +++ b/src/srtctl/frontends/dynamic_frontend.py @@ -0,0 +1,80 @@ +# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 + +"""Shared implementation for frontends whose workers register themselves. + +The counterpart of :class:`~srtctl.frontends.static_router.StaticRouterFrontend`. +A static router is launched with its worker URLs on the command line; a +dynamic frontend is launched with no worker list, workers announce +themselves over a discovery plane (Dynamo: etcd and NATS), and readiness is +the frontend's own registration count checked against the allocated +topology. Consequences shared by every such frontend live here: it fronts +any engine, it needs no per-worker URL gate before traffic, and no worker is +itself the public endpoint. + +A router binary that can also take static URLs (vLLM Router's ZMQ discovery +mode) is a mode of a static router, not a dynamic frontend. + +Dynamo is the only implementation today. A subclass sets ``type`` and +``worker_launch``, registers with ``@register_frontend``, parses its own +registration count in ``parse_health``, decides which rank serves which port, +and launches the process in ``start_frontends``. +""" + +from __future__ import annotations + +from typing import TYPE_CHECKING, Any, ClassVar, Literal + +if TYPE_CHECKING: + from srtctl.core.topology import Process + + +class DynamicFrontend: + """Base class for frontends that discover their workers through registration.""" + + type: ClassVar[str] + # Registration does not care which engine registers. + required_backend: ClassVar[str | None] = None + expands_node_local_dp: ClassVar[bool] = False + metrics_path: ClassVar[str] = "/metrics" + + @property + def health_endpoint(self) -> str: + """The frontend reports its registered workers here; ``parse_health`` counts them.""" + return "/health" + + def validate(self, config: Any) -> None: + """Recipe-level rules beyond the backend pairing; none by default.""" + del config + + def worker_api_port(self, mode: str) -> Literal["public", "allocated"]: + """A registered worker never binds the public port; the frontend owns it.""" + del mode + return "allocated" + + def get_backend_health_urls( + self, + backend: Any, + backend_processes: list[Process], + network_interface: str | None = None, + ) -> list[str]: + """Registration is the readiness gate; there is no per-worker URL to poll first.""" + del backend, backend_processes, network_interface + return [] + + def direct_endpoint_nodes(self, processes: list[Process]) -> list[str]: + """The frontend process is the endpoint, never a worker.""" + del processes + return [] + + def get_frontend_args_list(self, args: dict[str, Any] | None) -> list[str]: + """Convert ``frontend.args`` to CLI flags, keys verbatim.""" + if not args: + return [] + result: list[str] = [] + for key, value in args.items(): + if value is True: + result.append(f"--{key}") + elif value is not False and value is not None: + result.extend([f"--{key}", str(value)]) + return result diff --git a/src/srtctl/frontends/dynamo.py b/src/srtctl/frontends/dynamo.py index 7c8699a41..b424e12da 100644 --- a/src/srtctl/frontends/dynamo.py +++ b/src/srtctl/frontends/dynamo.py @@ -19,6 +19,7 @@ from srtctl.core.schema import build_otel_env from srtctl.core.slurm import CONTAINER_REMAP_ROOT_EXPORT, start_srun_process from srtctl.frontends.base import register_frontend +from srtctl.frontends.dynamic_frontend import DynamicFrontend from srtctl.services.implicit import discovery_env if TYPE_CHECKING: @@ -33,32 +34,18 @@ @register_frontend("dynamo") -class DynamoFrontend: +class DynamoFrontend(DynamicFrontend): """Dynamo frontend implementation. Uses dynamo.frontend module with NATS/etcd for worker discovery. - Health checks via /health endpoint. + Health checks via /health endpoint. The dynamo.* recipe rules (sidecar, + failover, worker_selection) are dynamo-config validations and stay in the + schema. """ - # Dynamo fronts every engine; the dynamo.* rules (sidecar, failover, - # worker_selection) are dynamo-config validations and stay in the schema. - required_backend: ClassVar[str | None] = None + type: ClassVar[str] = "dynamo" + # dynamo. workers register over the request plane; they bind no OpenAI port. worker_launch: ClassVar[Literal["dynamo", "direct"]] = "dynamo" - expands_node_local_dp: ClassVar[bool] = False - - @property - def type(self) -> str: - return "dynamo" - - def validate(self, config: Any) -> None: - del config - - def worker_api_port(self, mode: str) -> Literal["public", "allocated"]: - """Dynamo workers register over the request plane; they bind no OpenAI port.""" - del mode - return "allocated" - - metrics_path: ClassVar[str] = "/metrics" def worker_metrics_port(self, process: "Process", runtime: "RuntimeContext") -> int | None: """Every rank runs the Dynamo system status server (health, metrics) on its system port.""" @@ -82,18 +69,10 @@ def profiling_control_is_leader_only(self, config: Any) -> bool: """A Dynamo sidecar exposes one control server per logical endpoint, on its leader.""" return bool(config.dynamo.sidecar) - def direct_endpoint_nodes(self, processes: list["Process"]) -> list[str]: - del processes - return [] - def worker_ready_port(self, process: "Process") -> int: """DYN_SYSTEM_PORT: the per-worker axum server reports /health once registered.""" return process.sys_port - @property - def health_endpoint(self) -> str: - return "/health" - def parse_health( self, response_json: dict, @@ -103,27 +82,6 @@ def parse_health( """Parse dynamo /health endpoint response.""" return check_dynamo_health(response_json, expected_prefill, expected_decode) - def get_backend_health_urls( - self, - backend: Any, - backend_processes: list["Process"], - network_interface: str | None = None, - ) -> list[str]: - del backend, backend_processes, network_interface - return [] - - def get_frontend_args_list(self, args: dict[str, Any] | None) -> list[str]: - """Convert frontend args dict to CLI arguments.""" - if not args: - return [] - result = [] - for key, value in args.items(): - if value is True: - result.append(f"--{key}") - elif value is not False and value is not None: - result.extend([f"--{key}", str(value)]) - return result - def start_frontends( self, topology: Any, # FrontendTopology diff --git a/tests/test_frontends.py b/tests/test_frontends.py index 0b0bc73d3..5a01f4384 100644 --- a/tests/test_frontends.py +++ b/tests/test_frontends.py @@ -146,6 +146,34 @@ def test_dynamo_sidecar_moves_the_endpoint_to_the_engine_port(self): assert dynamo.profiling_control_is_leader_only(SimpleNamespace(dynamo=SimpleNamespace(sidecar=True))) is True assert dynamo.profiling_control_is_leader_only(SimpleNamespace(dynamo=SimpleNamespace(sidecar=False))) is False + def test_dynamic_frontend_base_carries_the_registration_defaults(self, monkeypatch): + """Dynamo is a DynamicFrontend; a future registration-based frontend inherits the same defaults.""" + from srtctl.frontends import DynamicFrontend, base + + assert isinstance(get_frontend("dynamo"), DynamicFrontend) + monkeypatch.setattr(base, "_FRONTENDS", dict(base._FRONTENDS)) + + @register_frontend("toy-discovery") + class ToyDiscovery(DynamicFrontend): + type = "toy-discovery" + worker_launch = "direct" + + toy = get_frontend("toy-discovery") + assert isinstance(toy, ToyDiscovery) + assert toy.required_backend is None + assert toy.health_endpoint == "/health" + assert toy.metrics_path == "/metrics" + assert toy.expands_node_local_dp is False + assert toy.worker_api_port("prefill") == "allocated" + assert toy.get_backend_health_urls(None, [], None) == [] + assert toy.direct_endpoint_nodes([]) == [] + assert toy.get_frontend_args_list({"router_mode": "kv", "flag": True, "off": False}) == [ + "--router_mode", + "kv", + "--flag", + ] + toy.validate(SimpleNamespace()) + def test_register_frontend_makes_a_type_resolvable(self, monkeypatch): from srtctl.frontends import base From 2e59112960bd937b573118cea4de00266ccaa739 Mon Sep 17 00:00:00 2001 From: Ishan Dhanani Date: Mon, 21 Sep 2026 17:44:49 -0700 Subject: [PATCH 5/7] refactor(frontends): readiness probe and health expectations on the protocol wait_for_model special-cased three frontends by name: trtllm_serve was ready on a bare 200, direct vllm and sglang went through /health plus /v1/models, everything else parsed a JSON worker count. And _get_health_expectations in the benchmark stage knew that Dynamo counts vLLM DP registrations and that vLLM Router counts expanded ranks. Both are frontend knowledge. probe_ready(host, port, n_prefill, n_decode) performs one readiness check and returns a WorkerHealthResult, raising requests.RequestException while the endpoint is down; wait_for_model keeps only timing, abort, and progress logging. core/health.py offers the three probe shapes to build on: probe_json_health (a JSON count through parse_health, the DynamicFrontend and StaticRouterFrontend default), probe_http_ok (a bare 200, trtllm_serve), and probe_direct_server (/health then /v1/models, direct vllm and sglang). A frontend with an unusual readiness contract, such as a router in a discovery mode, overrides probe_ready and touches nothing else. health_expectations(config, processes) returns the expected counts in the units the frontend reports plus a description; Dynamo's vLLM DP registration logic moves with it into frontends/dynamo.py, vLLM Router's expansion sum into frontends/vllm_router.py, and every other frontend counts logical workers through logical_health_expectations. _get_health_expectations now delegates and keeps its signature. Tests: probes for every frontend against mocked HTTP (bare 200, non-200, JSON registry, direct server with and without a listed model, connection errors propagating) and the wait_for_model loop (ready after a retry, timeout while refused, stop event). The non-Dynamo expectations test names a real frontend instead of none. Dry-run output for all 26 example recipes is unchanged. Signed-off-by: Ishan Dhanani --- CLAUDE.md | 6 +- src/srtctl/cli/mixins/benchmark_stage.py | 103 ++--------------- src/srtctl/core/health.py | 125 +++++++++++--------- src/srtctl/frontends/base.py | 28 +++++ src/srtctl/frontends/dynamic_frontend.py | 16 +++ src/srtctl/frontends/dynamo.py | 80 ++++++++++++- src/srtctl/frontends/sglang_direct.py | 13 ++- src/srtctl/frontends/static_router.py | 12 +- src/srtctl/frontends/trtllm_serve.py | 15 ++- src/srtctl/frontends/vllm.py | 13 ++- src/srtctl/frontends/vllm_router.py | 19 +++- tests/test_health.py | 139 +++++++++++++++++++++++ tests/test_health_expectations.py | 13 ++- 13 files changed, 412 insertions(+), 170 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 07877d44e..9f08cb179 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -57,7 +57,7 @@ Read these before adding a feature. Each rule names the existing pattern to reus - **One resolver per overridable setting.** A setting the recipe can set at engine level and override per role (`roles..args.connector`, DP size) has one accessor on the backend in the `get_config_for_mode` style, and every consumer uses it: command builder, process env, frontend, and schema validator. Two readers of the raw fields disagree the moment a role override appears. - **Frontends own readiness; backends own worker commands and ports.** `core/health.py` and the stage mixins contain no `frontend_type == "..."` checks and no `getattr(frontend, "hook", fallback)` probing. The frontend implements the protocol hook; if a hook is missing, add it to `FrontendProtocol`. A frontend asks a backend a question through a method (`backend.is_grpc_mode(mode)`), never by reading its fields by name. - **Every listener a process opens comes from the allocator.** Two processes can share a node in this repo (`nodes: colocate`, DP endpoints), so any port a worker binds (HTTP, bootstrap, side channel, handshake, notify, metrics) is allocated by `NodePortAllocator` and carried on `Process`. An upstream default port left in a generated config is a collision on the first colocated recipe. See Ports below. -- **Modes are not types.** A new `frontend.type`, `services[].type`, or `engine.type` is for a different process with its own launch, health API, and registration model. A different CLI shape, transport, or discovery mode of the same binary is an override inside the existing class: `trtllm_serve` handles aggregate and disaggregated in one type, `sglang-router` picks http or grpc per mode. A new frontend type is one registered module, but the name is still read by the health expectations and the readiness loop; count those sites before choosing. +- **Modes are not types.** A new `frontend.type`, `services[].type`, or `engine.type` is for a different process with its own launch, health API, and registration model. A different CLI shape, transport, or discovery mode of the same binary is an override inside the existing class: `trtllm_serve` handles aggregate and disaggregated in one type, `sglang-router` picks http or grpc per mode. A new frontend type is one registered module; the only remaining name checks are for Dynamo- and sglang-router-specific features (request tracing, the gateway's own metrics listener, `slow_down`). - **Check upstream before working around it.** When a change encodes an upstream behavior (what a health endpoint returns, which keys a connector reads, what a flag does), read the upstream source at the version the container ships and cite the commit in the PR. Do not add a probe, shim, or port-scan workaround for something upstream already handles. - **Reuse the machinery before adding a mechanism.** Services plus `placement` before a bespoke launcher, `roles..restart` before a wrapper loop, `host_setup` before a setup script that needs the host. The smallest diff that rides existing machinery beats a self-contained new module. - **A user-visible feature ships complete.** A `tests/` case (dry-run for visible config, mock orchestrator for behavior), a `docs/` page or section, an example recipe under `examples/`, and regenerated `docs/schema-reference.md`. In a stacked PR, a test lives in the layer that introduces the behavior it asserts. @@ -111,7 +111,7 @@ The frontend is the process that owns the public OpenAI port (`FRONTEND_PUBLIC_P Two base classes cover the two ways a frontend learns about its workers. `DynamicFrontend` (`frontends/dynamic_frontend.py`) is for frontends whose workers register themselves over a discovery plane: it fronts any engine, needs no per-worker URL gate, and no worker is the public endpoint; Dynamo is its only implementation. A router binary that can also take static URLs (vLLM Router's ZMQ discovery mode) is a mode of a static router, not a dynamic frontend. `StaticRouterFrontend` (`frontends/static_router.py`) is the base for routers that take worker URLs on the command line. Subclasses set `executable`, `pd_flag`, `process_name` and override only what differs: `worker_scheme` (http or grpc per mode), `worker_bootstrap_port` (the P/D port advertised next to a prefill URL), `resolve_worker_host`, `get_managed_frontend_args` (arguments derived from the allocated topology; a conflicting user value raises instead of being overwritten), `build_bash_preamble`, `build_router_command`, `start_process` (test seam). `collect_workers` treats a positive `Process.http_port` as the definition of a routable worker. -Readiness runs in `BenchmarkStageMixin._wait_for_service_ready`: `wait_for_model` polls `health_endpoint` for `health_check.max_attempts * interval_seconds` and hands the response to `parse_health` with counts from `_get_health_expectations`, then `get_backend_health_urls` are polled with `wait_for_http_endpoints`. Everything the frontend knows about readiness belongs in those hooks. +Readiness runs in `BenchmarkStageMixin._wait_for_service_ready`: `wait_for_model` owns timing, abort, and progress logging, and calls the frontend's `probe_ready(host, port, n_prefill, n_decode)` once per poll with counts from the frontend's `health_expectations(config, processes)`; then `get_backend_health_urls` are polled with `wait_for_http_endpoints`. A probe raises `requests.RequestException` while the endpoint is down and otherwise returns a `WorkerHealthResult`. `core/health.py` has the three probe shapes to build on: `probe_json_health` (a JSON worker count through `parse_health`), `probe_http_ok` (a bare 200), `probe_direct_server` (`/health` then `/v1/models`). No frontend name appears in `wait_for_model` or `_get_health_expectations`. The frontend also owns the worker shape, and backends read it instead of comparing names: `worker_launch` (`dynamo` workers register with the Dynamo runtime, `direct` workers are the engine's own server), `worker_api_port(mode)` (`public` when the worker is itself the endpoint and binds `runtime.frontend_port`, `allocated` when a router fronts it on `Process.http_port`), and `expands_node_local_dp` (vLLM Router expands per-node hybrid-LB pools, so the vLLM backend launches one API per node and refuses `per_gpu`). `build_worker_command` still takes `frontend_type` and resolves it through `get_frontend`; a new router mode is therefore one frontend override plus, at most, a new attribute on the protocol. @@ -379,7 +379,7 @@ with patch.dict(os.environ, H100Rack.slurm_env()): ### Adding a Router Mode or a New Frontend Type -Decide first whether it is a mode or a type (see Design Rules). A mode of an existing router (a discovery flag, another connector, a transport) is an override in the existing frontend class, keyed on a backend method that resolves the effective per-role setting, with `StaticRouterFrontend` left unchanged. A new type is a different process: one module under `frontends/` decorated with `@register_frontend("")`, imported from `frontends/__init__.py`, subclassing `StaticRouterFrontend` when the router takes worker URLs or `DynamicFrontend` when workers register themselves, carrying `required_backend` and its recipe rules in `validate(config)` (no schema edits); then `_get_health_expectations` if it reports counts differently, until that moves onto the protocol; then a `tests/test__frontend.py` using `start_process` as the seam, a `docs/.md` page, and an `examples/` recipe. +Decide first whether it is a mode or a type (see Design Rules). A mode of an existing router (a discovery flag, another connector, a transport) is an override in the existing frontend class, keyed on a backend method that resolves the effective per-role setting, with `StaticRouterFrontend` left unchanged. A new type is a different process: one module under `frontends/` decorated with `@register_frontend("")`, imported from `frontends/__init__.py`, subclassing `StaticRouterFrontend` when the router takes worker URLs or `DynamicFrontend` when workers register themselves, carrying `required_backend` and its recipe rules in `validate(config)` (no schema edits), its readiness in `probe_ready` and `health_expectations`, and its port answers; then a `tests/test__frontend.py` using `start_process` as the seam, a `docs/.md` page, and an `examples/` recipe. ### Adding a New Benchmark diff --git a/src/srtctl/cli/mixins/benchmark_stage.py b/src/srtctl/cli/mixins/benchmark_stage.py index cae7f96be..5d716bdfb 100644 --- a/src/srtctl/cli/mixins/benchmark_stage.py +++ b/src/srtctl/cli/mixins/benchmark_stage.py @@ -49,107 +49,18 @@ logger = logging.getLogger(__name__) -def _vllm_data_parallel_size(config: "SrtConfig", mode: str) -> int: - """Return vLLM data parallel size for a mode, defaulting to one.""" - backend = config.backend - if getattr(backend, "type", None) != "vllm": - return 1 - - vllm_config = getattr(backend, "vllm_config", None) - mode_config = getattr(vllm_config, mode, None) if vllm_config else None - if not mode_config: - # Special case: no vllm_config at all defaulting to 1 then - return 1 - - return int(mode_config.get("data-parallel-size") or mode_config.get("data_parallel_size") or 1) - - -def _vllm_health_entries( - config: "SrtConfig", - mode: str, - logical_workers: int, - backend_processes: list["Process"] | None, -) -> int: - """Return expected Dynamo generate registrations for a vLLM worker mode.""" - dp_size = _vllm_data_parallel_size(config, mode) - dp_launch_mode = getattr(config.backend, "dp_launch_mode", "per_node") - if dp_size > 1 and dp_launch_mode == "per_node": - if backend_processes is None: - raise ValueError("backend_processes are required for per-node DP health expectations") - endpoint_mode = "agg" if mode == "aggregated" else mode - mode_processes = [process for process in backend_processes if process.endpoint_mode == endpoint_mode] - if not mode_processes: - return 0 - - vllm_config = getattr(config.backend, "vllm_config", None) - mode_config = getattr(vllm_config, mode, None) if vllm_config else None - mode_config = mode_config or {} - tp_size = int(mode_config.get("tensor-parallel-size") or mode_config.get("tensor_parallel_size") or 1) - pp_size = int(mode_config.get("pipeline-parallel-size") or mode_config.get("pipeline_parallel_size") or 1) - gpu_indices = getattr(mode_processes[0], "gpu_indices", None) - if gpu_indices is None: - # Compatibility for callers that provide only registration-count - # process stubs. Real launch processes always carry GPU indices. - return len(mode_processes) - local_gpu_count = len(gpu_indices) - spans_nodes = tp_size * pp_size > local_gpu_count - if spans_nodes: - return logical_workers - return len(mode_processes) - - return logical_workers * dp_size - - def _get_health_expectations( config: "SrtConfig", backend_processes: list["Process"] | None = None ) -> tuple[int, int, str, int]: - """Compute expected health counts in the units reported by the frontend. + """Expected health counts in the units the frontend reports, a description, and their sum. - Dynamo's /health endpoint reports registered generate instances. For vLLM - DP workers, per-GPU launch registers one entry per DP rank, while per-node - launch registers one entry per node-local process. vLLM Router expands - each advertised base URL into its node-local DP ranks. + The frontend knows what its readiness endpoint counts (Dynamo generate + registrations, Router-expanded DP ranks, logical workers); see + ``FrontendProtocol.health_expectations``. """ - r = config.resources - - if r.num_agg > 0: - logical_prefill = 0 - logical_decode = r.num_agg - worker_desc = f"{r.num_agg} agg" - else: - logical_prefill = r.num_prefill - logical_decode = r.num_decode - worker_desc = f"{r.num_prefill}P + {r.num_decode}D" - - if config.frontend.type == "dynamo" and getattr(config.backend, "type", None) == "vllm": - if r.num_agg > 0: - n_prefill = 0 - n_decode = _vllm_health_entries(config, "aggregated", logical_decode, backend_processes) - else: - n_prefill = _vllm_health_entries(config, "prefill", logical_prefill, backend_processes) - n_decode = _vllm_health_entries(config, "decode", logical_decode, backend_processes) - - count_desc = f"{n_prefill}P + {n_decode}D Dynamo generate instances; logical workers: {worker_desc}" - return n_prefill, n_decode, count_desc, n_prefill + n_decode - - if config.frontend.type == "vllm-router" and backend_processes is not None: - from srtctl.frontends.vllm_router import routed_process_dp_size - - n_prefill = sum( - routed_process_dp_size(config.backend, process) - for process in backend_processes - if process.endpoint_mode == "prefill" and process.http_port > 0 - ) - n_decode = sum( - routed_process_dp_size(config.backend, process) - for process in backend_processes - if process.endpoint_mode in {"decode", "agg"} and process.http_port > 0 - ) - count_desc = f"{n_prefill}P + {n_decode}D Router workers; logical workers: {worker_desc}" - return n_prefill, n_decode, count_desc, n_prefill + n_decode - - count_desc = worker_desc - return logical_prefill, logical_decode, count_desc, logical_prefill + logical_decode + frontend = get_frontend(config.frontend.type) + n_prefill, n_decode, count_desc = frontend.health_expectations(config, backend_processes) + return n_prefill, n_decode, count_desc, n_prefill + n_decode SERVER_READY_FILENAME = "server_ready.json" diff --git a/src/srtctl/core/health.py b/src/srtctl/core/health.py index 73f673771..8b824fb5c 100644 --- a/src/srtctl/core/health.py +++ b/src/srtctl/core/health.py @@ -17,6 +17,7 @@ import socket import threading import time +from collections.abc import Callable from dataclasses import dataclass import requests @@ -204,8 +205,8 @@ def check_trtllm_serve_health( be empty). The trtllm_serve frontend already gates each worker for readiness before starting the orchestrator, so a 200 here means the stack is ready. - Note: wait_for_model() short-circuits to ready on the 200 for trtllm_serve and does - not call this, so this exists mainly as the FrontendProtocol.parse_health hook. + Note: TRTLLMServeFrontend.probe_ready treats the 200 as ready without parsing a + body, so this exists mainly as the FrontendProtocol.parse_health hook. """ return WorkerHealthResult( ready=True, @@ -255,6 +256,47 @@ def check_vllm_health( ) +def probe_json_health( + host: str, + port: int, + endpoint: str, + parse: Callable[[dict, int, int], WorkerHealthResult], + expected_prefill: int, + expected_decode: int, +) -> WorkerHealthResult: + """GET ``host:port/endpoint`` and hand a 200 JSON body to ``parse``. + + The default ``probe_ready`` for frontends whose health endpoint reports worker + counts as JSON (Dynamo ``/health``, the static routers' ``/workers``). A + non-200 status is not ready; connection errors propagate for the caller to retry. + """ + response = requests.get(f"http://{host}:{port}{endpoint}", timeout=5.0) + if response.status_code != 200: + return WorkerHealthResult(ready=False, message=f"{endpoint} returned HTTP {response.status_code}") + return parse(response.json(), expected_prefill, expected_decode) + + +def probe_http_ok(host: str, port: int, endpoint: str, ready_message: str) -> WorkerHealthResult: + """GET ``host:port/endpoint``; a 200 is ready, whatever the body. Connection errors propagate.""" + response = requests.get(f"http://{host}:{port}{endpoint}", timeout=5.0) + if response.status_code != 200: + return WorkerHealthResult(ready=False, message=f"{endpoint} returned HTTP {response.status_code}") + return WorkerHealthResult(ready=True, message=ready_message) + + +def probe_direct_server(host: str, port: int) -> WorkerHealthResult: + """Readiness of a direct OpenAI server: ``/health`` must be 200, then ``/v1/models`` must list a model. + + The ``probe_ready`` of the frontends whose one worker is the public endpoint + (direct vLLM and SGLang). Connection errors on ``/health`` propagate. + """ + health_url = f"http://{host}:{port}/health" + response = requests.get(health_url, timeout=5.0) + if response.status_code != 200: + return WorkerHealthResult(ready=False, message=f"/health returned HTTP {response.status_code}") + return check_vllm_health(host, port, health_url) + + def wait_for_port( host: str, port: int, @@ -464,10 +506,13 @@ def wait_for_model( frontend_type: str = "dynamo", stop_event: threading.Event | None = None, ) -> bool: - """Wait for model to be ready with expected worker counts. + """Wait for the public endpoint to report every expected worker. - This is the pure Python replacement for the bash wait_for_model function. - It polls the appropriate health endpoint and validates worker counts. + The loop owns timing, abort, and progress logging; the frontend owns the + probe. ``frontend.probe_ready`` performs one readiness check (a JSON worker + count, a bare 200, the direct server's ``/health`` plus ``/v1/models``) and + returns a ``WorkerHealthResult``; a ``requests.RequestException`` means the + endpoint is not up yet and is retried. Args: host: Model server hostname or IP @@ -477,7 +522,7 @@ def wait_for_model( poll_interval: Seconds between health checks timeout: Maximum wait time in seconds report_every: Log progress every N seconds - frontend_type: Frontend adapter used to select and parse health + frontend_type: Frontend whose probe_ready decides readiness stop_event: Optional threading.Event to abort waiting Returns: @@ -486,24 +531,16 @@ def wait_for_model( from srtctl.frontends import get_frontend frontend = get_frontend(frontend_type) - health_url = f"http://{host}:{port}{frontend.health_endpoint}" - if frontend.health_endpoint == "/workers": - logger.info( - "Polling %s every %.1fs for %d prefills and %d decodes (%s frontend)", - health_url, - poll_interval, - n_prefill, - n_decode, - frontend_type, - ) - else: - logger.info( - "Polling %s every %.1fs for %d prefills and %d decodes", - health_url, - poll_interval, - n_prefill, - n_decode, - ) + logger.info( + "Polling http://%s:%d%s every %.1fs for %d prefills and %d decodes (%s frontend)", + host, + port, + frontend.health_endpoint, + poll_interval, + n_prefill, + n_decode, + frontend_type, + ) start_time = time.time() last_report_time = start_time @@ -520,39 +557,15 @@ def wait_for_model( logger.error("Model did not get healthy in %.0f seconds", timeout) return False - # Try to fetch health try: - response = requests.get(health_url, timeout=5.0) - if response.status_code == 200: - # trtllm-serve /health may return an empty body; a 200 is sufficient - # (workers were gated by the frontend before the orchestrator started). - if frontend_type == "trtllm_serve": - logger.info("trtllm-serve frontend healthy at %s", health_url) - return True - if frontend_type in ("vllm", "sglang"): - # Direct modes: the worker's own /health + /v1/models. - result = check_vllm_health(host, port, health_url) - if result.ready: - logger.info(result.message) - return True - if time.time() - last_report_time >= report_every: - logger.info(result.message) - last_report_time = time.time() - time.sleep(poll_interval) - continue - - response_json = response.json() - - result = frontend.parse_health(response_json, n_prefill, n_decode) - - if result.ready: - logger.info(result.message) - return True - - # Report progress periodically - if time.time() - last_report_time >= report_every: - logger.info(result.message) - last_report_time = time.time() + result = frontend.probe_ready(host, port, n_prefill, n_decode) + if result.ready: + logger.info(result.message) + return True + # Report progress periodically + if time.time() - last_report_time >= report_every: + logger.info(result.message) + last_report_time = time.time() except requests.exceptions.RequestException as e: # Report connection errors periodically diff --git a/src/srtctl/frontends/base.py b/src/srtctl/frontends/base.py index d1b0984a7..8a88db4df 100644 --- a/src/srtctl/frontends/base.py +++ b/src/srtctl/frontends/base.py @@ -113,6 +113,26 @@ def worker_ready_port(self, process: "Process") -> int: """Port polled for a worker's own ``/health`` during sequential endpoint start.""" ... + def health_expectations(self, config: Any, processes: list["Process"] | None) -> tuple[int, int, str]: + """Expected ``(prefill, decode)`` counts in the units this frontend's readiness reports, plus a description. + + Aggregate workers count as decode. Dynamo counts registered generate + instances (one per vLLM DP rank or node-local process), vLLM Router + counts the ranks it expands each advertised URL into, every other + frontend counts logical workers. ``processes`` is ``None`` before the + topology is known; implementations fall back to logical counts then. + """ + ... + + def probe_ready(self, host: str, port: int, expected_prefill: int, expected_decode: int) -> "WorkerHealthResult": + """One readiness probe against the public endpoint at ``host:port``. + + Raise ``requests.RequestException`` while the endpoint is unreachable; + ``wait_for_model`` retries until its timeout. Anything else that is not + ready comes back as a result whose message explains why. + """ + ... + def validate(self, config: Any) -> None: """Recipe-level rules for this frontend. @@ -175,6 +195,14 @@ def get_frontend_args_list(self, args: dict[str, Any] | None) -> list[str]: ... +def logical_health_expectations(config: Any) -> tuple[int, int, str]: + """Expected counts in logical workers: aggregate workers count as decode.""" + r = config.resources + if r.num_agg > 0: + return 0, r.num_agg, f"{r.num_agg} agg" + return r.num_prefill, r.num_decode, f"{r.num_prefill}P + {r.num_decode}D" + + def agg_leader_nodes(processes: list["Process"]) -> list[str]: """Nodes of the aggregate workers' leader ranks, in topology order, without repeats. diff --git a/src/srtctl/frontends/dynamic_frontend.py b/src/srtctl/frontends/dynamic_frontend.py index f3f9257f3..1492903bb 100644 --- a/src/srtctl/frontends/dynamic_frontend.py +++ b/src/srtctl/frontends/dynamic_frontend.py @@ -25,6 +25,9 @@ from typing import TYPE_CHECKING, Any, ClassVar, Literal +from srtctl.core.health import WorkerHealthResult, probe_json_health +from srtctl.frontends.base import logical_health_expectations + if TYPE_CHECKING: from srtctl.core.topology import Process @@ -43,6 +46,19 @@ def health_endpoint(self) -> str: """The frontend reports its registered workers here; ``parse_health`` counts them.""" return "/health" + def parse_health(self, response_json: dict, expected_prefill: int, expected_decode: int) -> WorkerHealthResult: + """Count the registered workers in the frontend's health body; each implementation knows its format.""" + raise NotImplementedError(f"{type(self).__name__} must parse its own registration count") + + def probe_ready(self, host: str, port: int, expected_prefill: int, expected_decode: int) -> WorkerHealthResult: + """One GET of the registration endpoint, parsed against the expected counts.""" + return probe_json_health(host, port, self.health_endpoint, self.parse_health, expected_prefill, expected_decode) + + def health_expectations(self, config: Any, processes: list[Process] | None) -> tuple[int, int, str]: + """One registration per logical worker unless the implementation knows better.""" + del processes + return logical_health_expectations(config) + def validate(self, config: Any) -> None: """Recipe-level rules beyond the backend pairing; none by default.""" del config diff --git a/src/srtctl/frontends/dynamo.py b/src/srtctl/frontends/dynamo.py index b424e12da..dc62c5d49 100644 --- a/src/srtctl/frontends/dynamo.py +++ b/src/srtctl/frontends/dynamo.py @@ -18,7 +18,7 @@ from srtctl.core.observability_nsys import wrap_observability_nsys from srtctl.core.schema import build_otel_env from srtctl.core.slurm import CONTAINER_REMAP_ROOT_EXPORT, start_srun_process -from srtctl.frontends.base import register_frontend +from srtctl.frontends.base import logical_health_expectations, register_frontend from srtctl.frontends.dynamic_frontend import DynamicFrontend from srtctl.services.implicit import discovery_env @@ -33,6 +33,62 @@ ROUTER_POLICY_CONFIG_CONTAINER_PATH = f"/logs/{ROUTER_POLICY_CONFIG_FILENAME}" +def vllm_data_parallel_size(config: Any, mode: str) -> int: + """Return vLLM data parallel size for a mode, defaulting to one.""" + backend = config.backend + if getattr(backend, "type", None) != "vllm": + return 1 + + vllm_config = getattr(backend, "vllm_config", None) + mode_config = getattr(vllm_config, mode, None) if vllm_config else None + if not mode_config: + # Special case: no vllm_config at all defaulting to 1 then + return 1 + + return int(mode_config.get("data-parallel-size") or mode_config.get("data_parallel_size") or 1) + + +def vllm_health_entries( + config: Any, + mode: str, + logical_workers: int, + backend_processes: list["Process"] | None, +) -> int: + """Return expected Dynamo generate registrations for a vLLM worker mode. + + Per-GPU DP launch registers one entry per DP rank; per-node launch registers + one entry per node-local process, or one per logical worker when a replica + spans nodes. + """ + dp_size = vllm_data_parallel_size(config, mode) + dp_launch_mode = getattr(config.backend, "dp_launch_mode", "per_node") + if dp_size > 1 and dp_launch_mode == "per_node": + if backend_processes is None: + raise ValueError("backend_processes are required for per-node DP health expectations") + endpoint_mode = "agg" if mode == "aggregated" else mode + mode_processes = [process for process in backend_processes if process.endpoint_mode == endpoint_mode] + if not mode_processes: + return 0 + + vllm_config = getattr(config.backend, "vllm_config", None) + mode_config = getattr(vllm_config, mode, None) if vllm_config else None + mode_config = mode_config or {} + tp_size = int(mode_config.get("tensor-parallel-size") or mode_config.get("tensor_parallel_size") or 1) + pp_size = int(mode_config.get("pipeline-parallel-size") or mode_config.get("pipeline_parallel_size") or 1) + gpu_indices = getattr(mode_processes[0], "gpu_indices", None) + if gpu_indices is None: + # Compatibility for callers that provide only registration-count + # process stubs. Real launch processes always carry GPU indices. + return len(mode_processes) + local_gpu_count = len(gpu_indices) + spans_nodes = tp_size * pp_size > local_gpu_count + if spans_nodes: + return logical_workers + return len(mode_processes) + + return logical_workers * dp_size + + @register_frontend("dynamo") class DynamoFrontend(DynamicFrontend): """Dynamo frontend implementation. @@ -82,6 +138,28 @@ def parse_health( """Parse dynamo /health endpoint response.""" return check_dynamo_health(response_json, expected_prefill, expected_decode) + def health_expectations(self, config: Any, processes: list["Process"] | None) -> tuple[int, int, str]: + """Dynamo's /health counts registered generate instances. + + A vLLM worker registers one per DP rank (per_gpu launch) or one per + node-local process (per_node launch); every other engine registers one per + logical worker. + """ + logical_prefill, logical_decode, worker_desc = logical_health_expectations(config) + if getattr(config.backend, "type", None) != "vllm": + return logical_prefill, logical_decode, worker_desc + if config.resources.num_agg > 0: + n_prefill = 0 + n_decode = vllm_health_entries(config, "aggregated", logical_decode, processes) + else: + n_prefill = vllm_health_entries(config, "prefill", logical_prefill, processes) + n_decode = vllm_health_entries(config, "decode", logical_decode, processes) + return ( + n_prefill, + n_decode, + f"{n_prefill}P + {n_decode}D Dynamo generate instances; logical workers: {worker_desc}", + ) + def start_frontends( self, topology: Any, # FrontendTopology diff --git a/src/srtctl/frontends/sglang_direct.py b/src/srtctl/frontends/sglang_direct.py index 89a3f96ae..20641aefa 100644 --- a/src/srtctl/frontends/sglang_direct.py +++ b/src/srtctl/frontends/sglang_direct.py @@ -15,8 +15,8 @@ import threading from typing import TYPE_CHECKING, Any, ClassVar, Literal -from srtctl.core.health import WorkerHealthResult -from srtctl.frontends.base import agg_leader_nodes, register_frontend +from srtctl.core.health import WorkerHealthResult, probe_direct_server +from srtctl.frontends.base import agg_leader_nodes, logical_health_expectations, register_frontend if TYPE_CHECKING: from srtctl.core.processes import ManagedProcess @@ -74,6 +74,15 @@ def direct_endpoint_nodes(self, processes: list[Process]) -> list[str]: def worker_ready_port(self, process: Process) -> int: return process.sys_port + def probe_ready(self, host: str, port: int, expected_prefill: int, expected_decode: int) -> WorkerHealthResult: + """The worker's own /health, then /v1/models must list the model.""" + del expected_prefill, expected_decode + return probe_direct_server(host, port) + + def health_expectations(self, config: Any, processes: list[Process] | None) -> tuple[int, int, str]: + del processes + return logical_health_expectations(config) + def validate(self, config: Any) -> None: """One aggregate ``sglang.launch_server`` owns the public port. diff --git a/src/srtctl/frontends/static_router.py b/src/srtctl/frontends/static_router.py index e2c7510b1..e947314af 100644 --- a/src/srtctl/frontends/static_router.py +++ b/src/srtctl/frontends/static_router.py @@ -11,8 +11,9 @@ from dataclasses import dataclass from typing import TYPE_CHECKING, Any, ClassVar, Literal -from srtctl.core.health import WorkerHealthResult, check_static_router_health +from srtctl.core.health import WorkerHealthResult, check_static_router_health, probe_json_health from srtctl.core.slurm import get_hostname_ip, start_srun_process +from srtctl.frontends.base import logical_health_expectations if TYPE_CHECKING: from srtctl.core.processes import ManagedProcess @@ -100,6 +101,15 @@ def parse_health( ) -> WorkerHealthResult: return check_static_router_health(response_json, expected_prefill, expected_decode) + def probe_ready(self, host: str, port: int, expected_prefill: int, expected_decode: int) -> WorkerHealthResult: + """One GET of the router's worker registry, parsed against the expected counts.""" + return probe_json_health(host, port, self.health_endpoint, self.parse_health, expected_prefill, expected_decode) + + def health_expectations(self, config: Any, processes: list[Process] | None) -> tuple[int, int, str]: + """The registry lists one entry per logical worker unless the router expands them.""" + del processes + return logical_health_expectations(config) + def get_frontend_args_list(self, args: dict[str, Any] | None) -> list[str]: """Convert config values to CLI arguments, preserving repeated values.""" if not args: diff --git a/src/srtctl/frontends/trtllm_serve.py b/src/srtctl/frontends/trtllm_serve.py index b3ae643b8..5c39130ef 100644 --- a/src/srtctl/frontends/trtllm_serve.py +++ b/src/srtctl/frontends/trtllm_serve.py @@ -18,9 +18,9 @@ import yaml -from srtctl.core.health import WorkerHealthResult, check_trtllm_serve_health, wait_for_health +from srtctl.core.health import WorkerHealthResult, check_trtllm_serve_health, probe_http_ok, wait_for_health from srtctl.core.slurm import get_hostname_ip, start_srun_process -from srtctl.frontends.base import register_frontend +from srtctl.frontends.base import logical_health_expectations, register_frontend if TYPE_CHECKING: from srtctl.core.processes import ManagedProcess @@ -84,6 +84,17 @@ def worker_ready_port(self, process: "Process") -> int: """A trtllm-serve worker reports /health on its own OpenAI port.""" return process.http_port + def probe_ready(self, host: str, port: int, expected_prefill: int, expected_decode: int) -> WorkerHealthResult: + """A 200 from /health is ready: the body may be empty, and every worker was gated before the orchestrator started.""" + del expected_prefill, expected_decode + return probe_http_ok( + host, port, self.health_endpoint, f"trtllm-serve frontend healthy at http://{host}:{port}/health" + ) + + def health_expectations(self, config: Any, processes: list["Process"] | None) -> tuple[int, int, str]: + del processes + return logical_health_expectations(config) + def validate(self, config: Any) -> None: """One direct aggregate worker or one disaggregated orchestrator; either way one public endpoint.""" if config.frontend.enable_multiple_frontends: diff --git a/src/srtctl/frontends/vllm.py b/src/srtctl/frontends/vllm.py index 2d0b6a1fe..327edd95c 100644 --- a/src/srtctl/frontends/vllm.py +++ b/src/srtctl/frontends/vllm.py @@ -14,8 +14,8 @@ import threading from typing import TYPE_CHECKING, Any, ClassVar, Literal -from srtctl.core.health import WorkerHealthResult -from srtctl.frontends.base import agg_leader_nodes, register_frontend +from srtctl.core.health import WorkerHealthResult, probe_direct_server +from srtctl.frontends.base import agg_leader_nodes, logical_health_expectations, register_frontend if TYPE_CHECKING: from srtctl.core.processes import ManagedProcess @@ -75,6 +75,15 @@ def direct_endpoint_nodes(self, processes: list[Process]) -> list[str]: def worker_ready_port(self, process: Process) -> int: return process.sys_port + def probe_ready(self, host: str, port: int, expected_prefill: int, expected_decode: int) -> WorkerHealthResult: + """The worker's own /health, then /v1/models must list the model.""" + del expected_prefill, expected_decode + return probe_direct_server(host, port) + + def health_expectations(self, config: Any, processes: list[Process] | None) -> tuple[int, int, str]: + del processes + return logical_health_expectations(config) + def validate(self, config: Any) -> None: """The one aggregate ``vllm serve`` owns the public port: no nginx fan-out, no P/D, one worker.""" if config.frontend.enable_multiple_frontends: diff --git a/src/srtctl/frontends/vllm_router.py b/src/srtctl/frontends/vllm_router.py index 1b26252a3..a4ad974d5 100644 --- a/src/srtctl/frontends/vllm_router.py +++ b/src/srtctl/frontends/vllm_router.py @@ -8,7 +8,7 @@ import shlex from typing import TYPE_CHECKING, Any, ClassVar -from srtctl.frontends.base import register_frontend +from srtctl.frontends.base import logical_health_expectations, register_frontend from srtctl.frontends.static_router import StaticRouterFrontend if TYPE_CHECKING: @@ -205,6 +205,23 @@ def worker_bootstrap_port(self, backend: Any, process: Process) -> int | None: del backend return process.nixl_port + def health_expectations(self, config: Any, processes: list[Process] | None) -> tuple[int, int, str]: + """Router's /workers lists one entry per DP rank it expands each advertised URL into.""" + logical_prefill, logical_decode, worker_desc = logical_health_expectations(config) + if processes is None: + return logical_prefill, logical_decode, worker_desc + n_prefill = sum( + routed_process_dp_size(config.backend, process) + for process in processes + if process.endpoint_mode == "prefill" and process.http_port > 0 + ) + n_decode = sum( + routed_process_dp_size(config.backend, process) + for process in processes + if process.endpoint_mode in {"decode", "agg"} and process.http_port > 0 + ) + return n_prefill, n_decode, f"{n_prefill}P + {n_decode}D Router workers; logical workers: {worker_desc}" + def worker_metrics_port(self, process: Process, runtime: Any) -> int | None: """Every node-local hybrid-LB pool has its own API and /metrics; a positive http_port marks one.""" del runtime diff --git a/tests/test_health.py b/tests/test_health.py index b894104ef..23cbc1026 100644 --- a/tests/test_health.py +++ b/tests/test_health.py @@ -3,6 +3,8 @@ """Tests for health check parsing (Dynamo and SGLang router).""" +import pytest + from srtctl.core.health import ( WorkerHealthResult, check_dynamo_health, @@ -427,3 +429,140 @@ def test_with_counts(self): assert result.prefill_ready == 2 assert result.decode_ready == 4 + + +# ============================================================================ +# Frontend readiness probes and the wait_for_model loop +# ============================================================================ + + +def _http(status: int, body=None): + from unittest.mock import Mock + + response = Mock(status_code=status) + response.json = Mock(return_value=body if body is not None else {}) + return response + + +class TestFrontendProbes: + """Each frontend owns one readiness probe; the loop only retries and logs.""" + + def test_bare_200_frontend_is_ready_without_a_body(self, monkeypatch): + from srtctl.core import health + from srtctl.frontends import get_frontend + + monkeypatch.setattr(health.requests, "get", lambda url, timeout: _http(200)) + result = get_frontend("trtllm_serve").probe_ready("router", 8000, 1, 1) + assert result.ready is True + assert "router:8000/health" in result.message + + def test_non_200_is_not_ready_with_the_status_in_the_message(self, monkeypatch): + from srtctl.core import health + from srtctl.frontends import get_frontend + + monkeypatch.setattr(health.requests, "get", lambda url, timeout: _http(503)) + for frontend_type in ("dynamo", "sglang-router", "vllm-router", "trtllm_serve", "vllm", "sglang"): + result = get_frontend(frontend_type).probe_ready("router", 8000, 1, 1) + assert result.ready is False, frontend_type + assert "HTTP 503" in result.message, frontend_type + + def test_json_registry_goes_through_parse_health(self, monkeypatch): + from srtctl.core import health + from srtctl.frontends import get_frontend + + seen: dict[str, str] = {} + + def fake_get(url, timeout): + seen["url"] = url + return _http(200, {"stats": {"prefill_count": 1, "decode_count": 2, "regular_count": 0}, "workers": []}) + + monkeypatch.setattr(health.requests, "get", fake_get) + result = get_frontend("sglang-router").probe_ready("router", 8000, 1, 2) + assert seen["url"] == "http://router:8000/workers" + assert result.prefill_ready == 1 + assert result.decode_ready == 2 + + def test_direct_server_needs_health_and_a_listed_model(self, monkeypatch): + from srtctl.core import health + from srtctl.frontends import get_frontend + + bodies = { + "http://worker:8000/health": _http(200), + "http://worker:8000/v1/models": _http(200, {"data": [{"id": "Qwen/Qwen3-0.6B"}]}), + } + monkeypatch.setattr(health.requests, "get", lambda url, timeout: bodies[url]) + for frontend_type in ("vllm", "sglang"): + assert get_frontend(frontend_type).probe_ready("worker", 8000, 0, 1).ready is True + + bodies["http://worker:8000/v1/models"] = _http(200, {"data": []}) + result = get_frontend("vllm").probe_ready("worker", 8000, 0, 1) + assert result.ready is False + assert "no models" in result.message + + def test_connection_errors_propagate_for_the_loop_to_retry(self, monkeypatch): + import requests + + from srtctl.core import health + from srtctl.frontends import get_frontend + + def refuse(url, timeout): + raise requests.exceptions.ConnectionError("refused") + + monkeypatch.setattr(health.requests, "get", refuse) + with pytest.raises(requests.exceptions.RequestException): + get_frontend("dynamo").probe_ready("router", 8000, 1, 1) + + +class TestWaitForModel: + def _stub(self, monkeypatch, probes): + from types import SimpleNamespace + + import srtctl.frontends + + calls = iter(probes) + + def probe_ready(host, port, expected_prefill, expected_decode): + outcome = next(calls) + if isinstance(outcome, Exception): + raise outcome + return outcome + + stub = SimpleNamespace(type="stub", health_endpoint="/probe", probe_ready=probe_ready) + monkeypatch.setattr(srtctl.frontends, "get_frontend", lambda frontend_type: stub) + return stub + + def test_returns_true_once_the_probe_reports_ready(self, monkeypatch): + from srtctl.core.health import wait_for_model + + self._stub( + monkeypatch, + [ + WorkerHealthResult(ready=False, message="1/2 workers"), + WorkerHealthResult(ready=True, message="2/2 workers"), + ], + ) + assert wait_for_model("router", 8000, 1, 1, poll_interval=0.001, timeout=5, frontend_type="stub") is True + + def test_returns_false_on_timeout_while_the_endpoint_refuses(self, monkeypatch): + import itertools + + import requests + + from srtctl.core.health import wait_for_model + + self._stub(monkeypatch, itertools.repeat(requests.exceptions.ConnectionError("refused"))) + assert wait_for_model("router", 8000, 1, 1, poll_interval=0.001, timeout=0.05, frontend_type="stub") is False + + def test_stop_event_aborts(self, monkeypatch): + import itertools + import threading + + from srtctl.core.health import wait_for_model + + self._stub(monkeypatch, itertools.repeat(WorkerHealthResult(ready=False, message="waiting"))) + stop = threading.Event() + stop.set() + assert ( + wait_for_model("router", 8000, 1, 1, poll_interval=0.001, timeout=5, frontend_type="stub", stop_event=stop) + is False + ) diff --git a/tests/test_health_expectations.py b/tests/test_health_expectations.py index 2a4e0eb76..671f70790 100644 --- a/tests/test_health_expectations.py +++ b/tests/test_health_expectations.py @@ -5,7 +5,8 @@ from types import SimpleNamespace -from srtctl.cli.mixins.benchmark_stage import _get_health_expectations, _vllm_data_parallel_size +from srtctl.cli.mixins.benchmark_stage import _get_health_expectations +from srtctl.frontends.dynamo import vllm_data_parallel_size def _config( @@ -154,7 +155,7 @@ def test_dynamo_vllm_without_dp_config_defaults_to_logical_counts(): def test_non_dynamo_frontend_uses_logical_worker_counts(): """Only Dynamo reports per-DP-rank generate instances; others stay logical.""" vllm_config = SimpleNamespace(prefill={"data-parallel-size": 2}, decode={"data-parallel-size": 8}, aggregated=None) - config = _config("none", "vllm", num_prefill=6, num_decode=1, vllm_config=vllm_config) + config = _config("sglang-router", "vllm", num_prefill=6, num_decode=1, vllm_config=vllm_config) n_prefill, n_decode, count_desc, num_workers = _get_health_expectations(config) @@ -174,13 +175,13 @@ def test_dynamo_non_vllm_backend_uses_logical_worker_counts(): def test_vllm_data_parallel_size_reads_both_key_styles_and_defaults(): dashed = _config("dynamo", "vllm", vllm_config=SimpleNamespace(prefill={"data-parallel-size": 4})) - assert _vllm_data_parallel_size(dashed, "prefill") == 4 + assert vllm_data_parallel_size(dashed, "prefill") == 4 underscored = _config("dynamo", "vllm", vllm_config=SimpleNamespace(decode={"data_parallel_size": 3})) - assert _vllm_data_parallel_size(underscored, "decode") == 3 + assert vllm_data_parallel_size(underscored, "decode") == 3 no_vllm_config = _config("dynamo", "vllm", vllm_config=None) - assert _vllm_data_parallel_size(no_vllm_config, "prefill") == 1 + assert vllm_data_parallel_size(no_vllm_config, "prefill") == 1 non_vllm = _config("dynamo", "sglang") - assert _vllm_data_parallel_size(non_vllm, "prefill") == 1 + assert vllm_data_parallel_size(non_vllm, "prefill") == 1 From 298dc9e9db75f40415a81ba81ddeac90f6ee8e29 Mon Sep 17 00:00:00 2001 From: Ishan Dhanani Date: Mon, 21 Sep 2026 17:49:26 -0700 Subject: [PATCH 6/7] style(frontends): drop del-of-unused-argument lines Review comment on #487. Ruff's unused-argument rules are off in this repo, so 'del name' for a parameter a hook ignores adds nothing; the pattern had spread from a few existing sites to every new protocol method. Remove all 49 in the frontends package and note the rule in CLAUDE.md. Signed-off-by: Ishan Dhanani --- CLAUDE.md | 1 + src/srtctl/frontends/dynamic_frontend.py | 5 ----- src/srtctl/frontends/dynamo.py | 3 --- src/srtctl/frontends/sglang.py | 2 -- src/srtctl/frontends/sglang_direct.py | 8 -------- src/srtctl/frontends/static_router.py | 14 -------------- src/srtctl/frontends/trtllm_serve.py | 7 ------- src/srtctl/frontends/vllm.py | 7 ------- src/srtctl/frontends/vllm_router.py | 3 --- 9 files changed, 1 insertion(+), 49 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 9f08cb179..9b138415d 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -47,6 +47,7 @@ Follow these patterns when extending the codebase: - **TypedDict for external data** - Use `TypedDict` for typing dicts from JSON/external sources where you can't control the structure. - **Single source of truth** - Create context objects (like `RuntimeContext`) that compute all derived paths/values once at startup rather than recomputing. - **testing** - when we make a new significant feature change, we should always add a new test +- **Unused parameters stay untouched** - A hook that ignores an argument just ignores it. Ruff's unused-argument rules are off, so `del name` lines add nothing but noise. ## Design Rules diff --git a/src/srtctl/frontends/dynamic_frontend.py b/src/srtctl/frontends/dynamic_frontend.py index 1492903bb..92c87194d 100644 --- a/src/srtctl/frontends/dynamic_frontend.py +++ b/src/srtctl/frontends/dynamic_frontend.py @@ -56,16 +56,13 @@ def probe_ready(self, host: str, port: int, expected_prefill: int, expected_deco def health_expectations(self, config: Any, processes: list[Process] | None) -> tuple[int, int, str]: """One registration per logical worker unless the implementation knows better.""" - del processes return logical_health_expectations(config) def validate(self, config: Any) -> None: """Recipe-level rules beyond the backend pairing; none by default.""" - del config def worker_api_port(self, mode: str) -> Literal["public", "allocated"]: """A registered worker never binds the public port; the frontend owns it.""" - del mode return "allocated" def get_backend_health_urls( @@ -75,12 +72,10 @@ def get_backend_health_urls( network_interface: str | None = None, ) -> list[str]: """Registration is the readiness gate; there is no per-worker URL to poll first.""" - del backend, backend_processes, network_interface return [] def direct_endpoint_nodes(self, processes: list[Process]) -> list[str]: """The frontend process is the endpoint, never a worker.""" - del processes return [] def get_frontend_args_list(self, args: dict[str, Any] | None) -> list[str]: diff --git a/src/srtctl/frontends/dynamo.py b/src/srtctl/frontends/dynamo.py index dc62c5d49..05f1682e0 100644 --- a/src/srtctl/frontends/dynamo.py +++ b/src/srtctl/frontends/dynamo.py @@ -105,12 +105,10 @@ class DynamoFrontend(DynamicFrontend): def worker_metrics_port(self, process: "Process", runtime: "RuntimeContext") -> int | None: """Every rank runs the Dynamo system status server (health, metrics) on its system port.""" - del runtime return process.sys_port if process.sys_port > 0 else None def worker_endpoint_port(self, process: "Process", config: Any, runtime: "RuntimeContext") -> int | None: """One endpoint per logical worker: the leader's system port, or the native engine's port behind a sidecar.""" - del runtime if not process.is_leader: return None port = process.http_port if config.dynamo.sidecar else process.sys_port @@ -118,7 +116,6 @@ def worker_endpoint_port(self, process: "Process", config: Any, runtime: "Runtim def profiling_control_port(self, process: "Process", config: Any, runtime: "RuntimeContext") -> int | None: """Iteration-triggered captures are controlled per rank on the system port.""" - del config, runtime return process.sys_port if process.sys_port > 0 else None def profiling_control_is_leader_only(self, config: Any) -> bool: diff --git a/src/srtctl/frontends/sglang.py b/src/srtctl/frontends/sglang.py index 20d0f8dc2..2ed763b2a 100644 --- a/src/srtctl/frontends/sglang.py +++ b/src/srtctl/frontends/sglang.py @@ -50,7 +50,6 @@ def get_managed_frontend_args( is given (``PrometheusConfig`` is ``None`` otherwise), and tachometer is on by default, so srtctl always asks for it on every interface. """ - del backend, backend_processes frontend_args = config.frontend.args or {} normalized = {str(key).replace("_", "-") for key in frontend_args} managed: list[str] = [] @@ -64,7 +63,6 @@ def worker_scheme(self, backend: Any, mode: str) -> str: return "grpc" if backend.is_grpc_mode(mode) else "http" def resolve_worker_host(self, node: str, network_interface: str | None) -> str: - del network_interface return get_hostname_ip(node) def start_process(self, **kwargs: Any) -> Any: diff --git a/src/srtctl/frontends/sglang_direct.py b/src/srtctl/frontends/sglang_direct.py index 20641aefa..1ac5a49bd 100644 --- a/src/srtctl/frontends/sglang_direct.py +++ b/src/srtctl/frontends/sglang_direct.py @@ -44,7 +44,6 @@ def type(self) -> str: def worker_api_port(self, mode: str) -> Literal["public", "allocated"]: """The one ``sglang.launch_server`` is the endpoint, so it binds the public port.""" - del mode return "public" metrics_path: ClassVar[str] = "/metrics" @@ -56,16 +55,13 @@ def worker_metrics_port(self, process: Process, runtime: RuntimeContext) -> int return None def worker_endpoint_port(self, process: Process, config: Any, runtime: RuntimeContext) -> int | None: - del config return runtime.frontend_port if process.is_leader else None def profiling_control_port(self, process: Process, config: Any, runtime: RuntimeContext) -> int | None: """The leader's server on the public port carries the control routes; followers have none.""" - del config return runtime.frontend_port if process.is_leader else None def profiling_control_is_leader_only(self, config: Any) -> bool: - del config return False def direct_endpoint_nodes(self, processes: list[Process]) -> list[str]: @@ -76,11 +72,9 @@ def worker_ready_port(self, process: Process) -> int: def probe_ready(self, host: str, port: int, expected_prefill: int, expected_decode: int) -> WorkerHealthResult: """The worker's own /health, then /v1/models must list the model.""" - del expected_prefill, expected_decode return probe_direct_server(host, port) def health_expectations(self, config: Any, processes: list[Process] | None) -> tuple[int, int, str]: - del processes return logical_health_expectations(config) def validate(self, config: Any) -> None: @@ -134,7 +128,6 @@ def get_backend_health_urls( backend_processes: list[Process], network_interface: str | None = None, ) -> list[str]: - del backend, backend_processes, network_interface return [] def get_frontend_args_list(self, args: dict[str, Any] | None) -> list[str]: @@ -157,7 +150,6 @@ def start_frontends( backend_processes: list[Process], stop_event: threading.Event | None = None, ) -> list[ManagedProcess]: - del runtime, backend, backend_processes, stop_event if config.backend.type != "sglang": raise ValueError(f"frontend.type: sglang requires engine sglang (got {config.backend.type!r})") if topology.uses_nginx or len(topology.frontend_nodes) != 1: diff --git a/src/srtctl/frontends/static_router.py b/src/srtctl/frontends/static_router.py index e947314af..50c51ca88 100644 --- a/src/srtctl/frontends/static_router.py +++ b/src/srtctl/frontends/static_router.py @@ -56,38 +56,31 @@ def health_endpoint(self) -> str: def validate(self, config: Any) -> None: """Recipe-level rules beyond the backend pairing; none by default.""" - del config def worker_api_port(self, mode: str) -> Literal["public", "allocated"]: """A routed worker binds its own allocated port; the router owns the public one.""" - del mode return "allocated" metrics_path: ClassVar[str] = "/metrics" def worker_metrics_port(self, process: Process, runtime: RuntimeContext) -> int | None: """A native server's leader rank binds the HTTP server that carries /metrics; followers serve nothing.""" - del runtime if process.is_leader and process.http_port > 0: return process.http_port return None def worker_endpoint_port(self, process: Process, config: Any, runtime: RuntimeContext) -> int | None: - del config, runtime if process.is_leader and process.http_port > 0: return process.http_port return None def profiling_control_port(self, process: Process, config: Any, runtime: RuntimeContext) -> int | None: - del config, runtime return process.http_port if process.http_port > 0 else None def profiling_control_is_leader_only(self, config: Any) -> bool: - del config return False def direct_endpoint_nodes(self, processes: list[Process]) -> list[str]: - del processes return [] def worker_ready_port(self, process: Process) -> int: @@ -107,7 +100,6 @@ def probe_ready(self, host: str, port: int, expected_prefill: int, expected_deco def health_expectations(self, config: Any, processes: list[Process] | None) -> tuple[int, int, str]: """The registry lists one entry per logical worker unless the router expands them.""" - del processes return logical_health_expectations(config) def get_frontend_args_list(self, args: dict[str, Any] | None) -> list[str]: @@ -135,17 +127,14 @@ def get_managed_frontend_args( backend_processes: list[Process], ) -> list[str]: """Return adapter-managed CLI arguments derived from srtctl config.""" - del config, backend, backend_processes return [] def worker_scheme(self, backend: Any, mode: str) -> str: """Return the protocol used to reach one worker endpoint.""" - del backend, mode return "http" def worker_bootstrap_port(self, backend: Any, process: Process) -> int | None: """Return the optional P/D bootstrap port advertised for a worker.""" - del backend return process.bootstrap_port def resolve_worker_host(self, node: str, network_interface: str | None) -> str: @@ -158,7 +147,6 @@ def start_process(self, **kwargs: Any) -> Any: def build_bash_preamble(self, config: Any) -> str | None: """Return adapter-specific shell setup to run before the router.""" - del config return None def collect_workers( @@ -195,7 +183,6 @@ def get_backend_health_urls( network_interface: str | None = None, ) -> list[str]: """Return extra direct readiness requirements, if any.""" - del backend, backend_processes, network_interface return [] def build_router_command(self, workers: list[RouterWorker], host: str, port: int) -> list[str]: @@ -238,7 +225,6 @@ def start_frontends( backend_processes: list[Process], stop_event: threading.Event | None = None, ) -> list[ManagedProcess]: - del stop_event # Static routers return immediately after launch. from srtctl.core.processes import FRONTEND_TERMINATE_TIMEOUT_SECONDS, ManagedProcess configured_backend = getattr(getattr(config, "backend", None), "type", self.required_backend) diff --git a/src/srtctl/frontends/trtllm_serve.py b/src/srtctl/frontends/trtllm_serve.py index 5c39130ef..ffc050211 100644 --- a/src/srtctl/frontends/trtllm_serve.py +++ b/src/srtctl/frontends/trtllm_serve.py @@ -57,13 +57,11 @@ def worker_api_port(self, mode: str) -> Literal["public", "allocated"]: def worker_metrics_port(self, process: "Process", runtime: "RuntimeContext") -> int | None: """P/D leaders serve Prometheus on their OpenAI port; followers bind nothing. Aggregate is out of scope.""" - del runtime if process.endpoint_mode == "agg" or process.http_port <= 0: return None return process.http_port def worker_endpoint_port(self, process: "Process", config: Any, runtime: "RuntimeContext") -> int | None: - del config if not process.is_leader: return None port = runtime.frontend_port if self.worker_api_port(process.endpoint_mode) == "public" else process.http_port @@ -73,11 +71,9 @@ def profiling_control_port(self, process: "Process", config: Any, runtime: "Runt return self.worker_endpoint_port(process, config, runtime) def profiling_control_is_leader_only(self, config: Any) -> bool: - del config return False def direct_endpoint_nodes(self, processes: list["Process"]) -> list[str]: - del processes return [] def worker_ready_port(self, process: "Process") -> int: @@ -86,13 +82,11 @@ def worker_ready_port(self, process: "Process") -> int: def probe_ready(self, host: str, port: int, expected_prefill: int, expected_decode: int) -> WorkerHealthResult: """A 200 from /health is ready: the body may be empty, and every worker was gated before the orchestrator started.""" - del expected_prefill, expected_decode return probe_http_ok( host, port, self.health_endpoint, f"trtllm-serve frontend healthy at http://{host}:{port}/health" ) def health_expectations(self, config: Any, processes: list["Process"] | None) -> tuple[int, int, str]: - del processes return logical_health_expectations(config) def validate(self, config: Any) -> None: @@ -126,7 +120,6 @@ def get_backend_health_urls( backend_processes: list["Process"], network_interface: str | None = None, ) -> list[str]: - del backend, backend_processes, network_interface return [] def get_frontend_args_list(self, args: dict[str, Any] | None) -> list[str]: diff --git a/src/srtctl/frontends/vllm.py b/src/srtctl/frontends/vllm.py index 327edd95c..ed8a7839b 100644 --- a/src/srtctl/frontends/vllm.py +++ b/src/srtctl/frontends/vllm.py @@ -45,7 +45,6 @@ def type(self) -> str: def worker_api_port(self, mode: str) -> Literal["public", "allocated"]: """The one ``vllm serve`` is the endpoint, so it binds the public port in every mode it runs.""" - del mode return "public" metrics_path: ClassVar[str] = "/metrics" @@ -57,16 +56,13 @@ def worker_metrics_port(self, process: Process, runtime: RuntimeContext) -> int return None def worker_endpoint_port(self, process: Process, config: Any, runtime: RuntimeContext) -> int | None: - del config return runtime.frontend_port if process.is_leader else None def profiling_control_port(self, process: Process, config: Any, runtime: RuntimeContext) -> int | None: """One control server for the whole worker, on the public port.""" - del process, config return runtime.frontend_port def profiling_control_is_leader_only(self, config: Any) -> bool: - del config return True def direct_endpoint_nodes(self, processes: list[Process]) -> list[str]: @@ -77,11 +73,9 @@ def worker_ready_port(self, process: Process) -> int: def probe_ready(self, host: str, port: int, expected_prefill: int, expected_decode: int) -> WorkerHealthResult: """The worker's own /health, then /v1/models must list the model.""" - del expected_prefill, expected_decode return probe_direct_server(host, port) def health_expectations(self, config: Any, processes: list[Process] | None) -> tuple[int, int, str]: - del processes return logical_health_expectations(config) def validate(self, config: Any) -> None: @@ -126,7 +120,6 @@ def get_backend_health_urls( backend_processes: list[Process], network_interface: str | None = None, ) -> list[str]: - del backend, backend_processes, network_interface return [] def get_frontend_args_list(self, args: dict[str, Any] | None) -> list[str]: diff --git a/src/srtctl/frontends/vllm_router.py b/src/srtctl/frontends/vllm_router.py index a4ad974d5..7f5c42031 100644 --- a/src/srtctl/frontends/vllm_router.py +++ b/src/srtctl/frontends/vllm_router.py @@ -202,7 +202,6 @@ def get_managed_frontend_args( def worker_bootstrap_port(self, backend: Any, process: Process) -> int | None: """Advertise vLLM's NIXL side-channel port for P/D routing.""" - del backend return process.nixl_port def health_expectations(self, config: Any, processes: list[Process] | None) -> tuple[int, int, str]: @@ -224,10 +223,8 @@ def health_expectations(self, config: Any, processes: list[Process] | None) -> t def worker_metrics_port(self, process: Process, runtime: Any) -> int | None: """Every node-local hybrid-LB pool has its own API and /metrics; a positive http_port marks one.""" - del runtime return process.http_port if process.http_port > 0 else None def worker_endpoint_port(self, process: Process, config: Any, runtime: Any) -> int | None: """Router-facing pools are addressable whether or not they are the endpoint's leader rank.""" - del config, runtime return process.http_port if process.http_port > 0 else None From d376d8e4c2b80dcdec236f12d9a48dcc9bae9f9d Mon Sep 17 00:00:00 2001 From: Ishan Dhanani Date: Mon, 21 Sep 2026 18:08:48 -0700 Subject: [PATCH 7/7] refactor(frontends): slim the protocol to what consumers call; implied services and metrics listener on the frontend With probe_ready owning readiness, health_endpoint and parse_health were consulted only by the two bases' JSON probes and by a log line, and get_frontend_args_list only by the three start_frontends that launch a process. They leave the protocol: the bases keep them as their own hooks, the direct frontends drop their never-called stubs, the dead check_trtllm_serve_health parser goes, and the one verbatim-args implementation becomes frontend_args_to_cli. Two more decisions move onto the frontend. implied_services(config) returns the services a frontend brings along; Dynamo's etcd and NATS rule moves from services/implicit.py into frontends/dynamo.py, so a future registration-based frontend declares its own discovery plane. frontend_metrics_port(frontend_args) names a Prometheus listener separate from the routing port; the SGLang gateway's rule moves out of the telemetry stage. Two schema checks that re-derived frontend facts by name (profiler control on the leader only, Dynamo system ports) read profiling_control_is_leader_only and worker_launch instead. CLAUDE.md and docs/architecture.md describe the protocol as it is now. Dry-run output for all 26 example recipes is unchanged. Signed-off-by: Ishan Dhanani --- CLAUDE.md | 12 +- docs/architecture.md | 151 +++++++++++++---------- src/srtctl/cli/mixins/telemetry_stage.py | 10 +- src/srtctl/core/health.py | 29 +---- src/srtctl/core/schema.py | 16 ++- src/srtctl/frontends/base.py | 58 +++++---- src/srtctl/frontends/dynamic_frontend.py | 21 ++-- src/srtctl/frontends/dynamo.py | 39 +++++- src/srtctl/frontends/sglang.py | 4 + src/srtctl/frontends/sglang_direct.py | 35 +----- src/srtctl/frontends/static_router.py | 9 ++ src/srtctl/frontends/trtllm_serve.py | 40 ++---- src/srtctl/frontends/vllm.py | 35 +----- src/srtctl/services/implicit.py | 36 ++---- tests/test_frontends.py | 28 ++++- 15 files changed, 260 insertions(+), 263 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 9b138415d..c88756810 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -92,17 +92,15 @@ endpoints = allocate_endpoints( ### Health Checks -Two patterns for checking worker readiness: +Readiness is the frontend's: `probe_ready(host, port, expected_prefill, expected_decode)` performs one check against the public endpoint and returns a `WorkerHealthResult`, raising `requests.RequestException` while the endpoint is down. `wait_for_model` in `core/health.py` owns only timing, abort, and progress logging. The three probe shapes live in `core/health.py`: ```python -# Dynamo backend -check_dynamo_health(response_json, expected_prefill=2, expected_decode=4) - -# SGLang router -check_sglang_router_health(response_json, expected_prefill=2, expected_decode=4) +probe_json_health(host, port, "/workers", parse, n_prefill, n_decode) # a JSON worker count through parse (Dynamo /health, router /workers) +probe_http_ok(host, port, "/health", "ready message") # a bare 200 (trtllm-serve) +probe_direct_server(host, port) # /health, then /v1/models must list a model (direct vllm, sglang) ``` -For aggregated mode, pass `expected_prefill=0, expected_decode=num_agg`. +Expected counts come from the frontend too: `health_expectations(config, processes)` returns `(prefill, decode, description)` in the units its endpoint reports. Aggregate workers count as decode; Dynamo counts vLLM DP registrations, vLLM Router counts the ranks it expands each URL into, everything else counts logical workers. ### Frontends diff --git a/docs/architecture.md b/docs/architecture.md index 62370769a..d214bbc53 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -97,9 +97,9 @@ The `RuntimeContext` computes all paths and values **once** at startup. This eli ```python # All runtime values computed in one place runtime = RuntimeContext.from_config(config, job_id) -runtime.log_dir # /path/to/logs/12345/logs -runtime.head_node_ip # 10.0.0.1 -runtime.container_mounts # Dict[Path, Path] +runtime.log_dir # /path/to/logs/12345/logs +runtime.head_node_ip # 10.0.0.1 +runtime.container_mounts # Dict[Path, Path] ``` ### 2. Frozen Dataclasses @@ -144,8 +144,8 @@ Extensible component registration via decorators: ```python @register_benchmark("sa-bench") -class SABenchRunner(BenchmarkRunner): - ... +class SABenchRunner(BenchmarkRunner): ... + # Later: get_runner("sa-bench") returns instance runner = get_runner("sa-bench") @@ -360,18 +360,34 @@ src/srtctl/frontends/ #### FrontendProtocol +Implementations register with `@register_frontend("")` and are imported from the +package `__init__`; `frontend.type` resolves through that registry. Two bases carry the +shared behavior: `StaticRouterFrontend` (worker URLs on the CLI) and `DynamicFrontend` +(workers register themselves). The protocol is the set of questions the rest of srtctl asks: + ```python class FrontendProtocol(Protocol): - @property - def type(self) -> str: ... - - @property - def health_endpoint(self) -> str: ... - - def parse_health(response_json, expected_prefill, expected_decode) -> WorkerHealthResult: ... - + type: str + required_backend: str | None # pairing, checked at config load + worker_launch: Literal["dynamo", "direct"] # backends read this, never the name + expands_node_local_dp: bool + metrics_path: str + + def validate(self, config) -> None: ... # recipe rules (raise ValueError) + def worker_api_port(self, mode) -> Literal["public", "allocated"]: ... + def worker_metrics_port(self, process, runtime) -> int | None: ... + def worker_endpoint_port(self, process, config, runtime) -> int | None: ... + def profiling_control_port(self, process, config, runtime) -> int | None: ... + def profiling_control_is_leader_only(self, config) -> bool: ... + def direct_endpoint_nodes(self, processes) -> list[str]: ... + def worker_ready_port(self, process) -> int: ... + def health_expectations(self, config, processes) -> tuple[int, int, str]: ... + def probe_ready(self, host, port, expected_prefill, expected_decode) -> WorkerHealthResult: ... + def implied_services(self, config) -> list[EffectiveService]: ... + def frontend_metrics_port(self, frontend_args) -> int | None: ... + def get_backend_health_urls(self, backend, backend_processes, network_interface) -> list[str]: ... def start_frontends( - topology, runtime, config, backend, backend_processes + self, topology, runtime, config, backend, backend_processes, stop_event ) -> list[ManagedProcess]: ... ``` @@ -447,10 +463,10 @@ src/srtctl/core/ +----------------------------------------------------------------------------------------------+ | FRONTEND LAYER | +----------------------------------------------------------------------------------------------+ -| FrontendProtocol | DynamoFrontend | SGLangFrontend | TRTLLMServe | VLLMFrontend | +| FrontendProtocol | DynamoFrontend | SGLangRouter | TRTLLMServe | VLLMFrontend | | - start_frontends() | srun process | srun process | srun process | (no process) | -| - parse_health() | NATS/etcd | direct workers | /health | /health | -| - health_endpoint | /health | /workers | /health | agg leader node | +| - probe_ready() | /health JSON | /workers JSON | bare 200 | /health+models | +| - worker_launch | dynamo | direct | direct | direct | +----------------------------------------------------------------------------------------------+ | v @@ -604,26 +620,24 @@ SweepOrchestrator.run_benchmark() | v wait_for_model(host, port, n_prefill, n_decode, frontend_type) + | counts from frontend.health_expectations(config, processes) | - +-----> GET http://host:port/{health_endpoint} + +-----> frontend.probe_ready(host, port, n_prefill, n_decode) | | - | +------+------+ - | | | - | v v - | /health /workers - | (dynamo) (sglang) - | | | - | v v - | check_dynamo check_sglang - | _health() _router_health() - | | | - | v v - | WorkerHealthResult + | +------+-----------+----------------+ + | | | | + | v v v + | probe_json_health probe_http_ok probe_direct_server + | (dynamo /health, (trtllm-serve (direct vllm, sglang: + | routers /workers) bare 200) /health + /v1/models) + | | | | + | v v v + | WorkerHealthResult (RequestException while the endpoint is down) | - ready: bool | - prefill_ready vs expected | - decode_ready vs expected | - +<--- Loop until ready or timeout + +<--- Loop until ready or timeout (the loop owns timing, abort, logging) ``` --- @@ -736,7 +750,7 @@ class RuntimeContext: run_name: str # Node topology - nodes: Nodes # head, bench, worker tuple + nodes: Nodes # head, bench, worker tuple head_node_ip: str # Computed paths (all absolute) @@ -827,11 +841,11 @@ class ProcessRegistry: ```python @dataclass class ManagedProcess: - name: str # e.g., "prefill_0", "decode_1" + name: str # e.g., "prefill_0", "decode_1" popen: subprocess.Popen log_file: Path | None node: str | None - critical: bool = True # Failure triggers cleanup + critical: bool = True # Failure triggers cleanup @property def is_running(self) -> bool: ... @@ -895,40 +909,38 @@ BackendConfig = SGLangProtocol | MyBackendProtocol ### How to Add a New Frontend -1. **Create frontend module** at `frontends/myfrontend.py`: +Decide first whether it is a new process or a mode of an existing router (a discovery +flag, another connector); a mode is an override in the existing class. + +1. **Create frontend module** at `frontends/myrouter.py`, subclassing the base that matches + how it learns about workers, and register it: ```python -class MyFrontend: - @property - def type(self) -> str: - return "myfrontend" +from srtctl.frontends.base import register_frontend +from srtctl.frontends.static_router import StaticRouterFrontend # or DynamicFrontend - @property - def health_endpoint(self) -> str: - return "/health" - def parse_health(self, response_json, expected_prefill, expected_decode) -> WorkerHealthResult: - """Parse health check response.""" - ... +@register_frontend("myrouter") +class MyRouterFrontend(StaticRouterFrontend): + type = "myrouter" + required_backend = "vllm" + executable = ("myrouter",) + pd_flag = "--pd" + process_name = "myrouter" - def start_frontends(self, topology, runtime, config, backend, backend_processes) -> list[ManagedProcess]: - """Start frontend processes.""" - ... + def validate(self, config) -> None: + """Recipe rules; raise ValueError with the user-facing message.""" - def get_frontend_args_list(self, args: dict | None) -> list[str]: - """Convert args dict to CLI arguments.""" - ... + # Override only the hooks whose answer differs from the base: + # worker_bootstrap_port, build_router_command, probe_ready, health_expectations, + # worker_metrics_port, worker_endpoint_port, frontend_metrics_port, implied_services. ``` -2. **Register in `frontends/base.py`**: +2. **Import it from `frontends/__init__.py`**. That is the registration; the schema, + backends, telemetry, benchmark stage, and readiness loop need no edits. -```python -def get_frontend(frontend_type: str) -> FrontendProtocol: - if frontend_type == "myfrontend": - from srtctl.frontends.myfrontend import MyFrontend - return MyFrontend() - # ... existing frontends -``` +3. Add `tests/test_myrouter_frontend.py` (use `start_process` as the launch seam), a + `docs/myrouter.md` page, and an `examples/` recipe. ### How to Add a New Benchmark @@ -937,6 +949,7 @@ def get_frontend(frontend_type: str) -> FrontendProtocol: ```python from srtctl.benchmarks.base import BenchmarkRunner, register_benchmark + @register_benchmark("mybench") class MyBenchRunner(BenchmarkRunner): @property @@ -957,9 +970,12 @@ class MyBenchRunner(BenchmarkRunner): def build_command(self, config: SrtConfig, runtime: RuntimeContext) -> list[str]: """Build benchmark command.""" return [ - "python3", self.script_path, - "--host", runtime.nodes.head, - "--port", str(runtime.frontend_port), + "python3", + self.script_path, + "--host", + runtime.nodes.head, + "--port", + str(runtime.frontend_port), # ... other args ] ``` @@ -1023,18 +1039,17 @@ if TYPE_CHECKING: 2. **Lazy imports** - Import at function call time: ```python -def get_frontend(frontend_type: str) -> FrontendProtocol: - # Import here to avoid circular imports - from srtctl.frontends.dynamo import DynamoFrontend - from srtctl.frontends.sglang import SGLangFrontend +def _validate_frontend(self) -> None: + # Import here to avoid circular imports: frontends import core.schema. + from srtctl.frontends import get_frontend, list_frontend_types + ... ``` 3. **Forward references** - Use string annotations: ```python -def from_config(cls, config: "SrtConfig", job_id: str) -> "RuntimeContext": - ... +def from_config(cls, config: "SrtConfig", job_id: str) -> "RuntimeContext": ... ``` --- diff --git a/src/srtctl/cli/mixins/telemetry_stage.py b/src/srtctl/cli/mixins/telemetry_stage.py index f775c0764..c1c648a99 100644 --- a/src/srtctl/cli/mixins/telemetry_stage.py +++ b/src/srtctl/cli/mixins/telemetry_stage.py @@ -543,12 +543,12 @@ def _resolve_tachometer_binary(self, binary_path: str) -> str: return self._resolve_bundled_binary(binary_path) def _frontend_metrics_port(self) -> int | None: - """Frontends whose Prometheus listener is not the routing port: the SGLang Model Gateway.""" - if self.config.frontend.type == "sglang-router": - from srtctl.frontends.sglang import router_metrics_port + """Port of a frontend Prometheus listener separate from the routing port, if the frontend runs one.""" + from srtctl.frontends import FRONTEND_NONE, get_frontend - return router_metrics_port(self.config.frontend.args) - return None + if self.config.frontend.type == FRONTEND_NONE: + return None + return get_frontend(self.config.frontend.type).frontend_metrics_port(self.config.frontend.args) def _service_metrics_targets(self) -> list[ServiceMetricsTarget]: """One tachometer target per node for every service that serves metrics. diff --git a/src/srtctl/core/health.py b/src/srtctl/core/health.py index 8b824fb5c..264d54ff4 100644 --- a/src/srtctl/core/health.py +++ b/src/srtctl/core/health.py @@ -194,30 +194,6 @@ def check_dynamo_health( # ============================================================================ -def check_trtllm_serve_health( - response_json: dict, - expected_prefill: int, - expected_decode: int, -) -> WorkerHealthResult: - """Check trtllm-serve disaggregated health. - - trtllm-serve's /health returns HTTP 200 once the orchestrator is up (the body may - be empty). The trtllm_serve frontend already gates each worker for readiness before - starting the orchestrator, so a 200 here means the stack is ready. - - Note: TRTLLMServeFrontend.probe_ready treats the 200 as ready without parsing a - body, so this exists mainly as the FrontendProtocol.parse_health hook. - """ - return WorkerHealthResult( - ready=True, - message="trtllm-serve orchestrator healthy", - prefill_ready=expected_prefill, - prefill_expected=expected_prefill, - decode_ready=expected_decode, - decode_expected=expected_decode, - ) - - def check_vllm_health( host: str, port: int, @@ -532,14 +508,13 @@ def wait_for_model( frontend = get_frontend(frontend_type) logger.info( - "Polling http://%s:%d%s every %.1fs for %d prefills and %d decodes (%s frontend)", + "Polling %s readiness at http://%s:%d every %.1fs for %d prefills and %d decodes", + frontend_type, host, port, - frontend.health_endpoint, poll_interval, n_prefill, n_decode, - frontend_type, ) start_time = time.time() diff --git a/src/srtctl/core/schema.py b/src/srtctl/core/schema.py index 02446ba7f..d4852f6ec 100755 --- a/src/srtctl/core/schema.py +++ b/src/srtctl/core/schema.py @@ -2858,9 +2858,7 @@ def _validate_profiling(self): f"profiling.{phase_name}.worker_rank={phase_config.worker_rank} is not a physical " f"process rank for this worker layout; valid ranks: {ranks}" ) - if ( - self.frontend.type == "vllm" or (self.frontend.type == "dynamo" and self.dynamo.sidecar) - ) and phase_config.worker_rank != 0: + if phase_config.worker_rank != 0 and self._frontend_profiling_control_is_leader_only(): raise ValidationError( f"profiling.{phase_name}.worker_rank={phase_config.worker_rank} has no independent " "control endpoint; direct vLLM and Dynamo sidecar profiling must select rank 0" @@ -2951,9 +2949,19 @@ def _validate_dcgm_power(self): if not concurrencies or len(set(concurrencies)) != len(concurrencies) or any(c <= 0 for c in concurrencies): raise ValidationError("telemetry requires a non-empty list of unique positive benchmark.concurrencies") + def _frontend_profiling_control_is_leader_only(self) -> bool: + """Whether the frontend's workers expose one profiler control server per logical endpoint.""" + if self.frontend.type == "none": + return False + from srtctl.frontends import get_frontend + + return get_frontend(self.frontend.type).profiling_control_is_leader_only(self) + def _dynamo_system_ports(self) -> set[int]: """System-status ports that backend launches actually bind on worker nodes.""" - if self.frontend.type != "dynamo": + from srtctl.frontends import get_frontend + + if self.frontend.type == "none" or get_frontend(self.frontend.type).worker_launch != "dynamo": return set() resources = self.resources diff --git a/src/srtctl/frontends/base.py b/src/srtctl/frontends/base.py index 8a88db4df..6c115df1d 100644 --- a/src/srtctl/frontends/base.py +++ b/src/srtctl/frontends/base.py @@ -2,12 +2,13 @@ # SPDX-License-Identifier: Apache-2.0 """ -Base types and protocols for frontend configurations. +The frontend protocol and registry. -Frontend types handle: -- Starting router/frontend processes -- Health checking with appropriate endpoints -- Building CLI arguments from config +A frontend owns everything the rest of srtctl needs to know about the router +(or the direct server that stands in for one): which backend it pairs with, +its recipe rules, how its workers are launched and which ports they bind, +which rank serves metrics or an endpoint, how readiness is probed and counted, +the services it implies, and how its process starts. """ import threading @@ -18,6 +19,7 @@ from srtctl.core.processes import ManagedProcess from srtctl.core.runtime import RuntimeContext from srtctl.core.topology import Process + from srtctl.services.implicit import EffectiveService # ``frontend.type: none`` is a services-only job: no router process, no OpenAI # endpoint, no worker-count health gate (see SrtConfig._validate_services_only). @@ -29,11 +31,18 @@ class FrontendProtocol(Protocol): """Protocol that all frontend implementations must implement. - Each frontend is responsible for: - 1. Starting router/frontend processes on designated nodes - 2. Providing health check endpoint and response parsing - 3. Building CLI arguments from config - 4. Its own recipe-level rules (``required_backend``, ``validate``) + Each frontend answers, for the rest of srtctl: + 1. Which backend it pairs with and what its recipe rules are + (``required_backend``, ``validate``) + 2. How its workers are launched and which port each binds + (``worker_launch``, ``worker_api_port``, ``expands_node_local_dp``) + 3. Which rank serves metrics, an endpoint, or profiler control, and where + (``worker_metrics_port``, ``worker_endpoint_port``, ``profiling_control_port``, + ``direct_endpoint_nodes``, ``worker_ready_port``, ``metrics_path``) + 4. How readiness is probed and counted (``probe_ready``, ``health_expectations``, + ``get_backend_health_urls``) + 5. What it brings along (``implied_services``, ``frontend_metrics_port``) + 6. How its process starts (``start_frontends``) An implementation registers with ``@register_frontend("")``; the recipe's ``frontend.type`` is resolved through that registry and nowhere @@ -142,18 +151,12 @@ def validate(self, config: Any) -> None: """ ... - @property - def health_endpoint(self) -> str: - """HTTP endpoint for health checks (e.g., '/health', '/workers').""" + def implied_services(self, config: Any) -> list["EffectiveService"]: + """Services this frontend needs that the recipe did not name (Dynamo: its discovery plane).""" ... - def parse_health( - self, - response_json: dict, - expected_prefill: int, - expected_decode: int, - ) -> "WorkerHealthResult": - """Parse health check response and return worker status.""" + def frontend_metrics_port(self, frontend_args: dict[str, Any] | None) -> int | None: + """Port of a Prometheus listener separate from the routing port, or ``None`` when metrics share it.""" ... def get_backend_health_urls( @@ -190,9 +193,18 @@ def start_frontends( """ ... - def get_frontend_args_list(self, args: dict[str, Any] | None) -> list[str]: - """Convert frontend args dict to CLI argument list.""" - ... + +def frontend_args_to_cli(args: dict[str, Any] | None) -> list[str]: + """``frontend.args`` as CLI flags with keys verbatim: ``True`` is a bare flag, ``False``/``None`` are dropped.""" + if not args: + return [] + result: list[str] = [] + for key, value in args.items(): + if value is True: + result.append(f"--{key}") + elif value is not False and value is not None: + result.extend([f"--{key}", str(value)]) + return result def logical_health_expectations(config: Any) -> tuple[int, int, str]: diff --git a/src/srtctl/frontends/dynamic_frontend.py b/src/srtctl/frontends/dynamic_frontend.py index 92c87194d..dea7c8991 100644 --- a/src/srtctl/frontends/dynamic_frontend.py +++ b/src/srtctl/frontends/dynamic_frontend.py @@ -26,10 +26,11 @@ from typing import TYPE_CHECKING, Any, ClassVar, Literal from srtctl.core.health import WorkerHealthResult, probe_json_health -from srtctl.frontends.base import logical_health_expectations +from srtctl.frontends.base import frontend_args_to_cli, logical_health_expectations if TYPE_CHECKING: from srtctl.core.topology import Process + from srtctl.services.implicit import EffectiveService class DynamicFrontend: @@ -78,14 +79,14 @@ def direct_endpoint_nodes(self, processes: list[Process]) -> list[str]: """The frontend process is the endpoint, never a worker.""" return [] + def implied_services(self, config: Any) -> list[EffectiveService]: + """The discovery plane the frontend needs; none unless the implementation brings one.""" + return [] + + def frontend_metrics_port(self, frontend_args: dict[str, Any] | None) -> int | None: + """Metrics share the routing port unless the implementation runs a separate listener.""" + return None + def get_frontend_args_list(self, args: dict[str, Any] | None) -> list[str]: """Convert ``frontend.args`` to CLI flags, keys verbatim.""" - if not args: - return [] - result: list[str] = [] - for key, value in args.items(): - if value is True: - result.append(f"--{key}") - elif value is not False and value is not None: - result.extend([f"--{key}", str(value)]) - return result + return frontend_args_to_cli(args) diff --git a/src/srtctl/frontends/dynamo.py b/src/srtctl/frontends/dynamo.py index 05f1682e0..8dc3c9ef1 100644 --- a/src/srtctl/frontends/dynamo.py +++ b/src/srtctl/frontends/dynamo.py @@ -20,7 +20,15 @@ from srtctl.core.slurm import CONTAINER_REMAP_ROOT_EXPORT, start_srun_process from srtctl.frontends.base import logical_health_expectations, register_frontend from srtctl.frontends.dynamic_frontend import DynamicFrontend -from srtctl.services.implicit import discovery_env +from srtctl.services.config import ServiceConfig +from srtctl.services.implicit import ( + ETCD_SERVICE_NAME, + NATS_SERVICE_NAME, + EffectiveService, + discovery_env, + infra_placement, + nats_implied_reasons, +) if TYPE_CHECKING: from srtctl.core.processes import ManagedProcess @@ -126,6 +134,35 @@ def worker_ready_port(self, process: "Process") -> int: """DYN_SYSTEM_PORT: the per-worker axum server reports /health once registered.""" return process.sys_port + def implied_services(self, config: Any) -> list[EffectiveService]: + """The discovery plane: etcd always, NATS only when a Dynamo plane rides on it. + + The default request plane is tcp and KV events go over direct ZMQ, so a + plain Dynamo job runs etcd alone; a recipe declares a ``nats`` service to + force one anyway. + """ + placement = infra_placement(config) + implied = [ + EffectiveService( + ServiceConfig(name=ETCD_SERVICE_NAME, type="etcd", placement=placement), + implicit=True, + reason="frontend.type dynamo", + ) + ] + nats_reasons = nats_implied_reasons(config) + if nats_reasons: + nats_options = {} + if config.infra.nats_max_payload_mb is not None: + nats_options["max_payload_mb"] = config.infra.nats_max_payload_mb + implied.append( + EffectiveService( + ServiceConfig(name=NATS_SERVICE_NAME, type="nats", placement=placement, options=nats_options), + implicit=True, + reason=", ".join(nats_reasons), + ) + ) + return implied + def parse_health( self, response_json: dict, diff --git a/src/srtctl/frontends/sglang.py b/src/srtctl/frontends/sglang.py index 2ed763b2a..be010ed13 100644 --- a/src/srtctl/frontends/sglang.py +++ b/src/srtctl/frontends/sglang.py @@ -38,6 +38,10 @@ class SGLangRouterFrontend(StaticRouterFrontend): # that construct the frontend before populating worker processes. allow_empty_workers: ClassVar[bool] = True + def frontend_metrics_port(self, frontend_args: dict[str, Any] | None) -> int | None: + """The gateway serves Prometheus on its own listener, not on the routing port.""" + return router_metrics_port(frontend_args) + def get_managed_frontend_args( self, config: Any, diff --git a/src/srtctl/frontends/sglang_direct.py b/src/srtctl/frontends/sglang_direct.py index 1ac5a49bd..3aead6014 100644 --- a/src/srtctl/frontends/sglang_direct.py +++ b/src/srtctl/frontends/sglang_direct.py @@ -22,6 +22,7 @@ from srtctl.core.processes import ManagedProcess from srtctl.core.runtime import RuntimeContext from srtctl.core.topology import Process + from srtctl.services.implicit import EffectiveService logger = logging.getLogger(__name__) @@ -103,25 +104,6 @@ def validate(self, config: Any) -> None: if config.dynamo.sidecar: raise ValueError("frontend.type: sglang does not support dynamo.sidecar; use frontend.type: dynamo") - @property - def health_endpoint(self) -> str: - return "/health" - - def parse_health( - self, - response_json: dict, - expected_prefill: int, - expected_decode: int, - ) -> WorkerHealthResult: - return WorkerHealthResult( - ready=True, - message="SGLang OpenAI server healthy", - prefill_ready=expected_prefill, - prefill_expected=expected_prefill, - decode_ready=expected_decode, - decode_expected=expected_decode, - ) - def get_backend_health_urls( self, backend: Any, @@ -130,16 +112,11 @@ def get_backend_health_urls( ) -> list[str]: return [] - def get_frontend_args_list(self, args: dict[str, Any] | None) -> list[str]: - if not args: - return [] - result = [] - for key, value in args.items(): - if value is True: - result.append(f"--{key}") - elif value is not False and value is not None: - result.extend([f"--{key}", str(value)]) - return result + def implied_services(self, config: Any) -> list[EffectiveService]: + return [] + + def frontend_metrics_port(self, frontend_args: dict[str, Any] | None) -> int | None: + return None def start_frontends( self, diff --git a/src/srtctl/frontends/static_router.py b/src/srtctl/frontends/static_router.py index 50c51ca88..34e48e95e 100644 --- a/src/srtctl/frontends/static_router.py +++ b/src/srtctl/frontends/static_router.py @@ -19,6 +19,7 @@ from srtctl.core.processes import ManagedProcess from srtctl.core.runtime import RuntimeContext from srtctl.core.topology import Process + from srtctl.services.implicit import EffectiveService logger = logging.getLogger(__name__) @@ -102,6 +103,14 @@ def health_expectations(self, config: Any, processes: list[Process] | None) -> t """The registry lists one entry per logical worker unless the router expands them.""" return logical_health_expectations(config) + def implied_services(self, config: Any) -> list[EffectiveService]: + """A static router needs no discovery plane.""" + return [] + + def frontend_metrics_port(self, frontend_args: dict[str, Any] | None) -> int | None: + """Metrics share the routing port unless the router runs a separate listener.""" + return None + def get_frontend_args_list(self, args: dict[str, Any] | None) -> list[str]: """Convert config values to CLI arguments, preserving repeated values.""" if not args: diff --git a/src/srtctl/frontends/trtllm_serve.py b/src/srtctl/frontends/trtllm_serve.py index ffc050211..c3c7a194b 100644 --- a/src/srtctl/frontends/trtllm_serve.py +++ b/src/srtctl/frontends/trtllm_serve.py @@ -18,14 +18,15 @@ import yaml -from srtctl.core.health import WorkerHealthResult, check_trtllm_serve_health, probe_http_ok, wait_for_health +from srtctl.core.health import WorkerHealthResult, probe_http_ok, wait_for_health from srtctl.core.slurm import get_hostname_ip, start_srun_process -from srtctl.frontends.base import logical_health_expectations, register_frontend +from srtctl.frontends.base import frontend_args_to_cli, logical_health_expectations, register_frontend if TYPE_CHECKING: from srtctl.core.processes import ManagedProcess from srtctl.core.runtime import RuntimeContext from srtctl.core.topology import Process + from srtctl.services.implicit import EffectiveService logger = logging.getLogger(__name__) @@ -82,9 +83,7 @@ def worker_ready_port(self, process: "Process") -> int: def probe_ready(self, host: str, port: int, expected_prefill: int, expected_decode: int) -> WorkerHealthResult: """A 200 from /health is ready: the body may be empty, and every worker was gated before the orchestrator started.""" - return probe_http_ok( - host, port, self.health_endpoint, f"trtllm-serve frontend healthy at http://{host}:{port}/health" - ) + return probe_http_ok(host, port, "/health", f"trtllm-serve frontend healthy at http://{host}:{port}/health") def health_expectations(self, config: Any, processes: list["Process"] | None) -> tuple[int, int, str]: return logical_health_expectations(config) @@ -101,19 +100,6 @@ def validate(self, config: Any) -> None: "aggregate worker (set resources.agg_workers: 1)" ) - @property - def health_endpoint(self) -> str: - return "/health" - - def parse_health( - self, - response_json: dict, - expected_prefill: int, - expected_decode: int, - ) -> WorkerHealthResult: - """Parse trtllm-serve /health response (200 => ready).""" - return check_trtllm_serve_health(response_json, expected_prefill, expected_decode) - def get_backend_health_urls( self, backend: Any, @@ -122,17 +108,11 @@ def get_backend_health_urls( ) -> list[str]: return [] - def get_frontend_args_list(self, args: dict[str, Any] | None) -> list[str]: - """Convert frontend args dict to CLI arguments.""" - if not args: - return [] - result = [] - for key, value in args.items(): - if value is True: - result.append(f"--{key}") - elif value is not False and value is not None: - result.extend([f"--{key}", str(value)]) - return result + def implied_services(self, config: Any) -> list["EffectiveService"]: + return [] + + def frontend_metrics_port(self, frontend_args: dict[str, Any] | None) -> int | None: + return None @staticmethod def _build_ser(config: Any, prefill_urls: list[str], decode_urls: list[str], port: int) -> dict[str, Any]: @@ -239,7 +219,7 @@ def start_frontends( container_ser_path = "/logs/ser.yaml" cmd = ["trtllm-serve", "disaggregated", "--config", container_ser_path] - cmd.extend(self.get_frontend_args_list(config.frontend.args)) + cmd.extend(frontend_args_to_cli(config.frontend.args)) logger.info("Orchestrator command: %s", shlex.join(cmd)) env_to_set: dict[str, str] = {} diff --git a/src/srtctl/frontends/vllm.py b/src/srtctl/frontends/vllm.py index ed8a7839b..7f0339b71 100644 --- a/src/srtctl/frontends/vllm.py +++ b/src/srtctl/frontends/vllm.py @@ -21,6 +21,7 @@ from srtctl.core.processes import ManagedProcess from srtctl.core.runtime import RuntimeContext from srtctl.core.topology import Process + from srtctl.services.implicit import EffectiveService logger = logging.getLogger(__name__) @@ -95,25 +96,6 @@ def validate(self, config: Any) -> None: "worker across nodes with resources.agg_nodes." ) - @property - def health_endpoint(self) -> str: - return "/health" - - def parse_health( - self, - response_json: dict, - expected_prefill: int, - expected_decode: int, - ) -> WorkerHealthResult: - return WorkerHealthResult( - ready=True, - message="vLLM OpenAI server healthy", - prefill_ready=expected_prefill, - prefill_expected=expected_prefill, - decode_ready=expected_decode, - decode_expected=expected_decode, - ) - def get_backend_health_urls( self, backend: Any, @@ -122,16 +104,11 @@ def get_backend_health_urls( ) -> list[str]: return [] - def get_frontend_args_list(self, args: dict[str, Any] | None) -> list[str]: - if not args: - return [] - result = [] - for key, value in args.items(): - if value is True: - result.append(f"--{key}") - elif value is not False and value is not None: - result.extend([f"--{key}", str(value)]) - return result + def implied_services(self, config: Any) -> list[EffectiveService]: + return [] + + def frontend_metrics_port(self, frontend_args: dict[str, Any] | None) -> int | None: + return None def start_frontends( self, diff --git a/src/srtctl/services/implicit.py b/src/srtctl/services/implicit.py index cac81e270..2d615fab6 100644 --- a/src/srtctl/services/implicit.py +++ b/src/srtctl/services/implicit.py @@ -42,7 +42,8 @@ class EffectiveService: reason: str = "" -def _infra_placement(config: SrtConfig) -> ServicePlacementConfig: +def infra_placement(config: SrtConfig) -> ServicePlacementConfig: + """Where the discovery plane and other infra services run: the infra node, or a dedicated one.""" return ServicePlacementConfig(node="dedicated" if config.infra.etcd_nats_dedicated_node else "infra") @@ -82,30 +83,13 @@ def implied_services(config: SrtConfig) -> list[EffectiveService]: """Services the rest of the recipe asks for without naming them.""" implied: list[EffectiveService] = [] - if config.frontend.type == "dynamo": - placement = _infra_placement(config) - implied.append( - EffectiveService( - ServiceConfig(name=ETCD_SERVICE_NAME, type="etcd", placement=placement), - implicit=True, - reason="frontend.type dynamo", - ) - ) - # NATS is only a dependency when a plane actually rides on it. The default - # request plane is tcp and KV events go over direct ZMQ, so a plain Dynamo - # job runs etcd alone; declare a `nats` service to force one anyway. - nats_reasons = nats_implied_reasons(config) - if nats_reasons: - nats_options = {} - if config.infra.nats_max_payload_mb is not None: - nats_options["max_payload_mb"] = config.infra.nats_max_payload_mb - implied.append( - EffectiveService( - ServiceConfig(name=NATS_SERVICE_NAME, type="nats", placement=placement, options=nats_options), - implicit=True, - reason=", ".join(nats_reasons), - ) - ) + # The frontend brings its own discovery plane (Dynamo: etcd, and NATS when a + # plane rides on it); a services-only job has no frontend. Imported lazily: + # the frontend implementations import this module. + from srtctl.frontends import FRONTEND_NONE, get_frontend + + if config.frontend.type != FRONTEND_NONE: + implied.extend(get_frontend(config.frontend.type).implied_services(config)) if getattr(config.backend, "failover", None) is not None: # The kind's defaults are the placement: every worker node, one instance per worker. @@ -129,7 +113,7 @@ def implied_services(config: SrtConfig) -> list[EffectiveService]: type="mooncake-master", container=mooncake_cfg.container, args=list(mooncake_cfg.master_extra_args or []), - placement=_infra_placement(config), + placement=infra_placement(config), options=options, ), implicit=True, diff --git a/tests/test_frontends.py b/tests/test_frontends.py index 5a01f4384..b007f5aa2 100644 --- a/tests/test_frontends.py +++ b/tests/test_frontends.py @@ -268,10 +268,30 @@ def test_sglang_health_endpoint(self): frontend = SGLangRouterFrontend() assert frontend.health_endpoint == "/workers" - def test_vllm_health_endpoint(self): - """VLLMFrontend uses /health endpoint.""" - frontend = VLLMFrontend() - assert frontend.health_endpoint == "/health" + def test_frontend_metrics_port_and_implied_services(self): + """Only the SGLang gateway runs a separate metrics listener; only Dynamo brings a discovery plane.""" + from types import SimpleNamespace + + from srtctl.ports import SGLANG_ROUTER_METRICS_PORT + + gateway = SGLangRouterFrontend() + assert gateway.frontend_metrics_port(None) == SGLANG_ROUTER_METRICS_PORT + assert gateway.frontend_metrics_port({"prometheus-port": 31000}) == 31000 + for frontend_type in ("dynamo", "vllm", "sglang", "trtllm_serve", "vllm-router"): + assert get_frontend(frontend_type).frontend_metrics_port({"prometheus-port": 31000}) is None + + dynamo_config = SimpleNamespace( + frontend=SimpleNamespace(type="dynamo"), + dynamo=SimpleNamespace(request_plane="nats", event_plane="zmq"), + infra=SimpleNamespace(etcd_nats_dedicated_node=False, nats_max_payload_mb=None), + ) + implied = get_frontend("dynamo").implied_services(dynamo_config) + assert [(entry.service.name, entry.service.type, entry.reason) for entry in implied] == [ + ("etcd", "etcd", "frontend.type dynamo"), + ("nats", "nats", "dynamo.request_plane nats"), + ] + for frontend_type in ("vllm", "sglang", "sglang-router", "trtllm_serve", "vllm-router"): + assert get_frontend(frontend_type).implied_services(dynamo_config) == [] # ============================================================================