enable inspect tests

A surprisingly big change.  A core problem was that `podman inspect`
allows for passing containers AND images with the default `--type=all`.
This only worked partially as the data was processed in isolation which
caused various issues (e.g., two separate outputs instead of one) but it
also caused issues regarding error handling.

Signed-off-by: Valentin Rothberg <rothberg@redhat.com>

cmd/podman/common/inspect.go

@@ -1,18 +0,0 @@
-package common
-
-import (
-	"github.com/containers/libpod/pkg/domain/entities"
-	"github.com/spf13/cobra"
-)
-
-// AddInspectFlagSet takes a command and adds the inspect flags and returns an InspectOptions object
-// Since this cannot live in `package main` it lives here until a better home is found
-func AddInspectFlagSet(cmd *cobra.Command) *entities.InspectOptions {
-	opts := entities.InspectOptions{}
-
-	flags := cmd.Flags()
-	flags.BoolVarP(&opts.Size, "size", "s", false, "Display total file size")
-	flags.StringVarP(&opts.Format, "format", "f", "", "Change the output format to a Go template")
-
-	return &opts
-}

cmd/podman/containers/inspect.go

@@ -1,15 +1,8 @@
 package containers
 
 import (
-	"context"
-	"fmt"
-	"os"
-	"strings"
-	"text/template"
-
-	"github.com/containers/libpod/cmd/podman/common"
+	"github.com/containers/libpod/cmd/podman/inspect"
 	"github.com/containers/libpod/cmd/podman/registry"
-
 	"github.com/containers/libpod/pkg/domain/entities"
 	"github.com/spf13/cobra"
 )
@@ -20,7 +13,7 @@ var (
 		Use:   "inspect [flags] CONTAINER",
 		Short: "Display the configuration of a container",
 		Long:  `Displays the low-level information on a container identified by name or ID.`,
-		RunE:  inspect,
+		RunE:  inspectExec,
 		Example: `podman container inspect myCtr
   podman container inspect -l --format '{{.Id}} {{.Config.Labels}}'`,
 	}
@@ -33,45 +26,9 @@ func init() {
 		Command: inspectCmd,
 		Parent:  containerCmd,
 	})
-	inspectOpts = common.AddInspectFlagSet(inspectCmd)
-	flags := inspectCmd.Flags()
-
-	if !registry.IsRemote() {
-		flags.BoolVarP(&inspectOpts.Latest, "latest", "l", false, "Act on the latest container podman is aware of")
+	inspectOpts = inspect.AddInspectFlagSet(inspectCmd)
 }
 
