Merge pull request #19425 from rhatdan/service

Add support for passing container stop timeout as -1 (infinite)

cmd/podman/containers/restart.go

@@ -52,7 +52,7 @@ var (
 		Filters: make(map[string][]string),
 	}
 	restartCidFiles = []string{}
-	restartTimeout  uint
+	restartTimeout  int
 )
 
 func restartFlags(cmd *cobra.Command) {
@@ -70,7 +70,7 @@ func restartFlags(cmd *cobra.Command) {
 	_ = cmd.RegisterFlagCompletionFunc(filterFlagName, common.AutocompletePsFilters)
 
 	timeFlagName := "time"
-	flags.UintVarP(&restartTimeout, timeFlagName, "t", containerConfig.Engine.StopTimeout, "Seconds to wait for stop before killing the container")
+	flags.IntVarP(&restartTimeout, timeFlagName, "t", int(containerConfig.Engine.StopTimeout), "Seconds to wait for stop before killing the container")
 	_ = cmd.RegisterFlagCompletionFunc(timeFlagName, completion.AutocompleteNone)
 
 	if registry.IsRemote() {
@@ -102,7 +102,8 @@ func restart(cmd *cobra.Command, args []string) error {
 	args = utils.RemoveSlash(args)
 
 	if cmd.Flag("time").Changed {
-		restartOpts.Timeout = &restartTimeout
+		timeout := uint(restartTimeout)
+		restartOpts.Timeout = &timeout
 	}
 
 	for _, cidFile := range restartCidFiles {

cmd/podman/containers/rm.go

@@ -66,7 +66,7 @@ func rmFlags(cmd *cobra.Command) {
 	flags.BoolVarP(&rmOptions.Force, "force", "f", false, "Force removal of a running or unusable container")
 	flags.BoolVar(&rmOptions.Depend, "depend", false, "Remove container and all containers that depend on the selected container")
 	timeFlagName := "time"
-	flags.UintVarP(&stopTimeout, timeFlagName, "t", containerConfig.Engine.StopTimeout, "Seconds to wait for stop before killing the container")
+	flags.IntVarP(&stopTimeout, timeFlagName, "t", int(containerConfig.Engine.StopTimeout), "Seconds to wait for stop before killing the container")
 	_ = cmd.RegisterFlagCompletionFunc(timeFlagName, completion.AutocompleteNone)
 	flags.BoolVarP(&rmOptions.Volumes, "volumes", "v", false, "Remove anonymous volumes associated with the container")
 
@@ -105,7 +105,8 @@ func rm(cmd *cobra.Command, args []string) error {
 		if !rmOptions.Force {
 			return errors.New("--force option must be specified to use the --time option")
 		}
-		rmOptions.Timeout = &stopTimeout
+		timeout := uint(stopTimeout)
+		rmOptions.Timeout = &timeout
 	}
 	for _, cidFile := range rmCidFiles {
 		content, err := os.ReadFile(cidFile)

cmd/podman/containers/stop.go

@@ -53,7 +53,7 @@ var (
 		Filters: make(map[string][]string),
 	}
 	stopCidFiles = []string{}
-	stopTimeout  uint
+	stopTimeout  int
 )
 
 func stopFlags(cmd *cobra.Command) {
@@ -67,7 +67,7 @@ func stopFlags(cmd *cobra.Command) {
 	_ = cmd.RegisterFlagCompletionFunc(cidfileFlagName, completion.AutocompleteDefault)
 
 	timeFlagName := "time"
-	flags.UintVarP(&stopTimeout, timeFlagName, "t", containerConfig.Engine.StopTimeout, "Seconds to wait for stop before killing the container")
+	flags.IntVarP(&stopTimeout, timeFlagName, "t", int(containerConfig.Engine.StopTimeout), "Seconds to wait for stop before killing the container")
 	_ = cmd.RegisterFlagCompletionFunc(timeFlagName, completion.AutocompleteNone)
 
 	filterFlagName := "filter"
@@ -104,7 +104,8 @@ func stop(cmd *cobra.Command, args []string) error {
 	args = utils.RemoveSlash(args)
 
 	if cmd.Flag("time").Changed {
-		stopOptions.Timeout = &stopTimeout
+		timeout := uint(stopTimeout)
+		stopOptions.Timeout = &timeout
 	}
 	for _, cidFile := range stopCidFiles {
 		content, err := os.ReadFile(cidFile)

cmd/podman/networks/rm.go

@@ -27,7 +27,7 @@ var (
 		Args:              cobra.MinimumNArgs(1),
 		ValidArgsFunction: common.AutocompleteNetworks,
 	}
-	stopTimeout uint
+	stopTimeout int
 )
 
 var (
@@ -37,7 +37,7 @@ var (
 func networkRmFlags(flags *pflag.FlagSet) {
 	flags.BoolVarP(&networkRmOptions.Force, "force", "f", false, "remove any containers using network")
 	timeFlagName := "time"
-	flags.UintVarP(&stopTimeout, timeFlagName, "t", containerConfig.Engine.StopTimeout, "Seconds to wait for running containers to stop before killing the container")
+	flags.IntVarP(&stopTimeout, timeFlagName, "t", int(containerConfig.Engine.StopTimeout), "Seconds to wait for running containers to stop before killing the container")
 	_ = networkrmCommand.RegisterFlagCompletionFunc(timeFlagName, completion.AutocompleteNone)
 }
 
@@ -59,7 +59,8 @@ func networkRm(cmd *cobra.Command, args []string) error {
 		if !networkRmOptions.Force {
 			return errors.New("--force option must be specified to use the --time option")
 		}
-		networkRmOptions.Timeout = &stopTimeout
+		timeout := uint(stopTimeout)
+		networkRmOptions.Timeout = &timeout
 	}
 	responses, err := registry.ContainerEngine().NetworkRm(registry.Context(), args, networkRmOptions)
 	if err != nil {

cmd/podman/pods/rm.go

@@ -43,7 +43,7 @@ var (
   podman pod rm -f 860a4b23
   podman pod rm -f -a`,
 	}
-	stopTimeout uint
+	stopTimeout int
 )
 
 func init() {
@@ -62,7 +62,7 @@ func init() {
 	_ = rmCommand.RegisterFlagCompletionFunc(podIDFileFlagName, completion.AutocompleteDefault)
 
 	timeFlagName := "time"
-	flags.UintVarP(&stopTimeout, timeFlagName, "t", containerConfig.Engine.StopTimeout, "Seconds to wait for pod stop before killing the container")
+	flags.IntVarP(&stopTimeout, timeFlagName, "t", int(containerConfig.Engine.StopTimeout), "Seconds to wait for pod stop before killing the container")
 	_ = rmCommand.RegisterFlagCompletionFunc(timeFlagName, completion.AutocompleteNone)
 
 	validate.AddLatestFlag(rmCommand, &rmOptions.Latest)
@@ -79,7 +79,8 @@ func rm(cmd *cobra.Command, args []string) error {
 		if !rmOptions.Force {
 			return errors.New("--force option must be specified to use the --time option")
 		}
-		rmOptions.Timeout = &stopTimeout
+		timeout := uint(stopTimeout)
+		rmOptions.Timeout = &timeout
 	}
 
 	errs = append(errs, removePods(args, rmOptions.PodRmOptions, true)...)

cmd/podman/pods/stop.go

@@ -18,8 +18,8 @@ import (
 type podStopOptionsWrapper struct {
 	entities.PodStopOptions
 
-	PodIDFiles []string
+	podIDFiles []string
-	TimeoutCLI uint
+	timeoutCLI int
 }
 
 var (
@@ -55,11 +55,11 @@ func init() {
 	flags.BoolVarP(&stopOptions.Ignore, "ignore", "i", false, "Ignore errors when a specified pod is missing")
 
 	timeFlagName := "time"
-	flags.UintVarP(&stopOptions.TimeoutCLI, timeFlagName, "t", containerConfig.Engine.StopTimeout, "Seconds to wait for pod stop before killing the container")
+	flags.IntVarP(&stopOptions.timeoutCLI, timeFlagName, "t", int(containerConfig.Engine.StopTimeout), "Seconds to wait for pod stop before killing the container")
 	_ = stopCommand.RegisterFlagCompletionFunc(timeFlagName, completion.AutocompleteNone)
 
 	podIDFileFlagName := "pod-id-file"
-	flags.StringArrayVarP(&stopOptions.PodIDFiles, podIDFileFlagName, "", nil, "Write the pod ID to the file")
+	flags.StringArrayVarP(&stopOptions.podIDFiles, podIDFileFlagName, "", nil, "Write the pod ID to the file")
 	_ = stopCommand.RegisterFlagCompletionFunc(podIDFileFlagName, completion.AutocompleteDefault)
 
 	validate.AddLatestFlag(stopCommand, &stopOptions.Latest)
@@ -76,10 +76,10 @@ func stop(cmd *cobra.Command, args []string) error {
 		errs utils.OutputErrors
 	)
 	if cmd.Flag("time").Changed {
-		stopOptions.Timeout = int(stopOptions.TimeoutCLI)
+		stopOptions.Timeout = stopOptions.timeoutCLI
 	}
 
-	ids, err := specgenutil.ReadPodIDFiles(stopOptions.PodIDFiles)
+	ids, err := specgenutil.ReadPodIDFiles(stopOptions.podIDFiles)
 	if err != nil {
 		return err
 	}

cmd/podman/volumes/rm.go

@@ -34,7 +34,7 @@ var (
 
 var (
 	rmOptions   = entities.VolumeRmOptions{}
-	stopTimeout uint
+	stopTimeout int
 )
 
 func init() {
@@ -46,7 +46,7 @@ func init() {
 	flags.BoolVarP(&rmOptions.All, "all", "a", false, "Remove all volumes")
 	flags.BoolVarP(&rmOptions.Force, "force", "f", false, "Remove a volume by force, even if it is being used by a container")
 	timeFlagName := "time"
-	flags.UintVarP(&stopTimeout, timeFlagName, "t", containerConfig.Engine.StopTimeout, "Seconds to wait for running containers to stop before killing the container")
+	flags.IntVarP(&stopTimeout, timeFlagName, "t", int(containerConfig.Engine.StopTimeout), "Seconds to wait for running containers to stop before killing the container")
 	_ = rmCommand.RegisterFlagCompletionFunc(timeFlagName, completion.AutocompleteNone)
 }
 
@@ -61,7 +61,8 @@ func rm(cmd *cobra.Command, args []string) error {
 		if !rmOptions.Force {
 			return errors.New("--force option must be specified to use the --time option")
 		}
-		rmOptions.Timeout = &stopTimeout
+		timeout := uint(stopTimeout)
+		rmOptions.Timeout = &timeout
 	}
 	responses, err := registry.ContainerEngine().VolumeRm(context.Background(), args, rmOptions)
 	if err != nil {

docs/source/markdown/options/stop-timeout.md

@@ -5,4 +5,4 @@
 #### **--stop-timeout**=*seconds*
 
 Timeout to stop a container. Default is **10**.
-Remote connections use local containers.conf for defaults
+Remote connections use local containers.conf for defaults.

docs/source/markdown/options/time.md

@@ -5,3 +5,4 @@
 #### **--time**, **-t**=*seconds*
 
 Seconds to wait before forcibly stopping <<the container|running containers within the pod>>.
+Use -1 for infinite wait.

docs/source/markdown/podman-network-rm.1.md

@@ -17,7 +17,7 @@ running, the container is stopped and removed.
 
 #### **--time**, **-t**=*seconds*
 
-Seconds to wait before forcibly stopping the running containers that are using the specified network. The --force option must be specified to use the --time option.
+Seconds to wait before forcibly stopping the running containers that are using the specified network. The --force option must be specified to use the --time option. Use -1 for infinite wait.
 
 ## EXAMPLE
 

docs/source/markdown/podman-volume-rm.1.md

@@ -30,7 +30,7 @@ Print usage statement
 
 #### **--time**, **-t**=*seconds*
 
-Seconds to wait before forcibly stopping running containers that are using the specified volume. The --force option must be specified to use the --time option.
+Seconds to wait before forcibly stopping running containers that are using the specified volume. The --force option must be specified to use the --time option. Use -1 for infinite wait.
 
 ## EXAMPLES
 

libpod/oci_conmon_common.go

@@ -478,7 +478,7 @@ func (r *ConmonOCIRuntime) StopContainer(ctr *Container, timeout uint, all bool)
 			return nil
 		}
 
-		if err := waitContainerStop(ctr, time.Duration(timeout)*time.Second); err != nil {
+		if err := waitContainerStop(ctr, time.Duration(util.ConvertTimeout(int(timeout)))*time.Second); err != nil {
 			logrus.Debugf("Timed out stopping container %s with %s, resorting to SIGKILL: %v", ctr.ID(), unix.SignalName(syscall.Signal(stopSignal)), err)
 			logrus.Warnf("StopSignal %s failed to stop container %s in %d seconds, resorting to SIGKILL", unix.SignalName(syscall.Signal(stopSignal)), ctr.Name(), timeout)
 		} else {

libpod/options.go

@@ -884,7 +884,6 @@ func WithTimeout(timeout uint) CtrCreateOption {
 		if ctr.valid {
 			return define.ErrCtrFinalized
 		}
-
 		ctr.config.Timeout = timeout
 
 		return nil

pkg/api/handlers/compat/containers_stop.go

@@ -11,6 +11,7 @@ import (
 	api "github.com/containers/podman/v4/pkg/api/types"
 	"github.com/containers/podman/v4/pkg/domain/entities"
 	"github.com/containers/podman/v4/pkg/domain/infra/abi"
+	"github.com/containers/podman/v4/pkg/util"
 	"github.com/gorilla/schema"
 )
 
@@ -24,7 +25,7 @@ func StopContainer(w http.ResponseWriter, r *http.Request) {
 	// /{version}/containers/(name)/stop
 	query := struct {
 		Ignore        bool `schema:"ignore"`
-		DockerTimeout uint `schema:"t"`
+		DockerTimeout int  `schema:"t"`
 		LibpodTimeout uint `schema:"timeout"`
 	}{
 		// override any golang type defaults
@@ -43,7 +44,9 @@ func StopContainer(w http.ResponseWriter, r *http.Request) {
 		}
 	} else {
 		if _, found := r.URL.Query()["t"]; found {
-			options.Timeout = &query.DockerTimeout
+			// -1 is allowed in Docker API, meaning wait infinite long, translate -1 to math.MaxInt value seconds to wait.
+			timeout := util.ConvertTimeout(query.DockerTimeout)
+			options.Timeout = &timeout
 		}
 	}
 	con, err := runtime.LookupContainer(name)

pkg/domain/infra/tunnel/containers.go

@@ -182,7 +182,8 @@ func (ic *ContainerEngine) ContainerRestart(ctx context.Context, namesOrIds []st
 	)
 	options := new(containers.RestartOptions)
 	if to := opts.Timeout; to != nil {
-		options.WithTimeout(int(*to))
+		timeout := util.ConvertTimeout(int(*to))
+		options.WithTimeout(int(timeout))
 	}
 	ctrs, rawInputs, err := getContainersAndInputByContext(ic.ClientCtx, opts.All, false, namesOrIds, opts.Filters)
 	if err != nil {

pkg/util/utils.go

@@ -648,3 +648,11 @@ func ParseRestartPolicy(policy string) (string, uint, error) {
 	}
 	return policyType, retriesUint, nil
 }
+
+// ConvertTimeout converts negative timeout to MaxInt, which indicates approximately infinity, waiting to stop containers
+func ConvertTimeout(timeout int) uint {
+	if timeout < 0 {
+		return math.MaxInt
+	}
+	return uint(timeout)
+}

test/apiv2/22-stop.at

@@ -22,3 +22,13 @@ cid=$(jq -r .Id <<<"$output")
 t POST libpod/containers/$cid/stop  204
 t GET  libpod/containers/mytop/json 200 .State.Status~\\\(exited\\\|stopped\\\)
 t DELETE libpod/containers/mytop    200
+
+# Remember that podman() hides all output; we need to get our CID via inspect
+podman run -dt --name mytop $IMAGE top
+
+t GET  containers/mytop/json 200 .State.Status=running
+cid=$(jq -r .Id <<<"$output")
+t POST containers/$cid/stop?t=-1 204
+t POST "containers/$cid/wait" 200
+t GET  containers/mytop/json 200 .State.Status~\\\(exited\\\|stopped\\\)
+t DELETE containers/mytop    204

test/e2e/pod_stop_test.go

@@ -47,7 +47,7 @@ var _ = Describe("Podman pod stop", func() {
 		session.WaitWithDefaultTimeout()
 		Expect(session).Should(Exit(0))
 
-		session = podmanTest.Podman([]string{"pod", "stop", "--ignore", "test1", "bogus"})
+		session = podmanTest.Podman([]string{"pod", "stop", "-t", "-1", "--ignore", "test1", "bogus"})
 		session.WaitWithDefaultTimeout()
 		Expect(session).Should(Exit(0))
 	})

test/e2e/start_test.go

@@ -258,7 +258,7 @@ var _ = Describe("Podman start", func() {
 		Expect(env).To(ContainSubstring("HOME"))
 		Expect(env).ToNot(ContainSubstring(fmt.Sprintf("HOME=%s", home)))
 
-		session = podmanTest.Podman([]string{"restart", cid})
+		session = podmanTest.Podman([]string{"restart", "-t", "-1", cid})
 		session.WaitWithDefaultTimeout()
 		Expect(session).Should(Exit(0))
 

test/system/030-run.bats

@@ -884,6 +884,9 @@ EOF
     run_podman inspect --format "{{ .HostConfig.LogConfig.Size }}" $cid
     is "$output" "${size}MB"
     run_podman rm -t 0 -f $cid
+
+    # Make sure run_podman tm -t supports -1 option
+    run_podman rm -t -1 -f $cid
 }
 
 @test "podman run --kernel-memory warning" {
@@ -1025,14 +1028,16 @@ $IMAGE--c_ok" \
     run_podman stop -t 0 $cid
 
     # Actual test for 15895: with --systemd, no ttyN devices are passed through
-    run_podman run --rm -d --privileged --systemd=always $IMAGE ./pause
+    run_podman run -d --privileged --systemd=always $IMAGE top
     cid="$output"
 
     run_podman exec $cid sh -c "find /dev -regex '/dev/tty[0-9].*' | wc -w"
     assert "$output" = "0" \
            "ls /dev/tty[0-9] with --systemd=always: should have no ttyN devices"
 
-    run_podman stop -t 0 $cid
+    # Make sure run_podman stop supports -1 option
+    run_podman stop -t -1 $cid
+    run_podman rm -t -1 -f $cid
 }
 
 @test "podman run --privileged as rootless will not mount /dev/tty\d+" {

test/system/160-volumes.bats

@@ -475,7 +475,7 @@ NeedsChown    | true
     test -e "$mountpoint/passwd"
 
     # Clean up
-    run_podman volume rm $myvolume
+    run_podman volume rm -t -1 --force $myvolume
 }
 
 @test "podman volume mount" {

test/system/200-pod.bats

@@ -465,7 +465,7 @@ EOF
     run_podman run --pod $podID $IMAGE true
     run_podman pod inspect $podID --format "{{.State}}"
     _ensure_pod_state $podID Exited
-    run_podman pod rm $podID
+    run_podman pod rm -t -1 -f $podID
 }
 
 @test "pod exit policies - play kube" {
@@ -560,7 +560,7 @@ io.max          | $lomajmin rbps=1048576 wbps=1048576 riops=max wiops=max
 @test "podman pod rm --force bogus" {
     run_podman 1 pod rm bogus
     is "$output" "Error: .*bogus.*: no such pod" "Should print error"
-    run_podman pod rm --force bogus
+    run_podman pod rm -t -1 --force bogus
     is "$output" "" "Should print no output"
 }
 

test/system/500-networking.bats

@@ -790,7 +790,7 @@ EOF
 @test "podman network rm --force bogus" {
     run_podman 1 network rm bogus
     is "$output" "Error: unable to find network with name or ID bogus: network not found" "Should print error"
-    run_podman network rm --force bogus
+    run_podman network rm -t -1 --force bogus
     is "$output" "" "Should print no output"
 }
 

test/system/700-play.bats

@@ -597,7 +597,7 @@ EOF
     run cat $YAML
     is "$output" ".*filetype: usr_t" "Generated YAML file should contain filetype usr_t"
     run_podman pod rm --force pod1
-    run_podman volume rm myvol --force
+    run_podman volume rm -t -1 myvol --force
 
     run_podman kube play $YAML
     if selinux_enabled; then