From b241e2008e50f2d8e045642d6fd511a1af9bb52b Mon Sep 17 00:00:00 2001
From: Sebastiaan van Stijn <github@gone.nl>
Date: Fri, 3 Jun 2022 17:35:23 +0200
Subject: [PATCH] daemon.NewDaemon(): fix network feature detection on first
 start

Commit 483aa6294b457ad4f60df91e46c0038a0e953dad introduced a regression, causing
spurious warnings to be shown when starting a daemon for the first time after
a fresh install:

    docker info
    ...
    WARNING: IPv4 forwarding is disabled
    WARNING: bridge-nf-call-iptables is disabled
    WARNING: bridge-nf-call-ip6tables is disabled

The information shown is incorrect, as checking the corresponding options on
the system, shows that these options are available:

    cat /proc/sys/net/ipv4/ip_forward
    1
    cat /proc/sys/net/bridge/bridge-nf-call-iptables
    1
    cat /proc/sys/net/bridge/bridge-nf-call-ip6tables
    1

The reason this is failing is because the daemon itself reconfigures those
options during networking initialization in `configureIPForwarding()`;
https://github.com/moby/moby/blob/cf4595265e7703e1e9745a30f1dd265acbc075d3/libnetwork/drivers/bridge/setup_ip_forwarding.go#L14-L25

Network initialization happens in the `daemon.restore()` function within `daemon.NewDaemon()`:
https://github.com/moby/moby/blob/cf4595265e7703e1e9745a30f1dd265acbc075d3/daemon/daemon.go#L475-L478

However, 483aa6294b457ad4f60df91e46c0038a0e953dad moved detection of features
earlier in the `daemon.NewDaemon()` function, and collects the system information
(`d.RawSysInfo()`) before we enter `daemon.restore()`;
https://github.com/moby/moby/blob/cf4595265e7703e1e9745a30f1dd265acbc075d3/daemon/daemon.go#L1008-L1011

For optimization (collecting the system information comes at a cost), those
results are cached on the daemon, and will only be performed once (using a
`sync.Once`).

This patch:

- introduces a `getSysInfo()` utility, which collects system information without
  caching the results
- uses `getSysInfo()` to collect the preliminary information needed at that
  point in the daemon's lifecycle.
- moves printing warnings to the end of `daemon.NewDaemon()`, after all information
  can be read correctly.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
---
 daemon/daemon.go             | 17 +++++++++++------
 daemon/daemon_unix.go        |  4 ++--
 daemon/daemon_unsupported.go |  4 ++--
 daemon/daemon_windows.go     |  4 ++--
 4 files changed, 17 insertions(+), 12 deletions(-)

diff --git a/daemon/daemon.go b/daemon/daemon.go
index c9aece54f8..bb09038b63 100644
--- a/daemon/daemon.go
+++ b/daemon/daemon.go
@@ -1005,13 +1005,15 @@ func NewDaemon(ctx context.Context, config *config.Config, pluginStore *plugin.S
 		return nil, err
 	}
 
-	sysInfo := d.RawSysInfo()
-	for _, w := range sysInfo.Warnings {
-		logrus.Warn(w)
-	}
 	// Check if Devices cgroup is mounted, it is hard requirement for container security,
 	// on Linux.
-	if runtime.GOOS == "linux" && !sysInfo.CgroupDevicesEnabled && !userns.RunningInUserNS() {
+	//
+	// Important: we call getSysInfo() directly here, without storing the results,
+	// as networking has not yet been set up, so we only have partial system info
+	// at this point.
+	//
+	// TODO(thaJeztah) add a utility to only collect the CgroupDevicesEnabled information
+	if runtime.GOOS == "linux" && !userns.RunningInUserNS() && !getSysInfo(d).CgroupDevicesEnabled {
 		return nil, errors.New("Devices cgroup isn't mounted")
 	}
 
@@ -1096,6 +1098,9 @@ func NewDaemon(ctx context.Context, config *config.Config, pluginStore *plugin.S
 	close(d.startupDone)
 
 	info := d.SystemInfo()
+	for _, w := range info.Warnings {
+		logrus.Warn(w)
+	}
 
 	engineInfo.WithValues(
 		dockerversion.Version,
@@ -1487,7 +1492,7 @@ func (daemon *Daemon) RawSysInfo() *sysinfo.SysInfo {
 		// We check if sysInfo is not set here, to allow some test to
 		// override the actual sysInfo.
 		if daemon.sysInfo == nil {
-			daemon.loadSysInfo()
+			daemon.sysInfo = getSysInfo(daemon)
 		}
 	})
 
diff --git a/daemon/daemon_unix.go b/daemon/daemon_unix.go
index 380d039804..7e1fc55347 100644
--- a/daemon/daemon_unix.go
+++ b/daemon/daemon_unix.go
@@ -1726,14 +1726,14 @@ func (daemon *Daemon) setupSeccompProfile() error {
 	return nil
 }
 
-func (daemon *Daemon) loadSysInfo() {
+func getSysInfo(daemon *Daemon) *sysinfo.SysInfo {
 	var siOpts []sysinfo.Opt
 	if daemon.getCgroupDriver() == cgroupSystemdDriver {
 		if euid := os.Getenv("ROOTLESSKIT_PARENT_EUID"); euid != "" {
 			siOpts = append(siOpts, sysinfo.WithCgroup2GroupPath("/user.slice/user-"+euid+".slice"))
 		}
 	}
-	daemon.sysInfo = sysinfo.New(siOpts...)
+	return sysinfo.New(siOpts...)
 }
 
 func (daemon *Daemon) initLibcontainerd(ctx context.Context) error {
diff --git a/daemon/daemon_unsupported.go b/daemon/daemon_unsupported.go
index b2a1b5be9b..b154c6c8f5 100644
--- a/daemon/daemon_unsupported.go
+++ b/daemon/daemon_unsupported.go
@@ -13,6 +13,6 @@ const platformSupported = false
 func setupResolvConf(config *config.Config) {
 }
 
-func (daemon *Daemon) loadSysInfo() {
-	daemon.sysInfo = sysinfo.New()
+func getSysInfo(daemon *Daemon) *sysinfo.SysInfo {
+	return sysinfo.New()
 }
diff --git a/daemon/daemon_windows.go b/daemon/daemon_windows.go
index 22f50e06d3..7acc06c7f7 100644
--- a/daemon/daemon_windows.go
+++ b/daemon/daemon_windows.go
@@ -598,8 +598,8 @@ func (daemon *Daemon) loadRuntimes() error {
 
 func setupResolvConf(config *config.Config) {}
 
-func (daemon *Daemon) loadSysInfo() {
-	daemon.sysInfo = sysinfo.New()
+func getSysInfo(daemon *Daemon) *sysinfo.SysInfo {
+	return sysinfo.New()
 }
 
 func (daemon *Daemon) initLibcontainerd(ctx context.Context) error {