diff --git a/cmd/podman/containers/run.go b/cmd/podman/containers/run.go index be688c21f5..8abd08837e 100644 --- a/cmd/podman/containers/run.go +++ b/cmd/podman/containers/run.go @@ -16,7 +16,6 @@ import ( "github.com/containers/podman/v5/pkg/rootless" "github.com/containers/podman/v5/pkg/specgen" "github.com/containers/podman/v5/pkg/specgenutil" - "github.com/containers/storage/types" "github.com/sirupsen/logrus" "github.com/spf13/cobra" "golang.org/x/term" @@ -211,6 +210,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 { @@ -238,13 +242,9 @@ func run(cmd *cobra.Command, args []string) error { return nil } if runRmi { - _, rmErrors := registry.ImageEngine().Remove(registry.GetContext(), []string{imageName}, entities.ImageRemoveOptions{}) + _, rmErrors := registry.ImageEngine().Remove(registry.GetContext(), []string{imageName}, entities.ImageRemoveOptions{Ignore: true}) for _, err := range rmErrors { - // ImageUnknown would be a super-unlikely race - if !errors.Is(err, types.ErrImageUnknown) { - // Typical case: ErrImageUsedByContainer - logrus.Warn(err) - } + logrus.Warnf("Failed to remove image: %v", err) } } return nil diff --git a/libpod/container.go b/libpod/container.go index e424afa9f6..4234430726 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 ef4bac14e4..aa561a5cdc 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 0c2b604dd0..a98545c91f 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 ac1956f56b..e1da8a157e 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 240b09d21d..89b70e59e6 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 a3e5a24b0d..e7a5a4049d 100644 --- a/libpod/oci_conmon_common.go +++ b/libpod/oci_conmon_common.go @@ -1116,7 +1116,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 e7f29d727a..7c28458d57 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 3cbeb2cd65..9006dc9e0b 100644 --- a/pkg/domain/infra/abi/containers.go +++ b/pkg/domain/infra/abi/containers.go @@ -35,6 +35,7 @@ import ( "github.com/containers/podman/v5/pkg/specgenutil" "github.com/containers/podman/v5/pkg/util" "github.com/containers/storage" + "github.com/hashicorp/go-multierror" "github.com/sirupsen/logrus" ) @@ -290,15 +291,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 }) @@ -826,7 +847,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 d6247bbf67..d19cf9c03c 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 00a208921a..1299daf3de 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 b5eb5d914c..71065997b0 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 4080a15792..f5eb351e8a 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.