From f1eb8e816257d9dc810cfa6957e09db1ffa7db96 Mon Sep 17 00:00:00 2001 From: Daniel J Walsh Date: Tue, 9 Mar 2021 16:39:01 -0500 Subject: Removing a non existing container API should return 404 Currently we were overwrapping error returned from removal of a non existing container. $ podman rm bogus -f Error: failed to evict container: "": failed to find container "bogus" in state: no container with name or ID bogus found: no such container Removal of wraps gets us to. ./bin/podman rm bogus -f Error: no container with name or ID "bogus" found: no such container Finally also added quotes around container name to help make it standout when you get an error, currently it gets lost in the error. Signed-off-by: Daniel J Walsh --- libpod/boltdb_state.go | 6 +++--- libpod/boltdb_state_internal.go | 4 ++-- libpod/runtime_ctr.go | 4 ++-- pkg/api/handlers/compat/containers.go | 7 ++++++- pkg/domain/infra/abi/containers.go | 4 ++-- test/apiv2/20-containers.at | 1 + test/system/050-stop.bats | 2 +- 7 files changed, 17 insertions(+), 11 deletions(-) diff --git a/libpod/boltdb_state.go b/libpod/boltdb_state.go index 122dd080f..5df3e8961 100644 --- a/libpod/boltdb_state.go +++ b/libpod/boltdb_state.go @@ -879,7 +879,7 @@ func (s *BoltState) ContainerInUse(ctr *Container) ([]string, error) { ctrDB := ctrBucket.Bucket([]byte(ctr.ID())) if ctrDB == nil { ctr.valid = false - return errors.Wrapf(define.ErrNoSuchCtr, "no container with ID %s found in DB", ctr.ID()) + return errors.Wrapf(define.ErrNoSuchCtr, "no container with ID %q found in DB", ctr.ID()) } dependsBkt := ctrDB.Bucket(dependenciesBkt) @@ -1669,7 +1669,7 @@ func (s *BoltState) RewriteContainerConfig(ctr *Container, newCfg *ContainerConf ctrDB := ctrBkt.Bucket([]byte(ctr.ID())) if ctrDB == nil { ctr.valid = false - return errors.Wrapf(define.ErrNoSuchCtr, "no container with ID %s found in DB", ctr.ID()) + return errors.Wrapf(define.ErrNoSuchCtr, "no container with ID %q found in DB", ctr.ID()) } if err := ctrDB.Put(configKey, newCfgJSON); err != nil { @@ -1767,7 +1767,7 @@ func (s *BoltState) SafeRewriteContainerConfig(ctr *Container, oldName, newName ctrDB := ctrBkt.Bucket([]byte(ctr.ID())) if ctrDB == nil { ctr.valid = false - return errors.Wrapf(define.ErrNoSuchCtr, "no container with ID %s found in DB", ctr.ID()) + return errors.Wrapf(define.ErrNoSuchCtr, "no container with ID %q found in DB", ctr.ID()) } if err := ctrDB.Put(configKey, newCfgJSON); err != nil { diff --git a/libpod/boltdb_state_internal.go b/libpod/boltdb_state_internal.go index cf8f1c175..d4994334f 100644 --- a/libpod/boltdb_state_internal.go +++ b/libpod/boltdb_state_internal.go @@ -1055,9 +1055,9 @@ func (s *BoltState) lookupContainerID(idOrName string, ctrBucket, namesBucket, n return nil, err } else if !exists { if isPod { - return nil, errors.Wrapf(define.ErrNoSuchCtr, "%s is a pod, not a container", idOrName) + return nil, errors.Wrapf(define.ErrNoSuchCtr, "%q is a pod, not a container", idOrName) } - return nil, errors.Wrapf(define.ErrNoSuchCtr, "no container with name or ID %s found", idOrName) + return nil, errors.Wrapf(define.ErrNoSuchCtr, "no container with name or ID %q found", idOrName) } return id, nil } diff --git a/libpod/runtime_ctr.go b/libpod/runtime_ctr.go index 661ca7ff8..19690d79b 100644 --- a/libpod/runtime_ctr.go +++ b/libpod/runtime_ctr.go @@ -714,7 +714,7 @@ func (r *Runtime) evictContainer(ctx context.Context, idOrName string, removeVol id, err := r.state.LookupContainerID(idOrName) if err != nil { - return "", errors.Wrapf(err, "failed to find container %q in state", idOrName) + return "", err } // Begin by trying a normal removal. Valid containers will be removed normally. @@ -744,7 +744,7 @@ func (r *Runtime) evictContainer(ctx context.Context, idOrName string, removeVol return id, err } if !exists { - return id, errors.Wrapf(err, "failed to find container ID %q for eviction", id) + return id, err } // Re-create a container struct for removal purposes diff --git a/pkg/api/handlers/compat/containers.go b/pkg/api/handlers/compat/containers.go index d26bb50f4..d3277b815 100644 --- a/pkg/api/handlers/compat/containers.go +++ b/pkg/api/handlers/compat/containers.go @@ -76,7 +76,12 @@ func RemoveContainer(w http.ResponseWriter, r *http.Request) { return } if len(report) > 0 && report[0].Err != nil { - utils.InternalServerError(w, report[0].Err) + err = report[0].Err + if errors.Cause(err) == define.ErrNoSuchCtr { + utils.ContainerNotFound(w, name, err) + return + } + utils.InternalServerError(w, err) return } diff --git a/pkg/domain/infra/abi/containers.go b/pkg/domain/infra/abi/containers.go index 4790bd58c..637531ee9 100644 --- a/pkg/domain/infra/abi/containers.go +++ b/pkg/domain/infra/abi/containers.go @@ -301,14 +301,14 @@ func (ic *ContainerEngine) ContainerRm(ctx context.Context, namesOrIds []string, for _, ctr := range names { logrus.Debugf("Evicting container %q", ctr) report := entities.RmReport{Id: ctr} - id, err := ic.Libpod.EvictContainer(ctx, ctr, options.Volumes) + _, err := ic.Libpod.EvictContainer(ctx, ctr, options.Volumes) if err != nil { if options.Ignore && errors.Cause(err) == define.ErrNoSuchCtr { logrus.Debugf("Ignoring error (--allow-missing): %v", err) reports = append(reports, &report) continue } - report.Err = errors.Wrapf(err, "failed to evict container: %q", id) + report.Err = err reports = append(reports, &report) continue } diff --git a/test/apiv2/20-containers.at b/test/apiv2/20-containers.at index f73d03123..383d92ef3 100644 --- a/test/apiv2/20-containers.at +++ b/test/apiv2/20-containers.at @@ -162,6 +162,7 @@ t DELETE images/localhost/newrepo:v1?force=true 200 t DELETE images/localhost/newrepo:v2?force=true 200 t DELETE libpod/containers/$cid 204 t DELETE libpod/containers/myctr 204 +t DELETE libpod/containers/bogus 404 # test apiv2 create container with correct entrypoint and cmd diff --git a/test/system/050-stop.bats b/test/system/050-stop.bats index 7d9f1fcb3..0652a97e4 100644 --- a/test/system/050-stop.bats +++ b/test/system/050-stop.bats @@ -66,7 +66,7 @@ load helpers name=thiscontainerdoesnotexist run_podman 125 stop $name is "$output" \ - "Error: no container with name or ID $name found: no such container" \ + "Error: no container with name or ID \"$name\" found: no such container" \ "podman stop nonexistent container" run_podman stop --ignore $name -- cgit v1.2.3-54-g00ecf