From cfaaf1167c7a42f9d915b21b37b98d8bf9d69f76 Mon Sep 17 00:00:00 2001
From: sarankumar1325 <120080287+sarankumar1325@users.noreply.github.com>
Date: Sat, 24 Oct 2026 10:56:39 +0530
Subject: [PATCH] fix: return removal failures from rm -f

`nerdctl rm -f` logged every error from the container walker and exited 0,
so a container whose task deletion failed was left behind while the
command reported success.

In force mode, only ignore requests that match no container, which keeps
`rm -f` idempotent, and return any real removal failure.

Fixes #5288

Assisted-by: Grok Bot
Signed-off-by: sarankumar1325 <120080287+sarankumar1325@users.noreply.github.com>
---
 pkg/cmd/container/remove.go      | 31 +++++++++++++---
 pkg/cmd/container/remove_test.go | 64 ++++++++++++++++++++++++++++++++
 2 files changed, 90 insertions(+), 5 deletions(-)
 create mode 100644 pkg/cmd/container/remove_test.go

diff --git a/pkg/cmd/container/remove.go b/pkg/cmd/container/remove.go
index f946c91122c..99d6fd15ecf 100644
--- a/pkg/cmd/container/remove.go
+++ b/pkg/cmd/container/remove.go
@@ -22,6 +22,7 @@ import (
 	"errors"
 	"fmt"
 	"os"
+	"strings"
 	"syscall"
 
 	containerd "github.com/containerd/containerd/v2/client"
@@ -84,12 +85,32 @@ func Remove(ctx context.Context, client *containerd.Client, containers []string,
 		},
 	}
 
-	err := walker.WalkAll(ctx, containers, true)
-	if err != nil && options.Force {
-		log.G(ctx).Error(err)
-		return nil
+	return removeAll(ctx, containers, walker.Walk, options.Force)
+}
+
+// removeAll calls walk for every request in reqs and returns all errors joined by `\n`.
+// In force mode, a request that matches no container is only logged, so that
+// `rm -f` stays idempotent, but a failure to remove an existing container is still
+// returned to the caller.
+func removeAll(ctx context.Context, reqs []string, walk func(context.Context, string) (int, error), force bool) error {
+	var errs []string
+	for _, req := range reqs {
+		n, err := walk(ctx, req)
+		if err == nil && n == 0 {
+			err = fmt.Errorf("no such container: %s", req)
+			if force {
+				log.G(ctx).Error(err)
+				continue
+			}
+		}
+		if err != nil {
+			errs = append(errs, err.Error())
+		}
 	}
-	return err
+	if len(errs) > 0 {
+		return fmt.Errorf("%d errors:\n%s", len(errs), strings.Join(errs, "\n"))
+	}
+	return nil
 }
 
 // RemoveContainer removes a container from containerd store.
diff --git a/pkg/cmd/container/remove_test.go b/pkg/cmd/container/remove_test.go
new file mode 100644
index 00000000000..3611c8db77f
--- /dev/null
+++ b/pkg/cmd/container/remove_test.go
@@ -0,0 +1,64 @@
+/*
+   Copyright The containerd Authors.
+
+   Licensed under the Apache License, Version 2.0 (the "License");
+   you may not use this file except in compliance with the License.
+   You may obtain a copy of the License at
+
+       http://www.apache.org/licenses/LICENSE-2.0
+
+   Unless required by applicable law or agreed to in writing, software
+   distributed under the License is distributed on an "AS IS" BASIS,
+   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+   See the License for the specific language governing permissions and
+   limitations under the License.
+*/
+
+package container
+
+import (
+	"context"
+	"errors"
+	"testing"
+
+	"gotest.tools/v3/assert"
+)
+
+func TestRemoveAll(t *testing.T) {
+	walk := func(_ context.Context, req string) (int, error) {
+		switch req {
+		case "missing":
+			return 0, nil
+		case "broken":
+			return -1, errors.New("failed to delete task")
+		default:
+			return 1, nil
+		}
+	}
+
+	testCases := []struct {
+		name    string
+		reqs    []string
+		force   bool
+		wantErr string
+	}{
+		{name: "removed", reqs: []string{"ok"}},
+		{name: "missing without force", reqs: []string{"missing"}, wantErr: "1 errors:\nno such container: missing"},
+		{name: "missing with force", reqs: []string{"missing"}, force: true},
+		{name: "failure without force", reqs: []string{"broken"}, wantErr: "1 errors:\nfailed to delete task"},
+		{name: "failure with force", reqs: []string{"broken"}, force: true, wantErr: "1 errors:\nfailed to delete task"},
+		{name: "mixed with force", reqs: []string{"ok", "missing", "broken"}, force: true, wantErr: "1 errors:\nfailed to delete task"},
+		{name: "mixed without force", reqs: []string{"ok", "missing", "broken"}, wantErr: "2 errors:\nno such container: missing\nfailed to delete task"},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			err := removeAll(context.Background(), tc.reqs, walk, tc.force)
+			if tc.wantErr == "" {
+				assert.NilError(t, err)
+				return
+			}
+			assert.Error(t, err, tc.wantErr)
+		})
+	}
+}