Implement all inspect commands with the new inspector interface.

It makes the behavior completely consistent across commands.
It adds tests to check that execution stops when an element is not
found.

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

api/client/inspect.go

@@ -34,97 +34,100 @@ func (cli *DockerCli) CmdInspect(args ...string) error {
 		return fmt.Errorf("%q is not a valid value for --type", *inspectType)
 	}
 
-	var (
-		err              error
-		tmpl             *template.Template
-		elementInspector inspect.Inspector
-	)
-
-	if *tmplStr != "" {
-		if tmpl, err = template.New("").Funcs(funcMap).Parse(*tmplStr); err != nil {
-			return Cli.StatusError{StatusCode: 64,
-				Status: "Template parsing error: " + err.Error()}
-		}
-	}
-
-	if tmpl != nil {
-		elementInspector = inspect.NewTemplateInspector(cli.out, tmpl)
-	} else {
-		elementInspector = inspect.NewIndentedInspector(cli.out)
-	}
-
+	var elementSearcher inspectSearcher
 	switch *inspectType {
 	case "container":
-		err = cli.inspectContainers(cmd.Args(), *size, elementInspector)
-	case "images":
-		err = cli.inspectImages(cmd.Args(), *size, elementInspector)
+		elementSearcher = cli.inspectContainers(*size)
+	case "image":
+		elementSearcher = cli.inspectImages(*size)
 	default:
-		err = cli.inspectAll(cmd.Args(), *size, elementInspector)
+		elementSearcher = cli.inspectAll(*size)
+	}
+
+	return cli.inspectElements(*tmplStr, cmd.Args(), elementSearcher)
+}
+
+func (cli *DockerCli) inspectContainers(getSize bool) inspectSearcher {
+	return func(ref string) (interface{}, []byte, error) {
+		return cli.client.ContainerInspectWithRaw(ref, getSize)
+	}
+}
+
+func (cli *DockerCli) inspectImages(getSize bool) inspectSearcher {
+	return func(ref string) (interface{}, []byte, error) {
+		return cli.client.ImageInspectWithRaw(ref, getSize)
+	}
+}
+
+func (cli *DockerCli) inspectAll(getSize bool) inspectSearcher {
+	return func(ref string) (interface{}, []byte, error) {
+		c, rawContainer, err := cli.client.ContainerInspectWithRaw(ref, getSize)
+		if err != nil {
+			// Search for image with that id if a container doesn't exist.
+			if lib.IsErrContainerNotFound(err) {
+				i, rawImage, err := cli.client.ImageInspectWithRaw(ref, getSize)
+				if err != nil {
+					if lib.IsErrImageNotFound(err) {
+						return nil, nil, fmt.Errorf("Error: No such image or container: %s", ref)
+					}
+					return nil, nil, err
+				}
+				return i, rawImage, err
+			}
+			return nil, nil, err
+		}
+		return c, rawContainer, err
+	}
+}
+
+type inspectSearcher func(ref string) (interface{}, []byte, error)
+
+func (cli *DockerCli) inspectElements(tmplStr string, references []string, searchByReference inspectSearcher) error {
+	elementInspector, err := cli.newInspectorWithTemplate(tmplStr)
+	if err != nil {
+		return Cli.StatusError{StatusCode: 64, Status: err.Error()}
+	}
+
+	var inspectErr error
+	for _, ref := range references {
+		element, raw, err := searchByReference(ref)
+		if err != nil {
+			inspectErr = err
+			break
+		}
+
+		if err := elementInspector.Inspect(element, raw); err != nil {
+			inspectErr = err
+			break
+		}
 	}
 
 	if err := elementInspector.Flush(); err != nil {
-		return err
+		cli.inspectErrorStatus(err)
 	}
-	return err
-}
 
