From 9d87e6e0fb799d6ef3bb9a97bc523f8d343b5fb3 Mon Sep 17 00:00:00 2001 From: Michael Crosby Date: Fri, 30 Jun 2017 10:34:40 -0700 Subject: [PATCH] Do not set -1 for swappiness Do not set a default value for swappiness as the default value should be `nil` Signed-off-by: Michael Crosby --- daemon/daemon.go | 8 ++++++++ daemon/daemon_solaris.go | 3 ++- daemon/daemon_unix.go | 9 +++------ daemon/daemon_windows.go | 4 ++-- integration-cli/docker_api_containers_test.go | 2 +- 5 files changed, 16 insertions(+), 10 deletions(-) diff --git a/daemon/daemon.go b/daemon/daemon.go index ac03b75c2c..cdaa5c9a41 100644 --- a/daemon/daemon.go +++ b/daemon/daemon.go @@ -1243,3 +1243,11 @@ func (daemon *Daemon) checkpointAndSave(container *container.Container) error { } return nil } + +// because the CLI sends a -1 when it wants to unset the swappiness value +// we need to clear it on the server side +func fixMemorySwappiness(resources *containertypes.Resources) { + if resources.MemorySwappiness != nil && *resources.MemorySwappiness == -1 { + resources.MemorySwappiness = nil + } +} diff --git a/daemon/daemon_solaris.go b/daemon/daemon_solaris.go index 7f2004e65a..f464ee34b8 100644 --- a/daemon/daemon_solaris.go +++ b/daemon/daemon_solaris.go @@ -143,6 +143,7 @@ func UsingSystemd(config *Config) bool { // verifyPlatformContainerSettings performs platform-specific validation of the // hostconfig and config structures. func verifyPlatformContainerSettings(daemon *Daemon, hostConfig *containertypes.HostConfig, config *containertypes.Config, update bool) ([]string, error) { + fixMemorySwappiness(resources) warnings := []string{} sysInfo := sysinfo.New(true) // NOTE: We do not enforce a minimum value for swap limits for zones on Solaris and @@ -163,7 +164,7 @@ func verifyPlatformContainerSettings(daemon *Daemon, hostConfig *containertypes. } // Solaris NOTE: We allow and encourage setting the swap without setting the memory limit. - if hostConfig.MemorySwappiness != nil && *hostConfig.MemorySwappiness != -1 && !sysInfo.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 diff --git a/daemon/daemon_unix.go b/daemon/daemon_unix.go index 0778dde4f7..a7ba9c37b2 100644 --- a/daemon/daemon_unix.go +++ b/daemon/daemon_unix.go @@ -282,10 +282,6 @@ func (daemon *Daemon) adaptContainerSettings(hostConfig *containertypes.HostConf return err } hostConfig.SecurityOpt = append(hostConfig.SecurityOpt, opts...) - if hostConfig.MemorySwappiness == nil { - defaultSwappiness := int64(-1) - hostConfig.MemorySwappiness = &defaultSwappiness - } if hostConfig.OomKillDisable == nil { defaultOomKillDisable := false hostConfig.OomKillDisable = &defaultOomKillDisable @@ -296,6 +292,7 @@ func (daemon *Daemon) adaptContainerSettings(hostConfig *containertypes.HostConf func verifyContainerResources(resources *containertypes.Resources, sysInfo *sysinfo.SysInfo, update bool) ([]string, error) { warnings := []string{} + fixMemorySwappiness(resources) // memory subsystem checks and adjustments if resources.Memory != 0 && resources.Memory < linuxMinMemory { @@ -318,14 +315,14 @@ func verifyContainerResources(resources *containertypes.Resources, sysInfo *sysi if resources.Memory == 0 && resources.MemorySwap > 0 && !update { return warnings, fmt.Errorf("You should always set the Memory limit when using Memoryswap limit, see usage") } - if resources.MemorySwappiness != nil && *resources.MemorySwappiness != -1 && !sysInfo.MemorySwappiness { + if resources.MemorySwappiness != nil && !sysInfo.MemorySwappiness { warnings = append(warnings, "Your kernel does not support memory swappiness capabilities or the cgroup is not mounted. Memory swappiness discarded.") logrus.Warn("Your kernel does not support memory swappiness capabilities, or the cgroup is not mounted. Memory swappiness discarded.") resources.MemorySwappiness = nil } if resources.MemorySwappiness != nil { swappiness := *resources.MemorySwappiness - if swappiness < -1 || swappiness > 100 { + if swappiness < 0 || swappiness > 100 { return warnings, fmt.Errorf("Invalid value: %v, valid memory swappiness range is 0-100", swappiness) } } diff --git a/daemon/daemon_windows.go b/daemon/daemon_windows.go index c77f32a5ef..bb9c85b80e 100644 --- a/daemon/daemon_windows.go +++ b/daemon/daemon_windows.go @@ -100,7 +100,7 @@ func (daemon *Daemon) adaptContainerSettings(hostConfig *containertypes.HostConf func verifyContainerResources(resources *containertypes.Resources, isHyperv bool) ([]string, error) { warnings := []string{} - + fixMemorySwappiness(resources) if !isHyperv { // The processor resource controls are mutually exclusive on // Windows Server Containers, the order of precedence is @@ -197,7 +197,7 @@ func verifyContainerResources(resources *containertypes.Resources, isHyperv bool if resources.MemorySwap != 0 { return warnings, fmt.Errorf("invalid option: Windows does not support MemorySwap") } - if resources.MemorySwappiness != nil && *resources.MemorySwappiness != -1 { + if resources.MemorySwappiness != nil { return warnings, fmt.Errorf("invalid option: Windows does not support MemorySwappiness") } if resources.OomKillDisable != nil && *resources.OomKillDisable { diff --git a/integration-cli/docker_api_containers_test.go b/integration-cli/docker_api_containers_test.go index 84e009207a..25c724425e 100644 --- a/integration-cli/docker_api_containers_test.go +++ b/integration-cli/docker_api_containers_test.go @@ -1448,7 +1448,7 @@ func (s *DockerSuite) TestPostContainersCreateMemorySwappinessHostConfigOmitted( var containerJSON types.ContainerJSON c.Assert(json.Unmarshal(body, &containerJSON), check.IsNil) - c.Assert(*containerJSON.HostConfig.MemorySwappiness, check.Equals, int64(-1)) + c.Assert(containerJSON.HostConfig.MemorySwappiness, check.IsNil) } // check validation is done daemon side and not only in cli