From b2d06b6fba307a8972d08477ef8b711e31ace433 Mon Sep 17 00:00:00 2001 From: Antonio Murdaca Date: Thu, 6 Aug 2015 13:54:48 +0200 Subject: [PATCH 1/2] Move sysinfo out of daemon struct sysinfo struct was initialized at daemon startup to make sure kernel configs such as device cgroup are present and error out if not. The struct was embedded in daemon struct making impossible to detect if some system config is changed at daemon runtime (i.e. someone umount the memory cgroup). This leads to container's starts failure if some config is changed at daemon runtime. This patch moves sysinfo out of daemon and initilize and check it when needed (daemon startup, containers creation, contaienrs startup for now). Signed-off-by: Antonio Murdaca (cherry picked from commit 472b6f66e03f9a85fe8d23098dac6f55a87456d8) --- daemon/daemon.go | 6 ------ daemon/daemon_unix.go | 24 +++++++++++++----------- daemon/info.go | 19 +++++++++++-------- pkg/sysinfo/sysinfo_linux.go | 4 +++- 4 files changed, 27 insertions(+), 26 deletions(-) diff --git a/daemon/daemon.go b/daemon/daemon.go index 0fb8b0f2f3..bf1e34c097 100644 --- a/daemon/daemon.go +++ b/daemon/daemon.go @@ -92,7 +92,6 @@ type Daemon struct { graph *graph.Graph repositories *graph.TagStore idIndex *truncindex.TruncIndex - sysInfo *sysinfo.SysInfo config *Config containerGraph *graphdb.Database driver graphdriver.Driver @@ -725,7 +724,6 @@ func NewDaemon(config *Config, registryService *registry.Service) (daemon *Daemo d.graph = g d.repositories = repositories d.idIndex = truncindex.NewTruncIndex([]string{}) - d.sysInfo = sysInfo d.config = config d.sysInitPath = sysInitPath d.execDriver = ed @@ -858,10 +856,6 @@ func (daemon *Daemon) Config() *Config { return daemon.config } -func (daemon *Daemon) SystemConfig() *sysinfo.SysInfo { - return daemon.sysInfo -} - func (daemon *Daemon) SystemInitPath() string { return daemon.sysInitPath } diff --git a/daemon/daemon_unix.go b/daemon/daemon_unix.go index edc406d0e9..aa380b171d 100644 --- a/daemon/daemon_unix.go +++ b/daemon/daemon_unix.go @@ -18,6 +18,7 @@ import ( "github.com/docker/docker/pkg/fileutils" "github.com/docker/docker/pkg/parsers" "github.com/docker/docker/pkg/parsers/kernel" + "github.com/docker/docker/pkg/sysinfo" "github.com/docker/docker/pkg/system" "github.com/docker/docker/runconfig" "github.com/docker/docker/utils" @@ -148,7 +149,8 @@ func (daemon *Daemon) adaptContainerSettings(hostConfig *runconfig.HostConfig, a // verifyPlatformContainerSettings performs platform-specific validation of the // hostconfig and config structures. func verifyPlatformContainerSettings(daemon *Daemon, hostConfig *runconfig.HostConfig, config *runconfig.Config) ([]string, error) { - var warnings []string + warnings := []string{} + sysInfo := sysinfo.New(false) if hostConfig.LxcConf.Len() > 0 && !strings.Contains(daemon.ExecutionDriver().Name(), "lxc") { return warnings, fmt.Errorf("Cannot use --lxc-conf with execdriver: %s", daemon.ExecutionDriver().Name()) @@ -156,12 +158,12 @@ func verifyPlatformContainerSettings(daemon *Daemon, hostConfig *runconfig.HostC if hostConfig.Memory != 0 && hostConfig.Memory < 4194304 { return warnings, fmt.Errorf("Minimum memory limit allowed is 4MB") } - if hostConfig.Memory > 0 && !daemon.SystemConfig().MemoryLimit { + if hostConfig.Memory > 0 && !sysInfo.MemoryLimit { warnings = append(warnings, "Your kernel does not support memory limit capabilities. Limitation discarded.") logrus.Warnf("Your kernel does not support memory limit capabilities. Limitation discarded.") hostConfig.Memory = 0 } - if hostConfig.Memory > 0 && hostConfig.MemorySwap != -1 && !daemon.SystemConfig().SwapLimit { + if hostConfig.Memory > 0 && hostConfig.MemorySwap != -1 && !sysInfo.SwapLimit { warnings = append(warnings, "Your kernel does not support swap limit capabilities, memory limited without swap.") logrus.Warnf("Your kernel does not support swap limit capabilities, memory limited without swap.") hostConfig.MemorySwap = -1 @@ -172,7 +174,7 @@ func verifyPlatformContainerSettings(daemon *Daemon, hostConfig *runconfig.HostC if hostConfig.Memory == 0 && hostConfig.MemorySwap > 0 { return warnings, fmt.Errorf("You should always set the Memory limit when using Memoryswap limit, see usage.") } - if hostConfig.MemorySwappiness != nil && !daemon.SystemConfig().MemorySwappiness { + if hostConfig.MemorySwappiness != nil && !sysInfo.MemorySwappiness { warnings = append(warnings, "Your kernel does not support memory swappiness capabilities, memory swappiness discarded.") logrus.Warnf("Your kernel does not support memory swappiness capabilities, memory swappiness discarded.") hostConfig.MemorySwappiness = nil @@ -183,28 +185,28 @@ func verifyPlatformContainerSettings(daemon *Daemon, hostConfig *runconfig.HostC return warnings, fmt.Errorf("Invalid value: %v, valid memory swappiness range is 0-100.", swappiness) } } - if hostConfig.CPUShares > 0 && !daemon.SystemConfig().CPUShares { + if hostConfig.CPUShares > 0 && !sysInfo.CPUShares { warnings = append(warnings, "Your kernel does not support CPU shares. Shares discarded.") logrus.Warnf("Your kernel does not support CPU shares. Shares discarded.") hostConfig.CPUShares = 0 } - if hostConfig.CPUPeriod > 0 && !daemon.SystemConfig().CPUCfsPeriod { + if hostConfig.CPUPeriod > 0 && !sysInfo.CPUCfsPeriod { warnings = append(warnings, "Your kernel does not support CPU cfs period. Period discarded.") logrus.Warnf("Your kernel does not support CPU cfs period. Period discarded.") hostConfig.CPUPeriod = 0 } - if hostConfig.CPUQuota > 0 && !daemon.SystemConfig().CPUCfsQuota { + if hostConfig.CPUQuota > 0 && !sysInfo.CPUCfsQuota { warnings = append(warnings, "Your kernel does not support CPU cfs quota. Quota discarded.") logrus.Warnf("Your kernel does not support CPU cfs quota. Quota discarded.") hostConfig.CPUQuota = 0 } - if (hostConfig.CpusetCpus != "" || hostConfig.CpusetMems != "") && !daemon.SystemConfig().Cpuset { + if (hostConfig.CpusetCpus != "" || hostConfig.CpusetMems != "") && !sysInfo.Cpuset { warnings = append(warnings, "Your kernel does not support cpuset. Cpuset discarded.") logrus.Warnf("Your kernel does not support cpuset. Cpuset discarded.") hostConfig.CpusetCpus = "" hostConfig.CpusetMems = "" } - if hostConfig.BlkioWeight > 0 && !daemon.SystemConfig().BlkioWeight { + if hostConfig.BlkioWeight > 0 && !sysInfo.BlkioWeight { warnings = append(warnings, "Your kernel does not support Block I/O weight. Weight discarded.") logrus.Warnf("Your kernel does not support Block I/O weight. Weight discarded.") hostConfig.BlkioWeight = 0 @@ -212,11 +214,11 @@ func verifyPlatformContainerSettings(daemon *Daemon, hostConfig *runconfig.HostC if hostConfig.BlkioWeight > 0 && (hostConfig.BlkioWeight < 10 || hostConfig.BlkioWeight > 1000) { return warnings, fmt.Errorf("Range of blkio weight is from 10 to 1000.") } - if hostConfig.OomKillDisable && !daemon.SystemConfig().OomKillDisable { + if hostConfig.OomKillDisable && !sysInfo.OomKillDisable { hostConfig.OomKillDisable = false return warnings, fmt.Errorf("Your kernel does not support oom kill disable.") } - if daemon.SystemConfig().IPv4ForwardingDisabled { + if sysInfo.IPv4ForwardingDisabled { warnings = append(warnings, "IPv4 forwarding is disabled. Networking will not work.") logrus.Warnf("IPv4 forwarding is disabled. Networking will not work") } diff --git a/daemon/info.go b/daemon/info.go index 6b0a4b23ff..8de0fa7e9c 100644 --- a/daemon/info.go +++ b/daemon/info.go @@ -11,6 +11,7 @@ import ( "github.com/docker/docker/pkg/fileutils" "github.com/docker/docker/pkg/parsers/kernel" "github.com/docker/docker/pkg/parsers/operatingsystem" + "github.com/docker/docker/pkg/sysinfo" "github.com/docker/docker/pkg/system" "github.com/docker/docker/registry" "github.com/docker/docker/utils" @@ -56,15 +57,17 @@ func (daemon *Daemon) SystemInfo() (*types.Info, error) { initPath = daemon.SystemInitPath() } + sysInfo := sysinfo.New(false) + v := &types.Info{ ID: daemon.ID, Containers: len(daemon.List()), Images: imgcount, Driver: daemon.GraphDriver().String(), DriverStatus: daemon.GraphDriver().Status(), - IPv4Forwarding: !daemon.SystemConfig().IPv4ForwardingDisabled, - BridgeNfIptables: !daemon.SystemConfig().BridgeNfCallIptablesDisabled, - BridgeNfIp6tables: !daemon.SystemConfig().BridgeNfCallIP6tablesDisabled, + IPv4Forwarding: !sysInfo.IPv4ForwardingDisabled, + BridgeNfIptables: !sysInfo.BridgeNfCallIptablesDisabled, + BridgeNfIp6tables: !sysInfo.BridgeNfCallIP6tablesDisabled, Debug: os.Getenv("DEBUG") != "", NFd: fileutils.GetTotalUsedFds(), NGoroutines: runtime.NumGoroutine(), @@ -90,11 +93,11 @@ func (daemon *Daemon) SystemInfo() (*types.Info, error) { // sysinfo.cgroupCpuInfo will be nil otherwise and cause a SIGSEGV if // an attempt is made to access through them. if runtime.GOOS != "windows" { - v.MemoryLimit = daemon.SystemConfig().MemoryLimit - v.SwapLimit = daemon.SystemConfig().SwapLimit - v.OomKillDisable = daemon.SystemConfig().OomKillDisable - v.CpuCfsPeriod = daemon.SystemConfig().CPUCfsPeriod - v.CpuCfsQuota = daemon.SystemConfig().CPUCfsQuota + v.MemoryLimit = sysInfo.MemoryLimit + v.SwapLimit = sysInfo.SwapLimit + v.OomKillDisable = sysInfo.OomKillDisable + v.CpuCfsPeriod = sysInfo.CPUCfsPeriod + v.CpuCfsQuota = sysInfo.CPUCfsQuota } if httpProxy := os.Getenv("http_proxy"); httpProxy != "" { diff --git a/pkg/sysinfo/sysinfo_linux.go b/pkg/sysinfo/sysinfo_linux.go index 2994d67d48..e2f697080f 100644 --- a/pkg/sysinfo/sysinfo_linux.go +++ b/pkg/sysinfo/sysinfo_linux.go @@ -10,7 +10,9 @@ import ( "github.com/opencontainers/runc/libcontainer/cgroups" ) -// New returns a new SysInfo, using the filesystem to detect which features the kernel supports. +// New returns a new SysInfo, using the filesystem to detect which features +// the kernel supports. If `quiet` is `false` warnings are printed in logs +// whenever an error occurs or misconfigurations are present. func New(quiet bool) *SysInfo { sysInfo := &SysInfo{} sysInfo.cgroupMemInfo = checkCgroupMem(quiet) From 4177b0bae04bb41dfff65ea87b2efb87811e08e6 Mon Sep 17 00:00:00 2001 From: Antonio Murdaca Date: Thu, 6 Aug 2015 13:55:56 +0200 Subject: [PATCH 2/2] Add hostConfig check before starting a container It may happen that host system settings are changed while the daemon is running. This will cause errors at libcontainer level when starting a container with a particular hostConfig (e.g. hostConfig with memory swappiness but the memory cgroup was umounted). This patch adds an hostConfig check on container start to prevent the daemon from even calling libcontainer with the wrong configuration as we're already doing on container's creation). Signed-off-by: Antonio Murdaca (cherry picked from commit 0d2628cdf19783106ae8723f51fae0a7c7f361c6) --- daemon/daemon_unix.go | 7 ++++++- daemon/start.go | 10 ++++++---- runconfig/parse.go | 8 ++++---- 3 files changed, 16 insertions(+), 9 deletions(-) diff --git a/daemon/daemon_unix.go b/daemon/daemon_unix.go index aa380b171d..d57100d849 100644 --- a/daemon/daemon_unix.go +++ b/daemon/daemon_unix.go @@ -155,6 +155,8 @@ func verifyPlatformContainerSettings(daemon *Daemon, hostConfig *runconfig.HostC if hostConfig.LxcConf.Len() > 0 && !strings.Contains(daemon.ExecutionDriver().Name(), "lxc") { return warnings, fmt.Errorf("Cannot use --lxc-conf with execdriver: %s", daemon.ExecutionDriver().Name()) } + + // memory subsystem checks and adjustments if hostConfig.Memory != 0 && hostConfig.Memory < 4194304 { return warnings, fmt.Errorf("Minimum memory limit allowed is 4MB") } @@ -162,6 +164,7 @@ func verifyPlatformContainerSettings(daemon *Daemon, hostConfig *runconfig.HostC warnings = append(warnings, "Your kernel does not support memory limit capabilities. Limitation discarded.") logrus.Warnf("Your kernel does not support memory limit capabilities. Limitation discarded.") hostConfig.Memory = 0 + hostConfig.MemorySwap = -1 } if hostConfig.Memory > 0 && hostConfig.MemorySwap != -1 && !sysInfo.SwapLimit { warnings = append(warnings, "Your kernel does not support swap limit capabilities, memory limited without swap.") @@ -174,7 +177,7 @@ func verifyPlatformContainerSettings(daemon *Daemon, hostConfig *runconfig.HostC if hostConfig.Memory == 0 && hostConfig.MemorySwap > 0 { return warnings, fmt.Errorf("You should always set the Memory limit when using Memoryswap limit, see usage.") } - if hostConfig.MemorySwappiness != nil && !sysInfo.MemorySwappiness { + if hostConfig.MemorySwappiness != nil && *hostConfig.MemorySwappiness != -1 && !sysInfo.MemorySwappiness { warnings = append(warnings, "Your kernel does not support memory swappiness capabilities, memory swappiness discarded.") logrus.Warnf("Your kernel does not support memory swappiness capabilities, memory swappiness discarded.") hostConfig.MemorySwappiness = nil @@ -214,10 +217,12 @@ func verifyPlatformContainerSettings(daemon *Daemon, hostConfig *runconfig.HostC if hostConfig.BlkioWeight > 0 && (hostConfig.BlkioWeight < 10 || hostConfig.BlkioWeight > 1000) { return warnings, fmt.Errorf("Range of blkio weight is from 10 to 1000.") } + if hostConfig.OomKillDisable && !sysInfo.OomKillDisable { hostConfig.OomKillDisable = false return warnings, fmt.Errorf("Your kernel does not support oom kill disable.") } + if sysInfo.IPv4ForwardingDisabled { warnings = append(warnings, "IPv4 forwarding is disabled. Networking will not work.") logrus.Warnf("IPv4 forwarding is disabled. Networking will not work") diff --git a/daemon/start.go b/daemon/start.go index 9df56c5a34..2dc05d613c 100644 --- a/daemon/start.go +++ b/daemon/start.go @@ -21,10 +21,6 @@ func (daemon *Daemon) ContainerStart(name string, hostConfig *runconfig.HostConf return fmt.Errorf("Container already started") } - if _, err = daemon.verifyContainerSettings(hostConfig, nil); err != nil { - return err - } - // Windows does not have the backwards compatibilty issue here. if runtime.GOOS != "windows" { // This is kept for backward compatibility - hostconfig should be passed when @@ -40,6 +36,12 @@ func (daemon *Daemon) ContainerStart(name string, hostConfig *runconfig.HostConf } } + // check if hostConfig is in line with the current system settings. + // It may happen cgroups are umounted or the like. + if _, err = daemon.verifyContainerSettings(container.hostConfig, nil); err != nil { + return err + } + if err := container.Start(); err != nil { return fmt.Errorf("Cannot start container %s: %s", name, err) } diff --git a/runconfig/parse.go b/runconfig/parse.go index 5528d7af76..6488817cfd 100644 --- a/runconfig/parse.go +++ b/runconfig/parse.go @@ -188,16 +188,16 @@ func Parse(cmd *flag.FlagSet, args []string) (*Config, *HostConfig, *flag.FlagSe flMemory = parsedMemory } - var MemorySwap int64 + var memorySwap int64 if *flMemorySwap != "" { if *flMemorySwap == "-1" { - MemorySwap = -1 + memorySwap = -1 } else { parsedMemorySwap, err := units.RAMInBytes(*flMemorySwap) if err != nil { return nil, nil, cmd, err } - MemorySwap = parsedMemorySwap + memorySwap = parsedMemorySwap } } @@ -354,7 +354,7 @@ func Parse(cmd *flag.FlagSet, args []string) (*Config, *HostConfig, *flag.FlagSe ContainerIDFile: *flContainerIDFile, LxcConf: lxcConf, Memory: flMemory, - MemorySwap: MemorySwap, + MemorySwap: memorySwap, CPUShares: *flCPUShares, CPUPeriod: *flCPUPeriod, CpusetCpus: *flCpusetCpus,