-
Notifications
You must be signed in to change notification settings - Fork 347
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
proposal: BackendTrafficPolicy #1821
Comments
relates to #1492 |
thanks for raising this issue @AliceProxy, adding follow up questions about the higher level API (
|
In this scenario, policy B (attached to the xRoute) would win only for fields that they both configure. IE: if policy A configured
Using
Yep it definitely would make debugging a bit harder, but I think there are ways to solve for this. For example, we could add a new functionality to
Great idea, lets go ahead and do that, then shall we plan to keep the other resource as I think for points 1 and 2 let's try to get some more folks to weigh in and then go with whatever seems to resonate with more people for each one. Or maybe someone else will have additional ideas. |
|
@zirain @AliceProxy and I discussed this issue in the community meeting yesterday and managed to build some consensus on some of the above points
|
thanks for updating the API name, was chatting with @ZackButcher who mentioned that |
I was thinking about this too -- I kind of like |
|
All the choices have some ambiguity (remember that Envoy's use of "upstream" and "downstream" is the reverse of common usage in some other places). |
Oh @kflynn are you recommending replacing |
I like |
RIght, @arkodg – use |
I'd also like to note that I am still strongly opposed to having In real-world usage with Emissary, having the ability to set defaults has repeatedly proven critical. Attaching a TrafficPolicy to a Gateway meets this need, and allows overriding it at the Route level by attaching exactly the same kind of resource to the Route: there's no need to put everything under a (In fact, requiring them to state I have a harder time coming up with real-world use cases for |
I agree with you here, this decision will make us deviate from the upstream recommendation which was my only hesitation, but there isn't anyone who has voiced a strong need for explicit |
IMO, adding the |
top features I would like in this API
we shouldnt express |
I think we should go ahead and define retries in the BackendTrafficPolicy for now, since we're not sure yet that that can make it into Gateway API 1.0 -- just let's bear in mind that there's a parallel effort there and we may need to adapt to the upstream version when it happens. |
Outlier detection and fault injection are definitely very interesting – worth noting that outlier detection has some interaction with the load balancing policy as well, but both would be great to have and I think both belong here. |
Relates to envoyproxy#1821 & envoyproxy#2006 Signed-off-by: Arko Dasgupta <arko@tetrate.io>
Relates to envoyproxy#1821 & envoyproxy#2006 Signed-off-by: Arko Dasgupta <arko@tetrate.io>
* Add rateLimit to BackendTrafficPolicy Relates to #1821 & #2006 Signed-off-by: Arko Dasgupta <arko@tetrate.io> * tests Signed-off-by: Arko Dasgupta <arko@tetrate.io> * lint Signed-off-by: Arko Dasgupta <arko@tetrate.io> * fix build Signed-off-by: Arko Dasgupta <arko@tetrate.io> --------- Signed-off-by: Arko Dasgupta <arko@tetrate.io>
Relates to
#1492
#1821
#1845
What is this?
BackendTrafficPolicy
is a proposal for a new policy attachment resource that can be applied toGateways
andxRoute
resources. It is meant to supplement a lot of config that Gateway API currently lacks while also providing a way to specify global defaults. It is focused on configuring the behaviour of traffic between EnvoyProxy and the backend service.What if Gateway API implements a GEP that delivers the same functionality as some of the fields of this resource?
If Gateway improves any of their existing resources to deliver functionality that meets all of the needs of any of the below config,
then we will deprecate the field in this resource and use the Gateway API config instead. Ideally, a lot of this config can be
upstreamed into Gateway API in some form eventually since this resource only exists to solve areas where Gateway API is lacking for the needs of Envoy Gateway.
What about traffic between EnvoyProxy and the downstream clients?
I will be opening up a sister issue proposing an
InboundTrafficPolicy
(or similar) resource here shortly that will focus on this use case.What happens when I attach it to a
Gateway
resource?If you apply this resource to a
Gateway
resource, any config that applies to the "listener level" will be configured there.A lot of the config is more "route level," and so for those fields, they may not do anything if all you have is a Gateway, but they will be used as the default configuration for all children
xRoute
resources of theGateway
this resource is attached to.What happens when I attach it to an
xRoute
resource (HTTPRoute
,GRPCRoute
, etc.)If you apply this config to an
xRoute
resource, it will apply route-level config to this specific route.If you have an
BackendTrafficPolicy
attached to both aGateway
and anxRoute
resource, then config from the oneattached to the
xRoute
will win in a conflict. The "merging" in this case, will be primitive. If you have any sort ofconfig for a top-level field such as
circuitBreakers
, then the whole field will be replaced rather than comparing all of the subfieldsand merging/patching them.
Can you use multiple
BackendTrafficPolicy
resources at the same time?You may use multiple
BackendTrafficPolicies
that target different resources, but you may not attach multipleBackendTrafficPolicies
to the same resource. For example, it is invalid to attach two
BackendTrafficPolicies
to the sameGateway
. Merging config in this scenario would quickly become convoluted and confusing, and there are many edge-case scenarios that make supporting this undesirable.How will this be developed/implemented?
This issue serves as a proposal for the high-level design and fields of the API, but that does not mean that every field included below will be immediately available. My plan for the implementation is to add and implement one high-level field at a time until the whole resource is implemented.
API outline
Things that I considered but felt might make better
xRoute
externalRef
filtersNote that I explicitly chose to use the above
targets
field instead of the Gateway API standard oftargetRef
/targetRefs
.Using
targetRef
limits the policy to being attached to one object and it very quickly becomes very annoying to have to create a newBackendTrafficPolicy
for each resource you want to attach it to. A very common use case is creating oneBackendTrafficPolicy
and attaching it to severalGateways
,HTTPRoutes
, etc.Looking at
targetRefs
, I feel like it also quickly becomes very annoying to have to manually specify each resourceyou want the
BackendTrafficPolicy
to attach to. I can see many users wanting to create anBackendTrafficPolicy
that attaches to all theirHTTPRoutes
but notGRPCRoutes
(or some similar variation that enables efficient re-use of a singleBackendTrafficPolicy
). When you have hundreds or morexRoutes
, this becomes entirely unmanageable withtargetRefs
because defining each one individually is a pain. Thetargets
field I added functions as a compromise betweentargetRefs
and a Kubernetesselector
, but it likely needs further refinement. Definitely looking for feedback/input here.Problem: some of this config does not make sense to configure at a route level or gateway level.
For example, it doesn't make any sense to configure gRPC health checks at the Gateway level since it requires knowing the service name, which will be different for each route. Likewise, it does not make sense to configure
observability.metrics.statsPrefix
at the route level since that is config that is for an entire listener. I think there are a couple of potential solutions here:I don't like this solution but it is one way of handling it
Open Questions
For example, it doesn't make any sense to configure gRPC health checks at the Gateway level since it requires knowing the service name, which will be different for each route. Likewise, it does not make sense to configure
observability.metrics.statsPrefix
at the route level since that is config that is for an entire listener. I think there are a couple of potential solutions here:Gateway
, but you can configure multiple listeners on a singleGateway
. Realistically, this resource makes the most sense targeting listeners instead of forcing this to apply to the entire gateway. There should be a way to specify which listeners it applies to so that you can have different config for each listener.Gateway
and then adding in support for @zhaohuabing's SectionName to PolicyTargetReference enhancement when it lands and becomes available.Edits
cors
andbypassAuth
configurations moved to the proposal forAuthPolicy
resource.targets
switched to a singletargetRef
for now, but I do think we must solve for the use-case of applying anBackendTrafficPolicy
to multiple resources in a flexible way for the resource to be truly useful.UpstreamTrafficPolicy
toBackendTrafficPolicy
to reduce potential confusion about the traffic that this policy affects.The text was updated successfully, but these errors were encountered: