From 1549f980f4e610b46a56172b1a3e7d69a4ee65e0 Mon Sep 17 00:00:00 2001 From: Paul Yu Date: Tue, 16 Sep 2025 14:07:58 -0400 Subject: [PATCH 01/10] only add default route when skipdefaultroutes flag to false in dualnic scenario --- cni/network/multitenancy.go | 39 +++++++++++++++++++++++++++++++------ 1 file changed, 33 insertions(+), 6 deletions(-) diff --git a/cni/network/multitenancy.go b/cni/network/multitenancy.go index 67013863bd..a04daba07a 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) addDefaultRouteToGateway(gwStr string, epInfo *network.EndpointInfo, result *network.InterfaceInfo) error { + gw := net.ParseIP(gwStr) + if gw == nil { + return fmt.Errorf("invalid gateway IP: %s", gwStr) + } + + 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, @@ -170,13 +190,20 @@ func (m *Multitenancy) SetupRoutingForMultitenancy( // if snat enabled, add 169.254.128.1 as default gateway if nwCfg.EnableSnatOnHost { logger.Info("add default route for multitenancy.snat on host enabled") - addDefaultRoute(cnsNetworkConfig.LocalIPConfiguration.GatewayIPAddress, epInfo, result) + m.addDefaultRouteToGateway(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( + 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") From 693133f805febdff73bd100e4e3971a52b784181 Mon Sep 17 00:00:00 2001 From: Paul Yu Date: Tue, 16 Sep 2025 14:22:01 -0400 Subject: [PATCH 02/10] get addDefaultRoute back --- cni/network/multitenancy.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cni/network/multitenancy.go b/cni/network/multitenancy.go index a04daba07a..0e86e08ec7 100644 --- a/cni/network/multitenancy.go +++ b/cni/network/multitenancy.go @@ -190,7 +190,7 @@ func (m *Multitenancy) SetupRoutingForMultitenancy( // if snat enabled, add 169.254.128.1 as default gateway if nwCfg.EnableSnatOnHost { logger.Info("add default route for multitenancy.snat on host enabled") - m.addDefaultRouteToGateway(cnsNetworkConfig.LocalIPConfiguration.GatewayIPAddress, epInfo, result) + addDefaultRoute(cnsNetworkConfig.LocalIPConfiguration.GatewayIPAddress, epInfo, result) } else { // only set default route when skipDefaultRoutes is false to avoid duplicated default routes given to HNS if !epInfo.SkipDefaultRoutes { From ad012c431e8e555de4bceecb3161e2281b9e38f3 Mon Sep 17 00:00:00 2001 From: Paul Yu Date: Tue, 16 Sep 2025 14:56:12 -0400 Subject: [PATCH 03/10] add UT --- cni/network/multitenancy.go | 7 +-- cni/network/multitenancy_test.go | 80 ++++++++++++++++++++++++++++---- 2 files changed, 73 insertions(+), 14 deletions(-) diff --git a/cni/network/multitenancy.go b/cni/network/multitenancy.go index 0e86e08ec7..8674173eef 100644 --- a/cni/network/multitenancy.go +++ b/cni/network/multitenancy.go @@ -162,12 +162,9 @@ func (m *Multitenancy) DetermineSnatFeatureOnHost(snatFile, nmAgentSupportedApis // 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) - if gw == nil { - return fmt.Errorf("invalid gateway IP: %s", gwStr) - } - var dst net.IPNet + + gw := net.ParseIP(gwStr) if gw.To4() != nil { _, defaultIPNet, _ := net.ParseCIDR("0.0.0.0/0") dst = net.IPNet{IP: net.IPv4zero, Mask: defaultIPNet.Mask} diff --git a/cni/network/multitenancy_test.go b/cni/network/multitenancy_test.go index b6dbb9e19a..da1fc12223 100644 --- a/cni/network/multitenancy_test.go +++ b/cni/network/multitenancy_test.go @@ -187,8 +187,28 @@ func getIPNetWithString(ipaddrwithcidr string) *net.IPNet { return ipnet } +package // adjust + +import ( + "net" + "testing" + + cni "github.com/Azure/azure-container-networking/cni" + cniTypesCurr "github.com/containernetworking/cni/pkg/types/100" + "github.com/stretchr/testify/require" + + "github.com/Azure/azure-container-networking/cns" + "github.com/Azure/azure-container-networking/network" +) + +func defaultIPNetV4() *net.IPNet { + _, ipn, _ := net.ParseCIDR("0.0.0.0/0") + return ipn +} + func TestSetupRoutingForMultitenancy(t *testing.T) { require := require.New(t) //nolint:gocritic + type args struct { nwCfg *cni.NetworkConfig cnsNetworkConfig *cns.GetNetworkContainerResponse @@ -204,7 +224,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 +232,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,15 +246,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{ Routes: []network.RouteInfo{ { - Dst: net.IPNet{IP: net.ParseIP("0.0.0.0"), Mask: defaultIPNet().Mask}, + Dst: net.IPNet{IP: net.ParseIP("0.0.0.0"), Mask: defaultIPNetV4().Mask}, Gw: net.ParseIP("10.0.0.1"), }, }, @@ -243,18 +260,63 @@ func TestSetupRoutingForMultitenancy(t *testing.T) { result: &network.InterfaceInfo{ Routes: []network.RouteInfo{ { - Dst: net.IPNet{IP: net.ParseIP("0.0.0.0"), Mask: defaultIPNet().Mask}, + Dst: net.IPNet{IP: net.ParseIP("0.0.0.0"), Mask: defaultIPNetV4().Mask}, Gw: net.ParseIP("10.0.0.1"), }, }, }, }, }, + { + 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) From d6bafee2f1fe86d390819c98f02d7ee773db09e4 Mon Sep 17 00:00:00 2001 From: Paul Yu Date: Tue, 16 Sep 2025 14:58:25 -0400 Subject: [PATCH 04/10] add UT --- cni/network/multitenancy_test.go | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/cni/network/multitenancy_test.go b/cni/network/multitenancy_test.go index da1fc12223..78426f1eac 100644 --- a/cni/network/multitenancy_test.go +++ b/cni/network/multitenancy_test.go @@ -187,20 +187,6 @@ func getIPNetWithString(ipaddrwithcidr string) *net.IPNet { return ipnet } -package // adjust - -import ( - "net" - "testing" - - cni "github.com/Azure/azure-container-networking/cni" - cniTypesCurr "github.com/containernetworking/cni/pkg/types/100" - "github.com/stretchr/testify/require" - - "github.com/Azure/azure-container-networking/cns" - "github.com/Azure/azure-container-networking/network" -) - func defaultIPNetV4() *net.IPNet { _, ipn, _ := net.ParseCIDR("0.0.0.0/0") return ipn From 21846f17d723b0348adf36fee818c45ca235bddc Mon Sep 17 00:00:00 2001 From: Paul Yu Date: Tue, 16 Sep 2025 15:00:47 -0400 Subject: [PATCH 05/10] remove duplicated defaultIPNet() --- cni/network/multitenancy_test.go | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/cni/network/multitenancy_test.go b/cni/network/multitenancy_test.go index 78426f1eac..96d8db0a36 100644 --- a/cni/network/multitenancy_test.go +++ b/cni/network/multitenancy_test.go @@ -187,11 +187,6 @@ func getIPNetWithString(ipaddrwithcidr string) *net.IPNet { return ipnet } -func defaultIPNetV4() *net.IPNet { - _, ipn, _ := net.ParseCIDR("0.0.0.0/0") - return ipn -} - func TestSetupRoutingForMultitenancy(t *testing.T) { require := require.New(t) //nolint:gocritic @@ -238,7 +233,7 @@ func TestSetupRoutingForMultitenancy(t *testing.T) { epInfo: &network.EndpointInfo{ Routes: []network.RouteInfo{ { - Dst: net.IPNet{IP: net.ParseIP("0.0.0.0"), Mask: defaultIPNetV4().Mask}, + Dst: net.IPNet{IP: net.ParseIP("0.0.0.0"), Mask: defaultIPNet().Mask}, Gw: net.ParseIP("10.0.0.1"), }, }, @@ -246,7 +241,7 @@ func TestSetupRoutingForMultitenancy(t *testing.T) { result: &network.InterfaceInfo{ Routes: []network.RouteInfo{ { - Dst: net.IPNet{IP: net.ParseIP("0.0.0.0"), Mask: defaultIPNetV4().Mask}, + Dst: net.IPNet{IP: net.ParseIP("0.0.0.0"), Mask: defaultIPNet().Mask}, Gw: net.ParseIP("10.0.0.1"), }, }, From c6807835cf7b946fe405c923f1da30b9e44afb52 Mon Sep 17 00:00:00 2001 From: Paul Yu Date: Tue, 16 Sep 2025 15:09:05 -0400 Subject: [PATCH 06/10] fix linter issue --- cni/network/multitenancy_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cni/network/multitenancy_test.go b/cni/network/multitenancy_test.go index 96d8db0a36..6b23d84e56 100644 --- a/cni/network/multitenancy_test.go +++ b/cni/network/multitenancy_test.go @@ -216,7 +216,7 @@ func TestSetupRoutingForMultitenancy(t *testing.T) { GatewayIPAddress: "10.0.0.1", }, }, - epInfo: &network.EndpointInfo{}, // SkipDefaultRoutes defaults to false + epInfo: &network.EndpointInfo{}, // SkipDefaultRoutes defaults to false result: &network.InterfaceInfo{}, }, multitenancyClient: &Multitenancy{}, From 957a9464100de0ba0e64bf513af77c20a810e424 Mon Sep 17 00:00:00 2001 From: Paul Yu Date: Tue, 16 Sep 2025 16:09:01 -0400 Subject: [PATCH 07/10] add error return --- cni/network/multitenancy.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/cni/network/multitenancy.go b/cni/network/multitenancy.go index 8674173eef..ef98f4df16 100644 --- a/cni/network/multitenancy.go +++ b/cni/network/multitenancy.go @@ -162,9 +162,12 @@ func (m *Multitenancy) DetermineSnatFeatureOnHost(snatFile, nmAgentSupportedApis // 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 { - var dst net.IPNet - gw := net.ParseIP(gwStr) + if gw == nil { + return fmt.Errorf("invalid gateway IP: %s", gwStr) //nolint:errcheck + } + + 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} From 7560d1502c534e36cbb3ebbcd8fadf78ac23a6ef Mon Sep 17 00:00:00 2001 From: Paul Yu Date: Wed, 17 Sep 2025 12:04:50 -0400 Subject: [PATCH 08/10] fix linter issue --- cni/network/multitenancy.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cni/network/multitenancy.go b/cni/network/multitenancy.go index ef98f4df16..f70191084d 100644 --- a/cni/network/multitenancy.go +++ b/cni/network/multitenancy.go @@ -164,7 +164,7 @@ func (m *Multitenancy) DetermineSnatFeatureOnHost(snatFile, nmAgentSupportedApis func (m *Multitenancy) addDefaultRouteToGateway(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:errcheck + return fmt.Errorf("invalid gateway IP: %s", gwStr) //nolint } var dst net.IPNet From 04e2ba1b6e5dad082ba9d2d1a3d55765d5a28363 Mon Sep 17 00:00:00 2001 From: Paul Yu <129891899+paulyufan2@users.noreply.github.com> Date: Thu, 2 Oct 2025 11:12:54 -0400 Subject: [PATCH 09/10] Update cni/network/multitenancy.go Co-authored-by: tamilmani1989 Signed-off-by: Paul Yu <129891899+paulyufan2@users.noreply.github.com> --- cni/network/multitenancy.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cni/network/multitenancy.go b/cni/network/multitenancy.go index f70191084d..e1eac934fa 100644 --- a/cni/network/multitenancy.go +++ b/cni/network/multitenancy.go @@ -194,7 +194,7 @@ func (m *Multitenancy) SetupRoutingForMultitenancy( } else { // only set default route when skipDefaultRoutes is false to avoid duplicated default routes given to HNS if !epInfo.SkipDefaultRoutes { - if err := m.addDefaultRouteToGateway( + if err := m.addDefaultRoute( cnsNetworkConfig.IPConfiguration.GatewayIPAddress, epInfo, result, ); err != nil { From ea054b9824f2b327fcf1fb611c21cb6ea7df2e86 Mon Sep 17 00:00:00 2001 From: Paul Yu <129891899+paulyufan2@users.noreply.github.com> Date: Thu, 2 Oct 2025 11:13:11 -0400 Subject: [PATCH 10/10] Update cni/network/multitenancy.go Co-authored-by: tamilmani1989 Signed-off-by: Paul Yu <129891899+paulyufan2@users.noreply.github.com> --- cni/network/multitenancy.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cni/network/multitenancy.go b/cni/network/multitenancy.go index e1eac934fa..491049fe9d 100644 --- a/cni/network/multitenancy.go +++ b/cni/network/multitenancy.go @@ -161,7 +161,7 @@ func (m *Multitenancy) DetermineSnatFeatureOnHost(snatFile, nmAgentSupportedApis // 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 { +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