Skip to content
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

Switch the core plugin gRPC service to connect gRPC. #6148

Merged
merged 3 commits into from
Mar 29, 2023

Conversation

absoludity
Copy link
Contributor

@absoludity absoludity commented Mar 29, 2023

Description of the change

This is split out from the investigation done in #6097 and will be followed by a bunch more switching other services and plugins away from the improbable-eng gRPC backend (see #6013)

Benefits

This PR sets a clear example of switching one of the gRPC servers, the core plugin service, away from the improbable-eng server to the connect gRPC server. The switch of the server is trivial, the difficulty was in getting our kubeapps-apis service working correctly with a combination of improbable and connect gRPC servers. As a side-effect, I updated the liveness and readiness checks to use a grpc request (since we no longer have the gateway endpoint for the core plugin service).

Initially I'd tried using the existing server as the default and routing certain routes for transitioned servers to the new handler, but reading http2 headers requires writing the http2 settings frame, which meant the new handler, for some reason that I wasn't able to determine, did not accept the connections.

I then switched to default to the new server and proxying to the old improbable for unhandled routes, which worked for all requests that are initiated by the dashboard, but not those initiated from within the kubeapps-apis server itself (like where the GetResources handler makes a gRPC request to the packages plugin for the resource refs). In those cases it failed. I initially assumed this was something to do with the proxying, but I couldn't find the issue at first, so instead transitioned the resources plugin over as well (in #6097), which ended up needing much more changes than I wanted in a PR, including handling the different ways auth creds are passed by the two libraries etc.

But even after transitioning the resources plugin, I was still hitting an error for proxied requests (this time when the resources plugin called the packaging plugin). After much more digging, I finally found it was because the golang httputil.ReverseProxy proxies requests not transports. So the gRPC (http2) requests were being proxied using an http1 transport which was resulting in a 404 as the server was not recognising the request (gRPC assumes http2). Once understood, the solution to use a different reverse proxy (just different transport) depending on the request was straight forward.

Once I understood that, I was then able to go back to my original intention of a small PR demonstrating changing just a single service, the plugin service, which is what this PR is :)

Possible drawbacks

We lose the Gateway ReST-ish http endpoints. The only place we were using the gateway http endponts was in the liveness/readiness checks, which I've switched here too. We do publish the openapi docs for the gateway, but have not made any commitment to supporting it. Some devs have used the gateway with Postman for testing kubeapps-apis locally, but postman also supports gRPC now for a year.

Actually, I may even be able to re-use the proxy that I've setup here to keep the gateway available too. I'll check.

EDIT: Indeed we can - #6150

Applicable issues

Additional information

I'll follow up with a PR for switching the resources gRPC service, then PRs for packaging, repository and plugins etc.

Signed-off-by: Michael Nelson <minelson@vmware.com>
@netlify
Copy link

netlify bot commented Mar 29, 2023

Deploy Preview for kubeapps-dev canceled.

Name Link
🔨 Latest commit 20c2294
🔍 Latest deploy log https://app.netlify.com/sites/kubeapps-dev/deploys/6423c402e534dc000838ca28

Signed-off-by: Michael Nelson <minelson@vmware.com>
Signed-off-by: Michael Nelson <minelson@vmware.com>
@absoludity absoludity merged commit bade93f into main Mar 29, 2023
@absoludity absoludity deleted the 6013-connect-grpc-backend-2.1 branch March 29, 2023 19:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants