Introduce the Source field in GameServerAllocationStatus to indicate the allocation source #2860

gongmax · 2022-12-09T00:29:07Z

…the allocation source

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:
Introduce the Source field in GameServerAllocationStatus and AllocationResponse to indicate the allocation source. If the allocation is from a remote cluster, the value of the Source field will be the corresponding remote allocation endpoint.
This filed is used by the metrics.SetResponse method so it can skip the look up of the allocated game server in the local cluster in the case that the allocation is from remote cluster.

Which issue(s) this PR fixes:

Closes #2498

Special notes for your reviewer:

I accidentally closed #2799 and discarded all the commits when I tried to resolve merge conflict. This PR is identical as that one, with some documentation change which address the comments on the previous PR

…the allocation source

agones-bot · 2022-12-09T00:35:01Z

Build Failed 😱

Build Id: 82e2d831-ab7c-4cc5-9789-7663ef5b0074

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

agones-bot · 2022-12-09T01:44:55Z

Build Succeeded 👏

Build Id: 0cf4e983-9839-4cea-988e-c5eaf22c1c9c

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-9f4f050-amd64
image: us-docker.pkg.dev/agones-images/ci/agones-sdk:1.29.0-9f4f050-linux-amd64
image: us-docker.pkg.dev/agones-images/ci/agones-ping:1.29.0-9f4f050-amd64
image: us-docker.pkg.dev/agones-images/ci/agones-allocator:1.29.0-9f4f050-amd64
Linux C++ SDK (build): agonessdk-1.29.0-9f4f050-amd64-linux-arch_64.tar.gz
SDK Server: agonessdk-server-1.29.0-9f4f050-amd64.zip

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

https://9f4f050-dot-preview-dot-agones-images.appspot.com/

To install this version:

git fetch https://github.com/googleforgames/agones.git pull/2860/head:pr_2860 && git checkout pr_2860
helm install agones ./install/helm/agones --namespace agones-system --set agones.image.tag=1.29.0-9f4f050-amd64

agones-bot · 2022-12-09T06:00:15Z

Build Failed 😱

Build Id: 8307d4e7-52f3-402b-8805-ac4c5bb3ab5e

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

agones-bot · 2022-12-09T06:36:07Z

Build Succeeded 👏

Build Id: c6f050b1-57c3-4b41-9207-7c8351ecb3a8

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-e959de4-amd64
image: us-docker.pkg.dev/agones-images/ci/agones-sdk:1.29.0-e959de4-linux-amd64
image: us-docker.pkg.dev/agones-images/ci/agones-ping:1.29.0-e959de4-amd64
image: us-docker.pkg.dev/agones-images/ci/agones-allocator:1.29.0-e959de4-amd64
Linux C++ SDK (build): agonessdk-1.29.0-e959de4-amd64-linux-arch_64.tar.gz
SDK Server: agonessdk-server-1.29.0-e959de4-amd64.zip

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

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

To install this version:

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

agones-bot · 2022-12-09T07:03:45Z

Build Succeeded 👏

Build Id: d498a4e3-3da6-4d91-a5a9-8ab8ba649aba

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-ceef158-amd64
image: us-docker.pkg.dev/agones-images/ci/agones-sdk:1.29.0-ceef158-linux-amd64
image: us-docker.pkg.dev/agones-images/ci/agones-ping:1.29.0-ceef158-amd64
image: us-docker.pkg.dev/agones-images/ci/agones-allocator:1.29.0-ceef158-amd64
Linux C++ SDK (build): agonessdk-1.29.0-ceef158-amd64-linux-arch_64.tar.gz
SDK Server: agonessdk-server-1.29.0-ceef158-amd64.zip

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

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

To install this version:

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

markmandel · 2022-12-09T22:43:44Z

Just took a look at the preview website, and unfortunately this happened 😁

Looks like there's some malformed markdown in there that needs tweaking. Otherwise, this looks good to go.

gongmax · 2022-12-10T00:53:50Z

Fixed the markdown issue 😓
Lessons learned: run make site-server to verify before pushing any website change 😊

