[Windows] Fix the race condition in ifConfigurator

[tnqn]

ensureHNSNetwork may be called in parallel but the variable hnsNetwork
was not protected by lock, which could lead to data race. This patch
fixes it by initializing the variable when ifConfigurator is instantiated.

It also removes an unused variable.

Signed-off-by: Quan Tian qtian@vmware.com

[tnqn]

/test-all

[codecov-commenter]

Codecov Report

Merging #2253 (1ebe78a) into main (11c4c1f) will increase coverage by 3.59%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main    #2253      +/-   ##
==========================================
+ Coverage   61.62%   65.21%   +3.59%     
==========================================
  Files         279      279              
  Lines       21327    21327              
==========================================
+ Hits        13142    13908     +766     
+ Misses       6821     6002     -819     
- Partials     1364     1417      +53     

Flag	Coverage Δ
e2e-tests	56.49% <ø> (?)
kind-e2e-tests	52.97% <ø> (+0.19%)	⬆️
unit-tests	41.26% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
pkg/apis/controlplane/v1beta2/conversion.go	60.00% <0.00%> (-10.00%)	⬇️
...kg/agent/flowexporter/connections/conntrack_ovs.go	76.50% <0.00%> (-3.62%)	⬇️
pkg/agent/cniserver/server.go	68.91% <0.00%> (-0.65%)	⬇️
pkg/controller/networkpolicy/validate.go	69.91% <0.00%> (+0.27%)	⬆️
pkg/ovs/openflow/ofctrl_bridge.go	50.36% <0.00%> (+0.36%)	⬆️
pkg/agent/openflow/network_policy.go	76.41% <0.00%> (+0.59%)	⬆️
pkg/agent/flowexporter/exporter/exporter.go	78.66% <0.00%> (+0.68%)	⬆️
pkg/agent/cniserver/pod_configuration.go	54.68% <0.00%> (+0.78%)	⬆️
pkg/controller/egress/store/egressgroup.go	1.38% <0.00%> (+1.38%)	⬆️
pkg/ovs/openflow/ofctrl_flow.go	47.36% <0.00%> (+1.75%)	⬆️
... and 42 more

[antoninbas]

LGTM, I'm not sure why it wasn't done this way to begin with

seems you have a formatting issue

[antoninbas]

@tnqn is this a fix that needs to be cherry-picked / backported?

[tnqn]

@antoninbas thanks for the review. I think it should be backported. I'm not sure what could happen in worst case with the race condition.

[tnqn]

/test-all

[antoninbas]

@tnqn In this case, could you backport it to 0.13, 1.0 and 1.1 as per https://github.com/antrea-io/antrea/blob/main/docs/contributors/cherry-picks.md, once this PR is merged?

[lzhecheng]

Please correct me if I'm wrong. What if antrea-hnsnetwork is not ready when newInterfaceConfigurator()?

[tnqn]

@antoninbas sure.
@lzhecheng prepareHostNetwork, newInterfaceConfigurator and almost all Initialization code are executely synchronously. With that in mind, we just need to guarantee newInterfaceConfigurator is executed after prepareHostNetwork, which is true for this case. Ideally we should make newInterfaceConfigurator consume prepareHostNetwork's output to make the dependency more explicit, but the dependency is specific to Windows so no good way to do it in cmd/antrea-agent/agent.go without introducing more platform specific files/structs, which is not something this patch tries to improve. Although the dependency is implicit, it's not easy to break as the program will fail to start if someone tries to revert the order of prepareHostNetwork and newInterfaceConfigurator.

[lzhecheng]

Thank you for the explanation. @tnqn
LGTM.