[estroz]

Overview

Slice 1 of 8 of #17615, which stays open as the complete, CI-validated source of truth for the Multi-Cluster Services (KEP-1645) port. Review this PR for the build, tooling, documentation entry points and CI wiring of the new Go module; the other slices are listed under Issues and Ongoing Work.

This slice adds the shell of the new Go module deploy/global-discovery-mcs (github.com/ai-dynamo/dynamo/deploy/global-discovery-mcs, Go 1.27.3, k8s.io v0.36.3, controller-runtime v0.24.1, mcs-api v0.5.2, no replace directives) with its Makefile, Dockerfile, lint configuration, README and AGENTS.md, plus the CI filter, pre-merge check job, image build jobs and code ownership. The Go source itself arrives in slices 02 to 05; the README in this slice is the map of the whole module and is the best place to start reading.

Key Changes

• Add go.mod and go.sum with the full dependency set of the module (every later slice compiles against them unchanged).
• Add the Makefile: build lint test envtest test-integration manifests proto proto-check sync-crds tidy-check check-chart-crds check docker-build e2e, every tool pinned and installed into bin/, GOWORK=off.
• Add the Dockerfile with linter, tester, builder and distroless runtime stages (USER 65532, entrypoint /mcs-registry, image global-discovery-mcs), .dockerignore, .gitignore (re-includes internal/server/, which the root server/ pattern would otherwise drop), .golangci.yml from deploy/operator plus goheader.
• Add README.md (layout, build, flags of all three binaries, labels, how the Global Discovery Service reads the imports, follow-ups), AGENTS.md and CLAUDE.md.
• Add internal/version, the only Go package in this slice.
• CI: filter global_discovery_mcs in .github/filters.yaml and FILTERS.md (with a test-filters.js case); pre-merge jobs global-discovery-mcs (check-deploy-component plus make proto-check check and make test-integration) and global-discovery-mcs-chart; pr.yaml and post-merge-ci.yml image builds through build-deploy-component for amd64 and arm64; changed-files, check-deploy-component and build-deploy-component accept the new component.
• Code ownership: the module directory and the shared Dockerfile claimed under global-router in areas.yaml, global-router added as co-owner of the two deploy-component actions; CODEOWNERS regenerated. The chart and docs globs are claimed by slices 06 and 07.

Depends on (not in this PR)

