Fail when there is an error executing an inspect template.

- Do not execute the template directly in the cli outout, go is not atomic
in this operation and can send bytes before failing the execution.
- Fail after evaluating a raw interface if the typed execution also
failed, assuming there is a template parsing error.

Signed-off-by: David Calavera <david.calavera@gmail.com>

api/client/inspect.go

@@ -59,7 +59,6 @@ func (cli *DockerCli) CmdInspect(args ...string) error {
 	}
 
 	for _, name := range cmd.Args() {
-
 		if *inspectType == "" || *inspectType == "container" {
 			obj, _, err = readBody(cli.call("GET", "/containers/"+name+"/json?"+v.Encode(), nil, nil))
 			if err != nil && *inspectType == "container" {
@@ -101,42 +100,45 @@ func (cli *DockerCli) CmdInspect(args ...string) error {
 		} else {
 			rdr := bytes.NewReader(obj)
 			dec := json.NewDecoder(rdr)
+			buf := bytes.NewBufferString("")
 
 			if isImage {
 				inspPtr := types.ImageInspect{}
 				if err := dec.Decode(&inspPtr); err != nil {
-					fmt.Fprintf(cli.err, "%s\n", err)
+					fmt.Fprintf(cli.err, "Unable to read inspect data: %v\n", err)
 					status = 1
-					continue
+					break
 				}
-				if err := tmpl.Execute(cli.out, inspPtr); err != nil {
+				if err := tmpl.Execute(buf, inspPtr); err != nil {
 					rdr.Seek(0, 0)
-					var raw interface{}
-					if err := dec.Decode(&raw); err != nil {
-						return err
-					}
-					if err = tmpl.Execute(cli.out, raw); err != nil {
-						return err
+					var ok bool
+
+					if buf, ok = cli.decodeRawInspect(tmpl, dec); !ok {
+						fmt.Fprintf(cli.err, "Template parsing error: %v\n", err)
+						status = 1
+						break
 					}
 				}
 			} else {
 				inspPtr := types.ContainerJSON{}
 				if err := dec.Decode(&inspPtr); err != nil {
-					fmt.Fprintf(cli.err, "%s\n", err)
+					fmt.Fprintf(cli.err, "Unable to read inspect data: %v\n", err)
 					status = 1
-					continue
+					break
 				}
-				if err := tmpl.Execute(cli.out, inspPtr); err != nil {
+				if err := tmpl.Execute(buf, inspPtr); err != nil {
 					rdr.Seek(0, 0)
-					var raw interface{}
-					if err := dec.Decode(&raw); err != nil {
-						return err
-					}
-					if err = tmpl.Execute(cli.out, raw); err != nil {
-						return err
+					var ok bool
+
+					if buf, ok = cli.decodeRawInspect(tmpl, dec); !ok {
+						fmt.Fprintf(cli.err, "Template parsing error: %v\n", err)
+						status = 1
+						break
 					}
 				}
 			}
+
+			cli.out.Write(buf.Bytes())
 			cli.out.Write([]byte{'\n'})
 		}
 		indented.WriteString(",")
@@ -162,3 +164,33 @@ func (cli *DockerCli) CmdInspect(args ...string) error {
 	}
 	return nil
 }
+
+// decodeRawInspect executes the inspect template with a raw interface.
+// This allows docker cli to parse inspect structs injected with Swarm fields.
+// Unfortunately, go 1.4 doesn't fail executing invalid templates when the input is an interface.
+// It doesn't allow to modify this behavior either, sending <no value> messages to the output.
+// We assume that the template is invalid when there is a <no value>, if the template was valid
+// we'd get <nil> or "" values. In that case we fail with the original error raised executing the
+// template with the typed input.
+//
+// TODO: Go 1.5 allows to customize the error behavior, we can probably get rid of this as soon as
+// we build Docker with that version:
+// https://golang.org/pkg/text/template/#Template.Option
+func (cli *DockerCli) decodeRawInspect(tmpl *template.Template, dec *json.Decoder) (*bytes.Buffer, bool) {
+	var raw interface{}
+	buf := bytes.NewBufferString("")
+
+	if rawErr := dec.Decode(&raw); rawErr != nil {
+		fmt.Fprintf(cli.err, "Unable to read inspect data: %v\n", rawErr)
+		return buf, false
+	}
+
+	if rawErr := tmpl.Execute(buf, raw); rawErr != nil {
+		return buf, false
+	}
+
+	if strings.Contains(buf.String(), "<no value>") {
+		return buf, false
+	}
+	return buf, true
+}

integration-cli/docker_cli_inspect_test.go

@@ -82,7 +82,7 @@ func (s *DockerSuite) TestInspectTypeFlagContainer(c *check.C) {
 
 	dockerCmd(c, "run", "--name=busybox", "-d", "busybox", "top")
 
-	formatStr := fmt.Sprintf("--format='{{.State.Running}}'")
+	formatStr := "--format='{{.State.Running}}'"
 	out, _ := dockerCmd(c, "inspect", "--type=container", formatStr, "busybox")
 	c.Assert(out, checker.Equals, "true\n") // not a container JSON
 }
@@ -290,19 +290,15 @@ func (s *DockerSuite) TestInspectNoSizeFlagContainer(c *check.C) {
 
 	dockerCmd(c, "run", "--name=busybox", "-d", "busybox", "top")
 
-	formatStr := fmt.Sprintf("--format='{{.SizeRw}},{{.SizeRootFs}}'")
+	formatStr := "--format='{{.SizeRw}},{{.SizeRootFs}}'"
 	out, _ := dockerCmd(c, "inspect", "--type=container", formatStr, "busybox")
 	c.Assert(strings.TrimSpace(out), check.Equals, "<nil>,<nil>", check.Commentf("Exepcted not to display size info: %s", out))
 }
 
 func (s *DockerSuite) TestInspectSizeFlagContainer(c *check.C) {
-
-	//Both the container and image are named busybox. docker inspect will fetch container
-	//JSON SizeRw and SizeRootFs field. If there is a flag --size/-s, the fields are not <no value>.
-
 	dockerCmd(c, "run", "--name=busybox", "-d", "busybox", "top")
 
-	formatStr := fmt.Sprintf("--format='{{.SizeRw}},{{.SizeRootFs}}'")
+	formatStr := "--format='{{.SizeRw}},{{.SizeRootFs}}'"
 	out, _ := dockerCmd(c, "inspect", "-s", "--type=container", formatStr, "busybox")
 	sz := strings.Split(out, ",")
 
@@ -311,14 +307,25 @@ func (s *DockerSuite) TestInspectSizeFlagContainer(c *check.C) {
 }
 
 func (s *DockerSuite) TestInspectSizeFlagImage(c *check.C) {
+	dockerCmd(c, "run", "--name=busybox", "-d", "busybox", "top")
 
-	//Both the container and image are named busybox. docker inspect will fetch image
-	//JSON SizeRw and SizeRootFs field. There are no these fields since they are only in containers.
+	formatStr := "--format='{{.SizeRw}},{{.SizeRootFs}}'"
+	out, _, err := dockerCmdWithError("inspect", "-s", "--type=image", formatStr, "busybox")
+
+	// Template error rather than <no value>
+	// This is a more correct behavior because images don't have sizes associated.
+	c.Assert(err, check.Not(check.IsNil))
+	c.Assert(out, checker.Contains, "Template parsing error")
+}
+
+func (s *DockerSuite) TestInspectTempateError(c *check.C) {
+	//Both the container and image are named busybox. docker inspect will fetch container
+	//JSON State.Running field. If the field is true, it's a container.
 
 	dockerCmd(c, "run", "--name=busybox", "-d", "busybox", "top")
 
-	formatStr := fmt.Sprintf("--format='{{.SizeRw}},{{.SizeRootFs}}'")
-	out, _ := dockerCmd(c, "inspect", "-s", "--type=image", formatStr, "busybox")
+	out, _, err := dockerCmdWithError("inspect", "--type=container", "--format='Format container: {{.ThisDoesNotExist}}'", "busybox")
 
-	c.Assert(strings.TrimSpace(out), check.Equals, "<no value>,<no value>", check.Commentf("Fields SizeRw and SizeRootFs are not exepcted to exist"))
+	c.Assert(err, check.Not(check.IsNil))
+	c.Assert(out, checker.Contains, "Template parsing error")
 }