diff --git a/cni/network/invoker.go b/cni/network/invoker.go index 0d111933f8..453f0a1ca0 100644 --- a/cni/network/invoker.go +++ b/cni/network/invoker.go @@ -27,8 +27,18 @@ type IPAMAddConfig struct { } type IPAMAddResult struct { - ipv4Result *cniTypesCurr.Result - ipv6Result *cniTypesCurr.Result + // Splitting defaultInterfaceInfo from secondaryInterfacesInfo so we don't need to loop for default CNI result every time + defaultInterfaceInfo InterfaceInfo + secondaryInterfacesInfo []InterfaceInfo + // ncResponse is used for Swift 1.0 multitenancy ncResponse *cns.GetNetworkContainerResponse hostSubnetPrefix net.IPNet + ipv6Enabled bool +} + +type InterfaceInfo struct { + ipResult *cniTypesCurr.Result + nicType cns.NICType + macAddress net.HardwareAddr + skipDefaultRoutes bool } diff --git a/cni/network/invoker_azure.go b/cni/network/invoker_azure.go index 9b8303c39a..4e27b0bd46 100644 --- a/cni/network/invoker_azure.go +++ b/cni/network/invoker_azure.go @@ -9,6 +9,7 @@ import ( "github.com/Azure/azure-container-networking/cni" "github.com/Azure/azure-container-networking/cni/log" + "github.com/Azure/azure-container-networking/cns" "github.com/Azure/azure-container-networking/common" "github.com/Azure/azure-container-networking/ipam" "github.com/Azure/azure-container-networking/network" @@ -46,10 +47,7 @@ func NewAzureIpamInvoker(plugin *NetPlugin, nwInfo *network.NetworkInfo) *AzureI } func (invoker *AzureIPAMInvoker) Add(addConfig IPAMAddConfig) (IPAMAddResult, error) { - var ( - addResult = IPAMAddResult{} - err error - ) + addResult := IPAMAddResult{} if addConfig.nwCfg == nil { return addResult, invoker.plugin.Errorf("nil nwCfg passed to CNI ADD, stack: %+v", string(debug.Stack())) @@ -60,23 +58,25 @@ func (invoker *AzureIPAMInvoker) Add(addConfig IPAMAddConfig) (IPAMAddResult, er } // Call into IPAM plugin to allocate an address pool for the network. - addResult.ipv4Result, err = invoker.plugin.DelegateAdd(addConfig.nwCfg.IPAM.Type, addConfig.nwCfg) - + result, err := invoker.plugin.DelegateAdd(addConfig.nwCfg.IPAM.Type, addConfig.nwCfg) if err != nil && strings.Contains(err.Error(), ipam.ErrNoAvailableAddressPools.Error()) { invoker.deleteIpamState() logger.Info("Retry pool allocation after deleting IPAM state") - addResult.ipv4Result, err = invoker.plugin.DelegateAdd(addConfig.nwCfg.IPAM.Type, addConfig.nwCfg) + result, err = invoker.plugin.DelegateAdd(addConfig.nwCfg.IPAM.Type, addConfig.nwCfg) } if err != nil { err = invoker.plugin.Errorf("Failed to allocate pool: %v", err) return addResult, err } + if len(result.IPs) > 0 { + addResult.hostSubnetPrefix = result.IPs[0].Address + } defer func() { if err != nil { - if len(addResult.ipv4Result.IPs) > 0 { - if er := invoker.Delete(&addResult.ipv4Result.IPs[0].Address, addConfig.nwCfg, nil, addConfig.options); er != nil { + if len(addResult.defaultInterfaceInfo.ipResult.IPs) > 0 { + if er := invoker.Delete(&addResult.defaultInterfaceInfo.ipResult.IPs[0].Address, addConfig.nwCfg, nil, addConfig.options); er != nil { err = invoker.plugin.Errorf("Failed to clean up IP's during Delete with error %v, after Add failed with error %w", er, err) } } else { @@ -95,13 +95,18 @@ func (invoker *AzureIPAMInvoker) Add(addConfig IPAMAddConfig) (IPAMAddResult, er nwCfg6.IPAM.Subnet = invoker.nwInfo.Subnets[1].Prefix.String() } - addResult.ipv6Result, err = invoker.plugin.DelegateAdd(nwCfg6.IPAM.Type, &nwCfg6) + var ipv6Result *cniTypesCurr.Result + ipv6Result, err = invoker.plugin.DelegateAdd(nwCfg6.IPAM.Type, &nwCfg6) if err != nil { err = invoker.plugin.Errorf("Failed to allocate v6 pool: %v", err) + } else { + result.IPs = append(result.IPs, ipv6Result.IPs...) + result.Routes = append(result.Routes, ipv6Result.Routes...) + addResult.ipv6Enabled = true } } - addResult.hostSubnetPrefix = addResult.ipv4Result.IPs[0].Address + addResult.defaultInterfaceInfo = InterfaceInfo{ipResult: result, nicType: cns.InfraNIC} return addResult, err } diff --git a/cni/network/invoker_azure_test.go b/cni/network/invoker_azure_test.go index ad9e76b76d..9bd10af61d 100644 --- a/cni/network/invoker_azure_test.go +++ b/cni/network/invoker_azure_test.go @@ -16,16 +16,24 @@ import ( "github.com/stretchr/testify/require" ) +const ( + ipv4cidr = "10.0.0.1/24" + v4NetCidr = "10.0.0.0/24" + ipv4cidr2 = "10.0.0.4/24" + ipv6cidr = "2001:0db8:abcd:0015::0/64" + v6NetCidr = "2001:db8:abcd:0012::0/64" +) + type mockDelegatePlugin struct { add del } type add struct { + resultsIPv4 []*cniTypesCurr.Result + resultsIPv6 []*cniTypesCurr.Result resultsIPv4Index int - resultsIPv4 [](*cniTypesCurr.Result) resultsIPv6Index int - resultsIPv6 [](*cniTypesCurr.Result) errv4 error errv6 error } @@ -82,8 +90,8 @@ func getCIDRNotationForAddress(ipaddresswithcidr string) *net.IPNet { return ipnet } -func getResult(ip string) []*cniTypesCurr.Result { - res := []*cniTypesCurr.Result{ +func getSingleResult(ip string) []*cniTypesCurr.Result { + return []*cniTypesCurr.Result{ { IPs: []*cniTypesCurr.IPConfig{ { @@ -92,6 +100,14 @@ func getResult(ip string) []*cniTypesCurr.Result { }, }, } +} + +// getResult will return a slice of IPConfigs +func getResult(ips ...string) *cniTypesCurr.Result { + res := &cniTypesCurr.Result{} + for _, ip := range ips { + res.IPs = append(res.IPs, &cniTypesCurr.IPConfig{Address: *getCIDRNotationForAddress(ip)}) + } return res } @@ -127,7 +143,6 @@ func TestAzureIPAMInvoker_Add(t *testing.T) { fields fields args args want *cniTypesCurr.Result - want1 *cniTypesCurr.Result wantErr bool }{ { @@ -135,17 +150,17 @@ func TestAzureIPAMInvoker_Add(t *testing.T) { fields: fields{ plugin: &mockDelegatePlugin{ add: add{ - resultsIPv4: getResult("10.0.0.1/24"), + resultsIPv4: getSingleResult(ipv4cidr), }, del: del{}, }, - nwInfo: getNwInfo("10.0.0.0/24", ""), + nwInfo: getNwInfo(v4NetCidr, ""), }, args: args{ nwCfg: &cni.NetworkConfig{}, - subnetPrefix: getCIDRNotationForAddress("10.0.0.0/24"), + subnetPrefix: getCIDRNotationForAddress(v4NetCidr), }, - want: getResult("10.0.0.1/24")[0], + want: getResult(ipv4cidr), wantErr: false, }, { @@ -153,20 +168,19 @@ func TestAzureIPAMInvoker_Add(t *testing.T) { fields: fields{ plugin: &mockDelegatePlugin{ add: add{ - resultsIPv4: getResult("10.0.0.1/24"), - resultsIPv6: getResult("2001:0db8:abcd:0015::0/64"), + resultsIPv4: getSingleResult(ipv4cidr), + resultsIPv6: getSingleResult(ipv6cidr), }, }, - nwInfo: getNwInfo("10.0.0.0/24", "2001:db8:abcd:0012::0/64"), + nwInfo: getNwInfo(v4NetCidr, v6NetCidr), }, args: args{ nwCfg: &cni.NetworkConfig{ IPV6Mode: network.IPV6Nat, }, - subnetPrefix: getCIDRNotationForAddress("10.0.0.0/24"), + subnetPrefix: getCIDRNotationForAddress(v4NetCidr), }, - want: getResult("10.0.0.1/24")[0], - want1: getResult("2001:0db8:abcd:0015::0/64")[0], + want: getResult(ipv4cidr, ipv6cidr), wantErr: false, }, { @@ -177,13 +191,12 @@ func TestAzureIPAMInvoker_Add(t *testing.T) { errv4: errors.New("test error"), //nolint:goerr113 }, }, - nwInfo: getNwInfo("10.0.0.0/24", ""), + nwInfo: getNwInfo(v4NetCidr, ""), }, args: args{ nwCfg: &cni.NetworkConfig{}, }, want: nil, - want1: nil, wantErr: true, }, { @@ -191,20 +204,19 @@ func TestAzureIPAMInvoker_Add(t *testing.T) { fields: fields{ plugin: &mockDelegatePlugin{ add: add{ - resultsIPv4: getResult("10.0.0.1/24"), + resultsIPv4: getSingleResult(ipv4cidr), errv6: errors.New("test v6 error"), //nolint:goerr113 }, }, - nwInfo: getNwInfo("10.0.0.0/24", ""), + nwInfo: getNwInfo(v4NetCidr, ""), }, args: args{ nwCfg: &cni.NetworkConfig{ IPV6Mode: network.IPV6Nat, }, - subnetPrefix: getCIDRNotationForAddress("10.0.0.0/24"), + subnetPrefix: getCIDRNotationForAddress(v4NetCidr), }, - want: getResult("10.0.0.1/24")[0], - want1: nil, + want: getResult(ipv4cidr), wantErr: true, }, } @@ -226,8 +238,8 @@ func TestAzureIPAMInvoker_Add(t *testing.T) { require.Nil(err) } - require.Exactly(tt.want, ipamAddResult.ipv4Result) - require.Exactly(tt.want1, ipamAddResult.ipv6Result) + fmt.Printf("want:%+v\nrest:%+v\n", tt.want, ipamAddResult.defaultInterfaceInfo.ipResult) + require.Exactly(tt.want, ipamAddResult.defaultInterfaceInfo.ipResult) }) } } @@ -256,10 +268,10 @@ func TestAzureIPAMInvoker_Delete(t *testing.T) { plugin: &mockDelegatePlugin{ del: del{}, }, - nwInfo: getNwInfo("10.0.0.0/24", ""), + nwInfo: getNwInfo(v4NetCidr, ""), }, args: args{ - address: getCIDRNotationForAddress("10.0.0.4/24"), + address: getCIDRNotationForAddress(ipv4cidr2), nwCfg: &cni.NetworkConfig{ IPAM: cni.IPAM{ Address: "10.0.0.4", @@ -273,7 +285,7 @@ func TestAzureIPAMInvoker_Delete(t *testing.T) { plugin: &mockDelegatePlugin{ del: del{}, }, - nwInfo: getNwInfo("10.0.0.0/24", "2001:db8:abcd:0012::0/64"), + nwInfo: getNwInfo(v4NetCidr, v6NetCidr), }, args: args{ address: getCIDRNotationForAddress("2001:db8:abcd:0015::0/64"), @@ -292,7 +304,7 @@ func TestAzureIPAMInvoker_Delete(t *testing.T) { err: errors.New("error when address is nil"), //nolint:goerr113 }, }, - nwInfo: getNwInfo("", "2001:db8:abcd:0012::0/64"), + nwInfo: getNwInfo("", v6NetCidr), }, args: args{ address: nil, @@ -312,13 +324,13 @@ func TestAzureIPAMInvoker_Delete(t *testing.T) { err: errors.New("error on v4 delete"), //nolint:goerr113 }, }, - nwInfo: getNwInfo("10.0.0.0/24", ""), + nwInfo: getNwInfo(v4NetCidr, ""), }, args: args{ - address: getCIDRNotationForAddress("10.0.0.4/24"), + address: getCIDRNotationForAddress(ipv4cidr2), nwCfg: &cni.NetworkConfig{ IPAM: cni.IPAM{ - Address: "10.0.0.4/24", + Address: ipv4cidr2, }, }, }, @@ -332,13 +344,13 @@ func TestAzureIPAMInvoker_Delete(t *testing.T) { err: errors.New("error on v6 delete"), //nolint:goerr113 }, }, - nwInfo: getNwInfo("10.0.0.0/24", "2001:db8:abcd:0012::0/64"), + nwInfo: getNwInfo(v4NetCidr, v6NetCidr), }, args: args{ address: getCIDRNotationForAddress("2001:db8:abcd:0015::0/64"), nwCfg: &cni.NetworkConfig{ IPAM: cni.IPAM{ - Address: "10.0.0.4/24", + Address: ipv4cidr2, }, }, }, @@ -393,17 +405,17 @@ func TestRemoveIpamState_Add(t *testing.T) { fields: fields{ plugin: &mockDelegatePlugin{ add: add{ - resultsIPv4: getResult("10.0.0.1/24"), + resultsIPv4: getSingleResult(ipv4cidr), errv4: ipam.ErrNoAvailableAddressPools, }, }, - nwInfo: getNwInfo("10.0.0.0/24", ""), + nwInfo: getNwInfo(v4NetCidr, ""), }, args: args{ nwCfg: &cni.NetworkConfig{}, - subnetPrefix: getCIDRNotationForAddress("10.0.0.0/24"), + subnetPrefix: getCIDRNotationForAddress(v4NetCidr), }, - want: getResult("10.0.0.1/24")[0], + want: getResult(ipv4cidr), wantErrMsg: ipam.ErrNoAvailableAddressPools.Error(), wantErr: true, }, diff --git a/cni/network/invoker_cns.go b/cni/network/invoker_cns.go index ad60a2fb0a..d8867efe85 100644 --- a/cni/network/invoker_cns.go +++ b/cni/network/invoker_cns.go @@ -19,13 +19,20 @@ import ( cniTypesCurr "github.com/containernetworking/cni/pkg/types/100" "github.com/pkg/errors" "go.uber.org/zap" + "go.uber.org/zap/zapcore" +) + +const ( + expectedNumInterfacesWithDefaultRoutes = 1 ) var ( - errEmptyCNIArgs = errors.New("empty CNI cmd args not allowed") - errInvalidArgs = errors.New("invalid arg(s)") - overlayGatewayV6IP = "fe80::1234:5678:9abc" - watcherPath = "/var/run/azure-vnet/deleteIDs" + errEmptyCNIArgs = errors.New("empty CNI cmd args not allowed") + errInvalidArgs = errors.New("invalid arg(s)") + errInvalidDefaultRouting = errors.New("add result requires exactly one interface with default routes") + errInvalidGatewayIP = errors.New("invalid gateway IP") + overlayGatewayV6IP = "fe80::1234:5678:9abc" + watcherPath = "/var/run/azure-vnet/deleteIDs" ) type CNSIPAMInvoker struct { @@ -44,6 +51,25 @@ type IPResultInfo struct { hostSubnet string hostPrimaryIP string hostGateway string + nicType cns.NICType + macAddress string + skipDefaultRoutes bool + routes []cns.Route +} + +func (i IPResultInfo) MarshalLogObject(encoder zapcore.ObjectEncoder) error { + encoder.AddString("podIPAddress", i.podIPAddress) + encoder.AddUint8("ncSubnetPrefix", i.ncSubnetPrefix) + encoder.AddString("ncPrimaryIP", i.ncPrimaryIP) + encoder.AddString("ncGatewayIPAddress", i.ncGatewayIPAddress) + encoder.AddString("hostSubnet", i.hostSubnet) + encoder.AddString("hostPrimaryIP", i.hostPrimaryIP) + encoder.AddString("hostGateway", i.hostGateway) + encoder.AddString("nicType", string(i.nicType)) + encoder.AddString("macAddress", i.macAddress) + encoder.AddBool("skipDefaultRoutes", i.skipDefaultRoutes) + encoder.AddString("routes", fmt.Sprintf("%+v", i.routes)) + return nil } func NewCNSInvoker(podName, namespace string, cnsClient cnsclient, executionMode util.ExecutionMode, ipamMode util.IpamMode) *CNSIPAMInvoker { @@ -118,6 +144,7 @@ func (invoker *CNSIPAMInvoker) Add(addConfig IPAMAddConfig) (IPAMAddResult, erro } addResult := IPAMAddResult{} + numInterfacesWithDefaultRoutes := 0 for i := 0; i < len(response.PodIPInfo); i++ { info := IPResultInfo{ @@ -128,97 +155,48 @@ func (invoker *CNSIPAMInvoker) Add(addConfig IPAMAddConfig) (IPAMAddResult, erro hostSubnet: response.PodIPInfo[i].HostPrimaryIPInfo.Subnet, hostPrimaryIP: response.PodIPInfo[i].HostPrimaryIPInfo.PrimaryIP, hostGateway: response.PodIPInfo[i].HostPrimaryIPInfo.Gateway, - } - - // set the NC Primary IP in options - // SNATIPKey is not set for ipv6 - if net.ParseIP(info.ncPrimaryIP).To4() != nil { - addConfig.options[network.SNATIPKey] = info.ncPrimaryIP + nicType: response.PodIPInfo[i].NICType, + macAddress: response.PodIPInfo[i].MacAddress, + skipDefaultRoutes: response.PodIPInfo[i].SkipDefaultRoutes, + routes: response.PodIPInfo[i].Routes, } logger.Info("Received info for pod", - zap.Any("ipv4info", info), + zap.Any("ipInfo", info), zap.Any("podInfo", podInfo)) - ip, ncIPNet, err := net.ParseCIDR(info.podIPAddress + "/" + fmt.Sprint(info.ncSubnetPrefix)) - if ip == nil { - return IPAMAddResult{}, errors.Wrap(err, "Unable to parse IP from response: "+info.podIPAddress+" with err %w") - } - ncgw := net.ParseIP(info.ncGatewayIPAddress) - if ncgw == nil { - // TODO: Remove v4overlay and dualstackoverlay options, after 'overlay' rolls out in AKS-RP - if (invoker.ipamMode != util.V4Overlay) && (invoker.ipamMode != util.DualStackOverlay) && (invoker.ipamMode != util.Overlay) { - return IPAMAddResult{}, errors.Wrap(errInvalidArgs, "%w: Gateway address "+info.ncGatewayIPAddress+" from response is invalid") + //nolint:exhaustive // ignore exhaustive types check + switch info.nicType { + case cns.DelegatedVMNIC: + // only handling single v4 PodIPInfo for DelegatedVMNICs at the moment, will have to update once v6 gets added + if !info.skipDefaultRoutes { + numInterfacesWithDefaultRoutes++ } - if net.ParseIP(info.podIPAddress).To4() != nil { //nolint:gocritic - ncgw, err = getOverlayGateway(ncIPNet) - if err != nil { - return IPAMAddResult{}, err - } - } else if net.ParseIP(info.podIPAddress).To16() != nil { - ncgw = net.ParseIP(overlayGatewayV6IP) - } else { - return IPAMAddResult{}, errors.Wrap(err, "No podIPAddress is found: %w") - } - } - - // construct ipnet for result - resultIPnet := net.IPNet{ - IP: ip, - Mask: ncIPNet.Mask, - } - - if net.ParseIP(info.podIPAddress).To4() != nil { - addResult.ipv4Result = &cniTypesCurr.Result{ - IPs: []*cniTypesCurr.IPConfig{ - { - Address: resultIPnet, - Gateway: ncgw, - }, - }, - Routes: []*cniTypes.Route{ - { - Dst: network.Ipv4DefaultRouteDstPrefix, - GW: ncgw, - }, - }, + if err := configureSecondaryAddResult(&info, &addResult, &response.PodIPInfo[i].PodIPConfig); err != nil { + return IPAMAddResult{}, err } - } else if net.ParseIP(info.podIPAddress).To16() != nil { - addResult.ipv6Result = &cniTypesCurr.Result{ - IPs: []*cniTypesCurr.IPConfig{ - { - Address: resultIPnet, - Gateway: ncgw, - }, - }, - Routes: []*cniTypes.Route{ - { - Dst: network.Ipv6DefaultRouteDstPrefix, - GW: ncgw, - }, - }, + default: + // only count dualstack interface once + if addResult.defaultInterfaceInfo.ipResult == nil { + addResult.defaultInterfaceInfo.ipResult = &cniTypesCurr.Result{} + if !info.skipDefaultRoutes { + numInterfacesWithDefaultRoutes++ + } } - } - - // get the name of the primary IP address - _, hostIPNet, err := net.ParseCIDR(info.hostSubnet) - if err != nil { - return IPAMAddResult{}, fmt.Errorf("unable to parse hostSubnet: %w", err) - } - addResult.hostSubnetPrefix = *hostIPNet - - // set subnet prefix for host vm - // setHostOptions will execute if IPAM mode is not v4 overlay and not dualStackOverlay mode - // TODO: Remove v4overlay and dualstackoverlay options, after 'overlay' rolls out in AKS-RP - if (invoker.ipamMode != util.V4Overlay) && (invoker.ipamMode != util.DualStackOverlay) && (invoker.ipamMode != util.Overlay) { - if err := setHostOptions(ncIPNet, addConfig.options, &info); err != nil { + overlayMode := (invoker.ipamMode == util.V4Overlay) || (invoker.ipamMode == util.DualStackOverlay) || (invoker.ipamMode == util.Overlay) + if err := configureDefaultAddResult(&info, &addConfig, &addResult, overlayMode); err != nil { return IPAMAddResult{}, err } } } + // Make sure default routes exist for 1 interface + if numInterfacesWithDefaultRoutes != expectedNumInterfacesWithDefaultRoutes { + return IPAMAddResult{}, errInvalidDefaultRouting + } + return addResult, nil } @@ -352,3 +330,151 @@ func (invoker *CNSIPAMInvoker) Delete(address *net.IPNet, nwCfg *cni.NetworkConf return nil } + +func getRoutes(cnsRoutes []cns.Route, skipDefaultRoutes bool) ([]*cniTypes.Route, error) { + routes := make([]*cniTypes.Route, 0) + for _, route := range cnsRoutes { + _, dst, routeErr := net.ParseCIDR(route.IPAddress) + if routeErr != nil { + return nil, fmt.Errorf("unable to parse destination %s: %w", route.IPAddress, routeErr) + } + + gw := net.ParseIP(route.GatewayIPAddress) + if gw == nil && skipDefaultRoutes { + return nil, errors.Wrap(errInvalidGatewayIP, route.GatewayIPAddress) + } + + routes = append(routes, + &cniTypes.Route{ + Dst: *dst, + GW: gw, + }) + } + + return routes, nil +} + +func configureDefaultAddResult(info *IPResultInfo, addConfig *IPAMAddConfig, addResult *IPAMAddResult, overlayMode bool) error { + // set the NC Primary IP in options + // SNATIPKey is not set for ipv6 + if net.ParseIP(info.ncPrimaryIP).To4() != nil { + addConfig.options[network.SNATIPKey] = info.ncPrimaryIP + } + + ip, ncIPNet, err := net.ParseCIDR(info.podIPAddress + "/" + fmt.Sprint(info.ncSubnetPrefix)) + if ip == nil { + return errors.Wrap(err, "Unable to parse IP from response: "+info.podIPAddress+" with err %w") + } + + ncgw := net.ParseIP(info.ncGatewayIPAddress) + if ncgw == nil { + // TODO: Remove v4overlay and dualstackoverlay options, after 'overlay' rolls out in AKS-RP + if !overlayMode { + return errors.Wrap(errInvalidArgs, "%w: Gateway address "+info.ncGatewayIPAddress+" from response is invalid") + } + + if net.ParseIP(info.podIPAddress).To4() != nil { //nolint:gocritic + ncgw, err = getOverlayGateway(ncIPNet) + if err != nil { + return err + } + } else if net.ParseIP(info.podIPAddress).To16() != nil { + ncgw = net.ParseIP(overlayGatewayV6IP) + } else { + return errors.Wrap(err, "No podIPAddress is found: %w") + } + } + + if ip := net.ParseIP(info.podIPAddress); ip != nil { + defaultInterfaceInfo := addResult.defaultInterfaceInfo.ipResult + defaultRouteDstPrefix := network.Ipv4DefaultRouteDstPrefix + if ip.To4() == nil { + defaultRouteDstPrefix = network.Ipv6DefaultRouteDstPrefix + addResult.ipv6Enabled = true + } + + defaultInterfaceInfo.IPs = append(defaultInterfaceInfo.IPs, + &cniTypesCurr.IPConfig{ + Address: net.IPNet{ + IP: ip, + Mask: ncIPNet.Mask, + }, + Gateway: ncgw, + }) + + routes, getRoutesErr := getRoutes(info.routes, info.skipDefaultRoutes) + if getRoutesErr != nil { + return getRoutesErr + } + + if len(routes) > 0 { + defaultInterfaceInfo.Routes = append(defaultInterfaceInfo.Routes, routes...) + } else { // add default routes if none are provided + defaultInterfaceInfo.Routes = append(defaultInterfaceInfo.Routes, &cniTypes.Route{ + Dst: defaultRouteDstPrefix, + GW: ncgw, + }) + } + + addResult.defaultInterfaceInfo.ipResult = defaultInterfaceInfo + addResult.defaultInterfaceInfo.skipDefaultRoutes = info.skipDefaultRoutes + } + + // get the name of the primary IP address + _, hostIPNet, err := net.ParseCIDR(info.hostSubnet) + if err != nil { + return fmt.Errorf("unable to parse hostSubnet: %w", err) + } + + addResult.hostSubnetPrefix = *hostIPNet + addResult.defaultInterfaceInfo.nicType = cns.InfraNIC + + // set subnet prefix for host vm + // setHostOptions will execute if IPAM mode is not v4 overlay and not dualStackOverlay mode + // TODO: Remove v4overlay and dualstackoverlay options, after 'overlay' rolls out in AKS-RP + if !overlayMode { + if err := setHostOptions(ncIPNet, addConfig.options, info); err != nil { + return err + } + } + + return nil +} + +func configureSecondaryAddResult(info *IPResultInfo, addResult *IPAMAddResult, podIPConfig *cns.IPSubnet) error { + ip, ipnet, err := podIPConfig.GetIPNet() + if ip == nil { + return errors.Wrap(err, "Unable to parse IP from response: "+info.podIPAddress+" with err %w") + } + + macAddress, err := net.ParseMAC(info.macAddress) + if err != nil { + return errors.Wrap(err, "Invalid mac address") + } + + result := InterfaceInfo{ + ipResult: &cniTypesCurr.Result{ + IPs: []*cniTypesCurr.IPConfig{ + { + Address: net.IPNet{ + IP: ip, + Mask: ipnet.Mask, + }, + }, + }, + }, + nicType: cns.DelegatedVMNIC, + macAddress: macAddress, + skipDefaultRoutes: info.skipDefaultRoutes, + } + + routes, err := getRoutes(info.routes, info.skipDefaultRoutes) + if err != nil { + return err + } + + result.ipResult.Routes = append(result.ipResult.Routes, routes...) + addResult.secondaryInterfacesInfo = append(addResult.secondaryInterfacesInfo, result) + + return nil +} diff --git a/cni/network/invoker_cns_test.go b/cni/network/invoker_cns_test.go index 982f2cbacc..0b569e08ac 100644 --- a/cni/network/invoker_cns_test.go +++ b/cni/network/invoker_cns_test.go @@ -51,6 +51,9 @@ func TestCNSIPAMInvoker_Add_Overlay(t *testing.T) { unsupportedAPIs := make(map[cnsAPIName]struct{}) unsupportedAPIs["RequestIPs"] = struct{}{} + macAddress := "12:34:56:78:9a:bc" + parsedMacAddress, _ := net.ParseMAC(macAddress) + type fields struct { podName string podNamespace string @@ -65,12 +68,12 @@ func TestCNSIPAMInvoker_Add_Overlay(t *testing.T) { } tests := []struct { - name string - fields fields - args args - wantIpv4Result *cniTypesCurr.Result - wantIpv6Result *cniTypesCurr.Result - wantErr bool + name string + fields fields + args args + wantDefaultResult *cniTypesCurr.Result + wantSecondaryInterfacesInfo InterfaceInfo + wantErr bool }{ { name: "Test happy CNI Overlay add in v4overlay ipamMode", @@ -126,7 +129,7 @@ func TestCNSIPAMInvoker_Add_Overlay(t *testing.T) { hostSubnetPrefix: getCIDRNotationForAddress("10.224.0.0/16"), options: map[string]interface{}{}, }, - wantIpv4Result: &cniTypesCurr.Result{ + wantDefaultResult: &cniTypesCurr.Result{ IPs: []*cniTypesCurr.IPConfig{ { Address: *getCIDRNotationForAddress("10.240.1.242/16"), @@ -140,8 +143,7 @@ func TestCNSIPAMInvoker_Add_Overlay(t *testing.T) { }, }, }, - wantIpv6Result: nil, - wantErr: false, + wantErr: false, }, { name: "Test happy CNI Overlay add in dualstack overlay ipamMode", @@ -213,36 +215,256 @@ func TestCNSIPAMInvoker_Add_Overlay(t *testing.T) { hostSubnetPrefix: getCIDRNotationForAddress("10.0.0.1/24"), options: map[string]interface{}{}, }, - wantIpv4Result: &cniTypesCurr.Result{ + wantDefaultResult: &cniTypesCurr.Result{ IPs: []*cniTypesCurr.IPConfig{ { Address: *getCIDRNotationForAddress("10.0.1.10/24"), Gateway: net.ParseIP("10.0.0.1"), }, + { + Address: *getCIDRNotationForAddress("fd11:1234::1/112"), + Gateway: net.ParseIP("fe80::1234:5678:9abc"), + }, }, Routes: []*cniTypes.Route{ { Dst: network.Ipv4DefaultRouteDstPrefix, GW: net.ParseIP("10.0.0.1"), }, + { + Dst: network.Ipv6DefaultRouteDstPrefix, + GW: net.ParseIP("fe80::1234:5678:9abc"), + }, }, }, - wantIpv6Result: &cniTypesCurr.Result{ + wantErr: false, + }, + { + name: "Test happy CNI add with multitenant result", + fields: fields{ + podName: testPodInfo.PodName, + podNamespace: testPodInfo.PodNamespace, + ipamMode: util.Overlay, + cnsClient: &MockCNSClient{ + require: require, + requestIPs: requestIPsHandler{ + ipconfigArgument: cns.IPConfigsRequest{ + PodInterfaceID: "testcont-testifname3", + InfraContainerID: "testcontainerid3", + OrchestratorContext: marshallPodInfo(testPodInfo), + }, + result: &cns.IPConfigsResponse{ + PodIPInfo: []cns.PodIpInfo{ + { + PodIPConfig: cns.IPSubnet{ + IPAddress: "10.0.1.10", + PrefixLength: 24, + }, + NetworkContainerPrimaryIPConfig: cns.IPConfiguration{ + IPSubnet: cns.IPSubnet{ + IPAddress: "10.0.1.0", + PrefixLength: 24, + }, + DNSServers: nil, + GatewayIPAddress: "10.0.0.1", + }, + HostPrimaryIPInfo: cns.HostIPInfo{ + Gateway: "10.0.0.1", + PrimaryIP: "10.0.0.1", + Subnet: "10.0.0.0/24", + }, + NICType: cns.InfraNIC, + SkipDefaultRoutes: true, + }, + { + PodIPConfig: cns.IPSubnet{ + IPAddress: "20.240.1.242", + PrefixLength: 24, + }, + NICType: cns.DelegatedVMNIC, + MacAddress: macAddress, + }, + }, + Response: cns.Response{ + ReturnCode: 0, + Message: "", + }, + }, + err: nil, + }, + }, + }, + args: args{ + nwCfg: &cni.NetworkConfig{}, + args: &cniSkel.CmdArgs{ + ContainerID: "testcontainerid3", + Netns: "testnetns3", + IfName: "testifname3", + }, + hostSubnetPrefix: getCIDRNotationForAddress("10.0.0.1/24"), + options: map[string]interface{}{}, + }, + wantDefaultResult: &cniTypesCurr.Result{ IPs: []*cniTypesCurr.IPConfig{ { - Address: *getCIDRNotationForAddress("fd11:1234::1/112"), - Gateway: net.ParseIP("fe80::1234:5678:9abc"), + Address: *getCIDRNotationForAddress("10.0.1.10/24"), + Gateway: net.ParseIP("10.0.0.1"), }, }, Routes: []*cniTypes.Route{ { - Dst: network.Ipv6DefaultRouteDstPrefix, - GW: net.ParseIP("fe80::1234:5678:9abc"), + Dst: network.Ipv4DefaultRouteDstPrefix, + GW: net.ParseIP("10.0.0.1"), + }, + }, + }, + wantSecondaryInterfacesInfo: InterfaceInfo{ + ipResult: &cniTypesCurr.Result{ + IPs: []*cniTypesCurr.IPConfig{ + { + Address: *getCIDRNotationForAddress("20.240.1.242/24"), + }, }, }, + nicType: cns.DelegatedVMNIC, + macAddress: parsedMacAddress, }, wantErr: false, }, + { + name: "Test fail CNI add with invalid mac in delegated VM nic response", + fields: fields{ + podName: testPodInfo.PodName, + podNamespace: testPodInfo.PodNamespace, + ipamMode: util.Overlay, + cnsClient: &MockCNSClient{ + require: require, + requestIPs: requestIPsHandler{ + ipconfigArgument: cns.IPConfigsRequest{ + PodInterfaceID: "testcont-testifname3", + InfraContainerID: "testcontainerid3", + OrchestratorContext: marshallPodInfo(testPodInfo), + }, + result: &cns.IPConfigsResponse{ + PodIPInfo: []cns.PodIpInfo{ + { + PodIPConfig: cns.IPSubnet{ + IPAddress: "10.0.1.10", + PrefixLength: 24, + }, + NetworkContainerPrimaryIPConfig: cns.IPConfiguration{ + IPSubnet: cns.IPSubnet{ + IPAddress: "10.0.1.0", + PrefixLength: 24, + }, + DNSServers: nil, + GatewayIPAddress: "10.0.0.1", + }, + HostPrimaryIPInfo: cns.HostIPInfo{ + Gateway: "10.0.0.1", + PrimaryIP: "10.0.0.1", + Subnet: "10.0.0.0/24", + }, + NICType: cns.InfraNIC, + SkipDefaultRoutes: true, + }, + { + PodIPConfig: cns.IPSubnet{ + IPAddress: "20.240.1.242", + PrefixLength: 24, + }, + NICType: cns.DelegatedVMNIC, + MacAddress: "bad mac", + }, + }, + Response: cns.Response{ + ReturnCode: 0, + Message: "", + }, + }, + err: nil, + }, + }, + }, + args: args{ + nwCfg: &cni.NetworkConfig{}, + args: &cniSkel.CmdArgs{ + ContainerID: "testcontainerid3", + Netns: "testnetns3", + IfName: "testifname3", + }, + hostSubnetPrefix: getCIDRNotationForAddress("10.0.0.1/24"), + options: map[string]interface{}{}, + }, + wantErr: true, + }, + { + name: "Test fail CNI add with invalid ip config in delegated VM nic response", + fields: fields{ + podName: testPodInfo.PodName, + podNamespace: testPodInfo.PodNamespace, + ipamMode: util.Overlay, + cnsClient: &MockCNSClient{ + require: require, + requestIPs: requestIPsHandler{ + ipconfigArgument: cns.IPConfigsRequest{ + PodInterfaceID: "testcont-testifname3", + InfraContainerID: "testcontainerid3", + OrchestratorContext: marshallPodInfo(testPodInfo), + }, + result: &cns.IPConfigsResponse{ + PodIPInfo: []cns.PodIpInfo{ + { + PodIPConfig: cns.IPSubnet{ + IPAddress: "10.0.1.10", + PrefixLength: 24, + }, + NetworkContainerPrimaryIPConfig: cns.IPConfiguration{ + IPSubnet: cns.IPSubnet{ + IPAddress: "10.0.1.0", + PrefixLength: 24, + }, + DNSServers: nil, + GatewayIPAddress: "10.0.0.1", + }, + HostPrimaryIPInfo: cns.HostIPInfo{ + Gateway: "10.0.0.1", + PrimaryIP: "10.0.0.1", + Subnet: "10.0.0.0/24", + }, + NICType: cns.InfraNIC, + SkipDefaultRoutes: true, + }, + { + PodIPConfig: cns.IPSubnet{ + IPAddress: "bad ip", + PrefixLength: 24, + }, + NICType: cns.DelegatedVMNIC, + MacAddress: macAddress, + }, + }, + Response: cns.Response{ + ReturnCode: 0, + Message: "", + }, + }, + err: nil, + }, + }, + }, + args: args{ + nwCfg: &cni.NetworkConfig{}, + args: &cniSkel.CmdArgs{ + ContainerID: "testcontainerid3", + Netns: "testnetns3", + IfName: "testifname3", + }, + hostSubnetPrefix: getCIDRNotationForAddress("10.0.0.1/24"), + options: map[string]interface{}{}, + }, + wantErr: true, + }, } for _, tt := range tests { tt := tt @@ -262,9 +484,11 @@ func TestCNSIPAMInvoker_Add_Overlay(t *testing.T) { require.NoError(err) } - fmt.Printf("want:%+v\nrest:%+v\n", tt.wantIpv4Result, ipamAddResult.ipv4Result) - require.Equalf(tt.wantIpv4Result, ipamAddResult.ipv4Result, "incorrect ipv4 response") - require.Equalf(tt.wantIpv6Result, ipamAddResult.ipv6Result, "incorrect ipv6 response") + fmt.Printf("want:%+v\nrest:%+v\n", tt.wantSecondaryInterfacesInfo, ipamAddResult.secondaryInterfacesInfo) + require.Equalf(tt.wantDefaultResult, ipamAddResult.defaultInterfaceInfo.ipResult, "incorrect default response") + if tt.wantSecondaryInterfacesInfo.ipResult != nil { + require.EqualValues(tt.wantSecondaryInterfacesInfo, ipamAddResult.secondaryInterfacesInfo[0], "incorrect multitenant response") + } }) } } @@ -285,12 +509,12 @@ func TestCNSIPAMInvoker_Add(t *testing.T) { } tests := []struct { - name string - fields fields - args args - wantIpv4Result *cniTypesCurr.Result - wantIpv6Result *cniTypesCurr.Result - wantErr bool + name string + fields fields + args args + wantDefaultResult *cniTypesCurr.Result + wantMultitenantResult *cniTypesCurr.Result + wantErr bool }{ { name: "Test happy CNI add", @@ -342,7 +566,7 @@ func TestCNSIPAMInvoker_Add(t *testing.T) { hostSubnetPrefix: getCIDRNotationForAddress("10.0.0.1/24"), options: map[string]interface{}{}, }, - wantIpv4Result: &cniTypesCurr.Result{ + wantDefaultResult: &cniTypesCurr.Result{ IPs: []*cniTypesCurr.IPConfig{ { Address: *getCIDRNotationForAddress("10.0.1.10/24"), @@ -356,8 +580,7 @@ func TestCNSIPAMInvoker_Add(t *testing.T) { }, }, }, - wantIpv6Result: nil, - wantErr: false, + wantErr: false, }, { name: "Test happy CNI add for both ipv4 and ipv6", @@ -428,28 +651,22 @@ func TestCNSIPAMInvoker_Add(t *testing.T) { hostSubnetPrefix: getCIDRNotationForAddress("10.0.0.1/24"), options: map[string]interface{}{}, }, - wantIpv4Result: &cniTypesCurr.Result{ + wantDefaultResult: &cniTypesCurr.Result{ IPs: []*cniTypesCurr.IPConfig{ { Address: *getCIDRNotationForAddress("10.0.1.10/24"), Gateway: net.ParseIP("10.0.0.1"), }, - }, - Routes: []*cniTypes.Route{ - { - Dst: network.Ipv4DefaultRouteDstPrefix, - GW: net.ParseIP("10.0.0.1"), - }, - }, - }, - wantIpv6Result: &cniTypesCurr.Result{ - IPs: []*cniTypesCurr.IPConfig{ { Address: *getCIDRNotationForAddress("fd11:1234::1/112"), Gateway: net.ParseIP("fe80::1234:5678:9abc"), }, }, Routes: []*cniTypes.Route{ + { + Dst: network.Ipv4DefaultRouteDstPrefix, + GW: net.ParseIP("10.0.0.1"), + }, { Dst: network.Ipv6DefaultRouteDstPrefix, GW: net.ParseIP("fe80::1234:5678:9abc"), @@ -493,9 +710,11 @@ func TestCNSIPAMInvoker_Add(t *testing.T) { require.NoError(err) } - fmt.Printf("want:%+v\nrest:%+v\n", tt.wantIpv4Result, ipamAddResult.ipv4Result) - require.Equalf(tt.wantIpv4Result, ipamAddResult.ipv4Result, "incorrect ipv4 response") - require.Equalf(tt.wantIpv6Result, ipamAddResult.ipv6Result, "incorrect ipv6 response") + fmt.Printf("want:%+v\nrest:%+v\n", tt.wantMultitenantResult, ipamAddResult.secondaryInterfacesInfo) + require.Equalf(tt.wantDefaultResult, ipamAddResult.defaultInterfaceInfo.ipResult, "incorrect default response") + if tt.wantMultitenantResult != nil { + require.Equalf(tt.wantMultitenantResult, ipamAddResult.secondaryInterfacesInfo[0].ipResult, "incorrect multitenant response") + } }) } } @@ -610,7 +829,7 @@ func TestCNSIPAMInvoker_Add_UnsupportedAPI(t *testing.T) { t.Fatalf("expected an error %+v but none received", err) } require.NoError(err) - require.Equalf(tt.want, ipamAddResult.ipv4Result, "incorrect ipv4 response") + require.Equalf(tt.want, ipamAddResult.defaultInterfaceInfo.ipResult, "incorrect ipv4 response") }) } } diff --git a/cni/network/invoker_mock.go b/cni/network/invoker_mock.go index 51e04c29ce..db61751ce2 100644 --- a/cni/network/invoker_mock.go +++ b/cni/network/invoker_mock.go @@ -5,6 +5,7 @@ import ( "net" "github.com/Azure/azure-container-networking/cni" + "github.com/Azure/azure-container-networking/cns" "github.com/containernetworking/cni/pkg/skel" current "github.com/containernetworking/cni/pkg/types/100" ) @@ -17,24 +18,30 @@ const ( ) var ( - errV4 = errors.New("v4 fail") - errV6 = errors.New("v6 Fail") - errDeleteIpam = errors.New("delete fail") + errV4 = errors.New("v4 fail") + errV6 = errors.New("v6 Fail") + errDelegatedVMNIC = errors.New("delegatedVMNIC fail") + errDeleteIpam = errors.New("delete fail") ) type MockIpamInvoker struct { - isIPv6 bool - v4Fail bool - v6Fail bool - ipMap map[string]bool + isIPv6 bool + v4Fail bool + v6Fail bool + delegatedVMNIC bool + delegatedVMNICFail bool + ipMap map[string]bool } -func NewMockIpamInvoker(ipv6, v4Fail, v6Fail bool) *MockIpamInvoker { +func NewMockIpamInvoker(ipv6, v4Fail, v6Fail, delegatedVMNIC, delegatedVMNICFail bool) *MockIpamInvoker { return &MockIpamInvoker{ - isIPv6: ipv6, - v4Fail: v4Fail, - v6Fail: v6Fail, - ipMap: make(map[string]bool), + isIPv6: ipv6, + v4Fail: v4Fail, + v6Fail: v6Fail, + delegatedVMNIC: delegatedVMNIC, + delegatedVMNICFail: delegatedVMNICFail, + + ipMap: make(map[string]bool), } } @@ -53,9 +60,12 @@ func (invoker *MockIpamInvoker) Add(opt IPAMAddConfig) (ipamAddResult IPAMAddRes ip := net.ParseIP(ipv4Str) ipnet := net.IPNet{IP: ip, Mask: net.CIDRMask(subnetBits, ipv4Bits)} gwIP := net.ParseIP("10.240.0.1") - ipConfig := ¤t.IPConfig{Address: ipnet, Gateway: gwIP} - ipamAddResult.ipv4Result = ¤t.Result{} - ipamAddResult.ipv4Result.IPs = append(ipamAddResult.ipv4Result.IPs, ipConfig) + ipamAddResult.defaultInterfaceInfo = InterfaceInfo{ + ipResult: ¤t.Result{ + IPs: []*current.IPConfig{{Address: ipnet, Gateway: gwIP}}, + }, + nicType: cns.InfraNIC, + } invoker.ipMap[ipnet.String()] = true if invoker.v6Fail { return ipamAddResult, errV6 @@ -70,12 +80,25 @@ func (invoker *MockIpamInvoker) Add(opt IPAMAddConfig) (ipamAddResult IPAMAddRes ip := net.ParseIP(ipv6Str) ipnet := net.IPNet{IP: ip, Mask: net.CIDRMask(subnetv6Bits, ipv6Bits)} gwIP := net.ParseIP("fc00::1") - ipConfig := ¤t.IPConfig{Address: ipnet, Gateway: gwIP} - ipamAddResult.ipv6Result = ¤t.Result{} - ipamAddResult.ipv6Result.IPs = append(ipamAddResult.ipv6Result.IPs, ipConfig) + ipamAddResult.defaultInterfaceInfo.ipResult.IPs = append(ipamAddResult.defaultInterfaceInfo.ipResult.IPs, ¤t.IPConfig{Address: ipnet, Gateway: gwIP}) invoker.ipMap[ipnet.String()] = true } + if invoker.delegatedVMNIC { + if invoker.delegatedVMNICFail { + return IPAMAddResult{}, errDelegatedVMNIC + } + + ipStr := "20.20.20.20/32" + _, ipnet, _ := net.ParseCIDR(ipStr) + ipamAddResult.secondaryInterfacesInfo = append(ipamAddResult.secondaryInterfacesInfo, InterfaceInfo{ + ipResult: ¤t.Result{ + IPs: []*current.IPConfig{{Address: *ipnet}}, + }, + nicType: cns.DelegatedVMNIC, + }) + } + return ipamAddResult, nil } diff --git a/cni/network/multitenancy.go b/cni/network/multitenancy.go index 29eddc3d4b..4dd09f81c1 100644 --- a/cni/network/multitenancy.go +++ b/cni/network/multitenancy.go @@ -220,7 +220,8 @@ func (m *Multitenancy) GetAllNetworkContainers( for i := 0; i < len(ncResponses); i++ { ipamResults[i].ncResponse = &ncResponses[i] ipamResults[i].hostSubnetPrefix = hostSubnetPrefixes[i] - ipamResults[i].ipv4Result = convertToCniResult(ipamResults[i].ncResponse, ifName) + ipamResults[i].defaultInterfaceInfo.ipResult = convertToCniResult(ipamResults[i].ncResponse, ifName) + ipamResults[i].defaultInterfaceInfo.nicType = cns.InfraNIC } return ipamResults, err diff --git a/cni/network/multitenancy_mock.go b/cni/network/multitenancy_mock.go index d6b826c9ba..c979bd597d 100644 --- a/cni/network/multitenancy_mock.go +++ b/cni/network/multitenancy_mock.go @@ -158,7 +158,8 @@ func (m *MockMultitenancy) GetAllNetworkContainers( for i := 0; i < len(cnsResponses); i++ { ipamResults[i].ncResponse = &cnsResponses[i] ipamResults[i].hostSubnetPrefix = ipNets[i] - ipamResults[i].ipv4Result = convertToCniResult(ipamResults[i].ncResponse, ifName) + ipamResults[i].defaultInterfaceInfo.ipResult = convertToCniResult(ipamResults[i].ncResponse, ifName) + ipamResults[i].defaultInterfaceInfo.nicType = cns.InfraNIC } return ipamResults, nil diff --git a/cni/network/multitenancy_test.go b/cni/network/multitenancy_test.go index e3a2620af1..fe9e3da127 100644 --- a/cni/network/multitenancy_test.go +++ b/cni/network/multitenancy_test.go @@ -287,7 +287,7 @@ func TestCleanupMultitenancyResources(t *testing.T) { }, infraIPNet: &cniTypesCurr.Result{}, plugin: &NetPlugin{ - ipamInvoker: NewMockIpamInvoker(false, false, false), + ipamInvoker: NewMockIpamInvoker(false, false, false, false, false), }, }, expected: args{ @@ -298,7 +298,7 @@ func TestCleanupMultitenancyResources(t *testing.T) { }, infraIPNet: &cniTypesCurr.Result{}, plugin: &NetPlugin{ - ipamInvoker: NewMockIpamInvoker(false, false, false), + ipamInvoker: NewMockIpamInvoker(false, false, false, false, false), }, }, }, @@ -413,7 +413,7 @@ func TestGetMultiTenancyCNIResult(t *testing.T) { IPAM: cni.IPAM{Type: "azure-vnet-ipam"}, }, plugin: &NetPlugin{ - ipamInvoker: NewMockIpamInvoker(false, false, false), + ipamInvoker: NewMockIpamInvoker(false, false, false, false, false), multitenancyClient: &Multitenancy{ netioshim: &mockNetIOShim{}, cnsclient: &MockCNSClient{ @@ -626,7 +626,7 @@ func TestGetMultiTenancyCNIResultUnsupportedAPI(t *testing.T) { IPAM: cni.IPAM{Type: "azure-vnet-ipam"}, }, plugin: &NetPlugin{ - ipamInvoker: NewMockIpamInvoker(false, false, false), + ipamInvoker: NewMockIpamInvoker(false, false, false, false, false), multitenancyClient: &Multitenancy{ netioshim: &mockNetIOShim{}, cnsclient: &MockCNSClient{ @@ -765,7 +765,7 @@ func TestGetMultiTenancyCNIResultNotFound(t *testing.T) { IPAM: cni.IPAM{Type: "azure-vnet-ipam"}, }, plugin: &NetPlugin{ - ipamInvoker: NewMockIpamInvoker(false, false, false), + ipamInvoker: NewMockIpamInvoker(false, false, false, false, false), multitenancyClient: &Multitenancy{ netioshim: &mockNetIOShim{}, cnsclient: &MockCNSClient{ @@ -800,7 +800,7 @@ func TestGetMultiTenancyCNIResultNotFound(t *testing.T) { IPAM: cni.IPAM{Type: "azure-vnet-ipam"}, }, plugin: &NetPlugin{ - ipamInvoker: NewMockIpamInvoker(false, false, false), + ipamInvoker: NewMockIpamInvoker(false, false, false, false, false), multitenancyClient: &Multitenancy{ netioshim: &mockNetIOShim{}, cnsclient: &MockCNSClient{ diff --git a/cni/network/network.go b/cni/network/network.go index 09cc38792a..4c0dd34e84 100644 --- a/cni/network/network.go +++ b/cni/network/network.go @@ -117,7 +117,7 @@ func NewPlugin(name string, nl := netlink.NewNetlink() // Setup network manager. - nm, err := network.NewNetworkManager(nl, platform.NewExecClient(logger), &netio.NetIO{}) + nm, err := network.NewNetworkManager(nl, platform.NewExecClient(logger), &netio.NetIO{}, network.NewNamespaceClient()) if err != nil { return nil, err } @@ -362,23 +362,20 @@ func (plugin *NetPlugin) Add(args *cniSkel.CmdArgs) error { telemetry.SendCNIMetric(&cniMetric, plugin.tb) // Add Interfaces to result. - if ipamAddResult.ipv4Result == nil { - ipamAddResult.ipv4Result = &cniTypesCurr.Result{} + defaultCniResult := ipamAddResult.defaultInterfaceInfo.ipResult + if defaultCniResult == nil { + defaultCniResult = &cniTypesCurr.Result{} } iface := &cniTypesCurr.Interface{ Name: args.IfName, } - ipamAddResult.ipv4Result.Interfaces = append(ipamAddResult.ipv4Result.Interfaces, iface) + defaultCniResult.Interfaces = append(defaultCniResult.Interfaces, iface) - if ipamAddResult.ipv6Result != nil { - ipamAddResult.ipv4Result.IPs = append(ipamAddResult.ipv4Result.IPs, ipamAddResult.ipv6Result.IPs...) - } - - addSnatInterface(nwCfg, ipamAddResult.ipv4Result) + addSnatInterface(nwCfg, defaultCniResult) // Convert result to the requested CNI version. - res, vererr := ipamAddResult.ipv4Result.GetAsVersion(nwCfg.CNIVersion) + res, vererr := defaultCniResult.GetAsVersion(nwCfg.CNIVersion) if vererr != nil { logger.Error("GetAsVersion failed", zap.Error(vererr)) plugin.Error(vererr) @@ -391,7 +388,7 @@ func (plugin *NetPlugin) Add(args *cniSkel.CmdArgs) error { logger.Info("ADD command completed for", zap.String("pod", k8sPodName), - zap.Any("IPs", ipamAddResult.ipv4Result.IPs), + zap.Any("IPs", defaultCniResult.IPs), zap.Error(err)) }() @@ -424,7 +421,7 @@ func (plugin *NetPlugin) Add(args *cniSkel.CmdArgs) error { res, err = plugin.nnsClient.AddContainerNetworking(context.Background(), k8sPodName, args.Netns) if err == nil { - ipamAddResult.ipv4Result = convertNnsToCniResult(res, args.IfName, k8sPodName, "AddContainerNetworking") + ipamAddResult.defaultInterfaceInfo.ipResult = convertNnsToCniResult(res, args.IfName, k8sPodName, "AddContainerNetworking") } return err @@ -511,7 +508,7 @@ func (plugin *NetPlugin) Add(args *cniSkel.CmdArgs) error { } if resultSecondAdd != nil { - ipamAddResult.ipv4Result = resultSecondAdd + ipamAddResult.defaultInterfaceInfo.ipResult = resultSecondAdd return nil } } @@ -533,12 +530,12 @@ func (plugin *NetPlugin) Add(args *cniSkel.CmdArgs) error { if err != nil { return fmt.Errorf("IPAM Invoker Add failed with error: %w", err) } - sendEvent(plugin, fmt.Sprintf("Allocated IPAddress from ipam:%+v v6:%+v", ipamAddResult.ipv4Result, ipamAddResult.ipv6Result)) + sendEvent(plugin, fmt.Sprintf("Allocated IPAddress from ipam DefaultInterface: %+v, SecondaryInterfaces: %+v", ipamAddResult.defaultInterfaceInfo, ipamAddResult.secondaryInterfacesInfo)) } defer func() { //nolint:gocritic if err != nil { - plugin.cleanupAllocationOnError(ipamAddResult.ipv4Result, ipamAddResult.ipv6Result, nwCfg, args, options) + plugin.cleanupAllocationOnError(ipamAddResult.defaultInterfaceInfo.ipResult, nwCfg, args, options) } }() @@ -563,8 +560,7 @@ func (plugin *NetPlugin) Add(args *cniSkel.CmdArgs) error { createEndpointInternalOpt := createEndpointInternalOpt{ nwCfg: nwCfg, cnsNetworkConfig: ipamAddResult.ncResponse, - result: ipamAddResult.ipv4Result, - resultV6: ipamAddResult.ipv6Result, + ipamAddResult: ipamAddResult, azIpamResult: azIpamResult, args: args, nwInfo: &nwInfo, @@ -584,8 +580,8 @@ func (plugin *NetPlugin) Add(args *cniSkel.CmdArgs) error { return err } - sendEvent(plugin, fmt.Sprintf("CNI ADD succeeded : IP:%+v, VlanID: %v, podname %v, namespace %v numendpoints:%d", - ipamAddResult.ipv4Result.IPs, epInfo.Data[network.VlanIDKey], k8sPodName, k8sNamespace, plugin.nm.GetNumberOfEndpoints("", nwCfg.Name))) + sendEvent(plugin, fmt.Sprintf("CNI ADD succeeded: IP:%+v, VlanID: %v, podname %v, namespace %v numendpoints:%d", + ipamAddResult.defaultInterfaceInfo.ipResult.IPs, epInfo.Data[network.VlanIDKey], k8sPodName, k8sNamespace, plugin.nm.GetNumberOfEndpoints("", nwCfg.Name))) } return nil @@ -593,19 +589,16 @@ func (plugin *NetPlugin) Add(args *cniSkel.CmdArgs) error { // cleanup allocated ipv4 and ipv6 addresses if they exist func (plugin *NetPlugin) cleanupAllocationOnError( - result, resultV6 *cniTypesCurr.Result, + result *cniTypesCurr.Result, nwCfg *cni.NetworkConfig, args *cniSkel.CmdArgs, options map[string]interface{}, ) { - if result != nil && len(result.IPs) > 0 { - if er := plugin.ipamInvoker.Delete(&result.IPs[0].Address, nwCfg, args, options); er != nil { - logger.Error("Failed to cleanup ip allocation on failure", zap.Error(er)) - } - } - if resultV6 != nil && len(resultV6.IPs) > 0 { - if er := plugin.ipamInvoker.Delete(&resultV6.IPs[0].Address, nwCfg, args, options); er != nil { - logger.Error("Failed to cleanup ipv6 allocation on failure", zap.Error(er)) + if result != nil { + for i := 0; i < len(result.IPs); i++ { + if er := plugin.ipamInvoker.Delete(&result.IPs[i].Address, nwCfg, args, options); er != nil { + logger.Error("Failed to cleanup ip allocation on failure", zap.Error(er)) + } } } } @@ -634,7 +627,7 @@ func (plugin *NetPlugin) createNetworkInternal( return nwInfo, err } - nwDNSInfo, err := getNetworkDNSSettings(ipamAddConfig.nwCfg, ipamAddResult.ipv4Result) + nwDNSInfo, err := getNetworkDNSSettings(ipamAddConfig.nwCfg, ipamAddResult.defaultInterfaceInfo.ipResult) if err != nil { err = plugin.Errorf("Failed to getDNSSettings: %v", err) return nwInfo, err @@ -658,7 +651,7 @@ func (plugin *NetPlugin) createNetworkInternal( IPV6Mode: ipamAddConfig.nwCfg.IPV6Mode, // TODO: check if IPV6Mode field can be deprecated IPAMType: ipamAddConfig.nwCfg.IPAM.Type, ServiceCidrs: ipamAddConfig.nwCfg.ServiceCidrs, - IsIPv6Enabled: ipamAddResult.ipv6Result != nil, + IsIPv6Enabled: ipamAddResult.ipv6Enabled, } if err = addSubnetToNetworkInfo(ipamAddResult, &nwInfo); err != nil { @@ -678,36 +671,22 @@ func (plugin *NetPlugin) createNetworkInternal( // construct network info with ipv4/ipv6 subnets func addSubnetToNetworkInfo(ipamAddResult IPAMAddResult, nwInfo *network.NetworkInfo) error { - var ( - podSubnetPrefix *net.IPNet - podSubnetV6Prefix *net.IPNet - ) - - _, podSubnetPrefix, err := net.ParseCIDR(ipamAddResult.ipv4Result.IPs[0].Address.String()) - if err != nil { - return fmt.Errorf("Failed to ParseCIDR for pod subnet prefix: %w", err) - } - - ipv4Subnet := network.SubnetInfo{ - Family: platform.AfINET, - Prefix: *podSubnetPrefix, - Gateway: ipamAddResult.ipv4Result.IPs[0].Gateway, - } - nwInfo.Subnets = append(nwInfo.Subnets, ipv4Subnet) - - // parse the ipv6 address and only add it to nwInfo if it's dual stack mode - if ipamAddResult.ipv6Result != nil && len(ipamAddResult.ipv6Result.IPs) > 0 { - _, podSubnetV6Prefix, err = net.ParseCIDR(ipamAddResult.ipv6Result.IPs[0].Address.String()) + for _, ipConfig := range ipamAddResult.defaultInterfaceInfo.ipResult.IPs { + ip, podSubnetPrefix, err := net.ParseCIDR(ipConfig.Address.String()) if err != nil { - return fmt.Errorf("Failed to ParseCIDR for pod subnet IPv6 prefix: %w", err) + return fmt.Errorf("Failed to ParseCIDR for pod subnet prefix: %w", err) } - ipv6Subnet := network.SubnetInfo{ - Family: platform.AfINET6, - Prefix: *podSubnetV6Prefix, - Gateway: ipamAddResult.ipv6Result.IPs[0].Gateway, + subnet := network.SubnetInfo{ + Family: platform.AfINET, + Prefix: *podSubnetPrefix, + Gateway: ipConfig.Gateway, + } + if ip.To4() == nil { + subnet.Family = platform.AfINET6 } - nwInfo.Subnets = append(nwInfo.Subnets, ipv6Subnet) + + nwInfo.Subnets = append(nwInfo.Subnets, subnet) } return nil @@ -716,8 +695,7 @@ func addSubnetToNetworkInfo(ipamAddResult IPAMAddResult, nwInfo *network.Network type createEndpointInternalOpt struct { nwCfg *cni.NetworkConfig cnsNetworkConfig *cns.GetNetworkContainerResponse - result *cniTypesCurr.Result - resultV6 *cniTypesCurr.Result + ipamAddResult IPAMAddResult azIpamResult *cniTypesCurr.Result args *cniSkel.CmdArgs nwInfo *network.NetworkInfo @@ -733,7 +711,8 @@ type createEndpointInternalOpt struct { func (plugin *NetPlugin) createEndpointInternal(opt *createEndpointInternalOpt) (network.EndpointInfo, error) { epInfo := network.EndpointInfo{} - epDNSInfo, err := getEndpointDNSSettings(opt.nwCfg, opt.result, opt.k8sNamespace) + defaultIPResult := opt.ipamAddResult.defaultInterfaceInfo.ipResult + epDNSInfo, err := getEndpointDNSSettings(opt.nwCfg, defaultIPResult, opt.k8sNamespace) if err != nil { err = plugin.Errorf("Failed to getEndpointDNSSettings: %v", err) return epInfo, err @@ -741,7 +720,7 @@ func (plugin *NetPlugin) createEndpointInternal(opt *createEndpointInternalOpt) policyArgs := PolicyArgs{ nwInfo: opt.nwInfo, nwCfg: opt.nwCfg, - ipconfigs: opt.result.IPs, + ipconfigs: defaultIPResult.IPs, } endpointPolicies, err := getEndpointPolicies(policyArgs) if err != nil { @@ -779,32 +758,28 @@ func (plugin *NetPlugin) createEndpointInternal(opt *createEndpointInternalOpt) VnetCidrs: opt.nwCfg.VnetCidrs, ServiceCidrs: opt.nwCfg.ServiceCidrs, NATInfo: opt.natInfo, + NICType: cns.InfraNIC, + SkipDefaultRoutes: opt.ipamAddResult.defaultInterfaceInfo.skipDefaultRoutes, } - isIPv6Enabled := opt.resultV6 != nil - epPolicies, err := getPoliciesFromRuntimeCfg(opt.nwCfg, isIPv6Enabled) + epPolicies, err := getPoliciesFromRuntimeCfg(opt.nwCfg, opt.ipamAddResult.ipv6Enabled) if err != nil { logger.Error("failed to get policies from runtime configurations", zap.Error(err)) return epInfo, plugin.Errorf(err.Error()) } - epInfo.Policies = append(epInfo.Policies, epPolicies...) // Populate addresses. - for _, ipconfig := range opt.result.IPs { + for _, ipconfig := range defaultIPResult.IPs { epInfo.IPAddresses = append(epInfo.IPAddresses, ipconfig.Address) } - if opt.resultV6 != nil { - // inject ipv6 routes to Linux pod + if opt.ipamAddResult.ipv6Enabled { epInfo.IPV6Mode = string(util.IpamMode(opt.nwCfg.IPAM.Mode)) // TODO: check IPV6Mode field can be deprecated and can we add IsIPv6Enabled flag for generic working - for _, ipconfig := range opt.resultV6.IPs { - epInfo.IPAddresses = append(epInfo.IPAddresses, ipconfig.Address) - } } // Populate routes. - for _, route := range opt.result.Routes { + for _, route := range defaultIPResult.Routes { epInfo.Routes = append(epInfo.Routes, network.RouteInfo{Dst: route.Dst, Gw: route.GW}) } @@ -813,7 +788,7 @@ func (plugin *NetPlugin) createEndpointInternal(opt *createEndpointInternalOpt) } if opt.nwCfg.MultiTenancy { - plugin.multitenancyClient.SetupRoutingForMultitenancy(opt.nwCfg, opt.cnsNetworkConfig, opt.azIpamResult, &epInfo, opt.result) + plugin.multitenancyClient.SetupRoutingForMultitenancy(opt.nwCfg, opt.cnsNetworkConfig, opt.azIpamResult, &epInfo, defaultIPResult) } setEndpointOptions(opt.cnsNetworkConfig, &epInfo, vethName) @@ -825,10 +800,34 @@ func (plugin *NetPlugin) createEndpointInternal(opt *createEndpointInternalOpt) return epInfo, plugin.Errorf(err.Error()) } + epInfos := []*network.EndpointInfo{&epInfo} + // get secondary interface info + for _, secondaryCniResult := range opt.ipamAddResult.secondaryInterfacesInfo { + var addresses []net.IPNet + var routes []network.RouteInfo + for _, ipconfig := range secondaryCniResult.ipResult.IPs { + addresses = append(addresses, ipconfig.Address) + } + for _, route := range secondaryCniResult.ipResult.Routes { + routes = append(routes, network.RouteInfo{Dst: route.Dst, Gw: route.GW}) + } + + epInfos = append(epInfos, + &network.EndpointInfo{ + ContainerID: epInfo.ContainerID, + NetNsPath: epInfo.NetNsPath, + IPAddresses: addresses, + Routes: routes, + MacAddress: secondaryCniResult.macAddress, + NICType: cns.DelegatedVMNIC, + SkipDefaultRoutes: secondaryCniResult.skipDefaultRoutes, + }) + } + // Create the endpoint. logger.Info("Creating endpoint", zap.String("endpointInfo", epInfo.PrettyString())) sendEvent(plugin, fmt.Sprintf("[cni-net] Creating endpoint %s.", epInfo.PrettyString())) - err = plugin.nm.CreateEndpoint(cnsclient, opt.nwInfo.Id, &epInfo) + err = plugin.nm.CreateEndpoint(cnsclient, opt.nwInfo.Id, epInfos) if err != nil { err = plugin.Errorf("Failed to create endpoint: %v", err) } diff --git a/cni/network/network_test.go b/cni/network/network_test.go index 9d72757557..0df2010a8b 100644 --- a/cni/network/network_test.go +++ b/cni/network/network_test.go @@ -10,6 +10,7 @@ import ( "github.com/Azure/azure-container-networking/cni" "github.com/Azure/azure-container-networking/cni/api" "github.com/Azure/azure-container-networking/cni/util" + "github.com/Azure/azure-container-networking/cns" "github.com/Azure/azure-container-networking/common" acnnetwork "github.com/Azure/azure-container-networking/network" "github.com/Azure/azure-container-networking/network/networkutils" @@ -75,9 +76,9 @@ func GetTestResources() *NetPlugin { grpcClient := &nns.MockGrpcClient{} plugin, _ := NewPlugin(pluginName, config, grpcClient, &Multitenancy{}) plugin.report = &telemetry.CNIReport{} - mockNetworkManager := acnnetwork.NewMockNetworkmanager() + mockNetworkManager := acnnetwork.NewMockNetworkmanager(acnnetwork.NewMockEndpointClient(nil)) plugin.nm = mockNetworkManager - plugin.ipamInvoker = NewMockIpamInvoker(isIPv6, false, false) + plugin.ipamInvoker = NewMockIpamInvoker(isIPv6, false, false, false, false) return plugin } @@ -310,6 +311,7 @@ func TestIpamAddFail(t *testing.T) { methods []string cniArgs []cniSkel.CmdArgs wantErr []bool + wantEndpointErr bool wantErrMsg string expectedEndpoints int }{ @@ -366,6 +368,23 @@ func TestIpamAddFail(t *testing.T) { wantErrMsg: "v4 fail", expectedEndpoints: 1, }, + { + name: "cleanup ipam add fail", + methods: []string{CNI_ADD}, + cniArgs: []cniSkel.CmdArgs{ + { + ContainerID: "test1-container", + Netns: "test1-container", + StdinData: nwCfg.Serialize(), + Args: fmt.Sprintf("K8S_POD_NAME=%v;K8S_POD_NAMESPACE=%v", "container1", "container1-ns"), + IfName: eth0IfName, + }, + }, + wantErr: []bool{false}, + wantEndpointErr: true, + wantErrMsg: "Failed to create endpoint: MockEndpointClient Error : Endpoint Error", + expectedEndpoints: 0, + }, } for _, tt := range tests { @@ -375,9 +394,15 @@ func TestIpamAddFail(t *testing.T) { for i, method := range tt.methods { fmt.Println("method", method, "wanterr", tt.wantErr[i]) if tt.wantErr[i] { - plugin.ipamInvoker = NewMockIpamInvoker(false, true, false) + plugin.ipamInvoker = NewMockIpamInvoker(false, true, false, false, false) } else { - plugin.ipamInvoker = NewMockIpamInvoker(false, false, false) + plugin.ipamInvoker = NewMockIpamInvoker(false, false, false, false, false) + } + + if tt.wantEndpointErr { + plugin.nm = acnnetwork.NewMockNetworkmanager(acnnetwork.NewMockEndpointClient(func(*acnnetwork.EndpointInfo) error { + return acnnetwork.NewErrorMockEndpointClient("Endpoint Error") //nolint:wrapcheck // ignore wrapping for test + })) } if method == CNI_ADD { @@ -386,12 +411,16 @@ func TestIpamAddFail(t *testing.T) { err = plugin.Delete(&tt.cniArgs[i]) } - if tt.wantErr[i] { + if tt.wantErr[i] || tt.wantEndpointErr { require.Error(t, err) assert.Contains(t, err.Error(), tt.wantErrMsg) } else { require.NoError(t, err) } + + if tt.wantEndpointErr { + assert.Len(t, plugin.ipamInvoker.(*MockIpamInvoker).ipMap, 0) + } } }) @@ -430,7 +459,7 @@ func TestIpamDeleteFail(t *testing.T) { err := plugin.Add(tt.args) require.NoError(t, err) - plugin.ipamInvoker = NewMockIpamInvoker(false, true, false) + plugin.ipamInvoker = NewMockIpamInvoker(false, true, false, false, false) err = plugin.Delete(args) if tt.wantErr { require.Error(t, err) @@ -459,8 +488,8 @@ func TestAddDualStack(t *testing.T) { name: "Dualstack happy path", plugin: &NetPlugin{ Plugin: cniPlugin, - nm: acnnetwork.NewMockNetworkmanager(), - ipamInvoker: NewMockIpamInvoker(true, false, false), + nm: acnnetwork.NewMockNetworkmanager(acnnetwork.NewMockEndpointClient(nil)), + ipamInvoker: NewMockIpamInvoker(true, false, false, false, false), report: &telemetry.CNIReport{}, tb: &telemetry.TelemetryBuffer{}, }, @@ -470,8 +499,8 @@ func TestAddDualStack(t *testing.T) { name: "Dualstack ipv6 fail", plugin: &NetPlugin{ Plugin: cniPlugin, - nm: acnnetwork.NewMockNetworkmanager(), - ipamInvoker: NewMockIpamInvoker(true, false, true), + nm: acnnetwork.NewMockNetworkmanager(acnnetwork.NewMockEndpointClient(nil)), + ipamInvoker: NewMockIpamInvoker(true, false, true, false, false), report: &telemetry.CNIReport{}, tb: &telemetry.TelemetryBuffer{}, }, @@ -516,8 +545,8 @@ func TestPluginGet(t *testing.T) { methods: []string{CNI_ADD, "GET"}, plugin: &NetPlugin{ Plugin: plugin, - nm: acnnetwork.NewMockNetworkmanager(), - ipamInvoker: NewMockIpamInvoker(false, false, false), + nm: acnnetwork.NewMockNetworkmanager(acnnetwork.NewMockEndpointClient(nil)), + ipamInvoker: NewMockIpamInvoker(false, false, false, false, false), report: &telemetry.CNIReport{}, tb: &telemetry.TelemetryBuffer{}, }, @@ -528,8 +557,8 @@ func TestPluginGet(t *testing.T) { methods: []string{"GET"}, plugin: &NetPlugin{ Plugin: plugin, - nm: acnnetwork.NewMockNetworkmanager(), - ipamInvoker: NewMockIpamInvoker(false, false, false), + nm: acnnetwork.NewMockNetworkmanager(acnnetwork.NewMockEndpointClient(nil)), + ipamInvoker: NewMockIpamInvoker(false, false, false, false, false), report: &telemetry.CNIReport{}, tb: &telemetry.TelemetryBuffer{}, }, @@ -541,8 +570,8 @@ func TestPluginGet(t *testing.T) { methods: []string{CNI_ADD, CNI_DEL, "GET"}, plugin: &NetPlugin{ Plugin: plugin, - nm: acnnetwork.NewMockNetworkmanager(), - ipamInvoker: NewMockIpamInvoker(false, false, false), + nm: acnnetwork.NewMockNetworkmanager(acnnetwork.NewMockEndpointClient(nil)), + ipamInvoker: NewMockIpamInvoker(false, false, false, false, false), report: &telemetry.CNIReport{}, tb: &telemetry.TelemetryBuffer{}, }, @@ -604,7 +633,7 @@ func TestPluginMultitenancyAdd(t *testing.T) { name: "Add Happy path", plugin: &NetPlugin{ Plugin: plugin, - nm: acnnetwork.NewMockNetworkmanager(), + nm: acnnetwork.NewMockNetworkmanager(acnnetwork.NewMockEndpointClient(nil)), tb: &telemetry.TelemetryBuffer{}, report: &telemetry.CNIReport{}, multitenancyClient: NewMockMultitenancy(false), @@ -623,7 +652,7 @@ func TestPluginMultitenancyAdd(t *testing.T) { name: "Add Fail", plugin: &NetPlugin{ Plugin: plugin, - nm: acnnetwork.NewMockNetworkmanager(), + nm: acnnetwork.NewMockNetworkmanager(acnnetwork.NewMockEndpointClient(nil)), tb: &telemetry.TelemetryBuffer{}, report: &telemetry.CNIReport{}, multitenancyClient: NewMockMultitenancy(true), @@ -735,7 +764,7 @@ func TestPluginBaremetalAdd(t *testing.T) { name: "Baremetal Add Happy path", plugin: &NetPlugin{ Plugin: plugin, - nm: acnnetwork.NewMockNetworkmanager(), + nm: acnnetwork.NewMockNetworkmanager(acnnetwork.NewMockEndpointClient(nil)), tb: &telemetry.TelemetryBuffer{}, report: &telemetry.CNIReport{}, nnsClient: &nns.MockGrpcClient{}, @@ -753,7 +782,7 @@ func TestPluginBaremetalAdd(t *testing.T) { name: "Baremetal Add Fail", plugin: &NetPlugin{ Plugin: plugin, - nm: acnnetwork.NewMockNetworkmanager(), + nm: acnnetwork.NewMockNetworkmanager(acnnetwork.NewMockEndpointClient(nil)), tb: &telemetry.TelemetryBuffer{}, report: &telemetry.CNIReport{}, nnsClient: &nns.MockGrpcClient{Fail: true}, @@ -1013,13 +1042,13 @@ func TestGetAllEndpointState(t *testing.T) { ep2 := getTestEndpoint("podname2", "podnamespace2", "10.0.0.2/24", "podinterfaceid2", "testcontainerid2") ep3 := getTestEndpoint("podname3", "podnamespace3", "10.240.1.242/16", "podinterfaceid3", "testcontainerid3") - err := plugin.nm.CreateEndpoint(nil, networkid, ep1) + err := plugin.nm.CreateEndpoint(nil, networkid, []*acnnetwork.EndpointInfo{ep1}) require.NoError(t, err) - err = plugin.nm.CreateEndpoint(nil, networkid, ep2) + err = plugin.nm.CreateEndpoint(nil, networkid, []*acnnetwork.EndpointInfo{ep2}) require.NoError(t, err) - err = plugin.nm.CreateEndpoint(nil, networkid, ep3) + err = plugin.nm.CreateEndpoint(nil, networkid, []*acnnetwork.EndpointInfo{ep3}) require.NoError(t, err) state, err := plugin.GetAllEndpointState(networkid) @@ -1103,3 +1132,103 @@ func TestGetPodSubnetNatInfo(t *testing.T) { require.Empty(t, natInfo, "linux podsubnet natInfo should be empty") } } + +func TestPluginSwiftV2Add(t *testing.T) { + plugin, _ := cni.NewPlugin("name", "0.3.0") + + localNwCfg := cni.NetworkConfig{ + CNIVersion: "0.3.0", + Name: "swiftv2", + ExecutionMode: string(util.V4Overlay), + EnableExactMatchForPodName: true, + Master: "eth0", + } + + tests := []struct { + name string + plugin *NetPlugin + args *cniSkel.CmdArgs + wantErr bool + wantErrMsg string + }{ + { + name: "SwiftV2 Add Happy path", + plugin: &NetPlugin{ + Plugin: plugin, + nm: acnnetwork.NewMockNetworkmanager(acnnetwork.NewMockEndpointClient(nil)), + ipamInvoker: NewMockIpamInvoker(false, false, false, true, false), + report: &telemetry.CNIReport{}, + tb: &telemetry.TelemetryBuffer{}, + }, + args: &cniSkel.CmdArgs{ + StdinData: localNwCfg.Serialize(), + ContainerID: "test-container", + Netns: "test-container", + Args: fmt.Sprintf("K8S_POD_NAME=%v;K8S_POD_NAMESPACE=%v", "test-pod", "test-pod-ns"), + IfName: eth0IfName, + }, + wantErr: false, + }, + { + name: "SwiftV2 Invoker Add fail", + plugin: &NetPlugin{ + Plugin: plugin, + nm: acnnetwork.NewMockNetworkmanager(acnnetwork.NewMockEndpointClient(nil)), + ipamInvoker: NewMockIpamInvoker(false, false, false, true, true), + report: &telemetry.CNIReport{}, + tb: &telemetry.TelemetryBuffer{}, + }, + args: &cniSkel.CmdArgs{ + StdinData: localNwCfg.Serialize(), + ContainerID: "test-container", + Netns: "test-container", + Args: fmt.Sprintf("K8S_POD_NAME=%v;K8S_POD_NAMESPACE=%v", "test-pod", "test-pod-ns"), + IfName: eth0IfName, + }, + wantErr: true, + wantErrMsg: "IPAM Invoker Add failed with error: delegatedVMNIC fail", + }, + { + name: "SwiftV2 EndpointClient Add fail", + plugin: &NetPlugin{ + Plugin: plugin, + nm: acnnetwork.NewMockNetworkmanager(acnnetwork.NewMockEndpointClient(func(ep *acnnetwork.EndpointInfo) error { + if ep.NICType == cns.DelegatedVMNIC { + return acnnetwork.NewErrorMockEndpointClient("AddEndpoints Delegated VM NIC failed") //nolint:wrapcheck // ignore wrapping for test + } + + return nil + })), + ipamInvoker: NewMockIpamInvoker(false, false, false, true, false), + report: &telemetry.CNIReport{}, + tb: &telemetry.TelemetryBuffer{}, + }, + args: &cniSkel.CmdArgs{ + StdinData: localNwCfg.Serialize(), + ContainerID: "test-container", + Netns: "test-container", + Args: fmt.Sprintf("K8S_POD_NAME=%v;K8S_POD_NAMESPACE=%v", "test-pod", "test-pod-ns"), + IfName: eth0IfName, + }, + wantErr: true, + wantErrMsg: "Failed to create endpoint: MockEndpointClient Error : AddEndpoints Delegated VM NIC failed", + }, + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + err := tt.plugin.Add(tt.args) + if tt.wantErr { + require.Error(t, err) + assert.Equal(t, err.Error(), tt.wantErrMsg, "Expected %v but got %+v", tt.wantErrMsg, err.Error()) + endpoints, _ := tt.plugin.nm.GetAllEndpoints(localNwCfg.Name) + require.Condition(t, assert.Comparison(func() bool { return len(endpoints) == 0 })) + } else { + require.NoError(t, err) + endpoints, _ := tt.plugin.nm.GetAllEndpoints(localNwCfg.Name) + require.Condition(t, assert.Comparison(func() bool { return len(endpoints) == 1 })) + } + }) + } +} diff --git a/cni/network/network_windows.go b/cni/network/network_windows.go index 8b27f9e4a4..e8b0e54984 100644 --- a/cni/network/network_windows.go +++ b/cni/network/network_windows.go @@ -161,7 +161,7 @@ func (plugin *NetPlugin) getNetworkName(netNs string, ipamAddResult *IPAMAddResu // This will happen during ADD call if ipamAddResult != nil && ipamAddResult.ncResponse != nil { // networkName will look like ~ azure-vlan1-172-28-1-0_24 - ipAddrNet := ipamAddResult.ipv4Result.IPs[0].Address + ipAddrNet := ipamAddResult.defaultInterfaceInfo.ipResult.IPs[0].Address prefix, err := netip.ParsePrefix(ipAddrNet.String()) if err != nil { logger.Error("Error parsing network CIDR", diff --git a/cni/network/network_windows_test.go b/cni/network/network_windows_test.go index 3b4e92990b..48cb9f0c53 100644 --- a/cni/network/network_windows_test.go +++ b/cni/network/network_windows_test.go @@ -91,8 +91,8 @@ func TestPluginSecondAddSamePodWindows(t *testing.T) { }, plugin: &NetPlugin{ Plugin: plugin, - nm: network.NewMockNetworkmanager(), - ipamInvoker: NewMockIpamInvoker(false, false, false), + nm: network.NewMockNetworkmanager(network.NewMockEndpointClient(nil)), + ipamInvoker: NewMockIpamInvoker(false, false, false, false, false), report: &telemetry.CNIReport{}, tb: &telemetry.TelemetryBuffer{}, }, @@ -110,8 +110,8 @@ func TestPluginSecondAddSamePodWindows(t *testing.T) { }, plugin: &NetPlugin{ Plugin: plugin, - nm: network.NewMockNetworkmanager(), - ipamInvoker: NewMockIpamInvoker(false, false, false), + nm: network.NewMockNetworkmanager(network.NewMockEndpointClient(nil)), + ipamInvoker: NewMockIpamInvoker(false, false, false, false, false), report: &telemetry.CNIReport{}, tb: &telemetry.TelemetryBuffer{}, }, @@ -350,8 +350,8 @@ func TestGetNetworkNameFromCNS(t *testing.T) { name: "Get Network Name from CNS with correct CIDR", plugin: &NetPlugin{ Plugin: plugin, - nm: network.NewMockNetworkmanager(), - ipamInvoker: NewMockIpamInvoker(false, false, false), + nm: network.NewMockNetworkmanager(network.NewMockEndpointClient(nil)), + ipamInvoker: NewMockIpamInvoker(false, false, false, false, false), report: &telemetry.CNIReport{}, tb: &telemetry.TelemetryBuffer{}, }, @@ -367,12 +367,14 @@ func TestGetNetworkNameFromCNS(t *testing.T) { ID: 1, }, }, - ipv4Result: &cniTypesCurr.Result{ - IPs: []*cniTypesCurr.IPConfig{ - { - Address: net.IPNet{ - IP: net.ParseIP("10.240.0.5"), - Mask: net.CIDRMask(24, 32), + defaultInterfaceInfo: InterfaceInfo{ + ipResult: &cniTypesCurr.Result{ + IPs: []*cniTypesCurr.IPConfig{ + { + Address: net.IPNet{ + IP: net.ParseIP("10.240.0.5"), + Mask: net.CIDRMask(24, 32), + }, }, }, }, @@ -385,8 +387,8 @@ func TestGetNetworkNameFromCNS(t *testing.T) { name: "Get Network Name from CNS with malformed CIDR #1", plugin: &NetPlugin{ Plugin: plugin, - nm: network.NewMockNetworkmanager(), - ipamInvoker: NewMockIpamInvoker(false, false, false), + nm: network.NewMockNetworkmanager(network.NewMockEndpointClient(nil)), + ipamInvoker: NewMockIpamInvoker(false, false, false, false, false), report: &telemetry.CNIReport{}, tb: &telemetry.TelemetryBuffer{}, }, @@ -402,12 +404,14 @@ func TestGetNetworkNameFromCNS(t *testing.T) { ID: 1, }, }, - ipv4Result: &cniTypesCurr.Result{ - IPs: []*cniTypesCurr.IPConfig{ - { - Address: net.IPNet{ - IP: net.ParseIP(""), - Mask: net.CIDRMask(24, 32), + defaultInterfaceInfo: InterfaceInfo{ + ipResult: &cniTypesCurr.Result{ + IPs: []*cniTypesCurr.IPConfig{ + { + Address: net.IPNet{ + IP: net.ParseIP(""), + Mask: net.CIDRMask(24, 32), + }, }, }, }, @@ -420,8 +424,8 @@ func TestGetNetworkNameFromCNS(t *testing.T) { name: "Get Network Name from CNS with malformed CIDR #2", plugin: &NetPlugin{ Plugin: plugin, - nm: network.NewMockNetworkmanager(), - ipamInvoker: NewMockIpamInvoker(false, false, false), + nm: network.NewMockNetworkmanager(network.NewMockEndpointClient(nil)), + ipamInvoker: NewMockIpamInvoker(false, false, false, false, false), report: &telemetry.CNIReport{}, tb: &telemetry.TelemetryBuffer{}, }, @@ -437,12 +441,14 @@ func TestGetNetworkNameFromCNS(t *testing.T) { ID: 1, }, }, - ipv4Result: &cniTypesCurr.Result{ - IPs: []*cniTypesCurr.IPConfig{ - { - Address: net.IPNet{ - IP: net.ParseIP("10.0.00.6"), - Mask: net.CIDRMask(24, 32), + defaultInterfaceInfo: InterfaceInfo{ + ipResult: &cniTypesCurr.Result{ + IPs: []*cniTypesCurr.IPConfig{ + { + Address: net.IPNet{ + IP: net.ParseIP("10.0.00.6"), + Mask: net.CIDRMask(24, 32), + }, }, }, }, @@ -455,8 +461,8 @@ func TestGetNetworkNameFromCNS(t *testing.T) { name: "Get Network Name from CNS without NetNS", plugin: &NetPlugin{ Plugin: plugin, - nm: network.NewMockNetworkmanager(), - ipamInvoker: NewMockIpamInvoker(false, false, false), + nm: network.NewMockNetworkmanager(network.NewMockEndpointClient(nil)), + ipamInvoker: NewMockIpamInvoker(false, false, false, false, false), report: &telemetry.CNIReport{}, tb: &telemetry.TelemetryBuffer{}, }, @@ -472,12 +478,14 @@ func TestGetNetworkNameFromCNS(t *testing.T) { ID: 1, }, }, - ipv4Result: &cniTypesCurr.Result{ - IPs: []*cniTypesCurr.IPConfig{ - { - Address: net.IPNet{ - IP: net.ParseIP("10.0.0.6"), - Mask: net.CIDRMask(24, 32), + defaultInterfaceInfo: InterfaceInfo{ + ipResult: &cniTypesCurr.Result{ + IPs: []*cniTypesCurr.IPConfig{ + { + Address: net.IPNet{ + IP: net.ParseIP("10.0.0.6"), + Mask: net.CIDRMask(24, 32), + }, }, }, }, @@ -490,8 +498,8 @@ func TestGetNetworkNameFromCNS(t *testing.T) { name: "Get Network Name from CNS without multitenancy", plugin: &NetPlugin{ Plugin: plugin, - nm: network.NewMockNetworkmanager(), - ipamInvoker: NewMockIpamInvoker(false, false, false), + nm: network.NewMockNetworkmanager(network.NewMockEndpointClient(nil)), + ipamInvoker: NewMockIpamInvoker(false, false, false, false, false), report: &telemetry.CNIReport{}, tb: &telemetry.TelemetryBuffer{}, }, @@ -503,12 +511,14 @@ func TestGetNetworkNameFromCNS(t *testing.T) { }, ipamAddResult: &IPAMAddResult{ ncResponse: &cns.GetNetworkContainerResponse{}, - ipv4Result: &cniTypesCurr.Result{ - IPs: []*cniTypesCurr.IPConfig{ - { - Address: net.IPNet{ - IP: net.ParseIP("10.0.0.6"), - Mask: net.CIDRMask(24, 32), + defaultInterfaceInfo: InterfaceInfo{ + ipResult: &cniTypesCurr.Result{ + IPs: []*cniTypesCurr.IPConfig{ + { + Address: net.IPNet{ + IP: net.ParseIP("10.0.0.6"), + Mask: net.CIDRMask(24, 32), + }, }, }, }, diff --git a/cnm/network/network.go b/cnm/network/network.go index 0f10c5b94b..85a4545a11 100644 --- a/cnm/network/network.go +++ b/cnm/network/network.go @@ -53,7 +53,7 @@ func NewPlugin(config *common.PluginConfig) (NetPlugin, error) { nl := netlink.NewNetlink() // Setup network manager. - nm, err := network.NewNetworkManager(nl, platform.NewExecClient(nil), &netio.NetIO{}) + nm, err := network.NewNetworkManager(nl, platform.NewExecClient(nil), &netio.NetIO{}, network.NewNamespaceClient()) if err != nil { return nil, err } @@ -246,7 +246,7 @@ func (plugin *netPlugin) createEndpoint(w http.ResponseWriter, r *http.Request) if err != nil { log.Errorf("failed to init CNS client", err) } - err = plugin.nm.CreateEndpoint(cnscli, req.NetworkID, &epInfo) + err = plugin.nm.CreateEndpoint(cnscli, req.NetworkID, []*network.EndpointInfo{&epInfo}) if err != nil { plugin.SendErrorResponse(w, err) return diff --git a/cnms/service/networkmonitor.go b/cnms/service/networkmonitor.go index ff99114b20..545442c8f2 100644 --- a/cnms/service/networkmonitor.go +++ b/cnms/service/networkmonitor.go @@ -157,7 +157,7 @@ func main() { } nl := netlink.NewNetlink() - nm, err := network.NewNetworkManager(nl, platform.NewExecClient(nil), &netio.NetIO{}) + nm, err := network.NewNetworkManager(nl, platform.NewExecClient(nil), &netio.NetIO{}, network.NewNamespaceClient()) if err != nil { log.Printf("[monitor] Failed while creating network manager") return diff --git a/netio/mocknetio.go b/netio/mocknetio.go index b002e4dde4..b80a44bbb1 100644 --- a/netio/mocknetio.go +++ b/netio/mocknetio.go @@ -1,6 +1,7 @@ package netio import ( + "bytes" "errors" "fmt" "net" @@ -16,7 +17,10 @@ type MockNetIO struct { } // ErrMockNetIOFail - mock netio error -var ErrMockNetIOFail = errors.New("netio fail") +var ( + ErrMockNetIOFail = errors.New("netio fail") + hwAddr, _ = net.ParseMAC("ab:cd:ef:12:34:56") +) func NewMockNetIO(fail bool, failAttempt int) *MockNetIO { return &MockNetIO{ @@ -40,8 +44,6 @@ func (netshim *MockNetIO) GetNetworkInterfaceByName(name string) (*net.Interface return netshim.getInterfaceFn(name) } - hwAddr, _ := net.ParseMAC("ab:cd:ef:12:34:56") - return &net.Interface{ //nolint:gomnd // Dummy MTU MTU: 1000, @@ -55,3 +57,18 @@ func (netshim *MockNetIO) GetNetworkInterfaceByName(name string) (*net.Interface func (netshim *MockNetIO) GetNetworkInterfaceAddrs(iface *net.Interface) ([]net.Addr, error) { return []net.Addr{}, nil } + +func (netshim *MockNetIO) GetNetworkInterfaceByMac(mac net.HardwareAddr) (*net.Interface, error) { + if !bytes.Equal(mac, hwAddr) { + return nil, fmt.Errorf("%w: %s", ErrMockNetIOFail, mac) + } + + return &net.Interface{ + //nolint:gomnd // Dummy MTU + MTU: 1000, + Name: "eth1", + HardwareAddr: mac, + //nolint:gomnd // Dummy interface index + Index: 2, + }, nil +} diff --git a/netio/netio.go b/netio/netio.go index 0f2bf03274..9462c8ed33 100644 --- a/netio/netio.go +++ b/netio/netio.go @@ -1,6 +1,7 @@ package netio import ( + "bytes" "net" "github.com/pkg/errors" @@ -10,10 +11,12 @@ import ( type NetIOInterface interface { GetNetworkInterfaceByName(name string) (*net.Interface, error) GetNetworkInterfaceAddrs(iface *net.Interface) ([]net.Addr, error) + GetNetworkInterfaceByMac(mac net.HardwareAddr) (*net.Interface, error) } // ErrInterfaceNil - errors out when interface is nil var ErrInterfaceNil = errors.New("Interface is nil") +var ErrInterfaceNotFound = errors.New("Inteface not found") type NetIO struct{} @@ -30,3 +33,18 @@ func (ns *NetIO) GetNetworkInterfaceAddrs(iface *net.Interface) ([]net.Addr, err addrs, err := iface.Addrs() return addrs, errors.Wrap(err, "GetNetworkInterfaceAddrs failed") } + +func (ns *NetIO) GetNetworkInterfaceByMac(mac net.HardwareAddr) (*net.Interface, error) { + ifaces, err := net.Interfaces() + if err != nil { + return nil, errors.Wrap(err, "GetNetworkInterfaceByMac failed") + } + + for _, iface := range ifaces { + if bytes.Equal(iface.HardwareAddr, mac) { + return &iface, nil + } + } + + return nil, ErrInterfaceNotFound +} diff --git a/network/endpoint.go b/network/endpoint.go index a8fae33776..ab873e92d0 100644 --- a/network/endpoint.go +++ b/network/endpoint.go @@ -10,6 +10,7 @@ import ( "strings" "github.com/Azure/azure-container-networking/cni/log" + "github.com/Azure/azure-container-networking/cns" "github.com/Azure/azure-container-networking/netio" "github.com/Azure/azure-container-networking/netlink" "github.com/Azure/azure-container-networking/network/policy" @@ -52,6 +53,8 @@ type endpoint struct { PODNameSpace string `json:",omitempty"` InfraVnetAddressSpace string `json:",omitempty"` NetNs string `json:",omitempty"` + // SecondaryInterfaces is a map of interface name to InterfaceInfo + SecondaryInterfaces map[string]*InterfaceInfo } // EndpointInfo contains read-only information about an endpoint. @@ -86,6 +89,8 @@ type EndpointInfo struct { VnetCidrs string ServiceCidrs string NATInfo []policy.NATInfo + NICType cns.NICType + SkipDefaultRoutes bool } // RouteInfo contains information about an IP route. @@ -100,6 +105,16 @@ type RouteInfo struct { Table int } +// InterfaceInfo contains information for secondary interfaces +type InterfaceInfo struct { + Name string + MacAddress net.HardwareAddr + IPAddress []net.IPNet + Routes []RouteInfo + NICType cns.NICType + SkipDefaultRoutes bool +} + type apipaClient interface { DeleteHostNCApipaEndpoint(ctx context.Context, networkContainerID string) error CreateHostNCApipaEndpoint(ctx context.Context, networkContainerID string) (string, error) @@ -117,31 +132,32 @@ func (nw *network) newEndpoint( nl netlink.NetlinkInterface, plc platform.ExecClient, netioCli netio.NetIOInterface, - epInfo *EndpointInfo, + nsc NamespaceClientInterface, + epInfo []*EndpointInfo, ) (*endpoint, error) { var ep *endpoint var err error defer func() { if err != nil { - logger.Error("Failed to create endpoint with err", zap.String("id", epInfo.Id), zap.Error(err)) + logger.Error("Failed to create endpoint with err", zap.String("id", epInfo[0].Id), zap.Error(err)) } }() // Call the platform implementation. // Pass nil for epClient and will be initialized in newendpointImpl - ep, err = nw.newEndpointImpl(apipaCli, nl, plc, netioCli, nil, epInfo) + ep, err = nw.newEndpointImpl(apipaCli, nl, plc, netioCli, nil, nsc, epInfo) if err != nil { return nil, err } - nw.Endpoints[epInfo.Id] = ep + nw.Endpoints[ep.Id] = ep logger.Info("Created endpoint. Num of endpoints", zap.Any("ep", ep), zap.Int("numEndpoints", len(nw.Endpoints))) return ep, nil } // DeleteEndpoint deletes an existing endpoint from the network. -func (nw *network) deleteEndpoint(nl netlink.NetlinkInterface, plc platform.ExecClient, endpointID string) error { +func (nw *network) deleteEndpoint(nl netlink.NetlinkInterface, plc platform.ExecClient, nsc NamespaceClientInterface, endpointID string) error { var err error logger.Info("Deleting endpoint from network", zap.String("endpointID", endpointID), zap.String("id", nw.Id)) @@ -160,7 +176,7 @@ func (nw *network) deleteEndpoint(nl netlink.NetlinkInterface, plc platform.Exec // Call the platform implementation. // Pass nil for epClient and will be initialized in deleteEndpointImpl - err = nw.deleteEndpointImpl(nl, plc, nil, ep) + err = nw.deleteEndpointImpl(nl, plc, nil, nsc, ep) if err != nil { return err } diff --git a/network/endpoint_linux.go b/network/endpoint_linux.go index 5a55aa520b..3ceda20460 100644 --- a/network/endpoint_linux.go +++ b/network/endpoint_linux.go @@ -10,6 +10,7 @@ import ( "net" "strings" + "github.com/Azure/azure-container-networking/cns" "github.com/Azure/azure-container-networking/netio" "github.com/Azure/azure-container-networking/netlink" "github.com/Azure/azure-container-networking/network/networkutils" @@ -53,36 +54,38 @@ func (nw *network) newEndpointImpl( nl netlink.NetlinkInterface, plc platform.ExecClient, netioCli netio.NetIOInterface, - epClient EndpointClient, - epInfo *EndpointInfo, + testEpClient EndpointClient, + nsc NamespaceClientInterface, + epInfo []*EndpointInfo, ) (*endpoint, error) { - var containerIf *net.Interface - var ns *Namespace - var ep *endpoint - var err error - var hostIfName string - var contIfName string - var localIP string - var vlanid int = 0 - - if nw.Endpoints[epInfo.Id] != nil { - logger.Info("Endpoint alreday exists") + var ( + err error + hostIfName string + contIfName string + localIP string + vlanid = 0 + defaultEpInfo = epInfo[0] + containerIf *net.Interface + ) + + if nw.Endpoints[defaultEpInfo.Id] != nil { + logger.Info("[net] Endpoint already exists.") err = errEndpointExists return nil, err } - if epInfo.Data != nil { - if _, ok := epInfo.Data[VlanIDKey]; ok { - vlanid = epInfo.Data[VlanIDKey].(int) + if defaultEpInfo.Data != nil { + if _, ok := defaultEpInfo.Data[VlanIDKey]; ok { + vlanid = defaultEpInfo.Data[VlanIDKey].(int) } - if _, ok := epInfo.Data[LocalIPKey]; ok { - localIP = epInfo.Data[LocalIPKey].(string) + if _, ok := defaultEpInfo.Data[LocalIPKey]; ok { + localIP = defaultEpInfo.Data[LocalIPKey].(string) } } - if _, ok := epInfo.Data[OptVethName]; ok { - key := epInfo.Data[OptVethName].(string) + if _, ok := defaultEpInfo.Data[OptVethName]; ok { + key := defaultEpInfo.Data[OptVethName].(string) logger.Info("Generate veth name based on the key provided", zap.String("key", key)) vethname := generateVethName(key) hostIfName = fmt.Sprintf("%s%s", hostVEthInterfacePrefix, vethname) @@ -90,173 +93,169 @@ func (nw *network) newEndpointImpl( } else { // Create a veth pair. logger.Info("Generate veth name based on endpoint id") - hostIfName = fmt.Sprintf("%s%s", hostVEthInterfacePrefix, epInfo.Id[:7]) - contIfName = fmt.Sprintf("%s%s-2", hostVEthInterfacePrefix, epInfo.Id[:7]) + hostIfName = fmt.Sprintf("%s%s", hostVEthInterfacePrefix, defaultEpInfo.Id[:7]) + contIfName = fmt.Sprintf("%s%s-2", hostVEthInterfacePrefix, defaultEpInfo.Id[:7]) } - // epClient is non-nil only when the endpoint is created for the unit test. - if epClient == nil { - //nolint:gocritic - if vlanid != 0 { - if nw.Mode == opModeTransparentVlan { - logger.Info("Transparent vlan client") - if _, ok := epInfo.Data[SnatBridgeIPKey]; ok { - nw.SnatBridgeIP = epInfo.Data[SnatBridgeIPKey].(string) + ep := &endpoint{ + Id: defaultEpInfo.Id, + IfName: contIfName, // container veth pair name. In cnm, we won't rename this and docker expects veth name. + HostIfName: hostIfName, + InfraVnetIP: defaultEpInfo.InfraVnetIP, + LocalIP: localIP, + IPAddresses: defaultEpInfo.IPAddresses, + DNS: defaultEpInfo.DNS, + VlanID: vlanid, + EnableSnatOnHost: defaultEpInfo.EnableSnatOnHost, + EnableInfraVnet: defaultEpInfo.EnableInfraVnet, + EnableMultitenancy: defaultEpInfo.EnableMultiTenancy, + AllowInboundFromHostToNC: defaultEpInfo.AllowInboundFromHostToNC, + AllowInboundFromNCToHost: defaultEpInfo.AllowInboundFromNCToHost, + NetworkNameSpace: defaultEpInfo.NetNsPath, + ContainerID: defaultEpInfo.ContainerID, + PODName: defaultEpInfo.PODName, + PODNameSpace: defaultEpInfo.PODNameSpace, + Routes: defaultEpInfo.Routes, + SecondaryInterfaces: make(map[string]*InterfaceInfo), + } + if nw.extIf != nil { + ep.Gateways = []net.IP{nw.extIf.IPv4Gateway} + } + + for _, epInfo := range epInfo { + // testEpClient is non-nil only when the endpoint is created for the unit test + // resetting epClient to testEpClient in loop to use the test endpoint client if specified + epClient := testEpClient + if epClient == nil { + //nolint:gocritic + if vlanid != 0 { + if nw.Mode == opModeTransparentVlan { + logger.Info("Transparent vlan client") + if _, ok := epInfo.Data[SnatBridgeIPKey]; ok { + nw.SnatBridgeIP = epInfo.Data[SnatBridgeIPKey].(string) + } + epClient = NewTransparentVlanEndpointClient(nw, epInfo, hostIfName, contIfName, vlanid, localIP, nl, plc, nsc) + } else { + logger.Info("OVS client") + if _, ok := epInfo.Data[SnatBridgeIPKey]; ok { + nw.SnatBridgeIP = epInfo.Data[SnatBridgeIPKey].(string) + } + + epClient = NewOVSEndpointClient( + nw, + epInfo, + hostIfName, + contIfName, + vlanid, + localIP, + nl, + ovsctl.NewOvsctl(), + plc) } - epClient = NewTransparentVlanEndpointClient(nw, epInfo, hostIfName, contIfName, vlanid, localIP, nl, plc) + } else if nw.Mode != opModeTransparent { + logger.Info("Bridge client") + epClient = NewLinuxBridgeEndpointClient(nw.extIf, hostIfName, contIfName, nw.Mode, nl, plc) + } else if epInfo.NICType == cns.DelegatedVMNIC { + logger.Info("Secondary client") + epClient = NewSecondaryEndpointClient(nl, plc, nsc, ep) } else { - logger.Info("OVS client") - if _, ok := epInfo.Data[SnatBridgeIPKey]; ok { - nw.SnatBridgeIP = epInfo.Data[SnatBridgeIPKey].(string) - } - - epClient = NewOVSEndpointClient( - nw, - epInfo, - hostIfName, - contIfName, - vlanid, - localIP, - nl, - ovsctl.NewOvsctl(), - plc) + logger.Info("Transparent client") + epClient = NewTransparentEndpointClient(nw.extIf, hostIfName, contIfName, nw.Mode, nl, plc) } - } else if nw.Mode != opModeTransparent { - logger.Info("Bridge client") - epClient = NewLinuxBridgeEndpointClient(nw.extIf, hostIfName, contIfName, nw.Mode, nl, plc) - } else { - logger.Info("Transparent client") - epClient = NewTransparentEndpointClient(nw.extIf, hostIfName, contIfName, nw.Mode, nl, plc) } - } - // Cleanup on failure. - defer func() { - if err != nil { - logger.Error("CNI error. Delete Endpoint and rules that are created", zap.Error(err), zap.String("contIfName", contIfName)) - endpt := &endpoint{ - Id: epInfo.Id, - IfName: contIfName, - HostIfName: hostIfName, - LocalIP: localIP, - IPAddresses: epInfo.IPAddresses, - DNS: epInfo.DNS, - VlanID: vlanid, - EnableSnatOnHost: epInfo.EnableSnatOnHost, - EnableMultitenancy: epInfo.EnableMultiTenancy, - AllowInboundFromHostToNC: epInfo.AllowInboundFromHostToNC, - AllowInboundFromNCToHost: epInfo.AllowInboundFromNCToHost, + //nolint:gocritic + defer func(client EndpointClient, contIfName string) { + // Cleanup on failure. + if err != nil { + logger.Error("CNI error. Delete Endpoint and rules that are created", zap.Error(err), zap.String("contIfName", contIfName)) + if containerIf != nil { + client.DeleteEndpointRules(ep) + } + // set deleteHostVeth to true to cleanup host veth interface if created + //nolint:errcheck // ignore error + client.DeleteEndpoints(ep) } + }(epClient, contIfName) - if containerIf != nil { - endpt.MacAddress = containerIf.HardwareAddr - epClient.DeleteEndpointRules(endpt) + // wrapping endpoint client commands in anonymous func so that namespace can be exit and closed before the next loop + //nolint:wrapcheck // ignore wrap check + err = func() error { + if epErr := epClient.AddEndpoints(epInfo); epErr != nil { + return epErr } - if nw.extIf != nil { - endpt.Gateways = []net.IP{nw.extIf.IPv4Gateway} + if epInfo.NICType == cns.InfraNIC { + var epErr error + containerIf, epErr = netioCli.GetNetworkInterfaceByName(contIfName) + if epErr != nil { + return epErr + } + ep.MacAddress = containerIf.HardwareAddr } - // set deleteHostVeth to true to cleanup host veth interface if created - //nolint:errcheck // ignore error - epClient.DeleteEndpoints(endpt) - } - }() - if err = epClient.AddEndpoints(epInfo); err != nil { - return nil, err - } - - containerIf, err = netioCli.GetNetworkInterfaceByName(contIfName) - if err != nil { - return nil, err - } + // Setup rules for IP addresses on the container interface. + if epErr := epClient.AddEndpointRules(epInfo); epErr != nil { + return epErr + } - // Setup rules for IP addresses on the container interface. - if err = epClient.AddEndpointRules(epInfo); err != nil { - return nil, err - } + // If a network namespace for the container interface is specified... + if epInfo.NetNsPath != "" { + // Open the network namespace. + logger.Info("Opening netns", zap.Any("NetNsPath", epInfo.NetNsPath)) + ns, epErr := nsc.OpenNamespace(epInfo.NetNsPath) + if epErr != nil { + return epErr + } + defer ns.Close() - // If a network namespace for the container interface is specified... - if epInfo.NetNsPath != "" { - // Open the network namespace. - logger.Info("Opening netns", zap.Any("NetNsPath", epInfo.NetNsPath)) - ns, err = OpenNamespace(epInfo.NetNsPath) - if err != nil { - return nil, err - } - defer ns.Close() + if epErr := epClient.MoveEndpointsToContainerNS(epInfo, ns.GetFd()); epErr != nil { + return epErr + } - if err := epClient.MoveEndpointsToContainerNS(epInfo, ns.GetFd()); err != nil { - return nil, err - } + // Enter the container network namespace. + logger.Info("Entering netns", zap.Any("NetNsPath", epInfo.NetNsPath)) + if epErr := ns.Enter(); epErr != nil { + return epErr + } - // Enter the container network namespace. - logger.Info("Entering netns", zap.Any("NetNsPath", epInfo.NetNsPath)) - if err = ns.Enter(); err != nil { - return nil, err - } + // Return to host network namespace. + defer func() { + logger.Info("Exiting netns", zap.Any("NetNsPath", epInfo.NetNsPath)) + if epErr := ns.Exit(); epErr != nil { + logger.Error("Failed to exit netns with", zap.Error(epErr)) + } + }() + } - // Return to host network namespace. - defer func() { - logger.Info("Exiting netns", zap.Any("NetNsPath", epInfo.NetNsPath)) - if err := ns.Exit(); err != nil { - logger.Error("Failed to exit netns with", zap.Error(err)) + if epInfo.IPV6Mode != "" { + // Enable ipv6 setting in container + logger.Info("Enable ipv6 setting in container.") + nuc := networkutils.NewNetworkUtils(nl, plc) + if epErr := nuc.UpdateIPV6Setting(0); epErr != nil { + return fmt.Errorf("Enable ipv6 in container failed:%w", epErr) + } } - }() - } - if epInfo.IPV6Mode != "" { - // Enable ipv6 setting in container - logger.Info("Enable ipv6 setting in container.") - nuc := networkutils.NewNetworkUtils(nl, plc) - if err = nuc.UpdateIPV6Setting(0); err != nil { - return nil, fmt.Errorf("Enable ipv6 in container failed:%w", err) - } - } + // If a name for the container interface is specified... + if epInfo.IfName != "" { + if epErr := epClient.SetupContainerInterfaces(epInfo); epErr != nil { + return epErr + } + } - // If a name for the container interface is specified... - if epInfo.IfName != "" { - if err = epClient.SetupContainerInterfaces(epInfo); err != nil { + return epClient.ConfigureContainerInterfacesAndRoutes(epInfo) + }() + if err != nil { return nil, err } } - if err = epClient.ConfigureContainerInterfacesAndRoutes(epInfo); err != nil { - return nil, err - } - - // Create the endpoint object. - ep = &endpoint{ - Id: epInfo.Id, - IfName: contIfName, // container veth pair name. In cnm, we won't rename this and docker expects veth name. - HostIfName: hostIfName, - MacAddress: containerIf.HardwareAddr, - InfraVnetIP: epInfo.InfraVnetIP, - LocalIP: localIP, - IPAddresses: epInfo.IPAddresses, - DNS: epInfo.DNS, - VlanID: vlanid, - EnableSnatOnHost: epInfo.EnableSnatOnHost, - EnableInfraVnet: epInfo.EnableInfraVnet, - EnableMultitenancy: epInfo.EnableMultiTenancy, - AllowInboundFromHostToNC: epInfo.AllowInboundFromHostToNC, - AllowInboundFromNCToHost: epInfo.AllowInboundFromNCToHost, - NetworkNameSpace: epInfo.NetNsPath, - ContainerID: epInfo.ContainerID, - PODName: epInfo.PODName, - PODNameSpace: epInfo.PODNameSpace, - } - - if nw.extIf != nil { - ep.Gateways = []net.IP{nw.extIf.IPv4Gateway} - } - - ep.Routes = append(ep.Routes, epInfo.Routes...) return ep, nil } // deleteEndpointImpl deletes an existing endpoint from the network. -func (nw *network) deleteEndpointImpl(nl netlink.NetlinkInterface, plc platform.ExecClient, epClient EndpointClient, ep *endpoint) error { +func (nw *network) deleteEndpointImpl(nl netlink.NetlinkInterface, plc platform.ExecClient, epClient EndpointClient, nsc NamespaceClientInterface, ep *endpoint) error { // Delete the veth pair by deleting one of the peer interfaces. // Deleting the host interface is more convenient since it does not require // entering the container netns and hence works both for CNI and CNM. @@ -268,7 +267,7 @@ func (nw *network) deleteEndpointImpl(nl netlink.NetlinkInterface, plc platform. epInfo := ep.getInfo() if nw.Mode == opModeTransparentVlan { logger.Info("Transparent vlan client") - epClient = NewTransparentVlanEndpointClient(nw, epInfo, ep.HostIfName, "", ep.VlanID, ep.LocalIP, nl, plc) + epClient = NewTransparentVlanEndpointClient(nw, epInfo, ep.HostIfName, "", ep.VlanID, ep.LocalIP, nl, plc, nsc) } else { epClient = NewOVSEndpointClient(nw, epInfo, ep.HostIfName, "", ep.VlanID, ep.LocalIP, nl, ovsctl.NewOvsctl(), plc) @@ -276,6 +275,13 @@ func (nw *network) deleteEndpointImpl(nl netlink.NetlinkInterface, plc platform. } else if nw.Mode != opModeTransparent { epClient = NewLinuxBridgeEndpointClient(nw.extIf, ep.HostIfName, "", nw.Mode, nl, plc) } else { + if len(ep.SecondaryInterfaces) > 0 { + epClient = NewSecondaryEndpointClient(nl, plc, nsc, ep) + epClient.DeleteEndpointRules(ep) + //nolint:errcheck // ignore error + epClient.DeleteEndpoints(ep) + } + epClient = NewTransparentEndpointClient(nw.extIf, ep.HostIfName, "", nw.Mode, nl, plc) } } @@ -384,16 +390,13 @@ func deleteRoutes(nl netlink.NetlinkInterface, netioshim netio.NetIOInterface, i // updateEndpointImpl updates an existing endpoint in the network. func (nm *networkManager) updateEndpointImpl(nw *network, existingEpInfo *EndpointInfo, targetEpInfo *EndpointInfo) (*endpoint, error) { - var ns *Namespace var ep *endpoint - var err error existingEpFromRepository := nw.Endpoints[existingEpInfo.Id] logger.Info("[updateEndpointImpl] Going to retrieve endpoint with Id to update", zap.String("id", existingEpInfo.Id)) if existingEpFromRepository == nil { logger.Info("[updateEndpointImpl] Endpoint cannot be updated as it does not exist") - err = errEndpointNotFound - return nil, err + return nil, errEndpointNotFound } netns := existingEpFromRepository.NetworkNameSpace @@ -401,7 +404,7 @@ func (nm *networkManager) updateEndpointImpl(nw *network, existingEpInfo *Endpoi if netns != "" { // Open the network namespace. logger.Info("[updateEndpointImpl] Opening netns", zap.Any("netns", netns)) - ns, err = OpenNamespace(netns) + ns, err := nm.nsClient.OpenNamespace(netns) if err != nil { return nil, err } @@ -423,12 +426,11 @@ func (nm *networkManager) updateEndpointImpl(nw *network, existingEpInfo *Endpoi } else { logger.Info("[updateEndpointImpl] Endpoint cannot be updated as the network namespace does not exist: Epid", zap.String("id", existingEpInfo.Id), zap.String("component", "updateEndpointImpl")) - err = errNamespaceNotFound - return nil, err + return nil, errNamespaceNotFound } logger.Info("[updateEndpointImpl] Going to update routes in netns", zap.Any("netns", netns)) - if err = nm.updateRoutes(existingEpInfo, targetEpInfo); err != nil { + if err := nm.updateRoutes(existingEpInfo, targetEpInfo); err != nil { return nil, err } diff --git a/network/endpoint_test.go b/network/endpoint_test.go index 5ad1e7ceef..14927125df 100644 --- a/network/endpoint_test.go +++ b/network/endpoint_test.go @@ -7,6 +7,7 @@ import ( "net" "testing" + "github.com/Azure/azure-container-networking/cns" "github.com/Azure/azure-container-networking/netio" "github.com/Azure/azure-container-networking/netlink" "github.com/Azure/azure-container-networking/platform" @@ -173,15 +174,16 @@ var _ = Describe("Test Endpoint", func() { Endpoints: map[string]*endpoint{}, } epInfo := &EndpointInfo{ - Id: "768e8deb-eth1", - Data: make(map[string]interface{}), + Id: "768e8deb-eth1", + Data: make(map[string]interface{}), + IfName: eth0IfName, } epInfo.Data[VlanIDKey] = 100 It("Should be added", func() { // Add endpoint with valid id ep, err := nw.newEndpointImpl(nil, netlink.NewMockNetlink(false, ""), platform.NewMockExecClient(false), - netio.NewMockNetIO(false, 0), NewMockEndpointClient(false), epInfo) + netio.NewMockNetIO(false, 0), NewMockEndpointClient(nil), NewMockNamespaceClient(), []*EndpointInfo{epInfo}) Expect(err).NotTo(HaveOccurred()) Expect(ep).NotTo(BeNil()) Expect(ep.Id).To(Equal(epInfo.Id)) @@ -193,7 +195,7 @@ var _ = Describe("Test Endpoint", func() { extIf: &externalInterface{IPv4Gateway: net.ParseIP("192.168.0.1")}, } ep, err := nw2.newEndpointImpl(nil, netlink.NewMockNetlink(false, ""), platform.NewMockExecClient(false), - netio.NewMockNetIO(false, 0), NewMockEndpointClient(false), epInfo) + netio.NewMockNetIO(false, 0), NewMockEndpointClient(nil), NewMockNamespaceClient(), []*EndpointInfo{epInfo}) Expect(err).NotTo(HaveOccurred()) Expect(ep).NotTo(BeNil()) Expect(ep.Id).To(Equal(epInfo.Id)) @@ -204,12 +206,12 @@ var _ = Describe("Test Endpoint", func() { }) It("Should be not added", func() { // Adding an endpoint with an id. - mockCli := NewMockEndpointClient(false) + mockCli := NewMockEndpointClient(nil) err := mockCli.AddEndpoints(epInfo) Expect(err).ToNot(HaveOccurred()) // Adding endpoint with same id should fail and delete should cleanup the state ep2, err := nw.newEndpointImpl(nil, netlink.NewMockNetlink(false, ""), platform.NewMockExecClient(false), - netio.NewMockNetIO(false, 0), mockCli, epInfo) + netio.NewMockNetIO(false, 0), mockCli, NewMockNamespaceClient(), []*EndpointInfo{epInfo}) Expect(err).To(HaveOccurred()) Expect(ep2).To(BeNil()) assert.Contains(GinkgoT(), err.Error(), "Endpoint already exists") @@ -217,19 +219,19 @@ var _ = Describe("Test Endpoint", func() { }) It("Should be deleted", func() { // Adding an endpoint with an id. - mockCli := NewMockEndpointClient(false) + mockCli := NewMockEndpointClient(nil) ep2, err := nw.newEndpointImpl(nil, netlink.NewMockNetlink(false, ""), platform.NewMockExecClient(false), - netio.NewMockNetIO(false, 0), mockCli, epInfo) + netio.NewMockNetIO(false, 0), mockCli, NewMockNamespaceClient(), []*EndpointInfo{epInfo}) Expect(err).ToNot(HaveOccurred()) Expect(ep2).ToNot(BeNil()) Expect(len(mockCli.endpoints)).To(Equal(1)) // Deleting the endpoint //nolint:errcheck // ignore error - nw.deleteEndpointImpl(netlink.NewMockNetlink(false, ""), platform.NewMockExecClient(false), mockCli, ep2) + nw.deleteEndpointImpl(netlink.NewMockNetlink(false, ""), platform.NewMockExecClient(false), mockCli, NewMockNamespaceClient(), ep2) Expect(len(mockCli.endpoints)).To(Equal(0)) // Deleting same endpoint with same id should not fail //nolint:errcheck // ignore error - nw.deleteEndpointImpl(netlink.NewMockNetlink(false, ""), platform.NewMockExecClient(false), mockCli, ep2) + nw.deleteEndpointImpl(netlink.NewMockNetlink(false, ""), platform.NewMockExecClient(false), mockCli, NewMockNamespaceClient(), ep2) Expect(len(mockCli.endpoints)).To(Equal(0)) }) }) @@ -239,14 +241,22 @@ var _ = Describe("Test Endpoint", func() { Endpoints: map[string]*endpoint{}, } epInfo := &EndpointInfo{ - Id: "768e8deb-eth1", + Id: "768e8deb-eth1", + IfName: eth0IfName, + NICType: cns.InfraNIC, } ep, err := nw.newEndpointImpl(nil, netlink.NewMockNetlink(false, ""), platform.NewMockExecClient(false), - netio.NewMockNetIO(false, 0), NewMockEndpointClient(true), epInfo) + netio.NewMockNetIO(false, 0), NewMockEndpointClient(func(ep *EndpointInfo) error { + if ep.NICType == cns.InfraNIC { + return NewErrorMockEndpointClient("AddEndpoints Infra NIC failed") + } + + return nil + }), NewMockNamespaceClient(), []*EndpointInfo{epInfo}) Expect(err).To(HaveOccurred()) Expect(ep).To(BeNil()) ep, err = nw.newEndpointImpl(nil, netlink.NewMockNetlink(false, ""), platform.NewMockExecClient(false), - netio.NewMockNetIO(false, 0), NewMockEndpointClient(false), epInfo) + netio.NewMockNetIO(false, 0), NewMockEndpointClient(nil), NewMockNamespaceClient(), []*EndpointInfo{epInfo}) Expect(err).NotTo(HaveOccurred()) Expect(ep).NotTo(BeNil()) Expect(ep.Id).To(Equal(epInfo.Id)) @@ -261,7 +271,8 @@ var _ = Describe("Test Endpoint", func() { nw := &network{} existingEpInfo := &EndpointInfo{ - Id: "768e8deb-eth1", + Id: "768e8deb-eth1", + IfName: eth0IfName, } targetEpInfo := &EndpointInfo{} err := nm.updateEndpoint(nw, existingEpInfo, targetEpInfo) diff --git a/network/endpoint_windows.go b/network/endpoint_windows.go index 5e66f02e51..718dc613c1 100644 --- a/network/endpoint_windows.go +++ b/network/endpoint_windows.go @@ -64,16 +64,25 @@ func ConstructEndpointID(containerID string, netNsPath string, ifName string) (s } // newEndpointImpl creates a new endpoint in the network. -func (nw *network) newEndpointImpl(cli apipaClient, _ netlink.NetlinkInterface, plc platform.ExecClient, _ netio.NetIOInterface, _ EndpointClient, epInfo *EndpointInfo) (*endpoint, error) { - if useHnsV2, err := UseHnsV2(epInfo.NetNsPath); useHnsV2 { +func (nw *network) newEndpointImpl( + cli apipaClient, + _ netlink.NetlinkInterface, + plc platform.ExecClient, + _ netio.NetIOInterface, + _ EndpointClient, + _ NamespaceClientInterface, + epInfo []*EndpointInfo, +) (*endpoint, error) { + // there is only 1 epInfo for windows, multiple interfaces will be added in the future + if useHnsV2, err := UseHnsV2(epInfo[0].NetNsPath); useHnsV2 { if err != nil { return nil, err } - return nw.newEndpointImplHnsV2(cli, epInfo) + return nw.newEndpointImplHnsV2(cli, epInfo[0]) } - return nw.newEndpointImplHnsV1(epInfo, plc) + return nw.newEndpointImplHnsV1(epInfo[0], plc) } // newEndpointImplHnsV1 creates a new endpoint in the network using HnsV1 @@ -400,7 +409,7 @@ func (nw *network) newEndpointImplHnsV2(cli apipaClient, epInfo *EndpointInfo) ( } // deleteEndpointImpl deletes an existing endpoint from the network. -func (nw *network) deleteEndpointImpl(_ netlink.NetlinkInterface, _ platform.ExecClient, _ EndpointClient, ep *endpoint) error { +func (nw *network) deleteEndpointImpl(_ netlink.NetlinkInterface, _ platform.ExecClient, _ EndpointClient, _ NamespaceClientInterface, ep *endpoint) error { if useHnsV2, err := UseHnsV2(ep.NetNs); useHnsV2 { if err != nil { return err diff --git a/network/manager.go b/network/manager.go index ab849c307a..5204cc0c31 100644 --- a/network/manager.go +++ b/network/manager.go @@ -68,6 +68,7 @@ type networkManager struct { netlink netlink.NetlinkInterface netio netio.NetIOInterface plClient platform.ExecClient + nsClient NamespaceClientInterface sync.Mutex } @@ -85,7 +86,7 @@ type NetworkManager interface { FindNetworkIDFromNetNs(netNs string) (string, error) GetNumEndpointsByContainerID(containerID string) int - CreateEndpoint(client apipaClient, networkID string, epInfo *EndpointInfo) error + CreateEndpoint(client apipaClient, networkID string, epInfo []*EndpointInfo) error DeleteEndpoint(networkID string, endpointID string) error GetEndpointInfo(networkID string, endpointID string) (*EndpointInfo, error) GetAllEndpoints(networkID string) (map[string]*EndpointInfo, error) @@ -98,12 +99,13 @@ type NetworkManager interface { } // Creates a new network manager. -func NewNetworkManager(nl netlink.NetlinkInterface, plc platform.ExecClient, netioCli netio.NetIOInterface) (NetworkManager, error) { +func NewNetworkManager(nl netlink.NetlinkInterface, plc platform.ExecClient, netioCli netio.NetIOInterface, nsc NamespaceClientInterface) (NetworkManager, error) { nm := &networkManager{ ExternalInterfaces: make(map[string]*externalInterface), netlink: nl, plClient: plc, netio: netioCli, + nsClient: nsc, } return nm, nil @@ -327,7 +329,7 @@ func (nm *networkManager) GetNetworkInfo(networkId string) (NetworkInfo, error) } // CreateEndpoint creates a new container endpoint. -func (nm *networkManager) CreateEndpoint(cli apipaClient, networkID string, epInfo *EndpointInfo) error { +func (nm *networkManager) CreateEndpoint(cli apipaClient, networkID string, epInfo []*EndpointInfo) error { nm.Lock() defer nm.Unlock() @@ -337,13 +339,14 @@ func (nm *networkManager) CreateEndpoint(cli apipaClient, networkID string, epIn } if nw.VlanId != 0 { - if epInfo.Data[VlanIDKey] == nil { + // the first entry in epInfo is InfraNIC type + if epInfo[0].Data[VlanIDKey] == nil { logger.Info("overriding endpoint vlanid with network vlanid") - epInfo.Data[VlanIDKey] = nw.VlanId + epInfo[0].Data[VlanIDKey] = nw.VlanId } } - _, err = nw.newEndpoint(cli, nm.netlink, nm.plClient, nm.netio, epInfo) + _, err = nw.newEndpoint(cli, nm.netlink, nm.plClient, nm.netio, nm.nsClient, epInfo) if err != nil { return err } @@ -366,7 +369,7 @@ func (nm *networkManager) DeleteEndpoint(networkID, endpointID string) error { return err } - err = nw.deleteEndpoint(nm.netlink, nm.plClient, endpointID) + err = nw.deleteEndpoint(nm.netlink, nm.plClient, nm.nsClient, endpointID) if err != nil { return err } diff --git a/network/manager_mock.go b/network/manager_mock.go index 8b8f4af311..db5cd024f4 100644 --- a/network/manager_mock.go +++ b/network/manager_mock.go @@ -9,13 +9,15 @@ import ( type MockNetworkManager struct { TestNetworkInfoMap map[string]*NetworkInfo TestEndpointInfoMap map[string]*EndpointInfo + TestEndpointClient *MockEndpointClient } // NewMockNetworkmanager returns a new mock -func NewMockNetworkmanager() *MockNetworkManager { +func NewMockNetworkmanager(mockEndpointclient *MockEndpointClient) *MockNetworkManager { return &MockNetworkManager{ TestNetworkInfoMap: make(map[string]*NetworkInfo), TestEndpointInfoMap: make(map[string]*EndpointInfo), + TestEndpointClient: mockEndpointclient, } } @@ -52,8 +54,14 @@ func (nm *MockNetworkManager) GetNetworkInfo(networkID string) (NetworkInfo, err } // CreateEndpoint mock -func (nm *MockNetworkManager) CreateEndpoint(_ apipaClient, networkID string, epInfo *EndpointInfo) error { - nm.TestEndpointInfoMap[epInfo.Id] = epInfo +func (nm *MockNetworkManager) CreateEndpoint(_ apipaClient, _ string, epInfos []*EndpointInfo) error { + for _, epInfo := range epInfos { + if err := nm.TestEndpointClient.AddEndpoints(epInfo); err != nil { + return err + } + } + + nm.TestEndpointInfoMap[epInfos[0].Id] = epInfos[0] return nil } diff --git a/network/mock_endpointclient.go b/network/mock_endpointclient.go index deff106f3c..b95ab47890 100644 --- a/network/mock_endpointclient.go +++ b/network/mock_endpointclient.go @@ -7,36 +7,42 @@ import ( var errMockEpClient = errors.New("MockEndpointClient Error") -func newErrorMockEndpointClient(errStr string) error { +const ( + eth0IfName = "eth0" +) + +func NewErrorMockEndpointClient(errStr string) error { return fmt.Errorf("%w : %s", errMockEpClient, errStr) } type MockEndpointClient struct { - endpoints map[string]bool - returnError bool + endpoints map[string]bool + testAddEndpointFn func(*EndpointInfo) error } -func NewMockEndpointClient(returnError bool) *MockEndpointClient { +func NewMockEndpointClient(fn func(*EndpointInfo) error) *MockEndpointClient { + if fn == nil { + fn = func(_ *EndpointInfo) error { + return nil + } + } + client := &MockEndpointClient{ - endpoints: make(map[string]bool), - returnError: returnError, + endpoints: make(map[string]bool), + testAddEndpointFn: fn, } return client } func (client *MockEndpointClient) AddEndpoints(epInfo *EndpointInfo) error { - if ok := client.endpoints[epInfo.Id]; ok { - return newErrorMockEndpointClient("Endpoint already exists") + if ok := client.endpoints[epInfo.Id]; ok && epInfo.IfName == eth0IfName { + return NewErrorMockEndpointClient("Endpoint already exists") } client.endpoints[epInfo.Id] = true - if client.returnError { - return newErrorMockEndpointClient("AddEndpoints failed") - } - - return nil + return client.testAddEndpointFn(epInfo) } func (client *MockEndpointClient) AddEndpointRules(_ *EndpointInfo) error { diff --git a/network/mock_namespace.go b/network/mock_namespace.go new file mode 100644 index 0000000000..ec5be2c479 --- /dev/null +++ b/network/mock_namespace.go @@ -0,0 +1,60 @@ +// Copyright 2017 Microsoft. All rights reserved. +// MIT License + +package network + +import "github.com/pkg/errors" + +var errMockEnterNamespaceFailure = errors.New("failed to enter namespace") + +const failToEnterNamespaceName = "failns" + +type MockNamespace struct { + namespace string +} + +type MockNamespaceClient struct{} + +func NewMockNamespaceClient() *MockNamespaceClient { + return &MockNamespaceClient{} +} + +// OpenNamespace creates a new namespace object for the given netns path. +func (c *MockNamespaceClient) OpenNamespace(ns string) (NamespaceInterface, error) { + if ns == "" { + return nil, errFileNotExist + } + return &MockNamespace{namespace: ns}, nil +} + +// GetCurrentThreadNamespace returns the caller thread's current namespace. +func (c *MockNamespaceClient) GetCurrentThreadNamespace() (NamespaceInterface, error) { + return c.OpenNamespace("") +} + +// Close releases the resources associated with the namespace object. +func (ns *MockNamespace) Close() error { + return nil +} + +// GetFd returns the file descriptor of the namespace. +func (ns *MockNamespace) GetFd() uintptr { + return 1 +} + +func (ns *MockNamespace) GetName() string { + return ns.namespace +} + +// Enter puts the caller thread inside the namespace. +func (ns *MockNamespace) Enter() error { + if ns.namespace == failToEnterNamespaceName { + return errMockEnterNamespaceFailure + } + return nil +} + +// Exit puts the caller thread to its previous namespace. +func (ns *MockNamespace) Exit() error { + return nil +} diff --git a/network/namespace.go b/network/namespace.go new file mode 100644 index 0000000000..30be49d9f7 --- /dev/null +++ b/network/namespace.go @@ -0,0 +1,18 @@ +package network + +import "errors" + +var errFileNotExist = errors.New("no such file or directory") + +type NamespaceInterface interface { + GetFd() uintptr + GetName() string + Enter() error + Exit() error + Close() error +} + +type NamespaceClientInterface interface { + OpenNamespace(nsPath string) (NamespaceInterface, error) + GetCurrentThreadNamespace() (NamespaceInterface, error) +} diff --git a/network/namespace_linux.go b/network/namespace_linux.go index 455bf4b5c7..bc682ded3b 100644 --- a/network/namespace_linux.go +++ b/network/namespace_linux.go @@ -17,22 +17,29 @@ import ( type Namespace struct { file *os.File prevNs *Namespace + cli *NamespaceClient +} + +type NamespaceClient struct{} + +func NewNamespaceClient() *NamespaceClient { + return &NamespaceClient{} } // OpenNamespace creates a new namespace object for the given netns path. -func OpenNamespace(nsPath string) (*Namespace, error) { +func (c *NamespaceClient) OpenNamespace(nsPath string) (NamespaceInterface, error) { fd, err := os.Open(nsPath) if err != nil { return nil, err } - return &Namespace{file: fd}, nil + return &Namespace{file: fd, cli: c}, nil } // GetCurrentThreadNamespace returns the caller thread's current namespace. -func GetCurrentThreadNamespace() (*Namespace, error) { +func (c *NamespaceClient) GetCurrentThreadNamespace() (NamespaceInterface, error) { nsPath := fmt.Sprintf("/proc/%d/task/%d/ns/net", os.Getpid(), unix.Gettid()) - return OpenNamespace(nsPath) + return c.OpenNamespace(nsPath) } // Close releases the resources associated with the namespace object. @@ -43,7 +50,7 @@ func (ns *Namespace) Close() error { err := ns.file.Close() if err != nil { - return fmt.Errorf("Failed to close namespace %v, err:%v", ns.file.Name(), err) + return fmt.Errorf("failed to close namespace %v, err:%w", ns.file.Name(), err) } ns.file = nil @@ -56,11 +63,15 @@ func (ns *Namespace) GetFd() uintptr { return ns.file.Fd() } +func (ns *Namespace) GetName() string { + return ns.file.Name() +} + // Set sets the current namespace. func (ns *Namespace) set() error { _, _, err := unix.Syscall(unix.SYS_SETNS, ns.file.Fd(), uintptr(unix.CLONE_NEWNET), 0) if err != 0 { - return fmt.Errorf("Failed to set namespace %v, err:%v", ns.file.Name(), err) + return fmt.Errorf("failed to set namespace %v, err:%w", ns.file.Name(), err) } return nil @@ -68,13 +79,13 @@ func (ns *Namespace) set() error { // Enter puts the caller thread inside the namespace. func (ns *Namespace) Enter() error { - var err error - - ns.prevNs, err = GetCurrentThreadNamespace() + currentNs, err := ns.cli.GetCurrentThreadNamespace() if err != nil { return err } + ns.prevNs = currentNs.(*Namespace) + runtime.LockOSThread() err = ns.set() diff --git a/network/namespace_windows.go b/network/namespace_windows.go new file mode 100644 index 0000000000..19e3bbe3b9 --- /dev/null +++ b/network/namespace_windows.go @@ -0,0 +1,53 @@ +// Copyright 2017 Microsoft. All rights reserved. +// MIT License + +package network + +import ( + "errors" +) + +var errWindowsImpl = errors.New("windows impl err") + +// Namespace represents a network namespace. +type Namespace struct{} + +type NamespaceClient struct{} + +func NewNamespaceClient() *NamespaceClient { + return &NamespaceClient{} +} + +// OpenNamespace creates a new namespace object for the given netns path. +func (c *NamespaceClient) OpenNamespace(_ string) (NamespaceInterface, error) { + return nil, errWindowsImpl +} + +// GetCurrentThreadNamespace returns the caller thread's current namespace. +func (c *NamespaceClient) GetCurrentThreadNamespace() (NamespaceInterface, error) { + return c.OpenNamespace("") +} + +// Close releases the resources associated with the namespace object. +func (ns *Namespace) Close() error { + return nil +} + +// GetFd returns the file descriptor of the namespace. +func (ns *Namespace) GetFd() uintptr { + return 0 +} + +func (ns *Namespace) GetName() string { + return "windows impl" +} + +// Enter puts the caller thread inside the namespace. +func (ns *Namespace) Enter() error { + return nil +} + +// Exit puts the caller thread to its previous namespace. +func (ns *Namespace) Exit() error { + return nil +} diff --git a/network/secondary_endpoint_client_linux.go b/network/secondary_endpoint_client_linux.go new file mode 100644 index 0000000000..a5f256ea83 --- /dev/null +++ b/network/secondary_endpoint_client_linux.go @@ -0,0 +1,169 @@ +package network + +import ( + "os" + "strings" + + "github.com/Azure/azure-container-networking/netio" + "github.com/Azure/azure-container-networking/netlink" + "github.com/Azure/azure-container-networking/netns" + "github.com/Azure/azure-container-networking/network/networkutils" + "github.com/Azure/azure-container-networking/platform" + "github.com/pkg/errors" + "go.uber.org/zap" +) + +var errorSecondaryEndpointClient = errors.New("SecondaryEndpointClient Error") + +func newErrorSecondaryEndpointClient(err error) error { + return errors.Wrapf(err, "%s", errorSecondaryEndpointClient) +} + +type SecondaryEndpointClient struct { + netlink netlink.NetlinkInterface + netioshim netio.NetIOInterface + plClient platform.ExecClient + netUtilsClient networkutils.NetworkUtils + nsClient NamespaceClientInterface + ep *endpoint +} + +func NewSecondaryEndpointClient( + nl netlink.NetlinkInterface, + plc platform.ExecClient, + nsc NamespaceClientInterface, + endpoint *endpoint, +) *SecondaryEndpointClient { + client := &SecondaryEndpointClient{ + netlink: nl, + netioshim: &netio.NetIO{}, + plClient: plc, + netUtilsClient: networkutils.NewNetworkUtils(nl, plc), + nsClient: nsc, + ep: endpoint, + } + + return client +} + +func (client *SecondaryEndpointClient) AddEndpoints(epInfo *EndpointInfo) error { + iface, err := client.netioshim.GetNetworkInterfaceByMac(epInfo.MacAddress) + if err != nil { + return newErrorSecondaryEndpointClient(err) + } + + epInfo.IfName = iface.Name + if _, exists := client.ep.SecondaryInterfaces[iface.Name]; exists { + return newErrorSecondaryEndpointClient(errors.New(iface.Name + " already exists")) + } + client.ep.SecondaryInterfaces[iface.Name] = &InterfaceInfo{ + Name: iface.Name, + MacAddress: epInfo.MacAddress, + IPAddress: epInfo.IPAddresses, + NICType: epInfo.NICType, + SkipDefaultRoutes: epInfo.SkipDefaultRoutes, + } + + return nil +} + +func (client *SecondaryEndpointClient) AddEndpointRules(_ *EndpointInfo) error { + return nil +} + +func (client *SecondaryEndpointClient) DeleteEndpointRules(_ *endpoint) { +} + +func (client *SecondaryEndpointClient) MoveEndpointsToContainerNS(epInfo *EndpointInfo, nsID uintptr) error { + // Move the container interface to container's network namespace. + logger.Info("[net] Setting link %v netns %v.", zap.String("IfName", epInfo.IfName), zap.String("NetNsPath", epInfo.NetNsPath)) + if err := client.netlink.SetLinkNetNs(epInfo.IfName, nsID); err != nil { + return newErrorSecondaryEndpointClient(err) + } + + return nil +} + +func (client *SecondaryEndpointClient) SetupContainerInterfaces(epInfo *EndpointInfo) error { + logger.Info("[net] Setting link state up.", zap.String("IfName", epInfo.IfName)) + if err := client.netlink.SetLinkState(epInfo.IfName, true); err != nil { + return newErrorSecondaryEndpointClient(err) + } + + return nil +} + +func (client *SecondaryEndpointClient) ConfigureContainerInterfacesAndRoutes(epInfo *EndpointInfo) error { + if err := client.netUtilsClient.AssignIPToInterface(epInfo.IfName, epInfo.IPAddresses); err != nil { + return newErrorSecondaryEndpointClient(err) + } + + ifInfo, exists := client.ep.SecondaryInterfaces[epInfo.IfName] + if !exists { + return newErrorSecondaryEndpointClient(errors.New(epInfo.IfName + " does not exist")) + } + + if len(epInfo.Routes) < 1 { + return newErrorSecondaryEndpointClient(errors.New("routes expected for " + epInfo.IfName)) + } + + if err := addRoutes(client.netlink, client.netioshim, epInfo.IfName, epInfo.Routes); err != nil { + return newErrorSecondaryEndpointClient(err) + } + + ifInfo.Routes = append(ifInfo.Routes, epInfo.Routes...) + + return nil +} + +func (client *SecondaryEndpointClient) DeleteEndpoints(ep *endpoint) error { + // Get VM namespace + vmns, err := netns.New().Get() + if err != nil { + return newErrorSecondaryEndpointClient(err) + } + + // Open the network namespace. + logger.Info("Opening netns", zap.Any("NetNsPath", ep.NetworkNameSpace)) + ns, err := client.nsClient.OpenNamespace(ep.NetworkNameSpace) + if err != nil { + if strings.Contains(err.Error(), errFileNotExist.Error()) { + // clear SecondaryInterfaces map since network namespace doesn't exist anymore + ep.SecondaryInterfaces = make(map[string]*InterfaceInfo) + return nil + } + + return newErrorSecondaryEndpointClient(err) + } + defer ns.Close() + + // Enter the container network namespace. + logger.Info("Entering netns", zap.Any("NetNsPath", ep.NetworkNameSpace)) + if err := ns.Enter(); err != nil { + if errors.Is(err, os.ErrNotExist) { + ep.SecondaryInterfaces = make(map[string]*InterfaceInfo) + return nil + } + + return newErrorSecondaryEndpointClient(err) + } + + // Return to host network namespace. + defer func() { + logger.Info("Exiting netns", zap.Any("NetNsPath", ep.NetworkNameSpace)) + if err := ns.Exit(); err != nil { + logger.Error("Failed to exit netns with", zap.Error(newErrorSecondaryEndpointClient(err))) + } + }() + + for iface := range ep.SecondaryInterfaces { + if err := client.netlink.SetLinkNetNs(iface, uintptr(vmns)); err != nil { + logger.Error("Failed to move interface", zap.String("IfName", iface), zap.Error(newErrorSecondaryEndpointClient(err))) + continue + } + + delete(ep.SecondaryInterfaces, iface) + } + + return nil +} diff --git a/network/secondary_endpoint_linux_test.go b/network/secondary_endpoint_linux_test.go new file mode 100644 index 0000000000..775ffeeb6a --- /dev/null +++ b/network/secondary_endpoint_linux_test.go @@ -0,0 +1,338 @@ +//go:build linux +// +build linux + +package network + +import ( + "net" + "testing" + + "github.com/Azure/azure-container-networking/netio" + "github.com/Azure/azure-container-networking/netlink" + "github.com/Azure/azure-container-networking/network/networkutils" + "github.com/Azure/azure-container-networking/platform" + "github.com/stretchr/testify/require" +) + +func TestSecondaryAddEndpoints(t *testing.T) { + nl := netlink.NewMockNetlink(false, "") + plc := platform.NewMockExecClient(false) + mac, _ := net.ParseMAC("ab:cd:ef:12:34:56") + invalidMac, _ := net.ParseMAC("12:34:56:ab:cd:ef") + + tests := []struct { + name string + client *SecondaryEndpointClient + epInfo *EndpointInfo + wantErr bool + wantErrMsg string + }{ + { + name: "Add endpoints", + client: &SecondaryEndpointClient{ + netlink: netlink.NewMockNetlink(false, ""), + plClient: platform.NewMockExecClient(false), + netUtilsClient: networkutils.NewNetworkUtils(nl, plc), + netioshim: netio.NewMockNetIO(false, 0), + ep: &endpoint{SecondaryInterfaces: make(map[string]*InterfaceInfo)}, + }, + epInfo: &EndpointInfo{MacAddress: mac}, + wantErr: false, + }, + { + name: "Add endpoints invalid mac", + client: &SecondaryEndpointClient{ + netlink: netlink.NewMockNetlink(false, ""), + plClient: platform.NewMockExecClient(false), + netUtilsClient: networkutils.NewNetworkUtils(nl, plc), + netioshim: netio.NewMockNetIO(false, 0), + ep: &endpoint{SecondaryInterfaces: make(map[string]*InterfaceInfo)}, + }, + epInfo: &EndpointInfo{MacAddress: invalidMac}, + wantErr: true, + wantErrMsg: "SecondaryEndpointClient Error: " + netio.ErrMockNetIOFail.Error() + ": " + invalidMac.String(), + }, + { + name: "Add endpoints interface already added", + client: &SecondaryEndpointClient{ + netlink: netlink.NewMockNetlink(false, ""), + plClient: platform.NewMockExecClient(false), + netUtilsClient: networkutils.NewNetworkUtils(nl, plc), + netioshim: netio.NewMockNetIO(false, 0), + ep: &endpoint{SecondaryInterfaces: map[string]*InterfaceInfo{"eth1": {Name: "eth1"}}}, + }, + epInfo: &EndpointInfo{MacAddress: mac}, + wantErr: true, + wantErrMsg: "SecondaryEndpointClient Error: eth1 already exists", + }, + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + err := tt.client.AddEndpoints(tt.epInfo) + if tt.wantErr { + require.Error(t, err) + require.Contains(t, tt.wantErrMsg, err.Error(), "Expected:%v actual:%v", tt.wantErrMsg, err.Error()) + } else { + require.NoError(t, err) + require.Equal(t, tt.client.ep.SecondaryInterfaces["eth1"].MacAddress, tt.epInfo.MacAddress) + } + }) + } +} + +func TestSecondaryDeleteEndpoints(t *testing.T) { + nl := netlink.NewMockNetlink(false, "") + plc := platform.NewMockExecClient(false) + + tests := []struct { + name string + client *SecondaryEndpointClient + ep *endpoint + wantErr bool + }{ + { + name: "Delete endpoint happy path", + client: &SecondaryEndpointClient{ + netlink: netlink.NewMockNetlink(false, ""), + plClient: platform.NewMockExecClient(false), + netUtilsClient: networkutils.NewNetworkUtils(nl, plc), + netioshim: netio.NewMockNetIO(false, 0), + nsClient: NewMockNamespaceClient(), + }, + ep: &endpoint{ + NetworkNameSpace: "testns", + SecondaryInterfaces: map[string]*InterfaceInfo{ + "eth1": { + Name: "eth1", + Routes: []RouteInfo{ + { + Dst: net.IPNet{IP: net.ParseIP("192.168.0.4"), Mask: net.CIDRMask(ipv4FullMask, ipv4Bits)}, + }, + }, + }, + }, + }, + }, + { + name: "Delete endpoint happy path namespace not found", + client: &SecondaryEndpointClient{ + netlink: netlink.NewMockNetlink(false, ""), + plClient: platform.NewMockExecClient(false), + netUtilsClient: networkutils.NewNetworkUtils(nl, plc), + netioshim: netio.NewMockNetIO(false, 0), + nsClient: NewMockNamespaceClient(), + }, + ep: &endpoint{ + SecondaryInterfaces: map[string]*InterfaceInfo{ + "eth1": { + Name: "eth1", + Routes: []RouteInfo{ + { + Dst: net.IPNet{IP: net.ParseIP("192.168.0.4"), Mask: net.CIDRMask(ipv4FullMask, ipv4Bits)}, + }, + }, + }, + }, + }, + }, + { + name: "Delete endpoint enter namespace failure", + client: &SecondaryEndpointClient{ + netlink: netlink.NewMockNetlink(false, ""), + plClient: platform.NewMockExecClient(false), + netUtilsClient: networkutils.NewNetworkUtils(nl, plc), + netioshim: netio.NewMockNetIO(false, 0), + nsClient: NewMockNamespaceClient(), + }, + ep: &endpoint{ + NetworkNameSpace: failToEnterNamespaceName, + SecondaryInterfaces: map[string]*InterfaceInfo{ + "eth1": { + Name: "eth1", + Routes: []RouteInfo{ + { + Dst: net.IPNet{IP: net.ParseIP("192.168.0.4"), Mask: net.CIDRMask(ipv4FullMask, ipv4Bits)}, + }, + }, + }, + }, + }, + wantErr: true, + }, + { + name: "Delete endpoint netlink failure", + client: &SecondaryEndpointClient{ + netlink: netlink.NewMockNetlink(true, "netlink failure"), + plClient: platform.NewMockExecClient(false), + netUtilsClient: networkutils.NewNetworkUtils(nl, plc), + netioshim: netio.NewMockNetIO(false, 0), + nsClient: NewMockNamespaceClient(), + }, + ep: &endpoint{ + NetworkNameSpace: failToEnterNamespaceName, + SecondaryInterfaces: map[string]*InterfaceInfo{ + "eth1": { + Name: "eth1", + Routes: []RouteInfo{ + { + Dst: net.IPNet{IP: net.ParseIP("192.168.0.4"), Mask: net.CIDRMask(ipv4FullMask, ipv4Bits)}, + }, + }, + }, + }, + }, + wantErr: true, + }, + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + require.Len(t, tt.ep.SecondaryInterfaces, 1) + if tt.wantErr { + require.Error(t, tt.client.DeleteEndpoints(tt.ep)) + require.Len(t, tt.ep.SecondaryInterfaces, 1) + } else { + require.Nil(t, tt.client.DeleteEndpoints(tt.ep)) + require.Len(t, tt.ep.SecondaryInterfaces, 0) + } + }) + } +} + +func TestSecondaryConfigureContainerInterfacesAndRoutes(t *testing.T) { + nl := netlink.NewMockNetlink(false, "") + plc := platform.NewMockExecClient(false) + + tests := []struct { + name string + client *SecondaryEndpointClient + epInfo *EndpointInfo + wantErr bool + wantErrMsg string + }{ + { + name: "Configure Interface and routes happy path", + client: &SecondaryEndpointClient{ + netlink: netlink.NewMockNetlink(false, ""), + plClient: platform.NewMockExecClient(false), + netUtilsClient: networkutils.NewNetworkUtils(nl, plc), + netioshim: netio.NewMockNetIO(false, 0), + ep: &endpoint{SecondaryInterfaces: map[string]*InterfaceInfo{"eth1": {Name: "eth1"}}}, + }, + epInfo: &EndpointInfo{ + IfName: "eth1", + IPAddresses: []net.IPNet{ + { + IP: net.ParseIP("192.168.0.4"), + Mask: net.CIDRMask(subnetv4Mask, ipv4Bits), + }, + }, + Routes: []RouteInfo{ + { + Dst: net.IPNet{IP: net.ParseIP("192.168.0.4"), Mask: net.CIDRMask(ipv4FullMask, ipv4Bits)}, + }, + }, + }, + wantErr: false, + }, + { + name: "Configure Interface and routes assign ip fail", + client: &SecondaryEndpointClient{ + netlink: netlink.NewMockNetlink(false, ""), + plClient: platform.NewMockExecClient(false), + netUtilsClient: networkutils.NewNetworkUtils(netlink.NewMockNetlink(true, ""), plc), + netioshim: netio.NewMockNetIO(false, 0), + ep: &endpoint{SecondaryInterfaces: map[string]*InterfaceInfo{"eth1": {Name: "eth1"}}}, + }, + epInfo: &EndpointInfo{ + IfName: "eth1", + IPAddresses: []net.IPNet{ + { + IP: net.ParseIP("192.168.0.4"), + Mask: net.CIDRMask(subnetv4Mask, ipv4Bits), + }, + }, + }, + wantErr: true, + wantErrMsg: "", + }, + { + name: "Configure Interface and routes add routes fail", + client: &SecondaryEndpointClient{ + netlink: netlink.NewMockNetlink(false, ""), + plClient: platform.NewMockExecClient(false), + netUtilsClient: networkutils.NewNetworkUtils(nl, plc), + netioshim: netio.NewMockNetIO(true, 1), + ep: &endpoint{SecondaryInterfaces: map[string]*InterfaceInfo{"eth1": {Name: "eth1"}}}, + }, + epInfo: &EndpointInfo{ + IfName: "eth1", + IPAddresses: []net.IPNet{ + { + IP: net.ParseIP("192.168.0.4"), + Mask: net.CIDRMask(subnetv4Mask, ipv4Bits), + }, + }, + Routes: []RouteInfo{ + { + Dst: net.IPNet{IP: net.ParseIP("192.168.0.4"), Mask: net.CIDRMask(ipv4FullMask, ipv4Bits)}, + }, + }, + }, + wantErr: true, + wantErrMsg: netio.ErrMockNetIOFail.Error(), + }, + { + name: "Configure Interface and routes add routes invalid interface name", + client: &SecondaryEndpointClient{ + netlink: netlink.NewMockNetlink(false, ""), + plClient: platform.NewMockExecClient(false), + netUtilsClient: networkutils.NewNetworkUtils(nl, plc), + netioshim: netio.NewMockNetIO(false, 0), + ep: &endpoint{SecondaryInterfaces: map[string]*InterfaceInfo{"eth1": {Name: "eth1"}}}, + }, + epInfo: &EndpointInfo{ + IfName: "eth2", + IPAddresses: []net.IPNet{ + { + IP: net.ParseIP("192.168.0.4"), + Mask: net.CIDRMask(subnetv4Mask, ipv4Bits), + }, + }, + }, + wantErr: true, + wantErrMsg: "SecondaryEndpointClient Error: eth2 does not exist", + }, + { + name: "Configure Interface and routes add routes fail when no routes are provided", + client: &SecondaryEndpointClient{ + netlink: netlink.NewMockNetlink(false, ""), + plClient: platform.NewMockExecClient(false), + netUtilsClient: networkutils.NewNetworkUtils(nl, plc), + netioshim: netio.NewMockNetIO(false, 0), + ep: &endpoint{SecondaryInterfaces: map[string]*InterfaceInfo{"eth1": {Name: "eth1"}}}, + }, + epInfo: &EndpointInfo{ + IfName: "eth1", + }, + wantErr: true, + wantErrMsg: "SecondaryEndpointClient Error: routes expected for eth1", + }, + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + err := tt.client.ConfigureContainerInterfacesAndRoutes(tt.epInfo) + if tt.wantErr { + require.Error(t, err) + require.Contains(t, err.Error(), tt.wantErrMsg, "Expected:%v actual:%v", tt.wantErrMsg, err.Error()) + } else { + require.NoError(t, err) + } + }) + } +} diff --git a/network/transparent_endpoint_linux_test.go b/network/transparent_endpoint_linux_test.go index 4fa69772f1..09587fa0d5 100644 --- a/network/transparent_endpoint_linux_test.go +++ b/network/transparent_endpoint_linux_test.go @@ -57,7 +57,7 @@ func TestTransAddEndpoints(t *testing.T) { }, epInfo: &EndpointInfo{}, wantErr: true, - wantErrMsg: "TransparentEndpointClient Error : " + netlink.ErrorMockNetlink.Error() + " : netlink fail", + wantErrMsg: "TransparentEndpointClient Error: " + netlink.ErrorMockNetlink.Error() + " : netlink fail", }, { name: "Add endpoints get interface fail for old veth", @@ -86,7 +86,7 @@ func TestTransAddEndpoints(t *testing.T) { }, epInfo: &EndpointInfo{}, wantErr: true, - wantErrMsg: "TransparentEndpointClient Error : " + netio.ErrMockNetIOFail.Error() + ":eth0", + wantErrMsg: "TransparentEndpointClient Error: " + netio.ErrMockNetIOFail.Error() + ":eth0", }, { name: "Add endpoints get interface fail for host veth", @@ -101,7 +101,7 @@ func TestTransAddEndpoints(t *testing.T) { }, epInfo: &EndpointInfo{}, wantErr: true, - wantErrMsg: "TransparentEndpointClient Error : " + netio.ErrMockNetIOFail.Error() + ":azvcontainer", + wantErrMsg: "TransparentEndpointClient Error: " + netio.ErrMockNetIOFail.Error() + ":azvcontainer", }, { name: "get interface fail for container veth", @@ -116,7 +116,7 @@ func TestTransAddEndpoints(t *testing.T) { }, epInfo: &EndpointInfo{}, wantErr: true, - wantErrMsg: "TransparentEndpointClient Error : " + netio.ErrMockNetIOFail.Error() + ":azvhost", + wantErrMsg: "TransparentEndpointClient Error: " + netio.ErrMockNetIOFail.Error() + ":azvhost", }, } diff --git a/network/transparent_endpointclient_linux.go b/network/transparent_endpointclient_linux.go index 884ef9ff30..1159c1f19a 100644 --- a/network/transparent_endpointclient_linux.go +++ b/network/transparent_endpointclient_linux.go @@ -1,7 +1,6 @@ package network import ( - "errors" "fmt" "net" @@ -9,6 +8,7 @@ import ( "github.com/Azure/azure-container-networking/netlink" "github.com/Azure/azure-container-networking/network/networkutils" "github.com/Azure/azure-container-networking/platform" + "github.com/pkg/errors" "go.uber.org/zap" ) @@ -27,8 +27,8 @@ const ( var errorTransparentEndpointClient = errors.New("TransparentEndpointClient Error") -func newErrorTransparentEndpointClient(errStr string) error { - return fmt.Errorf("%w : %s", errorTransparentEndpointClient, errStr) +func newErrorTransparentEndpointClient(err error) error { + return errors.Wrapf(err, "%s", errorTransparentEndpointClient) } type TransparentEndpointClient struct { @@ -81,13 +81,13 @@ func (client *TransparentEndpointClient) AddEndpoints(epInfo *EndpointInfo) erro logger.Info("Deleting old host veth", zap.String("hostVethName", client.hostVethName)) if err = client.netlink.DeleteLink(client.hostVethName); err != nil { logger.Error("Failed to delete old", zap.String("hostVethName", client.hostVethName), zap.Error(err)) - return newErrorTransparentEndpointClient(err.Error()) + return newErrorTransparentEndpointClient(err) } } primaryIf, err := client.netioshim.GetNetworkInterfaceByName(client.hostPrimaryIfName) if err != nil { - return newErrorTransparentEndpointClient(err.Error()) + return newErrorTransparentEndpointClient(err) } mac, err := net.ParseMAC(defaultHostVethHwAddr) @@ -96,7 +96,7 @@ func (client *TransparentEndpointClient) AddEndpoints(epInfo *EndpointInfo) erro } if err = client.netUtilsClient.CreateEndpoint(client.hostVethName, client.containerVethName, mac); err != nil { - return newErrorTransparentEndpointClient(err.Error()) + return newErrorTransparentEndpointClient(err) } defer func() { @@ -109,14 +109,14 @@ func (client *TransparentEndpointClient) AddEndpoints(epInfo *EndpointInfo) erro containerIf, err := client.netioshim.GetNetworkInterfaceByName(client.containerVethName) if err != nil { - return newErrorTransparentEndpointClient(err.Error()) + return newErrorTransparentEndpointClient(err) } client.containerMac = containerIf.HardwareAddr hostVethIf, err := client.netioshim.GetNetworkInterfaceByName(client.hostVethName) if err != nil { - return newErrorTransparentEndpointClient(err.Error()) + return newErrorTransparentEndpointClient(err) } client.hostVethMac = hostVethIf.HardwareAddr @@ -157,7 +157,7 @@ func (client *TransparentEndpointClient) AddEndpointRules(epInfo *EndpointInfo) } if err := addRoutes(client.netlink, client.netioshim, client.hostVethName, routeInfoList); err != nil { - return newErrorTransparentEndpointClient(err.Error()) + return newErrorTransparentEndpointClient(err) } logger.Info("calling setArpProxy for", zap.String("hostVethName", client.hostVethName)) @@ -196,7 +196,7 @@ func (client *TransparentEndpointClient) MoveEndpointsToContainerNS(epInfo *Endp // Move the container interface to container's network namespace. logger.Info("Setting link netns", zap.String("containerVethName", client.containerVethName), zap.String("NetNsPath", epInfo.NetNsPath)) if err := client.netlink.SetLinkNetNs(client.containerVethName, nsID); err != nil { - return newErrorTransparentEndpointClient(err.Error()) + return newErrorTransparentEndpointClient(err) } return nil @@ -214,7 +214,7 @@ func (client *TransparentEndpointClient) SetupContainerInterfaces(epInfo *Endpoi func (client *TransparentEndpointClient) ConfigureContainerInterfacesAndRoutes(epInfo *EndpointInfo) error { if err := client.netUtilsClient.AssignIPToInterface(client.containerVethName, epInfo.IPAddresses); err != nil { - return newErrorTransparentEndpointClient(err.Error()) + return newErrorTransparentEndpointClient(err) } // ip route del 10.240.0.0/12 dev eth0 (removing kernel subnet route added by above call) @@ -226,7 +226,7 @@ func (client *TransparentEndpointClient) ConfigureContainerInterfacesAndRoutes(e Protocol: netlink.RTPROT_KERNEL, } if err := deleteRoutes(client.netlink, client.netioshim, client.containerVethName, []RouteInfo{routeInfo}); err != nil { - return newErrorTransparentEndpointClient(err.Error()) + return newErrorTransparentEndpointClient(err) } } @@ -238,18 +238,22 @@ func (client *TransparentEndpointClient) ConfigureContainerInterfacesAndRoutes(e Scope: netlink.RT_SCOPE_LINK, } if err := addRoutes(client.netlink, client.netioshim, client.containerVethName, []RouteInfo{routeInfo}); err != nil { - return newErrorTransparentEndpointClient(err.Error()) + return newErrorTransparentEndpointClient(err) } - // ip route add default via 169.254.1.1 dev eth0 - _, defaultIPNet, _ := net.ParseCIDR(defaultGwCidr) - dstIP := net.IPNet{IP: net.ParseIP(defaultGw), Mask: defaultIPNet.Mask} - routeInfo = RouteInfo{ - Dst: dstIP, - Gw: virtualGwIP, - } - if err := addRoutes(client.netlink, client.netioshim, client.containerVethName, []RouteInfo{routeInfo}); err != nil { - return err + if !epInfo.SkipDefaultRoutes { + // ip route add default via 169.254.1.1 dev eth0 + _, defaultIPNet, _ := net.ParseCIDR(defaultGwCidr) + dstIP := net.IPNet{IP: net.ParseIP(defaultGw), Mask: defaultIPNet.Mask} + routeInfo = RouteInfo{ + Dst: dstIP, + Gw: virtualGwIP, + } + if err := addRoutes(client.netlink, client.netioshim, client.containerVethName, []RouteInfo{routeInfo}); err != nil { + return err + } + } else if err := addRoutes(client.netlink, client.netioshim, client.containerVethName, epInfo.Routes); err != nil { + return newErrorTransparentEndpointClient(err) } // arp -s 169.254.1.1 e3:45:f4:ac:34:12 - add static arp entry for virtualgwip to hostveth interface mac diff --git a/network/transparent_vlan_endpointclient_linux.go b/network/transparent_vlan_endpointclient_linux.go index 19363bbfb0..5532db20d1 100644 --- a/network/transparent_vlan_endpointclient_linux.go +++ b/network/transparent_vlan_endpointclient_linux.go @@ -68,6 +68,7 @@ type TransparentVlanEndpointClient struct { netioshim netio.NetIOInterface plClient platform.ExecClient netUtilsClient networkutils.NetworkUtils + nsClient NamespaceClientInterface } func NewTransparentVlanEndpointClient( @@ -79,6 +80,7 @@ func NewTransparentVlanEndpointClient( localIP string, nl netlink.NetlinkInterface, plc platform.ExecClient, + nsc NamespaceClientInterface, ) *TransparentVlanEndpointClient { vlanVethName := fmt.Sprintf("%s_%d", nw.extIf.Name, vlanid) vnetNSName := fmt.Sprintf("az_ns_%d", vlanid) @@ -100,6 +102,7 @@ func NewTransparentVlanEndpointClient( netioshim: &netio.NetIO{}, plClient: plc, netUtilsClient: networkutils.NewNetworkUtils(nl, plc), + nsClient: nsc, } client.NewSnatClient(nw.SnatBridgeIP, localIP, ep) @@ -118,7 +121,7 @@ func (client *TransparentVlanEndpointClient) AddEndpoints(epInfo *EndpointInfo) return errors.Wrap(err, "failed to add snat endpoint") } // VNET Namespace - return ExecuteInNS(client.vnetNSName, func() error { + return ExecuteInNS(client.nsClient, client.vnetNSName, func() error { return client.PopulateVnet(epInfo) }) } @@ -315,7 +318,7 @@ func (client *TransparentVlanEndpointClient) AddEndpointRules(epInfo *EndpointIn return errors.Wrap(err, "failed to add snat endpoint rules") } logger.Info("[transparent-vlan] Adding tunneling rules in vnet namespace") - err := ExecuteInNS(client.vnetNSName, func() error { + err := ExecuteInNS(client.nsClient, client.vnetNSName, func() error { return client.AddVnetRules(epInfo) }) return err @@ -392,7 +395,7 @@ func (client *TransparentVlanEndpointClient) ConfigureContainerInterfacesAndRout } // Switch to vnet NS and call ConfigureVnetInterfacesAndRoutes - err = ExecuteInNS(client.vnetNSName, func() error { + err = ExecuteInNS(client.nsClient, client.vnetNSName, func() error { return client.ConfigureVnetInterfacesAndRoutesImpl(epInfo) }) if err != nil { @@ -544,7 +547,7 @@ func (client *TransparentVlanEndpointClient) AddDefaultArp(interfaceName, destMa func (client *TransparentVlanEndpointClient) DeleteEndpoints(ep *endpoint) error { // Vnet NS - err := ExecuteInNS(client.vnetNSName, func() error { + err := ExecuteInNS(client.nsClient, client.vnetNSName, func() error { // Passing in functionality to get number of routes after deletion getNumRoutesLeft := func() (int, error) { routes, err := vishnetlink.RouteList(nil, vishnetlink.FAMILY_V4) @@ -597,39 +600,39 @@ func (client *TransparentVlanEndpointClient) DeleteEndpointsImpl(ep *endpoint, _ // Helper function that allows executing a function in a VM namespace // Does not work for process namespaces -func ExecuteInNS(nsName string, f func() error) error { +func ExecuteInNS(nsc NamespaceClientInterface, nsName string, f func() error) error { // Current namespace - returnedTo, err := GetCurrentThreadNamespace() + returnedTo, err := nsc.GetCurrentThreadNamespace() if err != nil { logger.Error("[ExecuteInNS] Could not get NS we are in", zap.Error(err)) } else { - logger.Info("[ExecuteInNS] In NS before switch", zap.String("fileName", returnedTo.file.Name())) + logger.Info("[ExecuteInNS] In NS before switch", zap.String("fileName", returnedTo.GetName())) } // Open the network namespace logger.Info("[ExecuteInNS] Opening ns", zap.String("nsName", fmt.Sprintf("/var/run/netns/%s", nsName))) - ns, err := OpenNamespace(fmt.Sprintf("/var/run/netns/%s", nsName)) + ns, err := nsc.OpenNamespace(fmt.Sprintf("/var/run/netns/%s", nsName)) if err != nil { return err } defer ns.Close() // Enter the network namespace - logger.Info("[ExecuteInNS] Entering ns", zap.String("nsFileName", ns.file.Name())) + logger.Info("[ExecuteInNS] Entering ns", zap.String("nsFileName", ns.GetName())) if err := ns.Enter(); err != nil { return err } // Exit network namespace defer func() { - logger.Info("[ExecuteInNS] Exiting ns", zap.String("nsFileName", ns.file.Name())) + logger.Info("[ExecuteInNS] Exiting ns", zap.String("nsFileName", ns.GetName())) if err := ns.Exit(); err != nil { logger.Error("[ExecuteInNS] Could not exit ns", zap.Error(err)) } - returnedTo, err := GetCurrentThreadNamespace() + returnedTo, err := nsc.GetCurrentThreadNamespace() if err != nil { logger.Error("[ExecuteInNS] Could not get NS we returned to", zap.Error(err)) } else { - logger.Info("[ExecuteInNS] Returned to NS", zap.String("fileName", returnedTo.file.Name())) + logger.Info("[ExecuteInNS] Returned to NS", zap.String("fileName", returnedTo.GetName())) } }() return f()