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

Fix race condition in unit test for IPsec #4060

Merged
merged 1 commit into from
Jul 29, 2022

Conversation

xliuxu
Copy link
Contributor

@xliuxu xliuxu commented Jul 28, 2022

Wait for the signer goroutine to finish before quitting the tests.
This ensures that the *testing.T is always valid in the signer.

Fixes: #4059

Signed-off-by: Xu Liu [email protected]

@xliuxu xliuxu requested review from wenyingd and tnqn July 28, 2022 13:42
@codecov
Copy link

codecov bot commented Jul 28, 2022

Codecov Report

Merging #4060 (593c0bc) into main (28d2655) will increase coverage by 3.21%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #4060      +/-   ##
==========================================
+ Coverage   64.09%   67.30%   +3.21%     
==========================================
  Files         294      297       +3     
  Lines       44161    44434     +273     
==========================================
+ Hits        28305    29908    +1603     
+ Misses      13548    12167    -1381     
- Partials     2308     2359      +51     
Flag Coverage Δ
integration-tests 35.30% <ø> (?)
kind-e2e-tests 50.73% <ø> (+0.65%) ⬆️
unit-tests 44.20% <ø> (-0.03%) ⬇️
Impacted Files Coverage Δ
.../agent/flowexporter/priorityqueue/priorityqueue.go 65.55% <0.00%> (-27.78%) ⬇️
pkg/agent/flowexporter/exporter/certificate.go 50.00% <0.00%> (-16.67%) ⬇️
pkg/agent/nodeportlocal/k8s/annotations.go 84.44% <0.00%> (-15.56%) ⬇️
...s/multicluster/memberclusterannounce_controller.go 56.64% <0.00%> (-15.16%) ⬇️
pkg/controller/externalippool/controller.go 84.82% <0.00%> (-6.25%) ⬇️
pkg/agent/controller/networkpolicy/packetin.go 71.12% <0.00%> (-2.82%) ⬇️
...catesigningrequest/ipsec_csr_signing_controller.go 61.65% <0.00%> (-2.46%) ⬇️
pkg/agent/client.go 76.74% <0.00%> (-2.33%) ⬇️
pkg/agent/cniserver/ipam/ipam_delegator.go 54.54% <0.00%> (-1.52%) ⬇️
pkg/controller/ipam/antrea_ipam_controller.go 75.10% <0.00%> (-1.32%) ⬇️
... and 54 more

signCh := make(chan struct{})
defer func() {
<-signCh
}()
Copy link
Member

Choose a reason for hiding this comment

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

if the next line returns an error, the code will block forever?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moved the defer func below the watcher error check. Thanks!

Wait for the signer goroutine to finish before quitting the tests.
This ensures that the *testing.T is always valid in the signer.

Fixes: antrea-io#4059

Signed-off-by: Xu Liu <[email protected]>
@xliuxu xliuxu force-pushed the fix_ipsec_ut_race branch from 3dd2d30 to 593c0bc Compare July 29, 2022 06:19
Copy link
Contributor

@wenyingd wenyingd left a comment

Choose a reason for hiding this comment

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

LGTM

@tnqn
Copy link
Member

tnqn commented Jul 29, 2022

/skip-all

@tnqn
Copy link
Member

tnqn commented Jul 29, 2022

/skip-conformance
/skip-e2e
/skip-networkpolicy

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.

Race condition in unit test on IPsec
3 participants