xds: generate endpoints directly from API gateway snapshot#17390
xds: generate endpoints directly from API gateway snapshot#17390sarahalsmiller merged 14 commits intomainfrom
Conversation
nathancoleman
left a comment
There was a problem hiding this comment.
This LGTM, but I paired on it so would like an approval from someone else
Co-authored-by: John Maguire <john.maguire@hashicorp.com>
Each listener would previously have all upstreams from any route that bound to the listener. This is problematic when a route bound to one listener also binds to other listeners and so includes upstreams for multiple listeners. The list for a given listener would then wind up including upstreams for other listeners.
Co-authored-by: Nathan Coleman <nathan.coleman@hashicorp.com>
|
removing the backport label after talking with Nathan this morning |
| } | ||
|
|
||
| return nil | ||
| return h.recompileDiscoveryChains(snap) |
There was a problem hiding this comment.
Previously, we would recompile the discovery chain every time xDS resources were generated, sort of a "pull" style trigger. Instead, this will recompile the discovery chains every time a related resource that we're watching changes, so more of a "push" style trigger.
| return nil, err | ||
| } | ||
| return s.endpointsFromSnapshotIngressGateway(cfgSnap) | ||
| return s.endpointsFromSnapshotAPIGateway(cfgSnap) |
There was a problem hiding this comment.
This is the crux of the whole change that we're making: instead of converting to an ingress gateway snapshot and generating xDS resources from that, we generate xDS resources directly from our API gateway snapshot 🎉
|
|
||
| for i, service := range services { | ||
| id := NewUpstreamIDFromServiceName(structs.NewServiceName(service.Name, &service.EnterpriseMeta)) | ||
| synthesizedChains[id] = compiled[i] |
There was a problem hiding this comment.
Is it guaranteed that the compiled discovery chains will always be in the same order as services? Cross-indexing like this without some validation like checking compiled.name == service.name feels a little sketchy.
There was a problem hiding this comment.
For what its worth, I believe this was code was copied directly from the ToIngress function. I'll add a quick sanity check though since I'm not sure if its guaranteed.
…:hashicorp/consul into NET-3673-endpointsFromSnapshotAPIGateway
Description
In order to move the gateway off of using an underlying ingress gateway, add functionality to generate API Gateway natively. Previously this code was all in a single branch, but for ease of tracking and reviewing, breaking this code into 4 separate PRs.
Testing & Reproduction steps
Links
https://hermes.hashicorp.services/document/15kzB5wqhzzPOMWCD8SphpcYUAigUmJvV6R6y2MZUnJU
PR Checklist