Merge pull request #19 from eyakubovich/err-handling

Propagate json error object to the caller

pkg/plugin/ipam.go

@@ -41,6 +41,22 @@ func Find(plugin string) string {
 	return ""
 }
 
+func pluginErr(err error, output []byte) error {
+	if _, ok := err.(*exec.ExitError); ok {
+		emsg := Error{}
+		if perr := json.Unmarshal(output, &emsg); perr != nil {
+			return fmt.Errorf("netplugin failed but error parsing its diagnostic message %q: %v", string(output), perr)
+		}
+		details := ""
+		if emsg.Details != "" {
+			details = fmt.Sprintf("; %v", emsg.Details)
+		}
+		return fmt.Errorf("%v%v", emsg.Msg, details)
+	}
+
+	return err
+}
+
 // ExecAdd executes IPAM plugin, assuming CNI_COMMAND == ADD.
 // Parses and returns resulting IPConfig
 func ExecAdd(plugin string, netconf []byte) (*Result, error) {
@@ -63,7 +79,7 @@ func ExecAdd(plugin string, netconf []byte) (*Result, error) {
 		Stderr: os.Stderr,
 	}
 	if err := c.Run(); err != nil {
-		return nil, err
+		return nil, pluginErr(err, stdout.Bytes())
 	}
 
 	res := &Result{}
@@ -82,13 +98,19 @@ func ExecDel(plugin string, netconf []byte) error {
 		return fmt.Errorf("could not find %q plugin", plugin)
 	}
 
+	stdout := &bytes.Buffer{}
+
 	c := exec.Cmd{
 		Path:   pluginPath,
 		Args:   []string{pluginPath},
 		Stdin:  bytes.NewBuffer(netconf),
+		Stdout: stdout,
 		Stderr: os.Stderr,
 	}
-	return c.Run()
+	if err := c.Run(); err != nil {
+		return pluginErr(err, stdout.Bytes())
+	}
+	return nil
 }
 
 // ConfigureIface takes the result of IPAM plugin and
@@ -124,22 +146,3 @@ func ConfigureIface(ifName string, res *Result) error {
 
 	return nil
 }
-
-// PrintResult writes out prettified Result JSON to stdout
-func PrintResult(res *Result) error {
-	return prettyPrint(res)
-}
-
-// PrintError writes out prettified Error JSON to stdout
-func PrintError(err *Error) error {
-	return prettyPrint(err)
-}
-
-func prettyPrint(obj interface{}) error {
-	data, err := json.MarshalIndent(obj, "", "    ")
-	if err != nil {
-		return err
-	}
-	_, err = os.Stdout.Write(data)
-	return err
-}

pkg/plugin/types.go

@@ -17,6 +17,7 @@ package plugin
 import (
 	"encoding/json"
 	"net"
+	"os"
 
 	"github.com/appc/cni/pkg/ip"
 )
@@ -36,6 +37,10 @@ type Result struct {
 	IP6 *IPConfig `json:"ip6,omitempty"`
 }
 
