nerdctl rm -f exits 0 when task deletion fails, so callers cannot detect a failed removal #5288

Description

nerdctl rm -f exits 0 even when the removal actually failed. In
pkg/cmd/container/remove.go:87-91 (current main, 127dce535ff237bebdeab638d272fcfb9813d744),
every error from WalkAll is logged and converted to nil in force mode:

err := walker.WalkAll(ctx, containers, true)
if err != nil && options.Force {
    log.G(ctx).Error(err)
    return nil
}
return err

A caller therefore cannot distinguish "removed" from "printed an error and left the container
behind"; it has to list the containers again to find out.

This is separate from the idempotent -f case in #2400 (removing a non-existent image): here an
existing container is present and its deletion genuinely fails.

Steps to reproduce the issue

Measured with nerdctl 2.4.1, containerd v2.4.2 and crun 1.30.1 on Arch Linux (btrfs root), with
the rootful daemon and the crun runtime:

1. Start a container under crun:

   sudo nerdctl run -d --name up-rmrepro3 --runtime /usr/bin/crun \
     --network none alpine:latest sleep 600

2. Remove its runtime state directory, which is what happens after nerdctl checkpoint create
   on this host (the container is killed with 137 and the containerd task record stays):

   id=$(sudo nerdctl inspect -f '{{.Id}}' up-rmrepro3)
   sudo kill -9 $(sudo nerdctl inspect -f '{{.State.Pid}}' up-rmrepro3)
   sleep 3
   sudo ctr -n default task ls | grep "$id"   # -> STOPPED
   sudo rm -rf /run/containerd/runc/default/$id

3. Ask nerdctl to remove it:

   sudo nerdctl rm -f up-rmrepro3; echo "rc=$?"

Describe the results you received and expected

Actual (the timestamp is from the run; the container id is shortened here only in the prose):

time="2026-10-24T07:19:42+02:00" level=error msg="1 errors:\nfailed to delete task: failed to delete task: cannot open directory `/run/containerd/runc/default/909ae9f8978e00262fe637fba4f6df7aa6b920caa42a7d114d1701262f94cd7c`: No such file or directory"
rc=0

$ sudo nerdctl ps -a --format '{{.Names}} {{.Status}}'
up-rmrepro3 Exited (137) 3 seconds ago

The error is printed, the exit status is 0, and the container is still there. ctr task rm
with the same underlying error exits 1, so the failure is available to nerdctl; it is discarded
at remove.go:90.

The handler matters only because it produces the error: runc tolerates the missing state
directory and removes the container cleanly, while crun's delete fails. nerdctl's force path
then turns that failure into success for either handler.

Expected: rm -f should remain silent and exit 0 for a container that does not exist, but a
real failure to delete an existing container must be reflected in the exit status (or at least
be distinguishable from success).

What version of nerdctl are you using?

$ nerdctl --version
nerdctl version 2.4.1
$ containerd --version
containerd github.com/containerd/containerd/v2 v2.4.2 b7a25261e029d68978bbf290df0a62fc819b3c51.m
$ crun --version
crun version 1.30.1

The same code is on main at 127dce535ff237bebdeab638d272fcfb9813d744
(pkg/cmd/container/remove.go:87-91).

Are you using a variant of nerdctl? (e.g., Rancher Desktop)

None.

Why this matters

A container-backed lease manager (the consumer that found this) deletes a lease and trusts the
exit status. When the delete fails this way, the container and its task record remain; the next
operation fails with a different error (task <id>: already exists), which masks the real
one. The consumer's only working defense today is to re-list after every delete.

Proposed patch

Two small changes keep force mode idempotent for an absent container while propagating a real
failure.

1. Preserve error identity in pkg/idutil/containerwalker/containerwalker.go:83-101.
   WalkAll currently collects []string and joins formatted text, so errdefs.IsNotFound
   can never match the aggregate. Collect []error instead and use the wrapping pattern
   already present in pkg/cmd/image/inspect.go:192:

   var errs []error
   // ...
   errs = append(errs, err)
   // ...
   if len(errs) > 0 {
       return fmt.Errorf("%d errors: %w", len(errs), errors.Join(errs...))
   }

   and mark the "no such container" case as NotFound rather than a plain string:

   if err == nil && n == 0 {
       err = fmt.Errorf("no such container: %s: %w", req, errdefs.ErrNotFound)
   }

2. In Remove, swallow the aggregate only when every entry is NotFound (errors.Is is
   "any", so a mixed batch must not be treated as success):

   err := walker.WalkAll(ctx, containers, true)
   if err != nil && options.Force {
       log.G(ctx).Error(err)
       if allNotFound(err) {
           return nil
       }
       return err
   }
   return err

   with a small helper that walks the wrapped tree:

   func allNotFound(err error) bool {
       if err == nil {
           return false
       }
       switch e := err.(type) {
       case interface{ Unwrap() []error }:
           errs := e.Unwrap()
           if len(errs) == 0 {
               return false
           }
           for _, child := range errs {
               if !allNotFound(child) {
                   return false
               }
           }
           return true
       case interface{ Unwrap() error }:
           return allNotFound(e.Unwrap())
       default:
           return errors.Is(err, errdefs.ErrNotFound)
       }
   }

A regression test can create a task, remove its runtime state directory, and assert a non-zero
status (and that the container is gone).

What we do instead today

After every nerdctl rm -f, list the container again (nerdctl ps -a / ctr task ls); if it
survives, kill the shim, then ctr task rm and ctr containers rm. That cleanup works, but it
exists only because the status code cannot be trusted.

Agent: containerd-crabbox-program/upstream · session 8b1c01cf-7f79-4a75-bff8-240415a06208
Assisted-by: Claude Code deepseek-v4.1-flash:cloud (analysed on a live system)