feat: read locations from the locations service - #261
Merged
Conversation
Compute reads two location facts: the cities a project may place workloads in, and the location a cell serves. Both are served today by network-services-operator and are moving to the locations service, which projects a Location into a project's control plane alongside the legacy LocationBinding. A new locationSource config field selects which group is read. It defaults to NetworkServices, so a deployment that does not set it reads exactly what it reads today. It governs reads only; nothing compute writes changes with it. Key changes: - Add internal/locations, which resolves placement locations and serving locations from either group and collapses the three duplicated city-code lookups into one call - Read the locations service through unstructured: its module path does not yet resolve, so a typed dependency cannot be pinned. A kind that is not installed reads as no locations rather than failing the caller - Route the workload webhook, the workload reconciler and the deployment reconciler through the accessor, including the serving location watch - Reject an unknown source when the server config loads, rather than on every reconcile
The locations module is reachable after all: a replace directive bypasses the vanity path lookup that a bare `go get` fails on, which is the pattern network-services-operator already uses for the ipam module. Pin the same commit NSO pins, the one deployed to staging. Reading the locations service through its generated types replaces the unstructured client and its hand-rolled topology extraction, and lets the shared topology keys be taken from the module rather than restated. The NetworkServices path is untouched and still reads the same typed network services kinds it always has. Key changes: - Depend on go.miloapis.com/locations by pseudo-version through a replace, and register its scheme on the manager and the CLI client - Keep the absent-CRD degrade, which a typed client reaches differently: the no-match arrives wrapped in an ErrResourceDiscoveryFailed, so only errors.Is unwrapping finds it. A type missing from the scheme is a wiring mistake and still fails - Cover both states against a real API server, since compiling against the types says nothing about a client resolving the kinds - Assert the two groups agree on the city-code and serving-location keys, which is what makes switching sources safe
Reading a location the control plane does not serve degrades to no locations, but watching one cannot: registering the watch wedges the manager during cache sync, and a wedged manager says nothing about why. Setup now checks discovery first and refuses to start, naming the kind, the CustomResourceDefinition to install, and the locationSource that requires it. The check covers only the source actually selected. A deployment reading network services gains no dependency on the locations service, and it stays behind the networking integration gate, so a cell that registers no watch still needs no location kind at all. Key changes: - Add EnsureServingLocationKind, and call it before the watch is registered - Cover the guard against real discovery in envtest, across a control plane serving neither kind, one kind, and both, so each source is shown to gate on its own kind alone
The module path go.miloapis.com/locations did not resolve, so the dependency was pinned through a replace onto the GitHub path. The vanity import is now registered and the module downloads from the public proxy. Key changes: - Require go.miloapis.com/locations at its own path and drop the replace
scotwells
marked this pull request as ready for review
September 1, 2026 23:15
scotwells
enabled auto-merge
September 1, 2026 23:15
ecv
approved these changes
Sep 1, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Compute reads the platform's locations to decide where a workload can run, and to validate the location a user names. It reads them from the network-services operator, which no longer owns that data. The locations service does.
This moves compute onto the locations service, behind a config flag so a control plane can switch when its data is ready rather than on deploy.
Compute refuses to start if the group it is configured to read is not served, naming the missing resource. A silent fallback would let a control plane run with no locations and fail every placement at admission instead.
Related: milo-os/locations, datum-cloud/infra#4299.
Draft:
LocationBindingstill backs workload placement here, and dropping it needs network-services-operator#413 to land first.