[atchernych]

Summary

Before this PR the POST /pools answered with HTTP 501 "this service takes no registrations". This was ok for the original K8 design but the problem is that we also need to communicate the model from the pools to the Global Router.

Note that model info cannot be communicated to the Global Discovery service through our custom mcs implementation either. The controller copies a Service's dynamo.nvidia.com/mcs-* annotations to the other clusters, so a model annotation could ride along. But the annotations land on the ServiceImport, not on the EndpointSlices the discovery service reads. This would be a less robust approach because: the service would need a second watch; someone has to write the annotations manually. The PoolRelay already knows it.

This PR is an alternative to #17276. Both make MCS pools routable by giving each pool a model and live frontend addresses. #17276 reads the model from a hand-written --routing-config file. This PR gets it from the PoolRelay itself, through POST /pools, as the design says.

Stacked on #17371.

Part of #17600, the pool discovery DEP.

How it works

• With MCS pool discovery (--pool-discovery karmada or mcs), POST /pools now works (it returned 501). Each call is the pool's heartbeat, as with --pool-discovery registrations.
• So every --pool-discovery value takes registrations: the lease flags and --registration-storage apply to all of them, while the frontend flags work only with karmada or mcs.
• A pool is listed only when both sides agree: MCS shows a Ready PoolRelay, and the pool registered within the lease (15 s by default).
• MCS gives the addresses: the Relay's, and the frontends' from the <dgd>-frontend Service, the same way as feat(global-router): resolve MCS pool models and live frontends #17276. Each frontend pod is listed once, at its IPv4 address on a dual-stack cluster. The registration gives the model, capabilities, capacity and location.
• One discovery instance keeps the registrations it receives, so run one, and have every PoolRelay register with it. Sharing registrations between the instances in every cluster (--peer-service) moved to feat(global-router): share registrations between discovery instances #17637, stacked on this PR, since shared storage such as DynamoDB may replace it.

Problems with the file in #17276

1. The model is typed twice: in the pool's deployment and in the file. If they differ, requests go to a pool that doesn't serve that model.
2. The same file goes to every cluster. Adding a pool or changing a model means editing it, applying it everywhere, and restarting every discovery service.
3. MCS pools get no capacity or capabilities. So every MCS pool in feat(global-router): resolve MCS pool models and live frontends #17276 shows "capabilities": [] and "capacity": []. In this PR, both come from the PoolRelay's registration. Today's round-robin router doesn't use either field yet. They matter for later work, such as load-aware routing (how big is each pool) or sending requests only to pools that can handle them.
4. In MCS mode, POST /pools still returns 501, but the design has PoolRelays register in every mode. A PoolRelay pointed at an MCS-mode service would get an error on every heartbeat.
5. A pool is live only by MCS readiness. If a cluster loses contact with the MCS implementation, its pools can stay Ready after they are gone, until routers notice that the PoolRelay's connection is gone (see the comment above ready_addresses in hub.rs).

What this PR adds

• No routing file. New pools and model changes need no config change or restart.
• Capacity and capabilities in MCS mode.
• One PoolRelay contract for every --pool-discovery value.
• Missed heartbeats drop a pool even when MCS still shows it Ready.
• A warning when MCS and the registrations disagree for two lease durations, for example when cluster_id isn't the pool's MCS cluster name. Today such a pool is silently missing.
• A warning when a live pool has no Ready frontend, for example when <dgd>-frontend isn't exported. Such a pool is listed with frontends: [], so routers have nowhere to send its requests.

Trade-offs

• A pool appears only once one of its PoolRelays calls POST /pools, and no PoolRelay does yet. Until then, register a pool with curl, as feat(global-router): route round robin over pools from discovery #17273 did.
• In MCS mode, GET /pools says complete: false for one lease after the service starts, while PoolRelays may not have registered yet, and while the MCS watch keeps failing. A caller should then merge rather than replace: keep known pools that are missing, and remove pools only on a complete list. The GET /pools docs in service.rs say this, and so do the library's contract docs under "Partial lists" (feat(global-router): add the Global Discovery Service library (contract and backends) #17331).
• One instance holds every registration in memory, so a restart forgets them. Pools reappear as their PoolRelays heartbeat again, within one heartbeat interval, and GET /pools says complete: false until a lease has passed. feat(global-router): resolve MCS pool models and live frontends #17276 doesn't have this, because MCS alone keeps those pools listed. Registration storage that every region reads, such as a later --registration-storage on DynamoDB global tables, would remove it, and would replace feat(global-router): share registrations between discovery instances #17637 too.

