Merge pull request #23537 from Luap99/cleanup-err

podman container cleanup: ignore common errors

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