Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

CI: Wait for tests step before sdk-conformance #1808

Merged
merged 3 commits into from
Sep 14, 2020
Merged

CI: Wait for tests step before sdk-conformance #1808

merged 3 commits into from
Sep 14, 2020

Conversation

aLekSer
Copy link
Collaborator

@aLekSer aLekSer commented Sep 11, 2020 •
edited
Loading

Both these steps would trigger building with go run or npm run in
parallel on the same directories.

What type of PR is this?

Uncomment only one /kind <> line, hit enter to put that in a new line, and remove leading whitespace from that line:

/kind breaking

/kind bug

What this PR does / Why we need it:
CI is fluctuating and fails too frequently.

Which issue(s) this PR fixes:

Work on #1779

Special notes for your reviewer:

@aLekSer
Copy link
Collaborator Author

aLekSer commented Sep 11, 2020

/lint

@aLekSer

This comment has been minimized.

Sign in to view
@google-oss-robot

This comment has been minimized.

Sign in to view
@agones-bot
Copy link
Collaborator

Build Failed 😱

Build Id: ec6247fe-fe6a-4013-9b68-02030c22e808

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

@aLekSer
Copy link
Collaborator Author

aLekSer commented Sep 12, 2020 •
edited
Loading

Too many simultaneous E2E tests but conformance was OK:

Setting up lock at path: LockE2E/.lock
Attempting lock acquisition
Handling connection for 8500
Lock acquisition failed: failed to acquire lock: Unexpected response code: 500 (invalid session "a700b588-431a-01e4-b6f1-99e7d9461afb")

And from another test run:


Step #22 - "e2e-stable": completed e2e test
Step #22 - "e2e-stable": Child terminated, releasing lock
Step #22 - "e2e-stable": Cleanup aborted, lock in use
Step #22 - "e2e-stable": E0911 23:57:40.258280      21 portforward.go:385] error copying from local connection to remote stream: read tcp4 127.0.0.1:8500->127.0.0.1:47686: read: connection reset by peer
Step #22 - "e2e-stable": successfully killed kubectl proxy

@agones-bot
Copy link
Collaborator

Build Succeeded 👏

Build Id: 381c1160-2671-4776-b414-c9cf43a30b04

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:

  • git fetch https://github.com/googleforgames/agones.git pull/1808/head:pr_1808 && git checkout pr_1808
  • helm install ./install/helm/agones --namespace agones-system --name agones --set agones.image.tag=1.9.0-8d395d6

@aLekSer aLekSer force-pushed the fix/sdk-conformance branch from b1537c4 to b8e547f Compare September 12, 2020 00:16
@aLekSer
Wait for tests step before sdk-conformance
5730984 
Both this test would trigger building with `go run` or `npm run` in
parallel on the same volume mounts.
@aLekSer aLekSer force-pushed the fix/sdk-conformance branch from b8e547f to 5730984 Compare September 12, 2020 00:23
@agones-bot
Copy link
Collaborator

Build Succeeded 👏

Build Id: 766000f9-c222-4a5e-a778-5622786c8f3b

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:

  • git fetch https://github.com/googleforgames/agones.git pull/1808/head:pr_1808 && git checkout pr_1808
  • helm install ./install/helm/agones --namespace agones-system --name agones --set agones.image.tag=1.9.0-b8e547f

@agones-bot
Copy link
Collaborator

Build Succeeded 👏

Build Id: d15a6038-cdb4-4b8e-8cde-eefa6ab814b0

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:

  • git fetch https://github.com/googleforgames/agones.git pull/1808/head:pr_1808 && git checkout pr_1808
  • helm install ./install/helm/agones --namespace agones-system --name agones --set agones.image.tag=1.9.0-5730984

@aLekSer aLekSer mentioned this pull request Sep 14, 2020
Merged
@markmandel
Copy link
Collaborator

This is looking like it's solving the issue? Although it does look like it pushes the CI time to completion from around 18 minutes to around 30 - which is a big jump (that being said, we're probably at that already with retries, so 🤷)

Run it a few more times, and if it keeps passing, I say let's merge it. WDYT?

@aLekSer
Copy link
Collaborator Author

aLekSer commented Sep 14, 2020

Well, yes, longest running time is bad, we can fix this side effect somehow with caching. We can see SDK conformance would not come up at a later time, if it would be as frequent as now, we can remove this "wait_for": "tests" or might move tests which could not run together in a separate cloud build step, or add File Lock before running the same test twice, say NodeJS SDK test would wait for release of file lock in NodeJS SDK conformance test?

@agones-bot
Copy link
Collaborator

Build Succeeded 👏

Build Id: e802f517-5d92-4eb4-8526-55cf6522cd79

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:

  • git fetch https://github.com/googleforgames/agones.git pull/1808/head:pr_1808 && git checkout pr_1808
  • helm install ./install/helm/agones --namespace agones-system --name agones --set agones.image.tag=1.9.0-734ca8b

markmandel
markmandel approved these changes Sep 14, 2020
View reviewed changes
Copy link
Collaborator

@markmandel markmandel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hasn't failed yet!

@google-oss-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

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

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
Copy link

New changes are detected. LGTM label has been removed.

@agones-bot
Copy link
Collaborator

Build Succeeded 👏

Build Id: 6072076c-bcca-4d2f-a015-7c179efb4624

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:

  • git fetch https://github.com/googleforgames/agones.git pull/1808/head:pr_1808 && git checkout pr_1808
  • helm install ./install/helm/agones --namespace agones-system --name agones --set agones.image.tag=1.9.0-cddeb2f

@aLekSer aLekSer merged commit d9ab07a into googleforgames:master Sep 14, 2020
@aLekSer aLekSer deleted the fix/sdk-conformance branch September 14, 2020 19:45
@aLekSer aLekSer added the area/tests Unit tests, e2e tests, anything to make sure things don't break label Sep 16, 2020
@markmandel markmandel added this to the 1.9.0 milestone Sep 22, 2020
ilkercelikyilmaz pushed a commit to ilkercelikyilmaz/agones that referenced this pull request Oct 23, 2020
@aLekSer @markmandel @ilkercelikyilmaz
Wait for tests step before sdk-conformance (googleforgames#1808)
28b7fdf 
Both this test would trigger building with `go run` or `npm run` in
parallel on the same volume mounts.

Co-authored-by: Mark Mandel <markmandel@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@markmandel markmandel markmandel approved these changes

@cyriltovena cyriltovena Awaiting requested review from cyriltovena

@dzlier-gcp dzlier-gcp Awaiting requested review from dzlier-gcp

Assignees

@markmandel markmandel

Labels
approved area/tests Unit tests, e2e tests, anything to make sure things don't break cla: yes size/XS
Projects
None yet
Milestone
1.9.0
Development

Successfully merging this pull request may close these issues.
5 participants
@aLekSer @google-oss-robot @agones-bot @markmandel @googlebot