GameServer Pod: Stable Network ID #2826

markmandel · 2022-11-26T02:30:12Z

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

What this PR does / Why we need it:

This change sets the hostName (if not already set) on the Pod of the GameServer, if an end user would like a DNS entry to communicate directly to the GameServer Pod in the cluster.

Which issue(s) this PR fixes:

Closes #2704

Special notes for your reviewer:

Since this is a relatively minor change, no FeatureFlag was created. Please let me know if you disagree with this approach.

agones-bot · 2022-11-26T02:53:34Z

Build Failed 😱

Build Id: a2f5eb53-5503-4f37-9742-ef112879052e

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

markmandel · 2022-11-26T18:08:06Z

Legit bug:

time="2022-11-26 02:48:11.553" level=info msg="Event!" fleet=simple-fleet-1.0rbdrf gs=simple-fleet-1.0rbdrf-tp65r-gvtk9 lastTimestamp="2022-11-26 02:48:10 +0000 UTC" message="Pod \"simple-fleet-1.0rbdrf-tp65r-gvtk9\" is invalid: spec.hostname: Invalid value: \"simple-fleet-1.0rbdrf-tp65r-gvtk9\": a lowercase RFC 1123 label must consist of lower case alphanumeric characters or '-', and must start and end with an alphanumeric character (e.g. 'my-name',  or '123-abc', regex used for validation is '[a-z0-9]([-a-z0-9]*[a-z0-9])?')" reason=Error test=TestAllocatorWithDeprecatedRequired type=Warning
time="2022-11-26 02:48:11.554" level=info msg="GameServer state dump:" fleet=simple-fleet-1.0rbdrf gs=simple-fleet-1.0rbdrf-tp65r-p5vzt status="{State:PortAllocation Ports:[] Address: NodeName: ReservedUntil:<nil> Players:<nil>}" test=TestAllocatorWithDeprecatedRequired
time="2022-11-26 02:48:11.554" level=info msg="Dumping Events:" fleet=simple-fleet-1.0rbdrf gs=simple-fleet-1.0rbdrf-tp65r-p5vzt kind=GameServer test=TestAllocatorWithDeprecatedRequired

agones-bot · 2022-11-27T01:16:54Z

Build Succeeded 👏

Build Id: 08ac1206-8b79-4f51-bf5c-dbf3d0b4c84d

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

image: us-docker.pkg.dev/agones-images/ci/agones-controller:1.28.0-c50003c-amd64
image: us-docker.pkg.dev/agones-images/ci/agones-sdk:1.28.0-c50003c-linux-amd64
image: us-docker.pkg.dev/agones-images/ci/agones-ping:1.28.0-c50003c-amd64
image: us-docker.pkg.dev/agones-images/ci/agones-allocator:1.28.0-c50003c-amd64
Linux C++ SDK (build): agonessdk-1.28.0-c50003c-amd64-linux-arch_64.tar.gz
SDK Server: agonessdk-server-1.28.0-c50003c-amd64.zip

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

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

To install this version:

git fetch https://github.com/googleforgames/agones.git pull/2826/head:pr_2826 && git checkout pr_2826
helm install agones ./install/helm/agones --namespace agones-system --set agones.image.tag=1.28.0-c50003c-amd64

test/e2e/gameserver_test.go

roberthbailey · 2022-11-29T10:46:25Z

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

