cio: should not open fifo for stderr if terminal

fix: #4342 Signed-off-by: Wei Fu <fuweid89@gmail.com>
containerd · Jul 2, 2020 · 68b736d · 68b736d
1 parent 468d4e1
commit 68b736d
Show file tree

Hide file tree

Showing 3 changed files with 55 additions and 5 deletions.
diff --git a/cio/io_unix.go b/cio/io_unix.go
@@ -132,7 +132,7 @@ func openFifos(ctx context.Context, fifos *FIFOSet) (pipes, error) {
 			}
 		}()
 	}
-	if fifos.Stderr != "" {
+	if !fifos.Terminal && fifos.Stderr != "" {
 		if f.Stderr, err = fifo.OpenFifo(ctx, fifos.Stderr, syscall.O_RDONLY|syscall.O_CREAT|syscall.O_NONBLOCK, 0700); err != nil {
 			return f, errors.Wrapf(err, "failed to open stderr fifo")
 		}

diff --git a/cio/io_unix_test.go b/cio/io_unix_test.go
@@ -20,6 +20,9 @@ package cio
 
 import (
 	"context"
+	"fmt"
+	"io/ioutil"
+	"os"
 	"path/filepath"
 	"testing"
 
@@ -55,3 +58,46 @@ func TestOpenFifos(t *testing.T) {
 		assert.Assert(t, err != nil, scenario)
 	}
 }
+
+// TestOpenFifosWithTerminal tests openFifos should not open stderr if terminal
+// is set.
+func TestOpenFifosWithTerminal(t *testing.T) {
+	var ctx, cancel = context.WithCancel(context.Background())
+	defer cancel()
+
+	ioFifoDir, err := ioutil.TempDir("", fmt.Sprintf("cio-%s", t.Name()))
+	if err != nil {
+		t.Fatalf("unexpected error during creating temp dir: %v", err)
+	}
+	defer os.RemoveAll(ioFifoDir)
+
+	cfg := Config{
+		Stdout: filepath.Join(ioFifoDir, "test-stdout"),
+		Stderr: filepath.Join(ioFifoDir, "test-stderr"),
+	}
+
+	// Without terminal, pipes.Stderr should not be nil
+	{
+		p, err := openFifos(ctx, NewFIFOSet(cfg, nil))
+		if err != nil {
+			t.Fatalf("unexpected error during openFifos: %v", err)
+		}
+
+		if p.Stderr == nil {
+			t.Fatalf("unexpected empty stderr pipe")
+		}
+	}
+
+	// With terminal, pipes.Stderr should be nil
+	{
+		cfg.Terminal = true
+		p, err := openFifos(ctx, NewFIFOSet(cfg, nil))
+		if err != nil {
+			t.Fatalf("unexpected error during openFifos: %v", err)
+		}
+
+		if p.Stderr != nil {
+			t.Fatalf("unexpected stderr pipe")
+		}
+	}
+}
diff --git a/container_linux_test.go b/container_linux_test.go
@@ -723,11 +723,15 @@ func (f *directIO) Cancel() {
 // Close closes all open fds
 func (f *directIO) Close() error {
 	err := f.Stdin.Close()
-	if err2 := f.Stdout.Close(); err == nil {
-		err = err2
+	if f.Stdout != nil {
+		if err2 := f.Stdout.Close(); err == nil {
+			err = err2
+		}
 	}
-	if err2 := f.Stderr.Close(); err == nil {
-		err = err2
+	if f.Stderr != nil {
+		if err2 := f.Stderr.Close(); err == nil {
+			err = err2
+		}
 	}
 	return err
 }