From 759e3cbf9bd81f7634286a27cc90b89069346013 Mon Sep 17 00:00:00 2001 From: Bruce Ma Date: Wed, 3 Jul 2019 21:44:02 +0800 Subject: [PATCH] skel: clean up errors in skel, including unified error code and inherited printer Signed-off-by: Bruce Ma --- pkg/skel/skel.go | 41 ++++++++++++++---------------------- pkg/skel/skel_test.go | 49 +++++++++++++++---------------------------- 2 files changed, 33 insertions(+), 57 deletions(-) diff --git a/pkg/skel/skel.go b/pkg/skel/skel.go index af56b8a1c..512b59a7c 100644 --- a/pkg/skel/skel.go +++ b/pkg/skel/skel.go @@ -161,10 +161,11 @@ func (t *dispatcher) getCmdArgsFromEnv() (string, *CmdArgs, error) { return cmd, cmdArgs, nil } -func createTypedError(f string, args ...interface{}) *types.Error { +func createTypedError(printer io.Writer, code uint, f string, args ...interface{}) *types.Error { return &types.Error{ - Code: 100, - Msg: fmt.Sprintf(f, args...), + Printer: printer, + Code: code, + Msg: fmt.Sprintf(f, args...), } } @@ -175,11 +176,7 @@ func (t *dispatcher) checkVersionAndCall(cmdArgs *CmdArgs, pluginVersionInfo ver } verErr := t.VersionReconciler.Check(configVersion, pluginVersionInfo) if verErr != nil { - return &types.Error{ - Code: types.ErrIncompatibleCNIVersion, - Msg: "incompatible CNI versions", - Details: verErr.Details(), - } + return createTypedError(t.Stderr, types.ErrIncompatibleCNIVersion, "incompatible CNI versions : %s", verErr.Details()) } return toCall(cmdArgs) @@ -203,16 +200,16 @@ func (t *dispatcher) pluginMain(cmdAdd, cmdCheck, cmdDel func(_ *CmdArgs) error, if err != nil { // Print the about string to stderr when no command is set if _, ok := err.(missingEnvError); ok && t.Getenv("CNI_COMMAND") == "" && about != "" { - fmt.Fprintln(t.Stderr, about) + _, _ = fmt.Fprintln(t.Stderr, about) return nil } - return createTypedError(err.Error()) + return createTypedError(t.Stderr, types.ErrUnknown, err.Error()) } if cmd != "VERSION" { err = validateConfig(cmdArgs.StdinData) if err != nil { - return createTypedError(err.Error()) + return createTypedError(t.Stderr, types.ErrUnknown, err.Error()) } } @@ -222,37 +219,31 @@ func (t *dispatcher) pluginMain(cmdAdd, cmdCheck, cmdDel func(_ *CmdArgs) error, case "CHECK": configVersion, err := t.ConfVersionDecoder.Decode(cmdArgs.StdinData) if err != nil { - return createTypedError(err.Error()) + return createTypedError(t.Stderr, types.ErrUnknown, err.Error()) } if gtet, err := version.GreaterThanOrEqualTo(configVersion, "0.4.0"); err != nil { - return createTypedError(err.Error()) + return createTypedError(t.Stderr, types.ErrUnknown, err.Error()) } else if !gtet { - return &types.Error{ - Code: types.ErrIncompatibleCNIVersion, - Msg: "config version does not allow CHECK", - } + return createTypedError(t.Stderr, types.ErrIncompatibleCNIVersion, "config version does not allow CHECK") } for _, pluginVersion := range versionInfo.SupportedVersions() { gtet, err := version.GreaterThanOrEqualTo(pluginVersion, configVersion) if err != nil { - return createTypedError(err.Error()) + return createTypedError(t.Stderr, types.ErrUnknown, err.Error()) } else if gtet { if err := t.checkVersionAndCall(cmdArgs, versionInfo, cmdCheck); err != nil { - return createTypedError(err.Error()) + return createTypedError(t.Stderr, types.ErrUnknown, err.Error()) } return nil } } - return &types.Error{ - Code: types.ErrIncompatibleCNIVersion, - Msg: "plugin version does not allow CHECK", - } + return createTypedError(t.Stderr, types.ErrIncompatibleCNIVersion, "plugin version does not allow CHECK") case "DEL": err = t.checkVersionAndCall(cmdArgs, versionInfo, cmdDel) case "VERSION": err = versionInfo.Encode(t.Stdout) default: - return createTypedError("unknown CNI_COMMAND: %v", cmd) + return createTypedError(t.Stderr, types.ErrUnknown, "unknown CNI_COMMAND: %v", cmd) } if err != nil { @@ -260,7 +251,7 @@ func (t *dispatcher) pluginMain(cmdAdd, cmdCheck, cmdDel func(_ *CmdArgs) error, // don't wrap Error in Error return e } - return createTypedError(err.Error()) + return createTypedError(t.Stderr, types.ErrUnknown, err.Error()) } return nil } diff --git a/pkg/skel/skel_test.go b/pkg/skel/skel_test.go index 95e1b309e..3a3d78b5f 100644 --- a/pkg/skel/skel_test.go +++ b/pkg/skel/skel_test.go @@ -94,10 +94,8 @@ var _ = Describe("dispatching to the correct callback", func() { err := dispatch.pluginMain(cmdAdd.Func, cmdCheck.Func, cmdDel.Func, versionInfo, "") if isRequired { - Expect(err).To(Equal(&types.Error{ - Code: 100, - Msg: "required env variables [" + envVar + "] missing", - })) + Expect(err.Code).To(Equal(types.ErrUnknown)) + Expect(err.Msg).To(Equal("required env variables [" + envVar + "] missing")) } else { Expect(err).NotTo(HaveOccurred()) } @@ -142,10 +140,8 @@ var _ = Describe("dispatching to the correct callback", func() { err := dispatch.pluginMain(cmdAdd.Func, cmdCheck.Func, cmdDel.Func, versionInfo, "") Expect(err).To(HaveOccurred()) - Expect(err).To(Equal(&types.Error{ - Code: 100, - Msg: "required env variables [CNI_NETNS,CNI_IFNAME,CNI_PATH] missing", - })) + Expect(err.Code).To(Equal(types.ErrUnknown)) + Expect(err.Msg).To(Equal("required env variables [CNI_NETNS,CNI_IFNAME,CNI_PATH] missing")) }) }) @@ -178,8 +174,7 @@ var _ = Describe("dispatching to the correct callback", func() { It("immediately returns a useful error", func() { err := dispatch.pluginMain(cmdAdd.Func, cmdCheck.Func, cmdDel.Func, versionInfo, "") Expect(err.Code).To(Equal(types.ErrIncompatibleCNIVersion)) // see https://github.com/containernetworking/cni/blob/master/SPEC.md#well-known-error-codes - Expect(err.Msg).To(Equal("incompatible CNI versions")) - Expect(err.Details).To(Equal(`config is "0.1.0", plugin supports ["4.3.2"]`)) + Expect(err.Msg).To(Equal(`incompatible CNI versions : config is "0.1.0", plugin supports ["4.3.2"]`)) }) It("does not call either callback", func() { @@ -235,10 +230,8 @@ var _ = Describe("dispatching to the correct callback", func() { err := dispatch.pluginMain(cmdAdd.Func, cmdCheck.Func, cmdDel.Func, versionInfo, "") Expect(err).To(HaveOccurred()) - Expect(err).To(Equal(&types.Error{ - Code: 100, - Msg: "required env variables [CNI_NETNS,CNI_IFNAME,CNI_PATH] missing", - })) + Expect(err.Code).To(Equal(types.ErrUnknown)) + Expect(err.Msg).To(Equal("required env variables [CNI_NETNS,CNI_IFNAME,CNI_PATH] missing")) }) }) @@ -272,7 +265,7 @@ var _ = Describe("dispatching to the correct callback", func() { dispatch.Stdin = strings.NewReader(`{ "cniVersion": "adsfsadf", "some": "config" }`) versionInfo = version.PluginSupports("0.1.0", "0.2.0", "0.3.0") err := dispatch.pluginMain(cmdAdd.Func, cmdCheck.Func, cmdDel.Func, versionInfo, "") - Expect(err.Code).To(Equal(uint(100))) + Expect(err.Code).To(Equal(uint(0))) Expect(cmdAdd.CallCount).To(Equal(0)) Expect(cmdCheck.CallCount).To(Equal(0)) Expect(cmdDel.CallCount).To(Equal(0)) @@ -284,7 +277,7 @@ var _ = Describe("dispatching to the correct callback", func() { dispatch.Stdin = strings.NewReader(`{ "cniVersion": "0.4.0", "some": "config" }`) versionInfo = version.PluginSupports("0.1.0", "0.2.0", "adsfasdf") err := dispatch.pluginMain(cmdAdd.Func, cmdCheck.Func, cmdDel.Func, versionInfo, "") - Expect(err.Code).To(Equal(uint(100))) + Expect(err.Code).To(Equal(uint(0))) Expect(cmdAdd.CallCount).To(Equal(0)) Expect(cmdCheck.CallCount).To(Equal(0)) Expect(cmdDel.CallCount).To(Equal(0)) @@ -384,10 +377,8 @@ var _ = Describe("dispatching to the correct callback", func() { It("returns an error", func() { err := dispatch.pluginMain(cmdAdd.Func, cmdCheck.Func, cmdDel.Func, versionInfo, "") - Expect(err).To(Equal(&types.Error{ - Code: 100, - Msg: "unknown CNI_COMMAND: NOPE", - })) + Expect(err.Code).To(Equal(types.ErrUnknown)) + Expect(err.Msg).To(Equal("unknown CNI_COMMAND: NOPE")) }) It("prints the about string when the command is blank", func() { @@ -416,10 +407,8 @@ var _ = Describe("dispatching to the correct callback", func() { Expect(cmdAdd.CallCount).To(Equal(0)) Expect(cmdDel.CallCount).To(Equal(0)) - Expect(err).To(Equal(&types.Error{ - Code: 100, - Msg: "required env variables [CNI_COMMAND] missing", - })) + Expect(err.Code).To(Equal(types.ErrUnknown)) + Expect(err.Msg).To(Equal("required env variables [CNI_COMMAND] missing")) }) }) @@ -438,10 +427,8 @@ var _ = Describe("dispatching to the correct callback", func() { It("wraps and returns the error", func() { err := dispatch.pluginMain(cmdAdd.Func, cmdCheck.Func, cmdDel.Func, versionInfo, "") - Expect(err).To(Equal(&types.Error{ - Code: 100, - Msg: "error reading from stdin: banana", - })) + Expect(err.Code).To(Equal(types.ErrUnknown)) + Expect(err.Msg).To(Equal("error reading from stdin: banana")) }) }) @@ -472,10 +459,8 @@ var _ = Describe("dispatching to the correct callback", func() { It("wraps and returns the error", func() { err := dispatch.pluginMain(cmdAdd.Func, cmdCheck.Func, cmdDel.Func, versionInfo, "") - Expect(err).To(Equal(&types.Error{ - Code: 100, - Msg: "potato", - })) + Expect(err.Code).To(Equal(types.ErrUnknown)) + Expect(err.Msg).To(Equal("potato")) }) }) })