Skip to content

test(global-router): run shared MCS discovery cases with Cilium - #17692

Open
sttts wants to merge 5 commits into
sttts-mcs-upstream-conformancefrom
sttts-dgh-2319-mcs-provider-portability
Open

sttts wants to merge 5 commits into
sttts-mcs-upstream-conformancefrom
sttts-dgh-2319-mcs-provider-portability

Conversation

@sttts

@sttts sttts commented Oct 30, 2026 •

Copy link
Copy Markdown
Contributor

Overview

Keep the MCS-to-Global Discovery Service discovery boundary independent of the bundled MCS implementation by running one shared set of kind cases with either Dynamo MCS or Cilium. New discovery cases are written once and run against both providers; provider selection changes installation and teardown, not the workloads or assertions.

This tests discovery and lifecycle with real GDS. Router and PoolRelay Pods are busybox stand-ins: inference routing, relay handshakes, state streams and full Global Router failover are not exercised.

Stacked on #17681 (sttts-mcs-upstream-conformance at 244e98fa8639e85a589e7859dda334f3f8c020fa). That PR supplies the tested MCS baseline and remains a separate conformance change. Merge it first, then retarget this PR.

Key Changes

Production Rust change: +21 / -3 nonblank, non-comment lines, excluding tests. The provider swap adds no bundled MCS production changes.

  • Shared suite and provider adapter: add make e2e-discovery E2E_PROVIDER=dynamo|cilium. Both modes execute the same phase3.sh, headless Service fixtures, standard ServiceExports and GDS configuration. Cilium 1.20.4 runs with MCS API and EndpointSlice synchronization enabled; its mode asserts that bundled registry/controller workloads are absent. Phase 3 has no provider branches; strict mode rejects skips and known gaps.
  • Generic GDS region reader: cherry-pick only the two backend files from feat(global-router): take a pool's region from its EndpointSlice label in the mcs backend #17604, source commit b30c06b16cd122e726d0c9da5aa8593f2b4ea193. Read an optional imported region label and preserve the cluster-ID fallback. Keep the original author and commit reference; omit unrelated help/doc changes.
  • CI, diagnostics and review artifacts: run the same target in a Dynamo/Cilium PR matrix, with triggers covering the GDS local dependency closure and Cargo configuration. Keep provider setup separate from the shared case count, isolate owned kind clusters, preserve diagnostics and clean up after report-write failures. Fix the prompt's SPDX header format and include nine runner regression checks, dated provider results and architecture slides.

Configuration and Compatibility

make -C deploy/global-discovery-mcs e2e-discovery E2E_PROVIDER=dynamo
make -C deploy/global-discovery-mcs e2e-discovery E2E_PROVIDER=cilium

The new target always selects phase 3 and strict mode. The existing make e2e default still runs the bundled phases 1/2/3; requesting phases 1/2 with Cilium fails explicitly because those cases exercise bundled registry/controller internals. The upstream conformance runner remains pinned to the Dynamo provider.

GDS reads standard imported Kubernetes objects and does not call the bundled registry. For example, an MCS provider may publish:

apiVersion: discovery.k8s.io/v1
kind: EndpointSlice
metadata:
  name: demo-pool-relay-a
  namespace: demo
  labels:
    multicluster.kubernetes.io/service-name: demo-pool-relay
    multicluster.kubernetes.io/source-cluster: a
    topology.kubernetes.io/region: region-a
addressType: IPv4
ports:
  - name: grpc
    protocol: TCP
    port: 8080
endpoints:
  - addresses: [10.242.0.10]
    conditions:
      ready: true

The MCS provider owns that object; the suite never patches its slices. A nonempty region label yields location.region=region-a. Omit the label, or give it an empty value, and GDS retains location.region=a. The shared assertion checks this same label-or-cluster contract for both providers. No new CRD, Helm API, registry/storage API or data migration is introduced.

Bundled Dynamo MCS or Cilium with the same discovery boundary

The slide records the original 2026-10-30 run: 20 shared cases plus two setup checks. Review removed a redundant region-presence assertion; the retained region-contract case still requires a nonempty region and validates its value. The current suite has 19 shared cases plus two setup checks.

Validation

Review reruns on 2026-11-03, locally on macOS arm64 with kind v0.33.0 / Kubernetes v1.35.8 and Helm v3.21.0:

Provider Shared discovery cases Setup checks Failed GAP Skipped Wall time
Bundled Dynamo MCS 19 2 0 0 0 84 s
Cilium 1.20.4 19 2 0 0 0 204 s

Both runs reused the validated global-discovery-service:e2e-78fb0157db6a image after checking that rebased GDS sources and in-tree dependencies match its source; the GDS review edits remove two comments only. The bundled MCS image was rebuilt. Their executed discovery case names and order match exactly, including ServiceExport readiness, imported objects, pool/router identities and URLs, scale-to-zero/recovery, and export removal. Both pairs of dedicated clusters were deleted. Recorded summaries, case lists and source/image provenance distinguish the earlier baseline, region fix, original shared suite and 2026-11-03 review runs. The dated original evidence is preserved.

