Fix creation of DirectIO overwriting fifo config

Creating a direct IO should not overwrite the fifo configuration. The fifo configuration can be updated before creating the direct io if needed. This fixes an expected change in behavior for clients who were calling NewDirectIO previously with terminal configured on the fifo. Signed-off-by: Derek McGowan <derek@mcgstyle.net>
containerd · Jun 1, 2018 · 6b9be1b · 6b9be1b
1 parent 5b1f69b
commit 6b9be1b
Show file tree

Hide file tree

Showing 3 changed files with 10 additions and 32 deletions.
diff --git a/cio/io_unix.go b/cio/io_unix.go
@@ -141,18 +141,8 @@ func openFifos(ctx context.Context, fifos *FIFOSet) (pipes, error) {
 // NewDirectIO returns an IO implementation that exposes the IO streams as io.ReadCloser
 // and io.WriteCloser.
 func NewDirectIO(ctx context.Context, fifos *FIFOSet) (*DirectIO, error) {
-	return newDirectIO(ctx, fifos, false)
-}
-
-// NewDirectIOWithTerminal returns an IO implementation that exposes the streams with terminal enabled
-func NewDirectIOWithTerminal(ctx context.Context, fifos *FIFOSet) (*DirectIO, error) {
-	return newDirectIO(ctx, fifos, true)
-}
-
-func newDirectIO(ctx context.Context, fifos *FIFOSet, terminal bool) (*DirectIO, error) {
 	ctx, cancel := context.WithCancel(ctx)
 	pipes, err := openFifos(ctx, fifos)
-	fifos.Config.Terminal = terminal
 	return &DirectIO{
 		pipes: pipes,
 		cio: cio{

diff --git a/container_checkpoint_test.go b/container_checkpoint_test.go
@@ -60,7 +60,7 @@ func TestCheckpointRestorePTY(t *testing.T) {
 	}
 	defer container.Delete(ctx, WithSnapshotCleanup)
 
-	direct, err := newDirectIOWithTerminal(ctx)
+	direct, err := newDirectIO(ctx, true)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -92,7 +92,7 @@ func TestCheckpointRestorePTY(t *testing.T) {
 		t.Fatal(err)
 	}
 	direct.Delete()
-	direct, err = newDirectIOWithTerminal(ctx)
+	direct, err = newDirectIO(ctx, true)
 	if err != nil {
 		t.Fatal(err)
 	}

diff --git a/container_linux_test.go b/container_linux_test.go
@@ -281,7 +281,7 @@ func TestContainerPTY(t *testing.T) {
 	}
 	defer container.Delete(ctx, WithSnapshotCleanup)
 
-	direct, err := newDirectIOWithTerminal(ctx)
+	direct, err := newDirectIO(ctx, true)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -360,7 +360,7 @@ func TestContainerAttach(t *testing.T) {
 
 	expected := "hello" + newLine
 
-	direct, err := newDirectIOStandard(ctx)
+	direct, err := newDirectIO(ctx, false)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -429,24 +429,12 @@ func TestContainerAttach(t *testing.T) {
 	}
 }
 
-func newDirectIOStandard(ctx context.Context) (*directIO, error) {
-	return newDirectIO(ctx, false)
-}
-
-func newDirectIOWithTerminal(ctx context.Context) (*directIO, error) {
-	return newDirectIO(ctx, true)
-}
-
 func newDirectIO(ctx context.Context, terminal bool) (*directIO, error) {
-	fifos, err := cio.NewFIFOSetInDir("", "", false)
+	fifos, err := cio.NewFIFOSetInDir("", "", terminal)
 	if err != nil {
 		return nil, err
 	}
-	f := cio.NewDirectIO
-	if terminal {
-		f = cio.NewDirectIOWithTerminal
-	}
-	dio, err := f(ctx, fifos)
+	dio, err := cio.NewDirectIO(ctx, fifos)
 	if err != nil {
 		return nil, err
 	}
@@ -508,7 +496,7 @@ func TestContainerUsername(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	direct, err := newDirectIOStandard(ctx)
+	direct, err := newDirectIO(ctx, false)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -583,7 +571,7 @@ func testContainerUser(t *testing.T, userstr, expectedOutput string) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	direct, err := newDirectIOStandard(ctx)
+	direct, err := newDirectIO(ctx, false)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -668,7 +656,7 @@ func TestContainerAttachProcess(t *testing.T) {
 	expected := "hello" + newLine
 
 	// creating IO early for easy resource cleanup
-	direct, err := newDirectIOStandard(ctx)
+	direct, err := newDirectIO(ctx, false)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -775,7 +763,7 @@ func TestContainerUserID(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	direct, err := newDirectIOStandard(ctx)
+	direct, err := newDirectIO(ctx, false)
 	if err != nil {
 		t.Fatal(err)
 	}