Fault injection not implementable

[kyessenov]

Current spec allows faults to be applied per HTTPRoute.

Unfortunately, Envoy only allows one fault filter per upstream cluster. That means if you have two HTTP route rules for the same cluster, they'll be affecting each other.

Suggestion is to either:

1. modify route rule spec to align with envoy
2. modify envoy to allow faults per individual routes

[costinm]

I don't know about 'not implementable' - we could generate an extra cluster for the route that has a fault filter.

However the implementation may be too expensive or complicated - so maybe we should change the API to keep fault at cluster level.

[kyessenov]

That's a very wasteful solution on a self imposed problem :)

…

On Mon, Feb 26, 2018, 9:01 PM Costin Manolache ***@***.***> wrote: I don't know about 'not implementable' - we could generate an extra cluster for the route that has a fault filter. However the implementation may be too expensive or complicated - so maybe we should change the API to keep fault at cluster level. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#388 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AJGIxqk_NE72YHbDdckHNThGaUZYWhYmks5tY4wZgaJpZM4SUWot> .

[costinm]

If we believe users need to set fault injection at route level - I wouldn't say it's self imposed problem. However - we already provide the means for the user to achieve this ( by defining his own cluster with fault ) - so maybe we don't need the extra complexity.

…

On Mon, Feb 26, 2018 at 9:38 PM, Kuat ***@***.***> wrote: That's a very wasteful solution on a self imposed problem :) On Mon, Feb 26, 2018, 9:01 PM Costin Manolache ***@***.***> wrote: > I don't know about 'not implementable' - we could generate an extra > cluster for the route that has a fault filter. > > However the implementation may be too expensive or complicated - so maybe > we should change the API to keep fault at cluster level. > > — > You are receiving this because you authored the thread. > Reply to this email directly, view it on GitHub > <#388 (comment)>, or mute > the thread > <https://github.com/notifications/unsubscribe-auth/AJGIxqk_ NE72YHbDdckHNThGaUZYWhYmks5tY4wZgaJpZM4SUWot> > . > — You are receiving this because you were assigned. Reply to this email directly, view it on GitHub <#388 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAFI6plLGjOHnvZzCNaPhOU_h-oJjtGdks5tY5TOgaJpZM4SUWot> .

[louiscryan]

I would be fine with moving HTTPFaultInjection into DestinationRule.TrafficPolicy

[kyessenov]

@rshriram if you have time, what do you think? Should we fix envoy's fault injection or fix istio's API?

[mandarjog]

the current fault injection filter should accept per route configuration.
We are moving all our filters to support that anyway. Mixerclient filter / envoy rbac filter etc...

[kyessenov]

After talking to @rshriram , we'll park implementing v1alpha2 fault until it's corrected in envoy to apply config per route, not per upstream cluster. Upstream cluster granularity is too coarse for faults, and it does not match CDS-semantics in Envoy.

[rshriram]

Thinking more about this, fault filter cna filter based on HTTP headers and upstream clusters. So, why is this an issue? If two routes refer to same upstream cluster, then logically they should have some difference (header or prefix match). And these conditions can be added to the fault filter.
Keep in mind that filters in envoy can be stacked. You can have N fault filters, with different match conditions.. Only those that match will be triggered.

[rshriram]

refer to #405

[kyessenov]

I think they are still not implementable. Please reference the envoy fix here when you close it.

[rshriram]

envoyproxy/envoy#3084