Add -race Flag to GitHub Action and Fix Data Race in CreateTailscaleNodesInUser #2038
Conversation
…eateTailscaleNodesInUser
nadongjun
requested review from
ohdearaugustin,
juanfont and
kradalby
as code owners
|
Good job, looks like there are some more races! |
|
@nadongjun do you have time to have a look at the failing tests? |
|
@kradalby Sure, I should be able to complete that by the end of this week. |
Signed-off-by: Dongjun Na <[email protected]>
WalkthroughThis update enhances the testing framework and concurrency control across various components. The addition of the Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (4)
- .github/workflows/test-integration.yaml (1 hunks)
- Makefile (2 hunks)
- integration/dockertestutil/execute.go (5 hunks)
- integration/scenario.go (1 hunks)
Additional context used
golangci-lint
integration/dockertestutil/execute.go
41-41: variable name 'mu' is too short for the scope of its usage
(varnamelen)
Additional comments not posted (6)
Makefile (2)
25-25: Incorporate race detection in unit tests.The addition of the
-raceflag to thegotestsumcommand in thetesttarget enhances the detection of race conditions during unit testing.
36-36: Incorporate race detection in integration tests.The addition of the
-raceflag to thegotestsumcommand in thetest_integrationtarget enhances the detection of race conditions during integration testing.integration/dockertestutil/execute.go (1)
63-78: Enhance thread safety with local buffers and mutex.The use of local buffers for capturing command output and the addition of a mutex to synchronize writes to shared buffers effectively prevent race conditions.
.github/workflows/test-integration.yaml (1)
104-104: Enhance race condition detection in GitHub Actions.The inclusion of the
-raceflag in the test command within the GitHub Actions workflow improves the detection of race conditions during integration testing.integration/scenario.go (2)
330-335: Ensure thread safety with mutex locks.The addition of mutex locks around shared resource access in
CreateTailscaleNodesInUserhelps prevent race conditions, enhancing the method's reliability.
338-345: Maintain thread safety during Tailscale client creation.Mutex locks are correctly used to ensure thread safety during the creation of Tailscale clients, preventing concurrent access issues.
Signed-off-by: Dongjun Na <[email protected]>
|
@kradalby The timeout issue caused the -race flag to be removed from integration tests. It remains in the Makefile for local testing of race conditions. |
The changes include:
Log
Summary by CodeRabbit
New Features
Bug Fixes
Style