From 6daf56799fc9a6c024bcdfe15957d91b02437bd0 Mon Sep 17 00:00:00 2001
From: Michael Crosby <michael@crosbymichael.com>
Date: Mon, 24 Feb 2014 21:11:52 -0800
Subject: [PATCH] Refactor and improve libcontainer and driver

Remove logging for now because it is complicating things
Docker-DCO-1.1-Signed-off-by: Michael Crosby <michael@crosbymichael.com> (github: crosbymichael)
---
 libcontainer/network/veth.go       |  3 ++
 libcontainer/nsinit/command.go     |  8 ++--
 libcontainer/nsinit/exec.go        | 35 ++--------------
 libcontainer/nsinit/execin.go      |  6 +--
 libcontainer/nsinit/init.go        | 64 ++++++++----------------------
 libcontainer/nsinit/nsinit.go      |  9 ++---
 libcontainer/nsinit/nsinit/main.go | 26 +-----------
 libcontainer/nsinit/state.go       |  2 +
 libcontainer/utils/utils.go        | 11 +++++
 system/calls_linux.go              |  9 +++++
 system/pty_linux.go                | 27 +++++++++++++
 11 files changed, 84 insertions(+), 116 deletions(-)

diff --git a/libcontainer/network/veth.go b/libcontainer/network/veth.go
index 321c68e..49e63f0 100644
--- a/libcontainer/network/veth.go
+++ b/libcontainer/network/veth.go
@@ -6,6 +6,9 @@ import (
 	"github.com/dotcloud/docker/pkg/libcontainer/utils"
 )
 
+// Veth is a network strategy that uses a bridge and creates
+// a veth pair, one that stays outside on the host and the other
+// is placed inside the container's namespace
 type Veth struct {
 }
 
diff --git a/libcontainer/nsinit/command.go b/libcontainer/nsinit/command.go
index b1c5631..5eb378a 100644
--- a/libcontainer/nsinit/command.go
+++ b/libcontainer/nsinit/command.go
@@ -8,8 +8,11 @@ import (
 	"syscall"
 )
 
+// CommandFactory takes the container's configuration and options passed by the
+// parent processes and creates an *exec.Cmd that will be used to fork/exec the
+// namespaced init process
 type CommandFactory interface {
-	Create(container *libcontainer.Container, console, logFile string, syncFd uintptr, args []string) *exec.Cmd
+	Create(container *libcontainer.Container, console string, syncFd uintptr, args []string) *exec.Cmd
 }
 
 type DefaultCommandFactory struct{}
@@ -17,13 +20,12 @@ type DefaultCommandFactory struct{}
 // Create will return an exec.Cmd with the Cloneflags set to the proper namespaces
 // defined on the container's configuration and use the current binary as the init with the
 // args provided
