Skip to content

xds: generate clusters directly from API gateway snapshot#17391

Merged
nathancoleman merged 12 commits into
mainfrom
NET-3674-clustersFromSnapshotAPIGateway
May 22, 2023
Merged

xds: generate clusters directly from API gateway snapshot#17391
nathancoleman merged 12 commits into
mainfrom
NET-3674-clustersFromSnapshotAPIGateway

Conversation

@sarahalsmiller
Copy link
Copy Markdown
Member

@sarahalsmiller sarahalsmiller commented May 17, 2023

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

  • No user facing changes are present, current CI/CD process passing ensures no behavior changes.

Links

https://hermes.hashicorp.services/document/15kzB5wqhzzPOMWCD8SphpcYUAigUmJvV6R6y2MZUnJU

PR Checklist

  • updated test coverage
  • external facing docs updated
  • appropriate backport labels added
  • not a security concern

@sarahalsmiller sarahalsmiller added pr/no-changelog PR does not need a corresponding .changelog entry backport/1.15 labels May 17, 2023
@sarahalsmiller sarahalsmiller requested review from a team and nathancoleman May 17, 2023 17:22
@sarahalsmiller sarahalsmiller changed the base branch from main to NET-3673-endpointsFromSnapshotAPIGateway May 17, 2023 17:22
@github-actions github-actions Bot added the theme/envoy/xds Related to Envoy support label May 17, 2023
sarahalsmiller and others added 4 commits May 17, 2023 16:19
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>
@nathancoleman nathancoleman changed the title API Gateway clusters xds native generation xds: generate clusters directly from API gateway snapshot May 18, 2023
Comment thread agent/xds/clusters.go
return nil, err
}
res, err := s.clustersFromSnapshotIngressGateway(cfgSnap)
res, err := s.clustersFromSnapshotAPIGateway(cfgSnap)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 🎉

Comment thread agent/xds/clusters.go
Comment thread agent/xds/clusters.go
createdClusters := make(map[proxycfg.UpstreamID]bool)
readyUpstreamsList := getReadyUpstreams(cfgSnap)

for _, readyUpstreams := range readyUpstreamsList {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this where we could add cluster creation to serve 5xx responses for non-ready upstreams to match conformance expectations? Not suggesting that be added to the scope of this PR as we don't do that currently, just curious.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep! We could add any handling we like for listeners that aren't ready in the future

@mikemorris
Copy link
Copy Markdown
Contributor

mikemorris commented May 19, 2023

NOTE- this code relies on a helper function introduced in this PR #17390 and tests will not pass until that code is merged.

^ I think this is no longer the case, looks like this is branched off from (and targets) that PR branch, so should be fine, tests appear to be passing.

Base automatically changed from NET-3673-endpointsFromSnapshotAPIGateway to main May 19, 2023 18:51
@nathancoleman nathancoleman enabled auto-merge (squash) May 19, 2023 19:27
@nathancoleman nathancoleman merged commit d34bde0 into main May 22, 2023
@nathancoleman nathancoleman deleted the NET-3674-clustersFromSnapshotAPIGateway branch May 22, 2023 16:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr/no-backport pr/no-changelog PR does not need a corresponding .changelog entry theme/envoy/xds Related to Envoy support

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants