Merge pull request #5511 from crosbymichael/refactor-libcontainer

Refactor: remove statewriter type and all callback for process start

daemon/execdriver/native/driver.go

@@ -3,9 +3,7 @@ package native
 import (
 	"encoding/json"
 	"fmt"
-	"io"
 	"io/ioutil"
-	"log"
 	"os"
 	"os/exec"
 	"path/filepath"
@@ -31,7 +29,7 @@ func init() {
 	execdriver.RegisterInitFunc(DriverName, func(args *execdriver.InitArgs) error {
 		var (
 			container *libcontainer.Container
-			ns        = nsinit.NewNsInit(&nsinit.DefaultCommandFactory{}, &nsinit.DefaultStateWriter{args.Root}, createLogger(""))
+			ns        = nsinit.NewNsInit(&nsinit.DefaultCommandFactory{})
 		)
 		f, err := os.Open(filepath.Join(args.Root, "container.json"))
 		if err != nil {
@@ -95,15 +93,11 @@ func (d *driver) Run(c *execdriver.Command, pipes *execdriver.Pipes, startCallba
 	d.activeContainers[c.ID] = &c.Cmd
 
 	var (
-		term        nsinit.Terminal
+		term    nsinit.Terminal
-		factory     = &dockerCommandFactory{c: c, driver: d}
+		factory = &dockerCommandFactory{c: c, driver: d}
-		stateWriter = &dockerStateWriter{
+		pidRoot = filepath.Join(d.root, c.ID)
-			callback: startCallback,
+		ns      = nsinit.NewNsInit(factory)
-			c:        c,
+		args    = append([]string{c.Entrypoint}, c.Arguments...)
-			dsw:      &nsinit.DefaultStateWriter{filepath.Join(d.root, c.ID)},
-		}
-		ns   = nsinit.NewNsInit(factory, stateWriter, createLogger(os.Getenv("DEBUG")))
-		args = append([]string{c.Entrypoint}, c.Arguments...)
 	)
 	if err := d.createContainerRoot(c.ID); err != nil {
 		return -1, err
@@ -123,7 +117,11 @@ func (d *driver) Run(c *execdriver.Command, pipes *execdriver.Pipes, startCallba
 	if err := d.writeContainerFile(container, c.ID); err != nil {
 		return -1, err
 	}
-	return ns.Exec(container, term, args)
+	return ns.Exec(container, term, pidRoot, args, func() {
+		if startCallback != nil {
+			startCallback(c)
+		}
+	})
 }
 
 func (d *driver) Kill(p *execdriver.Command, sig int) error {
@@ -268,33 +266,3 @@ func (d *dockerCommandFactory) Create(container *libcontainer.Container, console
 
 	return &d.c.Cmd
 }
-
-type dockerStateWriter struct {
-	dsw      nsinit.StateWriter
-	c        *execdriver.Command
-	callback execdriver.StartCallback
-}
-
-func (d *dockerStateWriter) WritePid(pid int, started string) error {
-	d.c.ContainerPid = pid
-	err := d.dsw.WritePid(pid, started)
-	if d.callback != nil {
-		d.callback(d.c)
-	}
-	return err
-}
-
-func (d *dockerStateWriter) DeletePid() error {
-	return d.dsw.DeletePid()
-}
-
-func createLogger(debug string) *log.Logger {
-	var w io.Writer
-	// if we are in debug mode set the logger to stderr
-	if debug != "" {
-		w = os.Stderr
-	} else {
-		w = ioutil.Discard
-	}
-	return log.New(w, "[libcontainer] ", log.LstdFlags)
-}

pkg/libcontainer/nsinit/exec.go

@@ -3,8 +3,11 @@
 package nsinit
 
 import (
+	"fmt"
+	"io/ioutil"
 	"os"
 	"os/exec"
+	"path/filepath"
 	"syscall"
 
 	"github.com/dotcloud/docker/pkg/cgroups"
@@ -17,7 +20,7 @@ import (
 
 // Exec performes setup outside of a namespace so that a container can be
 // executed.  Exec is a high level function for working with container namespaces.
-func (ns *linuxNs) Exec(container *libcontainer.Container, term Terminal, args []string) (int, error) {
+func (ns *linuxNs) Exec(container *libcontainer.Container, term Terminal, pidRoot string, args []string, startCallback func()) (int, error) {
 	var (
 		master  *os.File
 		console string
@@ -30,10 +33,8 @@ func (ns *linuxNs) Exec(container *libcontainer.Container, term Terminal, args [
 	if err != nil {
 		return -1, err
 	}
-	ns.logger.Printf("created sync pipe parent fd %d child fd %d\n", syncPipe.parent.Fd(), syncPipe.child.Fd())
 
 	if container.Tty {
-		ns.logger.Println("creating master and console")
 		master, console, err = system.CreateMasterAndConsole()
 		if err != nil {
 			return -1, err
@@ -42,13 +43,11 @@ func (ns *linuxNs) Exec(container *libcontainer.Container, term Terminal, args [
 	}
 
 	command := ns.commandFactory.Create(container, console, syncPipe.child, args)
-	ns.logger.Println("attach terminal to command")
 	if err := term.Attach(command); err != nil {
 		return -1, err
 	}
 	defer term.Close()
 
-	ns.logger.Println("starting command")
 	if err := command.Start(); err != nil {
 		return -1, err
 	}
@@ -57,49 +56,47 @@ func (ns *linuxNs) Exec(container *libcontainer.Container, term Terminal, args [
 	if err != nil {
 		return -1, err
 	}
-	ns.logger.Printf("writing pid %d to file\n", command.Process.Pid)
+	if err := WritePid(pidRoot, command.Process.Pid, started); err != nil {
-	if err := ns.stateWriter.WritePid(command.Process.Pid, started); err != nil {
 		command.Process.Kill()
 		return -1, err
 	}
-	defer func() {
+	defer DeletePid(pidRoot)
-		ns.logger.Println("removing pid file")
-		ns.stateWriter.DeletePid()
-	}()
 
 	// Do this before syncing with child so that no children
 	// can escape the cgroup
-	ns.logger.Println("setting cgroups")
+	cleaner, err := SetupCgroups(container, command.Process.Pid)
-	activeCgroup, err := ns.SetupCgroups(container, command.Process.Pid)
 	if err != nil {
 		command.Process.Kill()
 		return -1, err
 	}
-	if activeCgroup != nil {
+	if cleaner != nil {
-		defer activeCgroup.Cleanup()
+		defer cleaner.Cleanup()
 	}
 
-	ns.logger.Println("setting up network")
+	if err := InitializeNetworking(container, command.Process.Pid, syncPipe); err != nil {
-	if err := ns.InitializeNetworking(container, command.Process.Pid, syncPipe); err != nil {
 		command.Process.Kill()
 		return -1, err
 	}
 
-	ns.logger.Println("closing sync pipe with child")
 	// Sync with child
 	syncPipe.Close()
 
+	if startCallback != nil {
+		startCallback()
+	}
+
 	if err := command.Wait(); err != nil {
 		if _, ok := err.(*exec.ExitError); !ok {
 			return -1, err
 		}
 	}
 	status := command.ProcessState.Sys().(syscall.WaitStatus).ExitStatus()
-	ns.logger.Printf("process exited with status %d\n", status)
 	return status, err
 }
 
-func (ns *linuxNs) SetupCgroups(container *libcontainer.Container, nspid int) (cgroups.ActiveCgroup, error) {
+// SetupCgroups applies the cgroup restrictions to the process running in the contaienr based
+// on the container's configuration
+func SetupCgroups(container *libcontainer.Container, nspid int) (cgroups.ActiveCgroup, error) {
 	if container.Cgroups != nil {
 		c := container.Cgroups
 		if systemd.UseSystemd() {
@@ -110,7 +107,9 @@ func (ns *linuxNs) SetupCgroups(container *libcontainer.Container, nspid int) (c
 	return nil, nil
 }
 
-func (ns *linuxNs) InitializeNetworking(container *libcontainer.Container, nspid int, pipe *SyncPipe) error {
+// InitializeNetworking creates the container's network stack outside of the namespace and moves
+// interfaces into the container's net namespaces if necessary
+func InitializeNetworking(container *libcontainer.Container, nspid int, pipe *SyncPipe) error {
 	context := libcontainer.Context{}
 	for _, config := range container.Networks {
 		strategy, err := network.GetStrategy(config.Type)
@@ -123,3 +122,23 @@ func (ns *linuxNs) InitializeNetworking(container *libcontainer.Container, nspid
 	}
 	return pipe.SendToChild(context)
 }
+
+// WritePid writes the namespaced processes pid to pid and it's start time
+// to the path specified
+func WritePid(path string, pid int, startTime string) error {
+	err := ioutil.WriteFile(filepath.Join(path, "pid"), []byte(fmt.Sprint(pid)), 0655)
+	if err != nil {
+		return err
+	}
+	return ioutil.WriteFile(filepath.Join(path, "start"), []byte(startTime), 0655)
+}
+
+// DeletePid removes the pid and started file from disk when the container's process
+// dies and the container is cleanly removed
+func DeletePid(path string) error {
+	err := os.Remove(filepath.Join(path, "pid"))
+	if serr := os.Remove(filepath.Join(path, "start")); err == nil {
+		err = serr
+	}
+	return err
+}

pkg/libcontainer/nsinit/execin.go

@@ -4,14 +4,15 @@ package nsinit
 
 import (
 	"fmt"
-	"github.com/dotcloud/docker/pkg/label"
-	"github.com/dotcloud/docker/pkg/libcontainer"
-	"github.com/dotcloud/docker/pkg/libcontainer/mount"
-	"github.com/dotcloud/docker/pkg/system"
 	"os"
 	"path/filepath"
 	"strconv"
 	"syscall"
+
+	"github.com/dotcloud/docker/pkg/label"
+	"github.com/dotcloud/docker/pkg/libcontainer"
+	"github.com/dotcloud/docker/pkg/libcontainer/mount"
+	"github.com/dotcloud/docker/pkg/system"
 )
 
 // ExecIn uses an existing pid and joins the pid's namespaces with the new command.
@@ -42,7 +43,6 @@ func (ns *linuxNs) ExecIn(container *libcontainer.Container, nspid int, args []s
 	// foreach namespace fd, use setns to join an existing container's namespaces
 	for _, fd := range fds {
 		if fd > 0 {
-			ns.logger.Printf("setns on %d\n", fd)
 			if err := system.Setns(fd, 0); err != nil {
 				closeFds()
 				return -1, fmt.Errorf("setns %s", err)
@@ -54,7 +54,6 @@ func (ns *linuxNs) ExecIn(container *libcontainer.Container, nspid int, args []s
 	// if the container has a new pid and mount namespace we need to
 	// remount proc and sys to pick up the changes
 	if container.Namespaces.Contains("NEWNS") && container.Namespaces.Contains("NEWPID") {
-		ns.logger.Println("forking to remount /proc and /sys")
 		pid, err := system.Fork()
 		if err != nil {
 			return -1, err

pkg/libcontainer/nsinit/init.go

@@ -29,17 +29,14 @@ func (ns *linuxNs) Init(container *libcontainer.Container, uncleanRootfs, consol
 	}
 
 	// We always read this as it is a way to sync with the parent as well
-	ns.logger.Printf("reading from sync pipe fd %d\n", syncPipe.child.Fd())
 	context, err := syncPipe.ReadFromParent()
 	if err != nil {
 		syncPipe.Close()
 		return err
 	}
-	ns.logger.Println("received context from parent")
 	syncPipe.Close()
 
 	if consolePath != "" {
-		ns.logger.Printf("setting up %s as console\n", consolePath)
 		if err := console.OpenAndDup(consolePath); err != nil {
 			return err
 		}
@@ -57,7 +54,6 @@ func (ns *linuxNs) Init(container *libcontainer.Container, uncleanRootfs, consol
 	}
 
 	label.Init()
-	ns.logger.Println("setup mount namespace")
 	if err := mount.InitializeMountNamespace(rootfs, consolePath, container); err != nil {
 		return fmt.Errorf("setup mount namespace %s", err)
 	}
@@ -69,7 +65,6 @@ func (ns *linuxNs) Init(container *libcontainer.Container, uncleanRootfs, consol
 	}
 
 	if profile := container.Context["apparmor_profile"]; profile != "" {
-		ns.logger.Printf("setting apparmor profile %s\n", profile)
 		if err := apparmor.ApplyProfile(os.Getpid(), profile); err != nil {
 			return err
 		}
@@ -79,14 +74,14 @@ func (ns *linuxNs) Init(container *libcontainer.Container, uncleanRootfs, consol
 	if err := label.SetProcessLabel(container.Context["process_label"]); err != nil {
 		return fmt.Errorf("set process label %s", err)
 	}
-	ns.logger.Printf("execing %s\n", args[0])
 	return system.Execv(args[0], args[0:], container.Env)
 }
 
-func setupUser(container *libcontainer.Container) error {
+// SetupUser changes the groups, gid, and uid for the user inside the container
-	uid, gid, suppGids, err := user.GetUserGroupSupplementary(container.User, syscall.Getuid(), syscall.Getgid())
+func SetupUser(u string) error {
+	uid, gid, suppGids, err := user.GetUserGroupSupplementary(u, syscall.Getuid(), syscall.Getgid())
 	if err != nil {
-		return fmt.Errorf("GetUserGroupSupplementary %s", err)
+		return fmt.Errorf("get supplementary groups %s", err)
 	}
 	if err := system.Setgroups(suppGids); err != nil {
 		return fmt.Errorf("setgroups %s", err)
@@ -128,7 +123,7 @@ func finalizeNamespace(container *libcontainer.Container) error {
 	if err := system.CloseFdsFrom(3); err != nil {
 		return fmt.Errorf("close open file descriptors %s", err)
 	}
-	if err := setupUser(container); err != nil {
+	if err := SetupUser(container.User); err != nil {
 		return fmt.Errorf("setup user %s", err)
 	}
 	if container.WorkingDir != "" {

pkg/libcontainer/nsinit/nsinit.go

@@ -1,14 +1,11 @@
 package nsinit
 
-import (
+import "github.com/dotcloud/docker/pkg/libcontainer"
-	"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)
+	Exec(container *libcontainer.Container, term Terminal, pidRoot string, args []string, startCallback func()) (int, error)
 	ExecIn(container *libcontainer.Container, nspid int, args []string) (int, error)
 	Init(container *libcontainer.Container, uncleanRootfs, console string, syncPipe *SyncPipe, args []string) error
 }
@@ -16,14 +13,10 @@ type NsInit interface {
 type linuxNs struct {
 	root           string
 	commandFactory CommandFactory
-	stateWriter    StateWriter
-	logger         *log.Logger
 }
 
-func NewNsInit(command CommandFactory, state StateWriter, logger *log.Logger) NsInit {
+func NewNsInit(command CommandFactory) NsInit {
 	return &linuxNs{
 		commandFactory: command,
-		stateWriter:    state,
-		logger:         logger,
 	}
 }

pkg/libcontainer/nsinit/nsinit/main.go

@@ -3,7 +3,6 @@ package main
 import (
 	"encoding/json"
 	"flag"
-	"io"
 	"io/ioutil"
 	"log"
 	"os"
@@ -38,12 +37,8 @@ func main() {
 	if err != nil {
 		log.Fatalf("Unable to load container: %s", err)
 	}
-	l, err := getLogger("[exec] ")
-	if err != nil {
-		log.Fatal(err)
-	}
 
-	ns, err := newNsInit(l)
+	ns, err := newNsInit()
 	if err != nil {
 		log.Fatalf("Unable to initialize nsinit: %s", err)
 	}
@@ -54,36 +49,36 @@ func main() {
 		nspid, err := readPid()
 		if err != nil {
 			if !os.IsNotExist(err) {
-				l.Fatalf("Unable to read pid: %s", err)
+				log.Fatalf("Unable to read pid: %s", err)
 			}
 		}
 		if nspid > 0 {
 			exitCode, err = ns.ExecIn(container, nspid, flag.Args()[1:])
 		} else {
 			term := nsinit.NewTerminal(os.Stdin, os.Stdout, os.Stderr, container.Tty)
-			exitCode, err = ns.Exec(container, term, flag.Args()[1:])
+			exitCode, err = ns.Exec(container, term, root, flag.Args()[1:], nil)
 		}
 		if err != nil {
-			l.Fatalf("Failed to exec: %s", err)
+			log.Fatalf("Failed to exec: %s", err)
 		}
 		os.Exit(exitCode)
 	case "init": // this is executed inside of the namespace to setup the container
 		cwd, err := os.Getwd()
 		if err != nil {
-			l.Fatal(err)
+			log.Fatal(err)
 		}
 		if flag.NArg() < 2 {
-			l.Fatalf("wrong number of arguments %d", flag.NArg())
+			log.Fatalf("wrong number of arguments %d", flag.NArg())
 		}
 		syncPipe, err := nsinit.NewSyncPipeFromFd(0, uintptr(pipeFd))
 		if err != nil {
-			l.Fatalf("Unable to create sync pipe: %s", err)
+			log.Fatalf("Unable to create sync pipe: %s", err)
 		}
 		if err := ns.Init(container, cwd, console, syncPipe, flag.Args()[1:]); err != nil {
-			l.Fatalf("Unable to initialize for container: %s", err)
+			log.Fatalf("Unable to initialize for container: %s", err)
 		}
 	default:
-		l.Fatalf("command not supported for nsinit %s", flag.Arg(0))
+		log.Fatalf("command not supported for nsinit %s", flag.Arg(0))
 	}
 }
 
@@ -113,23 +108,6 @@ func readPid() (int, error) {
 	return pid, nil
 }
 
-func newNsInit(l *log.Logger) (nsinit.NsInit, error) {
+func newNsInit() (nsinit.NsInit, error) {
-	return nsinit.NewNsInit(&nsinit.DefaultCommandFactory{root}, &nsinit.DefaultStateWriter{root}, l), nil
+	return nsinit.NewNsInit(&nsinit.DefaultCommandFactory{root}), nil
-}
-
-func getLogger(prefix string) (*log.Logger, error) {
-	var w io.Writer
-	switch logs {
-	case "", "none":
-		w = ioutil.Discard
-	case "stderr":
-		w = os.Stderr
-	default: // we have a filepath
-		f, err := os.OpenFile(logs, os.O_CREATE|os.O_RDWR|os.O_APPEND, 0755)
-		if err != nil {
-			return nil, err
-		}
-		w = f
-	}
-	return log.New(w, prefix, log.LstdFlags), nil
 }

pkg/libcontainer/nsinit/state.go

@@ -1,36 +0,0 @@
-package nsinit
-
-import (
-	"fmt"
-	"io/ioutil"
-	"os"
-	"path/filepath"
-)
-
-// StateWriter handles writing and deleting the pid file
-// on disk
-type StateWriter interface {
-	WritePid(pid int, startTime string) error
-	DeletePid() error
-}
-
-type DefaultStateWriter struct {
-	Root string
-}
-
-// writePidFile writes the namespaced processes pid to pid in the rootfs for the container
-func (d *DefaultStateWriter) WritePid(pid int, startTime string) error {
-	err := ioutil.WriteFile(filepath.Join(d.Root, "pid"), []byte(fmt.Sprint(pid)), 0655)
-	if err != nil {
-		return err
-	}
-	return ioutil.WriteFile(filepath.Join(d.Root, "start"), []byte(startTime), 0655)
-}
-
-func (d *DefaultStateWriter) DeletePid() error {
-	err := os.Remove(filepath.Join(d.Root, "pid"))
-	if serr := os.Remove(filepath.Join(d.Root, "start")); err == nil {
-		err = serr
-	}
-	return err
-}

pkg/libcontainer/nsinit/unsupported.go

@@ -6,7 +6,7 @@ import (
 	"github.com/dotcloud/docker/pkg/libcontainer"
 )
 
-func (ns *linuxNs) Exec(container *libcontainer.Container, term Terminal, args []string) (int, error) {
+func (ns *linuxNs) Exec(container *libcontainer.Container, term Terminal, pidRoot string, args []string, startCallback func()) (int, error) {
 	return -1, libcontainer.ErrUnsupported
 }