[atchernych]

Summary

Adds lib/global-discovery-service as a library: the contract for which Global Router pools exist and which router replicas a PoolRelay connects to, and the backends behind it. The HTTP service and its binary are in #17371. The crate's name and place follow #17354, and its shared types come from lib/global-common (#17330, merged).

Part of #17600, the pool discovery DEP.

We have 2 knobs in discovery:
--pool-discovery = registrations or karmada or mcs: where the pool and router lists come from. The  registrations means the data comes from the API calls i.e. POST /pools
 --registration-storage = memory or DynamoDB or Kubernetes : where registrations are kept.

Layout (Global Router coding standards)

• Contract, pool_discovery: PoolDirectory lists and watches pools, ReplicaDirectory lists and watches router replicas, and PoolRegistrar registers a pool. Each registration is also the pool's heartbeat.
• Backends, backends:
  • Memory: keeps each registration for its lease. Each replica keeps only the registrations it receives, so run one. Its lists are complete once a lease has passed since it started.
  • MCS: pools and router replicas from the EndpointSlices that MCS imports, named the Karmada way or the standard way. A pool is live while one of its PoolRelay endpoints is Ready. Its relay_identities name every Ready PoolRelay pod by each of its addresses, sorted, so every service instance lists the same pods: a pool runs active-active PoolRelay replicas, each reports to every router, and routers dedupe the overlapping reports.
• When the MCS watch fails: after a minute of errors, the backend keeps serving its last view but marks it incomplete and logs errors. Two minutes without errors count as recovery. Watchers get a Resync at both changes, and a relist counts only once it completes.
• Timeouts: watches end after 50 s, so a server that stops answering shows up as errors. The client's connect and read timeouts must add up to under 90 s (feat(global-router): add the Global Discovery HTTP service #17371 uses 10 s and 75 s). An empty relist, and a router EndpointSlice without the named TCP port, are logged.
• Karmada names: a Service name may have at most 55 characters, since Karmada imports it as derived-<name>, which must fit in 63. The router directory logs an error for a longer name, and the operator in feat(operator): export Global Router pools through the Multi-Cluster Services API #17412 does not export one.

Where the code comes from: #15933 and #15949, in the crate layout of the global-router-v2 test branch; this PR replaces both. Their router-side pieces (relay admission, state connections, router discovery modes) are left out, since relay connections belong with the router. #17355 already added the empty crate and its code-owner rule, so this PR adds no workspace member or code-owner line.

Main components