Links point at the parent branch estroz/mcs-port-module (#17615), where every piece is present together.

• The Go packages the Makefile and Dockerfile build: slices 02 (deploy/global-discovery-mcs/api), 03 (deploy/global-discovery-mcs/internal, deploy/global-discovery-mcs/cmd/mcs-registry, deploy/global-discovery-mcs/cmd/mcs-admin), 04 (deploy/global-discovery-mcs/pkg) and 05 (deploy/global-discovery-mcs/cmd/mcs-controller, deploy/global-discovery-mcs/config). Until they merge, make build and the image build fail and the pre-merge job added here cannot pass; the pre-merge chart job needs slice 06 (deploy/helm/charts/global-discovery-mcs).
• The README describes the whole module; the docs/ pages it links to are in slices 02, 03 and 05: deploy/global-discovery-mcs/docs.

Validation

On this branch alone: build_codeowners.py --strict 7330/7330 owned, emit_codeowners.py produces no diff in CODEOWNERS or CONTRIBUTORS.md, test-filters.js 87/87. make targets that need Go sources are not runnable here; see Depends on.

Every slice of #17615 was validated together at 0fd4c96b on the parent branch, from a fresh checkout of deploy/global-discovery-mcs:

• make lint 0 issues; make test (unit plus envtest, -race, coverage gate 85 on ./internal/...) 95.7 percent; make test-integration against amazon/dynamodb-local all pass; make proto-check and make check no drift; Docker linter and tester stages exit 0.
• Chart: make -C deploy/helm/charts/global-discovery-mcs lint test, 57 unittest cases; server-side dry run and a real install, upgrade and uninstall.
• CI metadata: build_codeowners.py --strict 100 percent owned, emit_codeowners.py no diff, test-filters.js 87/87.
• Docs: docs_lint.py 0 errors, fern check 0 errors, fern docs broken-links passes.
• End-to-end on three kind clusters (make e2e E2E_PHASES="1 2 3"): 99 passed, 0 failed, 1 known gap (pool region equals cluster id until feat(global-router): take a pool's region from its EndpointSlice label in the mcs backend #17604 lands).

Issues and Ongoing Work

• Relates to Ship a standard Multi-Cluster Services implementation for the Global Router #17601
• Part of feat(global-router): add the Multi-Cluster Services registry and controller #17615 (the complete port, kept open as the source of truth). The other slices:
• feat(global-router): add the Multi-Cluster Services registry API contract #17630: slice 2, the dynamo.mcs.v1 proto contract, generated code and the label and condition constants
• feat(global-router): add mcs-registry, its stores and mcs-admin #17631: slice 3, mcs-registry (view, merge rules, memory and DynamoDB stores, handlers, server) and mcs-admin
• feat(global-router): add the MCS registry Go client and fake registry #17632: slice 4, pkg/client and pkg/fakeregistry
• feat(global-router): add mcs-controller #17633: slice 5, mcs-controller (registration and lease, export reconciler, auto-export, importer, CRDs, RBAC)
• feat(global-router): add the global-discovery-mcs Helm chart #17634: slice 6, the Helm chart
• docs(global-router): document Multi-Cluster Services for the Global Router #17635: slice 7, the docs-site pages
• test(global-router): add the kind end-to-end test for global-discovery-mcs #17636: slice 8, the kind end-to-end test
• The /ok to test <sha> CI run and the module pre-merge jobs only pass once the slices they need are merged; see Depends on.

Pull Request Checklist

• I used Dynamo's /pr-checklist skill.

🤖 Generated with Claude Code

[dynamo-review-agent]

This comment only labels the immediately following AS base stage and narrates its role; the stage name and subsequent FROM base uses already convey that structure.

🤖 AI Fix

Remove this redundant base-stage comment.

[estroz]

Removed the six comments that only restated the stage names (base, copy source, lint, install make, build, runtime) in one pass, in #17615 and re-cut here. Kept the ones that explain a choice: the buildx TARGETOS/TARGETARCH args, the layer-caching order, and what the tester stage runs.

[dynamo-review-agent]

# Copy source code merely restates the following COPY . . instruction and preserves no rationale or constraint.

🤖 AI Fix

Remove the comment above COPY . ..

[dynamo-review-agent]

The AS linter declaration already identifies this as the lint stage, so this heading adds no lasting information.

🤖 AI Fix

Remove the redundant lint-stage comment.

[dynamo-review-agent]

The following apt-get install ... make command already states exactly what this comment narrates; there is no non-obvious reason recorded here.

🤖 AI Fix

Remove the comment above the make installation command.

[dynamo-review-agent]

The named AS builder stage makes this label self-evident, so the comment only repeats the code.

🤖 AI Fix

Remove the redundant build-stage comment.

[dynamo-review-agent]

The shipped-image build cannot succeed in this change: neither the reviewed head nor the supplied base contains cmd/mcs-registry, cmd/mcs-controller, or cmd/mcs-admin, yet this loop unconditionally builds all three. The newly enabled PR and post-merge image jobs therefore fail with a missing-package/directory error and can never publish the image.

🤖 AI Fix

Gate the image build jobs until all three command packages and their runtime implementation are present.

[estroz]

Expected: this PR is slice 1 of 8 of #17615 and the three cmd/ packages and config/crd arrive in #17631 and #17633 (see the Depends-on section). The image job is gated on the global_discovery_mcs filter and will only pass once those slices are merged; the complete tree in #17615 builds and was validated there.

[dynamo-review-agent]

This heading only announces the final runtime stage immediately before its FROM instruction and records no rationale or constraint.

🤖 AI Fix

Remove the redundant runtime-stage comment.

[devin-ai-integration]

❌ Dynamo PR CI failed — run 41132757117 (attempt 1) on bfaf160180

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

Other	Jobs
Global Discovery MCS	❌ 1

Failure details

1 job failed: the Global Discovery MCS image build cannot copy config/crd/bases because the directory is missing from the Docker build context.

❌ Global Discovery MCS: Dockerfile COPY "/config/crd/bases": not found

Job: Global Discovery MCS · Failed step: Build and push the MCS registry image · Logs: gh run view --job 124329481717 -R ai-dynamo/dynamo --log-failed

#17 [linux/amd64 stage-4 3/4] COPY config/crd/bases /opt/global-discovery-mcs/crds/
#17 ERROR: failed to calculate checksum of ref laqrivy22t3zxhgib21qvq3cm::1ybgsmbvubzdra94taw0zcwf2: "/config/crd/bases": not found
Dockerfile:59
--------------------
  57 |     WORKDIR /
  58 |     COPY --from=builder /out/mcs-registry /out/mcs-controller /out/mcs-admin /
  59 | >>> COPY config/crd/bases /opt/global-discovery-mcs/crds/
  60 |     USER 65532:65532
  61 |
--------------------
ERROR: failed to build: failed to solve: failed to compute cache key: failed to calculate checksum of ref laqrivy22t3zxhgib21qvq3cm::1ybgsmbvubzdra94taw0zcwf2: "/config/crd/bases": not found
##[error]Process completed with exit code 1.

The image build fails at Dockerfile line 59 when BuildKit resolves the final-stage COPY config/crd/bases /opt/global-discovery-mcs/crds/. The source directory is absent from the build context; the preceding Go builder and binary-copy stages shown in the log were cached.

For agents

{"pr": 17629, "run_id": 41132757117, "run_attempt": 1, "head_sha": "bfaf1601804f1689e995aa216d8d4c03ea5b6edb",
 "failures": [{"job": "Global Discovery MCS", "job_id": 124329481717, "failed_step": "Build and push the MCS registry image", "signature": "Dockerfile:59 COPY config/crd/bases: \"/config/crd/bases\": not found", "tests": [], "log_cmd": "gh run view --job 124329481717 -R ai-dynamo/dynamo --log-failed"}]}

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

[sttts]

Detailed inline review feedback. Correctness findings, non-blocking suggestions, and coverage requests are identified separately in the comments.

[sttts]

Coverage request: provide an explicit CI path for the three-cluster suite

The new end-to-end suite currently depends on manual execution

The added checks run unit/envtest, generated-file validation, DynamoDB-local integration and chart tests. They do not invoke the three-cluster suite introduced in #17636, directly or through the inspected shared targets.

That leaves the cross-cluster lifecycle and Global Discovery Service integration outside the automated execution path.

Requested change

Please provide an explicit path for running this suite in CI. If running it on every relevant PR is too expensive, a scheduled or manually dispatched workflow with a documented merge policy would still make execution and evidence reproducible.

Preserve diagnostic artifacts and distinguish documented gaps from passed assertions.

Validation

Show that the intended workflow runs the suite and fails when an e2e assertion fails. For automatic execution, verify that relevant controller/chart changes select the job.

This is a CI coverage request, not a claim that envtest is skipped or that production behavior is already broken.

[estroz]

Agreed. The suite runs locally today because it needs three kind clusters and about 12 minutes. We will add a global-discovery-mcs-e2e workflow with workflow_dispatch and a nightly schedule on the base branch, plus a PR path triggered by a label, that uploads the kind logs and the per-phase result file as artifacts and fails on any fail line (E2E_STRICT=1 turns documented gaps into failures; the default run reports them separately). It will also run the upstream conformance suite from #17681 so there is one scheduled job. We will land it once #17636 is in, since the workflow needs the suite.

[sttts]

[P3] Include useful build identity in container binaries

Container binaries retain dev/unknown while the Makefile build supplies metadata

The Makefile injects Version and GitHash, but this build path supplies only -s -w. The published image therefore retains the package defaults dev and unknown.

This module exposes that identity in startup logs and server information, and the controller advertises its version during registration. Useful metadata would make mixed-version deployments and upgrades easier to diagnose.

Suggested change

Pass version/commit metadata into the image build and share the relevant linker settings with the normal build path.

This is a non-blocking observability improvement. It is not a claim that the current build fails or violates a repository-wide requirement that all container builds inject these fields.

Validation

Build an image with known metadata and verify the identity reported by the container binary or server API. Checking the Makefile-built binary alone would not cover this path.

[estroz]

Done in #17615 (1a07d2f) and re-cut here (9197dce). The builder stage takes VERSION and GIT_HASH build args and links them with the same -X internal/version flags as the Makefile's LDFLAGS, keeping -s -w and -trimpath. make docker-build passes them from git describe / rev-parse, and the post-merge image build passes the ref name and github.sha through the action's extra_build_args. A local build with VERSION=test-v GIT_HASH=abc123 logs version=test-v git_hash=abc123 at startup.