diff --git a/cni/network/multitenancy.go b/cni/network/multitenancy.go index 67013863bd..491049fe9d 100644 --- a/cni/network/multitenancy.go +++ b/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,11 +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") diff --git a/cni/network/multitenancy_test.go b/cni/network/multitenancy_test.go index b6dbb9e19a..6b23d84e56 100644 --- a/cni/network/multitenancy_test.go +++ b/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,7 +205,7 @@ 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, @@ -212,14 +213,13 @@ func TestSetupRoutingForMultitenancy(t *testing.T) { }, 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, @@ -227,8 +227,6 @@ func TestSetupRoutingForMultitenancy(t *testing.T) { }, 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)