To deploy, each pool also exports <dgd>-frontend, as in #17276.

Validation

• cargo test -p dynamo-global-discovery-service: 73 passed (61 library, 12 binary); the binary's flag tests also cover the frontend flags. They cover:
  • the join: only pools both sides list, which side each field comes from, watch events from both sides, a lapsed registration dropping a pool that MCS still lists, one-sided pools (one warning each, timed from when the pool became one-sided, and forgotten after a relist), a relist that drops a joined pool, watching both sides again when a watch cannot start or ends (watchers keep the last view, marked Estimated, until both sides list again, and the backoff then starts over), and its completeness, Exact only while both sides are;
  • frontends from MCS: frontend-only pools stay hidden, IPv6, a dual-stack pod listed once at its IPv4 address (also when its slices do not name it), only the named TCP port (and not port 0), a deleted frontend slice leaving a live pool without frontends, one warning per such pool, also across the reads before the first sync, and the check that the two Service suffixes differ;
• Before the split, breaking each of 32 guarded behaviors on purpose made a test fail, for example one address family for frontends and the join's completeness. The peer ones (the self-skip, the readiness gate, the peer client's timeout, missing copies) moved to feat(global-router): share registrations between discovery instances #17637 with their tests. One bound has no test: the sweep runs at least once a second.
• cargo clippy --all-targets -- -D warnings and cargo fmt on the crate: clean.
• feat(global-router): add the Global Discovery Service library (contract and backends) #17331's and feat(global-router): add the Global Discovery HTTP service #17371's latest changes are merged in, and the frontend flags get the same checks as feat(global-router): add the Global Discovery HTTP service #17371's router flags: a suffix that starts with - goes after =, a suffix left without a value fails instead of taking the next flag, and no flag can be empty.
• A second independent review found that the join always said Estimated, that a dual-stack frontend pod was listed twice, that a misnamed frontend port dropped a pool's frontends without a log line, and that the manifest's comments predated registrations. A third found that the join could then say Exact while another instance's registrations were missing, and that slices that do not name their pods still listed a dual-stack frontend twice. All are fixed.
• A later review found four smaller issues, all fixed: overlapping frontend and PoolRelay suffixes were refused only after the Kubernetes client started, with exit code 1; a dual-stack pool's frontends followed whichever address family was Ready first; a frontend slice without the named port was warned about on every event; and the join's sweep test passed with any sweep period.
• A review on the PR found that a source that could not be watched at startup would stop the join for good, and asked for a warning when a live pool has no frontend, for a warning text that gives the expected cause first, and for tests of a relist that drops a joined pool and of a deleted frontend slice. All are done: the join now watches both sides again after a backoff. An independent review of that fix found that the backoff never started over, that watchers kept a view marked Exact while the join watched the sources again, and that each read before the first sync repeated the frontend warning. Those are fixed too. Breaking each of the eight new guards on purpose made its test fail.
• Not yet run on a cluster.

🤖 Generated with Claude Code

Summary by CodeRabbit

• New Features
  • Added a Global Discovery Service that provides pool and router discovery information, health checks, and optional secure connections.
  • Pool discovery can combine active registrations with Kubernetes service endpoint data.
  • Added configurable HTTP server support for TLS, request limits, connection timeouts, and graceful shutdown, helping services handle traffic and deployments more reliably.
  • Added frontend load and readiness reporting, including request activity and model status.

[devin-ai-integration]

❌ Dynamo PR CI failed — run 41559139567 (attempt 1) on 451d353046

Gate checks: ❌ backend-status-check · ✅ deploy-status-check · ✅ dynamo-status-check

Framework	1-GPU arm64
vLLM	❌ 1

Failure details

1 job failed: both CPU pytest stages of the vLLM arm64 test job died right after pytest started, with the self-hosted runner's container hook error and no test results. This looks like infrastructure; rerunning the failed job is likely enough.

❌ vllm-runtime / Test cuda13.0, arm64: runner container hook failed ("Executing the custom container implementation failed")

