Skip to content

Comments

Retry-gRPC-On From Envoy#456

Closed
cluckett-arista wants to merge 32 commits intoaristanetworks:mainfrom
cluckett-arista:cbl/retry-grpc-on
Closed

Retry-gRPC-On From Envoy#456
cluckett-arista wants to merge 32 commits intoaristanetworks:mainfrom
cluckett-arista:cbl/retry-grpc-on

Conversation

@cluckett-arista
Copy link

Adds retry-grpc-on functionality from Envoy. Allows use of the envoy-supported policies:

  • cancelled
  • deadline-exceeded
  • internal
  • resource-exhausted
  • unavailable

Since Arista is using gRPC for CV calls, we would like to add this functionality to allow retry logic via the proxy.

Lance Austin and others added 24 commits November 12, 2023 19:09
Signed-off-by: Lance Austin <laustin@datawire.io>
…ue to ordinal indexed route name

Signed-off-by: Joe Andaverde <joe@temporal.io>
(cherry picked from commit ecb43a1)
Signed-off-by: Alice Wasko <alicewasko@datawire.io>
Signed-off-by: Alice Wasko <alicewasko@datawire.io>
Signed-off-by: Tenshin Higashi <thigashi@datawire.io>
…nhigashi/route-shift-3.9

Pull in route shift to 3.9
Signed-off-by: Tenshin Higashi <thigashi@datawire.io>
Signed-off-by: Tenshin Higashi <thigashi@datawire.io>
- We switch from alpine to distroless.

This change makes our Python dependencies _dramatically_ easier to install.
At the moment it's a very, very stripped-down image (it doesn't even have "ls")
but we can tune that as needed.

- We switch to stock Envoy.

This gets us out of the business of building Envoy, and lets us take advantage
of the multiarch Envoy builds that are already happening.

There are three downsides:

1. We'll still have to build security changes by hand to test. This is a good
thing to talk to the Envoy project about.
2. Emissary no longer supports our existing custom error responses.
3. Emissary no longer supports our existing header-case mangling.

I don't believe that any open-source user cares about #2 or aristanetworks#3, so I plan to
make it more visibly unsupported rather than supporting it.
`is not None` is required to allow that the user can set falsely value
(like False).
Refactor V3HTTPFilter to support all the AuthService settings when proto
is either HTTP or gRPC.

Especially, "add_auth_headers" and "allowed_request_headers" wasn't
supported with gRPC. Added auth headers are sent to the gRPC service as
metadata, whereas the allowed request headers are added to the gRPC
message.

However, with AuthService gRPC, "allowed_authorization_headers" cannot
be supported. Instead, the gRPC response from the AuthService has fields
to control the behaviour.
…connection_window_size

For upstream clusters, the settings are upstream_initial_stream_window_size
and upstream_initial_connection_window_size.

For downstream clients, the settings are downstream_initial_stream_window_size
and downstream_initial_connection_window_size.

Signed-off-by: tdacquet <tdacquet@arista.com>
…ream one (aristanetworks#453)

* fix unit test for max_concurrent_streams
* rename max_concurrent_streams to be downstream_max_concurrent_streams
* expose new flag for upstream_max_concurrent_streams
* Basic CI

Very simple CI - only runs python unit tests, but hey...
that's something.

* max_concurrent_streams => max_concurrent_requests

* python/tests: Fix test_max_concurrent_streams

Check listener/HCM instead of cluster

* fix whitespace
@cluckett-arista cluckett-arista marked this pull request as draft January 13, 2026 22:22
@cluckett-arista cluckett-arista marked this pull request as ready for review January 13, 2026 22:22
Copy link
Author

Choose a reason for hiding this comment

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

We have problems running the kat tests. The Basic CI only runs the unit tests. Still, these should be added, but I don't have a way to run them. These are just a copy-and-paste from existing kat tests.

"unavailable",
}
# Split on comma and validate each condition
grpc_conditions = [c.strip() for c in retry_grpc_on.split(",")]
Copy link
Author

Choose a reason for hiding this comment

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

Ugh... Envoy allows specifying multiple conditions in a Comma-Separated list:
https://emissary-ingress.dev/docs/3.10/topics/using/retries/
But emissary does not for the HTTP codes. So I'm checking the entries in the (possible) list to account for this.

@dethi dethi changed the base branch from arista/v3.9 to main January 29, 2026 01:46
@dethi
Copy link
Collaborator

dethi commented Jan 29, 2026

Would need to rebase on main (sorry)

@cluckett-arista
Copy link
Author

kind of a mess. Closing out and starting again to try to keep this somewhat readable

@dethi
Copy link
Collaborator

dethi commented Jan 29, 2026

You should be able to just cherry-pick your own commit and port them on a new branch that starts from main

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants