[Windows] Fix br-int NetIPAddress not found issue #1660

ruicao93 · 2020-12-13T04:39:20Z

Do not remove the NetIPAddress on br-int anymore to avoid the
"No matching MSFT_NetIPAddress objects found" issue because the
NetIPAddress may not exist on br-int.

Instead, always try to configure NetIPAddress on the interface
and ignore the "MSFT_NetIPAddress already exists" error.

Fixes: #1644
Signed-off-by: Rui Cao rcao@vmware.com

ruicao93 · 2020-12-13T04:42:39Z

/test-all

codecov-io · 2020-12-13T04:43:05Z

Codecov Report

Merging #1660 (58e83c7) into master (9d3d10b) will increase coverage by 1.09%.
The diff coverage is 61.00%.

@@            Coverage Diff             @@
##           master    #1660      +/-   ##
==========================================
+ Coverage   63.31%   64.41%   +1.09%     
==========================================
  Files         170      181      +11     
  Lines       14250    15410    +1160     
==========================================
+ Hits         9023     9927     +904     
- Misses       4292     4460     +168     
- Partials      935     1023      +88

Flag	Coverage Δ
e2e-tests	`47.91% <39.50%> (?)`
kind-e2e-tests	`53.50% <54.26%> (-1.88%)`	⬇️
unit-tests	`40.75% <26.49%> (-0.53%)`	⬇️

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

Impacted Files	Coverage Δ
cmd/antrea-agent/agent.go	`0.00% <0.00%> (ø)`
.../agent/apiserver/handlers/networkpolicy/handler.go	`58.33% <ø> (ø)`
...gent/controller/noderoute/node_route_controller.go	`60.98% <0.00%> (+14.51%)`	⬆️
pkg/agent/stats/collector.go	`97.72% <ø> (ø)`
pkg/antctl/antctl.go	`100.00% <ø> (ø)`
pkg/antctl/transform/controllerinfo/transform.go	`0.00% <ø> (ø)`
pkg/antctl/transform/version/transform.go	`44.82% <ø> (ø)`
pkg/controller/networkpolicy/tier.go	`90.00% <ø> (ø)`
pkg/features/antrea_features.go	`16.66% <ø> (ø)`
pkg/ovs/openflow/ofctrl_builder.go	`60.94% <0.00%> (-1.23%)`	⬇️
... and 98 more

Do not remove the NetIPAddress on br-int anymore to avoid the "No matching MSFT_NetIPAddress objects found" issue because the NetIPAddress may not exist on br-int. Instead, always try to configure NetIPAddress on the interface and ignore the "MSFT_NetIPAddress already exists" error. Fixes: antrea-io#1644 Signed-off-by: Rui Cao <rcao@vmware.com>

ruicao93 · 2020-12-13T07:13:04Z

/test-all

ruicao93 · 2020-12-14T02:49:52Z

/test-all

tnqn · 2020-12-14T15:17:01Z

pkg/agent/agent_windows.go

@@ -150,14 +150,14 @@ func (i *Initializer) prepareOVSBridge() error {
 	if err = util.SetAdapterMACAddress(brName, &uplinkNetConfig.MAC); err != nil {
 		return err
 	}
-	// Remove existing IP addresses to avoid a candidate error of "Instance MSFT_NetIPAddress already exists" when


Why didn't the call fail before? When it initializes the node the first time, will it have IP on bright interface?

Because in our private CI, DHCP service is enabled. br-int always has chance to get IP from DHCP server.

Let me double check it.

Then could we have another issue if we don't clean up IPs got from DHCP server? or it's fine to keep as is?

Let me double check it.

I double check the testbed, the reason truly as I mentioned above.

I think it's fine to keep the IP from DHCP server.

Actually a potential issue is that if br-int might get a different IP from DHCP server intead of the IP we expect.

thanks for confirming.

pkg/agent/agent_windows.go

jayunit100 · 2020-12-14T22:16:58Z

any way we can merge this relatively quickly and iterate any minor issues huge blocker for us on the CAPV side :)

jianjuns · 2020-12-15T00:56:22Z

pkg/agent/agent_windows.go

