Add sidecar memory resources setting #1402

suecideTech · 2020-03-09T16:26:30Z

Add memory Requests/Limits for Gameserver resource estimation and Pod QoS class configuration.

About QoS

Details of Pod QoS is as below.
https://kubernetes.io/docs/tasks/configure-pod-container/quality-service-pod/

By setting the QoS to "guaranteed", if the node does not have enough resources, it is kill from pods other than "guaranteed".

Also, This is followed to be "Guaranteed" in the sample-udp example.
#487 (comment)

But, QoS is not assigned to "Guaranteed" unless CPU and Memory are the same in Limit and Request for each container.

When not set sidecar Requests/Limits.

$ kubectl describe po simple-udp-5xdrf
:
Containers:
  simple-udp:
    Image:          gcr.io/agones-images/udp-server:0.18
:
    Limits:
      cpu:     20m
      memory:  32Mi
    Requests:
      cpu:     20m
      memory:  32Mi
:
  agones-gameserver-sidecar:
    Image:         gcr.io/agones-images/agones-sdk:1.5.0-dcb266e
:
    Requests:
      cpu:     30m
:
QoS Class:       Burstable

When sidecar setting is "Guaranteed".

$ kubectl describe po simple-udp-k9lsh
:
Containers:
  simple-udp:
    Image:          gcr.io/agones-images/udp-server:0.18
:
    Limits:
      cpu:     20m
      memory:  32Mi
    Requests:
      cpu:     20m
      memory:  32Mi
:
  agones-gameserver-sidecar:
    Image:         gcr.io/agones-images/agones-sdk:1.5.0-dcb266e
:
    Limits:
      cpu:     100m
      memory:  100Mi
    Requests:
      cpu:     100m
      memory:  100Mi
:
QoS Class:       Guaranteed

Add Memory Requests/Limits for Gameserver resource estimation and Pod QoS settings

suecideTech · 2020-03-09T16:27:33Z

/assign @cyriltovena

agones-bot · 2020-03-09T16:40:42Z

Build Failed 😱

Build Id: b70c58d8-ecdf-477b-8962-c267886a8c3f

To get permission to view the Cloud Build view, join the agones-discuss Google Group.

agones-bot · 2020-03-09T16:47:01Z

Build Succeeded 👏

Build Id: a67c17c9-b4e9-43b3-ab29-c66776aaf818

The following development artifacts have been built, and will exist for the next 30 days:

image: gcr.io/agones-images/agones-controller:1.5.0-f554f7a
image: gcr.io/agones-images/agones-sdk:1.5.0-f554f7a
image: gcr.io/agones-images/agones-ping:1.5.0-f554f7a
Linux C++ SDK (build): agonessdk-1.5.0-f554f7a-linux-arch_64.tar.gz
SDK Server: agonessdk-server-1.5.0-f554f7a.zip

A preview of the website (the last 30 builds are retained):

https://f554f7a-dot-preview-dot-agones-images.appspot.com/

To install this version:

git fetch https://github.com/GoogleCloudPlatform/agones.git pull/1402/head:pr_1402 && git checkout pr_1402
helm install ./install/helm/agones --namespace agones-system --name agones --set agones.image.tag=1.5.0-f554f7a

roberthbailey · 2020-03-10T16:01:12Z

