Fix wrong condition check for Memory limit #1418

aLekSer · 2020-03-17T15:09:52Z

This is needed to start writing validation for these new limits as well as CPU ones.

Closes #1419.

agones-bot · 2020-03-17T15:28:17Z

Build Succeeded 👏

Build Id: 4804a0e6-cb18-401a-8ef3-80267e895773

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-5999c5c
image: gcr.io/agones-images/agones-sdk:1.5.0-5999c5c
image: gcr.io/agones-images/agones-ping:1.5.0-5999c5c
Linux C++ SDK (build): agonessdk-1.5.0-5999c5c-linux-arch_64.tar.gz
SDK Server: agonessdk-server-1.5.0-5999c5c.zip

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

https://5999c5c-dot-preview-dot-agones-images.appspot.com/

To install this version:

git fetch https://github.com/GoogleCloudPlatform/agones.git pull/1418/head:pr_1418 && git checkout pr_1418
helm install ./install/helm/agones --namespace agones-system --name agones --set agones.image.tag=1.5.0-5999c5c

agones-bot · 2020-03-17T15:31:24Z

Build Succeeded 👏

Build Id: 1537306f-4af7-4a32-b821-bd7594f1e647

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-7791336
image: gcr.io/agones-images/agones-sdk:1.5.0-7791336
image: gcr.io/agones-images/agones-ping:1.5.0-7791336
Linux C++ SDK (build): agonessdk-1.5.0-7791336-linux-arch_64.tar.gz
SDK Server: agonessdk-server-1.5.0-7791336.zip

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

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

To install this version:

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

pkg/gameservers/controller.go

This is needed to start writing validation

aLekSer · 2020-03-20T11:56:15Z

pkg/apis/agones/v1/gameserver.go

