Added game-server example #1771

Bmandk · 2020-08-25T15:56:25Z

What type of PR is this?

Uncomment only one /kind <> line, hit enter to put that in a new line, and remove leading whitespace from that line:

/kind breaking
/kind bug

/kind cleanup

/kind documentation
/kind feature
/kind hotfix

What this PR does / Why we need it:
As discussed in this PR, this PR adds a game-server-example, which is the simple-udp server with added TCP functionality, enabled through flags. While it could be generalized more (by consolidating the large switch to a protocol-independant function), I wanted to only expand on the udp server to not make any breaking changes here.

In the future, this could replace the udp example for all testing purposes, and the tcp server should also be removed.

Which issue(s) this PR fixes:

Closes #

Special notes for your reviewer:

aLekSer · 2020-08-25T16:02:37Z

examples/simple-game-server/Makefile

+
+mkfile_path := $(abspath $(lastword $(MAKEFILE_LIST)))
+project_path := $(dir $(mkfile_path))
+server_tag = $(REPOSITORY)/game-server:0.22


I assume we could start counting from the start:

Suggested change

server_tag = $(REPOSITORY)/game-server:0.22

server_tag = $(REPOSITORY)/game-server:0.1

aLekSer · 2020-08-25T16:03:05Z

examples/simple-game-server/fleet.yaml

+        spec:
+          containers:
+          - name: simple-game-server
+            image: gcr.io/agones-images/game-server:0.22


Same here, version tag could be 0.1

aLekSer · 2020-08-25T16:04:44Z

examples/simple-game-server/main.go

+		tcpRespond(conn, "ACK TCP: "+txt)
+		exit(s)
+
+	// turns off the health pings


Suggested change

// turns off the health pings

// allocates GameServer

aLekSer

Thanks, for this PR. First of all I assume we can list all commands which are available when using TCP server, they have been reduced for some reason, only ALLOCATE and EXIT left.
Other idea, can we remove simple-udp server in a single or an upcoming PR to reduce code duplication?

examples/simple-game-server/main.go

agones-bot · 2020-08-25T16:15:13Z

Build Succeeded 👏

Build Id: 4aa8a6ef-11a0-479e-b524-56bc0a800fc2

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

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

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

https://45d88ed-dot-preview-dot-agones-images.appspot.com/

To install this version:

git fetch https://github.com/googleforgames/agones.git pull/1771/head:pr_1771 && git checkout pr_1771
helm install ./install/helm/agones --namespace agones-system --name agones --set agones.image.tag=1.9.0-45d88ed

markmandel · 2020-08-25T23:50:29Z

Only other main thing I would add - put a entry in here: https://agones.dev/site/docs/examples/#game-server-implementations

Source: https://github.com/googleforgames/agones/blob/master/site/content/en/docs/Examples/_index.md

agones-bot · 2020-08-26T18:02:17Z

Build Failed 😱

Build Id: 41673126-e94c-428b-8265-0781ccc8fe5b

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

agones-bot · 2020-08-26T18:13:24Z

Build Succeeded 👏

Build Id: 7691b9f1-89a9-47cb-a9c6-655647074cb2

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

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

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

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

To install this version:

git fetch https://github.com/googleforgames/agones.git pull/1771/head:pr_1771 && git checkout pr_1771
helm install ./install/helm/agones --namespace agones-system --name agones --set agones.image.tag=1.9.0-b95e1c3

Bmandk · 2020-08-26T18:16:15Z

I've added the changes, and also used this image as the e2e image which passed. Before being merged, the e2e image should be changed and the gameserver image should also be uploaded to gcr.io/agones-images

roberthbailey

I didn't review main.go too thoroughly, since it's mostly copied from existing code.

It would also be nice to add yaml configs showing how to enable TCP (and maybe both udp and tcp) so that we can deprecate and delete the simple-udp and simple-tcp example directories.

roberthbailey · 2020-08-26T18:09:06Z

build/Makefile

