beenull/moby
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity

api: remove code for adjusting CPU shares (api < v1.19)

Browse source 
API versions before 1.19 allowed CpuShares that were greater than the maximum
or less than the minimum supported by the kernel, and relied on the kernel to
do the right thing.

Commit ed39fbeb2a introduced code to adjust the
CPU shares to be within the accepted range when using API version 1.18 or
lower.

API v1.23 and older are deprecated, so we can remove support for this
functionality.

Currently, there's no validation for CPU shares to be within an acceptable
range; a TODO was added to add validation for this option, and to use the
`linuxMinCPUShares` and `linuxMaxCPUShares` consts for this.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This commit is contained in:
Sebastiaan van Stijn 2024-01-21 16:54:06 +01:00
parent ef25f0aa52
commit 2970b320aa
No known key found for this signature in database
GPG key ID: 76698F39D527CE8C
8 changed files with 13 additions and 114 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
api/server/router/container/container_routes.go
View file

@ -505,7 +505,6 @@ func (s *containerRouter) postContainersCreate(ctx context.Context, w http.Respo
}
version := httputils.VersionFromContext(ctx)
adjustCPUShares := versions.LessThan(version, "1.19")
// When using API 1.24 and under, the client is responsible for removing the container
if versions.LessThan(version, "1.25") {
@ -649,7 +648,6 @@ func (s *containerRouter) postContainersCreate(ctx context.Context, w http.Respo
Config: config,
HostConfig: hostConfig,
NetworkingConfig: networkingConfig,
AdjustCPUShares: adjustCPUShares,
Platform: platform,
})
if err != nil {

1
api/types/backend/backend.go
View file

@ -18,7 +18,6 @@ type ContainerCreateConfig struct {
HostConfig *container.HostConfig
NetworkingConfig *network.NetworkingConfig
Platform *ocispec.Platform
AdjustCPUShares bool
}
// ContainerRmConfig holds arguments for the container remove

2
daemon/create.go
View file

@ -108,7 +108,7 @@ func (daemon *Daemon) containerCreate(ctx context.Context, daemonCfg *configStor
if opts.params.HostConfig == nil {
opts.params.HostConfig = &containertypes.HostConfig{}
}
err = daemon.adaptContainerSettings(&daemonCfg.Config, opts.params.HostConfig, opts.params.AdjustCPUShares)
err = daemon.adaptContainerSettings(&daemonCfg.Config, opts.params.HostConfig)
if err != nil {
return containertypes.CreateResponse{Warnings: warnings}, errdefs.InvalidParameter(err)
}

23
daemon/daemon_unix.go
View file

@ -54,9 +54,16 @@ import (
const (
isWindows = false
// These values were used to adjust the CPU-shares for older API versions,
// but were not used for validation.
//
// TODO(thaJeztah): validate min/max values for CPU-shares, similar to Windows: https://github.com/moby/moby/issues/47340
// https://github.com/moby/moby/blob/27e85c7b6885c2d21ae90791136d9aba78b83d01/daemon/daemon_windows.go#L97-L99
//
// See https://git.kernel.org/cgit/linux/kernel/git/tip/tip.git/tree/kernel/sched/sched.h?id=8cd9234c64c584432f6992fe944ca9e46ca8ea76#n269
linuxMinCPUShares = 2
linuxMaxCPUShares = 262144
// linuxMinCPUShares = 2
// linuxMaxCPUShares = 262144
// It's not kernel limit, we want this 6M limit to account for overhead during startup, and to supply a reasonable functional container
linuxMinMemory = 6291456
// constants for remapped root settings
@ -306,17 +313,7 @@ func adjustParallelLimit(n int, limit int) int {
// adaptContainerSettings is called during container creation to modify any
// settings necessary in the HostConfig structure.
func (daemon *Daemon) adaptContainerSettings(daemonCfg *config.Config, hostConfig *containertypes.HostConfig, adjustCPUShares bool) error {
if adjustCPUShares && hostConfig.CPUShares > 0 {
// Handle unsupported CPUShares
if hostConfig.CPUShares < linuxMinCPUShares {
log.G(context.TODO()).Warnf("Changing requested CPUShares of %d to minimum allowed of %d", hostConfig.CPUShares, linuxMinCPUShares)
hostConfig.CPUShares = linuxMinCPUShares
} else if hostConfig.CPUShares > linuxMaxCPUShares {
log.G(context.TODO()).Warnf("Changing requested CPUShares of %d to maximum allowed of %d", hostConfig.CPUShares, linuxMaxCPUShares)
hostConfig.CPUShares = linuxMaxCPUShares
}
}
func (daemon *Daemon) adaptContainerSettings(daemonCfg *config.Config, hostConfig *containertypes.HostConfig) error {
if hostConfig.Memory > 0 && hostConfig.MemorySwap == 0 {
// By default, MemorySwap is set to twice the size of Memory.
hostConfig.MemorySwap = hostConfig.Memory * 2

81
daemon/daemon_unix_test.go
View file

@ -57,87 +57,6 @@ func TestAdjustSharedNamespaceContainerName(t *testing.T) {
}
}
// Unix test as uses settings which are not available on Windows
func TestAdjustCPUShares(t *testing.T) {
tmp, err := os.MkdirTemp("", "docker-daemon-unix-test-")
if err != nil {
t.Fatal(err)
}
defer os.RemoveAll(tmp)
daemon := &Daemon{
repository: tmp,
root: tmp,
}
cfg := &config.Config{}
muteLogs(t)
hostConfig := &containertypes.HostConfig{
Resources: containertypes.Resources{CPUShares: linuxMinCPUShares - 1},
}
daemon.adaptContainerSettings(cfg, hostConfig, true)
if hostConfig.CPUShares != linuxMinCPUShares {
t.Errorf("Expected CPUShares to be %d", linuxMinCPUShares)
}
hostConfig.CPUShares = linuxMaxCPUShares + 1
daemon.adaptContainerSettings(cfg, hostConfig, true)
if hostConfig.CPUShares != linuxMaxCPUShares {
t.Errorf("Expected CPUShares to be %d", linuxMaxCPUShares)
}
hostConfig.CPUShares = 0
daemon.adaptContainerSettings(cfg, hostConfig, true)
if hostConfig.CPUShares != 0 {
t.Error("Expected CPUShares to be unchanged")
}
hostConfig.CPUShares = 1024
daemon.adaptContainerSettings(cfg, hostConfig, true)
if hostConfig.CPUShares != 1024 {
t.Error("Expected CPUShares to be unchanged")
}
}
// Unix test as uses settings which are not available on Windows
func TestAdjustCPUSharesNoAdjustment(t *testing.T) {
tmp, err := os.MkdirTemp("", "docker-daemon-unix-test-")
if err != nil {
t.Fatal(err)
}
defer os.RemoveAll(tmp)
daemon := &Daemon{
repository: tmp,
root: tmp,
}
cfg := &config.Config{}
hostConfig := &containertypes.HostConfig{
Resources: containertypes.Resources{CPUShares: linuxMinCPUShares - 1},
}
daemon.adaptContainerSettings(cfg, hostConfig, false)
if hostConfig.CPUShares != linuxMinCPUShares-1 {
t.Errorf("Expected CPUShares to be %d", linuxMinCPUShares-1)
}
hostConfig.CPUShares = linuxMaxCPUShares + 1
daemon.adaptContainerSettings(cfg, hostConfig, false)
if hostConfig.CPUShares != linuxMaxCPUShares+1 {
t.Errorf("Expected CPUShares to be %d", linuxMaxCPUShares+1)
}
hostConfig.CPUShares = 0
daemon.adaptContainerSettings(cfg, hostConfig, false)
if hostConfig.CPUShares != 0 {
t.Error("Expected CPUShares to be unchanged")
}
hostConfig.CPUShares = 1024
daemon.adaptContainerSettings(cfg, hostConfig, false)
if hostConfig.CPUShares != 1024 {
t.Error("Expected CPUShares to be unchanged")
}
}
// Unix test as uses settings which are not available on Windows
func TestParseSecurityOptWithDeprecatedColon(t *testing.T) {
opts := &container.SecurityOptions{}

2
daemon/daemon_windows.go
View file

@ -66,7 +66,7 @@ func setupInitLayer(idMapping idtools.IdentityMapping) func(string) error {
// adaptContainerSettings is called during container creation to modify any
// settings necessary in the HostConfig structure.
func (daemon *Daemon) adaptContainerSettings(daemonCfg *config.Config, hostConfig *containertypes.HostConfig, adjustCPUShares bool) error {
func (daemon *Daemon) adaptContainerSettings(daemonCfg *config.Config, hostConfig *containertypes.HostConfig) error {
return nil
}

2
daemon/start.go
View file

@ -96,7 +96,7 @@ func (daemon *Daemon) ContainerStart(ctx context.Context, name string, hostConfi
// Adapt for old containers in case we have updates in this function and
// old containers never have chance to call the new function in create stage.
if hostConfig != nil {
if err := daemon.adaptContainerSettings(&daemonCfg.Config, ctr.HostConfig, false); err != nil {
if err := daemon.adaptContainerSettings(&daemonCfg.Config, ctr.HostConfig); err != nil {
return errdefs.InvalidParameter(err)
}
}

14
integration-cli/docker_api_containers_test.go
View file

@ -1251,20 +1251,6 @@ func (s *DockerAPISuite) TestPostContainersCreateWithStringOrSliceCapAddDrop(c *
assert.NilError(c, err)
}
// #14915
func (s *DockerAPISuite) TestContainerAPICreateNoHostConfig118(c *testing.T) {
testRequires(c, DaemonIsLinux) // Windows only support 1.25 or later
config := container.Config{
Image: "busybox",
}
apiClient, err := client.NewClientWithOpts(client.FromEnv, client.WithVersion("v1.18"))
assert.NilError(c, err)
_, err = apiClient.ContainerCreate(testutil.GetContext(c), &config, &container.HostConfig{}, &network.NetworkingConfig{}, nil, "")
assert.NilError(c, err)
}
// Ensure an error occurs when you have a container read-only rootfs but you
// extract an archive to a symlink in a writable volume which points to a
// directory outside of the volume.