Switch Node.js SDK grpc dependency to grpc-js #1529

steven-supersolid · 2020-05-08T11:55:42Z

What type of PR is this?
/kind feature

What this PR does / Why we need it:
The original node.js grpc library has been replaced by a new implementation and after a year the original library will be deprecated. The recommendation from the gRPC team is to update.
https://grpc.io/blog/grpc-js-1.0/

Which issue(s) this PR fixes:
Closes #1489
Closes #1490

Special notes for your reviewer:

update generation of libs - now we do not use protoc installed in the base image
update libs
update sdk
update and fix tests - some tests failed due to the port test modifying state
change emitter to stream - the new library creates a stream instead of an event emitter
update example to 1.5.0
update node dependencies and audit

…sts. change emitter to stream. update example to 1.5.0

steven-supersolid · 2020-05-08T12:00:18Z

@BradfordMedeiros do you want to take a look at the change from emitters to streams? When updating the library call.cancel was no longer available and destroy is the recommended way to close a stream

agones-bot · 2020-05-08T12:13:36Z

Build Succeeded 👏

Build Id: f105e604-73e9-441d-83e0-f4d02c8429fa

The following development artifacts have been built, and will exist for the next 30 days:

image: gcr.io/agones-images/agones-controller:1.6.0-e8dd393
image: gcr.io/agones-images/agones-sdk:1.6.0-e8dd393
image: gcr.io/agones-images/agones-ping:1.6.0-e8dd393
Linux C++ SDK (build): agonessdk-1.6.0-e8dd393-linux-arch_64.tar.gz
SDK Server: agonessdk-server-1.6.0-e8dd393.zip

A preview of the website (the last 30 builds are retained):

https://e8dd393-dot-preview-dot-agones-images.appspot.com/

To install this version:

git fetch https://github.com/GoogleCloudPlatform/agones.git pull/1529/head:pr_1529 && git checkout pr_1529
helm install ./install/helm/agones --namespace agones-system --name agones --set agones.image.tag=1.6.0-e8dd393

agones-bot · 2020-05-08T12:18:15Z

Build Succeeded 👏

Build Id: dc0cc964-0d0a-41ea-ac73-77a8229bef6c

The following development artifacts have been built, and will exist for the next 30 days:

image: gcr.io/agones-images/agones-controller:1.6.0-3b94999
image: gcr.io/agones-images/agones-sdk:1.6.0-3b94999
image: gcr.io/agones-images/agones-ping:1.6.0-3b94999
Linux C++ SDK (build): agonessdk-1.6.0-3b94999-linux-arch_64.tar.gz
SDK Server: agonessdk-server-1.6.0-3b94999.zip

A preview of the website (the last 30 builds are retained):

https://3b94999-dot-preview-dot-agones-images.appspot.com/

To install this version:

git fetch https://github.com/GoogleCloudPlatform/agones.git pull/1529/head:pr_1529 && git checkout pr_1529
helm install ./install/helm/agones --namespace agones-system --name agones --set agones.image.tag=1.6.0-3b94999

markmandel · 2020-05-08T20:16:33Z

build/build-sdk-images/node/gen.sh

-protoc -I ${googleapis} -I ${sdk} --grpc_out=minimum_node_version=12:./sdks/nodejs/lib --plugin=protoc-gen-grpc=`which grpc_node_plugin` sdk.proto
-protoc -I ${googleapis} -I ${sdk} --js_out=import_style=commonjs,binary:./sdks/nodejs/lib sdk.proto ${googleapis}/google/api/annotations.proto ${googleapis}/google/api/http.proto
+npm install --unsafe-perm --global grpc-tools
+grpc_tools_node_protoc --proto_path=${googleapis} --proto_path=${sdk} --js_out=import_style=commonjs,binary:./sdks/nodejs/lib google/api/annotations.proto google/api/http.proto


Generation command makes sense 👍

markmandel · 2020-05-08T20:20:35Z

build/build-sdk-images/node/gen.sh

@@ -25,8 +25,9 @@ googleapis=/go/src/agones.dev/agones/proto/googleapis

 cd /go/src/agones.dev/agones

