Skip to content

Commit

Permalink
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
define maximumCPUShares
Browse files Browse the repository at this point in the history
singholt committed Jun 12, 2024
1 parent dcfc524 commit 7bfb1fd
Showing 2 changed files with 184 additions and 10 deletions.
29 changes: 22 additions & 7 deletions agent/api/task/task_linux.go
Original file line number Diff line number Diff line change
@@ -43,7 +43,11 @@ import (

const (
// Reference: http://docs.aws.amazon.com/AmazonECS/latest/APIReference/API_ContainerDefinition.html
minimumCPUShare = 2
// These min, max values are defined in the kernel:
// https://github.com/torvalds/linux/blob/0bddd227f3dc55975e2b8dfa7fc6f959b062a2c7/kernel/sched/sched.h#L427-L428
// We need to make sure task and container cgroups have cpu shares within this range.
minimumCPUShares = 2
maximumCPUShares = 262144

minimumCPUPercent = 0
bytesPerMegabyte = 1024 * 1024
@@ -178,13 +182,18 @@ func (task *Task) buildImplicitLinuxCPUSpec() specs.LinuxCPU {
// aggregate container CPU shares when present
var taskCPUShares uint64
for _, container := range task.Containers {
if container.CPU < minimumCPUShare {
taskCPUShares += minimumCPUShare
if container.CPU < minimumCPUShares {
taskCPUShares += minimumCPUShares
} else {
taskCPUShares += uint64(container.CPU)
}
}

// If the task CPU shares exceed the maximumCPUShares, we set it to be the maximum permitted by the kernel.
if taskCPUShares > maximumCPUShares {
taskCPUShares = maximumCPUShares
}

return specs.LinuxCPU{
Shares: &taskCPUShares,
}
@@ -236,12 +245,18 @@ func (task *Task) overrideCgroupParent(hostConfig *dockercontainer.HostConfig) e
// Docker silently converts 0 to 1024 CPU shares, which is probably not what we
// want. Instead, we convert 0 to 2 to be closer to expected behavior. The
// reason for 2 over 1 is that 1 is an invalid value (Linux's choice, not Docker's).
// Similarly, if the container CPU shares is more than the max permitted value (Linux's choice again),
// we set it to the maximum.
func (task *Task) dockerCPUShares(containerCPU uint) int64 {
if containerCPU <= 1 {
if containerCPU < minimumCPUShares {
seelog.Debugf(
"Converting CPU shares to allowed minimum of 2 for task arn: [%s] and cpu shares: %d",
task.Arn, containerCPU)
return 2
"Converting CPU shares to allowed minimum of %d for task arn: [%s] and cpu shares: %d",
minimumCPUShares, task.Arn, containerCPU)
return minimumCPUShares
} else if containerCPU > maximumCPUShares {
seelog.Debugf("Converting CPU shares to allowed maximum of %d for task arn [%s] and cpu shares: %d",
maximumCPUShares, task.Arn, containerCPU)
return maximumCPUShares
}
return int64(containerCPU)
}
165 changes: 162 additions & 3 deletions agent/api/task/task_linux_test.go
Original file line number Diff line number Diff line change
@@ -425,7 +425,7 @@ func TestBuildLinuxResourceSpecWithoutTaskCPULimits(t *testing.T) {
},
},
}
expectedCPUShares := uint64(minimumCPUShare)
expectedCPUShares := uint64(minimumCPUShares)
expectedLinuxResourceSpec := specs.LinuxResources{
CPU: &specs.LinuxCPU{
Shares: &expectedCPUShares,
@@ -449,7 +449,7 @@ func TestBuildLinuxResourceSpecWithoutTaskCPULimits_WithPidLimits(t *testing.T)
},
},
}
expectedCPUShares := uint64(minimumCPUShare)
expectedCPUShares := uint64(minimumCPUShares)
expectedLinuxResourceSpec := specs.LinuxResources{
CPU: &specs.LinuxCPU{
Shares: &expectedCPUShares,
@@ -490,7 +490,7 @@ func TestBuildLinuxResourceSpecWithoutTaskCPUWithContainerCPULimits(t *testing.T
}

// TestBuildLinuxResourceSpecWithoutTaskCPUWithLessThanMinimumContainerCPULimits validates behavior of CPU Shares
// when container CPU share is 1 (less than the current minimumCPUShare which is 2)
// when container CPU share is 1 (less than the current minimumCPUShares which is 2)
func TestBuildLinuxResourceSpecWithoutTaskCPUWithLessThanMinimumContainerCPULimits(t *testing.T) {
task := &Task{
Arn: validTaskArn,
@@ -1578,3 +1578,162 @@ func TestBuildCNIBridgeModeWithServiceConnect(t *testing.T) {
})
}
}

// TestBuildImplicitLinuxCPUSpec tests the task's CPU spec generation when "task.CPU" is not explicitly set.
func TestBuildImplicitLinuxCPUSpec(t *testing.T) {
testCases := []struct {
name string
task *Task
expectedTaskCPUSpec specs.LinuxCPU
}{
{
name: "2 containers: each with CPU within permitted range",
task: &Task{
Arn: validTaskArn,
Containers: []*apicontainer.Container{
{
CPU: uint(10),
},
{
CPU: uint(20),
},
},
},
expectedTaskCPUSpec: specs.LinuxCPU{
Shares: aws.Uint64(30),
},
},
{
name: "2 containers: 1 of them with CPU below min",
task: &Task{
Arn: validTaskArn,
Containers: []*apicontainer.Container{
{
CPU: uint(1),
},
{
CPU: uint(10),
},
},
},
expectedTaskCPUSpec: specs.LinuxCPU{
Shares: aws.Uint64(uint64(12)),
},
},
{
name: "2 containers: 1 of them with CPU above max",
task: &Task{
Arn: validTaskArn,
Containers: []*apicontainer.Container{
{
CPU: uint(1),
},
{
CPU: uint(364544),
},
},
},
expectedTaskCPUSpec: specs.LinuxCPU{
Shares: aws.Uint64(uint64(maximumCPUShares)),
},
},
{
name: "2 containers: each with CPU within permitted range, but the sum of both is beyond the max permitted",
task: &Task{
Arn: validTaskArn,
Containers: []*apicontainer.Container{
{
CPU: uint(200000),
},
{
CPU: uint(200000),
},
},
},
expectedTaskCPUSpec: specs.LinuxCPU{
Shares: aws.Uint64(uint64(maximumCPUShares)),
},
},
{
name: "3 containers: 1 with CPU within permitted range, 1 above max, and 1 below min",
task: &Task{
Arn: validTaskArn,
Containers: []*apicontainer.Container{
{
CPU: uint(5),
},
{
CPU: uint(1),
},
{
CPU: uint(364544),
},
},
},
expectedTaskCPUSpec: specs.LinuxCPU{
Shares: aws.Uint64(uint64(maximumCPUShares)),
},
},
}

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
result := tc.task.buildImplicitLinuxCPUSpec()
assert.Equal(t, tc.expectedTaskCPUSpec, result)
})
}
}

// TestDockerCPUShares tests the dockerCPUShares() conversion method,
// which is used to send the container shares info to Docker
func TestDockerCPUShares(t *testing.T) {
testCases := []struct {
name string
task *Task
expectedContainerShares int64
}{
{
name: "container CPU within permitted range",
task: &Task{
Arn: validTaskArn,
Containers: []*apicontainer.Container{
{
CPU: uint(10),
},
},
},
expectedContainerShares: int64(10),
},
{
name: "container CPU below min",
task: &Task{
Arn: validTaskArn,
Containers: []*apicontainer.Container{
{
CPU: uint(1),
},
},
},
expectedContainerShares: int64(minimumCPUShares),
},
{
name: "container CPU above max",
task: &Task{
Arn: validTaskArn,
Containers: []*apicontainer.Container{
{
CPU: uint(364544),
},
},
},
expectedContainerShares: int64(maximumCPUShares),
},
}

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
result := tc.task.dockerCPUShares(tc.task.Containers[0].CPU)
assert.Equal(t, tc.expectedContainerShares, result)
})
}
}

0 comments on commit 7bfb1fd

Please sign in to comment.