@suecideTech - can you also update the example yaml files so that the new api fields are documented for end users (who don't want to read the api spec / code)?

They live in two places: https://github.com/googleforgames/agones/blob/master/examples/gameserver.yaml (for developers looking at github) and https://github.com/googleforgames/agones/blob/master/site/content/en/docs/Reference/gameserver.md (for users looking at the website). For the latter, you will need to gate the changes based on the released agones version so that they don't appear immediately as described at https://agones.dev/site/docs/contribute/#within-a-page.

suecideTech · 2020-03-10T16:31:28Z

@roberthbailey
There is no content describing this change in gameserver.yaml.
This PR change specifies the sidecar config included in the GameServer.
Therefore, there is no change in GameServer API.

Instead, this item is added to the configuration of helm Install.
3a1396a#diff-1eda647ccb61b12e7d45fc6883dce41c

agones-bot · 2020-03-10T16:41:38Z

Build Succeeded 👏

Build Id: 233d536f-1578-4696-a88e-f6344ee338a8

The following development artifacts have been built, and will exist for the next 30 days:

image: gcr.io/agones-images/agones-controller:1.5.0-6981b14
image: gcr.io/agones-images/agones-sdk:1.5.0-6981b14
image: gcr.io/agones-images/agones-ping:1.5.0-6981b14
Linux C++ SDK (build): agonessdk-1.5.0-6981b14-linux-arch_64.tar.gz
SDK Server: agonessdk-server-1.5.0-6981b14.zip

A preview of the website (the last 30 builds are retained):

https://6981b14-dot-preview-dot-agones-images.appspot.com/

To install this version:

git fetch https://github.com/GoogleCloudPlatform/agones.git pull/1402/head:pr_1402 && git checkout pr_1402
helm install ./install/helm/agones --namespace agones-system --name agones --set agones.image.tag=1.5.0-6981b14

roberthbailey · 2020-03-10T18:13:55Z

Sorry, in my quick skim of the PR I thought you were adding api fields to set this on a per-sidecar basis, but upon a second look it's new flags to the controller that set it on a system-wide basis.

steven-supersolid

I can only really comment on nodejs changes

steven-supersolid · 2020-03-12T14:13:43Z

sdks/nodejs/package-lock.json

@@ -1,6 +1,6 @@
 {
  "name": "@google-cloud/agones-sdk",
-  "version": "1.3.0",


I don't think this file should be part of the review. Please can it be reverted although we should also look at why it is out of date

Thank you for checking!
It was a mistake...
revert and committed.

Revert files committed by mistake

agones-bot · 2020-03-12T15:31:42Z

Build Succeeded 👏

Build Id: 89c2e058-1f14-4560-9d36-1d28e1e4e8ae

The following development artifacts have been built, and will exist for the next 30 days:

image: gcr.io/agones-images/agones-controller:1.5.0-4888c02
image: gcr.io/agones-images/agones-sdk:1.5.0-4888c02
image: gcr.io/agones-images/agones-ping:1.5.0-4888c02
Linux C++ SDK (build): agonessdk-1.5.0-4888c02-linux-arch_64.tar.gz
SDK Server: agonessdk-server-1.5.0-4888c02.zip

A preview of the website (the last 30 builds are retained):

https://4888c02-dot-preview-dot-agones-images.appspot.com/

To install this version:

git fetch https://github.com/GoogleCloudPlatform/agones.git pull/1402/head:pr_1402 && git checkout pr_1402
helm install ./install/helm/agones --namespace agones-system --name agones --set agones.image.tag=1.5.0-4888c02

markmandel · 2020-03-13T22:40:21Z

site/content/en/docs/Installation/Install Agones/helm.md

@@ -119,8 +119,10 @@ The following tables lists the configurable parameters of the Agones chart and t
 | `agones.image.controller.pullPolicy`                | Image pull policy for the controller                                                            | `IfNotPresent`         |
 | `agones.image.controller.pullSecret`                | Image pull secret for the controller, allocator, sdk and ping image. Should be created both in `agones-system` and `default` namespaces | ``                     |
 | `agones.image.sdk.name`                             | Image name for the sdk                                                                          | `agones-sdk`           |
-| `agones.image.sdk.cpuRequest`                       | The [cpu request][constraints] for sdk server container                                         | `30m`                  |
-| `agones.image.sdk.cpuLimit`                         | The [cpu limit][constraints] for the sdk server container                                       | `0` (none)             |
+| `agones.image.sdk.cpuRequest`                       | The [cpu request][cpu-constraints] for sdk server container                                     | `30m`                  |


Only change I can see - this documentation should be in the "New Configuration Features:" section, since it's then hidden until the next release.

Other than that, this is a great new addition!

Thank you for checking!
I moved added config to "New configuration function".

agones-bot · 2020-03-14T16:59:29Z

Build Succeeded 👏

Build Id: 0f6e34d7-cfb6-4bde-a996-73d9215a9edf

The following development artifacts have been built, and will exist for the next 30 days:

image: gcr.io/agones-images/agones-controller:1.5.0-606f821
image: gcr.io/agones-images/agones-sdk:1.5.0-606f821
image: gcr.io/agones-images/agones-ping:1.5.0-606f821
Linux C++ SDK (build): agonessdk-1.5.0-606f821-linux-arch_64.tar.gz
SDK Server: agonessdk-server-1.5.0-606f821.zip

A preview of the website (the last 30 builds are retained):

https://606f821-dot-preview-dot-agones-images.appspot.com/

To install this version:

git fetch https://github.com/GoogleCloudPlatform/agones.git pull/1402/head:pr_1402 && git checkout pr_1402
helm install ./install/helm/agones --namespace agones-system --name agones --set agones.image.tag=1.5.0-606f821

aLekSer · 2020-03-16T13:15:13Z

Great to see these parameters.

What I see additionally to this PR is to validate these settings. Because Pod could not be created if cpuLimits < cpuRequests. I will add such validation for gameserver limits and requests.
Additional context could be found here:
#1298

roberthbailey · 2020-03-16T17:48:21Z

I don't have any additional feedback on top of what @markmandel and @aLekSer have said so I'll defer to them for final review / approval.

aLekSer

I think additional validation of limits and request proportions could be done in a separate PR. I will take a look on it.
Waiting for resolving one comment above about the docs.

markmandel

LGTM

google-oss-robot · 2020-03-16T20:46:58Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: aLekSer, markmandel, suecideTech

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:

~~OWNERS~~ [markmandel]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

google-oss-robot · 2020-03-16T20:47:36Z

New changes are detected. LGTM label has been removed.

agones-bot · 2020-03-16T21:05:22Z

Build Succeeded 👏

Build Id: 6c64f874-d2e5-4835-ae23-5f8ec87b3664

The following development artifacts have been built, and will exist for the next 30 days:

image: gcr.io/agones-images/agones-controller:1.5.0-a81195b
image: gcr.io/agones-images/agones-sdk:1.5.0-a81195b
image: gcr.io/agones-images/agones-ping:1.5.0-a81195b
Linux C++ SDK (build): agonessdk-1.5.0-a81195b-linux-arch_64.tar.gz
SDK Server: agonessdk-server-1.5.0-a81195b.zip

A preview of the website (the last 30 builds are retained):

https://a81195b-dot-preview-dot-agones-images.appspot.com/

To install this version:

git fetch https://github.com/GoogleCloudPlatform/agones.git pull/1402/head:pr_1402 && git checkout pr_1402
helm install ./install/helm/agones --namespace agones-system --name agones --set agones.image.tag=1.5.0-a81195b

Add Memory Requests/Limits for Gameserver resource estimation and Pod QoS settings Co-authored-by: Mark Mandel <markmandel@google.com>

Add sidecar memory resources setting

3a1396a

Add Memory Requests/Limits for Gameserver resource estimation and Pod QoS settings

google-oss-robot requested review from EricFortin and steven-supersolid March 9, 2020 16:26

google-oss-robot added the size/M label Mar 9, 2020

Merge branch 'master' into config-sidecar-memory-resources

f554f7a

google-oss-robot assigned cyriltovena Mar 9, 2020

aLekSer mentioned this pull request Mar 9, 2020

Allocated GameServer could be preempted by other Pod with higher PodPriorityClass #1350

Closed

Align indent

6981b14

steven-supersolid suggested changes Mar 12, 2020

View reviewed changes

google-oss-robot assigned steven-supersolid Mar 12, 2020

revert nodejs package-lock.json

4888c02

Revert files committed by mistake

markmandel reviewed Mar 13, 2020

View reviewed changes

Move added config to "New configuration function"

606f821

roberthbailey assigned markmandel, aLekSer and roberthbailey and unassigned cyriltovena and roberthbailey Mar 16, 2020

aLekSer approved these changes Mar 16, 2020

View reviewed changes

google-oss-robot added the lgtm label Mar 16, 2020

markmandel approved these changes Mar 16, 2020

View reviewed changes

google-oss-robot added the approved label Mar 16, 2020

markmandel added area/operations Installation, updating, metrics etc kind/feature New features for Agones labels Mar 16, 2020

markmandel added this to the 1.5.0 milestone Mar 16, 2020

Merge branch 'master' into config-sidecar-memory-resources

a81195b

google-oss-robot removed the lgtm label Mar 16, 2020

markmandel merged commit 951c50b into googleforgames:master Mar 16, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add sidecar memory resources setting #1402

Add sidecar memory resources setting #1402

suecideTech commented Mar 9, 2020

suecideTech commented Mar 9, 2020

agones-bot commented Mar 9, 2020

agones-bot commented Mar 9, 2020

roberthbailey commented Mar 10, 2020

suecideTech commented Mar 10, 2020

agones-bot commented Mar 10, 2020

roberthbailey commented Mar 10, 2020

steven-supersolid left a comment

steven-supersolid Mar 12, 2020

suecideTech Mar 12, 2020

agones-bot commented Mar 12, 2020

markmandel Mar 13, 2020

suecideTech Mar 14, 2020

agones-bot commented Mar 14, 2020

aLekSer commented Mar 16, 2020

roberthbailey commented Mar 16, 2020

aLekSer left a comment •

edited

Loading

markmandel left a comment

google-oss-robot commented Mar 16, 2020

google-oss-robot commented Mar 16, 2020

agones-bot commented Mar 16, 2020

Add sidecar memory resources setting #1402

Add sidecar memory resources setting #1402

Conversation

suecideTech commented Mar 9, 2020

About QoS

suecideTech commented Mar 9, 2020

agones-bot commented Mar 9, 2020

agones-bot commented Mar 9, 2020

roberthbailey commented Mar 10, 2020

suecideTech commented Mar 10, 2020

agones-bot commented Mar 10, 2020

roberthbailey commented Mar 10, 2020

steven-supersolid left a comment

Choose a reason for hiding this comment

steven-supersolid Mar 12, 2020

Choose a reason for hiding this comment

suecideTech Mar 12, 2020

Choose a reason for hiding this comment

agones-bot commented Mar 12, 2020

markmandel Mar 13, 2020

Choose a reason for hiding this comment

suecideTech Mar 14, 2020

Choose a reason for hiding this comment

agones-bot commented Mar 14, 2020

aLekSer commented Mar 16, 2020

roberthbailey commented Mar 16, 2020

aLekSer left a comment • edited Loading

Choose a reason for hiding this comment

markmandel left a comment

Choose a reason for hiding this comment

google-oss-robot commented Mar 16, 2020

google-oss-robot commented Mar 16, 2020

agones-bot commented Mar 16, 2020

aLekSer left a comment •

edited

Loading