@@ -57,7 +57,7 @@ KIND_PROFILE ?= agones
 KIND_CONTAINER_NAME=$(KIND_PROFILE)-control-plane

 # Game Server image to use while doing end-to-end tests
-GS_TEST_IMAGE ?= gcr.io/agones-images/udp-server:0.21
+GS_TEST_IMAGE ?= gcr.io/agones-dev-286708/game-server:0.1


Please revert this file in the PR (feel free to keep it locally for testing though).

The idea is to use this instead of the udp server, as discussed here. Do you just prefer that this is changed at a later point?

I think @roberthbailey 's point is that it shouldn't be GS_TEST_IMAGE ?= gcr.io/agones-dev-286708/game-server:0.1 it should be GS_TEST_IMAGE ?= gcr.io/agones-images/game-server:0.1

It should either be reverted (so that we continue to use the udp-server:0.21 in e2e tests for now) or updated to the agones-images project (if we feel like the new image is ready to go).

roberthbailey · 2020-08-26T18:09:45Z

examples/simple-game-server/Makefile

+
+mkfile_path := $(abspath $(lastword $(MAKEFILE_LIST)))
+project_path := $(dir $(mkfile_path))
+server_tag = $(REPOSITORY)/game-server:0.1


Can you call the image simple-game-server instead of game-server to be consistent with the directory name?

So we rather want it to be inconsistent with the other image names?

The udp server image is called udp-server, while the directory is called simple-udp, so I just want to make sure.

Well, @markmandel set the original naming scheme (which I copied for the simple-tcp server) but personally I think it makes more sense if the names match.

As another data point, the language specific examples are directory-name-server, e.g. rust-simple-server in the directory rust-simple.

+1 to @roberthbailey makes sense to me for the directory and the name to match 👍

Alright, will rename the image name 👍

roberthbailey · 2020-08-26T18:12:31Z

site/content/en/docs/Examples/_index.md

@@ -22,6 +22,7 @@ These are all examples of simple game server implementations, that integrate the

 - {{< ghlink href="examples/simple-udp" >}}Simple UDP{{< /ghlink >}} (Go) - simple server and client that send UDP packets back and forth.
 - {{< ghlink href="examples/simple-tcp" >}}Simple TCP{{< /ghlink >}} (Go) - simple server that responds to new-line delimited messages sent over a TCP connection.
+- {{< ghlink href="examples/simple-game-server" >}}Simple gameserver{{< /ghlink >}} (Go) - simple server that responds to TCP connections or UDP packets on the same port.


Please add release guards to this new link.

How do I do that/Where can I read about how to do that?

https://agones.dev/site/docs/contribute/#within-a-page 👍

roberthbailey · 2020-08-26T18:14:19Z

examples/simple-game-server/main.go

+	}
+
+	// prevents program from quitting as the server is listening on goroutines
+	for {


nit: Put on one line (for {}).

agones-bot · 2020-08-31T20:58:16Z

Build Failed 😱

Build Id: 68285ecd-321f-4692-9699-b66d1b62a499

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

markmandel · 2020-08-31T21:00:48Z

I'm assuming that this is failing since that image hasn't been built and pushed up. I assume @Bmandk you are good for me to create it and push it to the registry?

Bmandk · 2020-08-31T21:03:03Z

I'm assuming that this is failing since that image hasn't been built and pushed up. I assume @Bmandk you are good for me to create it and push it to the registry?

I would be inclined to say that I may have gotten the feature gate wrong, as that is the test that is failing. But please correct me if I'm wrong!

Bmandk · 2020-08-31T21:08:06Z

I just realized that I wrote the 0.9.0 and not 1.9.0, but I'm assuming that is not the reason of the test failing. I'm unsure if there's anything else, or if I'm just misinterpreting the test logs. Wdyt?

markmandel · 2020-08-31T21:25:32Z

I just realized that I wrote the 0.9.0 and not 1.9.0, but I'm assuming that is not the reason of the test failing. I'm unsure if there's anything else, or if I'm just misinterpreting the test logs. Wdyt?

Heh, yes, that will be something that does need to be fixed 👍

But looks like e2e-feature-gates is failing - which is our end to end tests that run against that image.

Did you want me to wait until the feature gate on the docs is fixed, or you good for me to go ahead and get that image up there?

Bmandk · 2020-08-31T21:26:48Z

I just realized that I wrote the 0.9.0 and not 1.9.0, but I'm assuming that is not the reason of the test failing. I'm unsure if there's anything else, or if I'm just misinterpreting the test logs. Wdyt?

Heh, yes, that will be something that does need to be fixed 👍

But looks like e2e-feature-gates is failing - which is our end to end tests that run against that image.

Did you want me to wait until the feature gate on the docs is fixed, or you good for me to go ahead and get that image up there?

Just pushed the new version, feel free to push it up now!

markmandel · 2020-08-31T21:28:50Z

Cool - thanks! We're in the middle of our performance review season, but will try and take a break today/tomorrow and get this up.

Thanks for working with us on this!

Bmandk · 2020-08-31T21:35:28Z

No hurry at all, and thanks for all your patience with me :)

