refactor: Modify CNI to Support AKS-Swift

cni/network/invoker.go

@@ -4,6 +4,7 @@ import (
 	"net"
 
 	"github.com/Azure/azure-container-networking/cni"
+	"github.com/Azure/azure-container-networking/cns"
 	cniSkel "github.com/containernetworking/cni/pkg/skel"
 	cniTypesCurr "github.com/containernetworking/cni/pkg/types/current"
 )
@@ -14,8 +15,21 @@ import (
 type IPAMInvoker interface {
 
 	// Add returns two results, one IPv4, the other IPv6.
-	Add(nwCfg *cni.NetworkConfig, args *cniSkel.CmdArgs, subnetPrefix *net.IPNet, options map[string]interface{}) (*cniTypesCurr.Result, *cniTypesCurr.Result, error)
+	Add(IPAMAddConfig) (IPAMAddResult, error)
 
 	// Delete calls to the invoker source, and returns error. Returning an error here will fail the CNI Delete call.
 	Delete(address *net.IPNet, nwCfg *cni.NetworkConfig, args *cniSkel.CmdArgs, options map[string]interface{}) error
 }
+
+type IPAMAddConfig struct {
+	nwCfg   *cni.NetworkConfig
+	args    *cniSkel.CmdArgs
+	options map[string]interface{}
+}
+
+type IPAMAddResult struct {
+	ipv4Result       *cniTypesCurr.Result
+	ipv6Result       *cniTypesCurr.Result
+	ncResponse       *cns.GetNetworkContainerResponse
+	hostSubnetPrefix net.IPNet
+}

cni/network/invoker_azure.go

@@ -32,33 +32,31 @@ func NewAzureIpamInvoker(plugin *NetPlugin, nwInfo *network.NetworkInfo) *AzureI
 	}
 }
 
