From 46d75e4bfa2b76f6d13150d7ea208a1c38d22b95 Mon Sep 17 00:00:00 2001 From: Mark Mandel Date: Thu, 25 Jul 2019 18:00:26 +0900 Subject: [PATCH] Implement block on connect with Rust SDK - Uses a simple polling mechanism to confirm the connection is live - Add `ready` to the SDK documentation. - Implements and documents a sdk.connect() function - Tweaks the conformance tests to add a random delay to ensure future testing of SDK blocking. Fixes #938 --- build/includes/sdk.mk | 8 +++-- sdks/nodejs/spec/agonesSDK.spec.js | 25 ++++++++++++++ sdks/nodejs/src/agonesSDK.js | 12 +++++++ sdks/rust/src/sdk.rs | 34 ++++++++++++++----- .../en/docs/Guides/Client SDKs/nodejs.md | 12 ++++++- .../en/docs/Guides/Client SDKs/rust.md | 9 +++++ test/sdk/nodejs/testSDKClient.js | 3 ++ test/sdk/rust/.gitignore | 16 +++++++++ test/sdk/rust/src/main.rs | 2 +- 9 files changed, 108 insertions(+), 13 deletions(-) create mode 100644 test/sdk/rust/.gitignore diff --git a/build/includes/sdk.mk b/build/includes/sdk.mk index 8ca9d25c06..768fb3c80b 100644 --- a/build/includes/sdk.mk +++ b/build/includes/sdk.mk @@ -122,15 +122,17 @@ run-sdk-conformance-local: ensure-agones-sdk-image -e "TEST=$(TESTS)" -e "TIMEOUT=$(TIMEOUT)" $(sidecar_tag) # Run SDK conformance test, previously built, for a specific SDK_FOLDER +# Sleeps the start of the sidecar to test that the SDK blocks on connection correctly run-sdk-conformance-no-build: TIMEOUT ?= 30 +run-sdk-conformance-no-build: RANDOM := $(shell bash -c 'echo $$RANDOM') +run-sdk-conformance-no-build: DELAY ?= $(shell bash -c "echo $$[ ($(RANDOM) % 5 ) + 1 ]s") run-sdk-conformance-no-build: TESTS ?= ready,allocate,setlabel,setannotation,gameserver,health,shutdown,watch run-sdk-conformance-no-build: ensure-agones-sdk-image run-sdk-conformance-no-build: ensure-build-sdk-image - sleep 2s && DOCKER_RUN_ARGS="--network=host $(DOCKER_RUN_ARGS)" COMMAND=sdktest $(MAKE) run-sdk-command & \ - docker run -p 59357:59357 -e "ADDRESS=" \ + DOCKER_RUN_ARGS="--network=host $(DOCKER_RUN_ARGS)" COMMAND=sdktest $(MAKE) run-sdk-command & \ + sleep $(DELAY) && docker run -p 59357:59357 -e "ADDRESS=" \ -e "TEST=$(TESTS)" -e "TIMEOUT=$(TIMEOUT)" --net=host $(sidecar_tag) - # Run SDK conformance test for a specific SDK_FOLDER run-sdk-conformance-test: $(MAKE) run-sdk-command COMMAND=build-sdk-test diff --git a/sdks/nodejs/spec/agonesSDK.spec.js b/sdks/nodejs/spec/agonesSDK.spec.js index 52b992b232..6849d5974b 100644 --- a/sdks/nodejs/spec/agonesSDK.spec.js +++ b/sdks/nodejs/spec/agonesSDK.spec.js @@ -26,6 +26,31 @@ describe('agones', () => { agonesSDK = new AgonesSDK(); }); + describe('connect', () => { + it('calls the server and handles success', async () => { + spyOn(agonesSDK.client, 'waitForReady').and.callFake((deadline, callback) => { + let result = new messages.Empty(); + callback(undefined, result); + }); + let result = await agonesSDK.connect(); + expect(agonesSDK.client.waitForReady).toHaveBeenCalled(); + expect(result).toEqual(undefined); + }); + + it('calls the server and handles failure', async () => { + spyOn(agonesSDK.client, 'waitForReady').and.callFake((deadline, callback) => { + callback('error', undefined); + }); + try { + await agonesSDK.connect(); + fail(); + } catch (error) { + expect(agonesSDK.client.waitForReady).toHaveBeenCalled(); + expect(error).toEqual('error'); + } + }); + }); + describe('allocate', () => { it('calls the server and handles success', async () => { spyOn(agonesSDK.client, 'allocate').and.callFake((request, callback) => { diff --git a/sdks/nodejs/src/agonesSDK.js b/sdks/nodejs/src/agonesSDK.js index 40bb950316..3dfd8f8a76 100644 --- a/sdks/nodejs/src/agonesSDK.js +++ b/sdks/nodejs/src/agonesSDK.js @@ -24,6 +24,18 @@ class AgonesSDK { this.emitters = []; } + async connect() { + return new Promise((resolve, reject) => { + this.client.waitForReady(Date.now() + 30000, (error) => { + if (error) { + reject(error); + } else { + resolve(); + } + }) + }); + } + async close() { if (this.healthStream !== undefined) { this.healthStream.destroy(); diff --git a/sdks/rust/src/sdk.rs b/sdks/rust/src/sdk.rs index 279e3e2a7f..81451b8c8b 100644 --- a/sdks/rust/src/sdk.rs +++ b/sdks/rust/src/sdk.rs @@ -12,15 +12,18 @@ // See the License for the specific language governing permissions and // limitations under the License. -use futures::{Future, Sink, Stream}; -use grpcio; use std::sync::{Arc, Mutex}; +use std::thread::sleep; use std::time::Duration; +use futures::{Future, Sink, Stream}; +use grpcio; +use grpcio::CallOption; +use protobuf::Message; + use errors::*; use grpc::sdk; use grpc::sdk_grpc; -use protobuf::Message; use types::*; const PORT: i32 = 59357; @@ -34,7 +37,7 @@ pub struct Sdk { impl Sdk { /// Starts a new SDK instance, and connects to localhost on port 59357. /// Blocks until connection and handshake are made. - /// Times out after 30 seconds. + /// Times out after ~30 seconds. pub fn new() -> Result { let addr = format!("localhost:{}", PORT); let env = Arc::new(grpcio::EnvBuilder::new().build()); @@ -43,7 +46,24 @@ impl Sdk { .connect(&addr); let cli = sdk_grpc::SdkClient::new(ch); let req = sdk::Empty::new(); - let _ = cli.ready(&req).map(Box::new)?; + + // Unfortunately there isn't a native way to block until connected + // so we had to roll our own. + let mut counter = 0; + loop { + counter += 1; + match cli.get_game_server(&req) { + Ok(_) => break, + Err(e) => { + if counter > 30 { + return Err(ErrorKind::Grpc(e).into()); + } + sleep(Duration::from_secs(1)); + continue; + } + } + } + let (sender, _) = cli.health()?; Ok(Sdk { client: Arc::new(cli), @@ -58,15 +78,13 @@ impl Sdk { Ok(res) } - - /// Allocate the Game Server + /// Allocate the Game Server pub fn allocate(&self) -> Result<()> { let req = sdk::Empty::default_instance(); let res = self.client.allocate(req).map(|_| ())?; Ok(res) } - /// Marks the Game Server as ready to shutdown pub fn shutdown(&self) -> Result<()> { let req = sdk::Empty::default_instance(); diff --git a/site/content/en/docs/Guides/Client SDKs/nodejs.md b/site/content/en/docs/Guides/Client SDKs/nodejs.md index 31815df24c..92859c6595 100644 --- a/site/content/en/docs/Guides/Client SDKs/nodejs.md +++ b/site/content/en/docs/Guides/Client SDKs/nodejs.md @@ -28,7 +28,7 @@ Add the agones dependency to `package.json`, replacing with the download locatio } ``` -To begin working with the SDK, create an instance of it. This will open a connection to the SDK server. +To begin working with the SDK, create an instance of it. {{< feature expiryVersion="0.12.0" >}}This will open a connection to the SDK server.{{< /feature >}} ```javascript const AgonesSDK = require('agones'); @@ -36,6 +36,16 @@ const AgonesSDK = require('agones'); let agonesSDK = new AgonesSDK(); ``` +{{% feature publishVersion="0.12.0" %}} + +To connect to the SDK server, either local or when running on Agones, run the `async` method `sdk.connect()`, which will +`resolve` once connected or `reject` on error or if no connection can be made after 30 seconds. + +```javascript +await agonesSDK.connect(); +``` +{{% /feature %}} + To send a [health check]({{< relref "_index.md#health" >}}) ping call `health()`. ```javascript diff --git a/site/content/en/docs/Guides/Client SDKs/rust.md b/site/content/en/docs/Guides/Client SDKs/rust.md index c25fabb1b4..0062c07416 100644 --- a/site/content/en/docs/Guides/Client SDKs/rust.md +++ b/site/content/en/docs/Guides/Client SDKs/rust.md @@ -46,6 +46,15 @@ if sdk.health().is_ok() { } ``` +{{% feature publishVersion="0.12.0" %}} + +To mark the [game session as ready]({{< relref "_index.md#ready" >}}) call `sdk.ready()`. + +```rust +sdk.ready()?; +``` +{{% /feature %}} + To mark that the [game session is completed]({{< relref "_index.md#shutdown" >}}) and the game server should be shut down call `sdk.shutdown()`. ```rust diff --git a/test/sdk/nodejs/testSDKClient.js b/test/sdk/nodejs/testSDKClient.js index f7d449ad74..5d65feacc2 100644 --- a/test/sdk/nodejs/testSDKClient.js +++ b/test/sdk/nodejs/testSDKClient.js @@ -18,6 +18,9 @@ const agonesSDK = new AgonesSDK(); const connect = async () => { let UID = ''; try { + console.log("attempting to connect"); + await agonesSDK.connect(); + console.log("connected!"); let once = true; agonesSDK.watchGameServer((result) => { console.log('watch', result); diff --git a/test/sdk/rust/.gitignore b/test/sdk/rust/.gitignore new file mode 100644 index 0000000000..0692b103e0 --- /dev/null +++ b/test/sdk/rust/.gitignore @@ -0,0 +1,16 @@ +# Copyright 2019 Google LLC All Rights Reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +/target/ +Cargo.lock \ No newline at end of file diff --git a/test/sdk/rust/src/main.rs b/test/sdk/rust/src/main.rs index 49e3a6efd4..2b372a1b5b 100644 --- a/test/sdk/rust/src/main.rs +++ b/test/sdk/rust/src/main.rs @@ -111,7 +111,7 @@ fn run() -> Result<(), String> { thread::sleep(Duration::from_secs(5)); } - println!("Shutting down after 20 seconds..."); + println!("Shutting down..."); sdk.shutdown() .map_err(|e| format!("Could not run Shutdown: {}. Exiting!", e))?; println!("...marked for Shutdown");