Cleanup: refactor shutdown and signal handling facility

This disentangles the following functions, which were previously all mixed together:

* 1) Waiting for jobs to terminate when shutting down
* 2) Handling signals in the Docker daemon
* 3) Per-subsystem cleanup handlers
* 4) pidfile management

Responsibilities are dispatched as follows:

* Signal traps are set in `main`, and trigger `engine.Shutdown`
* `engine.Shutdown` coordinates cleanup by waiting for jobs to complete, and calling shutdown handlers
* To perform cleanup at shutdown, each subsystem registers handlers with `engine.OnShutdown`
* `daemon` is one subsystem, so it registers cleanup via `engine.OnShutdown`.
* `daemon` owns the pidfile, which is used to lock access to `/var/lib/docker`. Part of its cleanup is to remove the pidfile.

Signed-off-by: Solomon Hykes <solomon@docker.com>

daemon/daemon.go

@@ -667,6 +667,20 @@ func NewDaemon(config *daemonconfig.Config, eng *engine.Engine) (*Daemon, error)
 }
 
 func NewDaemonFromDirectory(config *daemonconfig.Config, eng *engine.Engine) (*Daemon, error) {
+	// Claim the pidfile first, to avoid any and all unexpected race conditions.
+	// Some of the init doesn't need a pidfile lock - but let's not try to be smart.
+	if config.Pidfile != "" {
+		if err := utils.CreatePidFile(config.Pidfile); err != nil {
+			return nil, err
+		}
+		eng.OnShutdown(func() {
+			// Always release the pidfile last, just in case
+			utils.RemovePidFile(config.Pidfile)
+		})
+	}
+
+	// Check that the system is supported and we have sufficient privileges
+	// FIXME: return errors instead of calling Fatal
 	if runtime.GOOS != "linux" {
 		log.Fatalf("The Docker daemon is only supported on linux")
 	}
@@ -819,13 +833,32 @@ func NewDaemonFromDirectory(config *daemonconfig.Config, eng *engine.Engine) (*D
 		eng:            eng,
 		Sockets:        config.Sockets,
 	}
-
 	if err := daemon.checkLocaldns(); err != nil {
 		return nil, err
 	}
 	if err := daemon.restore(); err != nil {
 		return nil, err
 	}
+	// Setup shutdown handlers
+	// FIXME: can these shutdown handlers be registered closer to their source?
+	eng.OnShutdown(func() {
+		// FIXME: if these cleanup steps can be called concurrently, register
+		// them as separate handlers to speed up total shutdown time
+		// FIXME: use engine logging instead of utils.Errorf
+		if err := daemon.shutdown(); err != nil {
+			utils.Errorf("daemon.shutdown(): %s", err)
+		}
+		if err := portallocator.ReleaseAll(); err != nil {
+			utils.Errorf("portallocator.ReleaseAll(): %s", err)
+		}
+		if err := daemon.driver.Cleanup(); err != nil {
+			utils.Errorf("daemon.driver.Cleanup(): %s", err.Error())
+		}
+		if err := daemon.containerGraph.Close(); err != nil {
+			utils.Errorf("daemon.containerGraph.Close(): %s", err.Error())
+		}
+	})
+
 	return daemon, nil
 }
 
@@ -853,30 +886,6 @@ func (daemon *Daemon) shutdown() error {
 	return nil
 }
 
