agones icon indicating copy to clipboard operation
agones copied to clipboard

Upgrade gRPC version

Open markmandel opened this issue 5 years ago • 6 comments

We should upgrade gRPC. k8s.io/apiserver would like it to be 1.23.0 (even though we don't use that part of the code), so probably makes sense to align it with what libraries we use are requesting.

(Unless anyone can think of a good reason to go straight to latest?)

https://github.com/googleforgames/agones/blob/master/build/README.md#make-gen-sdk-grpc will be required here to update gRPC based SDKs and make sure they work. Might be worth splitting that work up as well as separate PRs.

Context: https://github.com/googleforgames/agones/pull/1794#discussion_r483975174

markmandel avatar Sep 06 '20 20:09 markmandel

One other thought on this - if we want to upgrade grpc-gateway (maybe to v2?) we should upgrade gRPC to the appropriate version first.

markmandel avatar Mar 25 '21 16:03 markmandel

Reopening this, to cover gRPC SDK tooling.

We did update gRPC for the SDK server, but haven't yet done any of the work to upgrade the codegen for the client sdks and related. They may not change, but usually there is some change in the generated code from gRPC between versions. At the very least we should check.

  • [x] update the SDK base image grpc version and rebuild the image. (this can take a while, have had to do it manually and push it to gcr because cloud build doesn't like how long it takes).
  • [x] regen allocated API endpoints: make gen-allocation-grpc (looks like this one isn't documented)
  • [x] regen all client sdks: make gen-sdk-grpc
  • [x] update the version number in C++ Cmake scripts
  • [x] Probably worth doing a global find for "1.20.1" to see if there is anything else that needs updating.
  • [ ] Fix all the things until they pass 😄

markmandel avatar Jun 25 '21 00:06 markmandel

@markmandel This upgrade would be very welcome 😅 Currently I have to apply a minor patch to gRPC to get the c++ SDK to compile under Alpine Linux: https://github.com/googleforgames/agones/commit/d93e0571e5ef3c82a388efb3d706f723c2d368bc

This wouldn't be required under more recent versions of gRPC (v1.24.0 onwards). See resolved issue https://github.com/grpc/grpc/issues/19964

Kind regards,

Deon

dbotha avatar Jul 08 '21 15:07 dbotha

Noting - this is the current gRPC version we are using server side, so we need to bring the client side SDK tooling inline with this version: https://github.com/googleforgames/agones/blob/main/go.mod#L33

google.golang.org/grpc v1.27.1

markmandel avatar Oct 21 '21 20:10 markmandel

Just noting that there are still a few more checkboxes in the list that need completing before we can close this out. https://github.com/googleforgames/agones/issues/1797#issuecomment-868089034

We might be able to close off "Probably worth doing a global find for "1.20.1" to see if there is anything else that needs updating." - @SaitejaTamma did you end up doing a search? If so, we can tick this item off.

markmandel avatar Nov 19 '21 18:11 markmandel

Yes, we can tick this off

SaitejaTamma avatar Nov 19 '21 18:11 SaitejaTamma

@markmandel - with the recent gRPC updates, can we close this tracking ticket?

roberthbailey avatar Sep 15 '22 15:09 roberthbailey

Yes! Let's do it!

markmandel avatar Sep 15 '22 16:09 markmandel