Add sidecar memory resources setting (googleforgames#1402)

Add Memory Requests/Limits for Gameserver resource estimation and Pod QoS settings Co-authored-by: Mark Mandel <markmandel@google.com>
ilkercelikyilmaz · Oct 23, 2020 · 68f720b · 68f720b
1 parent 02058e1
commit 68f720b
Show file tree

Hide file tree

Showing 7 changed files with 70 additions and 16 deletions.
diff --git a/cmd/controller/main.go b/cmd/controller/main.go
@@ -59,6 +59,8 @@ const (
 	sidecarImageFlag             = "sidecar-image"
 	sidecarCPURequestFlag        = "sidecar-cpu-request"
 	sidecarCPULimitFlag          = "sidecar-cpu-limit"
+	sidecarMemoryRequestFlag     = "sidecar-memory-request"
+	sidecarMemoryLimitFlag       = "sidecar-memory-limit"
 	sdkServerAccountFlag         = "sdk-service-account"
 	pullSidecarFlag              = "always-pull-sidecar"
 	minPortFlag                  = "min-port"
@@ -196,7 +198,8 @@ func main() {
 
 	gsController := gameservers.NewController(wh, health,
 		ctlConf.MinPort, ctlConf.MaxPort, ctlConf.SidecarImage, ctlConf.AlwaysPullSidecar,
-		ctlConf.SidecarCPURequest, ctlConf.SidecarCPULimit, ctlConf.SdkServiceAccount,
+		ctlConf.SidecarCPURequest, ctlConf.SidecarCPULimit,
+		ctlConf.SidecarMemoryRequest, ctlConf.SidecarMemoryLimit, ctlConf.SdkServiceAccount,
 		kubeClient, kubeInformerFactory, extClient, agonesClient, agonesInformerFactory)
 	gsSetController := gameserversets.NewController(wh, health, gsCounter,
 		kubeClient, extClient, agonesClient, agonesInformerFactory)
@@ -235,6 +238,8 @@ func parseEnvFlags() config {
 	viper.SetDefault(sidecarImageFlag, "gcr.io/agones-images/agones-sdk:"+pkg.Version)
 	viper.SetDefault(sidecarCPURequestFlag, "0")
 	viper.SetDefault(sidecarCPULimitFlag, "0")
+	viper.SetDefault(sidecarMemoryRequestFlag, "0")
+	viper.SetDefault(sidecarMemoryLimitFlag, "0")
 	viper.SetDefault(pullSidecarFlag, false)
 	viper.SetDefault(sdkServerAccountFlag, "agones-sdk")
 	viper.SetDefault(certFileFlag, filepath.Join(base, "certs/server.crt"))
@@ -254,6 +259,8 @@ func parseEnvFlags() config {
 	pflag.String(sidecarImageFlag, viper.GetString(sidecarImageFlag), "Flag to overwrite the GameServer sidecar image that is used. Can also use SIDECAR env variable")
 	pflag.String(sidecarCPULimitFlag, viper.GetString(sidecarCPULimitFlag), "Flag to overwrite the GameServer sidecar container's cpu limit. Can also use SIDECAR_CPU_LIMIT env variable")
 	pflag.String(sidecarCPURequestFlag, viper.GetString(sidecarCPURequestFlag), "Flag to overwrite the GameServer sidecar container's cpu request. Can also use SIDECAR_CPU_REQUEST env variable")
+	pflag.String(sidecarMemoryLimitFlag, viper.GetString(sidecarMemoryLimitFlag), "Flag to overwrite the GameServer sidecar container's memory limit. Can also use SIDECAR_MEMORY_LIMIT env variable")
+	pflag.String(sidecarMemoryRequestFlag, viper.GetString(sidecarMemoryRequestFlag), "Flag to overwrite the GameServer sidecar container's memory request. Can also use SIDECAR_MEMORY_REQUEST env variable")
 	pflag.Bool(pullSidecarFlag, viper.GetBool(pullSidecarFlag), "For development purposes, set the sidecar image to have a ImagePullPolicy of Always. Can also use ALWAYS_PULL_SIDECAR env variable")
 	pflag.String(sdkServerAccountFlag, viper.GetString(sdkServerAccountFlag), "Overwrite what service account default for GameServer Pods. Defaults to Can also use SDK_SERVICE_ACCOUNT")
 	pflag.Int32(minPortFlag, 0, "Required. The minimum port that that a GameServer can be allocated to. Can also use MIN_PORT env variable.")
@@ -278,6 +285,8 @@ func parseEnvFlags() config {
 	runtime.Must(viper.BindEnv(sidecarImageFlag))
 	runtime.Must(viper.BindEnv(sidecarCPULimitFlag))
 	runtime.Must(viper.BindEnv(sidecarCPURequestFlag))
+	runtime.Must(viper.BindEnv(sidecarMemoryLimitFlag))
+	runtime.Must(viper.BindEnv(sidecarMemoryRequestFlag))
 	runtime.Must(viper.BindEnv(pullSidecarFlag))
 	runtime.Must(viper.BindEnv(sdkServerAccountFlag))
 	runtime.Must(viper.BindEnv(minPortFlag))
@@ -300,22 +309,34 @@ func parseEnvFlags() config {
 
 	runtime.Must(runtime.ParseFeaturesFromEnv())
 
-	request, err := resource.ParseQuantity(viper.GetString(sidecarCPURequestFlag))
+	requestCPU, err := resource.ParseQuantity(viper.GetString(sidecarCPURequestFlag))
 	if err != nil {
 		logger.WithError(err).Fatalf("could not parse %s", sidecarCPURequestFlag)
 	}
 
-	limit, err := resource.ParseQuantity(viper.GetString(sidecarCPULimitFlag))
+	limitCPU, err := resource.ParseQuantity(viper.GetString(sidecarCPULimitFlag))
 	if err != nil {
 		logger.WithError(err).Fatalf("could not parse %s", sidecarCPULimitFlag)
 	}
 
+	requestMemory, err := resource.ParseQuantity(viper.GetString(sidecarMemoryRequestFlag))
+	if err != nil {
+		logger.WithError(err).Fatalf("could not parse %s", sidecarMemoryRequestFlag)
+	}
+
+	limitMemory, err := resource.ParseQuantity(viper.GetString(sidecarMemoryLimitFlag))
+	if err != nil {
+		logger.WithError(err).Fatalf("could not parse %s", sidecarMemoryLimitFlag)
+	}
+
 	return config{
 		MinPort:               int32(viper.GetInt64(minPortFlag)),
 		MaxPort:               int32(viper.GetInt64(maxPortFlag)),
 		SidecarImage:          viper.GetString(sidecarImageFlag),
-		SidecarCPURequest:     request,
-		SidecarCPULimit:       limit,
+		SidecarCPURequest:     requestCPU,
+		SidecarCPULimit:       limitCPU,
+		SidecarMemoryRequest:  requestMemory,
+		SidecarMemoryLimit:    limitMemory,
 		SdkServiceAccount:     viper.GetString(sdkServerAccountFlag),
 		AlwaysPullSidecar:     viper.GetBool(pullSidecarFlag),
 		KeyFile:               viper.GetString(keyFileFlag),
@@ -341,6 +362,8 @@ type config struct {
 	SidecarImage          string
 	SidecarCPURequest     resource.Quantity
 	SidecarCPULimit       resource.Quantity
+	SidecarMemoryRequest  resource.Quantity
+	SidecarMemoryLimit    resource.Quantity
 	SdkServiceAccount     string
 	AlwaysPullSidecar     bool
 	PrometheusMetrics     bool

diff --git a/install/helm/agones/templates/controller.yaml b/install/helm/agones/templates/controller.yaml
@@ -84,6 +84,12 @@ spec:
           value: {{ .Values.agones.image.sdk.alwaysPull | quote }}
         - name: SIDECAR_CPU_REQUEST
           value: {{ .Values.agones.image.sdk.cpuRequest | quote }}
+        - name: SIDECAR_CPU_LIMIT
+          value: {{ .Values.agones.image.sdk.cpuLimit | quote }}
+        - name: SIDECAR_MEMORY_REQUEST
+          value: {{ .Values.agones.image.sdk.memoryRequest | quote }}
+        - name: SIDECAR_MEMORY_LIMIT
+          value: {{ .Values.agones.image.sdk.memoryLimit | quote }}
         - name: SDK_SERVICE_ACCOUNT
           value: {{ .Values.agones.serviceaccount.sdk | quote }}
         - name: PROMETHEUS_EXPORTER
@@ -94,8 +100,6 @@ spec:
           value: {{ .Values.agones.metrics.stackdriverLabels | quote }}
         - name: GCP_PROJECT_ID
           value: {{ .Values.agones.metrics.stackdriverProjectID | quote }}
-        - name: SIDECAR_CPU_LIMIT
-          value: {{ .Values.agones.image.sdk.cpuLimit | quote }}
         - name: NUM_WORKERS
           value: {{ .Values.agones.controller.numWorkers | quote }}
         - name: API_SERVER_QPS

diff --git a/install/helm/agones/values.yaml b/install/helm/agones/values.yaml
@@ -133,6 +133,8 @@ agones:
       name: agones-sdk
       cpuRequest: 30m
       cpuLimit: 0
+      memoryRequest: 0
+      memoryLimit: 0
       alwaysPull: false
     ping:
       name: agones-ping

diff --git a/install/yaml/install.yaml b/install/yaml/install.yaml
@@ -1453,6 +1453,12 @@ spec:
           value: "false"
         - name: SIDECAR_CPU_REQUEST
           value: "30m"
+        - name: SIDECAR_CPU_LIMIT
+          value: "0"
+        - name: SIDECAR_MEMORY_REQUEST
+          value: "0"
+        - name: SIDECAR_MEMORY_LIMIT
+          value: "0"
         - name: SDK_SERVICE_ACCOUNT
           value: "agones-sdk"
         - name: PROMETHEUS_EXPORTER
@@ -1463,8 +1469,6 @@ spec:
           value: ""
         - name: GCP_PROJECT_ID
           value: ""
-        - name: SIDECAR_CPU_LIMIT
-          value: "0"
         - name: NUM_WORKERS
           value: "100"
         - name: API_SERVER_QPS

diff --git a/pkg/gameservers/controller.go b/pkg/gameservers/controller.go
@@ -67,6 +67,8 @@ type Controller struct {
 	alwaysPullSidecarImage bool
 	sidecarCPURequest      resource.Quantity
 	sidecarCPULimit        resource.Quantity
+	sidecarMemoryRequest   resource.Quantity
+	sidecarMemoryLimit     resource.Quantity
 	sdkServiceAccount      string
 	crdGetter              v1beta1.CustomResourceDefinitionInterface
 	podGetter              typedcorev1.PodsGetter
@@ -97,6 +99,8 @@ func NewController(
 	alwaysPullSidecarImage bool,
 	sidecarCPURequest resource.Quantity,
 	sidecarCPULimit resource.Quantity,
+	sidecarMemoryRequest resource.Quantity,
+	sidecarMemoryLimit resource.Quantity,
 	sdkServiceAccount string,
 	kubeClient kubernetes.Interface,
 	kubeInformerFactory informers.SharedInformerFactory,
@@ -112,6 +116,8 @@ func NewController(
 		sidecarImage:           sidecarImage,
 		sidecarCPULimit:        sidecarCPULimit,
 		sidecarCPURequest:      sidecarCPURequest,
+		sidecarMemoryLimit:     sidecarMemoryLimit,
+		sidecarMemoryRequest:   sidecarMemoryRequest,
 		alwaysPullSidecarImage: alwaysPullSidecarImage,
 		sdkServiceAccount:      sdkServiceAccount,
 		crdGetter:              extClient.ApiextensionsV1beta1().CustomResourceDefinitions(),
@@ -627,13 +633,23 @@ func (c *Controller) sidecar(gs *agonesv1.GameServer) corev1.Container {
 		sidecar.Args = append(sidecar.Args, fmt.Sprintf("--http-port=%d", gs.Spec.SdkServer.HTTPPort))
 	}
 
+	requests := corev1.ResourceList{}
 	if !c.sidecarCPURequest.IsZero() {
-		sidecar.Resources.Requests = corev1.ResourceList{corev1.ResourceCPU: c.sidecarCPURequest}
+		requests[corev1.ResourceCPU] = c.sidecarCPURequest
 	}
+	if !c.sidecarMemoryRequest.IsZero() {
+		requests[corev1.ResourceMemory] = c.sidecarMemoryRequest
+	}
+	sidecar.Resources.Requests = requests
 
+	limits := corev1.ResourceList{}
+	if !c.sidecarCPULimit.IsZero() {
+		limits[corev1.ResourceCPU] = c.sidecarCPULimit
+	}
 	if !c.sidecarCPULimit.IsZero() {
-		sidecar.Resources.Limits = corev1.ResourceList{corev1.ResourceCPU: c.sidecarCPULimit}
+		limits[corev1.ResourceMemory] = c.sidecarMemoryLimit
 	}
+	sidecar.Resources.Limits = limits
 
 	if c.alwaysPullSidecarImage {
 		sidecar.ImagePullPolicy = corev1.PullAlways

diff --git a/pkg/gameservers/controller_test.go b/pkg/gameservers/controller_test.go
@@ -828,6 +828,8 @@ func TestControllerCreateGameServerPod(t *testing.T) {
 			assert.Equal(t, pod.Spec.Containers[1].Image, c.sidecarImage)
 			assert.Equal(t, pod.Spec.Containers[1].Resources.Limits.Cpu(), &c.sidecarCPULimit)
 			assert.Equal(t, pod.Spec.Containers[1].Resources.Requests.Cpu(), &c.sidecarCPURequest)
+			assert.Equal(t, pod.Spec.Containers[1].Resources.Limits.Memory(), &c.sidecarMemoryLimit)
+			assert.Equal(t, pod.Spec.Containers[1].Resources.Requests.Memory(), &c.sidecarMemoryRequest)
 			assert.Len(t, pod.Spec.Containers[1].Env, 3, "3 env vars")
 			assert.Equal(t, "GAMESERVER_NAME", pod.Spec.Containers[1].Env[0].Name)
 			assert.Equal(t, fixture.ObjectMeta.Name, pod.Spec.Containers[1].Env[0].Value)
@@ -1466,7 +1468,8 @@ func newFakeController() (*Controller, agtesting.Mocks) {
 	wh := webhooks.NewWebHook(http.NewServeMux())
 	c := NewController(wh, healthcheck.NewHandler(),
 		10, 20, "sidecar:dev", false,
-		resource.MustParse("0.05"), resource.MustParse("0.1"), "sdk-service-account",
+		resource.MustParse("0.05"), resource.MustParse("0.1"),
+		resource.MustParse("50Mi"), resource.MustParse("100Mi"), "sdk-service-account",
 		m.KubeClient, m.KubeInformerFactory, m.ExtClient, m.AgonesClient, m.AgonesInformerFactory)
 	c.recorder = m.FakeRecorder
 	return c, m

diff --git a/site/content/en/docs/Installation/Install Agones/helm.md b/site/content/en/docs/Installation/Install Agones/helm.md
@@ -119,8 +119,8 @@ 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`                  |
+| `agones.image.sdk.cpuLimit`                         | The [cpu limit][cpu-constraints] for the sdk server container                                   | `0` (none)             |
 | `agones.image.sdk.alwaysPull`                       | Tells if the sdk image should always be pulled                                                  | `false`                |
 | `agones.image.ping.name`                            | Image name for the ping service                                                                 | `agones-ping`          |
 | `agones.image.ping.pullPolicy`                      | Image pull policy for the ping service                                                          | `IfNotPresent`         |
@@ -172,14 +172,16 @@ The following tables lists the configurable parameters of the Agones chart and t
 
 | Parameter                                           | Description                                                                                     | Default                |
 | --------------------------------------------------- | ----------------------------------------------------------------------------------------------- | ---------------------- |
-|                                                     |                                                                                                 |                        |
+| `agones.image.sdk.memoryRequest`                    | The [memory request][memory-constraints] for sdk server container                               | `0` (none)             |
+| `agones.image.sdk.memoryLimit`                      | The [memory limit][memory-constraints] for the sdk server container                             | `0` (none)             |
 
 {{% /feature %}}
 
 [toleration]: https://kubernetes.io/docs/concepts/configuration/taint-and-toleration/
 [nodeSelector]: https://kubernetes.io/docs/concepts/configuration/assign-pod-node/#nodeselector
 [affinity]: https://kubernetes.io/docs/concepts/configuration/assign-pod-node/#affinity-and-anti-affinity
-[constraints]: https://kubernetes.io/docs/tasks/administer-cluster/manage-resources/cpu-constraint-namespace/
+[cpu-constraints]: https://kubernetes.io/docs/tasks/administer-cluster/manage-resources/cpu-constraint-namespace/
+[memory-constraints]: https://kubernetes.io/docs/tasks/administer-cluster/manage-resources/memory-constraint-namespace/
 [ping]: {{< ref "/docs/Guides/ping-service.md" >}}
 [service]: https://kubernetes.io/docs/concepts/services-networking/service/
 [allocator]: {{< ref "/docs/advanced/allocator-service.md" >}}