Fix creation of DirectIO overwriting fifo config

dmcgowan · dmcgowan · commit 6b9be1bfc37d · 2018-06-01T16:04:22.000-07:00
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 &lt;derek@mcgstyle.net&gt;
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)
 	}

Original file line number	Diff line number	Diff line change
`@@ -60,7 +60,7 @@ func TestCheckpointRestorePTY(t *testing.T) {`
`60`	`60`	`}`
`61`	`61`	`defer container.Delete(ctx, WithSnapshotCleanup)`
`62`	`62`
`63`		`- direct, err := newDirectIOWithTerminal(ctx)`
	`63`	`+ direct, err := newDirectIO(ctx, true)`
`64`	`64`	`if err != nil {`
`65`	`65`	`t.Fatal(err)`
`66`	`66`	`}`
`@@ -92,7 +92,7 @@ func TestCheckpointRestorePTY(t *testing.T) {`
`92`	`92`	`t.Fatal(err)`
`93`	`93`	`}`
`94`	`94`	`direct.Delete()`
`95`		`- direct, err = newDirectIOWithTerminal(ctx)`
	`95`	`+ direct, err = newDirectIO(ctx, true)`
`96`	`96`	`if err != nil {`
`97`	`97`	`t.Fatal(err)`
`98`	`98`	`}`
Original file line number	Diff line number	Diff line change
`@@ -281,7 +281,7 @@ func TestContainerPTY(t *testing.T) {`
`281`	`281`	`}`
`282`	`282`	`defer container.Delete(ctx, WithSnapshotCleanup)`
`283`	`283`
`284`		`- direct, err := newDirectIOWithTerminal(ctx)`
	`284`	`+ direct, err := newDirectIO(ctx, true)`
`285`	`285`	`if err != nil {`
`286`	`286`	`t.Fatal(err)`
`287`	`287`	`}`
`@@ -360,7 +360,7 @@ func TestContainerAttach(t *testing.T) {`
`360`	`360`
`361`	`361`	`expected := "hello" + newLine`
`362`	`362`
`363`		`- direct, err := newDirectIOStandard(ctx)`
	`363`	`+ direct, err := newDirectIO(ctx, false)`
`364`	`364`	`if err != nil {`
`365`	`365`	`t.Fatal(err)`
`366`	`366`	`}`
`@@ -429,24 +429,12 @@ func TestContainerAttach(t *testing.T) {`
`429`	`429`	`}`
`430`	`430`	`}`
`431`	`431`
`432`		`-func newDirectIOStandard(ctx context.Context) (*directIO, error) {`
`433`		`- return newDirectIO(ctx, false)`
`434`		`-}`
`435`		`-`
`436`		`-func newDirectIOWithTerminal(ctx context.Context) (*directIO, error) {`
`437`		`- return newDirectIO(ctx, true)`
`438`		`-}`
`439`		`-`
`440`	`432`	`func newDirectIO(ctx context.Context, terminal bool) (*directIO, error) {`
`441`		`- fifos, err := cio.NewFIFOSetInDir("", "", false)`
	`433`	`+ fifos, err := cio.NewFIFOSetInDir("", "", terminal)`
`442`	`434`	`if err != nil {`
`443`	`435`	`return nil, err`
`444`	`436`	`}`
`445`		`- f := cio.NewDirectIO`
`446`		`- if terminal {`
`447`		`- f = cio.NewDirectIOWithTerminal`
`448`		`- }`
`449`		`- dio, err := f(ctx, fifos)`
	`437`	`+ dio, err := cio.NewDirectIO(ctx, fifos)`
`450`	`438`	`if err != nil {`
`451`	`439`	`return nil, err`
`452`	`440`	`}`
`@@ -508,7 +496,7 @@ func TestContainerUsername(t *testing.T) {`
`508`	`496`	`if err != nil {`
`509`	`497`	`t.Fatal(err)`
`510`	`498`	`}`
`511`		`- direct, err := newDirectIOStandard(ctx)`
	`499`	`+ direct, err := newDirectIO(ctx, false)`
`512`	`500`	`if err != nil {`
`513`	`501`	`t.Fatal(err)`
`514`	`502`	`}`
`@@ -583,7 +571,7 @@ func testContainerUser(t *testing.T, userstr, expectedOutput string) {`
`583`	`571`	`if err != nil {`
`584`	`572`	`t.Fatal(err)`
`585`	`573`	`}`
`586`		`- direct, err := newDirectIOStandard(ctx)`
	`574`	`+ direct, err := newDirectIO(ctx, false)`
`587`	`575`	`if err != nil {`
`588`	`576`	`t.Fatal(err)`
`589`	`577`	`}`
`@@ -668,7 +656,7 @@ func TestContainerAttachProcess(t *testing.T) {`
`668`	`656`	`expected := "hello" + newLine`
`669`	`657`
`670`	`658`	`// creating IO early for easy resource cleanup`
`671`		`- direct, err := newDirectIOStandard(ctx)`
	`659`	`+ direct, err := newDirectIO(ctx, false)`
`672`	`660`	`if err != nil {`
`673`	`661`	`t.Fatal(err)`
`674`	`662`	`}`
`@@ -775,7 +763,7 @@ func TestContainerUserID(t *testing.T) {`
`775`	`763`	`if err != nil {`
`776`	`764`	`t.Fatal(err)`
`777`	`765`	`}`
`778`		`- direct, err := newDirectIOStandard(ctx)`
	`766`	`+ direct, err := newDirectIO(ctx, false)`
`779`	`767`	`if err != nil {`
`780`	`768`	`t.Fatal(err)`
`781`	`769`	`}`