diff --git a/cmd/podman/containers/cleanup.go b/cmd/podman/containers/cleanup.go index db15c6d834..72c1feab53 100644 --- a/cmd/podman/containers/cleanup.go +++ b/cmd/podman/containers/cleanup.go @@ -53,6 +53,11 @@ func init() { flags.BoolVar(&cleanupOptions.Remove, "rm", false, "After cleanup, remove the container entirely") flags.BoolVar(&cleanupOptions.RemoveImage, "rmi", false, "After cleanup, remove the image entirely") + + stoppedOnlyFlag := "stopped-only" + flags.BoolVar(&cleanupOptions.StoppedOnly, stoppedOnlyFlag, false, "Only cleanup when the container is in the stopped state") + _ = flags.MarkHidden(stoppedOnlyFlag) + validate.AddLatestFlag(cleanupCommand, &cleanupOptions.Latest) } diff --git a/libpod/container_api.go b/libpod/container_api.go index 02e35199ba..2b5e305348 100644 --- a/libpod/container_api.go +++ b/libpod/container_api.go @@ -785,8 +785,10 @@ func (c *Container) WaitForConditionWithInterval(ctx context.Context, waitTimeou } // Cleanup unmounts all mount points in container and cleans up container storage -// It also cleans up the network stack -func (c *Container) Cleanup(ctx context.Context) error { +// It also cleans up the network stack. +// onlyStopped is set by the podman container cleanup to ensure we only cleanup a stopped container, +// all other states mean another process already called cleanup before us which is fine in such cases. +func (c *Container) Cleanup(ctx context.Context, onlyStopped bool) error { if !c.batched { c.lock.Lock() defer c.lock.Unlock() @@ -808,6 +810,9 @@ func (c *Container) Cleanup(ctx context.Context) error { if !c.ensureState(define.ContainerStateConfigured, define.ContainerStateCreated, define.ContainerStateStopped, define.ContainerStateStopping, define.ContainerStateExited) { return fmt.Errorf("container %s is running or paused, refusing to clean up: %w", c.ID(), define.ErrCtrStateInvalid) } + if onlyStopped && !c.ensureState(define.ContainerStateStopped) { + return fmt.Errorf("container %s is not stopped and only cleanup for a stopped container was requested: %w", c.ID(), define.ErrCtrStateInvalid) + } // if the container was not created in the oci runtime or was already cleaned up, then do nothing if c.ensureState(define.ContainerStateConfigured, define.ContainerStateExited) { diff --git a/libpod/pod_api.go b/libpod/pod_api.go index d385c56286..f0effef844 100644 --- a/libpod/pod_api.go +++ b/libpod/pod_api.go @@ -180,7 +180,7 @@ func (p *Pod) stopWithTimeout(ctx context.Context, cleanup bool, timeout int) (m } if cleanup { - err := c.Cleanup(ctx) + err := c.Cleanup(ctx, false) if err != nil && !errors.Is(err, define.ErrCtrStateInvalid) && !errors.Is(err, define.ErrCtrStopped) { return err } @@ -299,7 +299,7 @@ func (p *Pod) Cleanup(ctx context.Context) (map[string]error, error) { c := ctr logrus.Debugf("Adding parallel job to clean up container %s", c.ID()) retChan := parallel.Enqueue(ctx, func() error { - return c.Cleanup(ctx) + return c.Cleanup(ctx, false) }) ctrErrChan[c.ID()] = retChan diff --git a/pkg/domain/entities/containers.go b/pkg/domain/entities/containers.go index 24c1d78f8a..105b091549 100644 --- a/pkg/domain/entities/containers.go +++ b/pkg/domain/entities/containers.go @@ -371,6 +371,7 @@ type ContainerCleanupOptions struct { Latest bool Remove bool RemoveImage bool + StoppedOnly bool // Only cleanup if the container is stopped, i.e. was running before } // ContainerCleanupReport describes the response from a diff --git a/pkg/domain/infra/abi/containers.go b/pkg/domain/infra/abi/containers.go index 1694d6ce79..f20c412858 100644 --- a/pkg/domain/infra/abi/containers.go +++ b/pkg/domain/infra/abi/containers.go @@ -313,7 +313,7 @@ func (ic *ContainerEngine) ContainerStop(ctx context.Context, namesOrIds []strin } } } else { - if err = c.Cleanup(ctx); err != nil { + if err = c.Cleanup(ctx, false); 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) { @@ -1320,7 +1320,7 @@ func (ic *ContainerEngine) ContainerCleanup(ctx context.Context, namesOrIds []st report.RmErr = fmt.Errorf("failed to clean up and remove container %v: %w", ctr.ID(), err) } } else { - err := ctr.Cleanup(ctx) + err := ctr.Cleanup(ctx, options.StoppedOnly) // ignore error if ctr is removed or cannot be cleaned up, likely the ctr was already restarted by another process if err != nil && !errors.Is(err, define.ErrNoSuchCtr) && !errors.Is(err, define.ErrCtrStateInvalid) { report.CleanErr = fmt.Errorf("failed to clean up container %v: %w", ctr.ID(), err) diff --git a/pkg/specgenutil/util.go b/pkg/specgenutil/util.go index 71065997b0..222e50f934 100644 --- a/pkg/specgenutil/util.go +++ b/pkg/specgenutil/util.go @@ -311,7 +311,9 @@ func CreateExitCommandArgs(storageConfig storageTypes.StoreOptions, config *conf command = append(command, "--module", module) } - command = append(command, []string{"container", "cleanup"}...) + // --stopped-only is used to ensure we only cleanup stopped containers and do not race + // against other processes that did a cleanup() + init() again before we had the chance to run + command = append(command, []string{"container", "cleanup", "--stopped-only"}...) if rm { command = append(command, "--rm")