Skip to content

Commit

Permalink
daemon.NewDaemon(): fix network feature detection on first start
Browse files Browse the repository at this point in the history
Commit 483aa62 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, 483aa62 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>
  • Loading branch information
thaJeztah committed Jun 3, 2022
1 parent cf45952 commit b241e20
Show file tree
Hide file tree
Showing 4 changed files with 17 additions and 12 deletions.
17 changes: 11 additions & 6 deletions daemon/daemon.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}

Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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)
}
})

Expand Down
4 changes: 2 additions & 2 deletions daemon/daemon_unix.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
4 changes: 2 additions & 2 deletions daemon/daemon_unsupported.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}
4 changes: 2 additions & 2 deletions daemon/daemon_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down

0 comments on commit b241e20

Please sign in to comment.