-		return err
+		if strings.Contains(err.Error(), "Instance MSFT_NetIPAddress already exists") {
+			err = nil
+			klog.V(4).Infof("Address: %s already exists on interface %s", uplinkNetConfig.IP.String(), brName)


Could you explain to me when this can happen?
It is for IP already exists on br-int, or also when the IP exists on another adapter of the host?

@jianjuns :

Could you explain to me when this can happen?

The IP is on Ethernet0 at first.

Set the Ethernet0 as uplink and the Ethernet0 wil have no IP. The corresponding MSFT_NetIPAddress item will also be removed.

Add br-int to OVS and set the MAC same as Ethernet0.

If the DHCP is enabled. br-int may get the same IP, which will cause Instance MSFT_NetIPAddress already exists when we try to set the IP later.

It is for IP already exists on br-int, or also when the IP exists on another adapter of the host?

It's for IP already exists on br-int. Actually the IP has change exist on another adapter. But I think it's almost impossible unless some other process set the IP on another adapter during these steps. So I didn't put too much check here.

Change log info to: klog.V(4).Infof("Address: %s already exists when configuring IP on interface %s", uplinkNetConfig.IP.String(), brName). It should be more accurate.

Could be better if you add the explanation (DHCP etc.) to the code comments.

- Remove dead functions: - RemoveIPv4AddrsFromAdapter - GetAdapterIPv4Addr - Update if/else order. - Update log info Signed-off-by: Rui Cao <rcao@vmware.com>

ruicao93 · 2020-12-15T16:13:59Z

/test-all

tnqn

LGTM

tnqn · 2020-12-15T16:39:45Z

pkg/agent/agent_windows.go

@@ -150,14 +150,14 @@ func (i *Initializer) prepareOVSBridge() error {
 	if err = util.SetAdapterMACAddress(brName, &uplinkNetConfig.MAC); err != nil {
 		return err
 	}
-	// Remove existing IP addresses to avoid a candidate error of "Instance MSFT_NetIPAddress already exists" when


thanks for confirming.

* [Windows] Fix br-int NetIPAddress not found issue Do not remove the NetIPAddress on br-int anymore to avoid the "No matching MSFT_NetIPAddress objects found" issue because the NetIPAddress may not exist on br-int. Instead, always try to configure NetIPAddress on the interface and ignore the "MSFT_NetIPAddress already exists" error. Fixes: #1644 Signed-off-by: Rui Cao <rcao@vmware.com> * Address comments - Remove dead functions: - RemoveIPv4AddrsFromAdapter - GetAdapterIPv4Addr - Update if/else order. - Update log info Signed-off-by: Rui Cao <rcao@vmware.com>

vmwclabot added the cla-not-required label Dec 13, 2020

ruicao93 mentioned this pull request Dec 13, 2020

Windows Agent failing on NSX-T network #1644

Closed

ruicao93 force-pushed the bridge_ip_missed branch from 98a05a1 to 93b612d Compare December 13, 2020 07:12

ruicao93 added the area/OS/windows Issues or PRs related to the Windows operating system. label Dec 14, 2020

ruicao93 requested review from tnqn, antoninbas and jianjuns December 14, 2020 07:17

tnqn reviewed Dec 14, 2020

View reviewed changes

jianjuns reviewed Dec 15, 2020

View reviewed changes

Address comments

58e83c7

- Remove dead functions: - RemoveIPv4AddrsFromAdapter - GetAdapterIPv4Addr - Update if/else order. - Update log info Signed-off-by: Rui Cao <rcao@vmware.com>

tnqn approved these changes Dec 15, 2020

View reviewed changes

jianjuns approved these changes Dec 15, 2020

View reviewed changes

jianjuns merged commit 8b7430c into antrea-io:master Dec 15, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Windows] Fix br-int NetIPAddress not found issue #1660

[Windows] Fix br-int NetIPAddress not found issue #1660

ruicao93 commented Dec 13, 2020

ruicao93 commented Dec 13, 2020

codecov-io commented Dec 13, 2020 •

edited

Loading

ruicao93 commented Dec 13, 2020

ruicao93 commented Dec 14, 2020

tnqn Dec 14, 2020

ruicao93 Dec 15, 2020 •

edited

Loading

tnqn Dec 15, 2020

ruicao93 Dec 15, 2020 •

edited

Loading

tnqn Dec 15, 2020

jayunit100 commented Dec 14, 2020

jianjuns Dec 15, 2020

ruicao93 Dec 15, 2020 •

edited

Loading

jianjuns Dec 15, 2020

ruicao93 commented Dec 15, 2020

tnqn left a comment

tnqn Dec 15, 2020

[Windows] Fix br-int NetIPAddress not found issue #1660

[Windows] Fix br-int NetIPAddress not found issue #1660

Conversation

ruicao93 commented Dec 13, 2020

ruicao93 commented Dec 13, 2020

codecov-io commented Dec 13, 2020 • edited Loading

Codecov Report

ruicao93 commented Dec 13, 2020

ruicao93 commented Dec 14, 2020

tnqn Dec 14, 2020

Choose a reason for hiding this comment

ruicao93 Dec 15, 2020 • edited Loading

Choose a reason for hiding this comment

tnqn Dec 15, 2020

Choose a reason for hiding this comment

ruicao93 Dec 15, 2020 • edited Loading

Choose a reason for hiding this comment

tnqn Dec 15, 2020

Choose a reason for hiding this comment

jayunit100 commented Dec 14, 2020

jianjuns Dec 15, 2020

Choose a reason for hiding this comment

ruicao93 Dec 15, 2020 • edited Loading

Choose a reason for hiding this comment

jianjuns Dec 15, 2020

Choose a reason for hiding this comment

ruicao93 commented Dec 15, 2020

tnqn left a comment

Choose a reason for hiding this comment

tnqn Dec 15, 2020

Choose a reason for hiding this comment

codecov-io commented Dec 13, 2020 •

edited

Loading

ruicao93 Dec 15, 2020 •

edited

Loading

ruicao93 Dec 15, 2020 •

edited

Loading

ruicao93 Dec 15, 2020 •

edited

Loading