Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: dualnic options to avoid overwritting vlanMaps #2928

Merged
merged 14 commits into from
Aug 22, 2024
10 changes: 10 additions & 0 deletions cni/network/invoker.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,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
}
57 changes: 57 additions & 0 deletions cni/network/invoker_cns_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2094,3 +2094,60 @@ func TestCNSIPAMInvoker_Add_SwiftV2(t *testing.T) {
})
}
}

func TestShallowCopyIpamAddConfigOptions(t *testing.T) {
opts := IPAMAddConfig{
// mock different types of map value
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)
QxBytes marked this conversation as resolved.
Show resolved Hide resolved

// modified copied res and make sure original opts is not changed
newSNATIPKeyValue := "100"
newDockerNetworkOptionValue := "200"

res[network.SNATIPKey] = newSNATIPKeyValue
res[dockerNetworkOption] = newDockerNetworkOptionValue

expectedOpts := map[string]interface{}{
network.SNATIPKey: newSNATIPKeyValue,
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, expectedOriginalOpts, opts.options)

// shallow copy empty opts and make sure it does not break anything
emptyOpts := IPAMAddConfig{
options: map[string]interface{}{},
}
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)
}
16 changes: 16 additions & 0 deletions cni/network/multitenancy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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{
Expand Down
3 changes: 2 additions & 1 deletion cni/network/network.go
Original file line number Diff line number Diff line change
Expand Up @@ -608,6 +608,7 @@ func (plugin *NetPlugin) Add(args *cniSkel.CmdArgs) error {
endpointIndex := 1
for key := range ipamAddResult.interfaceInfo {
ifInfo := ipamAddResult.interfaceInfo[key]
logger.Info("Processing interfaceInfo:", zap.Any("ifInfo", ifInfo))

natInfo := getNATInfo(nwCfg, options[network.SNATIPKey], enableSnatForDNS)
networkID, _ := plugin.getNetworkID(args.Netns, &ifInfo, nwCfg)
Expand Down Expand Up @@ -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: opt.ipamAddConfig.shallowCopyIpamAddConfigOptions(),
DisableHairpinOnHostInterface: opt.ipamAddConfig.nwCfg.DisableHairpinOnHostInterface,
IsIPv6Enabled: opt.ipv6Enabled, // present infra only

Expand Down
10 changes: 6 additions & 4 deletions cni/network/network_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down Expand Up @@ -37,10 +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))
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
}
}

Expand Down
7 changes: 4 additions & 3 deletions cni/network/network_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +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)
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
}
}

Expand Down
4 changes: 2 additions & 2 deletions network/endpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Loading