-
Notifications
You must be signed in to change notification settings - Fork 813
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
More description on fleetautoscaler.md #3632
More description on fleetautoscaler.md #3632
Conversation
Updated fleetautoscaler.md with more general descriptions of each type of autoscaling strategy. Since we have 4 now, it seemed like it would be useful to provide some use cases around each type of autoscaling and why you would choose one over another. Work on googleforgames#2716
Build Succeeded 👏 Build Id: 8e0c721f-eb83-4020-810b-96118183e2bd 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:
|
``` | ||
|
||
## Spec Field Reference | ||
|
||
Since Agones defines a new |
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 didn't change this part, but this sentence reads strangely to me. It sounds like we are suggesting that other people add a CRD for autoscaling. I think what's its trying to say is that we created a CRD to do this. With the prevalence of CRDs (in k8s and agones), I don't think this is necessary. Can we say something more direct like
The CRD used for fleet autoscaling has the following fields:
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 - agreed 100%. (We have it in a few other places, but can clean that up later).
I've delete it and cleaned up the sentence before the spec
reference.
minCapacity: 10 | ||
# MaxCapacity is the maximum aggregate List total capacity across the fleet. | ||
# MaxCapacity must be greater than or equal to both MinCapacity and BufferSize. Required field. | ||
maxCapacity: 100 | ||
``` | ||
|
||
## Webhook Autoscaling |
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.
This section should link to the existing Webhook Endpoint Specification section with a note about how to implement the custom webhook (since once you point at your own service you need to understand what data gets passed back and forth).
And the below section can have some of the background information removed (or simplified) since it's now captured here.
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.
Done! Incoming!
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
Build Succeeded 👏 Build Id: 98c06fc4-76fa-44ad-9637-bc15e2033708 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 documentation
What this PR does / Why we need it:
Updated fleetautoscaler.md with more general descriptions of each type of autoscaling strategy.
Since we have 4 now, it seemed like it would be useful to provide some use cases around each type of autoscaling and why you would choose one over another.
Which issue(s) this PR fixes:
Work on #2716
Special notes for your reviewer:
Pulling out some of the docs work I've been doing on Counters and Lists. More to come as I feel it's in a happy place.