libnetwork/ipam: fix racy, flaky unit test#45019
Merged
thaJeztah merged 1 commit intomoby:masterfrom Feb 23, 2023
Merged
Conversation
TestRequestReleaseAddressDuplicate gets flagged by go test -race because the same err variable inside the test is assigned to from multiple goroutines without synchronization, which obscures whether or not there are any data races in the code under test. Trouble is, the test _depends on_ the data race to exit the loop if an error occurs inside a spawned goroutine. And the test contains a logical concurrency bug (not flagged by the Go race detector) which can result in false-positive test failures. Because a release operation is logged after the IP is released, the other goroutine could reacquire the address and log that it was reacquired before the release is logged. Fix up the test so it is no longer subject to data races or false-positive test failures, i.e. flakes. Signed-off-by: Cory Snider <csnider@mirantis.com>
neersighted
approved these changes
Feb 23, 2023
vvoland
approved these changes
Feb 23, 2023
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
TestRequestReleaseAddressDuplicategets flagged bygo test -racebecause the same err variable inside the test is assigned to from multiple goroutines without synchronization, which obscures whether or not there are any data races in the code under test.Trouble is, the test depends on the data race to exit the loop if an error occurs inside a spawned goroutine. And the test contains a logical concurrency bug (not flagged by the Go race detector) which can result in false-positive test failures. Because a release operation is logged after the IP is released, the other goroutine could reacquire the address and log that it was reacquired before the release is logged.
- What I did
Fixed up the test so it is no longer subject to data races or false-positive test failures, i.e. flakes.
- How I did it
ipsslice to err on the side of false negatives instead of false positives- How to verify it
$ go test -race -count=5 -run TestRequestReleaseAddressDuplicate ./libnetwork/ipam- Description for the changelog
- A picture of a cute animal (not mandatory but encouraged)