From 290717d6febb3a26a6a939d929e37c454c383ec6 Mon Sep 17 00:00:00 2001 From: Zachary Gershman Date: Wed, 10 Feb 2016 13:42:10 -0800 Subject: [PATCH] Better error message when plugin cannot be found --- libcni/api.go | 12 +++++-- pkg/invoke/exec.go | 5 --- pkg/invoke/find.go | 34 +++++++++++------- pkg/invoke/find_test.go | 64 +++++++++++++++++++++++++++++++++ pkg/invoke/invoke_suite_test.go | 13 +++++++ pkg/ipam/ipam.go | 21 +++++++++-- 6 files changed, 128 insertions(+), 21 deletions(-) create mode 100644 pkg/invoke/find_test.go create mode 100644 pkg/invoke/invoke_suite_test.go diff --git a/libcni/api.go b/libcni/api.go index b308ef84..77ca5d22 100644 --- a/libcni/api.go +++ b/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)) } diff --git a/pkg/invoke/exec.go b/pkg/invoke/exec.go index 90564970..337bfcb8 100644 --- a/pkg/invoke/exec.go +++ b/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{ diff --git a/pkg/invoke/find.go b/pkg/invoke/find.go index 82f9a9b7..3b037907 100644 --- a/pkg/invoke/find.go +++ b/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 } diff --git a/pkg/invoke/find_test.go b/pkg/invoke/find_test.go new file mode 100644 index 00000000..00baa313 --- /dev/null +++ b/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))) + }) + }) + }) +}) diff --git a/pkg/invoke/invoke_suite_test.go b/pkg/invoke/invoke_suite_test.go new file mode 100644 index 00000000..e570c964 --- /dev/null +++ b/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") +} diff --git a/pkg/ipam/ipam.go b/pkg/ipam/ipam.go index 4920838a..1d512ecd 100644 --- a/pkg/ipam/ipam.go +++ b/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