From 5b28a2b4a088b2e9fa0e6d574ac1d9b64054d65a Mon Sep 17 00:00:00 2001 From: Daniel J Walsh Date: Mon, 26 Feb 2024 10:06:35 -0500 Subject: [PATCH] Fix podman stop -t -1 CID Currently if a user specifies a negative time to stop a container the code ends up specifying the negative time to time.Duration which treats it as 0. By settine the default to max.Unint32 we end up with a positive number which indicates > 68 years which is probably close enough to infinity for our use case. Fixes: https://github.com/containers/podman/issues/21811 Signed-off-by: Daniel J Walsh --- pkg/util/utils.go | 4 ++-- pkg/util/utils_test.go | 15 +++++++++++++++ test/system/030-run.bats | 9 ++------- 3 files changed, 19 insertions(+), 9 deletions(-) diff --git a/pkg/util/utils.go b/pkg/util/utils.go index b93ec40c64..06ef0da3d5 100644 --- a/pkg/util/utils.go +++ b/pkg/util/utils.go @@ -1332,10 +1332,10 @@ 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 +// ConvertTimeout converts negative timeout to MaxUint32, which indicates approximately infinity, waiting to stop containers func ConvertTimeout(timeout int) uint { if timeout < 0 { - return math.MaxInt + return math.MaxUint32 } return uint(timeout) } diff --git a/pkg/util/utils_test.go b/pkg/util/utils_test.go index d680cfdc86..c412df5ffb 100644 --- a/pkg/util/utils_test.go +++ b/pkg/util/utils_test.go @@ -2,6 +2,7 @@ package util import ( "fmt" + "math" "testing" "time" @@ -573,3 +574,17 @@ func TestConvertMappings(t *testing.T) { assert.Equal(t, start[i].Size, convertedBack[i].Size) } } + +func TestConvertTimeout(t *testing.T) { + timeout := ConvertTimeout(0) + assert.Equal(t, uint(0), timeout) + + timeout = ConvertTimeout(100) + assert.Equal(t, uint(100), timeout) + + timeout = ConvertTimeout(-1) + assert.Equal(t, uint(math.MaxUint32), timeout) + + timeout = ConvertTimeout(-100) + assert.Equal(t, uint(math.MaxUint32), timeout) +} diff --git a/test/system/030-run.bats b/test/system/030-run.bats index a5f4e46a19..56acf6c5e1 100644 --- a/test/system/030-run.bats +++ b/test/system/030-run.bats @@ -1079,7 +1079,7 @@ $IMAGE--c_ok" \ run_podman stop -t 0 $cid # Actual test for 15895: with --systemd, no ttyN devices are passed through - run_podman run -d --privileged --systemd=always $IMAGE top + run_podman run -d --privileged --stop-signal=TERM --systemd=always $IMAGE top cid="$output" run_podman exec $cid sh -c "find /dev -regex '/dev/tty[0-9].*' | wc -w" @@ -1087,12 +1087,7 @@ $IMAGE--c_ok" \ "ls /dev/tty[0-9] with --systemd=always: should have no ttyN devices" # Make sure run_podman stop supports -1 option - # FIXME: do we really really mean to say FFFFFFFFFFFFFFFF here??? - run_podman 0+w stop -t -1 $cid - if ! is_remote; then - require_warning "StopSignal \(37\) failed to stop container .* in 18446744073709551615 seconds, resorting to SIGKILL" \ - "stop -t -1 (negative 1) issues warning" - fi + run_podman stop -t -1 $cid run_podman rm -t -1 -f $cid }