-func (invoker *AzureIPAMInvoker) Add(nwCfg *cni.NetworkConfig, _ *cniSkel.CmdArgs, subnetPrefix *net.IPNet, options map[string]interface{}) (*cniTypesCurr.Result, *cniTypesCurr.Result, error) {
+func (invoker *AzureIPAMInvoker) Add(addConfig IPAMAddConfig) (IPAMAddResult, error) {
 	var (
-		result   *cniTypesCurr.Result
-		resultV6 *cniTypesCurr.Result
-		err      error
+		addResult = IPAMAddResult{}
+		err       error
 	)
 
-	if nwCfg == nil {
-		err = invoker.plugin.Errorf("nil nwCfg passed to CNI ADD, stack: %+v", string(debug.Stack()))
-		return nil, nil, err
+	if addConfig.nwCfg == nil {
+		return addResult, invoker.plugin.Errorf("nil nwCfg passed to CNI ADD, stack: %+v", string(debug.Stack()))
 	}
 
 	if len(invoker.nwInfo.Subnets) > 0 {
-		nwCfg.Ipam.Subnet = invoker.nwInfo.Subnets[0].Prefix.String()
+		addConfig.nwCfg.Ipam.Subnet = invoker.nwInfo.Subnets[0].Prefix.String()
 	}
 
 	// Call into IPAM plugin to allocate an address pool for the network.
-	result, err = invoker.plugin.DelegateAdd(nwCfg.Ipam.Type, nwCfg)
+	addResult.ipv4Result, err = invoker.plugin.DelegateAdd(addConfig.nwCfg.Ipam.Type, addConfig.nwCfg)
 	if err != nil {
 		err = invoker.plugin.Errorf("Failed to allocate pool: %v", err)
-		return nil, nil, err
+		return addResult, err
 	}
 
 	defer func() {
 		if err != nil {
-			if len(result.IPs) > 0 {
-				if er := invoker.Delete(&result.IPs[0].Address, nwCfg, nil, options); er != nil {
+			if len(addResult.ipv4Result.IPs) > 0 {
+				if er := invoker.Delete(&addResult.ipv4Result.IPs[0].Address, addConfig.nwCfg, nil, addConfig.options); er != nil {
 					err = invoker.plugin.Errorf("Failed to clean up IP's during Delete with error %v, after Add failed with error %w", er, err)
 				}
 			} else {
@@ -67,8 +65,8 @@ func (invoker *AzureIPAMInvoker) Add(nwCfg *cni.NetworkConfig, _ *cniSkel.CmdArg
 		}
 	}()
 
-	if nwCfg.IPV6Mode != "" {
-		nwCfg6 := *nwCfg
+	if addConfig.nwCfg.IPV6Mode != "" {
+		nwCfg6 := *addConfig.nwCfg
 		nwCfg6.Ipam.Environment = common.OptEnvironmentIPv6NodeIpam
 		nwCfg6.Ipam.Type = ipamV6
 
@@ -77,16 +75,15 @@ func (invoker *AzureIPAMInvoker) Add(nwCfg *cni.NetworkConfig, _ *cniSkel.CmdArg
 			nwCfg6.Ipam.Subnet = invoker.nwInfo.Subnets[1].Prefix.String()
 		}
 
-		resultV6, err = invoker.plugin.DelegateAdd(nwCfg6.Ipam.Type, &nwCfg6)
+		addResult.ipv6Result, err = invoker.plugin.DelegateAdd(nwCfg6.Ipam.Type, &nwCfg6)
 		if err != nil {
 			err = invoker.plugin.Errorf("Failed to allocate v6 pool: %v", err)
 		}
 	}
 
-	sub := &result.IPs[0].Address
-	*subnetPrefix = *sub
+	addResult.hostSubnetPrefix = addResult.ipv4Result.IPs[0].Address
 
-	return result, resultV6, err
+	return addResult, err
 }
 
 func (invoker *AzureIPAMInvoker) Delete(address *net.IPNet, nwCfg *cni.NetworkConfig, _ *cniSkel.CmdArgs, options map[string]interface{}) error {

cni/network/invoker_azure_test.go

@@ -223,15 +223,16 @@ func TestAzureIPAMInvoker_Add(t *testing.T) {
 				plugin: tt.fields.plugin,
 				nwInfo: tt.fields.nwInfo,
 			}
-			got, got1, err := invoker.Add(tt.args.nwCfg, tt.args.in1, tt.args.subnetPrefix, tt.args.options)
+
+			ipamAddResult, err := invoker.Add(IPAMAddConfig{nwCfg: tt.args.nwCfg, args: tt.args.in1, options: tt.args.options})
 			if tt.wantErr {
 				require.NotNil(err) // use NotNil since *cniTypes.Error is not of type Error
 			} else {
 				require.Nil(err)
 			}
 
-			require.Exactly(tt.want, got)
-			require.Exactly(tt.want1, got1)
+			require.Exactly(tt.want, ipamAddResult.ipv4Result)
+			require.Exactly(tt.want1, ipamAddResult.ipv6Result)
 		})
 	}
 }

cni/network/invoker_cns.go

@@ -3,7 +3,6 @@ package network
 import (
 	"context"
 	"encoding/json"
-	"errors"
 	"fmt"
 	"net"
 
@@ -16,15 +15,13 @@ import (
 	cniSkel "github.com/containernetworking/cni/pkg/skel"
 	cniTypes "github.com/containernetworking/cni/pkg/types"
 	cniTypesCurr "github.com/containernetworking/cni/pkg/types/current"
+	"github.com/pkg/errors"
 )
 
 var (
 	errEmtpyHostSubnetPrefix = errors.New("empty host subnet prefix not allowed")
 	errEmptyCNIArgs          = errors.New("empty CNI cmd args not allowed")
-)
-
-const (
-	cnsPort = 10090
+	errInvalidArgs           = errors.New("invalid arg(s)")
 )
 
 type CNSIPAMInvoker struct {
@@ -52,11 +49,7 @@ func NewCNSInvoker(podName, namespace string, cnsClient cnsclient) *CNSIPAMInvok
 }
 
 // Add uses the requestipconfig API in cns, and returns ipv4 and a nil ipv6 as CNS doesn't support IPv6 yet
-func (invoker *CNSIPAMInvoker) Add( //nolint don't consider unnamedResult
-	_ *cni.NetworkConfig,
-	args *cniSkel.CmdArgs,
-	hostSubnetPrefix *net.IPNet,
-	options map[string]interface{}) (*cniTypesCurr.Result, *cniTypesCurr.Result, error) {
+func (invoker *CNSIPAMInvoker) Add(addConfig IPAMAddConfig) (IPAMAddResult, error) {
 	// Parse Pod arguments.
 	podInfo := cns.KubernetesPodInfo{
 		PodName:      invoker.podName,
@@ -66,24 +59,24 @@ func (invoker *CNSIPAMInvoker) Add( //nolint don't consider unnamedResult
 	log.Printf(podInfo.PodName)
 	orchestratorContext, err := json.Marshal(podInfo)
 	if err != nil {
-		return nil, nil, fmt.Errorf("Failed to unmarshal orchestrator context during add: %w", err)
+		return IPAMAddResult{}, errors.Wrap(err, "Failed to unmarshal orchestrator context during add: %w")
 	}
 
-	if args == nil {
-		return nil, nil, errEmptyCNIArgs
+	if addConfig.args == nil {
+		return IPAMAddResult{}, errEmptyCNIArgs
 	}
 
 	ipconfig := cns.IPConfigRequest{
 		OrchestratorContext: orchestratorContext,
-		PodInterfaceID:      GetEndpointID(args),
-		InfraContainerID:    args.ContainerID,
+		PodInterfaceID:      GetEndpointID(addConfig.args),
+		InfraContainerID:    addConfig.args.ContainerID,
 	}
 
 	log.Printf("Requesting IP for pod %+v using ipconfig %+v", podInfo, ipconfig)
 	response, err := invoker.cnsClient.RequestIPAddress(context.TODO(), ipconfig)
 	if err != nil {
 		log.Printf("Failed to get IP address from CNS with error %v, response: %v", err, response)
-		return nil, nil, err
+		return IPAMAddResult{}, errors.Wrap(err, "Failed to get IP address from CNS with error: %w")
 	}
 
 	info := IPv4ResultInfo{
@@ -97,19 +90,19 @@ func (invoker *CNSIPAMInvoker) Add( //nolint don't consider unnamedResult
 	}
 
 	// set the NC Primary IP in options
-	options[network.SNATIPKey] = info.ncPrimaryIP
+	addConfig.options[network.SNATIPKey] = info.ncPrimaryIP
 
 	log.Printf("[cni-invoker-cns] Received info %+v for pod %v", info, podInfo)
 
 	ncgw := net.ParseIP(info.ncGatewayIPAddress)
 	if ncgw == nil {
-		return nil, nil, fmt.Errorf("Gateway address %v from response is invalid", info.ncGatewayIPAddress)
+		return IPAMAddResult{}, errors.Wrap(errInvalidArgs, "%w: Gateway address "+info.ncGatewayIPAddress+" from response is invalid")
 	}
 
 	// set result ipconfigArgument from CNS Response Body
 	ip, ncipnet, err := net.ParseCIDR(info.podIPAddress + "/" + fmt.Sprint(info.ncSubnetPrefix))
 	if ip == nil {
-		return nil, nil, fmt.Errorf("Unable to parse IP from response: %v with err %v", info.podIPAddress, err)
+		return IPAMAddResult{}, errors.Wrap(err, "Unable to parse IP from response: "+info.podIPAddress+" with err %w")
 	}
 
 	// construct ipnet for result
@@ -118,7 +111,8 @@ func (invoker *CNSIPAMInvoker) Add( //nolint don't consider unnamedResult
 		Mask: ncipnet.Mask,
 	}
 
-	result := &cniTypesCurr.Result{
+	addResult := IPAMAddResult{}
+	addResult.ipv4Result = &cniTypesCurr.Result{
 		IPs: []*cniTypesCurr.IPConfig{
 			{
 				Version: "4",
@@ -135,13 +129,12 @@ func (invoker *CNSIPAMInvoker) Add( //nolint don't consider unnamedResult
 	}
 
 	// set subnet prefix for host vm
-	err = setHostOptions(hostSubnetPrefix, ncipnet, options, &info)
+	err = setHostOptions(&addResult.hostSubnetPrefix, ncipnet, addConfig.options, &info)
 	if err != nil {
-		return nil, nil, err
+		return IPAMAddResult{}, err
 	}
 
-	// first result is ipv4, second is ipv6, SWIFT doesn't currently support IPv6
-	return result, nil, nil
+	return addResult, nil
 }
 
 func setHostOptions(hostSubnetPrefix, ncSubnetPrefix *net.IPNet, options map[string]interface{}, info *IPv4ResultInfo) error {
@@ -196,7 +189,7 @@ func setHostOptions(hostSubnetPrefix, ncSubnetPrefix *net.IPNet, options map[str
 }
 
 // Delete calls into the releaseipconfiguration API in CNS
-func (invoker *CNSIPAMInvoker) Delete(address *net.IPNet, _ *cni.NetworkConfig, args *cniSkel.CmdArgs, _ map[string]interface{}) error {
+func (invoker *CNSIPAMInvoker) Delete(address *net.IPNet, nwCfg *cni.NetworkConfig, args *cniSkel.CmdArgs, _ map[string]interface{}) error {
 	// Parse Pod arguments.
 	podInfo := cns.KubernetesPodInfo{
 		PodName:      invoker.podName,
@@ -225,7 +218,7 @@ func (invoker *CNSIPAMInvoker) Delete(address *net.IPNet, _ *cni.NetworkConfig,
 	}
 
 	if err := invoker.cnsClient.ReleaseIPAddress(context.TODO(), req); err != nil {
-		return fmt.Errorf("failed to release IP %v with err %w", address, err)
+		return errors.Wrap(err, fmt.Sprintf("failed to release IP %v with err ", address)+"%w")
 	}
 
 	return nil

cni/network/invoker_cns_test.go

@@ -85,7 +85,7 @@ func TestCNSIPAMInvoker_Add(t *testing.T) {
 				},
 			},
 			args: args{
-				nwCfg: nil,
+				nwCfg: &cni.NetworkConfig{},
 				args: &cniSkel.CmdArgs{
 					ContainerID: "testcontainerid",
 					Netns:       "testnetns",
@@ -137,15 +137,15 @@ func TestCNSIPAMInvoker_Add(t *testing.T) {
 				podNamespace: tt.fields.podNamespace,
 				cnsClient:    tt.fields.cnsClient,
 			}
-			got, got1, err := invoker.Add(tt.args.nwCfg, tt.args.args, tt.args.hostSubnetPrefix, tt.args.options)
+			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)
 			}
 
-			require.Equalf(tt.want, got, "incorrect ipv4 response")
-			require.Equalf(tt.want1, got1, "incorrect ipv6 response")
+			require.Equalf(tt.want, ipamAddResult.ipv4Result, "incorrect ipv4 response")
+			require.Equalf(tt.want1, ipamAddResult.ipv6Result, "incorrect ipv6 response")
 		})
 	}
 }
@@ -230,7 +230,7 @@ func Test_setHostOptions(t *testing.T) {
 		hostSubnetPrefix *net.IPNet
 		ncSubnetPrefix   *net.IPNet
 		options          map[string]interface{}
-		info             *IPv4ResultInfo
+		info             IPv4ResultInfo
 	}
 	tests := []struct {
 		name        string
@@ -244,7 +244,7 @@ func Test_setHostOptions(t *testing.T) {
 				hostSubnetPrefix: getCIDRNotationForAddress("10.0.1.0/24"),
 				ncSubnetPrefix:   getCIDRNotationForAddress("10.0.1.0/24"),
 				options:          map[string]interface{}{},
-				info: &IPv4ResultInfo{
+				info: IPv4ResultInfo{
 					podIPAddress:       "10.0.1.10",
 					ncSubnetPrefix:     24,
 					ncPrimaryIP:        "10.0.1.20",
@@ -286,7 +286,7 @@ func Test_setHostOptions(t *testing.T) {
 		{
 			name: "test error on bad host subnet",
 			args: args{
-				info: &IPv4ResultInfo{
+				info: IPv4ResultInfo{
 					hostSubnet: "",
 				},
 			},
@@ -295,7 +295,7 @@ func Test_setHostOptions(t *testing.T) {
 		{
 			name: "test error on nil hostsubnetprefix",
 			args: args{
-				info: &IPv4ResultInfo{
+				info: IPv4ResultInfo{
 					hostSubnet: "10.0.0.0/24",
 				},
 			},
@@ -305,7 +305,7 @@ func Test_setHostOptions(t *testing.T) {
 	for _, tt := range tests {
 		tt := tt
 		t.Run(tt.name, func(t *testing.T) {
-			err := setHostOptions(tt.args.hostSubnetPrefix, tt.args.ncSubnetPrefix, tt.args.options, tt.args.info)
+			err := setHostOptions(tt.args.hostSubnetPrefix, tt.args.ncSubnetPrefix, tt.args.options, &tt.args.info)
 			if tt.wantErr {
 				require.Error(err)
 				return

cni/network/invoker_mock.go

@@ -38,14 +38,12 @@ func NewMockIpamInvoker(ipv6, v4Fail, v6Fail bool) *MockIpamInvoker {
 	}
 }
 
-func (invoker *MockIpamInvoker) Add(nwCfg *cni.NetworkConfig, _ *skel.CmdArgs, subnetPrefix *net.IPNet, options map[string]interface{}) (v4, v6 *current.Result, err error) {
-	var resultV6 *current.Result
-
+func (invoker *MockIpamInvoker) Add(opt IPAMAddConfig) (ipamAddResult IPAMAddResult, err error) {
 	if invoker.v4Fail {
-		return nil, nil, errV4
+		return ipamAddResult, errV4
 	}
 
-	result := &current.Result{}
+	ipamAddResult.hostSubnetPrefix = net.IPNet{}
 
 	ipv4Str := "10.240.0.5"
 	if _, ok := invoker.ipMap["10.240.0.5/24"]; ok {
@@ -56,14 +54,14 @@ func (invoker *MockIpamInvoker) Add(nwCfg *cni.NetworkConfig, _ *skel.CmdArgs, s
 	ipnet := net.IPNet{IP: ip, Mask: net.CIDRMask(subnetBits, ipv4Bits)}
 	gwIP := net.ParseIP("10.240.0.1")
 	ipConfig := &current.IPConfig{Address: ipnet, Gateway: gwIP, Version: "4"}
-	result.IPs = append(result.IPs, ipConfig)
+	ipamAddResult.ipv4Result = &current.Result{}
+	ipamAddResult.ipv4Result.IPs = append(ipamAddResult.ipv4Result.IPs, ipConfig)
 	invoker.ipMap[ipnet.String()] = true
 	if invoker.v6Fail {
-		return result, nil, errV6
+		return ipamAddResult, errV6
 	}
 
 	if invoker.isIPv6 {
-		resultV6 = &current.Result{}
 		ipv6Str := "fc00::2"
 		if _, ok := invoker.ipMap["fc00::2/128"]; ok {
 			ipv6Str = "fc00::3"
@@ -73,11 +71,12 @@ func (invoker *MockIpamInvoker) Add(nwCfg *cni.NetworkConfig, _ *skel.CmdArgs, s
 		ipnet := net.IPNet{IP: ip, Mask: net.CIDRMask(subnetv6Bits, ipv6Bits)}
 		gwIP := net.ParseIP("fc00::1")
 		ipConfig := &current.IPConfig{Address: ipnet, Gateway: gwIP, Version: "6"}
-		resultV6.IPs = append(resultV6.IPs, ipConfig)
+		ipamAddResult.ipv6Result = &current.Result{}
+		ipamAddResult.ipv6Result.IPs = append(ipamAddResult.ipv6Result.IPs, ipConfig)
 		invoker.ipMap[ipnet.String()] = true
 	}
 
-	return result, resultV6, nil
+	return ipamAddResult, nil
 }
 
 func (invoker *MockIpamInvoker) Delete(address *net.IPNet, nwCfg *cni.NetworkConfig, _ *skel.CmdArgs, options map[string]interface{}) error {