Merge pull request #1356 from crosbymichael/console-socket

Add separate console socket

Author: hqhq

contrib/cmd/recvtty/recvtty.go

@@ -24,7 +24,6 @@ import (
 	"os"
 	"strings"
 
-	"github.com/opencontainers/runc/libcontainer"
 	"github.com/opencontainers/runc/libcontainer/utils"
 	"github.com/urfave/cli"
 )
@@ -102,13 +101,6 @@ func handleSingle(path string) error {
 		return err
 	}
 
-	// Print the file descriptor tag.
-	ti, err := libcontainer.GetTerminalInfo(master.Name())
-	if err != nil {
-		return err
-	}
-	fmt.Printf("[recvtty] received masterfd: container '%s'\n", ti.ContainerID)
-
 	// Copy from our stdio to the master fd.
 	quitChan := make(chan struct{})
 	go func() {
@@ -163,13 +155,6 @@ func handleNull(path string) error {
 				return
 			}
 
-			// Print the file descriptor tag.
-			ti, err := libcontainer.GetTerminalInfo(master.Name())
-			if err != nil {
-				bail(err)
-			}
-			fmt.Printf("[recvtty] received masterfd: container '%s'\n", ti.ContainerID)
-
 			// Just do a dumb copy to /dev/null.
 			devnull, err := os.OpenFile("/dev/null", os.O_RDWR, 0)
 			if err != nil {

libcontainer/console.go

@@ -1,8 +1,6 @@
 package libcontainer
 
 import (
-	"encoding/json"
-	"fmt"
 	"io"
 	"os"
 )
@@ -17,57 +15,3 @@ type Console interface {
 	// Fd returns the fd for the master of the pty.
 	File() *os.File
 }
-
-const (
-	TerminalInfoVersion uint32 = 201610041
-	TerminalInfoType    uint8  = 'T'
-)
-
-// TerminalInfo is the structure which is passed as the non-ancillary data
-// in the sendmsg(2) call when runc is run with --console-socket. It
-// contains some information about the container which the console master fd
-// relates to (to allow for consumers to use a single unix socket to handle
-// multiple containers). This structure will probably move to runtime-spec
-// at some point. But for now it lies in libcontainer.
-type TerminalInfo struct {
-	// Version of the API.
-	Version uint32 `json:"version"`
-
-	// Type of message (future proofing).
-	Type uint8 `json:"type"`
-
-	// Container contains the ID of the container.
-	ContainerID string `json:"container_id"`
-}
-
-func (ti *TerminalInfo) String() string {
-	encoded, err := json.Marshal(*ti)
-	if err != nil {
-		panic(err)
-	}
-	return string(encoded)
-}
-
-func NewTerminalInfo(containerId string) *TerminalInfo {
-	return &TerminalInfo{
-		Version:     TerminalInfoVersion,
-		Type:        TerminalInfoType,
-		ContainerID: containerId,
-	}
-}
-
-func GetTerminalInfo(encoded string) (*TerminalInfo, error) {
-	ti := new(TerminalInfo)
-	if err := json.Unmarshal([]byte(encoded), ti); err != nil {
-		return nil, err
-	}
-
-	if ti.Type != TerminalInfoType {
-		return nil, fmt.Errorf("terminal info: incorrect type in payload (%q): %q", TerminalInfoType, ti.Type)
-	}
-	if ti.Version != TerminalInfoVersion {
-		return nil, fmt.Errorf("terminal info: incorrect version in payload (%q): %q", TerminalInfoVersion, ti.Version)
-	}
-
-	return ti, nil
-}

libcontainer/console_linux.go

@@ -7,6 +7,12 @@ import (
 	"unsafe"
 )
 
+func ConsoleFromFile(f *os.File) Console {
+	return &linuxConsole{
+		master: f,
+	}
+}
+
 // newConsole returns an initialized console that can be used within a container by copying bytes
 // from the master side to the slave that is attached as the tty for the container's init process.
 func newConsole() (Console, error) {

libcontainer/container_linux.go

@@ -335,7 +335,7 @@ func (c *linuxContainer) deleteExecFifo() {
 }
 
 func (c *linuxContainer) newParentProcess(p *Process, doInit bool) (parentProcess, error) {
-	parentPipe, childPipe, err := newPipe()
+	parentPipe, childPipe, err := utils.NewSockPair("init")
 	if err != nil {
 		return nil, newSystemErrorWithCause(err, "creating new init pipe")
 	}
@@ -370,9 +370,17 @@ func (c *linuxContainer) commandTemplate(p *Process, childPipe *os.File) (*exec.
 	if cmd.SysProcAttr == nil {
 		cmd.SysProcAttr = &syscall.SysProcAttr{}
 	}
-	cmd.ExtraFiles = append(p.ExtraFiles, childPipe)
+	cmd.ExtraFiles = append(cmd.ExtraFiles, p.ExtraFiles...)
+	if p.ConsoleSocket != nil {
+		cmd.ExtraFiles = append(cmd.ExtraFiles, p.ConsoleSocket)
+		cmd.Env = append(cmd.Env,
+			fmt.Sprintf("_LIBCONTAINER_CONSOLE=%d", stdioFdCount+len(cmd.ExtraFiles)-1),
+		)
+	}
+	cmd.ExtraFiles = append(cmd.ExtraFiles, childPipe)
 	cmd.Env = append(cmd.Env,
-		fmt.Sprintf("_LIBCONTAINER_INITPIPE=%d", stdioFdCount+len(cmd.ExtraFiles)-1))
+		fmt.Sprintf("_LIBCONTAINER_INITPIPE=%d", stdioFdCount+len(cmd.ExtraFiles)-1),
+	)
 	// NOTE: when running a container with no PID namespace and the parent process spawning the container is
 	// PID1 the pdeathsig is being delivered to the container's init process by the kernel for some reason
 	// even with the parent still running.
@@ -395,7 +403,6 @@ func (c *linuxContainer) newInitProcess(p *Process, cmd *exec.Cmd, parentPipe, c
 	if err != nil {
 		return nil, err
 	}
-	p.consoleChan = make(chan *os.File, 1)
 	return &initProcess{
 		cmd:           cmd,
 		childPipe:     childPipe,
@@ -422,8 +429,6 @@ func (c *linuxContainer) newSetnsProcess(p *Process, cmd *exec.Cmd, parentPipe,
 	if err != nil {
 		return nil, err
 	}
-	// TODO: set on container for process management
-	p.consoleChan = make(chan *os.File, 1)
 	return &setnsProcess{
 		cmd:           cmd,
 		cgroupPaths:   c.cgroupManager.GetPaths(),
@@ -463,28 +468,10 @@ func (c *linuxContainer) newInitConfig(process *Process) *initConfig {
 	if len(process.Rlimits) > 0 {
 		cfg.Rlimits = process.Rlimits
 	}
-	/*
-	 * TODO: This should not be automatically computed. We should implement
-	 *       this as a field in libcontainer.Process, and then we only dup the
-	 *       new console over the file descriptors which were not explicitly
-	 *       set with process.Std{in,out,err}. The reason I've left this as-is
-	 *       is because the GetConsole() interface is new, there's no need to
-	 *       polish this interface right now.
-	 */
-	if process.Stdin == nil && process.Stdout == nil && process.Stderr == nil {
-		cfg.CreateConsole = true
-	}
+	cfg.CreateConsole = process.ConsoleSocket != nil
 	return cfg
 }
 
-func newPipe() (parent *os.File, child *os.File, err error) {
-	fds, err := syscall.Socketpair(syscall.AF_LOCAL, syscall.SOCK_STREAM|syscall.SOCK_CLOEXEC, 0)
-	if err != nil {
-		return nil, nil, err
-	}
-	return os.NewFile(uintptr(fds[1]), "parent"), os.NewFile(uintptr(fds[0]), "child"), nil
-}
-
 func (c *linuxContainer) Destroy() error {
 	c.m.Lock()
 	defer c.m.Unlock()

libcontainer/factory_linux.go

@@ -222,8 +222,10 @@ func (l *LinuxFactory) Type() string {
 func (l *LinuxFactory) StartInitialization() (err error) {
 	var (
 		pipefd, rootfd int
+		consoleSocket  *os.File
 		envInitPipe    = os.Getenv("_LIBCONTAINER_INITPIPE")
 		envStateDir    = os.Getenv("_LIBCONTAINER_STATEDIR")
+		envConsole     = os.Getenv("_LIBCONTAINER_CONSOLE")
 	)
 
 	// Get the INITPIPE.
@@ -241,12 +243,20 @@ func (l *LinuxFactory) StartInitialization() (err error) {
 	// Only init processes have STATEDIR.
 	rootfd = -1
 	if it == initStandard {
-		rootfd, err = strconv.Atoi(envStateDir)
-		if err != nil {
+		if rootfd, err = strconv.Atoi(envStateDir); err != nil {
 			return fmt.Errorf("unable to convert _LIBCONTAINER_STATEDIR=%s to int: %s", envStateDir, err)
 		}
 	}
 
+	if envConsole != "" {
+		console, err := strconv.Atoi(envConsole)
+		if err != nil {
+			return fmt.Errorf("unable to convert _LIBCONTAINER_CONSOLE=%s to int: %s", envConsole, err)
+		}
+		consoleSocket = os.NewFile(uintptr(console), "console-socket")
+		defer consoleSocket.Close()
+	}
+
 	// clear the current process's environment to clean any libcontainer
 	// specific env vars.
 	os.Clearenv()
@@ -269,7 +279,7 @@ func (l *LinuxFactory) StartInitialization() (err error) {
 		}
 	}()
 
-	i, err := newContainerInit(it, pipe, rootfd)
+	i, err := newContainerInit(it, pipe, consoleSocket, rootfd)
 	if err != nil {
 		return err
 	}

libcontainer/init_linux.go

@@ -66,7 +66,7 @@ type initer interface {
 	Init() error
 }
 
-func newContainerInit(t initType, pipe *os.File, stateDirFD int) (initer, error) {
+func newContainerInit(t initType, pipe *os.File, consoleSocket *os.File, stateDirFD int) (initer, error) {
 	var config *initConfig
 	if err := json.NewDecoder(pipe).Decode(&config); err != nil {
 		return nil, err
@@ -77,15 +77,17 @@ func newContainerInit(t initType, pipe *os.File, stateDirFD int) (initer, error)
 	switch t {
 	case initSetns:
 		return &linuxSetnsInit{
-			pipe:   pipe,
-			config: config,
+			pipe:          pipe,
+			consoleSocket: consoleSocket,
+			config:        config,
 		}, nil
 	case initStandard:
 		return &linuxStandardInit{
-			pipe:       pipe,
-			parentPid:  syscall.Getppid(),
-			config:     config,
-			stateDirFD: stateDirFD,
+			pipe:          pipe,
+			consoleSocket: consoleSocket,
+			parentPid:     syscall.Getppid(),
+			config:        config,
+			stateDirFD:    stateDirFD,
 		}, nil
 	}
 	return nil, fmt.Errorf("unknown init type %q", t)
@@ -155,7 +157,8 @@ func finalizeNamespace(config *initConfig) error {
 // consoles are scoped to a container properly (see runc#814 and the many
 // issues related to that). This has to be run *after* we've pivoted to the new
 // rootfs (and the users' configuration is entirely set up).
-func setupConsole(pipe *os.File, config *initConfig, mount bool) error {
+func setupConsole(socket *os.File, config *initConfig, mount bool) error {
+	defer socket.Close()
 	// At this point, /dev/ptmx points to something that we would expect. We
 	// used to change the owner of the slave path, but since the /dev/pts mount
 	// can have gid=X set (at the users' option). So touching the owner of the
@@ -174,37 +177,16 @@ func setupConsole(pipe *os.File, config *initConfig, mount bool) error {
 	if !ok {
 		return fmt.Errorf("failed to cast console to *linuxConsole")
 	}
-
 	// Mount the console inside our rootfs.
 	if mount {
 		if err := linuxConsole.mount(); err != nil {
 			return err
 		}
 	}
-
-	if err := writeSync(pipe, procConsole); err != nil {
-		return err
-	}
-
-	// We need to have a two-way synchronisation here. Though it might seem
-	// pointless, it's important to make sure that the sendmsg(2) payload
-	// doesn't get swallowed by an out-of-place read(2) [which happens if the
-	// syscalls get reordered so that sendmsg(2) is before the other side's
-	// read(2) of procConsole].
-	if err := readSync(pipe, procConsoleReq); err != nil {
-		return err
-	}
-
 	// While we can access console.master, using the API is a good idea.
-	if err := utils.SendFd(pipe, linuxConsole.File()); err != nil {
-		return err
-	}
-
-	// Make sure the other side received the fd.
-	if err := readSync(pipe, procConsoleAck); err != nil {
+	if err := utils.SendFd(socket, linuxConsole.File()); err != nil {
 		return err
 	}
-
 	// Now, dup over all the things.
 	return linuxConsole.dupStdio()
 }

libcontainer/integration/execin_test.go

@@ -13,6 +13,7 @@ import (
 
 	"github.com/opencontainers/runc/libcontainer"
 	"github.com/opencontainers/runc/libcontainer/configs"
+	"github.com/opencontainers/runc/libcontainer/utils"
 )
 
 func TestExecIn(t *testing.T) {
@@ -279,9 +280,36 @@ func TestExecInTTY(t *testing.T) {
 		Args: []string{"ps"},
 		Env:  standardEnvironment,
 	}
+	parent, child, err := utils.NewSockPair("console")
+	if err != nil {
+		ok(t, err)
+	}
+	defer parent.Close()
+	defer child.Close()
+	ps.ConsoleSocket = child
+	type cdata struct {
+		c   libcontainer.Console
+		err error
+	}
+	dc := make(chan *cdata, 1)
+	go func() {
+		f, err := utils.RecvFd(parent)
+		if err != nil {
+			dc <- &cdata{
+				err: err,
+			}
+		}
+		dc <- &cdata{
+			c: libcontainer.ConsoleFromFile(f),
+		}
+	}()
 	err = container.Run(ps)
 	ok(t, err)
-	console, err := ps.GetConsole()
+	data := <-dc
+	if data.err != nil {
+		ok(t, data.err)
+	}
+	console := data.c
 	copy := make(chan struct{})
 	go func() {
 		io.Copy(&stdout, console)

libcontainer/process.go

@@ -47,10 +47,6 @@ type Process struct {
 	// ExtraFiles specifies additional open files to be inherited by the container
 	ExtraFiles []*os.File
 
-	// consoleChan provides the masterfd console.
-	// TODO: Make this persistent in Process.
-	consoleChan chan *os.File
-
 	// Capabilities specify the capabilities to keep when executing the process inside the container
 	// All capabilities not specified will be dropped from the processes capability mask
 	Capabilities *configs.Capabilities
@@ -69,6 +65,9 @@ type Process struct {
 	// If Rlimits are not set, the container will inherit rlimits from the parent process
 	Rlimits []configs.Rlimit
 
+	// ConsoleSocket provides the masterfd console.
+	ConsoleSocket *os.File
+
 	ops processOperations
 }
 
@@ -105,15 +104,3 @@ type IO struct {
 	Stdout io.ReadCloser
 	Stderr io.ReadCloser
 }
-
-func (p *Process) GetConsole() (Console, error) {
-	consoleFd, ok := <-p.consoleChan
-	if !ok {
-		return nil, fmt.Errorf("failed to get console from process")
-	}
-
-	// TODO: Fix this so that it used the console API.
-	return &linuxConsole{
-		master: consoleFd,
-	}, nil
-}