Skip to content

Commit

Permalink
kill: wait for the container
Browse files Browse the repository at this point in the history
Make sure to wait for the container to exit after kill. While the
cleanup process will take care eventually of transitioning the state, we
need to give a guarantee to the user to leave the container in the
expected state once the (kill) command has finished.

The issue could be observed in a flaking test (#16142) where
`podman rm -f -t0` failed because the preceding `podman kill`
left the container in "running" state which ultimately confused
the "stop" backend.

Note that we should only wait for the container to exit when SIGKILL is
being used.  Other signals have different semantics.

[NO NEW TESTS NEEDED] as I do not know how to reliably reproduce the
issue.  If #16142 stops flaking, we are good.

Fixes: #16142
Signed-off-by: Valentin Rothberg <[email protected]>
  • Loading branch information
vrothberg committed Oct 14, 2022
1 parent 0321165 commit b35fab6
Show file tree
Hide file tree
Showing 3 changed files with 10 additions and 1 deletion.
7 changes: 6 additions & 1 deletion libpod/container_api.go
Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,12 @@ func (c *Container) Kill(signal uint) error {

c.newContainerEvent(events.Kill)

return c.save()
// Make sure to wait for the container to exit in case of SIGKILL.
if signal == uint(unix.SIGKILL) {
return c.waitForConmonToExitAndSave()
}

return nil
}

// Attach attaches to a container.
Expand Down
3 changes: 3 additions & 0 deletions libpod/container_internal.go
Original file line number Diff line number Diff line change
Expand Up @@ -1325,7 +1325,10 @@ func (c *Container) stop(timeout uint) error {

c.newContainerEvent(events.Stop)
c.state.StoppedByUser = true
return c.waitForConmonToExitAndSave()
}

func (c *Container) waitForConmonToExitAndSave() error {
conmonAlive, err := c.ociRuntime.CheckConmonRunning(c)
if err != nil {
return err
Expand Down
1 change: 1 addition & 0 deletions test/e2e/attach_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ var _ = Describe("Podman attach", func() {
Expect(results.OutputToString()).To(ContainSubstring("test"))
Expect(podmanTest.NumberOfContainersRunning()).To(Equal(1))
})

It("podman attach to the latest container", func() {
session := podmanTest.Podman([]string{"run", "-d", "--name", "test1", ALPINE, "/bin/sh", "-c", "while true; do echo test1; sleep 1; done"})
session.WaitWithDefaultTimeout()
Expand Down

0 comments on commit b35fab6

Please sign in to comment.