Job: vllm-runtime / Test cuda13.0, arm64 · Failed step: Run CPU-only tests (parallelized) · Logs: gh run view --job 125680383412 -R ai-dynamo/dynamo --log-failed

▶️ Executing (cwd=/workspace): python -m pytest -n auto ... -m pre_merge and vllm and gpu_0 and not e2e
[Datadog Test Optimization] WARNING ... rerunfailures is installed alongside Datadog Test Optimization ...
##[error]Executing the custom container implementation failed. Please contact your self hosted runner administrator.
No files were found with the provided path: test-results/allure-results/. No artifacts will be uploaded.
▶️ Executing (cwd=/workspace): python -m pytest -n 0 ... -m pre_merge and vllm and gpu_0 and e2e
##[error]Executing the custom container implementation failed. Please contact your self hosted runner administrator.
...
❌ pre_merge_cpu: step outcome 'failure'
❌ pre_merge_cpu_e2e: step outcome 'failure'
##[error]A pytest stage did not record a passing result. See this step's log for the stage.

Neither pre_merge_cpu nor pre_merge_cpu_e2e printed a pytest result or test failure. Both ended with the runner's Kubernetes container hook error, and neither wrote allure results. That is an infrastructure signature (the runner or container exec failed), not a test failure, so a rerun of failed jobs is likely enough.

For agents

{"pr": 17332, "run_id": 41559139567, "run_attempt": 1, "head_sha": "451d3530462c824f8558866a69fcc7131f926524",
 "failures": [{"job": "vllm-runtime / Test cuda13.0, arm64", "job_id": 125680383412, "failed_step": "Run CPU-only tests (parallelized)", "signature": "Executing the custom container implementation failed (runner container hook, infra)", "tests": [], "log_cmd": "gh run view --job 125680383412 -R ai-dynamo/dynamo --log-failed"}]}

_{Posted automatically by Devin for run 41559139567. Updated on every full-CI run of this PR.}

[estroz]

lgtm

couple of questions:

• frontend_url is always http:// (hub.rs:200). is tls on the frontend out of scope for v1, or should the scheme come from the port's appProtocol?
• where do poolrelays get the mcs cluster name that cluster_id has to match?

[atchernych]

Thanks for the review, Eric. The five inline comments are addressed in 2ba2b8b, with a reply in each thread. On your two questions:

TLS to the frontends. Out of scope for V1, which assumes a trusted network, as the ip: relay identities do. When we add it, the scheme should come from appProtocol, as you suggest: the operator would set appProtocol: https on the frontend Service's port when the frontend serves TLS (it sets none today), Karmada copies ports into the slices it imports, and this service would build https:// URLs from it. Routers would also need to trust the frontends' certificates for the pod addresses they connect to, which is the larger part. I can open an issue for it.

Where PoolRelays get the cluster name. From DynamoPoolRelay.spec.clusterId, which the operator passes to the Relay container as --cluster-id. A person is expected to set this manually
With standard MCS, #17412 compares it with the About API's ClusterProperty cluster.clusterset.k8s.io and sets the DGD's GlobalRouterPoolExported condition to False, with reason ClusterIDMismatch, when they differ.

With Karmada the operator doesn't check it, since Karmada names clusters by their member names, so clusterId has to be set to that name by hand, and this service's one-sided warning ("check that cluster_id is the pool's MCS cluster name") is what shows a mismatch. Whether the operator should fill it in is open question 4 in #17600.

[coderabbitai]

Recent review info

Run configuration

• Source mode: Auto
• Repository: ai-dynamo/dynamo
• Branch: main
• File: .coderabbit.yaml
• Commit: b751c95
• Layers: Repository YAML
• Review profile: CHILL
• Plan: Enterprise
• Run ID: c35eca70-68c9-4d5a-9457-365bd8ecad32

Commits

Reviewing files that changed from the base of the PR and between b751c95 and ea29da0.

Files ignored due to path filters: 1