The region fix previously passed 45 library and 6 binary tests plus formatting and Clippy; these were not repeated for comment-only GDS edits. Nine targeted runner checks exercise real EXIT-handler/report paths with stubbed cluster operations: failures to create the report directory, open/write the summary, exit-status preservation and existing keep behavior. ShellCheck and commit-time checks pass. Independent harness/setup/adversarial reviews and a focused re-review checked the shared-suite boundary and the two introduced CI/cleanup fixes.

The full Go unit/envtest suite also passes with the race detector at 95.7% internal coverage, with AWS environment/profile settings isolated. No test source or filters were changed for that isolation. The DynamoDB integration suite and clean-tree generated-file check also pass after taking the parent's socket-override fix. The evidence above is local. Both Ubuntu provider-matrix jobs also passed on the original 01f2d4b5aa head (run 41092610002); new runs report their own results for the rebased head. This does not establish end-to-end request routing, DNS conformance, real relay protocol behavior, DynamoDB/multi-region behavior, or a complete extraction of a generic MCS package.

Issues and Ongoing Work

Pull Request Checklist
  • I used Dynamo's /pr-checklist skill, including independent review.

@sttts
sttts requested review from a team as code owners October 30, 2026 16:44
@github-actions github-actions Bot added test documentation Improvements or additions to documentation actions labels Oct 30, 2026
@devin-ai-integration

devin-ai-integration Bot commented Oct 30, 2026 •

Copy link
Copy Markdown
Contributor

✅ Dynamo PR CI passed — run 41732038322 (attempt 1) on e14b7c471f

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

Posted automatically by Devin for run 41732038322. Updated on every full-CI run of this PR.

@sttts
sttts force-pushed the sttts-mcs-upstream-conformance branch from 5ccd7b6 to 03b0d74 Compare November 2, 2026 12:42
@sttts
sttts requested review from a team as code owners November 2, 2026 12:42
@sttts
sttts force-pushed the sttts-mcs-upstream-conformance branch from 03b0d74 to 244e98f Compare November 3, 2026 22:24
estroz and others added 5 commits November 3, 2026 23:32
Select the backend logic and regression tests from:
#17604

(cherry picked from commit b30c06b)

Read topology.kubernetes.io/region from imported EndpointSlices and
propagate it to PoolListing.location.region. Choose a labeled slice
deterministically and publish region changes even when relay identity
does not change. Preserve the upstream fallback for missing or empty
labels.

Part of #17601.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Eric Stroczynski <estroczynski@nvidia.com>
Signed-off-by: Dr. Stefan Schimanski <sschimanski@nvidia.com>
Run identical strict GDS discovery cases with either bundled MCS or Cilium. Isolate provider setup, retain diagnostics, guard report writes before teardown, and record shared-case validation and architecture slides.

Part of #17601; refs DGH-2319. Stacked on #17681; the preceding minimal GDS region fix comes from #17604.

Signed-off-by: Dr. Stefan Schimanski <sschimanski@nvidia.com>
Signed-off-by: Dr. Stefan Schimanski <sschimanski@nvidia.com>
Signed-off-by: Dr. Stefan Schimanski <sschimanski@nvidia.com>
Signed-off-by: Dr. Stefan Schimanski <sschimanski@nvidia.com>
@sttts
sttts force-pushed the sttts-dgh-2319-mcs-provider-portability branch from 01f2d4b to e14b7c4 Compare November 3, 2026 22:34

sttts commented Nov 3, 2026

Copy link
Copy Markdown
Contributor Author

Rebased onto the updated #17681 (244e98fa86) and pushed signed head e14b7c471f with an explicit force-with-lease.

Addressed and resolved all three review threads: removed the duplicate region-presence assertion and the two redundant Rust comments. Also fixed this PR's prompt-file SPDX header and trailing blank line. Historical slide/results counts remain dated; the README and PR description now explain the current case count.

Provider Shared cases Setup checks Failures / gaps / skips Wall time
Dynamo MCS 19 2 0 / 0 / 0 84 s
Cilium 1.20.4 19 2 0 / 0 / 0 204 s

Case names and order match exactly; both runs removed their dedicated clusters. The MCS image was rebuilt. Both runs reused the validated GDS image after checking unchanged rebased runtime sources/dependencies; the new GDS edits remove comments only. Dated results and provenance are committed. Coverage remains discovery/lifecycle with real GDS and router/relay stand-ins.

The full Go unit/envtest suite passes with the race detector at 95.7% coverage, along with lint, nine cleanup regressions, Bash 3.2/ShellCheck, commit hooks and clean-tree generated-file checks. The parent's DynamoDB socket-override fix is inherited, not copied here; its integration suite also passes locally. Both independent reviewers confirmed the scope and review fixes.

Fresh CI is queued for this head: provider matrix, pre-merge, copyright.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

actions documentation Improvements or additions to documentation size/XXL test

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants