From 7cfe0328f1c231ed318c38938479f7dec7fc97fa Mon Sep 17 00:00:00 2001 From: Boaz Shuster Date: Tue, 20 Sep 2022 12:11:39 +0300 Subject: Add support to sig-proxy for podman-remote Signed-off-by: Boaz Shuster --- pkg/domain/infra/abi/terminal/sigproxy_commn.go | 16 ++------ pkg/domain/infra/tunnel/containers.go | 7 ++++ pkg/domain/infra/tunnel/runtime.go | 31 ++++++++++++++++ pkg/signal/signal_common.go | 15 ++++++++ pkg/signal/signal_common_test.go | 49 +++++++++++++++++++++++++ pkg/signal/signal_linux.go | 8 ++++ pkg/signal/signal_linux_mipsx.go | 8 ++++ pkg/signal/signal_unix.go | 8 ++++ pkg/signal/signal_unsupported.go | 6 +++ test/system/032-sig-proxy.bats | 43 ++++++++++++++++++++++ 10 files changed, 179 insertions(+), 12 deletions(-) create mode 100644 test/system/032-sig-proxy.bats diff --git a/pkg/domain/infra/abi/terminal/sigproxy_commn.go b/pkg/domain/infra/abi/terminal/sigproxy_commn.go index 3a0132ef3..d42685508 100644 --- a/pkg/domain/infra/abi/terminal/sigproxy_commn.go +++ b/pkg/domain/infra/abi/terminal/sigproxy_commn.go @@ -15,33 +15,25 @@ import ( "github.com/sirupsen/logrus" ) -// Make sure the signal buffer is sufficiently big. -// runc is using the same value. -const signalBufferSize = 2048 - // ProxySignals ... func ProxySignals(ctr *libpod.Container) { // Stop catching the shutdown signals (SIGINT, SIGTERM) - they're going // to the container now. shutdown.Stop() //nolint: errcheck - sigBuffer := make(chan os.Signal, signalBufferSize) + sigBuffer := make(chan os.Signal, signal.SignalBufferSize) signal.CatchAll(sigBuffer) logrus.Debugf("Enabling signal proxying") go func() { for s := range sigBuffer { - // Ignore SIGCHLD and SIGPIPE - these are mostly likely - // intended for the podman command itself. - // SIGURG was added because of golang 1.14 and its preemptive changes - // causing more signals to "show up". - // https://github.com/containers/podman/issues/5483 - if s == syscall.SIGCHLD || s == syscall.SIGPIPE || s == syscall.SIGURG { + syscallSignal := s.(syscall.Signal) + if signal.IsSignalIgnoredBySigProxy(syscallSignal) { continue } - if err := ctr.Kill(uint(s.(syscall.Signal))); err != nil { + if err := ctr.Kill(uint(syscallSignal)); err != nil { if errors.Is(err, define.ErrCtrStateInvalid) { logrus.Infof("Ceasing signal forwarding to container %s as it has stopped", ctr.ID()) } else { diff --git a/pkg/domain/infra/tunnel/containers.go b/pkg/domain/infra/tunnel/containers.go index 0dc73081d..dd296f710 100644 --- a/pkg/domain/infra/tunnel/containers.go +++ b/pkg/domain/infra/tunnel/containers.go @@ -825,6 +825,13 @@ func (ic *ContainerEngine) ContainerRun(ctx context.Context, opts entities.Conta } // Attach + if opts.SigProxy { + remoteProxySignals(con.ID, func(signal string) error { + killOpts := entities.KillOptions{All: false, Latest: false, Signal: signal} + _, err := ic.ContainerKill(ctx, []string{con.ID}, killOpts) + return err + }) + } if err := startAndAttach(ic, con.ID, &opts.DetachKeys, opts.InputStream, opts.OutputStream, opts.ErrorStream); err != nil { if err == define.ErrDetach { return &report, nil diff --git a/pkg/domain/infra/tunnel/runtime.go b/pkg/domain/infra/tunnel/runtime.go index 6542ea5b7..75bd4ef5e 100644 --- a/pkg/domain/infra/tunnel/runtime.go +++ b/pkg/domain/infra/tunnel/runtime.go @@ -2,6 +2,12 @@ package tunnel import ( "context" + "os" + "syscall" + + "github.com/containers/podman/v4/libpod/define" + "github.com/containers/podman/v4/pkg/signal" + "github.com/sirupsen/logrus" ) // Image-related runtime using an ssh-tunnel to utilize Podman service @@ -18,3 +24,28 @@ type ContainerEngine struct { type SystemEngine struct { ClientCtx context.Context } + +func remoteProxySignals(ctrID string, killFunc func(string) error) { + sigBuffer := make(chan os.Signal, signal.SignalBufferSize) + signal.CatchAll(sigBuffer) + + logrus.Debugf("Enabling signal proxying") + + go func() { + for s := range sigBuffer { + syscallSignal := s.(syscall.Signal) + if signal.IsSignalIgnoredBySigProxy(syscallSignal) { + continue + } + signalName, err := signal.ParseSysSignalToName(syscallSignal) + if err != nil { + logrus.Infof("Ceasing signal %v forwarding to container %s as it has stopped: %s", s, ctrID, err) + } + if err := killFunc(signalName); err != nil { + if err.Error() == define.ErrCtrStateInvalid.Error() { + logrus.Debugf("Ceasing signal %q forwarding to container %s as it has stopped", signalName, ctrID) + } + } + } + }() +} diff --git a/pkg/signal/signal_common.go b/pkg/signal/signal_common.go index fc1ecc04d..a81d0461b 100644 --- a/pkg/signal/signal_common.go +++ b/pkg/signal/signal_common.go @@ -9,6 +9,10 @@ import ( "syscall" ) +// Make sure the signal buffer is sufficiently big. +// runc is using the same value. +const SignalBufferSize = 2048 + // ParseSignal translates a string to a valid syscall signal. // It returns an error if the signal map doesn't include the given signal. func ParseSignal(rawSignal string) (syscall.Signal, error) { @@ -56,3 +60,14 @@ func StopCatch(sigc chan os.Signal) { signal.Stop(sigc) close(sigc) } + +// ParseSysSignalToName translates syscall.Signal to its name in the operating system. +// For example, syscall.Signal(9) will return "KILL" on Linux system. +func ParseSysSignalToName(s syscall.Signal) (string, error) { + for k, v := range SignalMap { + if v == s { + return k, nil + } + } + return "", fmt.Errorf("unknown syscall signal: %s", s) +} diff --git a/pkg/signal/signal_common_test.go b/pkg/signal/signal_common_test.go index c4ae6b389..bd9b230f7 100644 --- a/pkg/signal/signal_common_test.go +++ b/pkg/signal/signal_common_test.go @@ -118,3 +118,52 @@ func TestParseSignalNameOrNumber(t *testing.T) { }) } } + +func TestParseSysSignalToName(t *testing.T) { + type args struct { + signal syscall.Signal + } + tests := []struct { + name string + args args + want string + wantErr bool + }{ + { + name: "Kill should work", + args: args{ + signal: syscall.SIGKILL, + }, + want: "KILL", + wantErr: false, + }, + { + name: "Non-defined signal number should not work", + args: args{ + signal: 923, + }, + want: "", + wantErr: true, + }, + { + name: "garbage should fail", + args: args{ + signal: -1, + }, + want: "", + wantErr: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := ParseSysSignalToName(tt.args.signal) + if (err != nil) != tt.wantErr { + t.Errorf("ParseSysSignalToName() error = %v, wantErr %v", err, tt.wantErr) + return + } + if got != tt.want { + t.Errorf("ParseSysSignalToName() got = %v, want %v", got, tt.want) + } + }) + } +} diff --git a/pkg/signal/signal_linux.go b/pkg/signal/signal_linux.go index 5103b6033..81e4ed758 100644 --- a/pkg/signal/signal_linux.go +++ b/pkg/signal/signal_linux.go @@ -89,3 +89,11 @@ var SignalMap = map[string]syscall.Signal{ "RTMAX-1": sigrtmax - 1, "RTMAX": sigrtmax, } + +// IsSignalIgnoredBySigProxy determines whether sig-proxy should ignore syscall signal +func IsSignalIgnoredBySigProxy(s syscall.Signal) bool { + // Ignore SIGCHLD and SIGPIPE - these are most likely intended for the podman command itself. + // SIGURG was added because of golang 1.14 and its preemptive changes causing more signals to "show up". + // https://github.com/containers/podman/issues/5483 + return s == syscall.SIGCHLD || s == syscall.SIGPIPE || s == syscall.SIGURG +} diff --git a/pkg/signal/signal_linux_mipsx.go b/pkg/signal/signal_linux_mipsx.go index cdf9ad4c5..c97eeb23d 100644 --- a/pkg/signal/signal_linux_mipsx.go +++ b/pkg/signal/signal_linux_mipsx.go @@ -90,3 +90,11 @@ var SignalMap = map[string]syscall.Signal{ "RTMAX-1": sigrtmax - 1, "RTMAX": sigrtmax, } + +// IsSignalIgnoredBySigProxy determines whether sig-proxy should ignore syscall signal +func IsSignalIgnoredBySigProxy(s syscall.Signal) bool { + // Ignore SIGCHLD and SIGPIPE - these are most likely intended for the podman command itself. + // SIGURG was added because of golang 1.14 and its preemptive changes causing more signals to "show up". + // https://github.com/containers/podman/issues/5483 + return s == syscall.SIGCHLD || s == syscall.SIGPIPE || s == syscall.SIGURG +} diff --git a/pkg/signal/signal_unix.go b/pkg/signal/signal_unix.go index 7919e3670..01d99d7bc 100644 --- a/pkg/signal/signal_unix.go +++ b/pkg/signal/signal_unix.go @@ -87,3 +87,11 @@ var SignalMap = map[string]syscall.Signal{ "RTMAX-1": sigrtmax - 1, "RTMAX": sigrtmax, } + +// IsSignalIgnoredBySigProxy determines whether sig-proxy should ignore syscall signal +func IsSignalIgnoredBySigProxy(s syscall.Signal) bool { + // Ignore SIGCHLD and SIGPIPE - these are most likely intended for the podman command itself. + // SIGURG was added because of golang 1.14 and its preemptive changes causing more signals to "show up". + // https://github.com/containers/podman/issues/5483 + return s == syscall.SIGCHLD || s == syscall.SIGPIPE || s == syscall.SIGURG +} diff --git a/pkg/signal/signal_unsupported.go b/pkg/signal/signal_unsupported.go index 19ae93a61..590aaf978 100644 --- a/pkg/signal/signal_unsupported.go +++ b/pkg/signal/signal_unsupported.go @@ -87,3 +87,9 @@ var SignalMap = map[string]syscall.Signal{ "RTMAX-1": sigrtmax - 1, "RTMAX": sigrtmax, } + +// IsSignalIgnoredBySigProxy determines whether to sig-proxy should ignore syscall signal +// keep the container running or not. In unsupported OS this should not ignore any syscall signal. +func IsSignalIgnoredBySigProxy(s syscall.Signal) bool { + return false +} diff --git a/test/system/032-sig-proxy.bats b/test/system/032-sig-proxy.bats new file mode 100644 index 000000000..686df0e1b --- /dev/null +++ b/test/system/032-sig-proxy.bats @@ -0,0 +1,43 @@ +#!/usr/bin/env bats + +load helpers + +@test "podman sigkill" { + $PODMAN run -i --name foo $IMAGE sh -c 'trap "echo BYE;exit 0" INT;echo READY;while :;do sleep 0.1;done' & + local kidpid=$! + + # Wait for container to appear + local timeout=5 + while :;do + sleep 0.5 + run_podman '?' container exists foo + if [[ $status -eq 0 ]]; then + break + fi + timeout=$((timeout - 1)) + if [[ $timeout -eq 0 ]]; then + die "Timed out waiting for container to start" + fi + done + + wait_for_ready foo + + # Signal, and wait for container to exit + kill -INT $kidpid + local timeout=5 + while :;do + sleep 0.5 + run_podman logs foo + if [[ "$output" =~ BYE ]]; then + break + fi + timeout=$((timeout - 1)) + if [[ $timeout -eq 0 ]]; then + die "Timed out waiting for BYE from container" + fi + done + + run_podman rm -f -t0 foo +} + +# vim: filetype=sh -- cgit v1.2.3-54-g00ecf