-
Notifications
You must be signed in to change notification settings - Fork 825
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
Allocated GameServers updated on Fleet update #3101
Allocated GameServers updated on Fleet update #3101
Conversation
cc58c14
to
37bb8b2
Compare
Build Failed 😱 Build Id: 8ef68b27-c5e5-45cd-b863-4736f0ecf596 To get permission to view the Cloud Build view, join the agones-discuss Google Group. |
Build Failed 😱 Build Id: 1e12f288-8a7e-41f2-bf6d-f7099eceb1c6 To get permission to view the Cloud Build view, join the agones-discuss Google Group. |
71ffd15
to
f429ea5
Compare
Build Failed 😱 Build Id: b5d86899-2e39-4122-9211-f26b4632a0b8 To get permission to view the Cloud Build view, join the agones-discuss Google Group. |
huh. |
Build Failed 😱 Build Id: 955130e5-56c2-4a51-ae61-f11f4db5d89c To get permission to view the Cloud Build view, join the agones-discuss Google Group. |
Build Succeeded 👏 Build Id: 8873d8ff-d184-40ba-b37c-b7a9ebe31254 The following development artifacts have been built, and will exist for the next 30 days:
A preview of the website (the last 30 builds are retained): To install this version:
|
examples/fleet.yaml
Outdated
@@ -50,6 +50,17 @@ spec: | |||
maxSurge: 25% | |||
# the amount to decrements GameServers by. Defaults to 25% | |||
maxUnavailable: 25% | |||
# [Stage:Alpha] | |||
# [FeatureFlag:FleetAllocationOverflow] | |||
# Labels and/or Annotations to apply to GameServers when the number of Allocated GameServers drops below |
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.
Should this be "Labels and/or Annotations to apply to GameServers when the number of the desired replicas on the underlying GameServerSet
drops below the number of Allocated GameServers"?
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.
You are right - I've got it backwards.
Build Succeeded 👏 Build Id: 94a0e3ee-cef5-481f-8ec3-92ea7877bee2 The following development artifacts have been built, and will exist for the next 30 days:
A preview of the website (the last 30 builds are retained): 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
return nil | ||
} | ||
|
||
rest = sortGameServersByStrategy(gsSet.Spec.Scheduling, rest, c.counter.Counts()) |
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.
nit: shall we add some documentation about the sorting logic when picking up GameServers to add the overflow annotations/labels? Either in the design issue or somewhere on the website. I think it's kind of important for customer to be aware of, but right now it's not clear without looking into the code.
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: gongmax, 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:
Approvers can indicate their approval by writing |
New changes are detected. LGTM label has been removed. |
Build Failed 😱 Build Id: e1a57c48-e51e-42fa-957d-ac7f046fc378 To get permission to view the Cloud Build view, join the agones-discuss Google Group. |
Build Failed 😱 Build Id: 9ee52662-420f-4917-8c85-1d8a6c994f95 To get permission to view the Cloud Build view, join the agones-discuss Google Group. |
|
Oops, looks like I missed something. I'll update. |
Functional implementation and testing of applying labels and/or annotations to any `Allocated` `GameServers` that are overflowed past the configured replica count. Next ➡️ write some guides to close out the ticket. Work on googleforgames#2682
7f260ec
to
4760db7
Compare
Build Succeeded 👏 Build Id: ba0c8f33-cf13-48af-b61d-950fa46f72fc The following development artifacts have been built, and will exist for the next 30 days:
A preview of the website (the last 30 builds are retained): To install this version:
|
What type of PR is this?
/kind feature
What this PR does / Why we need it:
Functional implementation and testing of applying labels and/or annotations to any
Allocated
GameServers
that are overflowed past the configured replica count.Which issue(s) this PR fixes:
Work on #2682
Special notes for your reviewer:
Next ➡️ write some guides to close out the ticket.