diff options
author | Nicolas Lacasse <nlacasse@google.com> | 2018-10-15 11:06:20 -0700 |
---|---|---|
committer | Shentubot <shentubot@google.com> | 2018-10-15 11:08:49 -0700 |
commit | 3f0532595679c388362203bbce1d4b6c4d2e336b (patch) | |
tree | 2692dd38d924f4449cd3ca8b86c9347110f49a77 | |
parent | 4ea69fce8def9e030cbbc4d803b95e632175750c (diff) |
Never send boot process stdio to application stdio.
We treat handle the boot process stdio separately from the application stdio
(which gets passed via flags), but we were still sending both to same place. As
a result, some logs that are written directly to os.Stderr by the boot process
were ending up in the application logs.
This CL starts sendind boot process stdio to the null device (since we don't
have any better options). The boot process is already configured to send all
logs (and panics) to the log file, so we won't miss anything important.
PiperOrigin-RevId: 217173020
Change-Id: I5ab980da037f34620e7861a3736ba09c18d73794
-rw-r--r-- | runsc/sandbox/sandbox.go | 46 |
1 files changed, 28 insertions, 18 deletions
diff --git a/runsc/sandbox/sandbox.go b/runsc/sandbox/sandbox.go index 6c1b39be7..be68e864f 100644 --- a/runsc/sandbox/sandbox.go +++ b/runsc/sandbox/sandbox.go @@ -351,6 +351,15 @@ func (s *Sandbox) createSandboxProcess(spec *specs.Spec, conf *boot.Config, bund nextFD++ } + // The current process' stdio must be passed to the application via the + // --stdio-fds flag. The stdio of the sandbox process itself must not + // be connected to the same FDs, otherwise we risk leaking sandbox + // errors to the application, so we set the sandbox stdio to nil, + // causing them to read/write from the null device. + cmd.Stdin = nil + cmd.Stdout = nil + cmd.Stderr = nil + // If the console control socket file is provided, then create a new // pty master/slave pair and set the TTY on the sandbox process. if consoleSocket != "" { @@ -364,21 +373,13 @@ func (s *Sandbox) createSandboxProcess(spec *specs.Spec, conf *boot.Config, bund } defer tty.Close() - // Ideally we would set the sandbox stdin to this process' - // stdin, but for some reason Docker does not like that (it - // never calls `runsc start`). Instead we set stdio to the - // console TTY, but note that this is distinct from the - // container stdio, which is passed via the flags below. - cmd.Stdin = tty - cmd.Stdout = tty - cmd.Stderr = tty - // Set the TTY as a controlling TTY on the sandbox process. // Note that the Ctty field must be the FD of the TTY in the - // *new* process, not this process. Since we set the TTY to + // *new* process, not this process. Since we are about to + // assign the TTY to nextFD, we can use that value here. // stdin, we can use FD 0 here. cmd.SysProcAttr.Setctty = true - cmd.SysProcAttr.Ctty = 0 + cmd.SysProcAttr.Ctty = nextFD // Pass the tty as all stdio fds to sandbox. for i := 0; i < 3; i++ { @@ -386,14 +387,15 @@ func (s *Sandbox) createSandboxProcess(spec *specs.Spec, conf *boot.Config, bund cmd.Args = append(cmd.Args, "--stdio-fds="+strconv.Itoa(nextFD)) nextFD++ } - } else { - // Connect the sandbox process to this process's stdios. Note - // that this is distinct from the container's stdio, which is - // passed by the flags below. - cmd.Stdin = os.Stdin - cmd.Stdout = os.Stdout - cmd.Stderr = os.Stderr + if conf.Debug { + // If debugging, send the boot process stdio to the + // TTY, so that it is easier to find. + cmd.Stdin = tty + cmd.Stdout = tty + cmd.Stderr = tty + } + } else { // If not using a console, pass our current stdio as the // container stdio via flags. for _, f := range []*os.File{os.Stdin, os.Stdout, os.Stderr} { @@ -401,6 +403,14 @@ func (s *Sandbox) createSandboxProcess(spec *specs.Spec, conf *boot.Config, bund cmd.Args = append(cmd.Args, "--stdio-fds="+strconv.Itoa(nextFD)) nextFD++ } + + if conf.Debug { + // If debugging, send the boot process stdio to the + // this process' stdio, so that is is easier to find. + cmd.Stdin = os.Stdin + cmd.Stdout = os.Stdout + cmd.Stderr = os.Stderr + } } // Detach from this session, otherwise cmd will get SIGHUP and SIGCONT |