Parse daemon configuration in daemon.Config.InstallFlags instead of main

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

daemon/config.go

@@ -1,8 +1,11 @@
 package daemon
 
 import (
-	"github.com/docker/docker/daemon/networkdriver"
 	"net"
+
+	"github.com/docker/docker/daemon/networkdriver"
+	"github.com/docker/docker/opts"
+	flag "github.com/docker/docker/pkg/mflag"
 )
 
 const (
@@ -11,7 +14,7 @@ const (
 )
 
 // Config define the configuration of a docker daemon
-//  These are the configuration settings that you pass
+// These are the configuration settings that you pass
 // to the docker daemon when you launch it with say: `docker -d -e lxc`
 // FIXME: separate runtime configuration from http api configuration
 type Config struct {
@@ -36,6 +39,31 @@ type Config struct {
 	Sockets                     []string
 }
 
+// InstallFlags adds command-line options to the top-level flag parser for
+// the current process.
+// Subsequent calls to `flag.Parse` will populate config with values parsed
+// from the command-line.
+func (config *Config) InstallFlags() {
+	flag.StringVar(&config.Pidfile, []string{"p", "-pidfile"}, "/var/run/docker.pid", "Path to use for daemon PID file")
+	flag.StringVar(&config.Root, []string{"g", "-graph"}, "/var/lib/docker", "Path to use as the root of the Docker runtime")
+	flag.BoolVar(&config.AutoRestart, []string{"r", "-restart"}, true, "Restart previously running containers")
+	flag.BoolVar(&config.EnableIptables, []string{"#iptables", "-iptables"}, true, "Enable Docker's addition of iptables rules")
+	flag.BoolVar(&config.EnableIpForward, []string{"#ip-forward", "-ip-forward"}, true, "Enable net.ipv4.ip_forward")
+	flag.StringVar(&config.BridgeIP, []string{"#bip", "-bip"}, "", "Use this CIDR notation address for the network bridge's IP, not compatible with -b")
+	flag.StringVar(&config.BridgeIface, []string{"b", "-bridge"}, "", "Attach containers to a pre-existing network bridge\nuse 'none' to disable container networking")
+	flag.BoolVar(&config.InterContainerCommunication, []string{"#icc", "-icc"}, true, "Enable inter-container communication")
+	flag.StringVar(&config.GraphDriver, []string{"s", "-storage-driver"}, "", "Force the Docker runtime to use a specific storage driver")
+	flag.StringVar(&config.ExecDriver, []string{"e", "-exec-driver"}, "native", "Force the Docker runtime to use a specific exec driver")
+	flag.BoolVar(&config.EnableSelinuxSupport, []string{"-selinux-enabled"}, false, "Enable selinux support. SELinux does not presently support the BTRFS storage driver")
+	flag.IntVar(&config.Mtu, []string{"#mtu", "-mtu"}, 0, "Set the containers network MTU\nif no value is provided: default to the default route MTU or 1500 if no default route is available")
+	opts.IPVar(&config.DefaultIp, []string{"#ip", "-ip"}, "0.0.0.0", "Default IP address to use when binding container ports")
+	opts.ListVar(&config.GraphOptions, []string{"-storage-opt"}, "Set storage driver options")
+	// FIXME: why the inconsistency between "hosts" and "sockets"?
+	opts.HostListVar(&config.Sockets, []string{"H", "-host"}, "The socket(s) to bind to in daemon mode\nspecified using one or more tcp://host:port, unix:///path/to/socket, fd://* or fd://socketfd.")
+	opts.IPListVar(&config.Dns, []string{"#dns", "-dns"}, "Force Docker to use specific DNS servers")
+	opts.DnsSearchListVar(&config.DnsSearch, []string{"-dns-search"}, "Force Docker to use specific DNS search domains")
+}
+
 func GetDefaultNetworkMtu() int {
 	if iface, err := networkdriver.GetDefaultRouteIface(); err == nil {
 		return iface.MTU

daemon/daemon.go

@@ -672,6 +672,14 @@ func NewDaemon(config *Config, eng *engine.Engine) (*Daemon, error) {
 }
 
 func NewDaemonFromDirectory(config *Config, eng *engine.Engine) (*Daemon, error) {
+	// Apply configuration defaults
+	if config.Mtu == 0 {
+		// FIXME: GetDefaultNetwork Mtu doesn't need to be public anymore
+		config.Mtu = GetDefaultNetworkMtu()
+	}
+	// FIXME: DisableNetworkBidge doesn't need to be public anymore
+	config.DisableNetwork = config.BridgeIface == DisableNetworkBridge
+
 	// 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 != "" {

docker/daemon.go

@@ -4,7 +4,6 @@ package main
 
 import (
 	"log"
-	"net"
 
 	"github.com/docker/docker/builtins"
 	"github.com/docker/docker/daemon"
@@ -18,22 +17,32 @@ import (
 
 const CanDaemon = true
 
+var (
+	daemonCfg = &daemon.Config{}
+)
+
+func init() {
+	daemonCfg.InstallFlags()
+}
+
 func mainDaemon() {
 	if flag.NArg() != 0 {
 		flag.Usage()
 		return
 	}
 
-	if *bridgeName != "" && *bridgeIp != "" {
+	// FIXME: validate daemon.Config values in a method of daemon.Config
+	if daemonCfg.BridgeIface != "" && daemonCfg.BridgeIP != "" {
 		log.Fatal("You specified -b & --bip, mutually exclusive options. Please specify only one.")
 	}
 
-	if !*flEnableIptables && !*flInterContainerComm {
+	if !daemonCfg.EnableIptables && !daemonCfg.InterContainerCommunication {
 		log.Fatal("You specified --iptables=false with --icc=false. ICC uses iptables to function. Please set --icc or --iptables to true.")
 	}
 
-	if net.ParseIP(*flDefaultIp) == nil {
-		log.Fatalf("Specified --ip=%s is not in correct format \"0.0.0.0\".", *flDefaultIp)
+	// FIXME: move this validation to opts.IpOpt
+	if daemonCfg.DefaultIp == nil {
+		log.Fatalf("Specified --ip is not in correct format \"0.0.0.0\".")
 	}
 
 	eng := engine.New()
@@ -47,34 +56,7 @@ func mainDaemon() {
 	// the http api so that connections don't fail while the daemon
 	// is booting
 	go func() {
-		// FIXME: daemon config and CLI flag parsing should be directly integrated
-		cfg := &daemon.Config{
-			Pidfile:                     *pidfile,
-			Root:                        *flRoot,
-			AutoRestart:                 *flAutoRestart,
-			EnableIptables:              *flEnableIptables,
-			EnableIpForward:             *flEnableIpForward,
-			BridgeIP:                    *bridgeIp,
-			BridgeIface:                 *bridgeName,
-			DefaultIp:                   net.ParseIP(*flDefaultIp),
-			InterContainerCommunication: *flInterContainerComm,
-			GraphDriver:                 *flGraphDriver,
-			ExecDriver:                  *flExecDriver,
-			EnableSelinuxSupport:        *flSelinuxEnabled,
-			GraphOptions:                flGraphOpts.GetAll(),
-			Dns:                         flDns.GetAll(),
-			DnsSearch:                   flDnsSearch.GetAll(),
-			Mtu:                         *flMtu,
-			Sockets:                     flHosts.GetAll(),
-		}
-		// FIXME this should be initialized in NewDaemon or somewhere in daemon.
-		// Currently it is copy-pasted in `integration` to create test daemons that work.
-		if cfg.Mtu == 0 {
-			cfg.Mtu = daemon.GetDefaultNetworkMtu()
-		}
-		cfg.DisableNetwork = cfg.BridgeIface == daemon.DisableNetworkBridge
-
-		d, err := daemon.NewDaemon(cfg, eng)
+		d, err := daemon.NewDaemon(daemonCfg, eng)
 		if err != nil {
 			log.Fatal(err)
 		}
@@ -91,11 +73,13 @@ func mainDaemon() {
 	log.Printf("docker daemon: %s %s; execdriver: %s; graphdriver: %s",
 		dockerversion.VERSION,
 		dockerversion.GITCOMMIT,
-		*flExecDriver,
-		*flGraphDriver)
+		daemonCfg.ExecDriver,
+		daemonCfg.GraphDriver,
+	)
 
 	// Serve api
-	job := eng.Job("serveapi", flHosts.GetAll()...)
+	// FIXME: 'Sockets' should not be part of daemon.Config
+	job := eng.Job("serveapi", daemonCfg.Sockets...)
 	job.SetenvBool("Logging", true)
 	job.SetenvBool("EnableCors", *flEnableCors)
 	job.Setenv("Version", dockerversion.VERSION)

docker/docker.go

@@ -27,8 +27,8 @@ func main() {
 	if reexec.Init() {
 		return
 	}
-
 	flag.Parse()
+	// FIXME: validate daemon flags here
 
 	if *flVersion {
 		showVersion()
@@ -38,7 +38,7 @@ func main() {
 		os.Setenv("DEBUG", "1")
 	}
 
-	if flHosts.Len() == 0 {
+	if len(daemonCfg.Sockets) == 0 {
 		defaultHost := os.Getenv("DOCKER_HOST")
 		if defaultHost == "" || *flDaemon {
 			// If we do not have a host, default to unix socket
@@ -47,7 +47,7 @@ func main() {
 		if _, err := api.ValidateHost(defaultHost); err != nil {
 			log.Fatal(err)
 		}
-		flHosts.Set(defaultHost)
+		daemonCfg.Sockets = append(daemonCfg.Sockets, defaultHost)
 	}
 
 	if *flDaemon {
@@ -55,10 +55,10 @@ func main() {
 		return
 	}
 
-	if flHosts.Len() > 1 {
+	if len(daemonCfg.Sockets) > 1 {
 		log.Fatal("Please specify only one -H")
 	}
-	protoAddrParts := strings.SplitN(flHosts.GetAll()[0], "://", 2)
+	protoAddrParts := strings.SplitN(daemonCfg.Sockets[0], "://", 2)
 
 	var (
 		cli       *client.DockerCli

docker/flags.go

@@ -4,8 +4,6 @@ import (
 	"os"
 	"path/filepath"
 
-	"github.com/docker/docker/api"
-	"github.com/docker/docker/opts"
 	flag "github.com/docker/docker/pkg/mflag"
 )
 
@@ -20,30 +18,13 @@ func init() {
 }
 
 var (
-	flVersion            = flag.Bool([]string{"v", "-version"}, false, "Print version information and quit")
-	flDaemon             = flag.Bool([]string{"d", "-daemon"}, false, "Enable daemon mode")
-	flGraphOpts          = opts.NewListOpts(nil)
-	flDebug              = flag.Bool([]string{"D", "-debug"}, false, "Enable debug mode")
-	flAutoRestart        = flag.Bool([]string{"r", "-restart"}, true, "Restart previously running containers")
-	bridgeName           = flag.String([]string{"b", "-bridge"}, "", "Attach containers to a pre-existing network bridge\nuse 'none' to disable container networking")
-	bridgeIp             = flag.String([]string{"#bip", "-bip"}, "", "Use this CIDR notation address for the network bridge's IP, not compatible with -b")
-	pidfile              = flag.String([]string{"p", "-pidfile"}, "/var/run/docker.pid", "Path to use for daemon PID file")
-	flRoot               = flag.String([]string{"g", "-graph"}, "/var/lib/docker", "Path to use as the root of the Docker runtime")
-	flSocketGroup        = flag.String([]string{"G", "-group"}, "docker", "Group to assign the unix socket specified by -H when running in daemon mode\nuse '' (the empty string) to disable setting of a group")
-	flEnableCors         = flag.Bool([]string{"#api-enable-cors", "-api-enable-cors"}, false, "Enable CORS headers in the remote API")
-	flDns                = opts.NewListOpts(opts.ValidateIPAddress)
-	flDnsSearch          = opts.NewListOpts(opts.ValidateDnsSearch)
-	flEnableIptables     = flag.Bool([]string{"#iptables", "-iptables"}, true, "Enable Docker's addition of iptables rules")
-	flEnableIpForward    = flag.Bool([]string{"#ip-forward", "-ip-forward"}, true, "Enable net.ipv4.ip_forward")
-	flDefaultIp          = flag.String([]string{"#ip", "-ip"}, "0.0.0.0", "Default IP address to use when binding container ports")
-	flInterContainerComm = flag.Bool([]string{"#icc", "-icc"}, true, "Enable inter-container communication")
-	flGraphDriver        = flag.String([]string{"s", "-storage-driver"}, "", "Force the Docker runtime to use a specific storage driver")
-	flExecDriver         = flag.String([]string{"e", "-exec-driver"}, "native", "Force the Docker runtime to use a specific exec driver")
-	flHosts              = opts.NewListOpts(api.ValidateHost)
-	flMtu                = flag.Int([]string{"#mtu", "-mtu"}, 0, "Set the containers network MTU\nif no value is provided: default to the default route MTU or 1500 if no default route is available")
-	flTls                = flag.Bool([]string{"-tls"}, false, "Use TLS; implied by tls-verify flags")
-	flTlsVerify          = flag.Bool([]string{"-tlsverify"}, false, "Use TLS and verify the remote (daemon: verify client, client: verify daemon)")
-	flSelinuxEnabled     = flag.Bool([]string{"-selinux-enabled"}, false, "Enable selinux support. SELinux does not presently support the BTRFS storage driver")
+	flVersion     = flag.Bool([]string{"v", "-version"}, false, "Print version information and quit")
+	flDaemon      = flag.Bool([]string{"d", "-daemon"}, false, "Enable daemon mode")
+	flDebug       = flag.Bool([]string{"D", "-debug"}, false, "Enable debug mode")
+	flSocketGroup = flag.String([]string{"G", "-group"}, "docker", "Group to assign the unix socket specified by -H when running in daemon mode\nuse '' (the empty string) to disable setting of a group")
+	flEnableCors  = flag.Bool([]string{"#api-enable-cors", "-api-enable-cors"}, false, "Enable CORS headers in the remote API")
+	flTls         = flag.Bool([]string{"-tls"}, false, "Use TLS; implied by tls-verify flags")
+	flTlsVerify   = flag.Bool([]string{"-tlsverify"}, false, "Use TLS and verify the remote (daemon: verify client, client: verify daemon)")
 
 	// these are initialized in init() below since their default values depend on dockerCertPath which isn't fully initialized until init() runs
 	flCa   *string
@@ -55,9 +36,4 @@ func init() {
 	flCa = flag.String([]string{"-tlscacert"}, filepath.Join(dockerCertPath, defaultCaFile), "Trust only remotes providing a certificate signed by the CA given here")
 	flCert = flag.String([]string{"-tlscert"}, filepath.Join(dockerCertPath, defaultCertFile), "Path to TLS certificate file")
 	flKey = flag.String([]string{"-tlskey"}, filepath.Join(dockerCertPath, defaultKeyFile), "Path to TLS key file")
-
-	flag.Var(&flDns, []string{"#dns", "-dns"}, "Force Docker to use specific DNS servers")
-	flag.Var(&flDnsSearch, []string{"-dns-search"}, "Force Docker to use specific DNS search domains")
-	flag.Var(&flHosts, []string{"H", "-host"}, "The socket(s) to bind to in daemon mode\nspecified using one or more tcp://host:port, unix:///path/to/socket, fd://* or fd://socketfd.")
-	flag.Var(&flGraphOpts, []string{"-storage-opt"}, "Set storage driver options")
 }

integration/utils_test.go

@@ -186,12 +186,6 @@ func newTestEngine(t utils.Fataler, autorestart bool, root string) *engine.Engin
 		// otherwise NewDaemon will fail because of conflicting settings.
 		InterContainerCommunication: true,
 	}
-	// FIXME: this should be initialized in NewDaemon
-	// Currently it is copy-pasted from daemonMain()
-	if cfg.Mtu == 0 {
-		cfg.Mtu = daemon.GetDefaultNetworkMtu()
-	}
-	cfg.DisableNetwork = cfg.BridgeIface == daemon.DisableNetworkBridge
 	d, err := daemon.NewDaemon(cfg, eng)
 	if err != nil {
 		t.Fatal(err)