Propagate json error object to the caller

When plugin errors out, it prints out a JSON object to stdout
describing the failure. This object needs to be propagated out
through the plugins and to the container runtime. This change
also adds Print method to both the result and error structs
for easy serialization to stdout.

diff --git a/pkg/plugin/ipam.go b/pkg/plugin/ipam.go
index 4124d0f..f5a5033 100644 (file)
--- a/pkg/plugin/ipam.go
+++ b/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
-}

diff --git a/pkg/plugin/types.go b/pkg/plugin/types.go
index 9db0f10..d5952dd 100644 (file)
--- a/pkg/plugin/types.go
+++ b/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
+}

diff --git a/pkg/skel/skel.go b/pkg/skel/skel.go
index 4f159ad..bf79b91 100644 (file)
--- a/pkg/skel/skel.go
+++ b/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{}) {
-       plugin.PrintError(&plugin.Error{
+func dieMsg(f string, args ...interface{}) {
+       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)
 }

diff --git a/plugins/ipam/dhcp/main.go b/plugins/ipam/dhcp/main.go
index ccfb198..2bc6625 100644 (file)
--- a/plugins/ipam/dhcp/main.go
+++ b/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 {

diff --git a/plugins/ipam/host-local/main.go b/plugins/ipam/host-local/main.go
index 0df177b..799b114 100644 (file)
--- a/plugins/ipam/host-local/main.go
+++ b/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 {

diff --git a/plugins/main/bridge/bridge.go b/plugins/main/bridge/bridge.go
index c30f2ca..5a311ec 100644 (file)
--- a/plugins/main/bridge/bridge.go
+++ b/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 {

diff --git a/plugins/main/ipvlan/ipvlan.go b/plugins/main/ipvlan/ipvlan.go
index 0f3656c..9ca1ca1 100644 (file)
--- a/plugins/main/ipvlan/ipvlan.go
+++ b/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 {

diff --git a/plugins/main/macvlan/macvlan.go b/plugins/main/macvlan/macvlan.go
index f4b432d..4d3a94a 100644 (file)
--- a/plugins/main/macvlan/macvlan.go
+++ b/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 {

diff --git a/plugins/main/veth/veth.go b/plugins/main/veth/veth.go
index c736060..7cfdc17 100644 (file)
--- a/plugins/main/veth/veth.go
+++ b/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 {

diff --git a/scripts/exec-plugins.sh b/scripts/exec-plugins.sh
index 64ce1bc..05d0cd4 100755 (executable)
--- a/scripts/exec-plugins.sh
+++ b/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