fix dualnic windows duplicated issue

Author: paulyufan

cni/network/multitenancy.go

@@ -159,6 +159,26 @@ func (m *Multitenancy) DetermineSnatFeatureOnHost(snatFile, nmAgentSupportedApis
 	return snatConfig.EnableSnatForDns, snatConfig.EnableSnatOnHost, nil
 }
 
+// addDefaultRouteToGateway 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,15 +192,17 @@ 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})
-
-		if epInfo.EnableSnatForDns {
-			logger.Info("add SNAT for DNS enabled")
-			addSnatForDNS(cnsNetworkConfig.LocalIPConfiguration.GatewayIPAddress, epInfo, result)
+		// 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),
+				)
+			}
 		}
 	}
 

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)
@@ -861,4 +904,4 @@ func TestGetMultiTenancyCNIResultNotFound(t *testing.T) {
 			}
 		})
 	}
-}
+}