Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 32 additions & 5 deletions cni/network/multitenancy.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) addDefaultRouteToGateway(gwStr string, epInfo *network.EndpointInfo, result *network.InterfaceInfo) error {
gw := net.ParseIP(gwStr)
Comment on lines +164 to +165
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
func (m *Multitenancy) addDefaultRouteToGateway(gwStr string, epInfo *network.EndpointInfo, result *network.InterfaceInfo) error {
gw := net.ParseIP(gwStr)
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,
Expand All @@ -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.addDefaultRouteToGateway(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if err := m.addDefaultRouteToGateway(
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")
Expand Down
57 changes: 50 additions & 7 deletions cni/network/multitenancy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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",
},
},
Expand All @@ -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)
Expand Down
Loading