Skip to content

Commit

Permalink
[process][ecs fargate] Fix CPU limit used (#8253)
Browse files Browse the repository at this point in the history
* [process][ecs fargate] fix cpu limit calculation
  • Loading branch information
celenechang authored May 27, 2021
1 parent 65b4218 commit f4a3809
Show file tree
Hide file tree
Showing 4 changed files with 18 additions and 13 deletions.
3 changes: 2 additions & 1 deletion pkg/process/checks/container_common.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@ func calculateCtrPct(cur, prev, sys2, sys1 uint64, numCPU int, before time.Time)
}

// If we have system usage values then we need to calculate against those.
// XXX: Right now this only applies to ECS collection
// XXX: Right now this only applies to ECS collection. Note that the inclusion of CPUs is
// necessary because the value gets normalized against the CPU limit, which also accounts for CPUs.
if sys1 >= 0 && sys2 > 0 && sys2 != sys1 {
cpuDelta := float32(cur - prev)
sysDelta := float32(sys2 - sys1)
Expand Down
13 changes: 3 additions & 10 deletions pkg/util/ecs/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -131,9 +131,9 @@ func convertMetaV2Container(c v2.Container, taskLimits map[string]float64) (*con
}
}

if l, found := c.Limits[cpuKey]; found && l > 0 {
container.Limits.CPULimit = formatContainerCPULimit(float64(l))
} else if l, found := taskLimits[cpuKey]; found && l > 0 {
// The task limit is required in Fargate (so this should always exist). Use this as the basis for the CPU limit
// because the container limits configured are treated as CPU shares, rather than a fixed limit to adhere to.
if l, found := taskLimits[cpuKey]; found && l > 0 {
container.Limits.CPULimit = formatTaskCPULimit(l)
} else {
container.Limits.CPULimit = 100
Expand All @@ -148,15 +148,8 @@ func convertMetaV2Container(c v2.Container, taskLimits map[string]float64) (*con
return container, dateError
}

func formatContainerCPULimit(val float64) float64 {
// The ECS API exposes the container CPU limit in CPU units
// Value is reported in Hz
return val / 1024 * 100
}

func formatTaskCPULimit(val float64) float64 {
// The ECS API exposes the task CPU limit with the format: 0.25, 0.5, 1, 2, 4
// Value is reported in Hz
return val * 100
}

Expand Down
4 changes: 2 additions & 2 deletions pkg/util/ecs/common_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ func TestConvertMetaV2Container(t *testing.T) {
StartedAt: 1517518511,
Type: "ECS",
AddressList: []containers.NetworkAddress{},
Limits: metrics.ContainerLimits{CPULimit: 50, MemLimit: 1024000000},
Limits: metrics.ContainerLimits{CPULimit: 100, MemLimit: 1024000000},
},
},
{
Expand Down Expand Up @@ -158,7 +158,7 @@ func TestConvertMetaV2Container(t *testing.T) {
Name: "ecs-nginx-5-nginx-curl-ccccb9f49db0dfe0d901",
Type: "ECS",
AddressList: []containers.NetworkAddress{},
Limits: metrics.ContainerLimits{CPULimit: 50, MemLimit: 1024000000},
Limits: metrics.ContainerLimits{CPULimit: 100, MemLimit: 1024000000},
},
},
}
Expand Down
11 changes: 11 additions & 0 deletions releasenotes/notes/fix-ecs-fargate-cpu-limit-93bd4656ee607c08.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
# Each section from every release note are combined when the
# CHANGELOG.rst is rendered. So the text needs to be worded so that
# it does not depend on any information only available in another
# section. This may mean repeating some details, but each section
# must be readable independently of the other.
#
# Each section note must be formatted as reStructuredText.
---
fixes:
- |
Fix CPU limit used for Live Containers page in ECS Fargate environments.

0 comments on commit f4a3809

Please sign in to comment.