Fix `podman stop` and `podman run --rmi`

This started off as an attempt to make `podman stop` on a
container started with `--rm` actually remove the container,
instead of just cleaning it up and waiting for the cleanup
process to finish the removal.

In the process, I realized that `podman run --rmi` was rather
broken. It was only done as part of the Podman CLI, not the
cleanup process (meaning it only worked with attached containers)
and the way it was wired meant that I was fairly confident that
it wouldn't work if I did a `podman stop` on an attached
container run with `--rmi`. I rewired it to use the same
mechanism that `podman run --rm` uses, so it should be a lot more
durable now, and I also wired it into `podman inspect` so you can
tell that a container will remove its image.

Tests have been added for the changes to `podman run --rmi`. No
tests for `stop` on a `run --rm` container as that would be racy.

Fixes #22852
Fixes RHEL-39513

Signed-off-by: Matt Heon <mheon@redhat.com>

13 files changed, 110 insertions, 12 deletions

diff --git a/cmd/podman/containers/run.go b/cmd/podman/containers/run.go
index be688c21f..d50e787be 100644
--- a/cmd/podman/containers/run.go
+++ b/cmd/podman/containers/run.go
@@ -211,6 +211,11 @@ func run(cmd *cobra.Command, args []string) error {
 	s.ImageArch = cliVals.Arch
 	s.ImageVariant = cliVals.Variant
 	s.Passwd = &runOpts.Passwd
+
+	if runRmi {
+		s.RemoveImage = &runRmi
+	}
+
 	runOpts.Spec = s
 
 	if err := createPodIfNecessary(cmd, s, cliVals.Net); err != nil {
diff --git a/libpod/container.go b/libpod/container.go
index e424afa9f..423443072 100644
--- a/libpod/container.go
+++ b/libpod/container.go
@@ -1261,6 +1261,17 @@ func (c *Container) AutoRemove() bool {
 	return spec.Annotations[define.InspectAnnotationAutoremove] == define.InspectResponseTrue
 }
 
+// AutoRemoveImage indicates that the container will automatically remove the
+// image it is using after it exits and is removed.
+// Only allowed if AutoRemove is true.
+func (c *Container) AutoRemoveImage() bool {
+	spec := c.config.Spec
+	if spec.Annotations == nil {
+		return false
+	}
+	return spec.Annotations[define.InspectAnnotationAutoremoveImage] == define.InspectResponseTrue
+}
+
 // Timezone returns the timezone configured inside the container.
 // Local means it has the same timezone as the host machine
 func (c *Container) Timezone() string {
diff --git a/libpod/container_inspect.go b/libpod/container_inspect.go
index ef4bac14e..aa561a5cd 100644
--- a/libpod/container_inspect.go
+++ b/libpod/container_inspect.go
@@ -517,6 +517,9 @@ func (c *Container) generateInspectContainerHostConfig(ctrSpec *spec.Spec, named
 		if ctrSpec.Annotations[define.InspectAnnotationAutoremove] == define.InspectResponseTrue {
 			hostConfig.AutoRemove = true
 		}
+		if ctrSpec.Annotations[define.InspectAnnotationAutoremoveImage] == define.InspectResponseTrue {
+			hostConfig.AutoRemoveImage = true
+		}
 		if ctrs, ok := ctrSpec.Annotations[define.VolumesFromAnnotation]; ok {
 			hostConfig.VolumesFrom = strings.Split(ctrs, ";")
 		}
diff --git a/libpod/container_validate.go b/libpod/container_validate.go
index 0c2b604dd..a98545c91 100644
--- a/libpod/container_validate.go
+++ b/libpod/container_validate.go
@@ -158,6 +158,18 @@ func (c *Container) validate() error {
 		}
 	}
 
+	// Autoremoving image requires autoremoving the associated container
+	if c.config.Spec.Annotations != nil {
+		if c.config.Spec.Annotations[define.InspectAnnotationAutoremoveImage] == define.InspectResponseTrue {
+			if c.config.Spec.Annotations[define.InspectAnnotationAutoremove] != define.InspectResponseTrue {
+				return fmt.Errorf("autoremoving image requires autoremoving the container: %w", define.ErrInvalidArg)
+			}
+			if c.config.Rootfs != "" {
+				return fmt.Errorf("autoremoving image is not possible when a rootfs is in use: %w", define.ErrInvalidArg)
+			}
+		}
+	}
+
 	// Cannot set startup HC without a healthcheck
 	if c.config.HealthCheckConfig == nil && c.config.StartupHealthCheckConfig != nil {
 		return fmt.Errorf("cannot set a startup healthcheck when there is no regular healthcheck: %w", define.ErrInvalidArg)
diff --git a/libpod/define/annotations.go b/libpod/define/annotations.go
index ac1956f56..e1da8a157 100644
--- a/libpod/define/annotations.go
+++ b/libpod/define/annotations.go
@@ -18,6 +18,12 @@ const (
 	// the two supported boolean values (InspectResponseTrue and
 	// InspectResponseFalse) it will be used in the output of Inspect().
 	InspectAnnotationAutoremove = "io.podman.annotations.autoremove"
+	// InspectAnnotationAutoremoveImage is used by Inspect to identify
+	// containers which will automatically remove the image used by the
+	// container. If an annotation with this key is found in the OCI spec and
+	// is one of the two supported boolean values (InspectResponseTrue and
+	// InspectResponseFalse) it will be used in the output of Inspect().
+	InspectAnnotationAutoremoveImage = "io.podman.annotations.autoremove-image"
 	// InspectAnnotationPrivileged is used by Inspect to identify containers
 	// which are privileged (IE, running with elevated privileges).
 	// It is expected to be a boolean, populated by one of
diff --git a/libpod/define/container_inspect.go b/libpod/define/container_inspect.go
index 240b09d21..89b70e59e 100644
--- a/libpod/define/container_inspect.go
+++ b/libpod/define/container_inspect.go
@@ -390,6 +390,11 @@ type InspectContainerHostConfig struct {
 	// It is not handled directly within libpod and is stored in an
 	// annotation.
 	AutoRemove bool `json:"AutoRemove"`
+	// AutoRemoveImage is whether the container's image will be
+	// automatically removed on exiting.
+	// It is not handled directly within libpod and is stored in an
+	// annotation.
+	AutoRemoveImage bool `json:"AutoRemoveImage"`
 	// Annotations are provided to the runtime when the container is
 	// started.
 	Annotations map[string]string `json:"Annotations"`
diff --git a/libpod/oci_conmon_common.go b/libpod/oci_conmon_common.go
index 39a2a0d62..9a9607305 100644
--- a/libpod/oci_conmon_common.go
+++ b/libpod/oci_conmon_common.go
@@ -1035,7 +1035,7 @@ func (r *ConmonOCIRuntime) createOCIContainer(ctr *Container, restoreOptions *Co
 		args = append(args, "--no-pivot")
 	}
 
-	exitCommand, err := specgenutil.CreateExitCommandArgs(ctr.runtime.storageConfig, ctr.runtime.config, ctr.runtime.syslog || logrus.IsLevelEnabled(logrus.DebugLevel), ctr.AutoRemove(), false)
+	exitCommand, err := specgenutil.CreateExitCommandArgs(ctr.runtime.storageConfig, ctr.runtime.config, ctr.runtime.syslog || logrus.IsLevelEnabled(logrus.DebugLevel), ctr.AutoRemove(), ctr.AutoRemoveImage(), false)
 	if err != nil {
 		return 0, err
 	}
diff --git a/pkg/api/handlers/compat/exec.go b/pkg/api/handlers/compat/exec.go
index e7f29d727..7c28458d5 100644
--- a/pkg/api/handlers/compat/exec.go
+++ b/pkg/api/handlers/compat/exec.go
@@ -74,7 +74,7 @@ func ExecCreateHandler(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 	// Automatically log to syslog if the server has log-level=debug set
-	exitCommandArgs, err := specgenutil.CreateExitCommandArgs(storageConfig, runtimeConfig, logrus.IsLevelEnabled(logrus.DebugLevel), true, true)
+	exitCommandArgs, err := specgenutil.CreateExitCommandArgs(storageConfig, runtimeConfig, logrus.IsLevelEnabled(logrus.DebugLevel), true, false, true)
 	if err != nil {
 		utils.InternalServerError(w, err)
 		return
diff --git a/pkg/domain/infra/abi/containers.go b/pkg/domain/infra/abi/containers.go
index 47389e9fd..1694d6ce7 100644
--- a/pkg/domain/infra/abi/containers.go
+++ b/pkg/domain/infra/abi/containers.go
@@ -36,6 +36,7 @@ import (
 	"github.com/containers/podman/v5/pkg/util"
 	"github.com/containers/storage"
 	"github.com/containers/storage/types"
+	"github.com/hashicorp/go-multierror"
 	"github.com/sirupsen/logrus"
 )
 
@@ -291,15 +292,35 @@ func (ic *ContainerEngine) ContainerStop(ctx context.Context, namesOrIds []strin
 				return err
 			}
 		}
-		err = c.Cleanup(ctx)
-		if err != nil {
-			// Issue #7384 and #11384: If the container is configured for
-			// auto-removal, it might already have been removed at this point.
-			// We still need to clean up since we do not know if the other cleanup process is successful
-			if c.AutoRemove() && (errors.Is(err, define.ErrNoSuchCtr) || errors.Is(err, define.ErrCtrRemoved)) {
-				return nil
+		if c.AutoRemove() {
+			_, imageName := c.Image()
+
+			if err := ic.Libpod.RemoveContainer(ctx, c, false, true, nil); err != nil {
+				// Issue #7384 and #11384: If the container is configured for
+				// auto-removal, it might already have been removed at this point.
+				// We still need to clean up since we do not know if the other cleanup process is successful
+				if !(errors.Is(err, define.ErrNoSuchCtr) || errors.Is(err, define.ErrCtrRemoved)) {
+					return err
+				}
+			}
+
+			if c.AutoRemoveImage() {
+				imageEngine := ImageEngine{Libpod: ic.Libpod}
+				_, rmErrors := imageEngine.Remove(ctx, []string{imageName}, entities.ImageRemoveOptions{Ignore: true})
+				if len(rmErrors) > 0 {
+					mErr := multierror.Append(nil, rmErrors...)
+					return fmt.Errorf("removing container %s image %s: %w", c.ID(), imageName, mErr)
+				}
+			}
+		} else {
+			if err = c.Cleanup(ctx); err != nil {
+				// The container could still have been removed, as we unlocked
+				// after we stopped it.
+				if errors.Is(err, define.ErrNoSuchCtr) || errors.Is(err, define.ErrCtrRemoved) {
+					return nil
+				}
+				return err
 			}
-			return err
 		}
 		return nil
 	})
@@ -827,7 +848,7 @@ func makeExecConfig(options entities.ExecOptions, rt *libpod.Runtime) (*libpod.E
 		return nil, fmt.Errorf("retrieving Libpod configuration to build exec exit command: %w", err)
 	}
 	// TODO: Add some ability to toggle syslog
-	exitCommandArgs, err := specgenutil.CreateExitCommandArgs(storageConfig, runtimeConfig, logrus.IsLevelEnabled(logrus.DebugLevel), false, true)
+	exitCommandArgs, err := specgenutil.CreateExitCommandArgs(storageConfig, runtimeConfig, logrus.IsLevelEnabled(logrus.DebugLevel), false, false, true)
 	if err != nil {
 		return nil, fmt.Errorf("constructing exit command for exec session: %w", err)
 	}
diff --git a/pkg/specgen/generate/oci_linux.go b/pkg/specgen/generate/oci_linux.go
index d6247bbf6..d19cf9c03 100644
--- a/pkg/specgen/generate/oci_linux.go
+++ b/pkg/specgen/generate/oci_linux.go
@@ -318,6 +318,10 @@ func SpecGenToOCI(ctx context.Context, s *specgen.SpecGenerator, rt *libpod.Runt
 		configSpec.Annotations[define.InspectAnnotationAutoremove] = define.InspectResponseTrue
 	}
 
+	if s.RemoveImage != nil && *s.RemoveImage {
+		configSpec.Annotations[define.InspectAnnotationAutoremoveImage] = define.InspectResponseTrue
+	}
+
 	if len(s.VolumesFrom) > 0 {
 		configSpec.Annotations[define.VolumesFromAnnotation] = strings.Join(s.VolumesFrom, ";")
 	}
diff --git a/pkg/specgen/specgen.go b/pkg/specgen/specgen.go
index 00a208921..1299daf3d 100644
--- a/pkg/specgen/specgen.go
+++ b/pkg/specgen/specgen.go
@@ -159,6 +159,12 @@ type ContainerBasicConfig struct {
 	// and exits.
 	// Optional.
 	Remove *bool `json:"remove,omitempty"`
+	// RemoveImage indicates that the container should remove the image it
+	// was created from after it exits.
+	// Only allowed if Remove is set to true and Image, not Rootfs, is in
+	// use.
+	// Optional.
+	RemoveImage *bool `json:"removeImage,omitempty"`
 	// ContainerCreateCommand is the command that was used to create this
 	// container.
 	// This will be shown in the output of Inspect() on the container, and
diff --git a/pkg/specgenutil/util.go b/pkg/specgenutil/util.go
index b5eb5d914..71065997b 100644
--- a/pkg/specgenutil/util.go
+++ b/pkg/specgenutil/util.go
@@ -261,7 +261,7 @@ func parseAndValidatePort(port string) (uint16, error) {
 	return uint16(num), nil
 }
 
-func CreateExitCommandArgs(storageConfig storageTypes.StoreOptions, config *config.Config, syslog, rm, exec bool) ([]string, error) {
+func CreateExitCommandArgs(storageConfig storageTypes.StoreOptions, config *config.Config, syslog, rm, rmi, exec bool) ([]string, error) {
 	// We need a cleanup process for containers in the current model.
 	// But we can't assume that the caller is Podman - it could be another
 	// user of the API.
@@ -317,6 +317,10 @@ func CreateExitCommandArgs(storageConfig storageTypes.StoreOptions, config *conf
 		command = append(command, "--rm")
 	}
 
+	if rmi {
+		command = append(command, "--rmi")
+	}
+
 	// This has to be absolutely last, to ensure that the exec session ID
 	// will be added after it by Libpod.
 	if exec {
diff --git a/test/system/030-run.bats b/test/system/030-run.bats
index 1aa079dab..7dc0d9a1b 100644
--- a/test/system/030-run.bats
+++ b/test/system/030-run.bats
@@ -209,6 +209,27 @@ echo $rand        |   0 | $rand
 
     run_podman 125 run --rmi --rm=false $NONLOCAL_IMAGE /bin/true
     is "$output" "Error: the --rmi option does not work without --rm" "--rmi should refuse to remove images when --rm=false set by user"
+
+    # Try again with a detached container and verify it works
+    cname=c_$(safename)
+    run_podman run -d --name $cname --rmi $NONLOCAL_IMAGE /bin/true
+    # 10 chances for the image to disappear
+    for i in `seq 1 10`; do
+        sleep 0.5
+        run_podman '?' image exists $NONLOCAL_IMAGE
+        if [[ $status == 1 ]]; then
+           break
+        fi
+    done
+    # Final check will error if image still exists
+    run_podman 1 image exists $NONLOCAL_IMAGE
+
+    # Verify that the inspect annotation is set correctly
+    run_podman run -d --name $cname --rmi $NONLOCAL_IMAGE sleep 10
+    run_podman inspect --format '{{ .HostConfig.AutoRemoveImage }} {{ .HostConfig.AutoRemove }}' $cname
+    is "$output" "true true" "Inspect correctly shows image autoremove and normal autoremove"
+    run_podman stop -t0 $cname
+    run_podman 1 image exists $NONLOCAL_IMAGE
 }
 
 # 'run --conmon-pidfile --cid-file' makes sure we don't regress on these flags.