-}
-
-func inspect(cmd *cobra.Command, args []string) error {
-	responses, err := registry.ContainerEngine().ContainerInspect(context.Background(), args, *inspectOpts)
-	if err != nil {
-		return err
-	}
-	if inspectOpts.Format == "" {
-		b, err := json.MarshalIndent(responses, "", "  ")
-		if err != nil {
-			return err
-		}
-		fmt.Println(string(b))
-		return nil
-	}
-	format := inspectOpts.Format
-	if !strings.HasSuffix(format, "\n") {
-		format += "\n"
-	}
-	tmpl, err := template.New("inspect").Parse(format)
-	if err != nil {
-		return err
-	}
-	for _, i := range responses {
-		if err := tmpl.Execute(os.Stdout, i); err != nil {
-			return err
-		}
-	}
-	return nil
-}
-
-func Inspect(cmd *cobra.Command, args []string, options *entities.InspectOptions) error {
-	inspectOpts = options
-	return inspect(cmd, args)
+func inspectExec(cmd *cobra.Command, args []string) error {
+	return inspect.Inspect(args, *inspectOpts)
 }

cmd/podman/images/inspect.go

@@ -1,18 +1,9 @@
 package images
 
 import (
-	"context"
-	"fmt"
-	"os"
-	"strings"
-	"text/tabwriter"
-	"text/template"
-
-	"github.com/containers/buildah/pkg/formats"
-	"github.com/containers/libpod/cmd/podman/common"
+	"github.com/containers/libpod/cmd/podman/inspect"
 	"github.com/containers/libpod/cmd/podman/registry"
 	"github.com/containers/libpod/pkg/domain/entities"
-	"github.com/pkg/errors"
 	"github.com/spf13/cobra"
 )
 
@@ -21,8 +12,8 @@ var (
 	inspectCmd = &cobra.Command{
 		Use:   "inspect [flags] IMAGE",
 		Short: "Display the configuration of an image",
-		Long:  `Displays the low-level information on an image identified by name or ID.`,
-		RunE:  inspect,
+		Long:  `Displays the low-level information of an image identified by name or ID.`,
+		RunE:  inspectExec,
 		Example: `podman inspect alpine
   podman inspect --format "imageId: {{.Id}} size: {{.Size}}" alpine
   podman inspect --format "image: {{.ImageName}} driver: {{.Driver}}" myctr`,
@@ -36,78 +27,11 @@ func init() {
 		Command: inspectCmd,
 		Parent:  imageCmd,
 	})
-	inspectOpts = common.AddInspectFlagSet(inspectCmd)
+	inspectOpts = inspect.AddInspectFlagSet(inspectCmd)
+	flags := inspectCmd.Flags()
+	_ = flags.MarkHidden("latest") // Shared with container-inspect but not wanted here.
 }
 
-func inspect(cmd *cobra.Command, args []string) error {
-	if inspectOpts.Size {
-		return fmt.Errorf("--size can only be used for containers")
-	}
-	if inspectOpts.Latest {
-		return fmt.Errorf("--latest can only be used for containers")
-	}
-	if len(args) == 0 {
-		return errors.Errorf("image name must be specified: podman image inspect [options [...]] name")
-	}
-
-	results, err := registry.ImageEngine().Inspect(context.Background(), args, *inspectOpts)
-	if err != nil {
-		return err
-	}
-
-	if len(results.Images) > 0 {
-		if inspectOpts.Format == "" {
-			buf, err := json.MarshalIndent(results.Images, "", "  ")
-			if err != nil {
-				return err
-			}
-			fmt.Println(string(buf))
-
-			for id, e := range results.Errors {
-				fmt.Fprintf(os.Stderr, "%s: %s\n", id, e.Error())
-			}
-			return nil
-		}
-		row := inspectFormat(inspectOpts.Format)
-		format := "{{range . }}" + row + "{{end}}"
-		tmpl, err := template.New("inspect").Parse(format)
-		if err != nil {
-			return err
-		}
-
-		w := tabwriter.NewWriter(os.Stdout, 8, 2, 2, ' ', 0)
-		defer func() { _ = w.Flush() }()
-		err = tmpl.Execute(w, results.Images)
-		if err != nil {
-			return err
-		}
-	}
-
-	var lastErr error
-	for id, e := range results.Errors {
-		if lastErr != nil {
-			fmt.Fprintf(os.Stderr, "%s: %s\n", id, lastErr.Error())
-		}
-		lastErr = e
-	}
-	return lastErr
-}
-
-func inspectFormat(row string) string {
-	r := strings.NewReplacer("{{.Id}}", formats.IDString,
-		".Src", ".Source",
-		".Dst", ".Destination",
-		".ImageID", ".Image",
-	)
-	row = r.Replace(row)
-
-	if !strings.HasSuffix(row, "\n") {
-		row += "\n"
-	}
-	return row
-}
-
-func Inspect(cmd *cobra.Command, args []string, options *entities.InspectOptions) error {
-	inspectOpts = options
-	return inspect(cmd, args)
+func inspectExec(cmd *cobra.Command, args []string) error {
+	return inspect.Inspect(args, *inspectOpts)
 }

cmd/podman/inspect.go

@@ -1,31 +1,26 @@
 package main
 
 import (
-	"fmt"
-
-	"github.com/containers/libpod/cmd/podman/common"
-	"github.com/containers/libpod/cmd/podman/containers"
-	"github.com/containers/libpod/cmd/podman/images"
+	"github.com/containers/libpod/cmd/podman/inspect"
 	"github.com/containers/libpod/cmd/podman/registry"
 	"github.com/containers/libpod/pkg/domain/entities"
 	"github.com/spf13/cobra"
 )
 
-// Inspect is one of the outlier commands in that it operates on images/containers/...
-
 var (
-	inspectOpts *entities.InspectOptions
-
 	// Command: podman _inspect_ Object_ID
 	inspectCmd = &cobra.Command{
 		Use:              "inspect [flags] {CONTAINER_ID | IMAGE_ID}",
 		Short:            "Display the configuration of object denoted by ID",
 		Long:             "Displays the low-level information on an object identified by name or ID",
 		TraverseChildren: true,
-		RunE:             inspect,
-		Example: `podman inspect alpine
-  podman inspect --format "imageId: {{.Id}} size: {{.Size}}" alpine`,
+		RunE:             inspectExec,
+		Example: `podman inspect fedora
+  podman inspect --type image fedora
+  podman inspect CtrID ImgID
+  podman inspect --format "imageId: {{.Id}} size: {{.Size}}" fedora`,
 	}
+	inspectOpts *entities.InspectOptions
 )
 
 func init() {
@@ -33,26 +28,9 @@ func init() {
 		Mode:    []entities.EngineMode{entities.ABIMode, entities.TunnelMode},
 		Command: inspectCmd,
 	})
-	inspectOpts = common.AddInspectFlagSet(inspectCmd)
-	flags := inspectCmd.Flags()
-	flags.StringVarP(&inspectOpts.Type, "type", "t", "", "Return JSON for specified type, (image or container) (default \"all\")")
-	if !registry.IsRemote() {
-		flags.BoolVarP(&inspectOpts.Latest, "latest", "l", false, "Act on the latest container podman is aware of (containers only)")
-	}
+	inspectOpts = inspect.AddInspectFlagSet(inspectCmd)
 }
 
-func inspect(cmd *cobra.Command, args []string) error {
-	switch inspectOpts.Type {
-	case "image":
-		return images.Inspect(cmd, args, inspectOpts)
-	case "container":
-		return containers.Inspect(cmd, args, inspectOpts)
-	case "":
-		if err := images.Inspect(cmd, args, inspectOpts); err == nil {
-			return nil
-		}
-		return containers.Inspect(cmd, args, inspectOpts)
-	default:
-		return fmt.Errorf("invalid type %q is must be 'container' or 'image'", inspectOpts.Type)
-	}
+func inspectExec(cmd *cobra.Command, args []string) error {
+	return inspect.Inspect(args, *inspectOpts)
 }

cmd/podman/inspect/inspect.go

@@ -0,0 +1,159 @@
+package inspect
+
+import (
+	"context"
+	"fmt"
+	"strings"
+
+	"github.com/containers/buildah/pkg/formats"
+	"github.com/containers/libpod/cmd/podman/registry"
+	"github.com/containers/libpod/pkg/domain/entities"
+	"github.com/pkg/errors"
+	"github.com/spf13/cobra"
+)
+
+const (
+	// ImageType is the image type.
+	ImageType = "image"
+	// ContainerType is the container type.
+	ContainerType = "container"
+	// AllType can be of type ImageType or ContainerType.
+	AllType = "all"
+)
+
+// AddInspectFlagSet takes a command and adds the inspect flags and returns an
+// InspectOptions object.
+func AddInspectFlagSet(cmd *cobra.Command) *entities.InspectOptions {
+	opts := entities.InspectOptions{}
+
+	flags := cmd.Flags()
+	flags.BoolVarP(&opts.Size, "size", "s", false, "Display total file size")
+	flags.StringVarP(&opts.Format, "format", "f", "json", "Format the output to a Go template or json")
+	flags.StringVarP(&opts.Type, "type", "t", AllType, fmt.Sprintf("Specify inspect-oject type (%q, %q or %q)", ImageType, ContainerType, AllType))
+	flags.BoolVarP(&opts.Latest, "latest", "l", false, "Act on the latest container Podman is aware of")
+
+	return &opts
+}
+
+// Inspect inspects the specified container/image names or IDs.
+func Inspect(namesOrIDs []string, options entities.InspectOptions) error {
+	inspector, err := newInspector(options)
+	if err != nil {
+		return err
+	}
+	return inspector.inspect(namesOrIDs)
+}
+
+// inspector allows for inspecting images and containers.
+type inspector struct {
+	containerEngine entities.ContainerEngine
+	imageEngine     entities.ImageEngine
+	options         entities.InspectOptions
+}
+
+// newInspector creates a new inspector based on the specified options.
+func newInspector(options entities.InspectOptions) (*inspector, error) {
+	switch options.Type {
+	case ImageType, ContainerType, AllType:
+		// Valid types.
+	default:
+		return nil, errors.Errorf("invalid type %q: must be %q, %q or %q", options.Type, ImageType, ContainerType, AllType)
+	}
+	if options.Type == ImageType {
+		if options.Latest {
+			return nil, errors.Errorf("latest is not supported for type %q", ImageType)
+		}
+		if options.Size {
+			return nil, errors.Errorf("size is not supported for type %q", ImageType)
+		}
+	}
+	return &inspector{
+		containerEngine: registry.ContainerEngine(),
+		imageEngine:     registry.ImageEngine(),
+		options:         options,
+	}, nil
+}
+
+// inspect inspects the specified container/image names or IDs.
+func (i *inspector) inspect(namesOrIDs []string) error {
+	// data - dumping place for inspection results.
+	var data []interface{}
+	ctx := context.Background()
+
+	if len(namesOrIDs) == 0 {
+		if !i.options.Latest {
+			return errors.New("no containers or images specified")
+		}
+	}
+
+	tmpType := i.options.Type
+	if i.options.Latest {
+		if len(namesOrIDs) > 0 {
+			return errors.New("latest and containers are not allowed")
+		}
+		tmpType = ContainerType // -l works with --type=all
+	}
+
+	// Inspect - note that AllType requires us to expensively query one-by-one.
+	switch tmpType {
+	case AllType:
+		all, err := i.inspectAll(ctx, namesOrIDs)
+		if err != nil {
+			return err
+		}
+		data = all
+	case ImageType:
+		imgData, err := i.imageEngine.Inspect(ctx, namesOrIDs, i.options)
+		if err != nil {
+			return err
+		}
+		for i := range imgData {
+			data = append(data, imgData[i])
+		}
+	case ContainerType:
+		ctrData, err := i.containerEngine.ContainerInspect(ctx, namesOrIDs, i.options)
+		if err != nil {
+			return err
+		}
+		for i := range ctrData {
+			data = append(data, ctrData[i])
+		}
+	default:
+		return errors.Errorf("invalid type %q: must be %q, %q or %q", i.options.Type, ImageType, ContainerType, AllType)
+	}
+
+	var out formats.Writer
+	if i.options.Format == "json" || i.options.Format == "" { // "" for backwards compat
+		out = formats.JSONStructArray{Output: data}
+	} else {
+		out = formats.StdoutTemplateArray{Output: data, Template: inspectFormat(i.options.Format)}
+	}
+	return out.Out()
+}
+
+func (i *inspector) inspectAll(ctx context.Context, namesOrIDs []string) ([]interface{}, error) {
+	var data []interface{}
+	for _, name := range namesOrIDs {
+		imgData, err := i.imageEngine.Inspect(ctx, []string{name}, i.options)
+		if err == nil {
+			data = append(data, imgData[0])
+			continue
+		}
+		ctrData, err := i.containerEngine.ContainerInspect(ctx, []string{name}, i.options)
+		if err != nil {
+			return nil, err
+		}
+		data = append(data, ctrData[0])
+	}
+	return data, nil
+}
+
+func inspectFormat(row string) string {
+	r := strings.NewReplacer(
+		"{{.Id}}", formats.IDString,
+		".Src", ".Source",
+		".Dst", ".Destination",
+		".ImageID", ".Image",
+	)
+	return r.Replace(row)
+}

pkg/bindings/images/images.go

@@ -57,7 +57,7 @@ func List(ctx context.Context, all *bool, filters map[string][]string) ([]*entit
 
 // Get performs an image inspect.  To have the on-disk size of the image calculated, you can
 // use the optional size parameter.
-func GetImage(ctx context.Context, nameOrID string, size *bool) (*entities.ImageData, error) {
+func GetImage(ctx context.Context, nameOrID string, size *bool) (*entities.ImageInspectReport, error) {
 	conn, err := bindings.GetClient(ctx)
 	if err != nil {
 		return nil, err
@@ -66,7 +66,7 @@ func GetImage(ctx context.Context, nameOrID string, size *bool) (*entities.Image
 	if size != nil {
 		params.Set("size", strconv.FormatBool(*size))
 	}
-	inspectedData := entities.ImageData{}
+	inspectedData := entities.ImageInspectReport{}
 	response, err := conn.DoRequest(nil, http.MethodGet, "/images/%s/json", params, nameOrID)
 	if err != nil {
 		return &inspectedData, err

pkg/domain/entities/engine_image.go

@@ -13,7 +13,7 @@ type ImageEngine interface {
 	Exists(ctx context.Context, nameOrId string) (*BoolReport, error)
 	History(ctx context.Context, nameOrId string, opts ImageHistoryOptions) (*ImageHistoryReport, error)
 	Import(ctx context.Context, opts ImageImportOptions) (*ImageImportReport, error)
-	Inspect(ctx context.Context, names []string, opts InspectOptions) (*ImageInspectReport, error)
+	Inspect(ctx context.Context, namesOrIDs []string, opts InspectOptions) ([]*ImageInspectReport, error)
 	List(ctx context.Context, opts ImageListOptions) ([]*ImageSummary, error)
 	Load(ctx context.Context, opts ImageLoadOptions) (*ImageLoadReport, error)
 	Prune(ctx context.Context, opts ImagePruneOptions) (*ImagePruneReport, error)

pkg/domain/entities/images.go

@@ -238,13 +238,9 @@ type ImagePruneReport struct {
 type ImageTagOptions struct{}
 type ImageUntagOptions struct{}
 
-type ImageData struct {
-	*inspect.ImageData
-}
-
+// ImageInspectReport is the data when inspecting an image.
 type ImageInspectReport struct {
-	Images []*ImageData
-	Errors map[string]error
+	*inspect.ImageData
 }
 
 type ImageLoadOptions struct {

pkg/domain/entities/types.go

@@ -47,9 +47,13 @@ type NetOptions struct {
 
 // All CLI inspect commands and inspect sub-commands use the same options
 type InspectOptions struct {
+	// Format - change the output to JSON or a Go template.
 	Format string `json:",omitempty"`
+	// Latest - inspect the latest container Podman is aware of.
 	Latest bool `json:",omitempty"`
+	// Size (containers only) - display total file size.
 	Size bool `json:",omitempty"`
+	// Type -- return JSON for specified type.
 	Type string `json:",omitempty"`
 }
 

pkg/domain/infra/abi/images.go

@@ -170,29 +170,24 @@ func (ir *ImageEngine) Pull(ctx context.Context, rawImage string, options entiti
 	return &entities.ImagePullReport{Images: foundIDs}, nil
 }
 
-func (ir *ImageEngine) Inspect(ctx context.Context, names []string, opts entities.InspectOptions) (*entities.ImageInspectReport, error) {
-	report := entities.ImageInspectReport{
-		Errors: make(map[string]error),
-	}
-
-	for _, id := range names {
-		img, err := ir.Libpod.ImageRuntime().NewFromLocal(id)
+func (ir *ImageEngine) Inspect(ctx context.Context, namesOrIDs []string, opts entities.InspectOptions) ([]*entities.ImageInspectReport, error) {
+	reports := []*entities.ImageInspectReport{}
+	for _, i := range namesOrIDs {
+		img, err := ir.Libpod.ImageRuntime().NewFromLocal(i)
 		if err != nil {
-			report.Errors[id] = err
-			continue
+			return nil, err
 		}
-
-		results, err := img.Inspect(ctx)
+		result, err := img.Inspect(ctx)
 		if err != nil {
-			report.Errors[id] = err
-			continue
+			return nil, err
 		}
-
-		cookedResults := entities.ImageData{}
-		_ = domainUtils.DeepCopy(&cookedResults, results)
-		report.Images = append(report.Images, &cookedResults)
+		report := entities.ImageInspectReport{}
+		if err := domainUtils.DeepCopy(&report, result); err != nil {
+			return nil, err
 		}
-	return &report, nil
+		reports = append(reports, &report)
+	}
+	return reports, nil
 }
 
 func (ir *ImageEngine) Push(ctx context.Context, source string, destination string, options entities.ImagePushOptions) error {

pkg/domain/infra/tunnel/images.go

@@ -143,16 +143,16 @@ func (ir *ImageEngine) Untag(ctx context.Context, nameOrId string, tags []string
 	return nil
 }
 
-func (ir *ImageEngine) Inspect(_ context.Context, names []string, opts entities.InspectOptions) (*entities.ImageInspectReport, error) {
-	report := entities.ImageInspectReport{}
-	for _, id := range names {
-		r, err := images.GetImage(ir.ClientCxt, id, &opts.Size)
+func (ir *ImageEngine) Inspect(ctx context.Context, namesOrIDs []string, opts entities.InspectOptions) ([]*entities.ImageInspectReport, error) {
+	reports := []*entities.ImageInspectReport{}
+	for _, i := range namesOrIDs {
+		r, err := images.GetImage(ir.ClientCxt, i, &opts.Size)
 		if err != nil {
-			report.Errors[id] = err
+			return nil, err
 		}
-		report.Images = append(report.Images, r)
+		reports = append(reports, r)
 	}
-	return &report, nil
+	return reports, nil
 }
 
 func (ir *ImageEngine) Load(ctx context.Context, opts entities.ImageLoadOptions) (*entities.ImageLoadReport, error) {

test/e2e/inspect_test.go

@@ -17,7 +17,6 @@ var _ = Describe("Podman inspect", func() {
 	)
 
 	BeforeEach(func() {
-		Skip(v2fail)
 		tempdir, err = CreateTempDirInTempDir()
 		if err != nil {
 			os.Exit(1)