+func (r *Result) Print() error {
+	return prettyPrint(r)
+}
+
 // IPConfig contains values necessary to configure an interface
 type IPConfig struct {
 	IP      net.IPNet
@@ -54,6 +59,14 @@ type Error struct {
 	Details string `json:"details,omitempty"`
 }
 
+func (e *Error) Error() string {
+	return e.Msg
+}
+
+func (e *Error) Print() error {
+	return prettyPrint(e)
+}
+
 // net.IPNet is not JSON (un)marshallable so this duality is needed
 // for our custom ip.IPNet type
 
@@ -110,3 +123,12 @@ func (r *Route) MarshalJSON() ([]byte, error) {
 
 	return json.Marshal(rt)
 }
+
+func prettyPrint(obj interface{}) error {
+	data, err := json.MarshalIndent(obj, "", "    ")
+	if err != nil {
+		return err
+	}
+	_, err = os.Stdout.Write(data)
+	return err
+}

pkg/skel/skel.go

@@ -64,12 +64,12 @@ func PluginMain(cmdAdd, cmdDel func(_ *CmdArgs) error) {
 	}
 
 	if argsMissing {
-		die("required env variables missing")
+		dieMsg("required env variables missing")
 	}
 
 	stdinData, err := ioutil.ReadAll(os.Stdin)
 	if err != nil {
-		die("error reading from stdin: %v", err)
+		dieMsg("error reading from stdin: %v", err)
 	}
 
 	cmdArgs := &CmdArgs{
@@ -89,18 +89,29 @@ func PluginMain(cmdAdd, cmdDel func(_ *CmdArgs) error) {
 		err = cmdDel(cmdArgs)
 
 	default:
-		die("unknown CNI_COMMAND: %v", cmd)
+		dieMsg("unknown CNI_COMMAND: %v", cmd)
 	}
 
 	if err != nil {
-		die(err.Error())
+		if e, ok := err.(*plugin.Error); ok {
+			// don't wrap Error in Error
+			dieErr(e)
+		}
+		dieMsg(err.Error())
 	}
 }
 
-func die(f string, args ...interface{}) {
+func dieMsg(f string, args ...interface{}) {
-	plugin.PrintError(&plugin.Error{
+	e := &plugin.Error{
 		Code: 100,
 		Msg:  fmt.Sprintf(f, args...),
-	})
+	}
+	dieErr(e)
+}
+
+func dieErr(e *plugin.Error) {
+	if err := e.Print(); err != nil {
+		log.Print("Error writing error JSON to stdout: ", err)
+	}
 	os.Exit(1)
 }

plugins/ipam/dhcp/main.go

@@ -45,7 +45,7 @@ func cmdAdd(args *skel.CmdArgs) error {
 		return fmt.Errorf("error calling DHCP.Add: %v", err)
 	}
 
-	return plugin.PrintResult(result)
+	return result.Print()
 }
 
 func cmdDel(args *skel.CmdArgs) error {

plugins/ipam/host-local/main.go

@@ -59,9 +59,10 @@ func cmdAdd(args *skel.CmdArgs) error {
 		return err
 	}
 
-	return plugin.PrintResult(&plugin.Result{
+	r := &plugin.Result{
 		IP4: ipConf,
-	})
+	}
+	return r.Print()
 }
 
 func cmdDel(args *skel.CmdArgs) error {

plugins/main/bridge/bridge.go

@@ -221,7 +221,7 @@ func cmdAdd(args *skel.CmdArgs) error {
 		}
 	}
 
-	return plugin.PrintResult(result)
+	return result.Print()
 }
 
 func cmdDel(args *skel.CmdArgs) error {

plugins/main/ipvlan/ipvlan.go

@@ -145,7 +145,7 @@ func cmdAdd(args *skel.CmdArgs) error {
 		}
 	}
 
-	return plugin.PrintResult(result)
+	return result.Print()
 }
 
 func cmdDel(args *skel.CmdArgs) error {

plugins/main/macvlan/macvlan.go

@@ -149,7 +149,7 @@ func cmdAdd(args *skel.CmdArgs) error {
 		}
 	}
 
-	return plugin.PrintResult(result)
+	return result.Print()
 }
 
 func cmdDel(args *skel.CmdArgs) error {

plugins/main/veth/veth.go

@@ -121,7 +121,7 @@ func cmdAdd(args *skel.CmdArgs) error {
 		}
 	}
 
-	return plugin.PrintResult(result)
+	return result.Print()
 }
 
 func cmdDel(args *skel.CmdArgs) error {

scripts/exec-plugins.sh

@@ -16,9 +16,14 @@ function exec_plugins() {
 		plugin=$(jq -r '.type' <$netconf)
 		export CNI_IFNAME=$(printf eth%d $i)
 
-		$plugin <$netconf >/dev/null
+		res=$($plugin <$netconf)
 		if [ $? -ne 0 ]; then
-			echo "${name} : error executing $CNI_COMMAND"
+			errmsg=$(echo $res | jq -r '.msg')
+			if [ -z "$errmsg" ]; then
+				errmsg=$res
+			fi
+
+			echo "${name} : error executing $CNI_COMMAND: $errmsg"
 			exit 1
 		fi