From 1e1b2692909c347befb488009318dfa9f11c9d57 Mon Sep 17 00:00:00 2001 From: paulyufan2 Date: Fri, 16 Aug 2024 16:17:15 -0400 Subject: [PATCH 01/14] fix: dualnic options --- cni/network/invoker.go | 9 +++++++++ cni/network/network.go | 3 ++- 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/cni/network/invoker.go b/cni/network/invoker.go index 907aeca5d7..b649eab3a0 100644 --- a/cni/network/invoker.go +++ b/cni/network/invoker.go @@ -31,6 +31,15 @@ type IPAMAddResult struct { ipv6Enabled bool } +func (ipamAddConfig IPAMAddConfig) getOptions() *map[string]interface{} { + res := map[string]interface{}{} + for k, v := range ipamAddConfig.options { + // only support permittive type + res[k] = v + } + return &res +} + func (ipamAddResult IPAMAddResult) PrettyString() string { pStr := "InterfaceInfo: " for key := range ipamAddResult.interfaceInfo { diff --git a/cni/network/network.go b/cni/network/network.go index dc9a2a127d..cb21549074 100644 --- a/cni/network/network.go +++ b/cni/network/network.go @@ -764,6 +764,7 @@ func (plugin *NetPlugin) createEpInfo(opt *createEpInfoOpt) (*network.EndpointIn endpointID = plugin.nm.GetEndpointID(opt.args.ContainerID, ifName) } + option := opt.ipamAddConfig.getOptions() endpointInfo := network.EndpointInfo{ NetworkID: opt.networkID, Mode: opt.ipamAddConfig.nwCfg.Mode, @@ -772,7 +773,7 @@ func (plugin *NetPlugin) createEpInfo(opt *createEpInfoOpt) (*network.EndpointIn BridgeName: opt.ipamAddConfig.nwCfg.Bridge, NetworkPolicies: networkPolicies, // nw and ep policies separated to avoid possible conflicts NetNs: opt.ipamAddConfig.args.Netns, - Options: opt.ipamAddConfig.options, + Options: *option, DisableHairpinOnHostInterface: opt.ipamAddConfig.nwCfg.DisableHairpinOnHostInterface, IsIPv6Enabled: opt.ipv6Enabled, // present infra only From 31c9a1a5a398b86eacd2a1dd78895920463b6112 Mon Sep 17 00:00:00 2001 From: paulyufan2 Date: Fri, 16 Aug 2024 16:45:11 -0400 Subject: [PATCH 02/14] fix comments --- cni/network/invoker.go | 19 ++++++++++--------- cni/network/network.go | 3 +-- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/cni/network/invoker.go b/cni/network/invoker.go index b649eab3a0..9e766d020c 100644 --- a/cni/network/invoker.go +++ b/cni/network/invoker.go @@ -31,15 +31,6 @@ type IPAMAddResult struct { ipv6Enabled bool } -func (ipamAddConfig IPAMAddConfig) getOptions() *map[string]interface{} { - res := map[string]interface{}{} - for k, v := range ipamAddConfig.options { - // only support permittive type - res[k] = v - } - return &res -} - func (ipamAddResult IPAMAddResult) PrettyString() string { pStr := "InterfaceInfo: " for key := range ipamAddResult.interfaceInfo { @@ -48,3 +39,13 @@ func (ipamAddResult IPAMAddResult) PrettyString() string { } return pStr } + +// shallow copy options from one map to a new options map +func (ipamAddConfig IPAMAddConfig) shallowCopyIpamAddConfigOptions() map[string]interface{} { + res := map[string]interface{}{} + for k, v := range ipamAddConfig.options { + // only support primitive type + res[k] = v + } + return res +} diff --git a/cni/network/network.go b/cni/network/network.go index cb21549074..a9c022c425 100644 --- a/cni/network/network.go +++ b/cni/network/network.go @@ -764,7 +764,6 @@ func (plugin *NetPlugin) createEpInfo(opt *createEpInfoOpt) (*network.EndpointIn endpointID = plugin.nm.GetEndpointID(opt.args.ContainerID, ifName) } - option := opt.ipamAddConfig.getOptions() endpointInfo := network.EndpointInfo{ NetworkID: opt.networkID, Mode: opt.ipamAddConfig.nwCfg.Mode, @@ -773,7 +772,7 @@ func (plugin *NetPlugin) createEpInfo(opt *createEpInfoOpt) (*network.EndpointIn BridgeName: opt.ipamAddConfig.nwCfg.Bridge, NetworkPolicies: networkPolicies, // nw and ep policies separated to avoid possible conflicts NetNs: opt.ipamAddConfig.args.Netns, - Options: *option, + Options: opt.ipamAddConfig.shallowCopyIpamAddConfigOptions(), DisableHairpinOnHostInterface: opt.ipamAddConfig.nwCfg.DisableHairpinOnHostInterface, IsIPv6Enabled: opt.ipv6Enabled, // present infra only From a722a56e11b80cd9a9b0ed624d92c0108d7427bf Mon Sep 17 00:00:00 2001 From: paulyufan2 Date: Fri, 16 Aug 2024 17:52:15 -0400 Subject: [PATCH 03/14] add log --- cni/network/multitenancy_test.go | 16 ++++++++++++++++ cni/network/network_windows.go | 1 + 2 files changed, 17 insertions(+) diff --git a/cni/network/multitenancy_test.go b/cni/network/multitenancy_test.go index 9e43535484..f3b98273fc 100644 --- a/cni/network/multitenancy_test.go +++ b/cni/network/multitenancy_test.go @@ -346,6 +346,10 @@ func TestGetMultiTenancyCNIResult(t *testing.T) { GatewayIPAddress: "10.1.0.1", }, }, + MultiTenancyInfo: cns.MultiTenancyInfo{ + EncapType: "1", // vlanID 1 + ID: 1, + }, } ncResponseTwo := cns.GetNetworkContainerResponse{ @@ -377,6 +381,10 @@ func TestGetMultiTenancyCNIResult(t *testing.T) { GatewayIPAddress: "20.1.0.1", }, }, + MultiTenancyInfo: cns.MultiTenancyInfo{ + EncapType: "2", // vlanID 2 + ID: 2, + }, } ncResponses = append(ncResponses, ncResponseOne, ncResponseTwo) @@ -484,6 +492,10 @@ func TestGetMultiTenancyCNIResult(t *testing.T) { GatewayIPAddress: "10.1.0.1", }, }, + MultiTenancyInfo: cns.MultiTenancyInfo{ + EncapType: "1", + ID: 1, + }, }, want2: &cns.GetNetworkContainerResponse{ PrimaryInterfaceIdentifier: "20.0.0.0/16", @@ -514,6 +526,10 @@ func TestGetMultiTenancyCNIResult(t *testing.T) { GatewayIPAddress: "20.1.0.1", }, }, + MultiTenancyInfo: cns.MultiTenancyInfo{ + EncapType: "2", + ID: 2, + }, }, want3: *getCIDRNotationForAddress("10.0.0.0/16"), want4: &cniTypesCurr.Result{ diff --git a/cni/network/network_windows.go b/cni/network/network_windows.go index d78865e5ac..e189f8feee 100644 --- a/cni/network/network_windows.go +++ b/cni/network/network_windows.go @@ -43,6 +43,7 @@ func setNetworkOptions(cnsNwConfig *cns.GetNetworkContainerResponse, nwInfo *net logger.Info("Setting Network Options") vlanMap := make(map[string]interface{}) vlanMap[network.VlanIDKey] = strconv.Itoa(cnsNwConfig.MultiTenancyInfo.ID) + logger.Info("Add %s with %d to", zap.String("vlanIDKey", network.VlanIDKey), zap.Int("vlanID", cnsNwConfig.MultiTenancyInfo.ID), zap.Any("vlanMap", vlanMap)) nwInfo.Options[dockerNetworkOption] = vlanMap } } From d71257259a508de0300e5cd576427fded788425d Mon Sep 17 00:00:00 2001 From: paulyufan2 Date: Mon, 19 Aug 2024 11:34:37 -0400 Subject: [PATCH 04/14] add ut for ShallowCopyIpamAddConfigOptions --- cni/network/invoker_cns_test.go | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/cni/network/invoker_cns_test.go b/cni/network/invoker_cns_test.go index 8a75f645be..8b35f11ef5 100644 --- a/cni/network/invoker_cns_test.go +++ b/cni/network/invoker_cns_test.go @@ -2094,3 +2094,20 @@ func TestCNSIPAMInvoker_Add_SwiftV2(t *testing.T) { }) } } + +func TestShallowCopyIpamAddConfigOptions(t *testing.T) { + opts := IPAMAddConfig{ + // mock all optios' fields + options: map[string]interface{}{ + network.SNATIPKey: "10", + dockerNetworkOption: "20", + "intType": 10, + "floatType": 0.51, + "byteType": byte('A'), + }, + } + + // shallow copy all ipamAddConfig options + res := opts.shallowCopyIpamAddConfigOptions() + require.Equal(t, opts.options, res) +} From f6943a23221ba3c38b0d98eb0119b3f205e64b21 Mon Sep 17 00:00:00 2001 From: paulyufan2 Date: Mon, 19 Aug 2024 11:40:11 -0400 Subject: [PATCH 05/14] add log to setNetworkOptions in Linux --- cni/network/invoker_cns_test.go | 2 +- cni/network/network_linux.go | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/cni/network/invoker_cns_test.go b/cni/network/invoker_cns_test.go index 8b35f11ef5..ba1d023afd 100644 --- a/cni/network/invoker_cns_test.go +++ b/cni/network/invoker_cns_test.go @@ -2097,7 +2097,7 @@ func TestCNSIPAMInvoker_Add_SwiftV2(t *testing.T) { func TestShallowCopyIpamAddConfigOptions(t *testing.T) { opts := IPAMAddConfig{ - // mock all optios' fields + // mock different types of map value options: map[string]interface{}{ network.SNATIPKey: "10", dockerNetworkOption: "20", diff --git a/cni/network/network_linux.go b/cni/network/network_linux.go index 95c145e42b..ca0c098494 100644 --- a/cni/network/network_linux.go +++ b/cni/network/network_linux.go @@ -9,6 +9,7 @@ import ( "github.com/Azure/azure-container-networking/network" "github.com/Azure/azure-container-networking/network/policy" cniTypesCurr "github.com/containernetworking/cni/pkg/types/100" + "go.uber.org/zap" ) const ( @@ -40,6 +41,8 @@ func setNetworkOptions(cnsNwConfig *cns.GetNetworkContainerResponse, nwInfo *net vlanMap := make(map[string]interface{}) vlanMap[network.VlanIDKey] = strconv.Itoa(cnsNwConfig.MultiTenancyInfo.ID) vlanMap[network.SnatBridgeIPKey] = cnsNwConfig.LocalIPConfiguration.GatewayIPAddress + "/" + strconv.Itoa(int(cnsNwConfig.LocalIPConfiguration.IPSubnet.PrefixLength)) + logger.Info("Add %s and %s with %d to", zap.String("vlanIDKey", network.VlanIDKey), zap.String("SnatBridgeIPKey", network.SnatBridgeIPKey), + zap.Int("vlanID", cnsNwConfig.MultiTenancyInfo.ID), zap.Any("vlanMap", vlanMap)) nwInfo.Options[dockerNetworkOption] = vlanMap } } From 035ce31e0357c44fd2c60f28a7f069486ff660c7 Mon Sep 17 00:00:00 2001 From: paulyufan2 Date: Mon, 19 Aug 2024 16:06:06 -0400 Subject: [PATCH 06/14] add interfaceInfo print back --- cni/network/network.go | 1 + 1 file changed, 1 insertion(+) diff --git a/cni/network/network.go b/cni/network/network.go index a9c022c425..02a60515b0 100644 --- a/cni/network/network.go +++ b/cni/network/network.go @@ -435,6 +435,7 @@ func (plugin *NetPlugin) Add(args *cniSkel.CmdArgs) error { // previously we had a default interface info to select which interface info was the one to be returned from cni add cniResult := &cniTypesCurr.Result{} for key := range ipamAddResult.interfaceInfo { + logger.Info("Exiting add, interface info retrieved", zap.Any("ifInfo", ipamAddResult.interfaceInfo[key])) // now we have to infer which interface info should be returned // we assume that we want to return the infra nic always, and if that is not found, return any one of the secondary interfaces // if there is an infra nic + secondary, we will always return the infra nic (linux swift v2) From f27c4debf6c2830ab3fc1323dd4b35433194853c Mon Sep 17 00:00:00 2001 From: paulyufan2 Date: Tue, 20 Aug 2024 18:29:21 -0400 Subject: [PATCH 07/14] fix zap log --- cni/network/network_linux.go | 2 +- cni/network/network_windows.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/cni/network/network_linux.go b/cni/network/network_linux.go index ca0c098494..1c4900e96f 100644 --- a/cni/network/network_linux.go +++ b/cni/network/network_linux.go @@ -41,7 +41,7 @@ func setNetworkOptions(cnsNwConfig *cns.GetNetworkContainerResponse, nwInfo *net vlanMap := make(map[string]interface{}) vlanMap[network.VlanIDKey] = strconv.Itoa(cnsNwConfig.MultiTenancyInfo.ID) vlanMap[network.SnatBridgeIPKey] = cnsNwConfig.LocalIPConfiguration.GatewayIPAddress + "/" + strconv.Itoa(int(cnsNwConfig.LocalIPConfiguration.IPSubnet.PrefixLength)) - logger.Info("Add %s and %s with %d to", zap.String("vlanIDKey", network.VlanIDKey), zap.String("SnatBridgeIPKey", network.SnatBridgeIPKey), + logger.Info("Add vlanIDkey and SnatBridgeIPKey with vlanID to vlanMap", zap.String("vlanIDKey", network.VlanIDKey), zap.String("SnatBridgeIPKey", network.SnatBridgeIPKey), zap.Int("vlanID", cnsNwConfig.MultiTenancyInfo.ID), zap.Any("vlanMap", vlanMap)) nwInfo.Options[dockerNetworkOption] = vlanMap } diff --git a/cni/network/network_windows.go b/cni/network/network_windows.go index e189f8feee..c2b1c23740 100644 --- a/cni/network/network_windows.go +++ b/cni/network/network_windows.go @@ -43,7 +43,7 @@ func setNetworkOptions(cnsNwConfig *cns.GetNetworkContainerResponse, nwInfo *net logger.Info("Setting Network Options") vlanMap := make(map[string]interface{}) vlanMap[network.VlanIDKey] = strconv.Itoa(cnsNwConfig.MultiTenancyInfo.ID) - logger.Info("Add %s with %d to", zap.String("vlanIDKey", network.VlanIDKey), zap.Int("vlanID", cnsNwConfig.MultiTenancyInfo.ID), zap.Any("vlanMap", vlanMap)) + logger.Info("Add vlanIDKey with vlanID to vlanMap", zap.String("vlanIDKey", network.VlanIDKey), zap.Int("vlanID", cnsNwConfig.MultiTenancyInfo.ID), zap.Any("vlanMap", vlanMap)) nwInfo.Options[dockerNetworkOption] = vlanMap } } From 705ca6a16d255e8da33675aa982f82414902229a Mon Sep 17 00:00:00 2001 From: paulyufan2 Date: Wed, 21 Aug 2024 14:25:23 -0400 Subject: [PATCH 08/14] comment fix --- cni/network/invoker_cns_test.go | 18 ++++++++++++++++-- cni/network/network.go | 2 +- 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/cni/network/invoker_cns_test.go b/cni/network/invoker_cns_test.go index ba1d023afd..c2c3b62334 100644 --- a/cni/network/invoker_cns_test.go +++ b/cni/network/invoker_cns_test.go @@ -2108,6 +2108,20 @@ func TestShallowCopyIpamAddConfigOptions(t *testing.T) { } // shallow copy all ipamAddConfig options - res := opts.shallowCopyIpamAddConfigOptions() - require.Equal(t, opts.options, res) + res1 := opts.shallowCopyIpamAddConfigOptions() + require.Equal(t, opts.options, res1) + + // create a second opts for reference representing expected outputs + copyRes := res1 + // modify res1 fields and make sure expectedOpts has not changed + res1[network.SNATIPKey] = "100" + res1[dockerNetworkOption] = "200" + require.Equal(t, copyRes, res1) + + // shallow copy empty opts and make sure it does not break anything + emptyOpts := IPAMAddConfig{ + options: map[string]interface{}{}, + } + res3 := emptyOpts.shallowCopyIpamAddConfigOptions() + require.Equal(t, emptyOpts.options, res3) } diff --git a/cni/network/network.go b/cni/network/network.go index 02a60515b0..cf6955d348 100644 --- a/cni/network/network.go +++ b/cni/network/network.go @@ -435,7 +435,6 @@ func (plugin *NetPlugin) Add(args *cniSkel.CmdArgs) error { // previously we had a default interface info to select which interface info was the one to be returned from cni add cniResult := &cniTypesCurr.Result{} for key := range ipamAddResult.interfaceInfo { - logger.Info("Exiting add, interface info retrieved", zap.Any("ifInfo", ipamAddResult.interfaceInfo[key])) // now we have to infer which interface info should be returned // we assume that we want to return the infra nic always, and if that is not found, return any one of the secondary interfaces // if there is an infra nic + secondary, we will always return the infra nic (linux swift v2) @@ -609,6 +608,7 @@ func (plugin *NetPlugin) Add(args *cniSkel.CmdArgs) error { endpointIndex := 1 for key := range ipamAddResult.interfaceInfo { ifInfo := ipamAddResult.interfaceInfo[key] + logger.Info("retrieving interfaceInfo:", zap.Any("ifInfo", ifInfo)) natInfo := getNATInfo(nwCfg, options[network.SNATIPKey], enableSnatForDNS) networkID, _ := plugin.getNetworkID(args.Netns, &ifInfo, nwCfg) From b6aee1d3cc75ecb8ddce957a01d7913987fd1ff2 Mon Sep 17 00:00:00 2001 From: paulyufan2 Date: Wed, 21 Aug 2024 14:27:08 -0400 Subject: [PATCH 09/14] fix UT variables --- cni/network/invoker_cns_test.go | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/cni/network/invoker_cns_test.go b/cni/network/invoker_cns_test.go index c2c3b62334..3d373b0c57 100644 --- a/cni/network/invoker_cns_test.go +++ b/cni/network/invoker_cns_test.go @@ -2108,20 +2108,20 @@ func TestShallowCopyIpamAddConfigOptions(t *testing.T) { } // shallow copy all ipamAddConfig options - res1 := opts.shallowCopyIpamAddConfigOptions() - require.Equal(t, opts.options, res1) + res := opts.shallowCopyIpamAddConfigOptions() + require.Equal(t, opts.options, res) // create a second opts for reference representing expected outputs - copyRes := res1 - // modify res1 fields and make sure expectedOpts has not changed - res1[network.SNATIPKey] = "100" - res1[dockerNetworkOption] = "200" - require.Equal(t, copyRes, res1) + copyRes := res + // modify res1 fields and make sure copied res has changed + res[network.SNATIPKey] = "100" + res[dockerNetworkOption] = "200" + require.Equal(t, copyRes, res) // shallow copy empty opts and make sure it does not break anything emptyOpts := IPAMAddConfig{ options: map[string]interface{}{}, } - res3 := emptyOpts.shallowCopyIpamAddConfigOptions() - require.Equal(t, emptyOpts.options, res3) + emptyRes := emptyOpts.shallowCopyIpamAddConfigOptions() + require.Equal(t, emptyOpts.options, emptyRes) } From 0dbcc2bf8b7531a40d94b42a579af01330fb8eea Mon Sep 17 00:00:00 2001 From: paulyufan2 Date: Wed, 21 Aug 2024 14:40:27 -0400 Subject: [PATCH 10/14] print options in endpoint creation --- network/endpoint.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/network/endpoint.go b/network/endpoint.go index f9f59e72f7..c6daf4701f 100644 --- a/network/endpoint.go +++ b/network/endpoint.go @@ -151,9 +151,9 @@ type apipaClient interface { } func (epInfo *EndpointInfo) PrettyString() string { - return fmt.Sprintf("Id:%s ContainerID:%s NetNsPath:%s IfName:%s IfIndex:%d MacAddr:%s IPAddrs:%v Gateways:%v Data:%+v NICType: %s NetworkContainerID: %s HostIfName: %s NetNs: %s", + return fmt.Sprintf("Id:%s ContainerID:%s NetNsPath:%s IfName:%s IfIndex:%d MacAddr:%s IPAddrs:%v Gateways:%v Data:%+v NICType: %s NetworkContainerID: %s HostIfName: %s NetNs: %s Options: %v", epInfo.EndpointID, epInfo.ContainerID, epInfo.NetNsPath, epInfo.IfName, epInfo.IfIndex, epInfo.MacAddress.String(), epInfo.IPAddresses, - epInfo.Gateways, epInfo.Data, epInfo.NICType, epInfo.NetworkContainerID, epInfo.HostIfName, epInfo.NetNs) + epInfo.Gateways, epInfo.Data, epInfo.NICType, epInfo.NetworkContainerID, epInfo.HostIfName, epInfo.NetNs, epInfo.Options) } func (ifInfo *InterfaceInfo) PrettyString() string { From 0e140520cd0412d2a47dd730af9261e68c09fc07 Mon Sep 17 00:00:00 2001 From: paulyufan2 Date: Wed, 21 Aug 2024 16:21:50 -0400 Subject: [PATCH 11/14] fix UTs --- cni/network/invoker_cns_test.go | 23 +++++++++++++++++++---- cni/network/network.go | 2 +- 2 files changed, 20 insertions(+), 5 deletions(-) diff --git a/cni/network/invoker_cns_test.go b/cni/network/invoker_cns_test.go index 3d373b0c57..3545363cfd 100644 --- a/cni/network/invoker_cns_test.go +++ b/cni/network/invoker_cns_test.go @@ -2111,12 +2111,20 @@ func TestShallowCopyIpamAddConfigOptions(t *testing.T) { res := opts.shallowCopyIpamAddConfigOptions() require.Equal(t, opts.options, res) - // create a second opts for reference representing expected outputs - copyRes := res - // modify res1 fields and make sure copied res has changed + // modified copied res and make sure original opts is not changed res[network.SNATIPKey] = "100" res[dockerNetworkOption] = "200" - require.Equal(t, copyRes, res) + require.NotEqual(t, opts.options, res) + + // make sure original object is equal to expected opts after copied res is changed + expectedOpts := map[string]interface{}{ + network.SNATIPKey: "10", + dockerNetworkOption: "20", + "intType": 10, + "floatType": 0.51, + "byteType": byte('A'), + } + require.Equal(t, opts.options, expectedOpts) // shallow copy empty opts and make sure it does not break anything emptyOpts := IPAMAddConfig{ @@ -2124,4 +2132,11 @@ func TestShallowCopyIpamAddConfigOptions(t *testing.T) { } emptyRes := emptyOpts.shallowCopyIpamAddConfigOptions() require.Equal(t, emptyOpts.options, emptyRes) + + // shallow copy null opts and make sure it does not break anything + nullOpts := IPAMAddConfig{ + options: nil, + } + nullRes := nullOpts.shallowCopyIpamAddConfigOptions() + require.Equal(t, map[string]interface{}{}, nullRes) } diff --git a/cni/network/network.go b/cni/network/network.go index cf6955d348..0404604e60 100644 --- a/cni/network/network.go +++ b/cni/network/network.go @@ -608,7 +608,7 @@ func (plugin *NetPlugin) Add(args *cniSkel.CmdArgs) error { endpointIndex := 1 for key := range ipamAddResult.interfaceInfo { ifInfo := ipamAddResult.interfaceInfo[key] - logger.Info("retrieving interfaceInfo:", zap.Any("ifInfo", ifInfo)) + logger.Info("Processing interfaceInfo:", zap.Any("ifInfo", ifInfo)) natInfo := getNATInfo(nwCfg, options[network.SNATIPKey], enableSnatForDNS) networkID, _ := plugin.getNetworkID(args.Netns, &ifInfo, nwCfg) From e01f64bfa98e79e4e9b1ac5723a335038a35a5df Mon Sep 17 00:00:00 2001 From: paulyufan2 Date: Wed, 21 Aug 2024 16:50:46 -0400 Subject: [PATCH 12/14] add one more UT --- cni/network/invoker_cns_test.go | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/cni/network/invoker_cns_test.go b/cni/network/invoker_cns_test.go index 3545363cfd..d9f016f403 100644 --- a/cni/network/invoker_cns_test.go +++ b/cni/network/invoker_cns_test.go @@ -2112,19 +2112,30 @@ func TestShallowCopyIpamAddConfigOptions(t *testing.T) { require.Equal(t, opts.options, res) // modified copied res and make sure original opts is not changed - res[network.SNATIPKey] = "100" - res[dockerNetworkOption] = "200" - require.NotEqual(t, opts.options, res) + newSnapIPKeyValue := "100" + newDockerNetworkOptionValue := "200" + + res[network.SNATIPKey] = newSnapIPKeyValue + res[dockerNetworkOption] = newDockerNetworkOptionValue - // make sure original object is equal to expected opts after copied res is changed expectedOpts := map[string]interface{}{ + network.SNATIPKey: newSnapIPKeyValue, + dockerNetworkOption: newDockerNetworkOptionValue, + "intType": 10, + "floatType": 0.51, + "byteType": byte('A'), + } + require.Equal(t, expectedOpts, res) + + // make sure original object is equal to expected opts after copied res is changed + expectedOriginalOpts := map[string]interface{}{ network.SNATIPKey: "10", dockerNetworkOption: "20", "intType": 10, "floatType": 0.51, "byteType": byte('A'), } - require.Equal(t, opts.options, expectedOpts) + require.Equal(t, expectedOriginalOpts, opts.options) // shallow copy empty opts and make sure it does not break anything emptyOpts := IPAMAddConfig{ From 2c70824d6ee431341b6e4cd44e7709e2f592c9cc Mon Sep 17 00:00:00 2001 From: paulyufan2 Date: Wed, 21 Aug 2024 17:18:38 -0400 Subject: [PATCH 13/14] minor: change the newSNATIPKeyValue name --- cni/network/invoker_cns_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/cni/network/invoker_cns_test.go b/cni/network/invoker_cns_test.go index d9f016f403..82cdb0a9e0 100644 --- a/cni/network/invoker_cns_test.go +++ b/cni/network/invoker_cns_test.go @@ -2112,14 +2112,14 @@ func TestShallowCopyIpamAddConfigOptions(t *testing.T) { require.Equal(t, opts.options, res) // modified copied res and make sure original opts is not changed - newSnapIPKeyValue := "100" + newSNATIPKeyValue := "100" newDockerNetworkOptionValue := "200" - res[network.SNATIPKey] = newSnapIPKeyValue + res[network.SNATIPKey] = newSNATIPKeyValue res[dockerNetworkOption] = newDockerNetworkOptionValue expectedOpts := map[string]interface{}{ - network.SNATIPKey: newSnapIPKeyValue, + network.SNATIPKey: newSNATIPKeyValue, dockerNetworkOption: newDockerNetworkOptionValue, "intType": 10, "floatType": 0.51, From 1edf180a086af17bc791213552f55f9b2ac43819 Mon Sep 17 00:00:00 2001 From: paulyufan2 Date: Wed, 21 Aug 2024 18:08:15 -0400 Subject: [PATCH 14/14] enhance log line --- cni/network/network_linux.go | 11 +++++------ cni/network/network_windows.go | 8 ++++---- 2 files changed, 9 insertions(+), 10 deletions(-) diff --git a/cni/network/network_linux.go b/cni/network/network_linux.go index 1c4900e96f..34ffc07042 100644 --- a/cni/network/network_linux.go +++ b/cni/network/network_linux.go @@ -38,12 +38,11 @@ func addSnatForDNS(gwIPString string, epInfo *network.EndpointInfo, result *netw func setNetworkOptions(cnsNwConfig *cns.GetNetworkContainerResponse, nwInfo *network.EndpointInfo) { if cnsNwConfig != nil && cnsNwConfig.MultiTenancyInfo.ID != 0 { logger.Info("Setting Network Options") - vlanMap := make(map[string]interface{}) - vlanMap[network.VlanIDKey] = strconv.Itoa(cnsNwConfig.MultiTenancyInfo.ID) - vlanMap[network.SnatBridgeIPKey] = cnsNwConfig.LocalIPConfiguration.GatewayIPAddress + "/" + strconv.Itoa(int(cnsNwConfig.LocalIPConfiguration.IPSubnet.PrefixLength)) - logger.Info("Add vlanIDkey and SnatBridgeIPKey with vlanID to vlanMap", zap.String("vlanIDKey", network.VlanIDKey), zap.String("SnatBridgeIPKey", network.SnatBridgeIPKey), - zap.Int("vlanID", cnsNwConfig.MultiTenancyInfo.ID), zap.Any("vlanMap", vlanMap)) - nwInfo.Options[dockerNetworkOption] = vlanMap + optionsMap := make(map[string]interface{}) + optionsMap[network.VlanIDKey] = strconv.Itoa(cnsNwConfig.MultiTenancyInfo.ID) + optionsMap[network.SnatBridgeIPKey] = cnsNwConfig.LocalIPConfiguration.GatewayIPAddress + "/" + strconv.Itoa(int(cnsNwConfig.LocalIPConfiguration.IPSubnet.PrefixLength)) + logger.Info("Add vlanIDkey and SnatBridgeIPKey to optionsMap", zap.String("vlanIDKey", network.VlanIDKey), zap.String("SnatBridgeIPKey", network.SnatBridgeIPKey)) + nwInfo.Options[dockerNetworkOption] = optionsMap } } diff --git a/cni/network/network_windows.go b/cni/network/network_windows.go index c2b1c23740..65d3ccea40 100644 --- a/cni/network/network_windows.go +++ b/cni/network/network_windows.go @@ -41,10 +41,10 @@ func addSnatForDNS(_ string, _ *network.EndpointInfo, _ *network.InterfaceInfo) func setNetworkOptions(cnsNwConfig *cns.GetNetworkContainerResponse, nwInfo *network.EndpointInfo) { if cnsNwConfig != nil && cnsNwConfig.MultiTenancyInfo.ID != 0 { logger.Info("Setting Network Options") - vlanMap := make(map[string]interface{}) - vlanMap[network.VlanIDKey] = strconv.Itoa(cnsNwConfig.MultiTenancyInfo.ID) - logger.Info("Add vlanIDKey with vlanID to vlanMap", zap.String("vlanIDKey", network.VlanIDKey), zap.Int("vlanID", cnsNwConfig.MultiTenancyInfo.ID), zap.Any("vlanMap", vlanMap)) - nwInfo.Options[dockerNetworkOption] = vlanMap + optionsMap := make(map[string]interface{}) + optionsMap[network.VlanIDKey] = strconv.Itoa(cnsNwConfig.MultiTenancyInfo.ID) + logger.Info("Add vlanIDKey to optionsMap", zap.String("vlanIDKey", network.VlanIDKey)) + nwInfo.Options[dockerNetworkOption] = optionsMap } }