agones-bot · 2020-08-31T21:45:46Z

Build Failed 😱

Build Id: 186c61d1-d6cd-4b08-9019-045c9eaf5c76

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

agones-bot · 2020-09-03T20:00:16Z

Build Failed 😱

Build Id: e0b6e64f-4dfa-45c2-b35a-8f146cd65c33

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

agones-bot · 2020-09-04T07:07:09Z

Build Succeeded 👏

Build Id: c719e467-4e37-4744-8ad8-29e126e4e7d1

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

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

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

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

To install this version:

git fetch https://github.com/googleforgames/agones.git pull/1771/head:pr_1771 && git checkout pr_1771
helm install ./install/helm/agones --namespace agones-system --name agones --set agones.image.tag=1.9.0-d80d928

markmandel · 2020-09-04T15:03:57Z

examples/simple-game-server/gameserverset.yaml

+# Usually you would define a Fleet rather than a GameServerSet
+# directly. This is here mostly for testing purposes
+
+apiVersion: "agones.dev/v1"


Wondering if we should delete this file -- it actually serves no purpose other than potentially Agones developer testing.

+1 to deleting.

Makes sense

markmandel

Yay passing! I had some general minor cleanup things, but otherwise, this looks really good/

https://github.com/googleforgames/agones/blob/master/test/e2e/framework/framework.go#L165 < another place to update the default udp server image.

markmandel · 2020-09-04T15:07:02Z

examples/simple-game-server/fleet-distributed.yaml

+          - name: simple-game-server
+            image: gcr.io/agones-images/simple-game-server:0.1
+            env:
+            # Disables the UDP listener (Enabled by default)


I think on all of these, let's default to UDP being enabled, since that's the most common use case. WDYT?

Agreed, will do

markmandel · 2020-09-04T15:07:54Z

examples/simple-game-server/fleet.yaml

+          - name: simple-game-server
+            image: gcr.io/agones-images/simple-game-server:0.1
+            env:
+            # Disables the UDP listener (Enabled by default)


Same here, feels like UDP should be enabled by default.

(Does it make sense to remove the env cars entirely for the default version, and maybe have a fleet-tcp.yaml for a TCP version?)

Ah, that actually makes more sense. So remove the env vars from all the current yaml's, and just make a TCP-only fleet? That should show both how to disable UDP and enable TCP.

markmandel · 2020-09-04T15:09:13Z

examples/simple-game-server/gameserver-passthrough.yaml

+        env:
+        # Disables the UDP listener (Enabled by default)
+        - name: "UDP"
+          value: "FALSE"


Same thing here - wondering if it's simpler to remove the env vars -- here we are demo'ing passthrough, so I don't know if we want to make things more complicated with more env varas for UDP/TCP enablement?

markmandel · 2020-09-04T15:10:01Z

examples/simple-game-server/gameserver.yaml

+      - name: simple-game-server
+        image: gcr.io/agones-images/simple-game-server:0.1
+        env:
+        # Disables the UDP listener (Enabled by default)


