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 <[email protected]>
containers · Mar 10, 2021 · f1eb8e8 · f1eb8e8
1 parent 09473d4
commit f1eb8e8
Show file tree

Hide file tree

Showing 7 changed files with 17 additions and 11 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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