• Cargo.lock is excluded by !**/*.lock

Files selected for processing: 26

• .github/codeowners/areas.yaml
• .github/filters.yaml
• CODEOWNERS
• Cargo.toml
• container/Dockerfile.global-discovery-service
• container/global-discovery-service.yaml
• lib/global-discovery-service/Cargo.toml
• lib/global-discovery-service/src/backends.rs
• lib/global-discovery-service/src/backends/kubernetes.rs
• lib/global-discovery-service/src/backends/kubernetes/endpoints.rs
• lib/global-discovery-service/src/backends/kubernetes/hub.rs
• lib/global-discovery-service/src/backends/kubernetes/mcs.rs
• lib/global-discovery-service/src/backends/memory.rs
• lib/global-discovery-service/src/backends/registered.rs
• lib/global-discovery-service/src/lib.rs
• lib/global-discovery-service/src/main.rs
• lib/global-discovery-service/src/service.rs
• lib/http-server/Cargo.toml
• lib/http-server/src/disconnect.rs
• lib/http-server/src/lib.rs
• lib/http-server/src/lifecycle.rs
• lib/http-server/src/load.rs
• lib/http-server/src/metrics.rs
• lib/http-server/src/request.rs
• lib/http-server/src/server.rs
• lib/http-server/src/write_timeout.rs

Files not fully reviewed because their changes were unavailable: 1

• lib/http-server/src/metrics.rs

Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 11 remain after this review.

Walkthrough

The change adds a reusable HTTP server crate and a Global Discovery Service. The service combines registration and Kubernetes discovery data, exposes pool and router HTTP endpoints, and adds container and Kubernetes deployment configuration.

Changes

HTTP server and Global Discovery Service

Layer / File(s)	Summary
HTTP server crate and shared data types Cargo.toml, lib/http-server/*	The workspace adds the HTTP server crate. The crate exposes load and request utilities and declares its public modules.
Connection and request lifecycle lib/http-server/src/disconnect.rs, lib/http-server/src/lifecycle.rs	Connection handles report closure status to a monitor. The service observer tracks readiness stages and inflight responses.
HTTP server, shutdown, and write deadlines lib/http-server/src/server.rs, lib/http-server/src/write_timeout.rs	The server handles TCP and TLS connections, transfer and write-stall deadlines, and graceful shutdown. Tests cover listener configuration and timeout behavior.
Kubernetes pool and frontend discovery lib/global-discovery-service/src/backends/kubernetes/*, lib/global-discovery-service/src/backends/memory.rs	The Kubernetes backend builds pool listings from Ready relay endpoints and frontend addresses in EndpointSlices. It validates service naming and tracks frontend availability warnings. The memory backend caps its sweep interval at one second or the heartbeat interval, whichever is shorter.
Joined registration and MCS listings lib/global-discovery-service/src/backends/registered.rs, lib/global-discovery-service/src/backends.rs	The registered pool directory includes only pools present in both source directories. It combines their listing data, tracks completeness, and retries failed or ended watches.
Discovery HTTP API lib/global-discovery-service/src/service.rs, lib/global-discovery-service/src/lib.rs	The service adds pool and router routes, optional pool registration, health checks, and error responses. Tests cover route behavior, request limits, unavailable backends, and shutdown.
Discovery service configuration and startup lib/global-discovery-service/Cargo.toml, lib/global-discovery-service/src/main.rs	The binary adds discovery modes, CLI validation, backend construction, HTTP startup, and signal-based shutdown.
Container and deployment configuration container/Dockerfile.global-discovery-service, container/global-discovery-service.yaml, .github/codeowners/areas.yaml, .github/filters.yaml, CODEOWNERS	The change adds a container image and Kubernetes resources for the service. Ownership rules cover the new paths, and the CI ignore filter includes the Dockerfile and manifest.

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

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 46.25% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documenta…	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Title check		The title clearly describes the primary change: Global Discovery Service pool models now come from registrations instead of a routing file.
Description check		The description is detailed and covers the change overview, key behavior, validation results, trade-offs, ongoing limitations, and related issues. It does not use the exact template headings or includ…
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

Full details: Description check

Explanation

The description is detailed and covers the change overview, key behavior, validation results, trade-offs, ongoing limitations, and related issues. It does not use the exact template headings or include the checklist, but the required information is mostly present.

Full details: Docstring Coverage

Explanation

Docstring coverage is 46.25% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 80 functions across 17 files. (8 skipped: 8 unsupported.)

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

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

Warning

Review coverage is incomplete: 1 file could not be fully reviewed. Findings from completed review steps are included; see review info for details.