feat(gameserver): New DirectToGameServer PortPolicy allows direct traffic to a GameServer #3807

daniellee · 2024-04-26T12:31:56Z

What type of PR is this?

Uncomment only one /kind <> line, press 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
/kind release

What this PR does / Why we need it:

Directly connect to a Gameserver instance with a publicly routable address and a fixed port without any NAT
Removes the HostPort dependency of the other three port policies and allows connecting directly using the PodIP and the ContainerPort

Which issue(s) this PR fixes:

Closes #3804

Special notes for your reviewer:

Gofmt made a lot of formatting changes in the test files. Let me know if I should roll those back.
I don't know if you will accept a PR for another PortPolicy - let me know if you think there is a better way to approach this. I didn't come up with any good alternatives - see issue for details.
The e2e test was the hardest part of this PR. There is no easy way to set up networking for a minikube test to allow direct traffic to a pod via its IP address and port. We are using Calico and BGP to get this to work. I could add port-forwarding but then I am not testing anything relevant. Open to suggestions if there is a relevant e2e test that should be added to this PR.

agones-bot · 2024-04-26T12:43:41Z

Build Failed 😱

Build Id: 3731534a-b65b-4e77-a055-462ef5bdc46d

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

agones-bot · 2024-04-26T13:52:02Z

Build Failed 😱

Build Id: e6847a5f-161a-4f50-ab03-e6be1e0081c2

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

agones-bot · 2024-04-26T14:24:16Z

Build Failed 😱

Build Id: 030c7f3c-8935-4ec8-b72b-7e7ae9cc05e4

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

agones-bot · 2024-04-26T15:12:48Z

Build Failed 😱

Build Id: 41360ecd-663c-44d7-8e09-8587822142a9

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

daniellee · 2024-04-29T12:00:54Z

I got stuck on the e2e test as setting up the pod so that you can send direct traffic to via a Pod IP and container port is not simple. Working on getting a decent e2e test in place.

agones-bot · 2024-04-29T15:19:50Z

Build Succeeded 👏

Build Id: c6962355-3d36-4f21-9026-d3b93d385302

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

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

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

To install this version:

git fetch https://github.com/googleforgames/agones.git pull/3807/head:pr_3807 && git checkout pr_3807
helm install agones ./install/helm/agones --namespace agones-system --set agones.image.registry=us-docker.pkg.dev/agones-images/ci --set agones.image.tag=1.41.0-dev-cc9cf8f-amd64

agones-bot · 2024-04-29T18:12:56Z

Build Succeeded 👏

Build Id: e49c3f18-d140-410a-a33b-924dab8d4c7c

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

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

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

To install this version:

git fetch https://github.com/googleforgames/agones.git pull/3807/head:pr_3807 && git checkout pr_3807
helm install agones ./install/helm/agones --namespace agones-system --set agones.image.registry=us-docker.pkg.dev/agones-images/ci --set agones.image.tag=1.41.0-dev-beeb337-amd64

agones-bot · 2024-05-02T15:36:29Z

Build Succeeded 👏

Build Id: acb281ac-fcd1-4cb6-aaf6-4e62ec96e8e1

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

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

https://54e0509-dot-preview-dot-agones-images.appspot.com/

To install this version:

git fetch https://github.com/googleforgames/agones.git pull/3807/head:pr_3807 && git checkout pr_3807
helm install agones ./install/helm/agones --namespace agones-system --set agones.image.registry=us-docker.pkg.dev/agones-images/ci --set agones.image.tag=1.41.0-dev-54e0509-amd64

agones-bot · 2024-05-02T19:31:05Z

Build Failed 😱

Build Id: 337cf488-70d8-4027-af85-190136be894c

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

agones-bot · 2024-05-02T21:10:51Z

Build Succeeded 👏

Build Id: 083615e3-4a0a-4abc-b6ea-6580fd8c73fd

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

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

https://135f008-dot-preview-dot-agones-images.appspot.com/

To install this version:

git fetch https://github.com/googleforgames/agones.git pull/3807/head:pr_3807 && git checkout pr_3807
helm install agones ./install/helm/agones --namespace agones-system --set agones.image.registry=us-docker.pkg.dev/agones-images/ci --set agones.image.tag=1.41.0-dev-135f008-amd64