-protoc -I ${googleapis} -I ${sdk} --grpc_out=minimum_node_version=12:./sdks/nodejs/lib --plugin=protoc-gen-grpc=`which grpc_node_plugin` sdk.proto
-protoc -I ${googleapis} -I ${sdk} --js_out=import_style=commonjs,binary:./sdks/nodejs/lib sdk.proto ${googleapis}/google/api/annotations.proto ${googleapis}/google/api/http.proto
+npm install --unsafe-perm --global grpc-tools


Curious - do you think we should bake grpc-tools into the Dockerfile ?

Also, I think it would be good to lock this to a version, to avoid any weirdness between runs.

I started out this way and had some issues but now it's working gen.sh then will take a look at moving it. Will add the latest version too.

Another way of doing this is to add to the package.json of the sdk and have a build/pre-build step there. One advantage of this is that a dependency bot could keep it up to date (with other dependencies), also the SDK would be self contained and it's clear from the package.json which version of grpc-tools was used. We would then update gen.sh to just run npm run build or similar.

This is just an idea though and would not do as part of this code review :)

agones-bot · 2020-05-09T09:58:05Z

Build Failed 😱

Build Id: 800edb96-9d8f-4fb1-b011-28ad0a1ab1a7

To get permission to view the Cloud Build view, join the agones-discuss Google Group.

agones-bot · 2020-05-09T12:06:06Z

Build Failed 😱

Build Id: a5ec2a42-e595-4d7f-9bda-3a5003fe04fa

To get permission to view the Cloud Build view, join the agones-discuss Google Group.

steven-supersolid · 2020-05-11T08:32:41Z

The test failure may be due to this grpc/grpc-node#1362
Solution (not great) grpc/grpc-node#922 (comment)

agones-bot · 2020-05-11T08:40:58Z

Build Failed 😱

Build Id: 72df94e2-ea2c-4367-818b-a9075f779a24

To get permission to view the Cloud Build view, join the agones-discuss Google Group.

agones-bot · 2020-05-11T09:34:23Z

Build Failed 😱

Build Id: 888ba103-5e09-4dda-8b30-843b96db44bd

To get permission to view the Cloud Build view, join the agones-discuss Google Group.

steven-supersolid · 2020-05-11T09:41:05Z

Different tests are failing, most recent was e2e-runner

--- FAIL: TestGameServerReserve (161.02s)
    gameserver_test.go:632: 
        	Error Trace:	gameserver_test.go:632
        	Error:      	Received unexpected error:
        	            	timed out waiting for the condition
        	Test:       	TestGameServerReserve

markmandel · 2020-05-11T20:58:55Z

Wow, haven't seen TestGameServerReserve be flaky in a WHILE!

Can you rebase against master and push, github isn't giving me the options because I don't know why.

agones-bot · 2020-05-12T08:31:59Z

Build Failed 😱

Build Id: 6519ff55-222a-490a-a8d0-3b6119d18f85

To get permission to view the Cloud Build view, join the agones-discuss Google Group.

steven-supersolid · 2020-05-12T09:39:05Z

Same again. Perhaps I can try locally

--- FAIL: TestGameServerReserve (150.63s)
    gameserver_test.go:632: 
        	Error Trace:	gameserver_test.go:632
        	Error:      	Received unexpected error:
        	            	timed out waiting for the condition
        	Test:       	TestGameServerReserve

agones-bot · 2020-05-12T13:56:08Z

Build Succeeded 👏

Build Id: 8ff7d998-2ae0-4e26-9be0-f47d0ce3fea3

The following development artifacts have been built, and will exist for the next 30 days:

image: gcr.io/agones-images/agones-controller:1.6.0-d7b1f38
image: gcr.io/agones-images/agones-sdk:1.6.0-d7b1f38
image: gcr.io/agones-images/agones-ping:1.6.0-d7b1f38
Linux C++ SDK (build): agonessdk-1.6.0-d7b1f38-linux-arch_64.tar.gz
SDK Server: agonessdk-server-1.6.0-d7b1f38.zip

A preview of the website (the last 30 builds are retained):