+A group of `Pods` attached to `GameServers` can use a 
+[Headless Service](https://kubernetes.io/docs/concepts/services-networking/service/#headless-services) to control 
+the domain of the Pods, along with providing 
+a [`subdomain` value to the `GameServer` `PodTemplateSpec`](https://kubernetes.io/docs/concepts/services-networking/dns-pod-service/#pod-s-hostname-and-subdomain-fields)


This documentation says:

Currently when a Pod is created, its hostname is the Pod's metadata.name value.

Which sounds like k8s already does exactly what you are adding in this PR. Do we need to explicitly set the hostname field?

I went back and forth on this as well, but then I read this part:
https://kubernetes.io/docs/concepts/services-networking/dns-pod-service/#pod-s-hostname-and-subdomain-fields:~:text=Note%3A%20Because,on%20the%20Service.

Note: Because A or AAAA records are not created for Pod names, hostname is required for the Pod's A or AAAA record to be created. A Pod with no hostname but with subdomain will only create the A or AAAA record for the headless Service (default-subdomain.my-namespace.svc.cluster-domain.example), pointing to the Pod's IP address. Also, Pod needs to become ready in order to have a record unless publishNotReadyAddresses=True is set on the Service.

So TL;DR - to have a A record for the pod, it has to have a hostname specified.

This isn't surprising to me - since the (general) formatting for a resource name allows characters and lengths of strings that aren't allowed in host names for DNS entries.

I think where they say "hostname" above, what they mean is the internal hostname to the container - not an external DNS entry.

agones-bot · 2022-12-01T21:24:37Z

Build Succeeded 👏

Build Id: c10faba8-dbd1-41cf-95a3-e8aedde4948a

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

image: us-docker.pkg.dev/agones-images/ci/agones-controller:1.28.0-3a39772-amd64
image: us-docker.pkg.dev/agones-images/ci/agones-sdk:1.28.0-3a39772-linux-amd64
image: us-docker.pkg.dev/agones-images/ci/agones-ping:1.28.0-3a39772-amd64
image: us-docker.pkg.dev/agones-images/ci/agones-allocator:1.28.0-3a39772-amd64
Linux C++ SDK (build): agonessdk-1.28.0-3a39772-amd64-linux-arch_64.tar.gz
SDK Server: agonessdk-server-1.28.0-3a39772-amd64.zip

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

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

To install this version:

git fetch https://github.com/googleforgames/agones.git pull/2826/head:pr_2826 && git checkout pr_2826
helm install agones ./install/helm/agones --namespace agones-system --set agones.image.tag=1.28.0-3a39772-amd64

zmerlynn · 2022-12-05T21:56:15Z

I think I would be more comfortable with this under a feature gate. Honestly, I think it can go straight to beta, but I think it would be good if we let people turn it off for a little while.

zmerlynn · 2022-12-09T22:33:28Z

pkg/apis/agones/v1/gameserver.go

+		// replace . with - since it must match RFC 1123
+		pod.Spec.Hostname = strings.ReplaceAll(gs.ObjectMeta.Name, ".", "-")
+	}
+
 	gs.podObjectMeta(pod)


Just adding this as a formal review comment. I think since this is altering the generated pod in a possibly significant way, we should add a feature gate. It can go straight to beta, IMO - but especially in a world where we're releasing straight from main, I think we should be a little cautious.

Sounds good to me - I'll make the change.

...and done!

This change sets the hostName (if not already set) on the Pod of the GameServer, if an end user would like a DNS entry to communicate directly to the GameServer Pod in the cluster. Since this is a relatively minor change, no FeatureFlag was created. Closes googleforgames#2704

agones-bot · 2022-12-15T23:26:28Z

Build Succeeded 👏

Build Id: 1436c59a-5cb2-4b23-b5bf-c07f8160de75

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

image: us-docker.pkg.dev/agones-images/ci/agones-controller:1.29.0-c55a17d-amd64
image: us-docker.pkg.dev/agones-images/ci/agones-sdk:1.29.0-c55a17d-linux-amd64
image: us-docker.pkg.dev/agones-images/ci/agones-ping:1.29.0-c55a17d-amd64
image: us-docker.pkg.dev/agones-images/ci/agones-allocator:1.29.0-c55a17d-amd64
Linux C++ SDK (build): agonessdk-1.29.0-c55a17d-amd64-linux-arch_64.tar.gz
SDK Server: agonessdk-server-1.29.0-c55a17d-amd64.zip

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

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

To install this version:

git fetch https://github.com/googleforgames/agones.git pull/2826/head:pr_2826 && git checkout pr_2826
helm install agones ./install/helm/agones --namespace agones-system --set agones.image.tag=1.29.0-c55a17d-amd64

google-oss-prow · 2022-12-15T23:43:42Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: markmandel, zmerlynn

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

markmandel added kind/feature New features for Agones area/user-experience Pertaining to developers trying to use Agones, e.g. SDK, installation, etc labels Nov 26, 2022

markmandel requested a review from roberthbailey November 26, 2022 02:30

google-oss-prow bot requested review from aLekSer and EricFortin November 26, 2022 02:30

google-oss-prow bot added approved size/M labels Nov 26, 2022

markmandel force-pushed the feature/pod-hostname branch from 48fb90d to c50003c Compare November 27, 2022 00:48

roberthbailey reviewed Nov 29, 2022

View reviewed changes

mangalpalli added the feature-freeze-do-not-merge Only eligible to be merged once we are out of feature freeze (next full release) label Nov 29, 2022

markmandel force-pushed the feature/pod-hostname branch from c50003c to 3a39772 Compare December 1, 2022 20:57

mangalpalli removed the feature-freeze-do-not-merge Only eligible to be merged once we are out of feature freeze (next full release) label Dec 7, 2022

roberthbailey requested review from zmerlynn and removed request for EricFortin and aLekSer December 9, 2022 05:23

roberthbailey assigned zmerlynn Dec 9, 2022

zmerlynn reviewed Dec 9, 2022

View reviewed changes

markmandel added 3 commits December 15, 2022 14:02

Review updates.

2dbf677

Added feature gate.

c55a17d

markmandel force-pushed the feature/pod-hostname branch from 3a39772 to c55a17d Compare December 15, 2022 22:58

google-oss-prow bot added size/L and removed size/M labels Dec 15, 2022

zmerlynn approved these changes Dec 15, 2022

View reviewed changes

google-oss-prow bot added the lgtm label Dec 15, 2022

zmerlynn merged commit 533df8a into googleforgames:main Dec 15, 2022

markmandel deleted the feature/pod-hostname branch December 16, 2022 17:43

Kalaiselvi84 added this to the 1.29.0 milestone Jan 17, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

GameServer Pod: Stable Network ID #2826

GameServer Pod: Stable Network ID #2826

markmandel commented Nov 26, 2022

agones-bot commented Nov 26, 2022

markmandel commented Nov 26, 2022

agones-bot commented Nov 27, 2022

roberthbailey Nov 29, 2022

markmandel Dec 1, 2022

agones-bot commented Dec 1, 2022

zmerlynn commented Dec 5, 2022

zmerlynn Dec 9, 2022

markmandel Dec 9, 2022

markmandel Dec 15, 2022

agones-bot commented Dec 15, 2022

google-oss-prow bot commented Dec 15, 2022

GameServer Pod: Stable Network ID #2826

GameServer Pod: Stable Network ID #2826

Conversation

markmandel commented Nov 26, 2022

agones-bot commented Nov 26, 2022

markmandel commented Nov 26, 2022

agones-bot commented Nov 27, 2022

roberthbailey Nov 29, 2022

Choose a reason for hiding this comment

markmandel Dec 1, 2022

Choose a reason for hiding this comment

agones-bot commented Dec 1, 2022

zmerlynn commented Dec 5, 2022

zmerlynn Dec 9, 2022

Choose a reason for hiding this comment

markmandel Dec 9, 2022

Choose a reason for hiding this comment

markmandel Dec 15, 2022

Choose a reason for hiding this comment

agones-bot commented Dec 15, 2022

google-oss-prow bot commented Dec 15, 2022