-
Notifications
You must be signed in to change notification settings - Fork 801
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
GameServerAllocation implementation #465
GameServerAllocation implementation #465
Conversation
This is a bigger one, with lots of refactoring in it - I'm guessing 😄 it won't make 0.7.0, but wrote it as if it does, but can adjust as needed if need be. |
Build Failed 😱 Build Id: 7ae0d8d4-fbca-48fa-ad87-7cdcb11f5ac4 Build Logs
|
b6f9b4d
to
9aaf235
Compare
Build Failed 😱 Build Id: a0291727-5fab-4770-9b23-063f74b2ed4e Build Logs
|
9aaf235
to
8bf01ad
Compare
Build Succeeded 👏 Build Id: 28e99a13-93ee-454e-b7d4-b81fecacef68 The following development artifacts have been built, and will exist for the next 30 days:
To install this version:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM !
docs/create_fleet.md
Outdated
|
||
We can do allocation of a GameServer for usage through a `GameServerAllocation`, which will both | ||
return to us the details of a `GameServer` (assuming one is available), and also move it to the `Allocated` state, | ||
which demarkates that it |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
demarcates ? Did you finished your sentence ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch. Fixed!
docs/create_fleet.md
Outdated
return to us the details of a `GameServer` (assuming one is available), and also move it to the `Allocated` state, | ||
which demarkates that it | ||
|
||
It is worth noting that there is noting specific that ties a `GameServerAllocation`. A `GameServerAllocation` uses |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is worth noting that there is noting specific that ties a `GameServerAllocation`. A `GameServerAllocation` uses | |
It is worth noting that there is nothing specific that ties a `GameServerAllocation`. A `GameServerAllocation` uses |
examples/gameserverallocation.yaml
Outdated
stable.agones.dev/fleet: green-fleet | ||
- matchLabels: | ||
stable.agones.dev/fleet: blue-fleet | ||
replicas: 2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
replicas: 2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess you don't need replicas here.
|
||
c.logger.WithField("gsa", gsa.ObjectMeta.Name).WithField("patch", string(json)).Infof("patch created!") | ||
|
||
pt := admv1beta1.PatchTypeJSONPatch |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm curious what this does ? Tells the webhook to patch the resource ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So when you return the AdmissionReview.Response
, you can apply a `Patch, which states what changes you want to make to the object that you were originally passed - that's how you make changes via the mutationWebHook.
https://medium.com/ibm-cloud/diving-into-kubernetes-mutatingadmissionwebhook-6ef3c5695f74 has a good explanation.
|
||
// syncDelete takes unallocated GameServerAllocations, and deletes them! | ||
func (c *Controller) syncDelete(key string) error { | ||
c.logger.WithField("key", key).Info("Deleting gameserverallocation") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this means the GameServerAllocation
is added with a status GameServerAllocationUnAllocated
but deleted right after. Asking question mostly for metrics implementation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah - I couldn't think of a better way to handle UnAllocated GameServerAllocations - since there is no GameServer that will eventually get deleted, it need to be returned in the webhook, but eventually deleted, otherwise it would hang around forever.
8bf01ad
to
0236758
Compare
Build Failed 😱 Build Id: b6cc8a28-02f6-42f2-9283-2caee97aec3b Build Logs
|
This implements the `GameServerAllocation` as designed in googleforgames#436, which decouples Allocation from Fleets, and provides much more flexible functionality. This also moves FleetAllocation to deprecated, and includes notes that it will be removed in +2 releases. Closes googleforgames#436
0236758
to
e3ab125
Compare
Build Succeeded 👏 Build Id: 4328cebb-2edb-412b-aa51-715aa009125a The following development artifacts have been built, and will exist for the next 30 days:
To install this version:
|
This implements the
GameServerAllocation
as designed in #436, which decouples Allocation from Fleets, and provides much more flexible functionality.This also moves FleetAllocation to deprecated, and includes notes that it will be removed in +2 releases.
Closes #436