Let's start with UDP enabled by default (maybe delete the env vars?)

markmandel

whoops, just kicking this to "request changes" so we address the cleanup items before this gets merged.

agones-bot · 2020-09-05T06:56:55Z

Build Succeeded 👏

Build Id: f5064412-526c-4bbe-a2c2-6996ed9fa13d

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

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

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

https://1faaa39-dot-preview-dot-agones-images.appspot.com/

To install this version:

git fetch https://github.com/googleforgames/agones.git pull/1771/head:pr_1771 && git checkout pr_1771
helm install ./install/helm/agones --namespace agones-system --name agones --set agones.image.tag=1.9.0-1faaa39

agones-bot · 2020-09-05T10:45:05Z

Build Failed 😱

Build Id: d04a003e-de75-4792-bdba-7c4e34a5cbf7

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

Bmandk · 2020-09-05T11:02:09Z

I think I pressed a wrong button, as I accidentally created a new commit. I've reverted that, hopefully it should be fine now. Let me know if there are other changes required.

agones-bot · 2020-09-05T11:21:54Z

Build Succeeded 👏

Build Id: 6bcde4be-61b3-4586-8905-f273b016cafe

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

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

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

https://06c841d-dot-preview-dot-agones-images.appspot.com/

To install this version:

git fetch https://github.com/googleforgames/agones.git pull/1771/head:pr_1771 && git checkout pr_1771
helm install ./install/helm/agones --namespace agones-system --name agones --set agones.image.tag=1.9.0-06c841d

markmandel · 2020-09-08T21:18:05Z

.golangci.yml

@@ -46,7 +46,6 @@ output:

 linters:
  enable:


Something weird has happened with your git history -- there are all this extra changes in here 😞

You might want to rebase your local branch against master, and squash your changes, and force push over the top to make sure you're in a happy place.

Ok, everything looks fine now. Let me know if there's anything else I need to do (as you might have guessed, I'm not a git wizard!)

Change initial version to 0.1 Move response handling to function and use for both UDP and TCP Generalized the response handling to TCP and UDP Added documentation Change empty for loop to one-liner Changed repo for e2e test image Renamed image to simple-game-server Added release guards to docs Changed feature gate version Added yaml config of disabling UDP and enabling TCP Update build/Makefile to use simple-game-server:0.1 image name Co-authored-by: Mark Mandel <markmandel@google.com> Deleted gameserverset.yaml Changed default flags for gs image in test framework Removed env vars from yaml and create a new fleet-tcp.yaml instead Revert "Merge branch 'master' into simple-game-server-example" This reverts commit cda2966, reversing changes made to 1faaa39. Revert "Revert "Merge branch 'master' into simple-game-server-example"" This reverts commit 06c841d.

agones-bot · 2020-09-10T21:08:20Z

Build Failed 😱

Build Id: aca5009f-6106-4cb4-b683-7d49f479463c

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

Bmandk · 2020-09-10T21:13:16Z

Oh no..
Not sure what's happening with those tests now, seems to be some SDK test failing?

markmandel · 2020-09-11T02:22:49Z

It's okay, that's a standard flake 🙄

google-oss-robot · 2020-09-11T02:42:38Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: aLekSer, Bmandk, markmandel

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~~ [aLekSer,markmandel]

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

agones-bot · 2020-09-11T03:04:30Z

Build Succeeded 👏

Build Id: 5bd0bdd0-47ef-47d0-acc3-bb092ad7483f

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

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

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

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

To install this version:

git fetch https://github.com/googleforgames/agones.git pull/1771/head:pr_1771 && git checkout pr_1771
helm install ./install/helm/agones --namespace agones-system --name agones --set agones.image.tag=1.9.0-f42da54

Change initial version to 0.1 Move response handling to function and use for both UDP and TCP Generalized the response handling to TCP and UDP Added documentation Added yaml config of disabling UDP and enabling TCP Update build/Makefile to use simple-game-server:0.1 image name Co-authored-by: Mark Mandel <markmandel@google.com>

