From d837bf68a61d222f576d16c02f3fefd6c90febc3 Mon Sep 17 00:00:00 2001
From: Matt Heon <mheon@redhat.com>
Date: Wed, 10 Jan 2024 15:56:15 -0500
Subject: [PATCH] Fix `podman system reset` with external containers

It looks like we had some logic for this from #10789 but it does
not appear to have ever worked; we can't pull external containers
out of the DB, so the ContainerRm call failed unconditionally.

Instead, just handle them in Libpod when we're removing images.
We're removing every image, so setting Force when removing images
should get rid of all external containers. It's a little later in
the process than the current (nonfunctional) solution is but I
can't think of a reason why that would be bad.

[NO NEW TESTS NEEDED] We do not currently test `system reset`.
We should probably reevaluate that at some point this year.

Fixes https://issues.redhat.com/browse/RHEL-21261

Signed-off-by: Matt Heon <mheon@redhat.com>
---
 cmd/podman/system/reset.go | 15 ++++----------
 libpod/reset.go            | 41 ++++++++++++++++++++++++++++----------
 2 files changed, 34 insertions(+), 22 deletions(-)

diff --git a/cmd/podman/system/reset.go b/cmd/podman/system/reset.go
index 929ef16330..e4eb99a69a 100644
--- a/cmd/podman/system/reset.go
+++ b/cmd/podman/system/reset.go
@@ -49,10 +49,9 @@ func init() {
 
 func reset(cmd *cobra.Command, args []string) {
 	// Get all the external containers in use
-	listCtn, _ := registry.ContainerEngine().ContainerListExternal(registry.Context())
-	listCtnIds := make([]string, 0, len(listCtn))
-	for _, externalCtn := range listCtn {
-		listCtnIds = append(listCtnIds, externalCtn.ID)
+	listCtn, err := registry.ContainerEngine().ContainerListExternal(registry.Context())
+	if err != nil {
+		logrus.Error(err)
 	}
 	// Prompt for confirmation if --force is not set
 	if !forceFlag {
@@ -91,14 +90,8 @@ func reset(cmd *cobra.Command, args []string) {
 		}
 	}
 
-	// Purge all the external containers with storage
-	_, err := registry.ContainerEngine().ContainerRm(registry.Context(), listCtnIds, entities.RmOptions{Force: true, All: true, Ignore: true, Volumes: true})
-	if err != nil {
-		logrus.Error(err)
-	}
 	// Clean build cache if any
-	err = volumes.CleanCacheMount()
-	if err != nil {
+	if err := volumes.CleanCacheMount(); err != nil {
 		logrus.Error(err)
 	}
 	// Shutdown all running engines, `reset` will hijack repository
diff --git a/libpod/reset.go b/libpod/reset.go
index 8833eb94b2..c228e7ba17 100644
--- a/libpod/reset.go
+++ b/libpod/reset.go
@@ -119,13 +119,23 @@ func (r *Runtime) reset(ctx context.Context) error {
 	}
 
 	for _, c := range ctrs {
-		if err := r.RemoveContainer(ctx, c, true, true, timeout); err != nil {
-			if err := r.RemoveStorageContainer(c.ID(), true); err != nil {
-				if errors.Is(err, define.ErrNoSuchCtr) {
-					continue
-				}
-				logrus.Errorf("Removing container %s: %v", c.ID(), err)
+		if ctrs, _, err := r.RemoveContainerAndDependencies(ctx, c, true, true, timeout); err != nil {
+			for ctr, err := range ctrs {
+				logrus.Errorf("Error removing container %s: %v", ctr, err)
 			}
+
+			if errors.Is(err, define.ErrNoSuchCtr) || errors.Is(err, define.ErrCtrRemoved) {
+				continue
+			}
+
+			logrus.Errorf("Removing container %s: %v", c.ID(), err)
+
+			// There's no point trying a storage container removal
+			// here. High likelihood it doesn't work
+			// (RemoveStorageContainer will refuse to remove if a
+			// container exists in the Libpod database) and, even if
+			// it would, we'll get the container later on during
+			// image removal.
 		}
 	}
 
@@ -133,11 +143,7 @@ func (r *Runtime) reset(ctx context.Context) error {
 		logrus.Errorf("Stopping pause process: %v", err)
 	}
 
-	rmiOptions := &libimage.RemoveImagesOptions{Filters: []string{"readonly=false"}}
-	if _, rmiErrors := r.LibimageRuntime().RemoveImages(ctx, nil, rmiOptions); rmiErrors != nil {
-		return errorhandling.JoinErrors(rmiErrors)
-	}
-
+	// Volumes before images, as volumes can mount images.
 	volumes, err := r.state.AllVolumes()
 	if err != nil {
 		return err
@@ -151,6 +157,19 @@ func (r *Runtime) reset(ctx context.Context) error {
 		}
 	}
 
+	// Set force and ignore.
+	// Ignore shouldn't be necessary, but it seems safer. We want everything
+	// gone anyways...
+	rmiOptions := &libimage.RemoveImagesOptions{
+		Force:               true,
+		Ignore:              true,
+		RemoveContainerFunc: r.RemoveContainersForImageCallback(ctx),
+		Filters:             []string{"readonly=false"},
+	}
+	if _, rmiErrors := r.LibimageRuntime().RemoveImages(ctx, nil, rmiOptions); rmiErrors != nil {
+		return errorhandling.JoinErrors(rmiErrors)
+	}
+
 	// remove all networks
 	nets, err := r.network.NetworkList()
 	if err != nil {