-func (cli *DockerCli) inspectContainers(containerIDs []string, getSize bool, elementInspector inspect.Inspector) error {
-	for _, containerID := range containerIDs {
-		if err := cli.inspectContainer(containerID, getSize, elementInspector); err != nil {
-			if lib.IsErrContainerNotFound(err) {
-				return fmt.Errorf("Error: No such container: %s\n", containerID)
-			}
-			return err
-		}
+	if status := cli.inspectErrorStatus(inspectErr); status != 0 {
+		return Cli.StatusError{StatusCode: status}
 	}
 	return nil
 }
 
-func (cli *DockerCli) inspectImages(imageIDs []string, getSize bool, elementInspector inspect.Inspector) error {
-	for _, imageID := range imageIDs {
-		if err := cli.inspectImage(imageID, getSize, elementInspector); err != nil {
-			if lib.IsErrImageNotFound(err) {
-				return fmt.Errorf("Error: No such image: %s\n", imageID)
-			}
-			return err
-		}
-	}
-	return nil
-}
-
-func (cli *DockerCli) inspectAll(ids []string, getSize bool, elementInspector inspect.Inspector) error {
-	for _, id := range ids {
-		if err := cli.inspectContainer(id, getSize, elementInspector); err != nil {
-			// Search for image with that id if a container doesn't exist.
-			if lib.IsErrContainerNotFound(err) {
-				if err := cli.inspectImage(id, getSize, elementInspector); err != nil {
-					if lib.IsErrImageNotFound(err) {
-						return fmt.Errorf("Error: No such image or container: %s", id)
-					}
-					return err
-				}
-				continue
-			}
-			return err
-		}
-	}
-	return nil
-}
-
-func (cli *DockerCli) inspectContainer(containerID string, getSize bool, elementInspector inspect.Inspector) error {
-	c, raw, err := cli.client.ContainerInspectWithRaw(containerID, getSize)
+func (cli *DockerCli) inspectErrorStatus(err error) (status int) {
 	if err != nil {
-		return err
+		fmt.Fprintf(cli.err, "%s\n", err)
+		status = 1
 	}
-
-	return elementInspector.Inspect(c, raw)
+	return
 }
 
-func (cli *DockerCli) inspectImage(imageID string, getSize bool, elementInspector inspect.Inspector) error {
-	i, raw, err := cli.client.ImageInspectWithRaw(imageID, getSize)
-	if err != nil {
-		return err
+func (cli *DockerCli) newInspectorWithTemplate(tmplStr string) (inspect.Inspector, error) {
+	elementInspector := inspect.NewIndentedInspector(cli.out)
+	if tmplStr != "" {
+		tmpl, err := template.New("").Funcs(funcMap).Parse(tmplStr)
+		if err != nil {
+			return nil, fmt.Errorf("Template parsing error: %s", err)
+		}
+		elementInspector = inspect.NewTemplateInspector(cli.out, tmpl)
 	}
-
-	return elementInspector.Inspect(i, raw)
+	return elementInspector, nil
 }

api/client/inspect/inspector.go

@@ -33,29 +33,41 @@ func NewTemplateInspector(outputStream io.Writer, tmpl *template.Template) Inspe
 // Inspect executes the inspect template.
 // It decodes the raw element into a map if the initial execution fails.
 // This allows docker cli to parse inspect structs injected with Swarm fields.
-func (i TemplateInspector) Inspect(typedElement interface{}, rawElement []byte) error {
+func (i *TemplateInspector) Inspect(typedElement interface{}, rawElement []byte) error {
 	buffer := new(bytes.Buffer)
 	if err := i.tmpl.Execute(buffer, typedElement); err != nil {
-		var raw interface{}
-		rdr := bytes.NewReader(rawElement)
-		dec := json.NewDecoder(rdr)
-
-		if rawErr := dec.Decode(&raw); rawErr != nil {
-			return fmt.Errorf("unable to read inspect data: %v\n", rawErr)
-		}
-
-		tmplMissingKey := i.tmpl.Option("missingkey=error")
-		if rawErr := tmplMissingKey.Execute(buffer, raw); rawErr != nil {
-			return fmt.Errorf("Template parsing error: %v\n", err)
+		if rawElement == nil {
+			return fmt.Errorf("Template parsing error: %v", err)
 		}
+		return i.tryRawInspectFallback(rawElement)
 	}
 	i.buffer.Write(buffer.Bytes())
 	i.buffer.WriteByte('\n')
 	return nil
 }
 
+func (i *TemplateInspector) tryRawInspectFallback(rawElement []byte) error {
+	var raw interface{}
+	buffer := new(bytes.Buffer)
+	rdr := bytes.NewReader(rawElement)
+	dec := json.NewDecoder(rdr)
+
+	if rawErr := dec.Decode(&raw); rawErr != nil {
+		return fmt.Errorf("unable to read inspect data: %v", rawErr)
+	}
+
+	tmplMissingKey := i.tmpl.Option("missingkey=error")
+	if rawErr := tmplMissingKey.Execute(buffer, raw); rawErr != nil {
+		return fmt.Errorf("Template parsing error: %v", rawErr)
+	}
+
+	i.buffer.Write(buffer.Bytes())
+	i.buffer.WriteByte('\n')
+	return nil
+}
+
 // Flush write the result of inspecting all elements into the output stream.
-func (i TemplateInspector) Flush() error {
+func (i *TemplateInspector) Flush() error {
 	_, err := io.Copy(i.outputStream, i.buffer)
 	return err
 }
@@ -63,39 +75,37 @@ func (i TemplateInspector) Flush() error {
 // IndentedInspector uses a buffer to stop the indented representation of an element.
 type IndentedInspector struct {
 	outputStream io.Writer
-	indented     *bytes.Buffer
+	elements     []interface{}
 }
 
 // NewIndentedInspector generates a new IndentedInspector.
 func NewIndentedInspector(outputStream io.Writer) Inspector {
-	indented := new(bytes.Buffer)
-	indented.WriteString("[\n")
 	return &IndentedInspector{
 		outputStream: outputStream,
-		indented:     indented,
 	}
 }
 
 // Inspect writes the raw element with an indented json format.
-func (i IndentedInspector) Inspect(_ interface{}, rawElement []byte) error {
-	if err := json.Indent(i.indented, rawElement, "", "    "); err != nil {
-		return err
-	}
-	i.indented.WriteByte(',')
+func (i *IndentedInspector) Inspect(typedElement interface{}, _ []byte) error {
+	i.elements = append(i.elements, typedElement)
 	return nil
 }
 
 // Flush write the result of inspecting all elements into the output stream.
-func (i IndentedInspector) Flush() error {
-	if i.indented.Len() > 1 {
-		// Remove trailing ','
-		i.indented.Truncate(i.indented.Len() - 1)
+func (i *IndentedInspector) Flush() error {
+	if len(i.elements) == 0 {
+		_, err := io.WriteString(i.outputStream, "[]\n")
+		return err
 	}
-	i.indented.WriteString("]\n")
 
-	// Note that we will always write "[]" when "-f" isn't specified,
-	// to make sure the output would always be array, see
-	// https://github.com/docker/docker/pull/9500#issuecomment-65846734
-	_, err := io.Copy(i.outputStream, i.indented)
+	buffer, err := json.MarshalIndent(i.elements, "", "    ")
+	if err != nil {
+		return err
+	}
+
+	if _, err := io.Copy(i.outputStream, bytes.NewReader(buffer)); err != nil {
+		return err
+	}
+	_, err = io.WriteString(i.outputStream, "\n")
 	return err
 }

api/client/lib/errors.go

@@ -15,7 +15,7 @@ type imageNotFoundError struct {
 
 // Error returns a string representation of an imageNotFoundError
 func (i imageNotFoundError) Error() string {
-	return fmt.Sprintf("Image not found: %s", i.imageID)
+	return fmt.Sprintf("Error: No such image: %s", i.imageID)
 }
 
 // IsErrImageNotFound returns true if the error is caused
@@ -32,7 +32,7 @@ type containerNotFoundError struct {
 
 // Error returns a string representation of an containerNotFoundError
 func (e containerNotFoundError) Error() string {
-	return fmt.Sprintf("Container not found: %s", e.containerID)
+	return fmt.Sprintf("Error: No such container: %s", e.containerID)
 }
 
 // IsErrContainerNotFound returns true if the error is caused
@@ -42,6 +42,40 @@ func IsErrContainerNotFound(err error) bool {
 	return ok
 }
 
+// networkNotFoundError implements an error returned when a network is not in the docker host.
+type networkNotFoundError struct {
+	networkID string
+}
+
+// Error returns a string representation of an networkNotFoundError
+func (e networkNotFoundError) Error() string {
+	return fmt.Sprintf("Error: No such network: %s", e.networkID)
+}
+
+// IsErrNetworkNotFound returns true if the error is caused
+// when a network is not found in the docker host.
+func IsErrNetworkNotFound(err error) bool {
+	_, ok := err.(networkNotFoundError)
+	return ok
+}
+
+// volumeNotFoundError implements an error returned when a volume is not in the docker host.
+type volumeNotFoundError struct {
+	volumeID string
+}
+
+// Error returns a string representation of an networkNotFoundError
+func (e volumeNotFoundError) Error() string {
+	return fmt.Sprintf("Error: No such volume: %s", e.volumeID)
+}
+
+// IsErrVolumeNotFound returns true if the error is caused
+// when a volume is not found in the docker host.
+func IsErrVolumeNotFound(err error) bool {
+	_, ok := err.(networkNotFoundError)
+	return ok
+}
+
 // unauthorizedError represents an authorization error in a remote registry.
 type unauthorizedError struct {
 	cause error

api/client/lib/network.go

@@ -2,6 +2,7 @@ package lib
 
 import (
 	"encoding/json"
+	"net/http"
 
 	"github.com/docker/docker/api/types"
 )
@@ -59,6 +60,9 @@ func (cli *Client) NetworkInspect(networkID string) (types.NetworkResource, erro
 	var networkResource types.NetworkResource
 	resp, err := cli.get("/networks/"+networkID, nil, nil)
 	if err != nil {
+		if resp.statusCode == http.StatusNotFound {
+			return networkResource, networkNotFoundError{networkID}
+		}
 		return networkResource, err
 	}
 	defer ensureReaderClosed(resp)

api/client/lib/volume.go

@@ -2,6 +2,7 @@ package lib
 
 import (
 	"encoding/json"
+	"net/http"
 	"net/url"
 
 	"github.com/docker/docker/api/types"
@@ -35,6 +36,9 @@ func (cli *Client) VolumeInspect(volumeID string) (types.Volume, error) {
 	var volume types.Volume
 	resp, err := cli.get("/volumes/"+volumeID, nil, nil)
 	if err != nil {
+		if resp.statusCode == http.StatusNotFound {
+			return volume, volumeNotFoundError{volumeID}
+		}
 		return volume, err
 	}
 	defer ensureReaderClosed(resp)

api/client/network.go

@@ -1,14 +1,10 @@
 package client
 
 import (
-	"bytes"
-	"encoding/json"
 	"fmt"
-	"io"
 	"net"
 	"strings"
 	"text/tabwriter"
-	"text/template"
 
 	"github.com/docker/docker/api/types"
 	Cli "github.com/docker/docker/cli"
@@ -192,61 +188,12 @@ func (cli *DockerCli) CmdNetworkInspect(args ...string) error {
 		return err
 	}
 
-	var tmpl *template.Template
-	if *tmplStr != "" {
-		var err error
-		tmpl, err = template.New("").Funcs(funcMap).Parse(*tmplStr)
-		if err != nil {
-			return err
-		}
+	inspectSearcher := func(name string) (interface{}, []byte, error) {
+		i, err := cli.client.NetworkInspect(name)
+		return i, nil, err
 	}
 
-	status := 0
-	var networks []types.NetworkResource
-	buf := new(bytes.Buffer)
-	for _, name := range cmd.Args() {
-		networkResource, err := cli.client.NetworkInspect(name)
-		if err != nil {
-			fmt.Fprintf(cli.err, "%s\n", err)
-			return Cli.StatusError{StatusCode: 1}
-		}
-		if tmpl == nil {
-			networks = append(networks, networkResource)
-			continue
-		}
-
-		if err := tmpl.Execute(buf, &networkResource); err != nil {
-			fmt.Fprintf(cli.err, "%s\n", err)
-			return Cli.StatusError{StatusCode: 1}
-		}
-		buf.WriteString("\n")
-	}
-
-	if tmpl != nil {
-		if _, err := io.Copy(cli.out, buf); err != nil {
-			return err
-		}
-		return nil
-	}
-
-	if len(networks) == 0 {
-		io.WriteString(cli.out, "[]")
-	}
-
-	b, err := json.MarshalIndent(networks, "", "    ")
-	if err != nil {
-		return err
-	}
-
-	if _, err := io.Copy(cli.out, bytes.NewReader(b)); err != nil {
-		return err
-	}
-	io.WriteString(cli.out, "\n")
-
-	if status != 0 {
-		return Cli.StatusError{StatusCode: status}
-	}
-	return nil
+	return cli.inspectElements(*tmplStr, cmd.Args(), inspectSearcher)
 }
 
 // Consolidates the ipam configuration as a group from different related configurations

api/client/volume.go

@@ -1,12 +1,8 @@
 package client
 
 import (
-	"bytes"
-	"encoding/json"
 	"fmt"
-	"io"
 	"text/tabwriter"
-	"text/template"
 
 	"github.com/docker/docker/api/types"
 	Cli "github.com/docker/docker/cli"
@@ -98,58 +94,12 @@ func (cli *DockerCli) CmdVolumeInspect(args ...string) error {
 		return nil
 	}
 
-	var tmpl *template.Template
-	if *tmplStr != "" {
-		var err error
-		tmpl, err = template.New("").Funcs(funcMap).Parse(*tmplStr)
-		if err != nil {
-			return err
-		}
+	inspectSearcher := func(name string) (interface{}, []byte, error) {
+		i, err := cli.client.VolumeInspect(name)
+		return i, nil, err
 	}
 
-	var status = 0
-	var volumes []*types.Volume
-
-	for _, name := range cmd.Args() {
-		volume, err := cli.client.VolumeInspect(name)
-		if err != nil {
-			fmt.Fprintf(cli.err, "Unable to read inspect data: %v\n", err)
-			status = 1
-			break
-		}
-
-		if tmpl == nil {
-			volumes = append(volumes, &volume)
-			continue
-		}
-
-		buf := bytes.NewBufferString("")
-		if err := tmpl.Execute(buf, &volume); err != nil {
-			fmt.Fprintf(cli.err, "Template parsing error: %v\n", err)
-			status = 1
-			break
-		}
-
-		cli.out.Write(buf.Bytes())
-		cli.out.Write([]byte{'\n'})
-	}
-
-	if tmpl == nil {
-		b, err := json.MarshalIndent(volumes, "", "    ")
-		if err != nil {
-			return err
-		}
-		_, err = io.Copy(cli.out, bytes.NewReader(b))
-		if err != nil {
-			return err
-		}
-		io.WriteString(cli.out, "\n")
-	}
-
-	if status != 0 {
-		return Cli.StatusError{StatusCode: status}
-	}
-	return nil
+	return cli.inspectElements(*tmplStr, cmd.Args(), inspectSearcher)
 }
 
 // CmdVolumeCreate creates a new container from a given image.

integration-cli/docker_cli_inspect_test.go

@@ -356,3 +356,14 @@ func (s *DockerSuite) TestInspectByPrefix(c *check.C) {
 	c.Assert(err, checker.IsNil)
 	c.Assert(id, checker.Equals, id3)
 }
+
+func (s *DockerSuite) TestInspectStopWhenNotFound(c *check.C) {
+	dockerCmd(c, "run", "--name=busybox", "-d", "busybox", "top")
+	dockerCmd(c, "run", "--name=not-shown", "-d", "busybox", "top")
+	out, _, err := dockerCmdWithError("inspect", "--type=container", "--format='{{.Name}}'", "busybox", "missing", "not-shown")
+
+	c.Assert(err, checker.Not(check.IsNil))
+	c.Assert(out, checker.Contains, "busybox")
+	c.Assert(out, checker.Not(checker.Contains), "not-shown")
+	c.Assert(out, checker.Contains, "Error: No such container: missing")
+}

integration-cli/docker_cli_network_unix_test.go

@@ -317,7 +317,7 @@ func (s *DockerSuite) TestDockerInspectMultipleNetwork(c *check.C) {
 	c.Assert(exitCode, checker.Equals, 1)
 	c.Assert(out, checker.Contains, "Error: No such network: nonexistent")
 	networkResources = []types.NetworkResource{}
-	inspectOut := strings.SplitN(out, "\n", 2)[1]
+	inspectOut := strings.SplitN(out, "\nError: No such network: nonexistent\n", 2)[0]
 	err = json.Unmarshal([]byte(inspectOut), &networkResources)
 	c.Assert(networkResources, checker.HasLen, 1)
 

integration-cli/docker_cli_volume_test.go

@@ -53,15 +53,17 @@ func (s *DockerSuite) TestVolumeCliInspect(c *check.C) {
 func (s *DockerSuite) TestVolumeCliInspectMulti(c *check.C) {
 	dockerCmd(c, "volume", "create", "--name", "test1")
 	dockerCmd(c, "volume", "create", "--name", "test2")
+	dockerCmd(c, "volume", "create", "--name", "not-shown")
 
-	out, _, err := dockerCmdWithError("volume", "inspect", "--format='{{ .Name }}'", "test1", "test2", "doesntexist")
+	out, _, err := dockerCmdWithError("volume", "inspect", "--format='{{ .Name }}'", "test1", "test2", "doesntexist", "not-shown")
 	c.Assert(err, checker.NotNil)
 	outArr := strings.Split(strings.TrimSpace(out), "\n")
 	c.Assert(len(outArr), check.Equals, 3, check.Commentf("\n%s", out))
 
-	c.Assert(strings.Contains(out, "test1\n"), check.Equals, true)
-	c.Assert(strings.Contains(out, "test2\n"), check.Equals, true)
-	c.Assert(strings.Contains(out, "Error: No such volume: doesntexist\n"), check.Equals, true)
+	c.Assert(out, checker.Contains, "test1")
+	c.Assert(out, checker.Contains, "test2")
+	c.Assert(out, checker.Contains, "Error: No such volume: doesntexist")
+	c.Assert(out, checker.Not(checker.Contains), "not-shown")
 }
 
 func (s *DockerSuite) TestVolumeCliLs(c *check.C) {