Fix podman inspect on overlapping/missing objects

This started as a small fix to `podman inspect` where a container
and image, with the same name/tag, were present, and
`podman inspect` was run on that name. `podman inspect` in 1.9
(and `docker inspect`) will give you the container; in v2.0, we
gave the image. This was an easy fix (just reorder how we check
for image/container).

Unfortunately, in the process of testing this fix, I determined
that we regressed in a different area. When you run inspect on
a number of containers, some of which do not exist,
`podman inspect` should return an array of inspect results for
the objects that exist, then print a number of errors, one for
each object that could not be found. We were bailing after the
first error, and not printing output for the containers that
succeeded. (For reference, this applied to images as well). This
required a much more substantial set of changes to properly
handle - signatures for the inspect functions in ContainerEngine
and ImageEngine, plus the implementations of these interfaces,
plus the actual inspect frontend code needed to be adjusted to
use this.

Fixes #6556

Signed-off-by: Matthew Heon <matthew.heon@pm.me>

cmd/podman/common/specgen.go

@@ -625,7 +625,7 @@ func FillOutSpecGen(s *specgen.SpecGenerator, c *ContainerCLIOpts, args []string
 			if retries < 0 {
 				return errors.Errorf("must specify restart policy retry count as a number greater than 0")
 			}
-			var retriesUint uint = uint(retries)
+			var retriesUint = uint(retries)
 			s.RestartRetries = &retriesUint
 		default:
 			return errors.Errorf("invalid restart policy: may specify retries at most once")

cmd/podman/inspect/inspect.go

@@ -3,12 +3,14 @@ package inspect
 import (
 	"context"
 	"fmt"
+	"os"
 	"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/sirupsen/logrus"
 	"github.com/spf13/cobra"
 )
 
@@ -78,6 +80,7 @@ func newInspector(options entities.InspectOptions) (*inspector, error) {
 func (i *inspector) inspect(namesOrIDs []string) error {
 	// data - dumping place for inspection results.
 	var data []interface{} //nolint
+	var errs []error
 	ctx := context.Background()
 
 	if len(namesOrIDs) == 0 {
@@ -97,24 +100,27 @@ func (i *inspector) inspect(namesOrIDs []string) error {
 	// Inspect - note that AllType requires us to expensively query one-by-one.
 	switch tmpType {
 	case AllType:
-		all, err := i.inspectAll(ctx, namesOrIDs)
+		allData, allErrs, err := i.inspectAll(ctx, namesOrIDs)
 		if err != nil {
 			return err
 		}
-		data = all
+		data = allData
+		errs = allErrs
 	case ImageType:
-		imgData, err := i.imageEngine.Inspect(ctx, namesOrIDs, i.options)
+		imgData, allErrs, err := i.imageEngine.Inspect(ctx, namesOrIDs, i.options)
 		if err != nil {
 			return err
 		}
+		errs = allErrs
 		for i := range imgData {
 			data = append(data, imgData[i])
 		}
 	case ContainerType:
-		ctrData, err := i.containerEngine.ContainerInspect(ctx, namesOrIDs, i.options)
+		ctrData, allErrs, err := i.containerEngine.ContainerInspect(ctx, namesOrIDs, i.options)
 		if err != nil {
 			return err
 		}
+		errs = allErrs
 		for i := range ctrData {
 			data = append(data, ctrData[i])
 		}
@@ -122,30 +128,54 @@ func (i *inspector) inspect(namesOrIDs []string) error {
 		return errors.Errorf("invalid type %q: must be %q, %q or %q", i.options.Type, ImageType, ContainerType, AllType)
 	}
 
+	// Always print an empty array
+	if data == nil {
+		data = []interface{}{}
+	}
+
 	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()
+	if err := out.Out(); err != nil {
+		logrus.Errorf("Error printing inspect output: %v", err)
+	}
+	if len(errs) > 0 {
+		if len(errs) > 1 {
+			for _, err := range errs[1:] {
+				fmt.Fprintf(os.Stderr, "error inspecting object: %v\n", err)
+			}
+		}
+		return errors.Errorf("error inspecting object: %v", errs[0])
+	}
+	return nil
 }
 
-func (i *inspector) inspectAll(ctx context.Context, namesOrIDs []string) ([]interface{}, error) {
+func (i *inspector) inspectAll(ctx context.Context, namesOrIDs []string) ([]interface{}, []error, error) {
 	var data []interface{} //nolint
+	allErrs := []error{}
 	for _, name := range namesOrIDs {
-		imgData, err := i.imageEngine.Inspect(ctx, []string{name}, i.options)
-		if err == nil {
-			data = append(data, imgData[0])
+		ctrData, errs, err := i.containerEngine.ContainerInspect(ctx, []string{name}, i.options)
+		if err != nil {
+			return nil, nil, err
+		}
+		if len(errs) == 0 {
+			data = append(data, ctrData[0])
 			continue
 		}
-		ctrData, err := i.containerEngine.ContainerInspect(ctx, []string{name}, i.options)
+		imgData, errs, err := i.imageEngine.Inspect(ctx, []string{name}, i.options)
 		if err != nil {
-			return nil, err
+			return nil, nil, err
 		}
-		data = append(data, ctrData[0])
+		if len(errs) > 0 {
+			allErrs = append(allErrs, errors.Errorf("no such object: %q", name))
+			continue
+		}
+		data = append(data, imgData[0])
 	}
-	return data, nil
+	return data, allErrs, nil
 }
 
 func inspectFormat(row string) string {

pkg/api/handlers/compat/images_build.go

@@ -40,7 +40,7 @@ func BuildImage(w http.ResponseWriter, r *http.Request) {
 		}
 	}
 
-	anchorDir, err := extractTarFile(r, w)
+	anchorDir, err := extractTarFile(r)
 	if err != nil {
 		utils.InternalServerError(w, err)
 		return
@@ -241,7 +241,7 @@ func BuildImage(w http.ResponseWriter, r *http.Request) {
 		})
 }
 
-func extractTarFile(r *http.Request, w http.ResponseWriter) (string, error) {
+func extractTarFile(r *http.Request) (string, error) {
 	// build a home for the request body
 	anchorDir, err := ioutil.TempDir("", "libpod_builder")
 	if err != nil {

pkg/domain/entities/engine_container.go

@@ -24,7 +24,7 @@ type ContainerEngine interface {
 	ContainerExists(ctx context.Context, nameOrID string) (*BoolReport, error)
 	ContainerExport(ctx context.Context, nameOrID string, options ContainerExportOptions) error
 	ContainerInit(ctx context.Context, namesOrIds []string, options ContainerInitOptions) ([]*ContainerInitReport, error)
-	ContainerInspect(ctx context.Context, namesOrIds []string, options InspectOptions) ([]*ContainerInspectReport, error)
+	ContainerInspect(ctx context.Context, namesOrIds []string, options InspectOptions) ([]*ContainerInspectReport, []error, error)
 	ContainerKill(ctx context.Context, namesOrIds []string, options KillOptions) ([]*KillReport, error)
 	ContainerList(ctx context.Context, options ContainerListOptions) ([]ListContainer, error)
 	ContainerLogs(ctx context.Context, containers []string, options ContainerLogsOptions) error

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, namesOrIDs []string, opts InspectOptions) ([]*ImageInspectReport, error)
+	Inspect(ctx context.Context, namesOrIDs []string, opts InspectOptions) ([]*ImageInspectReport, []error, 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/infra/abi/containers.go

@@ -338,20 +338,51 @@ func (ic *ContainerEngine) ContainerRm(ctx context.Context, namesOrIds []string,
 	return reports, nil
 }
 
-func (ic *ContainerEngine) ContainerInspect(ctx context.Context, namesOrIds []string, options entities.InspectOptions) ([]*entities.ContainerInspectReport, error) {
-	ctrs, err := getContainersByContext(false, options.Latest, namesOrIds, ic.Libpod)
-	if err != nil {
-		return nil, err
-	}
-	reports := make([]*entities.ContainerInspectReport, 0, len(ctrs))
-	for _, c := range ctrs {
-		data, err := c.Inspect(options.Size)
+func (ic *ContainerEngine) ContainerInspect(ctx context.Context, namesOrIds []string, options entities.InspectOptions) ([]*entities.ContainerInspectReport, []error, error) {
+	if options.Latest {
+		ctr, err := ic.Libpod.GetLatestContainer()
 		if err != nil {
-			return nil, err
+			if errors.Cause(err) == define.ErrNoSuchCtr {
+				return nil, []error{errors.Wrapf(err, "no containers to inspect")}, nil
+			}
+			return nil, nil, err
 		}
-		reports = append(reports, &entities.ContainerInspectReport{InspectContainerData: data})
+
+		inspect, err := ctr.Inspect(options.Size)
+		if err != nil {
+			return nil, nil, err
+		}
+
+		return []*entities.ContainerInspectReport{
+			{
+				InspectContainerData: inspect,
+			},
+		}, nil, nil
 	}
-	return reports, nil
+	var (
+		reports = make([]*entities.ContainerInspectReport, 0, len(namesOrIds))
+		errs    = []error{}
+	)
+	for _, name := range namesOrIds {
+		ctr, err := ic.Libpod.LookupContainer(name)
+		if err != nil {
+			// ErrNoSuchCtr is non-fatal, other errors will be
+			// treated as fatal.
+			if errors.Cause(err) == define.ErrNoSuchCtr {
+				errs = append(errs, errors.Errorf("no such container %s", name))
+				continue
+			}
+			return nil, nil, err
+		}
+
+		inspect, err := ctr.Inspect(options.Size)
+		if err != nil {
+			return nil, nil, err
+		}
+
+		reports = append(reports, &entities.ContainerInspectReport{InspectContainerData: inspect})
+	}
+	return reports, errs, nil
 }
 
 func (ic *ContainerEngine) ContainerTop(ctx context.Context, options entities.TopOptions) (*entities.StringSliceReport, error) {

pkg/domain/infra/abi/images.go

@@ -184,24 +184,28 @@ func (ir *ImageEngine) Pull(ctx context.Context, rawImage string, options entiti
 	return &entities.ImagePullReport{Images: foundIDs}, nil
 }
 
-func (ir *ImageEngine) Inspect(ctx context.Context, namesOrIDs []string, opts entities.InspectOptions) ([]*entities.ImageInspectReport, error) {
+func (ir *ImageEngine) Inspect(ctx context.Context, namesOrIDs []string, opts entities.InspectOptions) ([]*entities.ImageInspectReport, []error, error) {
 	reports := []*entities.ImageInspectReport{}
+	errs := []error{}
 	for _, i := range namesOrIDs {
 		img, err := ir.Libpod.ImageRuntime().NewFromLocal(i)
 		if err != nil {
-			return nil, err
+			// This is probably a no such image, treat as nonfatal.
+			errs = append(errs, err)
+			continue
 		}
 		result, err := img.Inspect(ctx)
 		if err != nil {
-			return nil, err
+			// This is more likely to be fatal.
+			return nil, nil, err
 		}
 		report := entities.ImageInspectReport{}
 		if err := domainUtils.DeepCopy(&report, result); err != nil {
-			return nil, err
+			return nil, nil, err
 		}
 		reports = append(reports, &report)
 	}
-	return reports, nil
+	return reports, errs, nil
 }
 
 func (ir *ImageEngine) Push(ctx context.Context, source string, destination string, options entities.ImagePushOptions) error {

pkg/domain/infra/tunnel/containers.go

@@ -185,20 +185,27 @@ func (ic *ContainerEngine) ContainerPrune(ctx context.Context, options entities.
 	return containers.Prune(ic.ClientCxt, options.Filters)
 }
 
-func (ic *ContainerEngine) ContainerInspect(ctx context.Context, namesOrIds []string, options entities.InspectOptions) ([]*entities.ContainerInspectReport, error) {
-	ctrs, err := getContainersByContext(ic.ClientCxt, false, namesOrIds)
-	if err != nil {
-		return nil, err
-	}
-	reports := make([]*entities.ContainerInspectReport, 0, len(ctrs))
-	for _, con := range ctrs {
-		data, err := containers.Inspect(ic.ClientCxt, con.ID, &options.Size)
+func (ic *ContainerEngine) ContainerInspect(ctx context.Context, namesOrIds []string, options entities.InspectOptions) ([]*entities.ContainerInspectReport, []error, error) {
+	var (
+		reports = make([]*entities.ContainerInspectReport, 0, len(namesOrIds))
+		errs    = []error{}
+	)
+	for _, name := range namesOrIds {
+		inspect, err := containers.Inspect(ic.ClientCxt, name, &options.Size)
 		if err != nil {
-			return nil, err
+			errModel, ok := err.(entities.ErrorModel)
+			if !ok {
+				return nil, nil, err
+			}
+			if errModel.ResponseCode == 404 {
+				errs = append(errs, errors.Errorf("no such container %q", name))
+				continue
+			}
+			return nil, nil, err
 		}
-		reports = append(reports, &entities.ContainerInspectReport{InspectContainerData: data})
+		reports = append(reports, &entities.ContainerInspectReport{InspectContainerData: inspect})
 	}
-	return reports, nil
+	return reports, errs, nil
 }
 
 func (ic *ContainerEngine) ContainerTop(ctx context.Context, options entities.TopOptions) (*entities.StringSliceReport, error) {

pkg/domain/infra/tunnel/images.go

@@ -157,16 +157,25 @@ func (ir *ImageEngine) Untag(ctx context.Context, nameOrID string, tags []string
 	return nil
 }
 
-func (ir *ImageEngine) Inspect(ctx context.Context, namesOrIDs []string, opts entities.InspectOptions) ([]*entities.ImageInspectReport, error) {
+func (ir *ImageEngine) Inspect(ctx context.Context, namesOrIDs []string, opts entities.InspectOptions) ([]*entities.ImageInspectReport, []error, error) {
 	reports := []*entities.ImageInspectReport{}
+	errs := []error{}
 	for _, i := range namesOrIDs {
 		r, err := images.GetImage(ir.ClientCxt, i, &opts.Size)
 		if err != nil {
-			return nil, err
+			errModel, ok := err.(entities.ErrorModel)
+			if !ok {
+				return nil, nil, err
+			}
+			if errModel.ResponseCode == 404 {
+				errs = append(errs, errors.Wrapf(err, "unable to inspect %q", i))
+				continue
+			}
+			return nil, nil, err
 		}
 		reports = append(reports, r)
 	}
-	return reports, nil
+	return reports, errs, nil
 }
 
 func (ir *ImageEngine) Load(ctx context.Context, opts entities.ImageLoadOptions) (*entities.ImageLoadReport, error) {

test/e2e/inspect_test.go

@@ -223,4 +223,44 @@ var _ = Describe("Podman inspect", func() {
 
 		Expect(baseJSON[0].ID).To(Equal(ctrJSON[0].ID))
 	})
+
+	It("podman inspect always produces a valid array", func() {
+		baseInspect := podmanTest.Podman([]string{"inspect", "doesNotExist"})
+		baseInspect.WaitWithDefaultTimeout()
+		Expect(baseInspect.ExitCode()).To(Not(Equal(0)))
+		emptyJSON := baseInspect.InspectContainerToJSON()
+		Expect(len(emptyJSON)).To(Equal(0))
+	})
+
+	It("podman inspect one container with not exist returns 1-length valid array", func() {
+		ctrName := "testCtr"
+		create := podmanTest.Podman([]string{"create", "--name", ctrName, ALPINE, "sh"})
+		create.WaitWithDefaultTimeout()
+		Expect(create.ExitCode()).To(Equal(0))
+
+		baseInspect := podmanTest.Podman([]string{"inspect", ctrName, "doesNotExist"})
+		baseInspect.WaitWithDefaultTimeout()
+		Expect(baseInspect.ExitCode()).To(Not(Equal(0)))
+		baseJSON := baseInspect.InspectContainerToJSON()
+		Expect(len(baseJSON)).To(Equal(1))
+		Expect(baseJSON[0].Name).To(Equal(ctrName))
+	})
+
+	It("podman inspect container + image with same name gives container", func() {
+		ctrName := "testcontainer"
+		create := podmanTest.PodmanNoCache([]string{"create", "--name", ctrName, ALPINE, "sh"})
+		create.WaitWithDefaultTimeout()
+		Expect(create.ExitCode()).To(Equal(0))
+
+		tag := podmanTest.PodmanNoCache([]string{"tag", ALPINE, ctrName + ":latest"})
+		tag.WaitWithDefaultTimeout()
+		Expect(tag.ExitCode()).To(Equal(0))
+
+		baseInspect := podmanTest.Podman([]string{"inspect", ctrName})
+		baseInspect.WaitWithDefaultTimeout()
+		Expect(baseInspect.ExitCode()).To(Equal(0))
+		baseJSON := baseInspect.InspectContainerToJSON()
+		Expect(len(baseJSON)).To(Equal(1))
+		Expect(baseJSON[0].Name).To(Equal(ctrName))
+	})
 })