Split coredhcp.go into server/

coredhcp.go is not the entrypoint of main (that's cmds/coredhcp) and
contains many tightly-coupled things that could be separated.

This separates the logic into:
 * server/serve.go handles setting up connections
 * server/handle.go handles parsing and entry into handlers (in
   preparation of a larger rewrite)
 * plugins/plugin.go gets plugin loading (and doesn't need to access the
   Server struct either)

Signed-off-by: Anatole Denis <anatole@unverle.fr>

cmds/coredhcp/main.go

@@ -10,9 +10,10 @@ import (
 	"io/ioutil"
 	"time"
 
-	"github.com/coredhcp/coredhcp"
 	"github.com/coredhcp/coredhcp/config"
 	"github.com/coredhcp/coredhcp/logger"
+	"github.com/coredhcp/coredhcp/server"
+
 	"github.com/coredhcp/coredhcp/plugins"
 	"github.com/coredhcp/coredhcp/plugins/dns"
 	"github.com/coredhcp/coredhcp/plugins/file"
@@ -88,12 +89,12 @@ func main() {
 	}
 
 	// start server
-	server := coredhcp.NewServer(config)
-	if err := server.Start(); err != nil {
-		log.Fatalf("Failed to start server: %v", err)
+	srv := server.NewServer(config)
+	if err := srv.Start(); err != nil {
+		log.Fatal(err)
 	}
-	if err := server.Wait(); err != nil {
-		log.Error(err)
+	if err := srv.Wait(); err != nil {
+		log.Print(err)
 	}
 	time.Sleep(time.Second)
 }

plugins/plugin.go

@@ -7,6 +7,7 @@ package plugins
 import (
 	"errors"
 
+	"github.com/coredhcp/coredhcp/config"
 	"github.com/coredhcp/coredhcp/handler"
 	"github.com/coredhcp/coredhcp/logger"
 )
@@ -45,3 +46,69 @@ func RegisterPlugin(plugin *Plugin) error {
 	RegisteredPlugins[plugin.Name] = plugin
 	return nil
 }
+
+// LoadPlugins reads a Config object and loads the plugins as specified in the
+// `plugins` section, in order. For a plugin to be available, it must have been
+// previously registered with plugins.RegisterPlugin. This is normally done at
+// plugin import time.
+// This function returns the list of loaded v6 plugins, the list of loaded v4
+// plugins, and an error if any.
+func LoadPlugins(conf *config.Config) ([]handler.Handler4, []handler.Handler6, error) {
+	log.Print("Loading plugins...")
+	handlers4 := make([]handler.Handler4, 0)
+	handlers6 := make([]handler.Handler6, 0)
+
+	if conf.Server6 == nil && conf.Server4 == nil {
+		return nil, nil, errors.New("no configuration found for either DHCPv6 or DHCPv4")
+	}
+
+	// now load the plugins. We need to call its setup function with
+	// the arguments extracted above. The setup function is mapped in
+	// plugins.RegisteredPlugins .
+
+	// Load DHCPv6 plugins.
+	if conf.Server6 != nil {
+		for _, pluginConf := range conf.Server6.Plugins {
+			if plugin, ok := RegisteredPlugins[pluginConf.Name]; ok {
+				log.Printf("DHCPv6: loading plugin `%s`", pluginConf.Name)
+				if plugin.Setup6 == nil {
+					log.Warningf("DHCPv6: plugin `%s` has no setup function for DHCPv6", pluginConf.Name)
+					continue
+				}
+				h6, err := plugin.Setup6(pluginConf.Args...)
+				if err != nil {
+					return nil, nil, err
+				} else if h6 == nil {
+					return nil, nil, config.ConfigErrorFromString("no DHCPv6 handler for plugin %s", pluginConf.Name)
+				}
+				handlers6 = append(handlers6, h6)
+			} else {
+				return nil, nil, config.ConfigErrorFromString("DHCPv6: unknown plugin `%s`", pluginConf.Name)
+			}
+		}
+	}
+	// Load DHCPv4 plugins. Yes, duplicated code, there's not really much that
+	// can be deduplicated here.
+	if conf.Server4 != nil {
+		for _, pluginConf := range conf.Server4.Plugins {
+			if plugin, ok := RegisteredPlugins[pluginConf.Name]; ok {
+				log.Printf("DHCPv4: loading plugin `%s`", pluginConf.Name)
+				if plugin.Setup4 == nil {
+					log.Warningf("DHCPv4: plugin `%s` has no setup function for DHCPv4", pluginConf.Name)
+					continue
+				}
+				h4, err := plugin.Setup4(pluginConf.Args...)
+				if err != nil {
+					return nil, nil, err
+				} else if h4 == nil {
+					return nil, nil, config.ConfigErrorFromString("no DHCPv4 handler for plugin %s", pluginConf.Name)
+				}
+				handlers4 = append(handlers4, h4)
+			} else {
+				return nil, nil, config.ConfigErrorFromString("DHCPv4: unknown plugin `%s`", pluginConf.Name)
+			}
+		}
+	}
+
+	return handlers4, handlers6, nil
+}

coredhcp.go → server/handle.go

@@ -2,25 +2,20 @@
 // This source code is licensed under the MIT license found in the
 // LICENSE file in the root directory of this source tree.
 
-package coredhcp
+package server
 
 import (
-	"errors"
 	"fmt"
 	"net"
 
 	"github.com/coredhcp/coredhcp/config"
 	"github.com/coredhcp/coredhcp/handler"
-	"github.com/coredhcp/coredhcp/logger"
-	"github.com/coredhcp/coredhcp/plugins"
 	"github.com/insomniacslk/dhcp/dhcpv4"
 	"github.com/insomniacslk/dhcp/dhcpv4/server4"
 	"github.com/insomniacslk/dhcp/dhcpv6"
 	"github.com/insomniacslk/dhcp/dhcpv6/server6"
 )
 
-var log = logger.GetLogger("coredhcp")
-
 // Server is a CoreDHCP server structure that holds information about
 // DHCPv6 and DHCPv4 servers, and their respective handlers.
 type Server struct {
@@ -32,77 +27,6 @@ type Server struct {
 	errors    chan error
 }
 
-// LoadPlugins reads a Config object and loads the plugins as specified in the
-// `plugins` section, in order. For a plugin to be available, it must have been
-// previously registered with plugins.RegisterPlugin. This is normally done at
-// plugin import time.
-// This function returns the list of loaded v6 plugins, the list of loaded v4
-// plugins, and an error if any.
-func (s *Server) LoadPlugins(conf *config.Config) ([]*plugins.Plugin, []*plugins.Plugin, error) {
-	log.Print("Loading plugins...")
-	loadedPlugins6 := make([]*plugins.Plugin, 0)
-	loadedPlugins4 := make([]*plugins.Plugin, 0)
-
-	if conf.Server6 == nil && conf.Server4 == nil {
-		return nil, nil, errors.New("no configuration found for either DHCPv6 or DHCPv4")
-	}
-
-	// now load the plugins. We need to call its setup function with
-	// the arguments extracted above. The setup function is mapped in
-	// plugins.RegisteredPlugins .
-
-	// Load DHCPv6 plugins.
-	if conf.Server6 != nil {
-		for _, pluginConf := range conf.Server6.Plugins {
-			if plugin, ok := plugins.RegisteredPlugins[pluginConf.Name]; ok {
-				log.Printf("DHCPv6: loading plugin `%s`", pluginConf.Name)
-				if plugin.Setup6 == nil {
-					log.Warningf("DHCPv6: plugin `%s` has no setup function for DHCPv6", pluginConf.Name)
-					continue
-				}
-				h6, err := plugin.Setup6(pluginConf.Args...)
-				if err != nil {
-					return nil, nil, err
-				}
-				loadedPlugins6 = append(loadedPlugins6, plugin)
-				if h6 == nil {
-					return nil, nil, config.ConfigErrorFromString("no DHCPv6 handler for plugin %s", pluginConf.Name)
-				}
-				s.Handlers6 = append(s.Handlers6, h6)
-			} else {
-				return nil, nil, config.ConfigErrorFromString("DHCPv6: unknown plugin `%s`", pluginConf.Name)
-			}
-		}
-	}
-	// Load DHCPv4 plugins. Yes, duplicated code, there's not really much that
-	// can be deduplicated here.
-	if conf.Server4 != nil {
-		for _, pluginConf := range conf.Server4.Plugins {
-			if plugin, ok := plugins.RegisteredPlugins[pluginConf.Name]; ok {
-				log.Printf("DHCPv4: loading plugin `%s`", pluginConf.Name)
-				if plugin.Setup4 == nil {
-					log.Warningf("DHCPv4: plugin `%s` has no setup function for DHCPv4", pluginConf.Name)
-					continue
-				}
-				h4, err := plugin.Setup4(pluginConf.Args...)
-				if err != nil {
-					return nil, nil, err
-				}
-				loadedPlugins4 = append(loadedPlugins4, plugin)
-				if h4 == nil {
-					return nil, nil, config.ConfigErrorFromString("no DHCPv4 handler for plugin %s", pluginConf.Name)
-				}
-				s.Handlers4 = append(s.Handlers4, h4)
-				//s.Handlers4 = append(s.Handlers4, h4)
-			} else {
-				return nil, nil, config.ConfigErrorFromString("DHCPv4: unknown plugin `%s`", pluginConf.Name)
-			}
-		}
-	}
-
-	return loadedPlugins6, loadedPlugins4, nil
-}
-
 // BUG(Natolumin): Servers not bound to a specific interface may send responses
 // on the wrong interface as they will use the default route.
 // See https://github.com/coredhcp/coredhcp/issues/52
@@ -233,67 +157,3 @@ func (s *Server) MainHandler4(conn net.PacketConn, _peer net.Addr, req *dhcpv4.D
 		log.Print("MainHandler4: dropping request because response is nil")
 	}
 }
-
-// Start will start the server asynchronously. See `Wait` to wait until
-// the execution ends.
-func (s *Server) Start() error {
-	_, _, err := s.LoadPlugins(s.Config)
-	if err != nil {
-		return err
-	}
-
-	// listen
-	if s.Config.Server6 != nil {
-		log.Println("Starting DHCPv6 server")
-		for _, l := range s.Config.Server6.Addresses {
-			s6, err := server6.NewServer(l.Zone, l, s.MainHandler6)
-			if err != nil {
-				return err
-			}
-			s.Servers6 = append(s.Servers6, s6)
-			log.Infof("Listen %s", l)
-			go func() {
-				s.errors <- s6.Serve()
-			}()
-		}
-	}
-
-	if s.Config.Server4 != nil {
-		log.Println("Starting DHCPv4 server")
-		for _, l := range s.Config.Server4.Addresses {
-			s4, err := server4.NewServer(l.Zone, l, s.MainHandler4)
-			if err != nil {
-				return err
-			}
-			s.Servers4 = append(s.Servers4, s4)
-			log.Infof("Listen %s", l)
-			go func() {
-				s.errors <- s4.Serve()
-			}()
-		}
-	}
-
-	return nil
-}
-
-// Wait waits until the end of the execution of the server.
-func (s *Server) Wait() error {
-	log.Print("Waiting")
-	err := <-s.errors
-	for _, s6 := range s.Servers6 {
-		if s6 != nil {
-			s6.Close()
-		}
-	}
-	for _, s4 := range s.Servers4 {
-		if s4 != nil {
-			s4.Close()
-		}
-	}
-	return err
-}
-
-// NewServer creates a Server instance with the provided configuration.
-func NewServer(config *config.Config) *Server {
-	return &Server{Config: config, errors: make(chan error, 1)}
-}

server/serve.go

@@ -0,0 +1,81 @@
+// Copyright 2018-present the CoreDHCP Authors. All rights reserved
+// This source code is licensed under the MIT license found in the
+// LICENSE file in the root directory of this source tree.
+
+package server
+
+import (
+	"github.com/coredhcp/coredhcp/config"
+	"github.com/coredhcp/coredhcp/logger"
+	"github.com/coredhcp/coredhcp/plugins"
+	"github.com/insomniacslk/dhcp/dhcpv4/server4"
+	"github.com/insomniacslk/dhcp/dhcpv6/server6"
+)
+
+var log = logger.GetLogger("server")
+
+// Start will start the server asynchronously. See `Wait` to wait until
+// the execution ends.
+func (s *Server) Start() error {
+	var err error
+
+	s.Handlers4, s.Handlers6, err = plugins.LoadPlugins(s.Config)
+	if err != nil {
+		return err
+	}
+
+	// listen
+	if s.Config.Server6 != nil {
+		log.Println("Starting DHCPv6 server")
+		for _, l := range s.Config.Server6.Addresses {
+			s6, err := server6.NewServer(l.Zone, l, s.MainHandler6)
+			if err != nil {
+				return err
+			}
+			s.Servers6 = append(s.Servers6, s6)
+			log.Infof("Listen %s", l)
+			go func() {
+				s.errors <- s6.Serve()
+			}()
+		}
+	}
+
+	if s.Config.Server4 != nil {
+		log.Println("Starting DHCPv4 server")
+		for _, l := range s.Config.Server4.Addresses {
+			s4, err := server4.NewServer(l.Zone, l, s.MainHandler4)
+			if err != nil {
+				return err
+			}
+			s.Servers4 = append(s.Servers4, s4)
+			log.Infof("Listen %s", l)
+			go func() {
+				s.errors <- s4.Serve()
+			}()
+		}
+	}
+
+	return nil
+}
+
+// Wait waits until the end of the execution of the server.
+func (s *Server) Wait() error {
+	log.Print("Waiting")
+	err := <-s.errors
+	for _, s6 := range s.Servers6 {
+		if s6 != nil {
+			s6.Close()
+		}
+	}
+	for _, s4 := range s.Servers4 {
+		if s4 != nil {
+			s4.Close()
+		}
+	}
+	return err
+}
+
+// NewServer creates a Server instance with the provided configuration.
+func NewServer(config *config.Config) *Server {
+	return &Server{Config: config, errors: make(chan error, 1)}
+}