podman container cleanup: ignore common errors

The podman container cleanup command is not really intended for human
use. Instead each conmon will spawn this command after the container
exit to make sure we can cleanup resources asynchronously. However this
command will always race against other foreground process such as podman
rm -fa. Therefore it is possible that the ctr was already removed and we
should not log errors in this case.

While these errors are normally not seen as the command is int he
background you can see it if you enable syslog logging and then they
just spam the log with useless errors so just ignore them.

Signed-off-by: Paul Holzinger <pholzing@redhat.com>

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)
 		}
 

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