+// ValidateResource validates limit or Memory CPU resources used for containers in pods
+// If a GameServer is invalid there will be > 0 values in
+// the returned array
+func ValidateResource(request resource.Quantity, limit resource.Quantity, resourceName corev1.ResourceName) []error {


I have placed ValidateResource() here so we will check PodTemplate in a Fleet and GameServer using exact same function.

Which is part of #1298

agones-bot · 2020-03-20T11:56:29Z

Build Failed 😱

Build Id: 111003dc-428c-4116-bf36-d5f43c2ada5b

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

cmd/controller/main_test.go

agones-bot · 2020-03-20T12:24:02Z

Build Failed 😱

Build Id: 07cf6596-f93f-44a4-90a9-ce0f8de44c58

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

aLekSer · 2020-03-20T12:36:04Z

Flaky #1376 .

agones-bot · 2020-03-20T12:56:13Z

Build Failed 😱

Build Id: c2508e45-a672-477f-af41-96d77f021c72

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

Add test function for Agones Controller validate function. Function ValidateResource() would be reused for Fleet PodTemplate Resources validation.

agones-bot · 2020-03-20T13:39:53Z

Build Succeeded 👏

Build Id: c8113db5-2554-4a86-83b7-f2fb1eab111c

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-961a2c4
image: gcr.io/agones-images/agones-sdk:1.5.0-961a2c4
image: gcr.io/agones-images/agones-ping:1.5.0-961a2c4
Linux C++ SDK (build): agonessdk-1.5.0-961a2c4-linux-arch_64.tar.gz
SDK Server: agonessdk-server-1.5.0-961a2c4.zip

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

https://961a2c4-dot-preview-dot-agones-images.appspot.com/

To install this version:

git fetch https://github.com/GoogleCloudPlatform/agones.git pull/1418/head:pr_1418 && git checkout pr_1418
helm install ./install/helm/agones --namespace agones-system --name agones --set agones.image.tag=1.5.0-961a2c4

markmandel · 2020-03-23T22:16:17Z

cmd/controller/main.go

 	if c.MinPort <= 0 || c.MaxPort <= 0 {
-		return errors.New("min Port and Max Port values are required")
+		validationErrors = append(validationErrors, errors.New("min Port and Max Port values are required"))


Love this. 👍

google-oss-robot · 2020-03-24T10:56:19Z

New changes are detected. LGTM label has been removed.

akremsa · 2020-03-24T11:11:13Z

cmd/controller/main_test.go

+	found := false
+	for _, v := range errs {
+		if expected == v.Error() {
+			found = true


We can safely break the loop here because there is no point to check other elements of the slice

Added break

agones-bot · 2020-03-24T11:14:28Z

Build Succeeded 👏

Build Id: f5047ddc-e182-4430-91be-e06b0f5c4258

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-da3a56d
image: gcr.io/agones-images/agones-sdk:1.5.0-da3a56d
image: gcr.io/agones-images/agones-ping:1.5.0-da3a56d
Linux C++ SDK (build): agonessdk-1.5.0-da3a56d-linux-arch_64.tar.gz
SDK Server: agonessdk-server-1.5.0-da3a56d.zip

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

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

To install this version:

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

google-oss-robot · 2020-03-24T11:20:47Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: akremsa, aLekSer, 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:

~~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-24T11:20:50Z

@akremsa: changing LGTM is restricted to collaborators

In response to this:

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

agones-bot · 2020-03-24T11:36:44Z

Build Succeeded 👏

Build Id: e64dd6d2-ad74-483f-9e67-7454387f1ef3

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-e99cd82
image: gcr.io/agones-images/agones-sdk:1.5.0-e99cd82
image: gcr.io/agones-images/agones-ping:1.5.0-e99cd82
Linux C++ SDK (build): agonessdk-1.5.0-e99cd82-linux-arch_64.tar.gz
SDK Server: agonessdk-server-1.5.0-e99cd82.zip

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

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

To install this version:

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

* Fix wrong condition check for Memory limit * Add a function to validate Resource config Add test function for Agones Controller validate function. Function ValidateResource() would be reused for Fleet, GSS and GS PodTemplate CPU and Memory Resources validation.

google-oss-robot requested review from EricFortin and markmandel March 17, 2020 15:09

google-oss-robot added the size/XS label Mar 17, 2020

markmandel reviewed Mar 17, 2020

View reviewed changes

pkg/gameservers/controller.go Show resolved Hide resolved

Fix wrong condition check for Memory limit

b363dce

This is needed to start writing validation

aLekSer force-pushed the fix/memory-limit branch from 7791336 to 3eab7d5 Compare March 20, 2020 11:49

google-oss-robot added size/L and removed size/XS labels Mar 20, 2020

aLekSer commented Mar 20, 2020

View reviewed changes

aLekSer force-pushed the fix/memory-limit branch from 3eab7d5 to e88c997 Compare March 20, 2020 12:08

akremsa reviewed Mar 20, 2020

View reviewed changes

cmd/controller/main_test.go Outdated Show resolved Hide resolved

aLekSer force-pushed the fix/memory-limit branch from e88c997 to 15d07f7 Compare March 20, 2020 12:40

Add a function to validate Resource config

961a2c4

Add test function for Agones Controller validate function. Function ValidateResource() would be reused for Fleet PodTemplate Resources validation.

aLekSer force-pushed the fix/memory-limit branch from 15d07f7 to 961a2c4 Compare March 20, 2020 13:23

aLekSer mentioned this pull request Mar 20, 2020

Add validation for CPU and Memory Resources for GameServers, Fleets and GameServerSets #1423

Merged

markmandel approved these changes Mar 23, 2020

View reviewed changes

google-oss-robot assigned markmandel Mar 23, 2020

google-oss-robot added lgtm approved labels Mar 23, 2020

markmandel added area/user-experience Pertaining to developers trying to use Agones, e.g. SDK, installation, etc kind/feature New features for Agones labels Mar 23, 2020

markmandel added this to the 1.5.0 milestone Mar 23, 2020

Merge branch 'master' into fix/memory-limit

da3a56d

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

akremsa reviewed Mar 24, 2020

View reviewed changes

Applied comment - add break into test

e99cd82

akremsa approved these changes Mar 24, 2020

View reviewed changes

aLekSer merged commit 5a04207 into googleforgames:master Mar 24, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix wrong condition check for Memory limit #1418

Fix wrong condition check for Memory limit #1418

aLekSer commented Mar 17, 2020 •

edited

Loading

agones-bot commented Mar 17, 2020

agones-bot commented Mar 17, 2020

aLekSer Mar 20, 2020 •

edited

Loading

agones-bot commented Mar 20, 2020

agones-bot commented Mar 20, 2020

aLekSer commented Mar 20, 2020 •

edited

Loading

agones-bot commented Mar 20, 2020

agones-bot commented Mar 20, 2020

markmandel Mar 23, 2020

google-oss-robot commented Mar 24, 2020

akremsa Mar 24, 2020

aLekSer Mar 24, 2020

agones-bot commented Mar 24, 2020

google-oss-robot commented Mar 24, 2020

google-oss-robot commented Mar 24, 2020

agones-bot commented Mar 24, 2020

Fix wrong condition check for Memory limit #1418

Fix wrong condition check for Memory limit #1418

Conversation

aLekSer commented Mar 17, 2020 • edited Loading

agones-bot commented Mar 17, 2020

agones-bot commented Mar 17, 2020

aLekSer Mar 20, 2020 • edited Loading

Choose a reason for hiding this comment

agones-bot commented Mar 20, 2020

agones-bot commented Mar 20, 2020

aLekSer commented Mar 20, 2020 • edited Loading

agones-bot commented Mar 20, 2020

agones-bot commented Mar 20, 2020

markmandel Mar 23, 2020

Choose a reason for hiding this comment

google-oss-robot commented Mar 24, 2020

akremsa Mar 24, 2020

Choose a reason for hiding this comment

aLekSer Mar 24, 2020

Choose a reason for hiding this comment

agones-bot commented Mar 24, 2020

google-oss-robot commented Mar 24, 2020

google-oss-robot commented Mar 24, 2020

agones-bot commented Mar 24, 2020

aLekSer commented Mar 17, 2020 •

edited

Loading

aLekSer Mar 20, 2020 •

edited

Loading

aLekSer commented Mar 20, 2020 •

edited

Loading