From 8b1f06bf8f01f74d7ce28348f759eb2c866d313b Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Tue, 15 Jul 2025 16:49:31 +0200 Subject: [PATCH 1/2] podman inspect: fix error difference between local and remote There is no reason for local and rmeote to use a different error that just differs in quoting. Make them consitent to simplify the tests. Signed-off-by: Paul Holzinger --- pkg/domain/infra/abi/containers.go | 4 ++-- test/e2e/inspect_test.go | 12 ++---------- test/system/252-quadlet.bats | 2 +- 3 files changed, 5 insertions(+), 13 deletions(-) diff --git a/pkg/domain/infra/abi/containers.go b/pkg/domain/infra/abi/containers.go index 36b7e9fa2b..8b29821132 100644 --- a/pkg/domain/infra/abi/containers.go +++ b/pkg/domain/infra/abi/containers.go @@ -539,7 +539,7 @@ func (ic *ContainerEngine) ContainerInspect(ctx context.Context, namesOrIds []st // ErrNoSuchCtr is non-fatal, other errors will be // treated as fatal. if errors.Is(err, define.ErrNoSuchCtr) { - errs = append(errs, fmt.Errorf("no such container %s", name)) + errs = append(errs, fmt.Errorf("no such container %q", name)) continue } return nil, nil, err @@ -550,7 +550,7 @@ func (ic *ContainerEngine) ContainerInspect(ctx context.Context, namesOrIds []st // ErrNoSuchCtr is non-fatal, other errors will be // treated as fatal. if errors.Is(err, define.ErrNoSuchCtr) { - errs = append(errs, fmt.Errorf("no such container %s", name)) + errs = append(errs, fmt.Errorf("no such container %q", name)) continue } return nil, nil, err diff --git a/test/e2e/inspect_test.go b/test/e2e/inspect_test.go index be7809bd32..61f3f50f9d 100644 --- a/test/e2e/inspect_test.go +++ b/test/e2e/inspect_test.go @@ -188,11 +188,7 @@ var _ = Describe("Podman inspect", func() { ctrInspect := podmanTest.Podman([]string{"container", "inspect", ALPINE}) ctrInspect.WaitWithDefaultTimeout() - if IsRemote() { - Expect(ctrInspect).To(ExitWithError(125, fmt.Sprintf("no such container %q", ALPINE))) - } else { - Expect(ctrInspect).To(ExitWithError(125, fmt.Sprintf("no such container %s", ALPINE))) - } + Expect(ctrInspect).To(ExitWithError(125, fmt.Sprintf("no such container %q", ALPINE))) imageInspect := podmanTest.Podman([]string{"image", "inspect", ALPINE}) imageInspect.WaitWithDefaultTimeout() @@ -399,11 +395,7 @@ var _ = Describe("Podman inspect", func() { inspect := podmanTest.Podman([]string{"inspect", "--type", "container", podName}) inspect.WaitWithDefaultTimeout() - if IsRemote() { - Expect(inspect).To(ExitWithError(125, fmt.Sprintf("no such container %q", podName))) - } else { - Expect(inspect).To(ExitWithError(125, fmt.Sprintf("no such container %s", podName))) - } + Expect(inspect).To(ExitWithError(125, fmt.Sprintf("no such container %q", podName))) }) It("podman inspect --type network on a container should fail", func() { diff --git a/test/system/252-quadlet.bats b/test/system/252-quadlet.bats index a3d95309dc..a95e0730e4 100644 --- a/test/system/252-quadlet.bats +++ b/test/system/252-quadlet.bats @@ -1081,7 +1081,7 @@ EOF is "$output" "$exit_code_prop" \ "$basename: service container has the expected policy set in its annotations" else - assert "$output" =~ "no such container $service_container" \ + assert "$output" =~ "no such container \"$service_container\"" \ "$basename: unexpected error from podman container inspect" fi From c1cf4b0d89df84b97b043027d90741f3b177011e Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Tue, 15 Jul 2025 15:54:08 +0200 Subject: [PATCH 2/2] podman rm: handle case where conmon was killed When conmon was killed podman rm -f currently fails but running it again then works which doesn't really makes sense. We should properly remove the contianer even if conmon is dead. In fact the code already handles ErrConmonDead as stop error when we remove the container but this error was never thrown anywhere. To fix this throw ErrConmonDead instead of ErrInternal because that is not an intenral error if something else killed conmon. With this we can correctly cleanup and remove the container. The fact that this works on the first try is important for quadlet units as they only run the ExecStopPost= command once to remove it. Fixes: #26640 Signed-off-by: Paul Holzinger --- libpod/container_internal.go | 2 +- test/system/030-run.bats | 2 +- test/system/055-rm.bats | 16 ++++++++++++++++ 3 files changed, 18 insertions(+), 2 deletions(-) diff --git a/libpod/container_internal.go b/libpod/container_internal.go index 2968d2d48c..aeedbf3f42 100644 --- a/libpod/container_internal.go +++ b/libpod/container_internal.go @@ -1544,7 +1544,7 @@ func (c *Container) waitForConmonToExitAndSave() error { logrus.Errorf("Error cleaning up container %s after Conmon exited prematurely: %v", c.ID(), err) } - return fmt.Errorf("container %s conmon exited prematurely, exit code could not be retrieved: %w", c.ID(), define.ErrInternal) + return fmt.Errorf("container %s conmon exited prematurely, exit code could not be retrieved: %w", c.ID(), define.ErrConmonDead) } return c.save() diff --git a/test/system/030-run.bats b/test/system/030-run.bats index ce36b85ab0..eb0b90a926 100644 --- a/test/system/030-run.bats +++ b/test/system/030-run.bats @@ -1778,7 +1778,7 @@ search | $IMAGE | # Unclear why `-t0` is required here, works locally without. # But it shouldn't hurt and does make the test pass... PODMAN_TIMEOUT=5 run_podman 125 stop -t0 $cname - is "$output" "Error: container .* conmon exited prematurely, exit code could not be retrieved: internal libpod error" "correct error on missing conmon" + is "$output" "Error: container .* conmon exited prematurely, exit code could not be retrieved: conmon process killed" "correct error on missing conmon" # This should be safe because stop is guaranteed to call cleanup? run_podman inspect --format "{{ .State.Status }}" $cname diff --git a/test/system/055-rm.bats b/test/system/055-rm.bats index c0e674c0c3..6c8e058260 100644 --- a/test/system/055-rm.bats +++ b/test/system/055-rm.bats @@ -202,4 +202,20 @@ function __run_healthcheck_container() { die "Container never entered 'stopping' state" } +# bats test_tags=ci:parallel +@test "podman rm after killed conmon" { + cname=c_$(safename) + run_podman run -d --name $cname $IMAGE sleep 1000 + + run_podman inspect --format '{{ .State.ConmonPid }}' $cname + conmon_pid=$output + + kill -9 ${conmon_pid} + + run_podman rm -f -t0 $cname + + run_podman 125 container inspect $cname + assert "$output" =~ "no such container \"$cname\"" "Container should be removed" +} + # vim: filetype=sh