zmerlynn · 2024-05-03T15:29:38Z

test/e2e/gameserver_test.go

@@ -852,6 +848,28 @@ func TestGameServerPassthroughPort(t *testing.T) {
 	assert.Equal(t, "ACK: Hello World !\n", reply)
 }

+func TestGameServerPortPolicyNone(t *testing.T) {
+	framework.SkipOnCloudProduct(t, "gke-autopilot", "does not support None PortPolicy")


That's not true, or implemented, AFAICT. :) (It looks like Autopilot should support this just fine, but separately, if you intended it not to be the case, you would need to modify the validation webhook.)

Should I also change the error message "portPolicy must be Dynamic on GKE Autopilot" to something like "portPolicy must be Dynamic or None on GKE Autopilot"? Does that make sense?

That's a good idea, yeah!

zmerlynn

In general LGTM, left one issue and you have a conflict to resolve now - after that and tests pass on Autopilot, you should be good to go.

agones-bot · 2024-05-03T16:16:14Z

Build Failed 😱

Build Id: 1b60ca10-cfc5-4e29-95e7-1edf9131cf5b

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

agones-bot · 2024-05-03T19:54:29Z

Build Succeeded 👏

Build Id: e90fa6ad-4aa4-4046-8d4f-44a74c177a1f

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

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

https://26cfea8-dot-preview-dot-agones-images.appspot.com/

To install this version:

git fetch https://github.com/googleforgames/agones.git pull/3807/head:pr_3807 && git checkout pr_3807
helm install agones ./install/helm/agones --namespace agones-system --set agones.image.registry=us-docker.pkg.dev/agones-images/ci --set agones.image.tag=1.41.0-dev-26cfea8-amd64

agones-bot · 2024-05-06T08:01:51Z

Build Succeeded 👏

Build Id: 854189e9-5363-4ea9-b183-abbb8c4aee59

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

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

https://981f79a-dot-preview-dot-agones-images.appspot.com/

To install this version:

git fetch https://github.com/googleforgames/agones.git pull/3807/head:pr_3807 && git checkout pr_3807
helm install agones ./install/helm/agones --namespace agones-system --set agones.image.registry=us-docker.pkg.dev/agones-images/ci --set agones.image.tag=1.41.0-dev-981f79a-amd64

daniellee · 2024-05-06T14:29:02Z

@zmerlynn let me know if there is anything else I need to change. In the meantime, I will try and keep the branch synchronized.

markmandel

Thanks for all this work, and the discussions!

No issues with implementation here 👍🏻 a couple of notes on some docs.

Only thing I have -let's put it behind a feature gate, just so we have an opportunity to get feedback on the feature once it has landed, and can change it if need be - you may well find things you want to tweak once you've got it up and running.

I expect it'll promote pretty fast, but better safe than sorry.

Details on feature flags here:

https://agones.dev/site/docs/guides/feature-stages/
https://github.com/googleforgames/agones/blob/main/pkg/util/runtime/features.go (comments have implementation steps)

markmandel · 2024-05-06T23:50:40Z

site/content/en/docs/Reference/gameserver.md

@@ -245,10 +246,11 @@ The `spec` field is the actual GameServer specification and it is composed as fo
 - `container` is the name of container running the GameServer in case you have more than one container defined in the [pod](https://kubernetes.io/docs/concepts/workloads/pods/pod-overview/). If you do,  this is a mandatory field. For instance this is useful if you want to run a sidecar to ship logs.
 - `ports` are an array of ports that can be exposed as direct connections to the game server container
  - `name` is an optional descriptive name for a port
-  - `portPolicy` has three options:
+  - `portPolicy` has four options:


These changes will need to live behind a feature shortcode please (our docs publish immediately on merge).

See: https://agones.dev/site/docs/contribute/ for details.

pkg/apis/agones/v1/gameserver.go

github-actions · 2024-05-16T06:55:20Z

This PR exceeds the recommended size of 1000 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size.

agones-bot · 2024-05-16T06:58:40Z

Build Failed 😱

Build Id: 7b1e124e-4dc4-443c-aa3a-288e9ddcea09

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

github-actions · 2024-05-16T09:48:09Z

This PR exceeds the recommended size of 1000 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size.

agones-bot · 2024-05-16T11:08:24Z

Build Succeeded 👏

Build Id: e6767e78-02af-4024-b2e1-22621774be30

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

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

https://876693d-dot-preview-dot-agones-images.appspot.com/

To install this version:

git fetch https://github.com/googleforgames/agones.git pull/3807/head:pr_3807 && git checkout pr_3807
helm install agones ./install/helm/agones --namespace agones-system --set agones.image.registry=us-docker.pkg.dev/agones-images/ci --set agones.image.tag=1.41.0-dev-876693d-amd64

daniellee · 2024-05-16T14:15:00Z

Finally got a green build.

In the latest iteration, I have:

I have moved the docs changes behind feature shortcodes.
Fixed a small indentation bug for the current list of PortPolicy options - you can see the formatting error on the current gameserver docs page and I think this is the right preview link from the build with that fix.
Added feature gates in the code for policy none
Got all the tests to work with feature gates
Ran make gen-api-docs gen-crd-client and generated a new crd api reference file

Let me know if I missed anything else around feature gates.

zmerlynn

This LGTM. There's a conflict in generated docs - you can rebase and re-run make gen-crd-client to resolve it quickly enough.

To be able to access a GameServer with a publicly routable address, this policy allows connecting directly wihout going via the host port. The other policies assume that a host port is always needed.

Revert the line with line ending changes.

Can't find a good way to send UDP traffic directly to a pod. This is not something that k8s does out of the box and requires a product like Calico or similar that uses BGP to provide direct access to pods. Removing the udp call and making the test much simpler

agones-bot · 2024-05-16T18:06:29Z

Build Succeeded 👏

Build Id: 9b7ffde6-8875-4307-8e13-ea7b16dc8a45

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

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

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

To install this version:

git fetch https://github.com/googleforgames/agones.git pull/3807/head:pr_3807 && git checkout pr_3807
helm install agones ./install/helm/agones --namespace agones-system --set agones.image.registry=us-docker.pkg.dev/agones-images/ci --set agones.image.tag=1.41.0-dev-e8c699d-amd64

zmerlynn

Great job, thank you!

zmerlynn · 2024-05-16T18:25:29Z

cc @markmandel who may want to review, or I'll merge it soon

markmandel · 2024-05-16T18:27:49Z

LGTM!

daniellee · 2024-05-16T18:29:27Z

Thank you both for all the help!

github-actions bot added kind/feature New features for Agones size/L labels Apr 26, 2024

github-actions bot added the size/M label Apr 26, 2024

daniellee force-pushed the dl/direct-to-gs branch from cc9cf8f to beeb337 Compare April 29, 2024 16:55

daniellee marked this pull request as ready for review April 29, 2024 18:39

zmerlynn mentioned this pull request May 1, 2024

Direct connection to a GameServer/Pod without NAT #3804

Closed

daniellee force-pushed the dl/direct-to-gs branch 3 times, most recently from bc98c26 to 54e0509 Compare May 2, 2024 14:28

daniellee force-pushed the dl/direct-to-gs branch from 54e0509 to 81b9c75 Compare May 2, 2024 16:50

zmerlynn reviewed May 3, 2024

View reviewed changes

daniellee force-pushed the dl/direct-to-gs branch from 135f008 to c91aab1 Compare May 3, 2024 15:42

daniellee force-pushed the dl/direct-to-gs branch from 26cfea8 to 981f79a Compare May 6, 2024 06:54

markmandel reviewed May 6, 2024

View reviewed changes

markmandel mentioned this pull request May 6, 2024

Add PortPolicy::None googleforgames/quilkin#940

Merged

daniellee force-pushed the dl/direct-to-gs branch from 02b042e to 876693d Compare May 16, 2024 09:47

zmerlynn reviewed May 16, 2024

View reviewed changes

daniellee added 14 commits May 16, 2024 18:33

gameservers: adds a new DirectToGameServer PortPolicy

18f69c6

To be able to access a GameServer with a publicly routable address, this policy allows connecting directly wihout going via the host port. The other policies assume that a host port is always needed.

gameservers: e2e test for DirectToGameServer PortPolicy

cbe71ef

update install.yaml

161080a

Revert the line with line ending changes.

more yaml fixes

32d0539

docs: changes to the text for DirectToGameServer PortPolicy

523d0f1

test fix - int vs int32 fail

917f4ad

rename PortPolicy to None and removes unneeded validation rules

19b8273

Empty commit to retrigger hung build

7fe14ee

change e2e test to run on gke

368af94

changed gke validation to accept PortPolicy None

b3e24b0

PortPolicyNone feature gate

496052a

fix e2e test

bcca3de

docs: fix merge conflict

e8c699d

daniellee force-pushed the dl/direct-to-gs branch from 876693d to e8c699d Compare May 16, 2024 16:55

zmerlynn approved these changes May 16, 2024

View reviewed changes

markmandel merged commit 9cc12e6 into googleforgames:main May 16, 2024
4 checks passed

zmerlynn mentioned this pull request Jul 24, 2024

Support opt-out of node port binding #3590

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(gameserver): New DirectToGameServer PortPolicy allows direct traffic to a GameServer #3807

feat(gameserver): New DirectToGameServer PortPolicy allows direct traffic to a GameServer #3807

daniellee commented Apr 26, 2024 •

edited

Loading

agones-bot commented Apr 26, 2024

agones-bot commented Apr 26, 2024

agones-bot commented Apr 26, 2024

agones-bot commented Apr 26, 2024

daniellee commented Apr 29, 2024

agones-bot commented Apr 29, 2024

agones-bot commented Apr 29, 2024

agones-bot commented May 2, 2024

agones-bot commented May 2, 2024

agones-bot commented May 2, 2024

zmerlynn May 3, 2024

daniellee May 3, 2024

zmerlynn May 3, 2024

zmerlynn left a comment

agones-bot commented May 3, 2024

agones-bot commented May 3, 2024

agones-bot commented May 6, 2024

daniellee commented May 6, 2024

markmandel left a comment

markmandel May 6, 2024

github-actions bot commented May 16, 2024

agones-bot commented May 16, 2024

github-actions bot commented May 16, 2024

agones-bot commented May 16, 2024

daniellee commented May 16, 2024 •

edited

Loading

zmerlynn left a comment

agones-bot commented May 16, 2024

zmerlynn left a comment

zmerlynn commented May 16, 2024 •

edited

Loading

markmandel commented May 16, 2024

daniellee commented May 16, 2024

feat(gameserver): New DirectToGameServer PortPolicy allows direct traffic to a GameServer #3807

feat(gameserver): New DirectToGameServer PortPolicy allows direct traffic to a GameServer #3807

Conversation

daniellee commented Apr 26, 2024 • edited Loading

agones-bot commented Apr 26, 2024

agones-bot commented Apr 26, 2024

agones-bot commented Apr 26, 2024

agones-bot commented Apr 26, 2024

daniellee commented Apr 29, 2024

agones-bot commented Apr 29, 2024

agones-bot commented Apr 29, 2024

agones-bot commented May 2, 2024

agones-bot commented May 2, 2024

agones-bot commented May 2, 2024

zmerlynn May 3, 2024

Choose a reason for hiding this comment

daniellee May 3, 2024

Choose a reason for hiding this comment

zmerlynn May 3, 2024

Choose a reason for hiding this comment

zmerlynn left a comment

Choose a reason for hiding this comment

agones-bot commented May 3, 2024

agones-bot commented May 3, 2024

agones-bot commented May 6, 2024

daniellee commented May 6, 2024

markmandel left a comment

Choose a reason for hiding this comment

markmandel May 6, 2024

Choose a reason for hiding this comment

github-actions bot commented May 16, 2024

agones-bot commented May 16, 2024

github-actions bot commented May 16, 2024

agones-bot commented May 16, 2024

daniellee commented May 16, 2024 • edited Loading

zmerlynn left a comment

Choose a reason for hiding this comment

agones-bot commented May 16, 2024

zmerlynn left a comment

Choose a reason for hiding this comment

zmerlynn commented May 16, 2024 • edited Loading

markmandel commented May 16, 2024

daniellee commented May 16, 2024

daniellee commented Apr 26, 2024 •

edited

Loading

daniellee commented May 16, 2024 •

edited

Loading

zmerlynn commented May 16, 2024 •

edited

Loading