https://d7b1f38-dot-preview-dot-agones-images.appspot.com/

To install this version:

git fetch https://github.com/GoogleCloudPlatform/agones.git pull/1529/head:pr_1529 && git checkout pr_1529
helm install ./install/helm/agones --namespace agones-system --name agones --set agones.image.tag=1.6.0-d7b1f38

markmandel · 2020-05-12T14:06:35Z

There we go!

I marked is as breaking as more of a "just in case", and as a reminder to mention it in the release notes.

This looks good now!

markmandel

let's go!

google-oss-robot · 2020-05-12T14:07:04Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: markmandel, steven-supersolid

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [markmandel]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

* update generation of libs. update libs. update sdk. update and fix tests. change emitter to stream. update example to 1.5.0 * Move grpc-tools installation to Dockerfile and fix version * hopefully fix node-pre-gyp bug

update generation of libs. update libs. update sdk. update and fix te…

e8dd393

…sts. change emitter to stream. update example to 1.5.0

google-oss-robot requested review from EricFortin and roberthbailey May 8, 2020 11:55

google-oss-robot added the size/XXL label May 8, 2020

Merge branch 'master' into grpc-js

3b94999

markmandel reviewed May 8, 2020

View reviewed changes

Move grpc-tools installation to Dockerfile and fix version

7dba614

Merge branch 'master' into grpc-js

484b702

hopefully fix node-pre-gyp bug

6beadf4

Merge branch 'master' into grpc-js

f8a1a49

Merge branch 'master' into grpc-js

d7b1f38

markmandel added area/user-experience Pertaining to developers trying to use Agones, e.g. SDK, installation, etc kind/breaking Breaking change labels May 12, 2020

markmandel approved these changes May 12, 2020

View reviewed changes

google-oss-robot assigned markmandel May 12, 2020

google-oss-robot added the lgtm label May 12, 2020

google-oss-robot added the approved label May 12, 2020

markmandel added this to the 1.6.0 milestone May 12, 2020

markmandel merged commit 6c909e8 into googleforgames:master May 12, 2020

steven-supersolid deleted the grpc-js branch October 25, 2020 18:43

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Switch Node.js SDK grpc dependency to grpc-js #1529

Switch Node.js SDK grpc dependency to grpc-js #1529

steven-supersolid commented May 8, 2020

steven-supersolid commented May 8, 2020

agones-bot commented May 8, 2020

agones-bot commented May 8, 2020

markmandel May 8, 2020

markmandel May 8, 2020

markmandel May 8, 2020

steven-supersolid May 9, 2020

agones-bot commented May 9, 2020

agones-bot commented May 9, 2020

steven-supersolid commented May 11, 2020

agones-bot commented May 11, 2020

agones-bot commented May 11, 2020

steven-supersolid commented May 11, 2020

markmandel commented May 11, 2020

agones-bot commented May 12, 2020

steven-supersolid commented May 12, 2020

agones-bot commented May 12, 2020

markmandel commented May 12, 2020

markmandel left a comment

google-oss-robot commented May 12, 2020

Switch Node.js SDK grpc dependency to grpc-js #1529

Switch Node.js SDK grpc dependency to grpc-js #1529

Conversation

steven-supersolid commented May 8, 2020

steven-supersolid commented May 8, 2020

agones-bot commented May 8, 2020

agones-bot commented May 8, 2020

markmandel May 8, 2020

Choose a reason for hiding this comment

markmandel May 8, 2020

Choose a reason for hiding this comment

markmandel May 8, 2020

Choose a reason for hiding this comment

steven-supersolid May 9, 2020

Choose a reason for hiding this comment

agones-bot commented May 9, 2020

agones-bot commented May 9, 2020

steven-supersolid commented May 11, 2020

agones-bot commented May 11, 2020

agones-bot commented May 11, 2020

steven-supersolid commented May 11, 2020

markmandel commented May 11, 2020

agones-bot commented May 12, 2020

steven-supersolid commented May 12, 2020

agones-bot commented May 12, 2020

markmandel commented May 12, 2020

markmandel left a comment

Choose a reason for hiding this comment

google-oss-robot commented May 12, 2020