core: SIGUSR1 to reload config; some code cleanup

caddy/caddy.go

@@ -105,58 +105,15 @@ func Start(cdyfile Input) error {
 	caddyfile = cdyfile
 	caddyfileMu.Unlock()
 
-	groupings, err := Load(path.Base(caddyfile.Path()), bytes.NewReader(caddyfile.Body()))
+	groupings, err := Load(path.Base(cdyfile.Path()), bytes.NewReader(cdyfile.Body()))
+	if err != nil {
+		return err
+	}
 
 	// Start each server with its one or more configurations
-	for i, group := range groupings {
+	err = startServers(groupings)
-		s, err := server.New(group.BindAddr.String(), group.Configs)
 	if err != nil {
-			log.Fatal(err)
+		return err
-		}
-		s.HTTP2 = HTTP2 // TODO: This setting is temporary
-
-		var ln server.ListenerFile
-		if isRestart() {
-			// Look up this server's listener in the map of inherited file descriptors;
-			// if we don't have one, we must make a new one.
-			if fdIndex, ok := loadedGob.ListenerFds[s.Addr]; ok {
-				file := os.NewFile(fdIndex, "")
-
-				fln, err := net.FileListener(file)
-				if err != nil {
-					log.Fatal("FILE LISTENER:", err)
-				}
-
-				ln, ok = fln.(server.ListenerFile)
-				if !ok {
-					log.Fatal("Listener was not a ListenerFile")
-				}
-
-				delete(loadedGob.ListenerFds, s.Addr) // mark it as used
-			}
-		}
-
-		wg.Add(1)
-		go func(s *server.Server, i int, ln server.ListenerFile) {
-			defer wg.Done()
-			if ln == nil {
-				err := s.ListenAndServe()
-				// "use of closed network connection" is normal if doing graceful shutdown...
-				if !strings.Contains(err.Error(), "use of closed network connection") {
-					// But an error at initial startup must be fatal
-					log.Fatal(err)
-				}
-			} else {
-				err := s.Serve(ln)
-				if err != nil {
-					log.Println(err)
-				}
-			}
-		}(s, i, ln)
-
-		serversMu.Lock()
-		servers = append(servers, s)
-		serversMu.Unlock()
 	}
 
 	// Close remaining file descriptors we may have inherited that we don't need
@@ -191,7 +148,7 @@ func Start(cdyfile Input) error {
 		}
 	}
 
-	// Tell parent we're A-OK
+	// Tell parent process that we got this
 	if isRestart() {
 		file := os.NewFile(3, "")
 		file.Write([]byte("success"))
@@ -201,6 +158,64 @@ func Start(cdyfile Input) error {
 	return nil
 }
 
+// startServers starts all the servers in groupings,
+// taking into account whether or not this process is
+// a child from a graceful restart or not.
+func startServers(groupings Group) error {
+	for i, group := range groupings {
+		s, err := server.New(group.BindAddr.String(), group.Configs)
+		if err != nil {
+			log.Fatal(err)
+		}
+		s.HTTP2 = HTTP2 // TODO: This setting is temporary
+
+		var ln server.ListenerFile
+		if isRestart() {
+			// Look up this server's listener in the map of inherited file descriptors;
+			// if we don't have one, we must make a new one.
+			if fdIndex, ok := loadedGob.ListenerFds[s.Addr]; ok {
+				file := os.NewFile(fdIndex, "")
+
+				fln, err := net.FileListener(file)
+				if err != nil {
+					log.Fatal(err)
+				}
+
+				ln, ok = fln.(server.ListenerFile)
+				if !ok {
+					log.Fatal("listener was not a ListenerFile")
+				}
+
+				delete(loadedGob.ListenerFds, s.Addr) // mark it as used
+			}
+		}
+
+		wg.Add(1)
+		go func(s *server.Server, i int, ln server.ListenerFile) {
+			defer wg.Done()
+			if ln != nil {
+				err = s.Serve(ln)
+			} else {
+				err = s.ListenAndServe()
+			}
+
+			// "use of closed network connection" is normal if doing graceful shutdown...
+			if err != nil && !strings.Contains(err.Error(), "use of closed network connection") {
+				if isRestart() {
+					log.Fatal(err)
+				} else {
+					log.Println(err)
+				}
+			}
+		}(s, i, ln)
+
+		serversMu.Lock()
+		servers = append(servers, s)
+		serversMu.Unlock()
+	}
+	return nil
+}
+
 // isLocalhost returns true if the string looks explicitly like a localhost address.
 func isLocalhost(s string) bool {
 	return s == "localhost" || s == "::1" || strings.HasPrefix(s, "127.")
@@ -302,9 +317,9 @@ func Restart(newCaddyfile Input) error {
 		Env:   os.Environ(),
 		Files: fds,
 	}
-	pid, err := syscall.ForkExec(os.Args[0], os.Args, execSpec)
+	_, err = syscall.ForkExec(os.Args[0], os.Args, execSpec)
 	if err != nil {
-		log.Println("FORK ERR:", err, pid)
+		return err
 	}
 
 	// Feed it the Caddyfile
@@ -447,15 +462,15 @@ func init() {
 
 	// Trap signals
 	go func() {
-		// Wait for signal
+		shutdown, reload := make(chan os.Signal, 1), make(chan os.Signal, 1)
-		interrupt := make(chan os.Signal, 1)
+		signal.Notify(shutdown, os.Interrupt, os.Kill) // quit the process
-		signal.Notify(interrupt, os.Interrupt, os.Kill)
+		signal.Notify(reload, syscall.SIGUSR1)         // reload configuration
-		<-interrupt
 
-		// TODO: A signal just for graceful restart (reload config) - maybe SIGUSR1
+		for {
-
+			select {
-		// Run shutdown callbacks
+			case <-shutdown:
 				var exitCode int
+
 				serversMu.Lock()
 				errs := server.ShutdownCallbacks(servers)
 				serversMu.Unlock()
@@ -466,5 +481,13 @@ func init() {
 					exitCode = 1
 				}
 				os.Exit(exitCode)
+
+			case <-reload:
+				err := Restart(nil)
+				if err != nil {
+					log.Println(err)
+				}
+			}
+		}
 	}()
 }

main.go

@@ -10,7 +10,6 @@ import (
 	"runtime"
 	"strconv"
 	"strings"
-	"time"
 
 	"github.com/mholt/caddy/caddy"
 	"github.com/mholt/caddy/caddy/letsencrypt"
@@ -79,15 +78,6 @@ func main() {
 		log.Fatal(err)
 	}
 
-	// TODO: Temporary; testing restart
-	//if os.Getenv("CADDY_RESTART") != "true" {
-	go func() {
-		time.Sleep(5 * time.Second)
-		fmt.Println("restarting")
-		log.Println("RESTART ERR:", caddy.Restart(nil))
-	}()
-	//}
-
 	// Twiddle your thumbs
 	caddy.Wait()
 }

server/server.go

@@ -260,9 +260,6 @@ func (s *Server) ListenerFd() uintptr {
 // defined in the Host header so that the correct virtualhost
 // (configuration and middleware stack) will handle the request.
 func (s *Server) ServeHTTP(w http.ResponseWriter, r *http.Request) {
-	fmt.Println("Sleeping")
-	time.Sleep(5 * time.Second) // TODO: Temporarily making requests hang so we can test graceful restart
-	fmt.Println("Unblocking")
 	defer func() {
 		// In case the user doesn't enable error middleware, we still
 		// need to make sure that we stay alive up here