Merge pull request #38393 from thaJeztah/refactor_container_validation

Refactor container validation

daemon/container.go

@@ -24,6 +24,7 @@ import (
 	"github.com/docker/go-connections/nat"
 	"github.com/opencontainers/selinux/go-selinux/label"
 	"github.com/pkg/errors"
+	"github.com/sirupsen/logrus"
 )
 
 // GetContainer looks for a container using the provided information, which could be
@@ -231,128 +232,150 @@ func (daemon *Daemon) setHostConfig(container *container.Container, hostConfig *
 
 // verifyContainerSettings performs validation of the hostconfig and config
 // structures.
-func (daemon *Daemon) verifyContainerSettings(platform string, hostConfig *containertypes.HostConfig, config *containertypes.Config, update bool) ([]string, error) {
+func (daemon *Daemon) verifyContainerSettings(platform string, hostConfig *containertypes.HostConfig, config *containertypes.Config, update bool) (warnings []string, err error) {
 	// First perform verification of settings common across all platforms.
-	if config != nil {
-		if config.WorkingDir != "" {
-			wdInvalid := false
-			if runtime.GOOS == platform {
-				config.WorkingDir = filepath.FromSlash(config.WorkingDir) // Ensure in platform semantics
-				if !system.IsAbs(config.WorkingDir) {
-					wdInvalid = true
-				}
-			} else {
-				// LCOW. Force Unix semantics
-				config.WorkingDir = strings.Replace(config.WorkingDir, string(os.PathSeparator), "/", -1)
-				if !path.IsAbs(config.WorkingDir) {
-					wdInvalid = true
-				}
-			}
-			if wdInvalid {
-				return nil, fmt.Errorf("the working directory '%s' is invalid, it needs to be an absolute path", config.WorkingDir)
-			}
-		}
+	if err = validateContainerConfig(config, platform); err != nil {
+		return warnings, err
+	}
+	if err := validateHostConfig(hostConfig, platform); err != nil {
+		return warnings, err
+	}
 
-		if len(config.StopSignal) > 0 {
-			_, err := signal.ParseSignal(config.StopSignal)
-			if err != nil {
-				return nil, err
-			}
-		}
+	// Now do platform-specific verification
+	warnings, err = verifyPlatformContainerSettings(daemon, hostConfig, update)
+	for _, w := range warnings {
+		logrus.Warn(w)
+	}
+	return warnings, err
+}
 
-		// Validate if Env contains empty variable or not (e.g., ``, `=foo`)
-		for _, env := range config.Env {
-			if _, err := opts.ValidateEnv(env); err != nil {
-				return nil, err
-			}
+func validateContainerConfig(config *containertypes.Config, platform string) error {
+	if config == nil {
+		return nil
+	}
+	if err := translateWorkingDir(config, platform); err != nil {
+		return err
+	}
+	if len(config.StopSignal) > 0 {
+		if _, err := signal.ParseSignal(config.StopSignal); err != nil {
+			return err
 		}
-
-		// Validate the healthcheck params of Config
-		if config.Healthcheck != nil {
-			if config.Healthcheck.Interval != 0 && config.Healthcheck.Interval < containertypes.MinimumDuration {
-				return nil, errors.Errorf("Interval in Healthcheck cannot be less than %s", containertypes.MinimumDuration)
-			}
-
-			if config.Healthcheck.Timeout != 0 && config.Healthcheck.Timeout < containertypes.MinimumDuration {
-				return nil, errors.Errorf("Timeout in Healthcheck cannot be less than %s", containertypes.MinimumDuration)
-			}
-
-			if config.Healthcheck.Retries < 0 {
-				return nil, errors.Errorf("Retries in Healthcheck cannot be negative")
-			}
-
-			if config.Healthcheck.StartPeriod != 0 && config.Healthcheck.StartPeriod < containertypes.MinimumDuration {
-				return nil, errors.Errorf("StartPeriod in Healthcheck cannot be less than %s", containertypes.MinimumDuration)
-			}
+	}
+	// Validate if Env contains empty variable or not (e.g., ``, `=foo`)
+	for _, env := range config.Env {
+		if _, err := opts.ValidateEnv(env); err != nil {
+			return err
 		}
 	}
+	return validateHealthCheck(config.Healthcheck)
+}
 
+func validateHostConfig(hostConfig *containertypes.HostConfig, platform string) error {
 	if hostConfig == nil {
-		return nil, nil
+		return nil
 	}
-
 	if hostConfig.AutoRemove && !hostConfig.RestartPolicy.IsNone() {
-		return nil, errors.Errorf("can't create 'AutoRemove' container with restart policy")
+		return errors.Errorf("can't create 'AutoRemove' container with restart policy")
 	}
-
 	// Validate mounts; check if host directories still exist
 	parser := volumemounts.NewParser(platform)
 	for _, cfg := range hostConfig.Mounts {
 		if err := parser.ValidateMountConfig(&cfg); err != nil {
-			return nil, err
+			return err
 		}
 	}
-
 	for _, extraHost := range hostConfig.ExtraHosts {
 		if _, err := opts.ValidateExtraHost(extraHost); err != nil {
-			return nil, err
+			return err
 		}
 	}
+	if err := validatePortBindings(hostConfig.PortBindings); err != nil {
+		return err
+	}
+	if err := validateRestartPolicy(hostConfig.RestartPolicy); err != nil {
+		return err
+	}
+	if !hostConfig.Isolation.IsValid() {
+		return errors.Errorf("invalid isolation '%s' on %s", hostConfig.Isolation, runtime.GOOS)
+	}
+	return nil
+}
+
+// validateHealthCheck validates the healthcheck params of Config
+func validateHealthCheck(healthConfig *containertypes.HealthConfig) error {
+	if healthConfig == nil {
+		return nil
+	}
+	if healthConfig.Interval != 0 && healthConfig.Interval < containertypes.MinimumDuration {
+		return errors.Errorf("Interval in Healthcheck cannot be less than %s", containertypes.MinimumDuration)
+	}
+	if healthConfig.Timeout != 0 && healthConfig.Timeout < containertypes.MinimumDuration {
+		return errors.Errorf("Timeout in Healthcheck cannot be less than %s", containertypes.MinimumDuration)
+	}
+	if healthConfig.Retries < 0 {
+		return errors.Errorf("Retries in Healthcheck cannot be negative")
+	}
+	if healthConfig.StartPeriod != 0 && healthConfig.StartPeriod < containertypes.MinimumDuration {
+		return errors.Errorf("StartPeriod in Healthcheck cannot be less than %s", containertypes.MinimumDuration)
+	}
+	return nil
+}
 
-	for port := range hostConfig.PortBindings {
+func validatePortBindings(ports nat.PortMap) error {
+	for port := range ports {
 		_, portStr := nat.SplitProtoPort(string(port))
 		if _, err := nat.ParsePort(portStr); err != nil {
-			return nil, errors.Errorf("invalid port specification: %q", portStr)
+			return errors.Errorf("invalid port specification: %q", portStr)
 		}
-		for _, pb := range hostConfig.PortBindings[port] {
+		for _, pb := range ports[port] {
 			_, err := nat.NewPort(nat.SplitProtoPort(pb.HostPort))
 			if err != nil {
-				return nil, errors.Errorf("invalid port specification: %q", pb.HostPort)
+				return errors.Errorf("invalid port specification: %q", pb.HostPort)
 			}
 		}
 	}
+	return nil
+}
 
-	p := hostConfig.RestartPolicy
-
-	switch p.Name {
+func validateRestartPolicy(policy containertypes.RestartPolicy) error {
+	switch policy.Name {
 	case "always", "unless-stopped", "no":
-		if p.MaximumRetryCount != 0 {
-			return nil, errors.Errorf("maximum retry count cannot be used with restart policy '%s'", p.Name)
+		if policy.MaximumRetryCount != 0 {
+			return errors.Errorf("maximum retry count cannot be used with restart policy '%s'", policy.Name)
 		}
 	case "on-failure":
-		if p.MaximumRetryCount < 0 {
-			return nil, errors.Errorf("maximum retry count cannot be negative")
+		if policy.MaximumRetryCount < 0 {
+			return errors.Errorf("maximum retry count cannot be negative")
 		}
 	case "":
 		// do nothing
+		return nil
 	default:
-		return nil, errors.Errorf("invalid restart policy '%s'", p.Name)
+		return errors.Errorf("invalid restart policy '%s'", policy.Name)
 	}
+	return nil
+}
 
-	if !hostConfig.Isolation.IsValid() {
-		return nil, errors.Errorf("invalid isolation '%s' on %s", hostConfig.Isolation, runtime.GOOS)
+// translateWorkingDir translates the working-dir for the target platform,
+// and returns an error if the given path is not an absolute path.
+func translateWorkingDir(config *containertypes.Config, platform string) error {
+	if config.WorkingDir == "" {
+		return nil
 	}
-
-	var (
-		err      error
-		warnings []string
-	)
-	// Now do platform-specific verification
-	if warnings, err = verifyPlatformContainerSettings(daemon, hostConfig, config, update); err != nil {
-		return warnings, err
-	}
-	if hostConfig.NetworkMode.IsHost() && len(hostConfig.PortBindings) > 0 {
-		warnings = append(warnings, "Published ports are discarded when using host network mode")
+	wd := config.WorkingDir
+	switch {
+	case runtime.GOOS != platform:
+		// LCOW. Force Unix semantics
+		wd = strings.Replace(wd, string(os.PathSeparator), "/", -1)
+		if !path.IsAbs(wd) {
+			return fmt.Errorf("the working directory '%s' is invalid, it needs to be an absolute path", config.WorkingDir)
+		}
+	default:
+		wd = filepath.FromSlash(wd) // Ensure in platform semantics
+		if !system.IsAbs(wd) {
+			return fmt.Errorf("the working directory '%s' is invalid, it needs to be an absolute path", config.WorkingDir)
+		}
 	}
-	return warnings, err
+	config.WorkingDir = wd
+	return nil
 }

daemon/daemon_unix.go

@@ -354,8 +354,8 @@ func adaptSharedNamespaceContainer(daemon containerGetter, hostConfig *container
 	}
 }
 
-func verifyContainerResources(resources *containertypes.Resources, sysInfo *sysinfo.SysInfo, update bool) ([]string, error) {
-	warnings := []string{}
+// verifyPlatformContainerResources performs platform-specific validation of the container's resource-configuration
+func verifyPlatformContainerResources(resources *containertypes.Resources, sysInfo *sysinfo.SysInfo, update bool) (warnings []string, err error) {
 	fixMemorySwappiness(resources)
 
 	// memory subsystem checks and adjustments
@@ -364,13 +364,11 @@ func verifyContainerResources(resources *containertypes.Resources, sysInfo *sysi
 	}
 	if resources.Memory > 0 && !sysInfo.MemoryLimit {
 		warnings = append(warnings, "Your kernel does not support memory limit capabilities or the cgroup is not mounted. Limitation discarded.")
-		logrus.Warn("Your kernel does not support memory limit capabilities or the cgroup is not mounted. Limitation discarded.")
 		resources.Memory = 0
 		resources.MemorySwap = -1
 	}
 	if resources.Memory > 0 && resources.MemorySwap != -1 && !sysInfo.SwapLimit {
 		warnings = append(warnings, "Your kernel does not support swap limit capabilities or the cgroup is not mounted. Memory limited without swap.")
-		logrus.Warn("Your kernel does not support swap limit capabilities,or the cgroup is not mounted. Memory limited without swap.")
 		resources.MemorySwap = -1
 	}
 	if resources.Memory > 0 && resources.MemorySwap > 0 && resources.MemorySwap < resources.Memory {
@@ -381,7 +379,6 @@ func verifyContainerResources(resources *containertypes.Resources, sysInfo *sysi
 	}
 	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 {
@@ -392,7 +389,6 @@ func verifyContainerResources(resources *containertypes.Resources, sysInfo *sysi
 	}
 	if resources.MemoryReservation > 0 && !sysInfo.MemoryReservation {
 		warnings = append(warnings, "Your kernel does not support memory soft limit capabilities or the cgroup is not mounted. Limitation discarded.")
-		logrus.Warn("Your kernel does not support memory soft limit capabilities or the cgroup is not mounted. Limitation discarded.")
 		resources.MemoryReservation = 0
 	}
 	if resources.MemoryReservation > 0 && resources.MemoryReservation < linuxMinMemory {
@@ -403,7 +399,6 @@ func verifyContainerResources(resources *containertypes.Resources, sysInfo *sysi
 	}
 	if resources.KernelMemory > 0 && !sysInfo.KernelMemory {
 		warnings = append(warnings, "Your kernel does not support kernel memory limit capabilities or the cgroup is not mounted. Limitation discarded.")
-		logrus.Warn("Your kernel does not support kernel memory limit capabilities or the cgroup is not mounted. Limitation discarded.")
 		resources.KernelMemory = 0
 	}
 	if resources.KernelMemory > 0 && resources.KernelMemory < linuxMinMemory {
@@ -411,21 +406,20 @@ func verifyContainerResources(resources *containertypes.Resources, sysInfo *sysi
 	}
 	if resources.KernelMemory > 0 && !kernel.CheckKernelVersion(4, 0, 0) {
 		warnings = append(warnings, "You specified a kernel memory limit on a kernel older than 4.0. Kernel memory limits are experimental on older kernels, it won't work as expected and can cause your system to be unstable.")
-		logrus.Warn("You specified a kernel memory limit on a kernel older than 4.0. Kernel memory limits are experimental on older kernels, it won't work as expected and can cause your system to be unstable.")
 	}
 	if resources.OomKillDisable != nil && !sysInfo.OomKillDisable {
 		// only produce warnings if the setting wasn't to *disable* the OOM Kill; no point
 		// warning the caller if they already wanted the feature to be off
 		if *resources.OomKillDisable {
 			warnings = append(warnings, "Your kernel does not support OomKillDisable. OomKillDisable discarded.")
-			logrus.Warn("Your kernel does not support OomKillDisable. OomKillDisable discarded.")
 		}
 		resources.OomKillDisable = nil
 	}
-
+	if resources.OomKillDisable != nil && *resources.OomKillDisable && resources.Memory == 0 {
+		warnings = append(warnings, "OOM killer is disabled for the container, but no memory limit is set, this can result in the system running out of resources.")
+	}
 	if resources.PidsLimit != 0 && !sysInfo.PidsLimit {
 		warnings = append(warnings, "Your kernel does not support pids limit capabilities or the cgroup is not mounted. PIDs limit discarded.")
-		logrus.Warn("Your kernel does not support pids limit capabilities or the cgroup is not mounted. PIDs limit discarded.")
 		resources.PidsLimit = 0
 	}
 
@@ -452,12 +446,10 @@ func verifyContainerResources(resources *containertypes.Resources, sysInfo *sysi
 
 	if resources.CPUShares > 0 && !sysInfo.CPUShares {
 		warnings = append(warnings, "Your kernel does not support CPU shares or the cgroup is not mounted. Shares discarded.")
-		logrus.Warn("Your kernel does not support CPU shares or the cgroup is not mounted. Shares discarded.")
 		resources.CPUShares = 0
 	}
 	if resources.CPUPeriod > 0 && !sysInfo.CPUCfsPeriod {
 		warnings = append(warnings, "Your kernel does not support CPU cfs period or the cgroup is not mounted. Period discarded.")
-		logrus.Warn("Your kernel does not support CPU cfs period or the cgroup is not mounted. Period discarded.")
 		resources.CPUPeriod = 0
 	}
 	if resources.CPUPeriod != 0 && (resources.CPUPeriod < 1000 || resources.CPUPeriod > 1000000) {
@@ -465,7 +457,6 @@ func verifyContainerResources(resources *containertypes.Resources, sysInfo *sysi
 	}
 	if resources.CPUQuota > 0 && !sysInfo.CPUCfsQuota {
 		warnings = append(warnings, "Your kernel does not support CPU cfs quota or the cgroup is not mounted. Quota discarded.")
-		logrus.Warn("Your kernel does not support CPU cfs quota or the cgroup is not mounted. Quota discarded.")
 		resources.CPUQuota = 0
 	}
 	if resources.CPUQuota > 0 && resources.CPUQuota < 1000 {
@@ -473,14 +464,12 @@ func verifyContainerResources(resources *containertypes.Resources, sysInfo *sysi
 	}
 	if resources.CPUPercent > 0 {
 		warnings = append(warnings, fmt.Sprintf("%s does not support CPU percent. Percent discarded.", runtime.GOOS))
-		logrus.Warnf("%s does not support CPU percent. Percent discarded.", runtime.GOOS)
 		resources.CPUPercent = 0
 	}
 
 	// cpuset subsystem checks and adjustments
 	if (resources.CpusetCpus != "" || resources.CpusetMems != "") && !sysInfo.Cpuset {
 		warnings = append(warnings, "Your kernel does not support cpuset or the cgroup is not mounted. Cpuset discarded.")
-		logrus.Warn("Your kernel does not support cpuset or the cgroup is not mounted. Cpuset discarded.")
 		resources.CpusetCpus = ""
 		resources.CpusetMems = ""
 	}
@@ -502,7 +491,6 @@ func verifyContainerResources(resources *containertypes.Resources, sysInfo *sysi
 	// blkio subsystem checks and adjustments
 	if resources.BlkioWeight > 0 && !sysInfo.BlkioWeight {
 		warnings = append(warnings, "Your kernel does not support Block I/O weight or the cgroup is not mounted. Weight discarded.")
-		logrus.Warn("Your kernel does not support Block I/O weight or the cgroup is not mounted. Weight discarded.")
 		resources.BlkioWeight = 0
 	}
 	if resources.BlkioWeight > 0 && (resources.BlkioWeight < 10 || resources.BlkioWeight > 1000) {
@@ -513,28 +501,23 @@ func verifyContainerResources(resources *containertypes.Resources, sysInfo *sysi
 	}
 	if len(resources.BlkioWeightDevice) > 0 && !sysInfo.BlkioWeightDevice {
 		warnings = append(warnings, "Your kernel does not support Block I/O weight_device or the cgroup is not mounted. Weight-device discarded.")
-		logrus.Warn("Your kernel does not support Block I/O weight_device or the cgroup is not mounted. Weight-device discarded.")
 		resources.BlkioWeightDevice = []*pblkiodev.WeightDevice{}
 	}
 	if len(resources.BlkioDeviceReadBps) > 0 && !sysInfo.BlkioReadBpsDevice {
 		warnings = append(warnings, "Your kernel does not support BPS Block I/O read limit or the cgroup is not mounted. Block I/O BPS read limit discarded.")
-		logrus.Warn("Your kernel does not support BPS Block I/O read limit or the cgroup is not mounted. Block I/O BPS read limit discarded")
 		resources.BlkioDeviceReadBps = []*pblkiodev.ThrottleDevice{}
 	}
 	if len(resources.BlkioDeviceWriteBps) > 0 && !sysInfo.BlkioWriteBpsDevice {
 		warnings = append(warnings, "Your kernel does not support BPS Block I/O write limit or the cgroup is not mounted. Block I/O BPS write limit discarded.")
-		logrus.Warn("Your kernel does not support BPS Block I/O write limit or the cgroup is not mounted. Block I/O BPS write limit discarded.")
 		resources.BlkioDeviceWriteBps = []*pblkiodev.ThrottleDevice{}
 
 	}
 	if len(resources.BlkioDeviceReadIOps) > 0 && !sysInfo.BlkioReadIOpsDevice {
 		warnings = append(warnings, "Your kernel does not support IOPS Block read limit or the cgroup is not mounted. Block I/O IOPS read limit discarded.")
-		logrus.Warn("Your kernel does not support IOPS Block I/O read limit in IO or the cgroup is not mounted. Block I/O IOPS read limit discarded.")
 		resources.BlkioDeviceReadIOps = []*pblkiodev.ThrottleDevice{}
 	}
 	if len(resources.BlkioDeviceWriteIOps) > 0 && !sysInfo.BlkioWriteIOpsDevice {
 		warnings = append(warnings, "Your kernel does not support IOPS Block write limit or the cgroup is not mounted. Block I/O IOPS write limit discarded.")
-		logrus.Warn("Your kernel does not support IOPS Block I/O write limit or the cgroup is not mounted. Block I/O IOPS write limit discarded.")
 		resources.BlkioDeviceWriteIOps = []*pblkiodev.ThrottleDevice{}
 	}
 
@@ -578,11 +561,13 @@ func UsingSystemd(config *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) {
-	var warnings []string
+func verifyPlatformContainerSettings(daemon *Daemon, hostConfig *containertypes.HostConfig, update bool) (warnings []string, err error) {
+	if hostConfig == nil {
+		return nil, nil
+	}
 	sysInfo := sysinfo.New(true)
 
-	w, err := verifyContainerResources(&hostConfig.Resources, sysInfo, update)
+	w, err := verifyPlatformContainerResources(&hostConfig.Resources, sysInfo, update)
 
 	// no matter err is nil or not, w could have data in itself.
 	warnings = append(warnings, w...)
@@ -602,8 +587,11 @@ func verifyPlatformContainerSettings(daemon *Daemon, hostConfig *containertypes.
 	// ip-forwarding does not affect container with '--net=host' (or '--net=none')
 	if sysInfo.IPv4ForwardingDisabled && !(hostConfig.NetworkMode.IsHost() || hostConfig.NetworkMode.IsNone()) {
 		warnings = append(warnings, "IPv4 forwarding is disabled. Networking will not work.")
-		logrus.Warn("IPv4 forwarding is disabled. Networking will not work")
 	}
+	if hostConfig.NetworkMode.IsHost() && len(hostConfig.PortBindings) > 0 {
+		warnings = append(warnings, "Published ports are discarded when using host network mode")
+	}
+
 	// check for various conflicting options with user namespaces
 	if daemon.configStore.RemappedRoot != "" && hostConfig.UsernsMode.IsPrivate() {
 		if hostConfig.Privileged {

daemon/daemon_unix_test.go

@@ -11,6 +11,9 @@ import (
 	containertypes "github.com/docker/docker/api/types/container"
 	"github.com/docker/docker/container"
 	"github.com/docker/docker/daemon/config"
+	"github.com/docker/docker/pkg/sysinfo"
+	"gotest.tools/assert"
+	is "gotest.tools/assert/cmp"
 )
 
 type fakeContainerGetter struct {
@@ -266,3 +269,110 @@ func TestNetworkOptions(t *testing.T) {
 		t.Fatal("Expected networkOptions error, got nil")
 	}
 }
+
+func TestVerifyPlatformContainerResources(t *testing.T) {
+	t.Parallel()
+	var (
+		no  = false
+		yes = true
+	)
+
+	withMemoryLimit := func(si *sysinfo.SysInfo) {
+		si.MemoryLimit = true
+	}
+	withSwapLimit := func(si *sysinfo.SysInfo) {
+		si.SwapLimit = true
+	}
+	withOomKillDisable := func(si *sysinfo.SysInfo) {
+		si.OomKillDisable = true
+	}
+
+	tests := []struct {
+		name             string
+		resources        containertypes.Resources
+		sysInfo          sysinfo.SysInfo
+		update           bool
+		expectedWarnings []string
+	}{
+		{
+			name:             "no-oom-kill-disable",
+			resources:        containertypes.Resources{},
+			sysInfo:          sysInfo(t, withMemoryLimit),
+			expectedWarnings: []string{},
+		},
+		{
+			name: "oom-kill-disable-disabled",
+			resources: containertypes.Resources{
+				OomKillDisable: &no,
+			},
+			sysInfo:          sysInfo(t, withMemoryLimit),
+			expectedWarnings: []string{},
+		},
+		{
+			name: "oom-kill-disable-not-supported",
+			resources: containertypes.Resources{
+				OomKillDisable: &yes,
+			},
+			sysInfo: sysInfo(t, withMemoryLimit),
+			expectedWarnings: []string{
+				"Your kernel does not support OomKillDisable. OomKillDisable discarded.",
+			},
+		},
+		{
+			name: "oom-kill-disable-without-memory-constraints",
+			resources: containertypes.Resources{
+				OomKillDisable: &yes,
+				Memory:         0,
+			},
+			sysInfo: sysInfo(t, withMemoryLimit, withOomKillDisable, withSwapLimit),
+			expectedWarnings: []string{
+				"OOM killer is disabled for the container, but no memory limit is set, this can result in the system running out of resources.",
+			},
+		},
+		{
+			name: "oom-kill-disable-with-memory-constraints-but-no-memory-limit-support",
+			resources: containertypes.Resources{
+				OomKillDisable: &yes,
+				Memory:         linuxMinMemory,
+			},
+			sysInfo: sysInfo(t, withOomKillDisable),
+			expectedWarnings: []string{
+				"Your kernel does not support memory limit capabilities or the cgroup is not mounted. Limitation discarded.",
+				"OOM killer is disabled for the container, but no memory limit is set, this can result in the system running out of resources.",
+			},
+		},
+		{
+			name: "oom-kill-disable-with-memory-constraints",
+			resources: containertypes.Resources{
+				OomKillDisable: &yes,
+				Memory:         linuxMinMemory,
+			},
+			sysInfo:          sysInfo(t, withMemoryLimit, withOomKillDisable, withSwapLimit),
+			expectedWarnings: []string{},
+		},
+	}
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+			warnings, err := verifyPlatformContainerResources(&tc.resources, &tc.sysInfo, tc.update)
+			assert.NilError(t, err)
+			for _, w := range tc.expectedWarnings {
+				assert.Assert(t, is.Contains(warnings, w))
+			}
+		})
+	}
+}
+
+func sysInfo(t *testing.T, opts ...func(*sysinfo.SysInfo)) sysinfo.SysInfo {
+	t.Helper()
+	si := sysinfo.SysInfo{}
+
+	for _, opt := range opts {
+		opt(&si)
+	}
+
+	if si.OomKillDisable {
+		t.Log(t.Name(), "OOM disable supported")
+	}
+	return si
+}

daemon/daemon_windows.go

@@ -75,8 +75,8 @@ func (daemon *Daemon) adaptContainerSettings(hostConfig *containertypes.HostConf
 	return nil
 }
 
-func verifyContainerResources(resources *containertypes.Resources, isHyperv bool) ([]string, error) {
-	warnings := []string{}
+// verifyPlatformContainerResources performs platform-specific validation of the container's resource-configuration
+func verifyPlatformContainerResources(resources *containertypes.Resources, isHyperv bool) (warnings []string, err error) {
 	fixMemorySwappiness(resources)
 	if !isHyperv {
 		// The processor resource controls are mutually exclusive on
@@ -85,18 +85,15 @@ func verifyContainerResources(resources *containertypes.Resources, isHyperv bool
 		if resources.CPUCount > 0 {
 			if resources.CPUShares > 0 {
 				warnings = append(warnings, "Conflicting options: CPU count takes priority over CPU shares on Windows Server Containers. CPU shares discarded")
-				logrus.Warn("Conflicting options: CPU count takes priority over CPU shares on Windows Server Containers. CPU shares discarded")
 				resources.CPUShares = 0
 			}
 			if resources.CPUPercent > 0 {
 				warnings = append(warnings, "Conflicting options: CPU count takes priority over CPU percent on Windows Server Containers. CPU percent discarded")
-				logrus.Warn("Conflicting options: CPU count takes priority over CPU percent on Windows Server Containers. CPU percent discarded")
 				resources.CPUPercent = 0
 			}
 		} else if resources.CPUShares > 0 {
 			if resources.CPUPercent > 0 {
 				warnings = append(warnings, "Conflicting options: CPU shares takes priority over CPU percent on Windows Server Containers. CPU percent discarded")
-				logrus.Warn("Conflicting options: CPU shares takes priority over CPU percent on Windows Server Containers. CPU percent discarded")
 				resources.CPUPercent = 0
 			}
 		}
@@ -131,7 +128,6 @@ func verifyContainerResources(resources *containertypes.Resources, isHyperv bool
 			resources.NanoCPUs = ((resources.NanoCPUs + 1e9/2) / 1e9) * 1e9
 			warningString := fmt.Sprintf("Your current OS version does not support Hyper-V containers with NanoCPUs greater than 1000000000 but not divisible by 1000000000. NanoCPUs rounded to %d", resources.NanoCPUs)
 			warnings = append(warnings, warningString)
-			logrus.Warn(warningString)
 		}
 	}
 
@@ -191,8 +187,10 @@ func verifyContainerResources(resources *containertypes.Resources, isHyperv 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) {
-	warnings := []string{}
+func verifyPlatformContainerSettings(daemon *Daemon, hostConfig *containertypes.HostConfig, update bool) (warnings []string, err error) {
+	if hostConfig == nil {
+		return nil, nil
+	}
 	osv := system.GetOSVersion()
 	hyperv := daemon.runAsHyperVContainer(hostConfig)
 
@@ -204,7 +202,7 @@ func verifyPlatformContainerSettings(daemon *Daemon, hostConfig *containertypes.
 		return warnings, fmt.Errorf("Windows client operating systems earlier than version 1809 can only run Hyper-V containers")
 	}
 
-	w, err := verifyContainerResources(&hostConfig.Resources, hyperv)
+	w, err := verifyPlatformContainerResources(&hostConfig.Resources, hyperv)
 	warnings = append(warnings, w...)
 	return warnings, err
 }