Skip to content

Commit

Permalink
skel: clean up errors in skel, including unified error code and inher…
Browse files Browse the repository at this point in the history 
…ited printer

Signed-off-by: Bruce Ma <brucema19901024@gmail.com>
  • Loading branch information
@mars1024
mars1024 committed Jul 3, 2019
1 parent dd7b3c0 commit 759e3cb
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 57 deletions.
41 changes: 16 additions & 25 deletions pkg/skel/skel.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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...),
}
}

Expand All @@ -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)
Expand All @@ -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())
}
}

Expand All @@ -222,45 +219,39 @@ 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 {
if e, ok := err.(*types.Error); ok {
// don't wrap Error in Error
return e
}
return createTypedError(err.Error())
return createTypedError(t.Stderr, types.ErrUnknown, err.Error())
}
return nil
}
Expand Down
49 changes: 17 additions & 32 deletions pkg/skel/skel_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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())
}
Expand Down Expand Up @@ -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"))
})
})

Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -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"))
})
})

Expand Down Expand Up @@ -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))
Expand All @@ -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))
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -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"))
})
})

Expand All @@ -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"))
})
})

Expand Down Expand Up @@ -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"))
})
})
})
Expand Down

0 comments on commit 759e3cb

Please sign in to comment.