agones-bot · 2022-12-10T01:17:08Z

Build Succeeded 👏

Build Id: 170821e1-1694-4a91-9813-d0699f14f3b5

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-d55e0ce-amd64
image: us-docker.pkg.dev/agones-images/ci/agones-sdk:1.29.0-d55e0ce-linux-amd64
image: us-docker.pkg.dev/agones-images/ci/agones-ping:1.29.0-d55e0ce-amd64
image: us-docker.pkg.dev/agones-images/ci/agones-allocator:1.29.0-d55e0ce-amd64
Linux C++ SDK (build): agonessdk-1.29.0-d55e0ce-amd64-linux-arch_64.tar.gz
SDK Server: agonessdk-server-1.29.0-d55e0ce-amd64.zip

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

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

To install this version:

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

agones-bot · 2022-12-10T01:34:08Z

Build Failed 😱

Build Id: 4caa5065-80e2-49f9-a4d5-650c9ba96912

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

agones-bot · 2022-12-10T08:13:54Z

Build Succeeded 👏

Build Id: 7ec9e5a2-58be-4fea-9217-4405fa9fb817

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-a3e3c6e-amd64
image: us-docker.pkg.dev/agones-images/ci/agones-sdk:1.29.0-a3e3c6e-linux-amd64
image: us-docker.pkg.dev/agones-images/ci/agones-ping:1.29.0-a3e3c6e-amd64
image: us-docker.pkg.dev/agones-images/ci/agones-allocator:1.29.0-a3e3c6e-amd64
Linux C++ SDK (build): agonessdk-1.29.0-a3e3c6e-amd64-linux-arch_64.tar.gz
SDK Server: agonessdk-server-1.29.0-a3e3c6e-amd64.zip

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

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

To install this version:

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

aimuz · 2022-12-10T08:55:24Z

@markmandel

Can we add a CI check to avoid errors caused by not executing some compile commands after modifying the file?

If you don't click on it manually to see it, then the problem may be hidden, if we have a CI check to check if the files are consistent it can expose some problem areas

aimuz

Thank you

aimuz · 2022-12-10T09:04:25Z