Component	Contract (pool_discovery)	Backends (backends)
Pools: list and watch	PoolDirectory (list, list_and_watch)	memory (MemoryPoolStore); MCS (McsPoolDirectory, Karmada or standard)
Pools: register (heartbeat)	PoolRegistrar::register takes a PoolRegistration and returns its Lease	memory
Router replicas: list and watch	ReplicaDirectory	static (StaticReplicaDirectory, set by --router in #17371); MCS (McsReplicaDirectory)
Shared types	PoolKey, PoolLocation, PoolRegistration, PoolListing, Lease, WatchEvent, Snapshot, Completeness, RouterReplica	lib/global-common (#17330)

Per mode

The HTTP service in #17371 picks the backend with --pool-discovery.

Function	Memory backend (--pool-discovery registrations)	MCS backend (--pool-discovery karmada or mcs)
Lists and watches the pools: PoolDirectory (discovery service)	The registrations, in the discovery service's memory, not the router's. Run one replica	Watches the imported EndpointSlices of each pool's PoolRelay Service (<DGD name>-pool-relay by default), on the Karmada control plane or in a cluster that imports them. A pool is live while one of its PoolRelay endpoints is Ready
Saves a pool when its PoolRelay registers: PoolRegistrar (discovery service)	Keeps each registration until its lease runs out	None: POST /pools returns 501 (#17371), so records have no model, frontends, or capacity. #17332 takes the model and capacity from registrations, and the frontends from MCS
Admits a PoolRelay's state stream (router, not in this stack)	Not designed yet. Listings name no PoolRelay	Trusted network: relay_identities are the addresses of every Ready PoolRelay pod, for the router to match against the caller's source IP
Lets PoolRelays find the router replicas: ReplicaDirectory (discovery service)	GET /routers lists the replicas set with --router (#17371). Routers cannot register yet (#17333)	GET /routers lists the Ready replicas in the router Service's imported EndpointSlices. The router's Service is exported once (admin setup)
Registers the pool (PoolRelay, not in this stack)	Calls POST /pools on a timer, through lib/global-discovery-client (#17208). No PoolRelay calls it yet	Its PoolRelay Service is exported through MCS. The operator creates the ServiceExport in draft #17412. With #17332 the PoolRelay also calls POST /pools
Gets the router replicas (PoolRelay, not in this stack)	GET /routers, through the same client	The same GET /routers. The PoolRelay reads no EndpointSlices

Validation

• cargo test -p dynamo-global-discovery-service: 39 passed. They cover the memory backend's leases and completeness, the MCS pool and replica indexes (dual-stack PoolRelays and replicas, every Ready PoolRelay pod listed alike by every instance, a pod that stops being Ready leaving the listing, an IPv4-mapped address listed once), Karmada's name limit, a watcher that falls behind, and failing watches: going stale, recovering by silence, by an event or by a relist, a relist that fails partway, and backoff. Breaking each behavior the reviews fixed makes a test fail.
• cargo clippy --all-targets -- -D warnings, cargo fmt, and cargo doc with -D warnings: clean.
• Three adversarial reviews against the Global Router coding standards ran, and their verified findings are fixed. Reusing dynamo_runtime's Kubernetes watch code would need a refactor of dynamo-runtime, so it is not done here.
• Ran the MCS backend against a stand-in API server. Lists come from the API server's watch cache (resourceVersion=0, unpaged), watches resume with bookmarks, and after the first sync GET /pools and GET /routers make no API calls.
• CodeRabbit's docstring check stays under 80% because it counts test functions, which are named for what they check.
• Not yet run on a cluster.

🤖 Generated with Claude Code

Summary by CodeRabbit

• New Features
  • Added shared discovery data formats and pool identity handling, with validation for registrations and lease timings.
  • Added discovery service support for listing and watching pools and router replicas, including in-memory and Kubernetes-backed discovery.
  • Added replica and pool updates with resynchronization after changes or interruptions.
  • Updated the discovery client to use the shared router replica format.

[devin-ai-integration]

✅ Dynamo PR CI passed — run 41120860737 (attempt 2) on 73326260bc

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

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

[dynamo-review-agent]

Previously reported defects still present:

• Original discussion: Verified still present: SliceCache::apply clears failing_since before matching the event, so repeated relists that emit Init or InitApply but fail before InitDone continually reset the outage timer and can keep a stale snapshot classified as Exact indefinitely.
• Original discussion: Verified still present: watch failures only update failing_since; crossing STALE_AFTER does not broadcast an Estimated resync, and recovery does not necessarily broadcast an Exact resync when membership is unchanged. Existing subscribers can therefore retain obsolete completeness indefinitely.

[coderabbitai]

No actionable comments were generated in the recent review.

Recent review info

Run configuration

• Source mode: Auto
• Repository: ai-dynamo/dynamo
• Branch: atchernych/global-discovery-service
• File: .coderabbit.yaml
• Commit: 69d703e
• Layers: Repository YAML
• Review profile: CHILL
• Plan: Enterprise
• Run ID: 9cf649ee-4a02-41b4-b6fa-040ebbfd5ad4

Commits

Reviewing files that changed from the base of the PR and between 8a5b7f2 and 69d703e.

Files ignored due to path filters: 1

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

Files selected for processing: 28

• Cargo.toml
• lib/global-common/Cargo.toml
• lib/global-common/src/discovery.rs
• lib/global-common/src/lib.rs
• lib/global-common/src/pool.rs
• lib/global-common/src/replicas.rs
• lib/global-common/src/watch.rs
• lib/global-discovery-client/Cargo.toml
• lib/global-discovery-client/src/client.rs
• lib/global-discovery-client/src/lib.rs
• lib/global-discovery-client/src/replicas.rs
• lib/global-discovery-client/src/wire.rs
• lib/global-discovery-client/tests/relay_calls.rs
• 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/cache.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/stream.rs
• lib/global-discovery-service/src/lib.rs
• lib/global-discovery-service/src/pool_discovery.rs
• lib/global-discovery-service/src/pool_discovery/pools.rs
• lib/global-discovery-service/src/pool_discovery/registrar.rs
• lib/global-discovery-service/src/pool_discovery/replicas.rs
• lib/global-discovery-service/src/pool_discovery/watch.rs

Files with no reviewable changes: 2

• lib/global-discovery-client/src/replicas.rs
• lib/global-discovery-client/src/wire.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 pull request adds shared Global Discovery types and service contracts, updates the client to use those types, and introduces in-memory and Kubernetes-backed pool and router replica directories with listing and watch support.

Changes

Global Discovery

Layer / File(s)	Summary
Shared identity and discovery types Cargo.toml, lib/global-common/*	Adds shared pool identity and location types, discovery request and response types, replica types, and watch events. Adds validation, serialization, and derivation tests.
Discovery service contracts lib/global-discovery-service/src/pool_discovery*, lib/global-discovery-service/src/lib.rs, lib/global-discovery-service/src/backends.rs, lib/global-discovery-service/Cargo.toml	Adds pool registration and directory contracts, watch-stream types, a discovery error type, and public service modules.
Client migration to shared types lib/global-discovery-client/*	Moves client-facing wire and replica types to dynamo-global-common. Updates the router response type and relay-call test fixtures.
In-memory stores and watch streams lib/global-discovery-service/src/backends/memory.rs, lib/global-discovery-service/src/backends/stream.rs	Adds lease-based pool registration and expiration, a static replica directory, and watch-stream helpers for initial snapshots and lag recovery.
Shared Kubernetes slice cache lib/global-discovery-service/src/backends/kubernetes/cache.rs	Adds a shared EndpointSlice cache with relisting, watch-error completeness updates, retry handling, and directory listing and watch operations.
MCS replica discovery lib/global-discovery-service/src/backends/kubernetes.rs, lib/global-discovery-service/src/backends/kubernetes/mcs.rs, lib/global-discovery-service/src/backends/kubernetes/endpoints.rs	Adds Standard and Karmada slice naming support and replica discovery from imported service endpoints.
MCS pool discovery lib/global-discovery-service/src/backends/kubernetes/hub.rs	Adds pool listing and watch behavior derived from imported slices, including relay selection, liveness, and completeness updates.

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

Merge Risk: Minimal · up to 69d70

This change adds the pool discovery contracts, shared types, and in-memory and MCS-backed stores, and migrates the client to the shared types. No concrete merge-blocking defect was identified. Cluster-level validation of the Kubernetes backends is still pending and should be done in normal rollout.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 52.27% 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
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.
Title check		The title clearly and concisely identifies the main change: adding the Global Discovery Service library, its contract, and its backends.
Description check		The description is detailed and covers the change overview, key components, implementation modes, validation results, known limitations, and related issues. It does not use every template heading exac…

Full details: Description check

Explanation

The description is detailed and covers the change overview, key components, implementation modes, validation results, known limitations, and related issues. It does not use every template heading exactly and omits the checklist and explicit Issues and Ongoing Work section, but the required technical information is mostly complete.

Full details: Docstring Coverage

Explanation

Docstring coverage is 52.27% 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 44 functions across 22 files. (4 skipped: 4 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

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

[estroz]

LGTM on initial pass, may have some follow-ups

[jioffe502]

LGTM. Built at 06b4f63: cargo test -p dynamo-global-discovery-service passes (37/37) and clippy is clean. Later commits only touch docs in this crate. Haven't run it against a real API server or Karmada.

[dmitry-tokarev-nv]

No P0 or P1 found, and this review posts no findings. It covers head 73326260bcecf6edf2b4502039bd564304a1a8df, and a local merge of that head into main at ba9caeac492cdeef00fbb6096ae6efda40f81064, which merges without conflicts.

Open items: none. All earlier threads are resolved.

Tests, lock file and deliberate breaks, on the head and on merges with main.

• cargo test --locked -p dynamo-global-discovery-service: 39 passed on the head, and on merges with main at 3057891245 and at ba9caeac49.
• cargo tree --locked passes on the head and on both merges. The lock adds no package. Only the dependency list of dynamo-global-discovery-service changes, and every crate in it is already locked on main at the same version.
• Removing registered.expires_at = expires_at; at memory.rs:209 fails registering_again_only_renews.
• Changing >= to > at memory.rs:122 fails lists_are_exact_once_a_lease_has_passed.
• Changing >= to > at cache.rs:167 fails 4 tests in hub.rs.
• CI at this head, at the time of this review: the PR run on pull-request/17331 passed. In Pre Merge, rust-tests (.) was still running, and the other jobs passed.

Memory backend, measured with paused tokio time and with 8 worker threads.

• The lease uses tokio::time::Instant, a monotonic clock.
• With a 5 s / 15 s lease, a pool is listed at 14.999 s and gone at 15.000 s. A second registration at 10 s moves that to 24.999 s and 25.000 s. The sweep sends Removed at 15 s, with the Resync that marks the list Exact.
• 32 tasks sent 16,000 registrations of one pool. With one payload, the store keeps one listing and sends no event after the first. With two alternating payloads, it sends more events than the 1,024-event buffer holds, and the watcher gets a fresh Resync, as the docs say.
• The store has no bound on the number of pools: 100,000 distinct pools registered in 638 ms, and list() returned them in 61 ms.
• Memory: 1,224 heap bytes per pool for a 291-byte registration, plus 661 bytes per pool for each list() copy. For a registration with 1,000 capabilities of 100 characters, each list() copy takes about 125 KB per pool, and the store keeps about the same.

MCS backend, against an in-process fake of the EndpointSlice API on 127.0.0.1, with the 10 s / 75 s client timeouts from #17371.

• Pools, Karmada names: Ready endpoints and endpoints without ready are listed. NotReady endpoints are not, and a pool with only NotReady relays is not listed. A pool appears, grows and leaves through Upserted and Removed as its endpoints change.
• Pools, standard multicluster.kubernetes.io labels: listed from the same fake, and slices named the other way are skipped.
• Router replicas: both naming styles list Ready endpoints only. A 55-character Karmada Service is listed. A 56-character one logs the error from McsReplicaDirectory::new. Its list then fails, because the fake refuses a label value over 63 characters, as the API server does.
• An empty first list, and an empty relist after a 410, both send Resync with no items and log the warning.
• With the API server failing every request, the Resync marked Estimated came 80.4 s after the first error. Requests came at 0.8, 1.6, 3.2, 6.4, 12.8 and 25.6 s intervals, then every 30 s. The check runs only at an error. The first error past one minute came at 80.4 s, and until then list() stayed Exact.
• Recovery: a changed slice brought Upserted and an Exact Resync 30.0 s after the API server recovered. With no change, the Exact Resync came 120.0 s after the last error.
• Every request went to the fake. The client had no other API server address.

[dmitry-tokarev-nv]

Reviewed at 73326260bcecf6edf2b4502039bd564304a1a8df, and on a local merge with main at ba9caeac492cdeef00fbb6096ae6efda40f81064, which has no conflicts.

• Tests: 39 of 39 pass on both trees. Three deliberate one-line breaks each fail a test: the lease renewal, the completeness boundary of the memory backend, and the staleness boundary of the MCS cache.
• Lock file: no new crate, and cargo tree --locked passes on both trees.
• Memory backend: lease expiry and renewal are exact on the monotonic tokio clock. 16,000 concurrent registrations of one pool keep one listing. The number of pools has no bound (100,000 registered in 638 ms), at about 1.2 KB per pool.
• MCS backend, against an in-process fake of the EndpointSlice API: Ready and NotReady endpoints, both naming styles, the 55-character limit, empty relists, and the Estimated and Exact resyncs all match the description.

Open items: none. #17371 stacks on this PR.