fix dualnic windows duplicated routes issue

cni/network/multitenancy.go

@@ -159,6 +159,26 @@ func (m *Multitenancy) DetermineSnatFeatureOnHost(snatFile, nmAgentSupportedApis
 	return snatConfig.EnableSnatForDns, snatConfig.EnableSnatOnHost, nil
 }
 
+// addDefaultRoute appends a default route
+// to both epInfo and result. Returns error if gwStr is not a valid IP.
+func (m *Multitenancy) addDefaultRoute(gwStr string, epInfo *network.EndpointInfo, result *network.InterfaceInfo) error {
+	gw := net.ParseIP(gwStr)
+	if gw == nil {
+		return fmt.Errorf("invalid gateway IP: %s", gwStr) //nolint
+	}
+
+	var dst net.IPNet
+	if gw.To4() != nil {
+		_, defaultIPNet, _ := net.ParseCIDR("0.0.0.0/0")
+		dst = net.IPNet{IP: net.IPv4zero, Mask: defaultIPNet.Mask}
+	}
+
+	ri := network.RouteInfo{Dst: dst, Gw: gw}
+	epInfo.Routes = append(epInfo.Routes, ri)
+	result.Routes = append(result.Routes, ri)
+	return nil
+}
+
 func (m *Multitenancy) SetupRoutingForMultitenancy(
 	nwCfg *cni.NetworkConfig,
 	cnsNetworkConfig *cns.GetNetworkContainerResponse,
@@ -172,12 +192,18 @@ func (m *Multitenancy) SetupRoutingForMultitenancy(
 		logger.Info("add default route for multitenancy.snat on host enabled")
 		addDefaultRoute(cnsNetworkConfig.LocalIPConfiguration.GatewayIPAddress, epInfo, result)
 	} else {
-		_, defaultIPNet, _ := net.ParseCIDR("0.0.0.0/0")
-		dstIP := net.IPNet{IP: net.ParseIP("0.0.0.0"), Mask: defaultIPNet.Mask}
-		gwIP := net.ParseIP(cnsNetworkConfig.IPConfiguration.GatewayIPAddress)
-		epInfo.Routes = append(epInfo.Routes, network.RouteInfo{Dst: dstIP, Gw: gwIP})
-		result.Routes = append(result.Routes, network.RouteInfo{Dst: dstIP, Gw: gwIP})
-
+		// only set default route when skipDefaultRoutes is false to avoid duplicated default routes given to HNS
+		if !epInfo.SkipDefaultRoutes {
+			if err := m.addDefaultRoute(
+				cnsNetworkConfig.IPConfiguration.GatewayIPAddress,
+				epInfo, result,
+			); err != nil {
+				logger.Error("failed adding default route",
+					zap.String("gateway", cnsNetworkConfig.IPConfiguration.GatewayIPAddress),
+					zap.Error(err),
+				)
+			}
+		}
 		if epInfo.EnableSnatForDns {
 			logger.Info("add SNAT for DNS enabled")
 			addSnatForDNS(cnsNetworkConfig.LocalIPConfiguration.GatewayIPAddress, epInfo, result)

cni/network/multitenancy_test.go

@@ -189,6 +189,7 @@ func getIPNetWithString(ipaddrwithcidr string) *net.IPNet {
 
 func TestSetupRoutingForMultitenancy(t *testing.T) {
 	require := require.New(t) //nolint:gocritic
+
 	type args struct {
 		nwCfg            *cni.NetworkConfig
 		cnsNetworkConfig *cns.GetNetworkContainerResponse
@@ -204,31 +205,28 @@ func TestSetupRoutingForMultitenancy(t *testing.T) {
 		expected           args
 	}{
 		{
-			name: "test happy path",
+			name: "adds default v4 route when SNAT disabled and SkipDefaultRoutes=false",
 			args: args{
 				nwCfg: &cni.NetworkConfig{
 					MultiTenancy:     true,
 					EnableSnatOnHost: false,
 				},
 				cnsNetworkConfig: &cns.GetNetworkContainerResponse{
 					IPConfiguration: cns.IPConfiguration{
-						IPSubnet:         cns.IPSubnet{},
-						DNSServers:       nil,
 						GatewayIPAddress: "10.0.0.1",
 					},
 				},
-				epInfo: &network.EndpointInfo{},
+				epInfo: &network.EndpointInfo{}, // SkipDefaultRoutes defaults to false
 				result: &network.InterfaceInfo{},
 			},
+			multitenancyClient: &Multitenancy{},
 			expected: args{
 				nwCfg: &cni.NetworkConfig{
 					MultiTenancy:     true,
 					EnableSnatOnHost: false,
 				},
 				cnsNetworkConfig: &cns.GetNetworkContainerResponse{
 					IPConfiguration: cns.IPConfiguration{
-						IPSubnet:         cns.IPSubnet{},
-						DNSServers:       nil,
 						GatewayIPAddress: "10.0.0.1",
 					},
 				},
@@ -250,11 +248,56 @@ func TestSetupRoutingForMultitenancy(t *testing.T) {
 				},
 			},
 		},
+		{
+			name: "does not add default route when SkipDefaultRoutes=true",
+			args: args{
+				nwCfg: &cni.NetworkConfig{
+					MultiTenancy:     true,
+					EnableSnatOnHost: false,
+				},
+				cnsNetworkConfig: &cns.GetNetworkContainerResponse{
+					IPConfiguration: cns.IPConfiguration{
+						GatewayIPAddress: "10.0.0.1",
+					},
+				},
+				epInfo: &network.EndpointInfo{
+					SkipDefaultRoutes: true,
+				},
+				result: &network.InterfaceInfo{},
+			},
+			multitenancyClient: &Multitenancy{},
+			expected: args{
+				nwCfg: &cni.NetworkConfig{
+					MultiTenancy:     true,
+					EnableSnatOnHost: false,
+				},
+				cnsNetworkConfig: &cns.GetNetworkContainerResponse{
+					IPConfiguration: cns.IPConfiguration{
+						GatewayIPAddress: "10.0.0.1",
+					},
+				},
+				epInfo: &network.EndpointInfo{
+					SkipDefaultRoutes: true,
+					Routes:            nil, // unchanged
+				},
+				result: &network.InterfaceInfo{
+					Routes: nil, // unchanged
+				},
+			},
+		},
 	}
+
 	for _, tt := range tests {
 		tt := tt
 		t.Run(tt.name, func(t *testing.T) {
-			tt.multitenancyClient.SetupRoutingForMultitenancy(tt.args.nwCfg, tt.args.cnsNetworkConfig, tt.args.azIpamResult, tt.args.epInfo, tt.args.result)
+			tt.multitenancyClient.SetupRoutingForMultitenancy(
+				tt.args.nwCfg,
+				tt.args.cnsNetworkConfig,
+				tt.args.azIpamResult,
+				tt.args.epInfo,
+				tt.args.result,
+			)
+
 			require.Exactly(tt.expected.nwCfg, tt.args.nwCfg)
 			require.Exactly(tt.expected.cnsNetworkConfig, tt.args.cnsNetworkConfig)
 			require.Exactly(tt.expected.azIpamResult, tt.args.azIpamResult)

cni/network/network_windows.go

@@ -132,7 +132,25 @@ func (plugin *NetPlugin) getNetworkName(netNs string, interfaceInfo *network.Int
 func setupInfraVnetRoutingForMultitenancy(
 	_ *cni.NetworkConfig,
 	_ *cniTypesCurr.Result,
-	_ *network.EndpointInfo) {
+	epInfo *network.EndpointInfo,
+) {
+	// as a workaround, CNI needs to pass this dummy default route (0.0.0.0/0, nexthop: 0.0.0.0) to HNS
+	// the only usage for this dummy default is to bypass HNS setting default route, causing duplicated routes issue
+	// TODO: Remove this once HNS fix is ready
+	if !epInfo.SkipDefaultRoutes {
+		return
+	}
+
+	_, defaultIPNet, err := net.ParseCIDR("0.0.0.0/0")
+	if err != nil {
+		return
+	}
+
+	dummyRoute := network.RouteInfo{
+		Dst: *defaultIPNet,
+		Gw:  net.IPv4zero,
+	}
+	epInfo.Routes = append(epInfo.Routes, dummyRoute)
 }
 
 func getNetworkDNSSettings(nwCfg *cni.NetworkConfig, _ network.DNSInfo) (network.DNSInfo, error) {

cni/network/network_windows_test.go

@@ -1381,3 +1381,68 @@ func TestPluginWindowsAdd(t *testing.T) {
 		})
 	}
 }
+
+// When SkipDefaultRoutes == false => Function returns early. Route list is unchanged (still 1 route, and no dummy 0.0.0.0/0 via 0.0.0.0).
+// When SkipDefaultRoutes == true, the Function appends a new route. So the total routes becomes 2.
+// One of the routes is exactly Dst=0.0.0.0/0, Gw=0.0.0.0. The original route (1.1.1.1/24 via 10.0.0.1) is preserved.
+func TestSetupInfraVnetRoutingForMultitenancy(t *testing.T) {
+	tests := []struct {
+		name             string
+		epInfo           *network.EndpointInfo
+		wantRouteCount   int
+		expectDummyRoute bool
+	}{
+		{
+			name: "SkipDefaultRoutes=false — do not add dummy default route",
+			epInfo: &network.EndpointInfo{
+				SkipDefaultRoutes: false,
+				Routes: []network.RouteInfo{
+					{
+						Dst: *getCIDRNotationForAddress("1.1.1.1/24"),
+						Gw:  net.ParseIP("10.0.0.1"),
+					},
+				},
+			},
+			wantRouteCount:   1,
+			expectDummyRoute: false,
+		},
+		{
+			name: "SkipDefaultRoutes=true — append dummy default route 0.0.0.0/0 via 0.0.0.0",
+			epInfo: &network.EndpointInfo{
+				SkipDefaultRoutes: true,
+				Routes: []network.RouteInfo{
+					{
+						Dst: *getCIDRNotationForAddress("1.1.1.1/24"),
+						Gw:  net.ParseIP("10.0.0.1"),
+					},
+				},
+			},
+			wantRouteCount:   2,
+			expectDummyRoute: true,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			setupInfraVnetRoutingForMultitenancy(nil, nil, tt.epInfo)
+
+			require.Len(t, tt.epInfo.Routes, tt.wantRouteCount)
+
+			require.True(t, tt.epInfo.Routes[0].Gw.Equal(net.ParseIP("10.0.0.1")))
+
+			hasDummy := false
+			for _, r := range tt.epInfo.Routes {
+				if r.Gw.Equal(net.IPv4zero) && r.Dst.String() == "0.0.0.0/0" {
+					hasDummy = true
+					break
+				}
+			}
+
+			if tt.expectDummyRoute {
+				require.True(t, hasDummy, "expected dummy default route 0.0.0.0/0 via 0.0.0.0 to be added")
+			} else {
+				require.False(t, hasDummy, "did not expect dummy default route to be added")
+			}
+		})
+	}
+}

cns/hnsclient/hnsclient_windows.go

@@ -270,6 +270,7 @@ func configureHostNCApipaNetwork(localIPConfiguration cns.IPConfiguration) (*hcn
 			{
 				NextHop:           localIPConfiguration.GatewayIPAddress,
 				DestinationPrefix: "0.0.0.0/0",
+				Metric:            50,
 			},
 		},
 	}
@@ -540,7 +541,7 @@ func configureHostNCApipaEndpoint(
 	// keep Apipa Endpoint gw as 169.254.128.1 to make sure NC to host connectivity work for both Linux and Windows containers
 	hcnRoute := hcn.Route{
 		NextHop:           hnsLoopbackAdapterIPAddress,
-		DestinationPrefix: "0.0.0.0/0",
+		DestinationPrefix: "169.254.128.0/17",
 	}
 
 	endpoint.Routes = append(endpoint.Routes, hcnRoute)