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.

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

[19.03 backport] Fix update out of sequence and increase max recv gRPC message size for nodes and secrets #2889

Merged

Conversation

thaJeztah
Copy link
Member

relates to

Backports of

Cherry-pick were clean, no conflicts

# https://github.com/docker/swarmkit/pull/2808 Fix flaky tests
git cherry-pick -s -S -x 06a356671bc11e4fd5d754f257f9c5f93ec5c563

# https://github.com/docker/swarmkit/pull/2866 Swap gometalinter for golangci-lint
git cherry-pick -s -S -x 27c2d27e23e76243bb49ec5d803a6e40b3f96f7a

# https://github.com/docker/swarmkit/pull/2869 Increase max recv gRPC message size to initialize connection broker
git cherry-pick -s -S -x 127e816ed8c8de4c981e02c9d0b45c70d3baa824

# https://github.com/docker/swarmkit/pull/2870 Fix update out of sequence
git cherry-pick -s -S -x d68ac46e3b11d7384472677d210bb0ce941284dc

dperny and others added 4 commits September 6, 2019 23:56
It is likely that a large portion of test flakiness, especially in CI,
comes from the fact that swarmkit components under test are started in
goroutines, but those goroutines never have an opportunity to run. This
adds code ensuring those goroutines are scheduled and run, which should
hopefully solve many inexplicably flaky tests.

Additionally, increased test timeouts, to hopefully cover a few more
flaky cases.

Finally, removed direct use of the atomic package, in favor of less
efficient but higher-level mutexes.

Signed-off-by: Drew Erny <drew.erny@docker.com>
(cherry picked from commit 06a3566)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
gometalinter is deprecated, and golangci-lint is its recommended
successor. This commit adds golangci-lint as the linter for swarmkit. In
addition, golangci-lint found a few issues in the code that were not
yet identified, and so those issues have been fixed.

Signed-off-by: Drew Erny <drew.erny@docker.com>
(cherry picked from commit 27c2d27)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: nmengin <nicolas@containo.us>
(cherry picked from commit 127e816)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
A simple but old error has recently become evident. Due to the fact that
we read an object and then write it back across the boundaries of a
transaction, it is possible for the task object to have changed in
between transactions. This would cause the attempt to write out the old
task to suffer an "Update out of sequence" error.

This fix simply reads the latest version of the task back out within the
boundary of a transaction to avoid the race.

Signed-off-by: Drew Erny <drew.erny@docker.com>
(cherry picked from commit d68ac46)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@thaJeztah
Copy link
Member Author

ping @dperny PTAL

@tonistiigi
Copy link
Member

@thaJeztah CI not passing

@thaJeztah
Copy link
Member Author

yeah, flaky test; #2559

FAIL: TestUpdaterRollback (6.40s)
    --- FAIL: TestUpdaterRollback/pause/monitor_set/spec_version_unset (3.59s)
        update_test.go:272: service was updated
        <autogenerate

@dperny
Copy link
Collaborator

dperny commented Sep 11, 2019

test is known to be flaky despite many attempts to improve it.

@codecov
Copy link

codecov bot commented Sep 11, 2019

Codecov Report

Merging #2889 into bump_v19.03 will increase coverage by 0.01%.
The diff coverage is 31.25%.

@@               Coverage Diff               @@
##           bump_v19.03    #2889      +/-   ##
===============================================
+ Coverage         62.1%   62.12%   +0.01%     
===============================================
  Files              139      139              
  Lines            22314    22320       +6     
===============================================
+ Hits             13859    13866       +7     
  Misses            6976     6976              
+ Partials          1479     1478       -1

@thaJeztah
Copy link
Member Author

Looks like it's green now 🎉

@dperny dperny merged commit bbe3418 into moby:bump_v19.03 Sep 11, 2019
@thaJeztah thaJeztah deleted the 19.03_backport_fix_update_out_of_sequence branch September 12, 2019 07:54
thaJeztah added a commit to thaJeztah/docker that referenced this pull request Sep 12, 2019
…3 branch)

full diff: moby/swarmkit@4fb9e96...bbe3418

changes included:

- moby/swarmkit#2889 [19.03 backport] Fix update out of sequence and increase max recv gRPC message size for nodes and secrets

Which relates to

- moby#39531 integration-cli: fix swarm tests flakiness
- docker#345 [19.03 backport] integration-cli: fix swarm tests flakiness

And includes backports of

- moby/swarmkit#2808 Fix flaky tests
- moby/swarmkit#2866 Swap gometalinter for golangci-lint
- moby/swarmkit#2869 Increase max recv gRPC message size to initialize connection broker
 - related / similar to moby#38103 / docker#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
- moby/swarmkit#2870 Fix update out of sequence

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
docker-jenkins pushed a commit to docker/docker-ce that referenced this pull request Sep 12, 2019
…3 branch)

full diff: moby/swarmkit@4fb9e96...bbe3418

changes included:

- moby/swarmkit#2889 [19.03 backport] Fix update out of sequence and increase max recv gRPC message size for nodes and secrets

Which relates to

- moby/moby#39531 integration-cli: fix swarm tests flakiness
- docker/engine#345 [19.03 backport] integration-cli: fix swarm tests flakiness

And includes backports of

- moby/swarmkit#2808 Fix flaky tests
- moby/swarmkit#2866 Swap gometalinter for golangci-lint
- moby/swarmkit#2869 Increase max recv gRPC message size to initialize connection broker
 - related / similar to moby/moby#38103 / docker/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
- moby/swarmkit#2870 Fix update out of sequence

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Upstream-commit: f7dbee3eeaa1dd218116f85b8f60361acbd5b214
Component: engine
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants