-
Notifications
You must be signed in to change notification settings - Fork 8.1k
Support gRPC fault injection (allow specifying gRPC error codes for fault injection) #39295
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
Conversation
Signed-off-by: su225 <suchithjn22@gmail.com>
Signed-off-by: su225 <suchithjn22@gmail.com>
Signed-off-by: su225 <suchithjn22@gmail.com>
Signed-off-by: su225 <suchithjn22@gmail.com>
Signed-off-by: su225 <suchithjn22@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is great!
@@ -657,6 +658,36 @@ spec: | |||
workloadAgnostic: true, | |||
}) | |||
|
|||
t.RunTraffic(TrafficTestCase{ | |||
name: "fault abort gRPC", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we should try setting sourcematchers and targetMatchers to match.NotNaked, match.NotHeadless
.
Currently we have proxyless grpc opted out; this seems like a REALLY useful feature for proxyless grpc.
I don't know if itworks though, so not certain that will pass
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
…evisions We are adding min istio version for tests related to below PRs as this functionalities were not there in previous revisions :- a. gRPC fault injection(istio#39295) b. Ignore port number in domain matching(istio#40475) c. Tunneling outbound traffic :- new tunnel field got introduced(istio#37968) d. Fix consistent hash based on source IP for TCP proxy(istio#38438) e. Traffic policy load balancer API changes(istio#39742)
…evisions (#40892) We are adding min istio version for tests related to below PRs as this functionalities were not there in previous revisions :- a. gRPC fault injection(#39295) b. Ignore port number in domain matching(#40475) c. Tunneling outbound traffic :- new tunnel field got introduced(#37968) d. Fix consistent hash based on source IP for TCP proxy(#38438) e. Traffic policy load balancer API changes(#39742)
…evisions We are adding min istio version for tests related to below PRs as this functionalities were not there in previous revisions :- a. gRPC fault injection(istio#39295) b. Ignore port number in domain matching(istio#40475) c. Tunneling outbound traffic :- new tunnel field got introduced(istio#37968) d. Fix consistent hash based on source IP for TCP proxy(istio#38438) e. Traffic policy load balancer API changes(istio#39742)
…evisions (#40957) We are adding min istio version for tests related to below PRs as this functionalities were not there in previous revisions :- a. gRPC fault injection(#39295) b. Ignore port number in domain matching(#40475) c. Tunneling outbound traffic :- new tunnel field got introduced(#37968) d. Fix consistent hash based on source IP for TCP proxy(#38438) e. Traffic policy load balancer API changes(#39742) Co-authored-by: Hemendra Teli <hemendrat@google.com>
Please provide a description of this PR:
Original PR - #26059
This PR resurrects it and only adds integration test to complete it.
Brief description: This PR adds support to injecting gRPC faults with given status codes. It builds on the original PR which seemed to be abandoned. The actual changes like validation and config-generation were already there in the original PR (full credits to the person who raised it) and was working fine. The only thing I added is the integration test as suggested by @howardjohn in #26059 (comment) .
Once this goes through we have to remove
$hide_from_docs
for thegrpcStatus
field and update the documentation to indicate that users have to specify upper-cased string likeUNAVAILABLE
as mentioned in https://github.com/grpc/grpc/blob/master/doc/statuscodes.mdOpen question
OK
? This is because it is technically not a fault. We can allow it to be consistent with HTTP, where specifying 200 OK is allowed.