pkg/allocation/converters/converter_test.go

 			},
 			want: &allocationv1.GameServerAllocation{
 				TypeMeta: metav1.TypeMeta{
 					Kind:       "GameServerAllocation",
 					APIVersion: "allocation.agones.dev/v1",
 				},
 				Status: allocationv1.GameServerAllocationStatus{
-					State: allocationv1.GameServerAllocationAllocated,
+					State:  allocationv1.GameServerAllocationAllocated,
+					Source: "33.188.237.156:443",


If this is a real IP, it could leave you open to attack

Thanks for the remind. It's a fake one.

optional: https://datatracker.ietf.org/doc/html/rfc5737#section-3 if you want a test IP.

aimuz · 2022-12-10T09:06:13Z

pkg/apis/allocation/v1/gameserverallocation.go

@@ -279,6 +279,9 @@ type GameServerAllocationStatus struct {
 	Ports          []agonesv1.GameServerStatusPort `json:"ports,omitempty"`
 	Address        string                          `json:"address,omitempty"`
 	NodeName       string                          `json:"nodeName,omitempty"`
+	// If the allocation is from a remote cluster, Source is the endpoint of the remote agones-allocator.
+	// Otherwise, Source is "local"
+	Source         string                          `json:"source"`  //nolint:goimports


//nolint:goimports

What check is it to close?

It complains the extra tab and spaces that were added by gofmt to keep the alignment. The issue is that there are extra lines between line 281 and 284, and the linter seems not smart enough and thought those extra tabs and spaces in line 284 was violations.

I think we should use gofmt formatting, even though the formatting may not be ideal, but he can become a standard, and if other participants modify the problem subsequently, he will eventually become gofmt format, otherwise the participants need to control the aspect of the modification.

So I don't think we should have to force to align the format above.

Thanks

Sounds fair. Also I found the old discussion around the issue. So gofmt behaves as designed, though the behavior doesn't properly fit all the use cases.

agones-bot · 2022-12-11T06:41:03Z

Build Succeeded 👏

Build Id: b90cc9ea-a68d-4b3a-9bae-8912b010b626

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-70b0131-amd64
image: us-docker.pkg.dev/agones-images/ci/agones-sdk:1.29.0-70b0131-linux-amd64
image: us-docker.pkg.dev/agones-images/ci/agones-ping:1.29.0-70b0131-amd64
image: us-docker.pkg.dev/agones-images/ci/agones-allocator:1.29.0-70b0131-amd64
Linux C++ SDK (build): agonessdk-1.29.0-70b0131-amd64-linux-arch_64.tar.gz
SDK Server: agonessdk-server-1.29.0-70b0131-amd64.zip

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

https://70b0131-dot-preview-dot-agones-images.appspot.com/

To install this version:

git fetch https://github.com/googleforgames/agones.git pull/2860/head:pr_2860 && git checkout pr_2860
helm install agones ./install/helm/agones --namespace agones-system --set agones.image.tag=1.29.0-70b0131-amd64

aimuz · 2022-12-11T07:30:22Z

LGTM

agones-bot · 2022-12-12T18:04:16Z

Build Succeeded 👏

Build Id: ef620a5f-ca71-4c66-b091-4f5142e2aba1

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-7bfce84-amd64
image: us-docker.pkg.dev/agones-images/ci/agones-sdk:1.29.0-7bfce84-linux-amd64
image: us-docker.pkg.dev/agones-images/ci/agones-ping:1.29.0-7bfce84-amd64
image: us-docker.pkg.dev/agones-images/ci/agones-allocator:1.29.0-7bfce84-amd64
Linux C++ SDK (build): agonessdk-1.29.0-7bfce84-amd64-linux-arch_64.tar.gz
SDK Server: agonessdk-server-1.29.0-7bfce84-amd64.zip

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

https://7bfce84-dot-preview-dot-agones-images.appspot.com/

To install this version:

git fetch https://github.com/googleforgames/agones.git pull/2860/head:pr_2860 && git checkout pr_2860
helm install agones ./install/helm/agones --namespace agones-system --set agones.image.tag=1.29.0-7bfce84-amd64

zmerlynn

LGTM

zmerlynn · 2022-12-13T00:11:21Z

pkg/allocation/converters/converter_test.go

 			},
 			want: &allocationv1.GameServerAllocation{
 				TypeMeta: metav1.TypeMeta{
 					Kind:       "GameServerAllocation",
 					APIVersion: "allocation.agones.dev/v1",
 				},
 				Status: allocationv1.GameServerAllocationStatus{
-					State: allocationv1.GameServerAllocationAllocated,
+					State:  allocationv1.GameServerAllocationAllocated,
+					Source: "33.188.237.156:443",


optional: https://datatracker.ietf.org/doc/html/rfc5737#section-3 if you want a test IP.

google-oss-prow · 2022-12-13T00:13:22Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: aimuz, gongmax, zmerlynn
Once this PR has been reviewed and has the lgtm label, please assign roberthbailey for approval by writing /assign @roberthbailey in a comment. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

OWNERS

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

agones-bot · 2022-12-13T00:54:10Z

Build Succeeded 👏

Build Id: 98931481-1ab5-41c9-b001-c8b8f02b52cb

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-a847f81-amd64
image: us-docker.pkg.dev/agones-images/ci/agones-sdk:1.29.0-a847f81-linux-amd64
image: us-docker.pkg.dev/agones-images/ci/agones-ping:1.29.0-a847f81-amd64
image: us-docker.pkg.dev/agones-images/ci/agones-allocator:1.29.0-a847f81-amd64
Linux C++ SDK (build): agonessdk-1.29.0-a847f81-amd64-linux-arch_64.tar.gz
SDK Server: agonessdk-server-1.29.0-a847f81-amd64.zip

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

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

To install this version:

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

gongmax added 2 commits December 9, 2022 00:10

Introduce the Source field in GameServerAllocationStatus to indicate …

759538b

…the allocation source

fix indent

2d5c9a1

google-oss-prow bot added the do-not-merge/work-in-progress label Dec 9, 2022

google-oss-prow bot requested review from dzlier-gcp and EricFortin December 9, 2022 00:29

google-oss-prow bot added the size/XXL label Dec 9, 2022

ignore space check for the new added line

9f4f050

Merge branch 'googleforgames:main' into fix_warning

e959de4

gongmax marked this pull request as ready for review December 9, 2022 06:36

google-oss-prow bot removed the do-not-merge/work-in-progress label Dec 9, 2022

google-oss-prow bot requested a review from aLekSer December 9, 2022 06:36

Merge branch 'main' into fix_warning

ceef158

gongmax and others added 2 commits December 9, 2022 16:50

Merge branch 'googleforgames:main' into fix_warning

d55e0ce

fix markdown

a3e3c6e

roberthbailey assigned zmerlynn Dec 10, 2022

aimuz reviewed Dec 10, 2022

View reviewed changes

apply gofmt formatting

70b0131

aimuz approved these changes Dec 12, 2022

View reviewed changes

google-oss-prow bot assigned aimuz Dec 12, 2022

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

Merge branch 'main' into fix_warning

7bfce84

google-oss-prow bot removed the lgtm label Dec 12, 2022

gongmax requested review from markmandel and zmerlynn and removed request for EricFortin, aLekSer and dzlier-gcp December 13, 2022 00:04

Merge branch 'main' into fix_warning

a847f81

zmerlynn approved these changes Dec 13, 2022

View reviewed changes

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

gongmax merged commit 05da749 into googleforgames:main Dec 13, 2022

gongmax deleted the fix_warning branch December 13, 2022 00:55

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

Introduce the Source field in GameServerAllocationStatus to indicate the allocation source #2860

Introduce the Source field in GameServerAllocationStatus to indicate the allocation source #2860

gongmax commented Dec 9, 2022

agones-bot commented Dec 9, 2022

agones-bot commented Dec 9, 2022

agones-bot commented Dec 9, 2022

agones-bot commented Dec 9, 2022

agones-bot commented Dec 9, 2022

markmandel commented Dec 9, 2022

gongmax commented Dec 10, 2022

agones-bot commented Dec 10, 2022

agones-bot commented Dec 10, 2022

agones-bot commented Dec 10, 2022

aimuz commented Dec 10, 2022

aimuz left a comment

aimuz Dec 10, 2022

gongmax Dec 11, 2022

zmerlynn Dec 13, 2022

aimuz Dec 10, 2022

gongmax Dec 11, 2022

aimuz Dec 11, 2022

gongmax Dec 11, 2022

agones-bot commented Dec 11, 2022

aimuz commented Dec 11, 2022

agones-bot commented Dec 12, 2022

zmerlynn left a comment

zmerlynn Dec 13, 2022

google-oss-prow bot commented Dec 13, 2022

agones-bot commented Dec 13, 2022

Introduce the Source field in GameServerAllocationStatus to indicate the allocation source #2860

Introduce the Source field in GameServerAllocationStatus to indicate the allocation source #2860

Conversation

gongmax commented Dec 9, 2022

agones-bot commented Dec 9, 2022

agones-bot commented Dec 9, 2022

agones-bot commented Dec 9, 2022

agones-bot commented Dec 9, 2022

agones-bot commented Dec 9, 2022

markmandel commented Dec 9, 2022

gongmax commented Dec 10, 2022

agones-bot commented Dec 10, 2022

agones-bot commented Dec 10, 2022

agones-bot commented Dec 10, 2022

aimuz commented Dec 10, 2022

aimuz 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

Choose a reason for hiding this comment

agones-bot commented Dec 11, 2022

aimuz commented Dec 11, 2022

agones-bot commented Dec 12, 2022

zmerlynn left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

google-oss-prow bot commented Dec 13, 2022

agones-bot commented Dec 13, 2022