agones icon indicating copy to clipboard operation
agones copied to clipboard

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

Open daniellee opened this issue 10 months ago • 14 comments

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.

daniellee avatar Apr 26 '24 12:04 daniellee

Build Failed :scream:

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

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

agones-bot avatar Apr 26 '24 12:04 agones-bot

Build Failed :scream:

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

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

agones-bot avatar Apr 26 '24 13:04 agones-bot

Build Failed :scream:

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

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

agones-bot avatar Apr 26 '24 14:04 agones-bot

Build Failed :scream:

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

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

agones-bot avatar Apr 26 '24 15:04 agones-bot

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.

daniellee avatar Apr 29 '24 12:04 daniellee

Build Succeeded :clap:

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 avatar Apr 29 '24 15:04 agones-bot

Build Succeeded :clap:

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 avatar Apr 29 '24 18:04 agones-bot

Build Succeeded :clap:

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 avatar May 02 '24 15:05 agones-bot

Build Failed :scream:

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

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

agones-bot avatar May 02 '24 19:05 agones-bot

Build Succeeded :clap:

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

agones-bot avatar May 02 '24 21:05 agones-bot

Build Failed :scream:

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

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

agones-bot avatar May 03 '24 16:05 agones-bot

Build Succeeded :clap:

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 avatar May 03 '24 19:05 agones-bot

Build Succeeded :clap:

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

agones-bot avatar May 06 '24 08:05 agones-bot

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

daniellee avatar May 06 '24 14:05 daniellee

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.

github-actions[bot] avatar May 13 '24 13:05 github-actions[bot]

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.

github-actions[bot] avatar May 13 '24 13:05 github-actions[bot]

Build Failed :scream:

Build Id: cee0da50-89c6-4f42-957a-67c979cde17f

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

agones-bot avatar May 13 '24 14:05 agones-bot

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.

github-actions[bot] avatar May 13 '24 20:05 github-actions[bot]

Build Failed :scream:

Build Id: 3ed439f7-25ec-4bf7-8c96-ff04283db91b

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

agones-bot avatar May 13 '24 21:05 agones-bot

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.

github-actions[bot] avatar May 13 '24 21:05 github-actions[bot]

Build Failed :scream:

Build Id: 919b3ace-e280-41d6-a297-73ad1ad0ac53

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

agones-bot avatar May 13 '24 22:05 agones-bot

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.

github-actions[bot] avatar May 14 '24 06:05 github-actions[bot]

Getting a build error for submit-e2e-test-cloud-build that I am not sure if is related to my changes. Re-running to see if it fails again the same way. I encountered a seemingly sporadic error with the deploy-site-static previously where it couldn't save the assets.

{
  "error": {
    "code": 401,
    "message": "Request is missing required authentication credential. Expected OAuth 2 access token, login cookie or other valid authentication credential. See https://developers.google.com/identity/sign-in/web/devconsole-project.",
    "status": "UNAUTHENTICATED",
    "details": [
      {
        "@type": "type.googleapis.com/google.rpc.ErrorInfo",
        "reason": "CREDENTIALS_MISSING",
        "domain": "googleapis.com",
        "metadata": {
          "method": "google.devtools.cloudbuild.v1.CloudBuild.GetBuild",
          "service": "cloudbuild.googleapis.com"
        }
      }
    ]
  }
}

daniellee avatar May 14 '24 06:05 daniellee

Build Failed :scream:

Build Id: 1c388320-cd74-40f7-9775-faeea7b92966

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

agones-bot avatar May 14 '24 07:05 agones-bot

This last failure is on the new test:

=== CONT  TestGameServerPortPolicyNone
    gameserver_test.go:867: 
        	Error Trace:	/go/src/agones.dev/agones/test/e2e/gameserver_test.go:867
        	Error:      	Could not get a GameServer ready
        	Test:       	TestGameServerPortPolicyNone
        	Messages:   	creating {game-server [{  None <nil> 7777 0 }] {false 0 0 0}  { 0 0} {{      0 0001-01-01 00:00:00 +0000 UTC <nil> <nil> map[] map[] [] [] []} {[] [] [{game-server us-docker.pkg.dev/agones-images/examples/simple-game-server:0.32 [] []  [] [] [] {map[cpu:{{30 -3} {<nil>} 30m DecimalSI} memory:{{33554432 0} {<nil>}  BinarySI}] map[cpu:{{30 -3} {<nil>} 30m DecimalSI} memory:{{33554432 0} {<nil>}  BinarySI}] []} [] <nil> [] [] nil nil nil nil   IfNotPresent nil false false false}] []  <nil> <nil>  map[]   <nil>  false false false <nil> nil []   nil  [] []  <nil> nil [] <nil> <nil> <nil> map[] [] <nil> nil <nil> [] []}} <nil> map[] map[] <nil>} GameServer instances failed (): admission webhook "validations.agones.dev" denied the request: GameServer.agones.dev "game-serverthjnv" is invalid: spec.ports[0].portPolicy: Invalid value: "None": portPolicy must be Dynamic or None on GKE Autopilot
--- FAIL: TestGameServerPortPolicyNone (2.18s)
FAIL test/e2e.TestGameServerPortPolicyNone (2.18s)

I'll leave you a review comment in a sec explaining what's up - basically you're not dealing with the feature flag correctly for e2es, which is different than unit tests.

zmerlynn avatar May 14 '24 15:05 zmerlynn

Thanks, it worked locally and I thought that my change for the e2e seemed to work this time - I must have been looking at the wrong place in the build log files.

daniellee avatar May 14 '24 15:05 daniellee

Thanks, it worked locally and I thought that my change for the e2e seemed to work this time - I must have been looking at the wrong place in the build log files.

It may have worked locally because you had the feature gate enabled in your install. :) I suspect if you reinstalled with the gate off, you'd see the failure. (I rarely flip when I'm testing locally, so completely understood.)

zmerlynn avatar May 14 '24 15:05 zmerlynn

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.

github-actions[bot] avatar May 14 '24 20:05 github-actions[bot]

Build Failed :scream:

Build Id: 70b984e2-132b-4d1a-915a-862cb03f4830

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

agones-bot avatar May 14 '24 21:05 agones-bot

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.

github-actions[bot] avatar May 14 '24 21:05 github-actions[bot]