Add func to wait for fleet condition without Fatalf #1065

aLekSer · 2019-09-24T16:15:51Z

Fix stress test race condition.
Rename function WaitForFleetCondition() -> AssertFleetCondition() as it not only wait but also asserts.
It is forbidden to call t.Fatalf() from go routine other than main test routine.

Closes #1055.

agones-bot · 2019-09-24T16:36:31Z

Build Succeeded 👏

Build Id: 53def889-abed-48c3-8192-49923abb5846

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

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

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

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

To install this version:

git fetch https://github.com/GoogleCloudPlatform/agones.git pull/1065/head:pr_1065 && git checkout pr_1065
helm install install/helm/agones --namespace agones-system --name agones --set agones.image.tag=1.1.0-b5cad0c

agones-bot · 2019-09-25T13:28:38Z

Build Succeeded 👏

Build Id: c6a16e58-623a-4236-907f-f5bd22475a93

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

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

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

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

To install this version:

git fetch https://github.com/GoogleCloudPlatform/agones.git pull/1065/head:pr_1065 && git checkout pr_1065
helm install install/helm/agones --namespace agones-system --name agones --set agones.image.tag=1.1.0-f57c319

agones-bot · 2019-09-25T13:32:18Z

Build Succeeded 👏

Build Id: ace0f078-7a4c-465d-b118-f1cfaef5395e

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

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

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

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

To install this version:

git fetch https://github.com/GoogleCloudPlatform/agones.git pull/1065/head:pr_1065 && git checkout pr_1065
helm install install/helm/agones --namespace agones-system --name agones --set agones.image.tag=1.1.0-e8692b2

markmandel · 2019-09-26T15:55:38Z

test/e2e/framework/framework.go