googlebot added the cla: yes label Aug 25, 2020

google-oss-robot requested review from EricFortin and markmandel August 25, 2020 15:56

google-oss-robot added the size/XL label Aug 25, 2020

aLekSer reviewed Aug 25, 2020

View reviewed changes

aLekSer suggested changes Aug 25, 2020

View reviewed changes

google-oss-robot assigned aLekSer Aug 25, 2020

aLekSer reviewed Aug 25, 2020

View reviewed changes

examples/simple-game-server/main.go Outdated Show resolved Hide resolved

aLekSer reviewed Aug 25, 2020

View reviewed changes

examples/simple-game-server/main.go Outdated Show resolved Hide resolved

markmandel added area/examples Examples. Usually found in the `examples` directory area/tests Unit tests, e2e tests, anything to make sure things don't break labels Aug 25, 2020

Bmandk requested a review from aLekSer August 26, 2020 18:14

roberthbailey reviewed Aug 26, 2020

View reviewed changes

Bmandk mentioned this pull request Aug 26, 2020

Add TCPUDP protocol #1764

Merged

markmandel reviewed Sep 4, 2020

View reviewed changes

markmandel requested changes Sep 4, 2020

View reviewed changes

google-oss-robot assigned markmandel Sep 4, 2020

google-oss-robot added size/XXL and removed size/XL labels Sep 5, 2020

Bmandk requested a review from markmandel September 8, 2020 06:59

markmandel reviewed Sep 8, 2020

View reviewed changes

Bmandk force-pushed the simple-game-server-example branch from 06c841d to f42da54 Compare September 10, 2020 20:51

google-oss-robot added size/XL and removed size/XXL labels Sep 10, 2020

Bmandk requested a review from markmandel September 10, 2020 20:53

markmandel approved these changes Sep 11, 2020

View reviewed changes

google-oss-robot added the lgtm label Sep 11, 2020

markmandel merged commit fb3ea19 into googleforgames:master Sep 11, 2020

markmandel added this to the 1.9.0 milestone Sep 11, 2020

roberthbailey mentioned this pull request Nov 5, 2020

Deprecate and remove the udp-server and tcp-server images #1890

Closed

	server_tag = $(REPOSITORY)/game-server:0.22
	server_tag = $(REPOSITORY)/game-server:0.1

Added game-server example #1771

Added game-server example #1771

Conversation

Bmandk commented Aug 25, 2020

Choose a reason for hiding this comment

aLekSer Aug 25, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aLekSer left a comment

Choose a reason for hiding this comment

agones-bot commented Aug 25, 2020

markmandel commented Aug 25, 2020

agones-bot commented Aug 26, 2020

agones-bot commented Aug 26, 2020

Bmandk commented Aug 26, 2020

roberthbailey left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Bmandk Aug 26, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

agones-bot commented Aug 31, 2020

markmandel commented Aug 31, 2020

Bmandk commented Aug 31, 2020 • edited Loading

Bmandk commented Aug 31, 2020

markmandel commented Aug 31, 2020

Bmandk commented Aug 31, 2020

markmandel commented Aug 31, 2020

Bmandk commented Aug 31, 2020

agones-bot commented Aug 31, 2020

agones-bot commented Sep 3, 2020

agones-bot commented Sep 4, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

markmandel left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

markmandel left a comment

Choose a reason for hiding this comment

agones-bot commented Sep 5, 2020

agones-bot commented Sep 5, 2020

Bmandk commented Sep 5, 2020

agones-bot commented Sep 5, 2020

markmandel Sep 8, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

agones-bot commented Sep 10, 2020

Bmandk commented Sep 10, 2020

markmandel commented Sep 11, 2020

google-oss-robot commented Sep 11, 2020

agones-bot commented Sep 11, 2020

aLekSer Aug 25, 2020 •

edited

Loading

Bmandk Aug 26, 2020 •

edited

Loading

Bmandk commented Aug 31, 2020 •

edited

Loading

markmandel Sep 8, 2020 •

edited

Loading