Merge pull request #120 from zachgersh/find-better-error

Better error messages when plugin is not found

libcni/api.go

@@ -43,12 +43,20 @@ type CNIConfig struct {
 }
 
 func (c *CNIConfig) AddNetwork(net *NetworkConfig, rt *RuntimeConf) (*types.Result, error) {
-	pluginPath := invoke.FindInPath(net.Network.Type, c.Path)
+	pluginPath, err := invoke.FindInPath(net.Network.Type, c.Path)
+	if err != nil {
+		return nil, err
+	}
+
 	return invoke.ExecPluginWithResult(pluginPath, net.Bytes, c.args("ADD", rt))
 }
 
 func (c *CNIConfig) DelNetwork(net *NetworkConfig, rt *RuntimeConf) error {
-	pluginPath := invoke.FindInPath(net.Network.Type, c.Path)
+	pluginPath, err := invoke.FindInPath(net.Network.Type, c.Path)
+	if err != nil {
+		return err
+	}
+
 	return invoke.ExecPluginWithoutResult(pluginPath, net.Bytes, c.args("DEL", rt))
 }
 

pkg/invoke/exec.go

@@ -20,7 +20,6 @@ import (
 	"fmt"
 	"os"
 	"os/exec"
-	"path/filepath"
 
 	"github.com/appc/cni/pkg/types"
 )
@@ -58,10 +57,6 @@ func ExecPluginWithoutResult(pluginPath string, netconf []byte, args CNIArgs) er
 }
 
 func execPlugin(pluginPath string, netconf []byte, args CNIArgs) ([]byte, error) {
-	if pluginPath == "" {
-		return nil, fmt.Errorf("could not find %q plugin", filepath.Base(pluginPath))
-	}
-
 	stdout := &bytes.Buffer{}
 
 	c := exec.Cmd{

pkg/invoke/find.go

@@ -15,23 +15,33 @@
 package invoke
 
 import (
+	"fmt"
 	"os"
 	"path/filepath"
-	"strings"
 )
 
-func FindInPath(plugin string, path []string) string {
-	for _, p := range path {
-		fullname := filepath.Join(p, plugin)
-		if fi, err := os.Stat(fullname); err == nil && fi.Mode().IsRegular() {
-			return fullname
+// FindInPath returns the full path of the plugin by searching in the provided path
+func FindInPath(plugin string, paths []string) (string, error) {
+	if plugin == "" {
+		return "", fmt.Errorf("no plugin name provided")
+	}
+
+	if len(paths) == 0 {
+		return "", fmt.Errorf("no paths provided")
+	}
+
+	var fullpath string
+	for _, path := range paths {
+		full := filepath.Join(path, plugin)
+		if fi, err := os.Stat(full); err == nil && fi.Mode().IsRegular() {
+			fullpath = full
+			break
 		}
 	}
-	return ""
-}
 
-// Find returns the full path of the plugin by searching in CNI_PATH
-func Find(plugin string) string {
-	paths := strings.Split(os.Getenv("CNI_PATH"), ":")
-	return FindInPath(plugin, paths)
+	if fullpath == "" {
+		return "", fmt.Errorf("failed to find plugin %q in path %s", plugin, paths)
+	}
+
+	return fullpath, nil
 }

pkg/invoke/find_test.go

@@ -0,0 +1,64 @@
+package invoke_test
+
+import (
+	"fmt"
+	"io/ioutil"
+	"path/filepath"
+
+	"github.com/appc/cni/pkg/invoke"
+	. "github.com/onsi/ginkgo"
+	. "github.com/onsi/gomega"
+)
+
+var _ = Describe("FindInPath", func() {
+	var (
+		multiplePaths  []string
+		pluginName     string
+		pluginDir      string
+		anotherTempDir string
+	)
+
+	BeforeEach(func() {
+		tempDir, err := ioutil.TempDir("", "cni-find")
+		Expect(err).NotTo(HaveOccurred())
+
+		plugin, err := ioutil.TempFile(tempDir, "a-cni-plugin")
+
+		anotherTempDir, err = ioutil.TempDir("", "nothing-here")
+
+		multiplePaths = []string{anotherTempDir, tempDir}
+		pluginDir, pluginName = filepath.Split(plugin.Name())
+	})
+
+	Context("when multiple paths are provided", func() {
+		It("returns only the path to the plugin", func() {
+			pluginPath, err := invoke.FindInPath(pluginName, multiplePaths)
+			Expect(err).NotTo(HaveOccurred())
+			Expect(pluginPath).To(Equal(filepath.Join(pluginDir, pluginName)))
+		})
+	})
+
+	Context("when an error occurs", func() {
+		Context("when no paths are provided", func() {
+			It("returns an error noting no paths were provided", func() {
+				_, err := invoke.FindInPath(pluginName, []string{})
+				Expect(err).To(MatchError("no paths provided"))
+			})
+		})
+
+		Context("when no plugin is provided", func() {
+			It("returns an error noting the plugin name wasn't found", func() {
+				_, err := invoke.FindInPath("", multiplePaths)
+				Expect(err).To(MatchError("no plugin name provided"))
+			})
+		})
+
+		Context("when the plugin cannot be found", func() {
+			It("returns an error noting the path", func() {
+				pathsWithNothing := []string{anotherTempDir}
+				_, err := invoke.FindInPath(pluginName, pathsWithNothing)
+				Expect(err).To(MatchError(fmt.Sprintf("failed to find plugin %q in path %s", pluginName, pathsWithNothing)))
+			})
+		})
+	})
+})

pkg/invoke/invoke_suite_test.go

@@ -0,0 +1,13 @@
+package invoke_test
+
+import (
+	. "github.com/onsi/ginkgo"
+	. "github.com/onsi/gomega"
+
+	"testing"
+)
+
+func TestInvoke(t *testing.T) {
+	RegisterFailHandler(Fail)
+	RunSpecs(t, "Invoke Suite")
+}

pkg/ipam/ipam.go

@@ -17,6 +17,7 @@ package ipam
 import (
 	"fmt"
 	"os"
+	"strings"
 
 	"github.com/appc/cni/pkg/invoke"
 	"github.com/appc/cni/pkg/ip"
@@ -29,14 +30,30 @@ func ExecAdd(plugin string, netconf []byte) (*types.Result, error) {
 	if os.Getenv("CNI_COMMAND") != "ADD" {
 		return nil, fmt.Errorf("CNI_COMMAND is not ADD")
 	}
-	return invoke.ExecPluginWithResult(invoke.Find(plugin), netconf, invoke.ArgsFromEnv())
+
+	paths := strings.Split(os.Getenv("CNI_PATH"), ":")
+
+	pluginPath, err := invoke.FindInPath(plugin, paths)
+	if err != nil {
+		return nil, err
+	}
+
+	return invoke.ExecPluginWithResult(pluginPath, netconf, invoke.ArgsFromEnv())
 }
 
 func ExecDel(plugin string, netconf []byte) error {
 	if os.Getenv("CNI_COMMAND") != "DEL" {
 		return fmt.Errorf("CNI_COMMAND is not DEL")
 	}
-	return invoke.ExecPluginWithoutResult(invoke.Find(plugin), netconf, invoke.ArgsFromEnv())
+
+	paths := strings.Split(os.Getenv("CNI_PATH"), ":")
+
+	pluginPath, err := invoke.FindInPath(plugin, paths)
+	if err != nil {
+		return err
+	}
+
+	return invoke.ExecPluginWithoutResult(pluginPath, netconf, invoke.ArgsFromEnv())
 }
 
 // ConfigureIface takes the result of IPAM plugin and