Browse Source

Address comments.

Docker-DCO-1.1-Signed-off-by: Vishnu Kannan <vishnuk@google.com> (github: vishh)
Vishnu Kannan 10 years ago
parent
commit
46f2944977

+ 1 - 1
daemon/daemon.go

@@ -991,7 +991,7 @@ func (daemon *Daemon) Diff(container *Container) (archive.Archive, error) {
 	return daemon.driver.Diff(container.ID, initID)
 }
 
-func (daemon *Daemon) Run(c *Container, pipes *execdriver.Pipes, startCallback execdriver.StartCallback) (*execdriver.ExitStatus, error) {
+func (daemon *Daemon) Run(c *Container, pipes *execdriver.Pipes, startCallback execdriver.StartCallback) (execdriver.ExitStatus, error) {
 	return daemon.execDriver.Run(c.command, pipes, startCallback)
 }
 

+ 1 - 1
daemon/execdriver/driver.go

@@ -50,7 +50,7 @@ type ExitStatus struct {
 }
 
 type Driver interface {
-	Run(c *Command, pipes *Pipes, startCallback StartCallback) (int, error) // Run executes the process and blocks until the process exits and returns the exit code
+	Run(c *Command, pipes *Pipes, startCallback StartCallback) (ExitStatus, error) // Run executes the process and blocks until the process exits and returns the exit code
 	// Exec executes the process in an existing container, blocks until the process exits and returns the exit code
 	Exec(c *Command, processConfig *ProcessConfig, pipes *Pipes, startCallback StartCallback) (int, error)
 	Kill(c *Command, sig int) error

+ 7 - 7
daemon/execdriver/lxc/driver.go

@@ -55,7 +55,7 @@ func (d *driver) Name() string {
 	return fmt.Sprintf("%s-%s", DriverName, version)
 }
 
-func (d *driver) Run(c *execdriver.Command, pipes *execdriver.Pipes, startCallback execdriver.StartCallback) (*execdriver.ExitStatus, error) {
+func (d *driver) Run(c *execdriver.Command, pipes *execdriver.Pipes, startCallback execdriver.StartCallback) (execdriver.ExitStatus, error) {
 	var (
 		term execdriver.Terminal
 		err  error
@@ -76,11 +76,11 @@ func (d *driver) Run(c *execdriver.Command, pipes *execdriver.Pipes, startCallba
 	})
 
 	if err := d.generateEnvConfig(c); err != nil {
-		return nil, err
+		return execdriver.ExitStatus{-1, false}, err
 	}
 	configPath, err := d.generateLXCConfig(c)
 	if err != nil {
-		return nil, err
+		return execdriver.ExitStatus{-1, false}, err
 	}
 	params := []string{
 		"lxc-start",
@@ -155,11 +155,11 @@ func (d *driver) Run(c *execdriver.Command, pipes *execdriver.Pipes, startCallba
 	c.ProcessConfig.Args = append([]string{name}, arg...)
 
 	if err := nodes.CreateDeviceNodes(c.Rootfs, c.AutoCreatedDevices); err != nil {
-		return nil, err
+		return execdriver.ExitStatus{-1, false}, err
 	}
 
 	if err := c.ProcessConfig.Start(); err != nil {
-		return nil, err
+		return execdriver.ExitStatus{-1, false}, err
 	}
 
 	var (
@@ -183,7 +183,7 @@ func (d *driver) Run(c *execdriver.Command, pipes *execdriver.Pipes, startCallba
 			c.ProcessConfig.Process.Kill()
 			c.ProcessConfig.Wait()
 		}
-		return nil, err
+		return execdriver.ExitStatus{-1, false}, err
 	}
 
 	c.ContainerPid = pid
@@ -194,7 +194,7 @@ func (d *driver) Run(c *execdriver.Command, pipes *execdriver.Pipes, startCallba
 
 	<-waitLock
 
-	return &execdriver.ExitStatus{getExitCode(c), false}, waitErr
+	return execdriver.ExitStatus{getExitCode(c), false}, waitErr
 }
 
 /// Return the exit code of the process

+ 15 - 19
daemon/execdriver/native/driver.go

@@ -70,11 +70,11 @@ type execOutput struct {
 	err      error
 }
 
-func (d *driver) Run(c *execdriver.Command, pipes *execdriver.Pipes, startCallback execdriver.StartCallback) (*execdriver.ExitStatus, error) {
+func (d *driver) Run(c *execdriver.Command, pipes *execdriver.Pipes, startCallback execdriver.StartCallback) (execdriver.ExitStatus, error) {
 	// take the Command and populate the libcontainer.Config from it
 	container, err := d.createContainer(c)
 	if err != nil {
-		return nil, err
+		return execdriver.ExitStatus{-1, false}, err
 	}
 
 	var term execdriver.Terminal
@@ -85,7 +85,7 @@ func (d *driver) Run(c *execdriver.Command, pipes *execdriver.Pipes, startCallba
 		term, err = execdriver.NewStdConsole(&c.ProcessConfig, pipes)
 	}
 	if err != nil {
-		return nil, err
+		return execdriver.ExitStatus{-1, false}, err
 	}
 	c.ProcessConfig.Terminal = term
 
@@ -102,16 +102,16 @@ func (d *driver) Run(c *execdriver.Command, pipes *execdriver.Pipes, startCallba
 	)
 
 	if err := d.createContainerRoot(c.ID); err != nil {
-		return nil, err
+		return execdriver.ExitStatus{-1, false}, err
 	}
 	defer d.cleanContainer(c.ID)
 
 	if err := d.writeContainerFile(container, c.ID); err != nil {
-		return nil, err
+		return execdriver.ExitStatus{-1, false}, err
 	}
 
-	execOutputChan := make(chan execOutput, 0)
-	waitForStart := make(chan struct{}, 0)
+	execOutputChan := make(chan execOutput, 1)
+	waitForStart := make(chan struct{})
 
 	go func() {
 		exitCode, err := namespaces.Exec(container, c.ProcessConfig.Stdin, c.ProcessConfig.Stdout, c.ProcessConfig.Stderr, c.ProcessConfig.Console, dataPath, args, func(container *libcontainer.Config, console, dataPath, init string, child *os.File, args []string) *exec.Cmd {
@@ -146,26 +146,22 @@ func (d *driver) Run(c *execdriver.Command, pipes *execdriver.Pipes, startCallba
 
 	select {
 	case execOutput := <-execOutputChan:
-		return &execdriver.ExitStatus{execOutput.exitCode, false}, execOutput.err
+		return execdriver.ExitStatus{execOutput.exitCode, false}, execOutput.err
 	case <-waitForStart:
 		break
 	}
 
 	oomKill := false
-	go func() {
-		oomKillNotification, err := d.notifyOnOOM(container)
-		if err == nil {
-			if _, ok := <-oomKillNotification; ok {
-				oomKill = true
-			}
-		} else {
-			log.Infof("WARNING: Your kernel does not support OOM notifications: %s", err)
-		}
-	}()
+	oomKillNotification, err := d.notifyOnOOM(container)
+	if err == nil {
+		_, oomKill = <-oomKillNotification
+	} else {
+		log.Warnf("WARNING: Your kernel does not support OOM notifications: %s", err)
+	}
 	// wait for the container to exit.
 	execOutput := <-execOutputChan
 
-	return &execdriver.ExitStatus{execOutput.exitCode, oomKill}, execOutput.err
+	return execdriver.ExitStatus{execOutput.exitCode, oomKill}, execOutput.err
 }
 
 func (d *driver) Kill(p *execdriver.Command, sig int) error {

+ 6 - 6
daemon/monitor.go

@@ -100,7 +100,7 @@ func (m *containerMonitor) Close() error {
 func (m *containerMonitor) Start() error {
 	var (
 		err        error
-		exitStatus *execdriver.ExitStatus
+		exitStatus execdriver.ExitStatus
 		// this variable indicates where we in execution flow:
 		// before Run or after
 		afterRun bool
@@ -110,7 +110,7 @@ func (m *containerMonitor) Start() error {
 	defer func() {
 		if afterRun {
 			m.container.Lock()
-			m.container.setStopped(exitStatus)
+			m.container.setStopped(&exitStatus)
 			defer m.container.Unlock()
 		}
 		m.Close()
@@ -138,7 +138,7 @@ func (m *containerMonitor) Start() error {
 			// if we receive an internal error from the initial start of a container then lets
 			// return it instead of entering the restart loop
 			if m.container.RestartCount == 0 {
-				m.container.ExitCode = exitStatus
+				m.container.ExitCode = -1
 				m.resetContainer(false)
 
 				return err
@@ -153,7 +153,7 @@ func (m *containerMonitor) Start() error {
 		m.resetMonitor(err == nil && exitStatus.ExitCode == 0)
 
 		if m.shouldRestart(exitStatus.ExitCode) {
-			m.container.SetRestarting(exitStatus)
+			m.container.SetRestarting(&exitStatus)
 			m.container.LogEvent("die")
 			m.resetContainer(true)
 
@@ -164,12 +164,12 @@ func (m *containerMonitor) Start() error {
 			// we need to check this before reentering the loop because the waitForNextRestart could have
 			// been terminated by a request from a user
 			if m.shouldStop {
-				m.container.ExitCode = exitStatus
+				m.container.ExitCode = exitStatus.ExitCode
 				return err
 			}
 			continue
 		}
-		m.container.ExitCode = exitStatus
+		m.container.ExitCode = exitStatus.ExitCode
 		m.container.LogEvent("die")
 		m.resetContainer(true)
 		return err

+ 4 - 13
daemon/state.go

@@ -31,16 +31,12 @@ func NewState() *State {
 
 // String returns a human-readable description of the state
 func (s *State) String() string {
-	oomInfo := ""
-	if s.OOMKilled {
-		oomInfo = "possibly due to lack of memory"
-	}
 	if s.Running {
 		if s.Paused {
 			return fmt.Sprintf("Up %s (Paused)", units.HumanDuration(time.Now().UTC().Sub(s.StartedAt)))
 		}
 		if s.Restarting {
-			return fmt.Sprintf("Restarting (%d) %s ago %s", s.ExitCode, units.HumanDuration(time.Now().UTC().Sub(s.FinishedAt)), oomInfo)
+			return fmt.Sprintf("Restarting (%d) %s ago", s.ExitCode, units.HumanDuration(time.Now().UTC().Sub(s.FinishedAt)))
 		}
 
 		return fmt.Sprintf("Up %s", units.HumanDuration(time.Now().UTC().Sub(s.StartedAt)))
@@ -50,7 +46,7 @@ func (s *State) String() string {
 		return ""
 	}
 
-	return fmt.Sprintf("Exited (%d) %s ago %s", s.ExitCode, units.HumanDuration(time.Now().UTC().Sub(s.FinishedAt)), oomInfo)
+	return fmt.Sprintf("Exited (%d) %s ago", s.ExitCode, units.HumanDuration(time.Now().UTC().Sub(s.FinishedAt)))
 }
 
 // StateString returns a single string to describe state
@@ -167,10 +163,7 @@ func (s *State) setStopped(exitStatus *execdriver.ExitStatus) {
 	s.Pid = 0
 	s.FinishedAt = time.Now().UTC()
 	s.ExitCode = exitStatus.ExitCode
-	s.OOMKilled = false
-	if exitStatus.OOMKilled {
-		s.OOMKilled = true
-	}
+	s.OOMKilled = exitStatus.OOMKilled
 	close(s.waitChan) // fire waiters for stop
 	s.waitChan = make(chan struct{})
 }
@@ -186,9 +179,7 @@ func (s *State) SetRestarting(exitStatus *execdriver.ExitStatus) {
 	s.Pid = 0
 	s.FinishedAt = time.Now().UTC()
 	s.ExitCode = exitStatus.ExitCode
-	if exitStatus.OOMKilled {
-		s.OOMKilled = true
-	}
+	s.OOMKilled = exitStatus.OOMKilled
 	close(s.waitChan) // fire waiters for stop
 	s.waitChan = make(chan struct{})
 	s.Unlock()