Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support to sig-proxy for podman-remote #15131

Merged
merged 1 commit into from
Sep 22, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 4 additions & 12 deletions pkg/domain/infra/abi/terminal/sigproxy_commn.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
7 changes: 7 additions & 0 deletions pkg/domain/infra/tunnel/containers.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
31 changes: 31 additions & 0 deletions pkg/domain/infra/tunnel/runtime.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)
}
}
}
}()
}
15 changes: 15 additions & 0 deletions pkg/signal/signal_common.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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) {
vrothberg marked this conversation as resolved.
Show resolved Hide resolved
for k, v := range SignalMap {
if v == s {
return k, nil
}
}
return "", fmt.Errorf("unknown syscall signal: %s", s)
}
49 changes: 49 additions & 0 deletions pkg/signal/signal_common_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
})
}
}
8 changes: 8 additions & 0 deletions pkg/signal/signal_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
8 changes: 8 additions & 0 deletions pkg/signal/signal_linux_mipsx.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
8 changes: 8 additions & 0 deletions pkg/signal/signal_unix.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
6 changes: 6 additions & 0 deletions pkg/signal/signal_unsupported.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
43 changes: 43 additions & 0 deletions test/system/032-sig-proxy.bats
Original file line number Diff line number Diff line change
@@ -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