Merge pull request #7908 from rhatdan/diff

fix podman container exists and diff for storage containers

7 files changed, 58 insertions, 17 deletions

diff --git a/pkg/api/handlers/libpod/containers.go b/pkg/api/handlers/libpod/containers.go
index 7dde51102..7e6481321 100644
--- a/pkg/api/handlers/libpod/containers.go
+++ b/pkg/api/handlers/libpod/containers.go
@@ -11,6 +11,7 @@ import (
 	"github.com/containers/podman/v2/pkg/api/handlers/compat"
 	"github.com/containers/podman/v2/pkg/api/handlers/utils"
 	"github.com/containers/podman/v2/pkg/domain/entities"
+	"github.com/containers/podman/v2/pkg/domain/infra/abi"
 	"github.com/containers/podman/v2/pkg/ps"
 	"github.com/gorilla/schema"
 	"github.com/pkg/errors"
@@ -18,9 +19,30 @@ import (
 )
 
 func ContainerExists(w http.ResponseWriter, r *http.Request) {
+	decoder := r.Context().Value("decoder").(*schema.Decoder)
 	runtime := r.Context().Value("runtime").(*libpod.Runtime)
+	// Now use the ABI implementation to prevent us from having duplicate
+	// code.
+	containerEngine := abi.ContainerEngine{Libpod: runtime}
+
 	name := utils.GetName(r)
-	_, err := runtime.LookupContainer(name)
+	query := struct {
+		External bool `schema:"external"`
+	}{
+		// override any golang type defaults
+	}
+
+	if err := decoder.Decode(&query, r.URL.Query()); err != nil {
+		utils.Error(w, http.StatusText(http.StatusBadRequest), http.StatusBadRequest,
+			errors.Wrapf(err, "failed to parse parameters for %s", r.URL.String()))
+		return
+	}
+
+	options := entities.ContainerExistsOptions{
+		External: query.External,
+	}
+
+	report, err := containerEngine.ContainerExists(r.Context(), name, options)
 	if err != nil {
 		if errors.Cause(err) == define.ErrNoSuchCtr {
 			utils.ContainerNotFound(w, name, err)
@@ -30,7 +52,11 @@ func ContainerExists(w http.ResponseWriter, r *http.Request) {
 		return
 
 	}
-	utils.WriteResponse(w, http.StatusNoContent, "")
+	if report.Value {
+		utils.WriteResponse(w, http.StatusNoContent, "")
+	} else {
+		utils.ContainerNotFound(w, name, define.ErrNoSuchCtr)
+	}
 }
 
 func ListContainers(w http.ResponseWriter, r *http.Request) {
diff --git a/pkg/bindings/containers/containers.go b/pkg/bindings/containers/containers.go
index 708ad06cb..b5cd2128b 100644
--- a/pkg/bindings/containers/containers.go
+++ b/pkg/bindings/containers/containers.go
@@ -322,12 +322,14 @@ func Wait(ctx context.Context, nameOrID string, condition *define.ContainerStatu
 // Exists is a quick, light-weight way to determine if a given container
 // exists in local storage.  The nameOrID can be a container name
 // or a partial/full ID.
-func Exists(ctx context.Context, nameOrID string) (bool, error) {
+func Exists(ctx context.Context, nameOrID string, external bool) (bool, error) {
 	conn, err := bindings.GetClient(ctx)
 	if err != nil {
 		return false, err
 	}
-	response, err := conn.DoRequest(nil, http.MethodGet, "/containers/%s/exists", nil, nil, nameOrID)
+	params := url.Values{}
+	params.Set("external", strconv.FormatBool(external))
+	response, err := conn.DoRequest(nil, http.MethodGet, "/containers/%s/exists", params, nil, nameOrID)
 	if err != nil {
 		return false, err
 	}
diff --git a/pkg/bindings/test/containers_test.go b/pkg/bindings/test/containers_test.go
index 408b4769d..0fb677768 100644
--- a/pkg/bindings/test/containers_test.go
+++ b/pkg/bindings/test/containers_test.go
@@ -405,7 +405,7 @@ var _ = Describe("Podman containers ", func() {
 
 	It("podman bogus container does not exist in local storage", func() {
 		// Bogus container existence check should fail
-		containerExists, err := containers.Exists(bt.conn, "foobar")
+		containerExists, err := containers.Exists(bt.conn, "foobar", false)
 		Expect(err).To(BeNil())
 		Expect(containerExists).To(BeFalse())
 	})
@@ -415,7 +415,7 @@ var _ = Describe("Podman containers ", func() {
 		var name = "top"
 		_, err := bt.RunTopContainer(&name, bindings.PFalse, nil)
 		Expect(err).To(BeNil())
-		containerExists, err := containers.Exists(bt.conn, name)
+		containerExists, err := containers.Exists(bt.conn, name, false)
 		Expect(err).To(BeNil())
 		Expect(containerExists).To(BeTrue())
 	})
@@ -425,7 +425,7 @@ var _ = Describe("Podman containers ", func() {
 		var name = "top"
 		cid, err := bt.RunTopContainer(&name, bindings.PFalse, nil)
 		Expect(err).To(BeNil())
-		containerExists, err := containers.Exists(bt.conn, cid)
+		containerExists, err := containers.Exists(bt.conn, cid, false)
 		Expect(err).To(BeNil())
 		Expect(containerExists).To(BeTrue())
 	})
@@ -435,7 +435,7 @@ var _ = Describe("Podman containers ", func() {
 		var name = "top"
 		cid, err := bt.RunTopContainer(&name, bindings.PFalse, nil)
 		Expect(err).To(BeNil())
-		containerExists, err := containers.Exists(bt.conn, cid[0:12])
+		containerExists, err := containers.Exists(bt.conn, cid[0:12], false)
 		Expect(err).To(BeNil())
 		Expect(containerExists).To(BeTrue())
 	})
@@ -455,7 +455,7 @@ var _ = Describe("Podman containers ", func() {
 		Expect(err).To(BeNil())
 		err = containers.Kill(bt.conn, name, "SIGINT")
 		Expect(err).To(BeNil())
-		_, err = containers.Exists(bt.conn, name)
+		_, err = containers.Exists(bt.conn, name, false)
 		Expect(err).To(BeNil())
 	})
 
@@ -466,7 +466,7 @@ var _ = Describe("Podman containers ", func() {
 		Expect(err).To(BeNil())
 		err = containers.Kill(bt.conn, cid, "SIGTERM")
 		Expect(err).To(BeNil())
-		_, err = containers.Exists(bt.conn, cid)
+		_, err = containers.Exists(bt.conn, cid, false)
 		Expect(err).To(BeNil())
 	})
 
diff --git a/pkg/domain/entities/containers.go b/pkg/domain/entities/containers.go
index 3b6dd106f..46b169284 100644
--- a/pkg/domain/entities/containers.go
+++ b/pkg/domain/entities/containers.go
@@ -246,6 +246,11 @@ type ExecOptions struct {
 	WorkDir     string
 }
 
+// ContainerExistsOptions describes the cli values to check if a container exists
+type ContainerExistsOptions struct {
+	External bool
+}
+
 // ContainerStartOptions describes the val from the
 // CLI needed to start a container
 type ContainerStartOptions struct {
diff --git a/pkg/domain/entities/engine_container.go b/pkg/domain/entities/engine_container.go
index 803a59932..a20d3b404 100644
--- a/pkg/domain/entities/engine_container.go
+++ b/pkg/domain/entities/engine_container.go
@@ -21,7 +21,7 @@ type ContainerEngine interface {
 	ContainerDiff(ctx context.Context, nameOrID string, options DiffOptions) (*DiffReport, error)
 	ContainerExec(ctx context.Context, nameOrID string, options ExecOptions, streams define.AttachStreams) (int, error)
 	ContainerExecDetached(ctx context.Context, nameOrID string, options ExecOptions) (string, error)
-	ContainerExists(ctx context.Context, nameOrID string) (*BoolReport, error)
+	ContainerExists(ctx context.Context, nameOrID string, options ContainerExistsOptions) (*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, error)
diff --git a/pkg/domain/infra/abi/containers.go b/pkg/domain/infra/abi/containers.go
index 614fd5fe0..60dbbce6c 100644
--- a/pkg/domain/infra/abi/containers.go
+++ b/pkg/domain/infra/abi/containers.go
@@ -74,11 +74,19 @@ func getContainersByContext(all, latest bool, names []string, runtime *libpod.Ru
 	return
 }
 
-// TODO: Should return *entities.ContainerExistsReport, error
-func (ic *ContainerEngine) ContainerExists(ctx context.Context, nameOrID string) (*entities.BoolReport, error) {
+// ContainerExists returns whether the container exists in container storage
+func (ic *ContainerEngine) ContainerExists(ctx context.Context, nameOrID string, options entities.ContainerExistsOptions) (*entities.BoolReport, error) {
 	_, err := ic.Libpod.LookupContainer(nameOrID)
-	if err != nil && errors.Cause(err) != define.ErrNoSuchCtr {
-		return nil, err
+	if err != nil {
+		if errors.Cause(err) != define.ErrNoSuchCtr {
+			return nil, err
+		}
+		if options.External {
+			// Check if container exists in storage
+			if _, storageErr := ic.Libpod.StorageContainer(nameOrID); storageErr == nil {
+				err = nil
+			}
+		}
 	}
 	return &entities.BoolReport{Value: err == nil}, nil
 }
diff --git a/pkg/domain/infra/tunnel/containers.go b/pkg/domain/infra/tunnel/containers.go
index eb591d23d..7913d79cd 100644
--- a/pkg/domain/infra/tunnel/containers.go
+++ b/pkg/domain/infra/tunnel/containers.go
@@ -29,8 +29,8 @@ func (ic *ContainerEngine) ContainerRunlabel(ctx context.Context, label string,
 	return errors.New("not implemented")
 }
 
-func (ic *ContainerEngine) ContainerExists(ctx context.Context, nameOrID string) (*entities.BoolReport, error) {
-	exists, err := containers.Exists(ic.ClientCxt, nameOrID)
+func (ic *ContainerEngine) ContainerExists(ctx context.Context, nameOrID string, options entities.ContainerExistsOptions) (*entities.BoolReport, error) {
+	exists, err := containers.Exists(ic.ClientCxt, nameOrID, options.External)
 	return &entities.BoolReport{Value: exists}, err
 }