+	err := f.WaitForFleetConditionParallel(t, flt, condition)
+	if err != nil {
+		// Do not call Fatalf() from go routine other than main test go routine, because it could cause a race
+		t.Fatalf("error waiting for fleet condition on fleet %v", flt.Name)


Now that I'm looking at this, should this function instead return an error if it fails, much like we do for other Wait* commands (example). That would make it simpler no? (and we no longer need to pass through testing.T either.

There is a 73 occurrences of WaitForFleetCondition() so it is handy to enclose this Fatalf() function inside this helper ( if it called from main test routine).

Then I would argue should really be an AssertFleetCondition - rather than a wait. Because it's making an assertion, rather than waiting for a result.

agones-bot · 2019-09-30T12:39:33Z

Build Succeeded 👏

Build Id: 25a4b2f1-a469-48ab-be7c-eae4a66e8343

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

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

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

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

To install this version:

git fetch https://github.com/GoogleCloudPlatform/agones.git pull/1065/head:pr_1065 && git checkout pr_1065
helm install install/helm/agones --namespace agones-system --name agones --set agones.image.tag=1.1.0-e221a98

markmandel

Excellent!

google-oss-robot · 2019-10-01T00:23:54Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: aLekSer, markmandel
To complete the pull request process, please assign
You can assign the PR to them by writing /assign in a comment when ready.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

OWNERS

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 · 2019-10-01T01:20:30Z

New changes are detected. LGTM label has been removed.

agones-bot · 2019-10-01T01:29:06Z

Build Failed 😱

Build Id: d5147298-3f07-4279-8ee8-a2a4ddb4f7bf

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

roberthbailey · 2019-10-01T08:21:56Z

Looks like this failed a lint check.

aLekSer · 2019-10-01T08:35:21Z

Will fix that in a moment.

agones-bot · 2019-10-01T09:01:41Z

Build Failed 😱

Build Id: 98914077-3778-4329-bb5e-6f11884c3b40

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

Add a function which would return error instead failing the test. Add error stream to finish stress test gracefully in case there is no capacity in a cluster for a specific test. It is forbidden to call t.Fatalf() from go routine other than main test routine.

aLekSer · 2019-10-01T10:04:14Z

Oops, added but forgot to do git commit --amend

agones-bot · 2019-10-01T10:29:33Z

Build Succeeded 👏

Build Id: 8b7b2c8e-d0cb-4c38-b824-dda4be754eeb

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

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

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

https://4804c62-dot-preview-dot-agones-images.appspot.com/

To install this version:

git fetch https://github.com/GoogleCloudPlatform/agones.git pull/1065/head:pr_1065 && git checkout pr_1065
helm install install/helm/agones --namespace agones-system --name agones --set agones.image.tag=1.1.0-4804c62

google-oss-robot added do-not-merge/work-in-progress size/M labels Sep 24, 2019

aLekSer changed the title ~~Add func to wait for fleet condition without Failf~~ Add func to wait for fleet condition without Fatalf Sep 24, 2019

aLekSer force-pushed the wait-for-fleet-cond-parallel branch 2 times, most recently from f57c319 to e8692b2 Compare September 25, 2019 13:11

aLekSer marked this pull request as ready for review September 26, 2019 14:50

google-oss-robot removed the do-not-merge/work-in-progress label Sep 26, 2019

markmandel reviewed Sep 26, 2019

View reviewed changes

aLekSer force-pushed the wait-for-fleet-cond-parallel branch from e8692b2 to e221a98 Compare September 30, 2019 12:15

google-oss-robot added size/L and removed size/M labels Sep 30, 2019

markmandel approved these changes Oct 1, 2019

View reviewed changes

google-oss-robot assigned markmandel Oct 1, 2019

google-oss-robot added the lgtm label Oct 1, 2019

google-oss-robot removed the lgtm label Oct 1, 2019

aLekSer force-pushed the wait-for-fleet-cond-parallel branch from 2871be4 to 30469cc Compare October 1, 2019 08:52

aLekSer force-pushed the wait-for-fleet-cond-parallel branch from 30469cc to 4804c62 Compare October 1, 2019 10:03

markmandel added this to the 1.1.0 milestone Oct 1, 2019

markmandel added area/tests Unit tests, e2e tests, anything to make sure things don't break kind/cleanup Refactoring code, fixing up documentation, etc labels Oct 1, 2019

markmandel merged commit 72a3ddd into googleforgames:master Oct 1, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add func to wait for fleet condition without Fatalf #1065

Add func to wait for fleet condition without Fatalf #1065

aLekSer commented Sep 24, 2019 •

edited

Loading

agones-bot commented Sep 24, 2019

agones-bot commented Sep 25, 2019

agones-bot commented Sep 25, 2019

markmandel Sep 26, 2019

aLekSer Sep 26, 2019

markmandel Sep 26, 2019

agones-bot commented Sep 30, 2019

markmandel left a comment

google-oss-robot commented Oct 1, 2019

google-oss-robot commented Oct 1, 2019

agones-bot commented Oct 1, 2019

roberthbailey commented Oct 1, 2019

aLekSer commented Oct 1, 2019

agones-bot commented Oct 1, 2019

aLekSer commented Oct 1, 2019

agones-bot commented Oct 1, 2019

Add func to wait for fleet condition without Fatalf #1065

Add func to wait for fleet condition without Fatalf #1065

Conversation

aLekSer commented Sep 24, 2019 • edited Loading

agones-bot commented Sep 24, 2019

agones-bot commented Sep 25, 2019

agones-bot commented Sep 25, 2019

markmandel Sep 26, 2019

Choose a reason for hiding this comment

aLekSer Sep 26, 2019

Choose a reason for hiding this comment

markmandel Sep 26, 2019

Choose a reason for hiding this comment

agones-bot commented Sep 30, 2019

markmandel left a comment

Choose a reason for hiding this comment

google-oss-robot commented Oct 1, 2019

google-oss-robot commented Oct 1, 2019

agones-bot commented Oct 1, 2019

roberthbailey commented Oct 1, 2019

aLekSer commented Oct 1, 2019

agones-bot commented Oct 1, 2019

aLekSer commented Oct 1, 2019

agones-bot commented Oct 1, 2019

aLekSer commented Sep 24, 2019 •

edited

Loading