diff --git a/pkg/domain/infra/abi/containers.go b/pkg/domain/infra/abi/containers.go index b60a4a8e8f..9afed94b86 100644 --- a/pkg/domain/infra/abi/containers.go +++ b/pkg/domain/infra/abi/containers.go @@ -1267,6 +1267,10 @@ func (ic *ContainerEngine) ContainerLogs(ctx context.Context, namesOrIds []strin func (ic *ContainerEngine) ContainerCleanup(ctx context.Context, namesOrIds []string, options entities.ContainerCleanupOptions) ([]*entities.ContainerCleanupReport, error) { containers, err := getContainers(ic.Libpod, getContainersOptions{all: options.All, latest: options.Latest, names: namesOrIds}) if err != nil { + // cleanup command spawned by conmon lost race as another process already removed the ctr + if errors.Is(err, define.ErrNoSuchCtr) { + return nil, nil + } return nil, err } reports := []*entities.ContainerCleanupReport{} @@ -1276,13 +1280,13 @@ func (ic *ContainerEngine) ContainerCleanup(ctx context.Context, namesOrIds []st if options.Exec != "" { if options.Remove { - if err := ctr.ExecRemove(options.Exec, false); err != nil { - return nil, err - } + err = ctr.ExecRemove(options.Exec, false) } else { - if err := ctr.ExecCleanup(options.Exec); err != nil { - return nil, err - } + err = ctr.ExecCleanup(options.Exec) + } + // If ErrNoSuchExecSession then the exec session was already removed so do not report an error. + if err != nil && !errors.Is(err, define.ErrNoSuchExecSession) { + return nil, err } return []*entities.ContainerCleanupReport{}, nil } @@ -1290,12 +1294,13 @@ func (ic *ContainerEngine) ContainerCleanup(ctx context.Context, namesOrIds []st if options.Remove && !ctr.ShouldRestart(ctx) { var timeout *uint err = ic.Libpod.RemoveContainer(ctx, ctr.Container, false, true, timeout) - if err != nil { + if err != nil && !errors.Is(err, define.ErrNoSuchCtr) { report.RmErr = fmt.Errorf("failed to clean up and remove container %v: %w", ctr.ID(), err) } } else { err := ctr.Cleanup(ctx) - if err != nil { + // 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) } } @@ -1303,7 +1308,7 @@ func (ic *ContainerEngine) ContainerCleanup(ctx context.Context, namesOrIds []st if options.RemoveImage { _, imageName := ctr.Image() imageEngine := ImageEngine{Libpod: ic.Libpod} - _, rmErrors := imageEngine.Remove(ctx, []string{imageName}, entities.ImageRemoveOptions{}) + _, rmErrors := imageEngine.Remove(ctx, []string{imageName}, entities.ImageRemoveOptions{Ignore: true}) report.RmiErr = errorhandling.JoinErrors(rmErrors) } diff --git a/test/e2e/cleanup_test.go b/test/e2e/cleanup_test.go index 10bdd9d92c..aa48fab3c3 100644 --- a/test/e2e/cleanup_test.go +++ b/test/e2e/cleanup_test.go @@ -12,10 +12,10 @@ var _ = Describe("Podman container cleanup", func() { SkipIfRemote("podman container cleanup is not supported in remote") }) - It("podman cleanup bogus container", func() { + It("podman cleanup bogus container should not error", func() { session := podmanTest.Podman([]string{"container", "cleanup", "foobar"}) session.WaitWithDefaultTimeout() - Expect(session).Should(ExitWithError(125, `no container with name or ID "foobar" found: no such container`)) + Expect(session).Should(ExitCleanly()) }) It("podman cleanup container by id", func() { @@ -86,7 +86,13 @@ var _ = Describe("Podman container cleanup", func() { Expect(session).Should(ExitCleanly()) session = podmanTest.Podman([]string{"container", "cleanup", "running"}) session.WaitWithDefaultTimeout() - Expect(session).Should(ExitWithError(125, "is running or paused, refusing to clean up: container state improper")) + Expect(session).Should(ExitCleanly()) + + // cleanup should be a NOP here, ctr must still be running + session = podmanTest.Podman([]string{"container", "inspect", "--format={{.State.Status}}", "running"}) + session.WaitWithDefaultTimeout() + Expect(session).Should(ExitCleanly()) + Expect(session.OutputToString()).To(Equal("running")) }) It("podman cleanup paused container", func() { @@ -99,7 +105,13 @@ var _ = Describe("Podman container cleanup", func() { Expect(session).Should(ExitCleanly()) session = podmanTest.Podman([]string{"container", "cleanup", "paused"}) session.WaitWithDefaultTimeout() - Expect(session).Should(ExitWithError(125, "is running or paused, refusing to clean up: container state improper")) + Expect(session).Should(ExitCleanly()) + + // cleanup should be a NOP here, ctr must still be paused + session = podmanTest.Podman([]string{"container", "inspect", "--format={{.State.Status}}", "paused"}) + session.WaitWithDefaultTimeout() + Expect(session).Should(ExitCleanly()) + Expect(session.OutputToString()).To(Equal("paused")) // unpause so that the cleanup can stop the container, // otherwise it fails with container state improper