diff --git a/api/server/router/container/container_routes.go b/api/server/router/container/container_routes.go index 8b400e61cb..69dfbf9076 100644 --- a/api/server/router/container/container_routes.go +++ b/api/server/router/container/container_routes.go @@ -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 { diff --git a/api/types/backend/backend.go b/api/types/backend/backend.go index a4fb9d638a..c9fc978884 100644 --- a/api/types/backend/backend.go +++ b/api/types/backend/backend.go @@ -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 diff --git a/daemon/create.go b/daemon/create.go index aa063b60aa..4f8da469b7 100644 --- a/daemon/create.go +++ b/daemon/create.go @@ -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) } diff --git a/daemon/daemon_unix.go b/daemon/daemon_unix.go index 1143dda063..120f514ef7 100644 --- a/daemon/daemon_unix.go +++ b/daemon/daemon_unix.go @@ -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 diff --git a/daemon/daemon_unix_test.go b/daemon/daemon_unix_test.go index 37dab2dd9b..e681eac892 100644 --- a/daemon/daemon_unix_test.go +++ b/daemon/daemon_unix_test.go @@ -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{} diff --git a/daemon/daemon_windows.go b/daemon/daemon_windows.go index 1b03e4f5a7..54bc6d4941 100644 --- a/daemon/daemon_windows.go +++ b/daemon/daemon_windows.go @@ -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 } diff --git a/daemon/start.go b/daemon/start.go index 516c069958..d315ccff56 100644 --- a/daemon/start.go +++ b/daemon/start.go @@ -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) } } diff --git a/integration-cli/docker_api_containers_test.go b/integration-cli/docker_api_containers_test.go index c3839aad66..fdab67e0fc 100644 --- a/integration-cli/docker_api_containers_test.go +++ b/integration-cli/docker_api_containers_test.go @@ -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.