Skip to content

Commit 4dd287e

Browse files
singholtsingholt
committed
define maximumCPUShares
1 parent dcfc524 commit 4dd287e

File tree

2 files changed

+184
-10
lines changed

2 files changed

+184
-10
lines changed

‎agent/api/task/task_linux.go

+22-7
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,11 @@ import (
4343

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

4852
minimumCPUPercent = 0
4953
bytesPerMegabyte = 1024 * 1024
@@ -178,13 +182,18 @@ func (task *Task) buildImplicitLinuxCPUSpec() specs.LinuxCPU {
178182
// aggregate container CPU shares when present
179183
var taskCPUShares uint64
180184
for _, container := range task.Containers {
181-
if container.CPU < minimumCPUShare {
182-
taskCPUShares += minimumCPUShare
185+
if container.CPU < minimumCPUShares {
186+
taskCPUShares += minimumCPUShares
183187
} else {
184188
taskCPUShares += uint64(container.CPU)
185189
}
186190
}
187191

192+
// If the task CPU shares exceed the maximumCPUShares, we set it to be the maximum permitted by the kernel.
193+
if taskCPUShares > maximumCPUShares {
194+
taskCPUShares = maximumCPUShares
195+
}
196+
188197
return specs.LinuxCPU{
189198
Shares: &taskCPUShares,
190199
}
@@ -236,12 +245,18 @@ func (task *Task) overrideCgroupParent(hostConfig *dockercontainer.HostConfig) e
236245
// Docker silently converts 0 to 1024 CPU shares, which is probably not what we
237246
// want. Instead, we convert 0 to 2 to be closer to expected behavior. The
238247
// reason for 2 over 1 is that 1 is an invalid value (Linux's choice, not Docker's).
248+
// Similarly, if the container CPU shares is more than the max permitted value (Linux's choice again),
249+
// we set it to the maximum.
239250
func (task *Task) dockerCPUShares(containerCPU uint) int64 {
240-
if containerCPU <= 1 {
251+
if containerCPU < minimumCPUShares {
241252
seelog.Debugf(
242-
"Converting CPU shares to allowed minimum of 2 for task arn: [%s] and cpu shares: %d",
243-
task.Arn, containerCPU)
244-
return 2
253+
"Converting CPU shares to allowed minimum of %d for task arn: [%s] and cpu shares: %d",
254+
minimumCPUShares, task.Arn, containerCPU)
255+
return minimumCPUShares
256+
} else if containerCPU > maximumCPUShares {
257+
seelog.Debugf("Converting CPU shares to allowed maximum of %d for task arn [%s] and cpu shares: %d",
258+
maximumCPUShares, task.Arn, containerCPU)
259+
return maximumCPUShares
245260
}
246261
return int64(containerCPU)
247262
}

‎agent/api/task/task_linux_test.go

+162-3
Original file line numberDiff line numberDiff line change
@@ -425,7 +425,7 @@ func TestBuildLinuxResourceSpecWithoutTaskCPULimits(t *testing.T) {
425425
},
426426
},
427427
}
428-
expectedCPUShares := uint64(minimumCPUShare)
428+
expectedCPUShares := uint64(minimumCPUShares)
429429
expectedLinuxResourceSpec := specs.LinuxResources{
430430
CPU: &specs.LinuxCPU{
431431
Shares: &expectedCPUShares,
@@ -449,7 +449,7 @@ func TestBuildLinuxResourceSpecWithoutTaskCPULimits_WithPidLimits(t *testing.T)
449449
},
450450
},
451451
}
452-
expectedCPUShares := uint64(minimumCPUShare)
452+
expectedCPUShares := uint64(minimumCPUShares)
453453
expectedLinuxResourceSpec := specs.LinuxResources{
454454
CPU: &specs.LinuxCPU{
455455
Shares: &expectedCPUShares,
@@ -490,7 +490,7 @@ func TestBuildLinuxResourceSpecWithoutTaskCPUWithContainerCPULimits(t *testing.T
490490
}
491491

492492
// TestBuildLinuxResourceSpecWithoutTaskCPUWithLessThanMinimumContainerCPULimits validates behavior of CPU Shares
493-
// when container CPU share is 1 (less than the current minimumCPUShare which is 2)
493+
// when container CPU share is 1 (less than the current minimumCPUShares which is 2)
494494
func TestBuildLinuxResourceSpecWithoutTaskCPUWithLessThanMinimumContainerCPULimits(t *testing.T) {
495495
task := &Task{
496496
Arn: validTaskArn,
@@ -1578,3 +1578,162 @@ func TestBuildCNIBridgeModeWithServiceConnect(t *testing.T) {
15781578
})
15791579
}
15801580
}
1581+
1582+
// TestBuildImplicitLinuxCPUSpec tests the task's CPU spec generation when "task.CPU" is not explicitly set.
1583+
func TestBuildImplicitLinuxCPUSpec(t *testing.T) {
1584+
testCases := []struct {
1585+
name string
1586+
task *Task
1587+
expectedTaskCPUSpec specs.LinuxCPU
1588+
}{
1589+
{
1590+
name: "2 containers: each with CPU within permitted range",
1591+
task: &Task{
1592+
Arn: validTaskArn,
1593+
Containers: []*apicontainer.Container{
1594+
{
1595+
CPU: uint(10),
1596+
},
1597+
{
1598+
CPU: uint(20),
1599+
},
1600+
},
1601+
},
1602+
expectedTaskCPUSpec: specs.LinuxCPU{
1603+
Shares: aws.Uint64(30),
1604+
},
1605+
},
1606+
{
1607+
name: "2 containers: 1 of them with CPU below min",
1608+
task: &Task{
1609+
Arn: validTaskArn,
1610+
Containers: []*apicontainer.Container{
1611+
{
1612+
CPU: uint(1),
1613+
},
1614+
{
1615+
CPU: uint(10),
1616+
},
1617+
},
1618+
},
1619+
expectedTaskCPUSpec: specs.LinuxCPU{
1620+
Shares: aws.Uint64(uint64(12)),
1621+
},
1622+
},
1623+
{
1624+
name: "2 containers: 1 of them with CPU above max",
1625+
task: &Task{
1626+
Arn: validTaskArn,
1627+
Containers: []*apicontainer.Container{
1628+
{
1629+
CPU: uint(1),
1630+
},
1631+
{
1632+
CPU: uint(364544),
1633+
},
1634+
},
1635+
},
1636+
expectedTaskCPUSpec: specs.LinuxCPU{
1637+
Shares: aws.Uint64(uint64(maximumCPUShares)),
1638+
},
1639+
},
1640+
{
1641+
name: "2 containers: each with CPU within permitted range, but the sum of both is beyond the max permitted",
1642+
task: &Task{
1643+
Arn: validTaskArn,
1644+
Containers: []*apicontainer.Container{
1645+
{
1646+
CPU: uint(200000),
1647+
},
1648+
{
1649+
CPU: uint(200000),
1650+
},
1651+
},
1652+
},
1653+
expectedTaskCPUSpec: specs.LinuxCPU{
1654+
Shares: aws.Uint64(uint64(maximumCPUShares)),
1655+
},
1656+
},
1657+
{
1658+
name: "3 containers: 1 with CPU within permitted range, 1 above max, and 1 below min",
1659+
task: &Task{
1660+
Arn: validTaskArn,
1661+
Containers: []*apicontainer.Container{
1662+
{
1663+
CPU: uint(5),
1664+
},
1665+
{
1666+
CPU: uint(1),
1667+
},
1668+
{
1669+
CPU: uint(364544),
1670+
},
1671+
},
1672+
},
1673+
expectedTaskCPUSpec: specs.LinuxCPU{
1674+
Shares: aws.Uint64(uint64(maximumCPUShares)),
1675+
},
1676+
},
1677+
}
1678+
1679+
for _, tc := range testCases {
1680+
t.Run(tc.name, func(t *testing.T) {
1681+
result := tc.task.buildImplicitLinuxCPUSpec()
1682+
assert.Equal(t, tc.expectedTaskCPUSpec, result)
1683+
})
1684+
}
1685+
}
1686+
1687+
// TestDockerCPUShares tests the dockerCPUShares() conversion method,
1688+
// which is used to send the container shares info to Docker
1689+
func TestDockerCPUShares(t *testing.T) {
1690+
testCases := []struct {
1691+
name string
1692+
task *Task
1693+
expectedContainerShares int64
1694+
}{
1695+
{
1696+
name: "container CPU within permitted range",
1697+
task: &Task{
1698+
Arn: validTaskArn,
1699+
Containers: []*apicontainer.Container{
1700+
{
1701+
CPU: uint(10),
1702+
},
1703+
},
1704+
},
1705+
expectedContainerShares: int64(10),
1706+
},
1707+
{
1708+
name: "container CPU below min",
1709+
task: &Task{
1710+
Arn: validTaskArn,
1711+
Containers: []*apicontainer.Container{
1712+
{
1713+
CPU: uint(1),
1714+
},
1715+
},
1716+
},
1717+
expectedContainerShares: int64(minimumCPUShares),
1718+
},
1719+
{
1720+
name: "container CPU above max",
1721+
task: &Task{
1722+
Arn: validTaskArn,
1723+
Containers: []*apicontainer.Container{
1724+
{
1725+
CPU: uint(364544),
1726+
},
1727+
},
1728+
},
1729+
expectedContainerShares: int64(maximumCPUShares),
1730+
},
1731+
}
1732+
1733+
for _, tc := range testCases {
1734+
t.Run(tc.name, func(t *testing.T) {
1735+
result := tc.task.dockerCPUShares(tc.task.Containers[0].CPU)
1736+
assert.Equal(t, tc.expectedContainerShares, result)
1737+
})
1738+
}
1739+
}

0 commit comments

Comments
 (0)