[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

[devin-ai-integration]

✅ Dynamo PR CI passed — run 41146614737 (attempt 1) on d8f56723e9

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

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

[dynamo-review-agent]

expires_in_ms is computed by the remote handler before response serialization and network transit, but fetch timestamps the reply only after the body has arrived. Consequently absorb subtracts only the later wait for other peers, allowing a slow response to extend a copied registration beyond the source registration by up to the request timeout. Account for the full request/response elapsed time so copies cannot outlive their originals.

🤖 AI Fix

Capture an Instant before sending the request and subtract its full elapsed duration from every returned registration before adopting it.

[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?

[estroz]

once registrations come from ddb (#17644) a transient failure at startup would leave this instance watch-dead until restart while still reporting healthy

[atchernych]

Fixed in 2ba2b8b. join_sources no longer gives up. When it can't watch a source, or a source's watch ends, it unsyncs the join, waits with kube's DefaultBackoff (0.8 s doubling to 30 s), and watches both sources again. Until both have sent a Resync, list() joins the sources on each read and watch streams wait. rebuild now also visits pools already in the join, since after both sides relist a joined pool may be in neither. Tests: sources_that_cannot_be_watched_yet_are_watched_later and the_join_watches_again_when_a_watch_ends.

On #17644 itself: DynamoDbPoolStore::list_and_watch doesn't call DynamoDB. It starts the refresh task and subscribes, and a failed first read leaves the stream waiting while the refresh retries every second, so a table outage couldn't reach this branch. If the table is unreadable at startup, /healthz returns 503 until the first read succeeds, because list() reads the table until then. In the old give-up state GET /pools also stayed correct, since list() joined the sources on each read, but the join's watch streams and its one-sided warnings stopped for good.

[atchernych]

Follow-up in 892659d, from a second review of this fix: the backoff now starts over once the join follows both sources again, as kube's StreamBackoff does once its watch delivers, so a watch that keeps ending isn't retried every 30 s. And while the join watches the sources again, its watchers get the last view marked Estimated instead of keeping one marked Exact.

[estroz]

can you add a warn log here? a live pool whose <dgd>-frontend slice is missing or has no ready endpoint gets listed with frontends: []. that would make the usual misconfig (forgot to export the frontend service) obvious.

[atchernych]

Added in 2ba2b8b. PoolIndex::rebuild warns when a pool goes live without a Ready frontend, or loses its last one. The log has the pool key, the frontend Service it expects (<dgd>-frontend), the port name, and frontend_slices_imported: false means MCS imports no slice of that Service with that port, usually because the Service isn't exported. It's logged once per change, not on every slice event. A frontend slice without the named port already had its own warning.

[atchernych]

Follow-up in 892659d: warned pools are now recorded in the naming rules rather than the index. Before the first sync, list() builds a new index on each read, so the warning would have repeated on every /healthz until then. The field is now frontend_slices_with_port, since a frontend slice without the named port doesn't count.

[estroz]

test gap: tracks_pools_only_one_source_lists checks one_sided but nothing asserts that a source Resync which drops a joined pool removes it from pools and from the published Resync.

[atchernych]

Added a_relist_that_drops_a_joined_pool_removes_it in 2ba2b8b. For each source, a Resync without a joined pool removes it from pools and from the published Resync, with no per-pool event, and the pool becomes one-sided. It also covers both sources relisting, the case the new retry adds: a pool that neither lists any more leaves the join. That needed the rebuild change from the startup-failure thread.

[estroz]

test gap: every Event::Delete test deletes a poolrelay slice; deleting a frontend slice through upsert isn't covered, and that's the case that should flip frontends to empty (or warn, per the comment above).

[atchernych]

Added a_pool_that_loses_its_frontend_slice_is_listed_without_frontends in 2ba2b8b. An Event::Delete of the frontend slice goes through upsert with None, and the pool stays live, since its PoolRelay is Ready, with frontends: []. It also checks when the new warning fires: on losing the last frontend or going live without one, and not again while the pool has none.

[estroz]

nit: the warning should reflect that the module doc (16-19) says the expected cause is a stale Ready slice / lapsed lease.

[atchernych]

Reworded in 2ba2b8b to give that cause first: "Either its lease lapsed while MCS still shows a stale Ready endpoint, as when the pool's cluster loses contact with the MCS implementation, or its PoolRelays do not call POST /pools on this service with this key." In #17637 the second half also says to check that this instance shares registrations with the discovery instance in the pool's cluster.

[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. The user sets it; nothing derives it yet. 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. Also, no PoolRelay calls POST /pools yet; that comes next.