From 1ab833fb73e3ac27c8bd2d9fcec11d39ca04b490 Mon Sep 17 00:00:00 2001 From: Matthew Heon Date: Wed, 11 Jan 2023 10:26:26 -0500 Subject: [PATCH] Set StoppedByUser earlier in the process of stopping 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 #17069 Signed-off-by: Matthew Heon --- libpod/container_internal.go | 2 +- test/e2e/run_test.go | 16 ++++++++++++++++ 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/libpod/container_internal.go b/libpod/container_internal.go index bf6e1623e5..c37d6be2b1 100644 --- a/libpod/container_internal.go +++ b/libpod/container_internal.go @@ -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) @@ -1343,7 +1344,6 @@ func (c *Container) stop(timeout uint) error { } c.newContainerEvent(events.Stop) - c.state.StoppedByUser = true return c.waitForConmonToExitAndSave() } diff --git a/test/e2e/run_test.go b/test/e2e/run_test.go index 5f8af05e9f..5fb8a3def9 100644 --- a/test/e2e/run_test.go +++ b/test/e2e/run_test.go @@ -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")