Modify getReadyUpstreams to filter upstreams by listener#17410
Merged
nathancoleman merged 1 commit intoNET-3673-endpointsFromSnapshotAPIGatewayfrom May 18, 2023
Merged
Conversation
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.
nathancoleman
added
pr/no-changelog
nathancoleman
commented
May 18, 2023
|
|
||
| type APIGatewayListenerKey = IngressListenerKey | ||
|
|
||
| func APIGatewayListenerKeyFromListener(l structs.APIGatewayListener) APIGatewayListenerKey { |
Member
Author
There was a problem hiding this comment.
This change is just for ease of use, no functional impact
nathancoleman
commented
May 18, 2023
| for _, routeRef := range boundListener.Routes { | ||
| //get upstreams | ||
| upstreamMap := cfgSnap.APIGateway.Upstreams[routeRef] | ||
| for _, upstreams := range upstreamMap { |
Member
Author
There was a problem hiding this comment.
The big change is removing this for loop - instead of picking the upstreams specific to the listener, we were looping through all upstreams for all listeners and adding them
sarahalsmiller
added a commit
that referenced
this pull request
May 19, 2023
* endpoints xds cluster configuration * resources test fix * fix reversion in resources_test * Update agent/proxycfg/api_gateway.go Co-authored-by: John Maguire <john.maguire@hashicorp.com> * gofmt * Modify getReadyUpstreams to filter upstreams by listener (#17410) 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. * Update agent/proxycfg/api_gateway.go Co-authored-by: Nathan Coleman <nathan.coleman@hashicorp.com> * Restore import blocking * Skip to next route if route has no upstreams * cleanup * change set from bool to empty struct --------- Co-authored-by: John Maguire <john.maguire@hashicorp.com> Co-authored-by: Nathan Coleman <nathan.coleman@hashicorp.com>
nathancoleman
added a commit
that referenced
this pull request
May 22, 2023
* endpoints xds cluster configuration * clusters xds native generation * resources test fix * fix reversion in resources_test * Update agent/proxycfg/api_gateway.go Co-authored-by: John Maguire <john.maguire@hashicorp.com> * gofmt * Modify getReadyUpstreams to filter upstreams by listener (#17410) 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. * Update agent/proxycfg/api_gateway.go Co-authored-by: Nathan Coleman <nathan.coleman@hashicorp.com> * Restore import blocking * Undo removal of unrelated code --------- Co-authored-by: John Maguire <john.maguire@hashicorp.com> Co-authored-by: Nathan Coleman <nathan.coleman@hashicorp.com>
sarahalsmiller
added a commit
that referenced
this pull request
May 22, 2023
* API Gateway XDS Primitives, endpoints and clusters (#17002) * XDS primitive generation for endpoints and clusters Co-authored-by: Nathan Coleman <nathan.coleman@hashicorp.com> * server_test * deleted extra file * add missing parents to test --------- Co-authored-by: Nathan Coleman <nathan.coleman@hashicorp.com> * Routes for API Gateway (#17158) * XDS primitive generation for endpoints and clusters Co-authored-by: Nathan Coleman <nathan.coleman@hashicorp.com> * server_test * deleted extra file * add missing parents to test * checkpoint * delete extra file * httproute flattening code * linting issue * so close on this, calling for tonight * unit test passing * add in header manip to virtual host * upstream rebuild commented out * Use consistent upstream name whether or not we're rebuilding * Start working through route naming logic * Fix typos in test descriptions * Simplify route naming logic * Simplify RebuildHTTPRouteUpstream * Merge additional compiled discovery chains instead of overwriting * Use correct chain for flattened route, clean up + add TODOs * Remove empty conditional branch * Restore previous variable declaration Limit the scope of this PR * Clean up, improve TODO * add logging, clean up todos * clean up function --------- Co-authored-by: Nathan Coleman <nathan.coleman@hashicorp.com> * checkpoint, skeleton, tests not passing * checkpoint * endpoints xds cluster configuration * resources test fix * fix reversion in resources_test * checkpoint * Update agent/proxycfg/api_gateway.go Co-authored-by: John Maguire <john.maguire@hashicorp.com> * unit tests passing * gofmt * add deterministic sorting to appease the unit test gods * remove panic * Find ready upstream matching listener instead of first in list * Clean up, improve TODO * Modify getReadyUpstreams to filter upstreams by listener (#17410) 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. * clean up todos, references to api gateway in listeners_ingress * merge in Nathan's fix * Update agent/consul/discoverychain/gateway.go * cleanup current todos, remove snapshot manipulation from generation code * Update agent/structs/config_entry_gateways.go Co-authored-by: Thomas Eckert <teckert@hashicorp.com> * Update agent/consul/discoverychain/gateway.go Co-authored-by: Nathan Coleman <nathan.coleman@hashicorp.com> * Update agent/consul/discoverychain/gateway.go Co-authored-by: Nathan Coleman <nathan.coleman@hashicorp.com> * Update agent/proxycfg/snapshot.go Co-authored-by: Nathan Coleman <nathan.coleman@hashicorp.com> * clarified header comment for FlattenHTTPRoute, changed RebuildHTTPRouteUpstream to BuildHTTPRouteUpstream * simplify cert logic * Delete scratch * revert route related changes in listener PR * Update agent/consul/discoverychain/gateway.go * Update agent/proxycfg/snapshot.go * clean up uneeded extra lines in endpoints --------- Co-authored-by: Nathan Coleman <nathan.coleman@hashicorp.com> Co-authored-by: John Maguire <john.maguire@hashicorp.com> Co-authored-by: Thomas Eckert <teckert@hashicorp.com>
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
1 participant
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.
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.