Merge pull request #2379 from dmcgowan/fix-direct-io-terminal-setting

estesp · web-flow · commit 84bebdd91d34 · 2018-06-02T12:25:14.000+03:00
Fix creation of DirectIO overwriting fifo config
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`	`}`