Don't provide IO when it's not set

This makes sure that runc does not get any valid IO for the pipe. Some builds and other containers will be stuck if they inspect stdin expecially and its a pipe but not connected to any user input. Signed-off-by: Michael Crosby <crosbymichael@gmail.com>
containerd · Sep 7, 2018 · 906acb1 · 906acb1
1 parent b5274fe
commit 906acb1
Show file tree

Hide file tree

Showing 10 changed files with 174 additions and 58 deletions.
diff --git a/cio/io.go b/cio/io.go
@@ -141,6 +141,15 @@ func NewCreator(opts ...Opt) Creator {
 		if err != nil {
 			return nil, err
 		}
+		if streams.Stdin == nil {
+			fifos.Stdin = ""
+		}
+		if streams.Stdout == nil {
+			fifos.Stdout = ""
+		}
+		if streams.Stderr == nil {
+			fifos.Stderr = ""
+		}
 		return copyIO(fifos, streams)
 	}
 }

diff --git a/container_linux_test.go b/container_linux_test.go
@@ -23,6 +23,7 @@ import (
 	"context"
 	"fmt"
 	"io"
+	"io/ioutil"
 	"os/exec"
 	"runtime"
 	"strings"
@@ -1481,3 +1482,52 @@ func TestBindLowPortNonOpt(t *testing.T) {
 		t.Fatal(err)
 	}
 }
+
+func TestContainerNoSTDIN(t *testing.T) {
+	t.Parallel()
+
+	client, err := newClient(t, address)
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer client.Close()
+
+	var (
+		image       Image
+		ctx, cancel = testContext()
+		id          = t.Name()
+	)
+	defer cancel()
+
+	image, err = client.GetImage(ctx, testImage)
+	if err != nil {
+		t.Fatal(err)
+	}
+	container, err := client.NewContainer(ctx, id, WithNewSpec(oci.WithImageConfig(image), withExitStatus(0)), WithNewSnapshot(id, image))
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer container.Delete(ctx, WithSnapshotCleanup)
+
+	task, err := container.NewTask(ctx, cio.NewCreator(cio.WithStreams(nil, ioutil.Discard, ioutil.Discard)))
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer task.Delete(ctx)
+
+	statusC, err := task.Wait(ctx)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if err := task.Start(ctx); err != nil {
+		t.Fatal(err)
+	}
+	status := <-statusC
+	code, _, err := status.Result()
+	if err != nil {
+		t.Fatal(err)
+	}
+	if code != 0 {
+		t.Errorf("expected status 0 from wait but received %d", code)
+	}
+}
diff --git a/runtime/v1/linux/proc/exec.go b/runtime/v1/linux/proc/exec.go
@@ -147,7 +147,7 @@ func (e *execProcess) start(ctx context.Context) (err error) {
 			return errors.Wrap(err, "creating new NULL IO")
 		}
 	} else {
-		if e.io, err = runc.NewPipeIO(e.parent.IoUID, e.parent.IoGID); err != nil {
+		if e.io, err = runc.NewPipeIO(e.parent.IoUID, e.parent.IoGID, withConditionalIO(e.stdio)); err != nil {
 			return errors.Wrap(err, "failed to create runc io pipes")
 		}
 	}

diff --git a/runtime/v1/linux/proc/init.go b/runtime/v1/linux/proc/init.go
@@ -123,7 +123,7 @@ func (p *Init) Create(ctx context.Context, r *CreateConfig) error {
 			return errors.Wrap(err, "creating new NULL IO")
 		}
 	} else {
-		if p.io, err = runc.NewPipeIO(p.IoUID, p.IoGID); err != nil {
+		if p.io, err = runc.NewPipeIO(p.IoUID, p.IoGID, withConditionalIO(p.stdio)); err != nil {
 			return errors.Wrap(err, "failed to create OCI runtime io pipes")
 		}
 	}
@@ -399,3 +399,11 @@ func (p *Init) runtimeError(rErr error, msg string) error {
 		return errors.Errorf("%s: %s", msg, rMsg)
 	}
 }
+
+func withConditionalIO(c proc.Stdio) runc.IOOpt {
+	return func(o *runc.IOOption) {
+		o.OpenStdin = c.Stdin != ""
+		o.OpenStdout = c.Stdout != ""
+		o.OpenStderr = c.Stderr != ""
+	}
+}
diff --git a/runtime/v1/linux/proc/io.go b/runtime/v1/linux/proc/io.go
@@ -109,7 +109,6 @@ func copyPipes(ctx context.Context, rio runc.IO, stdin, stdout, stderr string, w
 		i.dest(fw, fr)
 	}
 	if stdin == "" {
-		rio.Stdin().Close()
 		return nil
 	}
 	f, err := fifo.OpenFifo(ctx, stdin, syscall.O_RDONLY|syscall.O_NONBLOCK, 0)

diff --git a/vendor.conf b/vendor.conf
@@ -1,4 +1,4 @@
-github.com/containerd/go-runc acb7c88cac264acca9b5eae187a117f4d77a1292
+github.com/containerd/go-runc 5a6d9f37cfa36b15efba46dc7ea349fa9b7143c3
 github.com/containerd/console c12b1e7919c14469339a5d38f2f8ed9b64a9de23
 github.com/containerd/cgroups 5e610833b72089b37d0e615de9a92dfc043757c2
 github.com/containerd/typeurl a93fcdb778cd272c6e9b3028b2f42d813e785d40

diff --git a/vendor/github.com/containerd/go-runc/io.go b/vendor/github.com/containerd/go-runc/io.go
diff --git a/vendor/github.com/containerd/go-runc/io_unix.go b/vendor/github.com/containerd/go-runc/io_unix.go
diff --git a/vendor/github.com/containerd/go-runc/io_windows.go b/vendor/github.com/containerd/go-runc/io_windows.go
diff --git a/vendor/github.com/containerd/go-runc/runc.go b/vendor/github.com/containerd/go-runc/runc.go