-func (daemon *Daemon) Close() error {
-	errorsStrings := []string{}
-	if err := daemon.shutdown(); err != nil {
-		utils.Errorf("daemon.shutdown(): %s", err)
-		errorsStrings = append(errorsStrings, err.Error())
-	}
-	if err := portallocator.ReleaseAll(); err != nil {
-		utils.Errorf("portallocator.ReleaseAll(): %s", err)
-		errorsStrings = append(errorsStrings, err.Error())
-	}
-	if err := daemon.driver.Cleanup(); err != nil {
-		utils.Errorf("daemon.driver.Cleanup(): %s", err.Error())
-		errorsStrings = append(errorsStrings, err.Error())
-	}
-	if err := daemon.containerGraph.Close(); err != nil {
-		utils.Errorf("daemon.containerGraph.Close(): %s", err.Error())
-		errorsStrings = append(errorsStrings, err.Error())
-	}
-	if len(errorsStrings) > 0 {
-		return fmt.Errorf("%s", strings.Join(errorsStrings, ", "))
-	}
-	return nil
-}
-
 func (daemon *Daemon) Mount(container *Container) error {
 	dir, err := daemon.driver.Get(container.ID, container.GetMountLabel())
 	if err != nil {
@@ -967,6 +976,8 @@ func (daemon *Daemon) Kill(c *Container, sig int) error {
 // from the content root, including images, volumes and
 // container filesystems.
 // Again: this will remove your entire docker daemon!
+// FIXME: this is deprecated, and only used in legacy
+// tests. Please remove.
 func (daemon *Daemon) Nuke() error {
 	var wg sync.WaitGroup
 	for _, container := range daemon.List() {
@@ -977,7 +988,6 @@ func (daemon *Daemon) Nuke() error {
 		}(container)
 	}
 	wg.Wait()
-	daemon.Close()
 
 	return os.RemoveAll(daemon.config.Root)
 }

docker/daemon.go

@@ -10,6 +10,7 @@ import (
 	"github.com/docker/docker/dockerversion"
 	"github.com/docker/docker/engine"
 	flag "github.com/docker/docker/pkg/mflag"
+	"github.com/docker/docker/pkg/signal"
 	"github.com/docker/docker/sysinit"
 )
 
@@ -39,6 +40,7 @@ func mainDaemon() {
 	}
 
 	eng := engine.New()
+	signal.Trap(eng.Shutdown)
 	// Load builtins
 	if err := builtins.Register(eng); err != nil {
 		log.Fatal(err)

server/init.go

@@ -10,7 +10,6 @@ import (
 	"github.com/docker/docker/daemon"
 	"github.com/docker/docker/daemonconfig"
 	"github.com/docker/docker/engine"
-	"github.com/docker/docker/pkg/signal"
 	"github.com/docker/docker/utils"
 )
 
@@ -41,15 +40,11 @@ func InitPidfile(job *engine.Job) engine.Status {
 // The signals SIGINT, SIGQUIT and SIGTERM are intercepted for cleanup.
 func InitServer(job *engine.Job) engine.Status {
 	job.Logf("Creating server")
-	srv, err := NewServer(job.Eng, daemonconfig.ConfigFromJob(job))
+	cfg := daemonconfig.ConfigFromJob(job)
+	srv, err := NewServer(job.Eng, cfg)
 	if err != nil {
 		return job.Error(err)
 	}
-	job.Logf("Setting up signal traps")
-	signal.Trap(func() {
-		utils.RemovePidFile(srv.daemon.Config().Pidfile)
-		srv.Close()
-	})
 	job.Eng.Hack_SetGlobalVar("httpapi.server", srv)
 	job.Eng.Hack_SetGlobalVar("httpapi.daemon", srv.daemon)
 

server/server.go

@@ -23,7 +23,6 @@ package server
 
 import (
 	"sync"
-	"time"
 
 	"github.com/docker/docker/daemon"
 	"github.com/docker/docker/engine"
@@ -42,27 +41,6 @@ func (srv *Server) IsRunning() bool {
 	return srv.running
 }
 
-func (srv *Server) Close() error {
-	if srv == nil {
-		return nil
-	}
-	srv.SetRunning(false)
-	done := make(chan struct{})
-	go func() {
-		srv.tasks.Wait()
-		close(done)
-	}()
-	select {
-	// Waiting server jobs for 15 seconds, shutdown immediately after that time
-	case <-time.After(time.Second * 15):
-	case <-done:
-	}
-	if srv.daemon == nil {
-		return nil
-	}
-	return srv.daemon.Close()
-}
-
 type Server struct {
 	sync.RWMutex
 	daemon      *daemon.Daemon