Skip to content

Increased wait time on test utils WaitForCluster and WatchTaskCreate #2762

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
Merged
dperny merged 1 commit into from
Oct 17, 2018
Merged

Increased wait time on test utils WaitForCluster and WatchTaskCreate #2762

dperny merged 1 commit into from
Oct 17, 2018

Conversation

@olljanat
Copy link
Contributor

@olljanat olljanat commented Oct 10, 2018 •
edited
Loading

I find out that most of the flaky tests looks to be caused by timeout during slowness on test machine.

This theory can be proven by running tests with CPU limit. I used these commands on my four core machine:

docker build . -t swarmkit-build
docker run --cpus="3" --rm --name test swarmkit-build make test

- What I did

  • Added 5 x 2 second wait time to WaitForCluster function in case when ReadyForProposals() does not return true.
  • Increased timeout from one second to two seconds on WatchTaskCreate() function, updated function TestSessionReconnectsIfDispatcherErrors() match with that one.

Fixes flaky tests on #2559 , #2600 and #2661

@olljanat olljanat force-pushed the fix-waitforcluster-test branch from ac348f2 to 9b5c11d Compare October 10, 2018 16:11
@olljanat olljanat changed the title Increased wait time on WaitForCluster testutils WIP: Increased wait time on WaitForCluster testutils Oct 10, 2018
@olljanat olljanat force-pushed the fix-waitforcluster-test branch 4 times, most recently from 15b0b09 to 7e7ccf7 Compare October 10, 2018 19:01
@codecov
Copy link

codecov bot commented Oct 10, 2018 •
edited
Loading

Codecov Report

❗ No coverage uploaded for pull request base (master@7d5d33b). Click here to learn what that means.
The diff coverage is n/a.

@@            Coverage Diff            @@
##             master    #2762   +/-   ##
=========================================
  Coverage          ?   61.81%           
=========================================
  Files             ?      134           
  Lines             ?    21857           
  Branches          ?        0           
=========================================
  Hits              ?    13510           
  Misses            ?     6886           
  Partials          ?     1461

@olljanat olljanat changed the title WIP: Increased wait time on WaitForCluster testutils Increased wait time on WaitForCluster testutils Oct 10, 2018
@olljanat
Copy link
Contributor Author

olljanat commented Oct 10, 2018

@cyli FYI

@olljanat olljanat force-pushed the fix-waitforcluster-test branch from 7e7ccf7 to 00dae50 Compare October 11, 2018 19:55
@olljanat olljanat changed the title Increased wait time on WaitForCluster testutils Increased wait time on test utils WaitForCluster and WatchTaskCreate Oct 11, 2018
@olljanat olljanat force-pushed the fix-waitforcluster-test branch from 00dae50 to 8e7aa1b Compare October 11, 2018 20:03
@olljanat olljanat force-pushed the fix-waitforcluster-test branch from 8e7aa1b to 5f167ca Compare October 11, 2018 20:21
wk8
wk8 approved these changes Oct 11, 2018
View reviewed changes
Copy link
Contributor

@wk8 wk8 left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@olljanat
Copy link
Contributor Author

olljanat commented Oct 15, 2018

Looks that CI test from master have been failed to error

--- FAIL: TestRaftLeaderLeave (10.23s)
	Error Trace:	testutils.go:89
			raft_test.go:426
	Error:		Received unexpected error did not find a ready leader in member list

which is fixed on this PR.

@dperny time to merge?

@dperny dperny merged commit a84c01f into moby:master Oct 17, 2018
@anshulpundir anshulpundir mentioned this pull request Oct 18, 2018
Merged
This was referenced Oct 18, 2018
Open
Closed
Closed
@thaJeztah thaJeztah mentioned this pull request Oct 18, 2018
Closed
@olljanat olljanat mentioned this pull request Nov 28, 2018
Closed
This was referenced Sep 25, 2019
Merged
Merged
@thaJeztah thaJeztah mentioned this pull request Oct 2, 2019
Merged
thaJeztah added a commit to thaJeztah/docker that referenced this pull request Oct 9, 2019
@thaJeztah
[18.09] bump SwarmKit 5c86095cef3ff480e69486da50f18fd1b3a0de78 (bump_…
e06f07e 
…v18.09)

