Merge pull request #6583 from mheon/inspect_ctr_before_img

Fix podman inspect on overlapping/missing objects

9 files changed, 166 insertions, 45 deletions

diff --git a/cmd/podman/common/specgen.go b/cmd/podman/common/specgen.go
index c8d9888b7..0e4747954 100644
--- a/cmd/podman/common/specgen.go
+++ b/cmd/podman/common/specgen.go
@@ -627,7 +627,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")
diff --git a/cmd/podman/inspect/inspect.go b/cmd/podman/inspect/inspect.go
index 1ed033ec3..d80bbffdd 100644
--- a/cmd/podman/inspect/inspect.go
+++ b/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
+		}
+		if len(errs) > 0 {
+			allErrs = append(allErrs, errors.Errorf("no such object: %q", name))
+			continue
 		}
-		data = append(data, ctrData[0])
+		data = append(data, imgData[0])
 	}
-	return data, nil
+	return data, allErrs, nil
 }
 
 func inspectFormat(row string) string {
diff --git a/pkg/domain/entities/engine_container.go b/pkg/domain/entities/engine_container.go
index 979df7581..837550a2e 100644
--- a/pkg/domain/entities/engine_container.go
+++ b/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
diff --git a/pkg/domain/entities/engine_image.go b/pkg/domain/entities/engine_image.go
index 60fb20b6e..7ece24c60 100644
--- a/pkg/domain/entities/engine_image.go
+++ b/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)
diff --git a/pkg/domain/infra/abi/containers.go b/pkg/domain/infra/abi/containers.go
index d2c8aefdc..19512ddf1 100644
--- a/pkg/domain/infra/abi/containers.go
+++ b/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
+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 {
+			if errors.Cause(err) == define.ErrNoSuchCtr {
+				return nil, []error{errors.Wrapf(err, "no containers to inspect")}, nil
+			}
+			return nil, nil, err
+		}
+
+		inspect, err := ctr.Inspect(options.Size)
+		if err != nil {
+			return nil, nil, err
+		}
+
+		return []*entities.ContainerInspectReport{
+			{
+				InspectContainerData: inspect,
+			},
+		}, nil, nil
 	}
-	reports := make([]*entities.ContainerInspectReport, 0, len(ctrs))
-	for _, c := range ctrs {
-		data, err := c.Inspect(options.Size)
+	var (
+		reports = make([]*entities.ContainerInspectReport, 0, len(namesOrIds))
+		errs    = []error{}
+	)
+	for _, name := range namesOrIds {
+		ctr, err := ic.Libpod.LookupContainer(name)
 		if err != nil {
-			return nil, err
+			// 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
 		}
-		reports = append(reports, &entities.ContainerInspectReport{InspectContainerData: data})
+
+		inspect, err := ctr.Inspect(options.Size)
+		if err != nil {
+			return nil, nil, err
+		}
+
+		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) {
diff --git a/pkg/domain/infra/abi/images.go b/pkg/domain/infra/abi/images.go
index e630d9bc8..0f9ddfec4 100644
--- a/pkg/domain/infra/abi/images.go
+++ b/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 {
diff --git a/pkg/domain/infra/tunnel/containers.go b/pkg/domain/infra/tunnel/containers.go
index 8f6f5c8b7..5f9e2bdd2 100644
--- a/pkg/domain/infra/tunnel/containers.go
+++ b/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) {
diff --git a/pkg/domain/infra/tunnel/images.go b/pkg/domain/infra/tunnel/images.go
index fc7ac0aa8..9ddc5f1a9 100644
--- a/pkg/domain/infra/tunnel/images.go
+++ b/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) {
diff --git a/test/e2e/inspect_test.go b/test/e2e/inspect_test.go
index 62f69f1c1..2fad38a36 100644
--- a/test/e2e/inspect_test.go
+++ b/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))
+	})
 })