GameServerSet Implementation #156
Conversation
|
/cc @ianlewis if you also want to take a look. If anyone has a better name than |
|
Build Succeeded 👏 Build Id: 62d85413-adb8-43f4-906c-aed851ee0d28 The following development artifacts have been built, and will exist for the next 30 days:
|
cmd/controller/main.go
Outdated
|
|
||
| logger.Info("Shut down gameserver controller") | ||
| <-stop | ||
| logger.Info("Shut down agones controller") |
pkg/gameserversets/controller.go
Outdated
| c.recorder.Eventf(gss, corev1.EventTypeNormal, "SuccessfulDelete", "Deleted GameServer: %s", gs.ObjectMeta.Name) | ||
| count++ | ||
| } | ||
| } |
There was a problem hiding this comment.
What happens if they are all allocated? Will this re-queue the sync event to be processed later on?
There was a problem hiding this comment.
If they are allocated, the event handler above will watch for when the GameServer is deleted for this GameServerSet, and when it catches that event, it will queue the owning GameServerSet to be processed again, and make sure it's at the right number.
markmandel
force-pushed
the
feature/gameserverset
branch
from
13d6936
to
ef92839
Compare
|
Build Succeeded 👏 Build Id: e9546851-b2a2-4a66-ba44-c9f88497c120 The following development artifacts have been built, and will exist for the next 30 days:
|
cmd/controller/main.go
Outdated
| @@ -128,7 +129,8 @@ func main() { | |||
| agonesInformerFactory := externalversions.NewSharedInformerFactory(agonesClient, 30*time.Second) | |||
| kubeInformationFactory := informers.NewSharedInformerFactory(kubeClient, 30*time.Second) | |||
|
|
|||
| c := gameservers.NewController(wh, health, minPort, maxPort, sidecarImage, alwaysPullSidecar, kubeClient, kubeInformationFactory, extClient, agonesClient, agonesInformerFactory) | |||
| gsController := gameservers.NewController(wh, health, minPort, maxPort, sidecarImage, alwaysPullSidecar, kubeClient, kubeInformationFactory, extClient, agonesClient, agonesInformerFactory) | |||
| gssController := gameserversets.NewController(wh, kubeClient, extClient, agonesClient, agonesInformerFactory) | |||
There was a problem hiding this comment.
@markmandel The one letter difference between variable names is easy to miss. How about gs and gsSet or something that is easier to parse quickly?
There was a problem hiding this comment.
I like gsSet better. I'm actually going to change that across the whole codebase. I've had issues writing this mixing up gs and gss and it took me ages to fix the bugs!
|
Small nitpick on variable names in |
markmandel
force-pushed
the
feature/gameserverset
branch
from
ef92839
to
b831787
Compare
|
Build Succeeded 👏 Build Id: 335365f9-2c98-4792-8ac4-03f4c7d4be98 The following development artifacts have been built, and will exist for the next 30 days:
|
markmandel
force-pushed
the
feature/gameserverset
branch
from
b831787
to
14363ea
Compare
|
Build Succeeded 👏 Build Id: 164ae42d-0501-47f1-90e9-1f1844709886 The following development artifacts have been built, and will exist for the next 30 days:
|
markmandel
force-pushed
the
feature/gameserverset
branch
from
14363ea
to
c77b212
Compare
|
Build Succeeded 👏 Build Id: a376d314-3a0b-431e-a57b-9dfa5d5b9b77 The following development artifacts have been built, and will exist for the next 30 days:
|
markmandel
force-pushed
the
feature/gameserverset
branch
from
c77b212
to
11c1a8c
Compare
|
Build Succeeded 👏 Build Id: f74aca98-186b-4602-b38f-7e72774b6d84 The following development artifacts have been built, and will exist for the next 30 days:
|
markmandel
force-pushed
the
feature/gameserverset
branch
from
11c1a8c
to
f021e4b
Compare
|
Build Failed 😱 Build Id: 6aea76ff-2017-4b2b-aef1-10da960c9837 Build Logs |
markmandel
force-pushed
the
feature/gameserverset
branch
2 times, most recently
from
739c4aa
to
af04795
Compare
|
Build Succeeded 👏 Build Id: 7620f887-fbe8-4bff-abe2-7d54fa9a2109 The following development artifacts have been built, and will exist for the next 30 days:
|
|
Build Succeeded 👏 Build Id: bc6b77a2-e97f-4511-8bfb-99e530446c6a The following development artifacts have been built, and will exist for the next 30 days:
|
|
Blocking #149 (Helm) before merging this - so as to not keep moving the goalposts on what the install.yaml should be. |
markmandel
force-pushed
the
feature/gameserverset
branch
from
af04795
to
f270450
Compare
|
Build Succeeded 👏 Build Id: ec7c14eb-2ae9-4b8e-87e5-f9e8be933f2b The following development artifacts have been built, and will exist for the next 30 days:
|
markmandel
force-pushed
the
feature/gameserverset
branch
from
f270450
to
cdfab72
Compare
|
Build Succeeded 👏 Build Id: 96b1ff35-cb75-4279-b938-917ba0d02ec2 The following development artifacts have been built, and will exist for the next 30 days:
|
markmandel
force-pushed
the
feature/gameserverset
branch
from
cdfab72
to
dca7a69
Compare
|
|
||
| # validation for a gameserver spec | ||
|
|
||
| {{- define "gameserver.validation" }} |
There was a problem hiding this comment.
@Kuqd @fooock - see any issues with me shifting this validation into a template like this?
Seems like the correct way to do it, but wanted to double check in case you could see anything obviously bad?
There was a problem hiding this comment.
For me it's ok. Small things:
- I would add a small documentation above the
define, line{{ /* Validation for a gameserver spec .... */ }} - Why
{{ end - }}instead of{{ - end }}?? - Normally these templates are inside the
_helpers.tplfile, but by organization, I would leave it like that.
There was a problem hiding this comment.
Good call on the first two. Done!.
Re:
Normally these templates are inside the _helpers.tpl file, but by organization, I would leave it like that.
So I looked at https://github.com/kubernetes/helm/blob/master/docs/chart_best_practices/templates.md
Template files should have the extension .yaml if they produce YAML output. The extension .tpl may be used for template files that produce no formatted content.
So I made it a .yaml file with an underscore, since it's a pretty big yaml block. WDYT?
|
Build Succeeded 👏 Build Id: 29386d56-8281-404a-bc38-235a59c3bb12 The following development artifacts have been built, and will exist for the next 30 days:
|
GameServerSets are the basic building block for Fleets. GameServerSets will be allow Fleet migrations to occur, similarly to how ReplicaSets allow Deployments to migrate one image type to another. This has not been formally documented, as this will likely be an internal CRD, and not (widely) used externally. Parent ticket: googleforgames#70
markmandel
force-pushed
the
feature/gameserverset
branch
from
dca7a69
to
08d9dd5
Compare
|
Build Succeeded 👏 Build Id: e745febb-7e4b-434f-b087-23191b1929cc The following development artifacts have been built, and will exist for the next 30 days:
|
GameServerSets are the basic building block for Fleets. They allow for creating a set of GameServers that can be scaled up and down in size, while also being aware of
AllocatedGameServers and ensuring they don't get deleted.GameServerSets will be allow Fleet migrations to occur, similarly to how ReplicaSets allow Deployments to migrate one image type to another.
This has not been formally documented, as this will likely be an internal CRD, and not (widely) used externally.
Parent ticket: #70