full diff: moby/swarmkit@142a737...5c86095

- moby/swarmkit#2892 [18.09 backport] Remove hardcoded IPAM config subnet value for ingress network
    - backport of moby/swarmkit#2890 Remove hardcoded IPAM config subnet value for ingress network
    - fixes [ENGORC-2651](https://docker.atlassian.net/browse/ENGORC-2651)
- moby/swarmkit#2836 [18.09 backport] Switch to go 1.11
    - backport of moby/swarmkit#2752 Switch to go 1.11
- moby/swarmkit#2901 [18.09 backport] Bump to golang 1.12.9
    - backport of moby/swarmkit#2880 Bump to golang 1.12.9
- moby/swarmkit#2900 [18.09 backport] Fix update out of sequence and increase max recv gRPC message size for nodes and secrets
    - backport of moby/swarmkit#2762 Increased wait time on test utils WaitForCluster and WatchTaskCreate
    - backport of moby/swarmkit#2771 Allow using Configs as CredentialSpecs
        - **second commit only** (attempt to fix weirdly broken tests)
    - backport of moby/swarmkit#2808 Fix flaky tests
    - backport of moby/swarmkit#2866 Swap gometalinter for golangci-lint
    - backport of moby/swarmkit#2869 Increase max recv gRPC message size to initialize connection broker
        - related / similar to moby#38103 / docker-archive#102 cluster: set bigger grpc limit for array requests
        - related / similar to moby#39306 Increase max recv gRPC message size for nodes and secrets
        - fixes moby/swarmkit#2733 Error generated when messages size is too big
    - backport of moby/swarmkit#2870 Fix update out of sequence

Signed-off-by: Sebastiaan van Stijn <[email protected]>
docker-jenkins pushed a commit to docker-archive/docker-ce that referenced this pull request Oct 23, 2019
@thaJeztah
[18.09] bump SwarmKit 5c86095cef3ff480e69486da50f18fd1b3a0de78 (bump_…
a7f9149 
…v18.09)

full diff: moby/swarmkit@142a737...5c86095

- moby/swarmkit#2892 [18.09 backport] Remove hardcoded IPAM config subnet value for ingress network
    - backport of moby/swarmkit#2890 Remove hardcoded IPAM config subnet value for ingress network
    - fixes [ENGORC-2651](https://docker.atlassian.net/browse/ENGORC-2651)
- moby/swarmkit#2836 [18.09 backport] Switch to go 1.11
    - backport of moby/swarmkit#2752 Switch to go 1.11
- moby/swarmkit#2901 [18.09 backport] Bump to golang 1.12.9
    - backport of moby/swarmkit#2880 Bump to golang 1.12.9
- moby/swarmkit#2900 [18.09 backport] Fix update out of sequence and increase max recv gRPC message size for nodes and secrets
    - backport of moby/swarmkit#2762 Increased wait time on test utils WaitForCluster and WatchTaskCreate
    - backport of moby/swarmkit#2771 Allow using Configs as CredentialSpecs
        - **second commit only** (attempt to fix weirdly broken tests)
    - backport of moby/swarmkit#2808 Fix flaky tests
    - backport of moby/swarmkit#2866 Swap gometalinter for golangci-lint
    - backport of moby/swarmkit#2869 Increase max recv gRPC message size to initialize connection broker
        - related / similar to moby/moby#38103 / docker-archive/engine#102 cluster: set bigger grpc limit for array requests
        - related / similar to moby/moby#39306 Increase max recv gRPC message size for nodes and secrets
        - fixes moby/swarmkit#2733 Error generated when messages size is too big
    - backport of moby/swarmkit#2870 Fix update out of sequence

Signed-off-by: Sebastiaan van Stijn <[email protected]>
Upstream-commit: e06f07ef337ab890f211397d6b408b75a2512dc5
Component: engine
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

2 more reviewers

@wk8 wk8 wk8 approved these changes

@anshulpundir anshulpundir anshulpundir approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@olljanat @wk8 @anshulpundir @dperny