Add allocator resources #1874
Add allocator resources #1874
Conversation
|
Build Succeeded 👏 Build Id: d1e87399-dde6-4d70-bb65-6315c431c91c 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:
|
markmandel
requested review from
pooneh-m
and removed request for
EricFortin and
dzlier-gcp
markmandel
added
area/operations
|
This LGTM! @pooneh-m can you see anything we should care about? (I don't think so, but figured I'd double check) |
| @@ -248,6 +248,7 @@ The following tables lists the configurable parameters of the Agones chart and t | |||
|
|
|||
| | Parameter | Description | Default | | |||
| | --------------------------------------------------- | ----------------------------------------------------------------------------------------------- | ---------------------- | | |||
| | `agones.allocator.resources` | Allocator pods resource requests/limit | `{}` | | |||
There was a problem hiding this comment.
Would it be helpful to add a small section with an example of the value for resources or a reference link?
There was a problem hiding this comment.
Ooh, I like it (we should add it to the other config options. How about this:
| | `agones.allocator.resources` | Allocator pods resource requests/limit | `{}` | | |
| | `agones.allocator.resources` | Allocator pods [resource requests/limit](https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/) | `{}` | |
Which also has explanations and examples of resource limits and requests. Sound good?
There was a problem hiding this comment.
+1 also, I think a simple example would also help with how to set the value. Since it is a compound structure, it is not easy to see how the input should look like.
There was a problem hiding this comment.
Definitely wouldn't hurt - especially as we have a few config options line this. Maybe add an extra example to:
Specify each parameter using the --set key=value[,key=value] argument to helm install. For example,
(Or is this something you would have to use a values.yaml for? Trying to find some helm docs we could even point to)
There was a problem hiding this comment.
As far as I know, resources often uses values.yaml.
I haven't seen many examples of using set to configure it.
What you see most often are commented out settings.
resources: {}
# requests:
# cpu: 1
# memory: 256MiThere was a problem hiding this comment.
I've added instructions on how to set the values.
|
Build Succeeded 👏 Build Id: edbbb4de-dcaf-45f6-bb66-86882fc40ae3 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:
|
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: 8398a7, pooneh-m 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 Succeeded 👏 Build Id: 5c36a806-d356-4397-ba48-f1defa74a19e 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:
|
|
Build Failed 😱 Build Id: 08e8b5e5-1a92-4eed-bc74-7c78eed47ee9 To get permission to view the Cloud Build view, join the agones-discuss Google Group. |
|
Build Succeeded 👏 Build Id: 01c1c624-42ae-45a2-99cb-f6001312bc0d 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:
I've made the allocator resources configurable.
Which issue(s) this PR fixes:
Closes #1873
Special notes for your reviewer: