Skip to content

Commit

Permalink
Set StoppedByUser earlier in the process of stopping
Browse files Browse the repository at this point in the history
The StoppedByUser variable indicates that the container was
requested to stop by a user. It's used to prevent restart policy
from firing (so that a restart=always container won't restart if
the user does a `podman stop`. The problem is we were setting it
*very* late in the stop() function. Originally, this was fine,
but after the changes to add the new Stopping state, the logic
that triggered restart policy was firing before StoppedByUser was
even set - so the container would still restart.

Setting it earlier shouldn't hurt anything and guarantees that
checks will see that the container was stopped manually.

Fixes containers#17069

Signed-off-by: Matthew Heon <[email protected]>
  • Loading branch information
mheon committed Jan 12, 2023
1 parent b107d77 commit 1ab833f
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 1 deletion.
2 changes: 1 addition & 1 deletion libpod/container_internal.go
Original file line number Diff line number Diff line change
Expand Up @@ -1296,6 +1296,7 @@ func (c *Container) stop(timeout uint) error {
// demonstrates nicely that a high stop timeout will block even simple
// commands such as `podman ps` from progressing if the container lock
// is held when busy-waiting for the container to be stopped.
c.state.StoppedByUser = true
c.state.State = define.ContainerStateStopping
if err := c.save(); err != nil {
return fmt.Errorf("saving container %s state before stopping: %w", c.ID(), err)
Expand Down Expand Up @@ -1343,7 +1344,6 @@ func (c *Container) stop(timeout uint) error {
}

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

Expand Down
16 changes: 16 additions & 0 deletions test/e2e/run_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1407,6 +1407,22 @@ USER mail`, BB)
Expect(found).To(BeTrue())
})

It("podman run with restart policy does not restart on manual stop", func() {
ctrName := "testCtr"
ctr := podmanTest.Podman([]string{"run", "-dt", "--restart=always", "--name", ctrName, ALPINE, "top"})
ctr.WaitWithDefaultTimeout()
Expect(ctr).Should(Exit(0))

stop := podmanTest.Podman([]string{"stop", ctrName})
stop.WaitWithDefaultTimeout()
Expect(stop).Should(Exit(0))

// This is ugly, but I don't see a better way
time.Sleep(10 * time.Second)

Expect(podmanTest.NumberOfContainersRunning()).To(Equal(0))
})

It("podman run with cgroups=split", func() {
SkipIfNotSystemd(podmanTest.CgroupManager, "do not test --cgroups=split if not running on systemd")
SkipIfRootlessCgroupsV1("Disable cgroups not supported on cgroupv1 for rootless users")
Expand Down

0 comments on commit 1ab833f

Please sign in to comment.