-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
gateway-api: Support GRPCRoute resource #28654
Conversation
/ci-gateway-api |
a2eaf9e
to
b712093
Compare
/ci-gateway-api |
b712093
to
7add21c
Compare
/ci-gateway-api |
/test |
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.
I came for the docs, and the docs looked good.
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.
Trivial ack for ci-structure.
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.
Approve for install/kubernetes
changes.
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.
nice work @sayboras 🥳
i took a first look and added some comments inline. I'm not sure whether the gateway-api conformance tests are already enabled without having added the grpcRoute feature to the list of supported features. Might be worth to activate that first and have some feedback from the conformance tests too 😄 (comment: #28654 (comment))
Signed-off-by: Tam Mach <tam.mach@cilium.io>
Signed-off-by: Tam Mach <tam.mach@cilium.io>
Signed-off-by: Tam Mach <tam.mach@cilium.io>
Signed-off-by: Tam Mach <tam.mach@cilium.io>
Use explicit http config in envoy cluster to avoid the below issue when trying to send curl command to GRPC endpoint. Before ``` < HTTP/1.1 502 Bad Gateway < content-length: 0 < content-type: application/grpc-web-text+proto < grpc-message: upstream connect error or disconnect/reset before headers. reset reason: protocol error < grpc-status: 14 < date: Tue, 17 Oct 2023 13:57:43 GMT < server: envoy ``` After ``` < HTTP/1.1 200 OK < content-type: application/grpc-web-text+proto < x-envoy-upstream-service-time: 0 < date: Tue, 17 Oct 2023 13:57:54 GMT < server: envoy < transfer-encoding: chunked ``` Signed-off-by: Tam Mach <tam.mach@cilium.io>
This should be created as part of recent version of Cilium CLI Signed-off-by: Tam Mach <tam.mach@cilium.io>
This is to add basic GPRCRoute test, till we have more extensive conformance test from upstream. Signed-off-by: Tam Mach <tam.mach@cilium.io>
/test |
46b1d86
to
42ea919
Compare
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.
LGTM, thanks again!
/test |
1 similar comment
/test |
Should this be |
This is to make sure that Gateway is updated whenever there is change in GRPCRoute. Relates: cilium#28654 Signed-off-by: Tam Mach <tam.mach@cilium.io>
This is to make sure that Gateway is updated whenever there is change in GRPCRoute. Relates: cilium#28654 Signed-off-by: Tam Mach <tam.mach@cilium.io>
This is to make sure that Gateway is updated whenever there is change in GRPCRoute. Relates: cilium#28654 Signed-off-by: Tam Mach <tam.mach@cilium.io>
This is to make sure that Gateway is updated whenever there is change in GRPCRoute. Relates: #28654 Signed-off-by: Tam Mach <tam.mach@cilium.io>
Description
Please refer to individual commits for more details.
Fixes: #21928