Merge pull request #26027 from Luap99/signal

sigproxy: ignore SIGSTOP and handle case where container is already removed

Author: openshift-merge-bot[bot]

pkg/domain/infra/abi/terminal/sigproxy_commn.go

@@ -28,12 +28,10 @@ func ProxySignals(ctr *libpod.Container) {
 	go func() {
 		for s := range sigBuffer {
 			syscallSignal := s.(syscall.Signal)
-			if signal.IsSignalIgnoredBySigProxy(syscallSignal) {
-				continue
-			}
 
 			if err := ctr.Kill(uint(syscallSignal)); err != nil {
-				if errors.Is(err, define.ErrCtrStateInvalid) {
+				// If the container is no longer running/removed do not log it as error.
+				if errors.Is(err, define.ErrCtrStateInvalid) || errors.Is(err, define.ErrNoSuchCtr) || errors.Is(err, define.ErrCtrRemoved) {
 					logrus.Infof("Ceasing signal forwarding to container %s as it has stopped", ctr.ID())
 				} else {
 					logrus.Errorf("forwarding signal %d to container %s: %v", s, ctr.ID(), err)

pkg/domain/infra/tunnel/runtime.go

@@ -46,9 +46,6 @@ func remoteProxySignals(ctrID string, killFunc func(string) error) {
 	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)

pkg/signal/signal_common.go

@@ -46,11 +46,14 @@ func ParseSignalNameOrNumber(rawSignal string) (syscall.Signal, error) {
 	return -1, fmt.Errorf("invalid signal: %s", basename)
 }
 
-// CatchAll catches all signals and relays them to the specified channel.
+// CatchAll catches all signals (except the ones that make no sense to handle/forward,
+// see isSignalIgnoredBySigProxy()) and relays them to the specified channel.
 func CatchAll(sigc chan os.Signal) {
 	handledSigs := make([]os.Signal, 0, len(SignalMap))
 	for _, s := range SignalMap {
-		handledSigs = append(handledSigs, s)
+		if !isSignalIgnoredBySigProxy(s) {
+			handledSigs = append(handledSigs, s)
+		}
 	}
 	signal.Notify(sigc, handledSigs...)
 }

pkg/signal/signal_linux.go

@@ -89,10 +89,16 @@ var SignalMap = map[string]syscall.Signal{
 	"RTMAX":    sigrtmax,
 }
 
-// IsSignalIgnoredBySigProxy determines whether sig-proxy should ignore syscall signal
-func IsSignalIgnoredBySigProxy(s syscall.Signal) bool {
+// 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
+	// SIGSTOP cannot be ignored/forwarded by userspace.
+	switch s {
+	case syscall.SIGCHLD, syscall.SIGPIPE, syscall.SIGURG, syscall.SIGSTOP:
+		return true
+	default:
+		return false
+	}
 }

pkg/signal/signal_linux_mipsx.go

@@ -89,10 +89,16 @@ var SignalMap = map[string]syscall.Signal{
 	"RTMAX":    sigrtmax,
 }
 
-// IsSignalIgnoredBySigProxy determines whether sig-proxy should ignore syscall signal
-func IsSignalIgnoredBySigProxy(s syscall.Signal) bool {
+// 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
+	// SIGSTOP cannot be ignored/forwarded by userspace.
+	switch s {
+	case syscall.SIGCHLD, syscall.SIGPIPE, syscall.SIGURG, syscall.SIGSTOP:
+		return true
+	default:
+		return false
+	}
 }

pkg/signal/signal_unix.go

@@ -87,10 +87,16 @@ var SignalMap = map[string]syscall.Signal{
 	"RTMAX":    sigrtmax,
 }
 
-// IsSignalIgnoredBySigProxy determines whether sig-proxy should ignore syscall signal
-func IsSignalIgnoredBySigProxy(s syscall.Signal) bool {
+// 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
+	// SIGSTOP cannot be ignored/forwarded by userspace.
+	switch s {
+	case syscall.SIGCHLD, syscall.SIGPIPE, syscall.SIGURG, syscall.SIGSTOP:
+		return true
+	default:
+		return false
+	}
 }

pkg/signal/signal_unsupported.go

@@ -87,8 +87,8 @@ var SignalMap = map[string]syscall.Signal{
 	"RTMAX":    sigrtmax,
 }
 
-// IsSignalIgnoredBySigProxy determines whether to sig-proxy should ignore syscall signal
+// 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 {
+func isSignalIgnoredBySigProxy(_ syscall.Signal) bool {
 	return false
 }