From dbd575ef915debc858a03ab6e5292d88bb9f77b7 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Tue, 26 Apr 2022 10:32:10 +0200 Subject: [PATCH] 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 --- daemon/daemon.go | 7 +++++-- daemon/daemon_unix.go | 39 ++++++++++++++++++++++----------------- daemon/daemon_windows.go | 34 ++++++++++++++++------------------ 3 files changed, 43 insertions(+), 37 deletions(-) diff --git a/daemon/daemon.go b/daemon/daemon.go index 9e87a56e40..5d9e57cca5 100644 --- a/daemon/daemon.go +++ b/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) } diff --git a/daemon/daemon_unix.go b/daemon/daemon_unix.go index 5086eac64a..e5d163db0c 100644 --- a/daemon/daemon_unix.go +++ b/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 } diff --git a/daemon/daemon_windows.go b/daemon/daemon_windows.go index 6894fe7184..22f50e06d3 100644 --- a/daemon/daemon_windows.go +++ b/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