Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

xds: don't attempt to load-balance sessions for local proxies #15789

Merged
merged 7 commits into from
Jan 18, 2023

Conversation

boxofrad
Copy link
Contributor

Description

Previously, we'd begin a session with the xDS concurrency limiter regardless of whether the proxy was registered in the catalog or in the server's local agent state.

This caused problems for users who run consul connect envoy directly against a server rather than a client agent, as the server's locally registered proxies wouldn't be included in the limiter's capacity.

Now, the ConfigSource is responsible for beginning the session and we only do so for services in the catalog (bypassing the limiter for locally-registered proxies).

Fixes: #15753

Also upgrades mockery to v2.15.0, as this was required to regenerate the mocks on Go 1.19.

Testing & Reproduction steps

  • Run a Consul cluster with >1 server.
  • Register the majority of the proxies to 1 server.
  • Run consul connect envoy for each proxy.
  • Previously you'd see "this server has too many xDS streams open" in the Envoy logs.
  • Now it should work as expected.

@boxofrad boxofrad added the backport-inactive/1.14 This release series is no longer active label Dec 14, 2022
@github-actions github-actions bot added theme/agent-cache Agent Cache theme/connect Anything related to Consul Connect, Service Mesh, Side Car Proxies theme/envoy/xds Related to Envoy support labels Dec 14, 2022
@boxofrad boxofrad force-pushed the boxofrad/bugfix-server-local-limit branch from e1f73a3 to 5ce183d Compare December 14, 2022 16:28
@mkeeler mkeeler removed their request for review December 20, 2022 16:07
@boxofrad boxofrad force-pushed the boxofrad/bugfix-server-local-limit branch from 2527368 to d66663f Compare January 4, 2023 13:53
@vercel
Copy link

vercel bot commented Jan 4, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
consul 🔄 Building (Inspect) Jan 4, 2023 at 5:03PM (UTC)

@boxofrad boxofrad force-pushed the boxofrad/bugfix-server-local-limit branch from 5240ae7 to 8679e2c Compare January 5, 2023 10:29
Previously, we'd begin a session with the xDS concurrency limiter
regardless of whether the proxy was registered in the catalog or in
the server's local agent state.

This caused problems for users who run `consul connect envoy` directly
against a server rather than a client agent, as the server's locally
registered proxies wouldn't be included in the limiter's capacity.

Now, the `ConfigSource` is responsible for beginning the session and we
only do so for services in the catalog.

Fixes: #15753
@boxofrad boxofrad force-pushed the boxofrad/bugfix-server-local-limit branch from 8679e2c to d19960c Compare January 9, 2023 10:52
@boxofrad boxofrad requested a review from rboyer January 9, 2023 11:09
@boxofrad
Copy link
Contributor Author

boxofrad commented Jan 9, 2023

Hey @rboyer 👋🏻

I've fixed CI and the missing End call you spotted. Would you mind giving this another 👀 please?

@boxofrad boxofrad enabled auto-merge (squash) January 10, 2023 12:00
@boxofrad boxofrad merged commit 7a55de3 into main Jan 18, 2023
@boxofrad boxofrad deleted the boxofrad/bugfix-server-local-limit branch January 18, 2023 18:33
boxofrad added a commit that referenced this pull request Jan 18, 2023
Previously, we'd begin a session with the xDS concurrency limiter
regardless of whether the proxy was registered in the catalog or in
the server's local agent state.

This caused problems for users who run `consul connect envoy` directly
against a server rather than a client agent, as the server's locally
registered proxies wouldn't be included in the limiter's capacity.

Now, the `ConfigSource` is responsible for beginning the session and we
only do so for services in the catalog.

Fixes: #15753
boxofrad added a commit that referenced this pull request Jan 19, 2023
#16004)

Previously, we'd begin a session with the xDS concurrency limiter
regardless of whether the proxy was registered in the catalog or in
the server's local agent state.

This caused problems for users who run `consul connect envoy` directly
against a server rather than a client agent, as the server's locally
registered proxies wouldn't be included in the limiter's capacity.

Now, the `ConfigSource` is responsible for beginning the session and we
only do so for services in the catalog.

Fixes: #15753

Co-authored-by: Dan Upton <daniel@floppy.co>
skpratt pushed a commit that referenced this pull request Jan 25, 2023
Previously, we'd begin a session with the xDS concurrency limiter
regardless of whether the proxy was registered in the catalog or in
the server's local agent state.

This caused problems for users who run `consul connect envoy` directly
against a server rather than a client agent, as the server's locally
registered proxies wouldn't be included in the limiter's capacity.

Now, the `ConfigSource` is responsible for beginning the session and we
only do so for services in the catalog.

Fixes: #15753
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-inactive/1.14 This release series is no longer active theme/agent-cache Agent Cache theme/connect Anything related to Consul Connect, Service Mesh, Side Car Proxies theme/envoy/xds Related to Envoy support
Projects
None yet
Development

Successfully merging this pull request may close these issues.

connect: ResourceExhausted desc = this server has too many xDS streams open, please try another
2 participants