refactor: code changes for stateless cni and swift v2

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
 }

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,15 +39,15 @@ 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,
 	}
 }
 
 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)
+}

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

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,29 +163,42 @@ 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
 			if !info.skipDefaultRoutes {
 				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,15 +365,15 @@ 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 {
 		addConfig.options[network.SNATIPKey] = info.ncPrimaryIP
 	}
 
 	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
-	}
-
-	// 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)
+		// 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,
+		}
 	}
 
-	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)
+}