-func (c *DefaultCommandFactory) Create(container *libcontainer.Container, console, logFile string, pipe uintptr, args []string) *exec.Cmd {
+func (c *DefaultCommandFactory) Create(container *libcontainer.Container, console string, pipe uintptr, args []string) *exec.Cmd {
 	// get our binary name so we can always reexec ourself
 	name := os.Args[0]
 	command := exec.Command(name, append([]string{
 		"-console", console,
 		"-pipe", fmt.Sprint(pipe),
-		"-log", logFile,
 		"init"}, args...)...)
 
 	command.SysProcAttr = &syscall.SysProcAttr{
diff --git a/libcontainer/nsinit/exec.go b/libcontainer/nsinit/exec.go
index c407323..b13326b 100644
--- a/libcontainer/nsinit/exec.go
+++ b/libcontainer/nsinit/exec.go
@@ -28,31 +28,27 @@ func (ns *linuxNs) Exec(container *libcontainer.Container, term Terminal, args [
 	}
 
 	if container.Tty {
-		ns.logger.Printf("setting up master and console")
-		master, console, err = CreateMasterAndConsole()
+		master, console, err = system.CreateMasterAndConsole()
 		if err != nil {
 			return -1, err
 		}
 		term.SetMaster(master)
 	}
 
-	command := ns.commandFactory.Create(container, console, ns.logFile, syncPipe.child.Fd(), args)
+	command := ns.commandFactory.Create(container, console, syncPipe.child.Fd(), args)
 	if err := term.Attach(command); err != nil {
 		return -1, err
 	}
 	defer term.Close()
 
-	ns.logger.Printf("staring init")
 	if err := command.Start(); err != nil {
 		return -1, err
 	}
-	ns.logger.Printf("writing state file")
 	if err := ns.stateWriter.WritePid(command.Process.Pid); err != nil {
 		command.Process.Kill()
 		return -1, err
 	}
 	defer func() {
-		ns.logger.Printf("removing state file")
 		ns.stateWriter.DeletePid()
 	}()
 
@@ -68,24 +64,18 @@ func (ns *linuxNs) Exec(container *libcontainer.Container, term Terminal, args [
 	}
 
 	// Sync with child
-	ns.logger.Printf("closing sync pipes")
 	syncPipe.Close()
 
-	ns.logger.Printf("waiting on process")
 	if err := command.Wait(); err != nil {
 		if _, ok := err.(*exec.ExitError); !ok {
 			return -1, err
 		}
 	}
-
-	exitCode := command.ProcessState.Sys().(syscall.WaitStatus).ExitStatus()
-	ns.logger.Printf("process ended with exit code %d", exitCode)
-	return exitCode, nil
+	return command.ProcessState.Sys().(syscall.WaitStatus).ExitStatus(), nil
 }
 
 func (ns *linuxNs) SetupCgroups(container *libcontainer.Container, nspid int) error {
 	if container.Cgroups != nil {
-		ns.logger.Printf("setting up cgroups")
 		if err := container.Cgroups.Apply(nspid); err != nil {
 			return err
 		}
@@ -95,7 +85,6 @@ func (ns *linuxNs) SetupCgroups(container *libcontainer.Container, nspid int) er
 
 func (ns *linuxNs) InitializeNetworking(container *libcontainer.Container, nspid int, pipe *SyncPipe) error {
 	if container.Network != nil {
-		ns.logger.Printf("creating host network configuration type %s", container.Network.Type)
 		strategy, err := network.GetStrategy(container.Network.Type)
 		if err != nil {
 			return err
@@ -104,27 +93,9 @@ func (ns *linuxNs) InitializeNetworking(container *libcontainer.Container, nspid
 		if err != nil {
 			return err
 		}
-		ns.logger.Printf("sending %v as network context", networkContext)
 		if err := pipe.SendToChild(networkContext); err != nil {
 			return err
 		}
 	}
 	return nil
 }
-
-// CreateMasterAndConsole will open /dev/ptmx on the host and retreive the
-// pts name for use as the pty slave inside the container
-func CreateMasterAndConsole() (*os.File, string, error) {
-	master, err := os.OpenFile("/dev/ptmx", syscall.O_RDWR|syscall.O_NOCTTY|syscall.O_CLOEXEC, 0)
-	if err != nil {
-		return nil, "", err
-	}
-	console, err := system.Ptsname(master)
-	if err != nil {
-		return nil, "", err
-	}
-	if err := system.Unlockpt(master); err != nil {
-		return nil, "", err
-	}
-	return master, console, nil
-}
diff --git a/libcontainer/nsinit/execin.go b/libcontainer/nsinit/execin.go
index 9c33f69..463196c 100644
--- a/libcontainer/nsinit/execin.go
+++ b/libcontainer/nsinit/execin.go
@@ -18,7 +18,7 @@ func (ns *linuxNs) ExecIn(container *libcontainer.Container, nspid int, args []s
 			return -1, err
 		}
 	}
-	fds, err := getNsFds(nspid, container)
+	fds, err := ns.getNsFds(nspid, container)
 	closeFds := func() {
 		for _, f := range fds {
 			system.Closefd(f)
@@ -75,13 +75,13 @@ dropAndExec:
 	if err := capabilities.DropCapabilities(container); err != nil {
 		return -1, fmt.Errorf("drop capabilities %s", err)
 	}
-	if err := system.Exec(args[0], args[0:], container.Env); err != nil {
+	if err := system.Execv(args[0], args[0:], container.Env); err != nil {
 		return -1, err
 	}
 	panic("unreachable")
 }
 
-func getNsFds(pid int, container *libcontainer.Container) ([]uintptr, error) {
+func (ns *linuxNs) getNsFds(pid int, container *libcontainer.Container) ([]uintptr, error) {
 	fds := make([]uintptr, len(container.Namespaces))
 	for i, ns := range container.Namespaces {
 		f, err := os.OpenFile(filepath.Join("/proc/", strconv.Itoa(pid), "ns", namespaceFileMap[ns]), os.O_RDONLY, 0)
diff --git a/libcontainer/nsinit/init.go b/libcontainer/nsinit/init.go
index 5e33169..1229560 100644
--- a/libcontainer/nsinit/init.go
+++ b/libcontainer/nsinit/init.go
@@ -7,18 +7,17 @@ import (
 	"github.com/dotcloud/docker/pkg/libcontainer"
 	"github.com/dotcloud/docker/pkg/libcontainer/capabilities"
 	"github.com/dotcloud/docker/pkg/libcontainer/network"
+	"github.com/dotcloud/docker/pkg/libcontainer/utils"
 	"github.com/dotcloud/docker/pkg/system"
 	"github.com/dotcloud/docker/pkg/user"
 	"os"
-	"os/exec"
-	"path/filepath"
 	"syscall"
 )
 
 // Init is the init process that first runs inside a new namespace to setup mounts, users, networking,
 // and other options required for the new container.
 func (ns *linuxNs) Init(container *libcontainer.Container, uncleanRootfs, console string, syncPipe *SyncPipe, args []string) error {
-	rootfs, err := resolveRootfs(uncleanRootfs)
+	rootfs, err := utils.ResolveRootfs(uncleanRootfs)
 	if err != nil {
 		return err
 	}
@@ -34,7 +33,7 @@ func (ns *linuxNs) Init(container *libcontainer.Container, uncleanRootfs, consol
 	if console != "" {
 		// close pipes so that we can replace it with the pty
 		closeStdPipes()
-		slave, err := openTerminal(console, syscall.O_RDWR)
+		slave, err := system.OpenTerminal(console, syscall.O_RDWR)
 		if err != nil {
 			return fmt.Errorf("open terminal %s", err)
 		}
@@ -50,6 +49,7 @@ func (ns *linuxNs) Init(container *libcontainer.Container, uncleanRootfs, consol
 			return fmt.Errorf("setctty %s", err)
 		}
 	}
+
 	if err := system.ParentDeathSignal(); err != nil {
 		return fmt.Errorf("parent deth signal %s", err)
 	}
@@ -73,18 +73,7 @@ func (ns *linuxNs) Init(container *libcontainer.Container, uncleanRootfs, consol
 			return fmt.Errorf("chdir to %s %s", container.WorkingDir, err)
 		}
 	}
-	return execArgs(args, container.Env)
-}
-
-func execArgs(args []string, env []string) error {
-	name, err := exec.LookPath(args[0])
-	if err != nil {
-		return err
-	}
-	if err := system.Exec(name, args[0:], env); err != nil {
-		return fmt.Errorf("exec %s", err)
-	}
-	panic("unreachable")
+	return system.Execv(args[0], args[0:], container.Env)
 }
 
 func closeStdPipes() {
@@ -93,18 +82,19 @@ func closeStdPipes() {
 	os.Stderr.Close()
 }
 
-// resolveRootfs ensures that the current working directory is
-// not a symlink and returns the absolute path to the rootfs
-func resolveRootfs(uncleanRootfs string) (string, error) {
-	rootfs, err := filepath.Abs(uncleanRootfs)
-	if err != nil {
-		return "", err
-	}
-	return filepath.EvalSymlinks(rootfs)
-}
-
 func setupUser(container *libcontainer.Container) error {
-	if container.User != "" && container.User != "root" {
+	switch container.User {
+	case "root", "":
+		if err := system.Setgroups(nil); err != nil {
+			return err
+		}
+		if err := system.Setresgid(0, 0, 0); err != nil {
+			return err
+		}
+		if err := system.Setresuid(0, 0, 0); err != nil {
+			return err
+		}
+	default:
 		uid, gid, suppGids, err := user.GetUserGroupSupplementary(container.User, syscall.Getuid(), syscall.Getgid())
 		if err != nil {
 			return err
@@ -118,16 +108,6 @@ func setupUser(container *libcontainer.Container) error {
 		if err := system.Setuid(uid); err != nil {
 			return err
 		}
-	} else {
-		if err := system.Setgroups(nil); err != nil {
-			return err
-		}
-		if err := system.Setresgid(0, 0, 0); err != nil {
-			return err
-		}
-		if err := system.Setresuid(0, 0, 0); err != nil {
-			return err
-		}
 	}
 	return nil
 }
@@ -147,16 +127,6 @@ func dupSlave(slave *os.File) error {
 	return nil
 }
 
-// openTerminal is a clone of os.OpenFile without the O_CLOEXEC
-// used to open the pty slave inside the container namespace
-func openTerminal(name string, flag int) (*os.File, error) {
-	r, e := syscall.Open(name, flag, 0)
-	if e != nil {
-		return nil, &os.PathError{"open", name, e}
-	}
-	return os.NewFile(uintptr(r), name), nil
-}
-
 // setupVethNetwork uses the Network config if it is not nil to initialize
 // the new veth interface inside the container for use by changing the name to eth0
 // setting the MTU and IP address along with the default gateway
diff --git a/libcontainer/nsinit/nsinit.go b/libcontainer/nsinit/nsinit.go
index 599461e..f09a130 100644
--- a/libcontainer/nsinit/nsinit.go
+++ b/libcontainer/nsinit/nsinit.go
@@ -2,9 +2,10 @@ package nsinit
 
 import (
 	"github.com/dotcloud/docker/pkg/libcontainer"
-	"log"
 )
 
+// NsInit is an interface with the public facing methods to provide high level
+// exec operations on a container
 type NsInit interface {
 	Exec(container *libcontainer.Container, term Terminal, args []string) (int, error)
 	ExecIn(container *libcontainer.Container, nspid int, args []string) (int, error)
@@ -13,17 +14,13 @@ type NsInit interface {
 
 type linuxNs struct {
 	root           string
-	logFile        string
-	logger         *log.Logger
 	commandFactory CommandFactory
 	stateWriter    StateWriter
 }
 
-func NewNsInit(logger *log.Logger, logFile string, command CommandFactory, state StateWriter) NsInit {
+func NewNsInit(command CommandFactory, state StateWriter) NsInit {
 	return &linuxNs{
-		logger:         logger,
 		commandFactory: command,
 		stateWriter:    state,
-		logFile:        logFile,
 	}
 }
diff --git a/libcontainer/nsinit/nsinit/main.go b/libcontainer/nsinit/nsinit/main.go
index c25037f..e385e7f 100644
--- a/libcontainer/nsinit/nsinit/main.go
+++ b/libcontainer/nsinit/nsinit/main.go
@@ -6,7 +6,6 @@ import (
 	"flag"
 	"github.com/dotcloud/docker/pkg/libcontainer"
 	"github.com/dotcloud/docker/pkg/libcontainer/nsinit"
-	"io"
 	"io/ioutil"
 	"log"
 	"os"
@@ -16,7 +15,6 @@ import (
 var (
 	console string
 	pipeFd  int
-	logFile string
 )
 
 var (
@@ -26,7 +24,6 @@ var (
 
 func registerFlags() {
 	flag.StringVar(&console, "console", "", "console (pty slave) path")
-	flag.StringVar(&logFile, "log", "none", "log options (none, stderr, or a file path)")
 	flag.IntVar(&pipeFd, "pipe", 0, "sync pipe fd")
 
 	flag.Parse()
@@ -113,26 +110,5 @@ func readPid() (int, error) {
 }
 
 func newNsInit() (nsinit.NsInit, error) {
-	logger, err := setupLogging()
-	if err != nil {
-		return nil, err
-	}
-	return nsinit.NewNsInit(logger, logFile, &nsinit.DefaultCommandFactory{}, &nsinit.DefaultStateWriter{}), nil
-}
-
-func setupLogging() (logger *log.Logger, err error) {
-	var writer io.Writer
-
-	switch logFile {
-	case "stderr":
-		writer = os.Stderr
-	case "none", "":
-		writer = ioutil.Discard
-	default:
-		if writer, err = os.OpenFile(logFile, os.O_RDWR|os.O_CREATE|os.O_APPEND, 0755); err != nil {
-			return
-		}
-	}
-	logger = log.New(writer, "", log.LstdFlags)
-	return
+	return nsinit.NewNsInit(&nsinit.DefaultCommandFactory{}, &nsinit.DefaultStateWriter{}), nil
 }
diff --git a/libcontainer/nsinit/state.go b/libcontainer/nsinit/state.go
index 2dbaaa5..5c719e1 100644
--- a/libcontainer/nsinit/state.go
+++ b/libcontainer/nsinit/state.go
@@ -7,6 +7,8 @@ import (
 	"path/filepath"
 )
 
+// StateWriter handles writing and deleting the pid file
+// on disk
 type StateWriter interface {
 	WritePid(pid int) error
 	DeletePid() error
diff --git a/libcontainer/utils/utils.go b/libcontainer/utils/utils.go
index 5050997..0d919bc 100644
--- a/libcontainer/utils/utils.go
+++ b/libcontainer/utils/utils.go
@@ -4,6 +4,7 @@ import (
 	"crypto/rand"
 	"encoding/hex"
 	"io"
+	"path/filepath"
 )
 
 // GenerateRandomName returns a new name joined with a prefix.  This size
@@ -15,3 +16,13 @@ func GenerateRandomName(prefix string, size int) (string, error) {
 	}
 	return prefix + hex.EncodeToString(id)[:size], nil
 }
+
+// ResolveRootfs ensures that the current working directory is
+// not a symlink and returns the absolute path to the rootfs
+func ResolveRootfs(uncleanRootfs string) (string, error) {
+	rootfs, err := filepath.Abs(uncleanRootfs)
+	if err != nil {
+		return "", err
+	}
+	return filepath.EvalSymlinks(rootfs)
+}
diff --git a/system/calls_linux.go b/system/calls_linux.go
index 0bf42e3..b7a8f14 100644
--- a/system/calls_linux.go
+++ b/system/calls_linux.go
@@ -1,6 +1,7 @@
 package system
 
 import (
+	"os/exec"
 	"syscall"
 )
 
@@ -16,6 +17,14 @@ func Exec(cmd string, args []string, env []string) error {
 	return syscall.Exec(cmd, args, env)
 }
 
+func Execv(cmd string, args []string, env []string) error {
+	name, err := exec.LookPath(cmd)
+	if err != nil {
+		return err
+	}
+	return Exec(name, args, env)
+}
+
 func Fork() (int, error) {
 	syscall.ForkLock.Lock()
 	pid, _, err := syscall.Syscall(syscall.SYS_FORK, 0, 0, 0)
diff --git a/system/pty_linux.go b/system/pty_linux.go
index b281b71..ca588d8 100644
--- a/system/pty_linux.go
+++ b/system/pty_linux.go
@@ -24,8 +24,35 @@ func Ptsname(f *os.File) (string, error) {
 	return fmt.Sprintf("/dev/pts/%d", n), nil
 }
 
+// CreateMasterAndConsole will open /dev/ptmx on the host and retreive the
+// pts name for use as the pty slave inside the container
+func CreateMasterAndConsole() (*os.File, string, error) {
+	master, err := os.OpenFile("/dev/ptmx", syscall.O_RDWR|syscall.O_NOCTTY|syscall.O_CLOEXEC, 0)
+	if err != nil {
+		return nil, "", err
+	}
+	console, err := Ptsname(master)
+	if err != nil {
+		return nil, "", err
+	}
+	if err := Unlockpt(master); err != nil {
+		return nil, "", err
+	}
+	return master, console, nil
+}
+
 // OpenPtmx opens /dev/ptmx, i.e. the PTY master.
 func OpenPtmx() (*os.File, error) {
 	// O_NOCTTY and O_CLOEXEC are not present in os package so we use the syscall's one for all.
 	return os.OpenFile("/dev/ptmx", syscall.O_RDONLY|syscall.O_NOCTTY|syscall.O_CLOEXEC, 0)
 }
+
+// OpenTerminal is a clone of os.OpenFile without the O_CLOEXEC
+// used to open the pty slave inside the container namespace
+func OpenTerminal(name string, flag int) (*os.File, error) {
+	r, e := syscall.Open(name, flag, 0)
+	if e != nil {
+		return nil, &os.PathError{"open", name, e}
+	}
+	return os.NewFile(uintptr(r), name), nil
+}