From 0ed00b36ff7d8651e4d11a41507f81441c081388 Mon Sep 17 00:00:00 2001 From: Darren Stahl Date: Wed, 9 Nov 2016 15:18:54 -0800 Subject: [PATCH] Adding more strict resource checks on Windows Signed-off-by: Darren Stahl --- daemon/daemon_windows.go | 114 ++++++++++++++++---------------- libcontainerd/client_windows.go | 12 +++- 2 files changed, 69 insertions(+), 57 deletions(-) diff --git a/daemon/daemon_windows.go b/daemon/daemon_windows.go index e283d76f5e..9bdf3f1c58 100644 --- a/daemon/daemon_windows.go +++ b/daemon/daemon_windows.go @@ -8,7 +8,6 @@ import ( "github.com/Microsoft/hcsshim" "github.com/Sirupsen/logrus" "github.com/docker/docker/api/types" - pblkiodev "github.com/docker/docker/api/types/blkiodev" containertypes "github.com/docker/docker/api/types/container" "github.com/docker/docker/container" "github.com/docker/docker/image" @@ -83,31 +82,6 @@ func (daemon *Daemon) adaptContainerSettings(hostConfig *containertypes.HostConf return nil } - numCPU := int64(sysinfo.NumCPU()) - if hostConfig.CPUCount < 0 { - logrus.Warnf("Changing requested CPUCount of %d to minimum allowed of %d", hostConfig.CPUCount, windowsMinCPUCount) - hostConfig.CPUCount = windowsMinCPUCount - } else if hostConfig.CPUCount > numCPU { - logrus.Warnf("Changing requested CPUCount of %d to current number of processors, %d", hostConfig.CPUCount, numCPU) - hostConfig.CPUCount = numCPU - } - - if hostConfig.CPUShares < 0 { - logrus.Warnf("Changing requested CPUShares of %d to minimum allowed of %d", hostConfig.CPUShares, windowsMinCPUShares) - hostConfig.CPUShares = windowsMinCPUShares - } else if hostConfig.CPUShares > windowsMaxCPUShares { - logrus.Warnf("Changing requested CPUShares of %d to maximum allowed of %d", hostConfig.CPUShares, windowsMaxCPUShares) - hostConfig.CPUShares = windowsMaxCPUShares - } - - if hostConfig.CPUPercent < 0 { - logrus.Warnf("Changing requested CPUPercent of %d to minimum allowed of %d", hostConfig.CPUPercent, windowsMinCPUPercent) - hostConfig.CPUPercent = windowsMinCPUPercent - } else if hostConfig.CPUPercent > windowsMaxCPUPercent { - logrus.Warnf("Changing requested CPUPercent of %d to maximum allowed of %d", hostConfig.CPUPercent, windowsMaxCPUPercent) - hostConfig.CPUPercent = windowsMaxCPUPercent - } - return nil } @@ -138,48 +112,76 @@ func verifyContainerResources(resources *containertypes.Resources, isHyperv bool } } - if resources.NanoCPUs > 0 && resources.CPUPercent > 0 { - return warnings, fmt.Errorf("Conflicting options: Nano CPUs and CPU Percent cannot both be set") + if resources.CPUShares < 0 || resources.CPUShares > windowsMaxCPUShares { + return warnings, fmt.Errorf("range of CPUShares is from %d to %d", windowsMinCPUShares, windowsMaxCPUShares) + } + if resources.CPUPercent < 0 || resources.CPUPercent > windowsMaxCPUPercent { + return warnings, fmt.Errorf("range of CPUPercent is from %d to %d", windowsMinCPUPercent, windowsMaxCPUPercent) + } + if resources.CPUCount < 0 { + return warnings, fmt.Errorf("invalid CPUCount: CPUCount cannot be negative") } + if resources.NanoCPUs > 0 && resources.CPUPercent > 0 { + return warnings, fmt.Errorf("conflicting options: Nano CPUs and CPU Percent cannot both be set") + } if resources.NanoCPUs > 0 && resources.CPUShares > 0 { - return warnings, fmt.Errorf("Conflicting options: Nano CPUs and CPU Shares cannot both be set") + return warnings, fmt.Errorf("conflicting options: Nano CPUs and CPU Shares cannot both be set") } if resources.NanoCPUs < 0 || resources.NanoCPUs > int64(sysinfo.NumCPU())*1e9 { - return warnings, fmt.Errorf("Range of Nano CPUs is from 1 to %d", int64(sysinfo.NumCPU())*1e9) + return warnings, fmt.Errorf("range of Nano CPUs is from 1 to %d", int64(sysinfo.NumCPU())*1e9) } - // TODO Windows: Add more validation of resource settings not supported on Windows - - if resources.BlkioWeight > 0 { - warnings = append(warnings, "Windows does not support Block I/O weight. Block I/O weight discarded.") - logrus.Warn("Windows does not support Block I/O weight. Block I/O weight discarded.") - resources.BlkioWeight = 0 - } - if len(resources.BlkioWeightDevice) > 0 { - warnings = append(warnings, "Windows does not support Block I/O weight-device. Weight-device discarded.") - logrus.Warn("Windows does not support Block I/O weight-device. Weight-device discarded.") - resources.BlkioWeightDevice = []*pblkiodev.WeightDevice{} - } if len(resources.BlkioDeviceReadBps) > 0 { - warnings = append(warnings, "Windows does not support Block read limit in bytes per second. Device read bps discarded.") - logrus.Warn("Windows does not support Block I/O read limit in bytes per second. Device read bps discarded.") - resources.BlkioDeviceReadBps = []*pblkiodev.ThrottleDevice{} - } - if len(resources.BlkioDeviceWriteBps) > 0 { - warnings = append(warnings, "Windows does not support Block write limit in bytes per second. Device write bps discarded.") - logrus.Warn("Windows does not support Block I/O write limit in bytes per second. Device write bps discarded.") - resources.BlkioDeviceWriteBps = []*pblkiodev.ThrottleDevice{} + return warnings, fmt.Errorf("invalid option: Windows does not support BlkioDeviceReadBps") } if len(resources.BlkioDeviceReadIOps) > 0 { - warnings = append(warnings, "Windows does not support Block read limit in IO per second. Device read iops discarded.") - logrus.Warn("Windows does not support Block I/O read limit in IO per second. Device read iops discarded.") - resources.BlkioDeviceReadIOps = []*pblkiodev.ThrottleDevice{} + return warnings, fmt.Errorf("invalid option: Windows does not support BlkioDeviceReadIOps") + } + if len(resources.BlkioDeviceWriteBps) > 0 { + return warnings, fmt.Errorf("invalid option: Windows does not support BlkioDeviceWriteBps") } if len(resources.BlkioDeviceWriteIOps) > 0 { - warnings = append(warnings, "Windows does not support Block write limit in IO per second. Device write iops discarded.") - logrus.Warn("Windows does not support Block I/O write limit in IO per second. Device write iops discarded.") - resources.BlkioDeviceWriteIOps = []*pblkiodev.ThrottleDevice{} + return warnings, fmt.Errorf("invalid option: Windows does not support BlkioDeviceWriteIOps") + } + if resources.BlkioWeight > 0 { + return warnings, fmt.Errorf("invalid option: Windows does not support BlkioWeight") + } + if len(resources.BlkioWeightDevice) > 0 { + return warnings, fmt.Errorf("invalid option: Windows does not support BlkioWeightDevice") + } + if resources.CgroupParent != "" { + return warnings, fmt.Errorf("invalid option: Windows does not support CgroupParent") + } + if resources.CPUPeriod != 0 { + return warnings, fmt.Errorf("invalid option: Windows does not support CPUPeriod") + } + if resources.CpusetCpus != "" { + return warnings, fmt.Errorf("invalid option: Windows does not support CpusetCpus") + } + if resources.CpusetMems != "" { + return warnings, fmt.Errorf("invalid option: Windows does not support CpusetMems") + } + if resources.KernelMemory != 0 { + return warnings, fmt.Errorf("invalid option: Windows does not support KernelMemory") + } + if resources.MemoryReservation != 0 { + return warnings, fmt.Errorf("invalid option: Windows does not support MemoryReservation") + } + if resources.MemorySwap != 0 { + return warnings, fmt.Errorf("invalid option: Windows does not support MemorySwap") + } + if resources.MemorySwappiness != nil && *resources.MemorySwappiness != -1 { + return warnings, fmt.Errorf("invalid option: Windows does not support MemorySwappiness") + } + if resources.OomKillDisable != nil && *resources.OomKillDisable { + return warnings, fmt.Errorf("invalid option: Windows does not support OomKillDisable") + } + if resources.PidsLimit != 0 { + return warnings, fmt.Errorf("invalid option: Windows does not support PidsLimit") + } + if len(resources.Ulimits) != 0 { + return warnings, fmt.Errorf("invalid option: Windows does not support Ulimits") } return warnings, nil } diff --git a/libcontainerd/client_windows.go b/libcontainerd/client_windows.go index 23e5b0dcbe..61c50e4c37 100644 --- a/libcontainerd/client_windows.go +++ b/libcontainerd/client_windows.go @@ -14,6 +14,7 @@ import ( "github.com/Microsoft/hcsshim" "github.com/Sirupsen/logrus" + "github.com/docker/docker/pkg/sysinfo" "github.com/opencontainers/runtime-spec/specs-go" ) @@ -111,7 +112,16 @@ func (clnt *client) Create(containerID string, checkpoint string, checkpointDir if spec.Windows.Resources != nil { if spec.Windows.Resources.CPU != nil { if spec.Windows.Resources.CPU.Count != nil { - configuration.ProcessorCount = uint32(*spec.Windows.Resources.CPU.Count) + // This check is being done here rather than in adaptContainerSettings + // because we don't want to update the HostConfig in case this container + // is moved to a host with more CPUs than this one. + cpuCount := *spec.Windows.Resources.CPU.Count + hostCPUCount := uint64(sysinfo.NumCPU()) + if cpuCount > hostCPUCount { + logrus.Warnf("Changing requested CPUCount of %d to current number of processors, %d", cpuCount, hostCPUCount) + cpuCount = hostCPUCount + } + configuration.ProcessorCount = uint32(cpuCount) } if spec.Windows.Resources.CPU.Shares != nil { configuration.ProcessorWeight = uint64(*spec.Windows.Resources.CPU.Shares)