From 0ba937f1741d82f57642584a02a7873db3bcbe41 Mon Sep 17 00:00:00 2001 From: paulyu Date: Tue, 21 Mar 2023 13:00:05 -0400 Subject: [PATCH 01/44] dualstack overlay cni PR --- cni/azure-linux-dualstack-overlay.conflist | 23 + cni/azure-windows-dualstack-overlay.conflist | 50 ++ cni/network/cnsclient.go | 2 + cni/network/invoker.go | 2 +- cni/network/invoker_azure.go | 63 +- cni/network/invoker_azure_test.go | 52 +- cni/network/invoker_cns.go | 182 +++-- cni/network/invoker_cns_test.go | 193 ++--- cni/network/invoker_mock.go | 12 +- cni/network/multitenancy.go | 721 +++++++++++-------- cni/network/multitenancy_test.go | 26 +- cni/network/network.go | 73 +- cni/network/network_test.go | 36 +- cni/network/network_windows.go | 5 +- cni/util/const.go | 3 +- network/manager.go | 6 + network/network.go | 2 + network/network_windows.go | 64 +- 18 files changed, 930 insertions(+), 585 deletions(-) create mode 100644 cni/azure-linux-dualstack-overlay.conflist create mode 100644 cni/azure-windows-dualstack-overlay.conflist diff --git a/cni/azure-linux-dualstack-overlay.conflist b/cni/azure-linux-dualstack-overlay.conflist new file mode 100644 index 0000000000..5d3deac813 --- /dev/null +++ b/cni/azure-linux-dualstack-overlay.conflist @@ -0,0 +1,23 @@ +{ + "cniVersion":"0.3.0", + "name":"azure", + "plugins":[ + { + "type":"azure-vnet", + "mode":"transparent", + "executionMode":"v4swift", + "ipsToRouteViaHost":["169.254.20.10"], + "ipam":{ + "type":"azure-cns", + "mode":"dualStackOverlay" + } + }, + { + "type":"portmap", + "capabilities":{ + "portMappings":true + }, + "snat":true + } + ] +} \ No newline at end of file diff --git a/cni/azure-windows-dualstack-overlay.conflist b/cni/azure-windows-dualstack-overlay.conflist new file mode 100644 index 0000000000..0c8c9acdce --- /dev/null +++ b/cni/azure-windows-dualstack-overlay.conflist @@ -0,0 +1,50 @@ +{ + "cniVersion": "0.3.0", + "name": "azure", + "adapterName" : "", + "plugins": [ + { + "type": "azure-vnet", + "mode": "bridge", + "bridge": "azure0", + "executionMode": "v4swift", + "capabilities": { + "portMappings": true, + "dns": true + }, + "ipam": { + "type": "azure-cns", + "mode": "dualStackOverlay" + }, + "dns": { + "Nameservers": [ + "10.0.0.10", + "168.63.129.16" + ], + "Search": [ + "svc.cluster.local" + ] + }, + "AdditionalArgs": [ + { + "Name": "EndpointPolicy", + "Value": { + "Type": "OutBoundNAT", + "ExceptionList": [ + "10.240.0.0/16", + "10.0.0.0/8" + ] + } + }, + { + "Name": "EndpointPolicy", + "Value": { + "Type": "ROUTE", + "DestinationPrefix": "10.0.0.0/8", + "NeedEncap": true + } + } + ] + } + ] +} \ No newline at end of file diff --git a/cni/network/cnsclient.go b/cni/network/cnsclient.go index ea26cae02c..b48a2a2b4d 100644 --- a/cni/network/cnsclient.go +++ b/cni/network/cnsclient.go @@ -9,6 +9,8 @@ import ( type cnsclient interface { RequestIPAddress(ctx context.Context, ipconfig cns.IPConfigRequest) (*cns.IPConfigResponse, error) ReleaseIPAddress(ctx context.Context, ipconfig cns.IPConfigRequest) error + RequestIPs(ctx context.Context, ipconfig cns.IPConfigsRequest) (*cns.IPConfigsResponse, error) + ReleaseIPs(ctx context.Context, ipconfig cns.IPConfigsRequest) error GetNetworkContainer(ctx context.Context, orchestratorContext []byte) (*cns.GetNetworkContainerResponse, error) GetAllNetworkContainers(ctx context.Context, orchestratorContext []byte) ([]cns.GetNetworkContainerResponse, error) } diff --git a/cni/network/invoker.go b/cni/network/invoker.go index 0d111933f8..dc38a6492a 100644 --- a/cni/network/invoker.go +++ b/cni/network/invoker.go @@ -17,7 +17,7 @@ type IPAMInvoker interface { Add(IPAMAddConfig) (IPAMAddResult, error) // Delete calls to the invoker source, and returns error. Returning an error here will fail the CNI Delete call. - Delete(address *net.IPNet, nwCfg *cni.NetworkConfig, args *cniSkel.CmdArgs, options map[string]interface{}) error + Delete(addresses []*net.IPNet, nwCfg *cni.NetworkConfig, args *cniSkel.CmdArgs, options map[string]interface{}) error } type IPAMAddConfig struct { diff --git a/cni/network/invoker_azure.go b/cni/network/invoker_azure.go index c0f85c8baa..34609c1d58 100644 --- a/cni/network/invoker_azure.go +++ b/cni/network/invoker_azure.go @@ -29,6 +29,9 @@ type delegatePlugin interface { Errorf(format string, args ...interface{}) *cniTypes.Error } +const bytesSize4 = 4 +const bytesSize16 = 16 + // Create an IPAM instance every time a CNI action is called. func NewAzureIpamInvoker(plugin *NetPlugin, nwInfo *network.NetworkInfo) *AzureIPAMInvoker { return &AzureIPAMInvoker{ @@ -65,7 +68,11 @@ func (invoker *AzureIPAMInvoker) Add(addConfig IPAMAddConfig) (IPAMAddResult, er 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 { + addresses := []*net.IPNet{} + for _, ip := range addResult.ipv4Result.IPs { + addresses = append(addresses, &ip.Address) + } + if er := invoker.Delete(addresses, 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 { @@ -122,7 +129,8 @@ func (invoker *AzureIPAMInvoker) deleteIpamState() { } } -func (invoker *AzureIPAMInvoker) Delete(address *net.IPNet, nwCfg *cni.NetworkConfig, _ *cniSkel.CmdArgs, options map[string]interface{}) error { +func (invoker *AzureIPAMInvoker) Delete(addresses []*net.IPNet, nwCfg *cni.NetworkConfig, _ *cniSkel.CmdArgs, options map[string]interface{}) error { + if nwCfg == nil { return invoker.plugin.Errorf("nil nwCfg passed to CNI ADD, stack: %+v", string(debug.Stack())) } @@ -131,35 +139,38 @@ func (invoker *AzureIPAMInvoker) Delete(address *net.IPNet, nwCfg *cni.NetworkCo nwCfg.IPAM.Subnet = invoker.nwInfo.Subnets[0].Prefix.String() } - if address == nil { + if len(addresses) == 0 { if err := invoker.plugin.DelegateDel(nwCfg.IPAM.Type, nwCfg); err != nil { return invoker.plugin.Errorf("Attempted to release address with error: %v", err) } - } else if len(address.IP.To4()) == 4 { - nwCfg.IPAM.Address = address.IP.String() - log.Printf("Releasing ipv4 address :%s pool: %s", - nwCfg.IPAM.Address, nwCfg.IPAM.Subnet) - if err := invoker.plugin.DelegateDel(nwCfg.IPAM.Type, nwCfg); err != nil { - log.Printf("Failed to release ipv4 address: %v", err) - return invoker.plugin.Errorf("Failed to release ipv4 address: %v", err) - } - } else if len(address.IP.To16()) == 16 { - nwCfgIpv6 := *nwCfg - nwCfgIpv6.IPAM.Environment = common.OptEnvironmentIPv6NodeIpam - nwCfgIpv6.IPAM.Type = ipamV6 - nwCfgIpv6.IPAM.Address = address.IP.String() - if len(invoker.nwInfo.Subnets) > 1 { - nwCfgIpv6.IPAM.Subnet = invoker.nwInfo.Subnets[1].Prefix.String() - } + } + + for _, address := range addresses { + if len(address.IP.To4()) == bytesSize4 { + nwCfg.IPAM.Address = address.IP.String() + log.Printf("Releasing ipv4 address :%s pool: %s", nwCfg.IPAM.Address, nwCfg.IPAM.Subnet) + if err := invoker.plugin.DelegateDel(nwCfg.IPAM.Type, nwCfg); err != nil { + log.Printf("Failed to release ipv4 address: %v", err) + return invoker.plugin.Errorf("Failed to release ipv4 address: %v with error: ", nwCfg.IPAM.Address, err) + } + } else if len(address.IP.To16()) == bytesSize16 { + nwCfgIpv6 := *nwCfg + nwCfgIpv6.IPAM.Environment = common.OptEnvironmentIPv6NodeIpam + nwCfgIpv6.IPAM.Type = ipamV6 + nwCfgIpv6.IPAM.Address = address.IP.String() + if len(invoker.nwInfo.Subnets) > 1 { + nwCfgIpv6.IPAM.Subnet = invoker.nwInfo.Subnets[1].Prefix.String() + } - log.Printf("Releasing ipv6 address :%s pool: %s", - nwCfgIpv6.IPAM.Address, nwCfgIpv6.IPAM.Subnet) - if err := invoker.plugin.DelegateDel(nwCfgIpv6.IPAM.Type, &nwCfgIpv6); err != nil { - log.Printf("Failed to release ipv6 address: %v", err) - return invoker.plugin.Errorf("Failed to release ipv6 address: %v", err) + log.Printf("Releasing ipv6 address :%s pool: %s", + nwCfgIpv6.IPAM.Address, nwCfgIpv6.IPAM.Subnet) + if err := invoker.plugin.DelegateDel(nwCfgIpv6.IPAM.Type, &nwCfgIpv6); err != nil { + log.Printf("Failed to release ipv6 address: %v", err) + return invoker.plugin.Errorf("Failed to release ipv6 address: %v", err) + } + } else { + return invoker.plugin.Errorf("Address is incorrect, not valid IPv4 or IPv6, stack: %+v", string(debug.Stack())) } - } else { - return invoker.plugin.Errorf("Address is incorrect, not valid IPv4 or IPv6, stack: %+v", string(debug.Stack())) } return nil diff --git a/cni/network/invoker_azure_test.go b/cni/network/invoker_azure_test.go index 68d96a4b66..8584e63250 100644 --- a/cni/network/invoker_azure_test.go +++ b/cni/network/invoker_azure_test.go @@ -235,10 +235,10 @@ func TestAzureIPAMInvoker_Delete(t *testing.T) { nwInfo *network.NetworkInfo } type args struct { - address *net.IPNet - nwCfg *cni.NetworkConfig - in2 *cniSkel.CmdArgs - options map[string]interface{} + addresses []*net.IPNet + nwCfg *cni.NetworkConfig + in2 *cniSkel.CmdArgs + options map[string]interface{} } tests := []struct { name string @@ -255,7 +255,9 @@ func TestAzureIPAMInvoker_Delete(t *testing.T) { nwInfo: getNwInfo("10.0.0.0/24", ""), }, args: args{ - address: getCIDRNotationForAddress("10.0.0.4/24"), + addresses: []*net.IPNet{ + getCIDRNotationForAddress("10.0.0.4/24"), + }, nwCfg: &cni.NetworkConfig{ IPAM: cni.IPAM{ Address: "10.0.0.4", @@ -272,7 +274,29 @@ func TestAzureIPAMInvoker_Delete(t *testing.T) { nwInfo: getNwInfo("10.0.0.0/24", "2001:db8:abcd:0012::0/64"), }, args: args{ - address: getCIDRNotationForAddress("2001:db8:abcd:0015::0/64"), + addresses: []*net.IPNet{ + getCIDRNotationForAddress("2001:db8:abcd:0015::0/64"), + }, + nwCfg: &cni.NetworkConfig{ + IPAM: cni.IPAM{ + Address: "2001:db8:abcd:0015::0/64", + }, + }, + }, + }, + { + name: "delete happy path ipv4+ipv6", + fields: fields{ + plugin: &mockDelegatePlugin{ + del: del{}, + }, + nwInfo: getNwInfo("10.0.0.0/24", "2001:db8:abcd:0012::0/64"), + }, + args: args{ + addresses: []*net.IPNet{ + getCIDRNotationForAddress("10.0.0.4/24"), + getCIDRNotationForAddress("2001:db8:abcd:0015::0/64"), + }, nwCfg: &cni.NetworkConfig{ IPAM: cni.IPAM{ Address: "2001:db8:abcd:0015::0/64", @@ -281,17 +305,17 @@ func TestAzureIPAMInvoker_Delete(t *testing.T) { }, }, { - name: "error address is nil", + name: "error addresses is nil", fields: fields{ plugin: &mockDelegatePlugin{ del: del{ - err: errors.New("error when address is nil"), //nolint:goerr113 + err: errors.New("error when addresses is nil"), //nolint:goerr113 // error }, }, nwInfo: getNwInfo("", "2001:db8:abcd:0012::0/64"), }, args: args{ - address: nil, + addresses: nil, nwCfg: &cni.NetworkConfig{ IPAM: cni.IPAM{ Address: "2001:db8:abcd:0015::0/64", @@ -311,7 +335,9 @@ func TestAzureIPAMInvoker_Delete(t *testing.T) { nwInfo: getNwInfo("10.0.0.0/24", ""), }, args: args{ - address: getCIDRNotationForAddress("10.0.0.4/24"), + addresses: []*net.IPNet{ + getCIDRNotationForAddress("10.0.0.4/24"), + }, nwCfg: &cni.NetworkConfig{ IPAM: cni.IPAM{ Address: "10.0.0.4/24", @@ -331,7 +357,9 @@ func TestAzureIPAMInvoker_Delete(t *testing.T) { nwInfo: getNwInfo("10.0.0.0/24", "2001:db8:abcd:0012::0/64"), }, args: args{ - address: getCIDRNotationForAddress("2001:db8:abcd:0015::0/64"), + addresses: []*net.IPNet{ + getCIDRNotationForAddress("2001:db8:abcd:0015::0/64"), + }, nwCfg: &cni.NetworkConfig{ IPAM: cni.IPAM{ Address: "10.0.0.4/24", @@ -349,7 +377,7 @@ func TestAzureIPAMInvoker_Delete(t *testing.T) { plugin: tt.fields.plugin, nwInfo: tt.fields.nwInfo, } - err := invoker.Delete(tt.args.address, tt.args.nwCfg, tt.args.in2, tt.args.options) + err := invoker.Delete(tt.args.addresses, tt.args.nwCfg, tt.args.in2, tt.args.options) if tt.wantErr { require.NotNil(err) return diff --git a/cni/network/invoker_cns.go b/cni/network/invoker_cns.go index c484824d7e..39b5c561dd 100644 --- a/cni/network/invoker_cns.go +++ b/cni/network/invoker_cns.go @@ -9,6 +9,7 @@ import ( "github.com/Azure/azure-container-networking/cni" "github.com/Azure/azure-container-networking/cni/util" "github.com/Azure/azure-container-networking/cns" + cnscli "github.com/Azure/azure-container-networking/cns/client" "github.com/Azure/azure-container-networking/iptables" "github.com/Azure/azure-container-networking/log" "github.com/Azure/azure-container-networking/network" @@ -20,8 +21,10 @@ import ( ) var ( - errEmptyCNIArgs = errors.New("empty CNI cmd args not allowed") - errInvalidArgs = errors.New("invalid arg(s)") + errEmptyCNIArgs = errors.New("empty CNI cmd args not allowed") + errInvalidArgs = errors.New("invalid arg(s)") + overlayGatewayv4IP = "169.254.1.1" + overlayGatewayv6IP = "fe80::1234:5678:9abc" ) type CNSIPAMInvoker struct { @@ -32,7 +35,7 @@ type CNSIPAMInvoker struct { ipamMode util.IpamMode } -type IPv4ResultInfo struct { +type IPResultInfo struct { podIPAddress string ncSubnetPrefix uint8 ncPrimaryIP string @@ -70,94 +73,115 @@ func (invoker *CNSIPAMInvoker) Add(addConfig IPAMAddConfig) (IPAMAddResult, erro return IPAMAddResult{}, errEmptyCNIArgs } - ipconfig := cns.IPConfigRequest{ + ipconfig := cns.IPConfigsRequest{ OrchestratorContext: orchestratorContext, PodInterfaceID: GetEndpointID(addConfig.args), InfraContainerID: addConfig.args.ContainerID, } log.Printf("Requesting IP for pod %+v using ipconfig %+v", podInfo, ipconfig) - response, err := invoker.cnsClient.RequestIPAddress(context.TODO(), ipconfig) + response, err := invoker.cnsClient.RequestIPs(context.TODO(), ipconfig) if err != nil { log.Printf("Failed to get IP address from CNS with error %v, response: %v", err, response) return IPAMAddResult{}, errors.Wrap(err, "Failed to get IP address from CNS with error: %w") } - info := IPv4ResultInfo{ - podIPAddress: response.PodIpInfo.PodIPConfig.IPAddress, - ncSubnetPrefix: response.PodIpInfo.NetworkContainerPrimaryIPConfig.IPSubnet.PrefixLength, - ncPrimaryIP: response.PodIpInfo.NetworkContainerPrimaryIPConfig.IPSubnet.IPAddress, - ncGatewayIPAddress: response.PodIpInfo.NetworkContainerPrimaryIPConfig.GatewayIPAddress, - hostSubnet: response.PodIpInfo.HostPrimaryIPInfo.Subnet, - hostPrimaryIP: response.PodIpInfo.HostPrimaryIPInfo.PrimaryIP, - hostGateway: response.PodIpInfo.HostPrimaryIPInfo.Gateway, - } + addResult := IPAMAddResult{} + + for i := 0; i < len(response.PodIPInfo); i++ { + info := IPResultInfo{ + podIPAddress: response.PodIPInfo[i].PodIPConfig.IPAddress, + ncSubnetPrefix: response.PodIPInfo[i].NetworkContainerPrimaryIPConfig.IPSubnet.PrefixLength, + ncPrimaryIP: response.PodIPInfo[i].NetworkContainerPrimaryIPConfig.IPSubnet.IPAddress, + ncGatewayIPAddress: response.PodIPInfo[i].NetworkContainerPrimaryIPConfig.GatewayIPAddress, + 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 - addConfig.options[network.SNATIPKey] = info.ncPrimaryIP + // set the NC Primary IP in options + addConfig.options[network.SNATIPKey] = info.ncPrimaryIP - log.Printf("[cni-invoker-cns] Received info %+v for pod %v", info, podInfo) + log.Printf("[cni-invoker-cns] Received info %+v for pod %v", info, podInfo) - // set result ipconfigArgument from CNS Response Body - 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 { + if (invoker.ipamMode != util.V4Overlay) && (invoker.ipamMode != util.DualStackOverlay) { + return IPAMAddResult{}, errors.Wrap(errInvalidArgs, "%w: Gateway address "+info.ncGatewayIPAddress+" from response is invalid") + } - ncgw := net.ParseIP(info.ncGatewayIPAddress) - if ncgw == nil { - if invoker.ipamMode != util.V4Overlay { - return IPAMAddResult{}, errors.Wrap(errInvalidArgs, "%w: Gateway address "+info.ncGatewayIPAddress+" from response is invalid") + if net.ParseIP(info.podIPAddress).To4() != nil { + ncgw = net.ParseIP(overlayGatewayv4IP) + } else { + ncgw = net.ParseIP(overlayGatewayv6IP) + } } - ncgw, err = getOverlayGateway(ncipnet) - if err != nil { - return IPAMAddResult{}, err + // set result ipconfigArgument from CNS Response Body + 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") } - } - // construct ipnet for result - resultIPnet := net.IPNet{ - IP: ip, - Mask: ncipnet.Mask, - } + // construct ipnet for result + resultIPnet := net.IPNet{ + IP: ip, + Mask: ncIPNet.Mask, + } - addResult := IPAMAddResult{} - addResult.ipv4Result = &cniTypesCurr.Result{ - IPs: []*cniTypesCurr.IPConfig{ - { - Address: resultIPnet, - Gateway: ncgw, - }, - }, - Routes: []*cniTypes.Route{ - { - Dst: network.Ipv4DefaultRouteDstPrefix, - GW: ncgw, - }, - }, - } + 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, + }, + }, + } + } else { + addResult.ipv6Result = &cniTypesCurr.Result{ + IPs: []*cniTypesCurr.IPConfig{ + { + Address: resultIPnet, + Gateway: ncgw, + }, + }, + Routes: []*cniTypes.Route{ + { + Dst: network.Ipv6DefaultRouteDstPrefix, + GW: ncgw, + }, + }, + } + } - // 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) - } + // 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 + addResult.hostSubnetPrefix = *hostIPNet - // set subnet prefix for host vm - // setHostOptions will execute if IPAM mode is not v4 overlay - if invoker.ipamMode != util.V4Overlay { - if err := setHostOptions(ncipnet, addConfig.options, &info); err != nil { - return IPAMAddResult{}, err + // set subnet prefix for host vm + // setHostOptions will execute if IPAM mode is not v4 overlay + if (invoker.ipamMode != util.V4Overlay) && (invoker.ipamMode != util.DualStackOverlay) { + if err := setHostOptions(ncIPNet, addConfig.options, &info); err != nil { + return IPAMAddResult{}, err + } } } return addResult, nil } -func setHostOptions(ncSubnetPrefix *net.IPNet, options map[string]interface{}, info *IPv4ResultInfo) error { +func setHostOptions(ncSubnetPrefix *net.IPNet, options map[string]interface{}, info *IPResultInfo) error { // get the host ip hostIP := net.ParseIP(info.hostPrimaryIP) if hostIP == nil { @@ -213,7 +237,7 @@ func setHostOptions(ncSubnetPrefix *net.IPNet, options map[string]interface{}, i } // Delete calls into the releaseipconfiguration API in CNS -func (invoker *CNSIPAMInvoker) Delete(address *net.IPNet, nwCfg *cni.NetworkConfig, args *cniSkel.CmdArgs, _ map[string]interface{}) error { +func (invoker *CNSIPAMInvoker) Delete(addresses []*net.IPNet, nwCfg *cni.NetworkConfig, args *cniSkel.CmdArgs, _ map[string]interface{}) error { // Parse Pod arguments. podInfo := cns.KubernetesPodInfo{ PodName: invoker.podName, @@ -229,20 +253,42 @@ func (invoker *CNSIPAMInvoker) Delete(address *net.IPNet, nwCfg *cni.NetworkConf return errEmptyCNIArgs } - req := cns.IPConfigRequest{ + req := cns.IPConfigsRequest{ OrchestratorContext: orchestratorContext, PodInterfaceID: GetEndpointID(args), InfraContainerID: args.ContainerID, } - if address != nil { - req.DesiredIPAddress = address.IP.String() + if len(addresses) > 0 { + req.DesiredIPAddresses = make([]string, len(addresses)) + for i, ipAddress := range addresses { + req.DesiredIPAddresses[i] = ipAddress.IP.String() + } } else { log.Printf("CNS invoker called with empty IP address") } - if err := invoker.cnsClient.ReleaseIPAddress(context.TODO(), req); err != nil { - return errors.Wrap(err, fmt.Sprintf("failed to release IP %v with err ", address)+"%w") + if err := invoker.cnsClient.ReleaseIPs(context.TODO(), req); err != nil { + // if we fail a release with a 404 error try using the old API + if errors.Is(err, cnscli.ErrAPINotFound) { + ipconfigRequest := cns.IPConfigRequest{ + OrchestratorContext: orchestratorContext, + PodInterfaceID: GetEndpointID(args), + InfraContainerID: args.ContainerID, + DesiredIPAddress: req.DesiredIPAddresses[0], + } + log.Errorf("Failed to release IPs using ReleaseIPs from CNS, going to try ReleaseIPAddress. error: %v request: %v", err, req) + + if err := invoker.cnsClient.ReleaseIPAddress(context.TODO(), ipconfigRequest); err != nil { + // if the old API fails as well then we just return the error + log.Errorf("Failed to release IP address from CNS using ReleaseIPAddress. error: %v request: %v", err, req) + return errors.Wrap(err, fmt.Sprintf("failed to release IP %v using ReleaseIPAddress with err ", addresses)+"%w") + } + } else { + log.Errorf("Failed to release IP address from CNS error: %v request: %v", err, req) + return errors.Wrap(err, fmt.Sprintf("failed to release IP %v using ReleaseIPs with err ", addresses)+"%w") + } + } return nil diff --git a/cni/network/invoker_cns_test.go b/cni/network/invoker_cns_test.go index 1506fa176f..9cb570406d 100644 --- a/cni/network/invoker_cns_test.go +++ b/cni/network/invoker_cns_test.go @@ -2,13 +2,10 @@ package network import ( "errors" - "fmt" "net" - "runtime" "testing" "github.com/Azure/azure-container-networking/cni" - "github.com/Azure/azure-container-networking/cni/util" "github.com/Azure/azure-container-networking/cns" "github.com/Azure/azure-container-networking/iptables" "github.com/Azure/azure-container-networking/network" @@ -20,29 +17,20 @@ import ( var testPodInfo cns.KubernetesPodInfo -func getTestIPConfigRequest() cns.IPConfigRequest { - return cns.IPConfigRequest{ +func getTestIPConfigsRequest() cns.IPConfigsRequest { + return cns.IPConfigsRequest{ PodInterfaceID: "testcont-testifname", InfraContainerID: "testcontainerid", OrchestratorContext: marshallPodInfo(testPodInfo), } } -func getTestOverlayGateway() net.IP { - if runtime.GOOS == "windows" { - return net.ParseIP("10.240.0.1") - } - - return net.ParseIP("169.254.1.1") -} - func TestCNSIPAMInvoker_Add(t *testing.T) { require := require.New(t) //nolint further usage of require without passing t type fields struct { podName string podNamespace string cnsClient cnsclient - ipamMode util.IpamMode } type args struct { nwCfg *cni.NetworkConfig @@ -50,7 +38,6 @@ func TestCNSIPAMInvoker_Add(t *testing.T) { hostSubnetPrefix *net.IPNet options map[string]interface{} } - tests := []struct { name string fields fields @@ -66,26 +53,47 @@ func TestCNSIPAMInvoker_Add(t *testing.T) { podNamespace: testPodInfo.PodNamespace, cnsClient: &MockCNSClient{ require: require, - request: requestIPAddressHandler{ - ipconfigArgument: getTestIPConfigRequest(), - result: &cns.IPConfigResponse{ - PodIpInfo: cns.PodIpInfo{ - PodIPConfig: cns.IPSubnet{ - IPAddress: "10.0.1.10", - PrefixLength: 24, - }, - NetworkContainerPrimaryIPConfig: cns.IPConfiguration{ - IPSubnet: cns.IPSubnet{ - IPAddress: "10.0.1.0", + requestIPs: requestIPsHandler{ + ipconfigArgument: getTestIPConfigsRequest(), + result: &cns.IPConfigsResponse{ + PodIPInfo: []cns.PodIpInfo{ + { + PodIPConfig: cns.IPSubnet{ + IPAddress: "10.0.1.10", PrefixLength: 24, }, - DNSServers: nil, - GatewayIPAddress: "10.0.0.1", + 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", + }, }, - HostPrimaryIPInfo: cns.HostIPInfo{ - Gateway: "10.0.0.1", - PrimaryIP: "10.0.0.1", - Subnet: "10.0.0.0/24", + { + PodIPConfig: cns.IPSubnet{ + IPAddress: "fd11:1234::1", + PrefixLength: 24, + }, + NetworkContainerPrimaryIPConfig: cns.IPConfiguration{ + IPSubnet: cns.IPSubnet{ + IPAddress: "fd11:1234::", + PrefixLength: 112, + }, + DNSServers: nil, + GatewayIPAddress: "fe80::1234:5678:9abc", + }, + HostPrimaryIPInfo: cns.HostIPInfo{ + Gateway: "fe80::1234:5678:9abc", + PrimaryIP: "fe80::1234:5678:9abc", + Subnet: "fd11:1234::/112", + }, }, }, Response: cns.Response{ @@ -121,18 +129,31 @@ func TestCNSIPAMInvoker_Add(t *testing.T) { }, }, }, - want1: nil, + want1: &cniTypesCurr.Result{ + IPs: []*cniTypesCurr.IPConfig{ + { + Address: *getCIDRNotationForAddress("fd11:1234::1/112"), + Gateway: net.ParseIP("fe80::1234:5678:9abc"), + }, + }, + Routes: []*cniTypes.Route{ + { + Dst: network.Ipv6DefaultRouteDstPrefix, + GW: net.ParseIP("fe80::1234:5678:9abc"), + }, + }, + }, wantErr: false, }, { - name: "fail to request IP address from cns", + name: "fail to request IP addresses from cns", fields: fields{ podName: testPodInfo.PodName, podNamespace: testPodInfo.PodNamespace, cnsClient: &MockCNSClient{ require: require, - request: requestIPAddressHandler{ - ipconfigArgument: getTestIPConfigRequest(), + requestIPs: requestIPsHandler{ + ipconfigArgument: getTestIPConfigsRequest(), result: nil, err: errors.New("failed error from CNS"), //nolint "error for ut" }, @@ -140,76 +161,6 @@ func TestCNSIPAMInvoker_Add(t *testing.T) { }, wantErr: true, }, - { - name: "Test happy CNI Overlay add", - fields: fields{ - podName: testPodInfo.PodName, - podNamespace: testPodInfo.PodNamespace, - ipamMode: util.V4Overlay, - cnsClient: &MockCNSClient{ - require: require, - request: requestIPAddressHandler{ - ipconfigArgument: cns.IPConfigRequest{ - PodInterfaceID: "testcont-testifname3", - InfraContainerID: "testcontainerid3", - OrchestratorContext: marshallPodInfo(testPodInfo), - }, - result: &cns.IPConfigResponse{ - PodIpInfo: cns.PodIpInfo{ - PodIPConfig: cns.IPSubnet{ - IPAddress: "10.240.1.242", - PrefixLength: 16, - }, - NetworkContainerPrimaryIPConfig: cns.IPConfiguration{ - IPSubnet: cns.IPSubnet{ - IPAddress: "10.240.1.0", - PrefixLength: 16, - }, - DNSServers: nil, - GatewayIPAddress: "", - }, - HostPrimaryIPInfo: cns.HostIPInfo{ - Gateway: "10.224.0.1", - PrimaryIP: "10.224.0.5", - Subnet: "10.224.0.0/16", - }, - }, - 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.224.0.0/16"), - options: map[string]interface{}{}, - }, - want: &cniTypesCurr.Result{ - IPs: []*cniTypesCurr.IPConfig{ - { - Address: *getCIDRNotationForAddress("10.240.1.242/16"), - Gateway: getTestOverlayGateway(), - }, - }, - Routes: []*cniTypes.Route{ - { - Dst: network.Ipv4DefaultRouteDstPrefix, - GW: getTestOverlayGateway(), - }, - }, - }, - want1: nil, - wantErr: false, - }, } for _, tt := range tests { tt := tt @@ -219,9 +170,6 @@ func TestCNSIPAMInvoker_Add(t *testing.T) { podNamespace: tt.fields.podNamespace, cnsClient: tt.fields.cnsClient, } - if tt.fields.ipamMode != "" { - invoker.ipamMode = tt.fields.ipamMode - } ipamAddResult, err := invoker.Add(IPAMAddConfig{nwCfg: tt.args.nwCfg, args: tt.args.args, options: tt.args.options}) if tt.wantErr { require.Error(err) @@ -229,7 +177,6 @@ func TestCNSIPAMInvoker_Add(t *testing.T) { require.NoError(err) } - fmt.Printf("want:%+v\nrest:%+v\n", tt.want, ipamAddResult.ipv4Result) require.Equalf(tt.want, ipamAddResult.ipv4Result, "incorrect ipv4 response") require.Equalf(tt.want1, ipamAddResult.ipv6Result, "incorrect ipv6 response") }) @@ -244,10 +191,10 @@ func TestCNSIPAMInvoker_Delete(t *testing.T) { cnsClient cnsclient } type args struct { - address *net.IPNet - nwCfg *cni.NetworkConfig - args *cniSkel.CmdArgs - options map[string]interface{} + addresses []*net.IPNet + nwCfg *cni.NetworkConfig + args *cniSkel.CmdArgs + options map[string]interface{} } tests := []struct { name string @@ -262,8 +209,8 @@ func TestCNSIPAMInvoker_Delete(t *testing.T) { podNamespace: testPodInfo.PodNamespace, cnsClient: &MockCNSClient{ require: require, - release: releaseIPAddressHandler{ - ipconfigArgument: getTestIPConfigRequest(), + release: releaseIPsHandler{ + ipconfigArgument: getTestIPConfigsRequest(), }, }, }, @@ -283,8 +230,8 @@ func TestCNSIPAMInvoker_Delete(t *testing.T) { podName: testPodInfo.PodName, podNamespace: testPodInfo.PodNamespace, cnsClient: &MockCNSClient{ - release: releaseIPAddressHandler{ - ipconfigArgument: getTestIPConfigRequest(), + release: releaseIPsHandler{ + ipconfigArgument: getTestIPConfigsRequest(), err: errors.New("handle CNS delete error"), //nolint ut error }, }, @@ -300,7 +247,7 @@ func TestCNSIPAMInvoker_Delete(t *testing.T) { podNamespace: tt.fields.podNamespace, cnsClient: tt.fields.cnsClient, } - err := invoker.Delete(tt.args.address, tt.args.nwCfg, tt.args.args, tt.args.options) + err := invoker.Delete(tt.args.addresses, tt.args.nwCfg, tt.args.args, tt.args.options) if tt.wantErr { require.Error(err) } else { @@ -316,7 +263,7 @@ func Test_setHostOptions(t *testing.T) { hostSubnetPrefix *net.IPNet ncSubnetPrefix *net.IPNet options map[string]interface{} - info IPv4ResultInfo + info IPResultInfo } tests := []struct { name string @@ -330,7 +277,7 @@ func Test_setHostOptions(t *testing.T) { hostSubnetPrefix: getCIDRNotationForAddress("10.0.1.0/24"), ncSubnetPrefix: getCIDRNotationForAddress("10.0.1.0/24"), options: map[string]interface{}{}, - info: IPv4ResultInfo{ + info: IPResultInfo{ podIPAddress: "10.0.1.10", ncSubnetPrefix: 24, ncPrimaryIP: "10.0.1.20", @@ -376,7 +323,7 @@ func Test_setHostOptions(t *testing.T) { { name: "test error on bad host subnet", args: args{ - info: IPv4ResultInfo{ + info: IPResultInfo{ hostSubnet: "", }, }, @@ -385,7 +332,7 @@ func Test_setHostOptions(t *testing.T) { { name: "test error on nil hostsubnetprefix", args: args{ - info: IPv4ResultInfo{ + info: IPResultInfo{ hostSubnet: "10.0.0.0/24", }, }, diff --git a/cni/network/invoker_mock.go b/cni/network/invoker_mock.go index 51e04c29ce..9b4afa884f 100644 --- a/cni/network/invoker_mock.go +++ b/cni/network/invoker_mock.go @@ -79,18 +79,20 @@ func (invoker *MockIpamInvoker) Add(opt IPAMAddConfig) (ipamAddResult IPAMAddRes return ipamAddResult, nil } -func (invoker *MockIpamInvoker) Delete(address *net.IPNet, nwCfg *cni.NetworkConfig, _ *skel.CmdArgs, options map[string]interface{}) error { +func (invoker *MockIpamInvoker) Delete(addresses []*net.IPNet, nwCfg *cni.NetworkConfig, _ *skel.CmdArgs, options map[string]interface{}) error { if invoker.v4Fail || invoker.v6Fail { return errDeleteIpam } - if address == nil || invoker.ipMap == nil { + if len(addresses) <= 0 || invoker.ipMap == nil { return nil } - if _, ok := invoker.ipMap[address.String()]; !ok { - return errDeleteIpam + for _, address := range addresses { + if _, ok := invoker.ipMap[address.String()]; !ok { + return errDeleteIpam + } + delete(invoker.ipMap, address.String()) } - delete(invoker.ipMap, address.String()) return nil } diff --git a/cni/network/multitenancy.go b/cni/network/multitenancy.go index f39a429bd2..011f99841a 100644 --- a/cni/network/multitenancy.go +++ b/cni/network/multitenancy.go @@ -3,348 +3,493 @@ package network import ( "context" "encoding/json" - "errors" - "fmt" - "io" "net" - "net/http" - "os" - "strings" - "time" + "testing" "github.com/Azure/azure-container-networking/cni" "github.com/Azure/azure-container-networking/cns" - "github.com/Azure/azure-container-networking/cns/client" - "github.com/Azure/azure-container-networking/common" - "github.com/Azure/azure-container-networking/log" "github.com/Azure/azure-container-networking/network" cniTypes "github.com/containernetworking/cni/pkg/types" cniTypesCurr "github.com/containernetworking/cni/pkg/types/100" + "github.com/stretchr/testify/require" ) -const ( - filePerm = 0o664 - httpTimeout = 5 -) +// Handler structs +type requestIPAddressHandler struct { + // arguments + ipconfigArgument cns.IPConfigRequest -// MultitenancyClient interface -type MultitenancyClient interface { - SetupRoutingForMultitenancy( - nwCfg *cni.NetworkConfig, - cnsNetworkConfig *cns.GetNetworkContainerResponse, - azIpamResult *cniTypesCurr.Result, - epInfo *network.EndpointInfo, - result *cniTypesCurr.Result) - DetermineSnatFeatureOnHost( - snatFile string, - nmAgentSupportedApisURL string) (bool, bool, error) - GetAllNetworkContainers( - ctx context.Context, - nwCfg *cni.NetworkConfig, - podName string, - podNamespace string, - ifName string) ([]IPAMAddResult, error) - Init(cnsclient cnsclient, netioshim netioshim) + // results + result *cns.IPConfigResponse + err error } -type Multitenancy struct { - // cnsclient is used to communicate with CNS - cnsclient cnsclient +type requestIPsHandler struct { + // arguments + ipconfigArgument cns.IPConfigsRequest - // netioshim is used to interact with networking syscalls - netioshim netioshim + // results + result *cns.IPConfigsResponse // this will return the IPConfigsResponse which contains a slice of IPs as opposed to one IP + err error } -type netioshim interface { - GetInterfaceSubnetWithSpecificIP(ipAddr string) *net.IPNet +type releaseIPsHandler struct { + ipconfigArgument cns.IPConfigsRequest + err error } -type AzureNetIOShim struct{} - -func (a AzureNetIOShim) GetInterfaceSubnetWithSpecificIP(ipAddr string) *net.IPNet { - return common.GetInterfaceSubnetWithSpecificIP(ipAddr) +type getNetworkContainerConfigurationHandler struct { + orchestratorContext []byte + returnResponse *cns.GetNetworkContainerResponse + err error } -var errNmaResponse = errors.New("nmagent request status code") - -func (m *Multitenancy) Init(cnsclient cnsclient, netioshim netioshim) { - m.cnsclient = cnsclient - m.netioshim = netioshim +// this is to get all the NCs for testing with given orchestratorContext +type getAllNetworkContainersConfigurationHandler struct { + orchestratorContext []byte + returnResponse []cns.GetNetworkContainerResponse + err error } -// DetermineSnatFeatureOnHost - Temporary function to determine whether we need to disable SNAT due to NMAgent support -func (m *Multitenancy) DetermineSnatFeatureOnHost(snatFile, nmAgentSupportedApisURL string) (snatForDNS, snatOnHost bool, err error) { - var ( - snatConfig snatConfiguration - retrieveSnatConfigErr error - jsonFile *os.File - httpClient = &http.Client{Timeout: time.Second * httpTimeout} - snatConfigFile = snatConfigFileName + jsonFileExtension - ) - - // Check if we've already retrieved NMAgent version and determined whether to disable snat on host - if jsonFile, retrieveSnatConfigErr = os.Open(snatFile); retrieveSnatConfigErr == nil { - bytes, _ := io.ReadAll(jsonFile) - jsonFile.Close() - if retrieveSnatConfigErr = json.Unmarshal(bytes, &snatConfig); retrieveSnatConfigErr != nil { - log.Errorf("[cni-net] failed to unmarshal to snatConfig with error %v", - retrieveSnatConfigErr) - } - } +type MockCNSClient struct { + require *require.Assertions + request requestIPAddressHandler + requestIPs requestIPsHandler + release releaseIPsHandler + getNetworkContainerConfiguration getNetworkContainerConfigurationHandler + getAllNetworkContainersConfiguration getAllNetworkContainersConfigurationHandler +} - // If we weren't able to retrieve snatConfiguration, query NMAgent - if retrieveSnatConfigErr != nil { - var resp *http.Response - req, err := http.NewRequestWithContext(context.TODO(), http.MethodGet, nmAgentSupportedApisURL, nil) - if err != nil { - log.Errorf("failed creating http request:%+v", err) - return false, false, fmt.Errorf("%w", err) - } - log.Printf("Query nma for dns snat support: %s", nmAgentSupportedApisURL) - resp, retrieveSnatConfigErr = httpClient.Do(req) - if retrieveSnatConfigErr == nil { - defer resp.Body.Close() - - if resp.StatusCode == http.StatusOK { - var bodyBytes []byte - // if the list of APIs (strings) contains the nmAgentSnatSupportAPI we will disable snat on host - if bodyBytes, retrieveSnatConfigErr = io.ReadAll(resp.Body); retrieveSnatConfigErr == nil { - bodyStr := string(bodyBytes) - if !strings.Contains(bodyStr, nmAgentSnatAndDnsSupportAPI) { - snatConfig.EnableSnatForDns = true - snatConfig.EnableSnatOnHost = !strings.Contains(bodyStr, nmAgentSnatSupportAPI) - } - - jsonStr, _ := json.Marshal(snatConfig) - fp, err := os.OpenFile(snatConfigFile, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, os.FileMode(filePerm)) - if err == nil { - _, err = fp.Write(jsonStr) - if err != nil { - log.Errorf("DetermineSnatFeatureOnHost: Write to json failed:%+v", err) - } - fp.Close() - } else { - log.Errorf("[cni-net] failed to save snat settings to %s with error: %+v", snatConfigFile, err) - } - } - } else { - retrieveSnatConfigErr = fmt.Errorf("%w:%d", errNmaResponse, resp.StatusCode) - } - } - } +func (c *MockCNSClient) RequestIPAddress(_ context.Context, ipconfig cns.IPConfigRequest) (*cns.IPConfigResponse, error) { + c.require.Exactly(c.request.ipconfigArgument, ipconfig) + return c.request.result, c.request.err +} - // Log and return the error when we fail acquire snat configuration for host and dns - if retrieveSnatConfigErr != nil { - log.Errorf("[cni-net] failed to acquire SNAT configuration with error %v", - retrieveSnatConfigErr) - return snatConfig.EnableSnatForDns, snatConfig.EnableSnatOnHost, retrieveSnatConfigErr - } +func (c *MockCNSClient) RequestIPs(_ context.Context, ipconfig cns.IPConfigsRequest) (*cns.IPConfigsResponse, error) { + c.require.Exactly(c.requestIPs.ipconfigArgument, ipconfig) + return c.requestIPs.result, c.requestIPs.err +} - log.Printf("[cni-net] saved snat settings %+v to %s", snatConfig, snatConfigFile) - if snatConfig.EnableSnatOnHost { - log.Printf("[cni-net] enabling SNAT on container host for outbound connectivity") - } - if snatConfig.EnableSnatForDns { - log.Printf("[cni-net] enabling SNAT on container host for DNS traffic") - } - if !snatConfig.EnableSnatForDns && !snatConfig.EnableSnatOnHost { - log.Printf("[cni-net] disabling SNAT on container host") - } +func (c *MockCNSClient) ReleaseIPs(_ context.Context, ipconfig cns.IPConfigsRequest) error { + c.require.Exactly(c.release.ipconfigArgument, ipconfig) + return c.release.err +} - return snatConfig.EnableSnatForDns, snatConfig.EnableSnatOnHost, nil +func (c *MockCNSClient) ReleaseIPAddress(_ context.Context, ipconfig cns.IPConfigRequest) error { + c.require.Exactly(c.release.ipconfigArgument, ipconfig) + return c.release.err } -func (m *Multitenancy) SetupRoutingForMultitenancy( - nwCfg *cni.NetworkConfig, - cnsNetworkConfig *cns.GetNetworkContainerResponse, - azIpamResult *cniTypesCurr.Result, - epInfo *network.EndpointInfo, - result *cniTypesCurr.Result, -) { - // Adding default gateway - // if snat enabled, add 169.254.128.1 as default gateway - if nwCfg.EnableSnatOnHost { - log.Printf("add default route for multitenancy.snat on host enabled") - addDefaultRoute(cnsNetworkConfig.LocalIPConfiguration.GatewayIPAddress, epInfo, result) - } else { - _, defaultIPNet, _ := net.ParseCIDR("0.0.0.0/0") - dstIP := net.IPNet{IP: net.ParseIP("0.0.0.0"), Mask: defaultIPNet.Mask} - gwIP := net.ParseIP(cnsNetworkConfig.IPConfiguration.GatewayIPAddress) - epInfo.Routes = append(epInfo.Routes, network.RouteInfo{Dst: dstIP, Gw: gwIP}) - result.Routes = append(result.Routes, &cniTypes.Route{Dst: dstIP, GW: gwIP}) - - if epInfo.EnableSnatForDns { - log.Printf("add SNAT for DNS enabled") - addSnatForDNS(cnsNetworkConfig.LocalIPConfiguration.GatewayIPAddress, epInfo, result) - } - } +func (c *MockCNSClient) GetNetworkContainer(ctx context.Context, orchestratorContext []byte) (*cns.GetNetworkContainerResponse, error) { + c.require.Exactly(c.getNetworkContainerConfiguration.orchestratorContext, orchestratorContext) + return c.getNetworkContainerConfiguration.returnResponse, c.getNetworkContainerConfiguration.err +} - setupInfraVnetRoutingForMultitenancy(nwCfg, azIpamResult, epInfo, result) +func (c *MockCNSClient) GetAllNetworkContainers(ctx context.Context, orchestratorContext []byte) ([]cns.GetNetworkContainerResponse, error) { + c.require.Exactly(c.getAllNetworkContainersConfiguration.orchestratorContext, orchestratorContext) + return c.getAllNetworkContainersConfiguration.returnResponse, c.getAllNetworkContainersConfiguration.err } -// get all network container configuration(s) for given orchestratorContext -func (m *Multitenancy) GetAllNetworkContainers( - ctx context.Context, nwCfg *cni.NetworkConfig, podName, podNamespace, ifName string, -) ([]IPAMAddResult, error) { - var podNameWithoutSuffix string +func defaultIPNet() *net.IPNet { + _, defaultIPNet, _ := net.ParseCIDR("0.0.0.0/0") + return defaultIPNet +} - if !nwCfg.EnableExactMatchForPodName { - podNameWithoutSuffix = network.GetPodNameWithoutSuffix(podName) - } else { - podNameWithoutSuffix = podName - } +func marshallPodInfo(podInfo cns.KubernetesPodInfo) []byte { + orchestratorContext, _ := json.Marshal(podInfo) + return orchestratorContext +} - log.Printf("Podname without suffix %v", podNameWithoutSuffix) +type mockNetIOShim struct{} - ncResponses, hostSubnetPrefixes, err := m.getNetworkContainersInternal(ctx, podNamespace, podNameWithoutSuffix) - if err != nil { - return []IPAMAddResult{}, fmt.Errorf("%w", err) - } +func (a *mockNetIOShim) GetInterfaceSubnetWithSpecificIP(ipAddr string) *net.IPNet { + return getCIDRNotationForAddress(ipAddr) +} - for i := 0; i < len(ncResponses); i++ { - if nwCfg.EnableSnatOnHost { - if ncResponses[i].LocalIPConfiguration.IPSubnet.IPAddress == "" { - log.Printf("Snat IP is not populated for ncs %+v. Got empty string", ncResponses) - return []IPAMAddResult{}, errSnatIP - } - } +func getIPNet(ipaddr net.IP, mask net.IPMask) net.IPNet { + return net.IPNet{ + IP: ipaddr, + Mask: mask, } +} - ipamResults := make([]IPAMAddResult, len(ncResponses)) - - 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) +func getIPNetWithString(ipaddrwithcidr string) *net.IPNet { + _, ipnet, err := net.ParseCIDR(ipaddrwithcidr) + if err != nil { + panic(err) } - return ipamResults, err + return ipnet } -// get all network containers configuration for given orchestratorContext -func (m *Multitenancy) getNetworkContainersInternal( - ctx context.Context, namespace, podName string, -) ([]cns.GetNetworkContainerResponse, []net.IPNet, error) { - podInfo := cns.KubernetesPodInfo{ - PodName: podName, - PodNamespace: namespace, - } - - orchestratorContext, err := json.Marshal(podInfo) - if err != nil { - log.Printf("Marshalling KubernetesPodInfo failed with %v", err) - return nil, []net.IPNet{}, fmt.Errorf("%w", err) +func TestSetupRoutingForMultitenancy(t *testing.T) { + require := require.New(t) //nolint:gocritic + type args struct { + nwCfg *cni.NetworkConfig + cnsNetworkConfig *cns.GetNetworkContainerResponse + azIpamResult *cniTypesCurr.Result + epInfo *network.EndpointInfo + result *cniTypesCurr.Result } - // First try the new CNS API that returns slice of nc responses. If CNS doesn't support the new API, an error will be returned and as a result - // try using the old CNS API that returns single nc response. - ncConfigs, err := m.cnsclient.GetAllNetworkContainers(ctx, orchestratorContext) - if err != nil && client.IsUnsupportedAPI(err) { - ncConfig, errGetNC := m.cnsclient.GetNetworkContainer(ctx, orchestratorContext) - if errGetNC != nil { - return nil, []net.IPNet{}, fmt.Errorf("%w", err) - } - ncConfigs = append(ncConfigs, *ncConfig) - } else if err != nil { - return nil, []net.IPNet{}, fmt.Errorf("%w", err) + tests := []struct { + name string + args args + multitenancyClient *Multitenancy + expected args + }{ + { + name: "test happy path", + args: args{ + nwCfg: &cni.NetworkConfig{ + MultiTenancy: true, + EnableSnatOnHost: false, + }, + cnsNetworkConfig: &cns.GetNetworkContainerResponse{ + IPConfiguration: cns.IPConfiguration{ + IPSubnet: cns.IPSubnet{}, + DNSServers: nil, + GatewayIPAddress: "10.0.0.1", + }, + }, + epInfo: &network.EndpointInfo{}, + result: &cniTypesCurr.Result{}, + }, + expected: args{ + nwCfg: &cni.NetworkConfig{ + MultiTenancy: true, + EnableSnatOnHost: false, + }, + cnsNetworkConfig: &cns.GetNetworkContainerResponse{ + IPConfiguration: cns.IPConfiguration{ + IPSubnet: cns.IPSubnet{}, + DNSServers: nil, + GatewayIPAddress: "10.0.0.1", + }, + }, + epInfo: &network.EndpointInfo{ + Routes: []network.RouteInfo{ + { + Dst: net.IPNet{IP: net.ParseIP("0.0.0.0"), Mask: defaultIPNet().Mask}, + Gw: net.ParseIP("10.0.0.1"), + }, + }, + }, + result: &cniTypesCurr.Result{ + Routes: []*cniTypes.Route{ + { + Dst: net.IPNet{IP: net.ParseIP("0.0.0.0"), Mask: defaultIPNet().Mask}, + GW: net.ParseIP("10.0.0.1"), + }, + }, + }, + }, + }, } - - log.Printf("Network config received from cns %+v", ncConfigs) - - subnetPrefixes := []net.IPNet{} - for i := 0; i < len(ncConfigs); i++ { - subnetPrefix := m.netioshim.GetInterfaceSubnetWithSpecificIP(ncConfigs[i].PrimaryInterfaceIdentifier) - if subnetPrefix == nil { - log.Printf("%w %s", errIfaceNotFound, ncConfigs[i].PrimaryInterfaceIdentifier) - return nil, []net.IPNet{}, errIfaceNotFound - } - subnetPrefixes = append(subnetPrefixes, *subnetPrefix) + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + tt.multitenancyClient.SetupRoutingForMultitenancy(tt.args.nwCfg, tt.args.cnsNetworkConfig, tt.args.azIpamResult, tt.args.epInfo, tt.args.result) + require.Exactly(tt.expected.nwCfg, tt.args.nwCfg) + require.Exactly(tt.expected.cnsNetworkConfig, tt.args.cnsNetworkConfig) + require.Exactly(tt.expected.azIpamResult, tt.args.azIpamResult) + require.Exactly(tt.expected.epInfo, tt.args.epInfo) + require.Exactly(tt.expected.result, tt.args.result) + }) } - - return ncConfigs, subnetPrefixes, nil } -func convertToCniResult(networkConfig *cns.GetNetworkContainerResponse, ifName string) *cniTypesCurr.Result { - result := &cniTypesCurr.Result{} - resultIpconfig := &cniTypesCurr.IPConfig{} - - ipconfig := networkConfig.IPConfiguration - ipAddr := net.ParseIP(ipconfig.IPSubnet.IPAddress) - - if ipAddr.To4() != nil { - resultIpconfig.Address = net.IPNet{IP: ipAddr, Mask: net.CIDRMask(int(ipconfig.IPSubnet.PrefixLength), 32)} - } else { - resultIpconfig.Address = net.IPNet{IP: ipAddr, Mask: net.CIDRMask(int(ipconfig.IPSubnet.PrefixLength), 128)} +func TestCleanupMultitenancyResources(t *testing.T) { + require := require.New(t) //nolint:gocritic + type args struct { + enableInfraVnet bool + nwCfg *cni.NetworkConfig + infraIPNet *cniTypesCurr.Result + plugin *NetPlugin } - - resultIpconfig.Gateway = net.ParseIP(ipconfig.GatewayIPAddress) - result.IPs = append(result.IPs, resultIpconfig) - - if networkConfig.Routes != nil && len(networkConfig.Routes) > 0 { - for _, route := range networkConfig.Routes { - _, routeIPnet, _ := net.ParseCIDR(route.IPAddress) - gwIP := net.ParseIP(route.GatewayIPAddress) - result.Routes = append(result.Routes, &cniTypes.Route{Dst: *routeIPnet, GW: gwIP}) - } + tests := []struct { + name string + args args + multitenancyClient *Multitenancy + expected args + }{ + { + name: "test happy path", + args: args{ + enableInfraVnet: true, + nwCfg: &cni.NetworkConfig{ + MultiTenancy: true, + }, + infraIPNet: &cniTypesCurr.Result{}, + plugin: &NetPlugin{ + ipamInvoker: NewMockIpamInvoker(false, false, false), + }, + }, + expected: args{ + nwCfg: &cni.NetworkConfig{ + MultiTenancy: true, + EnableSnatOnHost: false, + IPAM: cni.IPAM{}, + }, + infraIPNet: &cniTypesCurr.Result{}, + plugin: &NetPlugin{ + ipamInvoker: NewMockIpamInvoker(false, false, false), + }, + }, + }, } - - for _, ipRouteSubnet := range networkConfig.CnetAddressSpace { - routeIPnet := net.IPNet{IP: net.ParseIP(ipRouteSubnet.IPAddress), Mask: net.CIDRMask(int(ipRouteSubnet.PrefixLength), 32)} - gwIP := net.ParseIP(ipconfig.GatewayIPAddress) - result.Routes = append(result.Routes, &cniTypes.Route{Dst: routeIPnet, GW: gwIP}) + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + require.Exactly(tt.expected.nwCfg, tt.args.nwCfg) + require.Exactly(tt.expected.infraIPNet, tt.args.infraIPNet) + require.Exactly(tt.expected.plugin, tt.args.plugin) + }) } - - iface := &cniTypesCurr.Interface{Name: ifName} - result.Interfaces = append(result.Interfaces, iface) - - return result } -func getInfraVnetIP( - enableInfraVnet bool, - infraSubnet string, - nwCfg *cni.NetworkConfig, - plugin *NetPlugin, -) (*cniTypesCurr.Result, error) { - if enableInfraVnet { - _, ipNet, _ := net.ParseCIDR(infraSubnet) - nwCfg.IPAM.Subnet = ipNet.String() - - log.Printf("call ipam to allocate ip from subnet %v", nwCfg.IPAM.Subnet) - ipamAddOpt := IPAMAddConfig{nwCfg: nwCfg, options: make(map[string]interface{})} - ipamAddResult, err := plugin.ipamInvoker.Add(ipamAddOpt) - if err != nil { - err = plugin.Errorf("Failed to allocate address: %v", err) - return nil, err - } - - return ipamAddResult.ipv4Result, nil +func TestGetMultiTenancyCNIResult(t *testing.T) { + require := require.New(t) //nolint:gocritic + + var ncResponses []cns.GetNetworkContainerResponse + ncResponseOne := cns.GetNetworkContainerResponse{ + PrimaryInterfaceIdentifier: "10.0.0.0/16", + LocalIPConfiguration: cns.IPConfiguration{ + IPSubnet: cns.IPSubnet{ + IPAddress: "10.0.0.5", + PrefixLength: 16, + }, + GatewayIPAddress: "", + }, + CnetAddressSpace: []cns.IPSubnet{ + { + IPAddress: "10.1.0.0", + PrefixLength: 16, + }, + }, + IPConfiguration: cns.IPConfiguration{ + IPSubnet: cns.IPSubnet{ + IPAddress: "10.1.0.5", + PrefixLength: 16, + }, + DNSServers: nil, + GatewayIPAddress: "10.1.0.1", + }, + Routes: []cns.Route{ + { + IPAddress: "10.1.0.0/16", + GatewayIPAddress: "10.1.0.1", + }, + }, } - return nil, nil -} + ncResponseTwo := cns.GetNetworkContainerResponse{ + PrimaryInterfaceIdentifier: "20.0.0.0/16", + LocalIPConfiguration: cns.IPConfiguration{ + IPSubnet: cns.IPSubnet{ + IPAddress: "20.0.0.5", + PrefixLength: 16, + }, + GatewayIPAddress: "", + }, + CnetAddressSpace: []cns.IPSubnet{ + { + IPAddress: "20.1.0.0", + PrefixLength: 16, + }, + }, + IPConfiguration: cns.IPConfiguration{ + IPSubnet: cns.IPSubnet{ + IPAddress: "20.1.0.5", + PrefixLength: 16, + }, + DNSServers: nil, + GatewayIPAddress: "20.1.0.1", + }, + Routes: []cns.Route{ + { + IPAddress: "20.1.0.0/16", + GatewayIPAddress: "20.1.0.1", + }, + }, + } + ncResponses = append(ncResponses, ncResponseOne, ncResponseTwo) + + type args struct { + ctx context.Context + enableInfraVnet bool + nwCfg *cni.NetworkConfig + plugin *NetPlugin + k8sPodName string + k8sNamespace string + ifName string + } -func checkIfSubnetOverlaps(enableInfraVnet bool, nwCfg *cni.NetworkConfig, cnsNetworkConfig *cns.GetNetworkContainerResponse) bool { - if enableInfraVnet { - if cnsNetworkConfig != nil { - _, infraNet, _ := net.ParseCIDR(nwCfg.InfraVnetAddressSpace) - for _, cnetSpace := range cnsNetworkConfig.CnetAddressSpace { - cnetSpaceIPNet := &net.IPNet{ - IP: net.ParseIP(cnetSpace.IPAddress), - Mask: net.CIDRMask(int(cnetSpace.PrefixLength), 32), - } - - return infraNet.Contains(cnetSpaceIPNet.IP) || cnetSpaceIPNet.Contains(infraNet.IP) + tests := []struct { + name string + args args + want *cniTypesCurr.Result + want1 *cns.GetNetworkContainerResponse + want2 *cns.GetNetworkContainerResponse + want3 net.IPNet + want4 *cniTypesCurr.Result + want5 []cns.GetNetworkContainerResponse + wantErr bool + }{ + { + name: "test happy path", + args: args{ + enableInfraVnet: true, + nwCfg: &cni.NetworkConfig{ + MultiTenancy: true, + EnableSnatOnHost: true, + EnableExactMatchForPodName: true, + InfraVnetAddressSpace: "10.0.0.0/16", + IPAM: cni.IPAM{Type: "azure-vnet-ipam"}, + }, + plugin: &NetPlugin{ + ipamInvoker: NewMockIpamInvoker(false, false, false), + multitenancyClient: &Multitenancy{ + netioshim: &mockNetIOShim{}, + cnsclient: &MockCNSClient{ + require: require, + getAllNetworkContainersConfiguration: getAllNetworkContainersConfigurationHandler{ + orchestratorContext: marshallPodInfo(cns.KubernetesPodInfo{ + PodName: "testpod", + PodNamespace: "testnamespace", + }), + returnResponse: ncResponses, + }, + }, + }, + }, + k8sPodName: "testpod", + k8sNamespace: "testnamespace", + ifName: "eth0", + }, + want: &cniTypesCurr.Result{ + Interfaces: []*cniTypesCurr.Interface{ + { + Name: "eth0", + }, + }, + IPs: []*cniTypesCurr.IPConfig{ + { + Address: getIPNet(net.IPv4(10, 1, 0, 5), net.CIDRMask(16, 32)), + Gateway: net.ParseIP("10.1.0.1"), + }, + }, + Routes: []*cniTypes.Route{ + { + Dst: *getIPNetWithString("10.1.0.0/16"), + GW: net.ParseIP("10.1.0.1"), + }, + { + Dst: net.IPNet{IP: net.ParseIP("10.1.0.0"), Mask: net.CIDRMask(16, 32)}, + GW: net.ParseIP("10.1.0.1"), + }, + }, + }, + want1: &cns.GetNetworkContainerResponse{ + PrimaryInterfaceIdentifier: "10.0.0.0/16", + LocalIPConfiguration: cns.IPConfiguration{ + IPSubnet: cns.IPSubnet{ + IPAddress: "10.0.0.5", + PrefixLength: 16, + }, + GatewayIPAddress: "", + }, + CnetAddressSpace: []cns.IPSubnet{ + { + IPAddress: "10.1.0.0", + PrefixLength: 16, + }, + }, + IPConfiguration: cns.IPConfiguration{ + IPSubnet: cns.IPSubnet{ + IPAddress: "10.1.0.5", + PrefixLength: 16, + }, + DNSServers: nil, + GatewayIPAddress: "10.1.0.1", + }, + Routes: []cns.Route{ + { + IPAddress: "10.1.0.0/16", + GatewayIPAddress: "10.1.0.1", + }, + }, + }, + want2: &cns.GetNetworkContainerResponse{ + PrimaryInterfaceIdentifier: "20.0.0.0/16", + LocalIPConfiguration: cns.IPConfiguration{ + IPSubnet: cns.IPSubnet{ + IPAddress: "20.0.0.5", + PrefixLength: 16, + }, + GatewayIPAddress: "", + }, + CnetAddressSpace: []cns.IPSubnet{ + { + IPAddress: "20.1.0.0", + PrefixLength: 16, + }, + }, + IPConfiguration: cns.IPConfiguration{ + IPSubnet: cns.IPSubnet{ + IPAddress: "20.1.0.5", + PrefixLength: 16, + }, + DNSServers: nil, + GatewayIPAddress: "20.1.0.1", + }, + Routes: []cns.Route{ + { + IPAddress: "20.1.0.0/16", + GatewayIPAddress: "20.1.0.1", + }, + }, + }, + want3: *getCIDRNotationForAddress("10.0.0.0/16"), + want4: &cniTypesCurr.Result{ + IPs: []*cniTypesCurr.IPConfig{ + { + Address: net.IPNet{ + IP: net.ParseIP("10.240.0.5"), + Mask: net.CIDRMask(24, 32), + }, + Gateway: net.ParseIP("10.240.0.1"), + }, + }, + Routes: nil, + DNS: cniTypes.DNS{}, + }, + }, + } + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + got, err := tt.args.plugin.multitenancyClient.GetAllNetworkContainers( + tt.args.ctx, + tt.args.nwCfg, + tt.args.k8sPodName, + tt.args.k8sNamespace, + tt.args.ifName) + if (err != nil) != tt.wantErr { + t.Errorf("GetContainerNetworkConfiguration() error = %v, wantErr %v", err, tt.wantErr) + return + } + if tt.wantErr { + require.Error(err) } - } + require.NoError(err) + require.Exactly(tt.want1, got[0].ncResponse) + require.Exactly(tt.want2, got[1].ncResponse) + require.Exactly(tt.want3, got[0].hostSubnetPrefix) + + // check multiple responses + tt.want5 = append(tt.want5, *tt.want1, *tt.want2) + require.Exactly(tt.want5, ncResponses) + }) } - - return false } - -var ( - errSnatIP = errors.New("Snat IP not populated") - errInfraVnet = errors.New("infravnet not populated") - errSubnetOverlap = errors.New("subnet overlap error") - errIfaceNotFound = errors.New("Interface not found for this ip") -) diff --git a/cni/network/multitenancy_test.go b/cni/network/multitenancy_test.go index 0d1d66c82d..011f99841a 100644 --- a/cni/network/multitenancy_test.go +++ b/cni/network/multitenancy_test.go @@ -24,8 +24,17 @@ type requestIPAddressHandler struct { err error } -type releaseIPAddressHandler struct { - ipconfigArgument cns.IPConfigRequest +type requestIPsHandler struct { + // arguments + ipconfigArgument cns.IPConfigsRequest + + // results + result *cns.IPConfigsResponse // this will return the IPConfigsResponse which contains a slice of IPs as opposed to one IP + err error +} + +type releaseIPsHandler struct { + ipconfigArgument cns.IPConfigsRequest err error } @@ -45,7 +54,8 @@ type getAllNetworkContainersConfigurationHandler struct { type MockCNSClient struct { require *require.Assertions request requestIPAddressHandler - release releaseIPAddressHandler + requestIPs requestIPsHandler + release releaseIPsHandler getNetworkContainerConfiguration getNetworkContainerConfigurationHandler getAllNetworkContainersConfiguration getAllNetworkContainersConfigurationHandler } @@ -55,6 +65,16 @@ func (c *MockCNSClient) RequestIPAddress(_ context.Context, ipconfig cns.IPConfi return c.request.result, c.request.err } +func (c *MockCNSClient) RequestIPs(_ context.Context, ipconfig cns.IPConfigsRequest) (*cns.IPConfigsResponse, error) { + c.require.Exactly(c.requestIPs.ipconfigArgument, ipconfig) + return c.requestIPs.result, c.requestIPs.err +} + +func (c *MockCNSClient) ReleaseIPs(_ context.Context, ipconfig cns.IPConfigsRequest) error { + c.require.Exactly(c.release.ipconfigArgument, ipconfig) + return c.release.err +} + func (c *MockCNSClient) ReleaseIPAddress(_ context.Context, ipconfig cns.IPConfigRequest) error { c.require.Exactly(c.release.ipconfigArgument, ipconfig) return c.release.err diff --git a/cni/network/network.go b/cni/network/network.go index e6aeeae3a9..89e8589799 100644 --- a/cni/network/network.go +++ b/cni/network/network.go @@ -372,6 +372,7 @@ func (plugin *NetPlugin) Add(args *cniSkel.CmdArgs) error { } addSnatInterface(nwCfg, ipamAddResult.ipv4Result) + // Convert result to the requested CNI version. res, vererr := ipamAddResult.ipv4Result.GetAsVersion(nwCfg.CNIVersion) if vererr != nil { @@ -384,7 +385,7 @@ func (plugin *NetPlugin) Add(args *cniSkel.CmdArgs) error { res.Print() } - log.Printf("[cni-net] ADD command completed for pod %v with IPs:%+v err:%v.", k8sPodName, ipamAddResult.ipv4Result.IPs, err) + log.Printf("[cni-net] ADD command completed for pod %v with ip IPs:%+v err:%v.", k8sPodName, ipamAddResult.ipv4Result.IPs, err) }() // Parse Pod arguments. @@ -538,7 +539,7 @@ func (plugin *NetPlugin) Add(args *cniSkel.CmdArgs) error { logAndSendEvent(plugin, fmt.Sprintf("[cni-net] Created network %v with subnet %v.", networkID, ipamAddResult.hostSubnetPrefix.String())) } - natInfo := getNATInfo(nwCfg, options[network.SNATIPKey], enableSnatForDNS) + natInfo := getNATInfo(nwCfg.ExecutionMode, options[network.SNATIPKey], nwCfg.MultiTenancy, enableSnatForDNS) createEndpointInternalOpt := createEndpointInternalOpt{ nwCfg: nwCfg, @@ -577,12 +578,20 @@ func (plugin *NetPlugin) cleanupAllocationOnError( 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 { + addresses := make([]*net.IPNet, len(result.IPs)) + for i, ip := range result.IPs { + addresses[i] = &ip.Address + } + if er := plugin.ipamInvoker.Delete(addresses, nwCfg, args, options); er != nil { log.Errorf("Failed to cleanup ip allocation on failure: %v", er) } } if resultV6 != nil && len(resultV6.IPs) > 0 { - if er := plugin.ipamInvoker.Delete(&resultV6.IPs[0].Address, nwCfg, args, options); er != nil { + addressesV6 := make([]*net.IPNet, len(resultV6.IPs)) + for i, ip := range resultV6.IPs { + addressesV6[i] = &ip.Address + } + if er := plugin.ipamInvoker.Delete(addressesV6, nwCfg, args, options); er != nil { log.Errorf("Failed to cleanup ipv6 allocation on failure: %v", er) } } @@ -626,19 +635,21 @@ func (plugin *NetPlugin) createNetworkInternal( return nwInfo, fmt.Errorf("Failed to ParseCIDR for pod subnet prefix: %w", err) } + // parse the ipv6 address and only add it to nwInfo if it's dual stack mode + var podSubnetV6Prefix *net.IPNet + if ipamAddResult.ipv6Result != nil && len(ipamAddResult.ipv6Result.IPs) > 0 { + _, podSubnetV6Prefix, err = net.ParseCIDR(ipamAddResult.ipv6Result.IPs[0].Address.String()) + if err != nil { + return nwInfo, fmt.Errorf("Failed to ParseCIDR for pod subnet IPv6 prefix: %w", err) + } + } + // Create the network. nwInfo = network.NetworkInfo{ - Id: networkID, - Mode: ipamAddConfig.nwCfg.Mode, - MasterIfName: masterIfName, - AdapterName: ipamAddConfig.nwCfg.AdapterName, - Subnets: []network.SubnetInfo{ - { - Family: platform.AfINET, - Prefix: *podSubnetPrefix, - Gateway: ipamAddResult.ipv4Result.IPs[0].Gateway, - }, - }, + Id: networkID, + Mode: ipamAddConfig.nwCfg.Mode, + MasterIfName: masterIfName, + AdapterName: ipamAddConfig.nwCfg.AdapterName, BridgeName: ipamAddConfig.nwCfg.Bridge, EnableSnatOnHost: ipamAddConfig.nwCfg.EnableSnatOnHost, DNS: nwDNSInfo, @@ -651,6 +662,22 @@ func (plugin *NetPlugin) createNetworkInternal( ServiceCidrs: ipamAddConfig.nwCfg.ServiceCidrs, } + ipv4Subnet := network.SubnetInfo{ + Family: platform.AfINET, + Prefix: *podSubnetPrefix, + Gateway: ipamAddResult.ipv4Result.IPs[0].Gateway, + } + nwInfo.Subnets = append(nwInfo.Subnets, ipv4Subnet) + + if ipamAddResult.ipv6Result != nil && len(ipamAddResult.ipv6Result.IPs) > 0 { + ipv6Subnet := network.SubnetInfo{ + Family: platform.AfINET6, + Prefix: *podSubnetV6Prefix, + Gateway: ipamAddResult.ipv6Result.IPs[0].Gateway, + } + nwInfo.Subnets = append(nwInfo.Subnets, ipv6Subnet) + } + setNetworkOptions(ipamAddResult.ncResponse, &nwInfo) addNatIPV6SubnetInfo(ipamAddConfig.nwCfg, ipamAddResult.ipv6Result, &nwInfo) @@ -739,7 +766,11 @@ func (plugin *NetPlugin) createEndpointInternal(opt *createEndpointInternalOpt) epInfo.IPAddresses = append(epInfo.IPAddresses, ipconfig.Address) } + log.Printf("epInfo.IPV6Mode is %s", epInfo.IPV6Mode) + if opt.resultV6 != nil { + // inject routes to linux pod + epInfo.IPV6Mode = "dualstack" for _, ipconfig := range opt.resultV6.IPs { epInfo.IPAddresses = append(epInfo.IPAddresses, ipconfig.Address) } @@ -1013,12 +1044,14 @@ func (plugin *NetPlugin) Delete(args *cniSkel.CmdArgs) error { if !nwCfg.MultiTenancy { // Call into IPAM plugin to release the endpoint's addresses. + addresses := make([]*net.IPNet, len(epInfo.IPAddresses)) for i := range epInfo.IPAddresses { - logAndSendEvent(plugin, fmt.Sprintf("Release ip:%s", epInfo.IPAddresses[i].IP.String())) - err = plugin.ipamInvoker.Delete(&epInfo.IPAddresses[i], nwCfg, args, nwInfo.Options) - if err != nil { - return plugin.RetriableError(fmt.Errorf("failed to release address: %w", err)) - } + addresses[i] = &epInfo.IPAddresses[i] + } + logAndSendEvent(plugin, fmt.Sprintf("Releasing ips:%+v", epInfo.IPAddresses)) + err = plugin.ipamInvoker.Delete(addresses, nwCfg, args, nwInfo.Options) + if err != nil { + return plugin.RetriableError(fmt.Errorf("failed to release address: %w", err)) } } else if epInfo.EnableInfraVnet { nwCfg.IPAM.Subnet = nwInfo.Subnets[0].Prefix.String() diff --git a/cni/network/network_test.go b/cni/network/network_test.go index 7a9edc5bcb..5844ec37c9 100644 --- a/cni/network/network_test.go +++ b/cni/network/network_test.go @@ -4,7 +4,6 @@ import ( "fmt" "net" "os" - "runtime" "testing" "github.com/Azure/azure-container-networking/cni" @@ -12,8 +11,6 @@ import ( "github.com/Azure/azure-container-networking/cni/util" "github.com/Azure/azure-container-networking/common" acnnetwork "github.com/Azure/azure-container-networking/network" - "github.com/Azure/azure-container-networking/network/networkutils" - "github.com/Azure/azure-container-networking/network/policy" "github.com/Azure/azure-container-networking/nns" "github.com/Azure/azure-container-networking/telemetry" cniSkel "github.com/containernetworking/cni/pkg/skel" @@ -76,7 +73,7 @@ func GetTestResources() *NetPlugin { plugin.report = &telemetry.CNIReport{} mockNetworkManager := acnnetwork.NewMockNetworkmanager() plugin.nm = mockNetworkManager - plugin.ipamInvoker = NewMockIpamInvoker(false, false, false) + plugin.ipamInvoker = NewMockIpamInvoker(true, false, false) // enable ipv6 flag for dualstack test cases return plugin } @@ -1010,7 +1007,6 @@ func TestGetAllEndpointState(t *testing.T) { ep1 := getTestEndpoint("podname1", "podnamespace1", "10.0.0.1/24", "podinterfaceid1", "testcontainerid1") 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) require.NoError(t, err) @@ -1018,9 +1014,6 @@ func TestGetAllEndpointState(t *testing.T) { err = plugin.nm.CreateEndpoint(nil, networkid, ep2) require.NoError(t, err) - err = plugin.nm.CreateEndpoint(nil, networkid, ep3) - require.NoError(t, err) - state, err := plugin.GetAllEndpointState(networkid) require.NoError(t, err) @@ -1040,13 +1033,6 @@ func TestGetAllEndpointState(t *testing.T) { ContainerID: ep2.ContainerID, IPAddresses: ep2.IPAddresses, }, - ep3.Id: { - PodEndpointId: ep3.Id, - PodName: ep3.PODName, - PodNamespace: ep3.PODNameSpace, - ContainerID: ep3.ContainerID, - IPAddresses: ep3.IPAddresses, - }, }, } @@ -1082,23 +1068,3 @@ func TestGetNetworkName(t *testing.T) { }) } } - -func TestGetOverlayNatInfo(t *testing.T) { - nwCfg := &cni.NetworkConfig{ExecutionMode: string(util.V4Swift), IPAM: cni.IPAM{Mode: string(util.V4Overlay)}} - natInfo := getNATInfo(nwCfg, nil, false) - require.Empty(t, natInfo, "overlay natInfo should be empty") -} - -func TestGetPodSubnetNatInfo(t *testing.T) { - ncPrimaryIP := "10.241.0.4" - nwCfg := &cni.NetworkConfig{ExecutionMode: string(util.V4Swift)} - natInfo := getNATInfo(nwCfg, ncPrimaryIP, false) - if runtime.GOOS == "windows" { - require.Equalf(t, natInfo, []policy.NATInfo{ - {VirtualIP: ncPrimaryIP, Destinations: []string{networkutils.AzureDNS}}, - {Destinations: []string{networkutils.AzureIMDS}}, - }, "invalid windows podsubnet natInfo") - } else { - require.Empty(t, natInfo, "linux podsubnet natInfo should be empty") - } -} diff --git a/cni/network/network_windows.go b/cni/network/network_windows.go index b080046565..eeabacc005 100644 --- a/cni/network/network_windows.go +++ b/cni/network/network_windows.go @@ -385,7 +385,10 @@ func getNATInfo(nwCfg *cni.NetworkConfig, ncPrimaryIPIface interface{}, enableSn ncPrimaryIP = ncPrimaryIPIface.(string) } - natInfo = append(natInfo, []policy.NATInfo{{VirtualIP: ncPrimaryIP, Destinations: []string{networkutils.AzureDNS}}, {Destinations: []string{networkutils.AzureIMDS}}}...) + // only append natInfo if ncPrimaryIP is ipv4 + if net.ParseIP(ncPrimaryIP).To4() != nil { + natInfo = append(natInfo, []policy.NATInfo{{VirtualIP: ncPrimaryIP, Destinations: []string{networkutils.AzureDNS}}, {Destinations: []string{networkutils.AzureIMDS}}}...) + } } else if nwCfg.MultiTenancy && enableSnatForDNS { natInfo = append(natInfo, policy.NATInfo{Destinations: []string{networkutils.AzureDNS}}) } diff --git a/cni/util/const.go b/cni/util/const.go index 76a28e8ac7..d3f88b2499 100644 --- a/cni/util/const.go +++ b/cni/util/const.go @@ -13,5 +13,6 @@ type IpamMode string // IPAM modes const ( - V4Overlay IpamMode = "v4overlay" + V4Overlay IpamMode = "v4overlay" + DualStackOverlay IpamMode = "dualStackOverlay" ) diff --git a/network/manager.go b/network/manager.go index 0fbeb2d0a9..a9b167717d 100644 --- a/network/manager.go +++ b/network/manager.go @@ -33,6 +33,12 @@ var Ipv4DefaultRouteDstPrefix = net.IPNet{ Mask: net.IPv4Mask(0, 0, 0, 0), } +var Ipv6DefaultRouteDstPrefix = net.IPNet{ + IP: net.IPv6zero, + // This mask corresponds to a /0 subnet for IPv6 + Mask: net.CIDRMask(0, 128), +} + type NetworkClient interface { CreateBridge() error DeleteBridge() error diff --git a/network/network.go b/network/network.go index 9a9dfac69a..adfbe28876 100644 --- a/network/network.go +++ b/network/network.go @@ -25,6 +25,8 @@ const ( const ( // ipv6 modes IPV6Nat = "ipv6nat" + // dual stack mode + DualStack = "dualstack" ) // externalInterface is a host network interface that bridges containers to external networks. diff --git a/network/network_windows.go b/network/network_windows.go index b440e01842..6f80fc5269 100644 --- a/network/network_windows.go +++ b/network/network_windows.go @@ -7,13 +7,14 @@ import ( "encoding/json" "errors" "fmt" + "net" "strconv" "strings" "time" - "github.com/Azure/azure-container-networking/network/hnswrapper" - + "github.com/Azure/azure-container-networking/cni/util" "github.com/Azure/azure-container-networking/log" + "github.com/Azure/azure-container-networking/network/hnswrapper" "github.com/Azure/azure-container-networking/network/policy" "github.com/Microsoft/hcsshim" "github.com/Microsoft/hcsshim/hcn" @@ -31,10 +32,14 @@ const ( defaultRouteCIDR = "0.0.0.0/0" // prefix for interface name created by azure network ifNamePrefix = "vEthernet" + // ipv4 default hop + ipv4DefaultHop = "0.0.0.0" // ipv6 default hop ipv6DefaultHop = "::" // ipv6 route cmd routeCmd = "netsh interface ipv6 %s route \"%s\" \"%s\" \"%s\" store=persistent" + // add ipv4 and ipv6 route rules to windows node + netRouteCmd = "netsh interface %s add route \"%s\" \"%s\" \"%s\" metric=%s" ) // Windows implementation of route. @@ -185,6 +190,55 @@ func (nm *networkManager) newNetworkImplHnsV1(nwInfo *NetworkInfo, extIf *extern return nw, nil } +// add ipv4 and ipv6 routes (if dual stack mode) to windows Node +func (nm *networkManager) addNewNetRules(nwInfo *NetworkInfo) error { + var ( + err error + out string + ) + + // get interface name of VM adapter + ifName := nwInfo.MasterIfName + if !strings.Contains(nwInfo.MasterIfName, ifNamePrefix) { + ifName = fmt.Sprintf("%s (%s)", ifNamePrefix, nwInfo.MasterIfName) + } + + // add ipv4 and ipv6 new net rules to windows node + for _, subnet := range nwInfo.Subnets { + prefix := subnet.Prefix.String() + gateway := subnet.Gateway.String() + + ip, _, _ := net.ParseCIDR(prefix) + if ip.To4() != nil { + // netsh interface ipv4 add route $subnetV4 $hostInterfaceAlias "0.0.0.0" metric=270 + netshV4DefaultRoute := fmt.Sprintf(netRouteCmd, "ipv4", prefix, ifName, ipv4DefaultHop, "270") + if out, err = nm.plClient.ExecuteCommand(netshV4DefaultRoute); err != nil { + return fmt.Errorf("[net] Adding ipv4 default route failed: %v:%v", out, err) + } + + // netsh interface ipv4 add route $subnetV4 $hostInterfaceAlias $gatewayV4 metric=300 + netshV4GatewayRoute := fmt.Sprintf(netRouteCmd, "ipv4", prefix, ifName, gateway, "300") + if out, err = nm.plClient.ExecuteCommand(netshV4GatewayRoute); err != nil { + return fmt.Errorf("[net] Adding ipv4 gateway route failed: %v:%v", out, err) + } + } else { + // netsh interface ipv6 add route $subnetV6 $hostInterfaceAlias "::" metric=270 + netshV6DefaultRoute := fmt.Sprintf(netRouteCmd, "ipv6", prefix, ifName, ipv6DefaultHop, "270") + if out, err = nm.plClient.ExecuteCommand(netshV6DefaultRoute); err != nil { + return fmt.Errorf("[net] Adding ipv6 default route failed: %v:%v", out, err) + } + + // netsh interface ipv6 add route $subnetV6 $hostInterfaceAlias $gatewayV6 metric=300 + netshV6GatewayRoute := fmt.Sprintf(netRouteCmd, "ipv6", prefix, ifName, gateway, "300") + if out, err = nm.plClient.ExecuteCommand(netshV6GatewayRoute); err != nil { + return fmt.Errorf("[net] Adding ipv6 gateway route failed: %v:%v", out, err) + } + } + } + + return nil +} + func (nm *networkManager) appIPV6RouteEntry(nwInfo *NetworkInfo) error { var ( err error @@ -352,6 +406,12 @@ func (nm *networkManager) newNetworkImplHnsV2(nwInfo *NetworkInfo, extIf *extern vlanid = (int)(vlanID) } + if util.DualStackOverlay == DualStack { + if err = nm.addNewNetRules(nwInfo); err != nil { + return nil, err + } + } + // Create the network object. nw := &network{ Id: nwInfo.Id, From d056ddf0418f5ff49b1079639ba76a6a39c62594 Mon Sep 17 00:00:00 2001 From: paulyu Date: Tue, 21 Mar 2023 13:53:21 -0400 Subject: [PATCH 02/44] rebase current branch --- cni/azure-linux-dualstack-overlay.conflist | 2 +- cni/azure-windows-dualstack-overlay.conflist | 2 +- cni/network/invoker_azure.go | 8 +++-- cni/network/invoker_cns.go | 8 ++--- cni/network/invoker_cns_test.go | 11 +++++++ cni/network/network.go | 6 ++-- cni/network/network_test.go | 34 ++++++++++++++++++++ 7 files changed, 58 insertions(+), 13 deletions(-) diff --git a/cni/azure-linux-dualstack-overlay.conflist b/cni/azure-linux-dualstack-overlay.conflist index 5d3deac813..2c4ea7a5b7 100644 --- a/cni/azure-linux-dualstack-overlay.conflist +++ b/cni/azure-linux-dualstack-overlay.conflist @@ -20,4 +20,4 @@ "snat":true } ] -} \ No newline at end of file +} diff --git a/cni/azure-windows-dualstack-overlay.conflist b/cni/azure-windows-dualstack-overlay.conflist index 0c8c9acdce..e3936ea335 100644 --- a/cni/azure-windows-dualstack-overlay.conflist +++ b/cni/azure-windows-dualstack-overlay.conflist @@ -47,4 +47,4 @@ ] } ] -} \ No newline at end of file +} diff --git a/cni/network/invoker_azure.go b/cni/network/invoker_azure.go index 34609c1d58..64edbd826d 100644 --- a/cni/network/invoker_azure.go +++ b/cni/network/invoker_azure.go @@ -18,6 +18,11 @@ import ( cniTypesCurr "github.com/containernetworking/cni/pkg/types/100" ) +const ( + bytesSize4 = 4 + bytesSize16 = 16 +) + type AzureIPAMInvoker struct { plugin delegatePlugin nwInfo *network.NetworkInfo @@ -29,9 +34,6 @@ type delegatePlugin interface { Errorf(format string, args ...interface{}) *cniTypes.Error } -const bytesSize4 = 4 -const bytesSize16 = 16 - // Create an IPAM instance every time a CNI action is called. func NewAzureIpamInvoker(plugin *NetPlugin, nwInfo *network.NetworkInfo) *AzureIPAMInvoker { return &AzureIPAMInvoker{ diff --git a/cni/network/invoker_cns.go b/cni/network/invoker_cns.go index 39b5c561dd..fc8a2736b7 100644 --- a/cni/network/invoker_cns.go +++ b/cni/network/invoker_cns.go @@ -23,8 +23,8 @@ import ( var ( errEmptyCNIArgs = errors.New("empty CNI cmd args not allowed") errInvalidArgs = errors.New("invalid arg(s)") - overlayGatewayv4IP = "169.254.1.1" - overlayGatewayv6IP = "fe80::1234:5678:9abc" + overlayGatewayV4IP = "169.254.1.1" + overlayGatewayV6IP = "fe80::1234:5678:9abc" ) type CNSIPAMInvoker struct { @@ -111,9 +111,9 @@ func (invoker *CNSIPAMInvoker) Add(addConfig IPAMAddConfig) (IPAMAddResult, erro } if net.ParseIP(info.podIPAddress).To4() != nil { - ncgw = net.ParseIP(overlayGatewayv4IP) + ncgw = net.ParseIP(overlayGatewayV4IP) } else { - ncgw = net.ParseIP(overlayGatewayv6IP) + ncgw = net.ParseIP(overlayGatewayV6IP) } } diff --git a/cni/network/invoker_cns_test.go b/cni/network/invoker_cns_test.go index 9cb570406d..509d3ba54f 100644 --- a/cni/network/invoker_cns_test.go +++ b/cni/network/invoker_cns_test.go @@ -2,10 +2,13 @@ package network import ( "errors" + "fmt" "net" + "runtime" "testing" "github.com/Azure/azure-container-networking/cni" + "github.com/Azure/azure-container-networking/cni/util" "github.com/Azure/azure-container-networking/cns" "github.com/Azure/azure-container-networking/iptables" "github.com/Azure/azure-container-networking/network" @@ -25,6 +28,14 @@ func getTestIPConfigsRequest() cns.IPConfigsRequest { } } +func getTestOverlayGateway() net.IP { + if runtime.GOOS == "windows" { + return net.ParseIP("10.240.0.1") + } + + return net.ParseIP("169.254.1.1") +} + func TestCNSIPAMInvoker_Add(t *testing.T) { require := require.New(t) //nolint further usage of require without passing t type fields struct { diff --git a/cni/network/network.go b/cni/network/network.go index 89e8589799..fd2c284f23 100644 --- a/cni/network/network.go +++ b/cni/network/network.go @@ -385,7 +385,7 @@ func (plugin *NetPlugin) Add(args *cniSkel.CmdArgs) error { res.Print() } - log.Printf("[cni-net] ADD command completed for pod %v with ip IPs:%+v err:%v.", k8sPodName, ipamAddResult.ipv4Result.IPs, err) + log.Printf("[cni-net] ADD command completed for pod %v with IPs:%+v err:%v.", k8sPodName, ipamAddResult.ipv4Result.IPs, err) }() // Parse Pod arguments. @@ -766,11 +766,9 @@ func (plugin *NetPlugin) createEndpointInternal(opt *createEndpointInternalOpt) epInfo.IPAddresses = append(epInfo.IPAddresses, ipconfig.Address) } - log.Printf("epInfo.IPV6Mode is %s", epInfo.IPV6Mode) - if opt.resultV6 != nil { // inject routes to linux pod - epInfo.IPV6Mode = "dualstack" + epInfo.IPV6Mode = DualStack for _, ipconfig := range opt.resultV6.IPs { epInfo.IPAddresses = append(epInfo.IPAddresses, ipconfig.Address) } diff --git a/cni/network/network_test.go b/cni/network/network_test.go index 5844ec37c9..a8cfa59b29 100644 --- a/cni/network/network_test.go +++ b/cni/network/network_test.go @@ -4,6 +4,7 @@ import ( "fmt" "net" "os" + "runtime" "testing" "github.com/Azure/azure-container-networking/cni" @@ -11,6 +12,8 @@ import ( "github.com/Azure/azure-container-networking/cni/util" "github.com/Azure/azure-container-networking/common" acnnetwork "github.com/Azure/azure-container-networking/network" + "github.com/Azure/azure-container-networking/network/networkutils" + "github.com/Azure/azure-container-networking/network/policy" "github.com/Azure/azure-container-networking/nns" "github.com/Azure/azure-container-networking/telemetry" cniSkel "github.com/containernetworking/cni/pkg/skel" @@ -1007,6 +1010,7 @@ func TestGetAllEndpointState(t *testing.T) { ep1 := getTestEndpoint("podname1", "podnamespace1", "10.0.0.1/24", "podinterfaceid1", "testcontainerid1") 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) require.NoError(t, err) @@ -1014,6 +1018,9 @@ func TestGetAllEndpointState(t *testing.T) { err = plugin.nm.CreateEndpoint(nil, networkid, ep2) require.NoError(t, err) + err = plugin.nm.CreateEndpoint(nil, networkid, ep3) + require.NoError(t, err) + state, err := plugin.GetAllEndpointState(networkid) require.NoError(t, err) @@ -1033,6 +1040,13 @@ func TestGetAllEndpointState(t *testing.T) { ContainerID: ep2.ContainerID, IPAddresses: ep2.IPAddresses, }, + ep3.Id: { + PodEndpointId: ep3.Id, + PodName: ep3.PODName, + PodNamespace: ep3.PODNameSpace, + ContainerID: ep3.ContainerID, + IPAddresses: ep3.IPAddresses, + }, }, } @@ -1068,3 +1082,23 @@ func TestGetNetworkName(t *testing.T) { }) } } + +func TestGetOverlayNatInfo(t *testing.T) { + nwCfg := &cni.NetworkConfig{ExecutionMode: string(util.V4Swift), IPAM: cni.IPAM{Mode: string(util.V4Overlay)}} + natInfo := getNATInfo(nwCfg, nil, false) + require.Empty(t, natInfo, "overlay natInfo should be empty") +} + +func TestGetPodSubnetNatInfo(t *testing.T) { + ncPrimaryIP := "10.241.0.4" + nwCfg := &cni.NetworkConfig{ExecutionMode: string(util.V4Swift)} + natInfo := getNATInfo(nwCfg, ncPrimaryIP, false) + if runtime.GOOS == "windows" { + require.Equalf(t, natInfo, []policy.NATInfo{ + {VirtualIP: ncPrimaryIP, Destinations: []string{networkutils.AzureDNS}}, + {Destinations: []string{networkutils.AzureIMDS}}, + }, "invalid windows podsubnet natInfo") + } else { + require.Empty(t, natInfo, "linux podsubnet natInfo should be empty") + } +} From adc5caba039d094f9bfdf0be51d86e3887f6942e Mon Sep 17 00:00:00 2001 From: paulyu Date: Tue, 21 Mar 2023 13:58:00 -0400 Subject: [PATCH 03/44] fix conflicts --- cni/network/invoker_cns_test.go | 75 ++++++++++++++++++++++++++++++++- 1 file changed, 74 insertions(+), 1 deletion(-) diff --git a/cni/network/invoker_cns_test.go b/cni/network/invoker_cns_test.go index 509d3ba54f..e2f38a4af4 100644 --- a/cni/network/invoker_cns_test.go +++ b/cni/network/invoker_cns_test.go @@ -172,6 +172,76 @@ func TestCNSIPAMInvoker_Add(t *testing.T) { }, wantErr: true, }, + { + name: "Test happy CNI Overlay add", + fields: fields{ + podName: testPodInfo.PodName, + podNamespace: testPodInfo.PodNamespace, + ipamMode: util.V4Overlay, + cnsClient: &MockCNSClient{ + require: require, + request: requestIPAddressHandler{ + ipconfigArgument: cns.IPConfigRequest{ + PodInterfaceID: "testcont-testifname3", + InfraContainerID: "testcontainerid3", + OrchestratorContext: marshallPodInfo(testPodInfo), + }, + result: &cns.IPConfigResponse{ + PodIpInfo: cns.PodIpInfo{ + PodIPConfig: cns.IPSubnet{ + IPAddress: "10.240.1.242", + PrefixLength: 16, + }, + NetworkContainerPrimaryIPConfig: cns.IPConfiguration{ + IPSubnet: cns.IPSubnet{ + IPAddress: "10.240.1.0", + PrefixLength: 16, + }, + DNSServers: nil, + GatewayIPAddress: "", + }, + HostPrimaryIPInfo: cns.HostIPInfo{ + Gateway: "10.224.0.1", + PrimaryIP: "10.224.0.5", + Subnet: "10.224.0.0/16", + }, + }, + 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.224.0.0/16"), + options: map[string]interface{}{}, + }, + want: &cniTypesCurr.Result{ + IPs: []*cniTypesCurr.IPConfig{ + { + Address: *getCIDRNotationForAddress("10.240.1.242/16"), + Gateway: getTestOverlayGateway(), + }, + }, + Routes: []*cniTypes.Route{ + { + Dst: network.Ipv4DefaultRouteDstPrefix, + GW: getTestOverlayGateway(), + }, + }, + }, + want1: nil, + wantErr: false, + }, } for _, tt := range tests { tt := tt @@ -181,13 +251,16 @@ func TestCNSIPAMInvoker_Add(t *testing.T) { podNamespace: tt.fields.podNamespace, cnsClient: tt.fields.cnsClient, } + if tt.fields.ipamMode != "" { + invoker.ipamMode = tt.fields.ipamMode + } ipamAddResult, err := invoker.Add(IPAMAddConfig{nwCfg: tt.args.nwCfg, args: tt.args.args, options: tt.args.options}) if tt.wantErr { require.Error(err) } else { require.NoError(err) } - + fmt.Printf("want:%+v\nrest:%+v\n", tt.want, ipamAddResult.ipv4Result) require.Equalf(tt.want, ipamAddResult.ipv4Result, "incorrect ipv4 response") require.Equalf(tt.want1, ipamAddResult.ipv6Result, "incorrect ipv6 response") }) From 4111b401f15b2406bf09e02f7948fe8cf5086233 Mon Sep 17 00:00:00 2001 From: paulyu Date: Tue, 21 Mar 2023 14:00:49 -0400 Subject: [PATCH 04/44] fix conflicts --- cni/network/multitenancy.go | 699 ++++++++++++++---------------------- cni/network/network.go | 2 +- 2 files changed, 266 insertions(+), 435 deletions(-) diff --git a/cni/network/multitenancy.go b/cni/network/multitenancy.go index 011f99841a..ff60146ad9 100644 --- a/cni/network/multitenancy.go +++ b/cni/network/multitenancy.go @@ -3,493 +3,324 @@ package network import ( "context" "encoding/json" + "errors" + "fmt" + "io" "net" - "testing" + "net/http" + "os" + "strings" + "time" "github.com/Azure/azure-container-networking/cni" "github.com/Azure/azure-container-networking/cns" + "github.com/Azure/azure-container-networking/cns/client" + "github.com/Azure/azure-container-networking/common" + "github.com/Azure/azure-container-networking/log" "github.com/Azure/azure-container-networking/network" cniTypes "github.com/containernetworking/cni/pkg/types" cniTypesCurr "github.com/containernetworking/cni/pkg/types/100" - "github.com/stretchr/testify/require" ) -// Handler structs -type requestIPAddressHandler struct { - // arguments - ipconfigArgument cns.IPConfigRequest +const ( + filePerm = 0o664 + httpTimeout = 5 +) - // results - result *cns.IPConfigResponse - err error +// MultitenancyClient interface +type MultitenancyClient interface { + SetupRoutingForMultitenancy( + nwCfg *cni.NetworkConfig, + cnsNetworkConfig *cns.GetNetworkContainerResponse, + azIpamResult *cniTypesCurr.Result, + epInfo *network.EndpointInfo, + result *cniTypesCurr.Result) + DetermineSnatFeatureOnHost( + snatFile string, + nmAgentSupportedApisURL string) (bool, bool, error) + GetAllNetworkContainers( + ctx context.Context, + nwCfg *cni.NetworkConfig, + podName string, + podNamespace string, + ifName string) ([]IPAMAddResult, error) + Init(cnsclient cnsclient, netioshim netioshim) } -type requestIPsHandler struct { - // arguments - ipconfigArgument cns.IPConfigsRequest +type Multitenancy struct { + // cnsclient is used to communicate with CNS + cnsclient cnsclient - // results - result *cns.IPConfigsResponse // this will return the IPConfigsResponse which contains a slice of IPs as opposed to one IP - err error + // netioshim is used to interact with networking syscalls + netioshim netioshim } -type releaseIPsHandler struct { - ipconfigArgument cns.IPConfigsRequest - err error +type netioshim interface { + GetInterfaceSubnetWithSpecificIP(ipAddr string) *net.IPNet } -type getNetworkContainerConfigurationHandler struct { - orchestratorContext []byte - returnResponse *cns.GetNetworkContainerResponse - err error -} +type AzureNetIOShim struct{} -// this is to get all the NCs for testing with given orchestratorContext -type getAllNetworkContainersConfigurationHandler struct { - orchestratorContext []byte - returnResponse []cns.GetNetworkContainerResponse - err error +func (a AzureNetIOShim) GetInterfaceSubnetWithSpecificIP(ipAddr string) *net.IPNet { + return common.GetInterfaceSubnetWithSpecificIP(ipAddr) } -type MockCNSClient struct { - require *require.Assertions - request requestIPAddressHandler - requestIPs requestIPsHandler - release releaseIPsHandler - getNetworkContainerConfiguration getNetworkContainerConfigurationHandler - getAllNetworkContainersConfiguration getAllNetworkContainersConfigurationHandler -} +var errNmaResponse = errors.New("nmagent request status code") -func (c *MockCNSClient) RequestIPAddress(_ context.Context, ipconfig cns.IPConfigRequest) (*cns.IPConfigResponse, error) { - c.require.Exactly(c.request.ipconfigArgument, ipconfig) - return c.request.result, c.request.err +func (m *Multitenancy) Init(cnsclient cnsclient, netioshim netioshim) { + m.cnsclient = cnsclient + m.netioshim = netioshim } -func (c *MockCNSClient) RequestIPs(_ context.Context, ipconfig cns.IPConfigsRequest) (*cns.IPConfigsResponse, error) { - c.require.Exactly(c.requestIPs.ipconfigArgument, ipconfig) - return c.requestIPs.result, c.requestIPs.err -} +// DetermineSnatFeatureOnHost - Temporary function to determine whether we need to disable SNAT due to NMAgent support +func (m *Multitenancy) DetermineSnatFeatureOnHost(snatFile, nmAgentSupportedApisURL string) (snatForDNS, snatOnHost bool, err error) { + var ( + snatConfig snatConfiguration + retrieveSnatConfigErr error + jsonFile *os.File + httpClient = &http.Client{Timeout: time.Second * httpTimeout} + snatConfigFile = snatConfigFileName + jsonFileExtension + ) + + // Check if we've already retrieved NMAgent version and determined whether to disable snat on host + if jsonFile, retrieveSnatConfigErr = os.Open(snatFile); retrieveSnatConfigErr == nil { + bytes, _ := io.ReadAll(jsonFile) + jsonFile.Close() + if retrieveSnatConfigErr = json.Unmarshal(bytes, &snatConfig); retrieveSnatConfigErr != nil { + log.Errorf("[cni-net] failed to unmarshal to snatConfig with error %v", + retrieveSnatConfigErr) + } + } -func (c *MockCNSClient) ReleaseIPs(_ context.Context, ipconfig cns.IPConfigsRequest) error { - c.require.Exactly(c.release.ipconfigArgument, ipconfig) - return c.release.err -} + // If we weren't able to retrieve snatConfiguration, query NMAgent + if retrieveSnatConfigErr != nil { + var resp *http.Response + req, err := http.NewRequestWithContext(context.TODO(), http.MethodGet, nmAgentSupportedApisURL, nil) + if err != nil { + log.Errorf("failed creating http request:%+v", err) + return false, false, fmt.Errorf("%w", err) + } + log.Printf("Query nma for dns snat support: %s", nmAgentSupportedApisURL) + resp, retrieveSnatConfigErr = httpClient.Do(req) + if retrieveSnatConfigErr == nil { + defer resp.Body.Close() + + if resp.StatusCode == http.StatusOK { + var bodyBytes []byte + // if the list of APIs (strings) contains the nmAgentSnatSupportAPI we will disable snat on host + if bodyBytes, retrieveSnatConfigErr = io.ReadAll(resp.Body); retrieveSnatConfigErr == nil { + bodyStr := string(bodyBytes) + if !strings.Contains(bodyStr, nmAgentSnatAndDnsSupportAPI) { + snatConfig.EnableSnatForDns = true + snatConfig.EnableSnatOnHost = !strings.Contains(bodyStr, nmAgentSnatSupportAPI) + } + + jsonStr, _ := json.Marshal(snatConfig) + fp, err := os.OpenFile(snatConfigFile, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, os.FileMode(filePerm)) + if err == nil { + _, err = fp.Write(jsonStr) + if err != nil { + log.Errorf("DetermineSnatFeatureOnHost: Write to json failed:%+v", err) + } + fp.Close() + } else { + log.Errorf("[cni-net] failed to save snat settings to %s with error: %+v", snatConfigFile, err) + } + } + } else { + retrieveSnatConfigErr = fmt.Errorf("%w:%d", errNmaResponse, resp.StatusCode) + } + } + } -func (c *MockCNSClient) ReleaseIPAddress(_ context.Context, ipconfig cns.IPConfigRequest) error { - c.require.Exactly(c.release.ipconfigArgument, ipconfig) - return c.release.err -} + // Log and return the error when we fail acquire snat configuration for host and dns + if retrieveSnatConfigErr != nil { + log.Errorf("[cni-net] failed to acquire SNAT configuration with error %v", + retrieveSnatConfigErr) + return snatConfig.EnableSnatForDns, snatConfig.EnableSnatOnHost, retrieveSnatConfigErr + } -func (c *MockCNSClient) GetNetworkContainer(ctx context.Context, orchestratorContext []byte) (*cns.GetNetworkContainerResponse, error) { - c.require.Exactly(c.getNetworkContainerConfiguration.orchestratorContext, orchestratorContext) - return c.getNetworkContainerConfiguration.returnResponse, c.getNetworkContainerConfiguration.err -} + log.Printf("[cni-net] saved snat settings %+v to %s", snatConfig, snatConfigFile) + if snatConfig.EnableSnatOnHost { + log.Printf("[cni-net] enabling SNAT on container host for outbound connectivity") + } + if snatConfig.EnableSnatForDns { + log.Printf("[cni-net] enabling SNAT on container host for DNS traffic") + } + if !snatConfig.EnableSnatForDns && !snatConfig.EnableSnatOnHost { + log.Printf("[cni-net] disabling SNAT on container host") + } -func (c *MockCNSClient) GetAllNetworkContainers(ctx context.Context, orchestratorContext []byte) ([]cns.GetNetworkContainerResponse, error) { - c.require.Exactly(c.getAllNetworkContainersConfiguration.orchestratorContext, orchestratorContext) - return c.getAllNetworkContainersConfiguration.returnResponse, c.getAllNetworkContainersConfiguration.err + return snatConfig.EnableSnatForDns, snatConfig.EnableSnatOnHost, nil } -func defaultIPNet() *net.IPNet { - _, defaultIPNet, _ := net.ParseCIDR("0.0.0.0/0") - return defaultIPNet -} +func (m *Multitenancy) SetupRoutingForMultitenancy( + nwCfg *cni.NetworkConfig, + cnsNetworkConfig *cns.GetNetworkContainerResponse, + azIpamResult *cniTypesCurr.Result, + epInfo *network.EndpointInfo, + result *cniTypesCurr.Result, +) { + // Adding default gateway + // if snat enabled, add 169.254.128.1 as default gateway + if nwCfg.EnableSnatOnHost { + log.Printf("add default route for multitenancy.snat on host enabled") + addDefaultRoute(cnsNetworkConfig.LocalIPConfiguration.GatewayIPAddress, epInfo, result) + } else { + _, defaultIPNet, _ := net.ParseCIDR("0.0.0.0/0") + dstIP := net.IPNet{IP: net.ParseIP("0.0.0.0"), Mask: defaultIPNet.Mask} + gwIP := net.ParseIP(cnsNetworkConfig.IPConfiguration.GatewayIPAddress) + epInfo.Routes = append(epInfo.Routes, network.RouteInfo{Dst: dstIP, Gw: gwIP}) + result.Routes = append(result.Routes, &cniTypes.Route{Dst: dstIP, GW: gwIP}) + + if epInfo.EnableSnatForDns { + log.Printf("add SNAT for DNS enabled") + addSnatForDNS(cnsNetworkConfig.LocalIPConfiguration.GatewayIPAddress, epInfo, result) + } + } -func marshallPodInfo(podInfo cns.KubernetesPodInfo) []byte { - orchestratorContext, _ := json.Marshal(podInfo) - return orchestratorContext + setupInfraVnetRoutingForMultitenancy(nwCfg, azIpamResult, epInfo, result) } -type mockNetIOShim struct{} - -func (a *mockNetIOShim) GetInterfaceSubnetWithSpecificIP(ipAddr string) *net.IPNet { - return getCIDRNotationForAddress(ipAddr) -} +// get all network container configuration(s) for given orchestratorContext +func (m *Multitenancy) GetAllNetworkContainers( + ctx context.Context, nwCfg *cni.NetworkConfig, podName, podNamespace, ifName string, +) ([]IPAMAddResult, error) { + var podNameWithoutSuffix string -func getIPNet(ipaddr net.IP, mask net.IPMask) net.IPNet { - return net.IPNet{ - IP: ipaddr, - Mask: mask, + if !nwCfg.EnableExactMatchForPodName { + podNameWithoutSuffix = network.GetPodNameWithoutSuffix(podName) + } else { + podNameWithoutSuffix = podName } -} -func getIPNetWithString(ipaddrwithcidr string) *net.IPNet { - _, ipnet, err := net.ParseCIDR(ipaddrwithcidr) + log.Printf("Podname without suffix %v", podNameWithoutSuffix) + + ncResponses, hostSubnetPrefixes, err := m.getNetworkContainersInternal(ctx, podNamespace, podNameWithoutSuffix) if err != nil { - panic(err) + return []IPAMAddResult{}, fmt.Errorf("%w", err) } - return ipnet -} - -func TestSetupRoutingForMultitenancy(t *testing.T) { - require := require.New(t) //nolint:gocritic - type args struct { - nwCfg *cni.NetworkConfig - cnsNetworkConfig *cns.GetNetworkContainerResponse - azIpamResult *cniTypesCurr.Result - epInfo *network.EndpointInfo - result *cniTypesCurr.Result + for i := 0; i < len(ncResponses); i++ { + if nwCfg.EnableSnatOnHost { + if ncResponses[i].LocalIPConfiguration.IPSubnet.IPAddress == "" { + log.Printf("Snat IP is not populated for ncs %+v. Got empty string", ncResponses) + return []IPAMAddResult{}, errSnatIP + } + } } - tests := []struct { - name string - args args - multitenancyClient *Multitenancy - expected args - }{ - { - name: "test happy path", - args: args{ - nwCfg: &cni.NetworkConfig{ - MultiTenancy: true, - EnableSnatOnHost: false, - }, - cnsNetworkConfig: &cns.GetNetworkContainerResponse{ - IPConfiguration: cns.IPConfiguration{ - IPSubnet: cns.IPSubnet{}, - DNSServers: nil, - GatewayIPAddress: "10.0.0.1", - }, - }, - epInfo: &network.EndpointInfo{}, - result: &cniTypesCurr.Result{}, - }, - expected: args{ - nwCfg: &cni.NetworkConfig{ - MultiTenancy: true, - EnableSnatOnHost: false, - }, - cnsNetworkConfig: &cns.GetNetworkContainerResponse{ - IPConfiguration: cns.IPConfiguration{ - IPSubnet: cns.IPSubnet{}, - DNSServers: nil, - GatewayIPAddress: "10.0.0.1", - }, - }, - epInfo: &network.EndpointInfo{ - Routes: []network.RouteInfo{ - { - Dst: net.IPNet{IP: net.ParseIP("0.0.0.0"), Mask: defaultIPNet().Mask}, - Gw: net.ParseIP("10.0.0.1"), - }, - }, - }, - result: &cniTypesCurr.Result{ - Routes: []*cniTypes.Route{ - { - Dst: net.IPNet{IP: net.ParseIP("0.0.0.0"), Mask: defaultIPNet().Mask}, - GW: net.ParseIP("10.0.0.1"), - }, - }, - }, - }, - }, - } - for _, tt := range tests { - tt := tt - t.Run(tt.name, func(t *testing.T) { - tt.multitenancyClient.SetupRoutingForMultitenancy(tt.args.nwCfg, tt.args.cnsNetworkConfig, tt.args.azIpamResult, tt.args.epInfo, tt.args.result) - require.Exactly(tt.expected.nwCfg, tt.args.nwCfg) - require.Exactly(tt.expected.cnsNetworkConfig, tt.args.cnsNetworkConfig) - require.Exactly(tt.expected.azIpamResult, tt.args.azIpamResult) - require.Exactly(tt.expected.epInfo, tt.args.epInfo) - require.Exactly(tt.expected.result, tt.args.result) - }) + ipamResults := make([]IPAMAddResult, len(ncResponses)) + + 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) } + + return ipamResults, err } -func TestCleanupMultitenancyResources(t *testing.T) { - require := require.New(t) //nolint:gocritic - type args struct { - enableInfraVnet bool - nwCfg *cni.NetworkConfig - infraIPNet *cniTypesCurr.Result - plugin *NetPlugin +// get all network containers configuration for given orchestratorContext +func (m *Multitenancy) getNetworkContainersInternal( + ctx context.Context, namespace, podName string, +) ([]cns.GetNetworkContainerResponse, []net.IPNet, error) { + podInfo := cns.KubernetesPodInfo{ + PodName: podName, + PodNamespace: namespace, } - tests := []struct { - name string - args args - multitenancyClient *Multitenancy - expected args - }{ - { - name: "test happy path", - args: args{ - enableInfraVnet: true, - nwCfg: &cni.NetworkConfig{ - MultiTenancy: true, - }, - infraIPNet: &cniTypesCurr.Result{}, - plugin: &NetPlugin{ - ipamInvoker: NewMockIpamInvoker(false, false, false), - }, - }, - expected: args{ - nwCfg: &cni.NetworkConfig{ - MultiTenancy: true, - EnableSnatOnHost: false, - IPAM: cni.IPAM{}, - }, - infraIPNet: &cniTypesCurr.Result{}, - plugin: &NetPlugin{ - ipamInvoker: NewMockIpamInvoker(false, false, false), - }, - }, - }, + + orchestratorContext, err := json.Marshal(podInfo) + if err != nil { + log.Printf("Marshalling KubernetesPodInfo failed with %v", err) + return nil, []net.IPNet{}, fmt.Errorf("%w", err) } - for _, tt := range tests { - tt := tt - t.Run(tt.name, func(t *testing.T) { - require.Exactly(tt.expected.nwCfg, tt.args.nwCfg) - require.Exactly(tt.expected.infraIPNet, tt.args.infraIPNet) - require.Exactly(tt.expected.plugin, tt.args.plugin) - }) + + // First try the new CNS API that returns slice of nc responses. If CNS doesn't support the new API, an error will be returned and as a result + // try using the old CNS API that returns single nc response. + ncConfigs, err := m.cnsclient.GetAllNetworkContainers(ctx, orchestratorContext) + if err != nil && client.IsUnsupportedAPI(err) { + ncConfig, errGetNC := m.cnsclient.GetNetworkContainer(ctx, orchestratorContext) + if errGetNC != nil { + return nil, []net.IPNet{}, fmt.Errorf("%w", err) + } + ncConfigs = append(ncConfigs, *ncConfig) + } else if err != nil { + return nil, []net.IPNet{}, fmt.Errorf("%w", err) } -} -func TestGetMultiTenancyCNIResult(t *testing.T) { - require := require.New(t) //nolint:gocritic - - var ncResponses []cns.GetNetworkContainerResponse - ncResponseOne := cns.GetNetworkContainerResponse{ - PrimaryInterfaceIdentifier: "10.0.0.0/16", - LocalIPConfiguration: cns.IPConfiguration{ - IPSubnet: cns.IPSubnet{ - IPAddress: "10.0.0.5", - PrefixLength: 16, - }, - GatewayIPAddress: "", - }, - CnetAddressSpace: []cns.IPSubnet{ - { - IPAddress: "10.1.0.0", - PrefixLength: 16, - }, - }, - IPConfiguration: cns.IPConfiguration{ - IPSubnet: cns.IPSubnet{ - IPAddress: "10.1.0.5", - PrefixLength: 16, - }, - DNSServers: nil, - GatewayIPAddress: "10.1.0.1", - }, - Routes: []cns.Route{ - { - IPAddress: "10.1.0.0/16", - GatewayIPAddress: "10.1.0.1", - }, - }, + log.Printf("Network config received from cns %+v", ncConfigs) + + subnetPrefixes := []net.IPNet{} + for i := 0; i < len(ncConfigs); i++ { + subnetPrefix := m.netioshim.GetInterfaceSubnetWithSpecificIP(ncConfigs[i].PrimaryInterfaceIdentifier) + if subnetPrefix == nil { + log.Printf("%w %s", errIfaceNotFound, ncConfigs[i].PrimaryInterfaceIdentifier) + return nil, []net.IPNet{}, errIfaceNotFound + } + subnetPrefixes = append(subnetPrefixes, *subnetPrefix) } - ncResponseTwo := cns.GetNetworkContainerResponse{ - PrimaryInterfaceIdentifier: "20.0.0.0/16", - LocalIPConfiguration: cns.IPConfiguration{ - IPSubnet: cns.IPSubnet{ - IPAddress: "20.0.0.5", - PrefixLength: 16, - }, - GatewayIPAddress: "", - }, - CnetAddressSpace: []cns.IPSubnet{ - { - IPAddress: "20.1.0.0", - PrefixLength: 16, - }, - }, - IPConfiguration: cns.IPConfiguration{ - IPSubnet: cns.IPSubnet{ - IPAddress: "20.1.0.5", - PrefixLength: 16, - }, - DNSServers: nil, - GatewayIPAddress: "20.1.0.1", - }, - Routes: []cns.Route{ - { - IPAddress: "20.1.0.0/16", - GatewayIPAddress: "20.1.0.1", - }, - }, + return ncConfigs, subnetPrefixes, nil +} + +func convertToCniResult(networkConfig *cns.GetNetworkContainerResponse, ifName string) *cniTypesCurr.Result { + result := &cniTypesCurr.Result{} + resultIpconfig := &cniTypesCurr.IPConfig{} + + ipconfig := networkConfig.IPConfiguration + ipAddr := net.ParseIP(ipconfig.IPSubnet.IPAddress) + + if ipAddr.To4() != nil { + resultIpconfig.Address = net.IPNet{IP: ipAddr, Mask: net.CIDRMask(int(ipconfig.IPSubnet.PrefixLength), 32)} + } else { + resultIpconfig.Address = net.IPNet{IP: ipAddr, Mask: net.CIDRMask(int(ipconfig.IPSubnet.PrefixLength), 128)} } - ncResponses = append(ncResponses, ncResponseOne, ncResponseTwo) - - type args struct { - ctx context.Context - enableInfraVnet bool - nwCfg *cni.NetworkConfig - plugin *NetPlugin - k8sPodName string - k8sNamespace string - ifName string + + resultIpconfig.Gateway = net.ParseIP(ipconfig.GatewayIPAddress) + result.IPs = append(result.IPs, resultIpconfig) + + if networkConfig.Routes != nil && len(networkConfig.Routes) > 0 { + for _, route := range networkConfig.Routes { + _, routeIPnet, _ := net.ParseCIDR(route.IPAddress) + gwIP := net.ParseIP(route.GatewayIPAddress) + result.Routes = append(result.Routes, &cniTypes.Route{Dst: *routeIPnet, GW: gwIP}) + } } - tests := []struct { - name string - args args - want *cniTypesCurr.Result - want1 *cns.GetNetworkContainerResponse - want2 *cns.GetNetworkContainerResponse - want3 net.IPNet - want4 *cniTypesCurr.Result - want5 []cns.GetNetworkContainerResponse - wantErr bool - }{ - { - name: "test happy path", - args: args{ - enableInfraVnet: true, - nwCfg: &cni.NetworkConfig{ - MultiTenancy: true, - EnableSnatOnHost: true, - EnableExactMatchForPodName: true, - InfraVnetAddressSpace: "10.0.0.0/16", - IPAM: cni.IPAM{Type: "azure-vnet-ipam"}, - }, - plugin: &NetPlugin{ - ipamInvoker: NewMockIpamInvoker(false, false, false), - multitenancyClient: &Multitenancy{ - netioshim: &mockNetIOShim{}, - cnsclient: &MockCNSClient{ - require: require, - getAllNetworkContainersConfiguration: getAllNetworkContainersConfigurationHandler{ - orchestratorContext: marshallPodInfo(cns.KubernetesPodInfo{ - PodName: "testpod", - PodNamespace: "testnamespace", - }), - returnResponse: ncResponses, - }, - }, - }, - }, - k8sPodName: "testpod", - k8sNamespace: "testnamespace", - ifName: "eth0", - }, - want: &cniTypesCurr.Result{ - Interfaces: []*cniTypesCurr.Interface{ - { - Name: "eth0", - }, - }, - IPs: []*cniTypesCurr.IPConfig{ - { - Address: getIPNet(net.IPv4(10, 1, 0, 5), net.CIDRMask(16, 32)), - Gateway: net.ParseIP("10.1.0.1"), - }, - }, - Routes: []*cniTypes.Route{ - { - Dst: *getIPNetWithString("10.1.0.0/16"), - GW: net.ParseIP("10.1.0.1"), - }, - { - Dst: net.IPNet{IP: net.ParseIP("10.1.0.0"), Mask: net.CIDRMask(16, 32)}, - GW: net.ParseIP("10.1.0.1"), - }, - }, - }, - want1: &cns.GetNetworkContainerResponse{ - PrimaryInterfaceIdentifier: "10.0.0.0/16", - LocalIPConfiguration: cns.IPConfiguration{ - IPSubnet: cns.IPSubnet{ - IPAddress: "10.0.0.5", - PrefixLength: 16, - }, - GatewayIPAddress: "", - }, - CnetAddressSpace: []cns.IPSubnet{ - { - IPAddress: "10.1.0.0", - PrefixLength: 16, - }, - }, - IPConfiguration: cns.IPConfiguration{ - IPSubnet: cns.IPSubnet{ - IPAddress: "10.1.0.5", - PrefixLength: 16, - }, - DNSServers: nil, - GatewayIPAddress: "10.1.0.1", - }, - Routes: []cns.Route{ - { - IPAddress: "10.1.0.0/16", - GatewayIPAddress: "10.1.0.1", - }, - }, - }, - want2: &cns.GetNetworkContainerResponse{ - PrimaryInterfaceIdentifier: "20.0.0.0/16", - LocalIPConfiguration: cns.IPConfiguration{ - IPSubnet: cns.IPSubnet{ - IPAddress: "20.0.0.5", - PrefixLength: 16, - }, - GatewayIPAddress: "", - }, - CnetAddressSpace: []cns.IPSubnet{ - { - IPAddress: "20.1.0.0", - PrefixLength: 16, - }, - }, - IPConfiguration: cns.IPConfiguration{ - IPSubnet: cns.IPSubnet{ - IPAddress: "20.1.0.5", - PrefixLength: 16, - }, - DNSServers: nil, - GatewayIPAddress: "20.1.0.1", - }, - Routes: []cns.Route{ - { - IPAddress: "20.1.0.0/16", - GatewayIPAddress: "20.1.0.1", - }, - }, - }, - want3: *getCIDRNotationForAddress("10.0.0.0/16"), - want4: &cniTypesCurr.Result{ - IPs: []*cniTypesCurr.IPConfig{ - { - Address: net.IPNet{ - IP: net.ParseIP("10.240.0.5"), - Mask: net.CIDRMask(24, 32), - }, - Gateway: net.ParseIP("10.240.0.1"), - }, - }, - Routes: nil, - DNS: cniTypes.DNS{}, - }, - }, + for _, ipRouteSubnet := range networkConfig.CnetAddressSpace { + routeIPnet := net.IPNet{IP: net.ParseIP(ipRouteSubnet.IPAddress), Mask: net.CIDRMask(int(ipRouteSubnet.PrefixLength), 32)} + gwIP := net.ParseIP(ipconfig.GatewayIPAddress) + result.Routes = append(result.Routes, &cniTypes.Route{Dst: routeIPnet, GW: gwIP}) } - for _, tt := range tests { - tt := tt - t.Run(tt.name, func(t *testing.T) { - got, err := tt.args.plugin.multitenancyClient.GetAllNetworkContainers( - tt.args.ctx, - tt.args.nwCfg, - tt.args.k8sPodName, - tt.args.k8sNamespace, - tt.args.ifName) - if (err != nil) != tt.wantErr { - t.Errorf("GetContainerNetworkConfiguration() error = %v, wantErr %v", err, tt.wantErr) - return - } - if tt.wantErr { - require.Error(err) + + iface := &cniTypesCurr.Interface{Name: ifName} + result.Interfaces = append(result.Interfaces, iface) + + return result +} + +func checkIfSubnetOverlaps(enableInfraVnet bool, nwCfg *cni.NetworkConfig, cnsNetworkConfig *cns.GetNetworkContainerResponse) bool { + if enableInfraVnet { + if cnsNetworkConfig != nil { + _, infraNet, _ := net.ParseCIDR(nwCfg.InfraVnetAddressSpace) + for _, cnetSpace := range cnsNetworkConfig.CnetAddressSpace { + cnetSpaceIPNet := &net.IPNet{ + IP: net.ParseIP(cnetSpace.IPAddress), + Mask: net.CIDRMask(int(cnetSpace.PrefixLength), 32), + } + + return infraNet.Contains(cnetSpaceIPNet.IP) || cnetSpaceIPNet.Contains(infraNet.IP) } - require.NoError(err) - require.Exactly(tt.want1, got[0].ncResponse) - require.Exactly(tt.want2, got[1].ncResponse) - require.Exactly(tt.want3, got[0].hostSubnetPrefix) - - // check multiple responses - tt.want5 = append(tt.want5, *tt.want1, *tt.want2) - require.Exactly(tt.want5, ncResponses) - }) + } } + + return false } + +var ( + errSnatIP = errors.New("Snat IP not populated") + errInfraVnet = errors.New("infravnet not populated") + errSubnetOverlap = errors.New("subnet overlap error") + errIfaceNotFound = errors.New("Interface not found for this ip") +) diff --git a/cni/network/network.go b/cni/network/network.go index fd2c284f23..5090387702 100644 --- a/cni/network/network.go +++ b/cni/network/network.go @@ -385,7 +385,7 @@ func (plugin *NetPlugin) Add(args *cniSkel.CmdArgs) error { res.Print() } - log.Printf("[cni-net] ADD command completed for pod %v with IPs:%+v err:%v.", k8sPodName, ipamAddResult.ipv4Result.IPs, err) + log.Printf("[cni-net] ADD command completed for pod %v with IPs:%+v err:%v.", k8sPodName, ipamAddResult.ipv4Result.IPs, err) }() // Parse Pod arguments. From be7c1f0a4b9ac512996b029249054efc791e0f94 Mon Sep 17 00:00:00 2001 From: paulyu Date: Wed, 22 Mar 2023 14:25:24 -0400 Subject: [PATCH 05/44] fix few comments --- cni/network/invoker_cns.go | 2 +- cni/network/invoker_mock.go | 2 +- network/network_windows.go | 5 ++++- 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/cni/network/invoker_cns.go b/cni/network/invoker_cns.go index fc8a2736b7..25dec265cf 100644 --- a/cni/network/invoker_cns.go +++ b/cni/network/invoker_cns.go @@ -170,7 +170,7 @@ func (invoker *CNSIPAMInvoker) Add(addConfig IPAMAddConfig) (IPAMAddResult, erro addResult.hostSubnetPrefix = *hostIPNet // set subnet prefix for host vm - // setHostOptions will execute if IPAM mode is not v4 overlay + // setHostOptions will execute if IPAM mode is not v4 overlay and not dualStackOverlay mode if (invoker.ipamMode != util.V4Overlay) && (invoker.ipamMode != util.DualStackOverlay) { if err := setHostOptions(ncIPNet, addConfig.options, &info); err != nil { return IPAMAddResult{}, err diff --git a/cni/network/invoker_mock.go b/cni/network/invoker_mock.go index 9b4afa884f..b02dbdd43b 100644 --- a/cni/network/invoker_mock.go +++ b/cni/network/invoker_mock.go @@ -84,7 +84,7 @@ func (invoker *MockIpamInvoker) Delete(addresses []*net.IPNet, nwCfg *cni.Networ return errDeleteIpam } - if len(addresses) <= 0 || invoker.ipMap == nil { + if len(addresses) == 0 || invoker.ipMap == nil { return nil } diff --git a/network/network_windows.go b/network/network_windows.go index 6f80fc5269..69b5f9d25a 100644 --- a/network/network_windows.go +++ b/network/network_windows.go @@ -208,7 +208,10 @@ func (nm *networkManager) addNewNetRules(nwInfo *NetworkInfo) error { prefix := subnet.Prefix.String() gateway := subnet.Gateway.String() - ip, _, _ := net.ParseCIDR(prefix) + ip, _, er := net.ParseCIDR(prefix) + if er !- nil { + return fmt.Errorf("[net] failed to parse prefix %s", prefix) + } if ip.To4() != nil { // netsh interface ipv4 add route $subnetV4 $hostInterfaceAlias "0.0.0.0" metric=270 netshV4DefaultRoute := fmt.Sprintf(netRouteCmd, "ipv4", prefix, ifName, ipv4DefaultHop, "270") From e3ee07eaea35f270a844ed61260fdd067ab9a882 Mon Sep 17 00:00:00 2001 From: paulyu Date: Wed, 22 Mar 2023 15:52:39 -0400 Subject: [PATCH 06/44] fix comments --- cni/network/network.go | 1 + cni/network/network_test.go | 12 ++++++++++++ 2 files changed, 13 insertions(+) diff --git a/cni/network/network.go b/cni/network/network.go index 5090387702..cc1aafdece 100644 --- a/cni/network/network.go +++ b/cni/network/network.go @@ -571,6 +571,7 @@ func (plugin *NetPlugin) Add(args *cniSkel.CmdArgs) error { return nil } +// cleanup allocated ipv4 and ipv6 addresses if they exist func (plugin *NetPlugin) cleanupAllocationOnError( result, resultV6 *cniTypesCurr.Result, nwCfg *cni.NetworkConfig, diff --git a/cni/network/network_test.go b/cni/network/network_test.go index a8cfa59b29..c617f257ee 100644 --- a/cni/network/network_test.go +++ b/cni/network/network_test.go @@ -1102,3 +1102,15 @@ func TestGetPodSubnetNatInfo(t *testing.T) { require.Empty(t, natInfo, "linux podsubnet natInfo should be empty") } } + +func TestGetPodSubnetNatInfoV6(t *testing.T) { + ncPrimaryIP := "2001:2002:2003::1" + nwCfg := &cni.NetworkConfig{ExecutionMode: string(util.V4Swift)} + natInfo := getNATInfo(nwCfg, ncPrimaryIP, false) + // should not add any natInfo to policy if ncPrimaryIP is ipv6 + if runtime.GOOS == "windows" { + require.Equalf(t, natInfo, []policy.NATInfo{ + {}, + }, "no ipv6 natInfo is added") + } +} From 369ac155390580939bbf07dd098989ebae6aaf5c Mon Sep 17 00:00:00 2001 From: paulyu Date: Thu, 23 Mar 2023 11:29:06 -0400 Subject: [PATCH 07/44] add Jaeryn's changes --- cni/network/invoker_cns.go | 17 ++++++++++------- network/network.go | 2 +- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/cni/network/invoker_cns.go b/cni/network/invoker_cns.go index 25dec265cf..f039d373ca 100644 --- a/cni/network/invoker_cns.go +++ b/cni/network/invoker_cns.go @@ -104,6 +104,12 @@ func (invoker *CNSIPAMInvoker) Add(addConfig IPAMAddConfig) (IPAMAddResult, erro log.Printf("[cni-invoker-cns] Received info %+v for pod %v", info, podInfo) + // set result ipconfigArgument from CNS Response Body + 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 { if (invoker.ipamMode != util.V4Overlay) && (invoker.ipamMode != util.DualStackOverlay) { @@ -111,18 +117,15 @@ func (invoker *CNSIPAMInvoker) Add(addConfig IPAMAddConfig) (IPAMAddResult, erro } if net.ParseIP(info.podIPAddress).To4() != nil { - ncgw = net.ParseIP(overlayGatewayV4IP) + ncgw, err = getOverlayGateway(ncIPNet) + if err != nil { + return IPAMAddResult{}, err + } } else { ncgw = net.ParseIP(overlayGatewayV6IP) } } - // set result ipconfigArgument from CNS Response Body - 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") - } - // construct ipnet for result resultIPnet := net.IPNet{ IP: ip, diff --git a/network/network.go b/network/network.go index adfbe28876..99796ac54f 100644 --- a/network/network.go +++ b/network/network.go @@ -26,7 +26,7 @@ const ( // ipv6 modes IPV6Nat = "ipv6nat" // dual stack mode - DualStack = "dualstack" + DualStack = "dualStackOverlay" ) // externalInterface is a host network interface that bridges containers to external networks. From 9fa29767da613df315b2b4c9e0ea027e5668b036 Mon Sep 17 00:00:00 2001 From: paulyu Date: Thu, 23 Mar 2023 11:36:57 -0400 Subject: [PATCH 08/44] add Jaeryn's changes --- cni/network/invoker_cns.go | 1 - cni/network/network.go | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/cni/network/invoker_cns.go b/cni/network/invoker_cns.go index f039d373ca..72921cb610 100644 --- a/cni/network/invoker_cns.go +++ b/cni/network/invoker_cns.go @@ -23,7 +23,6 @@ import ( var ( errEmptyCNIArgs = errors.New("empty CNI cmd args not allowed") errInvalidArgs = errors.New("invalid arg(s)") - overlayGatewayV4IP = "169.254.1.1" overlayGatewayV6IP = "fe80::1234:5678:9abc" ) diff --git a/cni/network/network.go b/cni/network/network.go index cc1aafdece..18d063ea26 100644 --- a/cni/network/network.go +++ b/cni/network/network.go @@ -539,7 +539,7 @@ func (plugin *NetPlugin) Add(args *cniSkel.CmdArgs) error { logAndSendEvent(plugin, fmt.Sprintf("[cni-net] Created network %v with subnet %v.", networkID, ipamAddResult.hostSubnetPrefix.String())) } - natInfo := getNATInfo(nwCfg.ExecutionMode, options[network.SNATIPKey], nwCfg.MultiTenancy, enableSnatForDNS) + natInfo := getNATInfo(nwCfg, options[network.SNATIPKey], enableSnatForDNS) createEndpointInternalOpt := createEndpointInternalOpt{ nwCfg: nwCfg, From a21cbe827c8860bb7d156d3af89a4209db805ada Mon Sep 17 00:00:00 2001 From: paulyu Date: Fri, 24 Mar 2023 16:27:37 -0400 Subject: [PATCH 09/44] fix ipv4Result.IPs nil issue --- cni/network/network.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/cni/network/network.go b/cni/network/network.go index 18d063ea26..59b9996551 100644 --- a/cni/network/network.go +++ b/cni/network/network.go @@ -385,7 +385,11 @@ func (plugin *NetPlugin) Add(args *cniSkel.CmdArgs) error { res.Print() } - log.Printf("[cni-net] ADD command completed for pod %v with IPs:%+v err:%v.", k8sPodName, ipamAddResult.ipv4Result.IPs, err) + if ipamAddResult.ipv4Result.IPs != nil { + log.Printf("[cni-net] ADD command completed for pod %v with IPs:%+v err:%v.", k8sPodName, ipamAddResult.ipv4Result.IPs, err) + } else { + log.Printf("[cni-net] no IPs will be allocated for pod %v", k8sPodName) + } }() // Parse Pod arguments. From 956c6534dd8e1a0745cd001fc57c621c204c5c20 Mon Sep 17 00:00:00 2001 From: paulyu Date: Wed, 29 Mar 2023 18:04:08 -0400 Subject: [PATCH 10/44] add RequestIPs fix from Ryan --- cni/network/invoker_cns.go | 31 ++++++++++++++++++++++++++++--- 1 file changed, 28 insertions(+), 3 deletions(-) diff --git a/cni/network/invoker_cns.go b/cni/network/invoker_cns.go index 72921cb610..38a79ebaf3 100644 --- a/cni/network/invoker_cns.go +++ b/cni/network/invoker_cns.go @@ -81,8 +81,33 @@ func (invoker *CNSIPAMInvoker) Add(addConfig IPAMAddConfig) (IPAMAddResult, erro log.Printf("Requesting IP for pod %+v using ipconfig %+v", podInfo, ipconfig) response, err := invoker.cnsClient.RequestIPs(context.TODO(), ipconfig) if err != nil { - log.Printf("Failed to get IP address from CNS with error %v, response: %v", err, response) - return IPAMAddResult{}, errors.Wrap(err, "Failed to get IP address from CNS with error: %w") + // checks for 404 error + if errors.Is(err, cnscli.ErrAPINotFound) { + ipconfigRequest := cns.IPConfigRequest{ + OrchestratorContext: orchestratorContext, + PodInterfaceID: GetEndpointID(addConfig.args), + InfraContainerID: addConfig.args.ContainerID, + } + log.Errorf("Failed to request IPs using RequestIPs from CNS, going to try RequestIPAddress. error: %v request: %v", err, ipconfig) + + res, err := invoker.cnsClient.RequestIPAddress(context.TODO(), ipconfigRequest) + if err != nil { + // if the old API fails as well then we just return the error + log.Errorf("Failed to release IP address from CNS using ReleaseIPAddress. error: %v request: %v", err, ipconfigRequest) + return IPAMAddResult{}, errors.Wrap(err, fmt.Sprintf("failed to request IPs for pod %v with error ", GetEndpointID(addConfig.args))+"%w") + } + // sets the response to the new contract + response = &cns.IPConfigsResponse{ + Response: res.Response, + PodIPInfo: []cns.PodIpInfo{ + res.PodIpInfo, + }, + } + + } else { + log.Printf("Failed to get IP address from CNS with error %v, response: %v", err, response) + return IPAMAddResult{}, errors.Wrap(err, "Failed to get IP address from CNS with error: %w") + } } addResult := IPAMAddResult{} @@ -281,7 +306,7 @@ func (invoker *CNSIPAMInvoker) Delete(addresses []*net.IPNet, nwCfg *cni.Network } log.Errorf("Failed to release IPs using ReleaseIPs from CNS, going to try ReleaseIPAddress. error: %v request: %v", err, req) - if err := invoker.cnsClient.ReleaseIPAddress(context.TODO(), ipconfigRequest); err != nil { + if err = invoker.cnsClient.ReleaseIPAddress(context.TODO(), ipconfigRequest); err != nil { // if the old API fails as well then we just return the error log.Errorf("Failed to release IP address from CNS using ReleaseIPAddress. error: %v request: %v", err, req) return errors.Wrap(err, fmt.Sprintf("failed to release IP %v using ReleaseIPAddress with err ", addresses)+"%w") From d1b8bea6a36550416dc0a046ba00b651a410d7f7 Mon Sep 17 00:00:00 2001 From: paulyu Date: Thu, 30 Mar 2023 17:03:20 -0400 Subject: [PATCH 11/44] fix UT --- cni/network/invoker_cns_test.go | 148 ++++++++++++++++--------------- cni/network/multitenancy_test.go | 4 +- 2 files changed, 80 insertions(+), 72 deletions(-) diff --git a/cni/network/invoker_cns_test.go b/cni/network/invoker_cns_test.go index e2f38a4af4..cc94e8aad3 100644 --- a/cni/network/invoker_cns_test.go +++ b/cni/network/invoker_cns_test.go @@ -20,6 +20,14 @@ import ( var testPodInfo cns.KubernetesPodInfo +func getTestIPConfigRequest() cns.IPConfigRequest { + return cns.IPConfigRequest{ + PodInterfaceID: "testcont-testifname", + InfraContainerID: "testcontainerid", + OrchestratorContext: marshallPodInfo(testPodInfo), + } +} + func getTestIPConfigsRequest() cns.IPConfigsRequest { return cns.IPConfigsRequest{ PodInterfaceID: "testcont-testifname", @@ -42,6 +50,7 @@ func TestCNSIPAMInvoker_Add(t *testing.T) { podName string podNamespace string cnsClient cnsclient + ipamMode util.IpamMode } type args struct { nwCfg *cni.NetworkConfig @@ -49,6 +58,7 @@ func TestCNSIPAMInvoker_Add(t *testing.T) { hostSubnetPrefix *net.IPNet options map[string]interface{} } + tests := []struct { name string fields fields @@ -57,6 +67,73 @@ func TestCNSIPAMInvoker_Add(t *testing.T) { want1 *cniTypesCurr.Result wantErr bool }{ + { + name: "Test happy CNI add for IPv4", + fields: fields{ + podName: testPodInfo.PodName, + podNamespace: testPodInfo.PodNamespace, + cnsClient: &MockCNSClient{ + require: require, + requestIPs: requestIPsHandler{ + ipconfigArgument: getTestIPConfigsRequest(), + 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", + }, + }, + }, + Response: cns.Response{ + ReturnCode: 0, + Message: "", + }, + }, + err: nil, + }, + }, + }, + args: args{ + nwCfg: &cni.NetworkConfig{}, + args: &cniSkel.CmdArgs{ + ContainerID: "testcontainerid", + Netns: "testnetns", + IfName: "testifname", + }, + hostSubnetPrefix: getCIDRNotationForAddress("10.0.0.1/24"), + options: map[string]interface{}{}, + }, + want: &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"), + }, + }, + }, + want1: nil, + wantErr: false, + }, { name: "Test happy CNI add", fields: fields{ @@ -172,76 +249,6 @@ func TestCNSIPAMInvoker_Add(t *testing.T) { }, wantErr: true, }, - { - name: "Test happy CNI Overlay add", - fields: fields{ - podName: testPodInfo.PodName, - podNamespace: testPodInfo.PodNamespace, - ipamMode: util.V4Overlay, - cnsClient: &MockCNSClient{ - require: require, - request: requestIPAddressHandler{ - ipconfigArgument: cns.IPConfigRequest{ - PodInterfaceID: "testcont-testifname3", - InfraContainerID: "testcontainerid3", - OrchestratorContext: marshallPodInfo(testPodInfo), - }, - result: &cns.IPConfigResponse{ - PodIpInfo: cns.PodIpInfo{ - PodIPConfig: cns.IPSubnet{ - IPAddress: "10.240.1.242", - PrefixLength: 16, - }, - NetworkContainerPrimaryIPConfig: cns.IPConfiguration{ - IPSubnet: cns.IPSubnet{ - IPAddress: "10.240.1.0", - PrefixLength: 16, - }, - DNSServers: nil, - GatewayIPAddress: "", - }, - HostPrimaryIPInfo: cns.HostIPInfo{ - Gateway: "10.224.0.1", - PrimaryIP: "10.224.0.5", - Subnet: "10.224.0.0/16", - }, - }, - 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.224.0.0/16"), - options: map[string]interface{}{}, - }, - want: &cniTypesCurr.Result{ - IPs: []*cniTypesCurr.IPConfig{ - { - Address: *getCIDRNotationForAddress("10.240.1.242/16"), - Gateway: getTestOverlayGateway(), - }, - }, - Routes: []*cniTypes.Route{ - { - Dst: network.Ipv4DefaultRouteDstPrefix, - GW: getTestOverlayGateway(), - }, - }, - }, - want1: nil, - wantErr: false, - }, } for _, tt := range tests { tt := tt @@ -260,6 +267,7 @@ func TestCNSIPAMInvoker_Add(t *testing.T) { } else { require.NoError(err) } + fmt.Printf("want:%+v\nrest:%+v\n", tt.want, ipamAddResult.ipv4Result) require.Equalf(tt.want, ipamAddResult.ipv4Result, "incorrect ipv4 response") require.Equalf(tt.want1, ipamAddResult.ipv6Result, "incorrect ipv6 response") diff --git a/cni/network/multitenancy_test.go b/cni/network/multitenancy_test.go index 011f99841a..c9facdd0c0 100644 --- a/cni/network/multitenancy_test.go +++ b/cni/network/multitenancy_test.go @@ -70,12 +70,12 @@ func (c *MockCNSClient) RequestIPs(_ context.Context, ipconfig cns.IPConfigsRequ return c.requestIPs.result, c.requestIPs.err } -func (c *MockCNSClient) ReleaseIPs(_ context.Context, ipconfig cns.IPConfigsRequest) error { +func (c *MockCNSClient) ReleaseIPAddress(_ context.Context, ipconfig cns.IPConfigRequest) error { c.require.Exactly(c.release.ipconfigArgument, ipconfig) return c.release.err } -func (c *MockCNSClient) ReleaseIPAddress(_ context.Context, ipconfig cns.IPConfigRequest) error { +func (c *MockCNSClient) ReleaseIPs(_ context.Context, ipconfig cns.IPConfigsRequest) error { c.require.Exactly(c.release.ipconfigArgument, ipconfig) return c.release.err } From 230d6b9cefaa49dee483800299f715810145baf5 Mon Sep 17 00:00:00 2001 From: paulyu Date: Tue, 4 Apr 2023 12:21:55 -0400 Subject: [PATCH 12/44] fix != issue --- network/network_windows.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/network/network_windows.go b/network/network_windows.go index 69b5f9d25a..60500922e7 100644 --- a/network/network_windows.go +++ b/network/network_windows.go @@ -209,7 +209,7 @@ func (nm *networkManager) addNewNetRules(nwInfo *NetworkInfo) error { gateway := subnet.Gateway.String() ip, _, er := net.ParseCIDR(prefix) - if er !- nil { + if er != nil { return fmt.Errorf("[net] failed to parse prefix %s", prefix) } if ip.To4() != nil { From c2ac5ff729bb7895d975c693bb4b1468f5a0506c Mon Sep 17 00:00:00 2001 From: paulyu Date: Tue, 4 Apr 2023 18:16:05 -0400 Subject: [PATCH 13/44] fix linter issue --- network/manager.go | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/network/manager.go b/network/manager.go index a9b167717d..b117565069 100644 --- a/network/manager.go +++ b/network/manager.go @@ -19,13 +19,14 @@ import ( const ( // Network store key. - storeKey = "Network" - VlanIDKey = "VlanID" - AzureCNS = "azure-cns" - SNATIPKey = "NCPrimaryIPKey" - RoutesKey = "RoutesKey" - IPTablesKey = "IPTablesKey" - genericData = "com.docker.network.generic" + storeKey = "Network" + VlanIDKey = "VlanID" + AzureCNS = "azure-cns" + SNATIPKey = "NCPrimaryIPKey" + RoutesKey = "RoutesKey" + IPTablesKey = "IPTablesKey" + genericData = "com.docker.network.generic" + ipv6AddressMask = 128 ) var Ipv4DefaultRouteDstPrefix = net.IPNet{ @@ -36,7 +37,7 @@ var Ipv4DefaultRouteDstPrefix = net.IPNet{ var Ipv6DefaultRouteDstPrefix = net.IPNet{ IP: net.IPv6zero, // This mask corresponds to a /0 subnet for IPv6 - Mask: net.CIDRMask(0, 128), + Mask: net.CIDRMask(0, ipv6AddressMask), } type NetworkClient interface { From 4570853fba37cdf9d562742ec4d1b9c99d2b1476 Mon Sep 17 00:00:00 2001 From: paulyu Date: Wed, 5 Apr 2023 10:33:02 -0400 Subject: [PATCH 14/44] fix comments --- cni/network/invoker_azure.go | 1 - 1 file changed, 1 deletion(-) diff --git a/cni/network/invoker_azure.go b/cni/network/invoker_azure.go index 64edbd826d..4304e85567 100644 --- a/cni/network/invoker_azure.go +++ b/cni/network/invoker_azure.go @@ -132,7 +132,6 @@ func (invoker *AzureIPAMInvoker) deleteIpamState() { } func (invoker *AzureIPAMInvoker) Delete(addresses []*net.IPNet, nwCfg *cni.NetworkConfig, _ *cniSkel.CmdArgs, options map[string]interface{}) error { - if nwCfg == nil { return invoker.plugin.Errorf("nil nwCfg passed to CNI ADD, stack: %+v", string(debug.Stack())) } From 6e56bd524b9431d92f48b76af3d0522f38632b74 Mon Sep 17 00:00:00 2001 From: paulyu Date: Wed, 5 Apr 2023 13:04:42 -0400 Subject: [PATCH 15/44] fix comments --- cni/network/invoker_azure.go | 3 +-- cni/network/invoker_cns.go | 21 ++++++++++----------- cns/client/client.go | 6 +++--- 3 files changed, 14 insertions(+), 16 deletions(-) diff --git a/cni/network/invoker_azure.go b/cni/network/invoker_azure.go index 4304e85567..b40b972623 100644 --- a/cni/network/invoker_azure.go +++ b/cni/network/invoker_azure.go @@ -163,8 +163,7 @@ func (invoker *AzureIPAMInvoker) Delete(addresses []*net.IPNet, nwCfg *cni.Netwo nwCfgIpv6.IPAM.Subnet = invoker.nwInfo.Subnets[1].Prefix.String() } - log.Printf("Releasing ipv6 address :%s pool: %s", - nwCfgIpv6.IPAM.Address, nwCfgIpv6.IPAM.Subnet) + log.Printf("Releasing ipv6 address :%s pool: %s", nwCfgIpv6.IPAM.Address, nwCfgIpv6.IPAM.Subnet) if err := invoker.plugin.DelegateDel(nwCfgIpv6.IPAM.Type, &nwCfgIpv6); err != nil { log.Printf("Failed to release ipv6 address: %v", err) return invoker.plugin.Errorf("Failed to release ipv6 address: %v", err) diff --git a/cni/network/invoker_cns.go b/cni/network/invoker_cns.go index 38a79ebaf3..c079f049a1 100644 --- a/cni/network/invoker_cns.go +++ b/cni/network/invoker_cns.go @@ -81,29 +81,27 @@ func (invoker *CNSIPAMInvoker) Add(addConfig IPAMAddConfig) (IPAMAddResult, erro log.Printf("Requesting IP for pod %+v using ipconfig %+v", podInfo, ipconfig) response, err := invoker.cnsClient.RequestIPs(context.TODO(), ipconfig) if err != nil { - // checks for 404 error + // if RequestIPs call fails, we may receive API not Found error as new CNS is not supported, then try old API RequestIPAddress + log.Errorf("RequestIPs not supported by CNS. Invoking RequestIPAddress API. Request: %v", ipconfig) if errors.Is(err, cnscli.ErrAPINotFound) { ipconfigRequest := cns.IPConfigRequest{ OrchestratorContext: orchestratorContext, PodInterfaceID: GetEndpointID(addConfig.args), InfraContainerID: addConfig.args.ContainerID, } - log.Errorf("Failed to request IPs using RequestIPs from CNS, going to try RequestIPAddress. error: %v request: %v", err, ipconfig) res, err := invoker.cnsClient.RequestIPAddress(context.TODO(), ipconfigRequest) if err != nil { // if the old API fails as well then we just return the error - log.Errorf("Failed to release IP address from CNS using ReleaseIPAddress. error: %v request: %v", err, ipconfigRequest) - return IPAMAddResult{}, errors.Wrap(err, fmt.Sprintf("failed to request IPs for pod %v with error ", GetEndpointID(addConfig.args))+"%w") + log.Errorf("Failed to request IP address from CNS using RequestIPAddress. error: %v request: %v", err, ipconfigRequest) + return IPAMAddResult{}, errors.Wrap(err, "Failed to get IP address from CNS with error: %w") } - // sets the response to the new contract response = &cns.IPConfigsResponse{ Response: res.Response, PodIPInfo: []cns.PodIpInfo{ res.PodIpInfo, }, } - } else { log.Printf("Failed to get IP address from CNS with error %v, response: %v", err, response) return IPAMAddResult{}, errors.Wrap(err, "Failed to get IP address from CNS with error: %w") @@ -145,8 +143,10 @@ func (invoker *CNSIPAMInvoker) Add(addConfig IPAMAddConfig) (IPAMAddResult, erro if err != nil { return IPAMAddResult{}, err } - } else { + } else if net.ParseIP(info.podIPAddress).To16() != nil { ncgw = net.ParseIP(overlayGatewayV6IP) + } else { + return IPAMAddResult{}, errors.Wrap(err, "No podIPAddress is found: %w") } } @@ -171,7 +171,7 @@ func (invoker *CNSIPAMInvoker) Add(addConfig IPAMAddConfig) (IPAMAddResult, erro }, }, } - } else { + } else if net.ParseIP(info.podIPAddress).To16() != nil { addResult.ipv6Result = &cniTypesCurr.Result{ IPs: []*cniTypesCurr.IPConfig{ { @@ -296,15 +296,14 @@ func (invoker *CNSIPAMInvoker) Delete(addresses []*net.IPNet, nwCfg *cni.Network } if err := invoker.cnsClient.ReleaseIPs(context.TODO(), req); err != nil { - // if we fail a release with a 404 error try using the old API + // if ReleaseIPs call fails, we may receive API not Found error as new CNS is not supported, then try old API ReleaseIPAddress + log.Errorf("ReleaseIPs not supported by CNS. Invoking ReleaseIPAddress API. Request: %v", req) if errors.Is(err, cnscli.ErrAPINotFound) { ipconfigRequest := cns.IPConfigRequest{ OrchestratorContext: orchestratorContext, PodInterfaceID: GetEndpointID(args), InfraContainerID: args.ContainerID, - DesiredIPAddress: req.DesiredIPAddresses[0], } - log.Errorf("Failed to release IPs using ReleaseIPs from CNS, going to try ReleaseIPAddress. error: %v request: %v", err, req) if err = invoker.cnsClient.ReleaseIPAddress(context.TODO(), ipconfigRequest); err != nil { // if the old API fails as well then we just return the error diff --git a/cns/client/client.go b/cns/client/client.go index b01cac99e7..f7e41fba23 100644 --- a/cns/client/client.go +++ b/cns/client/client.go @@ -47,7 +47,7 @@ var clientPaths = []string{ cns.GetHomeAz, } -var errAPINotFound error = errors.New("api not found") +var ErrAPINotFound error = errors.New("api not found") type do interface { Do(*http.Request) (*http.Response, error) @@ -405,7 +405,7 @@ func (c *Client) RequestIPs(ctx context.Context, ipconfig cns.IPConfigsRequest) // if we get a 404 error if res.StatusCode == http.StatusNotFound { - return nil, fmt.Errorf("cannot find API RequestIPs %w: %v", errAPINotFound, err) //nolint:errorlint // multiple %w not supported in 1.19 + return nil, fmt.Errorf("cannot find API RequestIPs %w: %v", ErrAPINotFound, err) //nolint:errorlint // multiple %w not supported in 1.19 } if err != nil { @@ -448,7 +448,7 @@ func (c *Client) ReleaseIPs(ctx context.Context, ipconfig cns.IPConfigsRequest) // if we get a 404 error if res.StatusCode == http.StatusNotFound { - return fmt.Errorf("cannot find API ReleaseIPs %w: %v", errAPINotFound, err) //nolint:errorlint // multiple %w not supported in 1.19 + return fmt.Errorf("cannot find API ReleaseIPs %w: %v", ErrAPINotFound, err) //nolint:errorlint // multiple %w not supported in 1.19 } if err != nil { From 0cc0734cdb9041a5a59e77e28f23e128098bba1f Mon Sep 17 00:00:00 2001 From: paulyu Date: Wed, 5 Apr 2023 15:03:42 -0400 Subject: [PATCH 16/44] fix comments --- cni/network/invoker_azure.go | 2 +- cni/network/invoker_cns.go | 8 ++++---- cni/network/network.go | 2 +- network/network_windows.go | 2 +- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/cni/network/invoker_azure.go b/cni/network/invoker_azure.go index b40b972623..c3256b9c40 100644 --- a/cni/network/invoker_azure.go +++ b/cni/network/invoker_azure.go @@ -147,7 +147,7 @@ func (invoker *AzureIPAMInvoker) Delete(addresses []*net.IPNet, nwCfg *cni.Netwo } for _, address := range addresses { - if len(address.IP.To4()) == bytesSize4 { + if len(address.IP.To4()) == bytesSize4 { //nolint:gocritic nwCfg.IPAM.Address = address.IP.String() log.Printf("Releasing ipv4 address :%s pool: %s", nwCfg.IPAM.Address, nwCfg.IPAM.Subnet) if err := invoker.plugin.DelegateDel(nwCfg.IPAM.Type, nwCfg); err != nil { diff --git a/cni/network/invoker_cns.go b/cni/network/invoker_cns.go index c079f049a1..5a2927f647 100644 --- a/cni/network/invoker_cns.go +++ b/cni/network/invoker_cns.go @@ -90,8 +90,8 @@ func (invoker *CNSIPAMInvoker) Add(addConfig IPAMAddConfig) (IPAMAddResult, erro InfraContainerID: addConfig.args.ContainerID, } - res, err := invoker.cnsClient.RequestIPAddress(context.TODO(), ipconfigRequest) - if err != nil { + res, errRequestIP := invoker.cnsClient.RequestIPAddress(context.TODO(), ipconfigRequest) + if errRequestIP != nil { // if the old API fails as well then we just return the error log.Errorf("Failed to request IP address from CNS using RequestIPAddress. error: %v request: %v", err, ipconfigRequest) return IPAMAddResult{}, errors.Wrap(err, "Failed to get IP address from CNS with error: %w") @@ -138,7 +138,7 @@ func (invoker *CNSIPAMInvoker) Add(addConfig IPAMAddConfig) (IPAMAddResult, erro return IPAMAddResult{}, errors.Wrap(errInvalidArgs, "%w: Gateway address "+info.ncGatewayIPAddress+" from response is invalid") } - if net.ParseIP(info.podIPAddress).To4() != nil { + if net.ParseIP(info.podIPAddress).To4() != nil { //nolint:gocritic ncgw, err = getOverlayGateway(ncIPNet) if err != nil { return IPAMAddResult{}, err @@ -264,7 +264,7 @@ func setHostOptions(ncSubnetPrefix *net.IPNet, options map[string]interface{}, i } // Delete calls into the releaseipconfiguration API in CNS -func (invoker *CNSIPAMInvoker) Delete(addresses []*net.IPNet, nwCfg *cni.NetworkConfig, args *cniSkel.CmdArgs, _ map[string]interface{}) error { +func (invoker *CNSIPAMInvoker) Delete(addresses []*net.IPNet, nwCfg *cni.NetworkConfig, args *cniSkel.CmdArgs, _ map[string]interface{}) error { //nolint:gocritic // nwCfg will be used for future // Parse Pod arguments. podInfo := cns.KubernetesPodInfo{ PodName: invoker.podName, diff --git a/cni/network/network.go b/cni/network/network.go index 59b9996551..2fdcd246b4 100644 --- a/cni/network/network.go +++ b/cni/network/network.go @@ -773,7 +773,7 @@ func (plugin *NetPlugin) createEndpointInternal(opt *createEndpointInternalOpt) if opt.resultV6 != nil { // inject routes to linux pod - epInfo.IPV6Mode = DualStack + epInfo.IPV6Mode = network.DualStack for _, ipconfig := range opt.resultV6.IPs { epInfo.IPAddresses = append(epInfo.IPAddresses, ipconfig.Address) } diff --git a/network/network_windows.go b/network/network_windows.go index 60500922e7..ed1522bede 100644 --- a/network/network_windows.go +++ b/network/network_windows.go @@ -410,7 +410,7 @@ func (nm *networkManager) newNetworkImplHnsV2(nwInfo *NetworkInfo, extIf *extern } if util.DualStackOverlay == DualStack { - if err = nm.addNewNetRules(nwInfo); err != nil { + if err := nm.addNewNetRules(nwInfo); err != nil { return nil, err } } From 7030e8f901b2025c6ff6248fe184484f02889e5a Mon Sep 17 00:00:00 2001 From: paulyu Date: Wed, 5 Apr 2023 15:38:50 -0400 Subject: [PATCH 17/44] fix linter issues --- cni/network/invoker_azure.go | 2 +- cni/network/invoker_cns.go | 2 +- cni/network/invoker_mock.go | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/cni/network/invoker_azure.go b/cni/network/invoker_azure.go index c3256b9c40..bb616fa314 100644 --- a/cni/network/invoker_azure.go +++ b/cni/network/invoker_azure.go @@ -131,7 +131,7 @@ func (invoker *AzureIPAMInvoker) deleteIpamState() { } } -func (invoker *AzureIPAMInvoker) Delete(addresses []*net.IPNet, nwCfg *cni.NetworkConfig, _ *cniSkel.CmdArgs, options map[string]interface{}) error { +func (invoker *AzureIPAMInvoker) Delete(addresses []*net.IPNet, nwCfg *cni.NetworkConfig, _ *cniSkel.CmdArgs, options map[string]interface{}) error { //nolint if nwCfg == nil { return invoker.plugin.Errorf("nil nwCfg passed to CNI ADD, stack: %+v", string(debug.Stack())) } diff --git a/cni/network/invoker_cns.go b/cni/network/invoker_cns.go index 5a2927f647..3975942336 100644 --- a/cni/network/invoker_cns.go +++ b/cni/network/invoker_cns.go @@ -264,7 +264,7 @@ func setHostOptions(ncSubnetPrefix *net.IPNet, options map[string]interface{}, i } // Delete calls into the releaseipconfiguration API in CNS -func (invoker *CNSIPAMInvoker) Delete(addresses []*net.IPNet, nwCfg *cni.NetworkConfig, args *cniSkel.CmdArgs, _ map[string]interface{}) error { //nolint:gocritic // nwCfg will be used for future +func (invoker *CNSIPAMInvoker) Delete(addresses []*net.IPNet, nwCfg *cni.NetworkConfig, args *cniSkel.CmdArgs, _ map[string]interface{}) error { //nolint // Parse Pod arguments. podInfo := cns.KubernetesPodInfo{ PodName: invoker.podName, diff --git a/cni/network/invoker_mock.go b/cni/network/invoker_mock.go index b02dbdd43b..9300dc19c7 100644 --- a/cni/network/invoker_mock.go +++ b/cni/network/invoker_mock.go @@ -79,7 +79,7 @@ func (invoker *MockIpamInvoker) Add(opt IPAMAddConfig) (ipamAddResult IPAMAddRes return ipamAddResult, nil } -func (invoker *MockIpamInvoker) Delete(addresses []*net.IPNet, nwCfg *cni.NetworkConfig, _ *skel.CmdArgs, options map[string]interface{}) error { +func (invoker *MockIpamInvoker) Delete(addresses []*net.IPNet, nwCfg *cni.NetworkConfig, _ *skel.CmdArgs, options map[string]interface{}) error { //nolint if invoker.v4Fail || invoker.v6Fail { return errDeleteIpam } From 6d67c86b417958a659df18c3cc185aeff5390d1f Mon Sep 17 00:00:00 2001 From: paulyu Date: Fri, 7 Apr 2023 15:46:30 -0400 Subject: [PATCH 18/44] fix comments --- cni/network/invoker_azure.go | 10 +++++++++- cni/network/invoker_cns.go | 27 +++++++++++++-------------- cni/network/invoker_cns_test.go | 2 +- cni/network/network.go | 6 +++--- network/network.go | 2 +- network/network_windows.go | 2 +- 6 files changed, 28 insertions(+), 21 deletions(-) diff --git a/cni/network/invoker_azure.go b/cni/network/invoker_azure.go index bb616fa314..e2386e353d 100644 --- a/cni/network/invoker_azure.go +++ b/cni/network/invoker_azure.go @@ -75,7 +75,15 @@ func (invoker *AzureIPAMInvoker) Add(addConfig IPAMAddConfig) (IPAMAddResult, er addresses = append(addresses, &ip.Address) } if er := invoker.Delete(addresses, 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) + err = invoker.plugin.Errorf("Failed to clean up IPv4's during Delete with error %v, after Add failed with error %w", er, err) + } + } else if len(addResult.ipv6Result.IPs) > 0 { + ipv6Addresses := []*net.IPNet{} + for _, ip := range addResult.ipv6Result.IPs { + ipv6Addresses = append(ipv6Addresses, &ip.Address) + } + if er := invoker.Delete(ipv6Addresses, addConfig.nwCfg, nil, addConfig.options); er != nil { + err = invoker.plugin.Errorf("Failed to clean up IPv6's during Delete with error %v, after Add failed with error %w", er, err) } } else { err = fmt.Errorf("No IP's to delete on error: %v", err) diff --git a/cni/network/invoker_cns.go b/cni/network/invoker_cns.go index 3975942336..bf0485c77e 100644 --- a/cni/network/invoker_cns.go +++ b/cni/network/invoker_cns.go @@ -82,18 +82,18 @@ func (invoker *CNSIPAMInvoker) Add(addConfig IPAMAddConfig) (IPAMAddResult, erro response, err := invoker.cnsClient.RequestIPs(context.TODO(), ipconfig) if err != nil { // if RequestIPs call fails, we may receive API not Found error as new CNS is not supported, then try old API RequestIPAddress - log.Errorf("RequestIPs not supported by CNS. Invoking RequestIPAddress API. Request: %v", ipconfig) + log.Errorf("RequestIPs not supported by CNS. Invoking RequestIPAddress API with infracontainerid %s", ipconfig.InfraContainerID) if errors.Is(err, cnscli.ErrAPINotFound) { - ipconfigRequest := cns.IPConfigRequest{ + ipconfigs := cns.IPConfigRequest{ OrchestratorContext: orchestratorContext, PodInterfaceID: GetEndpointID(addConfig.args), InfraContainerID: addConfig.args.ContainerID, } - res, errRequestIP := invoker.cnsClient.RequestIPAddress(context.TODO(), ipconfigRequest) + res, errRequestIP := invoker.cnsClient.RequestIPAddress(context.TODO(), ipconfigs) if errRequestIP != nil { // if the old API fails as well then we just return the error - log.Errorf("Failed to request IP address from CNS using RequestIPAddress. error: %v request: %v", err, ipconfigRequest) + log.Errorf("Failed to request IP address from CNS using RequestIPAddress with infracontainerid %s. error: %v", ipconfigs.InfraContainerID, errRequestIP) return IPAMAddResult{}, errors.Wrap(err, "Failed to get IP address from CNS with error: %w") } response = &cns.IPConfigsResponse{ @@ -280,41 +280,40 @@ func (invoker *CNSIPAMInvoker) Delete(addresses []*net.IPNet, nwCfg *cni.Network return errEmptyCNIArgs } - req := cns.IPConfigsRequest{ + ipConfig := cns.IPConfigsRequest{ OrchestratorContext: orchestratorContext, PodInterfaceID: GetEndpointID(args), InfraContainerID: args.ContainerID, } if len(addresses) > 0 { - req.DesiredIPAddresses = make([]string, len(addresses)) + ipConfig.DesiredIPAddresses = make([]string, len(addresses)) for i, ipAddress := range addresses { - req.DesiredIPAddresses[i] = ipAddress.IP.String() + ipConfig.DesiredIPAddresses[i] = ipAddress.IP.String() } } else { log.Printf("CNS invoker called with empty IP address") } - if err := invoker.cnsClient.ReleaseIPs(context.TODO(), req); err != nil { + if err := invoker.cnsClient.ReleaseIPs(context.TODO(), ipConfig); err != nil { // if ReleaseIPs call fails, we may receive API not Found error as new CNS is not supported, then try old API ReleaseIPAddress - log.Errorf("ReleaseIPs not supported by CNS. Invoking ReleaseIPAddress API. Request: %v", req) + log.Errorf("ReleaseIPs not supported by CNS. Invoking ReleaseIPAddress API. Request: %v", ipConfig) if errors.Is(err, cnscli.ErrAPINotFound) { - ipconfigRequest := cns.IPConfigRequest{ + ipConfigs := cns.IPConfigRequest{ OrchestratorContext: orchestratorContext, PodInterfaceID: GetEndpointID(args), InfraContainerID: args.ContainerID, } - if err = invoker.cnsClient.ReleaseIPAddress(context.TODO(), ipconfigRequest); err != nil { + if err = invoker.cnsClient.ReleaseIPAddress(context.TODO(), ipConfigs); err != nil { // if the old API fails as well then we just return the error - log.Errorf("Failed to release IP address from CNS using ReleaseIPAddress. error: %v request: %v", err, req) + log.Errorf("Failed to release IP address from CNS using ReleaseIPAddress with infracontainerid %s. error: %v", ipConfigs.InfraContainerID, err) return errors.Wrap(err, fmt.Sprintf("failed to release IP %v using ReleaseIPAddress with err ", addresses)+"%w") } } else { - log.Errorf("Failed to release IP address from CNS error: %v request: %v", err, req) + log.Errorf("Failed to release IP address with infracontainerid %s from CNS error: %v", ipConfig.InfraContainerID, err) return errors.Wrap(err, fmt.Sprintf("failed to release IP %v using ReleaseIPs with err ", addresses)+"%w") } - } return nil diff --git a/cni/network/invoker_cns_test.go b/cni/network/invoker_cns_test.go index cc94e8aad3..af7e35ad85 100644 --- a/cni/network/invoker_cns_test.go +++ b/cni/network/invoker_cns_test.go @@ -135,7 +135,7 @@ func TestCNSIPAMInvoker_Add(t *testing.T) { wantErr: false, }, { - name: "Test happy CNI add", + name: "Test happy CNI add for dualstack mode", fields: fields{ podName: testPodInfo.PodName, podNamespace: testPodInfo.PodNamespace, diff --git a/cni/network/network.go b/cni/network/network.go index 2fdcd246b4..03c99fcb75 100644 --- a/cni/network/network.go +++ b/cni/network/network.go @@ -386,9 +386,9 @@ func (plugin *NetPlugin) Add(args *cniSkel.CmdArgs) error { } if ipamAddResult.ipv4Result.IPs != nil { - log.Printf("[cni-net] ADD command completed for pod %v with IPs:%+v err:%v.", k8sPodName, ipamAddResult.ipv4Result.IPs, err) + log.Printf("[cni-net] ADD command completed for pod %s with IPs:%v err:%v.", k8sPodName, ipamAddResult.ipv4Result.IPs, err) } else { - log.Printf("[cni-net] no IPs will be allocated for pod %v", k8sPodName) + log.Printf("[cni-net] no IPs will be allocated for pod %s", k8sPodName) } }() @@ -773,7 +773,7 @@ func (plugin *NetPlugin) createEndpointInternal(opt *createEndpointInternalOpt) if opt.resultV6 != nil { // inject routes to linux pod - epInfo.IPV6Mode = network.DualStack + epInfo.IPV6Mode = network.DualStackOverlay for _, ipconfig := range opt.resultV6.IPs { epInfo.IPAddresses = append(epInfo.IPAddresses, ipconfig.Address) } diff --git a/network/network.go b/network/network.go index 99796ac54f..7c2151fedf 100644 --- a/network/network.go +++ b/network/network.go @@ -26,7 +26,7 @@ const ( // ipv6 modes IPV6Nat = "ipv6nat" // dual stack mode - DualStack = "dualStackOverlay" + DualStackOverlay = "dualStackOverlay" ) // externalInterface is a host network interface that bridges containers to external networks. diff --git a/network/network_windows.go b/network/network_windows.go index ed1522bede..09128f37cd 100644 --- a/network/network_windows.go +++ b/network/network_windows.go @@ -409,7 +409,7 @@ func (nm *networkManager) newNetworkImplHnsV2(nwInfo *NetworkInfo, extIf *extern vlanid = (int)(vlanID) } - if util.DualStackOverlay == DualStack { + if util.DualStackOverlay == DualStackOverlay { if err := nm.addNewNetRules(nwInfo); err != nil { return nil, err } From f43501544376be24b57afe51057d6001f1510e70 Mon Sep 17 00:00:00 2001 From: paulyu Date: Fri, 7 Apr 2023 21:37:54 -0400 Subject: [PATCH 19/44] add Linux dropgz support --- Makefile | 8 ++++++++ ...flist => azure-linux-swift-overlay-dualstack.conflist} | 0 dropgz/build/linux.Dockerfile | 1 + 3 files changed, 9 insertions(+) rename cni/{azure-linux-dualstack-overlay.conflist => azure-linux-swift-overlay-dualstack.conflist} (100%) diff --git a/Makefile b/Makefile index 16070ac828..200c092ba4 100644 --- a/Makefile +++ b/Makefile @@ -65,6 +65,7 @@ CNI_MULTITENANCY_TRANSPARENT_VLAN_BUILD_DIR = $(BUILD_DIR)/cni-multitenancy-tran CNI_SWIFT_BUILD_DIR = $(BUILD_DIR)/cni-swift CNI_OVERLAY_BUILD_DIR = $(BUILD_DIR)/cni-overlay CNI_BAREMETAL_BUILD_DIR = $(BUILD_DIR)/cni-baremetal +CNI_DUALSTACK_BUILD_DIR = $(BUILD_DIR)/cni-dualstack CNS_BUILD_DIR = $(BUILD_DIR)/cns NPM_BUILD_DIR = $(BUILD_DIR)/npm TOOLS_DIR = $(REPO_ROOT)/build/tools @@ -94,6 +95,7 @@ CNI_MULTITENANCY_TRANSPARENT_VLAN_ARCHIVE_NAME = azure-vnet-cni-multitenancy-tra CNI_SWIFT_ARCHIVE_NAME = azure-vnet-cni-swift-$(GOOS)-$(GOARCH)-$(CNI_VERSION).$(ARCHIVE_EXT) CNI_OVERLAY_ARCHIVE_NAME = azure-vnet-cni-overlay-$(GOOS)-$(GOARCH)-$(CNI_VERSION).$(ARCHIVE_EXT) CNI_BAREMETAL_ARCHIVE_NAME = azure-vnet-cni-baremetal-$(GOOS)-$(GOARCH)-$(CNI_VERSION).$(ARCHIVE_EXT) +CNI_DUALSTACK_ARCHIVE_NAME = azure-vnet-cni-overlay-dualstack-$(GOOS)-$(GOARCH)-$(CNI_VERSION).$(ARCHIVE_EXT) CNM_ARCHIVE_NAME = azure-vnet-cnm-$(GOOS)-$(GOARCH)-$(ACN_VERSION).$(ARCHIVE_EXT) CNS_ARCHIVE_NAME = azure-cns-$(GOOS)-$(GOARCH)-$(CNS_VERSION).$(ARCHIVE_EXT) NPM_ARCHIVE_NAME = azure-npm-$(GOOS)-$(GOARCH)-$(NPM_VERSION).$(ARCHIVE_EXT) @@ -624,6 +626,12 @@ endif cp $(CNI_BUILD_DIR)/azure-vnet$(EXE_EXT) $(CNI_BUILD_DIR)/azure-vnet-ipam$(EXE_EXT) $(CNI_BUILD_DIR)/azure-vnet-telemetry$(EXE_EXT) $(CNI_OVERLAY_BUILD_DIR) cd $(CNI_OVERLAY_BUILD_DIR) && $(ARCHIVE_CMD) $(CNI_OVERLAY_ARCHIVE_NAME) azure-vnet$(EXE_EXT) azure-vnet-ipam$(EXE_EXT) azure-vnet-telemetry$(EXE_EXT) 10-azure.conflist azure-vnet-telemetry.config + $(MKDIR) $(CNI_DUALSTACK_BUILD_DIR) + cp cni/azure-$(GOOS)-swift-overlay-dualstack.conflist $(CNI_DUALSTACK_BUILD_DIR)/10-azure.conflist + cp telemetry/azure-vnet-telemetry.config $(CNI_DUALSTACK_BUILD_DIR)/azure-vnet-telemetry.config + cp $(CNI_BUILD_DIR)/azure-vnet$(EXE_EXT) $(CNI_BUILD_DIR)/azure-vnet-ipam$(EXE_EXT) $(CNI_BUILD_DIR)/azure-vnet-telemetry$(EXE_EXT) $(CNI_DUALSTACK_BUILD_DIR) + cd $(CNI_DUALSTACK_BUILD_DIR) && $(ARCHIVE_CMD) $(CNI_DUALSTACK_ARCHIVE_NAME) azure-vnet$(EXE_EXT) azure-vnet-ipam$(EXE_EXT) azure-vnet-telemetry$(EXE_EXT) 10-azure.conflist azure-vnet-telemetry.config + #baremetal mode is windows only (at least for now) ifeq ($(GOOS),windows) $(MKDIR) $(CNI_BAREMETAL_BUILD_DIR) diff --git a/cni/azure-linux-dualstack-overlay.conflist b/cni/azure-linux-swift-overlay-dualstack.conflist similarity index 100% rename from cni/azure-linux-dualstack-overlay.conflist rename to cni/azure-linux-swift-overlay-dualstack.conflist diff --git a/dropgz/build/linux.Dockerfile b/dropgz/build/linux.Dockerfile index 741ef6f65f..c39ac6a876 100644 --- a/dropgz/build/linux.Dockerfile +++ b/dropgz/build/linux.Dockerfile @@ -29,6 +29,7 @@ COPY --from=azure-ipam /azure-ipam/azure-ipam pkg/embed/fs COPY --from=azure-vnet /azure-container-networking/cni/azure-$OS.conflist pkg/embed/fs/azure.conflist COPY --from=azure-vnet /azure-container-networking/cni/azure-$OS-swift.conflist pkg/embed/fs/azure-swift.conflist COPY --from=azure-vnet /azure-container-networking/cni/azure-$OS-swift-overlay.conflist pkg/embed/fs/azure-swift-overlay.conflist +COPY --from=azure-vnet /azure-container-networking/cni/azure-$OS-swift-overlay-dualstack.conflist pkg/embed/fs/azure-swift-overlay-dualstack.conflist COPY --from=azure-vnet /azure-container-networking/azure-vnet pkg/embed/fs COPY --from=azure-vnet /azure-container-networking/azure-vnet-telemetry pkg/embed/fs COPY --from=azure-vnet /azure-container-networking/azure-vnet-ipam pkg/embed/fs From f5f30ece5fa034fad322c410aa85e9c142fb04c5 Mon Sep 17 00:00:00 2001 From: paulyu Date: Tue, 11 Apr 2023 11:52:43 -0400 Subject: [PATCH 20/44] fix TM's comments --- ...-windows-swift-overlay-dualstack.conflist} | 0 cni/network/invoker_azure.go | 21 ++++++++++++------- dropgz/build/cniTest.Dockerfile | 1 + 3 files changed, 15 insertions(+), 7 deletions(-) rename cni/{azure-windows-dualstack-overlay.conflist => azure-windows-swift-overlay-dualstack.conflist} (100%) diff --git a/cni/azure-windows-dualstack-overlay.conflist b/cni/azure-windows-swift-overlay-dualstack.conflist similarity index 100% rename from cni/azure-windows-dualstack-overlay.conflist rename to cni/azure-windows-swift-overlay-dualstack.conflist diff --git a/cni/network/invoker_azure.go b/cni/network/invoker_azure.go index e2386e353d..9b46e5e5ee 100644 --- a/cni/network/invoker_azure.go +++ b/cni/network/invoker_azure.go @@ -92,16 +92,19 @@ func (invoker *AzureIPAMInvoker) Add(addConfig IPAMAddConfig) (IPAMAddResult, er }() if addConfig.nwCfg.IPV6Mode != "" { - nwCfg6 := *addConfig.nwCfg - nwCfg6.IPAM.Environment = common.OptEnvironmentIPv6NodeIpam - nwCfg6.IPAM.Type = ipamV6 + nwCfgIpv6 := *addConfig.nwCfg + nwCfgIpv6.IPAM.Environment = common.OptEnvironmentIPv6NodeIpam + nwCfgIpv6.IPAM.Type = ipamV6 if len(invoker.nwInfo.Subnets) > 1 { - // ipv6 is the second subnet of the slice - nwCfg6.IPAM.Subnet = invoker.nwInfo.Subnets[1].Prefix.String() + for _, subnet := range invoker.nwInfo.Subnets { + if subnet.Prefix.IP.To16() != nil { + nwCfgIpv6.IPAM.Subnet = subnet.Prefix.String() + } + } } - addResult.ipv6Result, err = invoker.plugin.DelegateAdd(nwCfg6.IPAM.Type, &nwCfg6) + addResult.ipv6Result, err = invoker.plugin.DelegateAdd(nwCfgIpv6.IPAM.Type, &nwCfgIpv6) if err != nil { err = invoker.plugin.Errorf("Failed to allocate v6 pool: %v", err) } @@ -168,7 +171,11 @@ func (invoker *AzureIPAMInvoker) Delete(addresses []*net.IPNet, nwCfg *cni.Netwo nwCfgIpv6.IPAM.Type = ipamV6 nwCfgIpv6.IPAM.Address = address.IP.String() if len(invoker.nwInfo.Subnets) > 1 { - nwCfgIpv6.IPAM.Subnet = invoker.nwInfo.Subnets[1].Prefix.String() + for _, subnet := range invoker.nwInfo.Subnets { + if subnet.Prefix.IP.To16() != nil { + nwCfgIpv6.IPAM.Subnet = subnet.Prefix.String() + } + } } log.Printf("Releasing ipv6 address :%s pool: %s", nwCfgIpv6.IPAM.Address, nwCfgIpv6.IPAM.Subnet) diff --git a/dropgz/build/cniTest.Dockerfile b/dropgz/build/cniTest.Dockerfile index 0ee38de5ab..bad909886e 100644 --- a/dropgz/build/cniTest.Dockerfile +++ b/dropgz/build/cniTest.Dockerfile @@ -20,6 +20,7 @@ COPY --from=azure-ipam /azure-ipam/*.conflist pkg/embed/fs COPY --from=azure-ipam /azure-ipam/bin/* pkg/embed/fs COPY --from=azure-vnet /azure-container-networking/cni/azure-$OS-swift.conflist pkg/embed/fs/azure-swift.conflist COPY --from=azure-vnet /azure-container-networking/cni/azure-$OS-swift-overlay.conflist pkg/embed/fs/azure-swift-overlay.conflist +COPY --from=azure-vnet /azure-container-networking/cni/azure-$OS-swift-overlay-dualstack.conflist pkg/embed/fs/azure-swift-overlay-dualstack.conflist COPY --from=azure-vnet /azure-container-networking/bin/* pkg/embed/fs RUN cd pkg/embed/fs/ && sha256sum * > sum.txt RUN gzip --verbose --best --recursive pkg/embed/fs && for f in pkg/embed/fs/*.gz; do mv -- "$f" "${f%%.gz}"; done From fbc30c570fb4d5ea0865e0a66cfffeb17e5dc34e Mon Sep 17 00:00:00 2001 From: paulyu Date: Wed, 12 Apr 2023 10:22:13 -0400 Subject: [PATCH 21/44] fix addNewNetRules() --- network/network_windows.go | 30 ++++++++++++++++-------------- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/network/network_windows.go b/network/network_windows.go index 09128f37cd..c572063498 100644 --- a/network/network_windows.go +++ b/network/network_windows.go @@ -39,7 +39,7 @@ const ( // ipv6 route cmd routeCmd = "netsh interface ipv6 %s route \"%s\" \"%s\" \"%s\" store=persistent" // add ipv4 and ipv6 route rules to windows node - netRouteCmd = "netsh interface %s add route \"%s\" \"%s\" \"%s\" metric=%s" + netRouteCmd = "netsh interface %s add route \"%s\" \"%s\" \"%s\"" ) // Windows implementation of route. @@ -191,6 +191,8 @@ func (nm *networkManager) newNetworkImplHnsV1(nwInfo *NetworkInfo, extIf *extern } // add ipv4 and ipv6 routes (if dual stack mode) to windows Node +// in dualstack mode, pods are created from different subnets on different nodes, gateway has to be node ip if pods want to communicate with each other +// add routes to make node understand pod IPs come from different subnets and VFP will take decisions based on these routes to forward traffic and avoid Natting func (nm *networkManager) addNewNetRules(nwInfo *NetworkInfo) error { var ( err error @@ -213,33 +215,33 @@ func (nm *networkManager) addNewNetRules(nwInfo *NetworkInfo) error { return fmt.Errorf("[net] failed to parse prefix %s", prefix) } if ip.To4() != nil { - // netsh interface ipv4 add route $subnetV4 $hostInterfaceAlias "0.0.0.0" metric=270 - netshV4DefaultRoute := fmt.Sprintf(netRouteCmd, "ipv4", prefix, ifName, ipv4DefaultHop, "270") + // netsh interface ipv4 add route $subnetV4 $hostInterfaceAlias "0.0.0.0" + netshV4DefaultRoute := fmt.Sprintf(netRouteCmd, "ipv4", prefix, ifName, ipv4DefaultHop) if out, err = nm.plClient.ExecuteCommand(netshV4DefaultRoute); err != nil { - return fmt.Errorf("[net] Adding ipv4 default route failed: %v:%v", out, err) + log.Printf("[net] Adding ipv4 default route failed: %v:%v", out, err) } - // netsh interface ipv4 add route $subnetV4 $hostInterfaceAlias $gatewayV4 metric=300 - netshV4GatewayRoute := fmt.Sprintf(netRouteCmd, "ipv4", prefix, ifName, gateway, "300") + // netsh interface ipv4 add route $subnetV4 $hostInterfaceAlias $gatewayV4 + netshV4GatewayRoute := fmt.Sprintf(netRouteCmd, "ipv4", prefix, ifName, gateway) if out, err = nm.plClient.ExecuteCommand(netshV4GatewayRoute); err != nil { - return fmt.Errorf("[net] Adding ipv4 gateway route failed: %v:%v", out, err) + log.Printf("[net] Adding ipv4 gateway route failed: %v:%v", out, err) } } else { - // netsh interface ipv6 add route $subnetV6 $hostInterfaceAlias "::" metric=270 - netshV6DefaultRoute := fmt.Sprintf(netRouteCmd, "ipv6", prefix, ifName, ipv6DefaultHop, "270") + // netsh interface ipv6 add route $subnetV6 $hostInterfaceAlias "::" + netshV6DefaultRoute := fmt.Sprintf(netRouteCmd, "ipv6", prefix, ifName, ipv6DefaultHop) if out, err = nm.plClient.ExecuteCommand(netshV6DefaultRoute); err != nil { - return fmt.Errorf("[net] Adding ipv6 default route failed: %v:%v", out, err) + log.Printf("[net] Adding ipv6 default route failed: %v:%v", out, err) } - // netsh interface ipv6 add route $subnetV6 $hostInterfaceAlias $gatewayV6 metric=300 - netshV6GatewayRoute := fmt.Sprintf(netRouteCmd, "ipv6", prefix, ifName, gateway, "300") + // netsh interface ipv6 add route $subnetV6 $hostInterfaceAlias $gatewayV6 + netshV6GatewayRoute := fmt.Sprintf(netRouteCmd, "ipv6", prefix, ifName, gateway) if out, err = nm.plClient.ExecuteCommand(netshV6GatewayRoute); err != nil { - return fmt.Errorf("[net] Adding ipv6 gateway route failed: %v:%v", out, err) + log.Printf("[net] Adding ipv6 gateway route failed: %v:%v", out, err) } } } - return nil + return err } func (nm *networkManager) appIPV6RouteEntry(nwInfo *NetworkInfo) error { From 27f8b9441a68f144baa34314375c55715c40254a Mon Sep 17 00:00:00 2001 From: paulyu Date: Wed, 12 Apr 2023 17:29:33 -0400 Subject: [PATCH 22/44] fix Delete() signature --- cni/network/invoker.go | 2 +- cni/network/invoker_azure.go | 70 +++++++++++++------------------ cni/network/invoker_azure_test.go | 48 +++++---------------- cni/network/invoker_cns.go | 18 ++++---- cni/network/invoker_cns_test.go | 10 ++--- cni/network/invoker_mock.go | 12 +++--- cni/network/network.go | 24 ++++------- cni/network/network_windows.go | 7 +--- 8 files changed, 68 insertions(+), 123 deletions(-) diff --git a/cni/network/invoker.go b/cni/network/invoker.go index dc38a6492a..0d111933f8 100644 --- a/cni/network/invoker.go +++ b/cni/network/invoker.go @@ -17,7 +17,7 @@ type IPAMInvoker interface { Add(IPAMAddConfig) (IPAMAddResult, error) // Delete calls to the invoker source, and returns error. Returning an error here will fail the CNI Delete call. - Delete(addresses []*net.IPNet, nwCfg *cni.NetworkConfig, args *cniSkel.CmdArgs, options map[string]interface{}) error + Delete(address *net.IPNet, nwCfg *cni.NetworkConfig, args *cniSkel.CmdArgs, options map[string]interface{}) error } type IPAMAddConfig struct { diff --git a/cni/network/invoker_azure.go b/cni/network/invoker_azure.go index 9b46e5e5ee..7a069c6517 100644 --- a/cni/network/invoker_azure.go +++ b/cni/network/invoker_azure.go @@ -70,20 +70,12 @@ func (invoker *AzureIPAMInvoker) Add(addConfig IPAMAddConfig) (IPAMAddResult, er defer func() { if err != nil { if len(addResult.ipv4Result.IPs) > 0 { - addresses := []*net.IPNet{} - for _, ip := range addResult.ipv4Result.IPs { - addresses = append(addresses, &ip.Address) - } - if er := invoker.Delete(addresses, addConfig.nwCfg, nil, addConfig.options); er != nil { - err = invoker.plugin.Errorf("Failed to clean up IPv4's during Delete with error %v, after Add failed with error %w", er, err) + if er := invoker.Delete(&addResult.ipv4Result.IPs[0].Address, addConfig.nwCfg, nil, addConfig.options); er != nil { + err = invoker.plugin.Errorf("Failed to clean up IPv4 during Delete with error %v, after Add failed with error %w", er, err) } } else if len(addResult.ipv6Result.IPs) > 0 { - ipv6Addresses := []*net.IPNet{} - for _, ip := range addResult.ipv6Result.IPs { - ipv6Addresses = append(ipv6Addresses, &ip.Address) - } - if er := invoker.Delete(ipv6Addresses, addConfig.nwCfg, nil, addConfig.options); er != nil { - err = invoker.plugin.Errorf("Failed to clean up IPv6's during Delete with error %v, after Add failed with error %w", er, err) + if er := invoker.Delete(&addResult.ipv6Result.IPs[0].Address, addConfig.nwCfg, nil, addConfig.options); er != nil { + err = invoker.plugin.Errorf("Failed to clean up IPv6 during Delete with error %v, after Add failed with error %w", er, err) } } else { err = fmt.Errorf("No IP's to delete on error: %v", err) @@ -142,7 +134,7 @@ func (invoker *AzureIPAMInvoker) deleteIpamState() { } } -func (invoker *AzureIPAMInvoker) Delete(addresses []*net.IPNet, nwCfg *cni.NetworkConfig, _ *cniSkel.CmdArgs, options map[string]interface{}) error { //nolint +func (invoker *AzureIPAMInvoker) Delete(address *net.IPNet, nwCfg *cni.NetworkConfig, _ *cniSkel.CmdArgs, options map[string]interface{}) error { //nolint if nwCfg == nil { return invoker.plugin.Errorf("nil nwCfg passed to CNI ADD, stack: %+v", string(debug.Stack())) } @@ -151,41 +143,37 @@ func (invoker *AzureIPAMInvoker) Delete(addresses []*net.IPNet, nwCfg *cni.Netwo nwCfg.IPAM.Subnet = invoker.nwInfo.Subnets[0].Prefix.String() } - if len(addresses) == 0 { + if address == nil { if err := invoker.plugin.DelegateDel(nwCfg.IPAM.Type, nwCfg); err != nil { return invoker.plugin.Errorf("Attempted to release address with error: %v", err) } - } - - for _, address := range addresses { - if len(address.IP.To4()) == bytesSize4 { //nolint:gocritic - nwCfg.IPAM.Address = address.IP.String() - log.Printf("Releasing ipv4 address :%s pool: %s", nwCfg.IPAM.Address, nwCfg.IPAM.Subnet) - if err := invoker.plugin.DelegateDel(nwCfg.IPAM.Type, nwCfg); err != nil { - log.Printf("Failed to release ipv4 address: %v", err) - return invoker.plugin.Errorf("Failed to release ipv4 address: %v with error: ", nwCfg.IPAM.Address, err) - } - } else if len(address.IP.To16()) == bytesSize16 { - nwCfgIpv6 := *nwCfg - nwCfgIpv6.IPAM.Environment = common.OptEnvironmentIPv6NodeIpam - nwCfgIpv6.IPAM.Type = ipamV6 - nwCfgIpv6.IPAM.Address = address.IP.String() - if len(invoker.nwInfo.Subnets) > 1 { - for _, subnet := range invoker.nwInfo.Subnets { - if subnet.Prefix.IP.To16() != nil { - nwCfgIpv6.IPAM.Subnet = subnet.Prefix.String() - } + } else if len(address.IP.To4()) == bytesSize4 { //nolint:gocritic + nwCfg.IPAM.Address = address.IP.String() + log.Printf("Releasing ipv4 address :%s pool: %s", nwCfg.IPAM.Address, nwCfg.IPAM.Subnet) + if err := invoker.plugin.DelegateDel(nwCfg.IPAM.Type, nwCfg); err != nil { + log.Printf("Failed to release ipv4 address: %v", err) + return invoker.plugin.Errorf("Failed to release ipv4 address: %v with error: ", nwCfg.IPAM.Address, err) + } + } else if len(address.IP.To16()) == bytesSize16 { + nwCfgIpv6 := *nwCfg + nwCfgIpv6.IPAM.Environment = common.OptEnvironmentIPv6NodeIpam + nwCfgIpv6.IPAM.Type = ipamV6 + nwCfgIpv6.IPAM.Address = address.IP.String() + if len(invoker.nwInfo.Subnets) > 1 { + for _, subnet := range invoker.nwInfo.Subnets { + if subnet.Prefix.IP.To16() != nil { + nwCfgIpv6.IPAM.Subnet = subnet.Prefix.String() } } + } - log.Printf("Releasing ipv6 address :%s pool: %s", nwCfgIpv6.IPAM.Address, nwCfgIpv6.IPAM.Subnet) - if err := invoker.plugin.DelegateDel(nwCfgIpv6.IPAM.Type, &nwCfgIpv6); err != nil { - log.Printf("Failed to release ipv6 address: %v", err) - return invoker.plugin.Errorf("Failed to release ipv6 address: %v", err) - } - } else { - return invoker.plugin.Errorf("Address is incorrect, not valid IPv4 or IPv6, stack: %+v", string(debug.Stack())) + log.Printf("Releasing ipv6 address :%s pool: %s", nwCfgIpv6.IPAM.Address, nwCfgIpv6.IPAM.Subnet) + if err := invoker.plugin.DelegateDel(nwCfgIpv6.IPAM.Type, &nwCfgIpv6); err != nil { + log.Printf("Failed to release ipv6 address: %v", err) + return invoker.plugin.Errorf("Failed to release ipv6 address: %v", err) } + } else { + return invoker.plugin.Errorf("Address is incorrect, not valid IPv4 or IPv6, stack: %+v", string(debug.Stack())) } return nil diff --git a/cni/network/invoker_azure_test.go b/cni/network/invoker_azure_test.go index 8584e63250..f1ddd5b7d2 100644 --- a/cni/network/invoker_azure_test.go +++ b/cni/network/invoker_azure_test.go @@ -235,10 +235,10 @@ func TestAzureIPAMInvoker_Delete(t *testing.T) { nwInfo *network.NetworkInfo } type args struct { - addresses []*net.IPNet - nwCfg *cni.NetworkConfig - in2 *cniSkel.CmdArgs - options map[string]interface{} + address *net.IPNet + nwCfg *cni.NetworkConfig + in2 *cniSkel.CmdArgs + options map[string]interface{} } tests := []struct { name string @@ -255,9 +255,7 @@ func TestAzureIPAMInvoker_Delete(t *testing.T) { nwInfo: getNwInfo("10.0.0.0/24", ""), }, args: args{ - addresses: []*net.IPNet{ - getCIDRNotationForAddress("10.0.0.4/24"), - }, + address: getCIDRNotationForAddress("10.0.0.4/24"), nwCfg: &cni.NetworkConfig{ IPAM: cni.IPAM{ Address: "10.0.0.4", @@ -274,29 +272,7 @@ func TestAzureIPAMInvoker_Delete(t *testing.T) { nwInfo: getNwInfo("10.0.0.0/24", "2001:db8:abcd:0012::0/64"), }, args: args{ - addresses: []*net.IPNet{ - getCIDRNotationForAddress("2001:db8:abcd:0015::0/64"), - }, - nwCfg: &cni.NetworkConfig{ - IPAM: cni.IPAM{ - Address: "2001:db8:abcd:0015::0/64", - }, - }, - }, - }, - { - name: "delete happy path ipv4+ipv6", - fields: fields{ - plugin: &mockDelegatePlugin{ - del: del{}, - }, - nwInfo: getNwInfo("10.0.0.0/24", "2001:db8:abcd:0012::0/64"), - }, - args: args{ - addresses: []*net.IPNet{ - getCIDRNotationForAddress("10.0.0.4/24"), - getCIDRNotationForAddress("2001:db8:abcd:0015::0/64"), - }, + address: getCIDRNotationForAddress("2001:db8:abcd:0015::0/64"), nwCfg: &cni.NetworkConfig{ IPAM: cni.IPAM{ Address: "2001:db8:abcd:0015::0/64", @@ -315,7 +291,7 @@ func TestAzureIPAMInvoker_Delete(t *testing.T) { nwInfo: getNwInfo("", "2001:db8:abcd:0012::0/64"), }, args: args{ - addresses: nil, + address: nil, nwCfg: &cni.NetworkConfig{ IPAM: cni.IPAM{ Address: "2001:db8:abcd:0015::0/64", @@ -335,9 +311,7 @@ func TestAzureIPAMInvoker_Delete(t *testing.T) { nwInfo: getNwInfo("10.0.0.0/24", ""), }, args: args{ - addresses: []*net.IPNet{ - getCIDRNotationForAddress("10.0.0.4/24"), - }, + address: getCIDRNotationForAddress("10.0.0.4/24"), nwCfg: &cni.NetworkConfig{ IPAM: cni.IPAM{ Address: "10.0.0.4/24", @@ -357,9 +331,7 @@ func TestAzureIPAMInvoker_Delete(t *testing.T) { nwInfo: getNwInfo("10.0.0.0/24", "2001:db8:abcd:0012::0/64"), }, args: args{ - addresses: []*net.IPNet{ - getCIDRNotationForAddress("2001:db8:abcd:0015::0/64"), - }, + address: getCIDRNotationForAddress("2001:db8:abcd:0015::0/64"), nwCfg: &cni.NetworkConfig{ IPAM: cni.IPAM{ Address: "10.0.0.4/24", @@ -377,7 +349,7 @@ func TestAzureIPAMInvoker_Delete(t *testing.T) { plugin: tt.fields.plugin, nwInfo: tt.fields.nwInfo, } - err := invoker.Delete(tt.args.addresses, tt.args.nwCfg, tt.args.in2, tt.args.options) + err := invoker.Delete(tt.args.address, tt.args.nwCfg, tt.args.in2, tt.args.options) if tt.wantErr { require.NotNil(err) return diff --git a/cni/network/invoker_cns.go b/cni/network/invoker_cns.go index bf0485c77e..67d0511527 100644 --- a/cni/network/invoker_cns.go +++ b/cni/network/invoker_cns.go @@ -122,7 +122,10 @@ func (invoker *CNSIPAMInvoker) Add(addConfig IPAMAddConfig) (IPAMAddResult, erro } // set the NC Primary IP in options - addConfig.options[network.SNATIPKey] = info.ncPrimaryIP + // SNATIPKey set is not for ipv6 + if net.ParseIP(info.ncPrimaryIP).To4() != nil { + addConfig.options[network.SNATIPKey] = info.ncPrimaryIP + } log.Printf("[cni-invoker-cns] Received info %+v for pod %v", info, podInfo) @@ -264,7 +267,7 @@ func setHostOptions(ncSubnetPrefix *net.IPNet, options map[string]interface{}, i } // Delete calls into the releaseipconfiguration API in CNS -func (invoker *CNSIPAMInvoker) Delete(addresses []*net.IPNet, nwCfg *cni.NetworkConfig, args *cniSkel.CmdArgs, _ map[string]interface{}) error { //nolint +func (invoker *CNSIPAMInvoker) Delete(address *net.IPNet, nwCfg *cni.NetworkConfig, args *cniSkel.CmdArgs, _ map[string]interface{}) error { //nolint // Parse Pod arguments. podInfo := cns.KubernetesPodInfo{ PodName: invoker.podName, @@ -286,11 +289,8 @@ func (invoker *CNSIPAMInvoker) Delete(addresses []*net.IPNet, nwCfg *cni.Network InfraContainerID: args.ContainerID, } - if len(addresses) > 0 { - ipConfig.DesiredIPAddresses = make([]string, len(addresses)) - for i, ipAddress := range addresses { - ipConfig.DesiredIPAddresses[i] = ipAddress.IP.String() - } + if address != nil { + ipConfig.DesiredIPAddresses = append(ipConfig.DesiredIPAddresses, address.IP.String()) } else { log.Printf("CNS invoker called with empty IP address") } @@ -308,11 +308,11 @@ func (invoker *CNSIPAMInvoker) Delete(addresses []*net.IPNet, nwCfg *cni.Network if err = invoker.cnsClient.ReleaseIPAddress(context.TODO(), ipConfigs); err != nil { // if the old API fails as well then we just return the error log.Errorf("Failed to release IP address from CNS using ReleaseIPAddress with infracontainerid %s. error: %v", ipConfigs.InfraContainerID, err) - return errors.Wrap(err, fmt.Sprintf("failed to release IP %v using ReleaseIPAddress with err ", addresses)+"%w") + return errors.Wrap(err, fmt.Sprintf("failed to release IP %v using ReleaseIPAddress with err ", ipConfigs.DesiredIPAddress)+"%w") } } else { log.Errorf("Failed to release IP address with infracontainerid %s from CNS error: %v", ipConfig.InfraContainerID, err) - return errors.Wrap(err, fmt.Sprintf("failed to release IP %v using ReleaseIPs with err ", addresses)+"%w") + return errors.Wrap(err, fmt.Sprintf("failed to release IP %v using ReleaseIPs with err ", ipConfig.DesiredIPAddresses)+"%w") } } diff --git a/cni/network/invoker_cns_test.go b/cni/network/invoker_cns_test.go index af7e35ad85..e079b30d39 100644 --- a/cni/network/invoker_cns_test.go +++ b/cni/network/invoker_cns_test.go @@ -283,10 +283,10 @@ func TestCNSIPAMInvoker_Delete(t *testing.T) { cnsClient cnsclient } type args struct { - addresses []*net.IPNet - nwCfg *cni.NetworkConfig - args *cniSkel.CmdArgs - options map[string]interface{} + address *net.IPNet + nwCfg *cni.NetworkConfig + args *cniSkel.CmdArgs + options map[string]interface{} } tests := []struct { name string @@ -339,7 +339,7 @@ func TestCNSIPAMInvoker_Delete(t *testing.T) { podNamespace: tt.fields.podNamespace, cnsClient: tt.fields.cnsClient, } - err := invoker.Delete(tt.args.addresses, tt.args.nwCfg, tt.args.args, tt.args.options) + err := invoker.Delete(tt.args.address, tt.args.nwCfg, tt.args.args, tt.args.options) if tt.wantErr { require.Error(err) } else { diff --git a/cni/network/invoker_mock.go b/cni/network/invoker_mock.go index 9300dc19c7..917e531a64 100644 --- a/cni/network/invoker_mock.go +++ b/cni/network/invoker_mock.go @@ -79,20 +79,18 @@ func (invoker *MockIpamInvoker) Add(opt IPAMAddConfig) (ipamAddResult IPAMAddRes return ipamAddResult, nil } -func (invoker *MockIpamInvoker) Delete(addresses []*net.IPNet, nwCfg *cni.NetworkConfig, _ *skel.CmdArgs, options map[string]interface{}) error { //nolint +func (invoker *MockIpamInvoker) Delete(address *net.IPNet, nwCfg *cni.NetworkConfig, _ *skel.CmdArgs, options map[string]interface{}) error { //nolint if invoker.v4Fail || invoker.v6Fail { return errDeleteIpam } - if len(addresses) == 0 || invoker.ipMap == nil { + if address == nil || invoker.ipMap == nil { return nil } - for _, address := range addresses { - if _, ok := invoker.ipMap[address.String()]; !ok { - return errDeleteIpam - } - delete(invoker.ipMap, address.String()) + if _, ok := invoker.ipMap[address.String()]; !ok { + return errDeleteIpam } + delete(invoker.ipMap, address.String()) return nil } diff --git a/cni/network/network.go b/cni/network/network.go index 03c99fcb75..73af262775 100644 --- a/cni/network/network.go +++ b/cni/network/network.go @@ -583,20 +583,12 @@ func (plugin *NetPlugin) cleanupAllocationOnError( options map[string]interface{}, ) { if result != nil && len(result.IPs) > 0 { - addresses := make([]*net.IPNet, len(result.IPs)) - for i, ip := range result.IPs { - addresses[i] = &ip.Address - } - if er := plugin.ipamInvoker.Delete(addresses, nwCfg, args, options); er != nil { + if er := plugin.ipamInvoker.Delete(&result.IPs[0].Address, nwCfg, args, options); er != nil { log.Errorf("Failed to cleanup ip allocation on failure: %v", er) } } if resultV6 != nil && len(resultV6.IPs) > 0 { - addressesV6 := make([]*net.IPNet, len(resultV6.IPs)) - for i, ip := range resultV6.IPs { - addressesV6[i] = &ip.Address - } - if er := plugin.ipamInvoker.Delete(addressesV6, nwCfg, args, options); er != nil { + if er := plugin.ipamInvoker.Delete(&resultV6.IPs[0].Address, nwCfg, args, options); er != nil { log.Errorf("Failed to cleanup ipv6 allocation on failure: %v", er) } } @@ -1047,14 +1039,12 @@ func (plugin *NetPlugin) Delete(args *cniSkel.CmdArgs) error { if !nwCfg.MultiTenancy { // Call into IPAM plugin to release the endpoint's addresses. - addresses := make([]*net.IPNet, len(epInfo.IPAddresses)) - for i := range epInfo.IPAddresses { - addresses[i] = &epInfo.IPAddresses[i] - } logAndSendEvent(plugin, fmt.Sprintf("Releasing ips:%+v", epInfo.IPAddresses)) - err = plugin.ipamInvoker.Delete(addresses, nwCfg, args, nwInfo.Options) - if err != nil { - return plugin.RetriableError(fmt.Errorf("failed to release address: %w", err)) + for i := range epInfo.IPAddresses { + err = plugin.ipamInvoker.Delete(&epInfo.IPAddresses[i], nwCfg, args, nwInfo.Options) + if err != nil { + return plugin.RetriableError(fmt.Errorf("failed to release address: %w", err)) + } } } else if epInfo.EnableInfraVnet { nwCfg.IPAM.Subnet = nwInfo.Subnets[0].Prefix.String() diff --git a/cni/network/network_windows.go b/cni/network/network_windows.go index eeabacc005..a0de632838 100644 --- a/cni/network/network_windows.go +++ b/cni/network/network_windows.go @@ -379,16 +379,13 @@ func determineWinVer() { } func getNATInfo(nwCfg *cni.NetworkConfig, ncPrimaryIPIface interface{}, enableSnatForDNS bool) (natInfo []policy.NATInfo) { - if nwCfg.ExecutionMode == string(util.V4Swift) && nwCfg.IPAM.Mode != string(util.V4Overlay) { + if nwCfg.ExecutionMode == string(util.V4Swift) && nwCfg.IPAM.Mode != string(util.V4Overlay) && nwCfg.IPAM.Mode != string(util.DualStackOverlay) { ncPrimaryIP := "" if ncPrimaryIPIface != nil { ncPrimaryIP = ncPrimaryIPIface.(string) } - // only append natInfo if ncPrimaryIP is ipv4 - if net.ParseIP(ncPrimaryIP).To4() != nil { - natInfo = append(natInfo, []policy.NATInfo{{VirtualIP: ncPrimaryIP, Destinations: []string{networkutils.AzureDNS}}, {Destinations: []string{networkutils.AzureIMDS}}}...) - } + natInfo = append(natInfo, []policy.NATInfo{{VirtualIP: ncPrimaryIP, Destinations: []string{networkutils.AzureDNS}}, {Destinations: []string{networkutils.AzureIMDS}}}...) } else if nwCfg.MultiTenancy && enableSnatForDNS { natInfo = append(natInfo, policy.NATInfo{Destinations: []string{networkutils.AzureDNS}}) } From 25adfeb935414c2ddccd82653f6c8b85e4869a19 Mon Sep 17 00:00:00 2001 From: paulyu Date: Wed, 12 Apr 2023 23:04:13 -0400 Subject: [PATCH 23/44] add UTs --- cni/network/multitenancy_test.go | 45 +++++++++++++++++++++++++++++--- cns/client/client.go | 13 +++++---- 2 files changed, 49 insertions(+), 9 deletions(-) diff --git a/cni/network/multitenancy_test.go b/cni/network/multitenancy_test.go index c9facdd0c0..596453e537 100644 --- a/cni/network/multitenancy_test.go +++ b/cni/network/multitenancy_test.go @@ -11,6 +11,7 @@ import ( "github.com/Azure/azure-container-networking/network" cniTypes "github.com/containernetworking/cni/pkg/types" cniTypesCurr "github.com/containernetworking/cni/pkg/types/100" + "github.com/google/go-cmp/cmp" "github.com/stretchr/testify/require" ) @@ -51,7 +52,21 @@ type getAllNetworkContainersConfigurationHandler struct { err error } +type cnsAPIName string + +const ( + GetAllNetworkContainers cnsAPIName = "GetAllNetworkContainers" +) + +var ( + errUnsupportedAPI = errors.New("Unsupported API") + errNoOrchestratorContextFound = errors.New("No CNI OrchestratorContext Found") + errNoRequestIPFound = errors.New("No Request IP Found") + errNoReleaseIPFound = errors.New("No Release IP Found") +) + type MockCNSClient struct { + unsupportedAPIs map[cnsAPIName]struct{} require *require.Assertions request requestIPAddressHandler requestIPs requestIPsHandler @@ -61,22 +76,44 @@ type MockCNSClient struct { } func (c *MockCNSClient) RequestIPAddress(_ context.Context, ipconfig cns.IPConfigRequest) (*cns.IPConfigResponse, error) { - c.require.Exactly(c.request.ipconfigArgument, ipconfig) + if !cmp.Equal(c.request.ipconfigArgument, ipconfig) { + return nil, errNoRequestIPFound + } return c.request.result, c.request.err } func (c *MockCNSClient) RequestIPs(_ context.Context, ipconfig cns.IPConfigsRequest) (*cns.IPConfigsResponse, error) { - c.require.Exactly(c.requestIPs.ipconfigArgument, ipconfig) + if _, isUnsupported := c.unsupportedAPIs[RequestIPs]; isUnsupported { + e := &client.CNSClientError{} + e.Code = types.UnsupportedAPI + e.Err = errUnsupportedAPI + return nil, e + } + + if !cmp.Equal(c.requestIPs.ipconfigArgument, ipconfig) { + return nil, errUnsupportedAPI + } return c.requestIPs.result, c.requestIPs.err } func (c *MockCNSClient) ReleaseIPAddress(_ context.Context, ipconfig cns.IPConfigRequest) error { - c.require.Exactly(c.release.ipconfigArgument, ipconfig) + if !cmp.Equal(c.release.ipconfigArgument, ipconfig) { + return errNoReleaseIPFound + } return c.release.err } func (c *MockCNSClient) ReleaseIPs(_ context.Context, ipconfig cns.IPConfigsRequest) error { - c.require.Exactly(c.release.ipconfigArgument, ipconfig) + if _, isUnsupported := c.unsupportedAPIs[ReleaseIPs]; isUnsupported { + e := &client.CNSClientError{} + e.Code = types.UnsupportedAPI + e.Err = errUnsupportedAPI + return nil, e + } + + if !cmp.Equal(c.requestIPs.ipconfigArgument, ipconfig) { + return errUnsupportedAPI + } return c.release.err } diff --git a/cns/client/client.go b/cns/client/client.go index f7e41fba23..e12490a843 100644 --- a/cns/client/client.go +++ b/cns/client/client.go @@ -47,8 +47,6 @@ var clientPaths = []string{ cns.GetHomeAz, } -var ErrAPINotFound error = errors.New("api not found") - type do interface { Do(*http.Request) (*http.Response, error) } @@ -403,9 +401,11 @@ func (c *Client) RequestIPs(ctx context.Context, ipconfig cns.IPConfigsRequest) req.Header.Set(headerContentType, contentTypeJSON) res, err := c.client.Do(req) - // if we get a 404 error if res.StatusCode == http.StatusNotFound { - return nil, fmt.Errorf("cannot find API RequestIPs %w: %v", ErrAPINotFound, err) //nolint:errorlint // multiple %w not supported in 1.19 + return nil, &CNSClientError{ + Code: types.UnsupportedAPI, + Err: errors.Errorf("Unsupported API"), + } } if err != nil { @@ -448,7 +448,10 @@ func (c *Client) ReleaseIPs(ctx context.Context, ipconfig cns.IPConfigsRequest) // if we get a 404 error if res.StatusCode == http.StatusNotFound { - return fmt.Errorf("cannot find API ReleaseIPs %w: %v", ErrAPINotFound, err) //nolint:errorlint // multiple %w not supported in 1.19 + return &CNSClientError{ + Code: types.UnsupportedAPI, + Err: errors.Errorf("Unsupported API"), + } } if err != nil { From 4d942028f93f27d1fb3f8ac1353a255a9876a6d4 Mon Sep 17 00:00:00 2001 From: paulyu Date: Thu, 13 Apr 2023 11:51:21 -0400 Subject: [PATCH 24/44] add UTs --- cni/network/invoker_cns.go | 4 +- cni/network/invoker_cns_test.go | 232 +++++++++++++++++++++++++++++++ cni/network/multitenancy_test.go | 22 ++- 3 files changed, 249 insertions(+), 9 deletions(-) diff --git a/cni/network/invoker_cns.go b/cni/network/invoker_cns.go index 67d0511527..0dda7ec3e2 100644 --- a/cni/network/invoker_cns.go +++ b/cni/network/invoker_cns.go @@ -83,7 +83,7 @@ func (invoker *CNSIPAMInvoker) Add(addConfig IPAMAddConfig) (IPAMAddResult, erro if err != nil { // if RequestIPs call fails, we may receive API not Found error as new CNS is not supported, then try old API RequestIPAddress log.Errorf("RequestIPs not supported by CNS. Invoking RequestIPAddress API with infracontainerid %s", ipconfig.InfraContainerID) - if errors.Is(err, cnscli.ErrAPINotFound) { + if cnscli.IsUnsupportedAPI(err) { ipconfigs := cns.IPConfigRequest{ OrchestratorContext: orchestratorContext, PodInterfaceID: GetEndpointID(addConfig.args), @@ -298,7 +298,7 @@ func (invoker *CNSIPAMInvoker) Delete(address *net.IPNet, nwCfg *cni.NetworkConf if err := invoker.cnsClient.ReleaseIPs(context.TODO(), ipConfig); err != nil { // if ReleaseIPs call fails, we may receive API not Found error as new CNS is not supported, then try old API ReleaseIPAddress log.Errorf("ReleaseIPs not supported by CNS. Invoking ReleaseIPAddress API. Request: %v", ipConfig) - if errors.Is(err, cnscli.ErrAPINotFound) { + if cnscli.IsUnsupportedAPI(err) { ipConfigs := cns.IPConfigRequest{ OrchestratorContext: orchestratorContext, PodInterfaceID: GetEndpointID(args), diff --git a/cni/network/invoker_cns_test.go b/cni/network/invoker_cns_test.go index e079b30d39..45f15468ca 100644 --- a/cni/network/invoker_cns_test.go +++ b/cni/network/invoker_cns_test.go @@ -275,6 +275,238 @@ func TestCNSIPAMInvoker_Add(t *testing.T) { } } +func TestCNSIPAMInvoker_Add_UnsupportedAPI(t *testing.T) { + require := require.New(t) //nolint further usage of require without passing t + + // set new CNS API is not supported + unsupportedAPIs := make(map[cnsAPIName]struct{}) + unsupportedAPIs["RequestIPs"] = struct{}{} + + type fields struct { + podName string + podNamespace string + cnsClient cnsclient + ipamMode util.IpamMode + } + type args struct { + nwCfg *cni.NetworkConfig + args *cniSkel.CmdArgs + hostSubnetPrefix *net.IPNet + options map[string]interface{} + } + + tests := []struct { + name string + fields fields + args args + want *cniTypesCurr.Result + want1 *cniTypesCurr.Result + wantErr bool + }{ + { + name: "Test happy CNI add for IPv4 without RequestIPs supported", + fields: fields{ + podName: testPodInfo.PodName, + podNamespace: testPodInfo.PodNamespace, + cnsClient: &MockCNSClient{ + unsupportedAPIs: unsupportedAPIs, + require: require, + requestIP: requestIPAddressHandler{ + ipconfigArgument: getTestIPConfigRequest(), + result: &cns.IPConfigResponse{ + 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", + }, + }, + Response: cns.Response{ + ReturnCode: 0, + Message: "", + }, + }, + err: nil, + }, + }, + }, + args: args{ + nwCfg: &cni.NetworkConfig{}, + args: &cniSkel.CmdArgs{ + ContainerID: "testcontainerid", + Netns: "testnetns", + IfName: "testifname", + }, + hostSubnetPrefix: getCIDRNotationForAddress("10.0.0.1/24"), + options: map[string]interface{}{}, + }, + want: &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"), + }, + }, + }, + wantErr: true, + }, + } + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + invoker := &CNSIPAMInvoker{ + podName: tt.fields.podName, + podNamespace: tt.fields.podNamespace, + cnsClient: tt.fields.cnsClient, + } + if tt.fields.ipamMode != "" { + invoker.ipamMode = tt.fields.ipamMode + } + ipamAddResult, err := invoker.Add(IPAMAddConfig{nwCfg: tt.args.nwCfg, args: tt.args.args, options: tt.args.options}) + if err != nil && tt.wantErr { + t.Fatalf("expected an error %+v but none received", err) + } + require.NoError(err) + require.Equalf(tt.want, ipamAddResult.ipv4Result, "incorrect ipv4 response") + }) + } +} + +func TestCNSIPAMInvoker_Add_NotFound(t *testing.T) { + require := require.New(t) //nolint further usage of require without passing t + + type fields struct { + podName string + podNamespace string + cnsClient cnsclient + ipamMode util.IpamMode + } + type args struct { + nwCfg *cni.NetworkConfig + args *cniSkel.CmdArgs + hostSubnetPrefix *net.IPNet + options map[string]interface{} + } + + tests := []struct { + name string + fields fields + args args + want *cniTypesCurr.Result + want1 *cniTypesCurr.Result + wantErr bool + }{ + { + name: "Test happy CNI add for dualstack mode with both requestIP and requestIPs not working", + fields: fields{ + podName: testPodInfo.PodName, + podNamespace: testPodInfo.PodNamespace, + cnsClient: &MockCNSClient{ + require: require, + requestIPs: requestIPsHandler{ + ipconfigArgument: getTestIPConfigsRequest(), + 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", + }, + }, + { + PodIPConfig: cns.IPSubnet{ + IPAddress: "fd11:1234::1", + PrefixLength: 24, + }, + NetworkContainerPrimaryIPConfig: cns.IPConfiguration{ + IPSubnet: cns.IPSubnet{ + IPAddress: "fd11:1234::", + PrefixLength: 112, + }, + DNSServers: nil, + GatewayIPAddress: "fe80::1234:5678:9abc", + }, + HostPrimaryIPInfo: cns.HostIPInfo{ + Gateway: "fe80::1234:5678:9abc", + PrimaryIP: "fe80::1234:5678:9abc", + Subnet: "fd11:1234::/112", + }, + }, + }, + Response: cns.Response{ + ReturnCode: 0, + Message: "", + }, + }, + err: nil, + }, + }, + }, + args: args{ + nwCfg: &cni.NetworkConfig{}, + args: &cniSkel.CmdArgs{ + ContainerID: "testcontainerid", + Netns: "testnetns1", + IfName: "testifname1", + }, + hostSubnetPrefix: getCIDRNotationForAddress("10.0.0.1/24"), + options: map[string]interface{}{}, + }, + }, + } + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + invoker := &CNSIPAMInvoker{ + podName: tt.fields.podName, + podNamespace: tt.fields.podNamespace, + cnsClient: tt.fields.cnsClient, + } + if tt.fields.ipamMode != "" { + invoker.ipamMode = tt.fields.ipamMode + } + _, err := invoker.Add(IPAMAddConfig{nwCfg: tt.args.nwCfg, args: tt.args.args, options: tt.args.options}) + if err == nil && tt.wantErr { + t.Fatalf("expected an error %+v but none received", err) + } + if !errors.Is(err, errNoRequestIPFound) { + t.Fatalf("expected an error %s but %v received", errNoRequestIPFound, err) + } + }) + } +} + func TestCNSIPAMInvoker_Delete(t *testing.T) { require := require.New(t) //nolint further usage of require without passing t type fields struct { diff --git a/cni/network/multitenancy_test.go b/cni/network/multitenancy_test.go index 596453e537..d1bce45c41 100644 --- a/cni/network/multitenancy_test.go +++ b/cni/network/multitenancy_test.go @@ -3,11 +3,14 @@ package network import ( "context" "encoding/json" + "errors" "net" "testing" "github.com/Azure/azure-container-networking/cni" "github.com/Azure/azure-container-networking/cns" + "github.com/Azure/azure-container-networking/cns/client" + "github.com/Azure/azure-container-networking/cns/types" "github.com/Azure/azure-container-networking/network" cniTypes "github.com/containernetworking/cni/pkg/types" cniTypesCurr "github.com/containernetworking/cni/pkg/types/100" @@ -35,8 +38,11 @@ type requestIPsHandler struct { } type releaseIPsHandler struct { + // arguments ipconfigArgument cns.IPConfigsRequest - err error + + // results + err error } type getNetworkContainerConfigurationHandler struct { @@ -56,6 +62,8 @@ type cnsAPIName string const ( GetAllNetworkContainers cnsAPIName = "GetAllNetworkContainers" + RequestIPs cnsAPIName = "RequestIPs" + ReleaseIPs cnsAPIName = "ReleaseIPs" ) var ( @@ -68,7 +76,7 @@ var ( type MockCNSClient struct { unsupportedAPIs map[cnsAPIName]struct{} require *require.Assertions - request requestIPAddressHandler + requestIP requestIPAddressHandler requestIPs requestIPsHandler release releaseIPsHandler getNetworkContainerConfiguration getNetworkContainerConfigurationHandler @@ -76,10 +84,10 @@ type MockCNSClient struct { } func (c *MockCNSClient) RequestIPAddress(_ context.Context, ipconfig cns.IPConfigRequest) (*cns.IPConfigResponse, error) { - if !cmp.Equal(c.request.ipconfigArgument, ipconfig) { + if !cmp.Equal(c.requestIP.ipconfigArgument, ipconfig) { return nil, errNoRequestIPFound } - return c.request.result, c.request.err + return c.requestIP.result, c.requestIP.err } func (c *MockCNSClient) RequestIPs(_ context.Context, ipconfig cns.IPConfigsRequest) (*cns.IPConfigsResponse, error) { @@ -91,7 +99,7 @@ func (c *MockCNSClient) RequestIPs(_ context.Context, ipconfig cns.IPConfigsRequ } if !cmp.Equal(c.requestIPs.ipconfigArgument, ipconfig) { - return nil, errUnsupportedAPI + return nil, errNoRequestIPFound } return c.requestIPs.result, c.requestIPs.err } @@ -108,11 +116,11 @@ func (c *MockCNSClient) ReleaseIPs(_ context.Context, ipconfig cns.IPConfigsRequ e := &client.CNSClientError{} e.Code = types.UnsupportedAPI e.Err = errUnsupportedAPI - return nil, e + return e } if !cmp.Equal(c.requestIPs.ipconfigArgument, ipconfig) { - return errUnsupportedAPI + return errNoReleaseIPFound } return c.release.err } From c262ca9130d81e7b23c13acb2f5fdfd2d73b23ed Mon Sep 17 00:00:00 2001 From: paulyu Date: Thu, 13 Apr 2023 12:31:51 -0400 Subject: [PATCH 25/44] fix releaseIP UT --- cni/network/invoker_cns_test.go | 127 ++++++++++++++++++++++++++++++- cni/network/multitenancy_test.go | 19 +++-- 2 files changed, 139 insertions(+), 7 deletions(-) diff --git a/cni/network/invoker_cns_test.go b/cni/network/invoker_cns_test.go index 45f15468ca..1cfc174f5c 100644 --- a/cni/network/invoker_cns_test.go +++ b/cni/network/invoker_cns_test.go @@ -533,7 +533,7 @@ func TestCNSIPAMInvoker_Delete(t *testing.T) { podNamespace: testPodInfo.PodNamespace, cnsClient: &MockCNSClient{ require: require, - release: releaseIPsHandler{ + releaseIPs: releaseIPsHandler{ ipconfigArgument: getTestIPConfigsRequest(), }, }, @@ -554,7 +554,7 @@ func TestCNSIPAMInvoker_Delete(t *testing.T) { podName: testPodInfo.PodName, podNamespace: testPodInfo.PodNamespace, cnsClient: &MockCNSClient{ - release: releaseIPsHandler{ + releaseIPs: releaseIPsHandler{ ipconfigArgument: getTestIPConfigsRequest(), err: errors.New("handle CNS delete error"), //nolint ut error }, @@ -581,6 +581,129 @@ func TestCNSIPAMInvoker_Delete(t *testing.T) { } } +func TestCNSIPAMInvoker_Delete_NotSupportedAPI(t *testing.T) { + require := require.New(t) //nolint further usage of require without passing t + // set new CNS API is not supported + unsupportedAPIs := make(map[cnsAPIName]struct{}) + unsupportedAPIs["ReleaseIPs"] = struct{}{} + + type fields struct { + podName string + podNamespace string + cnsClient cnsclient + } + type args struct { + address *net.IPNet + nwCfg *cni.NetworkConfig + args *cniSkel.CmdArgs + options map[string]interface{} + } + tests := []struct { + name string + fields fields + args args + wantErr bool + }{ + { + name: "test delete happy path with unsupportedAPI", + fields: fields{ + podName: testPodInfo.PodName, + podNamespace: testPodInfo.PodNamespace, + cnsClient: &MockCNSClient{ + unsupportedAPIs: unsupportedAPIs, + require: require, + releaseIP: releaseIPHandler{ + ipconfigArgument: getTestIPConfigRequest(), + }, + }, + }, + args: args{ + nwCfg: nil, + args: &cniSkel.CmdArgs{ + ContainerID: "testcontainerid", + Netns: "testnetns", + IfName: "testifname", + }, + options: map[string]interface{}{}, + }, + }, + } + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + invoker := &CNSIPAMInvoker{ + podName: tt.fields.podName, + podNamespace: tt.fields.podNamespace, + cnsClient: tt.fields.cnsClient, + } + err := invoker.Delete(tt.args.address, tt.args.nwCfg, tt.args.args, tt.args.options) + if tt.wantErr { + require.Error(err) + } else { + require.NoError(err) + } + }) + } +} + +func TestCNSIPAMInvoker_Delete_NotFound(t *testing.T) { + require := require.New(t) //nolint further usage of require without passing t + type fields struct { + podName string + podNamespace string + cnsClient cnsclient + } + type args struct { + address *net.IPNet + nwCfg *cni.NetworkConfig + args *cniSkel.CmdArgs + options map[string]interface{} + } + tests := []struct { + name string + fields fields + args args + wantErr bool + }{ + { + name: "test delete happy path with both ReleaseIPs and ReleassIP not working", + fields: fields{ + podName: testPodInfo.PodName, + podNamespace: testPodInfo.PodNamespace, + cnsClient: &MockCNSClient{ + require: require, + releaseIPs: releaseIPsHandler{ + ipconfigArgument: getTestIPConfigsRequest(), + }, + }, + }, + args: args{ + nwCfg: nil, + args: &cniSkel.CmdArgs{ + ContainerID: "testcontainerid", + Netns: "testnetns1", + IfName: "testifname1", + }, + options: map[string]interface{}{}, + }, + }, + } + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + invoker := &CNSIPAMInvoker{ + podName: tt.fields.podName, + podNamespace: tt.fields.podNamespace, + cnsClient: tt.fields.cnsClient, + } + err := invoker.Delete(tt.args.address, tt.args.nwCfg, tt.args.args, tt.args.options) + if !errors.Is(err, errNoReleaseIPFound) { + t.Fatalf("expected an error %s but %v received", errNoReleaseIPFound, err) + } + }) + } +} + func Test_setHostOptions(t *testing.T) { require := require.New(t) //nolint further usage of require without passing t type args struct { diff --git a/cni/network/multitenancy_test.go b/cni/network/multitenancy_test.go index d1bce45c41..c64fb96a6b 100644 --- a/cni/network/multitenancy_test.go +++ b/cni/network/multitenancy_test.go @@ -37,6 +37,14 @@ type requestIPsHandler struct { err error } +type releaseIPHandler struct { + // arguments + ipconfigArgument cns.IPConfigRequest + + // results + err error +} + type releaseIPsHandler struct { // arguments ipconfigArgument cns.IPConfigsRequest @@ -78,7 +86,8 @@ type MockCNSClient struct { require *require.Assertions requestIP requestIPAddressHandler requestIPs requestIPsHandler - release releaseIPsHandler + releaseIP releaseIPHandler + releaseIPs releaseIPsHandler getNetworkContainerConfiguration getNetworkContainerConfigurationHandler getAllNetworkContainersConfiguration getAllNetworkContainersConfigurationHandler } @@ -105,10 +114,10 @@ func (c *MockCNSClient) RequestIPs(_ context.Context, ipconfig cns.IPConfigsRequ } func (c *MockCNSClient) ReleaseIPAddress(_ context.Context, ipconfig cns.IPConfigRequest) error { - if !cmp.Equal(c.release.ipconfigArgument, ipconfig) { + if !cmp.Equal(c.releaseIP.ipconfigArgument, ipconfig) { return errNoReleaseIPFound } - return c.release.err + return c.releaseIP.err } func (c *MockCNSClient) ReleaseIPs(_ context.Context, ipconfig cns.IPConfigsRequest) error { @@ -119,10 +128,10 @@ func (c *MockCNSClient) ReleaseIPs(_ context.Context, ipconfig cns.IPConfigsRequ return e } - if !cmp.Equal(c.requestIPs.ipconfigArgument, ipconfig) { + if !cmp.Equal(c.releaseIPs.ipconfigArgument, ipconfig) { return errNoReleaseIPFound } - return c.release.err + return c.releaseIPs.err } func (c *MockCNSClient) GetNetworkContainer(ctx context.Context, orchestratorContext []byte) (*cns.GetNetworkContainerResponse, error) { From 3e9be153d9944cf7aed8cd41e08c6ccaf7052638 Mon Sep 17 00:00:00 2001 From: paulyu Date: Thu, 13 Apr 2023 12:51:29 -0400 Subject: [PATCH 26/44] fix a variable name --- cni/network/invoker_azure_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cni/network/invoker_azure_test.go b/cni/network/invoker_azure_test.go index f1ddd5b7d2..ea0233045d 100644 --- a/cni/network/invoker_azure_test.go +++ b/cni/network/invoker_azure_test.go @@ -281,11 +281,11 @@ func TestAzureIPAMInvoker_Delete(t *testing.T) { }, }, { - name: "error addresses is nil", + name: "error address is nil", fields: fields{ plugin: &mockDelegatePlugin{ del: del{ - err: errors.New("error when addresses is nil"), //nolint:goerr113 // error + err: errors.New("error when address is nil"), //nolint:goerr113 // error }, }, nwInfo: getNwInfo("", "2001:db8:abcd:0012::0/64"), From 1a26c3ddc68b7b56ae41aa48b8a4c3495dde4a4d Mon Sep 17 00:00:00 2001 From: paulyu Date: Thu, 13 Apr 2023 21:43:56 -0400 Subject: [PATCH 27/44] fix minor issues --- cni/network/invoker_azure_test.go | 2 +- cni/network/invoker_mock.go | 2 +- cni/network/multitenancy_test.go | 7 +++---- 3 files changed, 5 insertions(+), 6 deletions(-) diff --git a/cni/network/invoker_azure_test.go b/cni/network/invoker_azure_test.go index ea0233045d..68d96a4b66 100644 --- a/cni/network/invoker_azure_test.go +++ b/cni/network/invoker_azure_test.go @@ -285,7 +285,7 @@ func TestAzureIPAMInvoker_Delete(t *testing.T) { fields: fields{ plugin: &mockDelegatePlugin{ del: del{ - err: errors.New("error when address is nil"), //nolint:goerr113 // error + err: errors.New("error when address is nil"), //nolint:goerr113 }, }, nwInfo: getNwInfo("", "2001:db8:abcd:0012::0/64"), diff --git a/cni/network/invoker_mock.go b/cni/network/invoker_mock.go index 917e531a64..51e04c29ce 100644 --- a/cni/network/invoker_mock.go +++ b/cni/network/invoker_mock.go @@ -79,7 +79,7 @@ func (invoker *MockIpamInvoker) Add(opt IPAMAddConfig) (ipamAddResult IPAMAddRes return ipamAddResult, nil } -func (invoker *MockIpamInvoker) Delete(address *net.IPNet, nwCfg *cni.NetworkConfig, _ *skel.CmdArgs, options map[string]interface{}) error { //nolint +func (invoker *MockIpamInvoker) Delete(address *net.IPNet, nwCfg *cni.NetworkConfig, _ *skel.CmdArgs, options map[string]interface{}) error { if invoker.v4Fail || invoker.v6Fail { return errDeleteIpam } diff --git a/cni/network/multitenancy_test.go b/cni/network/multitenancy_test.go index c64fb96a6b..152bdd9324 100644 --- a/cni/network/multitenancy_test.go +++ b/cni/network/multitenancy_test.go @@ -75,10 +75,9 @@ const ( ) var ( - errUnsupportedAPI = errors.New("Unsupported API") - errNoOrchestratorContextFound = errors.New("No CNI OrchestratorContext Found") - errNoRequestIPFound = errors.New("No Request IP Found") - errNoReleaseIPFound = errors.New("No Release IP Found") + errUnsupportedAPI = errors.New("Unsupported API") + errNoRequestIPFound = errors.New("No Request IP Found") + errNoReleaseIPFound = errors.New("No Release IP Found") ) type MockCNSClient struct { From 7c55a6dc8a99124534c50da5a2eb5e2ac0a77890 Mon Sep 17 00:00:00 2001 From: paulyu Date: Thu, 13 Apr 2023 22:06:51 -0400 Subject: [PATCH 28/44] fix linter issue --- network/network_windows.go | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/network/network_windows.go b/network/network_windows.go index c572063498..cf6f389846 100644 --- a/network/network_windows.go +++ b/network/network_windows.go @@ -211,32 +211,33 @@ func (nm *networkManager) addNewNetRules(nwInfo *NetworkInfo) error { gateway := subnet.Gateway.String() ip, _, er := net.ParseCIDR(prefix) - if er != nil { + if er != nil { //nolint return fmt.Errorf("[net] failed to parse prefix %s", prefix) } if ip.To4() != nil { // netsh interface ipv4 add route $subnetV4 $hostInterfaceAlias "0.0.0.0" netshV4DefaultRoute := fmt.Sprintf(netRouteCmd, "ipv4", prefix, ifName, ipv4DefaultHop) - if out, err = nm.plClient.ExecuteCommand(netshV4DefaultRoute); err != nil { + if out, err = nm.plClient.ExecuteCommand(netshV4DefaultRoute); err != nil { //nolint log.Printf("[net] Adding ipv4 default route failed: %v:%v", out, err) } // netsh interface ipv4 add route $subnetV4 $hostInterfaceAlias $gatewayV4 netshV4GatewayRoute := fmt.Sprintf(netRouteCmd, "ipv4", prefix, ifName, gateway) - if out, err = nm.plClient.ExecuteCommand(netshV4GatewayRoute); err != nil { + log.Printf("[net] Adding ipv4 gateway route failed: %v:%v", out, err) + if out, err = nm.plClient.ExecuteCommand(netshV4GatewayRoute); err != nil { //nolint log.Printf("[net] Adding ipv4 gateway route failed: %v:%v", out, err) } } else { // netsh interface ipv6 add route $subnetV6 $hostInterfaceAlias "::" netshV6DefaultRoute := fmt.Sprintf(netRouteCmd, "ipv6", prefix, ifName, ipv6DefaultHop) - if out, err = nm.plClient.ExecuteCommand(netshV6DefaultRoute); err != nil { + if out, err = nm.plClient.ExecuteCommand(netshV6DefaultRoute); err != nil { //nolint log.Printf("[net] Adding ipv6 default route failed: %v:%v", out, err) } // netsh interface ipv6 add route $subnetV6 $hostInterfaceAlias $gatewayV6 netshV6GatewayRoute := fmt.Sprintf(netRouteCmd, "ipv6", prefix, ifName, gateway) if out, err = nm.plClient.ExecuteCommand(netshV6GatewayRoute); err != nil { - log.Printf("[net] Adding ipv6 gateway route failed: %v:%v", out, err) + log.Printf("[net] Adding ipv6 gateway route failed: %v:%v", out, err) //nolint } } } From ea7a2d4e2b6ce9d5fe001942a6321c69214db43d Mon Sep 17 00:00:00 2001 From: paulyu Date: Sat, 15 Apr 2023 09:10:53 -0400 Subject: [PATCH 29/44] add proper logs --- network/network_windows.go | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/network/network_windows.go b/network/network_windows.go index cf6f389846..0715710ed4 100644 --- a/network/network_windows.go +++ b/network/network_windows.go @@ -211,27 +211,27 @@ func (nm *networkManager) addNewNetRules(nwInfo *NetworkInfo) error { gateway := subnet.Gateway.String() ip, _, er := net.ParseCIDR(prefix) - if er != nil { //nolint - return fmt.Errorf("[net] failed to parse prefix %s", prefix) + if er != nil { + return fmt.Errorf("[net] failed to parse prefix %s", prefix) //nolint } if ip.To4() != nil { // netsh interface ipv4 add route $subnetV4 $hostInterfaceAlias "0.0.0.0" netshV4DefaultRoute := fmt.Sprintf(netRouteCmd, "ipv4", prefix, ifName, ipv4DefaultHop) - if out, err = nm.plClient.ExecuteCommand(netshV4DefaultRoute); err != nil { //nolint - log.Printf("[net] Adding ipv4 default route failed: %v:%v", out, err) + if out, err = nm.plClient.ExecuteCommand(netshV4DefaultRoute); err != nil { + log.Printf("[net] Adding ipv4 default route failed: %v:%v", out, err) //nolint } // netsh interface ipv4 add route $subnetV4 $hostInterfaceAlias $gatewayV4 netshV4GatewayRoute := fmt.Sprintf(netRouteCmd, "ipv4", prefix, ifName, gateway) log.Printf("[net] Adding ipv4 gateway route failed: %v:%v", out, err) - if out, err = nm.plClient.ExecuteCommand(netshV4GatewayRoute); err != nil { //nolint - log.Printf("[net] Adding ipv4 gateway route failed: %v:%v", out, err) + if out, err = nm.plClient.ExecuteCommand(netshV4GatewayRoute); err != nil { + log.Printf("[net] Adding ipv4 gateway route failed: %v:%v", out, err) //nolint } } else { // netsh interface ipv6 add route $subnetV6 $hostInterfaceAlias "::" netshV6DefaultRoute := fmt.Sprintf(netRouteCmd, "ipv6", prefix, ifName, ipv6DefaultHop) - if out, err = nm.plClient.ExecuteCommand(netshV6DefaultRoute); err != nil { //nolint - log.Printf("[net] Adding ipv6 default route failed: %v:%v", out, err) + if out, err = nm.plClient.ExecuteCommand(netshV6DefaultRoute); err != nil { + log.Printf("[net] Adding ipv6 default route failed: %v:%v", out, err) //nolint } // netsh interface ipv6 add route $subnetV6 $hostInterfaceAlias $gatewayV6 From e2a87f197a0388cb0d0917282e7323392cf2f1df Mon Sep 17 00:00:00 2001 From: paulyu Date: Mon, 17 Apr 2023 10:17:47 -0400 Subject: [PATCH 30/44] remove nolint --- network/network_windows.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/network/network_windows.go b/network/network_windows.go index 0715710ed4..2c89600d69 100644 --- a/network/network_windows.go +++ b/network/network_windows.go @@ -212,32 +212,32 @@ func (nm *networkManager) addNewNetRules(nwInfo *NetworkInfo) error { ip, _, er := net.ParseCIDR(prefix) if er != nil { - return fmt.Errorf("[net] failed to parse prefix %s", prefix) //nolint + return fmt.Errorf("[net] failed to parse prefix %s", prefix) } if ip.To4() != nil { // netsh interface ipv4 add route $subnetV4 $hostInterfaceAlias "0.0.0.0" netshV4DefaultRoute := fmt.Sprintf(netRouteCmd, "ipv4", prefix, ifName, ipv4DefaultHop) if out, err = nm.plClient.ExecuteCommand(netshV4DefaultRoute); err != nil { - log.Printf("[net] Adding ipv4 default route failed: %v:%v", out, err) //nolint + log.Printf("[net] Adding ipv4 default route failed: %v:%v", out, err) } // netsh interface ipv4 add route $subnetV4 $hostInterfaceAlias $gatewayV4 netshV4GatewayRoute := fmt.Sprintf(netRouteCmd, "ipv4", prefix, ifName, gateway) log.Printf("[net] Adding ipv4 gateway route failed: %v:%v", out, err) if out, err = nm.plClient.ExecuteCommand(netshV4GatewayRoute); err != nil { - log.Printf("[net] Adding ipv4 gateway route failed: %v:%v", out, err) //nolint + log.Printf("[net] Adding ipv4 gateway route failed: %v:%v", out, err) } } else { // netsh interface ipv6 add route $subnetV6 $hostInterfaceAlias "::" netshV6DefaultRoute := fmt.Sprintf(netRouteCmd, "ipv6", prefix, ifName, ipv6DefaultHop) if out, err = nm.plClient.ExecuteCommand(netshV6DefaultRoute); err != nil { - log.Printf("[net] Adding ipv6 default route failed: %v:%v", out, err) //nolint + log.Printf("[net] Adding ipv6 default route failed: %v:%v", out, err) } // netsh interface ipv6 add route $subnetV6 $hostInterfaceAlias $gatewayV6 netshV6GatewayRoute := fmt.Sprintf(netRouteCmd, "ipv6", prefix, ifName, gateway) if out, err = nm.plClient.ExecuteCommand(netshV6GatewayRoute); err != nil { - log.Printf("[net] Adding ipv6 gateway route failed: %v:%v", out, err) //nolint + log.Printf("[net] Adding ipv6 gateway route failed: %v:%v", out, err) } } } From f57754bb5643a4e0706575b45ea3a654fcf3a83d Mon Sep 17 00:00:00 2001 From: paulyu Date: Mon, 17 Apr 2023 15:02:40 -0400 Subject: [PATCH 31/44] fix a bug to add netsh rule only it's dualStackOverlay mode and hnsnetwork is created at first time --- network/network_windows.go | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/network/network_windows.go b/network/network_windows.go index 2c89600d69..1e90fd8fcf 100644 --- a/network/network_windows.go +++ b/network/network_windows.go @@ -397,6 +397,13 @@ func (nm *networkManager) newNetworkImplHnsV2(nwInfo *NetworkInfo, extIf *extern } log.Printf("[net] Successfully created hcn network with response: %+v", hnsResponse) + + // only add net rules if it's dualStackOverlay mode and hnsNetwork is created at first time + if util.DualStackOverlay == DualStackOverlay { + if err := nm.addNewNetRules(nwInfo); err != nil { + return nil, err + } + } } else { // we can't validate if the network already exists, don't continue return nil, fmt.Errorf("Failed to create hcn network: %s, failed to query for existing network with error: %v", hcnNetwork.Name, err) @@ -412,12 +419,6 @@ func (nm *networkManager) newNetworkImplHnsV2(nwInfo *NetworkInfo, extIf *extern vlanid = (int)(vlanID) } - if util.DualStackOverlay == DualStackOverlay { - if err := nm.addNewNetRules(nwInfo); err != nil { - return nil, err - } - } - // Create the network object. nw := &network{ Id: nwInfo.Id, From ac74b1647159087ecb2b8cb9c54b25659c841cc4 Mon Sep 17 00:00:00 2001 From: paulyu Date: Mon, 17 Apr 2023 20:53:13 -0400 Subject: [PATCH 32/44] comment fix part1 --- cni/network/invoker_cns.go | 34 +++++++++++++++++----------------- cni/network/network.go | 12 +++--------- cns/client/client.go | 18 ++++++++---------- network/network.go | 2 -- network/network_windows.go | 4 ++-- 5 files changed, 30 insertions(+), 40 deletions(-) diff --git a/cni/network/invoker_cns.go b/cni/network/invoker_cns.go index 0dda7ec3e2..44c5065d1e 100644 --- a/cni/network/invoker_cns.go +++ b/cni/network/invoker_cns.go @@ -72,29 +72,29 @@ func (invoker *CNSIPAMInvoker) Add(addConfig IPAMAddConfig) (IPAMAddResult, erro return IPAMAddResult{}, errEmptyCNIArgs } - ipconfig := cns.IPConfigsRequest{ + ipconfigs := cns.IPConfigsRequest{ OrchestratorContext: orchestratorContext, PodInterfaceID: GetEndpointID(addConfig.args), InfraContainerID: addConfig.args.ContainerID, } - log.Printf("Requesting IP for pod %+v using ipconfig %+v", podInfo, ipconfig) - response, err := invoker.cnsClient.RequestIPs(context.TODO(), ipconfig) + log.Printf("Requesting IP for pod %+v using ipconfigs %+v", podInfo, ipconfigs) + response, err := invoker.cnsClient.RequestIPs(context.TODO(), ipconfigs) if err != nil { // if RequestIPs call fails, we may receive API not Found error as new CNS is not supported, then try old API RequestIPAddress - log.Errorf("RequestIPs not supported by CNS. Invoking RequestIPAddress API with infracontainerid %s", ipconfig.InfraContainerID) + log.Errorf("RequestIPs not supported by CNS. Invoking RequestIPAddress API with infracontainerid %s", ipconfigs.InfraContainerID) if cnscli.IsUnsupportedAPI(err) { - ipconfigs := cns.IPConfigRequest{ + ipconfig := cns.IPConfigRequest{ OrchestratorContext: orchestratorContext, PodInterfaceID: GetEndpointID(addConfig.args), InfraContainerID: addConfig.args.ContainerID, } - res, errRequestIP := invoker.cnsClient.RequestIPAddress(context.TODO(), ipconfigs) + res, errRequestIP := invoker.cnsClient.RequestIPAddress(context.TODO(), ipconfig) if errRequestIP != nil { // if the old API fails as well then we just return the error - log.Errorf("Failed to request IP address from CNS using RequestIPAddress with infracontainerid %s. error: %v", ipconfigs.InfraContainerID, errRequestIP) - return IPAMAddResult{}, errors.Wrap(err, "Failed to get IP address from CNS with error: %w") + log.Errorf("Failed to request IP address from CNS using RequestIPAddress with infracontainerid %s. error: %v", ipconfig.InfraContainerID, errRequestIP) + return IPAMAddResult{}, errors.Wrap(errRequestIP, "Failed to get IP address from CNS with error: %w") } response = &cns.IPConfigsResponse{ Response: res.Response, @@ -283,36 +283,36 @@ func (invoker *CNSIPAMInvoker) Delete(address *net.IPNet, nwCfg *cni.NetworkConf return errEmptyCNIArgs } - ipConfig := cns.IPConfigsRequest{ + ipConfigs := cns.IPConfigsRequest{ OrchestratorContext: orchestratorContext, PodInterfaceID: GetEndpointID(args), InfraContainerID: args.ContainerID, } if address != nil { - ipConfig.DesiredIPAddresses = append(ipConfig.DesiredIPAddresses, address.IP.String()) + ipConfigs.DesiredIPAddresses = append(ipConfigs.DesiredIPAddresses, address.IP.String()) } else { log.Printf("CNS invoker called with empty IP address") } - if err := invoker.cnsClient.ReleaseIPs(context.TODO(), ipConfig); err != nil { + if err := invoker.cnsClient.ReleaseIPs(context.TODO(), ipConfigs); err != nil { // if ReleaseIPs call fails, we may receive API not Found error as new CNS is not supported, then try old API ReleaseIPAddress - log.Errorf("ReleaseIPs not supported by CNS. Invoking ReleaseIPAddress API. Request: %v", ipConfig) + log.Errorf("ReleaseIPs not supported by CNS. Invoking ReleaseIPAddress API. Request: %v", ipConfigs) if cnscli.IsUnsupportedAPI(err) { - ipConfigs := cns.IPConfigRequest{ + ipConfig := cns.IPConfigRequest{ OrchestratorContext: orchestratorContext, PodInterfaceID: GetEndpointID(args), InfraContainerID: args.ContainerID, } - if err = invoker.cnsClient.ReleaseIPAddress(context.TODO(), ipConfigs); err != nil { + if err = invoker.cnsClient.ReleaseIPAddress(context.TODO(), ipConfig); err != nil { // if the old API fails as well then we just return the error log.Errorf("Failed to release IP address from CNS using ReleaseIPAddress with infracontainerid %s. error: %v", ipConfigs.InfraContainerID, err) - return errors.Wrap(err, fmt.Sprintf("failed to release IP %v using ReleaseIPAddress with err ", ipConfigs.DesiredIPAddress)+"%w") + return errors.Wrap(err, fmt.Sprintf("failed to release IP %v using ReleaseIPAddress with err ", ipConfig.DesiredIPAddress)+"%w") } } else { - log.Errorf("Failed to release IP address with infracontainerid %s from CNS error: %v", ipConfig.InfraContainerID, err) - return errors.Wrap(err, fmt.Sprintf("failed to release IP %v using ReleaseIPs with err ", ipConfig.DesiredIPAddresses)+"%w") + log.Errorf("Failed to release IP address with infracontainerid %s from CNS error: %v", ipConfigs.InfraContainerID, err) + return errors.Wrap(err, fmt.Sprintf("failed to release IP %v using ReleaseIPs with err ", ipConfigs.DesiredIPAddresses)+"%w") } } diff --git a/cni/network/network.go b/cni/network/network.go index 73af262775..c2197d1be0 100644 --- a/cni/network/network.go +++ b/cni/network/network.go @@ -385,11 +385,7 @@ func (plugin *NetPlugin) Add(args *cniSkel.CmdArgs) error { res.Print() } - if ipamAddResult.ipv4Result.IPs != nil { - log.Printf("[cni-net] ADD command completed for pod %s with IPs:%v err:%v.", k8sPodName, ipamAddResult.ipv4Result.IPs, err) - } else { - log.Printf("[cni-net] no IPs will be allocated for pod %s", k8sPodName) - } + log.Printf("[cni-net] ADD command completed for pod %v with IPs:%+v err:%v.", k8sPodName, ipamAddResult.ipv4Result.IPs, err) }() // Parse Pod arguments. @@ -677,8 +673,6 @@ func (plugin *NetPlugin) createNetworkInternal( setNetworkOptions(ipamAddResult.ncResponse, &nwInfo) - addNatIPV6SubnetInfo(ipamAddConfig.nwCfg, ipamAddResult.ipv6Result, &nwInfo) - err = plugin.nm.CreateNetwork(&nwInfo) if err != nil { err = plugin.Errorf("createNetworkInternal: Failed to create network: %v", err) @@ -764,8 +758,8 @@ func (plugin *NetPlugin) createEndpointInternal(opt *createEndpointInternalOpt) } if opt.resultV6 != nil { - // inject routes to linux pod - epInfo.IPV6Mode = network.DualStackOverlay + // inject routes to linux pod in ipam dualStackOverlay mode when IPv6Mode + epInfo.IPV6Mode = string(util.DualStackOverlay) for _, ipconfig := range opt.resultV6.IPs { epInfo.IPAddresses = append(epInfo.IPAddresses, ipconfig.Address) } diff --git a/cns/client/client.go b/cns/client/client.go index e12490a843..16b85710e2 100644 --- a/cns/client/client.go +++ b/cns/client/client.go @@ -400,6 +400,10 @@ func (c *Client) RequestIPs(ctx context.Context, ipconfig cns.IPConfigsRequest) } req.Header.Set(headerContentType, contentTypeJSON) res, err := c.client.Do(req) + if err != nil { + return nil, errors.Wrap(err, "http request failed") + } + defer res.Body.Close() if res.StatusCode == http.StatusNotFound { return nil, &CNSClientError{ @@ -408,11 +412,6 @@ func (c *Client) RequestIPs(ctx context.Context, ipconfig cns.IPConfigsRequest) } } - if err != nil { - return nil, errors.Wrap(err, "http request failed") - } - defer res.Body.Close() - if res.StatusCode != http.StatusOK { return nil, errors.Errorf("http response %d", res.StatusCode) } @@ -445,6 +444,10 @@ func (c *Client) ReleaseIPs(ctx context.Context, ipconfig cns.IPConfigsRequest) } req.Header.Set(headerContentType, contentTypeJSON) res, err := c.client.Do(req) + if err != nil { + return errors.Wrap(err, "http request failed") + } + defer res.Body.Close() // if we get a 404 error if res.StatusCode == http.StatusNotFound { @@ -454,11 +457,6 @@ func (c *Client) ReleaseIPs(ctx context.Context, ipconfig cns.IPConfigsRequest) } } - if err != nil { - return errors.Wrap(err, "http request failed") - } - defer res.Body.Close() - if res.StatusCode != http.StatusOK { return errors.Errorf("http response %d", res.StatusCode) } diff --git a/network/network.go b/network/network.go index 7c2151fedf..9a9dfac69a 100644 --- a/network/network.go +++ b/network/network.go @@ -25,8 +25,6 @@ const ( const ( // ipv6 modes IPV6Nat = "ipv6nat" - // dual stack mode - DualStackOverlay = "dualStackOverlay" ) // externalInterface is a host network interface that bridges containers to external networks. diff --git a/network/network_windows.go b/network/network_windows.go index 1e90fd8fcf..d1d30c9758 100644 --- a/network/network_windows.go +++ b/network/network_windows.go @@ -205,7 +205,7 @@ func (nm *networkManager) addNewNetRules(nwInfo *NetworkInfo) error { ifName = fmt.Sprintf("%s (%s)", ifNamePrefix, nwInfo.MasterIfName) } - // add ipv4 and ipv6 new net rules to windows node + // iterate subnet and add ipv4 and ipv6 default route and gateway for _, subnet := range nwInfo.Subnets { prefix := subnet.Prefix.String() gateway := subnet.Gateway.String() @@ -399,7 +399,7 @@ func (nm *networkManager) newNetworkImplHnsV2(nwInfo *NetworkInfo, extIf *extern log.Printf("[net] Successfully created hcn network with response: %+v", hnsResponse) // only add net rules if it's dualStackOverlay mode and hnsNetwork is created at first time - if util.DualStackOverlay == DualStackOverlay { + if string(util.DualStackOverlay) != "" { if err := nm.addNewNetRules(nwInfo); err != nil { return nil, err } From 64babe3503f204bbe2a79cfb76988ba92e20645a Mon Sep 17 00:00:00 2001 From: paulyu Date: Mon, 17 Apr 2023 22:53:52 -0400 Subject: [PATCH 33/44] reorg constructNetworkInfo --- cni/network/network.go | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/cni/network/network.go b/cni/network/network.go index 699bd6b474..5ecdf88b83 100644 --- a/cni/network/network.go +++ b/cni/network/network.go @@ -657,6 +657,19 @@ func (plugin *NetPlugin) createNetworkInternal( ServiceCidrs: ipamAddConfig.nwCfg.ServiceCidrs, } + constructNetworkInfo(ipamAddResult, podSubnetPrefix, podSubnetV6Prefix, nwInfo) + setNetworkOptions(ipamAddResult.ncResponse, &nwInfo) + + err = plugin.nm.CreateNetwork(&nwInfo) + if err != nil { + err = plugin.Errorf("createNetworkInternal: Failed to create network: %v", err) + } + + return nwInfo, err +} + +// construct network info with ipv4/ipv6 subnets +func constructNetworkInfo(ipamAddResult IPAMAddResult, podSubnetPrefix, podSubnetV6Prefix *net.IPNet, nwInfo network.NetworkInfo) { ipv4Subnet := network.SubnetInfo{ Family: platform.AfINET, Prefix: *podSubnetPrefix, @@ -672,15 +685,6 @@ func (plugin *NetPlugin) createNetworkInternal( } nwInfo.Subnets = append(nwInfo.Subnets, ipv6Subnet) } - - setNetworkOptions(ipamAddResult.ncResponse, &nwInfo) - - err = plugin.nm.CreateNetwork(&nwInfo) - if err != nil { - err = plugin.Errorf("createNetworkInternal: Failed to create network: %v", err) - } - - return nwInfo, err } type createEndpointInternalOpt struct { From 157386c7524f420a84a439e997d3c024a048a146 Mon Sep 17 00:00:00 2001 From: paulyu Date: Tue, 18 Apr 2023 13:40:41 -0400 Subject: [PATCH 34/44] fix comments --- Makefile | 4 ++-- cni/network/network.go | 8 +++++--- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/Makefile b/Makefile index 8c7fc231e2..688b550e82 100644 --- a/Makefile +++ b/Makefile @@ -629,8 +629,8 @@ endif $(MKDIR) $(CNI_DUALSTACK_BUILD_DIR) cp cni/azure-$(GOOS)-swift-overlay-dualstack.conflist $(CNI_DUALSTACK_BUILD_DIR)/10-azure.conflist cp telemetry/azure-vnet-telemetry.config $(CNI_DUALSTACK_BUILD_DIR)/azure-vnet-telemetry.config - cp $(CNI_BUILD_DIR)/azure-vnet$(EXE_EXT) $(CNI_BUILD_DIR)/azure-vnet-ipam$(EXE_EXT) $(CNI_BUILD_DIR)/azure-vnet-telemetry$(EXE_EXT) $(CNI_DUALSTACK_BUILD_DIR) - cd $(CNI_DUALSTACK_BUILD_DIR) && $(ARCHIVE_CMD) $(CNI_DUALSTACK_ARCHIVE_NAME) azure-vnet$(EXE_EXT) azure-vnet-ipam$(EXE_EXT) azure-vnet-telemetry$(EXE_EXT) 10-azure.conflist azure-vnet-telemetry.config + cp $(CNI_BUILD_DIR)/azure-vnet$(EXE_EXT) $(CNI_BUILD_DIR)/azure-vnet-telemetry$(EXE_EXT) $(CNI_DUALSTACK_BUILD_DIR) + cd $(CNI_DUALSTACK_BUILD_DIR) && $(ARCHIVE_CMD) $(CNI_DUALSTACK_ARCHIVE_NAME) azure-vnet$(EXE_EXT) azure-vnet-telemetry$(EXE_EXT) 10-azure.conflist azure-vnet-telemetry.config #baremetal mode is windows only (at least for now) ifeq ($(GOOS),windows) diff --git a/cni/network/network.go b/cni/network/network.go index 5ecdf88b83..9ecc246f20 100644 --- a/cni/network/network.go +++ b/cni/network/network.go @@ -764,8 +764,10 @@ func (plugin *NetPlugin) createEndpointInternal(opt *createEndpointInternalOpt) } if opt.resultV6 != nil { - // inject routes to linux pod in ipam dualStackOverlay mode when IPv6Mode - epInfo.IPV6Mode = string(util.DualStackOverlay) + // get IPAM mode from conflist file and add ipv6 routes to linux pod if needed + epInfo.IPV6Mode = string(util.IpamMode(opt.nwCfg.IPAM.Mode)) + log.Printf("current ipv6 mode is %s", epInfo.IPV6Mode) + for _, ipconfig := range opt.resultV6.IPs { epInfo.IPAddresses = append(epInfo.IPAddresses, ipconfig.Address) } @@ -1037,8 +1039,8 @@ func (plugin *NetPlugin) Delete(args *cniSkel.CmdArgs) error { if !nwCfg.MultiTenancy { // Call into IPAM plugin to release the endpoint's addresses. - logAndSendEvent(plugin, fmt.Sprintf("Releasing ips:%+v", epInfo.IPAddresses)) for i := range epInfo.IPAddresses { + logAndSendEvent(plugin, fmt.Sprintf("Release ip:%s", epInfo.IPAddresses[i].IP.String())) err = plugin.ipamInvoker.Delete(&epInfo.IPAddresses[i], nwCfg, args, nwInfo.Options) if err != nil { return plugin.RetriableError(fmt.Errorf("failed to release address: %w", err)) From 095f78662a5a62a8250de34f3f126a95aec7b86b Mon Sep 17 00:00:00 2001 From: paulyu Date: Tue, 18 Apr 2023 15:41:14 -0400 Subject: [PATCH 35/44] add UTs --- cni/network/invoker_azure.go | 21 +++++++-------------- cni/network/invoker_cns_test.go | 2 +- 2 files changed, 8 insertions(+), 15 deletions(-) diff --git a/cni/network/invoker_azure.go b/cni/network/invoker_azure.go index 7a069c6517..ec204222d4 100644 --- a/cni/network/invoker_azure.go +++ b/cni/network/invoker_azure.go @@ -71,11 +71,7 @@ func (invoker *AzureIPAMInvoker) Add(addConfig IPAMAddConfig) (IPAMAddResult, er 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 { - err = invoker.plugin.Errorf("Failed to clean up IPv4 during Delete with error %v, after Add failed with error %w", er, err) - } - } else if len(addResult.ipv6Result.IPs) > 0 { - if er := invoker.Delete(&addResult.ipv6Result.IPs[0].Address, addConfig.nwCfg, nil, addConfig.options); er != nil { - err = invoker.plugin.Errorf("Failed to clean up IPv6 during Delete with error %v, after Add failed with error %w", er, err) + err = invoker.plugin.Errorf("Failed to clean up IP's during Delete with error %v, after Add failed with error %w", er, err) } } else { err = fmt.Errorf("No IP's to delete on error: %v", err) @@ -84,19 +80,16 @@ func (invoker *AzureIPAMInvoker) Add(addConfig IPAMAddConfig) (IPAMAddResult, er }() if addConfig.nwCfg.IPV6Mode != "" { - nwCfgIpv6 := *addConfig.nwCfg - nwCfgIpv6.IPAM.Environment = common.OptEnvironmentIPv6NodeIpam - nwCfgIpv6.IPAM.Type = ipamV6 + nwCfg6 := *addConfig.nwCfg + nwCfg6.IPAM.Environment = common.OptEnvironmentIPv6NodeIpam + nwCfg6.IPAM.Type = ipamV6 if len(invoker.nwInfo.Subnets) > 1 { - for _, subnet := range invoker.nwInfo.Subnets { - if subnet.Prefix.IP.To16() != nil { - nwCfgIpv6.IPAM.Subnet = subnet.Prefix.String() - } - } + // ipv6 is the second subnet of the slice + nwCfg6.IPAM.Subnet = invoker.nwInfo.Subnets[1].Prefix.String() } - addResult.ipv6Result, err = invoker.plugin.DelegateAdd(nwCfgIpv6.IPAM.Type, &nwCfgIpv6) + addResult.ipv6Result, err = invoker.plugin.DelegateAdd(nwCfg6.IPAM.Type, &nwCfg6) if err != nil { err = invoker.plugin.Errorf("Failed to allocate v6 pool: %v", err) } diff --git a/cni/network/invoker_cns_test.go b/cni/network/invoker_cns_test.go index 1cfc174f5c..ab4322d706 100644 --- a/cni/network/invoker_cns_test.go +++ b/cni/network/invoker_cns_test.go @@ -135,7 +135,7 @@ func TestCNSIPAMInvoker_Add(t *testing.T) { wantErr: false, }, { - name: "Test happy CNI add for dualstack mode", + name: "Test happy CNI add ipv4 + ipv6 as dualstack mode", fields: fields{ podName: testPodInfo.PodName, podNamespace: testPodInfo.PodNamespace, From 275b8c6414bf3863f56623e4a27a7145111683f5 Mon Sep 17 00:00:00 2001 From: paulyu Date: Wed, 19 Apr 2023 11:58:58 -0400 Subject: [PATCH 36/44] fix linter issue --- network/network_windows.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/network/network_windows.go b/network/network_windows.go index d1d30c9758..b117b42871 100644 --- a/network/network_windows.go +++ b/network/network_windows.go @@ -212,7 +212,7 @@ func (nm *networkManager) addNewNetRules(nwInfo *NetworkInfo) error { ip, _, er := net.ParseCIDR(prefix) if er != nil { - return fmt.Errorf("[net] failed to parse prefix %s", prefix) + return fmt.Errorf("[net] failed to parse prefix %s", prefix) // nolint } if ip.To4() != nil { // netsh interface ipv4 add route $subnetV4 $hostInterfaceAlias "0.0.0.0" @@ -242,7 +242,7 @@ func (nm *networkManager) addNewNetRules(nwInfo *NetworkInfo) error { } } - return err + return err // nolint } func (nm *networkManager) appIPV6RouteEntry(nwInfo *NetworkInfo) error { @@ -400,7 +400,7 @@ func (nm *networkManager) newNetworkImplHnsV2(nwInfo *NetworkInfo, extIf *extern // only add net rules if it's dualStackOverlay mode and hnsNetwork is created at first time if string(util.DualStackOverlay) != "" { - if err := nm.addNewNetRules(nwInfo); err != nil { + if err = nm.addNewNetRules(nwInfo); err != nil { return nil, err } } From 4ad60eefa52192991febe3176bbc7b045145e26a Mon Sep 17 00:00:00 2001 From: paulyu Date: Wed, 19 Apr 2023 12:07:37 -0400 Subject: [PATCH 37/44] fix linter issue --- network/network_windows.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/network/network_windows.go b/network/network_windows.go index b117b42871..679c002005 100644 --- a/network/network_windows.go +++ b/network/network_windows.go @@ -400,7 +400,7 @@ func (nm *networkManager) newNetworkImplHnsV2(nwInfo *NetworkInfo, extIf *extern // only add net rules if it's dualStackOverlay mode and hnsNetwork is created at first time if string(util.DualStackOverlay) != "" { - if err = nm.addNewNetRules(nwInfo); err != nil { + if err := nm.addNewNetRules(nwInfo); err != nil { // nolint return nil, err } } From ed152f13b8bf03bc86ebd69e7dad9dd898df2c42 Mon Sep 17 00:00:00 2001 From: paulyu Date: Wed, 19 Apr 2023 13:20:57 -0400 Subject: [PATCH 38/44] fix UT --- cni/network/network.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cni/network/network.go b/cni/network/network.go index 9ecc246f20..27f230e867 100644 --- a/cni/network/network.go +++ b/cni/network/network.go @@ -484,7 +484,7 @@ func (plugin *NetPlugin) Add(args *cniSkel.CmdArgs) error { // Issue link: https://github.com/kubernetes/kubernetes/issues/57253 if nwInfoErr == nil { - log.Printf("[cni-net] Found network %v with subnet %v.", networkID, nwInfo.Subnets[0].Prefix.String()) + log.Printf("[cni-net] Found network %v.", networkID) nwInfo.IPAMType = nwCfg.IPAM.Type options = nwInfo.Options From 0e9828af211f44f80b910412351ced45b0e37ebc Mon Sep 17 00:00:00 2001 From: paulyu Date: Thu, 20 Apr 2023 11:34:17 -0400 Subject: [PATCH 39/44] fix comments --- cni/network/invoker_azure.go | 2 +- cni/network/invoker_cns.go | 12 +++++------- cni/network/invoker_cns_test.go | 4 ++-- cni/network/network.go | 13 ++++++++----- cni/network/network_test.go | 2 +- network/network_windows.go | 9 ++++----- 6 files changed, 21 insertions(+), 21 deletions(-) diff --git a/cni/network/invoker_azure.go b/cni/network/invoker_azure.go index ec204222d4..adf70dc23e 100644 --- a/cni/network/invoker_azure.go +++ b/cni/network/invoker_azure.go @@ -154,7 +154,7 @@ func (invoker *AzureIPAMInvoker) Delete(address *net.IPNet, nwCfg *cni.NetworkCo nwCfgIpv6.IPAM.Address = address.IP.String() if len(invoker.nwInfo.Subnets) > 1 { for _, subnet := range invoker.nwInfo.Subnets { - if subnet.Prefix.IP.To16() != nil { + if subnet.Prefix.IP.To4() == nil { nwCfgIpv6.IPAM.Subnet = subnet.Prefix.String() } } diff --git a/cni/network/invoker_cns.go b/cni/network/invoker_cns.go index 44c5065d1e..08b9a088ab 100644 --- a/cni/network/invoker_cns.go +++ b/cni/network/invoker_cns.go @@ -81,9 +81,9 @@ func (invoker *CNSIPAMInvoker) Add(addConfig IPAMAddConfig) (IPAMAddResult, erro log.Printf("Requesting IP for pod %+v using ipconfigs %+v", podInfo, ipconfigs) response, err := invoker.cnsClient.RequestIPs(context.TODO(), ipconfigs) if err != nil { - // if RequestIPs call fails, we may receive API not Found error as new CNS is not supported, then try old API RequestIPAddress - log.Errorf("RequestIPs not supported by CNS. Invoking RequestIPAddress API with infracontainerid %s", ipconfigs.InfraContainerID) if cnscli.IsUnsupportedAPI(err) { + // If RequestIPs is not supported by CNS, use RequestIPAddress API + log.Errorf("RequestIPs not supported by CNS. Invoking RequestIPAddress API with infracontainerid %s", ipconfigs.InfraContainerID) ipconfig := cns.IPConfigRequest{ OrchestratorContext: orchestratorContext, PodInterfaceID: GetEndpointID(addConfig.args), @@ -122,14 +122,12 @@ func (invoker *CNSIPAMInvoker) Add(addConfig IPAMAddConfig) (IPAMAddResult, erro } // set the NC Primary IP in options - // SNATIPKey set is not for ipv6 + // SNATIPKey is not set for ipv6 if net.ParseIP(info.ncPrimaryIP).To4() != nil { addConfig.options[network.SNATIPKey] = info.ncPrimaryIP } log.Printf("[cni-invoker-cns] Received info %+v for pod %v", info, podInfo) - - // set result ipconfigArgument from CNS Response Body 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") @@ -296,9 +294,9 @@ func (invoker *CNSIPAMInvoker) Delete(address *net.IPNet, nwCfg *cni.NetworkConf } if err := invoker.cnsClient.ReleaseIPs(context.TODO(), ipConfigs); err != nil { - // if ReleaseIPs call fails, we may receive API not Found error as new CNS is not supported, then try old API ReleaseIPAddress - log.Errorf("ReleaseIPs not supported by CNS. Invoking ReleaseIPAddress API. Request: %v", ipConfigs) if cnscli.IsUnsupportedAPI(err) { + // If ReleaseIPs is not supported by CNS, use ReleaseIPAddress API + log.Errorf("ReleaseIPs not supported by CNS. Invoking ReleaseIPAddress API. Request: %v", ipConfigs) ipConfig := cns.IPConfigRequest{ OrchestratorContext: orchestratorContext, PodInterfaceID: GetEndpointID(args), diff --git a/cni/network/invoker_cns_test.go b/cni/network/invoker_cns_test.go index ab4322d706..8efc24ad8f 100644 --- a/cni/network/invoker_cns_test.go +++ b/cni/network/invoker_cns_test.go @@ -68,7 +68,7 @@ func TestCNSIPAMInvoker_Add(t *testing.T) { wantErr bool }{ { - name: "Test happy CNI add for IPv4", + name: "Test happy CNI add", fields: fields{ podName: testPodInfo.PodName, podNamespace: testPodInfo.PodNamespace, @@ -135,7 +135,7 @@ func TestCNSIPAMInvoker_Add(t *testing.T) { wantErr: false, }, { - name: "Test happy CNI add ipv4 + ipv6 as dualstack mode", + name: "Test happy CNI add ipv4 and ipv6", fields: fields{ podName: testPodInfo.PodName, podNamespace: testPodInfo.PodNamespace, diff --git a/cni/network/network.go b/cni/network/network.go index 27f230e867..fb6b1f6500 100644 --- a/cni/network/network.go +++ b/cni/network/network.go @@ -484,7 +484,7 @@ func (plugin *NetPlugin) Add(args *cniSkel.CmdArgs) error { // Issue link: https://github.com/kubernetes/kubernetes/issues/57253 if nwInfoErr == nil { - log.Printf("[cni-net] Found network %v.", networkID) + log.Printf("[cni-net] Found network %v with subnet %v.", networkID, nwInfo.Subnets[0].Prefix.String()) nwInfo.IPAMType = nwCfg.IPAM.Type options = nwInfo.Options @@ -657,7 +657,7 @@ func (plugin *NetPlugin) createNetworkInternal( ServiceCidrs: ipamAddConfig.nwCfg.ServiceCidrs, } - constructNetworkInfo(ipamAddResult, podSubnetPrefix, podSubnetV6Prefix, nwInfo) + addSubnetToNetworkInfo(ipamAddResult, podSubnetPrefix, podSubnetV6Prefix, nwInfo) setNetworkOptions(ipamAddResult.ncResponse, &nwInfo) err = plugin.nm.CreateNetwork(&nwInfo) @@ -669,7 +669,7 @@ func (plugin *NetPlugin) createNetworkInternal( } // construct network info with ipv4/ipv6 subnets -func constructNetworkInfo(ipamAddResult IPAMAddResult, podSubnetPrefix, podSubnetV6Prefix *net.IPNet, nwInfo network.NetworkInfo) { +func addSubnetToNetworkInfo(ipamAddResult IPAMAddResult, podSubnetPrefix, podSubnetV6Prefix *net.IPNet, nwInfo network.NetworkInfo) { ipv4Subnet := network.SubnetInfo{ Family: platform.AfINET, Prefix: *podSubnetPrefix, @@ -764,8 +764,11 @@ func (plugin *NetPlugin) createEndpointInternal(opt *createEndpointInternalOpt) } if opt.resultV6 != nil { - // get IPAM mode from conflist file and add ipv6 routes to linux pod if needed - epInfo.IPV6Mode = string(util.IpamMode(opt.nwCfg.IPAM.Mode)) + // inject ipv6 routes to Linux pod + ipamMode := string(util.IpamMode(opt.nwCfg.IPAM.Mode)) + if ipamMode == "dualStackOverlay" { + epInfo.IPV6Mode = ipamMode + } log.Printf("current ipv6 mode is %s", epInfo.IPV6Mode) for _, ipconfig := range opt.resultV6.IPs { diff --git a/cni/network/network_test.go b/cni/network/network_test.go index c617f257ee..362fb87947 100644 --- a/cni/network/network_test.go +++ b/cni/network/network_test.go @@ -1107,7 +1107,7 @@ func TestGetPodSubnetNatInfoV6(t *testing.T) { ncPrimaryIP := "2001:2002:2003::1" nwCfg := &cni.NetworkConfig{ExecutionMode: string(util.V4Swift)} natInfo := getNATInfo(nwCfg, ncPrimaryIP, false) - // should not add any natInfo to policy if ncPrimaryIP is ipv6 + // should not add any natInfo to policy if ipam mode is neither V4Overlay and DualStackOverlay if runtime.GOOS == "windows" { require.Equalf(t, natInfo, []policy.NATInfo{ {}, diff --git a/network/network_windows.go b/network/network_windows.go index 679c002005..b5fa5c816d 100644 --- a/network/network_windows.go +++ b/network/network_windows.go @@ -210,9 +210,9 @@ func (nm *networkManager) addNewNetRules(nwInfo *NetworkInfo) error { prefix := subnet.Prefix.String() gateway := subnet.Gateway.String() - ip, _, er := net.ParseCIDR(prefix) - if er != nil { - return fmt.Errorf("[net] failed to parse prefix %s", prefix) // nolint + ip, _, err = net.ParseCIDR(prefix) + if err != nil { + return fmt.Errorf("[net] failed to parse prefix %s due to %+v", prefix, err) // nolint } if ip.To4() != nil { // netsh interface ipv4 add route $subnetV4 $hostInterfaceAlias "0.0.0.0" @@ -223,7 +223,6 @@ func (nm *networkManager) addNewNetRules(nwInfo *NetworkInfo) error { // netsh interface ipv4 add route $subnetV4 $hostInterfaceAlias $gatewayV4 netshV4GatewayRoute := fmt.Sprintf(netRouteCmd, "ipv4", prefix, ifName, gateway) - log.Printf("[net] Adding ipv4 gateway route failed: %v:%v", out, err) if out, err = nm.plClient.ExecuteCommand(netshV4GatewayRoute); err != nil { log.Printf("[net] Adding ipv4 gateway route failed: %v:%v", out, err) } @@ -397,10 +396,10 @@ func (nm *networkManager) newNetworkImplHnsV2(nwInfo *NetworkInfo, extIf *extern } log.Printf("[net] Successfully created hcn network with response: %+v", hnsResponse) - // only add net rules if it's dualStackOverlay mode and hnsNetwork is created at first time if string(util.DualStackOverlay) != "" { if err := nm.addNewNetRules(nwInfo); err != nil { // nolint + log.Printf("[net] Failed to add net rules due to %+v", err) return nil, err } } From e6a579f75c92abf8e210a055e66d60a8c226a69a Mon Sep 17 00:00:00 2001 From: paulyu Date: Thu, 20 Apr 2023 17:26:07 -0400 Subject: [PATCH 40/44] comments fix --- cni/network/invoker_azure.go | 1 + cni/network/invoker_cns_test.go | 355 ++++++++++++++++++-- cni/network/network.go | 44 ++- cni/network/network_test.go | 15 +- network/network_windows.go | 15 +- network/transparent_endpointclient_linux.go | 2 + 6 files changed, 374 insertions(+), 58 deletions(-) diff --git a/cni/network/invoker_azure.go b/cni/network/invoker_azure.go index adf70dc23e..360aab0f76 100644 --- a/cni/network/invoker_azure.go +++ b/cni/network/invoker_azure.go @@ -156,6 +156,7 @@ func (invoker *AzureIPAMInvoker) Delete(address *net.IPNet, nwCfg *cni.NetworkCo for _, subnet := range invoker.nwInfo.Subnets { if subnet.Prefix.IP.To4() == nil { nwCfgIpv6.IPAM.Subnet = subnet.Prefix.String() + break } } } diff --git a/cni/network/invoker_cns_test.go b/cni/network/invoker_cns_test.go index 8efc24ad8f..2888c19f7d 100644 --- a/cni/network/invoker_cns_test.go +++ b/cni/network/invoker_cns_test.go @@ -44,6 +44,230 @@ func getTestOverlayGateway() net.IP { return net.ParseIP("169.254.1.1") } +func TestCNSIPAMInvoker_Add_Overlay(t *testing.T) { + require := require.New(t) //nolint further usage of require without passing t + + // set new CNS API is not supported + unsupportedAPIs := make(map[cnsAPIName]struct{}) + unsupportedAPIs["RequestIPs"] = struct{}{} + + type fields struct { + podName string + podNamespace string + cnsClient cnsclient + ipamMode util.IpamMode + } + type args struct { + nwCfg *cni.NetworkConfig + args *cniSkel.CmdArgs + hostSubnetPrefix *net.IPNet + options map[string]interface{} + } + + tests := []struct { + name string + fields fields + args args + wantIpv4Result *cniTypesCurr.Result + wantIpv6Result *cniTypesCurr.Result + wantErr bool + }{ + { + name: "Test happy CNI Overlay add in v4overlay ipamMode", + fields: fields{ + podName: testPodInfo.PodName, + podNamespace: testPodInfo.PodNamespace, + ipamMode: util.V4Overlay, + cnsClient: &MockCNSClient{ + unsupportedAPIs: unsupportedAPIs, + require: require, + requestIP: requestIPAddressHandler{ + ipconfigArgument: cns.IPConfigRequest{ + PodInterfaceID: "testcont-testifname3", + InfraContainerID: "testcontainerid3", + OrchestratorContext: marshallPodInfo(testPodInfo), + }, + result: &cns.IPConfigResponse{ + PodIpInfo: cns.PodIpInfo{ + PodIPConfig: cns.IPSubnet{ + IPAddress: "10.240.1.242", + PrefixLength: 16, + }, + NetworkContainerPrimaryIPConfig: cns.IPConfiguration{ + IPSubnet: cns.IPSubnet{ + IPAddress: "10.240.1.0", + PrefixLength: 16, + }, + DNSServers: nil, + GatewayIPAddress: "", + }, + HostPrimaryIPInfo: cns.HostIPInfo{ + Gateway: "10.224.0.1", + PrimaryIP: "10.224.0.5", + Subnet: "10.224.0.0/16", + }, + }, + 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.224.0.0/16"), + options: map[string]interface{}{}, + }, + wantIpv4Result: &cniTypesCurr.Result{ + IPs: []*cniTypesCurr.IPConfig{ + { + Address: *getCIDRNotationForAddress("10.240.1.242/16"), + Gateway: getTestOverlayGateway(), + }, + }, + Routes: []*cniTypes.Route{ + { + Dst: network.Ipv4DefaultRouteDstPrefix, + GW: getTestOverlayGateway(), + }, + }, + }, + wantIpv6Result: nil, + wantErr: false, + }, + { + name: "Test happy CNI Overlay add in dualstack overlay ipamMode", + fields: fields{ + podName: testPodInfo.PodName, + podNamespace: testPodInfo.PodNamespace, + cnsClient: &MockCNSClient{ + require: require, + requestIPs: requestIPsHandler{ + ipconfigArgument: getTestIPConfigsRequest(), + 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", + }, + }, + { + PodIPConfig: cns.IPSubnet{ + IPAddress: "fd11:1234::1", + PrefixLength: 24, + }, + NetworkContainerPrimaryIPConfig: cns.IPConfiguration{ + IPSubnet: cns.IPSubnet{ + IPAddress: "fd11:1234::", + PrefixLength: 112, + }, + DNSServers: nil, + GatewayIPAddress: "fe80::1234:5678:9abc", + }, + HostPrimaryIPInfo: cns.HostIPInfo{ + Gateway: "fe80::1234:5678:9abc", + PrimaryIP: "fe80::1234:5678:9abc", + Subnet: "fd11:1234::/112", + }, + }, + }, + Response: cns.Response{ + ReturnCode: 0, + Message: "", + }, + }, + err: nil, + }, + }, + }, + args: args{ + nwCfg: &cni.NetworkConfig{}, + args: &cniSkel.CmdArgs{ + ContainerID: "testcontainerid", + Netns: "testnetns", + IfName: "testifname", + }, + hostSubnetPrefix: getCIDRNotationForAddress("10.0.0.1/24"), + options: map[string]interface{}{}, + }, + wantIpv4Result: &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.Ipv6DefaultRouteDstPrefix, + GW: net.ParseIP("fe80::1234:5678:9abc"), + }, + }, + }, + wantErr: false, + }, + } + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + invoker := &CNSIPAMInvoker{ + podName: tt.fields.podName, + podNamespace: tt.fields.podNamespace, + cnsClient: tt.fields.cnsClient, + } + if tt.fields.ipamMode != "" { + invoker.ipamMode = tt.fields.ipamMode + } + ipamAddResult, err := invoker.Add(IPAMAddConfig{nwCfg: tt.args.nwCfg, args: tt.args.args, options: tt.args.options}) + if tt.wantErr { + require.Error(err) + } else { + 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") + }) + } +} + func TestCNSIPAMInvoker_Add(t *testing.T) { require := require.New(t) //nolint further usage of require without passing t type fields struct { @@ -60,12 +284,12 @@ func TestCNSIPAMInvoker_Add(t *testing.T) { } tests := []struct { - name string - fields fields - args args - want *cniTypesCurr.Result - want1 *cniTypesCurr.Result - wantErr bool + name string + fields fields + args args + wantIpv4Result *cniTypesCurr.Result + wantIpv6Result *cniTypesCurr.Result + wantErr bool }{ { name: "Test happy CNI add", @@ -117,7 +341,7 @@ func TestCNSIPAMInvoker_Add(t *testing.T) { hostSubnetPrefix: getCIDRNotationForAddress("10.0.0.1/24"), options: map[string]interface{}{}, }, - want: &cniTypesCurr.Result{ + wantIpv4Result: &cniTypesCurr.Result{ IPs: []*cniTypesCurr.IPConfig{ { Address: *getCIDRNotationForAddress("10.0.1.10/24"), @@ -131,11 +355,11 @@ func TestCNSIPAMInvoker_Add(t *testing.T) { }, }, }, - want1: nil, - wantErr: false, + wantIpv6Result: nil, + wantErr: false, }, { - name: "Test happy CNI add ipv4 and ipv6", + name: "Test happy CNI add for both ipv4 and ipv6", fields: fields{ podName: testPodInfo.PodName, podNamespace: testPodInfo.PodNamespace, @@ -203,7 +427,7 @@ func TestCNSIPAMInvoker_Add(t *testing.T) { hostSubnetPrefix: getCIDRNotationForAddress("10.0.0.1/24"), options: map[string]interface{}{}, }, - want: &cniTypesCurr.Result{ + wantIpv4Result: &cniTypesCurr.Result{ IPs: []*cniTypesCurr.IPConfig{ { Address: *getCIDRNotationForAddress("10.0.1.10/24"), @@ -217,7 +441,7 @@ func TestCNSIPAMInvoker_Add(t *testing.T) { }, }, }, - want1: &cniTypesCurr.Result{ + wantIpv6Result: &cniTypesCurr.Result{ IPs: []*cniTypesCurr.IPConfig{ { Address: *getCIDRNotationForAddress("fd11:1234::1/112"), @@ -268,9 +492,9 @@ func TestCNSIPAMInvoker_Add(t *testing.T) { require.NoError(err) } - fmt.Printf("want:%+v\nrest:%+v\n", tt.want, ipamAddResult.ipv4Result) - require.Equalf(tt.want, ipamAddResult.ipv4Result, "incorrect ipv4 response") - require.Equalf(tt.want1, ipamAddResult.ipv6Result, "incorrect ipv6 response") + 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") }) } } @@ -390,7 +614,7 @@ func TestCNSIPAMInvoker_Add_UnsupportedAPI(t *testing.T) { } } -func TestCNSIPAMInvoker_Add_NotFound(t *testing.T) { +func TestAddAPIFail(t *testing.T) { require := require.New(t) //nolint further usage of require without passing t type fields struct { @@ -415,7 +639,7 @@ func TestCNSIPAMInvoker_Add_NotFound(t *testing.T) { wantErr bool }{ { - name: "Test happy CNI add for dualstack mode with both requestIP and requestIPs not working", + name: "Test happy CNI add for dualstack mode with both requestIP and requestIPs get failed", fields: fields{ podName: testPodInfo.PodName, podNamespace: testPodInfo.PodNamespace, @@ -447,7 +671,7 @@ func TestCNSIPAMInvoker_Add_NotFound(t *testing.T) { { PodIPConfig: cns.IPSubnet{ IPAddress: "fd11:1234::1", - PrefixLength: 24, + PrefixLength: 112, }, NetworkContainerPrimaryIPConfig: cns.IPConfiguration{ IPSubnet: cns.IPSubnet{ @@ -581,6 +805,97 @@ func TestCNSIPAMInvoker_Delete(t *testing.T) { } } +func TestCNSIPAMInvoker_Delete_Overlay(t *testing.T) { + require := require.New(t) //nolint further usage of require without passing t + + // set new CNS API is not supported + unsupportedAPIs := make(map[cnsAPIName]struct{}) + unsupportedAPIs["ReleaseIPs"] = struct{}{} + + type fields struct { + podName string + podNamespace string + cnsClient cnsclient + ipamMode util.IpamMode + } + type args struct { + address *net.IPNet + nwCfg *cni.NetworkConfig + args *cniSkel.CmdArgs + options map[string]interface{} + } + tests := []struct { + name string + fields fields + args args + wantErr bool + }{ + { + name: "test delete happy path in v4overlay ipamMode", + fields: fields{ + podName: testPodInfo.PodName, + podNamespace: testPodInfo.PodNamespace, + ipamMode: util.V4Overlay, + cnsClient: &MockCNSClient{ + unsupportedAPIs: unsupportedAPIs, + require: require, + releaseIP: releaseIPHandler{ + ipconfigArgument: getTestIPConfigRequest(), + }, + }, + }, + args: args{ + nwCfg: nil, + args: &cniSkel.CmdArgs{ + ContainerID: "testcontainerid", + Netns: "testnetns", + IfName: "testifname", + }, + options: map[string]interface{}{}, + }, + }, + { + name: "test delete happy path in dualStackOverlay ipamMode", + fields: fields{ + podName: testPodInfo.PodName, + podNamespace: testPodInfo.PodNamespace, + ipamMode: util.DualStackOverlay, + cnsClient: &MockCNSClient{ + require: require, + releaseIPs: releaseIPsHandler{ + ipconfigArgument: getTestIPConfigsRequest(), + }, + }, + }, + args: args{ + nwCfg: nil, + args: &cniSkel.CmdArgs{ + ContainerID: "testcontainerid", + Netns: "testnetns", + IfName: "testifname", + }, + options: map[string]interface{}{}, + }, + }, + } + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + invoker := &CNSIPAMInvoker{ + podName: tt.fields.podName, + podNamespace: tt.fields.podNamespace, + cnsClient: tt.fields.cnsClient, + } + err := invoker.Delete(tt.args.address, tt.args.nwCfg, tt.args.args, tt.args.options) + if tt.wantErr { + require.Error(err) + } else { + require.NoError(err) + } + }) + } +} + func TestCNSIPAMInvoker_Delete_NotSupportedAPI(t *testing.T) { require := require.New(t) //nolint further usage of require without passing t // set new CNS API is not supported @@ -646,7 +961,7 @@ func TestCNSIPAMInvoker_Delete_NotSupportedAPI(t *testing.T) { } } -func TestCNSIPAMInvoker_Delete_NotFound(t *testing.T) { +func TestDeleteAPIFail(t *testing.T) { require := require.New(t) //nolint further usage of require without passing t type fields struct { podName string @@ -666,7 +981,7 @@ func TestCNSIPAMInvoker_Delete_NotFound(t *testing.T) { wantErr bool }{ { - name: "test delete happy path with both ReleaseIPs and ReleassIP not working", + name: "test delete with both cns releaseIPs and releaseIP get failed", fields: fields{ podName: testPodInfo.PodName, podNamespace: testPodInfo.PodNamespace, diff --git a/cni/network/network.go b/cni/network/network.go index fb6b1f6500..53606c9a7c 100644 --- a/cni/network/network.go +++ b/cni/network/network.go @@ -624,21 +624,6 @@ func (plugin *NetPlugin) createNetworkInternal( log.Printf("[cni-net] nwDNSInfo: %v", nwDNSInfo) - var podSubnetPrefix *net.IPNet - _, podSubnetPrefix, err = net.ParseCIDR(ipamAddResult.ipv4Result.IPs[0].Address.String()) - if err != nil { - return nwInfo, fmt.Errorf("Failed to ParseCIDR for pod subnet prefix: %w", err) - } - - // parse the ipv6 address and only add it to nwInfo if it's dual stack mode - var podSubnetV6Prefix *net.IPNet - if ipamAddResult.ipv6Result != nil && len(ipamAddResult.ipv6Result.IPs) > 0 { - _, podSubnetV6Prefix, err = net.ParseCIDR(ipamAddResult.ipv6Result.IPs[0].Address.String()) - if err != nil { - return nwInfo, fmt.Errorf("Failed to ParseCIDR for pod subnet IPv6 prefix: %w", err) - } - } - // Create the network. nwInfo = network.NetworkInfo{ Id: networkID, @@ -657,7 +642,10 @@ func (plugin *NetPlugin) createNetworkInternal( ServiceCidrs: ipamAddConfig.nwCfg.ServiceCidrs, } - addSubnetToNetworkInfo(ipamAddResult, podSubnetPrefix, podSubnetV6Prefix, nwInfo) + if err := addSubnetToNetworkInfo(ipamAddResult, nwInfo); err != nil { + log.Printf("[cni-net] Failed to add subnets to networkInfo due to %+v", err) + return nwInfo, err + } setNetworkOptions(ipamAddResult.ncResponse, &nwInfo) err = plugin.nm.CreateNetwork(&nwInfo) @@ -669,7 +657,25 @@ func (plugin *NetPlugin) createNetworkInternal( } // construct network info with ipv4/ipv6 subnets -func addSubnetToNetworkInfo(ipamAddResult IPAMAddResult, podSubnetPrefix, podSubnetV6Prefix *net.IPNet, nwInfo network.NetworkInfo) { +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) + } + + // 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()) + if err != nil { + return fmt.Errorf("Failed to ParseCIDR for pod subnet IPv6 prefix: %w", err) + } + } + ipv4Subnet := network.SubnetInfo{ Family: platform.AfINET, Prefix: *podSubnetPrefix, @@ -685,6 +691,8 @@ func addSubnetToNetworkInfo(ipamAddResult IPAMAddResult, podSubnetPrefix, podSub } nwInfo.Subnets = append(nwInfo.Subnets, ipv6Subnet) } + + return nil } type createEndpointInternalOpt struct { @@ -764,7 +772,7 @@ func (plugin *NetPlugin) createEndpointInternal(opt *createEndpointInternalOpt) } if opt.resultV6 != nil { - // inject ipv6 routes to Linux pod + // inject ipv6 routes to Linux pod if ipamMode is dual stack overlay ipamMode := string(util.IpamMode(opt.nwCfg.IPAM.Mode)) if ipamMode == "dualStackOverlay" { epInfo.IPV6Mode = ipamMode diff --git a/cni/network/network_test.go b/cni/network/network_test.go index 362fb87947..9d72757557 100644 --- a/cni/network/network_test.go +++ b/cni/network/network_test.go @@ -70,13 +70,14 @@ func TestMain(m *testing.M) { func GetTestResources() *NetPlugin { pluginName := "testplugin" + isIPv6 := false config := &common.PluginConfig{} grpcClient := &nns.MockGrpcClient{} plugin, _ := NewPlugin(pluginName, config, grpcClient, &Multitenancy{}) plugin.report = &telemetry.CNIReport{} mockNetworkManager := acnnetwork.NewMockNetworkmanager() plugin.nm = mockNetworkManager - plugin.ipamInvoker = NewMockIpamInvoker(true, false, false) // enable ipv6 flag for dualstack test cases + plugin.ipamInvoker = NewMockIpamInvoker(isIPv6, false, false) return plugin } @@ -1102,15 +1103,3 @@ func TestGetPodSubnetNatInfo(t *testing.T) { require.Empty(t, natInfo, "linux podsubnet natInfo should be empty") } } - -func TestGetPodSubnetNatInfoV6(t *testing.T) { - ncPrimaryIP := "2001:2002:2003::1" - nwCfg := &cni.NetworkConfig{ExecutionMode: string(util.V4Swift)} - natInfo := getNATInfo(nwCfg, ncPrimaryIP, false) - // should not add any natInfo to policy if ipam mode is neither V4Overlay and DualStackOverlay - if runtime.GOOS == "windows" { - require.Equalf(t, natInfo, []policy.NATInfo{ - {}, - }, "no ipv6 natInfo is added") - } -} diff --git a/network/network_windows.go b/network/network_windows.go index b5fa5c816d..db92bf5b89 100644 --- a/network/network_windows.go +++ b/network/network_windows.go @@ -388,6 +388,14 @@ func (nm *networkManager) newNetworkImplHnsV2(nwInfo *NetworkInfo, extIf *extern if err != nil { // if network not found, create the HNS network. if errors.As(err, &hcn.NetworkNotFoundError{}) { + // in dualStackOverlay mode, need to add net routes to windows node + // check if it is dualStackOverlay mode and net routes are not existing + if string(util.DualStackOverlay) { + if err := nm.addNewNetRules(nwInfo); err != nil { // nolint + log.Printf("[net] Failed to add net rules due to %+v", err) + return nil, err + } + } log.Printf("[net] Creating hcn network: %+v", hcnNetwork) hnsResponse, err = Hnsv2.CreateNetwork(hcnNetwork) @@ -396,13 +404,6 @@ func (nm *networkManager) newNetworkImplHnsV2(nwInfo *NetworkInfo, extIf *extern } log.Printf("[net] Successfully created hcn network with response: %+v", hnsResponse) - // only add net rules if it's dualStackOverlay mode and hnsNetwork is created at first time - if string(util.DualStackOverlay) != "" { - if err := nm.addNewNetRules(nwInfo); err != nil { // nolint - log.Printf("[net] Failed to add net rules due to %+v", err) - return nil, err - } - } } else { // we can't validate if the network already exists, don't continue return nil, fmt.Errorf("Failed to create hcn network: %s, failed to query for existing network with error: %v", hcnNetwork.Name, err) diff --git a/network/transparent_endpointclient_linux.go b/network/transparent_endpointclient_linux.go index 013515af51..5fe3dcdf9f 100644 --- a/network/transparent_endpointclient_linux.go +++ b/network/transparent_endpointclient_linux.go @@ -262,6 +262,8 @@ func (client *TransparentEndpointClient) ConfigureContainerInterfacesAndRoutes(e return fmt.Errorf("Adding arp in container failed: %w", err) } + // IPv6Mode can be ipv6NAT or dual stack overlay + // set epInfo ipv6Mode to 'dualStackOverlay' to set ipv6Routes and ipv6NeighborEntries for Linux pod in dualstackOverlay ipam mode if epInfo.IPV6Mode != "" { if err := client.setupIPV6Routes(); err != nil { return err From 159a0da0ac7afd29b58b7cf89fce1ad565d1b425 Mon Sep 17 00:00:00 2001 From: paulyu Date: Thu, 20 Apr 2023 18:54:23 -0400 Subject: [PATCH 41/44] fix comments --- cni/network/network.go | 2 +- network/network_windows.go | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/cni/network/network.go b/cni/network/network.go index 53606c9a7c..55297c83ea 100644 --- a/cni/network/network.go +++ b/cni/network/network.go @@ -642,7 +642,7 @@ func (plugin *NetPlugin) createNetworkInternal( ServiceCidrs: ipamAddConfig.nwCfg.ServiceCidrs, } - if err := addSubnetToNetworkInfo(ipamAddResult, nwInfo); err != nil { + if err = addSubnetToNetworkInfo(ipamAddResult, nwInfo); err != nil { log.Printf("[cni-net] Failed to add subnets to networkInfo due to %+v", err) return nwInfo, err } diff --git a/network/network_windows.go b/network/network_windows.go index db92bf5b89..dedde9e981 100644 --- a/network/network_windows.go +++ b/network/network_windows.go @@ -390,7 +390,8 @@ func (nm *networkManager) newNetworkImplHnsV2(nwInfo *NetworkInfo, extIf *extern if errors.As(err, &hcn.NetworkNotFoundError{}) { // in dualStackOverlay mode, need to add net routes to windows node // check if it is dualStackOverlay mode and net routes are not existing - if string(util.DualStackOverlay) { + + if string(util.DualStackOverlay) != "" { if err := nm.addNewNetRules(nwInfo); err != nil { // nolint log.Printf("[net] Failed to add net rules due to %+v", err) return nil, err From a81a42a021ebbc37d6b09ecdf257770f1071da15 Mon Sep 17 00:00:00 2001 From: paulyu Date: Thu, 20 Apr 2023 21:26:18 -0400 Subject: [PATCH 42/44] add fixes for comments --- cni/azure-linux-swift-overlay-dualstack.conflist | 1 - ...zure-windows-swift-overlay-dualstack.conflist | 1 - cni/network/invoker_cns_test.go | 4 ++-- cni/network/network.go | 16 +++++++--------- network/network_windows.go | 7 +++---- 5 files changed, 12 insertions(+), 17 deletions(-) diff --git a/cni/azure-linux-swift-overlay-dualstack.conflist b/cni/azure-linux-swift-overlay-dualstack.conflist index 2c4ea7a5b7..92bd31a628 100644 --- a/cni/azure-linux-swift-overlay-dualstack.conflist +++ b/cni/azure-linux-swift-overlay-dualstack.conflist @@ -5,7 +5,6 @@ { "type":"azure-vnet", "mode":"transparent", - "executionMode":"v4swift", "ipsToRouteViaHost":["169.254.20.10"], "ipam":{ "type":"azure-cns", diff --git a/cni/azure-windows-swift-overlay-dualstack.conflist b/cni/azure-windows-swift-overlay-dualstack.conflist index e3936ea335..c84e56d19b 100644 --- a/cni/azure-windows-swift-overlay-dualstack.conflist +++ b/cni/azure-windows-swift-overlay-dualstack.conflist @@ -7,7 +7,6 @@ "type": "azure-vnet", "mode": "bridge", "bridge": "azure0", - "executionMode": "v4swift", "capabilities": { "portMappings": true, "dns": true diff --git a/cni/network/invoker_cns_test.go b/cni/network/invoker_cns_test.go index 2888c19f7d..9d3325440a 100644 --- a/cni/network/invoker_cns_test.go +++ b/cni/network/invoker_cns_test.go @@ -614,7 +614,7 @@ func TestCNSIPAMInvoker_Add_UnsupportedAPI(t *testing.T) { } } -func TestAddAPIFail(t *testing.T) { +func TestRequestIPAPIsFail(t *testing.T) { require := require.New(t) //nolint further usage of require without passing t type fields struct { @@ -961,7 +961,7 @@ func TestCNSIPAMInvoker_Delete_NotSupportedAPI(t *testing.T) { } } -func TestDeleteAPIFail(t *testing.T) { +func TestReleaseIPAPIsFail(t *testing.T) { require := require.New(t) //nolint further usage of require without passing t type fields struct { podName string diff --git a/cni/network/network.go b/cni/network/network.go index 55297c83ea..f4df5ff11d 100644 --- a/cni/network/network.go +++ b/cni/network/network.go @@ -668,14 +668,6 @@ func addSubnetToNetworkInfo(ipamAddResult IPAMAddResult, nwInfo network.NetworkI return fmt.Errorf("Failed to ParseCIDR for pod subnet prefix: %w", err) } - // 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()) - if err != nil { - return fmt.Errorf("Failed to ParseCIDR for pod subnet IPv6 prefix: %w", err) - } - } - ipv4Subnet := network.SubnetInfo{ Family: platform.AfINET, Prefix: *podSubnetPrefix, @@ -683,7 +675,13 @@ func addSubnetToNetworkInfo(ipamAddResult IPAMAddResult, nwInfo network.NetworkI } 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()) + if err != nil { + return fmt.Errorf("Failed to ParseCIDR for pod subnet IPv6 prefix: %w", err) + } + ipv6Subnet := network.SubnetInfo{ Family: platform.AfINET6, Prefix: *podSubnetV6Prefix, @@ -774,7 +772,7 @@ func (plugin *NetPlugin) createEndpointInternal(opt *createEndpointInternalOpt) if opt.resultV6 != nil { // inject ipv6 routes to Linux pod if ipamMode is dual stack overlay ipamMode := string(util.IpamMode(opt.nwCfg.IPAM.Mode)) - if ipamMode == "dualStackOverlay" { + if ipamMode == string(util.DualStackOverlay) { epInfo.IPV6Mode = ipamMode } log.Printf("current ipv6 mode is %s", epInfo.IPV6Mode) diff --git a/network/network_windows.go b/network/network_windows.go index dedde9e981..bacd751184 100644 --- a/network/network_windows.go +++ b/network/network_windows.go @@ -205,7 +205,7 @@ func (nm *networkManager) addNewNetRules(nwInfo *NetworkInfo) error { ifName = fmt.Sprintf("%s (%s)", ifNamePrefix, nwInfo.MasterIfName) } - // iterate subnet and add ipv4 and ipv6 default route and gateway + // iterate subnet and add ipv4 and ipv6 default route and gateway only if it is not existing for _, subnet := range nwInfo.Subnets { prefix := subnet.Prefix.String() gateway := subnet.Gateway.String() @@ -389,9 +389,8 @@ func (nm *networkManager) newNetworkImplHnsV2(nwInfo *NetworkInfo, extIf *extern // if network not found, create the HNS network. if errors.As(err, &hcn.NetworkNotFoundError{}) { // in dualStackOverlay mode, need to add net routes to windows node - // check if it is dualStackOverlay mode and net routes are not existing - - if string(util.DualStackOverlay) != "" { + // check if it is dualStackOverlay mode + if nwInfo.IPV6Mode == string(util.DualStackOverlay) { if err := nm.addNewNetRules(nwInfo); err != nil { // nolint log.Printf("[net] Failed to add net rules due to %+v", err) return nil, err From cb0c3091214f9bb502ffe1fd488b9d174cbea2d7 Mon Sep 17 00:00:00 2001 From: paulyu Date: Thu, 20 Apr 2023 21:47:05 -0400 Subject: [PATCH 43/44] fix UT --- cni/network/network.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/cni/network/network.go b/cni/network/network.go index f4df5ff11d..6c1542d0cb 100644 --- a/cni/network/network.go +++ b/cni/network/network.go @@ -642,7 +642,7 @@ func (plugin *NetPlugin) createNetworkInternal( ServiceCidrs: ipamAddConfig.nwCfg.ServiceCidrs, } - if err = addSubnetToNetworkInfo(ipamAddResult, nwInfo); err != nil { + if nwInfo, err = addSubnetToNetworkInfo(ipamAddResult, nwInfo); err != nil { log.Printf("[cni-net] Failed to add subnets to networkInfo due to %+v", err) return nwInfo, err } @@ -657,7 +657,7 @@ func (plugin *NetPlugin) createNetworkInternal( } // construct network info with ipv4/ipv6 subnets -func addSubnetToNetworkInfo(ipamAddResult IPAMAddResult, nwInfo network.NetworkInfo) error { +func addSubnetToNetworkInfo(ipamAddResult IPAMAddResult, nwInfo network.NetworkInfo) (network.NetworkInfo, error) { var ( podSubnetPrefix *net.IPNet podSubnetV6Prefix *net.IPNet @@ -665,7 +665,7 @@ func addSubnetToNetworkInfo(ipamAddResult IPAMAddResult, nwInfo network.NetworkI _, 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) + return network.NetworkInfo{}, fmt.Errorf("Failed to ParseCIDR for pod subnet prefix: %w", err) } ipv4Subnet := network.SubnetInfo{ @@ -679,7 +679,7 @@ func addSubnetToNetworkInfo(ipamAddResult IPAMAddResult, nwInfo network.NetworkI if ipamAddResult.ipv6Result != nil && len(ipamAddResult.ipv6Result.IPs) > 0 { _, podSubnetV6Prefix, err = net.ParseCIDR(ipamAddResult.ipv6Result.IPs[0].Address.String()) if err != nil { - return fmt.Errorf("Failed to ParseCIDR for pod subnet IPv6 prefix: %w", err) + return network.NetworkInfo{}, fmt.Errorf("Failed to ParseCIDR for pod subnet IPv6 prefix: %w", err) } ipv6Subnet := network.SubnetInfo{ @@ -690,7 +690,7 @@ func addSubnetToNetworkInfo(ipamAddResult IPAMAddResult, nwInfo network.NetworkI nwInfo.Subnets = append(nwInfo.Subnets, ipv6Subnet) } - return nil + return nwInfo, nil } type createEndpointInternalOpt struct { From 3bb1d938743e092f1861c9a41ad83252458eddb6 Mon Sep 17 00:00:00 2001 From: paulyufan2 Date: Fri, 21 Apr 2023 12:30:40 -0400 Subject: [PATCH 44/44] fix a bug --- network/network_windows.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/network/network_windows.go b/network/network_windows.go index bacd751184..ddc8049a4c 100644 --- a/network/network_windows.go +++ b/network/network_windows.go @@ -210,9 +210,9 @@ func (nm *networkManager) addNewNetRules(nwInfo *NetworkInfo) error { prefix := subnet.Prefix.String() gateway := subnet.Gateway.String() - ip, _, err = net.ParseCIDR(prefix) - if err != nil { - return fmt.Errorf("[net] failed to parse prefix %s due to %+v", prefix, err) // nolint + ip, _, errParseCIDR := net.ParseCIDR(prefix) + if errParseCIDR != nil { + return fmt.Errorf("[net] failed to parse prefix %s due to %+v", prefix, errParseCIDR) // nolint } if ip.To4() != nil { // netsh interface ipv4 add route $subnetV4 $hostInterfaceAlias "0.0.0.0"