Add labels and annotations to allocation response #3197

austin-space · 2023-06-01T22:06:43Z

What type of PR is this?

/kind feature

What this PR does / Why we need it:
This PR is not complete. It almost certainly needs more tests and I haven't touched the documentation. I'm mostly looking for a quick gut check to see if I'm missing anything glaring.

This PR returns the labels and annotations of an allocated game server at allocation time with the allocation response or Allocation CR status. This opens up a few very interesting scenarios:

the ability to get some information from the game server that was generated by the game server(additional connection info)
the ability to game server state information(such as map or hardware class)
the ability to get insight into how allocations are getting processed(for example, we can get an idea of how far we're falling through our allocation selector chain)

Which issue(s) this PR fixes:

Closes #2396

Special notes for your reviewer:
I tried to break out the changes I've made thus far into two commits: the first being my changes, the second being the result of running the various gen make commands. Hopefully this makes this PR a little more readable. I undid most of the autogen stuff since the sdk autogen stuff appears to be broken(related to the counts and lists changes it seems).

Also worth noting that I originally tried to put this behind a feature flag, but I don't think that actually makes much sense since the fields will be in the protobuf no matter whether we put something in there or not and it could potentially be confusing for users since there is no alpha allocation protobuf. If you want I can add the feature flag back.

agones-bot · 2023-06-01T22:12:51Z

Build Failed 😱

Build Id: 8e006635-244e-4c48-b9e0-b6522e067006

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

gongmax · 2023-06-01T22:15:48Z

linter failure

pkg/apis/allocation/v1/gameserverallocation.go:614:6: exported: exported type GameServerMetadata should have comment or be unexported (revive)
type GameServerMetadata struct {
     ^

agones-bot · 2023-06-01T22:26:24Z

Build Failed 😱

Build Id: 1adb67bb-0a91-4f46-9fbe-956fbef0d01d

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

This reverts commit adb968d.

austin-space · 2023-06-01T23:29:53Z

I removed the sdk generation and a change I had made to get that working for node js. Just as a heads up, it seems like that may have been broken since the last time someone made changes to the protobuf a few months ago.

agones-bot · 2023-06-01T23:38:27Z

Build Failed 😱

Build Id: 83d79f82-28e7-46e3-b67d-aedb92ac0a81

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

agones-bot · 2023-06-02T00:28:39Z

Build Failed 😱

Build Id: 569616a3-0c59-424a-932f-3ea2814a6fdb

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

agones-bot · 2023-06-02T20:14:58Z

Build Succeeded 👏

Build Id: 2b15d7b9-0cfe-43e0-bb7e-c973c9f66a33

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

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

https://6da62ab-dot-preview-dot-agones-images.appspot.com/

To install this version:

git fetch https://github.com/googleforgames/agones.git pull/3197/head:pr_3197 && git checkout pr_3197
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.33.0-6da62ab-amd64

markmandel

Thanks for doing all this work! Added a few questions inline.

Only other things I can think of to add:

Updates to the documentation, at the very least the GameServerAllocation Specification
Update to the example (which is basically the same as above) https://github.com/googleforgames/agones/blob/main/examples/gameserverallocation.yaml
End to end tests , could probably extend what we already have, primarily:

agones/test/e2e/gameserverallocation_test.go

Line 39 in 9c33780

func TestCreateFleetAndGameServerAllocate(t *testing.T) {

and

agones/test/e2e/allocator_test.go

Line 151 in 9c33780

func TestAllocatorWithSelectors(t *testing.T) {

Will suffice.

But really excited to have this in Agones!

@gongmax @zmerlynn - do we see any reason to put this behind a feature gate? I think it's likely fine, since I can't think of anything we'd want to change in this, but figured I would ask.

pkg/apis/allocation/v1/gameserverallocation.go

proto/allocation/allocation.proto

zmerlynn · 2023-06-09T15:52:18Z

@gongmax @zmerlynn - do we see any reason to put this behind a feature gate? I think it's likely fine, since I can't think of anything we'd want to change in this, but figured I would ask.

Since it's a new return from the API, I can't imagine needing a feature gate - rather, I think the question is whether it should be introduced straight to the v1 surface or go through the beta API surface first. I think the answer to that is still no, though - I'm ok with this going straight to v1.

gongmax · 2023-06-09T15:59:07Z

@gongmax @zmerlynn - do we see any reason to put this behind a feature gate? I think it's likely fine, since I can't think of anything we'd want to change in this, but figured I would ask.

Since it's a new return from the API, I can't imagine needing a feature gate - rather, I think the question is whether it should be introduced straight to the v1 surface or go through the beta API surface first. I think the answer to that is still no, though - I'm ok with this going straight to v1.

I agreed. Previously we introduced the Source field in allocation response, which was not behind a feature gate. I cannot think of any drawback to not follow the same pattern.

markmandel · 2023-06-09T18:01:16Z

SGTM!

in which case one we have some docs, examples and e2e tests, this should be good to merge 👍🏻 🤸🏻

agones-bot · 2023-06-14T20:50:50Z

Build Failed 😱

Build Id: 609091b9-9d7f-4996-8d0b-628d7a73f7a2

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

agones-bot · 2023-06-14T21:23:48Z

Build Failed 😱

Build Id: 5efa7beb-70f5-42c3-abf9-5c6086613f73

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

agones-bot · 2023-06-15T02:36:19Z

Build Succeeded 👏

Build Id: 7776b798-09ee-43fe-a956-e4381c0ed92f

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

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

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

To install this version:

git fetch https://github.com/googleforgames/agones.git pull/3197/head:pr_3197 && git checkout pr_3197
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.33.0-afd3a51-amd64

austin-space · 2023-06-15T19:01:00Z

Assuming this build succeeds, this pr should be ready for another round of review

agones-bot · 2023-06-15T20:29:25Z

Build Succeeded 👏

Build Id: f2d4b4f5-ab86-46ba-b7ad-e19a6eb4c24c

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

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

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

To install this version:

git fetch https://github.com/googleforgames/agones.git pull/3197/head:pr_3197 && git checkout pr_3197
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.33.0-7acb057-amd64

markmandel

Looks good! Added some notes on some code cleanup.

One extra e2e test to add and I think we're good to go:
https://github.com/googleforgames/agones/blob/main/test/e2e/allocator_test.go

Just to e2e test the conversion to grpc and rest work as expected - you could even just add labels and annotations to the existing tests rather than spin up a whole new one if that's easier (also avoids waiting for the tls certs to refresh any more).

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

test/e2e/gameserverallocation_test.go

markmandel · 2023-06-16T21:54:39Z

pkg/sdk/sdk_grpc.pb.go

@@ -16,7 +16,7 @@
 // Code generated by protoc-gen-go-grpc. DO NOT EDIT.
 // versions:
 // - protoc-gen-go-grpc v1.2.0
-// - protoc             v3.21.6
+// - protoc             v3.21.12


🤔 wondering why this is here? (and in the other sdk files)

That's a good question, and one I can't answer. I don't have a standalone install of protoc on my machine. Something I did notice is that some of the protobuf seems to have not been generated for some client libraries in some time(if you run it on main, it fails) so this may be an artifact of that?

Ooh I see - this is an artifact of running the make gen-allocation-grpc? (I would have thought it would have only happened for make gen-sdk-grpc 🤔 weird.

I mean it's a change in a comment - not the end of the world 🤷🏻 weird though.

Co-authored-by: Mark Mandel <markmandel@google.com>

agones-bot · 2023-06-16T22:39:48Z

Build Failed 😱

Build Id: a38915d6-ec0a-4bfc-807e-c4ad20279d94

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

…m/austin-space/agones into austin/add-labels-and-annotations

agones-bot · 2023-06-16T23:52:56Z

Build Succeeded 👏

Build Id: c04eb646-4e5c-4901-908e-bf50505f76fd

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

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

https://6bbc04f-dot-preview-dot-agones-images.appspot.com/

To install this version:

git fetch https://github.com/googleforgames/agones.git pull/3197/head:pr_3197 && git checkout pr_3197
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.33.0-6bbc04f-amd64

markmandel

Looks good! Sorry about the delay in final approval!

markmandel · 2023-06-22T21:21:04Z

test/e2e/allocator_test.go

@@ -208,6 +208,7 @@ func TestAllocatorWithSelectors(t *testing.T) {
 			require.NoError(t, err)
 			require.Equal(t, response.GameServerName, allocatedResponse.GameServerName)
 			helper.ValidateAllocatorResponse(t, allocatedResponse)
+			assert.Equal(t, flt.ObjectMeta.Name, allocatedResponse.Metadata.Labels[agonesv1.FleetNameLabel])


google-oss-prow · 2023-06-22T21:22:51Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: austin-space, 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~~ [markmandel]

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

google-oss-prow · 2023-06-22T21:23:24Z

New changes are detected. LGTM label has been removed.

agones-bot · 2023-06-22T22:15:49Z

Build Succeeded 👏

Build Id: 6881c65f-9871-4eb5-b2cf-d61750d52fd7

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

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

https://67b225e-dot-preview-dot-agones-images.appspot.com/

To install this version:

git fetch https://github.com/googleforgames/agones.git pull/3197/head:pr_3197 && git checkout pr_3197
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.33.0-dev-67b225e-amd64

agones-bot · 2023-06-28T04:42:11Z

Build Succeeded 👏

Build Id: 5508d002-df3c-425b-9011-231d35c00f9e

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

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

https://67b225e-dot-preview-dot-agones-images.appspot.com/

To install this version:

git fetch https://github.com/googleforgames/agones.git pull/3197/head:pr_3197 && git checkout pr_3197
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.33.0-dev-67b225e-amd64

austin-space added 2 commits June 1, 2023 13:52

Return labels and annotations with allocation responses

da91b5a

regen all sdk and crd files

adb968d

google-oss-prow bot requested review from markmandel and steven-supersolid June 1, 2023 22:06

google-oss-prow bot added the size/XXL label Jun 1, 2023

add comment to satisfy the linter overlords

44941a9

austin-space added 2 commits June 1, 2023 16:22

Revert "regen all sdk and crd files"

7e3f6ed

This reverts commit adb968d.

remove changes to sdk generation

de6913c

google-oss-prow bot added size/L and removed size/XXL labels Jun 1, 2023

austin-space changed the title ~~Austin/add labels and annotations~~ Add labels and annotations to allocation response Jun 1, 2023

regen deepcopy

77a3b80

regen api docs

6da62ab

google-oss-prow bot added size/XXL and removed size/L labels Jun 2, 2023

markmandel reviewed Jun 8, 2023

View reviewed changes

pkg/apis/allocation/v1/gameserverallocation.go Show resolved Hide resolved

proto/allocation/allocation.proto Show resolved Hide resolved

introduce new e2e test for allocations

9ade1c1

sacrifices to the linter

44103c9

austin-space added 2 commits June 15, 2023 10:43

Merge branch 'mainline/main' into austin/add-labels-and-annotations

2ae51a5

merge in main and regenerate site docs

7acb057

google-oss-prow bot added size/L and removed size/XXL labels Jun 15, 2023

markmandel reviewed Jun 16, 2023

View reviewed changes

austin-space and others added 4 commits June 16, 2023 15:31

Update test/e2e/gameserverallocation_test.go

f3b0f72

Co-authored-by: Mark Mandel <markmandel@google.com>

Update test/e2e/gameserverallocation_test.go

d73f884

Co-authored-by: Mark Mandel <markmandel@google.com>

Update site/content/en/docs/Reference/gameserverallocation.md

750dc7a

Co-authored-by: Mark Mandel <markmandel@google.com>

Add check for label in allocator test

b98ffce

austin-space added 2 commits June 16, 2023 15:40

Merge branch 'austin/add-labels-and-annotations' of https://github.co…

1010c4f

…m/austin-space/agones into austin/add-labels-and-annotations

fix indentation

6bbc04f

markmandel approved these changes Jun 22, 2023

View reviewed changes

google-oss-prow bot assigned markmandel Jun 22, 2023

google-oss-prow bot added the lgtm label Jun 22, 2023

google-oss-prow bot added the approved label Jun 22, 2023

markmandel enabled auto-merge (squash) June 22, 2023 21:23

Merge branch 'main' into austin/add-labels-and-annotations

67b225e

google-oss-prow bot removed the lgtm label Jun 22, 2023

markmandel added kind/feature New features for Agones lgtm labels Jun 22, 2023

markmandel merged commit e287cc3 into googleforgames:main Jun 22, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add labels and annotations to allocation response #3197

Add labels and annotations to allocation response #3197

austin-space commented Jun 1, 2023 •

edited

Loading

agones-bot commented Jun 1, 2023

gongmax commented Jun 1, 2023

agones-bot commented Jun 1, 2023

austin-space commented Jun 1, 2023

agones-bot commented Jun 1, 2023

agones-bot commented Jun 2, 2023

agones-bot commented Jun 2, 2023

markmandel left a comment •

edited

Loading

zmerlynn commented Jun 9, 2023

gongmax commented Jun 9, 2023

markmandel commented Jun 9, 2023

agones-bot commented Jun 14, 2023

agones-bot commented Jun 14, 2023

agones-bot commented Jun 15, 2023

austin-space commented Jun 15, 2023

agones-bot commented Jun 15, 2023

markmandel left a comment

markmandel Jun 16, 2023

austin-space Jun 16, 2023 •

edited

Loading

markmandel Jun 16, 2023

agones-bot commented Jun 16, 2023

agones-bot commented Jun 16, 2023

markmandel left a comment

markmandel Jun 22, 2023

google-oss-prow bot commented Jun 22, 2023

google-oss-prow bot commented Jun 22, 2023

agones-bot commented Jun 22, 2023

agones-bot commented Jun 28, 2023

Add labels and annotations to allocation response #3197

Add labels and annotations to allocation response #3197

Conversation

austin-space commented Jun 1, 2023 • edited Loading

agones-bot commented Jun 1, 2023

gongmax commented Jun 1, 2023

agones-bot commented Jun 1, 2023

austin-space commented Jun 1, 2023

agones-bot commented Jun 1, 2023

agones-bot commented Jun 2, 2023

agones-bot commented Jun 2, 2023

markmandel left a comment • edited Loading

Choose a reason for hiding this comment

zmerlynn commented Jun 9, 2023

gongmax commented Jun 9, 2023

markmandel commented Jun 9, 2023

agones-bot commented Jun 14, 2023

agones-bot commented Jun 14, 2023

agones-bot commented Jun 15, 2023

austin-space commented Jun 15, 2023

agones-bot commented Jun 15, 2023

markmandel left a comment

Choose a reason for hiding this comment

markmandel Jun 16, 2023

Choose a reason for hiding this comment

austin-space Jun 16, 2023 • edited Loading

Choose a reason for hiding this comment

markmandel Jun 16, 2023

Choose a reason for hiding this comment

agones-bot commented Jun 16, 2023

agones-bot commented Jun 16, 2023

markmandel left a comment

Choose a reason for hiding this comment

markmandel Jun 22, 2023

Choose a reason for hiding this comment

google-oss-prow bot commented Jun 22, 2023

google-oss-prow bot commented Jun 22, 2023

agones-bot commented Jun 22, 2023

agones-bot commented Jun 28, 2023

austin-space commented Jun 1, 2023 •

edited

Loading

markmandel left a comment •

edited

Loading

austin-space Jun 16, 2023 •

edited

Loading