Implement GameServerAlocation as API Extension #682

markmandel · 2019-03-31T22:49:19Z

~~Dependent on #659 which will be merged after 0.9.0 is released~~

This moves the implementation of GameServerAllocation (GSA) to a
Kubernetes API Extension instead of using CRDs. This was essentially done for performance reasons, but to break it down:

GSA is now create only. Since we had no need for GSA storage, and don't want the performance hit.
This removes the mutation and validation webhooks, which have 30s timeout and are run in serial
1. API Server still does cut off a response after 60s, but the api can continue processing (60s gives us enough time, I think for a SDK.Ack() on Allocate, which I don't think we had before)
2. Validation now happens in the request.
We can now do batching of requests for higher throughput (Use batching in GameServerAllocation controller to improve throughput. #536), since we control the entire http request.

The breaking changes are:

GameServerAllocation's group is now allocation.agones.dev rather than stable.agones.dev,
because a CRD group can't overlap with a api server.
Since there is only the create verb for GSA, there is no get/list/watch options for GameServerAllocations - so no informers/listers either. But this could be added at a later date, if needed.

markmandel · 2019-03-31T22:54:49Z

pkg/gameserverallocations/controller.go

-	gsa := &v1alpha1.GameServerAllocation{}
+// allocationHandler CRDHandler for allocating a gameserver. Only accepts POST
+// commands
+func (c *Controller) allocationHandler(w http.ResponseWriter, r *http.Request, namespace string) error {


/cc @pooneh-m @ilkercelikyilmaz for your review and testing (also, I don't think I broke anything you did @ilkercelikyilmaz , but would love for you to give it a look over to make sure) 👍

cmd/controller/main.go

agones-bot · 2019-03-31T23:16:56Z

Build Succeeded 👏

Build Id: f429d470-235b-4bb0-a514-5877a6c16ef3

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

image: gcr.io/agones-images/agones-controller:0.9.0-aaa6e91
image: gcr.io/agones-images/agones-sdk:0.9.0-aaa6e91
image: gcr.io/agones-images/agones-ping:0.9.0-aaa6e91
Linux C++ SDK (build: dev): agonessdk-0.9.0-aaa6e91-dev-linux-arch_64.tar.gz
Linux C++ SDK (build: runtime): agonessdk-0.9.0-aaa6e91-runtime-linux-arch_64.tar.gz
Linux C++ SDK (source): agonessdk-0.9.0-aaa6e91-src.zip
SDK Server: agonessdk-server-0.9.0-aaa6e91.zip

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

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

To install this version:

git fetch https://github.com/GoogleCloudPlatform/agones.git pull/682/head:pr_682 && git checkout pr_682
helm install install/helm/agones --namespace agones-system --name agones --set agones.image.tag=0.9.0-aaa6e91

ilkercelikyilmaz

LGTM

jkowalski · 2019-04-01T15:30:39Z

install/helm/agones/templates/extensions.yaml

@@ -16,12 +16,44 @@
 {{- $altName1 := printf "agones-controller-service.%s"  .Release.Namespace }}
 {{- $altName2 := printf "agones-controller-service.%s.svc" .Release.Namespace }}
 {{- $cert := genSignedCert $cn nil (list $altName1 $altName2) 3650 $ca }}
+---
+{{- if .Values.agones.registerApiService }}


is there a reason you don't want that?

also perhaps "registerAllocationApiService" ?

GCP Marketplace usually requires a different install step, so there are switches to turn various pieces off if people need to break the glass and copy things into their own install process.

@bbf to confirm.

Regarding the rename comment - we have registerWebhooks for all our webhooks - I don't think we need to be that granular. If there ever end up being more APIServices, they could also go here.

pkg/client/clientset/versioned/typed/allocation/v1alpha1/fake/fake_gameserverallocation.go

jkowalski · 2019-04-01T15:33:21Z

examples/simple-udp/gameserverallocation.yaml


-apiVersion: "stable.agones.dev/v1alpha1"
+apiVersion: "allocation.agones.dev/v1alpha1"


general question - is allocation the only thing that will be in this API going forward? Perhaps we need a broader name to indicate "services that external consumers are going to call".

That's a really interesting question. Groups are pretty broad, and we could definitely expand this - and anything that is "external" would likely need to be managed by multiple pods to ensure HA (as discussed above). Which would give us an internal delineation - anything under this group needs to be managed by multiple pods, anything in the stable group, is managed by the controller (which self heals).

Maybe external.agones.dev ?

It also begets a question of is "stable" the right name for the original group. I'm starting to wonder if core.agones.dev would have been better? Or something else?

Definitely open to suggestions here.

Looking at #703 - I'm feeling like allocation is actually the right choice here for the group name.

agones-bot · 2019-04-03T18:23:18Z

Build Failed 😱

Build Id: 28911819-68c5-432b-963c-77cc485c370f

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

agones-bot · 2019-04-04T01:06:42Z

Build Succeeded 👏

Build Id: 3bf0a23d-7862-488b-9259-86cc58634dc1

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

image: gcr.io/agones-images/agones-controller:0.10.0-3698601
image: gcr.io/agones-images/agones-sdk:0.10.0-3698601
image: gcr.io/agones-images/agones-ping:0.10.0-3698601
Linux C++ SDK (build): agonessdk-0.10.0-3698601-linux-arch_64.tar.gz
SDK Server: agonessdk-server-0.10.0-3698601.zip

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

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

To install this version:

git fetch https://github.com/GoogleCloudPlatform/agones.git pull/682/head:pr_682 && git checkout pr_682
helm install install/helm/agones --namespace agones-system --name agones --set agones.image.tag=0.10.0-3698601

markmandel · 2019-04-05T13:37:45Z

This should be good for review now 👍

agones-bot · 2019-04-05T13:51:45Z

Build Succeeded 👏

Build Id: c78236ba-778a-4e8d-a0d7-97e7218c6385

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

image: gcr.io/agones-images/agones-controller:0.10.0-c11accf
image: gcr.io/agones-images/agones-sdk:0.10.0-c11accf
image: gcr.io/agones-images/agones-ping:0.10.0-c11accf
Linux C++ SDK (build): agonessdk-0.10.0-c11accf-linux-arch_64.tar.gz
SDK Server: agonessdk-server-0.10.0-c11accf.zip

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

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

To install this version:

git fetch https://github.com/GoogleCloudPlatform/agones.git pull/682/head:pr_682 && git checkout pr_682
helm install install/helm/agones --namespace agones-system --name agones --set agones.image.tag=0.10.0-c11accf

markmandel · 2019-04-13T06:48:41Z

site/content/en/docs/Reference/agones_crd_api_reference.html

@@ -2219,6 +2219,10 @@ <h3 id="WebhookPolicy">WebhookPolicy



+
+
+


@aLekSer I can't work out why I'm not getting the allocation group for the GameServerAllocation on the docs? Do you have any ideas?

@markmandel, working on that.
I see errors when running make gen-api-docs:

W0415 09:33:04.694267 8 parse.go:239] Ignoring child directory agones.dev/agones/pkg/apis/allocation: unable to import "agones.dev/agones/pkg/apis/allocation": go/build: importGo agones.dev/agones/pkg/apis/allocation: exit status 1 go: finding agones.dev/agones/pkg/apis/allocation latest go: finding agones.dev/agones/pkg/apis latest go: finding agones.dev/agones/pkg latest go: finding agones.dev/agones v0.9.0 go: downloading agones.dev/agones v0.9.0 go: extracting agones.dev/agones v0.9.0 can't load package: package agones.dev/agones/pkg/apis/allocation: unknown import path "agones.dev/agones/pkg/apis/allocation": cannot find module providing package agones.dev/agones/pkg/apis/allocation W0415 09:33:06.845690 8 parse.go:239] Ignoring child directory agones.dev/agones/pkg/apis/allocation/v1alpha1: unable to import "agones.dev/agones/pkg/apis/allocation/v1alpha1": go/build: importGo agones.dev/agones/pkg/apis/allocation/v1alpha1: exit status 1

Fixed the issue using:

go mod edit --replace=agones.dev/agones@latest=../../../agones.dev/agones/

Resulted webpage could be found here (cherry-picked my go mod fix to this PR):
http://35.237.42.204:1313/docs/reference/agones_crd_api_reference/

agones-bot · 2019-04-13T07:08:08Z

Build Succeeded 👏

Build Id: 8708d939-c574-442a-87af-0564f749995b

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

image: gcr.io/agones-images/agones-controller:0.10.0-a1bf145
image: gcr.io/agones-images/agones-sdk:0.10.0-a1bf145
image: gcr.io/agones-images/agones-ping:0.10.0-a1bf145
Linux C++ SDK (build): agonessdk-0.10.0-a1bf145-linux-arch_64.tar.gz
SDK Server: agonessdk-server-0.10.0-a1bf145.zip

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

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

To install this version:

git fetch https://github.com/GoogleCloudPlatform/agones.git pull/682/head:pr_682 && git checkout pr_682
helm install install/helm/agones --namespace agones-system --name agones --set agones.image.tag=0.10.0-a1bf145

markmandel · 2019-04-16T02:35:11Z

@Kuqd @jkowalski any blockers from approving and merging this? I know it's slowing down @pooneh-m not having this merged?

Just updated it to master (and assuming it passes) I'm hoping this is good to go? 😄

agones-bot · 2019-04-16T02:58:26Z

Build Succeeded 👏

Build Id: c2e0ebaa-167c-45a0-8deb-285e72592320

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

image: gcr.io/agones-images/agones-controller:0.10.0-c19eb7a
image: gcr.io/agones-images/agones-sdk:0.10.0-c19eb7a
image: gcr.io/agones-images/agones-ping:0.10.0-c19eb7a
Linux C++ SDK (build): agonessdk-0.10.0-c19eb7a-linux-arch_64.tar.gz
SDK Server: agonessdk-server-0.10.0-c19eb7a.zip

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

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

To install this version:

git fetch https://github.com/GoogleCloudPlatform/agones.git pull/682/head:pr_682 && git checkout pr_682
helm install install/helm/agones --namespace agones-system --name agones --set agones.image.tag=0.10.0-c19eb7a

ilkercelikyilmaz · 2019-04-16T04:34:10Z

I am running the load test on this change and so far it is looking good. I don't see any reason not to merge this change.
P.S. I am working on few more improvements on top of this change so it will be great to have this change merged.

agones-bot · 2019-04-16T15:48:11Z

Build Failed 😱

Build Id: a6209f41-319d-4940-9446-6048d5e3195f

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

This moves the implementation of GameServerAllocation (GSA) to a [Kubernetes API Extension](https://kubernetes.io/docs/concepts/extend-kubernetes/api-extension/apiserver-aggregation/) instead of using CRDs. This was essentially done for performance reasons, but to break it down: 1. GSA is now create only. Since we had no need for GSA storage, and don't want the performance hit. 1. This removes the mutation and validation webhooks, which have 30s timeout and are run in serial 1. API Server still does cut off a response after 60s, but the api can continue processing (60s gives us enough time, I think for a SDK.Ack() on Allocate, which I don't think we had before) 1. Validation now happens in the request. 1. We can now do batching of requests for higher throughput (googleforgames#536), since we control the entire http request. The breaking changes are: 1. GameServerAllocation's group is now `allocation.agones.dev` rather than `stable.agones.dev`, because a CRD group can't overlap with a api server. 1. Since there is only the `create` verb for GSA, there is no get/list/watch options for GameServerAllocations - so no informers/listers either. But this could be added at a later date, if needed.

agones-bot · 2019-04-16T16:25:48Z

Build Succeeded 👏

Build Id: 2a0f9442-f5ac-4f6a-82f8-3f0e29282992

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

image: gcr.io/agones-images/agones-controller:0.10.0-cbc9cc3
image: gcr.io/agones-images/agones-sdk:0.10.0-cbc9cc3
image: gcr.io/agones-images/agones-ping:0.10.0-cbc9cc3
Linux C++ SDK (build): agonessdk-0.10.0-cbc9cc3-linux-arch_64.tar.gz
SDK Server: agonessdk-server-0.10.0-cbc9cc3.zip

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

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

To install this version:

git fetch https://github.com/GoogleCloudPlatform/agones.git pull/682/head:pr_682 && git checkout pr_682
helm install install/helm/agones --namespace agones-system --name agones --set agones.image.tag=0.10.0-cbc9cc3

markmandel commented Mar 31, 2019

View reviewed changes

markmandel force-pushed the feature/gsa-apiserver-2 branch from ebabf1c to aaa6e91 Compare March 31, 2019 22:57

markmandel added kind/feature New features for Agones kind/breaking Breaking change area/performance Anything to do with Agones being slow, or making it go faster. labels Mar 31, 2019

ilkercelikyilmaz reviewed Mar 31, 2019

View reviewed changes

cmd/controller/main.go Show resolved Hide resolved

ilkercelikyilmaz approved these changes Mar 31, 2019

View reviewed changes

jkowalski reviewed Apr 1, 2019

View reviewed changes

pkg/client/clientset/versioned/typed/allocation/v1alpha1/fake/fake_gameserverallocation.go Outdated Show resolved Hide resolved

jkowalski reviewed Apr 1, 2019

View reviewed changes

markmandel force-pushed the feature/gsa-apiserver-2 branch from aaa6e91 to 2f3567b Compare April 3, 2019 18:12

markmandel force-pushed the feature/gsa-apiserver-2 branch from 2f3567b to 3698601 Compare April 4, 2019 00:52

markmandel marked this pull request as ready for review April 5, 2019 13:36

markmandel mentioned this pull request Apr 13, 2019

Proposal: Split up the api group stable.agones.dev #703

Closed

markmandel force-pushed the feature/gsa-apiserver-2 branch from c11accf to a1bf145 Compare April 13, 2019 06:47

markmandel commented Apr 13, 2019

View reviewed changes

This was referenced Apr 15, 2019

gen-api-docs make target is not generating API docs for GameServerAllocation #705

Closed

Add explicit local version of agones in go.mod #706

Merged

pooneh-m approved these changes Apr 15, 2019

View reviewed changes

markmandel force-pushed the feature/gsa-apiserver-2 branch from a1bf145 to c19eb7a Compare April 16, 2019 02:32

markmandel force-pushed the feature/gsa-apiserver-2 branch from c19eb7a to b97522d Compare April 16, 2019 15:34

jkowalski approved these changes Apr 16, 2019

View reviewed changes

markmandel force-pushed the feature/gsa-apiserver-2 branch from b97522d to cbc9cc3 Compare April 16, 2019 16:02

markmandel merged commit 6d6524a into googleforgames:master Apr 16, 2019

markmandel deleted the feature/gsa-apiserver-2 branch April 16, 2019 16:27

markmandel added this to the 0.10.0 milestone Apr 27, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement GameServerAlocation as API Extension #682

Implement GameServerAlocation as API Extension #682

markmandel commented Mar 31, 2019 •

edited

Loading

markmandel Mar 31, 2019

ilkercelikyilmaz Mar 31, 2019

agones-bot commented Mar 31, 2019

ilkercelikyilmaz left a comment

jkowalski Apr 1, 2019

markmandel Apr 1, 2019

markmandel Apr 16, 2019

jkowalski Apr 1, 2019

markmandel Apr 1, 2019

markmandel Apr 13, 2019

agones-bot commented Apr 3, 2019

agones-bot commented Apr 4, 2019

markmandel commented Apr 5, 2019

agones-bot commented Apr 5, 2019

markmandel Apr 13, 2019

aLekSer Apr 15, 2019 •

edited

Loading

aLekSer Apr 15, 2019 •

edited

Loading

agones-bot commented Apr 13, 2019

markmandel commented Apr 16, 2019

agones-bot commented Apr 16, 2019

ilkercelikyilmaz commented Apr 16, 2019

agones-bot commented Apr 16, 2019

agones-bot commented Apr 16, 2019


		apiVersion: "stable.agones.dev/v1alpha1"
		apiVersion: "allocation.agones.dev/v1alpha1"

Implement GameServerAlocation as API Extension #682

Implement GameServerAlocation as API Extension #682

Conversation

markmandel commented Mar 31, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

agones-bot commented Mar 31, 2019

ilkercelikyilmaz 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

agones-bot commented Apr 3, 2019

agones-bot commented Apr 4, 2019

markmandel commented Apr 5, 2019

agones-bot commented Apr 5, 2019

Choose a reason for hiding this comment

aLekSer Apr 15, 2019 • edited Loading

Choose a reason for hiding this comment

aLekSer Apr 15, 2019 • edited Loading

Choose a reason for hiding this comment

agones-bot commented Apr 13, 2019

markmandel commented Apr 16, 2019

agones-bot commented Apr 16, 2019

ilkercelikyilmaz commented Apr 16, 2019

agones-bot commented Apr 16, 2019

agones-bot commented Apr 16, 2019

markmandel commented Mar 31, 2019 •

edited

Loading

aLekSer Apr 15, 2019 •

edited

Loading

aLekSer Apr 15, 2019 •

edited

Loading