diff options
| -rw-r--r-- | cmd/podman/kill.go | 4 | ||||
| -rw-r--r-- | cmd/podman/pod_kill.go | 4 | ||||
| -rw-r--r-- | cmd/podman/shared/create.go | 5 | ||||
| -rw-r--r-- | pkg/util/utils.go | 21 | ||||
| -rw-r--r-- | test/system/130-kill.bats | 92 |
5 files changed, 116 insertions, 10 deletions
diff --git a/cmd/podman/kill.go b/cmd/podman/kill.go index aba2008ca..a10546ea9 100644 --- a/cmd/podman/kill.go +++ b/cmd/podman/kill.go @@ -3,7 +3,7 @@ package main import ( "github.com/containers/libpod/cmd/podman/cliconfig" "github.com/containers/libpod/pkg/adapter" - "github.com/docker/docker/pkg/signal" + "github.com/containers/libpod/pkg/util" "github.com/opentracing/opentracing-go" "github.com/pkg/errors" "github.com/spf13/cobra" @@ -54,7 +54,7 @@ func killCmd(c *cliconfig.KillValues) error { // Check if the signalString provided by the user is valid // Invalid signals will return err - killSignal, err := signal.ParseSignal(c.Signal) + killSignal, err := util.ParseSignal(c.Signal) if err != nil { return err } diff --git a/cmd/podman/pod_kill.go b/cmd/podman/pod_kill.go index 064946f72..9f696073d 100644 --- a/cmd/podman/pod_kill.go +++ b/cmd/podman/pod_kill.go @@ -6,7 +6,7 @@ import ( "github.com/containers/libpod/cmd/podman/cliconfig" "github.com/containers/libpod/pkg/adapter" - "github.com/docker/docker/pkg/signal" + "github.com/containers/libpod/pkg/util" "github.com/pkg/errors" "github.com/sirupsen/logrus" "github.com/spf13/cobra" @@ -60,7 +60,7 @@ func podKillCmd(c *cliconfig.PodKillValues) error { if c.Signal != "" { // Check if the signalString provided by the user is valid // Invalid signals will return err - sysSignal, err := signal.ParseSignal(c.Signal) + sysSignal, err := util.ParseSignal(c.Signal) if err != nil { return err } diff --git a/cmd/podman/shared/create.go b/cmd/podman/shared/create.go index c1c5db7cb..58cf56eea 100644 --- a/cmd/podman/shared/create.go +++ b/cmd/podman/shared/create.go @@ -24,7 +24,6 @@ import ( "github.com/containers/libpod/pkg/rootless" cc "github.com/containers/libpod/pkg/spec" "github.com/containers/libpod/pkg/util" - "github.com/docker/docker/pkg/signal" "github.com/docker/go-connections/nat" "github.com/docker/go-units" "github.com/opentracing/opentracing-go" @@ -464,7 +463,7 @@ func ParseCreateOpts(ctx context.Context, c *GenericCLIResults, runtime *libpod. signalString = c.String("stop-signal") } if signalString != "" { - stopSignal, err = signal.ParseSignal(signalString) + stopSignal, err = util.ParseSignal(signalString) if err != nil { return nil, err } @@ -624,7 +623,7 @@ func ParseCreateOpts(ctx context.Context, c *GenericCLIResults, runtime *libpod. } if systemd { if signalString == "" { - stopSignal, err = signal.ParseSignal("RTMIN+3") + stopSignal, err = util.ParseSignal("RTMIN+3") if err != nil { return nil, errors.Wrapf(err, "error parsing systemd signal") } diff --git a/pkg/util/utils.go b/pkg/util/utils.go index 5b4dfe9fa..f7d04c73b 100644 --- a/pkg/util/utils.go +++ b/pkg/util/utils.go @@ -9,6 +9,7 @@ import ( "strconv" "strings" "sync" + "syscall" "time" "github.com/BurntSushi/toml" @@ -284,9 +285,7 @@ func GetImageConfig(changes []string) (ImageConfig, error) { config.Labels[key] = val case "STOPSIGNAL": // Check the provided signal for validity. - // TODO: Worth checking range? ParseSignal allows - // negative numbers. - killSignal, err := signal.ParseSignal(value) + killSignal, err := ParseSignal(value) if err != nil { return ImageConfig{}, errors.Wrapf(err, "invalid change %q - KILLSIGNAL must be given a valid signal", change) } @@ -305,6 +304,22 @@ func GetImageConfig(changes []string) (ImageConfig, error) { return config, nil } +// Parse and validate a signal name or number +func ParseSignal(rawSignal string) (syscall.Signal, error) { + // Strip off leading dash, to allow -1 or -HUP + basename := strings.TrimPrefix(rawSignal, "-") + + signal, err := signal.ParseSignal(basename) + if err != nil { + return -1, err + } + // 64 is SIGRTMAX; wish we could get this from a standard Go library + if signal < 1 || signal > 64 { + return -1, errors.Errorf("valid signals are 1 through 64") + } + return signal, nil +} + // ParseIDMapping takes idmappings and subuid and subgid maps and returns a storage mapping func ParseIDMapping(mode namespaces.UsernsMode, UIDMapSlice, GIDMapSlice []string, subUIDMap, subGIDMap string) (*storage.IDMappingOptions, error) { options := storage.IDMappingOptions{ diff --git a/test/system/130-kill.bats b/test/system/130-kill.bats new file mode 100644 index 000000000..af3409b9b --- /dev/null +++ b/test/system/130-kill.bats @@ -0,0 +1,92 @@ +#!/usr/bin/env bats -*- bats -*- +# +# tests for podman kill +# + +load helpers + +@test "podman kill - test signal handling in containers" { + # Start a container that will handle all signals by emitting 'got: N' + local -a signals=(1 2 3 4 5 6 8 10 12 13 14 15 16 20 21 22 23 24 25 26 64) + run_podman run -d $IMAGE sh -c "for i in ${signals[*]}; do trap \"echo got: \$i\" \$i; done; echo READY; while ! test -e /stop; do sleep 0.05; done;echo DONE" + cid="$output" + + # Run 'logs -f' on that container, but run it in the background with + # redirection to a named pipe from which we (foreground job) read + # and confirm that signals are received. We can't use run_podman here. + local fifo=${PODMAN_TMPDIR}/podman-kill-fifo.$(random_string 10) + mkfifo $fifo + $PODMAN logs -f $cid >$fifo & + podman_log_pid=$! + # First container emits READY when ready; wait for it. + read -t 10 ready <$fifo + is "$ready" "READY" "first log message from container" + + # Helper function: send the given signal, verify that it's received. + kill_and_check() { + local signal=$1 + local signum=${2:-$1} # e.g. if signal=HUP, we expect to see '1' + + run_podman kill -s $signal $cid + read -t 10 actual <$fifo + is "$actual" "got: $signum" "Signal $signal handled by container" + } + + # Send signals in random order; make sure each one is received + for s in $(fmt --width=2 <<< "${signals[*]}" | sort --random-sort);do + kill_and_check $s + done + + # Variations: with leading dash; by name, with/without dash or SIG + kill_and_check -1 1 + kill_and_check -INT 2 + kill_and_check FPE 8 + kill_and_check -SIGUSR1 10 + kill_and_check SIGUSR2 12 + + # Done. Tell the container to stop, and wait for final DONE + run_podman exec $cid touch /stop + read -t 5 done <$fifo + is "$done" "DONE" "final log message from container" + + # Clean up + run_podman wait $cid + run_podman rm $cid + wait $podman_log_pid +} + +@test "podman kill - rejects invalid args" { + # These errors are thrown by the imported docker/signal.ParseSignal() + local -a bad_signal_names=(0 SIGBADSIG SIG BADSIG %% ! "''" '""' " ") + for s in ${bad_signal_names[@]}; do + # 'nosuchcontainer' is fine: podman should bail before it gets there + run_podman 125 kill -s $s nosuchcontainer + is "$output" "Error: Invalid signal: $s" "Error from kill -s $s" + + run_podman 125 pod kill -s $s nosuchpod + is "$output" "Error: Invalid signal: $s" "Error from pod kill -s $s" + done + + # Special case: these too are thrown by docker/signal.ParseSignal(), + # but the dash sign is stripped by our wrapper in utils, so the + # error message doesn't include the dash. + local -a bad_dash_signals=(-0 -SIGBADSIG -SIG -BADSIG -) + for s in ${bad_dash_signals[@]}; do + run_podman 125 kill -s $s nosuchcontainer + is "$output" "Error: Invalid signal: ${s##-}" "Error from kill -s $s" + done + + # This error (signal out of range) is thrown by our wrapper + local -a bad_signal_nums=(65 -65 96 999 99999999) + for s in ${bad_signal_nums[@]}; do + run_podman 125 kill -s $s nosuchcontainer + is "$output" "Error: valid signals are 1 through 64" \ + "Error from kill -s $s" + done + + # 'podman create' uses the same parsing code + run_podman 125 create --stop-signal=99 $IMAGE + is "$output" "Error: valid signals are 1 through 64" "podman create" +} + +# vim: filetype=sh |