Fixed error paths to return CNI errors

2017-02-24 14:38:00 -08:00 · 2017-02-24 14:38:00 -08:00 · 863dd51b85
--- a/cni/ipam/ipam.go
+++ b/cni/ipam/ipam.go
@ -146,8 +146,7 @@ func (plugin *ipamPlugin) Add(args *cniSkel.CmdArgs) error {
 	// Parse network configuration from stdin.
 	nwCfg, err := plugin.Configure(args.StdinData)
 	if err != nil {
-		log.Printf("[cni-ipam] Failed to parse network configuration, err:%v.", err)
-		return nil
+		return plugin.Errorf("Failed to parse network configuration: %v", err)
 	}

 	var poolId string
@ -166,8 +165,7 @@ func (plugin *ipamPlugin) Add(args *cniSkel.CmdArgs) error {
 		// Allocate an address pool.
 		poolId, subnet, err = plugin.am.RequestPool(nwCfg.Ipam.AddrSpace, "", "", options, false)
 		if err != nil {
-			log.Printf("[cni-ipam] Failed to allocate pool, err:%v.", err)
-			return nil
+			return plugin.Errorf("Failed to allocate pool: %v.", err)
 		}

 		nwCfg.Ipam.Subnet = subnet
@ -177,7 +175,7 @@ func (plugin *ipamPlugin) Add(args *cniSkel.CmdArgs) error {
 	// Allocate an address for the endpoint.
 	address, err := plugin.am.RequestAddress(nwCfg.Ipam.AddrSpace, nwCfg.Ipam.Subnet, nwCfg.Ipam.Address, nil)
 	if err != nil {
-		log.Printf("[cni-ipam] Failed to allocate address, err:%v.", err)
+		err = plugin.Errorf("Failed to allocate address: %v", err)
 		goto Rollback
 	}

@ -186,12 +184,14 @@ func (plugin *ipamPlugin) Add(args *cniSkel.CmdArgs) error {
 	// Parse IP address.
 	ipv4Address, err = ipam.ConvertAddressToIPNet(address)
 	if err != nil {
+		err = plugin.Errorf("Failed to parse address: %v", err)
 		goto Rollback
 	}

 	// Query pool information for gateways and DNS servers.
 	apInfo, err = plugin.am.GetPoolInfo(nwCfg.Ipam.AddrSpace, nwCfg.Ipam.Subnet)
 	if err != nil {
+		err = plugin.Errorf("Failed to get pool information: %v", err)
 		goto Rollback
 	}

@ -217,6 +217,7 @@ func (plugin *ipamPlugin) Add(args *cniSkel.CmdArgs) error {
 	// Convert result to the requested CNI version.
 	result, err = resultImpl.GetAsVersion(nwCfg.CniVersion)
 	if err != nil {
+		err = plugin.Errorf("Failed to convert result: %v", err)
 		goto Rollback
 	}

@ -239,12 +240,12 @@ Rollback:

 	if address != "" {
 		log.Printf("[cni-ipam] Releasing address %v.", address)
-		err = plugin.am.ReleaseAddress(nwCfg.Ipam.AddrSpace, nwCfg.Ipam.Subnet, address)
+		plugin.am.ReleaseAddress(nwCfg.Ipam.AddrSpace, nwCfg.Ipam.Subnet, address)
 	}

 	if poolId != "" {
 		log.Printf("[cni-ipam] Releasing pool %v.", poolId)
-		err = plugin.am.ReleasePool(nwCfg.Ipam.AddrSpace, poolId)
+		plugin.am.ReleasePool(nwCfg.Ipam.AddrSpace, poolId)
 	}

 	return err
@ -258,8 +259,7 @@ func (plugin *ipamPlugin) Delete(args *cniSkel.CmdArgs) error {
 	// Parse network configuration from stdin.
 	nwCfg, err := plugin.Configure(args.StdinData)
 	if err != nil {
-		log.Printf("[cni-ipam] Failed to parse network configuration, err:%v.", err)
-		return nil
+		plugin.Errorf("Failed to parse network configuration: %v", err)
 	}

 	// If an address is specified, release that address. Otherwise, release the pool.
@ -267,15 +267,13 @@ func (plugin *ipamPlugin) Delete(args *cniSkel.CmdArgs) error {
 		// Release the address.
 		err := plugin.am.ReleaseAddress(nwCfg.Ipam.AddrSpace, nwCfg.Ipam.Subnet, nwCfg.Ipam.Address)
 		if err != nil {
-			log.Printf("[cni-ipam] Failed to release address, err:%v.", err)
-			return nil
+			plugin.Errorf("Failed to release address: %v", err)
 		}
 	} else {
 		// Release the pool.
 		err := plugin.am.ReleasePool(nwCfg.Ipam.AddrSpace, nwCfg.Ipam.Subnet)
 		if err != nil {
-			log.Printf("[cni-ipam] Failed to release pool, err:%v.", err)
-			return nil
+			plugin.Errorf("Failed to release pool: %v", err)
 		}
 	}

--- a/cni/network/network.go
+++ b/cni/network/network.go
@ -95,8 +95,7 @@ func (plugin *netPlugin) Add(args *cniSkel.CmdArgs) error {
 	// Parse network configuration from stdin.
 	nwCfg, err := cni.ParseNetworkConfig(args.StdinData)
 	if err != nil {
-		log.Printf("[cni-net] Failed to parse network configuration, err:%v.", err)
-		return nil
+		return plugin.Errorf("Failed to parse network configuration: %v.", err)
 	}

 	log.Printf("[cni-net] Read network configuration %+v.", nwCfg)
@ -116,8 +115,7 @@ func (plugin *netPlugin) Add(args *cniSkel.CmdArgs) error {
 		// Call into IPAM plugin to allocate an address pool for the network.
 		result, err = cniInvoke.DelegateAdd(nwCfg.Ipam.Type, nwCfg.Serialize())
 		if err != nil {
-			log.Printf("[cni-net] Failed to allocate pool, err:%v.", err)
-			return nil
+			return plugin.Errorf("Failed to allocate pool: %v", err)
 		}

 		resultImpl, err = cniTypesImpl.GetResult(result)
@ -131,8 +129,7 @@ func (plugin *netPlugin) Add(args *cniSkel.CmdArgs) error {
 		// Add the master as an external interface.
 		err = plugin.nm.AddExternalInterface(nwCfg.Master, subnet.String())
 		if err != nil {
-			log.Printf("[cni-net] Failed to add external interface, err:%v.", err)
-			return nil
+			return plugin.Errorf("Failed to add external interface: %v", err)
 		}

 		// Create the network.
@ -144,8 +141,7 @@ func (plugin *netPlugin) Add(args *cniSkel.CmdArgs) error {

 		err = plugin.nm.CreateNetwork(&nwInfo)
 		if err != nil {
-			log.Printf("[cni-net] Failed to create network, err:%v.", err)
-			return nil
+			return plugin.Errorf("Failed to create network: %v", err)
 		}

 		log.Printf("[cni-net] Created network %v with subnet %v.", networkId, subnet.String())
@ -157,8 +153,7 @@ func (plugin *netPlugin) Add(args *cniSkel.CmdArgs) error {
 		nwCfg.Ipam.Subnet = nwInfo.Subnets[0]
 		result, err = cniInvoke.DelegateAdd(nwCfg.Ipam.Type, nwCfg.Serialize())
 		if err != nil {
-			log.Printf("[cni-net] Failed to allocate address, err:%v.", err)
-			return nil
+			return plugin.Errorf("Failed to allocate address: %v", err)
 		}

 		resultImpl, err = cniTypesImpl.GetResult(result)
@ -186,14 +181,13 @@ func (plugin *netPlugin) Add(args *cniSkel.CmdArgs) error {
 	log.Printf("[cni-net] Creating endpoint %+v", epInfo)
 	err = plugin.nm.CreateEndpoint(networkId, epInfo)
 	if err != nil {
-		log.Printf("[cni-net] Failed to create endpoint, err:%v.", err)
-		return nil
+		return plugin.Errorf("Failed to create endpoint: %v", err)
 	}

 	// Convert result to the requested CNI version.
 	result, err = resultImpl.GetAsVersion(nwCfg.CniVersion)
 	if err != nil {
-		return err
+		return plugin.Error(err)
 	}

 	// Output the result to stdout.
@ -212,8 +206,7 @@ func (plugin *netPlugin) Delete(args *cniSkel.CmdArgs) error {
 	// Parse network configuration from stdin.
 	nwCfg, err := cni.ParseNetworkConfig(args.StdinData)
 	if err != nil {
-		log.Printf("[cni-net] Failed to parse network configuration, err:%v.", err)
-		return nil
+		return plugin.Errorf("Failed to parse network configuration: %v", err)
 	}

 	log.Printf("[cni-net] Read network configuration %+v.", nwCfg)
@ -225,22 +218,19 @@ func (plugin *netPlugin) Delete(args *cniSkel.CmdArgs) error {
 	// Query the network.
 	nwInfo, err := plugin.nm.GetNetworkInfo(networkId)
 	if err != nil {
-		log.Printf("[cni-net] Failed to query network, err:%v.", err)
-		return nil
+		return plugin.Errorf("Failed to query network: %v", err)
 	}

 	// Query the endpoint.
 	epInfo, err := plugin.nm.GetEndpointInfo(networkId, endpointId)
 	if err != nil {
-		log.Printf("[cni-net] Failed to query endpoint, err:%v.", err)
-		return nil
+		return plugin.Errorf("Failed to query endpoint: %v", err)
 	}

 	// Delete the endpoint.
 	err = plugin.nm.DeleteEndpoint(networkId, endpointId)
 	if err != nil {
-		log.Printf("[cni-net] Failed to delete endpoint, err:%v.", err)
-		return nil
+		return plugin.Errorf("Failed to delete endpoint: %v", err)
 	}

 	// Call into IPAM plugin to release the endpoint's addresses.
@ -249,8 +239,7 @@ func (plugin *netPlugin) Delete(args *cniSkel.CmdArgs) error {
 		nwCfg.Ipam.Address = address.IP.String()
 		err = cniInvoke.DelegateDel(nwCfg.Ipam.Type, nwCfg.Serialize())
 		if err != nil {
-			log.Printf("[cni-net] Failed to release address, err:%v.", err)
-			return nil
+			return plugin.Errorf("Failed to release address: %v", err)
 		}
 	}

--- a/cni/plugin.go
+++ b/cni/plugin.go
@ -4,12 +4,15 @@
 package cni

 import (
+	"fmt"
+
 	"github.com/Azure/azure-container-networking/common"
 	"github.com/Azure/azure-container-networking/log"
 	"github.com/Azure/azure-container-networking/platform"
 	"github.com/Azure/azure-container-networking/store"

 	cniSkel "github.com/containernetworking/cni/pkg/skel"
+	cniTypes "github.com/containernetworking/cni/pkg/types"
 	cniVers "github.com/containernetworking/cni/pkg/version"
 )

@ -92,3 +95,23 @@ func (plugin *Plugin) Execute(api PluginApi) error {

 	return nil
 }
+
+// Error creates and logs a structured CNI error.
+func (plugin *Plugin) Error(err error) *cniTypes.Error {
+	var cniErr *cniTypes.Error
+	var ok bool
+
+	// Wrap error if necessary.
+	if cniErr, ok = err.(*cniTypes.Error); !ok {
+		cniErr = &cniTypes.Error{Code: 100, Msg: err.Error()}
+	}
+
+	log.Printf("[%v] %+v.", plugin.Name, cniErr.Error())
+
+	return cniErr
+}
+
+// Errorf creates and logs a custom CNI error according to a format specifier.
+func (plugin *Plugin) Errorf(format string, args ...interface{}) *cniTypes.Error {
+	return plugin.Error(fmt.Errorf(format, args...))
+}