From 16cd21cdafc1b43a7e839617226a59b36f2fa428 Mon Sep 17 00:00:00 2001 From: paulyu Date: Thu, 20 Nov 2025 16:19:29 +0000 Subject: [PATCH 1/7] fix dualnic windows duplicated issue --- cni/network/multitenancy.go | 40 +++++++++++++++++----- cni/network/multitenancy_test.go | 59 +++++++++++++++++++++++++++----- 2 files changed, 82 insertions(+), 17 deletions(-) diff --git a/cni/network/multitenancy.go b/cni/network/multitenancy.go index e617afb7d6..171a55eac7 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,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), + ) + } } } diff --git a/cni/network/multitenancy_test.go b/cni/network/multitenancy_test.go index b6dbb9e19a..5a2a24479d 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) @@ -861,4 +904,4 @@ func TestGetMultiTenancyCNIResultNotFound(t *testing.T) { } }) } -} +} \ No newline at end of file From 1833f0768d979aaeb82067d81594683f203f5376 Mon Sep 17 00:00:00 2001 From: paulyu Date: Thu, 20 Nov 2025 16:21:04 +0000 Subject: [PATCH 2/7] 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 5a2a24479d..6b23d84e56 100644 --- a/cni/network/multitenancy_test.go +++ b/cni/network/multitenancy_test.go @@ -904,4 +904,4 @@ func TestGetMultiTenancyCNIResultNotFound(t *testing.T) { } }) } -} \ No newline at end of file +} From 8a4521754d442e521687b71a3360446c195cb607 Mon Sep 17 00:00:00 2001 From: Paul Yu <129891899+paulyufan2@users.noreply.github.com> Date: Mon, 24 Nov 2025 12:37:51 -0500 Subject: [PATCH 3/7] Update cni/network/multitenancy.go Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Paul Yu <129891899+paulyufan2@users.noreply.github.com> --- cni/network/multitenancy.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/cni/network/multitenancy.go b/cni/network/multitenancy.go index 171a55eac7..dd45abb104 100644 --- a/cni/network/multitenancy.go +++ b/cni/network/multitenancy.go @@ -204,6 +204,14 @@ func (m *Multitenancy) SetupRoutingForMultitenancy( ) } } + // Restore DNS SNAT routing if enabled + if epInfo.EnableSnatForDns { + if err := m.addSnatForDNS(epInfo, result); err != nil { + logger.Error("failed adding SNAT for DNS", + zap.Error(err), + ) + } + } } setupInfraVnetRoutingForMultitenancy(nwCfg, azIpamResult, epInfo) From a66697f5cabb70547e32ac695bd3b33de8ae0667 Mon Sep 17 00:00:00 2001 From: Paul Yu <129891899+paulyufan2@users.noreply.github.com> Date: Mon, 24 Nov 2025 12:42:34 -0500 Subject: [PATCH 4/7] Update cni/network/multitenancy.go Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> 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 dd45abb104..117e704b06 100644 --- a/cni/network/multitenancy.go +++ b/cni/network/multitenancy.go @@ -159,7 +159,7 @@ func (m *Multitenancy) DetermineSnatFeatureOnHost(snatFile, nmAgentSupportedApis return snatConfig.EnableSnatForDns, snatConfig.EnableSnatOnHost, nil } -// addDefaultRouteToGateway appends a default route +// 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) From 42fcea4e57fec78387d6ddc140186815ea5b6430 Mon Sep 17 00:00:00 2001 From: paulyu Date: Mon, 24 Nov 2025 17:48:55 +0000 Subject: [PATCH 5/7] get original codes back --- cni/network/multitenancy.go | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/cni/network/multitenancy.go b/cni/network/multitenancy.go index 117e704b06..44a14b9189 100644 --- a/cni/network/multitenancy.go +++ b/cni/network/multitenancy.go @@ -204,13 +204,9 @@ func (m *Multitenancy) SetupRoutingForMultitenancy( ) } } - // Restore DNS SNAT routing if enabled if epInfo.EnableSnatForDns { - if err := m.addSnatForDNS(epInfo, result); err != nil { - logger.Error("failed adding SNAT for DNS", - zap.Error(err), - ) - } + logger.Info("add SNAT for DNS enabled") + addSnatForDNS(cnsNetworkConfig.LocalIPConfiguration.GatewayIPAddress, epInfo, result) } } From 502994f6c33ebea5c147bb380c4ccf927346d0fe Mon Sep 17 00:00:00 2001 From: paulyu Date: Mon, 24 Nov 2025 18:34:22 +0000 Subject: [PATCH 6/7] fix a bug --- 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 44a14b9189..7e84129d34 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") - addDefaultRoute(cnsNetworkConfig.LocalIPConfiguration.GatewayIPAddress, epInfo, result) + m.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 56d10bc3e5a6edb82b0dc54bf6d9d163166a1c67 Mon Sep 17 00:00:00 2001 From: paulyu Date: Mon, 24 Nov 2025 18:36:15 +0000 Subject: [PATCH 7/7] remove preivious change --- 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 7e84129d34..44a14b9189 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.addDefaultRoute(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 {