From 2ab9cfe8236150147b2f551700a9d9e55585f236 Mon Sep 17 00:00:00 2001 From: QxBytes <39818795+QxBytes@users.noreply.github.com> Date: Fri, 31 May 2024 18:36:32 -0700 Subject: [PATCH] refactor: code changes for stateless cni and swift v2 (#2688) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * ci: changes up to endpointInternal * ci: remove defaultInterface from invoker * ci: change up to CreateEndpoint * ci: changes up to CreateEndpoint() * ci: invoker cns and UT fixes * ci: add fixes to UT(s), capture non populated defaultInterface failures * ci: multitenancy changes * ci: invoker azure changes & remove all defaultInterfaceInfo possible * ci add NICType to baremetal flow * chore: address comments * merge nw info fields to ep info and draft new createEndpoint function * restruct ipamAddResult struct * reorder code to create epinfo first, and then create network and ep based on epinfo * add getNwInfo and generate ipamAddResult * fix network windows.go * create nw info first and create nw and ep dns info * fix testIpamAddFail ut referencing wrong redeclared err variable, fix error message * UT fix part one * fix the getNetworkID and getNetworkInfo * move create endpoint to network package, remove ifIndex as needed * use function to get network id * unify creation of nw and endpoint info by removing switch * change functions to consume ep info instead of nw info * remove unused variable accidentally added earlier * update old azure ipam invoker to use ep info and change ids to network ids when appropriate previously we renamed the NetworkInfo symbol to EndpointInfo in lots of places, but the Id in Network Info is NOT the same as Endpoint Info, so while the code compiles, code that previously used the id field of the network info struct would now be using the id field of the endpoint info struct. It should use the NetworkId field of the endpoint info struct instead. * rename endpoint info id field to EndpointID to remove ambiguity * change nw info to ep info in windows * adjust comments * move all methods in create ep info dependent on nw info to use ep info instead (windows and linux) addSubnetToNetworkInfo, setNetworkOptions, and getEndpointPolicies. getEndpointPolicies will now take just the subnets needed as a parameter rather than the whole nw or ep info. * make cnm compile (not necessarily correct) * make all tests compile except endpoint test secondary client (windows and linux) (not necessarily correct) * comment out endpoint test secondary endpoint client case to make tests compile * address todos and comments from meeting * remove duplicated code for populating address in ep info generation * update EndpointCreate to support multiple infra nic * save all endpoints to state, regardless of type, use either stateless or cni statefile undos some changes in "move create endpoint to network package, remove ifIndex as needed" deletion flow needs to figure out how to tell if the nic type is delegated 1 interface info : 1 endpoint info : 1 endpoint struct mapping * fix dual nic support conditional and finding master interface ip the master interface ip must be in a particular form where the last few bits are zeroed out based on the mask or we won't find the ip for example, while the host subnet perfix is 10.224.0.113/16, the ip that should be passed into find master interface (subnet) should be 10.224.0.0/16 which matches one of the interfaces' ipnet (10.224.0.0/16) * fix empty network name when we need to create a network, we collect the network information, but if we do not find the network, we return an empty nw info and an error when we create the endpoint we need to use endpoint info's network id, not the (possibly) empty network info struct's network id * make network_test.go compile (linux and windows compile) unit tests are not necessarily correct at this point * add NICType to endpoint struct and populate it important: when getting the endpoint state, the NIC Type field is not populated, leading to deletes not having a NIC Type; this should be changed so that getting the state populates that field including the nic type allows us to simplify the secondary endpoints delete flow (just check if the nic type is delegated instead of checking if the secondary interfaces map is populated) smoke tested: linux aks podsubnet (same vm, multi vm, internet, cni statefile consistent) linux standalone transparent vlan multitenancy (same vm, multi vm, internet, multi vnet, no connection between coke pepsi, cni statefile consistent) windows standalone bridge multitenancy single customer (same vm connections, internet, dns only, cni statefile consistent, 2 pods deleting and recreating) * ci: InterfaceInfo Map * fix multitenancy_test ut by changing key * add endpoint id to secondary ep info test since we populate the id in the actual flow * fix cni network_test linux and ensure secondary create ep info does not break in network_test we pass in sample delegated (secondary) data to Add which we then create endpoint info from even with most fields empty, in linux, the ep info is created without erroring * make invoker_cns_test linux pass running all linux package tests for network and cni package pass (or also fail on master, like createBridge) windows unit tests mostly all fail for the same ones on master and this branch summary: - network_windows_test.go ○ TestFailToAddIPv6DefaultRoute already fails on master - network_test.go ○ 9 tests fail on master, 9 tests fail on my branch - manager_test.go ○ 9 tests fail on master, 9 tests fail on my branch - endpoint_windows_test.go ○ TestNewAndDeleteEndpointImplHnsV2 already timeouts on master - endpoint_test.go ○ 9 tests fail on master, 9 tests fail on my branch - network_windows_test.go ○ FAIL: TestPluginSecondAddSamePodWindows/CNI_consecutive_add_already_hot_attached ○ FAIL: TestPluginSecondAddSamePodWindows/CNI_consecutive_add_not_hot_attached ○ We don't handle consecutive add anymore - network_test.go ○ TestPluginMultitenancyAdd/Add_Happy_path fails on master and my branch (received multiple NC results [] from CNS while dualnic feature is not supported)-- we still get two items on our list/map though which is expected - invoker_cns_test passes - invoker_azure_test passes - multitenancy_test passes The consecutive add tests fail but that is expected since we no longer support it. * modify delete flow to handle multiple epinfos to delete delete ALL endpoints related to the endpoint infos list in the event cni fails half-way through an add (one failed endpoint create and we delete all would-be-create endpoints and the state) replace looping over deletion code "n" number of times with getting a slice of endpoint infos to delete modify stateless cni code to retrieve a slice of network endpoint infos from a single response based on the container id (container id can be used in stateless cni for retrieval) incorporate stateless cni changes from other branch (cns client/ipam/restserver changes) modify get endpoint state to return slice of endpoint infos, and getting an endpoint will return an endpoint from that slice with nic type infra move edge case where endpoint is not created in the state but ips are already allocated to immediately after retrieving all ep infos fix mock behavior for getting all endpoints by container id move getting network id and network info out of the loop because their values do not seem to change between iterations move deletion of endpoint logic into a dedicated loop, and then create a dedicate loop for calling ipam delete to prevent inconsistent state all expected unit tests on linux pass * address feedback * Make change to UpdateEndpointState API to support SwiftV2 for Stateless CNI * change save state to only call update endpoint state once with a slice of endpoints, uts pass * fix using nonexistent key by passing in current interface info directly * fix azure ipam invoker not getting a populated network info for legacy cni * add L1VH windows support * add nic type to windows endpoints * move adding an external interface code to run only when creating a new network this change reflects prior behavior, where we would only add an external interface to the statefile if the network (after searching through all external interfaces) was not found currently, if there are multiple interfaces that could be selected as the master, we would add each external interface to the statefile, even if the *network* is associated with one of the existing interfaces while we would still always find the same network (thanks to having a constant NetworkId, regardless of the external interface), you could get an extra empty external interface in your statefile this commit should remove that possibility (the extra external interface shouldn't really matter in the first place though because we always select the external interface that has a matching network created on it) this should be os agnostic * update comments, first todo check pass * address some linter issues * rename networkId to networkID in endpoint info ran package tests in windows and linux for cni and network packages ran package tests in linux for cns restserver all have expected outputs (either pass, or also fails on master branch) * address linter issues * preserve more logs and reduce timeout for restart for debugging * clean comments and rename for clarity if we use the endpoint info for the network info fields, we name it nwInfo as a hint * address more linter issues linux network, restserver, and cni package tests pass * Revert "preserve more logs and reduce timeout for restart for debugging" This reverts commit 0f004925cfb37a4594df63a4259242c37cf07785. * ignore error on delete flow network query if we are in stateful cni and do not find the network, we will not error, but when we search for the endpoint it will not be found, leading to us calling ipam invoker delete which is assumed idempotent before returning previously we would error in stateful cni and return before calling ipam invoker delete * delete network on endpoint delete if stateless and delegated vmnic (win + linux) * add nic name, set nicname in linux to master interface name stateless will key into interface map with the nicname field in windows, the nicname field is based on the args ifname (usually eth0) in linux, the nicname field is based on the master interface found (usually eth0) note: hostifname/hostvethname = linux veth pair peer in the host ns ifname/contifname = linux veth pair peer in the container ns, in windows it's just the args ifname nicname is something else ifname isn't used during deletion in linux, hns id is used for deletion in windows * return secondary interface as cni result if no infra nic found, include mac address in cni result * address linter issue * fix critical error where failing to add in windows stateless would lead to hns components not being deleted and add netns for hnsv2 tested by triggering a failure to save the stateless state and seeing that the hns endpoint and network are cleaned up we use the endpoint info to clean up on "add" error, but previously, we didn't populate it with the hns ids to do so adds netns to stateless as the presence of a valid guid in netns determines if hnsv2 is used * set nicname used in stateless cni according to feedback * add dummy guid to stateless delete since we assume stateless is always hnsv2 we assume that the netns value isn't used in stateless deletion * clean up createEpInfo, declare endpoint info once * address feedback from vipul * change comments only * revert change to cns package * fix stateless cni migration flow not having nictype on migrate * keep nwInfo variables named the same as before pr (noop) * separate endpoint and network policies in endpoint info behavior should not change except in hnsv1, where network policies passed into network create call will NOT include endpoint policies endpoint policies always include network policies * address feedback from reviewers * address feedback and account for case where cns provides info without nic type if nic type is empty from cns in invoker cns, we assume it is infra nic type and populate it with infra nic type * address feedback to declare endpoint info once and populate all fields at once moved add subnets to after endpoint info created moved retrieval of all endpoint policies (from getEndpointPolicies and getPoliciesFromRuntimeCfg)until after endpoint info created network policies are just passed in from the args unaltered * use ifname instead of nicname field in endpoint struct as key in stateless * convert macaddress only nictype is delegatedvmnic * address feedback by removing network dns settings * address linter issues (noop) * address feedback and linter (noop) * remove unused consecutive add funcs (noop) * fix release ips when create a container without nictype using older cni and then upgrade cni and delete if we create a pod with an older cni version, it won't have a nictype if we upgrade cni and then delete, we should treat an empty nictype as an infra nictype and call the invoker delete * prevent eps with delegated nic type present on ep from also calling transparent endpoint client on delete tested on swift v2 linux single pod add, change cni to this version, delete (ok) then add using this cni version and delete, no extraneous transparent endpoint client calls logged * mock get interface method for ut searched for "NetPlugin" in all files and determined all prod use of NetPlugin goes through NewNetPlugin where we set the get interface method to the real interface get method adds ut where the master interface (by mac) is not found * address feedback (noop) * add ut for handling empty nictype on cns add (noop) * add multitenancy delete net not found ut (noop) * add uts for multi interface infos single add call, verify endpoint id, cns to cni data conversion and vice versa, get endpoint info from container id (noop) verifies partial success will delete all endpoints, even successfully created ones in the same cni add call * add ut for all pods associated with container id delete in one del call, new secondary delete flow (noop) * add two UTs * fix a linter issue * add ut to check endpoint ifname on new endpoint creation based on nictype (noop) * add ut for fail to find interface by subnet (noop) * Adding support for Stateless CNI Delete Edge case when there in no HNS ID * fix uts * fix linter issues * fix ut --------- Co-authored-by: jpayne3506 Co-authored-by: paulyufan2 Co-authored-by: AzureAhai --- cni/network/invoker.go | 20 +- cni/network/invoker_azure.go | 33 +- cni/network/invoker_azure_test.go | 40 +- cni/network/invoker_cns.go | 79 +- cni/network/invoker_cns_test.go | 251 +++++- cni/network/invoker_mock.go | 48 +- cni/network/multitenancy.go | 37 +- cni/network/multitenancy_mock.go | 30 +- cni/network/multitenancy_test.go | 12 +- cni/network/network.go | 817 ++++++++++-------- cni/network/network_linux.go | 13 +- cni/network/network_linux_test.go | 4 +- cni/network/network_test.go | 291 ++++++- cni/network/network_windows.go | 122 +-- cni/network/network_windows_test.go | 196 +++-- cnm/network/network.go | 11 +- netio/netio.go | 5 + network/bridge_networkclient_linux.go | 4 +- network/endpoint.go | 58 +- network/endpoint_linux.go | 331 +++---- network/endpoint_snatroute_linux.go | 4 +- network/endpoint_test.go | 95 +- network/endpoint_windows.go | 64 +- network/endpoint_windows_test.go | 20 +- network/manager.go | 277 ++++-- network/manager_mock.go | 82 +- network/manager_test.go | 224 +++++ network/mock_endpointclient.go | 4 +- network/network.go | 50 +- network/network_linux.go | 17 +- network/network_test.go | 20 +- network/network_windows.go | 37 +- network/network_windows_test.go | 32 +- network/ovs_endpoint_infraroute_linux.go | 2 +- network/ovs_endpoint_snatroute_linux.go | 2 +- network/ovs_endpointclient_linux.go | 4 +- network/secondary_endpoint_client_linux.go | 2 +- network/secondary_endpoint_linux_test.go | 35 + ...ansparent_vlan_endpoint_snatroute_linux.go | 2 +- 39 files changed, 2263 insertions(+), 1112 deletions(-) diff --git a/cni/network/invoker.go b/cni/network/invoker.go index 9d0ed8444..907aeca5d 100644 --- a/cni/network/invoker.go +++ b/cni/network/invoker.go @@ -4,7 +4,6 @@ import ( "net" "github.com/Azure/azure-container-networking/cni" - "github.com/Azure/azure-container-networking/cns" "github.com/Azure/azure-container-networking/network" cniSkel "github.com/containernetworking/cni/pkg/skel" ) @@ -27,11 +26,16 @@ type IPAMAddConfig struct { } type IPAMAddResult struct { - // Splitting defaultInterfaceInfo from secondaryInterfacesInfo so we don't need to loop for default CNI result every time - defaultInterfaceInfo network.InterfaceInfo - secondaryInterfacesInfo []network.InterfaceInfo - // ncResponse is used for Swift 1.0 multitenancy - ncResponse *cns.GetNetworkContainerResponse - hostSubnetPrefix net.IPNet - ipv6Enabled bool + interfaceInfo map[string]network.InterfaceInfo + // ncResponse and host subnet prefix were moved into interface info + ipv6Enabled bool +} + +func (ipamAddResult IPAMAddResult) PrettyString() string { + pStr := "InterfaceInfo: " + for key := range ipamAddResult.interfaceInfo { + val := ipamAddResult.interfaceInfo[key] + pStr += val.PrettyString() + } + return pStr } diff --git a/cni/network/invoker_azure.go b/cni/network/invoker_azure.go index d453c3de1..6cd3940ef 100644 --- a/cni/network/invoker_azure.go +++ b/cni/network/invoker_azure.go @@ -29,7 +29,7 @@ const ( type AzureIPAMInvoker struct { plugin delegatePlugin - nwInfo *network.NetworkInfo + nwInfo *network.EndpointInfo } type delegatePlugin interface { @@ -39,7 +39,7 @@ type delegatePlugin interface { } // Create an IPAM instance every time a CNI action is called. -func NewAzureIpamInvoker(plugin *NetPlugin, nwInfo *network.NetworkInfo) *AzureIPAMInvoker { +func NewAzureIpamInvoker(plugin *NetPlugin, nwInfo *network.EndpointInfo) *AzureIPAMInvoker { return &AzureIPAMInvoker{ plugin: plugin, nwInfo: nwInfo, @@ -47,7 +47,7 @@ func NewAzureIpamInvoker(plugin *NetPlugin, nwInfo *network.NetworkInfo) *AzureI } func (invoker *AzureIPAMInvoker) Add(addConfig IPAMAddConfig) (IPAMAddResult, error) { - addResult := IPAMAddResult{} + addResult := IPAMAddResult{interfaceInfo: make(map[string]network.InterfaceInfo)} if addConfig.nwCfg == nil { return addResult, invoker.plugin.Errorf("nil nwCfg passed to CNI ADD, stack: %+v", string(debug.Stack())) @@ -69,14 +69,11 @@ func (invoker *AzureIPAMInvoker) Add(addConfig IPAMAddConfig) (IPAMAddResult, er err = invoker.plugin.Errorf("Failed to allocate pool: %v", err) return addResult, err } - if len(result.IPs) > 0 { - addResult.hostSubnetPrefix = result.IPs[0].Address - } defer func() { if err != nil { - if len(addResult.defaultInterfaceInfo.IPConfigs) > 0 { - if er := invoker.Delete(&addResult.defaultInterfaceInfo.IPConfigs[0].Address, addConfig.nwCfg, nil, addConfig.options); er != nil { + if len(addResult.interfaceInfo) > 0 && len(addResult.interfaceInfo[invoker.getInterfaceInfoKey(cns.InfraNIC)].IPConfigs) > 0 { + if er := invoker.Delete(&addResult.interfaceInfo[invoker.getInterfaceInfoKey(cns.InfraNIC)].IPConfigs[0].Address, addConfig.nwCfg, nil, addConfig.options); er != nil { err = invoker.plugin.Errorf("Failed to clean up IP's during Delete with error %v, after Add failed with error %w", er, err) } } else { @@ -116,7 +113,21 @@ func (invoker *AzureIPAMInvoker) Add(addConfig IPAMAddConfig) (IPAMAddResult, er routes[i] = network.RouteInfo{Dst: route.Dst, Gw: route.GW} } - addResult.defaultInterfaceInfo = network.InterfaceInfo{IPConfigs: ipconfigs, Routes: routes, DNS: network.DNSInfo{Suffix: result.DNS.Domain, Servers: result.DNS.Nameservers}, NICType: cns.InfraNIC} + // TODO: changed how host subnet prefix populated (check) + hostSubnetPrefix := net.IPNet{} + if len(result.IPs) > 0 { + hostSubnetPrefix = result.IPs[0].Address + } + addResult.interfaceInfo[invoker.getInterfaceInfoKey(cns.InfraNIC)] = network.InterfaceInfo{ + IPConfigs: ipconfigs, + Routes: routes, + DNS: network.DNSInfo{ + Suffix: result.DNS.Domain, + Servers: result.DNS.Nameservers, + }, + NICType: cns.InfraNIC, + HostSubnetPrefix: hostSubnetPrefix, + } return addResult, err } @@ -197,3 +208,7 @@ func (invoker *AzureIPAMInvoker) Delete(address *net.IPNet, nwCfg *cni.NetworkCo return nil } + +func (invoker *AzureIPAMInvoker) getInterfaceInfoKey(nicType cns.NICType) string { + return string(nicType) +} diff --git a/cni/network/invoker_azure_test.go b/cni/network/invoker_azure_test.go index 2fe6bcb5e..e41ee7171 100644 --- a/cni/network/invoker_azure_test.go +++ b/cni/network/invoker_azure_test.go @@ -8,6 +8,7 @@ import ( "github.com/Azure/azure-container-networking/cni" "github.com/Azure/azure-container-networking/cni/log" + "github.com/Azure/azure-container-networking/cns" "github.com/Azure/azure-container-networking/ipam" "github.com/Azure/azure-container-networking/network" cniSkel "github.com/containernetworking/cni/pkg/skel" @@ -81,6 +82,9 @@ func (m *mockDelegatePlugin) Errorf(format string, args ...interface{}) *cniType } } +// net.ParseCIDR will first get the ip, which contains byte data for the ip and mask, +// and the ipnet, which has a field for the *masked* ip and a field for the mask +// this function then replaces the masked ip with the "ip" field retrieved earlier and returns the ipnet func getCIDRNotationForAddress(ipaddresswithcidr string) *net.IPNet { ip, ipnet, err := net.ParseCIDR(ipaddresswithcidr) if err != nil { @@ -90,6 +94,15 @@ func getCIDRNotationForAddress(ipaddresswithcidr string) *net.IPNet { return ipnet } +// returns an ipnet, which contains the *masked* ip (zeroed out based on CIDR) and the mask itself +func parseCIDR(ipaddresswithcidr string) *net.IPNet { + _, ipnet, err := net.ParseCIDR(ipaddresswithcidr) + if err != nil { + panic(fmt.Sprintf("failed to parse cidr with err: %v", err)) + } + return ipnet +} + func getSingleResult(ip string) []*cniTypesCurr.Result { return []*cniTypesCurr.Result{ { @@ -111,26 +124,26 @@ func getResult(ips ...string) []*network.IPConfig { return res } -func getNwInfo(subnetv4, subnetv6 string) *network.NetworkInfo { - nwinfo := &network.NetworkInfo{} +func getNwInfo(subnetv4, subnetv6 string) *network.EndpointInfo { + nwInfo := &network.EndpointInfo{} if subnetv4 != "" { - nwinfo.Subnets = append(nwinfo.Subnets, network.SubnetInfo{ + nwInfo.Subnets = append(nwInfo.Subnets, network.SubnetInfo{ Prefix: *getCIDRNotationForAddress(subnetv4), }) } if subnetv6 != "" { - nwinfo.Subnets = append(nwinfo.Subnets, network.SubnetInfo{ + nwInfo.Subnets = append(nwInfo.Subnets, network.SubnetInfo{ Prefix: *getCIDRNotationForAddress(subnetv6), }) } - return nwinfo + return nwInfo } func TestAzureIPAMInvoker_Add(t *testing.T) { require := require.New(t) type fields struct { plugin delegatePlugin - nwInfo *network.NetworkInfo + nwInfo *network.EndpointInfo } type args struct { nwCfg *cni.NetworkConfig @@ -238,8 +251,15 @@ func TestAzureIPAMInvoker_Add(t *testing.T) { require.Nil(err) } - fmt.Printf("want:%+v\nrest:%+v\n", tt.want, ipamAddResult.defaultInterfaceInfo.IPConfigs) - require.Exactly(tt.want, ipamAddResult.defaultInterfaceInfo.IPConfigs) + for key, ifInfo := range ipamAddResult.interfaceInfo { + if ifInfo.NICType == cns.InfraNIC { + fmt.Printf("want:%+v\nrest:%+v\n", tt.want, ifInfo.IPConfigs) + require.Exactly(tt.want, ifInfo.IPConfigs) + } + // azure ipam invoker always sets key as infra nic + require.Equal(string(cns.InfraNIC), key) + require.Equal(cns.InfraNIC, ifInfo.NICType) + } }) } } @@ -248,7 +268,7 @@ func TestAzureIPAMInvoker_Delete(t *testing.T) { require := require.New(t) type fields struct { plugin delegatePlugin - nwInfo *network.NetworkInfo + nwInfo *network.EndpointInfo } type args struct { address *net.IPNet @@ -383,7 +403,7 @@ func TestRemoveIpamState_Add(t *testing.T) { requires := require.New(t) type fields struct { plugin delegatePlugin - nwInfo *network.NetworkInfo + nwInfo *network.EndpointInfo } type args struct { nwCfg *cni.NetworkConfig diff --git a/cni/network/invoker_cns.go b/cni/network/invoker_cns.go index 1a832ac09..54c5bca30 100644 --- a/cni/network/invoker_cns.go +++ b/cni/network/invoker_cns.go @@ -141,9 +141,8 @@ func (invoker *CNSIPAMInvoker) Add(addConfig IPAMAddConfig) (IPAMAddResult, erro } } - addResult := IPAMAddResult{} + addResult := IPAMAddResult{interfaceInfo: make(map[string]network.InterfaceInfo)} numInterfacesWithDefaultRoutes := 0 - for i := 0; i < len(response.PodIPInfo); i++ { info := IPResultInfo{ podIPAddress: response.PodIPInfo[i].PodIPConfig.IPAddress, @@ -164,6 +163,8 @@ func (invoker *CNSIPAMInvoker) Add(addConfig IPAMAddConfig) (IPAMAddResult, erro zap.Any("podInfo", podInfo)) //nolint:exhaustive // ignore exhaustive types check + // Do we want to leverage this lint skip in other places of our code? + key := invoker.getInterfaceInfoKey(info.nicType, info.macAddress) switch info.nicType { case cns.DelegatedVMNIC: // only handling single v4 PodIPInfo for DelegatedVMNICs at the moment, will have to update once v6 gets added @@ -171,22 +172,33 @@ func (invoker *CNSIPAMInvoker) Add(addConfig IPAMAddConfig) (IPAMAddResult, erro numInterfacesWithDefaultRoutes++ } - if err := configureSecondaryAddResult(&info, &addResult, &response.PodIPInfo[i].PodIPConfig); err != nil { + // Add secondary interface info from podIPInfo to ipamAddResult + info.hostSubnet = response.PodIPInfo[i].HostPrimaryIPInfo.Subnet + info.hostPrimaryIP = response.PodIPInfo[i].HostPrimaryIPInfo.PrimaryIP + info.hostGateway = response.PodIPInfo[i].HostPrimaryIPInfo.Gateway + + if err := configureSecondaryAddResult(&info, &addResult, &response.PodIPInfo[i].PodIPConfig, key); err != nil { return IPAMAddResult{}, err } - default: + case cns.InfraNIC, "": + // if we change from legacy cns, the nicType will be empty, so we assume it is infra nic + info.nicType = cns.InfraNIC + // only count dualstack interface once - if addResult.defaultInterfaceInfo.IPConfigs == nil { - addResult.defaultInterfaceInfo.IPConfigs = make([]*network.IPConfig, 0) + _, exist := addResult.interfaceInfo[key] + if !exist { + addResult.interfaceInfo[key] = network.InterfaceInfo{} if !info.skipDefaultRoutes { numInterfacesWithDefaultRoutes++ } } overlayMode := (invoker.ipamMode == util.V4Overlay) || (invoker.ipamMode == util.DualStackOverlay) || (invoker.ipamMode == util.Overlay) - if err := configureDefaultAddResult(&info, &addConfig, &addResult, overlayMode); err != nil { + if err := configureDefaultAddResult(&info, &addConfig, &addResult, overlayMode, key); err != nil { return IPAMAddResult{}, err } + default: + logger.Warn("Unknown NIC type received from cns pod ip info", zap.String("nicType", string(info.nicType))) } } @@ -353,7 +365,7 @@ func getRoutes(cnsRoutes []cns.Route, skipDefaultRoutes bool) ([]network.RouteIn return routes, nil } -func configureDefaultAddResult(info *IPResultInfo, addConfig *IPAMAddConfig, addResult *IPAMAddResult, overlayMode bool) error { +func configureDefaultAddResult(info *IPResultInfo, addConfig *IPAMAddConfig, addResult *IPAMAddResult, overlayMode bool, key string) error { // set the NC Primary IP in options // SNATIPKey is not set for ipv6 if net.ParseIP(info.ncPrimaryIP).To4() != nil { @@ -361,7 +373,7 @@ func configureDefaultAddResult(info *IPResultInfo, addConfig *IPAMAddConfig, add } ip, ncIPNet, err := net.ParseCIDR(info.podIPAddress + "/" + fmt.Sprint(info.ncSubnetPrefix)) - if ip == nil { + if ip == nil || err != nil { return errors.Wrap(err, "Unable to parse IP from response: "+info.podIPAddress+" with err %w") } @@ -384,15 +396,21 @@ func configureDefaultAddResult(info *IPResultInfo, addConfig *IPAMAddConfig, add } } + // get the name of the primary IP address + _, hostIPNet, err := net.ParseCIDR(info.hostSubnet) + if err != nil { + return errors.Wrap(err, "unable to parse hostSubnet") + } + if ip := net.ParseIP(info.podIPAddress); ip != nil { - defaultInterfaceInfo := &addResult.defaultInterfaceInfo defaultRouteDstPrefix := network.Ipv4DefaultRouteDstPrefix if ip.To4() == nil { defaultRouteDstPrefix = network.Ipv6DefaultRouteDstPrefix addResult.ipv6Enabled = true } - defaultInterfaceInfo.IPConfigs = append(defaultInterfaceInfo.IPConfigs, + ipConfigs := addResult.interfaceInfo[key].IPConfigs + ipConfigs = append(ipConfigs, &network.IPConfig{ Address: net.IPNet{ IP: ip, @@ -406,27 +424,26 @@ func configureDefaultAddResult(info *IPResultInfo, addConfig *IPAMAddConfig, add return getRoutesErr } + resRoute := addResult.interfaceInfo[key].Routes if len(routes) > 0 { - defaultInterfaceInfo.Routes = append(defaultInterfaceInfo.Routes, routes...) + resRoute = append(resRoute, routes...) } else { // add default routes if none are provided - defaultInterfaceInfo.Routes = append(defaultInterfaceInfo.Routes, network.RouteInfo{ + resRoute = append(resRoute, network.RouteInfo{ Dst: defaultRouteDstPrefix, Gw: ncgw, }) } - - addResult.defaultInterfaceInfo.SkipDefaultRoutes = info.skipDefaultRoutes + // if we have multiple infra ip result infos, we effectively append routes and ip configs to that same interface info each time + // the host subnet prefix (in ipv4 or ipv6) will always refer to the same interface regardless of which ip result info we look at + addResult.interfaceInfo[key] = network.InterfaceInfo{ + NICType: cns.InfraNIC, + SkipDefaultRoutes: info.skipDefaultRoutes, + IPConfigs: ipConfigs, + Routes: resRoute, + HostSubnetPrefix: *hostIPNet, + } } - // get the name of the primary IP address - _, hostIPNet, err := net.ParseCIDR(info.hostSubnet) - if err != nil { - return fmt.Errorf("unable to parse hostSubnet: %w", err) - } - - addResult.hostSubnetPrefix = *hostIPNet - addResult.defaultInterfaceInfo.NICType = cns.InfraNIC - // set subnet prefix for host vm // setHostOptions will execute if IPAM mode is not v4 overlay and not dualStackOverlay mode // TODO: Remove v4overlay and dualstackoverlay options, after 'overlay' rolls out in AKS-RP @@ -439,7 +456,7 @@ func configureDefaultAddResult(info *IPResultInfo, addConfig *IPAMAddConfig, add return nil } -func configureSecondaryAddResult(info *IPResultInfo, addResult *IPAMAddResult, podIPConfig *cns.IPSubnet) error { +func configureSecondaryAddResult(info *IPResultInfo, addResult *IPAMAddResult, podIPConfig *cns.IPSubnet, key string) error { ip, ipnet, err := podIPConfig.GetIPNet() if ip == nil { return errors.Wrap(err, "Unable to parse IP from response: "+info.podIPAddress+" with err %w") @@ -455,13 +472,14 @@ func configureSecondaryAddResult(info *IPResultInfo, addResult *IPAMAddResult, p return err } - result := network.InterfaceInfo{ + addResult.interfaceInfo[key] = network.InterfaceInfo{ IPConfigs: []*network.IPConfig{ { Address: net.IPNet{ IP: ip, Mask: ipnet.Mask, }, + Gateway: net.ParseIP(info.ncGatewayIPAddress), }, }, Routes: routes, @@ -470,7 +488,12 @@ func configureSecondaryAddResult(info *IPResultInfo, addResult *IPAMAddResult, p SkipDefaultRoutes: info.skipDefaultRoutes, } - addResult.secondaryInterfacesInfo = append(addResult.secondaryInterfacesInfo, result) - return nil } + +func (invoker *CNSIPAMInvoker) getInterfaceInfoKey(nicType cns.NICType, macAddress string) string { + if nicType == cns.DelegatedVMNIC { + return macAddress + } + return string(nicType) +} diff --git a/cni/network/invoker_cns_test.go b/cni/network/invoker_cns_test.go index b81456d2a..a12032ba2 100644 --- a/cni/network/invoker_cns_test.go +++ b/cni/network/invoker_cns_test.go @@ -107,6 +107,7 @@ func TestCNSIPAMInvoker_Add_Overlay(t *testing.T) { PrimaryIP: "10.224.0.5", Subnet: "10.224.0.0/16", }, + NICType: cns.InfraNIC, }, Response: cns.Response{ ReturnCode: 0, @@ -140,7 +141,8 @@ func TestCNSIPAMInvoker_Add_Overlay(t *testing.T) { Gw: getTestOverlayGateway(), }, }, - NICType: cns.InfraNIC, + NICType: cns.InfraNIC, + HostSubnetPrefix: *parseCIDR("10.224.0.0/16"), }, wantErr: false, }, @@ -174,6 +176,7 @@ func TestCNSIPAMInvoker_Add_Overlay(t *testing.T) { PrimaryIP: "10.0.0.1", Subnet: "10.0.0.0/24", }, + NICType: cns.InfraNIC, }, { PodIPConfig: cns.IPSubnet{ @@ -193,6 +196,7 @@ func TestCNSIPAMInvoker_Add_Overlay(t *testing.T) { PrimaryIP: "fe80::1234:5678:9abc", Subnet: "fd11:1234::/112", }, + NICType: cns.InfraNIC, }, }, Response: cns.Response{ @@ -235,7 +239,8 @@ func TestCNSIPAMInvoker_Add_Overlay(t *testing.T) { Gw: net.ParseIP("fe80::1234:5678:9abc"), }, }, - NICType: cns.InfraNIC, + NICType: cns.InfraNIC, + HostSubnetPrefix: *parseCIDR("fd11:1234::/112"), }, wantErr: false, }, @@ -319,6 +324,7 @@ func TestCNSIPAMInvoker_Add_Overlay(t *testing.T) { }, NICType: cns.InfraNIC, SkipDefaultRoutes: true, + HostSubnetPrefix: *parseCIDR("10.0.0.0/24"), }, wantSecondaryInterfacesInfo: network.InterfaceInfo{ IPConfigs: []*network.IPConfig{ @@ -329,6 +335,7 @@ func TestCNSIPAMInvoker_Add_Overlay(t *testing.T) { Routes: []network.RouteInfo{}, NICType: cns.DelegatedVMNIC, MacAddress: parsedMacAddress, + // secondaries don't have a host subnet prefix }, wantErr: false, }, @@ -485,10 +492,16 @@ func TestCNSIPAMInvoker_Add_Overlay(t *testing.T) { require.NoError(err) } - fmt.Printf("want:%+v\nrest:%+v\n", tt.wantSecondaryInterfacesInfo, ipamAddResult.secondaryInterfacesInfo) - require.Equalf(tt.wantDefaultResult, ipamAddResult.defaultInterfaceInfo, "incorrect default response") - if len(tt.wantSecondaryInterfacesInfo.IPConfigs) > 0 { - require.EqualValues(tt.wantSecondaryInterfacesInfo, ipamAddResult.secondaryInterfacesInfo[0], "incorrect multitenant response") + for _, ifInfo := range ipamAddResult.interfaceInfo { + if ifInfo.NICType == cns.DelegatedVMNIC { + fmt.Printf("want:%+v\nrest:%+v\n", tt.wantSecondaryInterfacesInfo, ifInfo) + if len(tt.wantSecondaryInterfacesInfo.IPConfigs) > 0 { + require.EqualValues(tt.wantSecondaryInterfacesInfo, ifInfo, "incorrect multitenant response") + } + } + if ifInfo.NICType == cns.InfraNIC { + require.Equalf(tt.wantDefaultResult, ifInfo, "incorrect default response") + } } }) } @@ -546,6 +559,7 @@ func TestCNSIPAMInvoker_Add(t *testing.T) { PrimaryIP: "10.0.0.1", Subnet: "10.0.0.0/24", }, + NICType: cns.InfraNIC, }, }, Response: cns.Response{ @@ -580,7 +594,76 @@ func TestCNSIPAMInvoker_Add(t *testing.T) { Gw: net.ParseIP("10.0.0.1"), }, }, - NICType: cns.InfraNIC, + NICType: cns.InfraNIC, + HostSubnetPrefix: *parseCIDR("10.0.0.0/24"), + }, + wantErr: false, + }, + { + name: "Test CNI add with pod ip info empty nictype", + 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{}{}, + }, + wantDefaultResult: network.InterfaceInfo{ + IPConfigs: []*network.IPConfig{ + { + Address: *getCIDRNotationForAddress("10.0.1.10/24"), + Gateway: net.ParseIP("10.0.0.1"), + }, + }, + Routes: []network.RouteInfo{ + { + Dst: network.Ipv4DefaultRouteDstPrefix, + Gw: net.ParseIP("10.0.0.1"), + }, + }, + NICType: cns.InfraNIC, + HostSubnetPrefix: *parseCIDR("10.0.0.0/24"), }, wantErr: false, }, @@ -613,6 +696,7 @@ func TestCNSIPAMInvoker_Add(t *testing.T) { PrimaryIP: "10.0.0.1", Subnet: "10.0.0.0/24", }, + NICType: cns.InfraNIC, }, { PodIPConfig: cns.IPSubnet{ @@ -632,6 +716,7 @@ func TestCNSIPAMInvoker_Add(t *testing.T) { PrimaryIP: "fe80::1234:5678:9abc", Subnet: "fd11:1234::/112", }, + NICType: cns.InfraNIC, }, }, Response: cns.Response{ @@ -674,7 +759,8 @@ func TestCNSIPAMInvoker_Add(t *testing.T) { Gw: net.ParseIP("fe80::1234:5678:9abc"), }, }, - NICType: cns.InfraNIC, + NICType: cns.InfraNIC, + HostSubnetPrefix: *parseCIDR("fd11:1234::/112"), }, wantErr: false, }, @@ -713,10 +799,17 @@ func TestCNSIPAMInvoker_Add(t *testing.T) { require.NoError(err) } - fmt.Printf("want:%+v\nrest:%+v\n", tt.wantMultitenantResult, ipamAddResult.secondaryInterfacesInfo) - require.Equalf(tt.wantDefaultResult, ipamAddResult.defaultInterfaceInfo, "incorrect default response") - if len(tt.wantMultitenantResult.IPConfigs) > 0 { - require.Equalf(tt.wantMultitenantResult, ipamAddResult.secondaryInterfacesInfo[0], "incorrect multitenant response") + for _, ifInfo := range ipamAddResult.interfaceInfo { + require.NotEqual("", string(ifInfo.NICType), "nictype should be auto populated if empty") + if ifInfo.NICType == cns.DelegatedVMNIC { + fmt.Printf("want:%+v\nrest:%+v\n", tt.wantMultitenantResult, ifInfo) + if len(tt.wantMultitenantResult.IPConfigs) > 0 { + require.Equalf(tt.wantMultitenantResult, ifInfo, "incorrect multitenant response") + } + } + if ifInfo.NICType == cns.InfraNIC { + require.Equalf(tt.wantDefaultResult, ifInfo, "incorrect default response") + } } }) } @@ -747,7 +840,6 @@ func TestCNSIPAMInvoker_Add_UnsupportedAPI(t *testing.T) { fields fields args args want network.InterfaceInfo - want1 network.InterfaceInfo wantErr bool }{ { @@ -779,6 +871,7 @@ func TestCNSIPAMInvoker_Add_UnsupportedAPI(t *testing.T) { PrimaryIP: "10.0.0.1", Subnet: "10.0.0.0/24", }, + NICType: cns.InfraNIC, }, Response: cns.Response{ ReturnCode: 0, @@ -812,7 +905,8 @@ func TestCNSIPAMInvoker_Add_UnsupportedAPI(t *testing.T) { Gw: net.ParseIP("10.0.0.1"), }, }, - NICType: cns.InfraNIC, + NICType: cns.InfraNIC, + HostSubnetPrefix: *parseCIDR("10.0.0.0/24"), }, wantErr: true, }, @@ -833,7 +927,12 @@ func TestCNSIPAMInvoker_Add_UnsupportedAPI(t *testing.T) { t.Fatalf("expected an error %+v but none received", err) } require.NoError(err) - require.Equalf(tt.want, ipamAddResult.defaultInterfaceInfo, "incorrect ipv4 response") + + for _, ifInfo := range ipamAddResult.interfaceInfo { + if ifInfo.NICType == cns.InfraNIC { + require.Equalf(tt.want, ifInfo, "incorrect ipv4 response") + } + } }) } } @@ -1337,3 +1436,125 @@ func Test_setHostOptions(t *testing.T) { }) } } + +func Test_getInterfaceInfoKey(t *testing.T) { + require := require.New(t) //nolint further usage of require without passing t + inv := &CNSIPAMInvoker{} + dummyMAC := "12:34:56:78:9a:bc" + require.Equal(string(cns.InfraNIC), inv.getInterfaceInfoKey(cns.InfraNIC, dummyMAC)) + require.Equal(dummyMAC, inv.getInterfaceInfoKey(cns.DelegatedVMNIC, dummyMAC)) + require.Equal("", inv.getInterfaceInfoKey(cns.DelegatedVMNIC, "")) + require.Equal(string(cns.BackendNIC), inv.getInterfaceInfoKey(cns.BackendNIC, dummyMAC)) +} + +func TestCNSIPAMInvoker_Add_SwiftV2(t *testing.T) { + require := require.New(t) //nolint further usage of require without passing t + + macAddress := "12:34:56:78:9a:bc" + parsedMacAddress, _ := net.ParseMAC(macAddress) + + type fields struct { + podName string + podNamespace string + cnsClient cnsclient + } + + type args struct { + nwCfg *cni.NetworkConfig + args *cniSkel.CmdArgs + hostSubnetPrefix *net.IPNet + options map[string]interface{} + } + + tests := []struct { + name string + fields fields + args args + wantSecondaryInterfacesInfo map[string]network.InterfaceInfo + wantErr bool + }{ + { + name: "Test happy CNI add with swiftv2 multitenant result", + fields: fields{ + podName: testPodInfo.PodName, + podNamespace: testPodInfo.PodNamespace, + cnsClient: &MockCNSClient{ + require: require, + requestIPs: requestIPsHandler{ + ipconfigArgument: cns.IPConfigsRequest{ + PodInterfaceID: "testcont-testifname1", + InfraContainerID: "testcontainerid1", + OrchestratorContext: marshallPodInfo(testPodInfo), + }, + result: &cns.IPConfigsResponse{ + PodIPInfo: []cns.PodIpInfo{ + { + PodIPConfig: cns.IPSubnet{ + IPAddress: "10.1.1.10", + PrefixLength: 24, + }, + HostPrimaryIPInfo: cns.HostIPInfo{ + Gateway: "10.0.0.1", + PrimaryIP: "10.0.0.2", + Subnet: "10.0.0.1/24", + }, + NICType: cns.DelegatedVMNIC, + MacAddress: macAddress, + }, + }, + Response: cns.Response{ + ReturnCode: 0, + Message: "", + }, + }, + err: nil, + }, + }, + }, + args: args{ + nwCfg: &cni.NetworkConfig{}, + args: &cniSkel.CmdArgs{ + ContainerID: "testcontainerid1", + Netns: "testnetns1", + IfName: "testifname1", + }, + hostSubnetPrefix: getCIDRNotationForAddress("10.0.0.1/24"), + options: map[string]interface{}{}, + }, + wantSecondaryInterfacesInfo: map[string]network.InterfaceInfo{ + macAddress: { + IPConfigs: []*network.IPConfig{ + { + Address: *getCIDRNotationForAddress("10.1.1.10/24"), + }, + }, + Routes: []network.RouteInfo{}, + NICType: cns.DelegatedVMNIC, + MacAddress: parsedMacAddress, + }, + }, + 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, + } + 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.wantSecondaryInterfacesInfo, ipamAddResult.interfaceInfo) + if len(tt.wantSecondaryInterfacesInfo[macAddress].IPConfigs) > 0 { + require.EqualValues(tt.wantSecondaryInterfacesInfo, ipamAddResult.interfaceInfo, "incorrect multitenant response") + } + }) + } +} diff --git a/cni/network/invoker_mock.go b/cni/network/invoker_mock.go index 5bc4ad5d0..146143626 100644 --- a/cni/network/invoker_mock.go +++ b/cni/network/invoker_mock.go @@ -31,6 +31,7 @@ type MockIpamInvoker struct { delegatedVMNIC bool delegatedVMNICFail bool ipMap map[string]bool + customReturn map[string]network.InterfaceInfo } func NewMockIpamInvoker(ipv6, v4Fail, v6Fail, delegatedVMNIC, delegatedVMNICFail bool) *MockIpamInvoker { @@ -40,6 +41,13 @@ func NewMockIpamInvoker(ipv6, v4Fail, v6Fail, delegatedVMNIC, delegatedVMNICFail v6Fail: v6Fail, delegatedVMNIC: delegatedVMNIC, delegatedVMNICFail: delegatedVMNICFail, + ipMap: make(map[string]bool), + } +} + +func NewCustomMockIpamInvoker(customReturn map[string]network.InterfaceInfo) *MockIpamInvoker { + return &MockIpamInvoker{ + customReturn: customReturn, ipMap: make(map[string]bool), } @@ -49,22 +57,24 @@ func (invoker *MockIpamInvoker) Add(opt IPAMAddConfig) (ipamAddResult IPAMAddRes if invoker.v4Fail { return ipamAddResult, errV4 } - - ipamAddResult.hostSubnetPrefix = net.IPNet{} + ipamAddResult = IPAMAddResult{interfaceInfo: make(map[string]network.InterfaceInfo)} + // TODO: the ipam add result host subnet prefix is in the interface info and ensure that when creating interface info, the host subnet prefix is set to an empty value (may break uts) ipv4Str := "10.240.0.5" if _, ok := invoker.ipMap["10.240.0.5/24"]; ok { ipv4Str = "10.240.0.6" } - ip := net.ParseIP(ipv4Str) ipnet := net.IPNet{IP: ip, Mask: net.CIDRMask(subnetBits, ipv4Bits)} gwIP := net.ParseIP("10.240.0.1") - ipamAddResult.defaultInterfaceInfo = network.InterfaceInfo{ - IPConfigs: []*network.IPConfig{ - {Address: ipnet, Gateway: gwIP}, - }, - NICType: cns.InfraNIC, + ipRes := []*network.IPConfig{ + {Address: ipnet, Gateway: gwIP}, + } + + ipamAddResult.interfaceInfo[string(cns.InfraNIC)] = network.InterfaceInfo{ + IPConfigs: ipRes, + NICType: cns.InfraNIC, + HostSubnetPrefix: net.IPNet{}, } invoker.ipMap[ipnet.String()] = true if invoker.v6Fail { @@ -80,7 +90,11 @@ func (invoker *MockIpamInvoker) Add(opt IPAMAddConfig) (ipamAddResult IPAMAddRes ip := net.ParseIP(ipv6Str) ipnet := net.IPNet{IP: ip, Mask: net.CIDRMask(subnetv6Bits, ipv6Bits)} gwIP := net.ParseIP("fc00::1") - ipamAddResult.defaultInterfaceInfo.IPConfigs = append(ipamAddResult.defaultInterfaceInfo.IPConfigs, &network.IPConfig{Address: ipnet, Gateway: gwIP}) + ipRes = append(ipRes, &network.IPConfig{Address: ipnet, Gateway: gwIP}) + ipamAddResult.interfaceInfo[string(cns.InfraNIC)] = network.InterfaceInfo{ + IPConfigs: ipRes, + NICType: cns.InfraNIC, + } invoker.ipMap[ipnet.String()] = true } @@ -91,12 +105,16 @@ func (invoker *MockIpamInvoker) Add(opt IPAMAddConfig) (ipamAddResult IPAMAddRes ipStr := "20.20.20.20/32" _, ipnet, _ := net.ParseCIDR(ipStr) - ipamAddResult.secondaryInterfacesInfo = append(ipamAddResult.secondaryInterfacesInfo, network.InterfaceInfo{ - IPConfigs: []*network.IPConfig{ - {Address: *ipnet}, - }, - NICType: cns.DelegatedVMNIC, - }) + ipRes = append(ipRes, &network.IPConfig{Address: *ipnet}) + ipamAddResult.interfaceInfo[string(cns.InfraNIC)] = network.InterfaceInfo{ + IPConfigs: ipRes, + NICType: cns.DelegatedVMNIC, + } + } + + if invoker.customReturn != nil { + ipamAddResult.interfaceInfo = invoker.customReturn + return ipamAddResult, nil } return ipamAddResult, nil diff --git a/cni/network/multitenancy.go b/cni/network/multitenancy.go index 33d161b78..67013863b 100644 --- a/cni/network/multitenancy.go +++ b/cni/network/multitenancy.go @@ -9,6 +9,7 @@ import ( "net" "net/http" "os" + "strconv" "strings" "time" @@ -43,7 +44,7 @@ type MultitenancyClient interface { nwCfg *cni.NetworkConfig, podName string, podNamespace string, - ifName string) ([]IPAMAddResult, error) + ifName string) (IPAMAddResult, error) Init(cnsclient cnsclient, netioshim netioshim) } @@ -189,7 +190,7 @@ func (m *Multitenancy) SetupRoutingForMultitenancy( // 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) { +) (IPAMAddResult, error) { var podNameWithoutSuffix string if !nwCfg.EnableExactMatchForPodName { @@ -202,7 +203,7 @@ func (m *Multitenancy) GetAllNetworkContainers( ncResponses, hostSubnetPrefixes, err := m.getNetworkContainersInternal(ctx, podNamespace, podNameWithoutSuffix) if err != nil { - return []IPAMAddResult{}, fmt.Errorf("%w", err) + return IPAMAddResult{}, fmt.Errorf("%w", err) } for i := 0; i < len(ncResponses); i++ { @@ -210,23 +211,31 @@ func (m *Multitenancy) GetAllNetworkContainers( if ncResponses[i].LocalIPConfiguration.IPSubnet.IPAddress == "" { logger.Info("Snat IP is not populated for ncs. Got empty string", zap.Any("response", ncResponses)) - return []IPAMAddResult{}, errSnatIP + return IPAMAddResult{}, errSnatIP } } } - ipamResults := make([]IPAMAddResult, len(ncResponses)) + ipamResult := IPAMAddResult{} + ipamResult.interfaceInfo = make(map[string]network.InterfaceInfo) for i := 0; i < len(ncResponses); i++ { - ipamResults[i].ncResponse = &ncResponses[i] - ipamResults[i].hostSubnetPrefix = hostSubnetPrefixes[i] - ipconfig, routes := convertToIPConfigAndRouteInfo(ipamResults[i].ncResponse) - ipamResults[i].defaultInterfaceInfo.IPConfigs = []*network.IPConfig{ipconfig} - ipamResults[i].defaultInterfaceInfo.Routes = routes - ipamResults[i].defaultInterfaceInfo.NICType = cns.InfraNIC + // one ncResponse gets you one interface info in the returned IPAMAddResult + ifInfo := network.InterfaceInfo{ + NCResponse: &ncResponses[i], + HostSubnetPrefix: hostSubnetPrefixes[i], + } + + ipconfig, routes := convertToIPConfigAndRouteInfo(ifInfo.NCResponse) + ifInfo.IPConfigs = append(ifInfo.IPConfigs, ipconfig) + ifInfo.Routes = routes + ifInfo.NICType = cns.InfraNIC + + // assuming we only assign infra nics in this function + ipamResult.interfaceInfo[m.getInterfaceInfoKey(ifInfo.NICType, i)] = ifInfo } - return ipamResults, err + return ipamResult, err } // get all network containers configuration for given orchestratorContext @@ -357,6 +366,10 @@ func checkIfSubnetOverlaps(enableInfraVnet bool, nwCfg *cni.NetworkConfig, cnsNe return false } +func (m *Multitenancy) getInterfaceInfoKey(nicType cns.NICType, i int) string { + return string(nicType) + strconv.Itoa(i) +} + var ( errSnatIP = errors.New("Snat IP not populated") errInfraVnet = errors.New("infravnet not populated") diff --git a/cni/network/multitenancy_mock.go b/cni/network/multitenancy_mock.go index 9cd6b0a5e..9c6327909 100644 --- a/cni/network/multitenancy_mock.go +++ b/cni/network/multitenancy_mock.go @@ -5,6 +5,7 @@ import ( "errors" "net" "runtime" + "strconv" "github.com/Azure/azure-container-networking/cni" "github.com/Azure/azure-container-networking/cns" @@ -88,9 +89,9 @@ func (m *MockMultitenancy) GetAllNetworkContainers( podName string, podNamespace string, ifName string, -) ([]IPAMAddResult, error) { +) (IPAMAddResult, error) { if m.fail { - return nil, errMockMulAdd + return IPAMAddResult{}, errMockMulAdd } var cnsResponses []cns.GetNetworkContainerResponse @@ -154,15 +155,24 @@ func (m *MockMultitenancy) GetAllNetworkContainers( ipNets = append(ipNets, *firstIPnet) cnsResponses = append(cnsResponses, *cnsResponseOne) - ipamResults := make([]IPAMAddResult, len(cnsResponses)) + ipamResult := IPAMAddResult{} + ipamResult.interfaceInfo = make(map[string]network.InterfaceInfo) + for i := 0; i < len(cnsResponses); i++ { - ipamResults[i].ncResponse = &cnsResponses[i] - ipamResults[i].hostSubnetPrefix = ipNets[i] - ipconfig, routes := convertToIPConfigAndRouteInfo(ipamResults[i].ncResponse) - ipamResults[i].defaultInterfaceInfo.IPConfigs = []*network.IPConfig{ipconfig} - ipamResults[i].defaultInterfaceInfo.Routes = routes - ipamResults[i].defaultInterfaceInfo.NICType = cns.InfraNIC + // one ncResponse gets you one interface info in the returned IPAMAddResult + ifInfo := network.InterfaceInfo{ + NCResponse: &cnsResponses[i], + HostSubnetPrefix: ipNets[i], + } + + ipconfig, routes := convertToIPConfigAndRouteInfo(ifInfo.NCResponse) + ifInfo.IPConfigs = append(ifInfo.IPConfigs, ipconfig) + ifInfo.Routes = routes + ifInfo.NICType = cns.InfraNIC + + // assuming we only assign infra nics in this function + ipamResult.interfaceInfo[string(ifInfo.NICType)+strconv.Itoa(i)] = ifInfo } - return ipamResults, nil + return ipamResult, nil } diff --git a/cni/network/multitenancy_test.go b/cni/network/multitenancy_test.go index 42ef5f39d..4b730d4b2 100644 --- a/cni/network/multitenancy_test.go +++ b/cni/network/multitenancy_test.go @@ -548,13 +548,16 @@ func TestGetMultiTenancyCNIResult(t *testing.T) { 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) + require.Exactly(tt.want1, got.interfaceInfo[string(cns.InfraNIC)+"0"].NCResponse) + require.Exactly(tt.want2, got.interfaceInfo[string(cns.InfraNIC)+"1"].NCResponse) + require.Exactly(tt.want3, got.interfaceInfo[string(cns.InfraNIC)+"0"].HostSubnetPrefix) // check multiple responses tt.want5 = append(tt.want5, *tt.want1, *tt.want2) require.Exactly(tt.want5, ncResponses) + + require.Equal(cns.InfraNIC, got.interfaceInfo[string(cns.InfraNIC)+"0"].NICType) + require.Equal(cns.InfraNIC, got.interfaceInfo[string(cns.InfraNIC)+"1"].NICType) }) } } @@ -691,7 +694,8 @@ func TestGetMultiTenancyCNIResultUnsupportedAPI(t *testing.T) { t.Fatalf("expected an error %+v but none received", err) } require.NoError(err) - require.Exactly(tt.want, got[0].ncResponse) + require.Exactly(tt.want, got.interfaceInfo[string(cns.InfraNIC)+"0"].NCResponse) + require.Equal(cns.InfraNIC, got.interfaceInfo[string(cns.InfraNIC)+"0"].NICType) }) } } diff --git a/cni/network/network.go b/cni/network/network.go index 813c58e37..7f47f7a1e 100644 --- a/cni/network/network.go +++ b/cni/network/network.go @@ -76,12 +76,13 @@ type NetPlugin struct { tb *telemetry.TelemetryBuffer nnsClient NnsClient multitenancyClient MultitenancyClient + netClient InterfaceGetter } type PolicyArgs struct { - nwInfo *network.NetworkInfo - nwCfg *cni.NetworkConfig - ipconfigs []*network.IPConfig + subnetInfos []network.SubnetInfo + nwCfg *cni.NetworkConfig + ipconfigs []*network.IPConfig } // client for node network service @@ -97,6 +98,11 @@ type NnsClient interface { DeleteContainerNetworking(ctx context.Context, podName, nwNamespace string) (*nnscontracts.ConfigureContainerNetworkingResponse, error) } +// client for getting interface +type InterfaceGetter interface { + GetNetworkInterfaces() ([]net.Interface, error) +} + // snatConfiguration contains a bool that determines whether CNI enables snat on host and snat for dns type snatConfiguration struct { EnableSnatOnHost bool @@ -129,6 +135,7 @@ func NewPlugin(name string, nm: nm, nnsClient: client, multitenancyClient: multitenancyClient, + netClient: &netio.NetIO{}, }, nil } @@ -183,11 +190,11 @@ func (plugin *NetPlugin) GetAllEndpointState(networkid string) (*api.AzureCNISta } for _, ep := range eps { - id := ep.Id + id := ep.EndpointID info := api.PodNetworkInterfaceInfo{ PodName: ep.PODName, PodNamespace: ep.PODNameSpace, - PodEndpointId: ep.Id, + PodEndpointId: ep.EndpointID, ContainerID: ep.ContainerID, IPAddresses: ep.IPAddresses, } @@ -205,8 +212,28 @@ func (plugin *NetPlugin) Stop() { logger.Info("Plugin stopped") } -// FindMasterInterface returns the name of the master interface. -func (plugin *NetPlugin) findMasterInterface(nwCfg *cni.NetworkConfig, subnetPrefix *net.IPNet) string { +// findInterfaceByMAC returns the name of the master interface +func (plugin *NetPlugin) findInterfaceByMAC(macAddress string) string { + interfaces, err := plugin.netClient.GetNetworkInterfaces() + if err != nil { + logger.Error("failed to get interfaces", zap.Error(err)) + return "" + } + macs := make([]string, 0, len(interfaces)) + for _, iface := range interfaces { + // find master interface by macAddress for Swiftv2 + macs = append(macs, iface.HardwareAddr.String()) + if iface.HardwareAddr.String() == macAddress { + return iface.Name + } + } + // Failed to find a suitable interface. + logger.Error("Failed to find interface by MAC", zap.String("macAddress", macAddress), zap.Strings("interfaces", macs)) + return "" +} + +// findMasterInterfaceBySubnet returns the name of the master interface. +func (plugin *NetPlugin) findMasterInterfaceBySubnet(nwCfg *cni.NetworkConfig, subnetPrefix *net.IPNet) string { // An explicit master configuration wins. Explicitly specifying a master is // useful if host has multiple interfaces with addresses in the same subnet. if nwCfg.Master != "" { @@ -215,7 +242,12 @@ func (plugin *NetPlugin) findMasterInterface(nwCfg *cni.NetworkConfig, subnetPre // Otherwise, pick the first interface with an IP address in the given subnet. subnetPrefixString := subnetPrefix.String() - interfaces, _ := net.Interfaces() + interfaces, err := plugin.netClient.GetNetworkInterfaces() + if err != nil { + logger.Error("failed to get interfaces", zap.Error(err)) + return "" + } + var ipnets []string for _, iface := range interfaces { addrs, _ := iface.Addrs() for _, addr := range addrs { @@ -223,6 +255,7 @@ func (plugin *NetPlugin) findMasterInterface(nwCfg *cni.NetworkConfig, subnetPre if err != nil { continue } + ipnets = append(ipnets, ipnet.String()) if subnetPrefixString == ipnet.String() { return iface.Name } @@ -230,6 +263,7 @@ func (plugin *NetPlugin) findMasterInterface(nwCfg *cni.NetworkConfig, subnetPre } // Failed to find a suitable interface. + logger.Error("Failed to find interface by subnet prefix", zap.String("subnetPrefix", subnetPrefixString), zap.Strings("interfaces", ipnets)) return "" } @@ -313,6 +347,32 @@ func addNatIPV6SubnetInfo(nwCfg *cni.NetworkConfig, } } +func (plugin *NetPlugin) addIpamInvoker(ipamAddConfig IPAMAddConfig) (IPAMAddResult, error) { + ipamAddResult, err := plugin.ipamInvoker.Add(ipamAddConfig) + if err != nil { + return IPAMAddResult{}, errors.Wrap(err, "failed to add ipam invoker") + } + sendEvent(plugin, fmt.Sprintf("Allocated IPAddress from ipam interface: %+v", ipamAddResult.PrettyString())) + return ipamAddResult, nil +} + +// get network +func (plugin *NetPlugin) getNetworkID(netNs string, interfaceInfo *network.InterfaceInfo, nwCfg *cni.NetworkConfig) (string, error) { + networkID, err := plugin.getNetworkName(netNs, interfaceInfo, nwCfg) + if err != nil { + return "", err + } + return networkID, nil +} + +// get network info for legacy +func (plugin *NetPlugin) getNetworkInfo(netNs string, interfaceInfo *network.InterfaceInfo, nwCfg *cni.NetworkConfig) network.EndpointInfo { + networkID, _ := plugin.getNetworkID(netNs, interfaceInfo, nwCfg) + nwInfo, _ := plugin.nm.GetNetworkInfo(networkID) + + return nwInfo +} + // CNI implementation // https://github.com/containernetworking/cni/blob/master/SPEC.md @@ -320,7 +380,6 @@ func addNatIPV6SubnetInfo(nwCfg *cni.NetworkConfig, func (plugin *NetPlugin) Add(args *cniSkel.CmdArgs) error { var ( ipamAddResult IPAMAddResult - ipamAddResults []IPAMAddResult azIpamResult *cniTypesCurr.Result enableInfraVnet bool enableSnatForDNS bool @@ -362,12 +421,24 @@ func (plugin *NetPlugin) Add(args *cniSkel.CmdArgs) error { telemetry.SendCNIMetric(&cniMetric, plugin.tb) // Add Interfaces to result. - defaultCniResult := convertInterfaceInfoToCniResult(ipamAddResult.defaultInterfaceInfo, args.IfName) + // previously just logged the default (infra) interface so this is equivalent behavior + cniResult := &cniTypesCurr.Result{} + for key := range ipamAddResult.interfaceInfo { + logger.Info("Exiting add, interface info retrieved", zap.Any("ifInfo", ipamAddResult.interfaceInfo[key])) + // previously we had a default interface info to select which interface info was the one to be returned from cni add + // now we have to infer which interface info should be returned + // we assume that we want to return the infra nic always, and if that is not found, return any one of the secondary interfaces + // if there is an infra nic + secondary, we will always return the infra nic (linux swift v2) + cniResult = convertInterfaceInfoToCniResult(ipamAddResult.interfaceInfo[key], args.IfName) + if ipamAddResult.interfaceInfo[key].NICType == cns.InfraNIC { + break + } + } - addSnatInterface(nwCfg, defaultCniResult) + addSnatInterface(nwCfg, cniResult) // Convert result to the requested CNI version. - res, vererr := defaultCniResult.GetAsVersion(nwCfg.CNIVersion) + res, vererr := cniResult.GetAsVersion(nwCfg.CNIVersion) if vererr != nil { logger.Error("GetAsVersion failed", zap.Error(vererr)) plugin.Error(vererr) @@ -380,10 +451,12 @@ func (plugin *NetPlugin) Add(args *cniSkel.CmdArgs) error { logger.Info("ADD command completed for", zap.String("pod", k8sPodName), - zap.Any("IPs", defaultCniResult.IPs), + zap.Any("IPs", cniResult.IPs), zap.Error(err)) }() + ipamAddResult = IPAMAddResult{interfaceInfo: make(map[string]network.InterfaceInfo)} + // Parse Pod arguments. k8sPodName, k8sNamespace, err := plugin.getPodInfo(args.Args) if err != nil { @@ -413,9 +486,11 @@ func (plugin *NetPlugin) Add(args *cniSkel.CmdArgs) error { res, err = plugin.nnsClient.AddContainerNetworking(context.Background(), k8sPodName, args.Netns) if err == nil { - ipamAddResult.defaultInterfaceInfo.IPConfigs = convertNnsToIPConfigs(res, args.IfName, k8sPodName, "AddContainerNetworking") + ipamAddResult.interfaceInfo[string(cns.InfraNIC)] = network.InterfaceInfo{ + IPConfigs: convertNnsToIPConfigs(res, args.IfName, k8sPodName, "AddContainerNetworking"), + NICType: cns.InfraNIC, + } } - return err } @@ -434,7 +509,13 @@ func (plugin *NetPlugin) Add(args *cniSkel.CmdArgs) error { return fmt.Errorf("failed to create cns client with error: %w", err) } + options := make(map[string]any) + ipamAddConfig := IPAMAddConfig{nwCfg: nwCfg, args: args, options: options} + if nwCfg.MultiTenancy { + // triggered only in swift v1 multitenancy + // dual nic multitenancy -> two interface infos + // multitenancy (swift v1) -> one interface info plugin.report.Context = "AzureCNIMultitenancy" plugin.multitenancyClient.Init(cnsClient, AzureNetIOShim{}) @@ -444,7 +525,7 @@ func (plugin *NetPlugin) Add(args *cniSkel.CmdArgs) error { return fmt.Errorf("%w", err) } - ipamAddResults, err = plugin.multitenancyClient.GetAllNetworkContainers(context.TODO(), nwCfg, k8sPodName, k8sNamespace, args.IfName) + ipamAddResult, err = plugin.multitenancyClient.GetAllNetworkContainers(context.TODO(), nwCfg, k8sPodName, k8sNamespace, args.IfName) if err != nil { err = fmt.Errorf("GetAllNetworkContainers failed for podname %s namespace %s. error: %w", k8sPodName, k8sNamespace, err) logger.Error("GetAllNetworkContainers failed", @@ -453,133 +534,298 @@ func (plugin *NetPlugin) Add(args *cniSkel.CmdArgs) error { zap.Error(err)) return err } - - if len(ipamAddResults) > 1 && !plugin.isDualNicFeatureSupported(args.Netns) { - errMsg := fmt.Sprintf("received multiple NC results %+v from CNS while dualnic feature is not supported", ipamAddResults) + // dual nic when we get multiple interface infos back (multitenancy does not necessarily have multiple if infos) + if len(ipamAddResult.interfaceInfo) > 1 && !plugin.isDualNicFeatureSupported(args.Netns) { + errMsg := fmt.Sprintf("received multiple NC results %+v from CNS while dualnic feature is not supported", ipamAddResult.interfaceInfo) logger.Error("received multiple NC results from CNS while dualnic feature is not supported", - zap.Any("results", ipamAddResult)) + zap.Any("results", ipamAddResult.interfaceInfo)) return plugin.Errorf(errMsg) } } else { - // TODO: refactor this code for simplification - // Add dummy ipamAddResult nil object for single tenancy mode - // this will be used for: ipamAddResult, err = plugin.ipamInvoker.Add(ipamAddConfig) - ipamAddResults = append(ipamAddResults, ipamAddResult) - } - - // iterate ipamAddResults and program the endpoint - for i := 0; i < len(ipamAddResults); i++ { - var networkID string - ipamAddResult = ipamAddResults[i] - - options := make(map[string]any) - networkID, err = plugin.getNetworkName(args.Netns, &ipamAddResult, nwCfg) - - endpointID := plugin.nm.GetEndpointID(args.ContainerID, args.IfName) - policies := cni.GetPoliciesFromNwCfg(nwCfg.AdditionalArgs) - - // Check whether the network already exists. - nwInfo, nwInfoErr := plugin.nm.GetNetworkInfo(networkID) - // Handle consecutive ADD calls for infrastructure containers. - // This is a temporary work around for issue #57253 of Kubernetes. - // We can delete this if statement once they fix it. - // Issue link: https://github.com/kubernetes/kubernetes/issues/57253 - - if nwInfoErr == nil { - logger.Info("Found network with subnet", - zap.String("network", networkID), - zap.String("subnet", nwInfo.Subnets[0].Prefix.String())) - nwInfo.IPAMType = nwCfg.IPAM.Type - options = nwInfo.Options - - var resultSecondAdd *cniTypesCurr.Result - resultSecondAdd, err = plugin.handleConsecutiveAdd(args, endpointID, networkID, &nwInfo, nwCfg) - if err != nil { - logger.Error("handleConsecutiveAdd failed", zap.Error(err)) - return err - } - - if resultSecondAdd != nil { - ipamAddResult.defaultInterfaceInfo = convertCniResultToInterfaceInfo(resultSecondAdd) - return nil - } - } - - // Initialize azureipam/cns ipam + // when nwcfg.multitenancy (use multitenancy flag for swift v1 only) is false if plugin.ipamInvoker == nil { switch nwCfg.IPAM.Type { case network.AzureCNS: plugin.ipamInvoker = NewCNSInvoker(k8sPodName, k8sNamespace, cnsClient, util.ExecutionMode(nwCfg.ExecutionMode), util.IpamMode(nwCfg.IPAM.Mode)) - default: + // legacy + nwInfo := plugin.getNetworkInfo(args.Netns, nil, nwCfg) plugin.ipamInvoker = NewAzureIpamInvoker(plugin, &nwInfo) } } - ipamAddConfig := IPAMAddConfig{nwCfg: nwCfg, args: args, options: options} - if !nwCfg.MultiTenancy { - ipamAddResult, err = plugin.ipamInvoker.Add(ipamAddConfig) - if err != nil { - return fmt.Errorf("IPAM Invoker Add failed with error: %w", err) - } - sendEvent(plugin, fmt.Sprintf("Allocated IPAddress from ipam DefaultInterface: %+v, SecondaryInterfaces: %+v", ipamAddResult.defaultInterfaceInfo, ipamAddResult.secondaryInterfacesInfo)) + ipamAddResult, err = plugin.addIpamInvoker(ipamAddConfig) + if err != nil { + return fmt.Errorf("IPAM Invoker Add failed with error: %w", err) } - defer func() { //nolint:gocritic - if err != nil { - // for multi-tenancies scenario, CNI is not supposed to invoke CNS for cleaning Ips - if !(nwCfg.MultiTenancy && nwCfg.IPAM.Type == network.AzureCNS) { - plugin.cleanupAllocationOnError(ipamAddResult.defaultInterfaceInfo.IPConfigs, nwCfg, args, options) + // TODO: This proably needs to be changed as we return all interfaces... + // sendEvent(plugin, fmt.Sprintf("Allocated IPAddress from ipam DefaultInterface: %+v, SecondaryInterfaces: %+v", ipamAddResult.interfaceInfo[ifIndex], ipamAddResult.interfaceInfo)) + } + + policies := cni.GetPoliciesFromNwCfg(nwCfg.AdditionalArgs) + // moved to addIpamInvoker + // sendEvent(plugin, fmt.Sprintf("Allocated IPAddress from ipam interface: %+v", ipamAddResult.PrettyString())) + + defer func() { //nolint:gocritic + if err != nil { + // for swift v1 multi-tenancies scenario, CNI is not supposed to invoke CNS for cleaning Ips + if !nwCfg.MultiTenancy { + for _, ifInfo := range ipamAddResult.interfaceInfo { + // This used to only be called for infraNIC, test if this breaks scenarios + // If it does then will have to search for infraNIC + if ifInfo.NICType == cns.InfraNIC { + plugin.cleanupAllocationOnError(ifInfo.IPConfigs, nwCfg, args, options) + } } } - }() - - // Create network - if nwInfoErr != nil { - // Network does not exist. - logger.Info("Creating network", zap.String("networkID", networkID)) - sendEvent(plugin, fmt.Sprintf("[cni-net] Creating network %v.", networkID)) - // opts map needs to get passed in here - if nwInfo, err = plugin.createNetworkInternal(networkID, policies, ipamAddConfig, ipamAddResult); err != nil { - logger.Error("Create network failed", zap.Error(err)) - return err - } - logger.Info("Created network", - zap.String("networkId", networkID), - zap.String("subnet", ipamAddResult.hostSubnetPrefix.String())) - sendEvent(plugin, fmt.Sprintf("[cni-net] Created network %v with subnet %v.", networkID, ipamAddResult.hostSubnetPrefix.String())) } + }() + + epInfos := []*network.EndpointInfo{} + infraSeen := false + endpointIndex := 0 + for key := range ipamAddResult.interfaceInfo { + ifInfo := ipamAddResult.interfaceInfo[key] natInfo := getNATInfo(nwCfg, options[network.SNATIPKey], enableSnatForDNS) + networkID, _ := plugin.getNetworkID(args.Netns, &ifInfo, nwCfg) - createEndpointInternalOpt := createEndpointInternalOpt{ + createEpInfoOpt := createEpInfoOpt{ nwCfg: nwCfg, - cnsNetworkConfig: ipamAddResult.ncResponse, + cnsNetworkConfig: ifInfo.NCResponse, ipamAddResult: ipamAddResult, azIpamResult: azIpamResult, args: args, - nwInfo: &nwInfo, policies: policies, - endpointID: endpointID, k8sPodName: k8sPodName, k8sNamespace: k8sNamespace, enableInfraVnet: enableInfraVnet, enableSnatForDNS: enableSnatForDNS, natInfo: natInfo, + networkID: networkID, + ifInfo: &ifInfo, + ipamAddConfig: &ipamAddConfig, + ipv6Enabled: ipamAddResult.ipv6Enabled, + infraSeen: &infraSeen, + endpointIndex: endpointIndex, } - - var epInfo network.EndpointInfo - epInfo, err = plugin.createEndpointInternal(&createEndpointInternalOpt) + var epInfo *network.EndpointInfo + epInfo, err = plugin.createEpInfo(&createEpInfoOpt) if err != nil { - logger.Error("Endpoint creation failed", zap.Error(err)) return err } + epInfos = append(epInfos, epInfo) + // TODO: should this statement be based on the current iteration instead of the constant ifIndex? + // TODO figure out where to put telemetry: sendEvent(plugin, fmt.Sprintf("CNI ADD succeeded: IP:%+v, VlanID: %v, podname %v, namespace %v numendpoints:%d", + // ipamAddResult.interfaceInfo[ifIndex].IPConfigs, epInfo.Data[network.VlanIDKey], k8sPodName, k8sNamespace, plugin.nm.GetNumberOfEndpoints("", nwCfg.Name))) + endpointIndex++ + } + cnsclient, err := cnscli.New(nwCfg.CNSUrl, defaultRequestTimeout) + if err != nil { + return errors.Wrap(err, "failed to create cns client") + } + defer func() { + if err != nil { - sendEvent(plugin, fmt.Sprintf("CNI ADD succeeded: IP:%+v, VlanID: %v, podname %v, namespace %v numendpoints:%d", - ipamAddResult.defaultInterfaceInfo.IPConfigs, epInfo.Data[network.VlanIDKey], k8sPodName, k8sNamespace, plugin.nm.GetNumberOfEndpoints("", nwCfg.Name))) + // Delete all endpoints + for _, epInfo := range epInfos { + deleteErr := plugin.nm.DeleteEndpoint(epInfo.NetworkID, epInfo.EndpointID, epInfo) + if deleteErr != nil { + // we already do not return an error when the endpoint is not found, so deleteErr is a real error + logger.Error("Could not delete endpoint after detecting add failure", zap.String("epInfo", epInfo.PrettyString()), zap.Error(deleteErr)) + return + } + } + // Rely on cleanupAllocationOnError declared above to delete ips + // Delete state in disk here + delErr := plugin.nm.DeleteState(epInfos) + if delErr != nil { + logger.Error("Could not delete state after detecting add failure", zap.Error(delErr)) + return + } + } + }() + + err = plugin.nm.EndpointCreate(cnsclient, epInfos) + if err != nil { + return errors.Wrap(err, "failed to create endpoint") // behavior can change if you don't assign to err prior to returning + } + // telemetry added + sendEvent(plugin, fmt.Sprintf("CNI ADD Process succeeded for interfaces: %v", ipamAddResult.PrettyString())) + return nil +} + +func (plugin *NetPlugin) findMasterInterface(opt *createEpInfoOpt) string { + switch opt.ifInfo.NICType { + case cns.InfraNIC: + return plugin.findMasterInterfaceBySubnet(opt.ipamAddConfig.nwCfg, &opt.ifInfo.HostSubnetPrefix) + case cns.DelegatedVMNIC: + return plugin.findInterfaceByMAC(opt.ifInfo.MacAddress.String()) + case cns.BackendNIC: + return "" + default: + return "" + } +} + +type createEpInfoOpt struct { + nwCfg *cni.NetworkConfig + cnsNetworkConfig *cns.GetNetworkContainerResponse + ipamAddResult IPAMAddResult + azIpamResult *cniTypesCurr.Result + args *cniSkel.CmdArgs + policies []policy.Policy + k8sPodName string + k8sNamespace string + enableInfraVnet bool + enableSnatForDNS bool + natInfo []policy.NATInfo + networkID string + + ifInfo *network.InterfaceInfo + ipamAddConfig *IPAMAddConfig + ipv6Enabled bool + + infraSeen *bool // Only the first infra gets args.ifName, even if the second infra is on a different network + endpointIndex int +} + +func (plugin *NetPlugin) createEpInfo(opt *createEpInfoOpt) (*network.EndpointInfo, error) { // you can modify to pass in whatever else you need + // ensure we can find the master interface + opt.ifInfo.HostSubnetPrefix.IP = opt.ifInfo.HostSubnetPrefix.IP.Mask(opt.ifInfo.HostSubnetPrefix.Mask) + opt.ipamAddConfig.nwCfg.IPAM.Subnet = opt.ifInfo.HostSubnetPrefix.String() + // populate endpoint info section + masterIfName := plugin.findMasterInterface(opt) + if masterIfName == "" { + err := plugin.Errorf("Failed to find the master interface") + return nil, err } - return nil + networkPolicies := opt.policies // save network policies before we modify the slice pointer for ep policies + + // populate endpoint info + epDNSInfo, err := getEndpointDNSSettings(opt.nwCfg, opt.ifInfo.DNS, opt.k8sNamespace) // Probably won't panic if given bad values + if err != nil { + + err = plugin.Errorf("Failed to getEndpointDNSSettings: %v", err) + return nil, err + } + + vethName := fmt.Sprintf("%s.%s", opt.k8sNamespace, opt.k8sPodName) + if opt.nwCfg.Mode != OpModeTransparent { + // this mechanism of using only namespace and name is not unique for different incarnations of POD/container. + // IT will result in unpredictable behavior if API server decides to + // reorder DELETE and ADD call for new incarnation of same POD. + vethName = fmt.Sprintf("%s%s%s", opt.networkID, opt.args.ContainerID, opt.args.IfName) + } + + // for secondary (Populate addresses) + // initially only for infra nic but now applied to all nic types + addresses := make([]net.IPNet, len(opt.ifInfo.IPConfigs)) + for i, ipconfig := range opt.ifInfo.IPConfigs { + addresses[i] = ipconfig.Address + } + + // generate endpoint info + var endpointID string + if opt.ifInfo.NICType == cns.InfraNIC && !*opt.infraSeen { + // so we do not break existing scenarios, only the first infra gets the original endpoint id generation + endpointID = plugin.nm.GetEndpointID(opt.args.ContainerID, opt.args.IfName) + *opt.infraSeen = true + } else { + endpointID = plugin.nm.GetEndpointID(opt.args.ContainerID, strconv.Itoa(opt.endpointIndex)) + } + + endpointInfo := network.EndpointInfo{ + NetworkID: opt.networkID, + Mode: opt.ipamAddConfig.nwCfg.Mode, + MasterIfName: masterIfName, + AdapterName: opt.ipamAddConfig.nwCfg.AdapterName, + BridgeName: opt.ipamAddConfig.nwCfg.Bridge, + NetworkPolicies: networkPolicies, // nw and ep policies separated to avoid possible conflicts + NetNs: opt.ipamAddConfig.args.Netns, + Options: opt.ipamAddConfig.options, + DisableHairpinOnHostInterface: opt.ipamAddConfig.nwCfg.DisableHairpinOnHostInterface, + IsIPv6Enabled: opt.ipv6Enabled, // present infra only + + EndpointID: endpointID, + ContainerID: opt.args.ContainerID, + NetNsPath: opt.args.Netns, // probably same value as epInfo.NetNs + IfName: opt.args.IfName, + Data: make(map[string]interface{}), + EndpointDNS: epDNSInfo, + // endpoint policies are populated later + IPsToRouteViaHost: opt.nwCfg.IPsToRouteViaHost, + EnableSnatOnHost: opt.nwCfg.EnableSnatOnHost, + EnableMultiTenancy: opt.nwCfg.MultiTenancy, + EnableInfraVnet: opt.enableInfraVnet, + EnableSnatForDns: opt.enableSnatForDNS, + PODName: opt.k8sPodName, + PODNameSpace: opt.k8sNamespace, + SkipHotAttachEp: false, // Hot attach at the time of endpoint creation + IPV6Mode: opt.nwCfg.IPV6Mode, + VnetCidrs: opt.nwCfg.VnetCidrs, + ServiceCidrs: opt.nwCfg.ServiceCidrs, + NATInfo: opt.natInfo, + NICType: opt.ifInfo.NICType, + SkipDefaultRoutes: opt.ifInfo.SkipDefaultRoutes, + Routes: opt.ifInfo.Routes, + // added the following for delegated vm nic + IPAddresses: addresses, + MacAddress: opt.ifInfo.MacAddress, + // the following is used for creating an external interface if we can't find an existing network + HostSubnetPrefix: opt.ifInfo.HostSubnetPrefix.String(), + } + + if err = addSubnetToEndpointInfo(*opt.ifInfo, &endpointInfo); err != nil { + logger.Info("Failed to add subnets to endpointInfo", zap.Error(err)) + return nil, err + } + setNetworkOptions(opt.ifInfo.NCResponse, &endpointInfo) + + // update endpoint policies + policyArgs := PolicyArgs{ + subnetInfos: endpointInfo.Subnets, // getEndpointPolicies requires nwInfo.Subnets only (checked) + nwCfg: opt.nwCfg, + ipconfigs: opt.ifInfo.IPConfigs, + } + endpointPolicies, err := getEndpointPolicies(policyArgs) + if err != nil { + logger.Error("Failed to get endpoint policies", zap.Error(err)) + return nil, err + } + // create endpoint policies by appending to network policies + // the value passed into NetworkPolicies should be unaffected since we reassign here + opt.policies = append(opt.policies, endpointPolicies...) + endpointInfo.EndpointPolicies = opt.policies + // add even more endpoint policies + epPolicies, err := getPoliciesFromRuntimeCfg(opt.nwCfg, opt.ipamAddResult.ipv6Enabled) // not specific to delegated or infra + if err != nil { + logger.Error("failed to get policies from runtime configurations", zap.Error(err)) + return nil, plugin.Errorf(err.Error()) + } + endpointInfo.EndpointPolicies = append(endpointInfo.EndpointPolicies, epPolicies...) + + if opt.ipamAddResult.ipv6Enabled { // not specific to this particular interface + endpointInfo.IPV6Mode = string(util.IpamMode(opt.nwCfg.IPAM.Mode)) // TODO: check IPV6Mode field can be deprecated and can we add IsIPv6Enabled flag for generic working + } + + if opt.azIpamResult != nil && opt.azIpamResult.IPs != nil { + endpointInfo.InfraVnetIP = opt.azIpamResult.IPs[0].Address + } + + if opt.nwCfg.MultiTenancy { + // previously only infra nic was passed into this function but now all nics are passed in (possibly breaks swift v2) + plugin.multitenancyClient.SetupRoutingForMultitenancy(opt.nwCfg, opt.cnsNetworkConfig, opt.azIpamResult, &endpointInfo, opt.ifInfo) + } + + setEndpointOptions(opt.cnsNetworkConfig, &endpointInfo, vethName) + + logger.Info("Generated endpoint info from fields", zap.String("epInfo", endpointInfo.PrettyString())) + + // now our ep info should have the full combined information from both the network and endpoint structs + return &endpointInfo, nil } // cleanup allocated ipv4 and ipv6 addresses if they exist @@ -598,75 +844,9 @@ func (plugin *NetPlugin) cleanupAllocationOnError( } } -func (plugin *NetPlugin) createNetworkInternal( - networkID string, - policies []policy.Policy, - ipamAddConfig IPAMAddConfig, - ipamAddResult IPAMAddResult, -) (network.NetworkInfo, error) { - nwInfo := network.NetworkInfo{} - ipamAddResult.hostSubnetPrefix.IP = ipamAddResult.hostSubnetPrefix.IP.Mask(ipamAddResult.hostSubnetPrefix.Mask) - ipamAddConfig.nwCfg.IPAM.Subnet = ipamAddResult.hostSubnetPrefix.String() - // Find the master interface. - masterIfName := plugin.findMasterInterface(ipamAddConfig.nwCfg, &ipamAddResult.hostSubnetPrefix) - if masterIfName == "" { - err := plugin.Errorf("Failed to find the master interface") - return nwInfo, err - } - logger.Info("Found master interface", zap.String("ifname", masterIfName)) - - // Add the master as an external interface. - err := plugin.nm.AddExternalInterface(masterIfName, ipamAddResult.hostSubnetPrefix.String()) - if err != nil { - err = plugin.Errorf("Failed to add external interface: %v", err) - return nwInfo, err - } - - nwDNSInfo, err := getNetworkDNSSettings(ipamAddConfig.nwCfg, ipamAddResult.defaultInterfaceInfo.DNS) - if err != nil { - err = plugin.Errorf("Failed to getDNSSettings: %v", err) - return nwInfo, err - } - - logger.Info("DNS Info", zap.Any("info", nwDNSInfo)) - - // Create the network. - nwInfo = network.NetworkInfo{ - Id: networkID, - Mode: ipamAddConfig.nwCfg.Mode, - MasterIfName: masterIfName, - AdapterName: ipamAddConfig.nwCfg.AdapterName, - BridgeName: ipamAddConfig.nwCfg.Bridge, - EnableSnatOnHost: ipamAddConfig.nwCfg.EnableSnatOnHost, - DNS: nwDNSInfo, - Policies: policies, - NetNs: ipamAddConfig.args.Netns, - Options: ipamAddConfig.options, - DisableHairpinOnHostInterface: ipamAddConfig.nwCfg.DisableHairpinOnHostInterface, - IPV6Mode: ipamAddConfig.nwCfg.IPV6Mode, // TODO: check if IPV6Mode field can be deprecated - IPAMType: ipamAddConfig.nwCfg.IPAM.Type, - ServiceCidrs: ipamAddConfig.nwCfg.ServiceCidrs, - IsIPv6Enabled: ipamAddResult.ipv6Enabled, - } - - if err = addSubnetToNetworkInfo(ipamAddResult, &nwInfo); err != nil { - logger.Info("Failed to add subnets to networkInfo", - zap.Error(err)) - return nwInfo, err - } - 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 addSubnetToNetworkInfo(ipamAddResult IPAMAddResult, nwInfo *network.NetworkInfo) error { - for _, ipConfig := range ipamAddResult.defaultInterfaceInfo.IPConfigs { +// construct network info with ipv4/ipv6 subnets (updates subnets field) +func addSubnetToEndpointInfo(interfaceInfo network.InterfaceInfo, nwInfo *network.EndpointInfo) error { + for _, ipConfig := range interfaceInfo.IPConfigs { ip, podSubnetPrefix, err := net.ParseCIDR(ipConfig.Address.String()) if err != nil { return fmt.Errorf("Failed to ParseCIDR for pod subnet prefix: %w", err) @@ -687,141 +867,6 @@ func addSubnetToNetworkInfo(ipamAddResult IPAMAddResult, nwInfo *network.Network return nil } -type createEndpointInternalOpt struct { - nwCfg *cni.NetworkConfig - cnsNetworkConfig *cns.GetNetworkContainerResponse - ipamAddResult IPAMAddResult - azIpamResult *cniTypesCurr.Result - args *cniSkel.CmdArgs - nwInfo *network.NetworkInfo - policies []policy.Policy - endpointID string - k8sPodName string - k8sNamespace string - enableInfraVnet bool - enableSnatForDNS bool - natInfo []policy.NATInfo -} - -func (plugin *NetPlugin) createEndpointInternal(opt *createEndpointInternalOpt) (network.EndpointInfo, error) { - epInfo := network.EndpointInfo{} - - defaultInterfaceInfo := opt.ipamAddResult.defaultInterfaceInfo - epDNSInfo, err := getEndpointDNSSettings(opt.nwCfg, defaultInterfaceInfo.DNS, opt.k8sNamespace) - if err != nil { - err = plugin.Errorf("Failed to getEndpointDNSSettings: %v", err) - return epInfo, err - } - policyArgs := PolicyArgs{ - nwInfo: opt.nwInfo, - nwCfg: opt.nwCfg, - ipconfigs: defaultInterfaceInfo.IPConfigs, - } - endpointPolicies, err := getEndpointPolicies(policyArgs) - if err != nil { - logger.Error("Failed to get endpoint policies", zap.Error(err)) - return epInfo, err - } - - opt.policies = append(opt.policies, endpointPolicies...) - - vethName := fmt.Sprintf("%s.%s", opt.k8sNamespace, opt.k8sPodName) - if opt.nwCfg.Mode != OpModeTransparent { - // this mechanism of using only namespace and name is not unique for different incarnations of POD/container. - // IT will result in unpredictable behavior if API server decides to - // reorder DELETE and ADD call for new incarnation of same POD. - vethName = fmt.Sprintf("%s%s%s", opt.nwInfo.Id, opt.args.ContainerID, opt.args.IfName) - } - - epInfo = network.EndpointInfo{ - Id: opt.endpointID, - ContainerID: opt.args.ContainerID, - NetNsPath: opt.args.Netns, - IfName: opt.args.IfName, - Data: make(map[string]interface{}), - DNS: epDNSInfo, - Policies: opt.policies, - IPsToRouteViaHost: opt.nwCfg.IPsToRouteViaHost, - EnableSnatOnHost: opt.nwCfg.EnableSnatOnHost, - EnableMultiTenancy: opt.nwCfg.MultiTenancy, - EnableInfraVnet: opt.enableInfraVnet, - EnableSnatForDns: opt.enableSnatForDNS, - PODName: opt.k8sPodName, - PODNameSpace: opt.k8sNamespace, - SkipHotAttachEp: false, // Hot attach at the time of endpoint creation - IPV6Mode: opt.nwCfg.IPV6Mode, - VnetCidrs: opt.nwCfg.VnetCidrs, - ServiceCidrs: opt.nwCfg.ServiceCidrs, - NATInfo: opt.natInfo, - NICType: cns.InfraNIC, - SkipDefaultRoutes: opt.ipamAddResult.defaultInterfaceInfo.SkipDefaultRoutes, - Routes: defaultInterfaceInfo.Routes, - } - - epPolicies, err := getPoliciesFromRuntimeCfg(opt.nwCfg, opt.ipamAddResult.ipv6Enabled) - if err != nil { - logger.Error("failed to get policies from runtime configurations", zap.Error(err)) - return epInfo, plugin.Errorf(err.Error()) - } - epInfo.Policies = append(epInfo.Policies, epPolicies...) - - // Populate addresses. - for _, ipconfig := range defaultInterfaceInfo.IPConfigs { - epInfo.IPAddresses = append(epInfo.IPAddresses, ipconfig.Address) - } - - if opt.ipamAddResult.ipv6Enabled { - epInfo.IPV6Mode = string(util.IpamMode(opt.nwCfg.IPAM.Mode)) // TODO: check IPV6Mode field can be deprecated and can we add IsIPv6Enabled flag for generic working - } - - if opt.azIpamResult != nil && opt.azIpamResult.IPs != nil { - epInfo.InfraVnetIP = opt.azIpamResult.IPs[0].Address - } - - if opt.nwCfg.MultiTenancy { - plugin.multitenancyClient.SetupRoutingForMultitenancy(opt.nwCfg, opt.cnsNetworkConfig, opt.azIpamResult, &epInfo, &defaultInterfaceInfo) - } - - setEndpointOptions(opt.cnsNetworkConfig, &epInfo, vethName) - - cnsclient, err := cnscli.New(opt.nwCfg.CNSUrl, defaultRequestTimeout) - if err != nil { - logger.Error("failed to initialized cns client", zap.String("url", opt.nwCfg.CNSUrl), - zap.String("error", err.Error())) - return epInfo, plugin.Errorf(err.Error()) - } - - epInfos := []*network.EndpointInfo{&epInfo} - // get secondary interface info - for _, secondaryCniResult := range opt.ipamAddResult.secondaryInterfacesInfo { - var addresses []net.IPNet - for _, ipconfig := range secondaryCniResult.IPConfigs { - addresses = append(addresses, ipconfig.Address) - } - - epInfos = append(epInfos, - &network.EndpointInfo{ - ContainerID: epInfo.ContainerID, - NetNsPath: epInfo.NetNsPath, - IPAddresses: addresses, - Routes: secondaryCniResult.Routes, - MacAddress: secondaryCniResult.MacAddress, - NICType: secondaryCniResult.NICType, - SkipDefaultRoutes: secondaryCniResult.SkipDefaultRoutes, - }) - } - - // Create the endpoint. - logger.Info("Creating endpoint", zap.String("endpointInfo", epInfo.PrettyString())) - sendEvent(plugin, fmt.Sprintf("[cni-net] Creating endpoint %s.", epInfo.PrettyString())) - err = plugin.nm.CreateEndpoint(cnsclient, opt.nwInfo.Id, epInfos) - if err != nil { - err = plugin.Errorf("Failed to create endpoint: %v", err) - } - - return epInfo, err -} - // Get handles CNI Get commands. func (plugin *NetPlugin) Get(args *cniSkel.CmdArgs) error { var ( @@ -911,8 +956,8 @@ func (plugin *NetPlugin) Get(args *cniSkel.CmdArgs) error { result.Routes = append(result.Routes, &cniTypes.Route{Dst: route.Dst, GW: route.Gw}) } - result.DNS.Nameservers = epInfo.DNS.Servers - result.DNS.Domain = epInfo.DNS.Suffix + result.DNS.Nameservers = epInfo.EndpointDNS.Servers + result.DNS.Domain = epInfo.EndpointDNS.Suffix return nil } @@ -925,8 +970,7 @@ func (plugin *NetPlugin) Delete(args *cniSkel.CmdArgs) error { k8sPodName string k8sNamespace string networkID string - nwInfo network.NetworkInfo - epInfo *network.EndpointInfo + nwInfo network.EndpointInfo cniMetric telemetry.AIMetric ) @@ -999,6 +1043,7 @@ func (plugin *NetPlugin) Delete(args *cniSkel.CmdArgs) error { plugin.ipamInvoker = NewCNSInvoker(k8sPodName, k8sNamespace, cnsClient, util.ExecutionMode(nwCfg.ExecutionMode), util.IpamMode(nwCfg.IPAM.Mode)) default: + // nwInfo gets populated later in the function plugin.ipamInvoker = NewAzureIpamInvoker(plugin, &nwInfo) } } @@ -1008,83 +1053,87 @@ func (plugin *NetPlugin) Delete(args *cniSkel.CmdArgs) error { // deleted, getNetworkName will return error of the type NetworkNotFoundError which will result in nil error as compliance // with CNI SPEC as mentioned below. - numEndpointsToDelete := 1 - // only get number of endpoints if it's multitenancy mode - if nwCfg.MultiTenancy { - numEndpointsToDelete = plugin.nm.GetNumEndpointsByContainerID(args.ContainerID) - } - - logger.Info("Endpoints to be deleted", zap.Int("count", numEndpointsToDelete)) - for i := 0; i < numEndpointsToDelete; i++ { - // Initialize values from network config. - networkID, err = plugin.getNetworkName(args.Netns, nil, nwCfg) - if err != nil { - // If error is not found error, then we ignore it, to comply with CNI SPEC. - if network.IsNetworkNotFoundError(err) { - err = nil - return err - } - - logger.Error("Failed to extract network name from network config", zap.Error(err)) - err = plugin.Errorf("Failed to extract network name from network config. error: %v", err) - return err - } - // Query the network. - if nwInfo, err = plugin.nm.GetNetworkInfo(networkID); err != nil { - if !nwCfg.MultiTenancy { - logger.Error("Failed to query network", - zap.String("network", networkID), - zap.Error(err)) - // Log the error but return success if the network is not found. - // if cni hits this, mostly state file would be missing and it can be reboot scenario where - // container runtime tries to delete and create pods which existed before reboot. - // this condition will not apply to stateless CNI since the network struct will be crated on each call - err = nil - if !plugin.nm.IsStatelessCNIMode() { - return err - } - } - } - - endpointID := plugin.nm.GetEndpointID(args.ContainerID, args.IfName) - // Query the endpoint. - if epInfo, err = plugin.nm.GetEndpointInfo(networkID, endpointID); err != nil { - logger.Info("GetEndpoint", - zap.String("endpoint", endpointID), + // We get the network id and nw info here to preserve existing behavior + networkID, err = plugin.getNetworkID(args.Netns, nil, nwCfg) + if nwInfo, err = plugin.nm.GetNetworkInfo(networkID); err != nil { + if !nwCfg.MultiTenancy { + logger.Error("Failed to query network", + zap.String("network", networkID), zap.Error(err)) - if !nwCfg.MultiTenancy { - // attempt to release address associated with this Endpoint id - // This is to ensure clean up is done even in failure cases - - logger.Error("Failed to query endpoint", - zap.String("endpoint", endpointID), - zap.Error(err)) - logger.Error("Release ip by ContainerID (endpoint not found)", - zap.String("containerID", args.ContainerID)) - sendEvent(plugin, fmt.Sprintf("Release ip by ContainerID (endpoint not found):%v", args.ContainerID)) - if err = plugin.ipamInvoker.Delete(nil, nwCfg, args, nwInfo.Options); err != nil { - return plugin.RetriableError(fmt.Errorf("failed to release address(no endpoint): %w", err)) - } - } - // Log the error but return success if the endpoint being deleted is not found. + // Log the error if the network is not found. + // if cni hits this, mostly state file would be missing and it can be reboot scenario where + // container runtime tries to delete and create pods which existed before reboot. + // this condition will not apply to stateless CNI since the network struct will be crated on each call + err = nil + } + } + // Initialize values from network config. + if err != nil { + // if swift v1 multitenancy and we got an error retrieving the nwInfo + // If error is not found error, then we ignore it, to comply with CNI SPEC. + if network.IsNetworkNotFoundError(err) { err = nil return err } - // schedule send metric before attempting delete - defer sendMetricFunc() //nolint:gocritic - logger.Info("Deleting endpoint", - zap.String("endpointID", endpointID)) - sendEvent(plugin, fmt.Sprintf("Deleting endpoint:%v", endpointID)) - // Delete the endpoint. - if err = plugin.nm.DeleteEndpoint(networkID, endpointID, epInfo); err != nil { + logger.Error("Failed to extract network name from network config", zap.Error(err)) + err = plugin.Errorf("Failed to extract network name from network config. error: %v", err) + return err + } + logger.Info("Retrieved network info, populating endpoint infos with container id", zap.String("containerID", args.ContainerID)) + + var epInfos []*network.EndpointInfo + if plugin.nm.IsStatelessCNIMode() { + // network ID is passed in and used only for migration + // otherwise, in stateless, we don't need the network id for deletion + epInfos, err = plugin.nm.GetEndpointState(networkID, args.ContainerID) + } else { + epInfos = plugin.nm.GetEndpointInfosFromContainerID(args.ContainerID) + } + + // for when the endpoint is not created, but the ips are already allocated (only works if single network, single infra) + // stateless cni won't have this issue + if len(epInfos) == 0 { + endpointID := plugin.nm.GetEndpointID(args.ContainerID, args.IfName) + if !nwCfg.MultiTenancy { + logger.Error("Failed to query endpoint", + zap.String("endpoint", endpointID), + zap.Error(err)) + logger.Error("Release ip by ContainerID (endpoint not found)", + zap.String("containerID", args.ContainerID)) + sendEvent(plugin, fmt.Sprintf("Release ip by ContainerID (endpoint not found):%v", args.ContainerID)) + if err = plugin.ipamInvoker.Delete(nil, nwCfg, args, nwInfo.Options); err != nil { + return plugin.RetriableError(fmt.Errorf("failed to release address(no endpoint): %w", err)) + } + } + // Log the error but return success if the endpoint being deleted is not found. + err = nil + return err + } + logger.Info("Deleting the endpoints", zap.Any("endpointInfos", epInfos)) + // populate ep infos here in loop if necessary + // delete endpoints + for _, epInfo := range epInfos { + // in stateless, network id is not populated in epInfo, but in stateful cni, it is (nw id is used in stateful) + if err = plugin.nm.DeleteEndpoint(epInfo.NetworkID, epInfo.EndpointID, epInfo); err != nil { + // An error will not be returned if the endpoint is not found // return a retriable error so the container runtime will retry this DEL later // the implementation of this function returns nil if the endpoint doens't exist, so // we don't have to check that here return plugin.RetriableError(fmt.Errorf("failed to delete endpoint: %w", err)) } + } + logger.Info("Deleting the endpoints from the ipam") + // delete endpoint state in cns and in statefile + for _, epInfo := range epInfos { + // schedule send metric before attempting delete + defer sendMetricFunc() //nolint:gocritic + logger.Info("Deleting endpoint", + zap.String("endpointID", epInfo.EndpointID)) + sendEvent(plugin, fmt.Sprintf("Deleting endpoint:%v", epInfo.EndpointID)) - if !nwCfg.MultiTenancy { + if !nwCfg.MultiTenancy && epInfo.NICType != cns.DelegatedVMNIC { + // Delegated/secondary nic ips are statically allocated so we don't need to release // Call into IPAM plugin to release the endpoint's addresses. for i := range epInfo.IPAddresses { logger.Info("Release ip", zap.String("ip", epInfo.IPAddresses[i].IP.String())) @@ -1094,7 +1143,7 @@ func (plugin *NetPlugin) Delete(args *cniSkel.CmdArgs) error { return plugin.RetriableError(fmt.Errorf("failed to release address: %w", err)) } } - } else if epInfo.EnableInfraVnet { + } else if epInfo.EnableInfraVnet { // remove in future PR nwCfg.IPAM.Subnet = nwInfo.Subnets[0].Prefix.String() nwCfg.IPAM.Address = epInfo.InfraVnetIP.IP.String() err = plugin.ipamInvoker.Delete(nil, nwCfg, args, nwInfo.Options) @@ -1103,6 +1152,11 @@ func (plugin *NetPlugin) Delete(args *cniSkel.CmdArgs) error { } } } + logger.Info("Deleting the state from the cni statefile") + err = plugin.nm.DeleteState(epInfos) + if err != nil { + return plugin.RetriableError(fmt.Errorf("failed to save state: %w", err)) + } sendEvent(plugin, fmt.Sprintf("CNI DEL succeeded : Released ip %+v podname %v namespace %v", nwCfg.IPAM.Address, k8sPodName, k8sNamespace)) return err @@ -1289,7 +1343,7 @@ func (plugin *NetPlugin) Update(args *cniSkel.CmdArgs) error { // Update the endpoint. logger.Info("Now updating existing endpoint with targetNetworkConfig", - zap.String("endpoint", existingEpInfo.Id), + zap.String("endpoint", existingEpInfo.EndpointID), zap.Any("config", targetNetworkConfig)) if err = plugin.nm.UpdateEndpoint(networkID, existingEpInfo, targetEpInfo); err != nil { err = plugin.Errorf("Failed to update endpoint: %v", err) @@ -1355,6 +1409,7 @@ func convertInterfaceInfoToCniResult(info network.InterfaceInfo, ifName string) Interfaces: []*cniTypesCurr.Interface{ { Name: ifName, + Mac: info.MacAddress.String(), }, }, DNS: cniTypes.DNS{ diff --git a/cni/network/network_linux.go b/cni/network/network_linux.go index 19c134a73..95c145e42 100644 --- a/cni/network/network_linux.go +++ b/cni/network/network_linux.go @@ -8,7 +8,6 @@ import ( "github.com/Azure/azure-container-networking/cns" "github.com/Azure/azure-container-networking/network" "github.com/Azure/azure-container-networking/network/policy" - cniSkel "github.com/containernetworking/cni/pkg/skel" cniTypesCurr "github.com/containernetworking/cni/pkg/types/100" ) @@ -19,13 +18,6 @@ const ( const snatConfigFileName = "/tmp/snatConfig" -// handleConsecutiveAdd is a dummy function for Linux platform. -func (plugin *NetPlugin) handleConsecutiveAdd(args *cniSkel.CmdArgs, endpointID string, networkID string, - nwInfo *network.NetworkInfo, nwCfg *cni.NetworkConfig, -) (*cniTypesCurr.Result, error) { - return nil, nil -} - func addDefaultRoute(gwIPString string, epInfo *network.EndpointInfo, result *network.InterfaceInfo) { _, defaultIPNet, _ := net.ParseCIDR("0.0.0.0/0") dstIP := net.IPNet{IP: net.ParseIP("0.0.0.0"), Mask: defaultIPNet.Mask} @@ -41,7 +33,8 @@ func addSnatForDNS(gwIPString string, epInfo *network.EndpointInfo, result *netw result.Routes = append(result.Routes, network.RouteInfo{Dst: *dnsIPNet, Gw: gwIP}) } -func setNetworkOptions(cnsNwConfig *cns.GetNetworkContainerResponse, nwInfo *network.NetworkInfo) { +// updates options field +func setNetworkOptions(cnsNwConfig *cns.GetNetworkContainerResponse, nwInfo *network.EndpointInfo) { if cnsNwConfig != nil && cnsNwConfig.MultiTenancyInfo.ID != 0 { logger.Info("Setting Network Options") vlanMap := make(map[string]interface{}) @@ -119,7 +112,7 @@ func addIPV6EndpointPolicy(nwInfo network.NetworkInfo) (policy.Policy, error) { return policy.Policy{}, nil } -func (plugin *NetPlugin) getNetworkName(_ string, _ *IPAMAddResult, nwCfg *cni.NetworkConfig) (string, error) { +func (plugin *NetPlugin) getNetworkName(_ string, _ *network.InterfaceInfo, nwCfg *cni.NetworkConfig) (string, error) { return nwCfg.Name, nil } diff --git a/cni/network/network_linux_test.go b/cni/network/network_linux_test.go index ed2994dbc..b0fda7454 100644 --- a/cni/network/network_linux_test.go +++ b/cni/network/network_linux_test.go @@ -16,7 +16,7 @@ func TestSetNetworkOptions(t *testing.T) { tests := []struct { name string cnsNwConfig cns.GetNetworkContainerResponse - nwInfo network.NetworkInfo + nwInfo network.EndpointInfo expectedVlanID string expectedSnatBrIP string }{ @@ -34,7 +34,7 @@ func TestSetNetworkOptions(t *testing.T) { GatewayIPAddress: "169.254.0.1", }, }, - nwInfo: network.NetworkInfo{ + nwInfo: network.EndpointInfo{ Options: make(map[string]interface{}), }, expectedVlanID: "1", diff --git a/cni/network/network_test.go b/cni/network/network_test.go index 0df2010a8..68a22d7bf 100644 --- a/cni/network/network_test.go +++ b/cni/network/network_test.go @@ -382,7 +382,7 @@ func TestIpamAddFail(t *testing.T) { }, wantErr: []bool{false}, wantEndpointErr: true, - wantErrMsg: "Failed to create endpoint: MockEndpointClient Error : Endpoint Error", + wantErrMsg: "failed to create endpoint: MockEndpointClient Error : Endpoint Error", expectedEndpoints: 0, }, } @@ -696,18 +696,41 @@ func TestPluginMultitenancyDelete(t *testing.T) { Master: "eth0", } + happyArgs := &cniSkel.CmdArgs{ + StdinData: localNwCfg.Serialize(), + ContainerID: "test-container", + Netns: "test-container", + Args: fmt.Sprintf("K8S_POD_NAME=%v;K8S_POD_NAMESPACE=%v", "test-pod", "test-pod-ns"), + IfName: eth0IfName, + } + tests := []struct { name string methods []string args *cniSkel.CmdArgs + delArgs *cniSkel.CmdArgs wantErr bool wantErrMsg string }{ { name: "Multitenancy delete success", methods: []string{CNI_ADD, CNI_DEL}, - args: &cniSkel.CmdArgs{ - StdinData: localNwCfg.Serialize(), + args: happyArgs, + delArgs: happyArgs, + wantErr: false, + }, + { + name: "Multitenancy delete net not found", + methods: []string{CNI_ADD, CNI_DEL}, + args: happyArgs, + delArgs: &cniSkel.CmdArgs{ + StdinData: (&cni.NetworkConfig{ + CNIVersion: "0.3.0", + Name: "othernet", + MultiTenancy: true, + EnableExactMatchForPodName: true, + Master: "eth0", + }).Serialize(), ContainerID: "test-container", Netns: "test-container", Args: fmt.Sprintf("K8S_POD_NAME=%v;K8S_POD_NAMESPACE=%v", "test-pod", "test-pod-ns"), @@ -725,7 +748,7 @@ func TestPluginMultitenancyDelete(t *testing.T) { if method == CNI_ADD { err = plugin.Add(tt.args) } else if method == CNI_DEL { - err = plugin.Delete(tt.args) + err = plugin.Delete(tt.delArgs) } } if tt.wantErr { @@ -1024,7 +1047,7 @@ func getTestEndpoint(podname, podnamespace, ipwithcidr, podinterfaceid, infracon ep := acnnetwork.EndpointInfo{ PODName: podname, PODNameSpace: podnamespace, - Id: podinterfaceid, + EndpointID: podinterfaceid, ContainerID: infracontainerid, IPAddresses: []net.IPNet{ *ipnet, @@ -1042,13 +1065,13 @@ func TestGetAllEndpointState(t *testing.T) { ep2 := getTestEndpoint("podname2", "podnamespace2", "10.0.0.2/24", "podinterfaceid2", "testcontainerid2") ep3 := getTestEndpoint("podname3", "podnamespace3", "10.240.1.242/16", "podinterfaceid3", "testcontainerid3") - err := plugin.nm.CreateEndpoint(nil, networkid, []*acnnetwork.EndpointInfo{ep1}) + err := plugin.nm.CreateEndpoint(nil, networkid, ep1) require.NoError(t, err) - err = plugin.nm.CreateEndpoint(nil, networkid, []*acnnetwork.EndpointInfo{ep2}) + err = plugin.nm.CreateEndpoint(nil, networkid, ep2) require.NoError(t, err) - err = plugin.nm.CreateEndpoint(nil, networkid, []*acnnetwork.EndpointInfo{ep3}) + err = plugin.nm.CreateEndpoint(nil, networkid, ep3) require.NoError(t, err) state, err := plugin.GetAllEndpointState(networkid) @@ -1056,22 +1079,22 @@ func TestGetAllEndpointState(t *testing.T) { res := &api.AzureCNIState{ ContainerInterfaces: map[string]api.PodNetworkInterfaceInfo{ - ep1.Id: { - PodEndpointId: ep1.Id, + ep1.EndpointID: { + PodEndpointId: ep1.EndpointID, PodName: ep1.PODName, PodNamespace: ep1.PODNameSpace, ContainerID: ep1.ContainerID, IPAddresses: ep1.IPAddresses, }, - ep2.Id: { - PodEndpointId: ep2.Id, + ep2.EndpointID: { + PodEndpointId: ep2.EndpointID, PodName: ep2.PODName, PodNamespace: ep2.PODNameSpace, ContainerID: ep2.ContainerID, IPAddresses: ep2.IPAddresses, }, - ep3.Id: { - PodEndpointId: ep3.Id, + ep3.EndpointID: { + PodEndpointId: ep3.EndpointID, PodName: ep3.PODName, PodNamespace: ep3.PODNameSpace, ContainerID: ep3.ContainerID, @@ -1133,6 +1156,18 @@ func TestGetPodSubnetNatInfo(t *testing.T) { } } +type InterfaceGetterMock struct { + interfaces []net.Interface + err error +} + +func (n *InterfaceGetterMock) GetNetworkInterfaces() ([]net.Interface, error) { + if n.err != nil { + return nil, n.err + } + return n.interfaces, nil +} + func TestPluginSwiftV2Add(t *testing.T) { plugin, _ := cni.NewPlugin("name", "0.3.0") @@ -1144,6 +1179,14 @@ func TestPluginSwiftV2Add(t *testing.T) { Master: "eth0", } + args := &cniSkel.CmdArgs{ + StdinData: localNwCfg.Serialize(), + ContainerID: "test-container", + Netns: "test-container", + Args: fmt.Sprintf("K8S_POD_NAME=%v;K8S_POD_NAMESPACE=%v", "test-pod", "test-pod-ns"), + IfName: eth0IfName, + } + tests := []struct { name string plugin *NetPlugin @@ -1159,14 +1202,13 @@ func TestPluginSwiftV2Add(t *testing.T) { ipamInvoker: NewMockIpamInvoker(false, false, false, true, false), report: &telemetry.CNIReport{}, tb: &telemetry.TelemetryBuffer{}, + netClient: &InterfaceGetterMock{ + interfaces: []net.Interface{ + {Name: "eth0"}, + }, + }, }, - args: &cniSkel.CmdArgs{ - StdinData: localNwCfg.Serialize(), - ContainerID: "test-container", - Netns: "test-container", - Args: fmt.Sprintf("K8S_POD_NAME=%v;K8S_POD_NAMESPACE=%v", "test-pod", "test-pod-ns"), - IfName: eth0IfName, - }, + args: args, wantErr: false, }, { @@ -1177,16 +1219,15 @@ func TestPluginSwiftV2Add(t *testing.T) { ipamInvoker: NewMockIpamInvoker(false, false, false, true, true), report: &telemetry.CNIReport{}, tb: &telemetry.TelemetryBuffer{}, + netClient: &InterfaceGetterMock{ + interfaces: []net.Interface{ + {Name: "eth0"}, + }, + }, }, - args: &cniSkel.CmdArgs{ - StdinData: localNwCfg.Serialize(), - ContainerID: "test-container", - Netns: "test-container", - Args: fmt.Sprintf("K8S_POD_NAME=%v;K8S_POD_NAMESPACE=%v", "test-pod", "test-pod-ns"), - IfName: eth0IfName, - }, + args: args, wantErr: true, - wantErrMsg: "IPAM Invoker Add failed with error: delegatedVMNIC fail", + wantErrMsg: "IPAM Invoker Add failed with error: failed to add ipam invoker: delegatedVMNIC fail", }, { name: "SwiftV2 EndpointClient Add fail", @@ -1202,16 +1243,58 @@ func TestPluginSwiftV2Add(t *testing.T) { ipamInvoker: NewMockIpamInvoker(false, false, false, true, false), report: &telemetry.CNIReport{}, tb: &telemetry.TelemetryBuffer{}, + netClient: &InterfaceGetterMock{ + interfaces: []net.Interface{ + {Name: "eth0"}, + }, + }, + }, + args: args, + wantErr: true, + wantErrMsg: "failed to create endpoint: MockEndpointClient Error : AddEndpoints Delegated VM NIC failed", + }, + { + name: "SwiftV2 Find Interface By MAC Address Fail", + plugin: &NetPlugin{ + Plugin: plugin, + nm: acnnetwork.NewMockNetworkmanager(acnnetwork.NewMockEndpointClient(nil)), + ipamInvoker: NewMockIpamInvoker(false, false, false, true, false), + report: &telemetry.CNIReport{}, + tb: &telemetry.TelemetryBuffer{}, + netClient: &InterfaceGetterMock{ + interfaces: []net.Interface{}, + }, + }, + args: args, + wantErr: true, + wantErrMsg: "Failed to find the master interface", + }, + { + name: "SwiftV2 Find Interface By Subnet Prefix Fail", + plugin: &NetPlugin{ + Plugin: plugin, + nm: acnnetwork.NewMockNetworkmanager(acnnetwork.NewMockEndpointClient(nil)), + ipamInvoker: NewMockIpamInvoker(false, false, false, false, false), + report: &telemetry.CNIReport{}, + tb: &telemetry.TelemetryBuffer{}, + netClient: &InterfaceGetterMock{ + interfaces: []net.Interface{}, + }, }, args: &cniSkel.CmdArgs{ - StdinData: localNwCfg.Serialize(), + StdinData: (&cni.NetworkConfig{ + CNIVersion: "0.3.0", + Name: "swiftv2", + ExecutionMode: string(util.V4Overlay), + EnableExactMatchForPodName: true, + }).Serialize(), ContainerID: "test-container", Netns: "test-container", Args: fmt.Sprintf("K8S_POD_NAME=%v;K8S_POD_NAMESPACE=%v", "test-pod", "test-pod-ns"), IfName: eth0IfName, }, wantErr: true, - wantErrMsg: "Failed to create endpoint: MockEndpointClient Error : AddEndpoints Delegated VM NIC failed", + wantErrMsg: "Failed to find the master interface", }, } @@ -1221,7 +1304,7 @@ func TestPluginSwiftV2Add(t *testing.T) { err := tt.plugin.Add(tt.args) if tt.wantErr { require.Error(t, err) - assert.Equal(t, err.Error(), tt.wantErrMsg, "Expected %v but got %+v", tt.wantErrMsg, err.Error()) + assert.Equal(t, tt.wantErrMsg, err.Error(), "Expected %v but got %+v", tt.wantErrMsg, err.Error()) endpoints, _ := tt.plugin.nm.GetAllEndpoints(localNwCfg.Name) require.Condition(t, assert.Comparison(func() bool { return len(endpoints) == 0 })) } else { @@ -1232,3 +1315,147 @@ func TestPluginSwiftV2Add(t *testing.T) { }) } } + +func TestPluginSwiftV2MultipleAddDelete(t *testing.T) { + // checks cases where we create multiple endpoints in one call (also checks endpoint id) + // assumes we never get two infras created in one add call + plugin, _ := cni.NewPlugin("name", "0.3.0") + + localNwCfg := cni.NetworkConfig{ + CNIVersion: "0.3.0", + Name: "swiftv2", + ExecutionMode: string(util.V4Overlay), + EnableExactMatchForPodName: true, + Master: "eth0", + } + + args := &cniSkel.CmdArgs{ + StdinData: localNwCfg.Serialize(), + ContainerID: "test-container", + Netns: "test-container", + Args: fmt.Sprintf("K8S_POD_NAME=%v;K8S_POD_NAMESPACE=%v", "test-pod", "test-pod-ns"), + IfName: eth0IfName, + } + + tests := []struct { + name string + plugin *NetPlugin + args *cniSkel.CmdArgs + wantErr bool + wantErrMsg string + wantNumEps int + validEpIDs map[string]struct{} + }{ + { + name: "SwiftV2 Add Infra and Delegated", + plugin: &NetPlugin{ + Plugin: plugin, + nm: acnnetwork.NewMockNetworkmanager(acnnetwork.NewMockEndpointClient(nil)), + ipamInvoker: NewCustomMockIpamInvoker(map[string]acnnetwork.InterfaceInfo{ + "eth0-1": { + NICType: cns.DelegatedVMNIC, + }, + "eth0": { + NICType: cns.InfraNIC, + }, + }), + report: &telemetry.CNIReport{}, + tb: &telemetry.TelemetryBuffer{}, + netClient: &InterfaceGetterMock{ + interfaces: []net.Interface{ + {Name: "eth0"}, + }, + }, + }, + args: args, + wantErr: false, + wantNumEps: 2, + }, + { + name: "SwiftV2 Add Two Delegated", + plugin: &NetPlugin{ + Plugin: plugin, + nm: acnnetwork.NewMockNetworkmanager(acnnetwork.NewMockEndpointClient(nil)), + ipamInvoker: NewCustomMockIpamInvoker(map[string]acnnetwork.InterfaceInfo{ + "eth0": { + NICType: cns.DelegatedVMNIC, + }, + "eth0-1": { + NICType: cns.DelegatedVMNIC, + }, + }), + report: &telemetry.CNIReport{}, + tb: &telemetry.TelemetryBuffer{}, + netClient: &InterfaceGetterMock{ + interfaces: []net.Interface{ + {Name: "eth0"}, + }, + }, + }, + args: args, + wantErr: false, + wantNumEps: 2, + }, + { + // creates 2 endpoints, the first succeeds, the second doesn't + // ensures that delete is called to clean up the first endpoint that succeeded + name: "SwiftV2 Partial Add fail", + plugin: &NetPlugin{ + Plugin: plugin, + nm: acnnetwork.NewMockNetworkmanager(acnnetwork.NewMockEndpointClient(func(ep *acnnetwork.EndpointInfo) error { + if ep.NICType == cns.DelegatedVMNIC { + return acnnetwork.NewErrorMockEndpointClient("AddEndpoints Delegated VM NIC failed") //nolint:wrapcheck // ignore wrapping for test + } + + return nil + })), + ipamInvoker: NewCustomMockIpamInvoker(map[string]acnnetwork.InterfaceInfo{ + "eth0": { + NICType: cns.InfraNIC, + }, + "eth0-1": { + NICType: cns.DelegatedVMNIC, + }, + }), + report: &telemetry.CNIReport{}, + tb: &telemetry.TelemetryBuffer{}, + netClient: &InterfaceGetterMock{ + interfaces: []net.Interface{ + {Name: "eth0"}, + }, + }, + }, + args: args, + wantNumEps: 0, + wantErr: true, + wantErrMsg: "failed to create endpoint: MockEndpointClient Error : AddEndpoints Delegated VM NIC failed", + }, + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + err := tt.plugin.Add(tt.args) + if tt.wantErr { + require.Error(t, err) + assert.Equal(t, tt.wantErrMsg, err.Error(), "Expected %v but got %+v", tt.wantErrMsg, err.Error()) + } else { + require.NoError(t, err) + } + endpoints, _ := tt.plugin.nm.GetAllEndpoints(localNwCfg.Name) + require.Condition(t, assert.Comparison(func() bool { return len(endpoints) == tt.wantNumEps })) + for _, ep := range endpoints { + if ep.NICType == cns.InfraNIC { + require.Equal(t, "test-con-"+tt.args.IfName, ep.EndpointID, "infra nic must use ifname for its endpoint id") + } else { + require.Regexp(t, `test-con-\d+$`, ep.EndpointID, "other nics must use an index for their endpoint ids") + } + } + + err = tt.plugin.Delete(tt.args) + require.NoError(t, err) + endpoints, _ = tt.plugin.nm.GetAllEndpoints(localNwCfg.Name) + require.Condition(t, assert.Comparison(func() bool { return len(endpoints) == 0 })) + }) + } +} diff --git a/cni/network/network_windows.go b/cni/network/network_windows.go index ba0fdfaf4..6ebfecb12 100644 --- a/cni/network/network_windows.go +++ b/cni/network/network_windows.go @@ -18,8 +18,6 @@ import ( "github.com/Azure/azure-container-networking/network/policy" "github.com/Microsoft/hcsshim" hnsv2 "github.com/Microsoft/hcsshim/hcn" - cniSkel "github.com/containernetworking/cni/pkg/skel" - cniTypes "github.com/containernetworking/cni/pkg/types" cniTypesCurr "github.com/containernetworking/cni/pkg/types/100" "github.com/pkg/errors" "go.uber.org/zap" @@ -30,93 +28,17 @@ var ( snatConfigFileName = filepath.FromSlash(os.Getenv("TEMP")) + "\\snatConfig" // windows build for version 1903 win1903Version = 18362 + dualStackCount = 2 ) -/* handleConsecutiveAdd handles consecutive add calls for infrastructure containers on Windows platform. - * This is a temporary work around for issue #57253 of Kubernetes. - * We can delete this if statement once they fix it. - * Issue link: https://github.com/kubernetes/kubernetes/issues/57253 - */ -func (plugin *NetPlugin) handleConsecutiveAdd(args *cniSkel.CmdArgs, endpointId string, networkId string, - nwInfo *network.NetworkInfo, nwCfg *cni.NetworkConfig, -) (*cniTypesCurr.Result, error) { - epInfo, _ := plugin.nm.GetEndpointInfo(networkId, endpointId) - if epInfo == nil { - return nil, nil - } - - // Return in case of HNSv2 as consecutive add call doesn't need to be handled - if useHnsV2, err := network.UseHnsV2(args.Netns); useHnsV2 { - return nil, err - } - - hnsEndpoint, err := network.Hnsv1.GetHNSEndpointByName(endpointId) - if hnsEndpoint != nil { - logger.Info("Found existing endpoint through hcsshim", - zap.Any("endpoint", hnsEndpoint)) - endpoint, _ := network.Hnsv1.GetHNSEndpointByID(hnsEndpoint.Id) - isAttached, _ := network.Hnsv1.IsAttached(endpoint, args.ContainerID) - // Attach endpoint if it's not attached yet. - if !isAttached { - logger.Info("Attaching endpoint to container", - zap.String("endpoint", hnsEndpoint.Id), - zap.String("container", args.ContainerID)) - err := network.Hnsv1.HotAttachEndpoint(args.ContainerID, hnsEndpoint.Id) - if err != nil { - logger.Error("Failed to hot attach shared endpoint to container", - zap.String("endpoint", hnsEndpoint.Id), - zap.String("container", args.ContainerID), - zap.Error(err)) - return nil, err - } - } - - // Populate result. - address := nwInfo.Subnets[0].Prefix - address.IP = hnsEndpoint.IPAddress - result := &cniTypesCurr.Result{ - IPs: []*cniTypesCurr.IPConfig{ - { - Address: address, - Gateway: net.ParseIP(hnsEndpoint.GatewayAddress), - }, - }, - Routes: []*cniTypes.Route{ - { - Dst: net.IPNet{net.IPv4zero, net.IPv4Mask(0, 0, 0, 0)}, - GW: net.ParseIP(hnsEndpoint.GatewayAddress), - }, - }, - } - - if nwCfg.IPV6Mode != "" && len(epInfo.IPAddresses) > 1 { - ipv6Config := &cniTypesCurr.IPConfig{ - Address: epInfo.IPAddresses[1], - } - - if len(nwInfo.Subnets) > 1 { - ipv6Config.Gateway = nwInfo.Subnets[1].Gateway - } - - result.IPs = append(result.IPs, ipv6Config) - } - - // Populate DNS servers. - result.DNS.Nameservers = nwCfg.DNS.Nameservers - return result, nil - } - - err = fmt.Errorf("GetHNSEndpointByName for %v returned nil with err %v", endpointId, err) - return nil, err -} - func addDefaultRoute(_ string, _ *network.EndpointInfo, _ *network.InterfaceInfo) { } func addSnatForDNS(_ string, _ *network.EndpointInfo, _ *network.InterfaceInfo) { } -func setNetworkOptions(cnsNwConfig *cns.GetNetworkContainerResponse, nwInfo *network.NetworkInfo) { +// updates options field +func setNetworkOptions(cnsNwConfig *cns.GetNetworkContainerResponse, nwInfo *network.EndpointInfo) { if cnsNwConfig != nil && cnsNwConfig.MultiTenancyInfo.ID != 0 { logger.Info("Setting Network Options") vlanMap := make(map[string]interface{}) @@ -125,7 +47,7 @@ func setNetworkOptions(cnsNwConfig *cns.GetNetworkContainerResponse, nwInfo *net } } -func setEndpointOptions(cnsNwConfig *cns.GetNetworkContainerResponse, epInfo *network.EndpointInfo, vethName string) { +func setEndpointOptions(cnsNwConfig *cns.GetNetworkContainerResponse, epInfo *network.EndpointInfo, _ string) { if cnsNwConfig != nil && cnsNwConfig.MultiTenancyInfo.ID != 0 { logger.Info("Setting Endpoint Options") var cnetAddressMap []string @@ -142,8 +64,20 @@ func setEndpointOptions(cnsNwConfig *cns.GetNetworkContainerResponse, epInfo *ne func addSnatInterface(nwCfg *cni.NetworkConfig, result *cniTypesCurr.Result) { } -func (plugin *NetPlugin) getNetworkName(netNs string, ipamAddResult *IPAMAddResult, nwCfg *cni.NetworkConfig) (string, error) { +func (plugin *NetPlugin) getNetworkName(netNs string, interfaceInfo *network.InterfaceInfo, nwCfg *cni.NetworkConfig) (string, error) { + var err error + // Swiftv2 path => interfaceInfo.NICType = delegated NIC + // For singletenancy => nwCfg.Name + // Swiftv1 => interfaceInfo.NCResponse != nil && ipamAddResult != nil + determineWinVer() + // Swiftv2 L1VH Network Name + swiftv2NetworkNamePrefix := "azure-" + if interfaceInfo != nil && interfaceInfo.NICType == cns.DelegatedVMNIC { + logger.Info("swiftv2", zap.String("network name", interfaceInfo.MacAddress.String())) + return swiftv2NetworkNamePrefix + interfaceInfo.MacAddress.String(), nil + } + // For singletenancy, the network name is simply the nwCfg.Name if !nwCfg.MultiTenancy { return nwCfg.Name, nil @@ -156,9 +90,15 @@ func (plugin *NetPlugin) getNetworkName(netNs string, ipamAddResult *IPAMAddResu // First try to build the network name from the cnsResponse if present // This will happen during ADD call - if ipamAddResult != nil && ipamAddResult.ncResponse != nil { + // ifIndex, err := findDefaultInterface(*ipamAddResult) + if interfaceInfo != nil && interfaceInfo.NCResponse != nil { // swiftv1 path + if err != nil { + logger.Error("Error finding InfraNIC interface", + zap.Error(err)) + return "", errors.Wrap(err, "cns did not return an InfraNIC") + } // networkName will look like ~ azure-vlan1-172-28-1-0_24 - ipAddrNet := ipamAddResult.defaultInterfaceInfo.IPConfigs[0].Address + ipAddrNet := interfaceInfo.IPConfigs[0].Address prefix, err := netip.ParsePrefix(ipAddrNet.String()) if err != nil { logger.Error("Error parsing network CIDR", @@ -168,7 +108,7 @@ func (plugin *NetPlugin) getNetworkName(netNs string, ipamAddResult *IPAMAddResu } networkName := strings.ReplaceAll(prefix.Masked().String(), ".", "-") networkName = strings.ReplaceAll(networkName, "/", "_") - networkName = fmt.Sprintf("%s-vlan%v-%v", nwCfg.Name, ipamAddResult.ncResponse.MultiTenancyInfo.ID, networkName) + networkName = fmt.Sprintf("%s-vlan%v-%v", nwCfg.Name, interfaceInfo.NCResponse.MultiTenancyInfo.ID, networkName) return networkName, nil } @@ -284,7 +224,6 @@ func getPoliciesFromRuntimeCfg(nwCfg *cni.NetworkConfig, isIPv6Enabled bool) ([] Protocol: protocol, Flags: flag, }) - if err != nil { return nil, errors.Wrap(err, "failed to marshal HNS portMappingPolicySetting") } @@ -293,7 +232,6 @@ func getPoliciesFromRuntimeCfg(nwCfg *cni.NetworkConfig, isIPv6Enabled bool) ([] Type: hnsv2.PortMapping, Settings: rawPolicy, }) - if err != nil { return nil, errors.Wrap(err, "failed to marshal HNS endpointPolicy") } @@ -315,7 +253,7 @@ func getEndpointPolicies(args PolicyArgs) ([]policy.Policy, error) { var policies []policy.Policy if args.nwCfg.IPV6Mode == network.IPV6Nat { - ipv6Policy, err := getIPV6EndpointPolicy(args.nwInfo) + ipv6Policy, err := getIPV6EndpointPolicy(args.subnetInfos) if err != nil { return nil, errors.Wrap(err, "failed to get ipv6 endpoint policy") } @@ -358,15 +296,15 @@ func getLoopbackDSRPolicy(args PolicyArgs) ([]policy.Policy, error) { return policies, nil } -func getIPV6EndpointPolicy(nwInfo *network.NetworkInfo) (policy.Policy, error) { +func getIPV6EndpointPolicy(subnetInfos []network.SubnetInfo) (policy.Policy, error) { var eppolicy policy.Policy - if len(nwInfo.Subnets) < 2 { + if len(subnetInfos) < dualStackCount { return eppolicy, fmt.Errorf("network state doesn't have ipv6 subnet") } // Everything should be snat'd except podcidr - exceptionList := []string{nwInfo.Subnets[1].Prefix.String()} + exceptionList := []string{subnetInfos[1].Prefix.String()} rawPolicy, _ := json.Marshal(&hcsshim.OutboundNatPolicy{ Policy: hcsshim.Policy{Type: hcsshim.OutboundNat}, Exceptions: exceptionList, diff --git a/cni/network/network_windows_test.go b/cni/network/network_windows_test.go index eaa61f426..cb0240559 100644 --- a/cni/network/network_windows_test.go +++ b/cni/network/network_windows_test.go @@ -31,13 +31,13 @@ func TestAddWithRunTimeNetPolicies(t *testing.T) { tests := []struct { name string - nwInfo network.NetworkInfo + nwInfo network.EndpointInfo wantErr bool wantErrMsg string }{ { name: "add ipv6 endpoint policy", - nwInfo: network.NetworkInfo{ + nwInfo: network.EndpointInfo{ Subnets: []network.SubnetInfo{ { Gateway: net.ParseIP("10.240.0.1"), @@ -56,7 +56,7 @@ func TestAddWithRunTimeNetPolicies(t *testing.T) { for _, tt := range tests { tt := tt t.Run(tt.name, func(t *testing.T) { - p, err := getIPV6EndpointPolicy(&tt.nwInfo) + p, err := getIPV6EndpointPolicy(tt.nwInfo.Subnets) if tt.wantErr { require.Error(t, err) } else { @@ -143,7 +143,7 @@ func TestSetNetworkOptions(t *testing.T) { tests := []struct { name string cnsNwConfig cns.GetNetworkContainerResponse - nwInfo network.NetworkInfo + nwInfo network.EndpointInfo expectedVlanID string }{ { @@ -153,7 +153,7 @@ func TestSetNetworkOptions(t *testing.T) { ID: 1, }, }, - nwInfo: network.NetworkInfo{ + nwInfo: network.EndpointInfo{ Options: make(map[string]interface{}), }, expectedVlanID: "1", @@ -294,7 +294,7 @@ func TestDSRPolciy(t *testing.T) { EnableLoopbackDSR: true, }, }, - nwInfo: &network.NetworkInfo{}, + subnetInfos: []network.SubnetInfo{}, ipconfigs: []*network.IPConfig{ { Address: func() net.IPNet { @@ -309,8 +309,8 @@ func TestDSRPolciy(t *testing.T) { { name: "test disable dsr policy", args: PolicyArgs{ - nwCfg: &cni.NetworkConfig{}, - nwInfo: &network.NetworkInfo{}, + nwCfg: &cni.NetworkConfig{}, + subnetInfos: []network.SubnetInfo{}, ipconfigs: []*network.IPConfig{ { Address: func() net.IPNet { @@ -341,7 +341,7 @@ func TestGetNetworkNameFromCNS(t *testing.T) { plugin *NetPlugin netNs string nwCfg *cni.NetworkConfig - ipamAddResult *IPAMAddResult + interfaceInfo *network.InterfaceInfo want string wantErr bool }{ @@ -360,20 +360,18 @@ func TestGetNetworkNameFromCNS(t *testing.T) { Name: "azure", MultiTenancy: true, }, - ipamAddResult: &IPAMAddResult{ - ncResponse: &cns.GetNetworkContainerResponse{ - MultiTenancyInfo: cns.MultiTenancyInfo{ - ID: 1, + interfaceInfo: &network.InterfaceInfo{ + IPConfigs: []*network.IPConfig{ + { + Address: net.IPNet{ + IP: net.ParseIP("10.240.0.5"), + Mask: net.CIDRMask(24, 32), + }, }, }, - defaultInterfaceInfo: network.InterfaceInfo{ - IPConfigs: []*network.IPConfig{ - { - Address: net.IPNet{ - IP: net.ParseIP("10.240.0.5"), - Mask: net.CIDRMask(24, 32), - }, - }, + NCResponse: &cns.GetNetworkContainerResponse{ + MultiTenancyInfo: cns.MultiTenancyInfo{ + ID: 1, }, }, }, @@ -395,20 +393,18 @@ func TestGetNetworkNameFromCNS(t *testing.T) { Name: "azure", MultiTenancy: true, }, - ipamAddResult: &IPAMAddResult{ - ncResponse: &cns.GetNetworkContainerResponse{ - MultiTenancyInfo: cns.MultiTenancyInfo{ - ID: 1, + interfaceInfo: &network.InterfaceInfo{ + IPConfigs: []*network.IPConfig{ + { + Address: net.IPNet{ + IP: net.ParseIP(""), + Mask: net.CIDRMask(24, 32), + }, }, }, - defaultInterfaceInfo: network.InterfaceInfo{ - IPConfigs: []*network.IPConfig{ - { - Address: net.IPNet{ - IP: net.ParseIP(""), - Mask: net.CIDRMask(24, 32), - }, - }, + NCResponse: &cns.GetNetworkContainerResponse{ + MultiTenancyInfo: cns.MultiTenancyInfo{ + ID: 1, }, }, }, @@ -430,20 +426,18 @@ func TestGetNetworkNameFromCNS(t *testing.T) { Name: "azure", MultiTenancy: true, }, - ipamAddResult: &IPAMAddResult{ - ncResponse: &cns.GetNetworkContainerResponse{ - MultiTenancyInfo: cns.MultiTenancyInfo{ - ID: 1, + interfaceInfo: &network.InterfaceInfo{ + IPConfigs: []*network.IPConfig{ + { + Address: net.IPNet{ + IP: net.ParseIP("10.0.00.6"), + Mask: net.CIDRMask(24, 32), + }, }, }, - defaultInterfaceInfo: network.InterfaceInfo{ - IPConfigs: []*network.IPConfig{ - { - Address: net.IPNet{ - IP: net.ParseIP("10.0.00.6"), - Mask: net.CIDRMask(24, 32), - }, - }, + NCResponse: &cns.GetNetworkContainerResponse{ + MultiTenancyInfo: cns.MultiTenancyInfo{ + ID: 1, }, }, }, @@ -465,20 +459,18 @@ func TestGetNetworkNameFromCNS(t *testing.T) { Name: "azure", MultiTenancy: true, }, - ipamAddResult: &IPAMAddResult{ - ncResponse: &cns.GetNetworkContainerResponse{ - MultiTenancyInfo: cns.MultiTenancyInfo{ - ID: 1, + interfaceInfo: &network.InterfaceInfo{ + IPConfigs: []*network.IPConfig{ + { + Address: net.IPNet{ + IP: net.ParseIP("10.0.0.6"), + Mask: net.CIDRMask(24, 32), + }, }, }, - defaultInterfaceInfo: network.InterfaceInfo{ - IPConfigs: []*network.IPConfig{ - { - Address: net.IPNet{ - IP: net.ParseIP("10.0.0.6"), - Mask: net.CIDRMask(24, 32), - }, - }, + NCResponse: &cns.GetNetworkContainerResponse{ + MultiTenancyInfo: cns.MultiTenancyInfo{ + ID: 1, }, }, }, @@ -500,18 +492,16 @@ func TestGetNetworkNameFromCNS(t *testing.T) { Name: "azure", MultiTenancy: false, }, - ipamAddResult: &IPAMAddResult{ - ncResponse: &cns.GetNetworkContainerResponse{}, - defaultInterfaceInfo: network.InterfaceInfo{ - IPConfigs: []*network.IPConfig{ - { - Address: net.IPNet{ - IP: net.ParseIP("10.0.0.6"), - Mask: net.CIDRMask(24, 32), - }, + interfaceInfo: &network.InterfaceInfo{ + IPConfigs: []*network.IPConfig{ + { + Address: net.IPNet{ + IP: net.ParseIP("10.0.0.6"), + Mask: net.CIDRMask(24, 32), }, }, }, + NCResponse: &cns.GetNetworkContainerResponse{}, }, want: "azure", wantErr: false, @@ -521,7 +511,7 @@ func TestGetNetworkNameFromCNS(t *testing.T) { for _, tt := range tests { tt := tt t.Run(tt.name, func(t *testing.T) { - networkName, err := tt.plugin.getNetworkName(tt.netNs, tt.ipamAddResult, tt.nwCfg) + networkName, err := tt.plugin.getNetworkName(tt.netNs, tt.interfaceInfo, tt.nwCfg) if tt.wantErr { require.Error(t, err) } else { @@ -531,3 +521,75 @@ func TestGetNetworkNameFromCNS(t *testing.T) { }) } } + +func TestGetNetworkNameSwiftv2FromCNS(t *testing.T) { + // TODO: Add IB and Accelnet NIC test to this test + plugin, _ := cni.NewPlugin("name", "0.3.0") + + macAddress := "00:00:5e:00:53:01" + swiftv2NetworkNamePrefix := "azure-" + parsedMacAddress, _ := net.ParseMAC(macAddress) + swiftv2L1VHSecondaryInterfacesInfo := make(map[string]network.InterfaceInfo) + + swiftv2L1VHInterfaceInfo := network.InterfaceInfo{ + Name: "swiftv2L1VHinterface", + MacAddress: parsedMacAddress, + NICType: cns.DelegatedVMNIC, + } + swiftv2L1VHSecondaryInterfacesInfo[macAddress] = swiftv2L1VHInterfaceInfo + + tests := []struct { + name string + plugin *NetPlugin + netNs string + nwCfg *cni.NetworkConfig + ipamAddResult *IPAMAddResult + want net.HardwareAddr + wantErr bool + }{ + { + name: "Get Network Name from CNS for swiftv2", + plugin: &NetPlugin{ + Plugin: plugin, + nm: network.NewMockNetworkmanager(network.NewMockEndpointClient(nil)), + ipamInvoker: NewMockIpamInvoker(false, false, false, true, false), + report: &telemetry.CNIReport{}, + tb: &telemetry.TelemetryBuffer{}, + }, + netNs: "azure", + nwCfg: &cni.NetworkConfig{ + CNIVersion: "0.3.0", + MultiTenancy: false, + }, + ipamAddResult: &IPAMAddResult{ + interfaceInfo: swiftv2L1VHSecondaryInterfacesInfo, + }, + want: parsedMacAddress, + wantErr: false, + }, + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Log(tt.ipamAddResult.interfaceInfo) + networkName, err := tt.plugin.getNetworkName(tt.netNs, &swiftv2L1VHInterfaceInfo, tt.nwCfg) + if tt.wantErr { + require.Error(t, err) + } else { + expectedMacAddress := swiftv2NetworkNamePrefix + tt.want.String() + require.NoError(t, err) + require.Equal(t, expectedMacAddress, networkName) + } + + networkID, err := tt.plugin.getNetworkID(tt.netNs, &swiftv2L1VHInterfaceInfo, tt.nwCfg) + if tt.wantErr { + require.Error(t, err) + } else { + expectedMacAddress := swiftv2NetworkNamePrefix + tt.want.String() + require.NoError(t, err) + require.Equal(t, expectedMacAddress, networkID) + } + }) + } +} diff --git a/cnm/network/network.go b/cnm/network/network.go index af9484e22..94fff34bf 100644 --- a/cnm/network/network.go +++ b/cnm/network/network.go @@ -145,9 +145,9 @@ func (plugin *netPlugin) createNetwork(w http.ResponseWriter, r *http.Request) { } // Process request. - nwInfo := network.NetworkInfo{ - Id: req.NetworkID, - Options: req.Options, + nwInfo := network.EndpointInfo{ + NetworkID: req.NetworkID, + Options: req.Options, } // Parse network options. @@ -236,7 +236,7 @@ func (plugin *netPlugin) createEndpoint(w http.ResponseWriter, r *http.Request) } epInfo := network.EndpointInfo{ - Id: req.EndpointID, + EndpointID: req.EndpointID, IPAddresses: []net.IPNet{*ipv4Address}, SkipHotAttachEp: true, // Skip hot attach endpoint as it's done in Join } @@ -247,7 +247,8 @@ func (plugin *netPlugin) createEndpoint(w http.ResponseWriter, r *http.Request) if err != nil { log.Errorf("failed to init CNS client", err) } - err = plugin.nm.CreateEndpoint(cnscli, req.NetworkID, []*network.EndpointInfo{&epInfo}) + err = plugin.nm.CreateEndpoint(cnscli, req.NetworkID, &epInfo) + // TODO: Because create endpoint no longer assigns to the map or saves to a file, you need to handle it in cnm right here! if err != nil { plugin.SendErrorResponse(w, err) return diff --git a/netio/netio.go b/netio/netio.go index 9462c8ed3..13937b957 100644 --- a/netio/netio.go +++ b/netio/netio.go @@ -48,3 +48,8 @@ func (ns *NetIO) GetNetworkInterfaceByMac(mac net.HardwareAddr) (*net.Interface, return nil, ErrInterfaceNotFound } + +func (ns *NetIO) GetNetworkInterfaces() ([]net.Interface, error) { + ifs, err := net.Interfaces() + return ifs, errors.Wrap(err, "GetNetworkInterfaces failed") +} diff --git a/network/bridge_networkclient_linux.go b/network/bridge_networkclient_linux.go index 48e394170..c5159b264 100644 --- a/network/bridge_networkclient_linux.go +++ b/network/bridge_networkclient_linux.go @@ -25,7 +25,7 @@ func newErrorLinuxBridgeClient(errStr string) error { type LinuxBridgeClient struct { bridgeName string hostInterfaceName string - nwInfo NetworkInfo + nwInfo EndpointInfo netlink netlink.NetlinkInterface nuClient networkutils.NetworkUtils } @@ -33,7 +33,7 @@ type LinuxBridgeClient struct { func NewLinuxBridgeClient( bridgeName string, hostInterfaceName string, - nwInfo NetworkInfo, + nwInfo EndpointInfo, nl netlink.NetlinkInterface, plc platform.ExecClient, ) *LinuxBridgeClient { diff --git a/network/endpoint.go b/network/endpoint.go index e530810e5..9b251b5e5 100644 --- a/network/endpoint.go +++ b/network/endpoint.go @@ -53,26 +53,29 @@ type endpoint struct { PODName string `json:",omitempty"` PODNameSpace string `json:",omitempty"` InfraVnetAddressSpace string `json:",omitempty"` - NetNs string `json:",omitempty"` + NetNs string `json:",omitempty"` // used in windows // SecondaryInterfaces is a map of interface name to InterfaceInfo SecondaryInterfaces map[string]*InterfaceInfo + // Store nic type since we no longer populate SecondaryInterfaces + NICType cns.NICType } // EndpointInfo contains read-only information about an endpoint. type EndpointInfo struct { - Id string + EndpointID string ContainerID string NetNsPath string - IfName string + IfName string // value differs during creation vs. deletion flow SandboxKey string IfIndex int MacAddress net.HardwareAddr - DNS DNSInfo + EndpointDNS DNSInfo IPAddresses []net.IPNet IPsToRouteViaHost []string InfraVnetIP net.IPNet Routes []RouteInfo - Policies []policy.Policy + EndpointPolicies []policy.Policy // used in windows + NetworkPolicies []policy.Policy // used in windows Gateways []net.IP EnableSnatOnHost bool EnableInfraVnet bool @@ -94,7 +97,20 @@ type EndpointInfo struct { SkipDefaultRoutes bool HNSEndpointID string HNSNetworkID string - HostIfName string + HostIfName string // unused in windows, and in linux + // Fields related to the network are below + MasterIfName string + AdapterName string + NetworkID string + Mode string + Subnets []SubnetInfo + BridgeName string + NetNs string // used in windows + Options map[string]interface{} + DisableHairpinOnHostInterface bool + IsIPv6Enabled bool + + HostSubnetPrefix string // can be used later to add an external interface } // RouteInfo contains information about an IP route. @@ -118,6 +134,8 @@ type InterfaceInfo struct { DNS DNSInfo NICType cns.NICType SkipDefaultRoutes bool + HostSubnetPrefix net.IPNet // Move this field from ipamAddResult + NCResponse *cns.GetNetworkContainerResponse } type IPConfig struct { @@ -131,9 +149,14 @@ type apipaClient interface { } func (epInfo *EndpointInfo) PrettyString() string { - return fmt.Sprintf("Id:%s ContainerID:%s NetNsPath:%s IfName:%s IfIndex:%d MacAddr:%s IPAddrs:%v Gateways:%v Data:%+v", - epInfo.Id, epInfo.ContainerID, epInfo.NetNsPath, epInfo.IfName, epInfo.IfIndex, epInfo.MacAddress.String(), epInfo.IPAddresses, - epInfo.Gateways, epInfo.Data) + return fmt.Sprintf("Id:%s ContainerID:%s NetNsPath:%s IfName:%s IfIndex:%d MacAddr:%s IPAddrs:%v Gateways:%v Data:%+v NICType: %s NetworkContainerID: %s HostIfName: %s NetNs: %s", + epInfo.EndpointID, epInfo.ContainerID, epInfo.NetNsPath, epInfo.IfName, epInfo.IfIndex, epInfo.MacAddress.String(), epInfo.IPAddresses, + epInfo.Gateways, epInfo.Data, epInfo.NICType, epInfo.NetworkContainerID, epInfo.HostIfName, epInfo.NetNs) +} + +func (ifInfo *InterfaceInfo) PrettyString() string { + return fmt.Sprintf("Name:%s NICType:%v MacAddr:%s IPConfigs:%+v Routes:%+v DNSInfo:%+v", + ifInfo.Name, ifInfo.NICType, ifInfo.MacAddress.String(), ifInfo.IPConfigs, ifInfo.Routes, ifInfo.DNS) } // NewEndpoint creates a new endpoint in the network. @@ -144,14 +167,14 @@ func (nw *network) newEndpoint( netioCli netio.NetIOInterface, nsc NamespaceClientInterface, iptc ipTablesClient, - epInfo []*EndpointInfo, + epInfo *EndpointInfo, ) (*endpoint, error) { var ep *endpoint var err error defer func() { if err != nil { - logger.Error("Failed to create endpoint with err", zap.String("id", epInfo[0].Id), zap.Error(err)) + logger.Error("Failed to create endpoint with err", zap.String("id", epInfo.EndpointID), zap.Error(err)) } }() @@ -251,14 +274,14 @@ func podNameMatches(source string, actualValue string, doExactMatch bool) bool { // GetInfo returns information about the endpoint. func (ep *endpoint) getInfo() *EndpointInfo { info := &EndpointInfo{ - Id: ep.Id, + EndpointID: ep.Id, IPAddresses: ep.IPAddresses, InfraVnetIP: ep.InfraVnetIP, Data: make(map[string]interface{}), MacAddress: ep.MacAddress, SandboxKey: ep.SandboxKey, IfIndex: 0, // Azure CNI supports only one interface - DNS: ep.DNS, + EndpointDNS: ep.DNS, EnableSnatOnHost: ep.EnableSnatOnHost, EnableInfraVnet: ep.EnableInfraVnet, EnableMultiTenancy: ep.EnableMultitenancy, @@ -272,6 +295,7 @@ func (ep *endpoint) getInfo() *EndpointInfo { NetworkContainerID: ep.NetworkContainerID, HNSEndpointID: ep.HnsId, HostIfName: ep.HostIfName, + NICType: ep.NICType, } info.Routes = append(info.Routes, ep.Routes...) @@ -318,11 +342,13 @@ func (nm *networkManager) updateEndpoint(nw *network, existingEpInfo, targetEpIn zap.String("id", nw.Id), zap.Any("targetEpInfo", targetEpInfo)) defer func() { if err != nil { - logger.Error("Failed to update endpoint with err", zap.String("id", existingEpInfo.Id), zap.Error(err)) + logger.Error("Failed to update endpoint with err", zap.String("id", existingEpInfo.EndpointID), zap.Error(err)) } }() - ep := nw.Endpoints[existingEpInfo.Id] + logger.Info("Trying to retrieve endpoint id", zap.String("id", existingEpInfo.EndpointID)) + + ep := nw.Endpoints[existingEpInfo.EndpointID] if ep == nil { return errEndpointNotFound } @@ -336,7 +362,7 @@ func (nm *networkManager) updateEndpoint(nw *network, existingEpInfo, targetEpIn } // Update routes for existing endpoint - nw.Endpoints[existingEpInfo.Id].Routes = ep.Routes + nw.Endpoints[existingEpInfo.EndpointID].Routes = ep.Routes return nil } diff --git a/network/endpoint_linux.go b/network/endpoint_linux.go index 729fdb7c3..6854d2e34 100644 --- a/network/endpoint_linux.go +++ b/network/endpoint_linux.go @@ -57,201 +57,209 @@ func (nw *network) newEndpointImpl( testEpClient EndpointClient, nsc NamespaceClientInterface, iptc ipTablesClient, - epInfo []*EndpointInfo, + epInfo *EndpointInfo, ) (*endpoint, error) { var ( - err error - hostIfName string - contIfName string - localIP string - vlanid = 0 - defaultEpInfo = epInfo[0] - containerIf *net.Interface + err error + hostIfName string + contIfName string + localIP string + vlanid = 0 + containerIf *net.Interface ) - if nw.Endpoints[defaultEpInfo.Id] != nil { + if nw.Endpoints[epInfo.EndpointID] != nil { logger.Info("[net] Endpoint already exists.") err = errEndpointExists return nil, err } - if defaultEpInfo.Data != nil { - if _, ok := defaultEpInfo.Data[VlanIDKey]; ok { - vlanid = defaultEpInfo.Data[VlanIDKey].(int) + if epInfo.Data != nil { + if _, ok := epInfo.Data[VlanIDKey]; ok { + vlanid = epInfo.Data[VlanIDKey].(int) } - if _, ok := defaultEpInfo.Data[LocalIPKey]; ok { - localIP = defaultEpInfo.Data[LocalIPKey].(string) + if _, ok := epInfo.Data[LocalIPKey]; ok { + localIP = epInfo.Data[LocalIPKey].(string) } } - if _, ok := defaultEpInfo.Data[OptVethName]; ok { - key := defaultEpInfo.Data[OptVethName].(string) + if _, ok := epInfo.Data[OptVethName]; ok { + key := epInfo.Data[OptVethName].(string) logger.Info("Generate veth name based on the key provided", zap.String("key", key)) vethname := generateVethName(key) hostIfName = fmt.Sprintf("%s%s", hostVEthInterfacePrefix, vethname) + // we don't save the contIfName here or below because it will be renamed to ep.IfName anyway when we call SetupContainerInterfaces in the clients + // however, contIfName is still passed into our clients contIfName = fmt.Sprintf("%s%s2", hostVEthInterfacePrefix, vethname) } else { // Create a veth pair. logger.Info("Generate veth name based on endpoint id") - hostIfName = fmt.Sprintf("%s%s", hostVEthInterfacePrefix, defaultEpInfo.Id[:7]) - contIfName = fmt.Sprintf("%s%s-2", hostVEthInterfacePrefix, defaultEpInfo.Id[:7]) + hostIfName = fmt.Sprintf("%s%s", hostVEthInterfacePrefix, epInfo.EndpointID[:7]) + contIfName = fmt.Sprintf("%s%s-2", hostVEthInterfacePrefix, epInfo.EndpointID[:7]) + } + + nicName := epInfo.IfName + // infra nic nicname will look like eth0, and delegated/secondary nics will be moved into the container namespace + if epInfo.NICType != cns.InfraNIC { + nicName = epInfo.MasterIfName } ep := &endpoint{ - Id: defaultEpInfo.Id, - IfName: contIfName, // container veth pair name. In cnm, we won't rename this and docker expects veth name. + Id: epInfo.EndpointID, + // IfName should end up being eth0 in non-delegated nic cases + IfName: nicName, // container veth pair name. In cnm, we won't rename this and docker expects veth name. HostIfName: hostIfName, - InfraVnetIP: defaultEpInfo.InfraVnetIP, + InfraVnetIP: epInfo.InfraVnetIP, LocalIP: localIP, - IPAddresses: defaultEpInfo.IPAddresses, - DNS: defaultEpInfo.DNS, + IPAddresses: epInfo.IPAddresses, + DNS: epInfo.EndpointDNS, VlanID: vlanid, - EnableSnatOnHost: defaultEpInfo.EnableSnatOnHost, - EnableInfraVnet: defaultEpInfo.EnableInfraVnet, - EnableMultitenancy: defaultEpInfo.EnableMultiTenancy, - AllowInboundFromHostToNC: defaultEpInfo.AllowInboundFromHostToNC, - AllowInboundFromNCToHost: defaultEpInfo.AllowInboundFromNCToHost, - NetworkNameSpace: defaultEpInfo.NetNsPath, - ContainerID: defaultEpInfo.ContainerID, - PODName: defaultEpInfo.PODName, - PODNameSpace: defaultEpInfo.PODNameSpace, - Routes: defaultEpInfo.Routes, + EnableSnatOnHost: epInfo.EnableSnatOnHost, + EnableInfraVnet: epInfo.EnableInfraVnet, + EnableMultitenancy: epInfo.EnableMultiTenancy, + AllowInboundFromHostToNC: epInfo.AllowInboundFromHostToNC, + AllowInboundFromNCToHost: epInfo.AllowInboundFromNCToHost, + NetworkNameSpace: epInfo.NetNsPath, + ContainerID: epInfo.ContainerID, + PODName: epInfo.PODName, + PODNameSpace: epInfo.PODNameSpace, + Routes: epInfo.Routes, SecondaryInterfaces: make(map[string]*InterfaceInfo), + NICType: epInfo.NICType, } if nw.extIf != nil { ep.Gateways = []net.IP{nw.extIf.IPv4Gateway} } - for _, epInfo := range epInfo { - // testEpClient is non-nil only when the endpoint is created for the unit test - // resetting epClient to testEpClient in loop to use the test endpoint client if specified - epClient := testEpClient - if epClient == nil { - //nolint:gocritic - if vlanid != 0 { - if nw.Mode == opModeTransparentVlan { - logger.Info("Transparent vlan client") - if _, ok := epInfo.Data[SnatBridgeIPKey]; ok { - nw.SnatBridgeIP = epInfo.Data[SnatBridgeIPKey].(string) - } - epClient = NewTransparentVlanEndpointClient(nw, epInfo, hostIfName, contIfName, vlanid, localIP, nl, plc, nsc, iptc) - } else { - logger.Info("OVS client") - if _, ok := epInfo.Data[SnatBridgeIPKey]; ok { - nw.SnatBridgeIP = epInfo.Data[SnatBridgeIPKey].(string) - } - - epClient = NewOVSEndpointClient( - nw, - epInfo, - hostIfName, - contIfName, - vlanid, - localIP, - nl, - ovsctl.NewOvsctl(), - plc, - iptc) - } - } else if nw.Mode != opModeTransparent { - logger.Info("Bridge client") - epClient = NewLinuxBridgeEndpointClient(nw.extIf, hostIfName, contIfName, nw.Mode, nl, plc) - } else if epInfo.NICType == cns.DelegatedVMNIC { - logger.Info("Secondary client") - epClient = NewSecondaryEndpointClient(nl, netioCli, plc, nsc, ep) - } else { - logger.Info("Transparent client") - epClient = NewTransparentEndpointClient(nw.extIf, hostIfName, contIfName, nw.Mode, nl, netioCli, plc) - } - } - + // testEpClient is non-nil only when the endpoint is created for the unit test + // resetting epClient to testEpClient in loop to use the test endpoint client if specified + epClient := testEpClient + if epClient == nil { //nolint:gocritic - defer func(client EndpointClient, contIfName string) { - // Cleanup on failure. - if err != nil { - logger.Error("CNI error. Delete Endpoint and rules that are created", zap.Error(err), zap.String("contIfName", contIfName)) - if containerIf != nil { - client.DeleteEndpointRules(ep) + if vlanid != 0 { + if nw.Mode == opModeTransparentVlan { + logger.Info("Transparent vlan client") + if _, ok := epInfo.Data[SnatBridgeIPKey]; ok { + nw.SnatBridgeIP = epInfo.Data[SnatBridgeIPKey].(string) } - // set deleteHostVeth to true to cleanup host veth interface if created - //nolint:errcheck // ignore error - client.DeleteEndpoints(ep) - } - }(epClient, contIfName) - - // wrapping endpoint client commands in anonymous func so that namespace can be exit and closed before the next loop - //nolint:wrapcheck // ignore wrap check - err = func() error { - if epErr := epClient.AddEndpoints(epInfo); epErr != nil { - return epErr - } - - if epInfo.NICType == cns.InfraNIC { - var epErr error - containerIf, epErr = netioCli.GetNetworkInterfaceByName(contIfName) - if epErr != nil { - return epErr - } - ep.MacAddress = containerIf.HardwareAddr - } - - // Setup rules for IP addresses on the container interface. - if epErr := epClient.AddEndpointRules(epInfo); epErr != nil { - return epErr - } - - // If a network namespace for the container interface is specified... - if epInfo.NetNsPath != "" { - // Open the network namespace. - logger.Info("Opening netns", zap.Any("NetNsPath", epInfo.NetNsPath)) - ns, epErr := nsc.OpenNamespace(epInfo.NetNsPath) - if epErr != nil { - return epErr - } - defer ns.Close() - - if epErr := epClient.MoveEndpointsToContainerNS(epInfo, ns.GetFd()); epErr != nil { - return epErr + epClient = NewTransparentVlanEndpointClient(nw, epInfo, hostIfName, contIfName, vlanid, localIP, nl, plc, nsc, iptc) + } else { + logger.Info("OVS client") + if _, ok := epInfo.Data[SnatBridgeIPKey]; ok { + nw.SnatBridgeIP = epInfo.Data[SnatBridgeIPKey].(string) } - // Enter the container network namespace. - logger.Info("Entering netns", zap.Any("NetNsPath", epInfo.NetNsPath)) - if epErr := ns.Enter(); epErr != nil { - return epErr - } - - // Return to host network namespace. - defer func() { - logger.Info("Exiting netns", zap.Any("NetNsPath", epInfo.NetNsPath)) - if epErr := ns.Exit(); epErr != nil { - logger.Error("Failed to exit netns with", zap.Error(epErr)) - } - }() + epClient = NewOVSEndpointClient( + nw, + epInfo, + hostIfName, + contIfName, + vlanid, + localIP, + nl, + ovsctl.NewOvsctl(), + plc, + iptc) } - - if epInfo.IPV6Mode != "" { - // Enable ipv6 setting in container - nuc := networkutils.NewNetworkUtils(nl, plc) - if epErr := nuc.UpdateIPV6Setting(0); epErr != nil { - return fmt.Errorf("enable ipv6 in container failed:%w", epErr) - } - } - - // If a name for the container interface is specified... - if epInfo.IfName != "" { - if epErr := epClient.SetupContainerInterfaces(epInfo); epErr != nil { - return epErr - } - } - - return epClient.ConfigureContainerInterfacesAndRoutes(epInfo) - }() - if err != nil { - return nil, err + } else if nw.Mode != opModeTransparent { + logger.Info("Bridge client") + epClient = NewLinuxBridgeEndpointClient(nw.extIf, hostIfName, contIfName, nw.Mode, nl, plc) + } else if epInfo.NICType == cns.DelegatedVMNIC { + logger.Info("Secondary client") + epClient = NewSecondaryEndpointClient(nl, netioCli, plc, nsc, ep) + } else { + logger.Info("Transparent client") + epClient = NewTransparentEndpointClient(nw.extIf, hostIfName, contIfName, nw.Mode, nl, netioCli, plc) } } + //nolint:gocritic + defer func(client EndpointClient, contIfName string) { + // Cleanup on failure. + if err != nil { + logger.Error("CNI error. Delete Endpoint and rules that are created", zap.Error(err), zap.String("contIfName", contIfName)) + if containerIf != nil { + client.DeleteEndpointRules(ep) + } + // set deleteHostVeth to true to cleanup host veth interface if created + //nolint:errcheck // ignore error + client.DeleteEndpoints(ep) + } + }(epClient, contIfName) + + // wrapping endpoint client commands in anonymous func so that namespace can be exit and closed before the next loop + //nolint:wrapcheck // ignore wrap check + err = func() error { + if epErr := epClient.AddEndpoints(epInfo); epErr != nil { + return epErr + } + + if epInfo.NICType == cns.InfraNIC { + var epErr error + containerIf, epErr = netioCli.GetNetworkInterfaceByName(contIfName) + if epErr != nil { + return epErr + } + ep.MacAddress = containerIf.HardwareAddr + } + + // Setup rules for IP addresses on the container interface. + if epErr := epClient.AddEndpointRules(epInfo); epErr != nil { + return epErr + } + + // If a network namespace for the container interface is specified... + if epInfo.NetNsPath != "" { + // Open the network namespace. + logger.Info("Opening netns", zap.Any("NetNsPath", epInfo.NetNsPath)) + ns, epErr := nsc.OpenNamespace(epInfo.NetNsPath) + if epErr != nil { + return epErr + } + defer ns.Close() + + if epErr := epClient.MoveEndpointsToContainerNS(epInfo, ns.GetFd()); epErr != nil { + return epErr + } + + // Enter the container network namespace. + logger.Info("Entering netns", zap.Any("NetNsPath", epInfo.NetNsPath)) + if epErr := ns.Enter(); epErr != nil { + return epErr + } + + // Return to host network namespace. + defer func() { + logger.Info("Exiting netns", zap.Any("NetNsPath", epInfo.NetNsPath)) + if epErr := ns.Exit(); epErr != nil { + logger.Error("Failed to exit netns with", zap.Error(epErr)) + } + }() + } + + if epInfo.IPV6Mode != "" { + // Enable ipv6 setting in container + logger.Info("Enable ipv6 setting in container.") + nuc := networkutils.NewNetworkUtils(nl, plc) + if epErr := nuc.UpdateIPV6Setting(0); epErr != nil { + return fmt.Errorf("enable ipv6 in container failed:%w", epErr) + } + } + + // If a name for the container interface is specified... + if epInfo.IfName != "" { + if epErr := epClient.SetupContainerInterfaces(epInfo); epErr != nil { + return epErr + } + } + + return epClient.ConfigureContainerInterfacesAndRoutes(epInfo) + }() + if err != nil { + return nil, err + } + return ep, nil } @@ -277,11 +285,16 @@ func (nw *network) deleteEndpointImpl(nl netlink.NetlinkInterface, plc platform. } else if nw.Mode != opModeTransparent { epClient = NewLinuxBridgeEndpointClient(nw.extIf, ep.HostIfName, "", nw.Mode, nl, plc) } else { - if len(ep.SecondaryInterfaces) > 0 { + // delete if secondary interfaces populated or endpoint of type delegated (new way) + if len(ep.SecondaryInterfaces) > 0 || ep.NICType == cns.DelegatedVMNIC { epClient = NewSecondaryEndpointClient(nl, nioc, plc, nsc, ep) epClient.DeleteEndpointRules(ep) //nolint:errcheck // ignore error epClient.DeleteEndpoints(ep) + if ep.NICType == cns.DelegatedVMNIC { + // if the ep itself is of type secondary (new way), don't use transparent client below + return nil + } } epClient = NewTransparentEndpointClient(nw.extIf, ep.HostIfName, "", nw.Mode, nl, nioc, plc) @@ -394,8 +407,8 @@ func deleteRoutes(nl netlink.NetlinkInterface, netioshim netio.NetIOInterface, i func (nm *networkManager) updateEndpointImpl(nw *network, existingEpInfo *EndpointInfo, targetEpInfo *EndpointInfo) (*endpoint, error) { var ep *endpoint - existingEpFromRepository := nw.Endpoints[existingEpInfo.Id] - logger.Info("[updateEndpointImpl] Going to retrieve endpoint with Id to update", zap.String("id", existingEpInfo.Id)) + existingEpFromRepository := nw.Endpoints[existingEpInfo.EndpointID] + logger.Info("[updateEndpointImpl] Going to retrieve endpoint with Id to update", zap.String("id", existingEpInfo.EndpointID)) if existingEpFromRepository == nil { logger.Info("[updateEndpointImpl] Endpoint cannot be updated as it does not exist") return nil, errEndpointNotFound @@ -426,7 +439,7 @@ func (nm *networkManager) updateEndpointImpl(nw *network, existingEpInfo *Endpoi } }() } else { - logger.Info("[updateEndpointImpl] Endpoint cannot be updated as the network namespace does not exist: Epid", zap.String("id", existingEpInfo.Id), + logger.Info("[updateEndpointImpl] Endpoint cannot be updated as the network namespace does not exist: Epid", zap.String("id", existingEpInfo.EndpointID), zap.String("component", "updateEndpointImpl")) return nil, errNamespaceNotFound } @@ -438,7 +451,7 @@ func (nm *networkManager) updateEndpointImpl(nw *network, existingEpInfo *Endpoi // Create the endpoint object. ep = &endpoint{ - Id: existingEpInfo.Id, + Id: existingEpInfo.EndpointID, } // Update existing endpoint state with the new routes to persist diff --git a/network/endpoint_snatroute_linux.go b/network/endpoint_snatroute_linux.go index 0f8de4e1a..c139393e9 100644 --- a/network/endpoint_snatroute_linux.go +++ b/network/endpoint_snatroute_linux.go @@ -11,11 +11,11 @@ import ( ) func GetSnatHostIfName(epInfo *EndpointInfo) string { - return fmt.Sprintf("%s%s", snatVethInterfacePrefix, epInfo.Id[:7]) + return fmt.Sprintf("%s%s", snatVethInterfacePrefix, epInfo.EndpointID[:7]) } func GetSnatContIfName(epInfo *EndpointInfo) string { - return fmt.Sprintf("%s%s-2", snatVethInterfacePrefix, epInfo.Id[:7]) + return fmt.Sprintf("%s%s-2", snatVethInterfacePrefix, epInfo.EndpointID[:7]) } func AddSnatEndpoint(snatClient *snat.Client) error { diff --git a/network/endpoint_test.go b/network/endpoint_test.go index 0fc0cf829..0da1a8cb2 100644 --- a/network/endpoint_test.go +++ b/network/endpoint_test.go @@ -175,19 +175,20 @@ var _ = Describe("Test Endpoint", func() { Endpoints: map[string]*endpoint{}, } epInfo := &EndpointInfo{ - Id: "768e8deb-eth1", - Data: make(map[string]interface{}), - IfName: eth0IfName, + EndpointID: "768e8deb-eth1", + Data: make(map[string]interface{}), + IfName: eth0IfName, + NICType: cns.InfraNIC, } epInfo.Data[VlanIDKey] = 100 It("Should be added", func() { // Add endpoint with valid id ep, err := nw.newEndpointImpl(nil, netlink.NewMockNetlink(false, ""), platform.NewMockExecClient(false), - netio.NewMockNetIO(false, 0), NewMockEndpointClient(nil), NewMockNamespaceClient(), iptables.NewClient(), []*EndpointInfo{epInfo}) + netio.NewMockNetIO(false, 0), NewMockEndpointClient(nil), NewMockNamespaceClient(), iptables.NewClient(), epInfo) Expect(err).NotTo(HaveOccurred()) Expect(ep).NotTo(BeNil()) - Expect(ep.Id).To(Equal(epInfo.Id)) + Expect(ep.Id).To(Equal(epInfo.EndpointID)) Expect(ep.Gateways).To(BeEmpty()) }) It("should have fields set", func() { @@ -196,14 +197,15 @@ var _ = Describe("Test Endpoint", func() { extIf: &externalInterface{IPv4Gateway: net.ParseIP("192.168.0.1")}, } ep, err := nw2.newEndpointImpl(nil, netlink.NewMockNetlink(false, ""), platform.NewMockExecClient(false), - netio.NewMockNetIO(false, 0), NewMockEndpointClient(nil), NewMockNamespaceClient(), iptables.NewClient(), []*EndpointInfo{epInfo}) + netio.NewMockNetIO(false, 0), NewMockEndpointClient(nil), NewMockNamespaceClient(), iptables.NewClient(), epInfo) Expect(err).NotTo(HaveOccurred()) Expect(ep).NotTo(BeNil()) - Expect(ep.Id).To(Equal(epInfo.Id)) + Expect(ep.Id).To(Equal(epInfo.EndpointID)) Expect(ep.Gateways).NotTo(BeNil()) Expect(len(ep.Gateways)).To(Equal(1)) Expect(ep.Gateways[0].String()).To(Equal("192.168.0.1")) Expect(ep.VlanID).To(Equal(epInfo.Data[VlanIDKey].(int))) + Expect(ep.IfName).To(Equal(epInfo.IfName)) }) It("Should be not added", func() { // Adding an endpoint with an id. @@ -212,7 +214,7 @@ var _ = Describe("Test Endpoint", func() { Expect(err).ToNot(HaveOccurred()) // Adding endpoint with same id should fail and delete should cleanup the state ep2, err := nw.newEndpointImpl(nil, netlink.NewMockNetlink(false, ""), platform.NewMockExecClient(false), - netio.NewMockNetIO(false, 0), mockCli, NewMockNamespaceClient(), iptables.NewClient(), []*EndpointInfo{epInfo}) + netio.NewMockNetIO(false, 0), mockCli, NewMockNamespaceClient(), iptables.NewClient(), epInfo) Expect(err).To(HaveOccurred()) Expect(ep2).To(BeNil()) assert.Contains(GinkgoT(), err.Error(), "Endpoint already exists") @@ -222,7 +224,7 @@ var _ = Describe("Test Endpoint", func() { // Adding an endpoint with an id. mockCli := NewMockEndpointClient(nil) ep2, err := nw.newEndpointImpl(nil, netlink.NewMockNetlink(false, ""), platform.NewMockExecClient(false), - netio.NewMockNetIO(false, 0), mockCli, NewMockNamespaceClient(), iptables.NewClient(), []*EndpointInfo{epInfo}) + netio.NewMockNetIO(false, 0), mockCli, NewMockNamespaceClient(), iptables.NewClient(), epInfo) Expect(err).ToNot(HaveOccurred()) Expect(ep2).ToNot(BeNil()) Expect(len(mockCli.endpoints)).To(Equal(1)) @@ -236,15 +238,42 @@ var _ = Describe("Test Endpoint", func() { Expect(len(mockCli.endpoints)).To(Equal(0)) }) }) + Context("When endpoint added delegated", func() { + epInfo := &EndpointInfo{ + EndpointID: "768e8deb-eth1", + Data: make(map[string]interface{}), + IfName: eth0IfName, + MasterIfName: "masterIfName", + NICType: cns.DelegatedVMNIC, + } + epInfo.Data[VlanIDKey] = 100 + + It("should have fields set", func() { + nw2 := &network{ + Endpoints: map[string]*endpoint{}, + extIf: &externalInterface{IPv4Gateway: net.ParseIP("192.168.0.1")}, + } + ep, err := nw2.newEndpointImpl(nil, netlink.NewMockNetlink(false, ""), platform.NewMockExecClient(false), + netio.NewMockNetIO(false, 0), NewMockEndpointClient(nil), NewMockNamespaceClient(), iptables.NewClient(), epInfo) + Expect(err).NotTo(HaveOccurred()) + Expect(ep).NotTo(BeNil()) + Expect(ep.Id).To(Equal(epInfo.EndpointID)) + Expect(ep.Gateways).NotTo(BeNil()) + Expect(len(ep.Gateways)).To(Equal(1)) + Expect(ep.Gateways[0].String()).To(Equal("192.168.0.1")) + Expect(ep.VlanID).To(Equal(epInfo.Data[VlanIDKey].(int))) + Expect(ep.IfName).To(Equal("masterIfName")) + }) + }) Context("When endpoint add failed", func() { It("Should not be added to the network", func() { nw := &network{ Endpoints: map[string]*endpoint{}, } epInfo := &EndpointInfo{ - Id: "768e8deb-eth1", - IfName: eth0IfName, - NICType: cns.InfraNIC, + EndpointID: "768e8deb-eth1", + IfName: eth0IfName, + NICType: cns.InfraNIC, } ep, err := nw.newEndpointImpl(nil, netlink.NewMockNetlink(false, ""), platform.NewMockExecClient(false), netio.NewMockNetIO(false, 0), NewMockEndpointClient(func(ep *EndpointInfo) error { @@ -253,16 +282,17 @@ var _ = Describe("Test Endpoint", func() { } return nil - }), NewMockNamespaceClient(), iptables.NewClient(), []*EndpointInfo{epInfo}) + }), NewMockNamespaceClient(), iptables.NewClient(), epInfo) Expect(err).To(HaveOccurred()) Expect(ep).To(BeNil()) ep, err = nw.newEndpointImpl(nil, netlink.NewMockNetlink(false, ""), platform.NewMockExecClient(false), - netio.NewMockNetIO(false, 0), NewMockEndpointClient(nil), NewMockNamespaceClient(), iptables.NewClient(), []*EndpointInfo{epInfo}) + netio.NewMockNetIO(false, 0), NewMockEndpointClient(nil), NewMockNamespaceClient(), iptables.NewClient(), epInfo) Expect(err).NotTo(HaveOccurred()) Expect(ep).NotTo(BeNil()) - Expect(ep.Id).To(Equal(epInfo.Id)) + Expect(ep.Id).To(Equal(epInfo.EndpointID)) }) }) + // TODO: Add secondary endpoint client test coverage in a different way Context("When secondary endpoint client is used", func() { _, ipnet, _ := net.ParseCIDR("0.0.0.0/0") nw := &network{ @@ -271,36 +301,43 @@ var _ = Describe("Test Endpoint", func() { extIf: &externalInterface{BridgeName: "testbridge"}, } epInfo := &EndpointInfo{ - Id: "768e8deb-eth1", - IfName: eth0IfName, - NICType: cns.InfraNIC, + EndpointID: "768e8deb-eth1", + IfName: eth0IfName, + NICType: cns.InfraNIC, } secondaryEpInfo := &EndpointInfo{ - NICType: cns.DelegatedVMNIC, - Routes: []RouteInfo{{Dst: *ipnet}}, + // When we create the secondary endpoint infos while looping over the interface infos, we pass in the same endpoint id + EndpointID: "768e8deb-eth1", + NICType: cns.DelegatedVMNIC, + Routes: []RouteInfo{{Dst: *ipnet}}, } - It("Should not endpoint to the network when there is an error", func() { + It("Should not add endpoint to the network when there is an error", func() { secondaryEpInfo.MacAddress = netio.BadHwAddr // mock netlink will fail to set link state on bad eth ep, err := nw.newEndpointImpl(nil, netlink.NewMockNetlink(false, ""), platform.NewMockExecClient(false), - netio.NewMockNetIO(false, 0), nil, NewMockNamespaceClient(), iptables.NewClient(), []*EndpointInfo{epInfo, secondaryEpInfo}) + netio.NewMockNetIO(false, 0), nil, NewMockNamespaceClient(), iptables.NewClient(), secondaryEpInfo) Expect(err).To(HaveOccurred()) Expect(err.Error()).To(Equal("SecondaryEndpointClient Error: " + netlink.ErrorMockNetlink.Error())) Expect(ep).To(BeNil()) - + // should not panic or error when going through the unified endpoint impl flow with only the delegated nic type fields secondaryEpInfo.MacAddress = netio.HwAddr ep, err = nw.newEndpointImpl(nil, netlink.NewMockNetlink(false, ""), platform.NewMockExecClient(false), - netio.NewMockNetIO(false, 0), nil, NewMockNamespaceClient(), iptables.NewClient(), []*EndpointInfo{epInfo, secondaryEpInfo}) + netio.NewMockNetIO(false, 0), nil, NewMockNamespaceClient(), iptables.NewClient(), secondaryEpInfo) Expect(err).ToNot(HaveOccurred()) - Expect(ep.Id).To(Equal(epInfo.Id)) + Expect(ep.Id).To(Equal(epInfo.EndpointID)) }) It("Should add endpoint when there are no errors", func() { secondaryEpInfo.MacAddress = netio.HwAddr ep, err := nw.newEndpointImpl(nil, netlink.NewMockNetlink(false, ""), platform.NewMockExecClient(false), - netio.NewMockNetIO(false, 0), nil, NewMockNamespaceClient(), iptables.NewClient(), []*EndpointInfo{epInfo, secondaryEpInfo}) + netio.NewMockNetIO(false, 0), nil, NewMockNamespaceClient(), iptables.NewClient(), secondaryEpInfo) Expect(err).ToNot(HaveOccurred()) - Expect(ep.Id).To(Equal(epInfo.Id)) + Expect(ep.Id).To(Equal(epInfo.EndpointID)) + + ep, err = nw.newEndpointImpl(nil, netlink.NewMockNetlink(false, ""), platform.NewMockExecClient(false), + netio.NewMockNetIO(false, 0), nil, NewMockNamespaceClient(), iptables.NewClient(), epInfo) + Expect(err).ToNot(HaveOccurred()) + Expect(ep.Id).To(Equal(epInfo.EndpointID)) }) }) }) @@ -312,8 +349,8 @@ var _ = Describe("Test Endpoint", func() { nw := &network{} existingEpInfo := &EndpointInfo{ - Id: "768e8deb-eth1", - IfName: eth0IfName, + EndpointID: "768e8deb-eth1", + IfName: eth0IfName, } targetEpInfo := &EndpointInfo{} err := nm.updateEndpoint(nw, existingEpInfo, targetEpInfo) diff --git a/network/endpoint_windows.go b/network/endpoint_windows.go index b1f4e9ab9..baf3d7267 100644 --- a/network/endpoint_windows.go +++ b/network/endpoint_windows.go @@ -9,6 +9,7 @@ import ( "net" "strings" + "github.com/Azure/azure-container-networking/cns" "github.com/Azure/azure-container-networking/netio" "github.com/Azure/azure-container-networking/netlink" "github.com/Azure/azure-container-networking/network/policy" @@ -73,18 +74,18 @@ func (nw *network) newEndpointImpl( _ EndpointClient, _ NamespaceClientInterface, _ ipTablesClient, - epInfo []*EndpointInfo, + epInfo *EndpointInfo, ) (*endpoint, error) { - // there is only 1 epInfo for windows, multiple interfaces will be added in the future - if useHnsV2, err := UseHnsV2(epInfo[0].NetNsPath); useHnsV2 { + if useHnsV2, err := UseHnsV2(epInfo.NetNsPath); useHnsV2 { if err != nil { return nil, err } - return nw.newEndpointImplHnsV2(cli, epInfo[0]) + return nw.newEndpointImplHnsV2(cli, epInfo) } - return nw.newEndpointImplHnsV1(epInfo[0], plc) + return nw.newEndpointImplHnsV1(epInfo, plc) + // TODO: add switch statement for NIC type for IB and Accelnet NIC support to create endpoint here in the future } // newEndpointImplHnsV1 creates a new endpoint in the network using HnsV1 @@ -103,9 +104,9 @@ func (nw *network) newEndpointImplHnsV1(epInfo *EndpointInfo, plc platform.ExecC hnsEndpoint := &hcsshim.HNSEndpoint{ Name: infraEpName, VirtualNetwork: nw.HnsId, - DNSSuffix: epInfo.DNS.Suffix, - DNSServerList: strings.Join(epInfo.DNS.Servers, ","), - Policies: policy.SerializePolicies(policy.EndpointPolicy, epInfo.Policies, epInfo.Data, epInfo.EnableSnatForDns, epInfo.EnableMultiTenancy), + DNSSuffix: epInfo.EndpointDNS.Suffix, + DNSServerList: strings.Join(epInfo.EndpointDNS.Servers, ","), + Policies: policy.SerializePolicies(policy.EndpointPolicy, epInfo.EndpointPolicies, epInfo.Data, epInfo.EnableSnatForDns, epInfo.EnableMultiTenancy), } // HNS currently supports one IP address and one IPv6 address per endpoint. @@ -164,11 +165,12 @@ func (nw *network) newEndpointImplHnsV1(epInfo *EndpointInfo, plc platform.ExecC IfName: epInfo.IfName, IPAddresses: epInfo.IPAddresses, Gateways: []net.IP{net.ParseIP(hnsResponse.GatewayAddress)}, - DNS: epInfo.DNS, + DNS: epInfo.EndpointDNS, VlanID: vlanid, EnableSnatOnHost: epInfo.EnableSnatOnHost, NetNs: epInfo.NetNsPath, ContainerID: epInfo.ContainerID, + NICType: epInfo.NICType, } for _, route := range epInfo.Routes { @@ -177,6 +179,8 @@ func (nw *network) newEndpointImplHnsV1(epInfo *EndpointInfo, plc platform.ExecC ep.MacAddress, _ = net.ParseMAC(hnsResponse.MacAddress) + epInfo.HNSEndpointID = hnsResponse.Id // we use the ep info hns id later in stateless to clean up in ADD if there is an error + return ep, nil } @@ -193,7 +197,7 @@ func (nw *network) addIPv6NeighborEntryForGateway(epInfo *EndpointInfo, plc plat // run powershell cmd to set neighbor entry for gw ip to 12-34-56-78-9a-bc cmd := fmt.Sprintf("New-NetNeighbor -IPAddress %s -InterfaceAlias \"%s (%s)\" -LinkLayerAddress \"%s\"", - nw.Subnets[1].Gateway.String(), containerIfNamePrefix, epInfo.Id, defaultGwMac) + nw.Subnets[1].Gateway.String(), containerIfNamePrefix, epInfo.EndpointID, defaultGwMac) if out, err = plc.ExecutePowershellCommand(cmd); err != nil { logger.Error("Adding ipv6 gw neigh entry failed", zap.Any("out", out), zap.Error(err)) @@ -212,21 +216,27 @@ func (nw *network) configureHcnEndpoint(epInfo *EndpointInfo) (*hcn.HostComputeE Name: infraEpName, HostComputeNetwork: nw.HnsId, Dns: hcn.Dns{ - Search: strings.Split(epInfo.DNS.Suffix, ","), - ServerList: epInfo.DNS.Servers, - Options: epInfo.DNS.Options, + Search: strings.Split(epInfo.EndpointDNS.Suffix, ","), + ServerList: epInfo.EndpointDNS.Servers, + Options: epInfo.EndpointDNS.Options, }, SchemaVersion: hcn.SchemaVersion{ Major: hcnSchemaVersionMajor, Minor: hcnSchemaVersionMinor, }, - MacAddress: epInfo.MacAddress.String(), } - if endpointPolicies, err := policy.GetHcnEndpointPolicies(policy.EndpointPolicy, epInfo.Policies, epInfo.Data, epInfo.EnableSnatForDns, epInfo.EnableMultiTenancy, epInfo.NATInfo); err == nil { - for _, epPolicy := range endpointPolicies { - hcnEndpoint.Policies = append(hcnEndpoint.Policies, epPolicy) - } + // macAddress type for InfraNIC is like "60:45:bd:12:45:65" + // if NICType is delegatedVMNIC, convert the macaddress format + macAddress := epInfo.MacAddress.String() + if epInfo.NICType == cns.DelegatedVMNIC { + // convert the format of macAddress that HNS can accept, i.e, "60-45-bd-12-45-65" if NIC type is delegated NIC + macAddress = strings.Join(strings.Split(macAddress, ":"), "-") + } + hcnEndpoint.MacAddress = macAddress + + if epPolicies, err := policy.GetHcnEndpointPolicies(policy.EndpointPolicy, epInfo.EndpointPolicies, epInfo.Data, epInfo.EnableSnatForDns, epInfo.EnableMultiTenancy, epInfo.NATInfo); err == nil { + hcnEndpoint.Policies = append(hcnEndpoint.Policies, epPolicies...) } else { logger.Error("Failed to get endpoint policies due to", zap.Error(err)) return nil, err @@ -328,7 +338,7 @@ func (nw *network) newEndpointImplHnsV2(cli apipaClient, epInfo *EndpointInfo) ( } // Create the HCN endpoint. - logger.Info("Creating hcn endpoint", zap.String("name", hcnEndpoint.Name), zap.String("computenetwork", hcnEndpoint.HostComputeNetwork)) + logger.Info("Creating hcn endpoint", zap.Any("hcnEndpoint", hcnEndpoint), zap.String("computenetwork", hcnEndpoint.HostComputeNetwork)) hnsResponse, err := Hnsv2.CreateEndpoint(hcnEndpoint) if err != nil { return nil, fmt.Errorf("Failed to create endpoint: %s due to error: %v", hcnEndpoint.Name, err) @@ -381,15 +391,21 @@ func (nw *network) newEndpointImplHnsV2(cli apipaClient, epInfo *EndpointInfo) ( gateway = net.ParseIP(hnsResponse.Routes[0].NextHop) } + nicName := epInfo.IfName + // infra nic nicname will look like eth0, but delegated/secondary nics will look like "vEthernet x" where x is 1-7 + if epInfo.NICType != cns.InfraNIC { + nicName = epInfo.MasterIfName + } + // Create the endpoint object. ep := &endpoint{ Id: hcnEndpoint.Name, HnsId: hnsResponse.Id, SandboxKey: epInfo.ContainerID, - IfName: epInfo.IfName, + IfName: nicName, IPAddresses: epInfo.IPAddresses, Gateways: []net.IP{gateway}, - DNS: epInfo.DNS, + DNS: epInfo.EndpointDNS, VlanID: vlanid, EnableSnatOnHost: epInfo.EnableSnatOnHost, NetNs: epInfo.NetNsPath, @@ -399,6 +415,7 @@ func (nw *network) newEndpointImplHnsV2(cli apipaClient, epInfo *EndpointInfo) ( ContainerID: epInfo.ContainerID, PODName: epInfo.PODName, PODNameSpace: epInfo.PODNameSpace, + NICType: epInfo.NICType, } for _, route := range epInfo.Routes { @@ -407,6 +424,11 @@ func (nw *network) newEndpointImplHnsV2(cli apipaClient, epInfo *EndpointInfo) ( ep.MacAddress, _ = net.ParseMAC(hnsResponse.MacAddress) + epInfo.HNSEndpointID = hnsResponse.Id // we use the ep info hns id later in stateless to clean up in ADD if there is an error + + // TODO: Confirm with TM: when we delete an endpoint, this code is to find ifName from endpoint and then we can delete this endpoint + // TODO: deal with ep.SecondaryInterfaces here at all anymore? + return ep, nil } diff --git a/network/endpoint_windows_test.go b/network/endpoint_windows_test.go index 4e65c6a2b..aa59a62c3 100644 --- a/network/endpoint_windows_test.go +++ b/network/endpoint_windows_test.go @@ -27,12 +27,12 @@ func TestNewAndDeleteEndpointImplHnsV2(t *testing.T) { } epInfo := &EndpointInfo{ - Id: "753d3fb6-e9b3-49e2-a109-2acc5dda61f1", + EndpointID: "753d3fb6-e9b3-49e2-a109-2acc5dda61f1", ContainerID: "545055c2-1462-42c8-b222-e75d0b291632", NetNsPath: "fakeNameSpace", IfName: "eth0", Data: make(map[string]interface{}), - DNS: DNSInfo{ + EndpointDNS: DNSInfo{ Suffix: "10.0.0.0", Servers: []string{"10.0.0.1, 10.0.0.2"}, Options: nil, @@ -71,12 +71,12 @@ func TestNewEndpointImplHnsv2Timesout(t *testing.T) { } epInfo := &EndpointInfo{ - Id: "753d3fb6-e9b3-49e2-a109-2acc5dda61f1", + EndpointID: "753d3fb6-e9b3-49e2-a109-2acc5dda61f1", ContainerID: "545055c2-1462-42c8-b222-e75d0b291632", NetNsPath: "fakeNameSpace", IfName: "eth0", Data: make(map[string]interface{}), - DNS: DNSInfo{ + EndpointDNS: DNSInfo{ Suffix: "10.0.0.0", Servers: []string{"10.0.0.1, 10.0.0.2"}, Options: nil, @@ -98,12 +98,12 @@ func TestDeleteEndpointImplHnsv2Timeout(t *testing.T) { Hnsv2 = hnswrapper.NewHnsv2wrapperFake() epInfo := &EndpointInfo{ - Id: "753d3fb6-e9b3-49e2-a109-2acc5dda61f1", + EndpointID: "753d3fb6-e9b3-49e2-a109-2acc5dda61f1", ContainerID: "545055c2-1462-42c8-b222-e75d0b291632", NetNsPath: "fakeNameSpace", IfName: "eth0", Data: make(map[string]interface{}), - DNS: DNSInfo{ + EndpointDNS: DNSInfo{ Suffix: "10.0.0.0", Servers: []string{"10.0.0.1, 10.0.0.2"}, Options: nil, @@ -147,12 +147,12 @@ func TestCreateEndpointImplHnsv1Timeout(t *testing.T) { } epInfo := &EndpointInfo{ - Id: "753d3fb6-e9b3-49e2-a109-2acc5dda61f1", + EndpointID: "753d3fb6-e9b3-49e2-a109-2acc5dda61f1", ContainerID: "545055c2-1462-42c8-b222-e75d0b291632", NetNsPath: "fakeNameSpace", IfName: "eth0", Data: make(map[string]interface{}), - DNS: DNSInfo{ + EndpointDNS: DNSInfo{ Suffix: "10.0.0.0", Servers: []string{"10.0.0.1, 10.0.0.2"}, Options: nil, @@ -174,12 +174,12 @@ func TestDeleteEndpointImplHnsv1Timeout(t *testing.T) { Hnsv1 = hnswrapper.NewHnsv1wrapperFake() epInfo := &EndpointInfo{ - Id: "753d3fb6-e9b3-49e2-a109-2acc5dda61f1", + EndpointID: "753d3fb6-e9b3-49e2-a109-2acc5dda61f1", ContainerID: "545055c2-1462-42c8-b222-e75d0b291632", NetNsPath: "fakeNameSpace", IfName: "eth0", Data: make(map[string]interface{}), - DNS: DNSInfo{ + EndpointDNS: DNSInfo{ Suffix: "10.0.0.0", Servers: []string{"10.0.0.1, 10.0.0.2"}, Options: nil, diff --git a/network/manager.go b/network/manager.go index 9775c84ff..6f855cdc9 100644 --- a/network/manager.go +++ b/network/manager.go @@ -39,6 +39,8 @@ const ( ContainerIDLength = 8 EndpointIfIndex = 0 // Azure CNI supports only one interface DefaultNetworkID = "azure" + // TODO: Remove dummy GUID and come up with more permanent solution + dummyGUID = "12345678-1234-1234-1234-123456789012" // guid to trigger hnsv2 in windows ) var Ipv4DefaultRouteDstPrefix = net.IPNet{ @@ -94,14 +96,15 @@ type NetworkManager interface { AddExternalInterface(ifName string, subnet string) error - CreateNetwork(nwInfo *NetworkInfo) error + CreateNetwork(nwInfo *EndpointInfo) error DeleteNetwork(networkID string) error - GetNetworkInfo(networkID string) (NetworkInfo, error) + GetNetworkInfo(networkID string) (EndpointInfo, error) // FindNetworkIDFromNetNs returns the network name that contains an endpoint created for this netNS, errNetworkNotFound if no network is found FindNetworkIDFromNetNs(netNs string) (string, error) GetNumEndpointsByContainerID(containerID string) int - CreateEndpoint(client apipaClient, networkID string, epInfo []*EndpointInfo) error + CreateEndpoint(client apipaClient, networkID string, epInfo *EndpointInfo) error + EndpointCreate(client apipaClient, epInfos []*EndpointInfo) error // TODO: change name DeleteEndpoint(networkID string, endpointID string, epInfo *EndpointInfo) error GetEndpointInfo(networkID string, endpointID string) (*EndpointInfo, error) GetAllEndpoints(networkID string) (map[string]*EndpointInfo, error) @@ -112,6 +115,10 @@ type NetworkManager interface { GetNumberOfEndpoints(ifName string, networkID string) int GetEndpointID(containerID, ifName string) string IsStatelessCNIMode() bool + SaveState(eps []*endpoint) error + DeleteState(epInfos []*EndpointInfo) error + GetEndpointInfosFromContainerID(containerID string) []*EndpointInfo + GetEndpointState(networkID, containerID string) ([]*EndpointInfo, error) } // Creates a new network manager. @@ -302,25 +309,15 @@ func (nm *networkManager) AddExternalInterface(ifName string, subnet string) err return err } - err = nm.save() - if err != nil { - return err - } - return nil } // CreateNetwork creates a new container network. -func (nm *networkManager) CreateNetwork(nwInfo *NetworkInfo) error { +func (nm *networkManager) CreateNetwork(epInfo *EndpointInfo) error { nm.Lock() defer nm.Unlock() - _, err := nm.newNetwork(nwInfo) - if err != nil { - return err - } - - err = nm.save() + _, err := nm.newNetwork(epInfo) if err != nil { return err } @@ -347,21 +344,20 @@ func (nm *networkManager) DeleteNetwork(networkID string) error { } // GetNetworkInfo returns information about the given network. -func (nm *networkManager) GetNetworkInfo(networkId string) (NetworkInfo, error) { +func (nm *networkManager) GetNetworkInfo(networkID string) (EndpointInfo, error) { nm.Lock() defer nm.Unlock() - nw, err := nm.getNetwork(networkId) + nw, err := nm.getNetwork(networkID) if err != nil { - return NetworkInfo{}, err + return EndpointInfo{}, err } - nwInfo := NetworkInfo{ - Id: networkId, + nwInfo := EndpointInfo{ + NetworkID: networkID, Subnets: nw.Subnets, Mode: nw.Mode, EnableSnatOnHost: nw.EnableSnatOnHost, - DNS: nw.DNS, Options: make(map[string]interface{}), } @@ -374,27 +370,25 @@ func (nm *networkManager) GetNetworkInfo(networkId string) (NetworkInfo, error) return nwInfo, nil } -// CreateEndpoint creates a new container endpoint. -func (nm *networkManager) CreateEndpoint(cli apipaClient, networkID string, epInfo []*EndpointInfo) error { +func (nm *networkManager) createEndpoint(cli apipaClient, networkID string, epInfo *EndpointInfo) (*endpoint, error) { nm.Lock() defer nm.Unlock() nw, err := nm.getNetwork(networkID) if err != nil { - return err + return nil, err } if nw.VlanId != 0 { - // the first entry in epInfo is InfraNIC type - if epInfo[0].Data[VlanIDKey] == nil { + if epInfo.Data[VlanIDKey] == nil { logger.Info("overriding endpoint vlanid with network vlanid") - epInfo[0].Data[VlanIDKey] = nw.VlanId + epInfo.Data[VlanIDKey] = nw.VlanId } } ep, err := nw.newEndpoint(cli, nm.netlink, nm.plClient, nm.netio, nm.nsClient, nm.iptablesClient, epInfo) if err != nil { - return err + return nil, err } // any error after this point should also clean up the endpoint we created above defer func() { @@ -408,25 +402,26 @@ func (nm *networkManager) CreateEndpoint(cli apipaClient, networkID string, epIn } }() - if nm.IsStatelessCNIMode() { - err = nm.UpdateEndpointState(ep) - return err - } + return ep, nil +} - err = nm.save() - if err != nil { - return err - } - - return nil +// CreateEndpoint creates a new container endpoint (this is for compatibility-- add flow should no longer use this). +func (nm *networkManager) CreateEndpoint(cli apipaClient, networkID string, epInfo *EndpointInfo) error { + _, err := nm.createEndpoint(cli, networkID, epInfo) + return err } // UpdateEndpointState will make a call to CNS updatEndpointState API in the stateless CNI mode // It will add HNSEndpointID or HostVeth name to the endpoint state -func (nm *networkManager) UpdateEndpointState(ep *endpoint) error { - ifnameToIPInfoMap := generateCNSIPInfoMap(ep) // key : interface name, value : IPInfo - logger.Info("Calling cns updateEndpoint API with ", zap.String("containerID: ", ep.ContainerID), zap.String("HnsId: ", ep.HnsId), zap.String("HostIfName: ", ep.HostIfName)) - response, err := nm.CnsClient.UpdateEndpoint(context.TODO(), ep.ContainerID, ifnameToIPInfoMap) +func (nm *networkManager) UpdateEndpointState(eps []*endpoint) error { + if len(eps) == 0 { + return nil + } + + ifnameToIPInfoMap := generateCNSIPInfoMap(eps) // key : interface name, value : IPInfo + // logger.Info("Calling cns updateEndpoint API with ", zap.String("containerID: ", ep.ContainerID), zap.String("HnsId: ", ep.HnsId), zap.String("HostIfName: ", ep.HostIfName)) + // we assume all endpoints have the same container id + response, err := nm.CnsClient.UpdateEndpoint(context.TODO(), eps[0].ContainerID, ifnameToIPInfoMap) if err != nil { return errors.Wrapf(err, "Update endpoint API returend with error") } @@ -436,24 +431,28 @@ func (nm *networkManager) UpdateEndpointState(ep *endpoint) error { // GetEndpointState will make a call to CNS GetEndpointState API in the stateless CNI mode to fetch the endpointInfo // TODO unit tests need to be added, WorkItem: 26606939 -func (nm *networkManager) GetEndpointState(networkID, endpointID string) (*EndpointInfo, error) { - endpointResponse, err := nm.CnsClient.GetEndpoint(context.TODO(), endpointID) +// In stateless cni, container id is the endpoint id, so you can pass in either +func (nm *networkManager) GetEndpointState(networkID, containerID string) ([]*EndpointInfo, error) { + endpointResponse, err := nm.CnsClient.GetEndpoint(context.TODO(), containerID) if err != nil { - return nil, errors.Wrapf(err, "Get endpoint API returend with error") - } - epInfo := cnsEndpointInfotoCNIEpInfo(endpointResponse.EndpointInfo, endpointID) - if epInfo.IsEndpointStateIncomplete() { - if networkID == "" { - networkID = DefaultNetworkID - } - epInfo, err = epInfo.GetEndpointInfoByIPImpl(epInfo.IPAddresses, networkID) - if err != nil { - return nil, errors.Wrapf(err, "Get endpoint API returend with error") - } + return nil, errors.Wrapf(err, "Get endpoint API returned with error") } + epInfos := cnsEndpointInfotoCNIEpInfos(endpointResponse.EndpointInfo, containerID) - logger.Info("returning getEndpoint API with", zap.String("Endpoint Info: ", epInfo.PrettyString()), zap.String("HNISID : ", epInfo.HNSEndpointID)) - return epInfo, nil + for i := 0; i < len(epInfos); i++ { + if epInfos[i].NICType == cns.InfraNIC { + if epInfos[i].IsEndpointStateIncomplete() { // assume false for swift v2 for now + if networkID == "" { + networkID = DefaultNetworkID + } + epInfos[i], err = epInfos[i].GetEndpointInfoByIPImpl(epInfos[i].IPAddresses, networkID) + if err != nil { + logger.Info("Endpoint State is incomlete for endpoint: ", zap.Error(err), zap.String("endpointID", epInfos[i].EndpointID)) + } + } + } + } + return epInfos, nil } // DeleteEndpoint deletes an existing container endpoint. @@ -462,6 +461,7 @@ func (nm *networkManager) DeleteEndpoint(networkID, endpointID string, epInfo *E defer nm.Unlock() if nm.IsStatelessCNIMode() { + // Calls deleteEndpointImpl directly, skipping the get network check; does not call cns return nm.DeleteEndpointState(networkID, epInfo) } @@ -475,19 +475,19 @@ func (nm *networkManager) DeleteEndpoint(networkID, endpointID string, epInfo *E return err } - err = nm.save() - if err != nil { - return err - } - return nil } func (nm *networkManager) DeleteEndpointState(networkID string, epInfo *EndpointInfo) error { + // we want to always use hnsv2 in stateless + // hnsv2 is only enabled if NetNs has a valid guid and the hnsv2 api is supported + // by passing in a dummy guid, we satisfy the first condition nw := &network{ - Id: networkID, + Id: networkID, // currently unused in stateless cni + HnsId: epInfo.HNSNetworkID, Mode: opModeTransparentVlan, SnatBridgeIP: "", + NetNs: dummyGUID, // to trigger hns v2, windows extIf: &externalInterface{ Name: InfraInterfaceName, MacAddress: nil, @@ -495,19 +495,39 @@ func (nm *networkManager) DeleteEndpointState(networkID string, epInfo *Endpoint } ep := &endpoint{ - Id: epInfo.Id, + Id: epInfo.EndpointID, HnsId: epInfo.HNSEndpointID, - HostIfName: epInfo.IfName, + HNSNetworkID: epInfo.HNSNetworkID, // unused (we use nw.HnsId for deleting the network) + HostIfName: epInfo.HostIfName, LocalIP: "", VlanID: 0, - AllowInboundFromHostToNC: false, + AllowInboundFromHostToNC: false, // stateless currently does not support apipa AllowInboundFromNCToHost: false, EnableSnatOnHost: false, EnableMultitenancy: false, - NetworkContainerID: epInfo.Id, + NetworkContainerID: epInfo.NetworkContainerID, // we don't use this as long as AllowInboundFromHostToNC and AllowInboundFromNCToHost are false + NetNs: dummyGUID, // to trigger hnsv2, windows + NICType: epInfo.NICType, + IfName: epInfo.IfName, // TODO: For stateless cni linux populate IfName here to use in deletion in secondary endpoint client } logger.Info("Deleting endpoint with", zap.String("Endpoint Info: ", epInfo.PrettyString()), zap.String("HNISID : ", ep.HnsId)) - return nw.deleteEndpointImpl(netlink.NewNetlink(), platform.NewExecClient(logger), nil, nil, nil, nil, ep) + // do not need to Delete HNS endpoint if the there is no HNS in state + if ep.HnsId != "" { + err := nw.deleteEndpointImpl(netlink.NewNetlink(), platform.NewExecClient(logger), nil, nil, nil, nil, ep) + if err != nil { + return err + } + } + if epInfo.NICType == cns.DelegatedVMNIC { + // we are currently assuming stateless is not running in linux + // CHECK: could this affect linux? (if it does, it could disconnect external interface, is that okay?) + // bad only when 1) stateless and 2) linux and 3) delegated vmnics exist + logger.Info("Deleting endpoint because delegated vmnic detected", zap.String("HNSNetworkID", nw.HnsId)) + err := nm.deleteNetworkImpl(nw) + // no need to clean up state in stateless + return err + } + return nil } // GetEndpointInfo returns information about the given endpoint. @@ -517,9 +537,17 @@ func (nm *networkManager) GetEndpointInfo(networkID, endpointID string) (*Endpoi if nm.IsStatelessCNIMode() { logger.Info("calling cns getEndpoint API") - epInfo, err := nm.GetEndpointState(networkID, endpointID) + epInfos, err := nm.GetEndpointState(networkID, endpointID) + if err != nil { + return nil, err + } + for _, epInfo := range epInfos { + if epInfo.NICType == cns.InfraNIC { + return epInfo, nil + } + } - return epInfo, err + return nil, err } nw, err := nm.getNetwork(networkID) @@ -692,46 +720,105 @@ func (nm *networkManager) GetEndpointID(containerID, ifName string) string { return containerID + "-" + ifName } -// cnsEndpointInfotoCNIEpInfo convert a CNS endpoint state to CNI EndpointInfo -func cnsEndpointInfotoCNIEpInfo(endpointInfo restserver.EndpointInfo, endpointID string) *EndpointInfo { - epInfo := &EndpointInfo{ - Id: endpointID, - IfIndex: EndpointIfIndex, // Azure CNI supports only one interface - ContainerID: endpointID, - PODName: endpointInfo.PodName, - PODNameSpace: endpointInfo.PodNamespace, - NetworkContainerID: endpointID, +// saves the map of network ids to endpoints to the state file +func (nm *networkManager) SaveState(eps []*endpoint) error { + nm.Lock() + defer nm.Unlock() + + logger.Info("Saving state") + // If we fail half way, we'll propagate an error up which should clean everything up + if nm.IsStatelessCNIMode() { + err := nm.UpdateEndpointState(eps) + return err } + // once endpoints and networks are in-memory, save once + return nm.save() +} + +func (nm *networkManager) DeleteState(_ []*EndpointInfo) error { + nm.Lock() + defer nm.Unlock() + + logger.Info("Deleting state") + // We do not use DeleteEndpointState for stateless cni because we already call it in DeleteEndpoint + // This function is only for saving to stateless cni or the cni statefile + // For stateless cni, plugin.ipamInvoker.Delete takes care of removing the state in the main Delete function + + if nm.IsStatelessCNIMode() { + return nil + } + + // once endpoints and networks are deleted in-memory, save once + return nm.save() +} + +// called to convert a cns restserver EndpointInfo into a network EndpointInfo +func cnsEndpointInfotoCNIEpInfos(endpointInfo restserver.EndpointInfo, endpointID string) []*EndpointInfo { + ret := []*EndpointInfo{} + for ifName, ipInfo := range endpointInfo.IfnameToIPMap { - // This is an special case for endpoint state that are being crated by statefull CNI + epInfo := &EndpointInfo{ + EndpointID: endpointID, // endpoint id is always the same, but we shouldn't use it in the stateless path + IfIndex: EndpointIfIndex, // Azure CNI supports only one interface + ContainerID: endpointID, + PODName: endpointInfo.PodName, + PODNameSpace: endpointInfo.PodNamespace, + NetworkContainerID: endpointID, + } + + // If we create an endpoint state with stateful cni and then swap to a stateless cni binary, ifname would not be populated + // triggered in migration to stateless only, assuming no incomplete state for delegated if ifName == "" { ifName = InfraInterfaceName + ipInfo.NICType = cns.InfraNIC } - // TODO: DelegatedNIC state will be added in a future PR - if ifName != InfraInterfaceName { - continue - } + + // filling out the InfraNIC from the state epInfo.IPAddresses = ipInfo.IPv4 epInfo.IPAddresses = append(epInfo.IPAddresses, ipInfo.IPv6...) - epInfo.IfName = ifName + epInfo.IfName = ifName // epInfo.IfName is set to the value of ep.IfName when the endpoint was added + // sidenote: ifname doesn't seem to be used in linux (or even windows) deletion epInfo.HostIfName = ipInfo.HostVethName epInfo.HNSEndpointID = ipInfo.HnsEndpointID + epInfo.NICType = ipInfo.NICType epInfo.HNSNetworkID = ipInfo.HnsNetworkID epInfo.MacAddress = net.HardwareAddr(ipInfo.MacAddress) + ret = append(ret, epInfo) } - return epInfo + return ret } -// generateCNSIPInfoMap generates a CNS ifNametoIPInfoMap structure based on CNI endpoint -func generateCNSIPInfoMap(ep *endpoint) map[string]*restserver.IPInfo { - ifNametoIPInfoMap := make(map[string]*restserver.IPInfo) // key : interface name, value : IPInfo - ifNametoIPInfoMap[ep.IfName] = &restserver.IPInfo{ - NICType: cns.InfraNIC, - HnsEndpointID: ep.HnsId, - HnsNetworkID: ep.HNSNetworkID, - HostVethName: ep.HostIfName, - MacAddress: ep.MacAddress.String(), +// gets all endpoint infos associated with a container id and populates the network id field +// nictype may be empty in which case it is likely of type "infra" +func (nm *networkManager) GetEndpointInfosFromContainerID(containerID string) []*EndpointInfo { + ret := []*EndpointInfo{} + for _, extIf := range nm.ExternalInterfaces { + for networkID, nw := range extIf.Networks { + for _, ep := range nw.Endpoints { + if ep.ContainerID == containerID { + val := ep.getInfo() + val.NetworkID = networkID // endpoint doesn't contain the network id + ret = append(ret, val) + } + } + } } + return ret +} + +func generateCNSIPInfoMap(eps []*endpoint) map[string]*restserver.IPInfo { + ifNametoIPInfoMap := make(map[string]*restserver.IPInfo) // key : interface name, value : IPInfo + + for _, ep := range eps { + ifNametoIPInfoMap[ep.IfName] = &restserver.IPInfo{ // in windows, the nicname is args ifname, in linux, it's ethX + NICType: ep.NICType, + HnsEndpointID: ep.HnsId, + HnsNetworkID: ep.HNSNetworkID, + HostVethName: ep.HostIfName, + MacAddress: ep.MacAddress.String(), + } + } + return ifNametoIPInfoMap } diff --git a/network/manager_mock.go b/network/manager_mock.go index 316af8d5d..3217484a8 100644 --- a/network/manager_mock.go +++ b/network/manager_mock.go @@ -6,17 +6,19 @@ import ( // MockNetworkManager is a mock structure for Network Manager type MockNetworkManager struct { - TestNetworkInfoMap map[string]*NetworkInfo + TestNetworkInfoMap map[string]*EndpointInfo TestEndpointInfoMap map[string]*EndpointInfo TestEndpointClient *MockEndpointClient + SaveStateMap map[string]*endpoint } // NewMockNetworkmanager returns a new mock func NewMockNetworkmanager(mockEndpointclient *MockEndpointClient) *MockNetworkManager { return &MockNetworkManager{ - TestNetworkInfoMap: make(map[string]*NetworkInfo), + TestNetworkInfoMap: make(map[string]*EndpointInfo), TestEndpointInfoMap: make(map[string]*EndpointInfo), TestEndpointClient: mockEndpointclient, + SaveStateMap: make(map[string]*endpoint), } } @@ -34,8 +36,8 @@ func (nm *MockNetworkManager) AddExternalInterface(ifName string, subnet string) } // CreateNetwork mock -func (nm *MockNetworkManager) CreateNetwork(nwInfo *NetworkInfo) error { - nm.TestNetworkInfoMap[nwInfo.Id] = nwInfo +func (nm *MockNetworkManager) CreateNetwork(nwInfo *EndpointInfo) error { + nm.TestNetworkInfoMap[nwInfo.NetworkID] = nwInfo return nil } @@ -45,22 +47,21 @@ func (nm *MockNetworkManager) DeleteNetwork(networkID string) error { } // GetNetworkInfo mock -func (nm *MockNetworkManager) GetNetworkInfo(networkID string) (NetworkInfo, error) { +func (nm *MockNetworkManager) GetNetworkInfo(networkID string) (EndpointInfo, error) { if info, exists := nm.TestNetworkInfoMap[networkID]; exists { return *info, nil } - return NetworkInfo{}, errNetworkNotFound + return EndpointInfo{}, errNetworkNotFound } // CreateEndpoint mock -func (nm *MockNetworkManager) CreateEndpoint(_ apipaClient, _ string, epInfos []*EndpointInfo) error { - for _, epInfo := range epInfos { - if err := nm.TestEndpointClient.AddEndpoints(epInfo); err != nil { - return err - } +// TODO: Fix mock behavior because create endpoint no longer also saves the state +func (nm *MockNetworkManager) CreateEndpoint(_ apipaClient, _ string, epInfo *EndpointInfo) error { + if err := nm.TestEndpointClient.AddEndpoints(epInfo); err != nil { + return err } - nm.TestEndpointInfoMap[epInfos[0].Id] = epInfos[0] + nm.TestEndpointInfoMap[epInfo.EndpointID] = epInfo return nil } @@ -147,10 +148,65 @@ func (nm *MockNetworkManager) GetNumEndpointsByContainerID(_ string) int { numEndpoints := 0 for _, network := range nm.TestNetworkInfoMap { - if _, err := nm.GetAllEndpoints(network.Id); err == nil { + if _, err := nm.GetAllEndpoints(network.NetworkID); err == nil { numEndpoints++ } } return numEndpoints } + +func (nm *MockNetworkManager) SaveState(eps []*endpoint) error { + for _, ep := range eps { + nm.SaveStateMap[ep.Id] = ep + } + + return nil +} + +func (nm *MockNetworkManager) EndpointCreate(client apipaClient, epInfos []*EndpointInfo) error { + eps := []*endpoint{} + for _, epInfo := range epInfos { + _, nwGetErr := nm.GetNetworkInfo(epInfo.NetworkID) + if nwGetErr != nil { + err := nm.CreateNetwork(epInfo) + if err != nil { + return err + } + } + + err := nm.CreateEndpoint(client, epInfo.NetworkID, epInfo) + if err != nil { + return err + } + eps = append(eps, &endpoint{ + Id: epInfo.EndpointID, + ContainerID: epInfo.ContainerID, + NICType: epInfo.NICType, + }) // mock append + } + + // mock save endpoints + return nm.SaveState(eps) +} + +func (nm *MockNetworkManager) DeleteState(epInfos []*EndpointInfo) error { + for _, epInfo := range epInfos { + delete(nm.SaveStateMap, epInfo.EndpointID) + } + return nil +} + +func (nm *MockNetworkManager) GetEndpointInfosFromContainerID(containerID string) []*EndpointInfo { + ret := []*EndpointInfo{} + for _, epInfo := range nm.TestEndpointInfoMap { + if epInfo.ContainerID == containerID { + ret = append(ret, epInfo) + } + } + return ret +} + +func (nm *MockNetworkManager) GetEndpointState(_, _ string) ([]*EndpointInfo, error) { + return []*EndpointInfo{}, nil +} diff --git a/network/manager_test.go b/network/manager_test.go index 567ef7c30..09569c25b 100644 --- a/network/manager_test.go +++ b/network/manager_test.go @@ -2,12 +2,16 @@ package network import ( "errors" + "net" + "sort" "testing" "time" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" + "github.com/Azure/azure-container-networking/cns" + "github.com/Azure/azure-container-networking/cns/restserver" "github.com/Azure/azure-container-networking/store" "github.com/Azure/azure-container-networking/testutils" ) @@ -226,4 +230,224 @@ var _ = Describe("Test Manager", func() { }) }) }) + Describe("Test EndpointCreate", func() { + Context("When no endpoints provided", func() { + It("Should return 0", func() { + nm := &networkManager{} + err := nm.EndpointCreate(nil, []*EndpointInfo{}) + Expect(err).NotTo(HaveOccurred()) + num := nm.GetNumberOfEndpoints("", "") + Expect(num).To(Equal(0)) + }) + }) + }) + Describe("Test GetEndpointInfosFromContainerID", func() { + Context("When getting containers based on container id regardless of network", func() { + It("Should return 0", func() { + nm := &networkManager{ + ExternalInterfaces: map[string]*externalInterface{ + "eth0": { + Networks: map[string]*network{ + "azure": { + Endpoints: map[string]*endpoint{ + "12345678-eth0": { + Id: "12345678-eth0", + ContainerID: "12345678", + // potentially empty nictype + }, + "abcdefgh-eth0": { + Id: "abcdefgh-eth0", + ContainerID: "abcdefgh", + }, + }, + }, + "other": { + Endpoints: map[string]*endpoint{ + "12345678-1": { + Id: "12345678-1", + ContainerID: "12345678", + NICType: cns.DelegatedVMNIC, + }, + }, + }, + }, + }, + }, + } + epInfos := nm.GetEndpointInfosFromContainerID("12345678") + sort.Slice(epInfos, func(i, j int) bool { + return epInfos[i].EndpointID < epInfos[j].EndpointID + }) + Expect(len(epInfos)).To(Equal(2)) + + Expect(epInfos[0].EndpointID).To(Equal("12345678-1")) + Expect(epInfos[0].NICType).To(Equal(cns.DelegatedVMNIC)) + Expect(epInfos[0].NetworkID).To(Equal("other")) + + Expect(epInfos[1].EndpointID).To(Equal("12345678-eth0")) + Expect(string(epInfos[1].NICType)).To(Equal("")) + Expect(epInfos[1].NetworkID).To(Equal("azure")) + }) + }) + }) + Describe("Test stateless cnsEndpointInfotoCNIEpInfos", func() { + endpointID := "" + _, dummyIP, _ := net.ParseCIDR("192.0.2.1/24") + dummyIPv4Slice := []net.IPNet{ + *dummyIP, + } + Context("When converting from cns to cni unmigrated", func() { + It("Should get the right cni endpoint info data", func() { + cnsEndpointInfo := restserver.EndpointInfo{ + IfnameToIPMap: map[string]*restserver.IPInfo{ + "": { + IPv4: dummyIPv4Slice, + HostVethName: "hostVeth1", + HnsEndpointID: "hnsID1", + HnsNetworkID: "hnsNetworkID1", + MacAddress: "12:34:56:78:9a:bc", + }, + }, + PodName: "test-pod", + PodNamespace: "test-pod-ns", + } + + epInfos := cnsEndpointInfotoCNIEpInfos(cnsEndpointInfo, endpointID) + + Expect(len(epInfos)).To(Equal(1)) + Expect(epInfos[0]).To(Equal( + &EndpointInfo{ + IPAddresses: dummyIPv4Slice, + IfName: InfraInterfaceName, + HostIfName: "hostVeth1", + HNSEndpointID: "hnsID1", + NICType: cns.InfraNIC, + HNSNetworkID: "hnsNetworkID1", + MacAddress: net.HardwareAddr("12:34:56:78:9a:bc"), + ContainerID: endpointID, + EndpointID: endpointID, + NetworkContainerID: endpointID, + PODName: "test-pod", + PODNameSpace: "test-pod-ns", + }, + ), "empty infos received from cns should be auto populated and treated as infra") + }) + }) + Context("When converting from cns to cni migrated", func() { + _, dummyIP2, _ := net.ParseCIDR("193.0.2.1/24") + dummyIPv4Slice2 := []net.IPNet{ + *dummyIP2, + } + It("Should get the right cni endpoint info data if there are multiple ip infos", func() { + cnsEndpointInfo := restserver.EndpointInfo{ + IfnameToIPMap: map[string]*restserver.IPInfo{ + "ifName1": { + IPv4: dummyIPv4Slice, + HostVethName: "hostVeth1", + HnsEndpointID: "hnsID1", + HnsNetworkID: "hnsNetworkID1", + MacAddress: "12:34:56:78:9a:bc", + NICType: cns.InfraNIC, + }, + "ifName2": { + IPv4: dummyIPv4Slice2, + HostVethName: "hostVeth2", + HnsEndpointID: "hnsID2", + HnsNetworkID: "hnsNetworkID2", + MacAddress: "22:34:56:78:9a:bc", + NICType: cns.DelegatedVMNIC, + }, + }, + PodName: "test-pod", + PodNamespace: "test-pod-ns", + } + + epInfos := cnsEndpointInfotoCNIEpInfos(cnsEndpointInfo, endpointID) + + Expect(len(epInfos)).To(Equal(2)) + Expect(epInfos).To(ContainElement( + &EndpointInfo{ + IPAddresses: dummyIPv4Slice, + IfName: "ifName1", + HostIfName: "hostVeth1", + HNSEndpointID: "hnsID1", + NICType: cns.InfraNIC, + HNSNetworkID: "hnsNetworkID1", + MacAddress: net.HardwareAddr("12:34:56:78:9a:bc"), + ContainerID: endpointID, + EndpointID: endpointID, + NetworkContainerID: endpointID, + PODName: "test-pod", + PODNameSpace: "test-pod-ns", + }, + )) + Expect(epInfos).To(ContainElement( + &EndpointInfo{ + IPAddresses: dummyIPv4Slice2, + IfName: "ifName2", + HostIfName: "hostVeth2", + HNSEndpointID: "hnsID2", + NICType: cns.DelegatedVMNIC, + HNSNetworkID: "hnsNetworkID2", + MacAddress: net.HardwareAddr("22:34:56:78:9a:bc"), + ContainerID: endpointID, + EndpointID: endpointID, + NetworkContainerID: endpointID, + PODName: "test-pod", + PODNameSpace: "test-pod-ns", + }, + )) + }) + }) + }) + Describe("Test stateless generateCNSIPInfoMap", func() { + Context("When converting from cni to cns", func() { + It("Should generate the cns endpoint info data from the endpoint structs", func() { + mac1, _ := net.ParseMAC("12:34:56:78:9a:bc") + mac2, _ := net.ParseMAC("22:34:56:78:9a:bc") + endpoints := []*endpoint{ + { + IfName: "eth0", + NICType: cns.InfraNIC, + HnsId: "hnsEndpointID1", + HNSNetworkID: "hnsNetworkID1", + HostIfName: "hostIfName1", + MacAddress: mac1, + }, + { + IfName: "eth1", + NICType: cns.DelegatedVMNIC, + HnsId: "hnsEndpointID2", + HNSNetworkID: "hnsNetworkID2", + HostIfName: "hostIfName2", + MacAddress: mac2, + }, + } + cnsEpInfos := generateCNSIPInfoMap(endpoints) + Expect(len(cnsEpInfos)).To(Equal(2)) + + Expect(cnsEpInfos).To(HaveKey("eth0")) + Expect(cnsEpInfos["eth0"]).To(Equal( + &restserver.IPInfo{ + NICType: cns.InfraNIC, + HnsEndpointID: "hnsEndpointID1", + HnsNetworkID: "hnsNetworkID1", + HostVethName: "hostIfName1", + MacAddress: "12:34:56:78:9a:bc", + }, + )) + + Expect(cnsEpInfos).To(HaveKey("eth1")) + Expect(cnsEpInfos["eth1"]).To(Equal( + &restserver.IPInfo{ + NICType: cns.DelegatedVMNIC, + HnsEndpointID: "hnsEndpointID2", + HnsNetworkID: "hnsNetworkID2", + HostVethName: "hostIfName2", + MacAddress: "22:34:56:78:9a:bc", + }, + )) + }) + }) + }) }) diff --git a/network/mock_endpointclient.go b/network/mock_endpointclient.go index b95ab4789..b73546a58 100644 --- a/network/mock_endpointclient.go +++ b/network/mock_endpointclient.go @@ -36,11 +36,11 @@ func NewMockEndpointClient(fn func(*EndpointInfo) error) *MockEndpointClient { } func (client *MockEndpointClient) AddEndpoints(epInfo *EndpointInfo) error { - if ok := client.endpoints[epInfo.Id]; ok && epInfo.IfName == eth0IfName { + if ok := client.endpoints[epInfo.EndpointID]; ok && epInfo.IfName == eth0IfName { return NewErrorMockEndpointClient("Endpoint already exists") } - client.endpoints[epInfo.Id] = true + client.endpoints[epInfo.EndpointID] = true return client.testAddEndpointFn(epInfo) } diff --git a/network/network.go b/network/network.go index 0c0622cec..319f82b9a 100644 --- a/network/network.go +++ b/network/network.go @@ -56,7 +56,7 @@ type network struct { SnatBridgeIP string } -// NetworkInfo contains read-only information about a container network. +// NetworkInfo contains read-only information about a container network. Use EndpointInfo instead when possible. type NetworkInfo struct { MasterIfName string AdapterName string @@ -160,14 +160,14 @@ func (nm *networkManager) findExternalInterfaceByName(ifName string) *externalIn } // NewNetwork creates a new container network. -func (nm *networkManager) newNetwork(nwInfo *NetworkInfo) (*network, error) { +func (nm *networkManager) newNetwork(nwInfo *EndpointInfo) (*network, error) { var nw *network var err error logger.Info("Creating", zap.String("network", nwInfo.PrettyString())) defer func() { if err != nil { - logger.Error("Failed to create network", zap.String("id", nwInfo.Id), zap.Error(err)) + logger.Error("Failed to create network", zap.String("id", nwInfo.NetworkID), zap.Error(err)) } }() @@ -190,7 +190,7 @@ func (nm *networkManager) newNetwork(nwInfo *NetworkInfo) (*network, error) { } // Make sure this network does not already exist. - if extIf.Networks[nwInfo.Id] != nil { + if extIf.Networks[nwInfo.NetworkID] != nil { err = errNetworkExists return nil, err } @@ -203,9 +203,9 @@ func (nm *networkManager) newNetwork(nwInfo *NetworkInfo) (*network, error) { // Add the network object. nw.Subnets = nwInfo.Subnets - extIf.Networks[nwInfo.Id] = nw + extIf.Networks[nwInfo.NetworkID] = nw - logger.Info("Created network on interface", zap.String("id", nwInfo.Id), zap.String("Name", extIf.Name)) + logger.Info("Created network on interface", zap.String("id", nwInfo.NetworkID), zap.String("Name", extIf.Name)) return nw, nil } @@ -296,3 +296,41 @@ func (nm *networkManager) GetNumEndpointsByContainerID(containerID string) int { return numEndpoints } + +// Creates the network and corresponding endpoint (should be called once during Add) +func (nm *networkManager) EndpointCreate(cnsclient apipaClient, epInfos []*EndpointInfo) error { + eps := []*endpoint{} // save endpoints for stateless + + for _, epInfo := range epInfos { + logger.Info("Creating endpoint and network", zap.String("endpointInfo", epInfo.PrettyString())) + // check if network exists by searching through all external interfaces for the network + _, nwGetErr := nm.GetNetworkInfo(epInfo.NetworkID) + if nwGetErr != nil { + logger.Info("Existing network not found", zap.String("networkID", epInfo.NetworkID)) + + logger.Info("Found master interface", zap.String("masterIfName", epInfo.MasterIfName)) + + // Add the master as an external interface. + err := nm.AddExternalInterface(epInfo.MasterIfName, epInfo.HostSubnetPrefix) + if err != nil { + return err + } + + // Create the network if it is not found + err = nm.CreateNetwork(epInfo) + if err != nil { + return err + } + } + + ep, err := nm.createEndpoint(cnsclient, epInfo.NetworkID, epInfo) + if err != nil { + return err + } + + eps = append(eps, ep) + } + + // save endpoints + return nm.SaveState(eps) +} diff --git a/network/network_linux.go b/network/network_linux.go index 2b13c6c68..7f688d2b2 100644 --- a/network/network_linux.go +++ b/network/network_linux.go @@ -59,7 +59,7 @@ func newErrorNetworkManager(errStr string) error { type route netlink.Route // NewNetworkImpl creates a new container network. -func (nm *networkManager) newNetworkImpl(nwInfo *NetworkInfo, extIf *externalInterface) (*network, error) { +func (nm *networkManager) newNetworkImpl(nwInfo *EndpointInfo, extIf *externalInterface) (*network, error) { // Connect the external interface. var ( vlanid int @@ -116,19 +116,18 @@ func (nm *networkManager) newNetworkImpl(nwInfo *NetworkInfo, extIf *externalInt // Create the network object. nw := &network{ - Id: nwInfo.Id, + Id: nwInfo.NetworkID, Mode: nwInfo.Mode, Endpoints: make(map[string]*endpoint), extIf: extIf, VlanId: vlanid, - DNS: nwInfo.DNS, EnableSnatOnHost: nwInfo.EnableSnatOnHost, } return nw, nil } -func (nm *networkManager) handleCommonOptions(ifName string, nwInfo *NetworkInfo) error { +func (nm *networkManager) handleCommonOptions(ifName string, nwInfo *EndpointInfo) error { var err error if routes, exists := nwInfo.Options[RoutesKey]; exists { err = addRoutes(nm.netlink, nm.netio, ifName, routes.([]RouteInfo)) @@ -154,7 +153,7 @@ func (nm *networkManager) deleteNetworkImpl(nw *network) error { if nw.VlanId != 0 { networkClient = NewOVSClient(nw.extIf.BridgeName, nw.extIf.Name, ovsctl.NewOvsctl(), nm.netlink, nm.plClient) } else { - networkClient = NewLinuxBridgeClient(nw.extIf.BridgeName, nw.extIf.Name, NetworkInfo{}, nm.netlink, nm.plClient) + networkClient = NewLinuxBridgeClient(nw.extIf.BridgeName, nw.extIf.Name, EndpointInfo{}, nm.netlink, nm.plClient) } // Disconnect the interface if this was the last network using it. @@ -460,7 +459,7 @@ func (nm *networkManager) applyDNSConfig(extIf *externalInterface, ifName string } // ConnectExternalInterface connects the given host interface to a bridge. -func (nm *networkManager) connectExternalInterface(extIf *externalInterface, nwInfo *NetworkInfo) error { +func (nm *networkManager) connectExternalInterface(extIf *externalInterface, nwInfo *EndpointInfo) error { var ( err error networkClient NetworkClient @@ -664,7 +663,7 @@ func (nm *networkManager) addToIptables(cmds []iptables.IPTableEntry) error { } // Add ipv6 nat gateway IP on bridge -func (nm *networkManager) addIpv6NatGateway(nwInfo *NetworkInfo) error { +func (nm *networkManager) addIpv6NatGateway(nwInfo *EndpointInfo) error { logger.Info("Adding ipv6 nat gateway on azure bridge") for _, subnetInfo := range nwInfo.Subnets { if subnetInfo.Family == platform.AfINET6 { @@ -684,7 +683,7 @@ func (nm *networkManager) addIpv6NatGateway(nwInfo *NetworkInfo) error { } // snat ipv6 traffic to secondary ipv6 ip before leaving VM -func (nm *networkManager) addIpv6SnatRule(extIf *externalInterface, nwInfo *NetworkInfo) error { +func (nm *networkManager) addIpv6SnatRule(extIf *externalInterface, nwInfo *EndpointInfo) error { var ( ipv6SnatRuleSet bool ipv6SubnetPrefix net.IPNet @@ -721,7 +720,7 @@ func (nm *networkManager) addIpv6SnatRule(extIf *externalInterface, nwInfo *Netw return nil } -func getNetworkInfoImpl(nwInfo *NetworkInfo, nw *network) { +func getNetworkInfoImpl(nwInfo *EndpointInfo, nw *network) { if nw.VlanId != 0 { vlanMap := make(map[string]interface{}) vlanMap[VlanIDKey] = strconv.Itoa(nw.VlanId) diff --git a/network/network_test.go b/network/network_test.go index 37fdcd860..2f4845102 100644 --- a/network/network_test.go +++ b/network/network_test.go @@ -100,7 +100,7 @@ var _ = Describe("Test Network", func() { nm := &networkManager{ ExternalInterfaces: map[string]*externalInterface{}, } - nwInfo := &NetworkInfo{ + nwInfo := &EndpointInfo{ MasterIfName: "eth0", } _, _ = nm.newNetwork(nwInfo) @@ -113,7 +113,7 @@ var _ = Describe("Test Network", func() { nm := &networkManager{ ExternalInterfaces: map[string]*externalInterface{}, } - nwInfo := &NetworkInfo{ + nwInfo := &EndpointInfo{ MasterIfName: "eth0", } nw, err := nm.newNetwork(nwInfo) @@ -127,7 +127,7 @@ var _ = Describe("Test Network", func() { nm := &networkManager{ ExternalInterfaces: map[string]*externalInterface{}, } - nwInfo := &NetworkInfo{ + nwInfo := &EndpointInfo{ Subnets: []SubnetInfo{{ Prefix: net.IPNet{ IP: net.IPv4(10, 0, 0, 1), @@ -150,8 +150,8 @@ var _ = Describe("Test Network", func() { Networks: map[string]*network{}, } nm.ExternalInterfaces["eth0"].Networks["nw"] = &network{} - nwInfo := &NetworkInfo{ - Id: "nw", + nwInfo := &EndpointInfo{ + NetworkID: "nw", MasterIfName: "eth0", } nw, err := nm.newNetwork(nwInfo) @@ -169,8 +169,8 @@ var _ = Describe("Test Network", func() { nm.ExternalInterfaces["eth0"] = &externalInterface{ Networks: map[string]*network{}, } - nwInfo := &NetworkInfo{ - Id: "nw", + nwInfo := &EndpointInfo{ + NetworkID: "nw", MasterIfName: "eth0", Mode: opModeTransparent, IPV6Mode: IPV6Nat, @@ -178,7 +178,7 @@ var _ = Describe("Test Network", func() { nw, err := nm.newNetwork(nwInfo) Expect(err).To(BeNil()) Expect(nw).NotTo(BeNil()) - Expect(nw.Id).To(Equal(nwInfo.Id)) + Expect(nw.Id).To(Equal(nwInfo.NetworkID)) }) }) @@ -191,8 +191,8 @@ var _ = Describe("Test Network", func() { nm.ExternalInterfaces["eth0"] = &externalInterface{ Networks: map[string]*network{}, } - nwInfo := &NetworkInfo{ - Id: "nw", + nwInfo := &EndpointInfo{ + NetworkID: "nw", MasterIfName: "eth0", Mode: opModeTransparentVlan, } diff --git a/network/network_windows.go b/network/network_windows.go index 29917db23..e588bdc04 100644 --- a/network/network_windows.go +++ b/network/network_windows.go @@ -10,6 +10,7 @@ import ( "strings" "time" + "github.com/Azure/azure-container-networking/cns" "github.com/Azure/azure-container-networking/network/hnswrapper" "github.com/Azure/azure-container-networking/network/policy" "github.com/Azure/azure-container-networking/platform" @@ -86,7 +87,7 @@ func EnableHnsV1Timeout(timeoutValue int) { } // newNetworkImplHnsV1 creates a new container network for HNSv1. -func (nm *networkManager) newNetworkImplHnsV1(nwInfo *NetworkInfo, extIf *externalInterface) (*network, error) { +func (nm *networkManager) newNetworkImplHnsV1(nwInfo *EndpointInfo, extIf *externalInterface) (*network, error) { var ( vlanid int err error @@ -111,10 +112,9 @@ func (nm *networkManager) newNetworkImplHnsV1(nwInfo *NetworkInfo, extIf *extern // Initialize HNS network. hnsNetwork := &hcsshim.HNSNetwork{ - Name: nwInfo.Id, + Name: nwInfo.NetworkID, NetworkAdapterName: networkAdapterName, - DNSServerList: strings.Join(nwInfo.DNS.Servers, ","), - Policies: policy.SerializePolicies(policy.NetworkPolicy, nwInfo.Policies, nil, false, false), + Policies: policy.SerializePolicies(policy.NetworkPolicy, nwInfo.NetworkPolicies, nil, false, false), } // Set the VLAN and OutboundNAT policies @@ -172,7 +172,7 @@ func (nm *networkManager) newNetworkImplHnsV1(nwInfo *NetworkInfo, extIf *extern // Create the network object. nw := &network{ - Id: nwInfo.Id, + Id: nwInfo.NetworkID, HnsId: hnsResponse.Id, Mode: nwInfo.Mode, Endpoints: make(map[string]*endpoint), @@ -182,6 +182,8 @@ func (nm *networkManager) newNetworkImplHnsV1(nwInfo *NetworkInfo, extIf *extern NetNs: nwInfo.NetNs, } + nwInfo.HNSNetworkID = hnsResponse.Id // we use this later in stateless to clean up in ADD if there is an error + globals, err := Hnsv1.GetHNSGlobals() if err != nil || globals.Version.Major <= hcsshim.HNSVersion1803.Major { // err would be not nil for windows 1709 & below @@ -193,7 +195,7 @@ func (nm *networkManager) newNetworkImplHnsV1(nwInfo *NetworkInfo, extIf *extern return nw, nil } -func (nm *networkManager) appIPV6RouteEntry(nwInfo *NetworkInfo) error { +func (nm *networkManager) appIPV6RouteEntry(nwInfo *EndpointInfo) error { var ( err error out string @@ -227,14 +229,10 @@ func (nm *networkManager) appIPV6RouteEntry(nwInfo *NetworkInfo) error { } // configureHcnEndpoint configures hcn endpoint for creation -func (nm *networkManager) configureHcnNetwork(nwInfo *NetworkInfo, extIf *externalInterface) (*hcn.HostComputeNetwork, error) { +func (nm *networkManager) configureHcnNetwork(nwInfo *EndpointInfo, extIf *externalInterface) (*hcn.HostComputeNetwork, error) { // Initialize HNS network. hcnNetwork := &hcn.HostComputeNetwork{ - Name: nwInfo.Id, - Dns: hcn.Dns{ - Domain: nwInfo.DNS.Suffix, - ServerList: nwInfo.DNS.Servers, - }, + Name: nwInfo.NetworkID, Ipams: []hcn.Ipam{ { Type: hcnIpamTypeStatic, @@ -299,6 +297,11 @@ func (nm *networkManager) configureHcnNetwork(nwInfo *NetworkInfo, extIf *extern return nil, errNetworkModeInvalid } + if nwInfo.NICType == cns.DelegatedVMNIC { + hcnNetwork.Type = hcn.Transparent + hcnNetwork.Flags = hcn.DisableHostPort + } + // Populate subnets. for _, subnet := range nwInfo.Subnets { hnsSubnet := hcn.Subnet{ @@ -352,7 +355,7 @@ func (nm *networkManager) addIPv6DefaultRoute() error { } // newNetworkImplHnsV2 creates a new container network for HNSv2. -func (nm *networkManager) newNetworkImplHnsV2(nwInfo *NetworkInfo, extIf *externalInterface) (*network, error) { +func (nm *networkManager) newNetworkImplHnsV2(nwInfo *EndpointInfo, extIf *externalInterface) (*network, error) { hcnNetwork, err := nm.configureHcnNetwork(nwInfo, extIf) if err != nil { logger.Error("Failed to configure hcn network due to", zap.Error(err)) @@ -400,7 +403,7 @@ func (nm *networkManager) newNetworkImplHnsV2(nwInfo *NetworkInfo, extIf *extern // Create the network object. nw := &network{ - Id: nwInfo.Id, + Id: nwInfo.NetworkID, HnsId: hnsResponse.Id, Mode: nwInfo.Mode, Endpoints: make(map[string]*endpoint), @@ -410,11 +413,13 @@ func (nm *networkManager) newNetworkImplHnsV2(nwInfo *NetworkInfo, extIf *extern NetNs: nwInfo.NetNs, } + nwInfo.HNSNetworkID = hnsResponse.Id // we use this later in stateless to clean up in ADD if there is an error + return nw, nil } // NewNetworkImpl creates a new container network. -func (nm *networkManager) newNetworkImpl(nwInfo *NetworkInfo, extIf *externalInterface) (*network, error) { +func (nm *networkManager) newNetworkImpl(nwInfo *EndpointInfo, extIf *externalInterface) (*network, error) { if useHnsV2, err := UseHnsV2(nwInfo.NetNs); useHnsV2 { if err != nil { return nil, err @@ -463,5 +468,5 @@ func (nm *networkManager) deleteNetworkImplHnsV2(nw *network) error { return err } -func getNetworkInfoImpl(nwInfo *NetworkInfo, nw *network) { +func getNetworkInfoImpl(_ *EndpointInfo, _ *network) { } diff --git a/network/network_windows_test.go b/network/network_windows_test.go index b52ef482a..e9990ce93 100644 --- a/network/network_windows_test.go +++ b/network/network_windows_test.go @@ -34,8 +34,8 @@ func TestNewAndDeleteNetworkImplHnsV2(t *testing.T) { // we do this to avoid passing around os specific objects in platform agnostic code Hnsv2 = hnswrapper.NewHnsv2wrapperFake() - nwInfo := &NetworkInfo{ - Id: "d3e97a83-ba4c-45d5-ba88-dc56757ece28", + nwInfo := &EndpointInfo{ + NetworkID: "d3e97a83-ba4c-45d5-ba88-dc56757ece28", MasterIfName: "eth0", Mode: "bridge", } @@ -75,8 +75,8 @@ func TestSuccesfulNetworkCreationWhenAlreadyExists(t *testing.T) { _, err := Hnsv2.CreateNetwork(network) // network name is derived from network info id - nwInfo := &NetworkInfo{ - Id: "azure-vlan1-172-28-1-0_24", + nwInfo := &EndpointInfo{ + NetworkID: "azure-vlan1-172-28-1-0_24", MasterIfName: "eth0", Mode: "bridge", } @@ -108,8 +108,8 @@ func TestNewNetworkImplHnsV2WithTimeout(t *testing.T) { HnsCallTimeout: 10 * time.Second, } - nwInfo := &NetworkInfo{ - Id: "d3e97a83-ba4c-45d5-ba88-dc56757ece28", + nwInfo := &EndpointInfo{ + NetworkID: "d3e97a83-ba4c-45d5-ba88-dc56757ece28", MasterIfName: "eth0", Mode: "bridge", } @@ -131,8 +131,8 @@ func TestDeleteNetworkImplHnsV2WithTimeout(t *testing.T) { ExternalInterfaces: map[string]*externalInterface{}, } - nwInfo := &NetworkInfo{ - Id: "d3e97a83-ba4c-45d5-ba88-dc56757ece28", + nwInfo := &EndpointInfo{ + NetworkID: "d3e97a83-ba4c-45d5-ba88-dc56757ece28", MasterIfName: "eth0", Mode: "bridge", } @@ -180,8 +180,8 @@ func TestNewNetworkImplHnsV1WithTimeout(t *testing.T) { HnsCallTimeout: 5 * time.Second, } - nwInfo := &NetworkInfo{ - Id: "d3e97a83-ba4c-45d5-ba88-dc56757ece28", + nwInfo := &EndpointInfo{ + NetworkID: "d3e97a83-ba4c-45d5-ba88-dc56757ece28", MasterIfName: "eth0", Mode: "bridge", } @@ -203,8 +203,8 @@ func TestDeleteNetworkImplHnsV1WithTimeout(t *testing.T) { ExternalInterfaces: map[string]*externalInterface{}, } - nwInfo := &NetworkInfo{ - Id: "d3e97a83-ba4c-45d5-ba88-dc56757ece28", + nwInfo := &EndpointInfo{ + NetworkID: "d3e97a83-ba4c-45d5-ba88-dc56757ece28", MasterIfName: "eth0", Mode: "bridge", } @@ -260,8 +260,8 @@ func TestAddIPv6DefaultRoute(t *testing.T) { }, } - nwInfo := &NetworkInfo{ - Id: "d3f97a83-ba4c-45d5-ba88-dc56757ece28", + nwInfo := &EndpointInfo{ + NetworkID: "d3f97a83-ba4c-45d5-ba88-dc56757ece28", MasterIfName: "eth0", Mode: "bridge", Subnets: networkSubnetInfo, @@ -303,8 +303,8 @@ func TestFailToAddIPv6DefaultRoute(t *testing.T) { }, } - nwInfo := &NetworkInfo{ - Id: "d3f97a83-ba4c-45d5-ba88-dc56757ece28", + nwInfo := &EndpointInfo{ + NetworkID: "d3f97a83-ba4c-45d5-ba88-dc56757ece28", MasterIfName: "eth0", Mode: "bridge", Subnets: networkSubnetInfo, diff --git a/network/ovs_endpoint_infraroute_linux.go b/network/ovs_endpoint_infraroute_linux.go index 829a58ee4..c1ace0423 100644 --- a/network/ovs_endpoint_infraroute_linux.go +++ b/network/ovs_endpoint_infraroute_linux.go @@ -62,7 +62,7 @@ func ConfigureInfraVnetContainerInterface(client *OVSEndpointClient, infraIP net return nil } -func DeleteInfraVnetEndpoint(client *OVSEndpointClient, epID string) error { +func DeleteInfraVnetEndpoint(client *OVSEndpointClient) error { if client.enableInfraVnet { return client.infraVnetClient.DeleteInfraVnetEndpoint() } diff --git a/network/ovs_endpoint_snatroute_linux.go b/network/ovs_endpoint_snatroute_linux.go index 3e10db761..12bd40ab8 100644 --- a/network/ovs_endpoint_snatroute_linux.go +++ b/network/ovs_endpoint_snatroute_linux.go @@ -28,7 +28,7 @@ func (client *OVSEndpointClient) NewSnatClient(snatBridgeIP, localIP string, epI localIP, snatBridgeIP, client.hostPrimaryMac, - epInfo.DNS.Servers, + epInfo.EndpointDNS.Servers, false, client.netlink, client.plClient, diff --git a/network/ovs_endpointclient_linux.go b/network/ovs_endpointclient_linux.go index 569232eb9..f82724a3e 100644 --- a/network/ovs_endpointclient_linux.go +++ b/network/ovs_endpointclient_linux.go @@ -74,7 +74,7 @@ func NewOVSEndpointClient( netioshim: &netio.NetIO{}, } - NewInfraVnetClient(client, epInfo.Id[:7]) + NewInfraVnetClient(client, epInfo.EndpointID[:7]) client.NewSnatClient(nw.SnatBridgeIP, localIP, epInfo) return client @@ -251,5 +251,5 @@ func (client *OVSEndpointClient) DeleteEndpoints(ep *endpoint) error { if err := client.DeleteSnatEndpoint(); err != nil { return err } - return DeleteInfraVnetEndpoint(client, ep.Id[:7]) + return DeleteInfraVnetEndpoint(client) } diff --git a/network/secondary_endpoint_client_linux.go b/network/secondary_endpoint_client_linux.go index efa7a2013..46fc3c26f 100644 --- a/network/secondary_endpoint_client_linux.go +++ b/network/secondary_endpoint_client_linux.go @@ -169,7 +169,7 @@ func (client *SecondaryEndpointClient) DeleteEndpoints(ep *endpoint) error { logger.Error("Failed to exit netns with", zap.Error(newErrorSecondaryEndpointClient(err))) } }() - + // TODO: For stateless cni linux, check if delegated vmnic type, and if so, delete using this *endpoint* struct's ifname for iface := range ep.SecondaryInterfaces { if err := client.netlink.SetLinkNetNs(iface, uintptr(vmns)); err != nil { logger.Error("Failed to move interface", zap.String("IfName", iface), zap.Error(newErrorSecondaryEndpointClient(err))) diff --git a/network/secondary_endpoint_linux_test.go b/network/secondary_endpoint_linux_test.go index 775ffeeb6..6073101b3 100644 --- a/network/secondary_endpoint_linux_test.go +++ b/network/secondary_endpoint_linux_test.go @@ -7,6 +7,7 @@ import ( "net" "testing" + "github.com/Azure/azure-container-networking/cns" "github.com/Azure/azure-container-networking/netio" "github.com/Azure/azure-container-networking/netlink" "github.com/Azure/azure-container-networking/network/networkutils" @@ -185,6 +186,40 @@ func TestSecondaryDeleteEndpoints(t *testing.T) { }, wantErr: true, }, + { + // new way to handle delegated nics + // if the nictype is delegated, the data is on the endpoint itself, not the secondary interfaces field + name: "Delete endpoint with nic type delegated", + client: &SecondaryEndpointClient{ + netlink: netlink.NewMockNetlink(false, ""), + plClient: platform.NewMockExecClient(false), + netUtilsClient: networkutils.NewNetworkUtils(nl, plc), + netioshim: netio.NewMockNetIO(false, 0), + nsClient: NewMockNamespaceClient(), + }, + // revisit in future, but currently the struct looks like this (with duplicated fields) + ep: &endpoint{ + NetworkNameSpace: "testns", + IfName: "eth1", + Routes: []RouteInfo{ + { + Dst: net.IPNet{IP: net.ParseIP("192.168.0.4"), Mask: net.CIDRMask(ipv4FullMask, ipv4Bits)}, + }, + }, + NICType: cns.DelegatedVMNIC, + SecondaryInterfaces: map[string]*InterfaceInfo{ + "eth1": { + Name: "eth1", + Routes: []RouteInfo{ + { + Dst: net.IPNet{IP: net.ParseIP("192.168.0.4"), Mask: net.CIDRMask(ipv4FullMask, ipv4Bits)}, + }, + }, + }, + }, + }, + wantErr: false, + }, } for _, tt := range tests { diff --git a/network/transparent_vlan_endpoint_snatroute_linux.go b/network/transparent_vlan_endpoint_snatroute_linux.go index b6f712758..d997ead96 100644 --- a/network/transparent_vlan_endpoint_snatroute_linux.go +++ b/network/transparent_vlan_endpoint_snatroute_linux.go @@ -16,7 +16,7 @@ func (client *TransparentVlanEndpointClient) NewSnatClient(snatBridgeIP, localIP localIP, snatBridgeIP, client.hostPrimaryMac.String(), - epInfo.DNS.Servers, + epInfo.EndpointDNS.Servers, true, client.netlink, client.plClient,