Merge pull request #5234 from crosbymichael/refactor-container-start

Refactor container start

runtime/container.go

@@ -76,42 +76,6 @@ type Container struct {
 	activeLinks map[string]*links.Link
 }
 
-// FIXME: move deprecated port stuff to nat to clean up the core.
-type PortMapping map[string]string // Deprecated
-
-type NetworkSettings struct {
-	IPAddress   string
-	IPPrefixLen int
-	Gateway     string
-	Bridge      string
-	PortMapping map[string]PortMapping // Deprecated
-	Ports       nat.PortMap
-}
-
-func (settings *NetworkSettings) PortMappingAPI() *engine.Table {
-	var outs = engine.NewTable("", 0)
-	for port, bindings := range settings.Ports {
-		p, _ := nat.ParsePort(port.Port())
-		if len(bindings) == 0 {
-			out := &engine.Env{}
-			out.SetInt("PublicPort", p)
-			out.Set("Type", port.Proto())
-			outs.Add(out)
-			continue
-		}
-		for _, binding := range bindings {
-			out := &engine.Env{}
-			h, _ := nat.ParsePort(binding.HostPort)
-			out.SetInt("PrivatePort", p)
-			out.SetInt("PublicPort", h)
-			out.Set("Type", port.Proto())
-			out.Set("IP", binding.HostIp)
-			outs.Add(out)
-		}
-	}
-	return outs
-}
-
 // Inject the io.Reader at the given path. Note: do not close the reader
 func (container *Container) Inject(file io.Reader, pth string) error {
 	if err := container.Mount(); err != nil {
@@ -148,10 +112,6 @@ func (container *Container) Inject(file io.Reader, pth string) error {
 	return nil
 }
 
-func (container *Container) When() time.Time {
-	return container.Created
-}
-
 func (container *Container) FromDisk() error {
 	data, err := ioutil.ReadFile(container.jsonPath())
 	if err != nil {
@@ -358,7 +318,7 @@ func (container *Container) Attach(stdin io.ReadCloser, stdinCloser io.Closer, s
 	})
 }
 
-func populateCommand(c *Container) {
+func populateCommand(c *Container, env []string) {
 	var (
 		en           *execdriver.Network
 		driverConfig = make(map[string][]string)
@@ -402,18 +362,7 @@ func populateCommand(c *Container) {
 		Resources:  resources,
 	}
 	c.command.SysProcAttr = &syscall.SysProcAttr{Setsid: true}
-}
-
-func (container *Container) ArgsAsString() string {
-	var args []string
-	for _, arg := range container.Args {
-		if strings.Contains(arg, " ") {
-			args = append(args, fmt.Sprintf("'%s'", arg))
-		} else {
-			args = append(args, arg)
-		}
-	}
-	return strings.Join(args, " ")
+	c.command.Env = env
 }
 
 func (container *Container) Start() (err error) {
@@ -423,186 +372,50 @@ func (container *Container) Start() (err error) {
 	if container.State.IsRunning() {
 		return nil
 	}
-
+	// if we encounter and error during start we need to ensure that any other
+	// setup has been cleaned up properly
 	defer func() {
 		if err != nil {
 			container.cleanup()
 		}
 	}()
 
-	if container.ResolvConfPath == "" {
-		if err := container.setupContainerDns(); err != nil {
-			return err
-		}
+	if err := container.setupContainerDns(); err != nil {
+		return err
 	}
-
 	if err := container.Mount(); err != nil {
 		return err
 	}
-
-	if container.runtime.config.DisableNetwork {
-		container.Config.NetworkDisabled = true
-		container.buildHostnameAndHostsFiles("127.0.1.1")
-	} else {
-		if err := container.allocateNetwork(); err != nil {
-			return err
-		}
-		container.buildHostnameAndHostsFiles(container.NetworkSettings.IPAddress)
-	}
-
-	// Make sure the config is compatible with the current kernel
-	if container.Config.Memory > 0 && !container.runtime.sysInfo.MemoryLimit {
-		log.Printf("WARNING: Your kernel does not support memory limit capabilities. Limitation discarded.\n")
-		container.Config.Memory = 0
-	}
-	if container.Config.Memory > 0 && !container.runtime.sysInfo.SwapLimit {
-		log.Printf("WARNING: Your kernel does not support swap limit capabilities. Limitation discarded.\n")
-		container.Config.MemorySwap = -1
-	}
-
-	if container.runtime.sysInfo.IPv4ForwardingDisabled {
-		log.Printf("WARNING: IPv4 forwarding is disabled. Networking will not work")
+	if err := container.initializeNetworking(); err != nil {
+		return err
 	}
-
+	container.verifyRuntimeSettings()
 	if err := prepareVolumesForContainer(container); err != nil {
 		return err
 	}
-
-	// Setup environment
-	env := []string{
-		"HOME=/",
-		"PATH=" + DefaultPathEnv,
-		"HOSTNAME=" + container.Config.Hostname,
-	}
-
-	if container.Config.Tty {
-		env = append(env, "TERM=xterm")
-	}
-
-	// Init any links between the parent and children
-	runtime := container.runtime
-
-	children, err := runtime.Children(container.Name)
+	linkedEnv, err := container.setupLinkedContainers()
 	if err != nil {
 		return err
 	}
-
-	if len(children) > 0 {
-		container.activeLinks = make(map[string]*links.Link, len(children))
-
-		// If we encounter an error make sure that we rollback any network
-		// config and ip table changes
-		rollback := func() {
-			for _, link := range container.activeLinks {
-				link.Disable()
-			}
-			container.activeLinks = nil
-		}
-
-		for linkAlias, child := range children {
-			if !child.State.IsRunning() {
-				return fmt.Errorf("Cannot link to a non running container: %s AS %s", child.Name, linkAlias)
-			}
-
-			link, err := links.NewLink(
-				container.NetworkSettings.IPAddress,
-				child.NetworkSettings.IPAddress,
-				linkAlias,
-				child.Config.Env,
-				child.Config.ExposedPorts,
-				runtime.eng)
-
-			if err != nil {
-				rollback()
-				return err
-			}
-
-			container.activeLinks[link.Alias()] = link
-			if err := link.Enable(); err != nil {
-				rollback()
-				return err
-			}
-
-			for _, envVar := range link.ToEnv() {
-				env = append(env, envVar)
-			}
-		}
-	}
-
-	// because the env on the container can override certain default values
-	// we need to replace the 'env' keys where they match and append anything
-	// else.
-	env = utils.ReplaceOrAppendEnvValues(env, container.Config.Env)
+	env := container.createRuntimeEnvironment(linkedEnv)
+	// TODO: This is only needed for lxc so we should look for a way to
+	// remove this dep
 	if err := container.generateEnvConfig(env); err != nil {
 		return err
 	}
-
-	if container.Config.WorkingDir != "" {
-		container.Config.WorkingDir = path.Clean(container.Config.WorkingDir)
-
-		pthInfo, err := os.Stat(path.Join(container.basefs, container.Config.WorkingDir))
-		if err != nil {
-			if !os.IsNotExist(err) {
-				return err
-			}
-			if err := os.MkdirAll(path.Join(container.basefs, container.Config.WorkingDir), 0755); err != nil {
-				return err
-			}
-		}
-		if pthInfo != nil && !pthInfo.IsDir() {
-			return fmt.Errorf("Cannot mkdir: %s is not a directory", container.Config.WorkingDir)
-		}
-	}
-
-	envPath, err := container.EnvConfigPath()
-	if err != nil {
+	if err := container.setupWorkingDirectory(); err != nil {
 		return err
 	}
-
-	populateCommand(container)
-	container.command.Env = env
-
-	if err := setupMountsForContainer(container, envPath); err != nil {
+	populateCommand(container, env)
+	if err := setupMountsForContainer(container); err != nil {
 		return err
 	}
-
-	// Setup logging of stdout and stderr to disk
-	if err := container.runtime.LogToDisk(container.stdout, container.logPath("json"), "stdout"); err != nil {
-		return err
-	}
-	if err := container.runtime.LogToDisk(container.stderr, container.logPath("json"), "stderr"); err != nil {
+	if err := container.startLoggingToDisk(); err != nil {
 		return err
 	}
 	container.waitLock = make(chan struct{})
 
-	callbackLock := make(chan struct{})
-	callback := func(command *execdriver.Command) {
-		container.State.SetRunning(command.Pid())
-		if command.Tty {
-			// The callback is called after the process Start()
-			// so we are in the parent process. In TTY mode, stdin/out/err is the PtySlace
-			// which we close here.
-			if c, ok := command.Stdout.(io.Closer); ok {
-				c.Close()
-			}
-		}
-		if err := container.ToDisk(); err != nil {
-			utils.Debugf("%s", err)
-		}
-		close(callbackLock)
-	}
-
-	// We use a callback here instead of a goroutine and an chan for
-	// syncronization purposes
-	cErr := utils.Go(func() error { return container.monitor(callback) })
-
-	// Start should not return until the process is actually running
-	select {
-	case <-callbackLock:
-	case err := <-cErr:
-		return err
-	}
-	return nil
+	return container.waitForStart()
 }
 
 func (container *Container) Run() error {
@@ -1182,6 +995,9 @@ func (container *Container) DisableLink(name string) {
 }
 
 func (container *Container) setupContainerDns() error {
+	if container.ResolvConfPath != "" {
+		return nil
+	}
 	var (
 		config  = container.hostConfig
 		runtime = container.runtime
@@ -1227,3 +1043,166 @@ func (container *Container) setupContainerDns() error {
 	}
 	return nil
 }
+
+func (container *Container) initializeNetworking() error {
+	if container.runtime.config.DisableNetwork {
+		container.Config.NetworkDisabled = true
+		container.buildHostnameAndHostsFiles("127.0.1.1")
+	} else {
+		if err := container.allocateNetwork(); err != nil {
+			return err
+		}
+		container.buildHostnameAndHostsFiles(container.NetworkSettings.IPAddress)
+	}
+	return nil
+}
+
+// Make sure the config is compatible with the current kernel
+func (container *Container) verifyRuntimeSettings() {
+	if container.Config.Memory > 0 && !container.runtime.sysInfo.MemoryLimit {
+		log.Printf("WARNING: Your kernel does not support memory limit capabilities. Limitation discarded.\n")
+		container.Config.Memory = 0
+	}
+	if container.Config.Memory > 0 && !container.runtime.sysInfo.SwapLimit {
+		log.Printf("WARNING: Your kernel does not support swap limit capabilities. Limitation discarded.\n")
+		container.Config.MemorySwap = -1
+	}
+	if container.runtime.sysInfo.IPv4ForwardingDisabled {
+		log.Printf("WARNING: IPv4 forwarding is disabled. Networking will not work")
+	}
+}
+
+func (container *Container) setupLinkedContainers() ([]string, error) {
+	var (
+		env     []string
+		runtime = container.runtime
+	)
+	children, err := runtime.Children(container.Name)
+	if err != nil {
+		return nil, err
+	}
+
+	if len(children) > 0 {
+		container.activeLinks = make(map[string]*links.Link, len(children))
+
+		// If we encounter an error make sure that we rollback any network
+		// config and ip table changes
+		rollback := func() {
+			for _, link := range container.activeLinks {
+				link.Disable()
+			}
+			container.activeLinks = nil
+		}
+
+		for linkAlias, child := range children {
+			if !child.State.IsRunning() {
+				return nil, fmt.Errorf("Cannot link to a non running container: %s AS %s", child.Name, linkAlias)
+			}
+
+			link, err := links.NewLink(
+				container.NetworkSettings.IPAddress,
+				child.NetworkSettings.IPAddress,
+				linkAlias,
+				child.Config.Env,
+				child.Config.ExposedPorts,
+				runtime.eng)
+
+			if err != nil {
+				rollback()
+				return nil, err
+			}
+
+			container.activeLinks[link.Alias()] = link
+			if err := link.Enable(); err != nil {
+				rollback()
+				return nil, err
+			}
+
+			for _, envVar := range link.ToEnv() {
+				env = append(env, envVar)
+			}
+		}
+	}
+	return env, nil
+}
+
+func (container *Container) createRuntimeEnvironment(linkedEnv []string) []string {
+	// Setup environment
+	env := []string{
+		"HOME=/",
+		"PATH=" + DefaultPathEnv,
+		"HOSTNAME=" + container.Config.Hostname,
+	}
+	if container.Config.Tty {
+		env = append(env, "TERM=xterm")
+	}
+	env = append(env, linkedEnv...)
+	// because the env on the container can override certain default values
+	// we need to replace the 'env' keys where they match and append anything
+	// else.
+	env = utils.ReplaceOrAppendEnvValues(env, container.Config.Env)
+
+	return env
+}
+
+func (container *Container) setupWorkingDirectory() error {
+	if container.Config.WorkingDir != "" {
+		container.Config.WorkingDir = path.Clean(container.Config.WorkingDir)
+
+		pthInfo, err := os.Stat(path.Join(container.basefs, container.Config.WorkingDir))
+		if err != nil {
+			if !os.IsNotExist(err) {
+				return err
+			}
+			if err := os.MkdirAll(path.Join(container.basefs, container.Config.WorkingDir), 0755); err != nil {
+				return err
+			}
+		}
+		if pthInfo != nil && !pthInfo.IsDir() {
+			return fmt.Errorf("Cannot mkdir: %s is not a directory", container.Config.WorkingDir)
+		}
+	}
+	return nil
+}
+
+func (container *Container) startLoggingToDisk() error {
+	// Setup logging of stdout and stderr to disk
+	if err := container.runtime.LogToDisk(container.stdout, container.logPath("json"), "stdout"); err != nil {
+		return err
+	}
+	if err := container.runtime.LogToDisk(container.stderr, container.logPath("json"), "stderr"); err != nil {
+		return err
+	}
+	return nil
+}
+
+func (container *Container) waitForStart() error {
+	callbackLock := make(chan struct{})
+	callback := func(command *execdriver.Command) {
+		container.State.SetRunning(command.Pid())
+		if command.Tty {
+			// The callback is called after the process Start()
+			// so we are in the parent process. In TTY mode, stdin/out/err is the PtySlace
+			// which we close here.
+			if c, ok := command.Stdout.(io.Closer); ok {
+				c.Close()
+			}
+		}
+		if err := container.ToDisk(); err != nil {
+			utils.Debugf("%s", err)
+		}
+		close(callbackLock)
+	}
+
+	// We use a callback here instead of a goroutine and an chan for
+	// syncronization purposes
+	cErr := utils.Go(func() error { return container.monitor(callback) })
+
+	// Start should not return until the process is actually running
+	select {
+	case <-callbackLock:
+	case err := <-cErr:
+		return err
+	}
+	return nil
+}

runtime/history.go

@@ -14,7 +14,7 @@ func (history *History) Len() int {
 
 func (history *History) Less(i, j int) bool {
 	containers := *history
-	return containers[j].When().Before(containers[i].When())
+	return containers[j].Created.Before(containers[i].Created)
 }
 
 func (history *History) Swap(i, j int) {

runtime/network_settings.go

@@ -0,0 +1,42 @@
+package runtime
+
+import (
+	"github.com/dotcloud/docker/engine"
+	"github.com/dotcloud/docker/nat"
+)
+
+// FIXME: move deprecated port stuff to nat to clean up the core.
+type PortMapping map[string]string // Deprecated
+
+type NetworkSettings struct {
+	IPAddress   string
+	IPPrefixLen int
+	Gateway     string
+	Bridge      string
+	PortMapping map[string]PortMapping // Deprecated
+	Ports       nat.PortMap
+}
+
+func (settings *NetworkSettings) PortMappingAPI() *engine.Table {
+	var outs = engine.NewTable("", 0)
+	for port, bindings := range settings.Ports {
+		p, _ := nat.ParsePort(port.Port())
+		if len(bindings) == 0 {
+			out := &engine.Env{}
+			out.SetInt("PublicPort", p)
+			out.Set("Type", port.Proto())
+			outs.Add(out)
+			continue
+		}
+		for _, binding := range bindings {
+			out := &engine.Env{}
+			h, _ := nat.ParsePort(binding.HostPort)
+			out.SetInt("PrivatePort", p)
+			out.SetInt("PublicPort", h)
+			out.Set("Type", port.Proto())
+			out.Set("IP", binding.HostIp)
+			outs.Add(out)
+		}
+	}
+	return outs
+}

runtime/volumes.go

@@ -33,7 +33,12 @@ func prepareVolumesForContainer(container *Container) error {
 	return nil
 }
 
-func setupMountsForContainer(container *Container, envPath string) error {
+func setupMountsForContainer(container *Container) error {
+	envPath, err := container.EnvConfigPath()
+	if err != nil {
+		return err
+	}
+
 	mounts := []execdriver.Mount{
 		{container.runtime.sysInitPath, "/.dockerinit", false, true},
 		{envPath, "/.dockerenv", false, true},

server/server.go

@@ -1063,7 +1063,17 @@ func (srv *Server) Containers(job *engine.Job) engine.Status {
 		out.SetList("Names", names[container.ID])
 		out.Set("Image", srv.runtime.Repositories().ImageName(container.Image))
 		if len(container.Args) > 0 {
-			out.Set("Command", fmt.Sprintf("\"%s %s\"", container.Path, container.ArgsAsString()))
+			args := []string{}
+			for _, arg := range container.Args {
+				if strings.Contains(arg, " ") {
+					args = append(args, fmt.Sprintf("'%s'", arg))
+				} else {
+					args = append(args, arg)
+				}
+			}
+			argsAsString := strings.Join(args, " ")
+
+			out.Set("Command", fmt.Sprintf("\"%s %s\"", container.Path, argsAsString))
 		} else {
 			out.Set("Command", fmt.Sprintf("\"%s\"", container.Path))
 		}