Merge pull request #5174 from fuweid/fix-5130

estesp · web-flow · commit fad66f94ece3 · 2021-03-15T10:38:41.000-04:00
runtime: ignore file-already-closed error if dead shim
diff --git a/runtime/v2/shim.go b/runtime/v2/shim.go
@@ -84,7 +84,7 @@ func loadShim(ctx context.Context, bundle *Bundle, events *exchange.Exchange, rt
 	}()
 	f, err := openShimLog(shimCtx, bundle, client.AnonReconnectDialer)
 	if err != nil {
-		return nil, errors.Wrap(err, "open shim log pipe")
+		return nil, errors.Wrap(err, "open shim log pipe when reload")
 	}
 	defer func() {
 		if err != nil {
@@ -96,13 +96,10 @@ func loadShim(ctx context.Context, bundle *Bundle, events *exchange.Exchange, rt
 	// copy the shim's logs to containerd's output
 	go func() {
 		defer f.Close()
-		if _, err := io.Copy(os.Stderr, f); err != nil {
-			// When using a multi-container shim the 2nd to Nth container in the
-			// shim will not have a separate log pipe. Ignore the failure log
-			// message here when the shim connect times out.
-			if !errors.Is(err, os.ErrNotExist) {
-				log.G(ctx).WithError(err).Error("copy shim log")
-			}
+		_, err := io.Copy(os.Stderr, f)
+		err = checkCopyShimLogError(ctx, err)
+		if err != nil {
+			log.G(ctx).WithError(err).Error("copy shim log after reload")
 		}
 	}()
 	onCloseWithShimLog := func() {
diff --git a/runtime/v2/shim_unix.go b/runtime/v2/shim_unix.go
@@ -22,10 +22,12 @@ import (
 	"context"
 	"io"
 	"net"
+	"os"
 	"path/filepath"
 	"time"
 
 	"github.com/containerd/fifo"
+	"github.com/pkg/errors"
 	"golang.org/x/sys/unix"
 )
 
@@ -34,12 +36,9 @@ func openShimLog(ctx context.Context, bundle *Bundle, _ func(string, time.Durati
 }
 
 func checkCopyShimLogError(ctx context.Context, err error) error {
-	// When using a multi-container shim, the fifo of the 2nd to Nth
-	// container will not be opened when the ctx is done. This will
-	// cause an ErrReadClosed that can be ignored.
 	select {
 	case <-ctx.Done():
-		if err == fifo.ErrReadClosed {
+		if err == fifo.ErrReadClosed || errors.Is(err, os.ErrClosed) {
 			return nil
 		}
 	default:
diff --git a/runtime/v2/shim_unix_test.go b/runtime/v2/shim_unix_test.go
@@ -20,6 +20,7 @@ package v2
 
 import (
 	"context"
+	"os"
 	"testing"
 
 	"github.com/containerd/fifo"
@@ -43,6 +44,9 @@ func TestCheckCopyShimLogError(t *testing.T) {
 	if err := checkCopyShimLogError(ctx, nil); err != nil {
 		t.Fatalf("should return the actual error after context is done, but %v", err)
 	}
+	if err := checkCopyShimLogError(ctx, os.ErrClosed); err != nil {
+		t.Fatalf("should return the actual error after context is done, but %v", err)
+	}
 	if err := checkCopyShimLogError(ctx, fifo.ErrRdFrmWRONLY); err != fifo.ErrRdFrmWRONLY {
 		t.Fatalf("should return the actual error after context is done, but %v", err)
 	}
