Merge pull request #1216 from zmrow/containerd-backport

Containerd backports for shim logs and signal forwarder issues

packages/containerd/4001-Exit-signal-forward-if-process-not-found.patch

@@ -0,0 +1,38 @@
+From 11325afdb7e906c65e84009029a95011b077858f Mon Sep 17 00:00:00 2001
+From: Brian Goff <cpuguy83@gmail.com>
+Date: Fri, 4 Sep 2020 15:51:30 -0700
+Subject: [PATCH 1/2] Exit signal forward if process not found
+
+Previously the signal loop can end up racing with the process exiting.
+Intead of logging and continuing the loop, exit early.
+
+Signed-off-by: Brian Goff <cpuguy83@gmail.com>
+(cherry picked from commit 6650510836704c3ccf2af4eca60e4e9487f91601)
+Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
+---
+ cmd/ctr/commands/signals.go | 5 +++++
+ 1 file changed, 5 insertions(+)
+
+diff --git a/cmd/ctr/commands/signals.go b/cmd/ctr/commands/signals.go
+index 51afb0f7bd..d0c1daa9b5 100644
+--- a/cmd/ctr/commands/signals.go
++++ b/cmd/ctr/commands/signals.go
+@@ -23,6 +23,7 @@ import (
+ 	"syscall"
+
+ 	"github.com/containerd/containerd"
++	"github.com/containerd/containerd/errdefs"
+ 	"github.com/sirupsen/logrus"
+ )
+
+@@ -38,6 +39,10 @@ func ForwardAllSignals(ctx gocontext.Context, task killer) chan os.Signal {
+ 		for s := range sigc {
+ 			logrus.Debug("forwarding signal ", s)
+ 			if err := task.Kill(ctx, s.(syscall.Signal)); err != nil {
++				if errdefs.IsNotFound(err) {
++					logrus.WithError(err).Debugf("Not forwarding signal %s", s)
++					return
++				}
+ 				logrus.WithError(err).Errorf("forward signal %s", s)
+ 			}
+ 		}

packages/containerd/4002-Ignore-SIGURG-signals-in-signal-forwarder.patch

@@ -0,0 +1,104 @@
+From 1850de7af9edd5a40e9be1c6e2b925ac993274fa Mon Sep 17 00:00:00 2001
+From: Brian Goff <cpuguy83@gmail.com>
+Date: Fri, 4 Sep 2020 15:57:30 -0700
+Subject: [PATCH 2/2] Ignore SIGURG signals in signal forwarder
+
+Starting with go1.14, the go runtime hijacks SIGURG but with no way to
+not send to other signal handlers.
+
+In practice, we get this signal frequently.
+I found this while testing out go1.15 with ctr and multiple execs with
+only `echo hello`. When the process exits quickly, if the previous
+commit is not applied, you end up with an error message that it couldn't
+forward SIGURG to the container (due to the process being gone).
+
+Signed-off-by: Brian Goff <cpuguy83@gmail.com>
+(cherry picked from commit 899b4e3cb55b8e3f41d5b26d312d7c29a5b53b09)
+Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
+---
+ cmd/ctr/commands/signals.go          |  4 ++++
+ cmd/ctr/commands/signals_linux.go    | 27 +++++++++++++++++++++++++++
+ cmd/ctr/commands/signals_notlinux.go | 25 +++++++++++++++++++++++++
+ 3 files changed, 56 insertions(+)
+ create mode 100644 cmd/ctr/commands/signals_linux.go
+ create mode 100644 cmd/ctr/commands/signals_notlinux.go
+
+diff --git a/cmd/ctr/commands/signals.go b/cmd/ctr/commands/signals.go
+index d0c1daa9b5..311608c26c 100644
+--- a/cmd/ctr/commands/signals.go
++++ b/cmd/ctr/commands/signals.go
+@@ -37,6 +37,10 @@ func ForwardAllSignals(ctx gocontext.Context, task killer) chan os.Signal {
+ 	signal.Notify(sigc)
+ 	go func() {
+ 		for s := range sigc {
++			if canIgnoreSignal(s) {
++				logrus.Debugf("Ignoring signal %s", s)
++				continue
++			}
+ 			logrus.Debug("forwarding signal ", s)
+ 			if err := task.Kill(ctx, s.(syscall.Signal)); err != nil {
+ 				if errdefs.IsNotFound(err) {
+diff --git a/cmd/ctr/commands/signals_linux.go b/cmd/ctr/commands/signals_linux.go
+new file mode 100644
+index 0000000000..f41abfcfd3
+--- /dev/null
++++ b/cmd/ctr/commands/signals_linux.go
+@@ -0,0 +1,27 @@
++/*
++   Copyright The containerd Authors.
++
++   Licensed under the Apache License, Version 2.0 (the "License");
++   you may not use this file except in compliance with the License.
++   You may obtain a copy of the License at
++
++       http://www.apache.org/licenses/LICENSE-2.0
++
++   Unless required by applicable law or agreed to in writing, software
++   distributed under the License is distributed on an "AS IS" BASIS,
++   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
++   See the License for the specific language governing permissions and
++   limitations under the License.
++*/
++
++package commands
++
++import (
++	"os"
++
++	"golang.org/x/sys/unix"
++)
++
++func canIgnoreSignal(s os.Signal) bool {
++	return s == unix.SIGURG
++}
+diff --git a/cmd/ctr/commands/signals_notlinux.go b/cmd/ctr/commands/signals_notlinux.go
+new file mode 100644
+index 0000000000..6a9dccbc4e
+--- /dev/null
++++ b/cmd/ctr/commands/signals_notlinux.go
+@@ -0,0 +1,25 @@
++//+build !linux
++
++/*
++   Copyright The containerd Authors.
++
++   Licensed under the Apache License, Version 2.0 (the "License");
++   you may not use this file except in compliance with the License.
++   You may obtain a copy of the License at
++
++       http://www.apache.org/licenses/LICENSE-2.0
++
++   Unless required by applicable law or agreed to in writing, software
++   distributed under the License is distributed on an "AS IS" BASIS,
++   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
++   See the License for the specific language governing permissions and
++   limitations under the License.
++*/
++
++package commands
++
++import "os"
++
++func canIgnoreSignal(_ os.Signal) bool {
++	return false
++}

packages/containerd/5001-Always-consume-shim-logs.patch

@@ -0,0 +1,65 @@
+From 42f3871864a49f492d7a3f014ea3930c5f20b14d Mon Sep 17 00:00:00 2001
+From: Brian Goff <cpuguy83@gmail.com>
+Date: Wed, 9 Sep 2020 16:42:35 -0700
+Subject: [PATCH] Always consume shim logs
+
+These fifos fill up if unconsumed, so always consume them.
+
+Signed-off-by: Brian Goff <cpuguy83@gmail.com>
+(cherry picked from commit dab7bd0c4549a6a012004326f7415770c23afde4)
+Signed-off-by: Derek McGowan <derek@mcg.dev>
+---
+ runtime/v1/shim/client/client.go | 31 +++++++++++++++++--------------
+ 1 file changed, 17 insertions(+), 14 deletions(-)
+
+diff --git a/runtime/v1/shim/client/client.go b/runtime/v1/shim/client/client.go
+index 562ee6ca48..9653454afc 100644
+--- a/runtime/v1/shim/client/client.go
++++ b/runtime/v1/shim/client/client.go
+@@ -22,6 +22,7 @@ import (
+ 	"context"
+ 	"fmt"
+ 	"io"
++	"io/ioutil"
+ 	"net"
+ 	"os"
+ 	"os/exec"
+@@ -67,22 +68,24 @@ func WithStart(binary, address, daemonAddress, cgroup string, debug bool, exitHa
+ 		}
+ 		defer f.Close()
+
+-		var stdoutLog io.ReadWriteCloser
+-		var stderrLog io.ReadWriteCloser
+-		if debug {
+-			stdoutLog, err = v1.OpenShimStdoutLog(ctx, config.WorkDir)
+-			if err != nil {
+-				return nil, nil, errors.Wrapf(err, "failed to create stdout log")
+-			}
+-
+-			stderrLog, err = v1.OpenShimStderrLog(ctx, config.WorkDir)
+-			if err != nil {
+-				return nil, nil, errors.Wrapf(err, "failed to create stderr log")
+-			}
++		stdoutCopy := ioutil.Discard
++		stderrCopy := ioutil.Discard
++		stdoutLog, err := v1.OpenShimStdoutLog(ctx, config.WorkDir)
++		if err != nil {
++			return nil, nil, errors.Wrapf(err, "failed to create stdout log")
++		}
+
+-			go io.Copy(os.Stdout, stdoutLog)
+-			go io.Copy(os.Stderr, stderrLog)
++		stderrLog, err := v1.OpenShimStderrLog(ctx, config.WorkDir)
++		if err != nil {
++			return nil, nil, errors.Wrapf(err, "failed to create stderr log")
+ 		}
++		if debug {
++			stdoutCopy = os.Stdout
++			stderrCopy = os.Stderr
++		}
++
++		go io.Copy(stdoutCopy, stdoutLog)
++		go io.Copy(stderrCopy, stderrLog)
+
+ 		cmd, err := newCommand(binary, daemonAddress, debug, config, f, stdoutLog, stderrLog)
+ 		if err != nil {

packages/containerd/containerd.spec

@@ -36,6 +36,13 @@ Patch2002: 2002-cri-use-default-SELinux-labels-as-a-fallback.patch
 # TODO: submit this upstream, including a unit test.
 Patch3001: 3001-cri-set-default-RLIMIT_NOFILE.patch
 
+# Upstream patches; can drop when we move to 1.4.1
+Patch4001: 4001-Exit-signal-forward-if-process-not-found.patch
+Patch4002: 4002-Ignore-SIGURG-signals-in-signal-forwarder.patch
+
+# Upstream patch; can drop when we move to 1.4.1
+Patch5001: 5001-Always-consume-shim-logs.patch
+
 BuildRequires: git
 BuildRequires: %{_cross_os}glibc-devel
 BuildRequires: %{_cross_os}libseccomp-devel