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 <dwalsh@redhat.com>
This commit is contained in:
Daniel J Walsh
2021-03-09 16:39:01 -05:00
parent 09473d4300
commit f1eb8e8162
7 changed files with 17 additions and 11 deletions

View File

@ -879,7 +879,7 @@ func (s *BoltState) ContainerInUse(ctr *Container) ([]string, error) {
ctrDB := ctrBucket.Bucket([]byte(ctr.ID())) ctrDB := ctrBucket.Bucket([]byte(ctr.ID()))
if ctrDB == nil { if ctrDB == nil {
ctr.valid = false 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) dependsBkt := ctrDB.Bucket(dependenciesBkt)
@ -1669,7 +1669,7 @@ func (s *BoltState) RewriteContainerConfig(ctr *Container, newCfg *ContainerConf
ctrDB := ctrBkt.Bucket([]byte(ctr.ID())) ctrDB := ctrBkt.Bucket([]byte(ctr.ID()))
if ctrDB == nil { if ctrDB == nil {
ctr.valid = false 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 { 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())) ctrDB := ctrBkt.Bucket([]byte(ctr.ID()))
if ctrDB == nil { if ctrDB == nil {
ctr.valid = false 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 { if err := ctrDB.Put(configKey, newCfgJSON); err != nil {

View File

@ -1055,9 +1055,9 @@ func (s *BoltState) lookupContainerID(idOrName string, ctrBucket, namesBucket, n
return nil, err return nil, err
} else if !exists { } else if !exists {
if isPod { 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 return id, nil
} }

View File

@ -714,7 +714,7 @@ func (r *Runtime) evictContainer(ctx context.Context, idOrName string, removeVol
id, err := r.state.LookupContainerID(idOrName) id, err := r.state.LookupContainerID(idOrName)
if err != nil { 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. // 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 return id, err
} }
if !exists { 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 // Re-create a container struct for removal purposes

View File

@ -76,7 +76,12 @@ func RemoveContainer(w http.ResponseWriter, r *http.Request) {
return return
} }
if len(report) > 0 && report[0].Err != nil { 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 return
} }

View File

@ -301,14 +301,14 @@ func (ic *ContainerEngine) ContainerRm(ctx context.Context, namesOrIds []string,
for _, ctr := range names { for _, ctr := range names {
logrus.Debugf("Evicting container %q", ctr) logrus.Debugf("Evicting container %q", ctr)
report := entities.RmReport{Id: 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 err != nil {
if options.Ignore && errors.Cause(err) == define.ErrNoSuchCtr { if options.Ignore && errors.Cause(err) == define.ErrNoSuchCtr {
logrus.Debugf("Ignoring error (--allow-missing): %v", err) logrus.Debugf("Ignoring error (--allow-missing): %v", err)
reports = append(reports, &report) reports = append(reports, &report)
continue continue
} }
report.Err = errors.Wrapf(err, "failed to evict container: %q", id) report.Err = err
reports = append(reports, &report) reports = append(reports, &report)
continue continue
} }

View File

@ -162,6 +162,7 @@ t DELETE images/localhost/newrepo:v1?force=true 200
t DELETE images/localhost/newrepo:v2?force=true 200 t DELETE images/localhost/newrepo:v2?force=true 200
t DELETE libpod/containers/$cid 204 t DELETE libpod/containers/$cid 204
t DELETE libpod/containers/myctr 204 t DELETE libpod/containers/myctr 204
t DELETE libpod/containers/bogus 404
# test apiv2 create container with correct entrypoint and cmd # test apiv2 create container with correct entrypoint and cmd

View File

@ -66,7 +66,7 @@ load helpers
name=thiscontainerdoesnotexist name=thiscontainerdoesnotexist
run_podman 125 stop $name run_podman 125 stop $name
is "$output" \ 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" "podman stop nonexistent container"
run_podman stop --ignore $name run_podman stop --ignore $name