daemon: daemon.initNetworkController(): dont return the controller

This method returned the network controller, only to set it on the daemon.

While making this change, also;

- update some error messages to be in the correct format
- use errors.Wrap() where possible
- extract configuring networks into a separate function to make the flow
  slightly easier to follow.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

daemon/daemon.go

@@ -471,8 +471,11 @@ func (daemon *Daemon) restore() error {
 	}
 	group.Wait()
 
-	daemon.netController, err = daemon.initNetworkController(activeSandboxes)
-	if err != nil {
+	// Initialize the network controller and configure network settings.
+	//
+	// Note that we cannot initialize the network controller earlier, as it
+	// needs to know if there's active sandboxes (running containers).
+	if err = daemon.initNetworkController(activeSandboxes); err != nil {
 		return fmt.Errorf("Error initializing network controller: %v", err)
 	}
 

daemon/daemon_unix.go

@@ -836,42 +836,50 @@ func configureKernelSecuritySupport(config *config.Config, driverName string) er
 	return nil
 }
 
-func (daemon *Daemon) initNetworkController(activeSandboxes map[string]interface{}) (libnetwork.NetworkController, error) {
+// initNetworkController initializes the libnetwork controller and configures
+// network settings. If there's active sandboxes, configuration changes will not
+// take effect.
+func (daemon *Daemon) initNetworkController(activeSandboxes map[string]interface{}) error {
 	netOptions, err := daemon.networkOptions(daemon.PluginStore, activeSandboxes)
 	if err != nil {
-		return nil, err
+		return err
 	}
 
-	controller, err := libnetwork.New(netOptions...)
+	daemon.netController, err = libnetwork.New(netOptions...)
 	if err != nil {
-		return nil, fmt.Errorf("error obtaining controller instance: %v", err)
+		return fmt.Errorf("error obtaining controller instance: %v", err)
 	}
 
-	conf := daemon.configStore
 	if len(activeSandboxes) > 0 {
-		logrus.Info("There are old running containers, the network config will not take affect")
-		setHostGatewayIP(conf, controller)
-		return controller, nil
+		logrus.Info("there are running containers, updated network configuration will not take affect")
+	} else if err := configureNetworking(daemon.netController, daemon.configStore); err != nil {
+		return err
 	}
 
+	// Set HostGatewayIP to the default bridge's IP if it is empty
+	setHostGatewayIP(daemon.netController, daemon.configStore)
+	return nil
+}
+
+func configureNetworking(controller libnetwork.NetworkController, conf *config.Config) error {
 	// Initialize default network on "null"
 	if n, _ := controller.NetworkByName("none"); n == nil {
 		if _, err := controller.NewNetwork("null", "none", "", libnetwork.NetworkOptionPersist(true)); err != nil {
-			return nil, fmt.Errorf("Error creating default \"null\" network: %v", err)
+			return errors.Wrap(err, `error creating default "null" network`)
 		}
 	}
 
 	// Initialize default network on "host"
 	if n, _ := controller.NetworkByName("host"); n == nil {
 		if _, err := controller.NewNetwork("host", "host", "", libnetwork.NetworkOptionPersist(true)); err != nil {
-			return nil, fmt.Errorf("Error creating default \"host\" network: %v", err)
+			return errors.Wrap(err, `error creating default "host" network`)
 		}
 	}
 
 	// Clear stale bridge network
 	if n, err := controller.NetworkByName("bridge"); err == nil {
 		if err = n.Delete(); err != nil {
-			return nil, fmt.Errorf("could not delete the default bridge network: %v", err)
+			return errors.Wrap(err, `could not delete the default "bridge"" network`)
 		}
 		if len(conf.NetworkConfig.DefaultAddressPools.Value()) > 0 && !conf.LiveRestoreEnabled {
 			removeDefaultBridgeInterface()
@@ -881,20 +889,17 @@ func (daemon *Daemon) initNetworkController(activeSandboxes map[string]interface
 	if !conf.DisableBridge {
 		// Initialize default driver "bridge"
 		if err := initBridgeDriver(controller, conf); err != nil {
-			return nil, err
+			return err
 		}
 	} else {
 		removeDefaultBridgeInterface()
 	}
 
-	// Set HostGatewayIP to the default bridge's IP  if it is empty
-	setHostGatewayIP(conf, controller)
-
-	return controller, nil
+	return nil
 }
 
 // setHostGatewayIP sets cfg.HostGatewayIP to the default bridge's IP if it is empty.
-func setHostGatewayIP(config *config.Config, controller libnetwork.NetworkController) {
+func setHostGatewayIP(controller libnetwork.NetworkController, config *config.Config) {
 	if config.HostGatewayIP != nil {
 		return
 	}

daemon/daemon_windows.go

@@ -236,23 +236,23 @@ func configureMaxThreads(config *config.Config) error {
 	return nil
 }
 
-func (daemon *Daemon) initNetworkController(activeSandboxes map[string]interface{}) (libnetwork.NetworkController, error) {
+func (daemon *Daemon) initNetworkController(activeSandboxes map[string]interface{}) error {
 	netOptions, err := daemon.networkOptions(nil, nil)
 	if err != nil {
-		return nil, err
+		return err
 	}
-	controller, err := libnetwork.New(netOptions...)
+	daemon.netController, err = libnetwork.New(netOptions...)
 	if err != nil {
-		return nil, fmt.Errorf("error obtaining controller instance: %v", err)
+		return errors.Wrap(err, "error obtaining controller instance")
 	}
 
 	hnsresponse, err := hcsshim.HNSListNetworkRequest("GET", "", "")
 	if err != nil {
-		return nil, err
+		return err
 	}
 
 	// Remove networks not present in HNS
-	for _, v := range controller.Networks() {
+	for _, v := range daemon.netController.Networks() {
 		hnsid := v.Info().DriverOptions()[winlibnetwork.HNSID]
 		found := false
 
@@ -274,14 +274,14 @@ func (daemon *Daemon) initNetworkController(activeSandboxes map[string]interface
 		}
 	}
 
-	_, err = controller.NewNetwork("null", "none", "", libnetwork.NetworkOptionPersist(false))
+	_, err = daemon.netController.NewNetwork("null", "none", "", libnetwork.NetworkOptionPersist(false))
 	if err != nil {
-		return nil, err
+		return err
 	}
 
 	defaultNetworkExists := false
 
-	if network, err := controller.NetworkByName(runconfig.DefaultDaemonNetworkMode().NetworkName()); err == nil {
+	if network, err := daemon.netController.NetworkByName(runconfig.DefaultDaemonNetworkMode().NetworkName()); err == nil {
 		hnsid := network.Info().DriverOptions()[winlibnetwork.HNSID]
 		for _, v := range hnsresponse {
 			if hnsid == v.Id {
@@ -308,7 +308,7 @@ func (daemon *Daemon) initNetworkController(activeSandboxes map[string]interface
 			return false
 		}
 
-		controller.WalkNetworks(s)
+		daemon.netController.WalkNetworks(s)
 
 		drvOptions := make(map[string]string)
 		nid := ""
@@ -359,7 +359,7 @@ func (daemon *Daemon) initNetworkController(activeSandboxes map[string]interface
 		}
 
 		v6Conf := []*libnetwork.IpamConf{}
-		_, err := controller.NewNetwork(strings.ToLower(v.Type), name, nid,
+		_, err := daemon.netController.NewNetwork(strings.ToLower(v.Type), name, nid,
 			libnetwork.NetworkOptionGeneric(options.Generic{
 				netlabel.GenericData: netOption,
 			}),
@@ -371,15 +371,14 @@ func (daemon *Daemon) initNetworkController(activeSandboxes map[string]interface
 		}
 	}
 
-	conf := daemon.configStore
-	if !conf.DisableBridge {
+	if !daemon.configStore.DisableBridge {
 		// Initialize default driver "bridge"
-		if err := initBridgeDriver(controller, conf); err != nil {
-			return nil, err
+		if err := initBridgeDriver(daemon.netController, daemon.configStore); err != nil {
+			return err
 		}
 	}
 
-	return controller, nil
+	return nil
 }
 
 func initBridgeDriver(controller libnetwork.NetworkController, config *config.Config) error {
@@ -411,9 +410,8 @@ func initBridgeDriver(controller libnetwork.NetworkController, config *config.Co
 		}),
 		ipamOption,
 	)
-
 	if err != nil {
-		return fmt.Errorf("Error creating default network: %v", err)
+		return errors.Wrap(err, "error creating default network")
 	}
 
 	return nil