pkg/skel: missing env var log lines appear in stderr

Previously, the log lines appeared in stdout before the JSON encoding of
the error message.  That would break JSON parsing of stdout.  Instead, we use
stderr for these unstructured logs, consistent with the CNI spec.
This commit is contained in:
Gabe Rosenhouse 2016-07-13 22:24:34 -04:00
parent 3c43314926
commit f2b6ec0375
2 changed files with 25 additions and 2 deletions

View File

@ -40,6 +40,7 @@ type CmdArgs struct {
type dispatcher struct { type dispatcher struct {
Getenv func(string) string Getenv func(string) string
Stdin io.Reader Stdin io.Reader
Stderr io.Writer
} }
type reqForCmdEntry map[string]bool type reqForCmdEntry map[string]bool
@ -106,8 +107,7 @@ func (t *dispatcher) getCmdArgsFromEnv() (string, *CmdArgs, error) {
for _, v := range vars { for _, v := range vars {
*v.val = t.Getenv(v.name) *v.val = t.Getenv(v.name)
if v.reqForCmd[cmd] && *v.val == "" { if v.reqForCmd[cmd] && *v.val == "" {
log.Printf("%v env variable missing", v.name) fmt.Fprintf(t.Stderr, "%v env variable missing\n", v.name)
// TODO: test this logging ^^^ and log to stderr instead of stdout
argsMissing = true argsMissing = true
} }
} }
@ -172,6 +172,7 @@ func PluginMain(cmdAdd, cmdDel func(_ *CmdArgs) error) {
caller := dispatcher{ caller := dispatcher{
Getenv: os.Getenv, Getenv: os.Getenv,
Stdin: os.Stdin, Stdin: os.Stdin,
Stderr: os.Stderr,
} }
err := caller.pluginMain(cmdAdd, cmdDel) err := caller.pluginMain(cmdAdd, cmdDel)

View File

@ -15,6 +15,7 @@
package skel package skel
import ( import (
"bytes"
"errors" "errors"
"io" "io"
"strings" "strings"
@ -47,6 +48,7 @@ var _ = Describe("dispatching to the correct callback", func() {
var ( var (
environment map[string]string environment map[string]string
stdin io.Reader stdin io.Reader
stderr *bytes.Buffer
cmdAdd, cmdDel *fakeCmd cmdAdd, cmdDel *fakeCmd
dispatch *dispatcher dispatch *dispatcher
expectedCmdArgs *CmdArgs expectedCmdArgs *CmdArgs
@ -62,9 +64,11 @@ var _ = Describe("dispatching to the correct callback", func() {
"CNI_PATH": "/some/cni/path", "CNI_PATH": "/some/cni/path",
} }
stdin = strings.NewReader(`{ "some": "config" }`) stdin = strings.NewReader(`{ "some": "config" }`)
stderr = &bytes.Buffer{}
dispatch = &dispatcher{ dispatch = &dispatcher{
Getenv: func(key string) string { return environment[key] }, Getenv: func(key string) string { return environment[key] },
Stdin: stdin, Stdin: stdin,
Stderr: stderr,
} }
cmdAdd = &fakeCmd{} cmdAdd = &fakeCmd{}
cmdDel = &fakeCmd{} cmdDel = &fakeCmd{}
@ -87,6 +91,7 @@ var _ = Describe("dispatching to the correct callback", func() {
Code: 100, Code: 100,
Msg: "required env variables missing", Msg: "required env variables missing",
})) }))
Expect(stderr.String()).To(ContainSubstring(envVar + " env variable missing\n"))
} else { } else {
Expect(err).NotTo(HaveOccurred()) Expect(err).NotTo(HaveOccurred())
} }
@ -117,6 +122,23 @@ var _ = Describe("dispatching to the correct callback", func() {
Entry("args", "CNI_ARGS", false), Entry("args", "CNI_ARGS", false),
Entry("path", "CNI_PATH", true), Entry("path", "CNI_PATH", true),
) )
Context("when multiple required env vars are missing", func() {
BeforeEach(func() {
delete(environment, "CNI_NETNS")
delete(environment, "CNI_IFNAME")
delete(environment, "CNI_PATH")
})
It("reports that all of them are missing, not just the first", func() {
Expect(dispatch.pluginMain(cmdAdd.Func, cmdDel.Func)).NotTo(Succeed())
log := stderr.String()
Expect(log).To(ContainSubstring("CNI_NETNS env variable missing\n"))
Expect(log).To(ContainSubstring("CNI_IFNAME env variable missing\n"))
Expect(log).To(ContainSubstring("CNI_PATH env variable missing\n"))
})
})
}) })
Context("when the CNI_COMMAND is DEL", func() { Context("when the CNI_COMMAND is DEL", func() {