From 04c9f86bdcf9f42deb09df76922a8c61205721a2 Mon Sep 17 00:00:00 2001 From: Michael Crosby Date: Wed, 8 Jul 2015 10:55:42 -0700 Subject: [PATCH 1/3] Remove exec config from container after exit This removes the exec config from the container after the command exits so that dead exec commands are not displayed in the container inspect. The commands are still kept on the daemon so that when you inspect the exec command, not the container, you are still able to get it's exit status. This also changes the ProcessConfig to a pointer. Signed-off-by: Michael Crosby --- daemon/container.go | 6 +++--- daemon/exec.go | 7 +++---- daemon/inspect.go | 1 - 3 files changed, 6 insertions(+), 8 deletions(-) diff --git a/daemon/container.go b/daemon/container.go index a06033c89..129097226 100644 --- a/daemon/container.go +++ b/daemon/container.go @@ -835,13 +835,11 @@ func (container *Container) monitorExec(execConfig *execConfig, callback execdri err error exitCode int ) - pipes := execdriver.NewPipes(execConfig.StreamConfig.stdin, execConfig.StreamConfig.stdout, execConfig.StreamConfig.stderr, execConfig.OpenStdin) exitCode, err = container.daemon.Exec(container, execConfig, pipes, callback) if err != nil { logrus.Errorf("Error running command in existing container %s: %s", container.ID, err) } - logrus.Debugf("Exec task in container %s exited with code %d", container.ID, exitCode) if execConfig.OpenStdin { if err := execConfig.StreamConfig.stdin.Close(); err != nil { @@ -859,7 +857,9 @@ func (container *Container) monitorExec(execConfig *execConfig, callback execdri logrus.Errorf("Error closing terminal while running in container %s: %s", container.ID, err) } } - + // remove the exec command from the container's store only and not the + // daemon's store so that the exec command can be inspected. + container.execCommands.Delete(execConfig.ID) return err } diff --git a/daemon/exec.go b/daemon/exec.go index 71f042919..e1cfcd7f6 100644 --- a/daemon/exec.go +++ b/daemon/exec.go @@ -21,7 +21,7 @@ type execConfig struct { ID string Running bool ExitCode int - ProcessConfig execdriver.ProcessConfig + ProcessConfig *execdriver.ProcessConfig StreamConfig OpenStdin bool OpenStderr bool @@ -128,7 +128,7 @@ func (d *Daemon) ContainerExecCreate(config *runconfig.ExecConfig) (string, erro user = container.Config.User } - processConfig := execdriver.ProcessConfig{ + processConfig := &execdriver.ProcessConfig{ Tty: config.Tty, Entrypoint: entrypoint, Arguments: args, @@ -221,7 +221,6 @@ func (d *Daemon) ContainerExecStart(execName string, stdin io.ReadCloser, stdout execErr <- fmt.Errorf("Cannot run exec command %s in container %s: %s", execName, container.ID, err) } }() - select { case err := <-attachErr: if err != nil { @@ -236,7 +235,7 @@ func (d *Daemon) ContainerExecStart(execName string, stdin io.ReadCloser, stdout } func (d *Daemon) Exec(c *Container, execConfig *execConfig, pipes *execdriver.Pipes, startCallback execdriver.StartCallback) (int, error) { - exitStatus, err := d.execDriver.Exec(c.command, &execConfig.ProcessConfig, pipes, startCallback) + exitStatus, err := d.execDriver.Exec(c.command, execConfig.ProcessConfig, pipes, startCallback) // On err, make sure we don't leave ExitCode at zero if err != nil && exitStatus == 0 { diff --git a/daemon/inspect.go b/daemon/inspect.go index 73b394ca2..7e3edaef9 100644 --- a/daemon/inspect.go +++ b/daemon/inspect.go @@ -124,6 +124,5 @@ func (daemon *Daemon) ContainerExecInspect(id string) (*execConfig, error) { if err != nil { return nil, err } - return eConfig, nil } From 5f017bba48e5c763157e1b35a5edea64cc41fc6a Mon Sep 17 00:00:00 2001 From: Michael Crosby Date: Wed, 8 Jul 2015 11:13:47 -0700 Subject: [PATCH 2/3] Add GC loop to clean exec command refs on daemon This adds an event loop for running a GC cleanup for exec command references that are on the daemon. These cannot be cleaned up immediately because processes may need to get the exit status of the exec command but it should not grow out of bounds. The loop is set to a default 5 minute interval to perform cleanup. It should be safe to perform this cleanup because unless the clients are remembering the exec id of the process they launched they can query for the status and see that it has exited. If they don't save the exec id they will have to do an inspect on the container for all exec instances and anything that is not live inside that container will not be returned in the container inspect. Signed-off-by: Michael Crosby --- daemon/daemon.go | 1 + daemon/exec.go | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 33 insertions(+) diff --git a/daemon/daemon.go b/daemon/daemon.go index 549a1c188..6e4d7ba20 100644 --- a/daemon/daemon.go +++ b/daemon/daemon.go @@ -731,6 +731,7 @@ func NewDaemon(config *Config, registryService *registry.Service) (daemon *Daemo d.RegistryService = registryService d.EventsService = eventsService d.root = config.Root + go d.execCommandGC() if err := d.restore(); err != nil { return nil, err diff --git a/daemon/exec.go b/daemon/exec.go index e1cfcd7f6..8ee7c1787 100644 --- a/daemon/exec.go +++ b/daemon/exec.go @@ -6,6 +6,7 @@ import ( "io/ioutil" "strings" "sync" + "time" "github.com/Sirupsen/logrus" "github.com/docker/docker/daemon/execdriver" @@ -247,3 +248,34 @@ func (d *Daemon) Exec(c *Container, execConfig *execConfig, pipes *execdriver.Pi return exitStatus, err } + +// execCommandGC runs a ticker to clean up the daemon references +// of exec configs that are no longer part of the container. +func (d *Daemon) execCommandGC() { + for range time.Tick(5 * time.Minute) { + var ( + cleaned int + liveExecCommands = d.containerExecIds() + ids = d.execCommands.List() + ) + for _, id := range ids { + if _, exists := liveExecCommands[id]; !exists { + cleaned++ + d.execCommands.Delete(id) + } + } + logrus.Debugf("clean %d unused exec commands", cleaned) + } +} + +// containerExecIds returns a list of all the current exec ids that are in use +// and running inside a container. +func (d *Daemon) containerExecIds() map[string]struct{} { + ids := map[string]struct{}{} + for _, c := range d.containers.List() { + for _, id := range c.execCommands.List() { + ids[id] = struct{}{} + } + } + return ids +} From 34ab8c432691934745d66ee94ff4aec1120518e0 Mon Sep 17 00:00:00 2001 From: Michael Crosby Date: Thu, 9 Jul 2015 14:51:10 -0700 Subject: [PATCH 3/3] Use mark and sweep for exec command removal This takes the final removal for exec commands in two steps. The first GC tick will mark the exec commands for removal and then the second tick will remove the config from the daemon. Signed-off-by: Michael Crosby --- daemon/exec.go | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/daemon/exec.go b/daemon/exec.go index 8ee7c1787..82e267bc2 100644 --- a/daemon/exec.go +++ b/daemon/exec.go @@ -28,6 +28,7 @@ type execConfig struct { OpenStderr bool OpenStdout bool Container *Container + canRemove bool } type execStore struct { @@ -256,12 +257,15 @@ func (d *Daemon) execCommandGC() { var ( cleaned int liveExecCommands = d.containerExecIds() - ids = d.execCommands.List() ) - for _, id := range ids { - if _, exists := liveExecCommands[id]; !exists { + for id, config := range d.execCommands.s { + if config.canRemove { cleaned++ d.execCommands.Delete(id) + } else { + if _, exists := liveExecCommands[id]; !exists { + config.canRemove = true + } } } logrus.Debugf("clean %d unused exec commands", cleaned)